From: "Laszlo Ersek" <lersek@redhat.com>
To: devel@edk2.groups.io, jon@solid-run.com
Cc: Hao A Wu <hao.a.wu@intel.com>,
Liming Gao <gaoliming@byosoft.com.cn>,
"Ard Biesheuvel (TianoCore)" <ardb+tianocore@kernel.org>,
"Leif Lindholm (Nuvia address)" <leif@nuviainc.com>
Subject: Re: [edk2-devel] Conflicting virtual addresses causing Runtime Services issues
Date: Thu, 11 Mar 2021 23:25:24 +0100 [thread overview]
Message-ID: <4841241f-fc6d-6185-efe6-ed9a536534dd@redhat.com> (raw)
In-Reply-To: <290a35ce-9116-af00-85f4-8df1c5228680@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 7335 bytes --]
Adding Ard and Leif, comments below:
On 03/11/21 15:50, Laszlo Ersek wrote:
> On 03/11/21 10:48, Jon Nettleton wrote:
[...]
>> And this is where the pointer gets remapped again and into the MMIO
>> space of the nor flash. If I remove the calls to ConvertPointer for
>> the FvbProtocol I am still seeing those addresses getting remapped
>> but only once and runtime works as expected.
>>
>> I am seeing that in
>> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
>> &mVariableModuleGlobal->FvbInstance->* are all being converted. It
>> is possible this is a long standing bug and it just so happens that
>> our configuration has caused a conflict and exposed it.
>
> Yes, this is curious, I noticed it too yesterday, trying to see where
> the FVB protocol member function pointers were converted. I found that
> OVMF's flash driver (OvmfPkg/QemuFlashFvbServicesRuntimeDxe) didn't do
> it, but MdeModulePkg/Universal/Variable/RuntimeDxe did. That was
> certainly strange, as the variable driver is a consumer of the
> protocol (not the producer thereof), so I'd say it has no business
> poking new values into the protocol interface structure.
[...]
> ... Strangely, the other flash (FVB) driver in edk2,
> ArmPlatformPkg/Drivers/NorFlashDxe, *does* perform the conversion
> itself! See NorFlashVirtualNotifyEvent().
>
> I don't understand that. Is it possible that, with
> "ArmPlatformPkg/Drivers/NorFlashDxe" too, the conversion happens
> *twice*, but (at least) one of those mappings is "identity"?
Confirmed.
I had to write some elaborate debug patches for determining this,
because in ArmVirtQemu, I cannot produce DEBUG output from the
SetVirtualAddressMap() notification functions. So here's the approach I
took:
(1) Introduce a new GUID-ed HOB structure in MdeModulePkg. The structure
itself lives in reserved memory, but its address is exposed in a GUID-ed
HOB. The structure is named FVB_ADDRESS_LIST, and it has the following
fields:
- signature ("FVBADRLS" -- FVB Address List)
- 16 entries of:
- owner signature [what driver set this entry]
- address
- number of entries used (aka next entry to fill)
(2) In PlatformPei, allocate and initialize this structure (in reserved
memory), and expose its address via the GUID-ed HOB. Furthermore,
produce a log message with the allocation address.
(3) In NorFlashDxe, look up the structure via the GUID-ed HOB, in the
entry point function; remember the address in a global variable. In the
SetVirtualAddressMap() handler function, treat the conversion of the
"GetPhysicalAddress" FVB member function specially: via the global
variable pointer to FVB_ADDRESS_LIST in reserved memory, save both the
physical (original) and the virtual (converted) address of the
"GetPhysicalAddress" FVB member function, in new entries. As owner
signature in both entries, use "NORFLASH".
(4) In the runtime DXE variable driver, do the exact same thing, just
use a different "owner signature" -- "VARIABLE".
(5) Once the guest is up and running, run "efibootmgr --delete-timeout"
at a root prompt in the guest, deleting the existent "Timeout" UEFI
non-volatile variable, for verifying that the runtime variable (write)
service is functional.
(6) Using the log message from point (2):
> PlatformPeim: FvbAddressList @ 13FEC9000
hexdump the guest memory containing the FVB_ADDRESS_LIST, as follows:
> $ virsh qemu-monitor-command aavmf.rhel7.registered --hmp xp /268cb 0x13FEC9000
Ccomments to the right of the hexdump:
> 000000013fec9000: 'F' 'V' 'B' 'A' 'D' 'R' 'L' 'S' <- structure signature: FVBADRLS
> 000000013fec9008: 'N' 'O' 'R' 'F' 'L' 'A' 'S' 'H' <- entry[0], signature: NORFLASH
> 000000013fec9010: 'T' ' ' '\xc6' ';' '\x01' '\x00' '\x00' '\x00' <- entry[0], GetPhysicalAddress *physical*: 0x000000013bc62054
> 000000013fec9018: 'N' 'O' 'R' 'F' 'L' 'A' 'S' 'H' <- entry[1], signature: NORFLASH
> 000000013fec9020: 'T' ' ' 'N' '$' '\x00' '\x00' '\x00' '\x00' <- entry[1], GetPhysicalAddress *virtual*: 0x00000000244e2054
> 000000013fec9028: 'V' 'A' 'R' 'I' 'A' 'B' 'L' 'E' <- entry[2], signature: VARIABLE
> 000000013fec9030: 'T' ' ' 'N' '$' '\x00' '\x00' '\x00' '\x00' <- entry[2], GetPhysicalAddress *physical*: 0x00000000244e2054
> 000000013fec9038: 'V' 'A' 'R' 'I' 'A' 'B' 'L' 'E' <- entry[3], signature: VARIABLE
> 000000013fec9040: 'T' ' ' 'N' '$' '\x00' '\x00' '\x00' '\x00' <- entry[3], GetPhysicalAddress *virtual*: 0x00000000244e2054
> 000000013fec9048: '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00'
> 000000013fec9050: '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00'
> 000000013fec9058: '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00'
> 000000013fec9060: '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00'
> 000000013fec9068: '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00'
> 000000013fec9070: '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00'
> 000000013fec9078: '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00'
> 000000013fec9080: '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00'
> 000000013fec9088: '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00'
> 000000013fec9090: '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00'
> 000000013fec9098: '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00'
> 000000013fec90a0: '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00'
> 000000013fec90a8: '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00'
> 000000013fec90b0: '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00'
> 000000013fec90b8: '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00'
> 000000013fec90c0: '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00'
> 000000013fec90c8: '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00'
> 000000013fec90d0: '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00'
> 000000013fec90d8: '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00'
> 000000013fec90e0: '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00'
> 000000013fec90e8: '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00'
> 000000013fec90f0: '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00'
> 000000013fec90f8: '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00'
> 000000013fec9100: '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00'
> 000000013fec9108: '\x04' '\x00' '\x00' '\x00' <- number of entries used: 4
This shows the following:
- both NorFlashDxe and the runtime DXE variable driver converted the
FVB.GetPhysicalAddress member function,
- the NorFlashDxe driver acted first, the runtime DXE variable driver
acted second,
- when the runtime DXE variable driver "converted" the "physical"
address to virtual address, there was no change (and no crash!),
because the virtual address map passed in by the Linux kernel
apparently identity maps this area -- just as I guessed.
So we definitely have a bug (only Linux's page tables save us from the
crash); now the question is:
Which driver is wrong to even attempt the conversion of the FVB member
functions?
The answer must be documented somewhere highly visible.
Debug patches attached, for the record (based on commit edd46cd407ea).
Thanks
Laszlo
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-MdeModulePkg-Guid-introduce-FVB_ADDRESS_LIST.patch --]
[-- Type: text/x-patch; name="0001-MdeModulePkg-Guid-introduce-FVB_ADDRESS_LIST.patch", Size: 3391 bytes --]
From 6670b74f94dca9dcdcfdcabc370527c735bb152d Mon Sep 17 00:00:00 2001
From: Laszlo Ersek <lersek@redhat.com>
Date: Thu, 11 Mar 2021 21:08:09 +0100
Subject: [PATCH 1/4] MdeModulePkg/Guid: introduce FVB_ADDRESS_LIST
Introduce a new GUIDed HOB. The GUIDed HOB contains a 64-bit physical
address (FVB_ADDRESS_LIST_PTR), pointing to FVB_ADDRESS_LIST.
FVB_ADDRESS_LIST itself lives in reserved memory, and drivers can populate
it with up to 16 FVB-related addresses (64-bit words).
This will be used to stash the original (physical) and converted (virtual)
addresses of some FVB member functions, for guest memory dumping and
debugging purposes.
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
MdeModulePkg/MdeModulePkg.dec | 3 ++
MdeModulePkg/Include/Guid/FvbAddressList.h | 45 ++++++++++++++++++++
2 files changed, 48 insertions(+)
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index f1144144be14..a67f4e8eb92c 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -400,14 +400,17 @@ [Guids]
## GUID indicates the capsule is to store Capsule On Disk file names.
gEdkiiCapsuleOnDiskNameGuid = { 0x98c80a4f, 0xe16b, 0x4d11, { 0x93, 0x9a, 0xab, 0xe5, 0x61, 0x26, 0x3, 0x30 } }
## Include/Guid/MigratedFvInfo.h
gEdkiiMigratedFvInfoGuid = { 0xc1ab12f7, 0x74aa, 0x408d, { 0xa2, 0xf4, 0xc6, 0xce, 0xfd, 0x17, 0x98, 0x71 } }
+ ## Include/Guid/FvbAddressList.h
+ gEdkiiFvbAddressListGuid = { 0xaa87bf00, 0x7f76, 0x4e32, { 0xb3, 0x92, 0x86, 0xa1, 0x3e, 0x37, 0x33, 0x11 } }
+
[Ppis]
## Include/Ppi/AtaController.h
gPeiAtaControllerPpiGuid = { 0xa45e60d1, 0xc719, 0x44aa, { 0xb0, 0x7a, 0xaa, 0x77, 0x7f, 0x85, 0x90, 0x6d }}
## Include/Ppi/UsbHostController.h
gPeiUsbHostControllerPpiGuid = { 0x652B38A9, 0x77F4, 0x453F, { 0x89, 0xD5, 0xE7, 0xBD, 0xC3, 0x52, 0xFC, 0x53 }}
diff --git a/MdeModulePkg/Include/Guid/FvbAddressList.h b/MdeModulePkg/Include/Guid/FvbAddressList.h
new file mode 100644
index 000000000000..92aa781383f5
--- /dev/null
+++ b/MdeModulePkg/Include/Guid/FvbAddressList.h
@@ -0,0 +1,45 @@
+/** @file
+ Stash FVB-related addresses in a reserved memory structure, pointed-to by a
+ pointer living in a GUID-ed HOB.
+
+ Copyright (C) 2021 Red Hat, Inc.
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef FVB_ADDRESS_LIST_H_
+#define FVB_ADDRESS_LIST_H_
+
+#define EDKII_FVB_ADDRESS_LIST_GUID \
+ { \
+ 0xaa87bf00, \
+ 0x7f76, \
+ 0x4e32, \
+ { 0xb3, 0x92, 0x86, 0xa1, 0x3e, 0x37, 0x33, 0x11 } \
+ }
+
+#define FVB_ADDRESS_LIST_SIGNATURE \
+ SIGNATURE_64 ('F', 'V', 'B', 'A', 'D', 'R', 'L', 'S')
+
+typedef struct {
+ UINT64 OwnerSignature;
+ UINT64 Address;
+} FVB_ADDRESS_LIST_ENTRY;
+
+//
+// lives in reserved memory
+//
+typedef struct {
+ UINT64 Signature;
+ FVB_ADDRESS_LIST_ENTRY Entry[16];
+ UINT32 Next;
+} FVB_ADDRESS_LIST;
+
+//
+// HOB contents: a pointer to FVB_ADDRESS_LIST
+//
+typedef UINT64 FVB_ADDRESS_LIST_PTR;
+
+extern EFI_GUID gEdkiiFvbAddressListGuid;
+
+#endif // FVB_ADDRESS_LIST_H_
--
2.19.1.3.g30247aa5d201
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-ArmVirtPkg-PlatformPeiLib-create-FVB_ADDRESS_LIST.patch --]
[-- Type: text/x-patch; name="0002-ArmVirtPkg-PlatformPeiLib-create-FVB_ADDRESS_LIST.patch", Size: 4986 bytes --]
From d61bf620beb379097eca2705e0cf135f941f13bd Mon Sep 17 00:00:00 2001
From: Laszlo Ersek <lersek@redhat.com>
Date: Thu, 11 Mar 2021 21:35:55 +0100
Subject: [PATCH 2/4] ArmVirtPkg/PlatformPeiLib: create FVB_ADDRESS_LIST
Allocate and initialize FVB_ADDRESS_LIST in a new reserved memory page,
and expose the address in the EDKII_FVB_ADDRESS_LIST_GUID HOB.
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf | 2 +
ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.c | 58 +++++++++++++-------
2 files changed, 41 insertions(+), 19 deletions(-)
diff --git a/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf b/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf
index 3f97ef080520..78eef962fdac 100644
--- a/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf
+++ b/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf
@@ -27,14 +27,15 @@ [Packages]
OvmfPkg/OvmfPkg.dec
SecurityPkg/SecurityPkg.dec
[FeaturePcd]
gArmVirtTokenSpaceGuid.PcdTpm2SupportEnabled
[LibraryClasses]
+ BaseMemoryLib
DebugLib
HobLib
FdtLib
PcdLib
PeiServicesLib
[FixedPcd]
@@ -48,11 +49,12 @@ [Pcd]
[Ppis]
gOvmfTpmDiscoveredPpiGuid ## SOMETIMES_PRODUCES
gPeiTpmInitializationDonePpiGuid ## SOMETIMES_PRODUCES
[Guids]
gEarlyPL011BaseAddressGuid
+ gEdkiiFvbAddressListGuid
gFdtHobGuid
[Depex]
gEfiPeiMemoryDiscoveredPpiGuid
diff --git a/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.c b/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.c
index 6c4028e17995..079ecfad336d 100644
--- a/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.c
+++ b/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.c
@@ -5,14 +5,16 @@
*
* SPDX-License-Identifier: BSD-2-Clause-Patent
*
**/
#include <PiPei.h>
+#include <Guid/FvbAddressList.h>
+#include <Library/BaseMemoryLib.h>
#include <Library/MemoryAllocationLib.h>
#include <Library/DebugLib.h>
#include <Library/HobLib.h>
#include <Library/PcdLib.h>
#include <Library/PeiServicesLib.h>
#include <libfdt.h>
@@ -33,33 +35,35 @@ STATIC CONST EFI_PEI_PPI_DESCRIPTOR mTpm2InitializationDonePpi = {
EFI_STATUS
EFIAPI
PlatformPeim (
VOID
)
{
- VOID *Base;
- VOID *NewBase;
- UINTN FdtSize;
- UINTN FdtPages;
- UINT64 *FdtHobData;
- UINT64 *UartHobData;
- INT32 Node, Prev;
- INT32 Parent, Depth;
- CONST CHAR8 *Compatible;
- CONST CHAR8 *CompItem;
- CONST CHAR8 *NodeStatus;
- INT32 Len;
- INT32 RangesLen;
- INT32 StatusLen;
- CONST UINT64 *RegProp;
- CONST UINT32 *RangesProp;
- UINT64 UartBase;
- UINT64 TpmBase;
- EFI_STATUS Status;
+ VOID *Base;
+ VOID *NewBase;
+ UINTN FdtSize;
+ UINTN FdtPages;
+ UINT64 *FdtHobData;
+ UINT64 *UartHobData;
+ INT32 Node, Prev;
+ INT32 Parent, Depth;
+ CONST CHAR8 *Compatible;
+ CONST CHAR8 *CompItem;
+ CONST CHAR8 *NodeStatus;
+ INT32 Len;
+ INT32 RangesLen;
+ INT32 StatusLen;
+ CONST UINT64 *RegProp;
+ CONST UINT32 *RangesProp;
+ UINT64 UartBase;
+ UINT64 TpmBase;
+ EFI_STATUS Status;
+ FVB_ADDRESS_LIST *FvbAddressList;
+ FVB_ADDRESS_LIST_PTR *FvbAddressListPtrHobData;
Base = (VOID*)(UINTN)PcdGet64 (PcdDeviceTreeInitialBaseAddress);
ASSERT (Base != NULL);
ASSERT (fdt_check_header (Base) == 0);
FdtSize = fdt_totalsize (Base) + PcdGet32 (PcdDeviceTreeAllocationPadding);
FdtPages = EFI_SIZE_TO_PAGES (FdtSize);
@@ -181,9 +185,25 @@ PlatformPeim (
Status = PeiServicesInstallPpi (&mTpm2InitializationDonePpi);
}
ASSERT_EFI_ERROR (Status);
}
BuildFvHob (PcdGet64 (PcdFvBaseAddress), PcdGet32 (PcdFvSize));
+ //
+ // Allocate, initialize, and expose FVB_ADDRESS_LIST.
+ //
+ FvbAddressList = AllocateReservedPages (
+ EFI_SIZE_TO_PAGES (sizeof *FvbAddressList));
+ ASSERT (FvbAddressList != NULL);
+ DEBUG ((DEBUG_VERBOSE, "%a: FvbAddressList @ %p\n", __FUNCTION__,
+ (VOID *)FvbAddressList));
+ FvbAddressList->Signature = FVB_ADDRESS_LIST_SIGNATURE;
+ ZeroMem (FvbAddressList->Entry, sizeof FvbAddressList->Entry);
+ FvbAddressList->Next = 0;
+ FvbAddressListPtrHobData = BuildGuidHob (&gEdkiiFvbAddressListGuid,
+ sizeof *FvbAddressListPtrHobData);
+ ASSERT (FvbAddressListPtrHobData != NULL);
+ *FvbAddressListPtrHobData = (UINTN)FvbAddressList;
+
return EFI_SUCCESS;
}
--
2.19.1.3.g30247aa5d201
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0003-ArmPlatformPkg-NorFlashDxe-populate-FVB_ADDRESS_LIST.patch --]
[-- Type: text/x-patch; name="0003-ArmPlatformPkg-NorFlashDxe-populate-FVB_ADDRESS_LIST.patch", Size: 7153 bytes --]
From a656046171cd98600d4f6810f1d9f856aba19b6e Mon Sep 17 00:00:00 2001
From: Laszlo Ersek <lersek@redhat.com>
Date: Thu, 11 Mar 2021 22:16:41 +0100
Subject: [PATCH 3/4] ArmPlatformPkg/NorFlashDxe: populate FVB_ADDRESS_LIST
When the SetVirtualAddressMap() handler runs, and we convert (among other
things) the GetPhysicalAddress() FVB member function pointer, stash both
the physical and the virtual addresses of this member function, in
FVB_ADDRESS_LIST.
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf | 1 +
ArmPlatformPkg/Drivers/NorFlashDxe/NorFlash.c | 27 ++++++++++++++++++--
ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c | 11 ++++++++
3 files changed, 37 insertions(+), 2 deletions(-)
diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
index f8d4c2703143..1eba5f869c55 100644
--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
@@ -43,14 +43,15 @@ [LibraryClasses]
[Guids]
gEfiSystemNvDataFvGuid
gEfiVariableGuid
gEfiAuthenticatedVariableGuid
gEfiEventVirtualAddressChangeGuid
gEdkiiNvVarStoreFormattedGuid ## PRODUCES ## PROTOCOL
+ gEdkiiFvbAddressListGuid
[Protocols]
gEfiBlockIoProtocolGuid
gEfiDevicePathProtocolGuid
gEfiFirmwareVolumeBlockProtocolGuid
gEfiDiskIoProtocolGuid
diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlash.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlash.c
index a9e23db4461b..73faca549bda 100644
--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlash.c
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlash.c
@@ -3,23 +3,25 @@
Copyright (c) 2011 - 2020, Arm Limited. All rights reserved.<BR>
Copyright (c) 2020, Linaro, Ltd. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
+#include <Guid/FvbAddressList.h>
#include <Library/BaseMemoryLib.h>
#include "NorFlash.h"
//
// Global variable declarations
//
extern NOR_FLASH_INSTANCE **mNorFlashInstances;
extern UINT32 mNorFlashDeviceCount;
+extern FVB_ADDRESS_LIST *mFvbAddressList;
UINT32
NorFlashReadStatusRegister (
IN NOR_FLASH_INSTANCE *Instance,
IN UINTN SR_Address
)
{
@@ -939,35 +941,56 @@ NorFlashReset (
VOID
EFIAPI
NorFlashVirtualNotifyEvent (
IN EFI_EVENT Event,
IN VOID *Context
)
{
- UINTN Index;
+ UINT64 OwnerSignature;
+ FVB_ADDRESS_LIST_ENTRY *Entry;
+ UINTN Index;
+
+ OwnerSignature = SIGNATURE_64 ('N', 'O', 'R', 'F', 'L', 'A', 'S', 'H');
+ Entry = mFvbAddressList->Entry + mFvbAddressList->Next;
for (Index = 0; Index < mNorFlashDeviceCount; Index++) {
+ VOID *Pointer;
+
EfiConvertPointer (0x0, (VOID**)&mNorFlashInstances[Index]->DeviceBaseAddress);
EfiConvertPointer (0x0, (VOID**)&mNorFlashInstances[Index]->RegionBaseAddress);
// Convert BlockIo protocol
EfiConvertPointer (0x0, (VOID**)&mNorFlashInstances[Index]->BlockIoProtocol.FlushBlocks);
EfiConvertPointer (0x0, (VOID**)&mNorFlashInstances[Index]->BlockIoProtocol.ReadBlocks);
EfiConvertPointer (0x0, (VOID**)&mNorFlashInstances[Index]->BlockIoProtocol.Reset);
EfiConvertPointer (0x0, (VOID**)&mNorFlashInstances[Index]->BlockIoProtocol.WriteBlocks);
// Convert Fvb
EfiConvertPointer (0x0, (VOID**)&mNorFlashInstances[Index]->FvbProtocol.EraseBlocks);
EfiConvertPointer (0x0, (VOID**)&mNorFlashInstances[Index]->FvbProtocol.GetAttributes);
EfiConvertPointer (0x0, (VOID**)&mNorFlashInstances[Index]->FvbProtocol.GetBlockSize);
- EfiConvertPointer (0x0, (VOID**)&mNorFlashInstances[Index]->FvbProtocol.GetPhysicalAddress);
+
+ Pointer = (VOID *)(UINTN)mNorFlashInstances[Index]->FvbProtocol.GetPhysicalAddress;
+ Entry->OwnerSignature = OwnerSignature;
+ Entry->Address = (UINTN)Pointer;
+ ++Entry;
+
+ EfiConvertPointer (0x0, &Pointer);
+ mNorFlashInstances[Index]->FvbProtocol.GetPhysicalAddress =
+ (EFI_FVB_GET_PHYSICAL_ADDRESS)(UINTN)Pointer;
+
+ Entry->OwnerSignature = OwnerSignature;
+ Entry->Address = (UINTN)Pointer;
+ ++Entry;
+
EfiConvertPointer (0x0, (VOID**)&mNorFlashInstances[Index]->FvbProtocol.Read);
EfiConvertPointer (0x0, (VOID**)&mNorFlashInstances[Index]->FvbProtocol.SetAttributes);
EfiConvertPointer (0x0, (VOID**)&mNorFlashInstances[Index]->FvbProtocol.Write);
if (mNorFlashInstances[Index]->ShadowBuffer != NULL) {
EfiConvertPointer (0x0, (VOID**)&mNorFlashInstances[Index]->ShadowBuffer);
}
}
+ mFvbAddressList->Next = Entry - mFvbAddressList->Entry;
return;
}
diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
index f412731200cf..fad0b8c39723 100644
--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
@@ -2,14 +2,15 @@
Copyright (c) 2011 - 2021, Arm Limited. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
+#include <Guid/FvbAddressList.h>
#include <Library/UefiLib.h>
#include <Library/BaseMemoryLib.h>
#include <Library/MemoryAllocationLib.h>
#include <Library/UefiBootServicesTableLib.h>
#include <Library/PcdLib.h>
#include <Library/HobLib.h>
#include <Library/DxeServicesTableLib.h>
@@ -21,14 +22,16 @@ STATIC EFI_EVENT mNorFlashVirtualAddrChangeEvent;
//
// Global variable declarations
//
NOR_FLASH_INSTANCE **mNorFlashInstances;
UINT32 mNorFlashDeviceCount;
UINTN mFlashNvStorageVariableBase;
EFI_EVENT mFvbVirtualAddrChangeEvent;
+FVB_ADDRESS_LIST *mFvbAddressList;
+
NOR_FLASH_INSTANCE mNorFlashInstanceTemplate = {
NOR_FLASH_SIGNATURE, // Signature
NULL, // Handle ... NEED TO BE FILLED
0, // DeviceBaseAddress ... NEED TO BE FILLED
0, // RegionBaseAddress ... NEED TO BE FILLED
@@ -318,19 +321,27 @@ NorFlashWriteFullBlock (
EFI_STATUS
EFIAPI
NorFlashInitialise (
IN EFI_HANDLE ImageHandle,
IN EFI_SYSTEM_TABLE *SystemTable
)
{
+ VOID *Hob;
+ FVB_ADDRESS_LIST_PTR *FvbAddressListPtrHobData;
EFI_STATUS Status;
UINT32 Index;
NOR_FLASH_DESCRIPTION* NorFlashDevices;
BOOLEAN ContainVariableStorage;
+ Hob = GetFirstGuidHob (&gEdkiiFvbAddressListGuid);
+ ASSERT (Hob != NULL);
+ FvbAddressListPtrHobData = GET_GUID_HOB_DATA (Hob);
+ mFvbAddressList = (VOID*)(UINTN)*FvbAddressListPtrHobData;
+ ASSERT (mFvbAddressList->Signature == FVB_ADDRESS_LIST_SIGNATURE);
+
Status = NorFlashPlatformInitialization ();
if (EFI_ERROR(Status)) {
DEBUG((EFI_D_ERROR,"NorFlashInitialise: Fail to initialize Nor Flash devices\n"));
return Status;
}
Status = NorFlashPlatformGetDevices (&NorFlashDevices, &mNorFlashDeviceCount);
--
2.19.1.3.g30247aa5d201
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #5: 0004-MdeModulePkg-VariableRuntimeDxe-populate-FVB_ADDRESS_LIST.patch --]
[-- Type: text/x-patch; name="0004-MdeModulePkg-VariableRuntimeDxe-populate-FVB_ADDRESS_LIST.patch", Size: 8524 bytes --]
From f424470f69ce8811766b1118fc801238f2242596 Mon Sep 17 00:00:00 2001
From: Laszlo Ersek <lersek@redhat.com>
Date: Thu, 11 Mar 2021 22:16:41 +0100
Subject: [PATCH 4/4] MdeModulePkg/VariableRuntimeDxe: populate
FVB_ADDRESS_LIST
When the SetVirtualAddressMap() handler runs, and we convert (among other
things) the GetPhysicalAddress() FVB member function pointer, stash both
the physical and the virtual addresses of this member function, in
FVB_ADDRESS_LIST.
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf | 2 ++
MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h | 2 ++
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c | 36 ++++++++++++++++++--
3 files changed, 38 insertions(+), 2 deletions(-)
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
index c9434df631ee..e4cc49692f2d 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
@@ -120,14 +120,16 @@ [Guids]
gEdkiiVarErrorFlagGuid
## SOMETIMES_CONSUMES ## Variable:L"db"
## SOMETIMES_CONSUMES ## Variable:L"dbx"
## SOMETIMES_CONSUMES ## Variable:L"dbt"
gEfiImageSecurityDatabaseGuid
+ gEdkiiFvbAddressListGuid
+
[Pcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase ## SOMETIMES_CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVolatileVariableSize ## CONSUMES
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
index 0b2bb6ae6648..77c390586d35 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
@@ -33,14 +33,15 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Library/VarCheckLib.h>
#include <Guid/GlobalVariable.h>
#include <Guid/EventGroup.h>
#include <Guid/VariableFormat.h>
#include <Guid/SystemNvDataGuid.h>
#include <Guid/FaultTolerantWrite.h>
#include <Guid/VarErrorFlag.h>
+#include <Guid/FvbAddressList.h>
#include "PrivilegePolymorphic.h"
#define NV_STORAGE_VARIABLE_BASE (EFI_PHYSICAL_ADDRESS) \
(PcdGet64 (PcdFlashNvStorageVariableBase64) != 0 ? \
PcdGet64 (PcdFlashNvStorageVariableBase64) : \
PcdGet32 (PcdFlashNvStorageVariableBase))
@@ -693,14 +694,15 @@ extern VARIABLE_MODULE_GLOBAL *mVariableModuleGlobal;
extern EFI_FIRMWARE_VOLUME_HEADER *mNvFvHeaderCache;
extern VARIABLE_STORE_HEADER *mNvVariableCache;
extern VARIABLE_INFO_ENTRY *gVariableInfo;
extern BOOLEAN mEndOfDxe;
extern VAR_CHECK_REQUEST_SOURCE mRequestSource;
extern AUTH_VAR_LIB_CONTEXT_OUT mAuthContextOut;
+extern FVB_ADDRESS_LIST *mFvbAddressList;
/**
Finds variable in storage blocks of volatile and non-volatile storage areas.
This code finds variable in storage blocks of volatile and non-volatile storage areas.
If VariableName is an empty string, then we just return the first
qualified variable without comparing VariableName and VendorGuid.
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
index 0fca0bb2a9b5..b7a33e3a348d 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
@@ -32,14 +32,15 @@ EDKII_VARIABLE_POLICY_PROTOCOL mVariablePolicyProtocol = { EDKII_VARIABL
ProtocolIsVariablePolicyEnabled,
RegisterVariablePolicy,
DumpVariablePolicy,
LockVariablePolicy };
EDKII_VAR_CHECK_PROTOCOL mVarCheck = { VarCheckRegisterSetVariableCheckHandler,
VarCheckVariablePropertySet,
VarCheckVariablePropertyGet };
+FVB_ADDRESS_LIST *mFvbAddressList;
/**
Some Secure Boot Policy Variable may update following other variable changes(SecureBoot follows PK change, etc).
Record their initial State when variable write service is ready.
**/
VOID
@@ -240,26 +241,49 @@ GetFvbCountAndBuffer (
VOID
EFIAPI
VariableClassAddressChangeEvent (
IN EFI_EVENT Event,
IN VOID *Context
)
{
- UINTN Index;
+ UINT64 OwnerSignature;
+ FVB_ADDRESS_LIST_ENTRY *Entry;
+ UINTN Index;
+
+ OwnerSignature = SIGNATURE_64 ('V', 'A', 'R', 'I', 'A', 'B', 'L', 'E');
+ Entry = mFvbAddressList->Entry + mFvbAddressList->Next;
if (mVariableModuleGlobal->FvbInstance != NULL) {
+ VOID *Pointer;
+
EfiConvertPointer (0x0, (VOID **) &mVariableModuleGlobal->FvbInstance->GetBlockSize);
- EfiConvertPointer (0x0, (VOID **) &mVariableModuleGlobal->FvbInstance->GetPhysicalAddress);
+
+ Pointer = (VOID *)(UINTN)mVariableModuleGlobal->FvbInstance->GetPhysicalAddress;
+ Entry->OwnerSignature = OwnerSignature;
+ Entry->Address = (UINTN)Pointer;
+ ++Entry;
+
+ EfiConvertPointer (0x0, &Pointer);
+ mVariableModuleGlobal->FvbInstance->GetPhysicalAddress =
+ (EFI_FVB_GET_PHYSICAL_ADDRESS)(UINTN)Pointer;
+
+ Entry->OwnerSignature = OwnerSignature;
+ Entry->Address = (UINTN)Pointer;
+ ++Entry;
+
EfiConvertPointer (0x0, (VOID **) &mVariableModuleGlobal->FvbInstance->GetAttributes);
EfiConvertPointer (0x0, (VOID **) &mVariableModuleGlobal->FvbInstance->SetAttributes);
EfiConvertPointer (0x0, (VOID **) &mVariableModuleGlobal->FvbInstance->Read);
EfiConvertPointer (0x0, (VOID **) &mVariableModuleGlobal->FvbInstance->Write);
EfiConvertPointer (0x0, (VOID **) &mVariableModuleGlobal->FvbInstance->EraseBlocks);
EfiConvertPointer (0x0, (VOID **) &mVariableModuleGlobal->FvbInstance);
}
+
+ mFvbAddressList->Next = Entry - mFvbAddressList->Entry;
+
EfiConvertPointer (0x0, (VOID **) &mVariableModuleGlobal->PlatformLangCodes);
EfiConvertPointer (0x0, (VOID **) &mVariableModuleGlobal->LangCodes);
EfiConvertPointer (0x0, (VOID **) &mVariableModuleGlobal->PlatformLang);
EfiConvertPointer (0x0, (VOID **) &mVariableModuleGlobal->VariableGlobal.NonVolatileVariableBase);
EfiConvertPointer (0x0, (VOID **) &mVariableModuleGlobal->VariableGlobal.VolatileVariableBase);
EfiConvertPointer (0x0, (VOID **) &mVariableModuleGlobal->VariableGlobal.HobVariableBase);
EfiConvertPointer (0x0, (VOID **) &mVariableModuleGlobal);
@@ -528,18 +552,26 @@ ProtocolIsVariablePolicyEnabled (
EFI_STATUS
EFIAPI
VariableServiceInitialize (
IN EFI_HANDLE ImageHandle,
IN EFI_SYSTEM_TABLE *SystemTable
)
{
+ VOID *Hob;
+ FVB_ADDRESS_LIST_PTR *FvbAddressListPtrHobData;
EFI_STATUS Status;
EFI_EVENT ReadyToBootEvent;
EFI_EVENT EndOfDxeEvent;
+ Hob = GetFirstGuidHob (&gEdkiiFvbAddressListGuid);
+ ASSERT (Hob != NULL);
+ FvbAddressListPtrHobData = GET_GUID_HOB_DATA (Hob);
+ mFvbAddressList = (VOID*)(UINTN)*FvbAddressListPtrHobData;
+ ASSERT (mFvbAddressList->Signature == FVB_ADDRESS_LIST_SIGNATURE);
+
Status = VariableCommonInitialize ();
ASSERT_EFI_ERROR (Status);
Status = gBS->InstallMultipleProtocolInterfaces (
&mHandle,
&gEdkiiVariableLockProtocolGuid,
&mVariableLock,
--
2.19.1.3.g30247aa5d201
next prev parent reply other threads:[~2021-03-11 22:25 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-10 8:04 Conflicting virtual addresses causing Runtime Services issues Jon Nettleton
2021-03-10 14:52 ` [edk2-devel] " Laszlo Ersek
2021-03-11 6:53 ` Jon Nettleton
[not found] ` <166B374585A9D8FC.18699@groups.io>
2021-03-11 9:48 ` Jon Nettleton
2021-03-11 14:50 ` Laszlo Ersek
2021-03-11 15:49 ` Jon Nettleton
2021-03-11 22:25 ` Laszlo Ersek [this message]
2021-03-11 22:39 ` Ard Biesheuvel
2021-03-12 3:01 ` Jon Nettleton
[not found] ` <166B792D1514133B.31346@groups.io>
2021-03-12 5:59 ` Jon Nettleton
2021-03-12 19:30 ` Laszlo Ersek
2021-03-12 19:52 ` Ard Biesheuvel
2021-03-12 19:17 ` Laszlo Ersek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4841241f-fc6d-6185-efe6-ed9a536534dd@redhat.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox