[edk2,07/10] StandaloneMmPkg/Core: dispatch all drivers at init time

Message ID 20190305133248.4828-8-ard.biesheuvel@linaro.org
State Accepted
Commit 84249babd7033dc65976b763419074e89d88e170
Headers show
Series
  • StandaloneMmPkg, ArmPkg: cleanups and improvements
Related show

Commit Message

Ard Biesheuvel March 5, 2019, 1:32 p.m.
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(-)

-- 
2.20.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Yao, Jiewen March 5, 2019, 3:51 p.m. | #1
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


_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Achin Gupta March 6, 2019, 4:56 p.m. | #2
Reviewed-by: achin.gupta@arm.com


On Tue, Mar 05, 2019 at 02:32:45PM +0100, Ard Biesheuvel wrote:
> 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

>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patch

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 },