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

[Tim Deegan <tim@xen.org>]

At 19:40 +0100 on 26 Oct (1351280413), Stefano Stabellini wrote:
> > 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));
>     }
> }

I think we should have two functions.  One should look almost like that
and be for flushing large ranges, and one much simpler for flushing
small items.  Like this (totally untested, uncompiled even):

 #define MIN_CACHELINE_BYTES 32 // or whatever

 /* In setup.c somewhere. */
 if ( READ_CP32(CCSIDR) < MIN_CACHELINE_BYTES )
     panic("CPU has preposterously small cache lines");

 /* Function for flushing medium-sized areas.
  * if 'range' is large enough we might want to use model-specific
  * full-cache flushes. */
 static inline void flush_xen_dcache_va_range(void *p, unsigned long size)
 {
     void *end;
     unsigned long cacheline_bytes = READ_CP32(CCSIDR);
     barrier();       /* So the compiler issues all writes to the range */
     dsb();           /* So the CPU issues all writes to the range */ 
     for ( end = p + size; p < end; p += cacheline_bytes )
         WRITE_CP32(DCCMVAC, p);
     dsb();           /* So we know the flushes happen before continuing */
 }

 /* Macro for flushing a single small item.  The predicate is always 
  * compile-time constant so this will compile down to 3 instructions in
  * the common case.  Make sure to call it with the correct type of
  * pointer! */
 #define flush_xen_dcache_va(p) do {                       \
     typeof(p) _p = (p);                                   \
     if ( (sizeof *_p) > MIN_CACHELINE_BYTES )             \
         flush_xen_dcache_va_range(_p, sizeof *_p);        \
     else                                                  \
         asm volatile (                                    \
             "dsb;"   /* Finish all earlier writes */      \
             STORE_CP32(0, DCCMVAC)                        \
             "dsb;"   /* Finish flush before continuing */ \
             : : "r" (_p), "m" (*_p));                     \
 } while (0)

What do you think?

Tim.