Re: [PATCH] ArmVirtPkg/FdtParser: avoid unaligned accesses with the MMU off

[Laszlo Ersek <lersek@redhat.com>]

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>