Re: [PATCH 1/1] OvmfPkg/VirtNorFlashDxe: map flash memory as uncacheable

["Ard Biesheuvel" <ardb@kernel.org>]

On Mon, 16 Jan 2023 at 07:38, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> On Thu, Jan 12, 2023 at 11:20:00AM +0100, Ard Biesheuvel wrote:
> > On Wed, 11 Jan 2023 at 19:00, Gerd Hoffmann <kraxel@redhat.com> wrote:
> > >
> > > 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: b92298af8218 ("ArmVirtPkg/ArmVirtQemu: migrate to OVMF's VirtNorFlashDxe")
> > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> >
> > Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
>
> Laszlo pointed me to commit 789a72328553 ("OvmfPkg/VirtNorFlashDxe: use
> EFI_MEMORY_WC and drop AlignedCopyMem()") and I'm wondering whenever we
> need to also bring back AlignedCopyMem, or is it safe to use CopyMem
> because we know we operate on virtual hardware?
>

Ugh, thanks for spotting that.

So there is one occurrence of CopyMem() that may operate on a device
region with a misaligned address, and this is the one in
NorFlashRead(). This call is made while the flash is in array mode,
and so the device region is backed by a KVM memslot at that point, and
a misaligned load or store will trigger a fault.

We already swap out the optimized BaseMemoryLib for the generic one
when building VariableRuntimeDxe, and so the best course of action
here is probably to do the same for NorFlashDxe, rather than bringing
back AlignedCopyMem.