Hi Roy,
I need to think more about it. And I also need your point of view on some points.
Here are few questions and thoughts: - Do you think the flash topology is the only difference between qEmu and RTSM? I believe it is not the only difference. I think the Cortex A15 used to not support the Security extension in qEmu. IS it still the case? - The correct approach when you have flash with 2 regions (small and large block size regions) is to use the fine grain for the UEFI variables (faster accesses in NVM). Using the large grain when the fine grain is available would not be appropriate. But at least it would ease the support between qEmu and RTSM. - In the past the RTSM team told me that the goal of RTSM was to be able to run the same binary you would run on your HW onto the model (RTSM). But it is actually not the case with RTSM A15 (its HW equivalent should be the ARM Test Chip ver1 (TC1)) as the HW and RTSM memory map are different...
There are two solutions: 1) either I accept your patch - that will ease the maintenance (no difference between qEmu and RTSM support) 2) or I reject your patch. And I ask you to submit a new version that would introduce a build flag (eg: SUPPORT_QEMU) to differentiate RTSM and qEmu settings in Tianocore.
I have to admit I prefer the solution 2) - but I am quite open to any valid arguments. My argument is I would prefer to expose the correct implementation when possible. And if qEmu adds support for the missing VExpress IP block in the future then it will be easier to restore the correct approach (ie: the default RTSM approach) for qEmu.
Thanks, Olivier
-----Original Message----- From: Roy Franz [mailto:roy.franz@linaro.org] Sent: 10 January 2014 16:11 To: ryan.harkin@linaro.org; Olivier Martin Cc: edk2-devel@lists.sourceforge.net; linaro-uefi; Patch Tracking Subject: Re: [PATCH V2] Move RTSM VExpress variable storage to 256k flash blocks
Hi Olivier - Any thoughts on this?
Thanks, Roy
On Thu, Dec 19, 2013 at 7:40 AM, Ryan Harkin ryan.harkin@linaro.org wrote:
I just wanted to confirm that this works in the 13.12 release and in
my own
builds from the latest tree.
Thanks Roy!
On 12 December 2013 22:08, Roy Franz roy.franz@linaro.org wrote:
Change the addresses/sizes of the variable storage areas to use 256k blocks so UEFI is compatible with both the RTSM models and QEMU.
The VExpress flash has non-uniform block sizes, with most blocks
being
256k and the top 4 blocks being 64k. UEFI has been using these top
64k
blocks for persistent variable storage. The RTSM models the non-
uniform
sizes, while QEMU only supports emulating flash with uniform block
sizes
which results in the top 256k (the 4 64k blocks) of flash being
unusable
for writing in QEMU. The ARM UEFI NOR flash driver currently
requires
that firmware volumes start at the base of a flash region, so the variables are now stored at the base the region that consists of the 256k blocks. It was previously at the base of the region of 64k blocks.
Note that this change will require RTSM flash images to be updated,
as
the variable storage has moved. Currently only the A15 model is
supported
by QEMU RTSM VExpress configurations. This patch only changes the A15 configurations.
Signed-off-by: Roy Franz roy.franz@linaro.org Contributed-under: TianoCore Contribution Agreement 1.0
ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc | 12 ++++++------ .../ArmVExpressPkg/ArmVExpress-RTSM-A15_MPCore.dsc | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc index 2d12f4b..c0196d9 100644 --- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc +++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc @@ -77,12 +77,12 @@ # # NV Storage PCDs. Use base of 0x0C000000 for NOR1 #
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x0FFC0000
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00010000
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x0FFD00 00
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x000100 00
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x0FFE0000
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00010000
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x0C000000
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00040000
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x0C0400 00
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x000400 00
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x0C080000
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00040000
gArmTokenSpaceGuid.PcdVFPEnabled|1
diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-
A15_MPCore.dsc
b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15_MPCore.dsc index efd80ab..69088ff 100644 --- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15_MPCore.dsc +++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15_MPCore.dsc @@ -79,12 +79,12 @@ # # NV Storage PCDs. Use base of 0x0C000000 for NOR1 #
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x0FFC0000
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00010000
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x0FFD00 00
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x000100 00
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x0FFE0000
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00010000
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x0C000000
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00040000
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x0C0400 00
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x000400 00
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x0C080000
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00040000
gArmTokenSpaceGuid.PcdVFPEnabled|1
-- 1.7.10.4