some comments
On 08/26/14 15:03, Ard Biesheuvel wrote:
> This adds support for retaining UEFI environment variables in the second
> emulated NOR flash which resides at phys address 0x04000000 (64 MB).
>
> Note that this requires booting QEMU with two -pflash arguments, each of which
> points to a NOR image file of exactly 64 MB in size. The second one will be
> used
> as the variable store.
>
> Signed-off-by: Ard Biesheuvel <[email protected]>
> ---
> .../AArch64Virtualization-KVM.dsc | 16 +++++-
> .../AArch64Virtualization-KVM.fdf | 4 +-
> .../Include/Platform/KVM/ArmPlatform.h | 6 +++
> .../Library/NorFlashKVM/NorFlashKVM.c | 63
> ++++++++++++++++++++++
> .../Library/NorFlashKVM/NorFlashKVM.inf | 35 ++++++++++++
> 5 files changed, 122 insertions(+), 2 deletions(-)
> create mode 100644
> ArmPlatformPkg/AArch64VirtualizationPkg/Library/NorFlashKVM/NorFlashKVM.c
> create mode 100644
> ArmPlatformPkg/AArch64VirtualizationPkg/Library/NorFlashKVM/NorFlashKVM.inf
>
> diff --git
> a/ArmPlatformPkg/AArch64VirtualizationPkg/AArch64Virtualization-KVM.dsc
> b/ArmPlatformPkg/AArch64VirtualizationPkg/AArch64Virtualization-KVM.dsc
> index e0817fca927e..78e83e92be2d 100644
> --- a/ArmPlatformPkg/AArch64VirtualizationPkg/AArch64Virtualization-KVM.dsc
> +++ b/ArmPlatformPkg/AArch64VirtualizationPkg/AArch64Virtualization-KVM.dsc
> @@ -117,6 +117,16 @@
> #
> gArmTokenSpaceGuid.PcdArmArchTimerFreqInHz|100000000
>
> + #
> + # NV Storage PCDs. Use base of 0x04000000 for NOR1
> + #
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x04000000
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00040000
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x04040000
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00040000
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x04080000
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00040000
> +
Hrmpf. These look strange. I'll explain why.
The promise of this patch is to set up a 64 MB flash chip (which is
absolutely huge, at least for varstore purposes). Then, supposedly,
looking at the definition and the use of the QEMU_NOR_BSIZE macro
(SIZE_256KB, NOR_FLASH_DESCRIPTION.BlockSize), this 64MB flash chip
would be handled in blocks of 256KB.
Which I could imagine, in theory, but the above PCDs mean different
things. These PCD's control the "Fault Tolerant Write" driver
(MdeModulePkg/Universal/FaultTolerantWriteDxe).
The FTW driver (no, it doesn't mean "for the win", not by far :)),
implements a kind of "journaled storage" in the flash, the point being
that at no point during flash writes should a power loss cause data
corruption.
Let me quote the PCD descriptions from "MdeModulePkg/MdeModulePkg.dec",
in the above order, rewrapping the descriptions:
(1)
PcdFlashNvStorageVariableBase --> 0x04000000
## Base address of the NV variable range in flash device
Okay.
(2)
PcdFlashNvStorageVariableSize --> 0x00040000 (256 kilobytes)
## Size of the NV variable range. Note that this value should less
## than or equal to PcdFlashNvStorageFtwSpareSize
# The root cause is that variable driver will use FTW protocol to
# reclaim variable region. If the length of variable region is larger
# than FTW spare size, it means the whole variable region can not be
# reflushed through the manner of fault tolerant write.
I'll try to explain this "reclaim" thing in a minute (in a quite
hand-wavy manner), but first, let's just realize that this PCD controls
the *complete size* of the *live data* in the varstore.
Then,
(3)
PcdFlashNvStorageFtwWorkingBase --> 0x04040000
## Base address of the FTW working block range in flash device.
This does make sense. The working block is tacked to the end of the raw
varstore blocks. Fine.
(4)
PcdFlashNvStorageFtwWorkingSize --> 0x00040000 (256 KB again)
## Size of the FTW working block range.
This seems valid, but I'm not sure why it is so large.
(5)
PcdFlashNvStorageFtwSpareBase --> 0x04080000
## Base address of the FTW spare block range in flash device. Note
## that this value should be block size aligned.
The spare area starts after the end of the working area, good.
(6)
PcdFlashNvStorageFtwSpareSize --> 0x00040000 (256 KB)
## Size of the FTW spare block range. Note that this value should
## larger than PcdFlashNvStorageVariableSize and block size aligned.
# The root cause is that variable driver will use FTW protocol to
# reclaim variable region. If the length of variable region is larger
# than FTW spare size, it means the whole variable region can not be
# reflushed through the manner of fault tolerant write.
So, the first 768 KB of the flash drive in question will look like:
65536 KB +--------------------+----------------------------+
| live data, 256 KB | working block area, 256 KB |
66048 KB +--------------------+----------------------------+
| spare area, 256 KB |
+--------------------+
Live variables are stored in the "live data" area. The working block
area is used for one-off updates, as far as I know.
The spare area is used for "reclaim", ie. compaction of the live area,
when it becomes fragmented due to many variable writes.
I had investigated that earlier in some detail, but very roughly, there
are such data movements between temporary buffers and the live and spare
areas, during reclaim (quoting an earlier email of mine, "I'm skipping
the erasures and the in-memory buffer manipulations"):
- copy LIVE to MyBuffer
- copy SPARE to SpareBuffer
- copy MyBuffer to SPARE
- copy SPARE to Buffer
- copy Buffer to LIVE
- copy SpareBuffer to SPARE
("MyBuffer" and "SpareBuffer" are temporary buffers in RAM.)
Here's the things that surprise me:
- Why require a 64MB flash drive, if we only use 768 KB of it?
- Why set the block size of the flash drive to 256 KB? That will mean
basically that each of the three areas, live, working, and spare,
consists of 1 block only. Since these beasts are addressed on
the block level, any modification to an area will always rewrite the
entire area (== 1 block).
- In comparison, here's how the OVMF varstore looks like. The block
size is 4 KB.
+------------------------------+-----------------+--------------------+
| live data, 14 * 4KB == 56 KB | event log, 4 KB | working area, 4 KB |
+------------------------------+-----------------+--------------------+
| spare area, 16 * 4KB == 64 KB |
+---------------------------------------------------------------------+
The 1st difference is that OVMF has much smaller flash block size (4KB
vs. 256 KB), and that OVMF's varstore is actually organized of *several*
of these 4KB blocks. (Whereas ARM's is just 1 block / area.)
The 2nd difference is that OVMF has an event log area. No clue why it is
useful, but it's interesting why ARM doesn't have one.
The 3rd difference is that OVMF's working area is just 1 block (a
fraction of the live data area), while on ARM the live and working block
areas are same-sized. I think this is a direct consequence of the flash
block size being huge (256KB). All areas must be an integral multiple of
the block size, and you can't go below 1 block.
The 4th difference is that OVMF's spare area, 64KB, mirrors not only the
live area (56KB), but also the event log (4KB) and the working area
(4KB). Whereas on ARM, the spare area (256KB) only mirrors the live area
(256KB), and doesn't cover the working area.
This all seems valid, in both ARM and OVMF, looking at the
MdeModulePkg.dec descriptions. (And anyway the FTW driver is chock-full
of ASSERT()s, so any invalid config would be caught quickly.) I'm just
wondering where these differences in flash layout come from.
> [PcdsDynamicDefault.common]
> # System Memory -- 1 MB initially, actual size will be fetched from DT
> gArmTokenSpaceGuid.PcdSystemMemoryBase|0x40000000
> @@ -179,7 +189,7 @@
> MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
> MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
> MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
> - MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDxe.inf
> + MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
I'll mention in passing that
"MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf" is
good for a non-authenticated variable store only, ie. no secure boot
support. For secure boot support, CryptoPkg / OpensslLib would be
necessary too, and another variable driver from under SecurityPkg, etc
etc etc, but for this series, the patch definitely suffices.
In total, although I'm surprised by some of the PCDs here, and by the
size of the flash drive(s), I think this patch is correct. Those PCDs
are probably aligned with ArmPkg / ArmPlatformPkg tradition.
Thanks,
Laszlo
------------------------------------------------------------------------------
Slashdot TV.
Video for Nerds. Stuff that matters.
http://tv.slashdot.org/
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel
