* [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