From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH 1/3] x86: streamline hypercall_create_continuation() Date: Thu, 8 Jan 2015 16:01:18 +0000 Message-ID: <54AEA9CE.9020606@citrix.com> References: <54AEAD2C0200007800052B56@mail.emea.novell.com> <54AEAEC10200007800052B79@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============5359637010477770784==" Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Y9FWY-0004cI-ER for xen-devel@lists.xenproject.org; Thu, 08 Jan 2015 16:01:27 +0000 In-Reply-To: <54AEAEC10200007800052B79@mail.emea.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 , xen-devel Cc: Keir Fraser List-Id: xen-devel@lists.xenproject.org --===============5359637010477770784== Content-Type: multipart/alternative; boundary="------------010304090607070400060408" --------------010304090607070400060408 Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: quoted-printable On 08/01/15 15:22, Jan Beulich wrote: > - drop clearing of excessive multicall arguments in compat case (no > longer needed now that hypercall_xlat_continuation() only checks the > actual arguments) > - latch current into a local variable > - use the cached value of hvm_guest_x86_mode() instead of re-executing > it > - scope restrict "regs" > - while at it, convert the remaining two argument checking BUG_ON()s in= > hypercall_xlat_continuation() to ASSERT()s > > Signed-off-by: Jan Beulich Reviewed-by: Andrew Cooper There appear to other many other places which could benifit from a caching of guest_x86_mode() (especially in the nested virt case). Is it worth considering unconditionally calculating on vmexit and removing the function? > > --- a/xen/arch/x86/domain.c > +++ b/xen/arch/x86/domain.c > @@ -1675,7 +1675,6 @@ unsigned long hypercall_create_continuat > unsigned int op, const char *format, ...) > { > struct mc_state *mcs =3D ¤t->mc_state; > - struct cpu_user_regs *regs; > const char *p =3D format; > unsigned long arg; > unsigned int i; > @@ -1689,26 +1688,23 @@ unsigned long hypercall_create_continuat > =20 > for ( i =3D 0; *p !=3D '\0'; i++ ) > mcs->call.args[i] =3D next_arg(p, args); > - if ( is_pv_32on64_domain(current->domain) ) > - { > - for ( ; i < 6; i++ ) > - mcs->call.args[i] =3D 0; > - } > } > else > { > - regs =3D guest_cpu_user_regs(); > - regs->eax =3D op; > + struct cpu_user_regs *regs =3D guest_cpu_user_regs(); > + struct vcpu *curr =3D current; > + > + regs->eax =3D op; > =20 > /* Ensure the hypercall trap instruction is re-executed. */ > - if ( is_pv_vcpu(current) ) > + if ( is_pv_vcpu(curr) ) > regs->eip -=3D 2; /* re-execute 'syscall' / 'int $xx' */ > else > - current->arch.hvm_vcpu.hcall_preempted =3D 1; > + curr->arch.hvm_vcpu.hcall_preempted =3D 1; > =20 > - if ( is_pv_vcpu(current) ? > - !is_pv_32on64_vcpu(current) : > - (hvm_guest_x86_mode(current) =3D=3D 8) ) > + if ( is_pv_vcpu(curr) ? > + !is_pv_32on64_vcpu(curr) : > + curr->arch.hvm_vcpu.hcall_64bit ) > { > for ( i =3D 0; *p !=3D '\0'; i++ ) > { > @@ -1759,9 +1755,8 @@ int hypercall_xlat_continuation(unsigned > =20 > ASSERT(nr <=3D ARRAY_SIZE(mcs->call.args)); > ASSERT(!(mask >> nr)); > - > - BUG_ON(id && *id >=3D nr); > - BUG_ON(id && (mask & (1U << *id))); > + ASSERT(!id || *id < nr); > + ASSERT(!id || !(mask & (1U << *id))); > =20 > va_start(args, mask); > =20 > > > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel --------------010304090607070400060408 Content-Type: text/html; charset="windows-1252" Content-Length: 4001 Content-Transfer-Encoding: quoted-printable
On 08/01/15 15:22, Jan Beulich wrote:
- drop clearing of excessive multicall arguments in compat case (no
  longer needed now that hypercall_xlat_continuation() only checks the
  actual arguments)
- latch current into a local variable
- use the cached value of hvm_guest_x86_mode() instead of re-executing
  it
- scope restrict "regs"
- while at it, convert the remaining two argument checking BUG_ON()s in
  hypercall_xlat_continuation() to ASSERT()s

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

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

There appear to other many other places which could benifit from a caching of guest_x86_mode() (especially in the nested virt case).=A0 Is it worth considering unconditionally calculating on vmexit and removing the function=3F


--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1675,7 +1675,6 @@ unsigned long hypercall_create_continuat
     unsigned int op, const char *format, ...)
 {
     struct mc_state *mcs =3D &current->mc_state;
-    struct cpu_user_regs *regs;
     const char *p =3D format;
     unsigned long arg;
     unsigned int i;
@@ -1689,26 +1688,23 @@ unsigned long hypercall_create_continuat
 
         for ( i =3D 0; *p !=3D '\0'; i++ )
             mcs->call.args[i] =3D next_arg(p, args);
-        if ( is_pv_32on64_domain(current->domain) )
-        {
-            for ( ; i < 6; i++ )
-                mcs->call.args[i] =3D 0;
-        }
     }
     else
     {
-        regs       =3D guest_cpu_user_regs();
-        regs->eax  =3D op;
+        struct cpu_user_regs *regs =3D guest_cpu_user_regs();
+        struct vcpu *curr =3D current;
+
+        regs->eax =3D op;
 
         /* Ensure the hypercall trap instruction is re-executed. */
-        if ( is_pv_vcpu(current) )
+        if ( is_pv_vcpu(curr) )
             regs->eip -=3D 2;  /* re-execute 'syscall' / 'int $xx' */
         else
-            current->arch.hvm_vcpu.hcall_preempted =3D 1;
+            curr->arch.hvm_vcpu.hcall_preempted =3D 1;
 
-        if ( is_pv_vcpu(current) =3F
-             !is_pv_32on64_vcpu(current) :
-             (hvm_guest_x86_mode(current) =3D=3D 8) )
+        if ( is_pv_vcpu(curr) =3F
+             !is_pv_32on64_vcpu(curr) :
+             curr->arch.hvm_vcpu.hcall_64bit )
         {
             for ( i =3D 0; *p !=3D '\0'; i++ )
             {
@@ -1759,9 +1755,8 @@ int hypercall_xlat_continuation(unsigned
 
     ASSERT(nr <=3D ARRAY_SIZE(mcs->call.args));
     ASSERT(!(mask >> nr));
-
-    BUG_ON(id && *id >=3D nr);
-    BUG_ON(id && (mask & (1U << *id)));
+    ASSERT(!id || *id < nr);
+    ASSERT(!id || !(mask & (1U << *id)));
 
     va_start(args, mask);
 






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

--------------010304090607070400060408-- --===============5359637010477770784== 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 --===============5359637010477770784==--