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

["Yao, Jiewen" <jiewen.yao@intel.com>]

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