Re: [PATCH] ArmPlatformPkg/ArmVExpressPkg: Fix memory attributes for NOR Flash

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

Hi Achin,

On Wed, Jan 18, 2017 at 08:24:06PM +0000, achin.gupta@arm.com wrote:
> From: Achin Gupta <achin.gupta@arm.com>
> 
> The NOR flash banks were being mapped in the translation tables with the same
> memory attributes as RAM in the system. These attributes mark the region as
> Normal Memory and could additionally be cacheable or non-cacheable.
> 
> Either type of attributes are unsuitable for NOR Flash since write operations
> could be performed on it. Normal Memory does not guarantee ordering of
> transactions that Device memory does. So the commands sent to the Flash device
> may not arrive in the right order unless barriers are used. Commands might not
> get past the CPU caches in case the region has been mapped with cacheable
> attributes.
> 
> This patch fixes the problem by mapping the NOR Flash memory region with Device
> memory attributes.

To add some background to Ard's comment - this was not unintentionally
done:
https://github.com/tianocore/edk2/commit/19bb46c411279dcd30d540c56e5993c5f771c319

Was the reasoning behind this commit incorrect - do you have a
(pre-DXE?) use-case that creates a problem?

Regards,

Leif

> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Achin Gupta <achin.gupta@arm.com>
> ---
>  ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
> index 14c7e8e..2685114 100644
> --- a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
> +++ b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
> @@ -116,7 +116,7 @@ ArmPlatformGetVirtualMemoryMap (
>    VirtualMemoryTable[++Index].PhysicalBase = ARM_VE_SMB_NOR0_BASE;
>    VirtualMemoryTable[Index].VirtualBase = ARM_VE_SMB_NOR0_BASE;
>    VirtualMemoryTable[Index].Length = ARM_VE_SMB_NOR0_SZ + ARM_VE_SMB_NOR1_SZ;
> -  VirtualMemoryTable[Index].Attributes = CacheAttributes;
> +  VirtualMemoryTable[Index].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
>  
>    // SMB CS2 - SRAM
>    VirtualMemoryTable[++Index].PhysicalBase = ARM_VE_SMB_SRAM_BASE;
> -- 
> 1.9.1
>