Re: [PATCH] xen/arm64: Use __flush_dcache_area instead of __flush_dcache_all

[Ian Campbell <ian.campbell@citrix.com>]

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.