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.
Saving of UEFI variables has been tested on RTSM and QEMU.
changes since V1:
* Change addresses used to be at base of flash region, as this
is a restriction of the ARM NOR flash driver.
* Remove A9 changes. The A15 is the only CPU currently support
in QEMU, so I removed the A9 changes since they can't be tested.
Roy Franz (1):
Move RTSM VExpress variable storage to 256k flash blocks
ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc | 12 ++++++------
.../ArmVExpressPkg/ArmVExpress-RTSM-A15_MPCore.dsc | 12 ++++++------
2 files changed, 12 insertions(+), 12 deletions(-)
--
1.7.10.4
The RTSM VExpress model emulates a different networking controller (91C111)
than the VExpres board (9118). QEMU emulates the 9118 which matches the
real hardare. This is the only configuration difference for UEFI between
building for RTSM or UEFI.
This patch adds a EDK2_ARMVE_USE_9118 macro that can be defined at build time
that can be used to build an image that supports QEMU. The default build is
unchanged and builds the RTSM configuration.
Signed-off-by: Roy Franz <roy.franz(a)linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.0
---
ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc | 12 +++++++++---
ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.fdf | 8 +++++++-
2 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc
index 4dcdfae..5323efd 100644
--- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc
+++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc
@@ -141,9 +141,15 @@
gArmTokenSpaceGuid.PcdGicDistributorBase|0x2C001000
gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase|0x2C002000
- # Ethernet (SMSC 91C111)
- gArmPlatformTokenSpaceGuid.PcdLan91xDxeBaseAddress|0x1A000000
-
+ # Select network device based on build time macro
+!if $(EDK2_ARMVE_USE_9118) == 1
+ # Ethernet (SMSC 9118, for QEMU, matches real hardware)
+ gArmPlatformTokenSpaceGuid.PcdLan9118DxeBaseAddress|0x1A000000
+!else
+ # Ethernet (SMSC 91C111, for RTSM)
+ gArmPlatformTokenSpaceGuid.PcdLan91xDxeBaseAddress|0x1A000000
+!endif
+
#
# ARM OS Loader
#
diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.fdf b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.fdf
index be79efd..146f6f4 100644
--- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.fdf
+++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.fdf
@@ -144,7 +144,13 @@ READ_LOCK_STATUS = TRUE
INF MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
INF MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
INF MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
- INF ArmPlatformPkg/Drivers/LAN91xDxe/LAN91xDxe.inf
+!if $(EDK2_ARMVE_USE_9118) == 1
+ # LAN9118Dxe.inf for QEMU (matches use of 9118 on real VExpress board)
+ INF ArmPlatformPkg/Drivers/LAN9118Dxe/LAN9118Dxe.inf
+!else
+ # LAN91xDxe.inf for RTSM
+ INF ArmPlatformPkg/Drivers/LAN91xDxe/LAN91xDxe.inf
+!endif
#
# Multiple Console IO support
--
1.7.10.4
I have gotten the UEFI networking problems sorted out on QEMU (patches
posted to the QEMU list), and have updated the wiki page describing
running UEFI on QEMU.
https://wiki.linaro.org/LEG/Engineering/Kernel/UEFI/VersatileExpress/QEMU
Please let me know if you have any questions or feedback on the documentation.
Thanks,
Roy
It appears to me that while TianoCore ARMv8 support is in place, support
for 64-bit memory maps is not. The ARM models have all memory (or at
least first 2GB) and peripherals below 4GB. The ARM code is very much
not 32/64 bit portable. I see lots of places where UINTN is used to cast
addresses. This will need to be addressed for any "real" 64-bit platforms.
The things I've found and started fixing so far are:
- Flash base
- System memory base
- Global variable base
It's not clear to me how much more of this stuff there is. Any knowledge
or input here would be helpful. I would guess the common code is 64-bit
safe and this issue is only with the ARM code.
While I hope these are all restrictions of the ARM port, are there any
general EFI requirements about having memory or peripherals below 4GB
(on 64-bit systems)?
Rob
It appears to me that while TianoCore ARMv8 support is in place, support
for 64-bit memory maps is not. The ARM models have all memory (or at
least first 2GB) and peripherals below 4GB. The ARM code is very much
not 32/64 bit portable. I see lots of places where UINTN is used to cast
addresses. This will need to be addressed for any "real" 64-bit platforms.
The things I've found and started fixing so far are:
- Flash base
- System memory base
- Global variable base
It's not clear to me how much more of this stuff there is. Any knowledge
or input here would be helpful. I would guess the common code is 64-bit
safe and this issue is only with the ARM code.
While I hope these are all restrictions of the ARM port, are there any
general EFI requirements about having memory or peripherals below 4GB
(on 64-bit systems)?
Rob
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.
Note that this change will require RTSM flash images to be updated, as
the variable storage has moved. Currently on the A15 model is supported
by QEMU, but the A9 configuration is being updated as well to keep all
RTSM VExpress configurations consistent.
Signed-off-by: Roy Franz <roy.franz(a)linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Steven Kinney <steven.kinney(a)linaro.org>
---
ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc | 12 ++++++------
.../ArmVExpressPkg/ArmVExpress-RTSM-A15_MPCore.dsc | 12 ++++++------
ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A9x4.dsc | 12 ++++++------
3 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc
index 2d12f4b..8883213 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|0x0FFD0000
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00010000
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x0FFE0000
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00010000
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x0FF00000
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00040000
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x0FF40000
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00040000
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x0FF80000
+ 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..ae42de2 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|0x0FFD0000
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00010000
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x0FFE0000
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00010000
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x0FF00000
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00040000
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x0FF40000
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00040000
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x0FF80000
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00040000
gArmTokenSpaceGuid.PcdVFPEnabled|1
diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A9x4.dsc b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A9x4.dsc
index b635502..4d4d8b1 100644
--- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A9x4.dsc
+++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A9x4.dsc
@@ -81,12 +81,12 @@
#
# NV Storage PCDs. Use base of 0x0C000000 for NOR1
#
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x0FFC0000
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00010000
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x0FFD0000
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00010000
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x0FFE0000
- gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00010000
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x0FF00000
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00040000
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x0FF40000
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00040000
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x0FF80000
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00040000
gArmTokenSpaceGuid.PcdVFPEnabled|1
--
1.7.10.4
On Wed, Dec 11, 2013 at 2:56 AM, Olivier Martin <olivier.martin(a)arm.com> wrote:
> Thanks Roy for the investigation.
>
> FYI, I am working on fixing our UEFI in our CI infrastructure to validate
> your patch.
> The 'restriction' you found out is interesting, I need to review the code to
> see if it can be fixed or if it is really a restriction.
>
> Olivier
>
Hi Olivier,
I'm still a bit unsure how all the NOR and Fvb initialization fits
together, but it looks like the problem is at least partially in:
NorFlashFvbInitialize(), which calls ValidateFvHeader() with a NOR
flash instance.
The ARM version of ValidateFvHeader is defined as:
EFI_STATUS
ValidateFvHeader (
IN NOR_FLASH_INSTANCE *Instance
)
And then checks for a header at Instance->RegionBaseAddress.
Others are:
EFI_STATUS
ValidateFvHeader (
IN EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader
)
I have not looked at the others to see if the volume header is tied to
the start of flash or not, but at least the APIs
allow for it to be elsewhere. Since each set of block sizes gets its
own flash region, the original flash addresses worked
because it was at the start of the boot block region.
Roy
>> -----Original Message-----
>> From: linaro-uefi-bounces(a)lists.linaro.org [mailto:linaro-uefi-
>> bounces(a)lists.linaro.org] On Behalf Of Roy Franz
>> Sent: 11 December 2013 03:24
>> To: Steven Kinney; ryan.harkin(a)linaro.org; linaro-uefi
>> Subject: [Linaro-uefi] Updated QEMU flash topic branch
>>
>> I have pushed an updated flash topic branch that fixes the persistent
>> storage on RTSM and QEMU.
>> The ARM NOR support in UEFI only supports Firmware volumes that start
>> at the beginning of a NOR flash region, so I have moved the variable
>> storage to be at the base of the secondary flash. This seems like an
>> odd restriction, and I'm unsure if this was an intentional design
>> decision of just a simplification to get NOR working.
>>
>> Topic branch:
>>
>> https://git.linaro.org/people/roy.franz/uefi-
>> next.git/shortlog/refs/heads/topic-qemu-flash-v2
>>
>>
>> I will need to send out updated patches to the list based on these
>> changes as well.
>>
>> Roy
>>
>> _______________________________________________
>> Linaro-uefi mailing list
>> Linaro-uefi(a)lists.linaro.org
>> http://lists.linaro.org/mailman/listinfo/linaro-uefi
>
>
>
>
I have pushed an updated flash topic branch that fixes the persistent
storage on RTSM and QEMU.
The ARM NOR support in UEFI only supports Firmware volumes that start
at the beginning of a NOR flash region, so I have moved the variable
storage to be at the base of the secondary flash. This seems like an
odd restriction, and I'm unsure if this was an intentional design
decision of just a simplification to get NOR working.
Topic branch:
https://git.linaro.org/people/roy.franz/uefi-next.git/shortlog/refs/heads/t…
I will need to send out updated patches to the list based on these
changes as well.
Roy
On Mon, Dec 02, 2013 at 01:51:22PM -0600, Matt Sealey wrote:
> Here's where I think this whole thing falls down as being the weirdest
> possible implementation of this. It defies logic to put this
> information in the device tree /chosen node while also attempting to
> boot the kernel using an EFI stub; the stub is going to have this
> information because it is going to have the pointer to the system
> System Table (since it was called by StartImage()). Why not stash the
> System Table pointer somewhere safe in the stub?
We do. In the DT.
> The information in the device tree is all accessible from Boot
> Services and as long as the System Table isn't being thrown away (my
> suggestion would be.. stuff it in r2, and set r1 = "EFI\0" then work
> with arch/arm/kernel/head{-common,}.S code to do the right thing)
You left out the bit of redefining the kernel boot protocol to permit
calling it with caches, MMU and interrupts enabled - also known as
before ExitBootServices().
> It seems like the advantages of booting from UEFI and having all this
> information and API around are being thrown away very early, and
> picked up when it's no longer relevant to gain access to the very
> minimal runtime services. What's missing is a UUID for a "Device Tree
> Blob" in the Configuration Table, so you can very easily go grab that
> information from the firmware.
Which is what we are going to implement anyway in order to permit
firmware to supply DT hardware description in the same way as with
ACPI. Yes, we could pass the system table pointer directly - but that
doesn't get us the memory map.
> As implemented, these patches employ a very long-winded and complex
> method of recovering UEFI after throwing the system table pointer away
> early in boot, and then implements an EFI calling convention which
> isn't strictly necessary according to the UEFI spec - the question is,
> is this a workaround for SetVirtualAddressMap() not actually doing the
> right thing on ARM UEFI implementations? If you can't guarantee that
> most of the calls from Boot Services or Runtime Services are going to
> allow this, then having any UEFI support in the kernel at all seems
> rather weird.
No, it is a workaround for it being explicitly against the kernel boot
protocol (not to mention slightly hairy) to enter head.S with MMU and
caches enabled and interrupts firing.
The EFI calling convention (as pointed out in the patch itself) is
there in order to not have to duplicate code already there for x86.
> What I'm worried about is that this is basically a hack to try and
> shoehorn an existing UEFI implementation to an existing Linux boot
> method - and implements it in a way nobody is ever going to be able to
> justify improving. Part of the reason the OpenFirmware CIF got thrown
> away early in SPARC/PowerPC boot (after "flattening" the device tree
> using the CIF calls to parse it out) was because you had to disable
> the MMU, caches, interrupts etc. which caused all kinds of slow
> firmware code to be all kinds of extra-slow.
I prefer to see it as a way to not reinvent things that do not need
reinventing, while not adding more special-case code to the kernel.
> What that meant is nobody bothered to implement working, re-entrant,
> re-locatable firmware to a great degree. This ended up being a
> self-fulfilling prophecy of "don't trust the bootloader" and "get rid
> of it as soon as we can," which essentially meant Linux never took
> advantage of the resources available. In OF's case, the CIF sucked by
> specification. In UEFI's case here, it's been implemented in Linux in
> such a way that guarantees poor-performing firmware code with huge
> penalties to call them, which isn't even required by UEFI if the
> earlier boot code did the right things in the first place.
I don't follow. In which way does this implementation result in poor
performance or reduced functionality?
We deal with a highly quirky set of requirements for calling
SetVirtualAddressMap() in a clunky way - after which calls into UEFI
are direct and cachable.
/
Leif
The kernel commandline has been simplified to remove deprecated and
unwanted options.
The patch has been split into 5 due to the relevant files being managed in different topic branches in the tree:
linaro-topic-tc2:
[PATCH 1/5] TC2: update default kernel commandline
linaro-topic-tc1:
[PATCH 2/5] TC1: update default kernel commandline
linaro-topic-a5:
[PATCH 3/5] VEA5: update default kernel commandline
linaro-topic-a9:
[PATCH 4/5] VEA9: update default kernel commandline
linaro-topic-bds:
[PATCH 5/5] RTSM: update default kernel commandline