* [edk2-platforms: PATCH v2 0/3] Armada7k8k FVB improvements
@ 2019-04-24 6:51 Marcin Wojtas
2019-04-24 6:51 ` [edk2-platforms: PATCH v2 1/3] Marvell/Drivers: MvFvbDxe: Change Pcd parameters to be 64 bit Marcin Wojtas
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Marcin Wojtas @ 2019-04-24 6:51 UTC (permalink / raw)
To: devel; +Cc: leif.lindholm, ard.biesheuvel, mw, jsd, jaz, kostap, Jici.Gao
Hi,
The second version of the patchset, as a result of discussions,
abandons gate-clock handling of the SPI flash, as when booting
from UEFI the proper flow is to rely on description provided
by firmware and the clock handling itself should be left
for the OS entirely.
Instead a new patch is introduced, which removes unused
components from the PEI FV.
The patches are available in the github:
https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/fvb-upstream-r20190424
I'm looking forward to your comments or remarks.
Best regards,
Marcin
Changelog:
v1->v2
* Replace clock-enabling patch with PEI phase FV cleanup
Hanna Hawa (1):
Marvell/Drivers: MvFvbDxe: Change Pcd parameters to be 64 bit
Kornel Duleba (1):
Marvell/Drivers: Add non-mmio mode to MvFvbDxe
Marcin Wojtas (1):
Marvell/Armada7k8k: Cleanup PEI phase FV
Silicon/Marvell/Marvell.dec | 10 +-
Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 13 +-
Silicon/Marvell/Armada7k8k/Armada7k8k.fdf | 3 -
Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf | 17 ++-
Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.h | 1 +
Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c | 149 +++++++++++++++-----
6 files changed, 143 insertions(+), 50 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 9+ messages in thread* [edk2-platforms: PATCH v2 1/3] Marvell/Drivers: MvFvbDxe: Change Pcd parameters to be 64 bit 2019-04-24 6:51 [edk2-platforms: PATCH v2 0/3] Armada7k8k FVB improvements Marcin Wojtas @ 2019-04-24 6:51 ` Marcin Wojtas 2019-04-24 6:51 ` [edk2-platforms: PATCH v2 2/3] Marvell/Armada7k8k: Cleanup PEI phase FV Marcin Wojtas 2019-04-24 6:51 ` [edk2-platforms: PATCH v2 3/3] Marvell/Drivers: Add non-mmio mode to MvFvbDxe Marcin Wojtas 2 siblings, 0 replies; 9+ messages in thread From: Marcin Wojtas @ 2019-04-24 6:51 UTC (permalink / raw) To: devel Cc: leif.lindholm, ard.biesheuvel, mw, jsd, jaz, kostap, Jici.Gao, Hanna Hawa From: Hanna Hawa <hannah@marvell.com> Update PCD paramters to be 64 bit, so that to add more flexibility for the platforms in terms of configuring memory-mapped SPI access. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Marcin Wojtas <mw@semihalf.com> --- Silicon/Marvell/Marvell.dec | 2 +- Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 6 +++--- Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf | 6 +++--- Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c | 18 +++++++++--------- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/Silicon/Marvell/Marvell.dec b/Silicon/Marvell/Marvell.dec index c927078..7210ba2 100644 --- a/Silicon/Marvell/Marvell.dec +++ b/Silicon/Marvell/Marvell.dec @@ -139,7 +139,7 @@ #SPI gMarvellTokenSpaceGuid.PcdSpiRegBase|0|UINT32|0x3000051 - gMarvellTokenSpaceGuid.PcdSpiMemoryBase|0|UINT32|0x3000059 + gMarvellTokenSpaceGuid.PcdSpiMemoryBase|0|UINT64|0x3000059 gMarvellTokenSpaceGuid.PcdSpiMaxFrequency|0|UINT32|0x30000052 gMarvellTokenSpaceGuid.PcdSpiClockFrequency|0|UINT32|0x30000053 diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc index 0ced400..a1ebb81 100644 --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc @@ -396,11 +396,11 @@ # Variable store - default values # gMarvellTokenSpaceGuid.PcdSpiMemoryBase|0xF9000000 - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0xF93C0000 + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0xF93C0000 gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00010000 - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0xF93D0000 + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64|0xF93D0000 gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00010000 - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0xF93E0000 + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64|0xF93E0000 gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00010000 !if $(CAPSULE_ENABLE) diff --git a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf index fd3f2f7..ef10bfd 100644 --- a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf +++ b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf @@ -76,11 +76,11 @@ gMarvellSpiMasterProtocolGuid [FixedPcd] - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64 gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64 gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize gMarvellTokenSpaceGuid.PcdSpiMemoryBase diff --git a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c index 1a41a4f..cb006cd 100644 --- a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c +++ b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c @@ -145,12 +145,12 @@ MvFvbInitFvAndVariableStoreHeaders ( // FirmwareVolumeHeader->FvLength is declared to have the Variable area // AND the FTW working area AND the FTW Spare contiguous. // - ASSERT (PcdGet32 (PcdFlashNvStorageVariableBase) + + ASSERT (PcdGet64 (PcdFlashNvStorageVariableBase64) + PcdGet32 (PcdFlashNvStorageVariableSize) == - PcdGet32 (PcdFlashNvStorageFtwWorkingBase)); - ASSERT (PcdGet32 (PcdFlashNvStorageFtwWorkingBase) + + PcdGet64 (PcdFlashNvStorageFtwWorkingBase64)); + ASSERT (PcdGet64 (PcdFlashNvStorageFtwWorkingBase64) + PcdGet32 (PcdFlashNvStorageFtwWorkingSize) == - PcdGet32 (PcdFlashNvStorageFtwSpareBase)); + PcdGet64 (PcdFlashNvStorageFtwSpareBase64)); // Check if the size of the area is at least one block size ASSERT ((PcdGet32 (PcdFlashNvStorageVariableSize) > 0) && @@ -161,9 +161,9 @@ MvFvbInitFvAndVariableStoreHeaders ( (PcdGet32 (PcdFlashNvStorageFtwSpareSize) / BlockSize > 0)); // Ensure the Variable areas are aligned on block size boundaries - ASSERT ((PcdGet32 (PcdFlashNvStorageVariableBase) % BlockSize) == 0); - ASSERT ((PcdGet32 (PcdFlashNvStorageFtwWorkingBase) % BlockSize) == 0); - ASSERT ((PcdGet32 (PcdFlashNvStorageFtwSpareBase) % BlockSize) == 0); + ASSERT ((PcdGet64 (PcdFlashNvStorageVariableBase64) % BlockSize) == 0); + ASSERT ((PcdGet64 (PcdFlashNvStorageFtwWorkingBase64) % BlockSize) == 0); + ASSERT ((PcdGet64 (PcdFlashNvStorageFtwSpareBase64) % BlockSize) == 0); // // EFI_FIRMWARE_VOLUME_HEADER @@ -1009,8 +1009,8 @@ MvFvbConfigureFlashInstance ( } // Fill remaining flash description - FlashInstance->DeviceBaseAddress = PcdGet32 (PcdSpiMemoryBase); - FlashInstance->RegionBaseAddress = FixedPcdGet32 (PcdFlashNvStorageVariableBase); + FlashInstance->DeviceBaseAddress = PcdGet64 (PcdSpiMemoryBase); + FlashInstance->RegionBaseAddress = FixedPcdGet64 (PcdFlashNvStorageVariableBase64); FlashInstance->FvbOffset = FlashInstance->RegionBaseAddress - FlashInstance->DeviceBaseAddress; FlashInstance->FvbSize = PcdGet32(PcdFlashNvStorageVariableSize) + -- 2.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [edk2-platforms: PATCH v2 2/3] Marvell/Armada7k8k: Cleanup PEI phase FV 2019-04-24 6:51 [edk2-platforms: PATCH v2 0/3] Armada7k8k FVB improvements Marcin Wojtas 2019-04-24 6:51 ` [edk2-platforms: PATCH v2 1/3] Marvell/Drivers: MvFvbDxe: Change Pcd parameters to be 64 bit Marcin Wojtas @ 2019-04-24 6:51 ` Marcin Wojtas 2019-04-24 6:51 ` [edk2-platforms: PATCH v2 3/3] Marvell/Drivers: Add non-mmio mode to MvFvbDxe Marcin Wojtas 2 siblings, 0 replies; 9+ messages in thread From: Marcin Wojtas @ 2019-04-24 6:51 UTC (permalink / raw) To: devel; +Cc: leif.lindholm, ard.biesheuvel, mw, jsd, jaz, kostap, Jici.Gao Currently supported the capsule update scenarios on Armada platforms require only the SystemFirmwareDescriptor to be installed in the PEI phase. Remove redundant components and reduce PEI phase FV footprint. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Marcin Wojtas <mw@semihalf.com> --- Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 3 --- Silicon/Marvell/Armada7k8k/Armada7k8k.fdf | 3 --- 2 files changed, 6 deletions(-) diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc index a1ebb81..ca3de2e 100644 --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc @@ -427,10 +427,7 @@ ArmPlatformPkg/PlatformPei/PlatformPeim.inf ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf ArmPkg/Drivers/CpuPei/CpuPei.inf - MdeModulePkg/Universal/Variable/Pei/VariablePei.inf !if $(CAPSULE_ENABLE) - MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf - MdeModulePkg/Universal/CapsulePei/CapsulePei.inf Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf !endif MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf { diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.fdf b/Silicon/Marvell/Armada7k8k/Armada7k8k.fdf index d739020..e90e10e 100644 --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.fdf +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.fdf @@ -249,10 +249,7 @@ READ_LOCK_STATUS = TRUE INF ArmPlatformPkg/PlatformPei/PlatformPeim.inf INF ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf INF ArmPkg/Drivers/CpuPei/CpuPei.inf - INF MdeModulePkg/Universal/Variable/Pei/VariablePei.inf !if $(CAPSULE_ENABLE) - INF MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf - INF MdeModulePkg/Universal/CapsulePei/CapsulePei.inf INF RuleOverride = FMP_IMAGE_DESC Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf !endif INF MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf -- 2.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [edk2-platforms: PATCH v2 3/3] Marvell/Drivers: Add non-mmio mode to MvFvbDxe 2019-04-24 6:51 [edk2-platforms: PATCH v2 0/3] Armada7k8k FVB improvements Marcin Wojtas 2019-04-24 6:51 ` [edk2-platforms: PATCH v2 1/3] Marvell/Drivers: MvFvbDxe: Change Pcd parameters to be 64 bit Marcin Wojtas 2019-04-24 6:51 ` [edk2-platforms: PATCH v2 2/3] Marvell/Armada7k8k: Cleanup PEI phase FV Marcin Wojtas @ 2019-04-24 6:51 ` Marcin Wojtas 2019-04-24 7:11 ` Ard Biesheuvel 2 siblings, 1 reply; 9+ messages in thread From: Marcin Wojtas @ 2019-04-24 6:51 UTC (permalink / raw) To: devel Cc: leif.lindholm, ard.biesheuvel, mw, jsd, jaz, kostap, Jici.Gao, Kornel Duleba From: Kornel Duleba <mindal@semihalf.com> This path enables support for reading variables directly from flash without relying on it to be memory mapped. It adds PcdSpiMemoryMapped PCD that allows to switch between the modes. When in non-memory-mapped mode the driver will copy the variables from flash to previously allocated buffer and set PcdFlashNvStorageVariableBase64, PcdFlashNvStorageFtwWorkingBase64 and PcdFlashNvStorageFtwSpareBase64 accordingly. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Marcin Wojtas <mw@semihalf.com> --- Silicon/Marvell/Marvell.dec | 8 ++ Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 10 +- Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf | 17 ++- Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.h | 1 + Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c | 135 +++++++++++++++----- 5 files changed, 135 insertions(+), 36 deletions(-) diff --git a/Silicon/Marvell/Marvell.dec b/Silicon/Marvell/Marvell.dec index 7210ba2..a23c329 100644 --- a/Silicon/Marvell/Marvell.dec +++ b/Silicon/Marvell/Marvell.dec @@ -58,6 +58,12 @@ gMarvellFvbDxeGuid = { 0x42903750, 0x7e61, 0x4aaf, { 0x83, 0x29, 0xbf, 0x42, 0x36, 0x4e, 0x24, 0x85 } } gMarvellSpiFlashDxeGuid = { 0x49d7fb74, 0x306d, 0x42bd, { 0x94, 0xc8, 0xc0, 0xc5, 0x4b, 0x18, 0x1d, 0xd7 } } + # + # Generic FaultTolerantWriteDxe driver use variables, + # whose setting is done in MvFvbDxe driver in case + # the SPI contents are not mapped in memory. + # + gFaultTolerantWriteDxeFileGuid = { 0xfe5cea76, 0x4f72, 0x49e8, { 0x98, 0x6f, 0x2c, 0xd8, 0x99, 0xdf, 0xfe, 0x5d} } [LibraryClasses] ArmadaBoardDescLib|Include/Library/ArmadaBoardDescLib.h @@ -140,6 +146,8 @@ #SPI gMarvellTokenSpaceGuid.PcdSpiRegBase|0|UINT32|0x3000051 gMarvellTokenSpaceGuid.PcdSpiMemoryBase|0|UINT64|0x3000059 + gMarvellTokenSpaceGuid.PcdSpiMemoryMapped|TRUE|BOOLEAN|0x3000060 + gMarvellTokenSpaceGuid.PcdSpiVariableOffset|0|UINT32|0x3000061 gMarvellTokenSpaceGuid.PcdSpiMaxFrequency|0|UINT32|0x30000052 gMarvellTokenSpaceGuid.PcdSpiClockFrequency|0|UINT32|0x30000053 diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc index ca3de2e..d53d128 100644 --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc @@ -256,6 +256,11 @@ # USB support gEfiMdeModulePkgTokenSpaceGuid.PcdTurnOffUsbLegacySupport|TRUE +[PcdsDynamicDefault.common] + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0xF93C0000 + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64|0xF93E0000 + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64|0xF93D0000 + [PcdsFixedAtBuild.common] gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"MARVELL_EFI" gArmPlatformTokenSpaceGuid.PcdCoreCount|4 @@ -396,11 +401,10 @@ # Variable store - default values # gMarvellTokenSpaceGuid.PcdSpiMemoryBase|0xF9000000 - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0xF93C0000 + gMarvellTokenSpaceGuid.PcdSpiMemoryMapped|TRUE + gMarvellTokenSpaceGuid.PcdSpiVariableOffset|0x3C0000 gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00010000 - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64|0xF93D0000 gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00010000 - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64|0xF93E0000 gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00010000 !if $(CAPSULE_ENABLE) diff --git a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf index ef10bfd..c85e8a6 100644 --- a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf +++ b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf @@ -76,13 +76,22 @@ gMarvellSpiMasterProtocolGuid [FixedPcd] - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64 gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64 gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize gMarvellTokenSpaceGuid.PcdSpiMemoryBase + gMarvellTokenSpaceGuid.PcdSpiMemoryMapped + gMarvellTokenSpaceGuid.PcdSpiVariableOffset + +[Pcd] + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64 + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64 [Depex] - gEfiCpuArchProtocolGuid + # + # Generic FaultTolerantWriteDxe driver use variables, + # whose setting is done in MvFvbDxe driver in case + # the SPI contents are not mapped in memory. + # + BEFORE gFaultTolerantWriteDxeFileGuid diff --git a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.h b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.h index 31e6e44..e8df9a5 100644 --- a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.h +++ b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.h @@ -55,6 +55,7 @@ typedef struct { UINT32 Signature; + BOOLEAN IsMemoryMapped; UINTN DeviceBaseAddress; UINTN RegionBaseAddress; UINTN Size; diff --git a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c index cb006cd..b4fd29c 100644 --- a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c +++ b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c @@ -52,6 +52,7 @@ STATIC CONST FVB_DEVICE mMvFvbFlashInstanceTemplate = { FVB_FLASH_SIGNATURE, // Signature + FALSE, // IsMemoryMapped ... NEED TO BE FILLED 0, // DeviceBaseAddress ... NEED TO BE FILLED 0, // RegionBaseAddress ... NEED TO BE FILLED SIZE_256KB, // Size @@ -175,11 +176,14 @@ MvFvbInitFvAndVariableStoreHeaders ( FirmwareVolumeHeader->Attributes = EFI_FVB2_READ_ENABLED_CAP | EFI_FVB2_READ_STATUS | EFI_FVB2_STICKY_WRITE | - EFI_FVB2_MEMORY_MAPPED | EFI_FVB2_ERASE_POLARITY | EFI_FVB2_WRITE_STATUS | EFI_FVB2_WRITE_ENABLED_CAP; + if (FlashInstance->IsMemoryMapped) { + FirmwareVolumeHeader->Attributes |= EFI_FVB2_MEMORY_MAPPED; + } + FirmwareVolumeHeader->HeaderLength = sizeof (EFI_FIRMWARE_VOLUME_HEADER) + sizeof (EFI_FV_BLOCK_MAP_ENTRY); FirmwareVolumeHeader->Revision = EFI_FVH_REVISION; @@ -349,10 +353,13 @@ MvFvbSetAttributes ( EFI_FVB_ATTRIBUTES_2 OldAttributes; EFI_FVB_ATTRIBUTES_2 FlashFvbAttributes; EFI_FVB_ATTRIBUTES_2 UnchangedAttributes; + FVB_DEVICE *FlashInstance; UINT32 Capabilities; UINT32 OldStatus; UINT32 NewStatus; + FlashInstance = INSTANCE_FROM_FVB_THIS (This); + // // Obtain attributes from FVB header // @@ -369,12 +376,15 @@ MvFvbSetAttributes ( EFI_FVB2_WRITE_ENABLED_CAP | \ EFI_FVB2_LOCK_CAP | \ EFI_FVB2_STICKY_WRITE | \ - EFI_FVB2_MEMORY_MAPPED | \ EFI_FVB2_ERASE_POLARITY | \ EFI_FVB2_READ_LOCK_CAP | \ EFI_FVB2_WRITE_LOCK_CAP | \ EFI_FVB2_ALIGNMENT; + if (FlashInstance->IsMemoryMapped) { + UnchangedAttributes |= EFI_FVB2_MEMORY_MAPPED; + } + // // Some attributes of FV is read only can *not* be set // @@ -692,6 +702,7 @@ MvFvbWrite ( IN UINT8 *Buffer ) { + EFI_STATUS Status; FVB_DEVICE *FlashInstance; UINTN DataOffset; @@ -701,10 +712,27 @@ MvFvbWrite ( FlashInstance->StartLba + Lba, FlashInstance->Media.BlockSize); - return FlashInstance->SpiFlashProtocol->Write (&FlashInstance->SpiDevice, - DataOffset, - *NumBytes, - Buffer); + Status = FlashInstance->SpiFlashProtocol->Write (&FlashInstance->SpiDevice, + DataOffset, + *NumBytes, + Buffer); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, + "%a: Failed to write to Spi device\n", + __FUNCTION__)); + return Status; + } + + // Update shadow buffer + if (!FlashInstance->IsMemoryMapped) { + DataOffset = GET_DATA_OFFSET (FlashInstance->RegionBaseAddress + Offset, + FlashInstance->StartLba + Lba, + FlashInstance->Media.BlockSize); + + CopyMem ((UINTN *)DataOffset, Buffer, *NumBytes); + } + + return EFI_SUCCESS; } /** @@ -975,6 +1003,9 @@ MvFvbConfigureFlashInstance ( ) { EFI_STATUS Status; + UINTN *NumBytes; + UINTN DataOffset; + UINTN VariableSize, FtwWorkingSize, FtwSpareSize, MemorySize; // Locate SPI protocols @@ -1009,25 +1040,62 @@ MvFvbConfigureFlashInstance ( } // Fill remaining flash description - FlashInstance->DeviceBaseAddress = PcdGet64 (PcdSpiMemoryBase); - FlashInstance->RegionBaseAddress = FixedPcdGet64 (PcdFlashNvStorageVariableBase64); - FlashInstance->FvbOffset = FlashInstance->RegionBaseAddress - - FlashInstance->DeviceBaseAddress; - FlashInstance->FvbSize = PcdGet32(PcdFlashNvStorageVariableSize) + - PcdGet32(PcdFlashNvStorageFtwWorkingSize) + - PcdGet32(PcdFlashNvStorageFtwSpareSize); + VariableSize = PcdGet32 (PcdFlashNvStorageVariableSize); + FtwWorkingSize = PcdGet32 (PcdFlashNvStorageFtwWorkingSize); + FtwSpareSize = PcdGet32 (PcdFlashNvStorageFtwSpareSize); + + FlashInstance->IsMemoryMapped = PcdGetBool (PcdSpiMemoryMapped); + FlashInstance->FvbSize = VariableSize + FtwWorkingSize + FtwSpareSize; + FlashInstance->FvbOffset = PcdGet32 (PcdSpiVariableOffset); FlashInstance->Media.MediaId = 0; FlashInstance->Media.BlockSize = FlashInstance->SpiDevice.Info->SectorSize; FlashInstance->Media.LastBlock = FlashInstance->Size / FlashInstance->Media.BlockSize - 1; + if (FlashInstance->IsMemoryMapped) { + FlashInstance->DeviceBaseAddress = PcdGet64 (PcdSpiMemoryBase); + FlashInstance->RegionBaseAddress = PcdGet64 (PcdFlashNvStorageVariableBase64); + } else { + MemorySize = EFI_SIZE_TO_PAGES (FlashInstance->FvbSize); + + // FaultTolerantWriteDxe requires memory to be aligned to FtwWorkingSize + FlashInstance->RegionBaseAddress = (UINTN) AllocateAlignedRuntimePages (MemorySize, + SIZE_64KB); + if (FlashInstance->RegionBaseAddress == (UINTN) NULL) { + return EFI_OUT_OF_RESOURCES; + } + + PcdSet64 (PcdFlashNvStorageVariableBase64, + (UINT64) FlashInstance->RegionBaseAddress); + PcdSet64 (PcdFlashNvStorageFtwWorkingBase64, + (UINT64) FlashInstance->RegionBaseAddress + + VariableSize); + PcdSet64 (PcdFlashNvStorageFtwSpareBase64, + (UINT64) FlashInstance->RegionBaseAddress + + VariableSize + + FtwWorkingSize); + + // Fill the buffer with data from flash + DataOffset = GET_DATA_OFFSET (FlashInstance->FvbOffset, + FlashInstance->StartLba, + FlashInstance->Media.BlockSize); + *NumBytes = FlashInstance->FvbSize; + Status = FlashInstance->SpiFlashProtocol->Read (&FlashInstance->SpiDevice, + DataOffset, + *NumBytes, + (VOID *)FlashInstance->RegionBaseAddress); + if (EFI_ERROR (Status)) { + goto ErrorFreeAllocatedPages; + } + } + Status = gBS->InstallMultipleProtocolInterfaces (&FlashInstance->Handle, &gEfiDevicePathProtocolGuid, &FlashInstance->DevicePath, &gEfiFirmwareVolumeBlockProtocolGuid, &FlashInstance->FvbProtocol, NULL); if (EFI_ERROR (Status)) { - return Status; + goto ErrorFreeAllocatedPages; } Status = MvFvbPrepareFvHeader (FlashInstance); @@ -1043,6 +1111,12 @@ ErrorPrepareFvbHeader: &gEfiFirmwareVolumeBlockProtocolGuid, NULL); +ErrorFreeAllocatedPages: + if (!FlashInstance->IsMemoryMapped) { + FreeAlignedPages ((VOID *)FlashInstance->RegionBaseAddress, + MemorySize); + } + return Status; } @@ -1094,24 +1168,27 @@ MvFvbEntryPoint ( // // Declare the Non-Volatile storage as EFI_MEMORY_RUNTIME // - RuntimeMmioRegionSize = mFvbDevice->FvbSize; RegionBaseAddress = mFvbDevice->RegionBaseAddress; - Status = gDS->AddMemorySpace (EfiGcdMemoryTypeMemoryMappedIo, - RegionBaseAddress, - RuntimeMmioRegionSize, - EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); - if (EFI_ERROR (Status)) { - DEBUG ((DEBUG_ERROR, "%a: Failed to add memory space\n", __FUNCTION__)); - goto ErrorAddSpace; - } + if (mFvbDevice->IsMemoryMapped) { + RuntimeMmioRegionSize = mFvbDevice->FvbSize; + Status = gDS->AddMemorySpace (EfiGcdMemoryTypeMemoryMappedIo, + RegionBaseAddress, + RuntimeMmioRegionSize, + EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, "%a: Failed to add memory space\n", __FUNCTION__)); + goto ErrorAddSpace; + } - Status = gDS->SetMemorySpaceAttributes (RegionBaseAddress, - RuntimeMmioRegionSize, - EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); - if (EFI_ERROR (Status)) { - DEBUG ((DEBUG_ERROR, "%a: Failed to set memory attributes\n", __FUNCTION__)); - goto ErrorSetMemAttr; + + Status = gDS->SetMemorySpaceAttributes (RegionBaseAddress, + RuntimeMmioRegionSize, + EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, "%a: Failed to set memory attributes\n", __FUNCTION__)); + goto ErrorSetMemAttr; + } } // -- 2.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [edk2-platforms: PATCH v2 3/3] Marvell/Drivers: Add non-mmio mode to MvFvbDxe 2019-04-24 6:51 ` [edk2-platforms: PATCH v2 3/3] Marvell/Drivers: Add non-mmio mode to MvFvbDxe Marcin Wojtas @ 2019-04-24 7:11 ` Ard Biesheuvel 2019-04-24 13:01 ` Ard Biesheuvel 0 siblings, 1 reply; 9+ messages in thread From: Ard Biesheuvel @ 2019-04-24 7:11 UTC (permalink / raw) To: Marcin Wojtas Cc: edk2-devel-groups-io, Leif Lindholm, Jan Dąbroś, Grzegorz Jaszczyk, Kostya Porotchkin, Jici Gao, Kornel Duleba On Wed, 24 Apr 2019 at 08:52, Marcin Wojtas <mw@semihalf.com> wrote: > > From: Kornel Duleba <mindal@semihalf.com> > > This path enables support for reading variables directly from flash without > relying on it to be memory mapped. It adds PcdSpiMemoryMapped PCD that > allows to switch between the modes. When in non-memory-mapped mode the > driver will copy the variables from flash to previously allocated buffer > and set PcdFlashNvStorageVariableBase64, PcdFlashNvStorageFtwWorkingBase64 > and PcdFlashNvStorageFtwSpareBase64 accordingly. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Marcin Wojtas <mw@semihalf.com> > --- > Silicon/Marvell/Marvell.dec | 8 ++ > Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 10 +- > Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf | 17 ++- > Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.h | 1 + > Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c | 135 +++++++++++++++----- > 5 files changed, 135 insertions(+), 36 deletions(-) > > diff --git a/Silicon/Marvell/Marvell.dec b/Silicon/Marvell/Marvell.dec > index 7210ba2..a23c329 100644 > --- a/Silicon/Marvell/Marvell.dec > +++ b/Silicon/Marvell/Marvell.dec > @@ -58,6 +58,12 @@ > > gMarvellFvbDxeGuid = { 0x42903750, 0x7e61, 0x4aaf, { 0x83, 0x29, 0xbf, 0x42, 0x36, 0x4e, 0x24, 0x85 } } > gMarvellSpiFlashDxeGuid = { 0x49d7fb74, 0x306d, 0x42bd, { 0x94, 0xc8, 0xc0, 0xc5, 0x4b, 0x18, 0x1d, 0xd7 } } > + # > + # Generic FaultTolerantWriteDxe driver use variables, > + # whose setting is done in MvFvbDxe driver in case > + # the SPI contents are not mapped in memory. > + # > + gFaultTolerantWriteDxeFileGuid = { 0xfe5cea76, 0x4f72, 0x49e8, { 0x98, 0x6f, 0x2c, 0xd8, 0x99, 0xdf, 0xfe, 0x5d} } > > [LibraryClasses] > ArmadaBoardDescLib|Include/Library/ArmadaBoardDescLib.h > @@ -140,6 +146,8 @@ > #SPI > gMarvellTokenSpaceGuid.PcdSpiRegBase|0|UINT32|0x3000051 > gMarvellTokenSpaceGuid.PcdSpiMemoryBase|0|UINT64|0x3000059 > + gMarvellTokenSpaceGuid.PcdSpiMemoryMapped|TRUE|BOOLEAN|0x3000060 > + gMarvellTokenSpaceGuid.PcdSpiVariableOffset|0|UINT32|0x3000061 > gMarvellTokenSpaceGuid.PcdSpiMaxFrequency|0|UINT32|0x30000052 > gMarvellTokenSpaceGuid.PcdSpiClockFrequency|0|UINT32|0x30000053 > > diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc > index ca3de2e..d53d128 100644 > --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc > +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc > @@ -256,6 +256,11 @@ > # USB support > gEfiMdeModulePkgTokenSpaceGuid.PcdTurnOffUsbLegacySupport|TRUE > > +[PcdsDynamicDefault.common] > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0xF93C0000 > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64|0xF93E0000 > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64|0xF93D0000 > + > [PcdsFixedAtBuild.common] > gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"MARVELL_EFI" > gArmPlatformTokenSpaceGuid.PcdCoreCount|4 > @@ -396,11 +401,10 @@ > # Variable store - default values > # > gMarvellTokenSpaceGuid.PcdSpiMemoryBase|0xF9000000 > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0xF93C0000 > + gMarvellTokenSpaceGuid.PcdSpiMemoryMapped|TRUE > + gMarvellTokenSpaceGuid.PcdSpiVariableOffset|0x3C0000 > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00010000 > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64|0xF93D0000 > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00010000 > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64|0xF93E0000 > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00010000 > > !if $(CAPSULE_ENABLE) > diff --git a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf > index ef10bfd..c85e8a6 100644 > --- a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf > +++ b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf > @@ -76,13 +76,22 @@ > gMarvellSpiMasterProtocolGuid > > [FixedPcd] > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64 > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64 > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize > gMarvellTokenSpaceGuid.PcdSpiMemoryBase > + gMarvellTokenSpaceGuid.PcdSpiMemoryMapped > + gMarvellTokenSpaceGuid.PcdSpiVariableOffset > + > +[Pcd] > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64 > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64 > > [Depex] > - gEfiCpuArchProtocolGuid > + # > + # Generic FaultTolerantWriteDxe driver use variables, > + # whose setting is done in MvFvbDxe driver in case > + # the SPI contents are not mapped in memory. > + # > + BEFORE gFaultTolerantWriteDxeFileGuid Apologies for not spotting this before, but there is a problem here: FaultTolerantWriteDxe.inf does not depend on gEfiCpuArchProtocolGuid, but we do, and so we could now be dispatched before gEfiCpuArchProtocolGuid becomes available. This means you need to update the code to deal with that (or explain to me how if it already does) > diff --git a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.h b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.h > index 31e6e44..e8df9a5 100644 > --- a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.h > +++ b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.h > @@ -55,6 +55,7 @@ typedef struct { > > UINT32 Signature; > > + BOOLEAN IsMemoryMapped; > UINTN DeviceBaseAddress; > UINTN RegionBaseAddress; > UINTN Size; > diff --git a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c > index cb006cd..b4fd29c 100644 > --- a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c > +++ b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c > @@ -52,6 +52,7 @@ STATIC CONST FVB_DEVICE mMvFvbFlashInstanceTemplate = { > > FVB_FLASH_SIGNATURE, // Signature > > + FALSE, // IsMemoryMapped ... NEED TO BE FILLED > 0, // DeviceBaseAddress ... NEED TO BE FILLED > 0, // RegionBaseAddress ... NEED TO BE FILLED > SIZE_256KB, // Size > @@ -175,11 +176,14 @@ MvFvbInitFvAndVariableStoreHeaders ( > FirmwareVolumeHeader->Attributes = EFI_FVB2_READ_ENABLED_CAP | > EFI_FVB2_READ_STATUS | > EFI_FVB2_STICKY_WRITE | > - EFI_FVB2_MEMORY_MAPPED | > EFI_FVB2_ERASE_POLARITY | > EFI_FVB2_WRITE_STATUS | > EFI_FVB2_WRITE_ENABLED_CAP; > > + if (FlashInstance->IsMemoryMapped) { > + FirmwareVolumeHeader->Attributes |= EFI_FVB2_MEMORY_MAPPED; > + } > + > FirmwareVolumeHeader->HeaderLength = sizeof (EFI_FIRMWARE_VOLUME_HEADER) + > sizeof (EFI_FV_BLOCK_MAP_ENTRY); > FirmwareVolumeHeader->Revision = EFI_FVH_REVISION; > @@ -349,10 +353,13 @@ MvFvbSetAttributes ( > EFI_FVB_ATTRIBUTES_2 OldAttributes; > EFI_FVB_ATTRIBUTES_2 FlashFvbAttributes; > EFI_FVB_ATTRIBUTES_2 UnchangedAttributes; > + FVB_DEVICE *FlashInstance; > UINT32 Capabilities; > UINT32 OldStatus; > UINT32 NewStatus; > > + FlashInstance = INSTANCE_FROM_FVB_THIS (This); > + > // > // Obtain attributes from FVB header > // > @@ -369,12 +376,15 @@ MvFvbSetAttributes ( > EFI_FVB2_WRITE_ENABLED_CAP | \ > EFI_FVB2_LOCK_CAP | \ > EFI_FVB2_STICKY_WRITE | \ > - EFI_FVB2_MEMORY_MAPPED | \ > EFI_FVB2_ERASE_POLARITY | \ > EFI_FVB2_READ_LOCK_CAP | \ > EFI_FVB2_WRITE_LOCK_CAP | \ > EFI_FVB2_ALIGNMENT; > > + if (FlashInstance->IsMemoryMapped) { > + UnchangedAttributes |= EFI_FVB2_MEMORY_MAPPED; > + } > + > // > // Some attributes of FV is read only can *not* be set > // > @@ -692,6 +702,7 @@ MvFvbWrite ( > IN UINT8 *Buffer > ) > { > + EFI_STATUS Status; > FVB_DEVICE *FlashInstance; > UINTN DataOffset; > > @@ -701,10 +712,27 @@ MvFvbWrite ( > FlashInstance->StartLba + Lba, > FlashInstance->Media.BlockSize); > > - return FlashInstance->SpiFlashProtocol->Write (&FlashInstance->SpiDevice, > - DataOffset, > - *NumBytes, > - Buffer); > + Status = FlashInstance->SpiFlashProtocol->Write (&FlashInstance->SpiDevice, > + DataOffset, > + *NumBytes, > + Buffer); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, > + "%a: Failed to write to Spi device\n", > + __FUNCTION__)); > + return Status; > + } > + > + // Update shadow buffer > + if (!FlashInstance->IsMemoryMapped) { > + DataOffset = GET_DATA_OFFSET (FlashInstance->RegionBaseAddress + Offset, > + FlashInstance->StartLba + Lba, > + FlashInstance->Media.BlockSize); > + > + CopyMem ((UINTN *)DataOffset, Buffer, *NumBytes); > + } > + > + return EFI_SUCCESS; > } > > /** > @@ -975,6 +1003,9 @@ MvFvbConfigureFlashInstance ( > ) > { > EFI_STATUS Status; > + UINTN *NumBytes; > + UINTN DataOffset; > + UINTN VariableSize, FtwWorkingSize, FtwSpareSize, MemorySize; > > > // Locate SPI protocols > @@ -1009,25 +1040,62 @@ MvFvbConfigureFlashInstance ( > } > > // Fill remaining flash description > - FlashInstance->DeviceBaseAddress = PcdGet64 (PcdSpiMemoryBase); > - FlashInstance->RegionBaseAddress = FixedPcdGet64 (PcdFlashNvStorageVariableBase64); > - FlashInstance->FvbOffset = FlashInstance->RegionBaseAddress - > - FlashInstance->DeviceBaseAddress; > - FlashInstance->FvbSize = PcdGet32(PcdFlashNvStorageVariableSize) + > - PcdGet32(PcdFlashNvStorageFtwWorkingSize) + > - PcdGet32(PcdFlashNvStorageFtwSpareSize); > + VariableSize = PcdGet32 (PcdFlashNvStorageVariableSize); > + FtwWorkingSize = PcdGet32 (PcdFlashNvStorageFtwWorkingSize); > + FtwSpareSize = PcdGet32 (PcdFlashNvStorageFtwSpareSize); > + > + FlashInstance->IsMemoryMapped = PcdGetBool (PcdSpiMemoryMapped); > + FlashInstance->FvbSize = VariableSize + FtwWorkingSize + FtwSpareSize; > + FlashInstance->FvbOffset = PcdGet32 (PcdSpiVariableOffset); > > FlashInstance->Media.MediaId = 0; > FlashInstance->Media.BlockSize = FlashInstance->SpiDevice.Info->SectorSize; > FlashInstance->Media.LastBlock = FlashInstance->Size / > FlashInstance->Media.BlockSize - 1; > > + if (FlashInstance->IsMemoryMapped) { > + FlashInstance->DeviceBaseAddress = PcdGet64 (PcdSpiMemoryBase); > + FlashInstance->RegionBaseAddress = PcdGet64 (PcdFlashNvStorageVariableBase64); > + } else { > + MemorySize = EFI_SIZE_TO_PAGES (FlashInstance->FvbSize); > + > + // FaultTolerantWriteDxe requires memory to be aligned to FtwWorkingSize > + FlashInstance->RegionBaseAddress = (UINTN) AllocateAlignedRuntimePages (MemorySize, > + SIZE_64KB); > + if (FlashInstance->RegionBaseAddress == (UINTN) NULL) { > + return EFI_OUT_OF_RESOURCES; > + } > + > + PcdSet64 (PcdFlashNvStorageVariableBase64, > + (UINT64) FlashInstance->RegionBaseAddress); > + PcdSet64 (PcdFlashNvStorageFtwWorkingBase64, > + (UINT64) FlashInstance->RegionBaseAddress > + + VariableSize); > + PcdSet64 (PcdFlashNvStorageFtwSpareBase64, > + (UINT64) FlashInstance->RegionBaseAddress > + + VariableSize > + + FtwWorkingSize); > + > + // Fill the buffer with data from flash > + DataOffset = GET_DATA_OFFSET (FlashInstance->FvbOffset, > + FlashInstance->StartLba, > + FlashInstance->Media.BlockSize); > + *NumBytes = FlashInstance->FvbSize; > + Status = FlashInstance->SpiFlashProtocol->Read (&FlashInstance->SpiDevice, > + DataOffset, > + *NumBytes, > + (VOID *)FlashInstance->RegionBaseAddress); > + if (EFI_ERROR (Status)) { > + goto ErrorFreeAllocatedPages; > + } > + } > + > Status = gBS->InstallMultipleProtocolInterfaces (&FlashInstance->Handle, > &gEfiDevicePathProtocolGuid, &FlashInstance->DevicePath, > &gEfiFirmwareVolumeBlockProtocolGuid, &FlashInstance->FvbProtocol, > NULL); > if (EFI_ERROR (Status)) { > - return Status; > + goto ErrorFreeAllocatedPages; > } > > Status = MvFvbPrepareFvHeader (FlashInstance); > @@ -1043,6 +1111,12 @@ ErrorPrepareFvbHeader: > &gEfiFirmwareVolumeBlockProtocolGuid, > NULL); > > +ErrorFreeAllocatedPages: > + if (!FlashInstance->IsMemoryMapped) { > + FreeAlignedPages ((VOID *)FlashInstance->RegionBaseAddress, > + MemorySize); > + } > + > return Status; > } > > @@ -1094,24 +1168,27 @@ MvFvbEntryPoint ( > // > // Declare the Non-Volatile storage as EFI_MEMORY_RUNTIME > // > - RuntimeMmioRegionSize = mFvbDevice->FvbSize; > RegionBaseAddress = mFvbDevice->RegionBaseAddress; > > - Status = gDS->AddMemorySpace (EfiGcdMemoryTypeMemoryMappedIo, > - RegionBaseAddress, > - RuntimeMmioRegionSize, > - EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); > - if (EFI_ERROR (Status)) { > - DEBUG ((DEBUG_ERROR, "%a: Failed to add memory space\n", __FUNCTION__)); > - goto ErrorAddSpace; > - } > + if (mFvbDevice->IsMemoryMapped) { > + RuntimeMmioRegionSize = mFvbDevice->FvbSize; > + Status = gDS->AddMemorySpace (EfiGcdMemoryTypeMemoryMappedIo, > + RegionBaseAddress, > + RuntimeMmioRegionSize, > + EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "%a: Failed to add memory space\n", __FUNCTION__)); > + goto ErrorAddSpace; > + } > > - Status = gDS->SetMemorySpaceAttributes (RegionBaseAddress, > - RuntimeMmioRegionSize, > - EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); > - if (EFI_ERROR (Status)) { > - DEBUG ((DEBUG_ERROR, "%a: Failed to set memory attributes\n", __FUNCTION__)); > - goto ErrorSetMemAttr; > + > + Status = gDS->SetMemorySpaceAttributes (RegionBaseAddress, > + RuntimeMmioRegionSize, > + EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "%a: Failed to set memory attributes\n", __FUNCTION__)); > + goto ErrorSetMemAttr; > + } > } > > // > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [edk2-platforms: PATCH v2 3/3] Marvell/Drivers: Add non-mmio mode to MvFvbDxe 2019-04-24 7:11 ` Ard Biesheuvel @ 2019-04-24 13:01 ` Ard Biesheuvel 2019-04-24 13:48 ` Marcin Wojtas 0 siblings, 1 reply; 9+ messages in thread From: Ard Biesheuvel @ 2019-04-24 13:01 UTC (permalink / raw) To: Marcin Wojtas Cc: edk2-devel-groups-io, Leif Lindholm, Jan Dąbroś, Grzegorz Jaszczyk, Kostya Porotchkin, Jici Gao, Kornel Duleba On Wed, 24 Apr 2019 at 09:11, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > On Wed, 24 Apr 2019 at 08:52, Marcin Wojtas <mw@semihalf.com> wrote: > > > > From: Kornel Duleba <mindal@semihalf.com> > > > > This path enables support for reading variables directly from flash without > > relying on it to be memory mapped. It adds PcdSpiMemoryMapped PCD that > > allows to switch between the modes. When in non-memory-mapped mode the > > driver will copy the variables from flash to previously allocated buffer > > and set PcdFlashNvStorageVariableBase64, PcdFlashNvStorageFtwWorkingBase64 > > and PcdFlashNvStorageFtwSpareBase64 accordingly. > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Marcin Wojtas <mw@semihalf.com> > > --- > > Silicon/Marvell/Marvell.dec | 8 ++ > > Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 10 +- > > Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf | 17 ++- > > Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.h | 1 + > > Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c | 135 +++++++++++++++----- > > 5 files changed, 135 insertions(+), 36 deletions(-) > > > > diff --git a/Silicon/Marvell/Marvell.dec b/Silicon/Marvell/Marvell.dec > > index 7210ba2..a23c329 100644 > > --- a/Silicon/Marvell/Marvell.dec > > +++ b/Silicon/Marvell/Marvell.dec > > @@ -58,6 +58,12 @@ > > > > gMarvellFvbDxeGuid = { 0x42903750, 0x7e61, 0x4aaf, { 0x83, 0x29, 0xbf, 0x42, 0x36, 0x4e, 0x24, 0x85 } } > > gMarvellSpiFlashDxeGuid = { 0x49d7fb74, 0x306d, 0x42bd, { 0x94, 0xc8, 0xc0, 0xc5, 0x4b, 0x18, 0x1d, 0xd7 } } > > + # > > + # Generic FaultTolerantWriteDxe driver use variables, > > + # whose setting is done in MvFvbDxe driver in case > > + # the SPI contents are not mapped in memory. > > + # > > + gFaultTolerantWriteDxeFileGuid = { 0xfe5cea76, 0x4f72, 0x49e8, { 0x98, 0x6f, 0x2c, 0xd8, 0x99, 0xdf, 0xfe, 0x5d} } > > > > [LibraryClasses] > > ArmadaBoardDescLib|Include/Library/ArmadaBoardDescLib.h > > @@ -140,6 +146,8 @@ > > #SPI > > gMarvellTokenSpaceGuid.PcdSpiRegBase|0|UINT32|0x3000051 > > gMarvellTokenSpaceGuid.PcdSpiMemoryBase|0|UINT64|0x3000059 > > + gMarvellTokenSpaceGuid.PcdSpiMemoryMapped|TRUE|BOOLEAN|0x3000060 > > + gMarvellTokenSpaceGuid.PcdSpiVariableOffset|0|UINT32|0x3000061 > > gMarvellTokenSpaceGuid.PcdSpiMaxFrequency|0|UINT32|0x30000052 > > gMarvellTokenSpaceGuid.PcdSpiClockFrequency|0|UINT32|0x30000053 > > > > diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc > > index ca3de2e..d53d128 100644 > > --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc > > +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc > > @@ -256,6 +256,11 @@ > > # USB support > > gEfiMdeModulePkgTokenSpaceGuid.PcdTurnOffUsbLegacySupport|TRUE > > > > +[PcdsDynamicDefault.common] > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0xF93C0000 > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64|0xF93E0000 > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64|0xF93D0000 > > + > > [PcdsFixedAtBuild.common] > > gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"MARVELL_EFI" > > gArmPlatformTokenSpaceGuid.PcdCoreCount|4 > > @@ -396,11 +401,10 @@ > > # Variable store - default values > > # > > gMarvellTokenSpaceGuid.PcdSpiMemoryBase|0xF9000000 > > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0xF93C0000 > > + gMarvellTokenSpaceGuid.PcdSpiMemoryMapped|TRUE > > + gMarvellTokenSpaceGuid.PcdSpiVariableOffset|0x3C0000 > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00010000 > > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64|0xF93D0000 > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00010000 > > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64|0xF93E0000 > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00010000 > > > > !if $(CAPSULE_ENABLE) > > diff --git a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf > > index ef10bfd..c85e8a6 100644 > > --- a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf > > +++ b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf > > @@ -76,13 +76,22 @@ > > gMarvellSpiMasterProtocolGuid > > > > [FixedPcd] > > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize > > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64 > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize > > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64 > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize > > gMarvellTokenSpaceGuid.PcdSpiMemoryBase > > + gMarvellTokenSpaceGuid.PcdSpiMemoryMapped > > + gMarvellTokenSpaceGuid.PcdSpiVariableOffset > > + > > +[Pcd] > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64 > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64 > > > > [Depex] > > - gEfiCpuArchProtocolGuid > > + # > > + # Generic FaultTolerantWriteDxe driver use variables, > > + # whose setting is done in MvFvbDxe driver in case > > + # the SPI contents are not mapped in memory. > > + # > > + BEFORE gFaultTolerantWriteDxeFileGuid > > Apologies for not spotting this before, but there is a problem here: > FaultTolerantWriteDxe.inf does not depend on gEfiCpuArchProtocolGuid, > but we do, and so we could now be dispatched before > gEfiCpuArchProtocolGuid becomes available. This means you need to > update the code to deal with that (or explain to me how if it already > does) > You should be able to fix this by adding a NULL resolution for EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf to the FTW driver, similar to what you are already doing for the variable driver. > > > diff --git a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.h b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.h > > index 31e6e44..e8df9a5 100644 > > --- a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.h > > +++ b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.h > > @@ -55,6 +55,7 @@ typedef struct { > > > > UINT32 Signature; > > > > + BOOLEAN IsMemoryMapped; > > UINTN DeviceBaseAddress; > > UINTN RegionBaseAddress; > > UINTN Size; > > diff --git a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c > > index cb006cd..b4fd29c 100644 > > --- a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c > > +++ b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c > > @@ -52,6 +52,7 @@ STATIC CONST FVB_DEVICE mMvFvbFlashInstanceTemplate = { > > > > FVB_FLASH_SIGNATURE, // Signature > > > > + FALSE, // IsMemoryMapped ... NEED TO BE FILLED > > 0, // DeviceBaseAddress ... NEED TO BE FILLED > > 0, // RegionBaseAddress ... NEED TO BE FILLED > > SIZE_256KB, // Size > > @@ -175,11 +176,14 @@ MvFvbInitFvAndVariableStoreHeaders ( > > FirmwareVolumeHeader->Attributes = EFI_FVB2_READ_ENABLED_CAP | > > EFI_FVB2_READ_STATUS | > > EFI_FVB2_STICKY_WRITE | > > - EFI_FVB2_MEMORY_MAPPED | > > EFI_FVB2_ERASE_POLARITY | > > EFI_FVB2_WRITE_STATUS | > > EFI_FVB2_WRITE_ENABLED_CAP; > > > > + if (FlashInstance->IsMemoryMapped) { > > + FirmwareVolumeHeader->Attributes |= EFI_FVB2_MEMORY_MAPPED; > > + } > > + > > FirmwareVolumeHeader->HeaderLength = sizeof (EFI_FIRMWARE_VOLUME_HEADER) + > > sizeof (EFI_FV_BLOCK_MAP_ENTRY); > > FirmwareVolumeHeader->Revision = EFI_FVH_REVISION; > > @@ -349,10 +353,13 @@ MvFvbSetAttributes ( > > EFI_FVB_ATTRIBUTES_2 OldAttributes; > > EFI_FVB_ATTRIBUTES_2 FlashFvbAttributes; > > EFI_FVB_ATTRIBUTES_2 UnchangedAttributes; > > + FVB_DEVICE *FlashInstance; > > UINT32 Capabilities; > > UINT32 OldStatus; > > UINT32 NewStatus; > > > > + FlashInstance = INSTANCE_FROM_FVB_THIS (This); > > + > > // > > // Obtain attributes from FVB header > > // > > @@ -369,12 +376,15 @@ MvFvbSetAttributes ( > > EFI_FVB2_WRITE_ENABLED_CAP | \ > > EFI_FVB2_LOCK_CAP | \ > > EFI_FVB2_STICKY_WRITE | \ > > - EFI_FVB2_MEMORY_MAPPED | \ > > EFI_FVB2_ERASE_POLARITY | \ > > EFI_FVB2_READ_LOCK_CAP | \ > > EFI_FVB2_WRITE_LOCK_CAP | \ > > EFI_FVB2_ALIGNMENT; > > > > + if (FlashInstance->IsMemoryMapped) { > > + UnchangedAttributes |= EFI_FVB2_MEMORY_MAPPED; > > + } > > + > > // > > // Some attributes of FV is read only can *not* be set > > // > > @@ -692,6 +702,7 @@ MvFvbWrite ( > > IN UINT8 *Buffer > > ) > > { > > + EFI_STATUS Status; > > FVB_DEVICE *FlashInstance; > > UINTN DataOffset; > > > > @@ -701,10 +712,27 @@ MvFvbWrite ( > > FlashInstance->StartLba + Lba, > > FlashInstance->Media.BlockSize); > > > > - return FlashInstance->SpiFlashProtocol->Write (&FlashInstance->SpiDevice, > > - DataOffset, > > - *NumBytes, > > - Buffer); > > + Status = FlashInstance->SpiFlashProtocol->Write (&FlashInstance->SpiDevice, > > + DataOffset, > > + *NumBytes, > > + Buffer); > > + if (EFI_ERROR (Status)) { > > + DEBUG ((DEBUG_ERROR, > > + "%a: Failed to write to Spi device\n", > > + __FUNCTION__)); > > + return Status; > > + } > > + > > + // Update shadow buffer > > + if (!FlashInstance->IsMemoryMapped) { > > + DataOffset = GET_DATA_OFFSET (FlashInstance->RegionBaseAddress + Offset, > > + FlashInstance->StartLba + Lba, > > + FlashInstance->Media.BlockSize); > > + > > + CopyMem ((UINTN *)DataOffset, Buffer, *NumBytes); > > + } > > + > > + return EFI_SUCCESS; > > } > > > > /** > > @@ -975,6 +1003,9 @@ MvFvbConfigureFlashInstance ( > > ) > > { > > EFI_STATUS Status; > > + UINTN *NumBytes; > > + UINTN DataOffset; > > + UINTN VariableSize, FtwWorkingSize, FtwSpareSize, MemorySize; > > > > > > // Locate SPI protocols > > @@ -1009,25 +1040,62 @@ MvFvbConfigureFlashInstance ( > > } > > > > // Fill remaining flash description > > - FlashInstance->DeviceBaseAddress = PcdGet64 (PcdSpiMemoryBase); > > - FlashInstance->RegionBaseAddress = FixedPcdGet64 (PcdFlashNvStorageVariableBase64); > > - FlashInstance->FvbOffset = FlashInstance->RegionBaseAddress - > > - FlashInstance->DeviceBaseAddress; > > - FlashInstance->FvbSize = PcdGet32(PcdFlashNvStorageVariableSize) + > > - PcdGet32(PcdFlashNvStorageFtwWorkingSize) + > > - PcdGet32(PcdFlashNvStorageFtwSpareSize); > > + VariableSize = PcdGet32 (PcdFlashNvStorageVariableSize); > > + FtwWorkingSize = PcdGet32 (PcdFlashNvStorageFtwWorkingSize); > > + FtwSpareSize = PcdGet32 (PcdFlashNvStorageFtwSpareSize); > > + > > + FlashInstance->IsMemoryMapped = PcdGetBool (PcdSpiMemoryMapped); > > + FlashInstance->FvbSize = VariableSize + FtwWorkingSize + FtwSpareSize; > > + FlashInstance->FvbOffset = PcdGet32 (PcdSpiVariableOffset); > > > > FlashInstance->Media.MediaId = 0; > > FlashInstance->Media.BlockSize = FlashInstance->SpiDevice.Info->SectorSize; > > FlashInstance->Media.LastBlock = FlashInstance->Size / > > FlashInstance->Media.BlockSize - 1; > > > > + if (FlashInstance->IsMemoryMapped) { > > + FlashInstance->DeviceBaseAddress = PcdGet64 (PcdSpiMemoryBase); > > + FlashInstance->RegionBaseAddress = PcdGet64 (PcdFlashNvStorageVariableBase64); > > + } else { > > + MemorySize = EFI_SIZE_TO_PAGES (FlashInstance->FvbSize); > > + > > + // FaultTolerantWriteDxe requires memory to be aligned to FtwWorkingSize > > + FlashInstance->RegionBaseAddress = (UINTN) AllocateAlignedRuntimePages (MemorySize, > > + SIZE_64KB); > > + if (FlashInstance->RegionBaseAddress == (UINTN) NULL) { > > + return EFI_OUT_OF_RESOURCES; > > + } > > + > > + PcdSet64 (PcdFlashNvStorageVariableBase64, > > + (UINT64) FlashInstance->RegionBaseAddress); > > + PcdSet64 (PcdFlashNvStorageFtwWorkingBase64, > > + (UINT64) FlashInstance->RegionBaseAddress > > + + VariableSize); > > + PcdSet64 (PcdFlashNvStorageFtwSpareBase64, > > + (UINT64) FlashInstance->RegionBaseAddress > > + + VariableSize > > + + FtwWorkingSize); > > + > > + // Fill the buffer with data from flash > > + DataOffset = GET_DATA_OFFSET (FlashInstance->FvbOffset, > > + FlashInstance->StartLba, > > + FlashInstance->Media.BlockSize); > > + *NumBytes = FlashInstance->FvbSize; > > + Status = FlashInstance->SpiFlashProtocol->Read (&FlashInstance->SpiDevice, > > + DataOffset, > > + *NumBytes, > > + (VOID *)FlashInstance->RegionBaseAddress); > > + if (EFI_ERROR (Status)) { > > + goto ErrorFreeAllocatedPages; > > + } > > + } > > + > > Status = gBS->InstallMultipleProtocolInterfaces (&FlashInstance->Handle, > > &gEfiDevicePathProtocolGuid, &FlashInstance->DevicePath, > > &gEfiFirmwareVolumeBlockProtocolGuid, &FlashInstance->FvbProtocol, > > NULL); > > if (EFI_ERROR (Status)) { > > - return Status; > > + goto ErrorFreeAllocatedPages; > > } > > > > Status = MvFvbPrepareFvHeader (FlashInstance); > > @@ -1043,6 +1111,12 @@ ErrorPrepareFvbHeader: > > &gEfiFirmwareVolumeBlockProtocolGuid, > > NULL); > > > > +ErrorFreeAllocatedPages: > > + if (!FlashInstance->IsMemoryMapped) { > > + FreeAlignedPages ((VOID *)FlashInstance->RegionBaseAddress, > > + MemorySize); > > + } > > + > > return Status; > > } > > > > @@ -1094,24 +1168,27 @@ MvFvbEntryPoint ( > > // > > // Declare the Non-Volatile storage as EFI_MEMORY_RUNTIME > > // > > - RuntimeMmioRegionSize = mFvbDevice->FvbSize; > > RegionBaseAddress = mFvbDevice->RegionBaseAddress; > > > > - Status = gDS->AddMemorySpace (EfiGcdMemoryTypeMemoryMappedIo, > > - RegionBaseAddress, > > - RuntimeMmioRegionSize, > > - EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); > > - if (EFI_ERROR (Status)) { > > - DEBUG ((DEBUG_ERROR, "%a: Failed to add memory space\n", __FUNCTION__)); > > - goto ErrorAddSpace; > > - } > > + if (mFvbDevice->IsMemoryMapped) { > > + RuntimeMmioRegionSize = mFvbDevice->FvbSize; > > + Status = gDS->AddMemorySpace (EfiGcdMemoryTypeMemoryMappedIo, > > + RegionBaseAddress, > > + RuntimeMmioRegionSize, > > + EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); > > + if (EFI_ERROR (Status)) { > > + DEBUG ((DEBUG_ERROR, "%a: Failed to add memory space\n", __FUNCTION__)); > > + goto ErrorAddSpace; > > + } > > > > - Status = gDS->SetMemorySpaceAttributes (RegionBaseAddress, > > - RuntimeMmioRegionSize, > > - EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); > > - if (EFI_ERROR (Status)) { > > - DEBUG ((DEBUG_ERROR, "%a: Failed to set memory attributes\n", __FUNCTION__)); > > - goto ErrorSetMemAttr; > > + > > + Status = gDS->SetMemorySpaceAttributes (RegionBaseAddress, > > + RuntimeMmioRegionSize, > > + EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); > > + if (EFI_ERROR (Status)) { > > + DEBUG ((DEBUG_ERROR, "%a: Failed to set memory attributes\n", __FUNCTION__)); > > + goto ErrorSetMemAttr; > > + } > > } > > > > // > > -- > > 2.7.4 > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [edk2-platforms: PATCH v2 3/3] Marvell/Drivers: Add non-mmio mode to MvFvbDxe 2019-04-24 13:01 ` Ard Biesheuvel @ 2019-04-24 13:48 ` Marcin Wojtas 2019-04-24 13:51 ` Ard Biesheuvel 0 siblings, 1 reply; 9+ messages in thread From: Marcin Wojtas @ 2019-04-24 13:48 UTC (permalink / raw) To: Ard Biesheuvel Cc: edk2-devel-groups-io, Leif Lindholm, Jan Dąbroś, Grzegorz Jaszczyk, Kostya Porotchkin, Jici Gao, Kornel Duleba Hi Ard, śr., 24 kwi 2019 o 15:02 Ard Biesheuvel <ard.biesheuvel@linaro.org> napisał(a): > > On Wed, 24 Apr 2019 at 09:11, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > > > On Wed, 24 Apr 2019 at 08:52, Marcin Wojtas <mw@semihalf.com> wrote: > > > > > > From: Kornel Duleba <mindal@semihalf.com> > > > > > > This path enables support for reading variables directly from flash without > > > relying on it to be memory mapped. It adds PcdSpiMemoryMapped PCD that > > > allows to switch between the modes. When in non-memory-mapped mode the > > > driver will copy the variables from flash to previously allocated buffer > > > and set PcdFlashNvStorageVariableBase64, PcdFlashNvStorageFtwWorkingBase64 > > > and PcdFlashNvStorageFtwSpareBase64 accordingly. > > > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > > Signed-off-by: Marcin Wojtas <mw@semihalf.com> > > > --- > > > Silicon/Marvell/Marvell.dec | 8 ++ > > > Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 10 +- > > > Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf | 17 ++- > > > Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.h | 1 + > > > Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c | 135 +++++++++++++++----- > > > 5 files changed, 135 insertions(+), 36 deletions(-) > > > > > > diff --git a/Silicon/Marvell/Marvell.dec b/Silicon/Marvell/Marvell.dec > > > index 7210ba2..a23c329 100644 > > > --- a/Silicon/Marvell/Marvell.dec > > > +++ b/Silicon/Marvell/Marvell.dec > > > @@ -58,6 +58,12 @@ > > > > > > gMarvellFvbDxeGuid = { 0x42903750, 0x7e61, 0x4aaf, { 0x83, 0x29, 0xbf, 0x42, 0x36, 0x4e, 0x24, 0x85 } } > > > gMarvellSpiFlashDxeGuid = { 0x49d7fb74, 0x306d, 0x42bd, { 0x94, 0xc8, 0xc0, 0xc5, 0x4b, 0x18, 0x1d, 0xd7 } } > > > + # > > > + # Generic FaultTolerantWriteDxe driver use variables, > > > + # whose setting is done in MvFvbDxe driver in case > > > + # the SPI contents are not mapped in memory. > > > + # > > > + gFaultTolerantWriteDxeFileGuid = { 0xfe5cea76, 0x4f72, 0x49e8, { 0x98, 0x6f, 0x2c, 0xd8, 0x99, 0xdf, 0xfe, 0x5d} } > > > > > > [LibraryClasses] > > > ArmadaBoardDescLib|Include/Library/ArmadaBoardDescLib.h > > > @@ -140,6 +146,8 @@ > > > #SPI > > > gMarvellTokenSpaceGuid.PcdSpiRegBase|0|UINT32|0x3000051 > > > gMarvellTokenSpaceGuid.PcdSpiMemoryBase|0|UINT64|0x3000059 > > > + gMarvellTokenSpaceGuid.PcdSpiMemoryMapped|TRUE|BOOLEAN|0x3000060 > > > + gMarvellTokenSpaceGuid.PcdSpiVariableOffset|0|UINT32|0x3000061 > > > gMarvellTokenSpaceGuid.PcdSpiMaxFrequency|0|UINT32|0x30000052 > > > gMarvellTokenSpaceGuid.PcdSpiClockFrequency|0|UINT32|0x30000053 > > > > > > diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc > > > index ca3de2e..d53d128 100644 > > > --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc > > > +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc > > > @@ -256,6 +256,11 @@ > > > # USB support > > > gEfiMdeModulePkgTokenSpaceGuid.PcdTurnOffUsbLegacySupport|TRUE > > > > > > +[PcdsDynamicDefault.common] > > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0xF93C0000 > > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64|0xF93E0000 > > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64|0xF93D0000 > > > + > > > [PcdsFixedAtBuild.common] > > > gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"MARVELL_EFI" > > > gArmPlatformTokenSpaceGuid.PcdCoreCount|4 > > > @@ -396,11 +401,10 @@ > > > # Variable store - default values > > > # > > > gMarvellTokenSpaceGuid.PcdSpiMemoryBase|0xF9000000 > > > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0xF93C0000 > > > + gMarvellTokenSpaceGuid.PcdSpiMemoryMapped|TRUE > > > + gMarvellTokenSpaceGuid.PcdSpiVariableOffset|0x3C0000 > > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00010000 > > > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64|0xF93D0000 > > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00010000 > > > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64|0xF93E0000 > > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00010000 > > > > > > !if $(CAPSULE_ENABLE) > > > diff --git a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf > > > index ef10bfd..c85e8a6 100644 > > > --- a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf > > > +++ b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf > > > @@ -76,13 +76,22 @@ > > > gMarvellSpiMasterProtocolGuid > > > > > > [FixedPcd] > > > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 > > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize > > > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64 > > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize > > > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64 > > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize > > > gMarvellTokenSpaceGuid.PcdSpiMemoryBase > > > + gMarvellTokenSpaceGuid.PcdSpiMemoryMapped > > > + gMarvellTokenSpaceGuid.PcdSpiVariableOffset > > > + > > > +[Pcd] > > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 > > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64 > > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64 > > > > > > [Depex] > > > - gEfiCpuArchProtocolGuid > > > + # > > > + # Generic FaultTolerantWriteDxe driver use variables, > > > + # whose setting is done in MvFvbDxe driver in case > > > + # the SPI contents are not mapped in memory. > > > + # > > > + BEFORE gFaultTolerantWriteDxeFileGuid > > > > Apologies for not spotting this before, but there is a problem here: > > FaultTolerantWriteDxe.inf does not depend on gEfiCpuArchProtocolGuid, > > but we do, and so we could now be dispatched before > > gEfiCpuArchProtocolGuid becomes available. This means you need to > > update the code to deal with that (or explain to me how if it already > > does) > > > > You should be able to fix this by adding a NULL resolution for > > EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf > > to the FTW driver, similar to what you are already doing for the > variable driver. > Thanks for the hint, I'll try that. I tried to modify dependencies within edk2-platforms to explicitly cover FaultTolerantWriteDxe.inf and gEfiCpuArchProtocolGuid, but did not succeed. With this patch the things work properly, but I am wondering if only by luck. I found following line in BaseTools/Source/Python/UPT/Xml/XmlParser.py: DxeObj.SetDepex("gEfiBdsArchProtocolGuid AND \ngEfiCpuArchProtocolGuid AND\n" + \ it seems not to apply for MvFvbDxe though. So simply it may be an order of entries in Armada7k8k.fdf file. Best regards, Marcin > > > > > diff --git a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.h b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.h > > > index 31e6e44..e8df9a5 100644 > > > --- a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.h > > > +++ b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.h > > > @@ -55,6 +55,7 @@ typedef struct { > > > > > > UINT32 Signature; > > > > > > + BOOLEAN IsMemoryMapped; > > > UINTN DeviceBaseAddress; > > > UINTN RegionBaseAddress; > > > UINTN Size; > > > diff --git a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c > > > index cb006cd..b4fd29c 100644 > > > --- a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c > > > +++ b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c > > > @@ -52,6 +52,7 @@ STATIC CONST FVB_DEVICE mMvFvbFlashInstanceTemplate = { > > > > > > FVB_FLASH_SIGNATURE, // Signature > > > > > > + FALSE, // IsMemoryMapped ... NEED TO BE FILLED > > > 0, // DeviceBaseAddress ... NEED TO BE FILLED > > > 0, // RegionBaseAddress ... NEED TO BE FILLED > > > SIZE_256KB, // Size > > > @@ -175,11 +176,14 @@ MvFvbInitFvAndVariableStoreHeaders ( > > > FirmwareVolumeHeader->Attributes = EFI_FVB2_READ_ENABLED_CAP | > > > EFI_FVB2_READ_STATUS | > > > EFI_FVB2_STICKY_WRITE | > > > - EFI_FVB2_MEMORY_MAPPED | > > > EFI_FVB2_ERASE_POLARITY | > > > EFI_FVB2_WRITE_STATUS | > > > EFI_FVB2_WRITE_ENABLED_CAP; > > > > > > + if (FlashInstance->IsMemoryMapped) { > > > + FirmwareVolumeHeader->Attributes |= EFI_FVB2_MEMORY_MAPPED; > > > + } > > > + > > > FirmwareVolumeHeader->HeaderLength = sizeof (EFI_FIRMWARE_VOLUME_HEADER) + > > > sizeof (EFI_FV_BLOCK_MAP_ENTRY); > > > FirmwareVolumeHeader->Revision = EFI_FVH_REVISION; > > > @@ -349,10 +353,13 @@ MvFvbSetAttributes ( > > > EFI_FVB_ATTRIBUTES_2 OldAttributes; > > > EFI_FVB_ATTRIBUTES_2 FlashFvbAttributes; > > > EFI_FVB_ATTRIBUTES_2 UnchangedAttributes; > > > + FVB_DEVICE *FlashInstance; > > > UINT32 Capabilities; > > > UINT32 OldStatus; > > > UINT32 NewStatus; > > > > > > + FlashInstance = INSTANCE_FROM_FVB_THIS (This); > > > + > > > // > > > // Obtain attributes from FVB header > > > // > > > @@ -369,12 +376,15 @@ MvFvbSetAttributes ( > > > EFI_FVB2_WRITE_ENABLED_CAP | \ > > > EFI_FVB2_LOCK_CAP | \ > > > EFI_FVB2_STICKY_WRITE | \ > > > - EFI_FVB2_MEMORY_MAPPED | \ > > > EFI_FVB2_ERASE_POLARITY | \ > > > EFI_FVB2_READ_LOCK_CAP | \ > > > EFI_FVB2_WRITE_LOCK_CAP | \ > > > EFI_FVB2_ALIGNMENT; > > > > > > + if (FlashInstance->IsMemoryMapped) { > > > + UnchangedAttributes |= EFI_FVB2_MEMORY_MAPPED; > > > + } > > > + > > > // > > > // Some attributes of FV is read only can *not* be set > > > // > > > @@ -692,6 +702,7 @@ MvFvbWrite ( > > > IN UINT8 *Buffer > > > ) > > > { > > > + EFI_STATUS Status; > > > FVB_DEVICE *FlashInstance; > > > UINTN DataOffset; > > > > > > @@ -701,10 +712,27 @@ MvFvbWrite ( > > > FlashInstance->StartLba + Lba, > > > FlashInstance->Media.BlockSize); > > > > > > - return FlashInstance->SpiFlashProtocol->Write (&FlashInstance->SpiDevice, > > > - DataOffset, > > > - *NumBytes, > > > - Buffer); > > > + Status = FlashInstance->SpiFlashProtocol->Write (&FlashInstance->SpiDevice, > > > + DataOffset, > > > + *NumBytes, > > > + Buffer); > > > + if (EFI_ERROR (Status)) { > > > + DEBUG ((DEBUG_ERROR, > > > + "%a: Failed to write to Spi device\n", > > > + __FUNCTION__)); > > > + return Status; > > > + } > > > + > > > + // Update shadow buffer > > > + if (!FlashInstance->IsMemoryMapped) { > > > + DataOffset = GET_DATA_OFFSET (FlashInstance->RegionBaseAddress + Offset, > > > + FlashInstance->StartLba + Lba, > > > + FlashInstance->Media.BlockSize); > > > + > > > + CopyMem ((UINTN *)DataOffset, Buffer, *NumBytes); > > > + } > > > + > > > + return EFI_SUCCESS; > > > } > > > > > > /** > > > @@ -975,6 +1003,9 @@ MvFvbConfigureFlashInstance ( > > > ) > > > { > > > EFI_STATUS Status; > > > + UINTN *NumBytes; > > > + UINTN DataOffset; > > > + UINTN VariableSize, FtwWorkingSize, FtwSpareSize, MemorySize; > > > > > > > > > // Locate SPI protocols > > > @@ -1009,25 +1040,62 @@ MvFvbConfigureFlashInstance ( > > > } > > > > > > // Fill remaining flash description > > > - FlashInstance->DeviceBaseAddress = PcdGet64 (PcdSpiMemoryBase); > > > - FlashInstance->RegionBaseAddress = FixedPcdGet64 (PcdFlashNvStorageVariableBase64); > > > - FlashInstance->FvbOffset = FlashInstance->RegionBaseAddress - > > > - FlashInstance->DeviceBaseAddress; > > > - FlashInstance->FvbSize = PcdGet32(PcdFlashNvStorageVariableSize) + > > > - PcdGet32(PcdFlashNvStorageFtwWorkingSize) + > > > - PcdGet32(PcdFlashNvStorageFtwSpareSize); > > > + VariableSize = PcdGet32 (PcdFlashNvStorageVariableSize); > > > + FtwWorkingSize = PcdGet32 (PcdFlashNvStorageFtwWorkingSize); > > > + FtwSpareSize = PcdGet32 (PcdFlashNvStorageFtwSpareSize); > > > + > > > + FlashInstance->IsMemoryMapped = PcdGetBool (PcdSpiMemoryMapped); > > > + FlashInstance->FvbSize = VariableSize + FtwWorkingSize + FtwSpareSize; > > > + FlashInstance->FvbOffset = PcdGet32 (PcdSpiVariableOffset); > > > > > > FlashInstance->Media.MediaId = 0; > > > FlashInstance->Media.BlockSize = FlashInstance->SpiDevice.Info->SectorSize; > > > FlashInstance->Media.LastBlock = FlashInstance->Size / > > > FlashInstance->Media.BlockSize - 1; > > > > > > + if (FlashInstance->IsMemoryMapped) { > > > + FlashInstance->DeviceBaseAddress = PcdGet64 (PcdSpiMemoryBase); > > > + FlashInstance->RegionBaseAddress = PcdGet64 (PcdFlashNvStorageVariableBase64); > > > + } else { > > > + MemorySize = EFI_SIZE_TO_PAGES (FlashInstance->FvbSize); > > > + > > > + // FaultTolerantWriteDxe requires memory to be aligned to FtwWorkingSize > > > + FlashInstance->RegionBaseAddress = (UINTN) AllocateAlignedRuntimePages (MemorySize, > > > + SIZE_64KB); > > > + if (FlashInstance->RegionBaseAddress == (UINTN) NULL) { > > > + return EFI_OUT_OF_RESOURCES; > > > + } > > > + > > > + PcdSet64 (PcdFlashNvStorageVariableBase64, > > > + (UINT64) FlashInstance->RegionBaseAddress); > > > + PcdSet64 (PcdFlashNvStorageFtwWorkingBase64, > > > + (UINT64) FlashInstance->RegionBaseAddress > > > + + VariableSize); > > > + PcdSet64 (PcdFlashNvStorageFtwSpareBase64, > > > + (UINT64) FlashInstance->RegionBaseAddress > > > + + VariableSize > > > + + FtwWorkingSize); > > > + > > > + // Fill the buffer with data from flash > > > + DataOffset = GET_DATA_OFFSET (FlashInstance->FvbOffset, > > > + FlashInstance->StartLba, > > > + FlashInstance->Media.BlockSize); > > > + *NumBytes = FlashInstance->FvbSize; > > > + Status = FlashInstance->SpiFlashProtocol->Read (&FlashInstance->SpiDevice, > > > + DataOffset, > > > + *NumBytes, > > > + (VOID *)FlashInstance->RegionBaseAddress); > > > + if (EFI_ERROR (Status)) { > > > + goto ErrorFreeAllocatedPages; > > > + } > > > + } > > > + > > > Status = gBS->InstallMultipleProtocolInterfaces (&FlashInstance->Handle, > > > &gEfiDevicePathProtocolGuid, &FlashInstance->DevicePath, > > > &gEfiFirmwareVolumeBlockProtocolGuid, &FlashInstance->FvbProtocol, > > > NULL); > > > if (EFI_ERROR (Status)) { > > > - return Status; > > > + goto ErrorFreeAllocatedPages; > > > } > > > > > > Status = MvFvbPrepareFvHeader (FlashInstance); > > > @@ -1043,6 +1111,12 @@ ErrorPrepareFvbHeader: > > > &gEfiFirmwareVolumeBlockProtocolGuid, > > > NULL); > > > > > > +ErrorFreeAllocatedPages: > > > + if (!FlashInstance->IsMemoryMapped) { > > > + FreeAlignedPages ((VOID *)FlashInstance->RegionBaseAddress, > > > + MemorySize); > > > + } > > > + > > > return Status; > > > } > > > > > > @@ -1094,24 +1168,27 @@ MvFvbEntryPoint ( > > > // > > > // Declare the Non-Volatile storage as EFI_MEMORY_RUNTIME > > > // > > > - RuntimeMmioRegionSize = mFvbDevice->FvbSize; > > > RegionBaseAddress = mFvbDevice->RegionBaseAddress; > > > > > > - Status = gDS->AddMemorySpace (EfiGcdMemoryTypeMemoryMappedIo, > > > - RegionBaseAddress, > > > - RuntimeMmioRegionSize, > > > - EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); > > > - if (EFI_ERROR (Status)) { > > > - DEBUG ((DEBUG_ERROR, "%a: Failed to add memory space\n", __FUNCTION__)); > > > - goto ErrorAddSpace; > > > - } > > > + if (mFvbDevice->IsMemoryMapped) { > > > + RuntimeMmioRegionSize = mFvbDevice->FvbSize; > > > + Status = gDS->AddMemorySpace (EfiGcdMemoryTypeMemoryMappedIo, > > > + RegionBaseAddress, > > > + RuntimeMmioRegionSize, > > > + EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); > > > + if (EFI_ERROR (Status)) { > > > + DEBUG ((DEBUG_ERROR, "%a: Failed to add memory space\n", __FUNCTION__)); > > > + goto ErrorAddSpace; > > > + } > > > > > > - Status = gDS->SetMemorySpaceAttributes (RegionBaseAddress, > > > - RuntimeMmioRegionSize, > > > - EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); > > > - if (EFI_ERROR (Status)) { > > > - DEBUG ((DEBUG_ERROR, "%a: Failed to set memory attributes\n", __FUNCTION__)); > > > - goto ErrorSetMemAttr; > > > + > > > + Status = gDS->SetMemorySpaceAttributes (RegionBaseAddress, > > > + RuntimeMmioRegionSize, > > > + EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); > > > + if (EFI_ERROR (Status)) { > > > + DEBUG ((DEBUG_ERROR, "%a: Failed to set memory attributes\n", __FUNCTION__)); > > > + goto ErrorSetMemAttr; > > > + } > > > } > > > > > > // > > > -- > > > 2.7.4 > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [edk2-platforms: PATCH v2 3/3] Marvell/Drivers: Add non-mmio mode to MvFvbDxe 2019-04-24 13:48 ` Marcin Wojtas @ 2019-04-24 13:51 ` Ard Biesheuvel 2019-04-24 14:04 ` Marcin Wojtas 0 siblings, 1 reply; 9+ messages in thread From: Ard Biesheuvel @ 2019-04-24 13:51 UTC (permalink / raw) To: Marcin Wojtas Cc: edk2-devel-groups-io, Leif Lindholm, Jan Dąbroś, Grzegorz Jaszczyk, Kostya Porotchkin, Jici Gao, Kornel Duleba On Wed, 24 Apr 2019 at 15:48, Marcin Wojtas <mw@semihalf.com> wrote: > > Hi Ard, > > śr., 24 kwi 2019 o 15:02 Ard Biesheuvel <ard.biesheuvel@linaro.org> napisał(a): > > > > On Wed, 24 Apr 2019 at 09:11, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > > > > > On Wed, 24 Apr 2019 at 08:52, Marcin Wojtas <mw@semihalf.com> wrote: > > > > > > > > From: Kornel Duleba <mindal@semihalf.com> > > > > > > > > This path enables support for reading variables directly from flash without > > > > relying on it to be memory mapped. It adds PcdSpiMemoryMapped PCD that > > > > allows to switch between the modes. When in non-memory-mapped mode the > > > > driver will copy the variables from flash to previously allocated buffer > > > > and set PcdFlashNvStorageVariableBase64, PcdFlashNvStorageFtwWorkingBase64 > > > > and PcdFlashNvStorageFtwSpareBase64 accordingly. > > > > > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > > > Signed-off-by: Marcin Wojtas <mw@semihalf.com> > > > > --- > > > > Silicon/Marvell/Marvell.dec | 8 ++ > > > > Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 10 +- > > > > Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf | 17 ++- > > > > Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.h | 1 + > > > > Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c | 135 +++++++++++++++----- > > > > 5 files changed, 135 insertions(+), 36 deletions(-) > > > > > > > > diff --git a/Silicon/Marvell/Marvell.dec b/Silicon/Marvell/Marvell.dec > > > > index 7210ba2..a23c329 100644 > > > > --- a/Silicon/Marvell/Marvell.dec > > > > +++ b/Silicon/Marvell/Marvell.dec > > > > @@ -58,6 +58,12 @@ > > > > > > > > gMarvellFvbDxeGuid = { 0x42903750, 0x7e61, 0x4aaf, { 0x83, 0x29, 0xbf, 0x42, 0x36, 0x4e, 0x24, 0x85 } } > > > > gMarvellSpiFlashDxeGuid = { 0x49d7fb74, 0x306d, 0x42bd, { 0x94, 0xc8, 0xc0, 0xc5, 0x4b, 0x18, 0x1d, 0xd7 } } > > > > + # > > > > + # Generic FaultTolerantWriteDxe driver use variables, > > > > + # whose setting is done in MvFvbDxe driver in case > > > > + # the SPI contents are not mapped in memory. > > > > + # > > > > + gFaultTolerantWriteDxeFileGuid = { 0xfe5cea76, 0x4f72, 0x49e8, { 0x98, 0x6f, 0x2c, 0xd8, 0x99, 0xdf, 0xfe, 0x5d} } > > > > > > > > [LibraryClasses] > > > > ArmadaBoardDescLib|Include/Library/ArmadaBoardDescLib.h > > > > @@ -140,6 +146,8 @@ > > > > #SPI > > > > gMarvellTokenSpaceGuid.PcdSpiRegBase|0|UINT32|0x3000051 > > > > gMarvellTokenSpaceGuid.PcdSpiMemoryBase|0|UINT64|0x3000059 > > > > + gMarvellTokenSpaceGuid.PcdSpiMemoryMapped|TRUE|BOOLEAN|0x3000060 > > > > + gMarvellTokenSpaceGuid.PcdSpiVariableOffset|0|UINT32|0x3000061 > > > > gMarvellTokenSpaceGuid.PcdSpiMaxFrequency|0|UINT32|0x30000052 > > > > gMarvellTokenSpaceGuid.PcdSpiClockFrequency|0|UINT32|0x30000053 > > > > > > > > diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc > > > > index ca3de2e..d53d128 100644 > > > > --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc > > > > +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc > > > > @@ -256,6 +256,11 @@ > > > > # USB support > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdTurnOffUsbLegacySupport|TRUE > > > > > > > > +[PcdsDynamicDefault.common] > > > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0xF93C0000 > > > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64|0xF93E0000 > > > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64|0xF93D0000 > > > > + > > > > [PcdsFixedAtBuild.common] > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"MARVELL_EFI" > > > > gArmPlatformTokenSpaceGuid.PcdCoreCount|4 > > > > @@ -396,11 +401,10 @@ > > > > # Variable store - default values > > > > # > > > > gMarvellTokenSpaceGuid.PcdSpiMemoryBase|0xF9000000 > > > > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0xF93C0000 > > > > + gMarvellTokenSpaceGuid.PcdSpiMemoryMapped|TRUE > > > > + gMarvellTokenSpaceGuid.PcdSpiVariableOffset|0x3C0000 > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00010000 > > > > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64|0xF93D0000 > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00010000 > > > > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64|0xF93E0000 > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00010000 > > > > > > > > !if $(CAPSULE_ENABLE) > > > > diff --git a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf > > > > index ef10bfd..c85e8a6 100644 > > > > --- a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf > > > > +++ b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf > > > > @@ -76,13 +76,22 @@ > > > > gMarvellSpiMasterProtocolGuid > > > > > > > > [FixedPcd] > > > > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize > > > > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64 > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize > > > > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64 > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize > > > > gMarvellTokenSpaceGuid.PcdSpiMemoryBase > > > > + gMarvellTokenSpaceGuid.PcdSpiMemoryMapped > > > > + gMarvellTokenSpaceGuid.PcdSpiVariableOffset > > > > + > > > > +[Pcd] > > > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 > > > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64 > > > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64 > > > > > > > > [Depex] > > > > - gEfiCpuArchProtocolGuid > > > > + # > > > > + # Generic FaultTolerantWriteDxe driver use variables, > > > > + # whose setting is done in MvFvbDxe driver in case > > > > + # the SPI contents are not mapped in memory. > > > > + # > > > > + BEFORE gFaultTolerantWriteDxeFileGuid > > > > > > Apologies for not spotting this before, but there is a problem here: > > > FaultTolerantWriteDxe.inf does not depend on gEfiCpuArchProtocolGuid, > > > but we do, and so we could now be dispatched before > > > gEfiCpuArchProtocolGuid becomes available. This means you need to > > > update the code to deal with that (or explain to me how if it already > > > does) > > > > > > > You should be able to fix this by adding a NULL resolution for > > > > EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf > > > > to the FTW driver, similar to what you are already doing for the > > variable driver. > > > > Thanks for the hint, I'll try that. > > I tried to modify dependencies within edk2-platforms to explicitly > cover FaultTolerantWriteDxe.inf and gEfiCpuArchProtocolGuid, but did > not succeed. > You cannot combine BEFORE/AFTER depexes with boolean depex expressions. In general, BEFORE/AFTER should be avoided since it defeats the purpose of protocol dependencies. > With this patch the things work properly, but I am wondering if only > by luck. I found following line in > BaseTools/Source/Python/UPT/Xml/XmlParser.py: > DxeObj.SetDepex("gEfiBdsArchProtocolGuid AND \ngEfiCpuArchProtocolGuid > AND\n" + \ > it seems not to apply for MvFvbDxe though. So simply it may be an > order of entries in Armada7k8k.fdf file. > Most likely. The implicit depex is probably for UEFI_DRIVER modules not DXE_DRIVER ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [edk2-platforms: PATCH v2 3/3] Marvell/Drivers: Add non-mmio mode to MvFvbDxe 2019-04-24 13:51 ` Ard Biesheuvel @ 2019-04-24 14:04 ` Marcin Wojtas 0 siblings, 0 replies; 9+ messages in thread From: Marcin Wojtas @ 2019-04-24 14:04 UTC (permalink / raw) To: Ard Biesheuvel Cc: edk2-devel-groups-io, Leif Lindholm, Jan Dąbroś, Grzegorz Jaszczyk, Kostya Porotchkin, Jici Gao, Kornel Duleba śr., 24 kwi 2019 o 15:51 Ard Biesheuvel <ard.biesheuvel@linaro.org> napisał(a): > > On Wed, 24 Apr 2019 at 15:48, Marcin Wojtas <mw@semihalf.com> wrote: > > > > Hi Ard, > > > > śr., 24 kwi 2019 o 15:02 Ard Biesheuvel <ard.biesheuvel@linaro.org> napisał(a): > > > > > > On Wed, 24 Apr 2019 at 09:11, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > > > > > > > On Wed, 24 Apr 2019 at 08:52, Marcin Wojtas <mw@semihalf.com> wrote: > > > > > > > > > > From: Kornel Duleba <mindal@semihalf.com> > > > > > > > > > > This path enables support for reading variables directly from flash without > > > > > relying on it to be memory mapped. It adds PcdSpiMemoryMapped PCD that > > > > > allows to switch between the modes. When in non-memory-mapped mode the > > > > > driver will copy the variables from flash to previously allocated buffer > > > > > and set PcdFlashNvStorageVariableBase64, PcdFlashNvStorageFtwWorkingBase64 > > > > > and PcdFlashNvStorageFtwSpareBase64 accordingly. > > > > > > > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > > > > Signed-off-by: Marcin Wojtas <mw@semihalf.com> > > > > > --- > > > > > Silicon/Marvell/Marvell.dec | 8 ++ > > > > > Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 10 +- > > > > > Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf | 17 ++- > > > > > Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.h | 1 + > > > > > Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.c | 135 +++++++++++++++----- > > > > > 5 files changed, 135 insertions(+), 36 deletions(-) > > > > > > > > > > diff --git a/Silicon/Marvell/Marvell.dec b/Silicon/Marvell/Marvell.dec > > > > > index 7210ba2..a23c329 100644 > > > > > --- a/Silicon/Marvell/Marvell.dec > > > > > +++ b/Silicon/Marvell/Marvell.dec > > > > > @@ -58,6 +58,12 @@ > > > > > > > > > > gMarvellFvbDxeGuid = { 0x42903750, 0x7e61, 0x4aaf, { 0x83, 0x29, 0xbf, 0x42, 0x36, 0x4e, 0x24, 0x85 } } > > > > > gMarvellSpiFlashDxeGuid = { 0x49d7fb74, 0x306d, 0x42bd, { 0x94, 0xc8, 0xc0, 0xc5, 0x4b, 0x18, 0x1d, 0xd7 } } > > > > > + # > > > > > + # Generic FaultTolerantWriteDxe driver use variables, > > > > > + # whose setting is done in MvFvbDxe driver in case > > > > > + # the SPI contents are not mapped in memory. > > > > > + # > > > > > + gFaultTolerantWriteDxeFileGuid = { 0xfe5cea76, 0x4f72, 0x49e8, { 0x98, 0x6f, 0x2c, 0xd8, 0x99, 0xdf, 0xfe, 0x5d} } > > > > > > > > > > [LibraryClasses] > > > > > ArmadaBoardDescLib|Include/Library/ArmadaBoardDescLib.h > > > > > @@ -140,6 +146,8 @@ > > > > > #SPI > > > > > gMarvellTokenSpaceGuid.PcdSpiRegBase|0|UINT32|0x3000051 > > > > > gMarvellTokenSpaceGuid.PcdSpiMemoryBase|0|UINT64|0x3000059 > > > > > + gMarvellTokenSpaceGuid.PcdSpiMemoryMapped|TRUE|BOOLEAN|0x3000060 > > > > > + gMarvellTokenSpaceGuid.PcdSpiVariableOffset|0|UINT32|0x3000061 > > > > > gMarvellTokenSpaceGuid.PcdSpiMaxFrequency|0|UINT32|0x30000052 > > > > > gMarvellTokenSpaceGuid.PcdSpiClockFrequency|0|UINT32|0x30000053 > > > > > > > > > > diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc > > > > > index ca3de2e..d53d128 100644 > > > > > --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc > > > > > +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc > > > > > @@ -256,6 +256,11 @@ > > > > > # USB support > > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdTurnOffUsbLegacySupport|TRUE > > > > > > > > > > +[PcdsDynamicDefault.common] > > > > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0xF93C0000 > > > > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64|0xF93E0000 > > > > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64|0xF93D0000 > > > > > + > > > > > [PcdsFixedAtBuild.common] > > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"MARVELL_EFI" > > > > > gArmPlatformTokenSpaceGuid.PcdCoreCount|4 > > > > > @@ -396,11 +401,10 @@ > > > > > # Variable store - default values > > > > > # > > > > > gMarvellTokenSpaceGuid.PcdSpiMemoryBase|0xF9000000 > > > > > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0xF93C0000 > > > > > + gMarvellTokenSpaceGuid.PcdSpiMemoryMapped|TRUE > > > > > + gMarvellTokenSpaceGuid.PcdSpiVariableOffset|0x3C0000 > > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00010000 > > > > > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64|0xF93D0000 > > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00010000 > > > > > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64|0xF93E0000 > > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00010000 > > > > > > > > > > !if $(CAPSULE_ENABLE) > > > > > diff --git a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf > > > > > index ef10bfd..c85e8a6 100644 > > > > > --- a/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf > > > > > +++ b/Silicon/Marvell/Drivers/Spi/MvFvbDxe/MvFvbDxe.inf > > > > > @@ -76,13 +76,22 @@ > > > > > gMarvellSpiMasterProtocolGuid > > > > > > > > > > [FixedPcd] > > > > > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 > > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize > > > > > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64 > > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize > > > > > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64 > > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize > > > > > gMarvellTokenSpaceGuid.PcdSpiMemoryBase > > > > > + gMarvellTokenSpaceGuid.PcdSpiMemoryMapped > > > > > + gMarvellTokenSpaceGuid.PcdSpiVariableOffset > > > > > + > > > > > +[Pcd] > > > > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 > > > > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64 > > > > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64 > > > > > > > > > > [Depex] > > > > > - gEfiCpuArchProtocolGuid > > > > > + # > > > > > + # Generic FaultTolerantWriteDxe driver use variables, > > > > > + # whose setting is done in MvFvbDxe driver in case > > > > > + # the SPI contents are not mapped in memory. > > > > > + # > > > > > + BEFORE gFaultTolerantWriteDxeFileGuid > > > > > > > > Apologies for not spotting this before, but there is a problem here: > > > > FaultTolerantWriteDxe.inf does not depend on gEfiCpuArchProtocolGuid, > > > > but we do, and so we could now be dispatched before > > > > gEfiCpuArchProtocolGuid becomes available. This means you need to > > > > update the code to deal with that (or explain to me how if it already > > > > does) > > > > > > > > > > You should be able to fix this by adding a NULL resolution for > > > > > > EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf > > > > > > to the FTW driver, similar to what you are already doing for the > > > variable driver. > > > > > > > Thanks for the hint, I'll try that. > > > > I tried to modify dependencies within edk2-platforms to explicitly > > cover FaultTolerantWriteDxe.inf and gEfiCpuArchProtocolGuid, but did > > not succeed. > > > > You cannot combine BEFORE/AFTER depexes with boolean depex > expressions. In general, BEFORE/AFTER should be avoided since it > defeats the purpose of protocol dependencies. > I know that. But since I have chain of dependencies between MvFvbDxe / MvSpiFlashDxe / MvSpiOrionDxe, I wanted to blend gEfiCpuArchProtocolGuid somewhere. > > With this patch the things work properly, but I am wondering if only > > by luck. I found following line in > > BaseTools/Source/Python/UPT/Xml/XmlParser.py: > > DxeObj.SetDepex("gEfiBdsArchProtocolGuid AND \ngEfiCpuArchProtocolGuid > > AND\n" + \ > > it seems not to apply for MvFvbDxe though. So simply it may be an > > order of entries in Armada7k8k.fdf file. > > > > Most likely. The implicit depex is probably for UEFI_DRIVER modules > not DXE_DRIVER ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-04-24 14:05 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-04-24 6:51 [edk2-platforms: PATCH v2 0/3] Armada7k8k FVB improvements Marcin Wojtas 2019-04-24 6:51 ` [edk2-platforms: PATCH v2 1/3] Marvell/Drivers: MvFvbDxe: Change Pcd parameters to be 64 bit Marcin Wojtas 2019-04-24 6:51 ` [edk2-platforms: PATCH v2 2/3] Marvell/Armada7k8k: Cleanup PEI phase FV Marcin Wojtas 2019-04-24 6:51 ` [edk2-platforms: PATCH v2 3/3] Marvell/Drivers: Add non-mmio mode to MvFvbDxe Marcin Wojtas 2019-04-24 7:11 ` Ard Biesheuvel 2019-04-24 13:01 ` Ard Biesheuvel 2019-04-24 13:48 ` Marcin Wojtas 2019-04-24 13:51 ` Ard Biesheuvel 2019-04-24 14:04 ` Marcin Wojtas
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox