Re: [Xen-devel] [PATCH v4 4/7] xen/x86: introduce more cache maintenance operations

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Fri, 10 Oct 2014, Jan Beulich wrote:
> >>> On 10.10.14 at 13:43, <stefano.stabellini@xxxxxxxxxxxxx> wrote:
> > +void clean_and_invalidate_dcache_va_range(const void *p, unsigned long 
> > size)
> > +{
> > +    int order = get_order_from_bytes(size);
> 
> unsigned int if the variable is really needed.
> 
> > +    flush_area_local(p, FLUSH_CACHE|FLUSH_ORDER(order));
> > +}
> 
> Overall I wonder whether not making this an inline function is really
> appropriate.

Introducing this function as static inline in page.h would cause the
number of included files to grow a bit too much I think.


> Oh, and then I may have slightly misguided you: The caller you're
> adding for this wants to be able to clean/invalidate sub-page chunks,
> which this interface doesn't allow. This isn't a problem unless the
> operation gets done frequently and always degenerates to wbinvd().
> At the very least I'd suggest adding a comment saying that sub-page
> granularity support would need to be added if necessary here.

Good point.


> > --- a/xen/include/asm-x86/page.h
> > +++ b/xen/include/asm-x86/page.h
> > @@ -344,8 +344,13 @@ static inline uint32_t cacheattr_to_pte_flags(uint32_t 
> > cacheattr)
> >      return ((cacheattr & 4) << 5) | ((cacheattr & 3) << 3);
> >  }
> >  
> > -/* No cache maintenance required on x86 architecture. */
> >  static inline void flush_page_to_ram(unsigned long mfn) {}
> > +static inline void invalidate_dcache_va_range(const void *p, unsigned long 
> > size) {}
> 
> This should fail, not appear to succeed. We just can't do what is being
> requested.

You are right, however this will cause all the arm cache flushing
functions to change prototype too.

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