From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH] xen/arm64: Use __flush_dcache_area instead of __flush_dcache_all Date: Tue, 14 Oct 2014 11:39:37 +0100 Message-ID: <1413283177.10417.34.camel@citrix.com> References: <1412610550-26964-1-git-send-email-suravee.suthikulpanit@amd.com> <20141006162828.GA22575@leverpostej> <20141007104035.GB24725@leverpostej> <20141014092106.GF16598@leverpostej> <1413279323.1497.27.camel@citrix.com> <20141014103233.GH16598@leverpostej> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20141014103233.GH16598@leverpostej> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Mark Rutland Cc: "julien.grall@linaro.org" , "xen-devel@lists.xen.org" , Roy Franz , "suravee.suthikulpanit@amd.com" , "stefano.stabellini@eu.citrix.com" List-Id: xen-devel@lists.xenproject.org On Tue, 2014-10-14 at 11:32 +0100, Mark Rutland wrote: > On Tue, Oct 14, 2014 at 10:35:23AM +0100, Ian Campbell wrote: > > On Tue, 2014-10-14 at 10:21 +0100, Mark Rutland wrote: > > > Hi Roy, > > > > > > [...] > > > > > > > It seems that for Xen we do need to flush the FDT as well - I get a > > > > variety of crashes > > > > with a corrupt FDT when cache state is modeled on the FVP model, and > > > > Suravee sees similar > > > > behavior on Seattle. I was not expecting this, as I looked at the code > > > > in Xen and the caches/TLB > > > > are enabled quite early on, before the FDT is accessed by Xen. I then > > > > looked at the mappings > > > > used by edk2 and Xen, and found some differences. Even after > > > > modifying edk2 to use the same > > > > configuration as Xen, the flushing of the FDT is still required. Xen > > > > and edk2 use the same memory > > > > attributes in the MAIR_EL2 register (0xFF), but had different > > > > sharing, access perm, and nG settings. > > > > > > I don't think the access perm or nG settings should have any effect, but > > > the shareability forms part of the memory attributes (along with the > > > memory type and cacheability), and there are several rules that apply > > > when accessing a memory location with mismatched attributes. See the > > > ARMv8 ARM - The AArch64 Application Level Memory Model - Mismatched > > > memory attributes. > > > > > > In Linux we're likely getting lucky, and the shareability we use varies > > > for an SMP or UP kernel. So we need maintenance in at least one of those > > > cases. This would also apply to any initrd or other image. > > > > > > Do you happen to know the shareability used by EDK2 and Xen? > > > > Xen maps everything inner-shareable. Dunno about EDK2. > > Ok. That matches what an SMP Linux kernel will do, so it looks like > we're just getting lucky with Linux. I'lll have a play and see if I can > trigger similar issues. > > > Is the real issue here not a lack of specification for some corner cases > > of the boot protocol? Can we get that fixed somehow? > > To an extent, yes. We can try to fix up the Linux side with patche to > Documentation/arm64/booting.txt. As far as I am aware, for UEFI that > will require membership of the UEFI forum. > Is Documentation/arm64/booting.txt relevant here since the kernel is being launched as an EFI app, which already has a standardised calling convention of its own. I suppose booting.txt is in addition to the UEFI convention. It probably would be best to formalise that (what if a second OS comes along with contradictory requirements?) > > Part of me wants to suggest that UEFI (and bootloaders generally) ought > > to be cleaning caches for anything they have loaded into RAM before > > launching an OS as a matter of good hygiene. > > In general, yes. > > Unfortunately, UEFI can't perform the maintenance in this case, because > the stub modifies things. I was under the impression it copied and > modified the FDT to embed the command line -- UEFI has no visibiltiy of > this and therefore cannot be in charge of flushing it. So in this case, > the stub needs to be thought of as the bootloader, and needs to be in > charge of any required maintenance. Right, that's what I was thinking. UEFI enters bootloader with everything it has done all nice and clean and consistent. Anything the stub then does it is responsible for maintaining the cleanliness. > There are a tonne of subtleties here, and certain properties we would > like (e.g. a completely clean cache hierarchy upon entry to the OS) > aren't necessarily possible to provide in general (thanks to the wonders > of non-architected system level caches, interaction with bootloaders, > etc). I suppose it is easier for the UEFI implementation, since it knows the platform it runs on and there knows about the caches. Harder for the stub though :-/ Ian.