Re: [edk2-devel] [PATCH 33/35] StandaloneMmPkg/Core: stop abusing EFI_HANDLE for FwVolHeader tracking
Makes sense!
Reviewed-by: Achin Gupta
On Thu, Oct 03, 2019 at 01:10:53PM +0200, Laszlo Ersek wrote:
> Pinging StandaloneMmPkg maintainers again, for reviewing this patch.
>
> Thanks
> Laszlo
>
> On 09/26/19 14:48, Laszlo Ersek wrote:
> > Achin, Jiewen, Supreeth,
> >
> > can one of you guys please review this patch?
> >
> > Thanks
> > Laszlo
> >
> > On 09/17/19 21:49, Laszlo Ersek wrote:
> >> The FvHasBeenProcessed() and FvIsBeingProcesssed() functions make sure
> >> that every firmware volume is processed only once (every driver in every
> >> firmware volume should be discovered only once). For this, the functions
> >> use a linked list.
> >>
> >> In MdeModulePkg's DXE Core and SMM Core, the key used for identifying
> >> those firmware volumes that have been processed is the EFI_HANDLE on which
> >> the DXE or SMM firmware volume protocol is installed. In the
> >> StandaloneMmPkg core however, the key is the address of the firmware
> >> volume header; that is, it has type (EFI_FIRMWARE_VOLUME_HEADER*).
> >>
> >> (EFI_FIRMWARE_VOLUME_HEADER*) has nothing to do with EFI_HANDLE.
> >> EFI_HANDLE just happens to be specified as (VOID*), and therefore the
> >> conversion between (EFI_FIRMWARE_VOLUME_HEADER*) and EFI_HANDLE is silent.
> >>
> >> (The FvHasBeenProcessed() and FvIsBeingProcesssed() functions were likely
> >> copied verbatim from MdeModulePkg's DXE Core and/or the SMM Core, and not
> >> flagged by the compiler in StandaloneMmPkg due to UEFI regrettably
> >> specifying EFI_HANDLE as (VOID*), thereby enabling the above implicit
> >> conversion.)
> >>
> >> We should not exploit this circumstance. Represent the key type faithfully
> >> instead.
> >>
> >> This is a semantic fix; there is no change in operation.
> >>
> >> Cc: Achin Gupta
> >> Cc: Jiewen Yao
> >> Cc: Supreeth Venkatesh
> >> Signed-off-by: Laszlo Ersek
> >> ---
> >>
> >> Notes:
> >> build-tested only
> >>
> >> StandaloneMmPkg/Core/StandaloneMmCore.h | 2 +-
> >> StandaloneMmPkg/Core/Dispatcher.c | 80 +++-
> >> StandaloneMmPkg/Core/FwVol.c| 16 ++--
> >> 3 files changed, 52 insertions(+), 46 deletions(-)
> >>
> >> diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.h
> >> b/StandaloneMmPkg/Core/StandaloneMmCore.h
> >> index dcf91bc5e916..23ddbe169faf 100644
> >> --- a/StandaloneMmPkg/Core/StandaloneMmCore.h
> >> +++ b/StandaloneMmPkg/Core/StandaloneMmCore.h
> >> @@ -67,7 +67,7 @@ typedef struct {
> >>
> >>LIST_ENTRY ScheduledLink;// mScheduledQueue
> >>
> >> - EFI_HANDLE FvHandle;
> >> + EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader;
> >>EFI_GUIDFileName;
> >>VOID*Pe32Data;
> >>UINTN Pe32DataSize;
> >> diff --git a/StandaloneMmPkg/Core/Dispatcher.c
> >> b/StandaloneMmPkg/Core/Dispatcher.c
> >> index 3788389f95ed..9853445a64a1 100644
> >> --- a/StandaloneMmPkg/Core/Dispatcher.c
> >> +++ b/StandaloneMmPkg/Core/Dispatcher.c
> >> @@ -5,7 +5,7 @@
> >> is added to the mDiscoveredList. The Before, and After Depex
> >> are
> >> pre-processed as drivers are added to the mDiscoveredList. If
> >> an Apriori
> >> file exists in the FV those drivers are addeded to the
> >> -mScheduledQueue. The mFvHandleList is used to make sure a
> >> +mScheduledQueue. The mFwVolList is used to make sure a
> >> FV is only processed once.
> >>
> >>Step #2 - Dispatch. Remove driver from the mScheduledQueue and load and
> >> @@ -40,13 +40,13 @@
> >> //
> >> // MM Dispatcher Data structures
> >> //
> >> -#define KNOWN_HANDLE_SIGNATURE SIGNATURE_32('k','n','o','w')
> >> +#define KNOWN_FWVOL_SIGNATURE SIGNATURE_32('k','n','o','w')
> >>
> >> typedef struct {
> >> - UINTN Signature;
> >> - LIST_ENTRY Link; // mFvHandleList
> >> - EFI_HANDLE Handle;
> >> -} KNOWN_HANDLE;
> >> + UINTN Signature;
> >> + LIST_ENTRY Link; // mFwVolList
> >> + EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader;
> >> +} KNOWN_FWVOL;
> >>
> >> //
> >> // Function Prototypes
> >> @@ -86,9 +86,10 @@ LIST_ENTRY mDiscoveredList =
> >> INITIALIZE_LIST_HEAD_VARIABLE (mDiscoveredList);
> >> LIST_ENTRY mScheduledQueue = INITIALIZE_LIST_HEAD_VARIABLE
> >> (mScheduledQueue);
> >>
> >> //
> >> -// List of handles who's Fv's have been parsed and added to the
> >> mFwDriverList.
> >> +// List of firmware volume headers whose containing firmware volumes have
> >> been
> >> +// parsed and added to the mFwDriverList.
> >> //
> >> -LIST_ENTRY mFvHandleList = INITIALIZE_LIST_HEAD_VARIABLE (mFvHandleList);
> >> +LIST_ENTRY mFwVolList = INITIALIZE_LIST_HEAD_VARIABLE (mFwVolList);
> >>
> >> //
> >> // Flag for the MM Dispacher. TRUE if dispatcher is execuing.
> >> @@ -769,26 +770,30 @@
> >> MmInsertOnScheduledQueueWhileProcessingBeforeAndAfter (
> >> }
> >>
> >>
Re: [edk2-devel] [PATCH 33/35] StandaloneMmPkg/Core: stop abusing EFI_HANDLE for FwVolHeader tracking
Hi Laszlo,
Apologies for not getting back earlier as I was travelling. I will have a look
and get back by tomorrow.
cheers,
Achin
On Thu, Oct 03, 2019 at 01:10:53PM +0200, Laszlo Ersek wrote:
> Pinging StandaloneMmPkg maintainers again, for reviewing this patch.
>
> Thanks
> Laszlo
>
> On 09/26/19 14:48, Laszlo Ersek wrote:
> > Achin, Jiewen, Supreeth,
> >
> > can one of you guys please review this patch?
> >
> > Thanks
> > Laszlo
> >
> > On 09/17/19 21:49, Laszlo Ersek wrote:
> >> The FvHasBeenProcessed() and FvIsBeingProcesssed() functions make sure
> >> that every firmware volume is processed only once (every driver in every
> >> firmware volume should be discovered only once). For this, the functions
> >> use a linked list.
> >>
> >> In MdeModulePkg's DXE Core and SMM Core, the key used for identifying
> >> those firmware volumes that have been processed is the EFI_HANDLE on which
> >> the DXE or SMM firmware volume protocol is installed. In the
> >> StandaloneMmPkg core however, the key is the address of the firmware
> >> volume header; that is, it has type (EFI_FIRMWARE_VOLUME_HEADER*).
> >>
> >> (EFI_FIRMWARE_VOLUME_HEADER*) has nothing to do with EFI_HANDLE.
> >> EFI_HANDLE just happens to be specified as (VOID*), and therefore the
> >> conversion between (EFI_FIRMWARE_VOLUME_HEADER*) and EFI_HANDLE is silent.
> >>
> >> (The FvHasBeenProcessed() and FvIsBeingProcesssed() functions were likely
> >> copied verbatim from MdeModulePkg's DXE Core and/or the SMM Core, and not
> >> flagged by the compiler in StandaloneMmPkg due to UEFI regrettably
> >> specifying EFI_HANDLE as (VOID*), thereby enabling the above implicit
> >> conversion.)
> >>
> >> We should not exploit this circumstance. Represent the key type faithfully
> >> instead.
> >>
> >> This is a semantic fix; there is no change in operation.
> >>
> >> Cc: Achin Gupta
> >> Cc: Jiewen Yao
> >> Cc: Supreeth Venkatesh
> >> Signed-off-by: Laszlo Ersek
> >> ---
> >>
> >> Notes:
> >> build-tested only
> >>
> >> StandaloneMmPkg/Core/StandaloneMmCore.h | 2 +-
> >> StandaloneMmPkg/Core/Dispatcher.c | 80 +++-
> >> StandaloneMmPkg/Core/FwVol.c| 16 ++--
> >> 3 files changed, 52 insertions(+), 46 deletions(-)
> >>
> >> diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.h
> >> b/StandaloneMmPkg/Core/StandaloneMmCore.h
> >> index dcf91bc5e916..23ddbe169faf 100644
> >> --- a/StandaloneMmPkg/Core/StandaloneMmCore.h
> >> +++ b/StandaloneMmPkg/Core/StandaloneMmCore.h
> >> @@ -67,7 +67,7 @@ typedef struct {
> >>
> >>LIST_ENTRY ScheduledLink;// mScheduledQueue
> >>
> >> - EFI_HANDLE FvHandle;
> >> + EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader;
> >>EFI_GUIDFileName;
> >>VOID*Pe32Data;
> >>UINTN Pe32DataSize;
> >> diff --git a/StandaloneMmPkg/Core/Dispatcher.c
> >> b/StandaloneMmPkg/Core/Dispatcher.c
> >> index 3788389f95ed..9853445a64a1 100644
> >> --- a/StandaloneMmPkg/Core/Dispatcher.c
> >> +++ b/StandaloneMmPkg/Core/Dispatcher.c
> >> @@ -5,7 +5,7 @@
> >> is added to the mDiscoveredList. The Before, and After Depex
> >> are
> >> pre-processed as drivers are added to the mDiscoveredList. If
> >> an Apriori
> >> file exists in the FV those drivers are addeded to the
> >> -mScheduledQueue. The mFvHandleList is used to make sure a
> >> +mScheduledQueue. The mFwVolList is used to make sure a
> >> FV is only processed once.
> >>
> >>Step #2 - Dispatch. Remove driver from the mScheduledQueue and load and
> >> @@ -40,13 +40,13 @@
> >> //
> >> // MM Dispatcher Data structures
> >> //
> >> -#define KNOWN_HANDLE_SIGNATURE SIGNATURE_32('k','n','o','w')
> >> +#define KNOWN_FWVOL_SIGNATURE SIGNATURE_32('k','n','o','w')
> >>
> >> typedef struct {
> >> - UINTN Signature;
> >> - LIST_ENTRY Link; // mFvHandleList
> >> - EFI_HANDLE Handle;
> >> -} KNOWN_HANDLE;
> >> + UINTN Signature;
> >> + LIST_ENTRY Link; // mFwVolList
> >> + EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader;
> >> +} KNOWN_FWVOL;
> >>
> >> //
> >> // Function Prototypes
> >> @@ -86,9 +86,10 @@ LIST_ENTRY mDiscoveredList =
> >> INITIALIZE_LIST_HEAD_VARIABLE (mDiscoveredList);
> >> LIST_ENTRY mScheduledQueue = INITIALIZE_LIST_HEAD_VARIABLE
> >> (mScheduledQueue);
> >>
> >> //
> >> -// List of handles who's Fv's have been parsed and added to the
> >> mFwDriverList.
> >> +// List of firmware volume headers whose containing firmware volumes have
> >> been
> >> +// parsed and added to the mFwDriverList.
> >> //
> >> -LIST_ENTRY mFvHandleList = INITIALIZE_LIST_HEAD_VARIABLE (mFvHandleList);
> >> +LIST_ENTRY mFwVolList = INITIALIZE_LIST_HEAD_VARIABLE (mFwVolList);
> >>
> >> //
> >> // Flag for the MM Dispacher. TRUE if dispatcher is execuing.
> >> @@ -769,26
Re: [edk2-devel] [PATCH 33/35] StandaloneMmPkg/Core: stop abusing EFI_HANDLE for FwVolHeader tracking
Pinging StandaloneMmPkg maintainers again, for reviewing this patch.
Thanks
Laszlo
On 09/26/19 14:48, Laszlo Ersek wrote:
> Achin, Jiewen, Supreeth,
>
> can one of you guys please review this patch?
>
> Thanks
> Laszlo
>
> On 09/17/19 21:49, Laszlo Ersek wrote:
>> The FvHasBeenProcessed() and FvIsBeingProcesssed() functions make sure
>> that every firmware volume is processed only once (every driver in every
>> firmware volume should be discovered only once). For this, the functions
>> use a linked list.
>>
>> In MdeModulePkg's DXE Core and SMM Core, the key used for identifying
>> those firmware volumes that have been processed is the EFI_HANDLE on which
>> the DXE or SMM firmware volume protocol is installed. In the
>> StandaloneMmPkg core however, the key is the address of the firmware
>> volume header; that is, it has type (EFI_FIRMWARE_VOLUME_HEADER*).
>>
>> (EFI_FIRMWARE_VOLUME_HEADER*) has nothing to do with EFI_HANDLE.
>> EFI_HANDLE just happens to be specified as (VOID*), and therefore the
>> conversion between (EFI_FIRMWARE_VOLUME_HEADER*) and EFI_HANDLE is silent.
>>
>> (The FvHasBeenProcessed() and FvIsBeingProcesssed() functions were likely
>> copied verbatim from MdeModulePkg's DXE Core and/or the SMM Core, and not
>> flagged by the compiler in StandaloneMmPkg due to UEFI regrettably
>> specifying EFI_HANDLE as (VOID*), thereby enabling the above implicit
>> conversion.)
>>
>> We should not exploit this circumstance. Represent the key type faithfully
>> instead.
>>
>> This is a semantic fix; there is no change in operation.
>>
>> Cc: Achin Gupta
>> Cc: Jiewen Yao
>> Cc: Supreeth Venkatesh
>> Signed-off-by: Laszlo Ersek
>> ---
>>
>> Notes:
>> build-tested only
>>
>> StandaloneMmPkg/Core/StandaloneMmCore.h | 2 +-
>> StandaloneMmPkg/Core/Dispatcher.c | 80 +++-
>> StandaloneMmPkg/Core/FwVol.c| 16 ++--
>> 3 files changed, 52 insertions(+), 46 deletions(-)
>>
>> diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.h
>> b/StandaloneMmPkg/Core/StandaloneMmCore.h
>> index dcf91bc5e916..23ddbe169faf 100644
>> --- a/StandaloneMmPkg/Core/StandaloneMmCore.h
>> +++ b/StandaloneMmPkg/Core/StandaloneMmCore.h
>> @@ -67,7 +67,7 @@ typedef struct {
>>
>>LIST_ENTRY ScheduledLink;// mScheduledQueue
>>
>> - EFI_HANDLE FvHandle;
>> + EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader;
>>EFI_GUIDFileName;
>>VOID*Pe32Data;
>>UINTN Pe32DataSize;
>> diff --git a/StandaloneMmPkg/Core/Dispatcher.c
>> b/StandaloneMmPkg/Core/Dispatcher.c
>> index 3788389f95ed..9853445a64a1 100644
>> --- a/StandaloneMmPkg/Core/Dispatcher.c
>> +++ b/StandaloneMmPkg/Core/Dispatcher.c
>> @@ -5,7 +5,7 @@
>> is added to the mDiscoveredList. The Before, and After Depex are
>> pre-processed as drivers are added to the mDiscoveredList. If
>> an Apriori
>> file exists in the FV those drivers are addeded to the
>> -mScheduledQueue. The mFvHandleList is used to make sure a
>> +mScheduledQueue. The mFwVolList is used to make sure a
>> FV is only processed once.
>>
>>Step #2 - Dispatch. Remove driver from the mScheduledQueue and load and
>> @@ -40,13 +40,13 @@
>> //
>> // MM Dispatcher Data structures
>> //
>> -#define KNOWN_HANDLE_SIGNATURE SIGNATURE_32('k','n','o','w')
>> +#define KNOWN_FWVOL_SIGNATURE SIGNATURE_32('k','n','o','w')
>>
>> typedef struct {
>> - UINTN Signature;
>> - LIST_ENTRY Link; // mFvHandleList
>> - EFI_HANDLE Handle;
>> -} KNOWN_HANDLE;
>> + UINTN Signature;
>> + LIST_ENTRY Link; // mFwVolList
>> + EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader;
>> +} KNOWN_FWVOL;
>>
>> //
>> // Function Prototypes
>> @@ -86,9 +86,10 @@ LIST_ENTRY mDiscoveredList =
>> INITIALIZE_LIST_HEAD_VARIABLE (mDiscoveredList);
>> LIST_ENTRY mScheduledQueue = INITIALIZE_LIST_HEAD_VARIABLE
>> (mScheduledQueue);
>>
>> //
>> -// List of handles who's Fv's have been parsed and added to the
>> mFwDriverList.
>> +// List of firmware volume headers whose containing firmware volumes have
>> been
>> +// parsed and added to the mFwDriverList.
>> //
>> -LIST_ENTRY mFvHandleList = INITIALIZE_LIST_HEAD_VARIABLE (mFvHandleList);
>> +LIST_ENTRY mFwVolList = INITIALIZE_LIST_HEAD_VARIABLE (mFwVolList);
>>
>> //
>> // Flag for the MM Dispacher. TRUE if dispatcher is execuing.
>> @@ -769,26 +770,30 @@ MmInsertOnScheduledQueueWhileProcessingBeforeAndAfter (
>> }
>>
>> /**
>> - Return TRUE if the Fv has been processed, FALSE if not.
>> + Return TRUE if the firmware volume has been processed, FALSE if not.
>>
>> - @param FvHandle The handle of a FV that's being tested
>> + @param FwVolHeader The header of the firmware volume that's
>> being
>> +
Re: [edk2-devel] [PATCH 33/35] StandaloneMmPkg/Core: stop abusing EFI_HANDLE for FwVolHeader tracking
Achin, Jiewen, Supreeth,
can one of you guys please review this patch?
Thanks
Laszlo
On 09/17/19 21:49, Laszlo Ersek wrote:
> The FvHasBeenProcessed() and FvIsBeingProcesssed() functions make sure
> that every firmware volume is processed only once (every driver in every
> firmware volume should be discovered only once). For this, the functions
> use a linked list.
>
> In MdeModulePkg's DXE Core and SMM Core, the key used for identifying
> those firmware volumes that have been processed is the EFI_HANDLE on which
> the DXE or SMM firmware volume protocol is installed. In the
> StandaloneMmPkg core however, the key is the address of the firmware
> volume header; that is, it has type (EFI_FIRMWARE_VOLUME_HEADER*).
>
> (EFI_FIRMWARE_VOLUME_HEADER*) has nothing to do with EFI_HANDLE.
> EFI_HANDLE just happens to be specified as (VOID*), and therefore the
> conversion between (EFI_FIRMWARE_VOLUME_HEADER*) and EFI_HANDLE is silent.
>
> (The FvHasBeenProcessed() and FvIsBeingProcesssed() functions were likely
> copied verbatim from MdeModulePkg's DXE Core and/or the SMM Core, and not
> flagged by the compiler in StandaloneMmPkg due to UEFI regrettably
> specifying EFI_HANDLE as (VOID*), thereby enabling the above implicit
> conversion.)
>
> We should not exploit this circumstance. Represent the key type faithfully
> instead.
>
> This is a semantic fix; there is no change in operation.
>
> Cc: Achin Gupta
> Cc: Jiewen Yao
> Cc: Supreeth Venkatesh
> Signed-off-by: Laszlo Ersek
> ---
>
> Notes:
> build-tested only
>
> StandaloneMmPkg/Core/StandaloneMmCore.h | 2 +-
> StandaloneMmPkg/Core/Dispatcher.c | 80 +++-
> StandaloneMmPkg/Core/FwVol.c| 16 ++--
> 3 files changed, 52 insertions(+), 46 deletions(-)
>
> diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.h
> b/StandaloneMmPkg/Core/StandaloneMmCore.h
> index dcf91bc5e916..23ddbe169faf 100644
> --- a/StandaloneMmPkg/Core/StandaloneMmCore.h
> +++ b/StandaloneMmPkg/Core/StandaloneMmCore.h
> @@ -67,7 +67,7 @@ typedef struct {
>
>LIST_ENTRY ScheduledLink;// mScheduledQueue
>
> - EFI_HANDLE FvHandle;
> + EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader;
>EFI_GUIDFileName;
>VOID*Pe32Data;
>UINTN Pe32DataSize;
> diff --git a/StandaloneMmPkg/Core/Dispatcher.c
> b/StandaloneMmPkg/Core/Dispatcher.c
> index 3788389f95ed..9853445a64a1 100644
> --- a/StandaloneMmPkg/Core/Dispatcher.c
> +++ b/StandaloneMmPkg/Core/Dispatcher.c
> @@ -5,7 +5,7 @@
> is added to the mDiscoveredList. The Before, and After Depex are
> pre-processed as drivers are added to the mDiscoveredList. If an
> Apriori
> file exists in the FV those drivers are addeded to the
> -mScheduledQueue. The mFvHandleList is used to make sure a
> +mScheduledQueue. The mFwVolList is used to make sure a
> FV is only processed once.
>
>Step #2 - Dispatch. Remove driver from the mScheduledQueue and load and
> @@ -40,13 +40,13 @@
> //
> // MM Dispatcher Data structures
> //
> -#define KNOWN_HANDLE_SIGNATURE SIGNATURE_32('k','n','o','w')
> +#define KNOWN_FWVOL_SIGNATURE SIGNATURE_32('k','n','o','w')
>
> typedef struct {
> - UINTN Signature;
> - LIST_ENTRY Link; // mFvHandleList
> - EFI_HANDLE Handle;
> -} KNOWN_HANDLE;
> + UINTN Signature;
> + LIST_ENTRY Link; // mFwVolList
> + EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader;
> +} KNOWN_FWVOL;
>
> //
> // Function Prototypes
> @@ -86,9 +86,10 @@ LIST_ENTRY mDiscoveredList =
> INITIALIZE_LIST_HEAD_VARIABLE (mDiscoveredList);
> LIST_ENTRY mScheduledQueue = INITIALIZE_LIST_HEAD_VARIABLE
> (mScheduledQueue);
>
> //
> -// List of handles who's Fv's have been parsed and added to the
> mFwDriverList.
> +// List of firmware volume headers whose containing firmware volumes have
> been
> +// parsed and added to the mFwDriverList.
> //
> -LIST_ENTRY mFvHandleList = INITIALIZE_LIST_HEAD_VARIABLE (mFvHandleList);
> +LIST_ENTRY mFwVolList = INITIALIZE_LIST_HEAD_VARIABLE (mFwVolList);
>
> //
> // Flag for the MM Dispacher. TRUE if dispatcher is execuing.
> @@ -769,26 +770,30 @@ MmInsertOnScheduledQueueWhileProcessingBeforeAndAfter (
> }
>
> /**
> - Return TRUE if the Fv has been processed, FALSE if not.
> + Return TRUE if the firmware volume has been processed, FALSE if not.
>
> - @param FvHandle The handle of a FV that's being tested
> + @param FwVolHeader The header of the firmware volume that's
> being
> +tested.
>
> - @retval TRUE Fv protocol on FvHandle has been processed
> - @retval FALSE Fv protocol on FvHandle has not yet been
> -processed
> + @retval TRUE
[edk2-devel] [PATCH 33/35] StandaloneMmPkg/Core: stop abusing EFI_HANDLE for FwVolHeader tracking
The FvHasBeenProcessed() and FvIsBeingProcesssed() functions make sure
that every firmware volume is processed only once (every driver in every
firmware volume should be discovered only once). For this, the functions
use a linked list.
In MdeModulePkg's DXE Core and SMM Core, the key used for identifying
those firmware volumes that have been processed is the EFI_HANDLE on which
the DXE or SMM firmware volume protocol is installed. In the
StandaloneMmPkg core however, the key is the address of the firmware
volume header; that is, it has type (EFI_FIRMWARE_VOLUME_HEADER*).
(EFI_FIRMWARE_VOLUME_HEADER*) has nothing to do with EFI_HANDLE.
EFI_HANDLE just happens to be specified as (VOID*), and therefore the
conversion between (EFI_FIRMWARE_VOLUME_HEADER*) and EFI_HANDLE is silent.
(The FvHasBeenProcessed() and FvIsBeingProcesssed() functions were likely
copied verbatim from MdeModulePkg's DXE Core and/or the SMM Core, and not
flagged by the compiler in StandaloneMmPkg due to UEFI regrettably
specifying EFI_HANDLE as (VOID*), thereby enabling the above implicit
conversion.)
We should not exploit this circumstance. Represent the key type faithfully
instead.
This is a semantic fix; there is no change in operation.
Cc: Achin Gupta
Cc: Jiewen Yao
Cc: Supreeth Venkatesh
Signed-off-by: Laszlo Ersek
---
Notes:
build-tested only
StandaloneMmPkg/Core/StandaloneMmCore.h | 2 +-
StandaloneMmPkg/Core/Dispatcher.c | 80 +++-
StandaloneMmPkg/Core/FwVol.c| 16 ++--
3 files changed, 52 insertions(+), 46 deletions(-)
diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.h
b/StandaloneMmPkg/Core/StandaloneMmCore.h
index dcf91bc5e916..23ddbe169faf 100644
--- a/StandaloneMmPkg/Core/StandaloneMmCore.h
+++ b/StandaloneMmPkg/Core/StandaloneMmCore.h
@@ -67,7 +67,7 @@ typedef struct {
LIST_ENTRY ScheduledLink;// mScheduledQueue
- EFI_HANDLE FvHandle;
+ EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader;
EFI_GUIDFileName;
VOID*Pe32Data;
UINTN Pe32DataSize;
diff --git a/StandaloneMmPkg/Core/Dispatcher.c
b/StandaloneMmPkg/Core/Dispatcher.c
index 3788389f95ed..9853445a64a1 100644
--- a/StandaloneMmPkg/Core/Dispatcher.c
+++ b/StandaloneMmPkg/Core/Dispatcher.c
@@ -5,7 +5,7 @@
is added to the mDiscoveredList. The Before, and After Depex are
pre-processed as drivers are added to the mDiscoveredList. If an
Apriori
file exists in the FV those drivers are addeded to the
-mScheduledQueue. The mFvHandleList is used to make sure a
+mScheduledQueue. The mFwVolList is used to make sure a
FV is only processed once.
Step #2 - Dispatch. Remove driver from the mScheduledQueue and load and
@@ -40,13 +40,13 @@
//
// MM Dispatcher Data structures
//
-#define KNOWN_HANDLE_SIGNATURE SIGNATURE_32('k','n','o','w')
+#define KNOWN_FWVOL_SIGNATURE SIGNATURE_32('k','n','o','w')
typedef struct {
- UINTN Signature;
- LIST_ENTRY Link; // mFvHandleList
- EFI_HANDLE Handle;
-} KNOWN_HANDLE;
+ UINTN Signature;
+ LIST_ENTRY Link; // mFwVolList
+ EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader;
+} KNOWN_FWVOL;
//
// Function Prototypes
@@ -86,9 +86,10 @@ LIST_ENTRY mDiscoveredList = INITIALIZE_LIST_HEAD_VARIABLE
(mDiscoveredList);
LIST_ENTRY mScheduledQueue = INITIALIZE_LIST_HEAD_VARIABLE (mScheduledQueue);
//
-// List of handles who's Fv's have been parsed and added to the mFwDriverList.
+// List of firmware volume headers whose containing firmware volumes have been
+// parsed and added to the mFwDriverList.
//
-LIST_ENTRY mFvHandleList = INITIALIZE_LIST_HEAD_VARIABLE (mFvHandleList);
+LIST_ENTRY mFwVolList = INITIALIZE_LIST_HEAD_VARIABLE (mFwVolList);
//
// Flag for the MM Dispacher. TRUE if dispatcher is execuing.
@@ -769,26 +770,30 @@ MmInsertOnScheduledQueueWhileProcessingBeforeAndAfter (
}
/**
- Return TRUE if the Fv has been processed, FALSE if not.
+ Return TRUE if the firmware volume has been processed, FALSE if not.
- @param FvHandle The handle of a FV that's being tested
+ @param FwVolHeader The header of the firmware volume that's being
+tested.
- @retval TRUE Fv protocol on FvHandle has been processed
- @retval FALSE Fv protocol on FvHandle has not yet been
-processed
+ @retval TRUE The firmware volume denoted by FwVolHeader has
+been processed
+ @retval FALSE The firmware volume denoted by FwVolHeader has
+not yet been processed
**/
BOOLEAN
FvHasBeenProcessed (
- IN EFI_HANDLE FvHandle
+ IN EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader
)
{
LIST_ENTRY
