| Message ID | 20190305133248.4828-11-ard.biesheuvel@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | StandaloneMmPkg, ArmPkg: cleanups and improvements | expand |
Hi In original SMM infrastructure, there are lots of interaction that SMM has to know the DXE status. In StandaloneMm, I don't expect we have many interaction. Is there any specific example? I am totally OK to add those. And I just want to know more usage. Reviewed-by: Jiewen.yao@intel.com Thank you Yao Jiewen > -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > Sent: Tuesday, March 5, 2019 5:33 AM > To: edk2-devel@lists.01.org > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Achin Gupta > <achin.gupta@arm.com>; Supreeth Venkatesh > <supreeth.venkatesh@arm.com>; Yao, Jiewen <jiewen.yao@intel.com>; > Leif Lindholm <leif.lindholm@linaro.org>; Jagadeesh Ujja > <jagadeesh.ujja@arm.com> > Subject: [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected > PI events into MM context > > PI defines a few architected events that have significance in the MM > context as well as in the non-secure DXE context. So register notify > handlers for these events, and relay them into the standalone MM world. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf | 5 +++ > ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c | 47 > +++++++++++++++++++- > 2 files changed, 50 insertions(+), 2 deletions(-) > > diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf > index 88beafa39c05..8bf269270f9d 100644 > --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf > +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf > @@ -48,6 +48,11 @@ [LibraryClasses] > [Protocols] > gEfiMmCommunicationProtocolGuid ## PRODUCES > > +[Guids] > + gEfiEndOfDxeEventGroupGuid > + gEfiEventExitBootServicesGuid > + gEfiEventReadyToBootGuid > + > [Pcd.common] > gArmTokenSpaceGuid.PcdMmBufferBase > gArmTokenSpaceGuid.PcdMmBufferSize > diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c > index feb9fa9f4ead..3203cf801a19 100644 > --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c > +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c > @@ -265,6 +265,43 @@ GetMmCompatibility () > return Status; > } > > +STATIC EFI_GUID* CONST mGuidedEventGuid[] = { > + &gEfiEndOfDxeEventGroupGuid, > + &gEfiEventExitBootServicesGuid, > + &gEfiEventReadyToBootGuid, > +}; > + > +STATIC EFI_EVENT mGuidedEvent[ARRAY_SIZE (mGuidedEventGuid)]; > + > +/** > + Event notification that is fired when GUIDed Event Group is signaled. > + > + @param Event The Event that is being processed, > not used. > + @param Context Event Context, not used. > + > +**/ > +STATIC > +VOID > +EFIAPI > +MmGuidedEventNotify ( > + IN EFI_EVENT Event, > + IN VOID *Context > + ) > +{ > + EFI_MM_COMMUNICATE_HEADER Header; > + UINTN Size; > + > + // > + // Use Guid to initialize EFI_SMM_COMMUNICATE_HEADER structure > + // > + CopyGuid (&Header.HeaderGuid, Context); > + Header.MessageLength = 1; > + Header.Data[0] = 0; > + > + Size = sizeof (Header); > + MmCommunicationCommunicate (&mMmCommunication, &Header, > &Size); > +} > + > /** > The Entry Point for MM Communication > > @@ -287,6 +324,7 @@ MmCommunicationInitialize ( > ) > { > EFI_STATUS Status; > + UINTN Index; > > // Check if we can make the MM call > Status = GetMmCompatibility (); > @@ -351,8 +389,13 @@ MmCommunicationInitialize ( > NULL, > &mSetVirtualAddressMapEvent > ); > - if (Status == EFI_SUCCESS) { > - return Status; > + ASSERT_EFI_ERROR (Status); > + > + for (Index = 0; Index < ARRAY_SIZE (mGuidedEventGuid); Index++) { > + Status = gBS->CreateEventEx (EVT_NOTIFY_SIGNAL, TPL_CALLBACK, > + MmGuidedEventNotify, mGuidedEventGuid[Index], > + mGuidedEventGuid[Index], > &mGuidedEvent[Index]); > + ASSERT_EFI_ERROR (Status); > } > > gBS->UninstallProtocolInterface ( > -- > 2.20.1 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Tue, 5 Mar 2019 at 16:56, Yao, Jiewen <jiewen.yao@intel.com> wrote: > > Hi > In original SMM infrastructure, there are lots of interaction that SMM has to know the DXE status. > > In StandaloneMm, I don't expect we have many interaction. Is there any specific example? > > I am totally OK to add those. And I just want to know more usage. > > Reviewed-by: Jiewen.yao@intel.com > Jiewen, Thanks for the review. It is not 100% clear at the moment, but since existing third party software designed to run in MM context may make assumptions about security of the platform (e.g., before vs after end of dxe) based on these events, we should at least signal the common ones added in this patch. > > -----Original Message----- > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > > Sent: Tuesday, March 5, 2019 5:33 AM > > To: edk2-devel@lists.01.org > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Achin Gupta > > <achin.gupta@arm.com>; Supreeth Venkatesh > > <supreeth.venkatesh@arm.com>; Yao, Jiewen <jiewen.yao@intel.com>; > > Leif Lindholm <leif.lindholm@linaro.org>; Jagadeesh Ujja > > <jagadeesh.ujja@arm.com> > > Subject: [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected > > PI events into MM context > > > > PI defines a few architected events that have significance in the MM > > context as well as in the non-secure DXE context. So register notify > > handlers for these events, and relay them into the standalone MM world. > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > --- > > ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf | 5 +++ > > ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c | 47 > > +++++++++++++++++++- > > 2 files changed, 50 insertions(+), 2 deletions(-) > > > > diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf > > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf > > index 88beafa39c05..8bf269270f9d 100644 > > --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf > > +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf > > @@ -48,6 +48,11 @@ [LibraryClasses] > > [Protocols] > > gEfiMmCommunicationProtocolGuid ## PRODUCES > > > > +[Guids] > > + gEfiEndOfDxeEventGroupGuid > > + gEfiEventExitBootServicesGuid > > + gEfiEventReadyToBootGuid > > + > > [Pcd.common] > > gArmTokenSpaceGuid.PcdMmBufferBase > > gArmTokenSpaceGuid.PcdMmBufferSize > > diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c > > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c > > index feb9fa9f4ead..3203cf801a19 100644 > > --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c > > +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c > > @@ -265,6 +265,43 @@ GetMmCompatibility () > > return Status; > > } > > > > +STATIC EFI_GUID* CONST mGuidedEventGuid[] = { > > + &gEfiEndOfDxeEventGroupGuid, > > + &gEfiEventExitBootServicesGuid, > > + &gEfiEventReadyToBootGuid, > > +}; > > + > > +STATIC EFI_EVENT mGuidedEvent[ARRAY_SIZE (mGuidedEventGuid)]; > > + > > +/** > > + Event notification that is fired when GUIDed Event Group is signaled. > > + > > + @param Event The Event that is being processed, > > not used. > > + @param Context Event Context, not used. > > + > > +**/ > > +STATIC > > +VOID > > +EFIAPI > > +MmGuidedEventNotify ( > > + IN EFI_EVENT Event, > > + IN VOID *Context > > + ) > > +{ > > + EFI_MM_COMMUNICATE_HEADER Header; > > + UINTN Size; > > + > > + // > > + // Use Guid to initialize EFI_SMM_COMMUNICATE_HEADER structure > > + // > > + CopyGuid (&Header.HeaderGuid, Context); > > + Header.MessageLength = 1; > > + Header.Data[0] = 0; > > + > > + Size = sizeof (Header); > > + MmCommunicationCommunicate (&mMmCommunication, &Header, > > &Size); > > +} > > + > > /** > > The Entry Point for MM Communication > > > > @@ -287,6 +324,7 @@ MmCommunicationInitialize ( > > ) > > { > > EFI_STATUS Status; > > + UINTN Index; > > > > // Check if we can make the MM call > > Status = GetMmCompatibility (); > > @@ -351,8 +389,13 @@ MmCommunicationInitialize ( > > NULL, > > &mSetVirtualAddressMapEvent > > ); > > - if (Status == EFI_SUCCESS) { > > - return Status; > > + ASSERT_EFI_ERROR (Status); > > + > > + for (Index = 0; Index < ARRAY_SIZE (mGuidedEventGuid); Index++) { > > + Status = gBS->CreateEventEx (EVT_NOTIFY_SIGNAL, TPL_CALLBACK, > > + MmGuidedEventNotify, mGuidedEventGuid[Index], > > + mGuidedEventGuid[Index], > > &mGuidedEvent[Index]); > > + ASSERT_EFI_ERROR (Status); > > } > > > > gBS->UninstallProtocolInterface ( > > -- > > 2.20.1 > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
OK. To keep the compatibility of existing MM driver. That makes sense. If it is for security, I think EndOfDxe is the only point. ReadyToBoot and ExitBootService cannot be used for security purpose. Then do we need SmmReadyToLock ? :-) Thank you Yao Jiewen > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > Ard Biesheuvel > Sent: Tuesday, March 5, 2019 7:58 AM > To: Yao, Jiewen <jiewen.yao@intel.com> > Cc: edk2-devel@lists.01.org > Subject: Re: [edk2] [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal > architected PI events into MM context > > On Tue, 5 Mar 2019 at 16:56, Yao, Jiewen <jiewen.yao@intel.com> wrote: > > > > Hi > > In original SMM infrastructure, there are lots of interaction that SMM has > to know the DXE status. > > > > In StandaloneMm, I don't expect we have many interaction. Is there any > specific example? > > > > I am totally OK to add those. And I just want to know more usage. > > > > Reviewed-by: Jiewen.yao@intel.com > > > > Jiewen, > > Thanks for the review. > > It is not 100% clear at the moment, but since existing third party > software designed to run in MM context may make assumptions about > security of the platform (e.g., before vs after end of dxe) based on > these events, we should at least signal the common ones added in this > patch. > > > > > > > -----Original Message----- > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > > > Sent: Tuesday, March 5, 2019 5:33 AM > > > To: edk2-devel@lists.01.org > > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Achin Gupta > > > <achin.gupta@arm.com>; Supreeth Venkatesh > > > <supreeth.venkatesh@arm.com>; Yao, Jiewen <jiewen.yao@intel.com>; > > > Leif Lindholm <leif.lindholm@linaro.org>; Jagadeesh Ujja > > > <jagadeesh.ujja@arm.com> > > > Subject: [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal > architected > > > PI events into MM context > > > > > > PI defines a few architected events that have significance in the MM > > > context as well as in the non-secure DXE context. So register notify > > > handlers for these events, and relay them into the standalone MM world. > > > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > > --- > > > ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf | 5 > +++ > > > ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c | 47 > > > +++++++++++++++++++- > > > 2 files changed, 50 insertions(+), 2 deletions(-) > > > > > > diff --git > a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf > > > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf > > > index 88beafa39c05..8bf269270f9d 100644 > > > --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf > > > +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf > > > @@ -48,6 +48,11 @@ [LibraryClasses] > > > [Protocols] > > > gEfiMmCommunicationProtocolGuid ## PRODUCES > > > > > > +[Guids] > > > + gEfiEndOfDxeEventGroupGuid > > > + gEfiEventExitBootServicesGuid > > > + gEfiEventReadyToBootGuid > > > + > > > [Pcd.common] > > > gArmTokenSpaceGuid.PcdMmBufferBase > > > gArmTokenSpaceGuid.PcdMmBufferSize > > > diff --git > a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c > > > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c > > > index feb9fa9f4ead..3203cf801a19 100644 > > > --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c > > > +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c > > > @@ -265,6 +265,43 @@ GetMmCompatibility () > > > return Status; > > > } > > > > > > +STATIC EFI_GUID* CONST mGuidedEventGuid[] = { > > > + &gEfiEndOfDxeEventGroupGuid, > > > + &gEfiEventExitBootServicesGuid, > > > + &gEfiEventReadyToBootGuid, > > > +}; > > > + > > > +STATIC EFI_EVENT mGuidedEvent[ARRAY_SIZE (mGuidedEventGuid)]; > > > + > > > +/** > > > + Event notification that is fired when GUIDed Event Group is signaled. > > > + > > > + @param Event The Event that is being > processed, > > > not used. > > > + @param Context Event Context, not used. > > > + > > > +**/ > > > +STATIC > > > +VOID > > > +EFIAPI > > > +MmGuidedEventNotify ( > > > + IN EFI_EVENT Event, > > > + IN VOID *Context > > > + ) > > > +{ > > > + EFI_MM_COMMUNICATE_HEADER Header; > > > + UINTN Size; > > > + > > > + // > > > + // Use Guid to initialize EFI_SMM_COMMUNICATE_HEADER structure > > > + // > > > + CopyGuid (&Header.HeaderGuid, Context); > > > + Header.MessageLength = 1; > > > + Header.Data[0] = 0; > > > + > > > + Size = sizeof (Header); > > > + MmCommunicationCommunicate (&mMmCommunication, &Header, > > > &Size); > > > +} > > > + > > > /** > > > The Entry Point for MM Communication > > > > > > @@ -287,6 +324,7 @@ MmCommunicationInitialize ( > > > ) > > > { > > > EFI_STATUS Status; > > > + UINTN Index; > > > > > > // Check if we can make the MM call > > > Status = GetMmCompatibility (); > > > @@ -351,8 +389,13 @@ MmCommunicationInitialize ( > > > NULL, > > > &mSetVirtualAddressMapEvent > > > ); > > > - if (Status == EFI_SUCCESS) { > > > - return Status; > > > + ASSERT_EFI_ERROR (Status); > > > + > > > + for (Index = 0; Index < ARRAY_SIZE (mGuidedEventGuid); Index++) { > > > + Status = gBS->CreateEventEx (EVT_NOTIFY_SIGNAL, > TPL_CALLBACK, > > > + MmGuidedEventNotify, > mGuidedEventGuid[Index], > > > + mGuidedEventGuid[Index], > > > &mGuidedEvent[Index]); > > > + ASSERT_EFI_ERROR (Status); > > > } > > > > > > gBS->UninstallProtocolInterface ( > > > -- > > > 2.20.1 > > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Tue, 5 Mar 2019 at 17:04, Yao, Jiewen <jiewen.yao@intel.com> wrote: > > OK. To keep the compatibility of existing MM driver. That makes sense. > > If it is for security, I think EndOfDxe is the only point. > ReadyToBoot and ExitBootService cannot be used for security purpose. > OK, good to know. I will keep them for the time being - MM drivers may be able to release resources or do other useful things when the non-secure side enters runtime mode. > Then do we need SmmReadyToLock ? :-) > Good point. It looked fairly x86 specific to me. Do you think it is likely to be used in OEM code running in MM mode? > > -----Original Message----- > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > > Ard Biesheuvel > > Sent: Tuesday, March 5, 2019 7:58 AM > > To: Yao, Jiewen <jiewen.yao@intel.com> > > Cc: edk2-devel@lists.01.org > > Subject: Re: [edk2] [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal > > architected PI events into MM context > > > > On Tue, 5 Mar 2019 at 16:56, Yao, Jiewen <jiewen.yao@intel.com> wrote: > > > > > > Hi > > > In original SMM infrastructure, there are lots of interaction that SMM has > > to know the DXE status. > > > > > > In StandaloneMm, I don't expect we have many interaction. Is there any > > specific example? > > > > > > I am totally OK to add those. And I just want to know more usage. > > > > > > Reviewed-by: Jiewen.yao@intel.com > > > > > > > Jiewen, > > > > Thanks for the review. > > > > It is not 100% clear at the moment, but since existing third party > > software designed to run in MM context may make assumptions about > > security of the platform (e.g., before vs after end of dxe) based on > > these events, we should at least signal the common ones added in this > > patch. > > > > > > > > > > > > -----Original Message----- > > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > > > > Sent: Tuesday, March 5, 2019 5:33 AM > > > > To: edk2-devel@lists.01.org > > > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Achin Gupta > > > > <achin.gupta@arm.com>; Supreeth Venkatesh > > > > <supreeth.venkatesh@arm.com>; Yao, Jiewen <jiewen.yao@intel.com>; > > > > Leif Lindholm <leif.lindholm@linaro.org>; Jagadeesh Ujja > > > > <jagadeesh.ujja@arm.com> > > > > Subject: [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal > > architected > > > > PI events into MM context > > > > > > > > PI defines a few architected events that have significance in the MM > > > > context as well as in the non-secure DXE context. So register notify > > > > handlers for these events, and relay them into the standalone MM world. > > > > > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > > > --- > > > > ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf | 5 > > +++ > > > > ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c | 47 > > > > +++++++++++++++++++- > > > > 2 files changed, 50 insertions(+), 2 deletions(-) > > > > > > > > diff --git > > a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf > > > > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf > > > > index 88beafa39c05..8bf269270f9d 100644 > > > > --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf > > > > +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf > > > > @@ -48,6 +48,11 @@ [LibraryClasses] > > > > [Protocols] > > > > gEfiMmCommunicationProtocolGuid ## PRODUCES > > > > > > > > +[Guids] > > > > + gEfiEndOfDxeEventGroupGuid > > > > + gEfiEventExitBootServicesGuid > > > > + gEfiEventReadyToBootGuid > > > > + > > > > [Pcd.common] > > > > gArmTokenSpaceGuid.PcdMmBufferBase > > > > gArmTokenSpaceGuid.PcdMmBufferSize > > > > diff --git > > a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c > > > > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c > > > > index feb9fa9f4ead..3203cf801a19 100644 > > > > --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c > > > > +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c > > > > @@ -265,6 +265,43 @@ GetMmCompatibility () > > > > return Status; > > > > } > > > > > > > > +STATIC EFI_GUID* CONST mGuidedEventGuid[] = { > > > > + &gEfiEndOfDxeEventGroupGuid, > > > > + &gEfiEventExitBootServicesGuid, > > > > + &gEfiEventReadyToBootGuid, > > > > +}; > > > > + > > > > +STATIC EFI_EVENT mGuidedEvent[ARRAY_SIZE (mGuidedEventGuid)]; > > > > + > > > > +/** > > > > + Event notification that is fired when GUIDed Event Group is signaled. > > > > + > > > > + @param Event The Event that is being > > processed, > > > > not used. > > > > + @param Context Event Context, not used. > > > > + > > > > +**/ > > > > +STATIC > > > > +VOID > > > > +EFIAPI > > > > +MmGuidedEventNotify ( > > > > + IN EFI_EVENT Event, > > > > + IN VOID *Context > > > > + ) > > > > +{ > > > > + EFI_MM_COMMUNICATE_HEADER Header; > > > > + UINTN Size; > > > > + > > > > + // > > > > + // Use Guid to initialize EFI_SMM_COMMUNICATE_HEADER structure > > > > + // > > > > + CopyGuid (&Header.HeaderGuid, Context); > > > > + Header.MessageLength = 1; > > > > + Header.Data[0] = 0; > > > > + > > > > + Size = sizeof (Header); > > > > + MmCommunicationCommunicate (&mMmCommunication, &Header, > > > > &Size); > > > > +} > > > > + > > > > /** > > > > The Entry Point for MM Communication > > > > > > > > @@ -287,6 +324,7 @@ MmCommunicationInitialize ( > > > > ) > > > > { > > > > EFI_STATUS Status; > > > > + UINTN Index; > > > > > > > > // Check if we can make the MM call > > > > Status = GetMmCompatibility (); > > > > @@ -351,8 +389,13 @@ MmCommunicationInitialize ( > > > > NULL, > > > > &mSetVirtualAddressMapEvent > > > > ); > > > > - if (Status == EFI_SUCCESS) { > > > > - return Status; > > > > + ASSERT_EFI_ERROR (Status); > > > > + > > > > + for (Index = 0; Index < ARRAY_SIZE (mGuidedEventGuid); Index++) { > > > > + Status = gBS->CreateEventEx (EVT_NOTIFY_SIGNAL, > > TPL_CALLBACK, > > > > + MmGuidedEventNotify, > > mGuidedEventGuid[Index], > > > > + mGuidedEventGuid[Index], > > > > &mGuidedEvent[Index]); > > > > + ASSERT_EFI_ERROR (Status); > > > > } > > > > > > > > gBS->UninstallProtocolInterface ( > > > > -- > > > > 2.20.1 > > > > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.01.org > > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
For current X86 world, we do use both SmmReadyToLock and EndOfDxe, for almost all X86 platform. So, let me clarify: If we try to align with PI spec, we can add EndOfDxe/ReadyToBoot/ExitBootService. If we try to align with security, we can add EndOfDxe/SmmReadyToLock. It depends upon the what goal we want to achieve. That is why I ask if we have specific use case. Anyway, I think we can add when it is really needed later. So I am OK with current patch. Thank you Yao Jiewen > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > Ard Biesheuvel > Sent: Tuesday, March 5, 2019 8:07 AM > To: Yao, Jiewen <jiewen.yao@intel.com> > Cc: edk2-devel@lists.01.org > Subject: Re: [edk2] [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal > architected PI events into MM context > > On Tue, 5 Mar 2019 at 17:04, Yao, Jiewen <jiewen.yao@intel.com> wrote: > > > > OK. To keep the compatibility of existing MM driver. That makes sense. > > > > If it is for security, I think EndOfDxe is the only point. > > ReadyToBoot and ExitBootService cannot be used for security purpose. > > > > OK, good to know. I will keep them for the time being - MM drivers may > be able to release resources or do other useful things when the > non-secure side enters runtime mode. > > > Then do we need SmmReadyToLock ? :-) > > > > Good point. It looked fairly x86 specific to me. Do you think it is > likely to be used in OEM code running in MM mode? > > > > > > > -----Original Message----- > > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf > Of > > > Ard Biesheuvel > > > Sent: Tuesday, March 5, 2019 7:58 AM > > > To: Yao, Jiewen <jiewen.yao@intel.com> > > > Cc: edk2-devel@lists.01.org > > > Subject: Re: [edk2] [PATCH 10/10] ArmPkg/MmCommunicationDxe: > signal > > > architected PI events into MM context > > > > > > On Tue, 5 Mar 2019 at 16:56, Yao, Jiewen <jiewen.yao@intel.com> > wrote: > > > > > > > > Hi > > > > In original SMM infrastructure, there are lots of interaction that SMM > has > > > to know the DXE status. > > > > > > > > In StandaloneMm, I don't expect we have many interaction. Is there > any > > > specific example? > > > > > > > > I am totally OK to add those. And I just want to know more usage. > > > > > > > > Reviewed-by: Jiewen.yao@intel.com > > > > > > > > > > Jiewen, > > > > > > Thanks for the review. > > > > > > It is not 100% clear at the moment, but since existing third party > > > software designed to run in MM context may make assumptions about > > > security of the platform (e.g., before vs after end of dxe) based on > > > these events, we should at least signal the common ones added in this > > > patch. > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > > > > > Sent: Tuesday, March 5, 2019 5:33 AM > > > > > To: edk2-devel@lists.01.org > > > > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Achin Gupta > > > > > <achin.gupta@arm.com>; Supreeth Venkatesh > > > > > <supreeth.venkatesh@arm.com>; Yao, Jiewen > <jiewen.yao@intel.com>; > > > > > Leif Lindholm <leif.lindholm@linaro.org>; Jagadeesh Ujja > > > > > <jagadeesh.ujja@arm.com> > > > > > Subject: [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal > > > architected > > > > > PI events into MM context > > > > > > > > > > PI defines a few architected events that have significance in the MM > > > > > context as well as in the non-secure DXE context. So register notify > > > > > handlers for these events, and relay them into the standalone MM > world. > > > > > > > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > > > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > > > > --- > > > > > ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf | > 5 > > > +++ > > > > > ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c | > 47 > > > > > +++++++++++++++++++- > > > > > 2 files changed, 50 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git > > > a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf > > > > > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf > > > > > index 88beafa39c05..8bf269270f9d 100644 > > > > > --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf > > > > > +++ > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf > > > > > @@ -48,6 +48,11 @@ [LibraryClasses] > > > > > [Protocols] > > > > > gEfiMmCommunicationProtocolGuid ## > PRODUCES > > > > > > > > > > +[Guids] > > > > > + gEfiEndOfDxeEventGroupGuid > > > > > + gEfiEventExitBootServicesGuid > > > > > + gEfiEventReadyToBootGuid > > > > > + > > > > > [Pcd.common] > > > > > gArmTokenSpaceGuid.PcdMmBufferBase > > > > > gArmTokenSpaceGuid.PcdMmBufferSize > > > > > diff --git > > > a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c > > > > > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c > > > > > index feb9fa9f4ead..3203cf801a19 100644 > > > > > --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c > > > > > +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c > > > > > @@ -265,6 +265,43 @@ GetMmCompatibility () > > > > > return Status; > > > > > } > > > > > > > > > > +STATIC EFI_GUID* CONST mGuidedEventGuid[] = { > > > > > + &gEfiEndOfDxeEventGroupGuid, > > > > > + &gEfiEventExitBootServicesGuid, > > > > > + &gEfiEventReadyToBootGuid, > > > > > +}; > > > > > + > > > > > +STATIC EFI_EVENT mGuidedEvent[ARRAY_SIZE > (mGuidedEventGuid)]; > > > > > + > > > > > +/** > > > > > + Event notification that is fired when GUIDed Event Group is > signaled. > > > > > + > > > > > + @param Event The Event that is being > > > processed, > > > > > not used. > > > > > + @param Context Event Context, not used. > > > > > + > > > > > +**/ > > > > > +STATIC > > > > > +VOID > > > > > +EFIAPI > > > > > +MmGuidedEventNotify ( > > > > > + IN EFI_EVENT Event, > > > > > + IN VOID *Context > > > > > + ) > > > > > +{ > > > > > + EFI_MM_COMMUNICATE_HEADER Header; > > > > > + UINTN Size; > > > > > + > > > > > + // > > > > > + // Use Guid to initialize EFI_SMM_COMMUNICATE_HEADER > structure > > > > > + // > > > > > + CopyGuid (&Header.HeaderGuid, Context); > > > > > + Header.MessageLength = 1; > > > > > + Header.Data[0] = 0; > > > > > + > > > > > + Size = sizeof (Header); > > > > > + MmCommunicationCommunicate (&mMmCommunication, > &Header, > > > > > &Size); > > > > > +} > > > > > + > > > > > /** > > > > > The Entry Point for MM Communication > > > > > > > > > > @@ -287,6 +324,7 @@ MmCommunicationInitialize ( > > > > > ) > > > > > { > > > > > EFI_STATUS Status; > > > > > + UINTN Index; > > > > > > > > > > // Check if we can make the MM call > > > > > Status = GetMmCompatibility (); > > > > > @@ -351,8 +389,13 @@ MmCommunicationInitialize ( > > > > > NULL, > > > > > &mSetVirtualAddressMapEvent > > > > > ); > > > > > - if (Status == EFI_SUCCESS) { > > > > > - return Status; > > > > > + ASSERT_EFI_ERROR (Status); > > > > > + > > > > > + for (Index = 0; Index < ARRAY_SIZE (mGuidedEventGuid); Index++) > { > > > > > + Status = gBS->CreateEventEx (EVT_NOTIFY_SIGNAL, > > > TPL_CALLBACK, > > > > > + MmGuidedEventNotify, > > > mGuidedEventGuid[Index], > > > > > + mGuidedEventGuid[Index], > > > > > &mGuidedEvent[Index]); > > > > > + ASSERT_EFI_ERROR (Status); > > > > > } > > > > > > > > > > gBS->UninstallProtocolInterface ( > > > > > -- > > > > > 2.20.1 > > > > > > > _______________________________________________ > > > edk2-devel mailing list > > > edk2-devel@lists.01.org > > > https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
There is an architectural difference between EndOfDxe and SmmReadyToLock events. It's important to have both of them. Here is what PI specification says: == Transition from the environment where all of the components are under the authority of the platform manufacturer to the environment where third party modules are executed is a two-step process: 1.End of DXE Event is signaled. This event presents the last opportunity to use resources or interfaces that are going to be locked or disabled in anticipation of the invocation of 3rd party extensible modules. 2.DXE SMM Ready to Lock Protocol is installed. PI modules that need to lock or protect their resources in anticipation of the invocation of 3rd party extensible modules should register for notification on installation of this protocol and effect the appropriate protections in their notification handlers == -----Original Message----- From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Yao, Jiewen Sent: Tuesday, March 05, 2019 11:19 AM To: Ard Biesheuvel Cc: edk2-devel@lists.01.org Subject: Re: [edk2] [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected PI events into MM context For current X86 world, we do use both SmmReadyToLock and EndOfDxe, for almost all X86 platform. So, let me clarify: If we try to align with PI spec, we can add EndOfDxe/ReadyToBoot/ExitBootService. If we try to align with security, we can add EndOfDxe/SmmReadyToLock. It depends upon the what goal we want to achieve. That is why I ask if we have specific use case. Anyway, I think we can add when it is really needed later. So I am OK with current patch. Thank you Yao Jiewen > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > Ard Biesheuvel > Sent: Tuesday, March 5, 2019 8:07 AM > To: Yao, Jiewen <jiewen.yao@intel.com> > Cc: edk2-devel@lists.01.org > Subject: Re: [edk2] [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal > architected PI events into MM context > > On Tue, 5 Mar 2019 at 17:04, Yao, Jiewen <jiewen.yao@intel.com> wrote: > > > > OK. To keep the compatibility of existing MM driver. That makes sense. > > > > If it is for security, I think EndOfDxe is the only point. > > ReadyToBoot and ExitBootService cannot be used for security purpose. > > > > OK, good to know. I will keep them for the time being - MM drivers may > be able to release resources or do other useful things when the > non-secure side enters runtime mode. > > > Then do we need SmmReadyToLock ? :-) > > > > Good point. It looked fairly x86 specific to me. Do you think it is > likely to be used in OEM code running in MM mode? > > > > > > > -----Original Message----- > > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf > Of > > > Ard Biesheuvel > > > Sent: Tuesday, March 5, 2019 7:58 AM > > > To: Yao, Jiewen <jiewen.yao@intel.com> > > > Cc: edk2-devel@lists.01.org > > > Subject: Re: [edk2] [PATCH 10/10] ArmPkg/MmCommunicationDxe: > signal > > > architected PI events into MM context > > > > > > On Tue, 5 Mar 2019 at 16:56, Yao, Jiewen <jiewen.yao@intel.com> > wrote: > > > > > > > > Hi > > > > In original SMM infrastructure, there are lots of interaction that SMM > has > > > to know the DXE status. > > > > > > > > In StandaloneMm, I don't expect we have many interaction. Is there > any > > > specific example? > > > > > > > > I am totally OK to add those. And I just want to know more usage. > > > > > > > > Reviewed-by: Jiewen.yao@intel.com > > > > > > > > > > Jiewen, > > > > > > Thanks for the review. > > > > > > It is not 100% clear at the moment, but since existing third party > > > software designed to run in MM context may make assumptions about > > > security of the platform (e.g., before vs after end of dxe) based on > > > these events, we should at least signal the common ones added in this > > > patch. > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > > > > > Sent: Tuesday, March 5, 2019 5:33 AM > > > > > To: edk2-devel@lists.01.org > > > > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Achin Gupta > > > > > <achin.gupta@arm.com>; Supreeth Venkatesh > > > > > <supreeth.venkatesh@arm.com>; Yao, Jiewen > <jiewen.yao@intel.com>; > > > > > Leif Lindholm <leif.lindholm@linaro.org>; Jagadeesh Ujja > > > > > <jagadeesh.ujja@arm.com> > > > > > Subject: [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal > > > architected > > > > > PI events into MM context > > > > > > > > > > PI defines a few architected events that have significance in the MM > > > > > context as well as in the non-secure DXE context. So register notify > > > > > handlers for these events, and relay them into the standalone MM > world. > > > > > > > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > > > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > > > > --- > > > > > ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf | > 5 > > > +++ > > > > > ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c | > 47 > > > > > +++++++++++++++++++- > > > > > 2 files changed, 50 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git > > > a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf > > > > > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf > > > > > index 88beafa39c05..8bf269270f9d 100644 > > > > > --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf > > > > > +++ > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf > > > > > @@ -48,6 +48,11 @@ [LibraryClasses] > > > > > [Protocols] > > > > > gEfiMmCommunicationProtocolGuid ## > PRODUCES > > > > > > > > > > +[Guids] > > > > > + gEfiEndOfDxeEventGroupGuid > > > > > + gEfiEventExitBootServicesGuid > > > > > + gEfiEventReadyToBootGuid > > > > > + > > > > > [Pcd.common] > > > > > gArmTokenSpaceGuid.PcdMmBufferBase > > > > > gArmTokenSpaceGuid.PcdMmBufferSize > > > > > diff --git > > > a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c > > > > > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c > > > > > index feb9fa9f4ead..3203cf801a19 100644 > > > > > --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c > > > > > +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c > > > > > @@ -265,6 +265,43 @@ GetMmCompatibility () > > > > > return Status; > > > > > } > > > > > > > > > > +STATIC EFI_GUID* CONST mGuidedEventGuid[] = { > > > > > + &gEfiEndOfDxeEventGroupGuid, > > > > > + &gEfiEventExitBootServicesGuid, > > > > > + &gEfiEventReadyToBootGuid, > > > > > +}; > > > > > + > > > > > +STATIC EFI_EVENT mGuidedEvent[ARRAY_SIZE > (mGuidedEventGuid)]; > > > > > + > > > > > +/** > > > > > + Event notification that is fired when GUIDed Event Group is > signaled. > > > > > + > > > > > + @param Event The Event that is being > > > processed, > > > > > not used. > > > > > + @param Context Event Context, not used. > > > > > + > > > > > +**/ > > > > > +STATIC > > > > > +VOID > > > > > +EFIAPI > > > > > +MmGuidedEventNotify ( > > > > > + IN EFI_EVENT Event, > > > > > + IN VOID *Context > > > > > + ) > > > > > +{ > > > > > + EFI_MM_COMMUNICATE_HEADER Header; > > > > > + UINTN Size; > > > > > + > > > > > + // > > > > > + // Use Guid to initialize EFI_SMM_COMMUNICATE_HEADER > structure > > > > > + // > > > > > + CopyGuid (&Header.HeaderGuid, Context); > > > > > + Header.MessageLength = 1; > > > > > + Header.Data[0] = 0; > > > > > + > > > > > + Size = sizeof (Header); > > > > > + MmCommunicationCommunicate (&mMmCommunication, > &Header, > > > > > &Size); > > > > > +} > > > > > + > > > > > /** > > > > > The Entry Point for MM Communication > > > > > > > > > > @@ -287,6 +324,7 @@ MmCommunicationInitialize ( > > > > > ) > > > > > { > > > > > EFI_STATUS Status; > > > > > + UINTN Index; > > > > > > > > > > // Check if we can make the MM call > > > > > Status = GetMmCompatibility (); > > > > > @@ -351,8 +389,13 @@ MmCommunicationInitialize ( > > > > > NULL, > > > > > &mSetVirtualAddressMapEvent > > > > > ); > > > > > - if (Status == EFI_SUCCESS) { > > > > > - return Status; > > > > > + ASSERT_EFI_ERROR (Status); > > > > > + > > > > > + for (Index = 0; Index < ARRAY_SIZE (mGuidedEventGuid); Index++) > { > > > > > + Status = gBS->CreateEventEx (EVT_NOTIFY_SIGNAL, > > > TPL_CALLBACK, > > > > > + MmGuidedEventNotify, > > > mGuidedEventGuid[Index], > > > > > + mGuidedEventGuid[Index], > > > > > &mGuidedEvent[Index]); > > > > > + ASSERT_EFI_ERROR (Status); > > > > > } > > > > > > > > > > gBS->UninstallProtocolInterface ( > > > > > -- > > > > > 2.20.1 > > > > > > > _______________________________________________ > > > edk2-devel mailing list > > > edk2-devel@lists.01.org > > > https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel Please consider the environment before printing this email. The information contained in this message may be confidential and proprietary to American Megatrends, Inc. This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited. Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Tue, 5 Mar 2019 at 17:53, Felix Polyudov <Felixp@ami.com> wrote: > > There is an architectural difference between EndOfDxe and SmmReadyToLock events. > It's important to have both of them. > Here is what PI specification says: > == > Transition from the environment where all of the components are under the authority of the platform manufacturer to the environment where third party modules are executed is a two-step process: > 1.End of DXE Event is signaled. This event presents the last opportunity to use resources or interfaces that are going to be locked or disabled in anticipation of the invocation of 3rd party extensible modules. > 2.DXE SMM Ready to Lock Protocol is installed. PI modules that need to lock or protect their resources in anticipation of the invocation of 3rd party extensible modules should register for notification on installation of this protocol and effect the appropriate protections in their notification handlers Thanks for pointing that out, Felix. I will add SmmReadyToLock as well. > == > > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Yao, Jiewen > Sent: Tuesday, March 05, 2019 11:19 AM > To: Ard Biesheuvel > Cc: edk2-devel@lists.01.org > Subject: Re: [edk2] [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected PI events into MM context > > For current X86 world, we do use both SmmReadyToLock and EndOfDxe, for almost all X86 platform. > > So, let me clarify: > If we try to align with PI spec, we can add EndOfDxe/ReadyToBoot/ExitBootService. > If we try to align with security, we can add EndOfDxe/SmmReadyToLock. > > It depends upon the what goal we want to achieve. That is why I ask if we have specific use case. > > Anyway, I think we can add when it is really needed later. > So I am OK with current patch. > > Thank you > Yao Jiewen > > > -----Original Message----- > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > > Ard Biesheuvel > > Sent: Tuesday, March 5, 2019 8:07 AM > > To: Yao, Jiewen <jiewen.yao@intel.com> > > Cc: edk2-devel@lists.01.org > > Subject: Re: [edk2] [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal > > architected PI events into MM context > > > > On Tue, 5 Mar 2019 at 17:04, Yao, Jiewen <jiewen.yao@intel.com> wrote: > > > > > > OK. To keep the compatibility of existing MM driver. That makes sense. > > > > > > If it is for security, I think EndOfDxe is the only point. > > > ReadyToBoot and ExitBootService cannot be used for security purpose. > > > > > > > OK, good to know. I will keep them for the time being - MM drivers may > > be able to release resources or do other useful things when the > > non-secure side enters runtime mode. > > > > > Then do we need SmmReadyToLock ? :-) > > > > > > > Good point. It looked fairly x86 specific to me. Do you think it is > > likely to be used in OEM code running in MM mode? > > > > > > > > > > > > -----Original Message----- > > > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf > > Of > > > > Ard Biesheuvel > > > > Sent: Tuesday, March 5, 2019 7:58 AM > > > > To: Yao, Jiewen <jiewen.yao@intel.com> > > > > Cc: edk2-devel@lists.01.org > > > > Subject: Re: [edk2] [PATCH 10/10] ArmPkg/MmCommunicationDxe: > > signal > > > > architected PI events into MM context > > > > > > > > On Tue, 5 Mar 2019 at 16:56, Yao, Jiewen <jiewen.yao@intel.com> > > wrote: > > > > > > > > > > Hi > > > > > In original SMM infrastructure, there are lots of interaction that SMM > > has > > > > to know the DXE status. > > > > > > > > > > In StandaloneMm, I don't expect we have many interaction. Is there > > any > > > > specific example? > > > > > > > > > > I am totally OK to add those. And I just want to know more usage. > > > > > > > > > > Reviewed-by: Jiewen.yao@intel.com > > > > > > > > > > > > > Jiewen, > > > > > > > > Thanks for the review. > > > > > > > > It is not 100% clear at the moment, but since existing third party > > > > software designed to run in MM context may make assumptions about > > > > security of the platform (e.g., before vs after end of dxe) based on > > > > these events, we should at least signal the common ones added in this > > > > patch. > > > > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > > > > > > Sent: Tuesday, March 5, 2019 5:33 AM > > > > > > To: edk2-devel@lists.01.org > > > > > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Achin Gupta > > > > > > <achin.gupta@arm.com>; Supreeth Venkatesh > > > > > > <supreeth.venkatesh@arm.com>; Yao, Jiewen > > <jiewen.yao@intel.com>; > > > > > > Leif Lindholm <leif.lindholm@linaro.org>; Jagadeesh Ujja > > > > > > <jagadeesh.ujja@arm.com> > > > > > > Subject: [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal > > > > architected > > > > > > PI events into MM context > > > > > > > > > > > > PI defines a few architected events that have significance in the MM > > > > > > context as well as in the non-secure DXE context. So register notify > > > > > > handlers for these events, and relay them into the standalone MM > > world. > > > > > > > > > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > > > > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > > > > > --- > > > > > > ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf | > > 5 > > > > +++ > > > > > > ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c | > > 47 > > > > > > +++++++++++++++++++- > > > > > > 2 files changed, 50 insertions(+), 2 deletions(-) > > > > > > > > > > > > diff --git > > > > a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf > > > > > > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf > > > > > > index 88beafa39c05..8bf269270f9d 100644 > > > > > > --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf > > > > > > +++ > > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf > > > > > > @@ -48,6 +48,11 @@ [LibraryClasses] > > > > > > [Protocols] > > > > > > gEfiMmCommunicationProtocolGuid ## > > PRODUCES > > > > > > > > > > > > +[Guids] > > > > > > + gEfiEndOfDxeEventGroupGuid > > > > > > + gEfiEventExitBootServicesGuid > > > > > > + gEfiEventReadyToBootGuid > > > > > > + > > > > > > [Pcd.common] > > > > > > gArmTokenSpaceGuid.PcdMmBufferBase > > > > > > gArmTokenSpaceGuid.PcdMmBufferSize > > > > > > diff --git > > > > a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c > > > > > > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c > > > > > > index feb9fa9f4ead..3203cf801a19 100644 > > > > > > --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c > > > > > > +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c > > > > > > @@ -265,6 +265,43 @@ GetMmCompatibility () > > > > > > return Status; > > > > > > } > > > > > > > > > > > > +STATIC EFI_GUID* CONST mGuidedEventGuid[] = { > > > > > > + &gEfiEndOfDxeEventGroupGuid, > > > > > > + &gEfiEventExitBootServicesGuid, > > > > > > + &gEfiEventReadyToBootGuid, > > > > > > +}; > > > > > > + > > > > > > +STATIC EFI_EVENT mGuidedEvent[ARRAY_SIZE > > (mGuidedEventGuid)]; > > > > > > + > > > > > > +/** > > > > > > + Event notification that is fired when GUIDed Event Group is > > signaled. > > > > > > + > > > > > > + @param Event The Event that is being > > > > processed, > > > > > > not used. > > > > > > + @param Context Event Context, not used. > > > > > > + > > > > > > +**/ > > > > > > +STATIC > > > > > > +VOID > > > > > > +EFIAPI > > > > > > +MmGuidedEventNotify ( > > > > > > + IN EFI_EVENT Event, > > > > > > + IN VOID *Context > > > > > > + ) > > > > > > +{ > > > > > > + EFI_MM_COMMUNICATE_HEADER Header; > > > > > > + UINTN Size; > > > > > > + > > > > > > + // > > > > > > + // Use Guid to initialize EFI_SMM_COMMUNICATE_HEADER > > structure > > > > > > + // > > > > > > + CopyGuid (&Header.HeaderGuid, Context); > > > > > > + Header.MessageLength = 1; > > > > > > + Header.Data[0] = 0; > > > > > > + > > > > > > + Size = sizeof (Header); > > > > > > + MmCommunicationCommunicate (&mMmCommunication, > > &Header, > > > > > > &Size); > > > > > > +} > > > > > > + > > > > > > /** > > > > > > The Entry Point for MM Communication > > > > > > > > > > > > @@ -287,6 +324,7 @@ MmCommunicationInitialize ( > > > > > > ) > > > > > > { > > > > > > EFI_STATUS Status; > > > > > > + UINTN Index; > > > > > > > > > > > > // Check if we can make the MM call > > > > > > Status = GetMmCompatibility (); > > > > > > @@ -351,8 +389,13 @@ MmCommunicationInitialize ( > > > > > > NULL, > > > > > > &mSetVirtualAddressMapEvent > > > > > > ); > > > > > > - if (Status == EFI_SUCCESS) { > > > > > > - return Status; > > > > > > + ASSERT_EFI_ERROR (Status); > > > > > > + > > > > > > + for (Index = 0; Index < ARRAY_SIZE (mGuidedEventGuid); Index++) > > { > > > > > > + Status = gBS->CreateEventEx (EVT_NOTIFY_SIGNAL, > > > > TPL_CALLBACK, > > > > > > + MmGuidedEventNotify, > > > > mGuidedEventGuid[Index], > > > > > > + mGuidedEventGuid[Index], > > > > > > &mGuidedEvent[Index]); > > > > > > + ASSERT_EFI_ERROR (Status); > > > > > > } > > > > > > > > > > > > gBS->UninstallProtocolInterface ( > > > > > > -- > > > > > > 2.20.1 > > > > > > > > > _______________________________________________ > > > > edk2-devel mailing list > > > > edk2-devel@lists.01.org > > > > https://lists.01.org/mailman/listinfo/edk2-devel > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.01.org > > https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel > > Please consider the environment before printing this email. > > The information contained in this message may be confidential and proprietary to American Megatrends, Inc. This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited. Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Reviewed-by: achin.gupta@arm.com On Tue, Mar 05, 2019 at 02:32:48PM +0100, Ard Biesheuvel wrote: > PI defines a few architected events that have significance in the MM > context as well as in the non-secure DXE context. So register notify > handlers for these events, and relay them into the standalone MM world. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf | 5 +++ > ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c | 47 +++++++++++++++++++- > 2 files changed, 50 insertions(+), 2 deletions(-) > > diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf > index 88beafa39c05..8bf269270f9d 100644 > --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf > +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf > @@ -48,6 +48,11 @@ [LibraryClasses] > [Protocols] > gEfiMmCommunicationProtocolGuid ## PRODUCES > > +[Guids] > + gEfiEndOfDxeEventGroupGuid > + gEfiEventExitBootServicesGuid > + gEfiEventReadyToBootGuid > + > [Pcd.common] > gArmTokenSpaceGuid.PcdMmBufferBase > gArmTokenSpaceGuid.PcdMmBufferSize > diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c > index feb9fa9f4ead..3203cf801a19 100644 > --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c > +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c > @@ -265,6 +265,43 @@ GetMmCompatibility () > return Status; > } > > +STATIC EFI_GUID* CONST mGuidedEventGuid[] = { > + &gEfiEndOfDxeEventGroupGuid, > + &gEfiEventExitBootServicesGuid, > + &gEfiEventReadyToBootGuid, > +}; > + > +STATIC EFI_EVENT mGuidedEvent[ARRAY_SIZE (mGuidedEventGuid)]; > + > +/** > + Event notification that is fired when GUIDed Event Group is signaled. > + > + @param Event The Event that is being processed, not used. > + @param Context Event Context, not used. > + > +**/ > +STATIC > +VOID > +EFIAPI > +MmGuidedEventNotify ( > + IN EFI_EVENT Event, > + IN VOID *Context > + ) > +{ > + EFI_MM_COMMUNICATE_HEADER Header; > + UINTN Size; > + > + // > + // Use Guid to initialize EFI_SMM_COMMUNICATE_HEADER structure > + // > + CopyGuid (&Header.HeaderGuid, Context); > + Header.MessageLength = 1; > + Header.Data[0] = 0; > + > + Size = sizeof (Header); > + MmCommunicationCommunicate (&mMmCommunication, &Header, &Size); > +} > + > /** > The Entry Point for MM Communication > > @@ -287,6 +324,7 @@ MmCommunicationInitialize ( > ) > { > EFI_STATUS Status; > + UINTN Index; > > // Check if we can make the MM call > Status = GetMmCompatibility (); > @@ -351,8 +389,13 @@ MmCommunicationInitialize ( > NULL, > &mSetVirtualAddressMapEvent > ); > - if (Status == EFI_SUCCESS) { > - return Status; > + ASSERT_EFI_ERROR (Status); > + > + for (Index = 0; Index < ARRAY_SIZE (mGuidedEventGuid); Index++) { > + Status = gBS->CreateEventEx (EVT_NOTIFY_SIGNAL, TPL_CALLBACK, > + MmGuidedEventNotify, mGuidedEventGuid[Index], > + mGuidedEventGuid[Index], &mGuidedEvent[Index]); > + ASSERT_EFI_ERROR (Status); > } > > gBS->UninstallProtocolInterface ( > -- > 2.20.1 > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf index 88beafa39c05..8bf269270f9d 100644 --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf @@ -48,6 +48,11 @@ [LibraryClasses] [Protocols] gEfiMmCommunicationProtocolGuid ## PRODUCES +[Guids] + gEfiEndOfDxeEventGroupGuid + gEfiEventExitBootServicesGuid + gEfiEventReadyToBootGuid + [Pcd.common] gArmTokenSpaceGuid.PcdMmBufferBase gArmTokenSpaceGuid.PcdMmBufferSize diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c index feb9fa9f4ead..3203cf801a19 100644 --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c @@ -265,6 +265,43 @@ GetMmCompatibility () return Status; } +STATIC EFI_GUID* CONST mGuidedEventGuid[] = { + &gEfiEndOfDxeEventGroupGuid, + &gEfiEventExitBootServicesGuid, + &gEfiEventReadyToBootGuid, +}; + +STATIC EFI_EVENT mGuidedEvent[ARRAY_SIZE (mGuidedEventGuid)]; + +/** + Event notification that is fired when GUIDed Event Group is signaled. + + @param Event The Event that is being processed, not used. + @param Context Event Context, not used. + +**/ +STATIC +VOID +EFIAPI +MmGuidedEventNotify ( + IN EFI_EVENT Event, + IN VOID *Context + ) +{ + EFI_MM_COMMUNICATE_HEADER Header; + UINTN Size; + + // + // Use Guid to initialize EFI_SMM_COMMUNICATE_HEADER structure + // + CopyGuid (&Header.HeaderGuid, Context); + Header.MessageLength = 1; + Header.Data[0] = 0; + + Size = sizeof (Header); + MmCommunicationCommunicate (&mMmCommunication, &Header, &Size); +} + /** The Entry Point for MM Communication @@ -287,6 +324,7 @@ MmCommunicationInitialize ( ) { EFI_STATUS Status; + UINTN Index; // Check if we can make the MM call Status = GetMmCompatibility (); @@ -351,8 +389,13 @@ MmCommunicationInitialize ( NULL, &mSetVirtualAddressMapEvent ); - if (Status == EFI_SUCCESS) { - return Status; + ASSERT_EFI_ERROR (Status); + + for (Index = 0; Index < ARRAY_SIZE (mGuidedEventGuid); Index++) { + Status = gBS->CreateEventEx (EVT_NOTIFY_SIGNAL, TPL_CALLBACK, + MmGuidedEventNotify, mGuidedEventGuid[Index], + mGuidedEventGuid[Index], &mGuidedEvent[Index]); + ASSERT_EFI_ERROR (Status); } gBS->UninstallProtocolInterface (
PI defines a few architected events that have significance in the MM context as well as in the non-secure DXE context. So register notify handlers for these events, and relay them into the standalone MM world. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf | 5 +++ ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c | 47 +++++++++++++++++++- 2 files changed, 50 insertions(+), 2 deletions(-) -- 2.20.1 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel