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

[Tim Deegan <tim@xxxxxxx>]

Sorry for the late review - I meant to get to these on Thursday but
hadn't time.  

At 12:55 +0000 on 19 Nov (1353329757), Ian Campbell wrote:
> > +/* 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. 

This is no longer true!  The predicate now depends on the alignment of
the pointer, so this will compile to quite a bit of code, including the
memory barrier I was trying to avoid.

If we can sensibly restrict this to naturally-aligned objects smaller
than a cacheline, we should go back to the previous version of this
macro.  If not, I think we might as well just use
flush_xen_dcache_va_range(_p, sizeof *_p) and be done with it.

Tim.

> >                       Make sure to call it with the correct type of
> > + * pointer! */
> > +#define flush_xen_dcache_va(p) do {                                     \
> > +    int cacheline_bytes  = READ_CP32(CCSIDR);                           \
> > +    typeof(p) _p = (p);                                                 \
> > +    if ( ((unsigned long)_p & ~(cacheline_bytes - 1)) !=                \
> > +        (((unsigned long)_p + (sizeof *_p)) & ~(cacheline_bytes - 1)) ) \
> > +        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)
> > +
> > +
> >  /*
> >   * Flush all hypervisor mappings from the TLB and branch predictor.
> > - * This is needed after changing Xen code mappings. 
> > + * This is needed after changing Xen code mappings.
> > + *
> > + * The caller needs to issue the necessary DSB and D-cache flushes
> > + * before calling flush_xen_text_tlb.
> >   */
> >  static inline void flush_xen_text_tlb(void)
> >  {
> >      register unsigned long r0 asm ("r0");
> >      asm volatile (
> > -        "dsb;"                        /* Ensure visibility of PTE writes */
> > +        "isb;"                        /* Ensure synchronization with 
> > previous changes to text */
> >          STORE_CP32(0, TLBIALLH)       /* Flush hypervisor TLB */
> > +        STORE_CP32(0, ICIALLU)        /* Flush I-cache */
> >          STORE_CP32(0, BPIALL)         /* Flush branch predictor */
> >          "dsb;"                        /* Ensure completion of TLB+BP flush 
> > */
> >          "isb;"
> 
> 

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