From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.101.70]) by ml01.01.org (Postfix) with ESMTP id 065F881E46 for ; Wed, 18 Jan 2017 14:26:03 -0800 (PST) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id C938CF; Wed, 18 Jan 2017 14:26:02 -0800 (PST) Received: from [192.168.150.233] (u201365.usa.arm.com [10.118.28.82]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id 7E5233F220; Wed, 18 Jan 2017 14:26:02 -0800 (PST) Message-ID: <1484778362.3022.4.camel@arm.com> From: Supreeth Venkatesh To: Leif Lindholm , achin.gupta@arm.com Cc: edk2-devel@lists.01.org, nd@arm.com, ard.biesheuvel@linaro.org Date: Wed, 18 Jan 2017 16:26:02 -0600 In-Reply-To: <20170118220500.GW25883@bivouac.eciton.net> References: <1484771046-21296-1-git-send-email-achin.gupta@arm.com> <20170118220500.GW25883@bivouac.eciton.net> X-Mailer: Evolution 3.18.5.2-0ubuntu3 Mime-Version: 1.0 Subject: Re: [PATCH] ArmPlatformPkg/ArmVExpressPkg: Fix memory attributes for NOR Flash X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 18 Jan 2017 22:26:03 -0000 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit On Wed, 2017-01-18 at 22:05 +0000, Leif Lindholm wrote: > Hi Achin, > > On Wed, Jan 18, 2017 at 08:24:06PM +0000, achin.gupta@arm.com wrote: > > > > From: Achin Gupta > > > > 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/19bb46c411279dcd30d540c56e59 > 93c5f771c319 > > Was the reasoning behind this commit incorrect - do you have a > (pre-DXE?) use-case that creates a problem? > Use Case that creates a problem: With cache state modeling enabled (i.e, setting it to "1") in FVP, it will not boot up.  Perhaps, need to check FVP implementation is correct?  > Regards, > > Leif > > > > > Contributed-under: TianoCore Contribution Agreement 1.0 > > Signed-off-by: Achin Gupta > > --- > >  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;