Move the DoErase code block into a separate function, call the function
instead of jumping around with goto.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-Id: <20240116171105.37831-7-kraxel@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
It is possible to find variable entries with State being 0xff, i.e. not
updated since flash block erase. This indicates the variable driver
could not complete the header write while appending a new entry, and
therefore State was not set to VAR_HEADER_VALID_ONLY.
This can only happen at the end of the variable list, so treat this as
additional "end of variable list" condition.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20240116171105.37831-6-kraxel@redhat.com>
Raise the limit for writes without block erase from two to four
P30_MAX_BUFFER_SIZE_IN_BYTES blocks. With this in place almost all efi
variable updates are handled without block erase. With the old limit
some variable updates (with device paths) took the block erase code
path.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20240116171105.37831-5-kraxel@redhat.com>
Replace the two NorFlashWriteBuffer() calls with a loop containing a
single NorFlashWriteBuffer() call.
With the changes in place the code is able to handle updates larger
than two P30_MAX_BUFFER_SIZE_IN_BYTES blocks, even though the patch
does not actually change the size limit.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20240116171105.37831-4-kraxel@redhat.com>
Introduce 'Start' and 'End' variables to make it easier to follow the
logic and code flow. Also add a ascii art diagram (based on a
suggestion by Laszlo).
This also fixes the 'Size' calculation for the NorFlashRead() call.
Without this patch the code will read only one instead of two
P30_MAX_BUFFER_SIZE_IN_BYTES blocks in case '*NumBytes' is smaller than
P30_MAX_BUFFER_SIZE_IN_BYTES but 'Offset + *NumBytes' is not, i.e. the
update range crosses a P30_MAX_BUFFER_SIZE_IN_BYTES boundary.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20240116171105.37831-3-kraxel@redhat.com>
This is needed to avoid bit operations being applied to signed integers.
Suggested-by: László Érsek <lersek@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20240116171105.37831-2-kraxel@redhat.com>
Extend the ValidateFvHeader function, additionally to the header checks
walk over the list of variables and sanity check them.
In case we find inconsistencies indicating variable store corruption
return EFI_NOT_FOUND so the variable store will be re-initialized.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-Id: <20240109112902.30002-4-kraxel@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
[lersek@redhat.com: fix StartId initialization/assignment coding style]
Only accept gEfiAuthenticatedVariableGuid when checking the variable
store header in ValidateFvHeader().
The edk2 code base has been switched to use the authenticated varstore
format unconditionally (even in case secure boot is not used or
supported) a few years ago.
Suggested-by: László Érsek <lersek@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20240109112902.30002-3-kraxel@redhat.com>
__FUNCTION__ is a pre-standard extension that gcc and Visual C++ among
others support, while __func__ was standardized in C99.
Since it's more standard, replace __FUNCTION__ with __func__ throughout
OvmfPkg.
Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Sunil V L <sunilvl@ventanamicro.com>
Switching from the ArmPlatformPkg/NorFlashDxe driver to the
OvmfPkg/VirtNorFlashDxe driver had the side effect that flash address
space got registered as EFI_MEMORY_WC instead of EFI_MEMORY_UC.
That confuses the linux kernel's numa code, seems this makes kernel
consider the flash being node memory. "lsmem" changes from ...
RANGE SIZE STATE REMOVABLE BLOCK
0x0000000040000000-0x000000013fffffff 4G online yes 8-39
... to ...
RANGE SIZE STATE REMOVABLE BLOCK
0x0000000000000000-0x0000000007ffffff 128M online yes 0
0x0000000040000000-0x000000013fffffff 4G online yes 8-39
... and in the kernel log got new error lines:
NUMA: Warning: invalid memblk node 512 [mem 0x0000000004000000-0x0000000007ffffff]
NUMA: Faking a node at [mem 0x0000000004000000-0x000000013fffffff]
Changing the attributes back to EFI_MEMORY_UC fixes this.
Fixes: b92298af82 ("ArmVirtPkg/ArmVirtQemu: migrate to OVMF's VirtNorFlashDxe")
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
NOR flash emulation under KVM involves switching between two modes,
where array mode is backed by a read-only memslot, and programming mode
is fully emulated, i.e., the memory region is not backed by anything,
and the faulting accesses are forwarded to the VMM by the hypervisor,
which translates them into NOR flash programming commands.
Normally, we are limited to the use of device attributes when mapping
such regions, given that the programming mode has MMIO semantics.
However, when running under KVM, the chosen memory attributes only take
effect when in array mode, since no memory mapping exists otherwise.
This means we can tune the memory mapping so it behaves a bit more like
a ROM, by switching to EFI_MEMORY_WC attributes. This means we no longer
need a special CopyMem() implementation that avoids unaligned accesses
at all cost.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Sunil V L <sunilvl@ventanamicro.com>
Currently, when dealing with small updates that can be written out
directly (i.e., if they only involve clearing bits and not setting bits,
as the latter requires a block level erase), we iterate over the data
one word at a time, read the old value, compare it, write the new value,
and repeat, unless we encountered a value that we cannot write (0->1
transition), in which case we fall back to a block level operation.
This is inefficient for two reasons:
- reading and writing a word at a time involves switching between array
and programming mode for every word of data, which is
disproportionately costly when running under KVM;
- we end up writing some data twice, as we may not notice that a block
erase is needed until after some data has been written to flash.
So replace this sequence with a single read of up to twice the buffered
write maximum size, followed by one or two buffered writes if the data
can be written directly. Otherwise, fall back to the existing block
level sequence, but without writing out part of the data twice.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Sunil V L <sunilvl@ventanamicro.com>
NorFlashWriteSingleWord() switches into programming mode and back into
array mode for every single word that it writes. Under KVM, this
involves tearing down the read-only memslot, and setting it up again,
which is costly and unnecessary.
Instead, move the array mode switch into the callers, and only make the
switch when the writing is done.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Sunil V L <sunilvl@ventanamicro.com>
We never boot from NOR flash, and generally rely on the firmware volume
PI protocols to expose the contents. So drop the block I/O protocol
implementation from VirtNorFlashDxe.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Sunil V L <sunilvl@ventanamicro.com>
We only use NOR flash for firmware volumes, either for executable images
or for the variable store. So we have no need for exposing disk I/O on
top of the NOR flash partitions so let's remove it.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Sunil V L <sunilvl@ventanamicro.com>
We inherited a feature from the ArmPlatformPkg version of this driver
that never gets enabled. Let's remove it.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Sunil V L <sunilvl@ventanamicro.com>
QEMU's mach-virt is loosely based on ARM Versatile Express, and inherits
its NOR flash driver, which is now being used on other QEMU emulated
architectures as well.
In order to permit ourselves the freedom to optimize this driver for
use under KVM emulation, let's clone it into OvmfPkg, so we have a
version we can hack without the risk of regressing bare metal platforms.
The cloned version is mostly identical to the original, but it depends
on the newly added VirtNorFlashPlatformLib library class instead of the
original one from ArmPlatformPkg. Beyond that, only cosmetic changes
related to #include order etc were made.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Sunil V L <sunilvl@ventanamicro.com>
Gerd Hoffmann
Rebecca Cran
Ard Biesheuvel