* [PATCH 1/1] OvmfPkg/VirtNorFlashDxe: map flash memory as uncacheable
@ 2023-01-11 18:00 Gerd Hoffmann
2023-01-12 10:20 ` Ard Biesheuvel
0 siblings, 1 reply; 4+ messages in thread
From: Gerd Hoffmann @ 2023-01-11 18:00 UTC (permalink / raw)
To: devel
Cc: Ard Biesheuvel, Gerd Hoffmann, Jiewen Yao, Oliver Steffen,
Jordan Justen, Pawel Polawski
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>
---
OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.c b/OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.c
index ff3121af2a40..f9a41f6aab0f 100644
--- a/OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.c
+++ b/OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.c
@@ -394,14 +394,14 @@ NorFlashFvbInitialize (
EfiGcdMemoryTypeMemoryMappedIo,
Instance->DeviceBaseAddress,
RuntimeMmioRegionSize,
- EFI_MEMORY_WC | EFI_MEMORY_RUNTIME
+ EFI_MEMORY_UC | EFI_MEMORY_RUNTIME
);
ASSERT_EFI_ERROR (Status);
Status = gDS->SetMemorySpaceAttributes (
Instance->DeviceBaseAddress,
RuntimeMmioRegionSize,
- EFI_MEMORY_WC | EFI_MEMORY_RUNTIME
+ EFI_MEMORY_UC | EFI_MEMORY_RUNTIME
);
ASSERT_EFI_ERROR (Status);
--
2.39.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] OvmfPkg/VirtNorFlashDxe: map flash memory as uncacheable
2023-01-11 18:00 [PATCH 1/1] OvmfPkg/VirtNorFlashDxe: map flash memory as uncacheable Gerd Hoffmann
@ 2023-01-12 10:20 ` Ard Biesheuvel
2023-01-16 6:38 ` Gerd Hoffmann
0 siblings, 1 reply; 4+ messages in thread
From: Ard Biesheuvel @ 2023-01-12 10:20 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: devel, Ard Biesheuvel, Jiewen Yao, Oliver Steffen, Jordan Justen,
Pawel Polawski
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>
Queued up as #3887
> ---
> OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.c b/OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.c
> index ff3121af2a40..f9a41f6aab0f 100644
> --- a/OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.c
> +++ b/OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.c
> @@ -394,14 +394,14 @@ NorFlashFvbInitialize (
> EfiGcdMemoryTypeMemoryMappedIo,
> Instance->DeviceBaseAddress,
> RuntimeMmioRegionSize,
> - EFI_MEMORY_WC | EFI_MEMORY_RUNTIME
> + EFI_MEMORY_UC | EFI_MEMORY_RUNTIME
> );
> ASSERT_EFI_ERROR (Status);
>
> Status = gDS->SetMemorySpaceAttributes (
> Instance->DeviceBaseAddress,
> RuntimeMmioRegionSize,
> - EFI_MEMORY_WC | EFI_MEMORY_RUNTIME
> + EFI_MEMORY_UC | EFI_MEMORY_RUNTIME
> );
> ASSERT_EFI_ERROR (Status);
>
> --
> 2.39.0
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] OvmfPkg/VirtNorFlashDxe: map flash memory as uncacheable
2023-01-12 10:20 ` Ard Biesheuvel
@ 2023-01-16 6:38 ` Gerd Hoffmann
2023-01-16 8:18 ` Ard Biesheuvel
0 siblings, 1 reply; 4+ messages in thread
From: Gerd Hoffmann @ 2023-01-16 6:38 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: devel, Ard Biesheuvel, Jiewen Yao, Oliver Steffen, Jordan Justen,
Pawel Polawski
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?
take care,
Gerd
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] OvmfPkg/VirtNorFlashDxe: map flash memory as uncacheable
2023-01-16 6:38 ` Gerd Hoffmann
@ 2023-01-16 8:18 ` Ard Biesheuvel
0 siblings, 0 replies; 4+ messages in thread
From: Ard Biesheuvel @ 2023-01-16 8:18 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: devel, Ard Biesheuvel, Jiewen Yao, Oliver Steffen, Jordan Justen,
Pawel Polawski
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.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-01-16 8:18 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-11 18:00 [PATCH 1/1] OvmfPkg/VirtNorFlashDxe: map flash memory as uncacheable Gerd Hoffmann
2023-01-12 10:20 ` Ard Biesheuvel
2023-01-16 6:38 ` Gerd Hoffmann
2023-01-16 8:18 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox