[PATCH for-4.5] tools/libxc: Avoid cacheflush toolstack hypercalls on x86

[PATCH for-4.5] tools/libxc: Avoid cacheflush toolstack hypercalls on x86

[Andrew Cooper]

XEN_DOMCTL_cacheflush hypercalls are (and will always be) -ENOSYS on x86, but
xc_domain_cacheflush() is called often during domain build and migrate for
correct behaviour on ARM.

Stub xc_domain_cacheflush() out on x86 to remove its pressure on the global
domctl lock, and the hypercall overhead (which applies further pressure to the
already heavily-contended TLB flush lock).

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>
CC: Tim Deegan <tim@xen.org>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxc/xc_domain.c |   16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
index 1a6f90a..a68127f 100644
--- a/tools/libxc/xc_domain.c
+++ b/tools/libxc/xc_domain.c
@@ -51,12 +51,28 @@ int xc_domain_create(xc_interface *xch,
 int xc_domain_cacheflush(xc_interface *xch, uint32_t domid,
                          xen_pfn_t start_pfn, xen_pfn_t nr_pfns)
 {
+#if defined (__i386__) || defined (__x86_64__)
+    /*
+     * The x86 architecture provides cache coherency guarantees which prevent
+     * the need for this hypercall.
+     *
+     * This call however is made frequently during domain build and migrate
+     * for the benefit of ARM, where cache flushes are needed.
+     *
+     * Stub it out here on x86 as domctls take the global domctl lock in Xen
+     * which is detrimental to concurrent toolstack operations on other
+     * domains.
+     */
+    errno = ENOSYS;
+    return -1;
+#else
     DECLARE_DOMCTL;
     domctl.cmd = XEN_DOMCTL_cacheflush;
     domctl.domain = (domid_t)domid;
     domctl.u.cacheflush.start_pfn = start_pfn;
     domctl.u.cacheflush.nr_pfns = nr_pfns;
     return do_domctl(xch, &domctl);
+#endif
 }
 
 int xc_domain_pause(xc_interface *xch,
-- 
1.7.10.4

Re: [PATCH for-4.5] tools/libxc: Avoid cacheflush toolstack hypercalls on x86

[Ian Campbell]

On Wed, 2014-09-24 at 14:47 +0100, Andrew Cooper wrote:
> XEN_DOMCTL_cacheflush hypercalls are (and will always be) -ENOSYS on x86, but
> xc_domain_cacheflush() is called often during domain build and migrate for
> correct behaviour on ARM.
> 
> Stub xc_domain_cacheflush() out on x86 to remove its pressure on the global
> domctl lock, and the hypercall overhead (which applies further pressure to the
> already heavily-contended TLB flush lock).
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Keir Fraser <keir@xen.org>
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Tim Deegan <tim@xen.org>
> CC: Ian Campbell <Ian.Campbell@citrix.com>
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Although IMHO only the first paragraph of the comment is useful, the
rest belongs in the commit log (where it already mostly is already).

> ---
>  tools/libxc/xc_domain.c |   16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
> index 1a6f90a..a68127f 100644
> --- a/tools/libxc/xc_domain.c
> +++ b/tools/libxc/xc_domain.c
> @@ -51,12 +51,28 @@ int xc_domain_create(xc_interface *xch,
>  int xc_domain_cacheflush(xc_interface *xch, uint32_t domid,
>                           xen_pfn_t start_pfn, xen_pfn_t nr_pfns)
>  {
> +#if defined (__i386__) || defined (__x86_64__)
> +    /*
> +     * The x86 architecture provides cache coherency guarantees which prevent
> +     * the need for this hypercall.
> +     *
> +     * This call however is made frequently during domain build and migrate
> +     * for the benefit of ARM, where cache flushes are needed.
> +     *
> +     * Stub it out here on x86 as domctls take the global domctl lock in Xen
> +     * which is detrimental to concurrent toolstack operations on other
> +     * domains.
> +     */
> +    errno = ENOSYS;
> +    return -1;
> +#else
>      DECLARE_DOMCTL;
>      domctl.cmd = XEN_DOMCTL_cacheflush;
>      domctl.domain = (domid_t)domid;
>      domctl.u.cacheflush.start_pfn = start_pfn;
>      domctl.u.cacheflush.nr_pfns = nr_pfns;
>      return do_domctl(xch, &domctl);
> +#endif
>  }
>  
>  int xc_domain_pause(xc_interface *xch,

Re: [PATCH for-4.5] tools/libxc: Avoid cacheflush toolstack hypercalls on x86

[Andrew Cooper]

On 24/09/14 15:22, Ian Campbell wrote:
> On Wed, 2014-09-24 at 14:47 +0100, Andrew Cooper wrote:
>> XEN_DOMCTL_cacheflush hypercalls are (and will always be) -ENOSYS on x86, but
>> xc_domain_cacheflush() is called often during domain build and migrate for
>> correct behaviour on ARM.
>>
>> Stub xc_domain_cacheflush() out on x86 to remove its pressure on the global
>> domctl lock, and the hypercall overhead (which applies further pressure to the
>> already heavily-contended TLB flush lock).
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> CC: Keir Fraser <keir@xen.org>
>> CC: Jan Beulich <JBeulich@suse.com>
>> CC: Tim Deegan <tim@xen.org>
>> CC: Ian Campbell <Ian.Campbell@citrix.com>
>> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
>> CC: Wei Liu <wei.liu2@citrix.com>
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
>
> Although IMHO only the first paragraph of the comment is useful, the
> rest belongs in the commit log (where it already mostly is already).

I am happy for that to be fixed while committing, or I could resubmit?

~Andrew

>
>> ---
>>  tools/libxc/xc_domain.c |   16 ++++++++++++++++
>>  1 file changed, 16 insertions(+)
>>
>> diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
>> index 1a6f90a..a68127f 100644
>> --- a/tools/libxc/xc_domain.c
>> +++ b/tools/libxc/xc_domain.c
>> @@ -51,12 +51,28 @@ int xc_domain_create(xc_interface *xch,
>>  int xc_domain_cacheflush(xc_interface *xch, uint32_t domid,
>>                           xen_pfn_t start_pfn, xen_pfn_t nr_pfns)
>>  {
>> +#if defined (__i386__) || defined (__x86_64__)
>> +    /*
>> +     * The x86 architecture provides cache coherency guarantees which prevent
>> +     * the need for this hypercall.
>> +     *
>> +     * This call however is made frequently during domain build and migrate
>> +     * for the benefit of ARM, where cache flushes are needed.
>> +     *
>> +     * Stub it out here on x86 as domctls take the global domctl lock in Xen
>> +     * which is detrimental to concurrent toolstack operations on other
>> +     * domains.
>> +     */
>> +    errno = ENOSYS;
>> +    return -1;
>> +#else
>>      DECLARE_DOMCTL;
>>      domctl.cmd = XEN_DOMCTL_cacheflush;
>>      domctl.domain = (domid_t)domid;
>>      domctl.u.cacheflush.start_pfn = start_pfn;
>>      domctl.u.cacheflush.nr_pfns = nr_pfns;
>>      return do_domctl(xch, &domctl);
>> +#endif
>>  }
>>  
>>  int xc_domain_pause(xc_interface *xch,
>

Re: [PATCH for-4.5] tools/libxc: Avoid cacheflush toolstack hypercalls on x86

[Ian Campbell]

On Wed, 2014-09-24 at 15:24 +0100, Andrew Cooper wrote:
> On 24/09/14 15:22, Ian Campbell wrote:
> > On Wed, 2014-09-24 at 14:47 +0100, Andrew Cooper wrote:
> >> XEN_DOMCTL_cacheflush hypercalls are (and will always be) -ENOSYS on x86, but
> >> xc_domain_cacheflush() is called often during domain build and migrate for
> >> correct behaviour on ARM.
> >>
> >> Stub xc_domain_cacheflush() out on x86 to remove its pressure on the global
> >> domctl lock, and the hypercall overhead (which applies further pressure to the
> >> already heavily-contended TLB flush lock).
> >>
> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> >> CC: Keir Fraser <keir@xen.org>
> >> CC: Jan Beulich <JBeulich@suse.com>
> >> CC: Tim Deegan <tim@xen.org>
> >> CC: Ian Campbell <Ian.Campbell@citrix.com>
> >> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> >> CC: Wei Liu <wei.liu2@citrix.com>
> > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> >
> > Although IMHO only the first paragraph of the comment is useful, the
> > rest belongs in the commit log (where it already mostly is already).
> 
> I am happy for that to be fixed while committing, or I could resubmit?

If you think it's just a case of nuking the other two paras then I'll do
it on commit, if you want to add some other words to the commit message
best to resubmit.

Ian.

> 
> ~Andrew
> 
> >
> >> ---
> >>  tools/libxc/xc_domain.c |   16 ++++++++++++++++
> >>  1 file changed, 16 insertions(+)
> >>
> >> diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
> >> index 1a6f90a..a68127f 100644
> >> --- a/tools/libxc/xc_domain.c
> >> +++ b/tools/libxc/xc_domain.c
> >> @@ -51,12 +51,28 @@ int xc_domain_create(xc_interface *xch,
> >>  int xc_domain_cacheflush(xc_interface *xch, uint32_t domid,
> >>                           xen_pfn_t start_pfn, xen_pfn_t nr_pfns)
> >>  {
> >> +#if defined (__i386__) || defined (__x86_64__)
> >> +    /*
> >> +     * The x86 architecture provides cache coherency guarantees which prevent
> >> +     * the need for this hypercall.
> >> +     *
> >> +     * This call however is made frequently during domain build and migrate
> >> +     * for the benefit of ARM, where cache flushes are needed.
> >> +     *
> >> +     * Stub it out here on x86 as domctls take the global domctl lock in Xen
> >> +     * which is detrimental to concurrent toolstack operations on other
> >> +     * domains.
> >> +     */
> >> +    errno = ENOSYS;
> >> +    return -1;
> >> +#else
> >>      DECLARE_DOMCTL;
> >>      domctl.cmd = XEN_DOMCTL_cacheflush;
> >>      domctl.domain = (domid_t)domid;
> >>      domctl.u.cacheflush.start_pfn = start_pfn;
> >>      domctl.u.cacheflush.nr_pfns = nr_pfns;
> >>      return do_domctl(xch, &domctl);
> >> +#endif
> >>  }
> >>  
> >>  int xc_domain_pause(xc_interface *xch,
> >
>