From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH 3/3] domctl: pause vCPU for context reads Date: Wed, 5 Feb 2014 15:29:19 +0000 Message-ID: <52F258CF.50706@citrix.com> References: <52F25D4202000078001196AB@nat28.tlf.novell.com> <52F25EB702000078001196EC@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============5996960502112969408==" Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1WB4Pk-0001nH-2O for xen-devel@lists.xenproject.org; Wed, 05 Feb 2014 15:29:24 +0000 In-Reply-To: <52F25EB702000078001196EC@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: George Dunlap , xen-devel , Keir Fraser List-Id: xen-devel@lists.xenproject.org --===============5996960502112969408== Content-Type: multipart/alternative; boundary="------------060902040206030101090001" --------------060902040206030101090001 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit On 05/02/14 14:54, Jan Beulich wrote: > "Base" context reads already paused the subject vCPU when being the > current one, but that special case isn't being properly dealt with > anyway (at the very least when x86's fsgsbase feature is in use), so > just disallow it. > > "Extended" context reads so far didn't do any pausing. > > While we can't avoid the reported data being stale by the time it > arrives at the caller, this way we at least guarantee that it is > consistent. > > Signed-off-by: Jan Beulich Now I come to think about it, is this an ABI change, as we are now disallowing a control domain to issue these hypercalls on itself? ~Andrew > > --- a/xen/arch/x86/domctl.c > +++ b/xen/arch/x86/domctl.c > @@ -819,7 +819,13 @@ long arch_do_domctl( > > if ( domctl->cmd == XEN_DOMCTL_get_ext_vcpucontext ) > { > + if ( v == current ) /* no vcpu_pause() */ > + break; > + > evc->size = sizeof(*evc); > + > + vcpu_pause(v); > + > if ( is_pv_domain(d) ) > { > evc->sysenter_callback_cs = > @@ -849,6 +855,7 @@ long arch_do_domctl( > evc->vmce.mci_ctl2_bank1 = v->arch.vmce.bank[1].mci_ctl2; > > ret = 0; > + vcpu_unpause(v); > copyback = 1; > } > else > --- a/xen/common/domctl.c > +++ b/xen/common/domctl.c > @@ -675,11 +675,9 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xe > struct vcpu *v; > > ret = -EINVAL; > - if ( op->u.vcpucontext.vcpu >= d->max_vcpus ) > - goto getvcpucontext_out; > - > - ret = -ESRCH; > - if ( (v = d->vcpu[op->u.vcpucontext.vcpu]) == NULL ) > + if ( op->u.vcpucontext.vcpu >= d->max_vcpus || > + (v = d->vcpu[op->u.vcpucontext.vcpu]) == NULL || > + v == current ) /* no vcpu_pause() */ > goto getvcpucontext_out; > > ret = -ENODATA; > @@ -694,14 +692,12 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xe > if ( (c.nat = xmalloc(struct vcpu_guest_context)) == NULL ) > goto getvcpucontext_out; > > - if ( v != current ) > - vcpu_pause(v); > + vcpu_pause(v); > > arch_get_info_guest(v, c); > ret = 0; > > - if ( v != current ) > - vcpu_unpause(v); > + vcpu_unpause(v); > > #ifdef CONFIG_COMPAT > if ( !is_pv_32on64_vcpu(v) ) > > > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel --------------060902040206030101090001 Content-Type: text/html; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit
On 05/02/14 14:54, Jan Beulich wrote:
"Base" context reads already paused the subject vCPU when being the
current one, but that special case isn't being properly dealt with
anyway (at the very least when x86's fsgsbase feature is in use), so
just disallow it.

"Extended" context reads so far didn't do any pausing.

While we can't avoid the reported data being stale by the time it
arrives at the caller, this way we at least guarantee that it is
consistent.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

Now I come to think about it, is this an ABI change, as we are now disallowing a control domain to issue these hypercalls on itself?

~Andrew


--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -819,7 +819,13 @@ long arch_do_domctl(
 
         if ( domctl->cmd == XEN_DOMCTL_get_ext_vcpucontext )
         {
+            if ( v == current ) /* no vcpu_pause() */
+                break;
+
             evc->size = sizeof(*evc);
+
+            vcpu_pause(v);
+
             if ( is_pv_domain(d) )
             {
                 evc->sysenter_callback_cs      =
@@ -849,6 +855,7 @@ long arch_do_domctl(
             evc->vmce.mci_ctl2_bank1 = v->arch.vmce.bank[1].mci_ctl2;
 
             ret = 0;
+            vcpu_unpause(v);
             copyback = 1;
         }
         else
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -675,11 +675,9 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xe
         struct vcpu         *v;
 
         ret = -EINVAL;
-        if ( op->u.vcpucontext.vcpu >= d->max_vcpus )
-            goto getvcpucontext_out;
-
-        ret = -ESRCH;
-        if ( (v = d->vcpu[op->u.vcpucontext.vcpu]) == NULL )
+        if ( op->u.vcpucontext.vcpu >= d->max_vcpus ||
+             (v = d->vcpu[op->u.vcpucontext.vcpu]) == NULL ||
+             v == current ) /* no vcpu_pause() */
             goto getvcpucontext_out;
 
         ret = -ENODATA;
@@ -694,14 +692,12 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xe
         if ( (c.nat = xmalloc(struct vcpu_guest_context)) == NULL )
             goto getvcpucontext_out;
 
-        if ( v != current )
-            vcpu_pause(v);
+        vcpu_pause(v);
 
         arch_get_info_guest(v, c);
         ret = 0;
 
-        if ( v != current )
-            vcpu_unpause(v);
+        vcpu_unpause(v);
 
 #ifdef CONFIG_COMPAT
         if ( !is_pv_32on64_vcpu(v) )






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

--------------060902040206030101090001-- --===============5996960502112969408== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============5996960502112969408==--