Re: [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected PI events into MM context

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

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
>