Re: [PATCH 07/10] StandaloneMmPkg/Core: dispatch all drivers at init time

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

Reviewed-by: jiewen.yao@intel.com


> -----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 07/10] StandaloneMmPkg/Core: dispatch all drivers at init
> time
> 
> Instead of deferring dispatch of the remaining MM drivers once the
> CPU driver has been dispatched, proceed and dispatch all drivers.
> This makes sense for standalone MM, since all dispatchable drivers
> should be present in the initial firmware volume anyway: dispatch
> of additional FVs originating in the non-secure side is not supported.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  StandaloneMmPkg/Core/Dispatcher.c       | 92 --------------------
>  StandaloneMmPkg/Core/StandaloneMmCore.c |  1 -
>  2 files changed, 93 deletions(-)
> 
> diff --git a/StandaloneMmPkg/Core/Dispatcher.c
> b/StandaloneMmPkg/Core/Dispatcher.c
> index 8a2ad5118d92..bede4832cfb7 100644
> --- a/StandaloneMmPkg/Core/Dispatcher.c
> +++ b/StandaloneMmPkg/Core/Dispatcher.c
> @@ -575,7 +575,6 @@ MmDispatcher (
>    LIST_ENTRY            *Link;
>    EFI_MM_DRIVER_ENTRY  *DriverEntry;
>    BOOLEAN               ReadyToRun;
> -  BOOLEAN               PreviousMmEntryPointRegistered;
> 
>    DEBUG ((DEBUG_INFO, "MmDispatcher\n"));
> 
> @@ -639,11 +638,6 @@ MmDispatcher (
>        DriverEntry->Initialized  = TRUE;
>        RemoveEntryList (&DriverEntry->ScheduledLink);
> 
> -      //
> -      // Cache state of MmEntryPointRegistered before calling entry point
> -      //
> -      PreviousMmEntryPointRegistered =
> gMmCorePrivate->MmEntryPointRegistered;
> -
>        //
>        // For each MM driver, pass NULL as ImageHandle
>        //
> @@ -661,20 +655,6 @@ MmDispatcher (
>          DEBUG ((DEBUG_INFO, "StartImage Status - %r\n", Status));
>          MmFreePages(DriverEntry->ImageBuffer,
> DriverEntry->NumberOfPage);
>        }
> -
> -      if (!PreviousMmEntryPointRegistered &&
> gMmCorePrivate->MmEntryPointRegistered) {
> -        //
> -        // Return immediately if the MM Entry Point was registered by the
> MM
> -        // Driver that was just dispatched.  The MM IPL will reinvoke the
> MM
> -        // Core Dispatcher.  This is required so MM Mode may be
> enabled as soon
> -        // as all the dependent MM Drivers for MM Mode have been
> dispatched.
> -        // Once the MM Entry Point has been registered, then MM Mode
> will be
> -        // used.
> -        //
> -        gRequestDispatch = TRUE;
> -        gDispatcherRunning = FALSE;
> -        return EFI_NOT_READY;
> -      }
>      }
> 
>      //
> @@ -903,78 +883,6 @@ MmAddToDriverList (
>    return EFI_SUCCESS;
>  }
> 
> -/**
> -  This function is the main entry point for an MM handler dispatch
> -  or communicate-based callback.
> -
> -  Event notification that is fired every time a FV dispatch protocol is added.
> -  More than one protocol may have been added when this event is fired, so
> you
> -  must loop on MmLocateHandle () to see how many protocols were added
> and
> -  do the following to each FV:
> -  If the Fv has already been processed, skip it. If the Fv has not been
> -  processed then mark it as being processed, as we are about to process it.
> -  Read the Fv and add any driver in the Fv to the mDiscoveredList.The
> -  mDiscoveredList is never free'ed and contains variables that define
> -  the other states the MM driver transitions to..
> -  While you are at it read the A Priori file into memory.
> -  Place drivers in the A Priori list onto the mScheduledQueue.
> -
> -  @param  DispatchHandle  The unique handle assigned to this handler
> by SmiHandlerRegister().
> -  @param  Context         Points to an optional handler context
> which was specified when the handler was registered.
> -  @param  CommBuffer      A pointer to a collection of data in
> memory that will
> -                          be conveyed from a non-MM environment
> into an MM environment.
> -  @param  CommBufferSize  The size of the CommBuffer.
> -
> -  @return Status Code
> -
> -**/
> -EFI_STATUS
> -EFIAPI
> -MmDriverDispatchHandler (
> -  IN     EFI_HANDLE  DispatchHandle,
> -  IN     CONST VOID  *Context,        OPTIONAL
> -  IN OUT VOID        *CommBuffer,     OPTIONAL
> -  IN OUT UINTN       *CommBufferSize  OPTIONAL
> -  )
> -{
> -  EFI_STATUS                            Status;
> -
> -  DEBUG ((DEBUG_INFO, "MmDriverDispatchHandler\n"));
> -
> -  //
> -  // Execute the MM Dispatcher on any newly discovered FVs and
> previously
> -  // discovered MM drivers that have been discovered but not dispatched.
> -  //
> -  Status = MmDispatcher ();
> -
> -  //
> -  // Check to see if CommBuffer and CommBufferSize are valid
> -  //
> -  if (CommBuffer != NULL && CommBufferSize != NULL) {
> -    if (*CommBufferSize > 0) {
> -      if (Status == EFI_NOT_READY) {
> -        //
> -        // If a the MM Core Entry Point was just registered, then set flag
> to
> -        // request the MM Dispatcher to be restarted.
> -        //
> -        *(UINT8 *)CommBuffer =
> COMM_BUFFER_MM_DISPATCH_RESTART;
> -      } else if (!EFI_ERROR (Status)) {
> -        //
> -        // Set the flag to show that the MM Dispatcher executed without
> errors
> -        //
> -        *(UINT8 *)CommBuffer =
> COMM_BUFFER_MM_DISPATCH_SUCCESS;
> -      } else {
> -        //
> -        // Set the flag to show that the MM Dispatcher encountered an
> error
> -        //
> -        *(UINT8 *)CommBuffer =
> COMM_BUFFER_MM_DISPATCH_ERROR;
> -      }
> -    }
> -  }
> -
> -  return EFI_SUCCESS;
> -}
> -
>  /**
>    This function is the main entry point for an MM handler dispatch
>    or communicate-based callback.
> diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.c
> b/StandaloneMmPkg/Core/StandaloneMmCore.c
> index 74432320bfc7..ec53b8d8bec4 100644
> --- a/StandaloneMmPkg/Core/StandaloneMmCore.c
> +++ b/StandaloneMmPkg/Core/StandaloneMmCore.c
> @@ -100,7 +100,6 @@ BOOLEAN  mInLegacyBoot = FALSE;
>  //
>  MM_CORE_MMI_HANDLERS  mMmCoreMmiHandlers[] = {
>    { MmFvDispatchHandler,     &gMmFvDispatchGuid,
> NULL, TRUE  },
> -  { MmDriverDispatchHandler, &gEfiEventDxeDispatchGuid,
> NULL, TRUE  },
>    { MmReadyToLockHandler,    &gEfiDxeMmReadyToLockProtocolGuid,
> NULL, TRUE  },
>    { MmEndOfDxeHandler,       &gEfiEndOfDxeEventGroupGuid,
> NULL, FALSE },
>    { MmLegacyBootHandler,     &gEfiEventLegacyBootGuid,
> NULL, FALSE },
> --
> 2.20.1