Re: [PATCH] ArmPlatformPkg: remove EFI_MEMORY_UC attribute from normal memory

[Leif Lindholm <leif.lindholm@linaro.org>]

On Thu, Sep 08, 2016 at 09:13:38AM +0100, Ard Biesheuvel wrote:
> On ARM systems, mapping normal memory as device memory may have unintended
> side effects, given that unaligned accesses or loads and stores with special
> semantics (e.g., load/store exclusive) may fault or may not work as expected.
> Similarly, DC ZVA instructions are only supported on normal memory, not
> device memory.

I think this is the right thing to do; Arguably, on the modern ARM
architectures, UNCACHEABLE and WRITE_COMBINEABLE are mutually
exclusive. I'll discuss with Charles whether we should codify this in
the UEFI specification.

> So remove the EFI_MEMORY_UC attribute that we set by default on system RAM.
> If any region requires this attribute, it is up to the driver to set this
> attribute, and to ensure that no offending operations are performed on it.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
>  ArmPlatformPkg/ArmJunoPkg/Library/ArmJunoLib/ArmJunoMem.c          | 1 -
>  ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c | 1 -
>  ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c                    | 1 -
>  3 files changed, 3 deletions(-)
> 
> diff --git a/ArmPlatformPkg/ArmJunoPkg/Library/ArmJunoLib/ArmJunoMem.c b/ArmPlatformPkg/ArmJunoPkg/Library/ArmJunoLib/ArmJunoMem.c
> index 41731c1ebdb8..aa8d7d9c3b0d 100644
> --- a/ArmPlatformPkg/ArmJunoPkg/Library/ArmJunoLib/ArmJunoMem.c
> +++ b/ArmPlatformPkg/ArmJunoPkg/Library/ArmJunoLib/ArmJunoMem.c
> @@ -56,7 +56,6 @@ ArmPlatformGetVirtualMemoryMap (
>    ResourceAttributes =
>        EFI_RESOURCE_ATTRIBUTE_PRESENT |
>        EFI_RESOURCE_ATTRIBUTE_INITIALIZED |
> -      EFI_RESOURCE_ATTRIBUTE_UNCACHEABLE |
>        EFI_RESOURCE_ATTRIBUTE_WRITE_COMBINEABLE |
>        EFI_RESOURCE_ATTRIBUTE_WRITE_THROUGH_CACHEABLE |
>        EFI_RESOURCE_ATTRIBUTE_WRITE_BACK_CACHEABLE |
> diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
> index c6df2e1b3de4..115df246796d 100644
> --- a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
> +++ b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
> @@ -61,7 +61,6 @@ ArmPlatformGetVirtualMemoryMap (
>      ResourceAttributes =
>          EFI_RESOURCE_ATTRIBUTE_PRESENT |
>          EFI_RESOURCE_ATTRIBUTE_INITIALIZED |
> -        EFI_RESOURCE_ATTRIBUTE_UNCACHEABLE |
>          EFI_RESOURCE_ATTRIBUTE_WRITE_COMBINEABLE |
>          EFI_RESOURCE_ATTRIBUTE_WRITE_THROUGH_CACHEABLE |
>          EFI_RESOURCE_ATTRIBUTE_WRITE_BACK_CACHEABLE |
> diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
> index 75e6631d7f30..2feb11f21d5d 100644
> --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
> +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
> @@ -89,7 +89,6 @@ MemoryPeim (
>    ResourceAttributes = (
>        EFI_RESOURCE_ATTRIBUTE_PRESENT |
>        EFI_RESOURCE_ATTRIBUTE_INITIALIZED |
> -      EFI_RESOURCE_ATTRIBUTE_UNCACHEABLE |
>        EFI_RESOURCE_ATTRIBUTE_WRITE_COMBINEABLE |
>        EFI_RESOURCE_ATTRIBUTE_WRITE_THROUGH_CACHEABLE |
>        EFI_RESOURCE_ATTRIBUTE_WRITE_BACK_CACHEABLE |
> -- 
> 2.7.4
>