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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2014-10-06 at 21:15 -0700, Roy Franz wrote:
> On Mon, Oct 6, 2014 at 9:28 AM, Mark Rutland <mark.rutland@xxxxxxx> wrote:
> > Hi Suravee,
> >
> > On Mon, Oct 06, 2014 at 04:49:10PM +0100, suravee.suthikulpanit@xxxxxxx 
> > wrote:
> >> From: Suravee Suthikulpanit <Suravee.Suthikulpanit@xxxxxxx>
> >>
> >> when booting with EFI, __flush_dcache_all does not correctly flush data.
> >>
> >> According to Mark Rutland, __flush_dcache_all does not guaranteed to push
> >> data to the PoC if there is a system-level cache as it uses Set/Way
> >> operations.
> >
> > A better way to look at this is that Set/Way operations are never
> > guaranteed to flush data to the PoC, regardless of the presence of a
> > system-level cache. They might on certain implementations, but that's
> > not an architectural guarantee. The same caveat applies to using them to
> > push data to other points in the cache hierarchy (PoUU or PoUIS).
> >
> > Generally, Set/Way cache maintenance operations can only be used to
> > empty or clean the architected caches visible to a given CPU, and only
> > when all masters sharing those caches have been prevented from
> > allocating any cache entries. Outside of IMPLEMENTATION DEFINED
> > power-down sequences or reset-like operations they are typically the
> > wrong thing to use.
> >
> > So any other uses of Set/Way operations should also be treated as
> > suspect, and are likely to be problematic on platforms with system-level
> > caches.
> 
> So what all do we need to flush?  Do we need to flush all modified
> (dirty) cache lines,
> or just a specific subset?
> 
> In Linux the FDT which is modified in the Linux EFI stub isn't
> flushed, nor is the EFI memory map,
> both of which are modified by the UEFI firmware/boot stub.  I feel
> like I'm missing
> something here.

Mark was making reference on IRC to other missing flushes even in Linux.
Not sure if those include the ones which you mention...

> Also, we should remove flush_dcache_all, as that was added for use in
> the EFI boot code.  If we
> don't use it there it doesn't have a user in Xen.

Absolutely, especially given that it turns out to be dangerous to use
under most circumstances!

> > I take it Xen doesn't relocate itself?
> 
> Xen does relocate itself, but that is done later in the boot process
> that is common between the EFI and Image boot methods.

Even with it happening later it's possible that we might need to flush
some additional stuff on entry via the EFI path? e.g. there could be
stuff which the non-EFI code path was previously implicitly assuming
wouldn't be cached (because caches were never enabled on such
bootloaders, etc).

In fact I'd suggest that those missing flushes (if any, maybe we already
got it all right) really belong in the relocation code rather than in
the EFI stub, since even on non-EFI it seems fragile to rely on specific
caching behaviour from the bootloader. I suppose we will cross that
bridge when Suravee get's as far as that!

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel