From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by groups.io with SMTP; Tue, 17 Sep 2019 12:50:44 -0700 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 1843C10F09; Tue, 17 Sep 2019 19:50:44 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-37.rdu2.redhat.com [10.10.120.37]) by smtp.corp.redhat.com (Postfix) with ESMTP id D2765600C8; Tue, 17 Sep 2019 19:50:42 +0000 (UTC) From: "Laszlo Ersek" To: edk2-devel-groups-io Cc: Achin Gupta , Jiewen Yao , Supreeth Venkatesh Subject: [PATCH 33/35] StandaloneMmPkg/Core: stop abusing EFI_HANDLE for FwVolHeader tracking Date: Tue, 17 Sep 2019 21:49:33 +0200 Message-Id: <20190917194935.24322-34-lersek@redhat.com> In-Reply-To: <20190917194935.24322-1-lersek@redhat.com> References: <20190917194935.24322-1-lersek@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Tue, 17 Sep 2019 19:50:44 +0000 (UTC) Content-Transfer-Encoding: quoted-printable 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 whic= h 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 faithfull= y 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/Co= re/StandaloneMmCore.h index dcf91bc5e916..23ddbe169faf 100644 --- a/StandaloneMmPkg/Core/StandaloneMmCore.h +++ b/StandaloneMmPkg/Core/StandaloneMmCore.h @@ -67,7 +67,7 @@ typedef struct { =20 LIST_ENTRY ScheduledLink; // mScheduledQueue =20 - EFI_HANDLE FvHandle; + EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader; EFI_GUID FileName; VOID *Pe32Data; UINTN Pe32DataSize; diff --git a/StandaloneMmPkg/Core/Dispatcher.c b/StandaloneMmPkg/Core/Dis= patcher.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. I= f 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. =20 Step #2 - Dispatch. Remove driver from the mScheduledQueue and load an= d @@ -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') =20 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; =20 // // Function Prototypes @@ -86,9 +86,10 @@ LIST_ENTRY mDiscoveredList =3D INITIALIZE_LIST_HEAD_V= ARIABLE (mDiscoveredList); LIST_ENTRY mScheduledQueue =3D INITIALIZE_LIST_HEAD_VARIABLE (mSchedule= dQueue); =20 // -// List of handles who's Fv's have been parsed and added to the mFwDrive= rList. +// List of firmware volume headers whose containing firmware volumes hav= e been +// parsed and added to the mFwDriverList. // -LIST_ENTRY mFvHandleList =3D INITIALIZE_LIST_HEAD_VARIABLE (mFvHandleLi= st); +LIST_ENTRY mFwVolList =3D INITIALIZE_LIST_HEAD_VARIABLE (mFwVolList); =20 // // Flag for the MM Dispacher. TRUE if dispatcher is execuing. @@ -769,26 +770,30 @@ MmInsertOnScheduledQueueWhileProcessingBeforeAndAft= er ( } =20 /** - Return TRUE if the Fv has been processed, FALSE if not. + Return TRUE if the firmware volume has been processed, FALSE if not. =20 - @param FvHandle The handle of a FV that's being tested + @param FwVolHeader The header of the firmware volume that's= being + tested. =20 - @retval TRUE Fv protocol on FvHandle has been process= ed - @retval FALSE Fv protocol on FvHandle has not yet been - processed + @retval TRUE The firmware volume denoted by FwVolHead= er has + been processed + @retval FALSE The firmware volume denoted by FwVolHead= er has + not yet been processed =20 **/ BOOLEAN FvHasBeenProcessed ( - IN EFI_HANDLE FvHandle + IN EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader ) { LIST_ENTRY *Link; - KNOWN_HANDLE *KnownHandle; + KNOWN_FWVOL *KnownFwVol; =20 - for (Link =3D mFvHandleList.ForwardLink; Link !=3D &mFvHandleList; Lin= k =3D Link->ForwardLink) { - KnownHandle =3D CR (Link, KNOWN_HANDLE, Link, KNOWN_HANDLE_SIGNATURE= ); - if (KnownHandle->Handle =3D=3D FvHandle) { + for (Link =3D mFwVolList.ForwardLink; + Link !=3D &mFwVolList; + Link =3D Link->ForwardLink) { + KnownFwVol =3D CR (Link, KNOWN_FWVOL, Link, KNOWN_FWVOL_SIGNATURE); + if (KnownFwVol->FwVolHeader =3D=3D FwVolHeader) { return TRUE; } } @@ -796,28 +801,29 @@ FvHasBeenProcessed ( } =20 /** - Remember that Fv protocol on FvHandle has had it's drivers placed on t= he - mDiscoveredList. This fucntion adds entries on the mFvHandleList. Item= s are - never removed/freed from the mFvHandleList. + Remember that the firmware volume denoted by FwVolHeader has had its d= rivers + placed on mDiscoveredList. This function adds entries to mFwVolList. I= tems + are never removed/freed from mFwVolList. =20 - @param FvHandle The handle of a FV that has been process= ed + @param FwVolHeader The header of the firmware volume that's= being + processed. =20 **/ VOID FvIsBeingProcesssed ( - IN EFI_HANDLE FvHandle + IN EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader ) { - KNOWN_HANDLE *KnownHandle; + KNOWN_FWVOL *KnownFwVol; =20 - DEBUG ((DEBUG_INFO, "FvIsBeingProcesssed - 0x%08x\n", FvHandle)); + DEBUG ((DEBUG_INFO, "FvIsBeingProcesssed - 0x%08x\n", KnownFwVol)); =20 - KnownHandle =3D AllocatePool (sizeof (KNOWN_HANDLE)); - ASSERT (KnownHandle !=3D NULL); + KnownFwVol =3D AllocatePool (sizeof (KNOWN_FWVOL)); + ASSERT (KnownFwVol !=3D NULL); =20 - KnownHandle->Signature =3D KNOWN_HANDLE_SIGNATURE; - KnownHandle->Handle =3D FvHandle; - InsertTailList (&mFvHandleList, &KnownHandle->Link); + KnownFwVol->Signature =3D KNOWN_FWVOL_SIGNATURE; + KnownFwVol->FwVolHeader =3D FwVolHeader; + InsertTailList (&mFwVolList, &KnownFwVol->Link); } =20 /** @@ -842,12 +848,12 @@ FvIsBeingProcesssed ( **/ EFI_STATUS MmAddToDriverList ( - IN EFI_HANDLE FvHandle, - IN VOID *Pe32Data, - IN UINTN Pe32DataSize, - IN VOID *Depex, - IN UINTN DepexSize, - IN EFI_GUID *DriverName + IN EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader, + IN VOID *Pe32Data, + IN UINTN Pe32DataSize, + IN VOID *Depex, + IN UINTN DepexSize, + IN EFI_GUID *DriverName ) { EFI_MM_DRIVER_ENTRY *DriverEntry; @@ -863,7 +869,7 @@ MmAddToDriverList ( =20 DriverEntry->Signature =3D EFI_MM_DRIVER_ENTRY_SIGNATURE; CopyGuid (&DriverEntry->FileName, DriverName); - DriverEntry->FvHandle =3D FvHandle; + DriverEntry->FwVolHeader =3D FwVolHeader; DriverEntry->Pe32Data =3D Pe32Data; DriverEntry->Pe32DataSize =3D Pe32DataSize; DriverEntry->Depex =3D Depex; diff --git a/StandaloneMmPkg/Core/FwVol.c b/StandaloneMmPkg/Core/FwVol.c index 9fe0c257a43a..99ecf4af4714 100644 --- a/StandaloneMmPkg/Core/FwVol.c +++ b/StandaloneMmPkg/Core/FwVol.c @@ -24,22 +24,22 @@ EFI_FV_FILETYPE mMmFileTypes[] =3D { =20 EFI_STATUS MmAddToDriverList ( - IN EFI_HANDLE FvHandle, - IN VOID *Pe32Data, - IN UINTN Pe32DataSize, - IN VOID *Depex, - IN UINTN DepexSize, - IN EFI_GUID *DriverName + IN EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader, + IN VOID *Pe32Data, + IN UINTN Pe32DataSize, + IN VOID *Depex, + IN UINTN DepexSize, + IN EFI_GUID *DriverName ); =20 BOOLEAN FvHasBeenProcessed ( - IN EFI_HANDLE FvHandle + IN EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader ); =20 VOID FvIsBeingProcesssed ( - IN EFI_HANDLE FvHandle + IN EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader ); =20 EFI_STATUS --=20 2.19.1.3.g30247aa5d201