[PATCH] xen: cache current vcpu

[PATCH] xen: cache current vcpu

[Christoph Egger]

[-- Attachment #1: Type: text/plain, Size: 533 bytes --]


Hi!

Attached patch caches current vcpu.

Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>


-- 
AMD Saxony, Dresden, Germany
Operating System Research Center

Legal Information:
AMD Saxony Limited Liability Company & Co. KG
Sitz (Geschäftsanschrift):
   Wilschdorfer Landstr. 101, 01109 Dresden, Deutschland
Registergericht Dresden: HRA 4896
vertretungsberechtigter Komplementär:
   AMD Saxony LLC (Sitz Wilmington, Delaware, USA)
Geschäftsführer der AMD Saxony LLC:
   Dr. Hans-R. Deppe, Thomas McCoy

[-- Attachment #2: xen_v.diff --]
[-- Type: text/plain, Size: 6111 bytes --]

diff -r 3bb94bb35dad xen/arch/x86/traps.c
--- a/xen/arch/x86/traps.c	Mon Oct 29 12:56:27 2007 +0000
+++ b/xen/arch/x86/traps.c	Mon Oct 29 17:45:01 2007 +0000
@@ -128,12 +128,13 @@ static void show_guest_stack(struct cpu_
 static void show_guest_stack(struct cpu_user_regs *regs)
 {
     int i;
+    struct vcpu *v = current;
     unsigned long *stack, addr;
 
-    if ( is_hvm_vcpu(current) )
+    if ( is_hvm_vcpu(v) )
         return;
 
-    if ( is_pv_32on64_vcpu(current) )
+    if ( is_pv_32on64_vcpu(v) )
     {
         compat_show_guest_stack(regs, debug_stack_lines);
         return;
@@ -800,12 +801,13 @@ static int handle_gdt_ldt_mapping_fault(
 static int handle_gdt_ldt_mapping_fault(
     unsigned long offset, struct cpu_user_regs *regs)
 {
+    struct vcpu *v = current;
     /* Which vcpu's area did we fault in, and is it in the ldt sub-area? */
     unsigned int is_ldt_area = (offset >> (GDT_LDT_VCPU_VA_SHIFT-1)) & 1;
     unsigned int vcpu_area   = (offset >> GDT_LDT_VCPU_VA_SHIFT);
 
     /* Should never fault in another vcpu's area. */
-    BUG_ON(vcpu_area != current->vcpu_id);
+    BUG_ON(vcpu_area != v->vcpu_id);
 
     /* Byte offset within the gdt/ldt sub-area. */
     offset &= (1UL << (GDT_LDT_VCPU_VA_SHIFT-1)) - 1UL;
@@ -826,7 +828,7 @@ static int handle_gdt_ldt_mapping_fault(
                 return 0;
             /* In guest mode? Propagate #PF to guest, with adjusted %cr2. */
             propagate_page_fault(
-                current->arch.guest_context.ldt_base + offset,
+                v->arch.guest_context.ldt_base + offset,
                 regs->error_code);
         }
     }
@@ -851,6 +853,7 @@ static int __spurious_page_fault(
 static int __spurious_page_fault(
     unsigned long addr, struct cpu_user_regs *regs)
 {
+    struct vcpu *v = current;
     unsigned long mfn, cr3 = read_cr3();
 #if CONFIG_PAGING_LEVELS >= 4
     l4_pgentry_t l4e, *l4t;
@@ -930,7 +933,7 @@ static int __spurious_page_fault(
  spurious:
     dprintk(XENLOG_WARNING, "Spurious fault in domain %u:%u "
             "at addr %lx, e/c %04x\n",
-            current->domain->domain_id, current->vcpu_id,
+            v->domain->domain_id, v->vcpu_id,
             addr, regs->error_code);
 #if CONFIG_PAGING_LEVELS >= 4
     dprintk(XENLOG_WARNING, " l4e = %"PRIpte"\n", l4e_get_intpte(l4e));
@@ -2470,14 +2473,16 @@ void unset_nmi_callback(void)
 
 asmlinkage int do_device_not_available(struct cpu_user_regs *regs)
 {
+    struct vcpu *v = current;
+
     BUG_ON(!guest_mode(regs));
 
-    setup_fpu(current);
-
-    if ( current->arch.guest_context.ctrlreg[0] & X86_CR0_TS )
+    setup_fpu(v);
+
+    if ( v->arch.guest_context.ctrlreg[0] & X86_CR0_TS )
     {
         do_guest_trap(TRAP_no_device, regs, 0);
-        current->arch.guest_context.ctrlreg[0] &= ~X86_CR0_TS;
+        v->arch.guest_context.ctrlreg[0] &= ~X86_CR0_TS;
     }
     else
         TRACE_0D(TRC_PV_MATH_STATE_RESTORE);
@@ -2658,7 +2663,7 @@ long register_guest_nmi_callback(unsigne
 long register_guest_nmi_callback(unsigned long address)
 {
     struct vcpu *v = current;
-    struct domain *d = current->domain;
+    struct domain *d = v->domain;
     struct trap_info *t = &v->arch.guest_context.trap_ctxt[TRAP_nmi];
 
     t->vector  = TRAP_nmi;
@@ -2690,14 +2695,15 @@ long do_set_trap_table(XEN_GUEST_HANDLE(
 long do_set_trap_table(XEN_GUEST_HANDLE(trap_info_t) traps)
 {
     struct trap_info cur;
-    struct trap_info *dst = current->arch.guest_context.trap_ctxt;
+    struct vcpu *v = current;
+    struct trap_info *dst = v->arch.guest_context.trap_ctxt;
     long rc = 0;
 
     /* If no table is presented then clear the entire virtual IDT. */
     if ( guest_handle_is_null(traps) )
     {
         memset(dst, 0, 256 * sizeof(*dst));
-        init_int80_direct_trap(current);
+        init_int80_direct_trap(v);
         return 0;
     }
 
@@ -2719,12 +2725,12 @@ long do_set_trap_table(XEN_GUEST_HANDLE(
         if ( cur.address == 0 )
             break;
 
-        fixup_guest_code_selector(current->domain, cur.cs);
+        fixup_guest_code_selector(v->domain, cur.cs);
 
         memcpy(&dst[cur.vector], &cur, sizeof(cur));
 
         if ( cur.vector == 0x80 )
-            init_int80_direct_trap(current);
+            init_int80_direct_trap(v);
 
         guest_handle_add_offset(traps, 1);
     }
@@ -2736,31 +2742,32 @@ long set_debugreg(struct vcpu *p, int re
 long set_debugreg(struct vcpu *p, int reg, unsigned long value)
 {
     int i;
+    struct vcpu *v = current;
 
     switch ( reg )
     {
     case 0: 
         if ( !access_ok(value, sizeof(long)) )
             return -EPERM;
-        if ( p == current ) 
+        if ( p == v ) 
             asm volatile ( "mov %0, %%db0" : : "r" (value) );
         break;
     case 1: 
         if ( !access_ok(value, sizeof(long)) )
             return -EPERM;
-        if ( p == current ) 
+        if ( p == v ) 
             asm volatile ( "mov %0, %%db1" : : "r" (value) );
         break;
     case 2: 
         if ( !access_ok(value, sizeof(long)) )
             return -EPERM;
-        if ( p == current ) 
+        if ( p == v ) 
             asm volatile ( "mov %0, %%db2" : : "r" (value) );
         break;
     case 3:
         if ( !access_ok(value, sizeof(long)) )
             return -EPERM;
-        if ( p == current ) 
+        if ( p == v ) 
             asm volatile ( "mov %0, %%db3" : : "r" (value) );
         break;
     case 6:
@@ -2770,7 +2777,7 @@ long set_debugreg(struct vcpu *p, int re
          */
         value &= 0xffffefff; /* reserved bits => 0 */
         value |= 0xffff0ff0; /* reserved bits => 1 */
-        if ( p == current ) 
+        if ( p == v ) 
             asm volatile ( "mov %0, %%db6" : : "r" (value) );
         break;
     case 7:
@@ -2791,7 +2798,7 @@ long set_debugreg(struct vcpu *p, int re
             for ( i = 0; i < 16; i += 2 )
                 if ( ((value >> (i+16)) & 3) == 2 ) return -EPERM;
         }
-        if ( p == current ) 
+        if ( p == v ) 
             asm volatile ( "mov %0, %%db7" : : "r" (value) );
         break;
     default:

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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

Re: [PATCH] xen: cache current vcpu

[Keir Fraser]

Gcc is quite capable of eliminating common subexpressions involving
current(), and I don't think this patch aids code readability (especially
the set_debugreg() change -- p==current means more to me than p==v).

 -- Keir

On 29/10/07 15:43, "Christoph Egger" <Christoph.Egger@amd.com> wrote:

> 
> Hi!
> 
> Attached patch caches current vcpu.
> 
> Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>
> 

Re: [PATCH] xen: cache current vcpu

[Christoph Egger]

I thought, the changes to register_guest_nmi_callback() and 
do_set_trap_table() make some sense. The rest is just for consistency.

Take what you like, leave the rest :)

Christoph



On Monday 29 October 2007 18:23:09 Keir Fraser wrote:
> Gcc is quite capable of eliminating common subexpressions involving
> current(), and I don't think this patch aids code readability (especially
> the set_debugreg() change -- p==current means more to me than p==v).
>
>  -- Keir
>
> On 29/10/07 15:43, "Christoph Egger" <Christoph.Egger@amd.com> wrote:
> > Hi!
> >
> > Attached patch caches current vcpu.
> >
> > Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>



-- 
AMD Saxony, Dresden, Germany
Operating System Research Center

Legal Information:
AMD Saxony Limited Liability Company & Co. KG
Sitz (Geschäftsanschrift):
   Wilschdorfer Landstr. 101, 01109 Dresden, Deutschland
Registergericht Dresden: HRA 4896
vertretungsberechtigter Komplementär:
   AMD Saxony LLC (Sitz Wilmington, Delaware, USA)
Geschäftsführer der AMD Saxony LLC:
   Dr. Hans-R. Deppe, Thomas McCoy

Re: [PATCH] xen: cache current vcpu

[Jan Beulich]

>>> Keir Fraser <Keir.Fraser@cl.cam.ac.uk> 29.10.07 18:23 >>>
>Gcc is quite capable of eliminating common subexpressions involving
>current(), ...

I have to partly disagree here: While gcc is capable of doing this, it cannot
do anything about uses of current that have function calls in between, and
I also don't think making get_current() an __attribute__((__pure__)) inline
function would be correct.

I do agree that in certain places using current is more readable, albeit I
think that there are quite a few places where it is already non-obvious that
'v' denotes 'current' (perhaps naming might just help here - if 'v' only ever
gets set from 'current' in a function, renaming it to 'cur' might be reasonable
to consider).

Jan