From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH v2 3/4] xen/arm: clean and invalidate all guest caches by VMID after domain build. Date: Tue, 4 Feb 2014 15:43:00 +0000 Message-ID: <1391528580.6497.36.camel@kazak.uk.xensource.com> References: <1391523701.5635.6.camel@kazak.uk.xensource.com> <1391523745-21139-3-git-send-email-ian.campbell@citrix.com> <52F10E8202000078001190A7@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <52F10E8202000078001190A7@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: keir@xen.org, stefano.stabellini@eu.citrix.com, tim@xen.org, julien.grall@linaro.org, ian.jackson@eu.citrix.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On Tue, 2014-02-04 at 15:00 +0000, Jan Beulich wrote: > >>> On 04.02.14 at 15:22, Ian Campbell 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.