Re: [Xen-devel] [PATCH v2 3/4] xen/arm: clean and invalidate all guest caches by VMID after domain build.

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2014-02-04 at 15:00 +0000, Jan Beulich wrote:
> >>> On 04.02.14 at 15:22, Ian Campbell <ian.campbell@xxxxxxxxxx> wrote:
> > --- a/tools/libxc/xc_dom_boot.c
> > +++ b/tools/libxc/xc_dom_boot.c
> > @@ -351,6 +351,10 @@ int xc_dom_gnttab_seed(xc_interface *xch, domid_t 
> > domid,
> >          return -1;
> >      }
> >  
> > +    /* Guest shouldn't really touch its grant table until it has
> > +     * enabled its caches. But lets be nice. */
> > +    xc_domain_cacheflush(xch, domid, gnttab_gmfn, gnttab_gmfn + 1);
> 
> Looking at this and further similar code I think it would be cleaner
> for the xc interface to take a start MFN and a count, and for the
> hypervisor interface to use an inclusive range (such that overflow
> is not a problem).

I think you might be right, I'll give that a go.

> 
> > --- a/xen/include/asm-x86/page.h
> > +++ b/xen/include/asm-x86/page.h
> > @@ -346,6 +346,9 @@ 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 cacheflush_page(unsigned long mfn) {}
> 
> The function name is certainly sub-optimal:

Yes, I should have said that I wasn't really happy with it in the
context of x86.

>  If I needed a page-range
> cache flush and found a function named like this, I'd assume it does
> what its name says - flush the page from the cache. sync_page() or
> some such may be better suited to express that this is something
> that may be a no-op on certain architectures.

sync_page is a much better suggestion. Perhaps even
sync_page_to/from_cache, I suppose that might actually be more
confusing.

Ian.


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