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

[Tim Deegan <tim@xen.org>]

At 12:01 +0000 on 13 Nov (1352808094), Stefano Stabellini wrote:
> > 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?
> 
> I think that is OK, but I would like to avoid reading CCSIDR every
> single time we need to do a dcache flush

The code above only reads it once for each large dcache flush.  When I
was writing it I did think of just stashing cacheline_bytes in a
read_mostly somewhere, but I had the opposite concern -- wouldn't
reading this constant from on-chip be quicker than going to the memory
bus for it? :)

I'm happy either way.

Tim.