public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] ArmVirtPkg/FdtParser: avoid unaligned accesses with the MMU off
@ 2016-09-13 14:17 Ard Biesheuvel
  2016-09-13 14:30 ` Laszlo Ersek
  0 siblings, 1 reply; 2+ messages in thread
From: Ard Biesheuvel @ 2016-09-13 14:17 UTC (permalink / raw)
  To: edk2-devel, lersek; +Cc: julien.grall, Ard Biesheuvel

When parsing the device tree to find the memory node, we are still running
with the MMU off, which means unaligned memory accesses are not allowed.
Since the FDT only mandates 32-bit alignment, 64-bit quantities are not
guaranteed to appear naturally aligned, and so should be accessed using
32-bit accesses instead.

Reported-by: Julien Grall <julien.grall@arm.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmVirtPkg/Library/ArmQemuRelocatablePlatformLib/FdtParser.c | 14 ++++++--------
 ArmVirtPkg/Library/ArmXenRelocatablePlatformLib/FdtParser.c  | 14 ++++++--------
 2 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/ArmVirtPkg/Library/ArmQemuRelocatablePlatformLib/FdtParser.c b/ArmVirtPkg/Library/ArmQemuRelocatablePlatformLib/FdtParser.c
index 46a5fe6409f6..afdc81a8839d 100644
--- a/ArmVirtPkg/Library/ArmQemuRelocatablePlatformLib/FdtParser.c
+++ b/ArmVirtPkg/Library/ArmQemuRelocatablePlatformLib/FdtParser.c
@@ -65,17 +65,15 @@ FindMemnode (
     return FALSE;
   }
 
-  if (AddressCells == 1) {
-    *SystemMemoryBase = fdt32_to_cpu (*Prop);
-  } else {
-    *SystemMemoryBase = fdt64_to_cpu (*(UINT64 *)Prop);
+  *SystemMemoryBase = fdt32_to_cpu (Prop[0]);
+  if (AddressCells > 1) {
+    *SystemMemoryBase = (*SystemMemoryBase << 32) | fdt32_to_cpu (Prop[1]);
   }
   Prop += AddressCells;
 
-  if (SizeCells == 1) {
-    *SystemMemorySize = fdt32_to_cpu (*Prop);
-  } else {
-    *SystemMemorySize = fdt64_to_cpu (*(UINT64 *)Prop);
+  *SystemMemorySize = fdt32_to_cpu (Prop[0]);
+  if (SizeCells > 1) {
+    *SystemMemorySize = (*SystemMemorySize << 32) | fdt32_to_cpu (Prop[1]);
   }
 
   return TRUE;
diff --git a/ArmVirtPkg/Library/ArmXenRelocatablePlatformLib/FdtParser.c b/ArmVirtPkg/Library/ArmXenRelocatablePlatformLib/FdtParser.c
index 992932ee9754..38fd5d3ed00c 100644
--- a/ArmVirtPkg/Library/ArmXenRelocatablePlatformLib/FdtParser.c
+++ b/ArmVirtPkg/Library/ArmXenRelocatablePlatformLib/FdtParser.c
@@ -65,17 +65,15 @@ FindMemnode (
     return FALSE;
   }
 
-  if (AddressCells == 1) {
-    *SystemMemoryBase = fdt32_to_cpu (*Prop);
-  } else {
-    *SystemMemoryBase = fdt64_to_cpu (*(UINT64 *)Prop);
+  *SystemMemoryBase = fdt32_to_cpu (Prop[0]);
+  if (AddressCells > 1) {
+    *SystemMemoryBase = (*SystemMemoryBase << 32) | fdt32_to_cpu (Prop[1]);
   }
   Prop += AddressCells;
 
-  if (SizeCells == 1) {
-    *SystemMemorySize = fdt32_to_cpu (*Prop);
-  } else {
-    *SystemMemorySize = fdt64_to_cpu (*(UINT64 *)Prop);
+  *SystemMemorySize = fdt32_to_cpu (Prop[0]);
+  if (SizeCells > 1) {
+    *SystemMemorySize = (*SystemMemorySize << 32) | fdt32_to_cpu (Prop[1]);
   }
 
   return TRUE;
-- 
2.7.4



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

* Re: [PATCH] ArmVirtPkg/FdtParser: avoid unaligned accesses with the MMU off
  2016-09-13 14:17 [PATCH] ArmVirtPkg/FdtParser: avoid unaligned accesses with the MMU off Ard Biesheuvel
@ 2016-09-13 14:30 ` Laszlo Ersek
  0 siblings, 0 replies; 2+ messages in thread
From: Laszlo Ersek @ 2016-09-13 14:30 UTC (permalink / raw)
  To: Ard Biesheuvel, edk2-devel; +Cc: julien.grall

On 09/13/16 16:17, Ard Biesheuvel wrote:
> When parsing the device tree to find the memory node, we are still running
> with the MMU off, which means unaligned memory accesses are not allowed.
> Since the FDT only mandates 32-bit alignment, 64-bit quantities are not
> guaranteed to appear naturally aligned, and so should be accessed using
> 32-bit accesses instead.
> 
> Reported-by: Julien Grall <julien.grall@arm.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmVirtPkg/Library/ArmQemuRelocatablePlatformLib/FdtParser.c | 14 ++++++--------
>  ArmVirtPkg/Library/ArmXenRelocatablePlatformLib/FdtParser.c  | 14 ++++++--------
>  2 files changed, 12 insertions(+), 16 deletions(-)
> 
> diff --git a/ArmVirtPkg/Library/ArmQemuRelocatablePlatformLib/FdtParser.c b/ArmVirtPkg/Library/ArmQemuRelocatablePlatformLib/FdtParser.c
> index 46a5fe6409f6..afdc81a8839d 100644
> --- a/ArmVirtPkg/Library/ArmQemuRelocatablePlatformLib/FdtParser.c
> +++ b/ArmVirtPkg/Library/ArmQemuRelocatablePlatformLib/FdtParser.c
> @@ -65,17 +65,15 @@ FindMemnode (
>      return FALSE;
>    }
>  
> -  if (AddressCells == 1) {
> -    *SystemMemoryBase = fdt32_to_cpu (*Prop);
> -  } else {
> -    *SystemMemoryBase = fdt64_to_cpu (*(UINT64 *)Prop);
> +  *SystemMemoryBase = fdt32_to_cpu (Prop[0]);
> +  if (AddressCells > 1) {
> +    *SystemMemoryBase = (*SystemMemoryBase << 32) | fdt32_to_cpu (Prop[1]);
>    }
>    Prop += AddressCells;
>  
> -  if (SizeCells == 1) {
> -    *SystemMemorySize = fdt32_to_cpu (*Prop);
> -  } else {
> -    *SystemMemorySize = fdt64_to_cpu (*(UINT64 *)Prop);
> +  *SystemMemorySize = fdt32_to_cpu (Prop[0]);
> +  if (SizeCells > 1) {
> +    *SystemMemorySize = (*SystemMemorySize << 32) | fdt32_to_cpu (Prop[1]);
>    }
>  
>    return TRUE;
> diff --git a/ArmVirtPkg/Library/ArmXenRelocatablePlatformLib/FdtParser.c b/ArmVirtPkg/Library/ArmXenRelocatablePlatformLib/FdtParser.c
> index 992932ee9754..38fd5d3ed00c 100644
> --- a/ArmVirtPkg/Library/ArmXenRelocatablePlatformLib/FdtParser.c
> +++ b/ArmVirtPkg/Library/ArmXenRelocatablePlatformLib/FdtParser.c
> @@ -65,17 +65,15 @@ FindMemnode (
>      return FALSE;
>    }
>  
> -  if (AddressCells == 1) {
> -    *SystemMemoryBase = fdt32_to_cpu (*Prop);
> -  } else {
> -    *SystemMemoryBase = fdt64_to_cpu (*(UINT64 *)Prop);
> +  *SystemMemoryBase = fdt32_to_cpu (Prop[0]);
> +  if (AddressCells > 1) {
> +    *SystemMemoryBase = (*SystemMemoryBase << 32) | fdt32_to_cpu (Prop[1]);
>    }
>    Prop += AddressCells;
>  
> -  if (SizeCells == 1) {
> -    *SystemMemorySize = fdt32_to_cpu (*Prop);
> -  } else {
> -    *SystemMemorySize = fdt64_to_cpu (*(UINT64 *)Prop);
> +  *SystemMemorySize = fdt32_to_cpu (Prop[0]);
> +  if (SizeCells > 1) {
> +    *SystemMemorySize = (*SystemMemorySize << 32) | fdt32_to_cpu (Prop[1]);
>    }
>  
>    return TRUE;
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>


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

end of thread, other threads:[~2016-09-13 14:30 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-13 14:17 [PATCH] ArmVirtPkg/FdtParser: avoid unaligned accesses with the MMU off Ard Biesheuvel
2016-09-13 14:30 ` Laszlo Ersek

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