Re: [PATCH 6/7] xen/arm: flush D-cache and I-cache when appropriate

[Tim Deegan <tim@xen.org>]

At 17:03 +0100 on 26 Oct (1351271018), Stefano Stabellini wrote:
> On Fri, 26 Oct 2012, Stefano Stabellini wrote:
> > On Fri, 26 Oct 2012, Stefano Stabellini wrote:
> > > On Fri, 26 Oct 2012, Tim Deegan wrote:
> > > > At 18:35 +0100 on 24 Oct (1351103740), Stefano Stabellini wrote:
> > > > > > I don't think this is necessary - why not just pass va directly to the
> > > > > > inline asm?  We don't care what register it's in (and if we did I'm not
> > > > > > convinced this would guarantee it was r0).
> > > > > > 
> > > > > > > +    asm volatile (
> > > > > > > +        "dsb;"
> > > > > > > +        STORE_CP32(0, DCCMVAC)
> > > > > > > +        "isb;"
> > > > > > > +        : : "r" (r0) : "memory");
> > > > > > 
> > > > > > Does this need a 'memory' clobber?  Can we get away with just saying it
> > > > > > consumes *va as an input?  All we need to be sure of is that the
> > > > > > particular thing we're flushing has been written out; no need to stop
> > > > > > any other optimizations.
> > > > > 
> > > > > you are right on both points
> > > > > 
> > > > > > I guess it might need to be re-cast as a macro so the compiler knows how
> > > > > > big *va is?
> > > > > 
> > > > > I don't think it is necessary, after all the size of a register has to
> > > > > be the same of a virtual address
> > > > 
> > > > But it's the size of the thing in memory that's being flushed that
> > > > matters, not the size of the pointer to it!
> > > >
> > > > E.g. after a PTE write we
> > > > need a 64-bit memory input operand to stop the compiler from hoisting
> > > > any part of the PTE write past the cache flush. (well OK we explicitly use
> > > > a 64-bit atomic write for PTE writes, but YKWIM).
> > > 
> > > The implementation of write_pte is entirely in assembly so I doubt that
> > > the compiler is going to reorder it.
> > > 
> > > However I see your point in case of flush_xen_dcache_va.
> > > Wouldn't a barrier() at the beginning of the function be enough?
> > >
> > 
> > Actually we already have a memory clobber in flush_xen_dcache_va,
> > shouldn't that prevent the compiler from reordering instructions?
> > 
> 
> After reading again the entire thread I think that I understand what you
> meant: can't we get rid of the memory clobber and specify *va as input?
> 
> However in order to do that correctly we need to make clear how big *va is
> otherwise the compiler could reorder things.

Yes!

> The problem is that we don't know at compile time how big a cacheline
> is, so I think that we need to keep the memory clobber.

No!  It's always safe to flush the entire line -- regardless of what
other writes might be happening to it.  After all, the cache controller
might evict that line on its own at any time, so nothing can be relying
on its _not_ being flushed.

The only problem with not knowing how big a cacheline is is this: if the
object is _bigger_ than a cache line we might use one DCCMVAC where more
than one is needed.  We can avoid that by a compile-time check on some
sort of architectural minimum cacheline size.

Tim.