public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH 1/1] ArmVirt: don't use unaligned CopyMem () on NOR flash
@ 2023-01-16  9:46 Gerd Hoffmann
  2023-01-16 11:10 ` [edk2-devel] " Ard Biesheuvel
  0 siblings, 1 reply; 3+ messages in thread
From: Gerd Hoffmann @ 2023-01-16  9:46 UTC (permalink / raw)
  To: devel
  Cc: Oliver Steffen, Leif Lindholm, Pawel Polawski, Gerd Hoffmann,
	Ard Biesheuvel, Sami Mujawar

Fixes: e5ec3ba409b5 ("OvmfPkg/VirtNorFlashDxe: map flash memory as uncacheable")
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ArmVirtPkg/ArmVirtKvmTool.dsc    | 6 +++++-
 ArmVirtPkg/ArmVirtQemu.dsc       | 6 +++++-
 ArmVirtPkg/ArmVirtQemuKernel.dsc | 6 +++++-
 3 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/ArmVirtPkg/ArmVirtKvmTool.dsc b/ArmVirtPkg/ArmVirtKvmTool.dsc
index 2ba00bd08ff1..d0afe1b49e25 100644
--- a/ArmVirtPkg/ArmVirtKvmTool.dsc
+++ b/ArmVirtPkg/ArmVirtKvmTool.dsc
@@ -296,7 +296,11 @@ [Components.common]
       NULL|ArmVirtPkg/Library/ArmVirtTimerFdtClientLib/ArmVirtTimerFdtClientLib.inf
   }
 
-  OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.inf
+  OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.inf {
+    <LibraryClasses>
+      # don't use unaligned CopyMem () on the UEFI varstore NOR flash region
+      BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
+  }
 
   MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
 
diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index 5dd8b6104cca..0f1c6395488a 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -428,7 +428,11 @@ [Components.common]
     <LibraryClasses>
       NULL|ArmVirtPkg/Library/ArmVirtTimerFdtClientLib/ArmVirtTimerFdtClientLib.inf
   }
-  OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.inf
+  OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.inf {
+    <LibraryClasses>
+      # don't use unaligned CopyMem () on the UEFI varstore NOR flash region
+      BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
+  }
   MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
 
   #
diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
index f5db3ac432f3..807c85d48285 100644
--- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
+++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
@@ -331,7 +331,11 @@ [Components.common]
     <LibraryClasses>
       NULL|ArmVirtPkg/Library/ArmVirtTimerFdtClientLib/ArmVirtTimerFdtClientLib.inf
   }
-  OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.inf
+  OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.inf {
+    <LibraryClasses>
+      # don't use unaligned CopyMem () on the UEFI varstore NOR flash region
+      BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
+  }
   MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
 
   #
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [edk2-devel] [PATCH 1/1] ArmVirt: don't use unaligned CopyMem () on NOR flash
  2023-01-16  9:46 [PATCH 1/1] ArmVirt: don't use unaligned CopyMem () on NOR flash Gerd Hoffmann
@ 2023-01-16 11:10 ` Ard Biesheuvel
  2023-01-16 11:50   ` Ard Biesheuvel
  0 siblings, 1 reply; 3+ messages in thread
From: Ard Biesheuvel @ 2023-01-16 11:10 UTC (permalink / raw)
  To: devel, kraxel
  Cc: Oliver Steffen, Leif Lindholm, Pawel Polawski, Ard Biesheuvel,
	Sami Mujawar

On Mon, 16 Jan 2023 at 10:46, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> Fixes: e5ec3ba409b5 ("OvmfPkg/VirtNorFlashDxe: map flash memory as uncacheable")
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Thanks Gerd.

I'll tweak the fixes line and the commit log a bit:

Commit 789a72328553 reclassified the NOR flash region as EFI_MEMORY_WC
in the OS visible EFI memory map, and dropped the explicit aligned
CopyMem() implementation, in the assumption that EFI_MEMORY_WC will be
honored by the OS, and that the region will be mapped in a way that
tolerates misaligned accesses. However, Linux today uses device
attributes for all EFI MMIO regions, in spite of the memory type
attributes, and so using misaligned accesses is never safe.

So instead, switch to the generic CopyMem() implementation entirely,
just like we already did for VariableRuntimeDxe.

Fixes: 789a72328553 ("OvmfPkg/VirtNorFlashDxe: use EFI_MEMORY_WC and
drop AlignedCopyMem()")

> ---
>  ArmVirtPkg/ArmVirtKvmTool.dsc    | 6 +++++-
>  ArmVirtPkg/ArmVirtQemu.dsc       | 6 +++++-
>  ArmVirtPkg/ArmVirtQemuKernel.dsc | 6 +++++-
>  3 files changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/ArmVirtPkg/ArmVirtKvmTool.dsc b/ArmVirtPkg/ArmVirtKvmTool.dsc
> index 2ba00bd08ff1..d0afe1b49e25 100644
> --- a/ArmVirtPkg/ArmVirtKvmTool.dsc
> +++ b/ArmVirtPkg/ArmVirtKvmTool.dsc
> @@ -296,7 +296,11 @@ [Components.common]
>        NULL|ArmVirtPkg/Library/ArmVirtTimerFdtClientLib/ArmVirtTimerFdtClientLib.inf
>    }
>
> -  OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.inf
> +  OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.inf {
> +    <LibraryClasses>
> +      # don't use unaligned CopyMem () on the UEFI varstore NOR flash region
> +      BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
> +  }
>
>    MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
>
> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
> index 5dd8b6104cca..0f1c6395488a 100644
> --- a/ArmVirtPkg/ArmVirtQemu.dsc
> +++ b/ArmVirtPkg/ArmVirtQemu.dsc
> @@ -428,7 +428,11 @@ [Components.common]
>      <LibraryClasses>
>        NULL|ArmVirtPkg/Library/ArmVirtTimerFdtClientLib/ArmVirtTimerFdtClientLib.inf
>    }
> -  OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.inf
> +  OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.inf {
> +    <LibraryClasses>
> +      # don't use unaligned CopyMem () on the UEFI varstore NOR flash region
> +      BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
> +  }
>    MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
>
>    #
> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> index f5db3ac432f3..807c85d48285 100644
> --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
> +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> @@ -331,7 +331,11 @@ [Components.common]
>      <LibraryClasses>
>        NULL|ArmVirtPkg/Library/ArmVirtTimerFdtClientLib/ArmVirtTimerFdtClientLib.inf
>    }
> -  OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.inf
> +  OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.inf {
> +    <LibraryClasses>
> +      # don't use unaligned CopyMem () on the UEFI varstore NOR flash region
> +      BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
> +  }
>    MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
>
>    #
> --
> 2.39.0
>
>
>
> 
>
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [edk2-devel] [PATCH 1/1] ArmVirt: don't use unaligned CopyMem () on NOR flash
  2023-01-16 11:10 ` [edk2-devel] " Ard Biesheuvel
@ 2023-01-16 11:50   ` Ard Biesheuvel
  0 siblings, 0 replies; 3+ messages in thread
From: Ard Biesheuvel @ 2023-01-16 11:50 UTC (permalink / raw)
  To: devel, kraxel
  Cc: Oliver Steffen, Leif Lindholm, Pawel Polawski, Ard Biesheuvel,
	Sami Mujawar

On Mon, 16 Jan 2023 at 12:10, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Mon, 16 Jan 2023 at 10:46, Gerd Hoffmann <kraxel@redhat.com> wrote:
> >
> > Fixes: e5ec3ba409b5 ("OvmfPkg/VirtNorFlashDxe: map flash memory as uncacheable")
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>
> Thanks Gerd.
>
> I'll tweak the fixes line and the commit log a bit:
>
> Commit 789a72328553 reclassified the NOR flash region as EFI_MEMORY_WC
> in the OS visible EFI memory map, and dropped the explicit aligned
> CopyMem() implementation, in the assumption that EFI_MEMORY_WC will be
> honored by the OS, and that the region will be mapped in a way that
> tolerates misaligned accesses. However, Linux today uses device
> attributes for all EFI MMIO regions, in spite of the memory type
> attributes, and so using misaligned accesses is never safe.
>
> So instead, switch to the generic CopyMem() implementation entirely,
> just like we already did for VariableRuntimeDxe.
>
> Fixes: 789a72328553 ("OvmfPkg/VirtNorFlashDxe: use EFI_MEMORY_WC and
> drop AlignedCopyMem()")
>

Merged as #3903.

> > ---
> >  ArmVirtPkg/ArmVirtKvmTool.dsc    | 6 +++++-
> >  ArmVirtPkg/ArmVirtQemu.dsc       | 6 +++++-
> >  ArmVirtPkg/ArmVirtQemuKernel.dsc | 6 +++++-
> >  3 files changed, 15 insertions(+), 3 deletions(-)
> >
> > diff --git a/ArmVirtPkg/ArmVirtKvmTool.dsc b/ArmVirtPkg/ArmVirtKvmTool.dsc
> > index 2ba00bd08ff1..d0afe1b49e25 100644
> > --- a/ArmVirtPkg/ArmVirtKvmTool.dsc
> > +++ b/ArmVirtPkg/ArmVirtKvmTool.dsc
> > @@ -296,7 +296,11 @@ [Components.common]
> >        NULL|ArmVirtPkg/Library/ArmVirtTimerFdtClientLib/ArmVirtTimerFdtClientLib.inf
> >    }
> >
> > -  OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.inf
> > +  OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.inf {
> > +    <LibraryClasses>
> > +      # don't use unaligned CopyMem () on the UEFI varstore NOR flash region
> > +      BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
> > +  }
> >
> >    MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
> >
> > diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
> > index 5dd8b6104cca..0f1c6395488a 100644
> > --- a/ArmVirtPkg/ArmVirtQemu.dsc
> > +++ b/ArmVirtPkg/ArmVirtQemu.dsc
> > @@ -428,7 +428,11 @@ [Components.common]
> >      <LibraryClasses>
> >        NULL|ArmVirtPkg/Library/ArmVirtTimerFdtClientLib/ArmVirtTimerFdtClientLib.inf
> >    }
> > -  OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.inf
> > +  OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.inf {
> > +    <LibraryClasses>
> > +      # don't use unaligned CopyMem () on the UEFI varstore NOR flash region
> > +      BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
> > +  }
> >    MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
> >
> >    #
> > diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> > index f5db3ac432f3..807c85d48285 100644
> > --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
> > +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> > @@ -331,7 +331,11 @@ [Components.common]
> >      <LibraryClasses>
> >        NULL|ArmVirtPkg/Library/ArmVirtTimerFdtClientLib/ArmVirtTimerFdtClientLib.inf
> >    }
> > -  OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.inf
> > +  OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.inf {
> > +    <LibraryClasses>
> > +      # don't use unaligned CopyMem () on the UEFI varstore NOR flash region
> > +      BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
> > +  }
> >    MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
> >
> >    #
> > --
> > 2.39.0
> >
> >
> >
> > 
> >
> >

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-01-16 11:50 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-16  9:46 [PATCH 1/1] ArmVirt: don't use unaligned CopyMem () on NOR flash Gerd Hoffmann
2023-01-16 11:10 ` [edk2-devel] " Ard Biesheuvel
2023-01-16 11:50   ` Ard Biesheuvel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox