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

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Fri, 26 Oct 2012, Tim Deegan wrote:
> 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.
 
If we want to do that then we need to pass a size argument to
flush_xen_dcache_va and have a loop in the function, each iteration
flushing a cacheline:


static inline void flush_xen_dcache_va(void *p, unsigned long size)
{
    unsigned long cacheline_size = READ_CP32(CCSIDR);
    unsigned long i;
    for (i = 0; i < size; i += cacheline_size, p += cacheline_size) {
        asm volatile (
            "dsb;"
            STORE_CP32(0, DCCMVAC)
            "dsb;"
            : : "r" (p));
    }
}


Even though I think that it is really unlikely that the compiler moves
the writes after the loop, in theory we would still need to specify the
correct size of the input parameter to the inline assembly.

Is there a better way?

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