Skip to contentSkip to conversation

nestedhvm: ASID emulation (cleanup)

PATCH In review Christoph Egger 3 participants 6 messages Apr 13, 2011

lore
Share & tools
lore

Christoph Egger

author
#0 lore raw

PATCH 1/1 nestedhvm: ASID emulation (cleanup)

Cleanup for ASID emulation:
- Use C99 integer types for asid numbers
- asid.c: consistently use 'v' instead of 'curr'
- Introduce svm_invlpga() used in ASID emulation patch

Signed-off-by Christoph Egger <[email protected]>
Signature · 6 lines
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632
xen/arch/x86/hvm/asid.c +10 -10
diff -r d1677c0438fb -r 5cd1060fb63e xen/arch/x86/hvm/asid.c
@@ -48,9 +48,9 @@
 
 /* Per-CPU ASID management. */
 struct hvm_asid_data {
-   u64 core_asid_generation;
-   u32 next_asid;
-   u32 max_asid;
+   uint64_t core_asid_generation;
+   uint32_t next_asid;
+   uint32_t max_asid;
    bool_t disabled;
 };
 

@@ -58,7 +58,7 @@ static DEFINE_PER_CPU(struct hvm_asid_da
 
 void hvm_asid_init(int nasids)
 {
-    static s8 g_disabled = -1;
+    static int8_t g_disabled = -1;
     struct hvm_asid_data *data = &this_cpu(hvm_asid_data);
 
     data->max_asid = nasids - 1;

@@ -104,7 +104,7 @@ void hvm_asid_flush_core(void)
 
 bool_t hvm_asid_handle_vmenter(void)
 {
-    struct vcpu *curr = current;
+    struct vcpu *v = current;
     struct hvm_asid_data *data = &this_cpu(hvm_asid_data);
 
     /* On erratum #170 systems we must flush the TLB. 

@@ -113,7 +113,7 @@ bool_t hvm_asid_handle_vmenter(void)
         goto disabled;
 
     /* Test if VCPU has valid ASID. */
-    if ( curr->arch.hvm_vcpu.asid_generation == data->core_asid_generation )
+    if ( v->arch.hvm_vcpu.asid_generation == data->core_asid_generation )
         return 0;
 
     /* If there are no free ASIDs, need to go to a new generation */

@@ -126,17 +126,17 @@ bool_t hvm_asid_handle_vmenter(void)
     }
 
     /* Now guaranteed to be a free ASID. */
-    curr->arch.hvm_vcpu.asid = data->next_asid++;
-    curr->arch.hvm_vcpu.asid_generation = data->core_asid_generation;
+    v->arch.hvm_vcpu.asid = data->next_asid++;
+    v->arch.hvm_vcpu.asid_generation = data->core_asid_generation;
 
     /*
      * When we assign ASID 1, flush all TLB entries as we are starting a new
      * generation, and all old ASID allocations are now stale. 
      */
-    return (curr->arch.hvm_vcpu.asid == 1);
+    return (v->arch.hvm_vcpu.asid == 1);
 
  disabled:
-    curr->arch.hvm_vcpu.asid = 0;
+    v->arch.hvm_vcpu.asid = 0;
     return 0;
 }
xen/include/asm-x86/hvm/svm/asid.h +1 -4
diff -r d1677c0438fb -r 5cd1060fb63e xen/include/asm-x86/hvm/svm/asid.h
@@ -34,10 +34,7 @@ static inline void svm_asid_g_invlpg(str
 {
 #if 0
     /* Optimization? */
-    asm volatile (".byte 0x0F,0x01,0xDF    \n"
-                  : /* output */
-                  : /* input */
-                  "a" (g_vaddr), "c"(v->arch.hvm_svm.vmcb->guest_asid) );
+    svm_invlpga(g_vaddr, v->arch.hvm_svm.vmcb->guest_asid);
 #endif
 
     /* Safe fallback. Take a new ASID. */
xen/include/asm-x86/hvm/svm/svm.h +9 -0
diff -r d1677c0438fb -r 5cd1060fb63e xen/include/asm-x86/hvm/svm/svm.h
@@ -60,6 +60,15 @@ static inline void svm_vmsave(void *vmcb
         : : "a" (__pa(vmcb)) : "memory" );
 }
 
+static inline void svm_invlpga(unsigned long vaddr, uint32_t asid)
+{
+    asm volatile (
+        ".byte 0x0f,0x01,0xdf"
+        : /* output */
+        : /* input */
+        "a" (vaddr), "c" (asid));
+}
+
 unsigned long *svm_msrbit(unsigned long *msr_bitmap, uint32_t msr);
 void __update_guest_eip(struct cpu_user_regs *regs, unsigned int inst_len);
xen/include/asm-x86/hvm/vcpu.h +2 -2
diff -r d1677c0438fb -r 5cd1060fb63e xen/include/asm-x86/hvm/vcpu.h
@@ -100,8 +100,8 @@ struct hvm_vcpu {
     bool_t              hcall_preempted;
     bool_t              hcall_64bit;
 
-    u64                 asid_generation;
-    u32                 asid;
+    uint64_t            asid_generation;
+    uint32_t            asid;
 
     u32                 msr_tsc_aux;

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#1 lore raw
Quoted 4 lines · Christoph Egger

Cleanup for ASID emulation:
- Use C99 integer types for asid numbers

- asid.c: consistently use 'v' instead of 'curr'

On what planet is that an improvement??

We use 'curr' as a convenient idiom to represent cached current. Whereas 'v' is supposed to represent an arbitrary vcpu.

I know it costs three extra characters per use, but it does make code easier to understand.

-- Keir

Quoted 3 lines

- Introduce svm_invlpga() used in ASID emulation patch

Signed-off-by Christoph Egger <[email protected]>
author
#2 lore raw
Quoted 10 lines · On Wednesday 13 April 2011 15:06:43 Keir Fraser wrote:
Quoted 4 lines · Christoph Egger

Cleanup for ASID emulation:
- Use C99 integer types for asid numbers

- asid.c: consistently use 'v' instead of 'curr'

On what planet is that an improvement??

We use 'curr' as a convenient idiom to represent cached current. Whereas 'v' is supposed to represent an arbitrary vcpu.

Oh, that's the difference. I thought 'curr' is just another random name for an arbitrary vcpu.

I will resend the patch w/o that hunk.

Quoted 14 lines

I know it costs three extra characters per use, but it does make code easier to understand.

-- Keir

Quoted 3 lines

- Introduce svm_invlpga() used in ASID emulation patch

Signed-off-by Christoph Egger <[email protected]>

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Signature · 6 lines
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632
#3 lore raw
Quoted 9 lines · Christoph Egger
Quoted 8 lines · On Wednesday 13 April 2011 15:06:43 Keir Fraser wrote:
Quoted 4 lines · Christoph Egger

Cleanup for ASID emulation:
- Use C99 integer types for asid numbers

- asid.c: consistently use 'v' instead of 'curr'

On what planet is that an improvement??

Quoted 7 lines
Quoted 2 lines

We use 'curr' as a convenient idiom to represent cached current. Whereas 'v' is supposed to represent an arbitrary vcpu.

Oh, that's the difference. I thought 'curr' is just another random name for an arbitrary vcpu.

I will resend the patch w/o that hunk.

I already applied the two cleanups that I didn't mind, as two separate changesets. And I've nacked your main patch so you can go take a look at that. :-)

-- Keir

Quoted 16 lines
Quoted 14 lines

I know it costs three extra characters per use, but it does make code easier to understand.

-- Keir

Quoted 3 lines

- Introduce svm_invlpga() used in ASID emulation patch

Signed-off-by Christoph Egger <[email protected]>

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

author
#5 lore raw
In reply to Christoph Egger
Quoted 9 lines · Christoph Egger
Quoted 8 lines · On Wednesday 13 April 2011 15:06:43 Keir Fraser wrote:
Quoted 4 lines · Christoph Egger

Cleanup for ASID emulation:
- Use C99 integer types for asid numbers

- asid.c: consistently use 'v' instead of 'curr'

On what planet is that an improvement??

Quoted 17 lines
Quoted 2 lines

We use 'curr' as a convenient idiom to represent cached current. Whereas 'v' is supposed to represent an arbitrary vcpu.

Oh, that's the difference. I thought 'curr' is just another random name for an arbitrary vcpu.

I will resend the patch w/o that hunk.

Quoted 9 lines

I know it costs three extra characters per use, but it does make code easier to understand.

-- Keir

Quoted 3 lines

- Introduce svm_invlpga() used in ASID emulation patch

Signed-off-by Christoph Egger<[email protected]>

Here we go.

Cleanup for ASID emulation:
- Use C99 integer types for asid numbers
- Introduce svm_invlpga() used in ASID emulation patch

Signed-off-by Christoph Egger <[email protected]>
Signature · 6 lines
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632
xen/arch/x86/hvm/asid.c +4 -4
diff -r d1677c0438fb -r 5458a9862db2 xen/arch/x86/hvm/asid.c
@@ -48,9 +48,9 @@
 
 /* Per-CPU ASID management. */
 struct hvm_asid_data {
-   u64 core_asid_generation;
-   u32 next_asid;
-   u32 max_asid;
+   uint64_t core_asid_generation;
+   uint32_t next_asid;
+   uint32_t max_asid;
    bool_t disabled;
 };
 

@@ -58,7 +58,7 @@ static DEFINE_PER_CPU(struct hvm_asid_da
 
 void hvm_asid_init(int nasids)
 {
-    static s8 g_disabled = -1;
+    static int8_t g_disabled = -1;
     struct hvm_asid_data *data = &this_cpu(hvm_asid_data);
 
     data->max_asid = nasids - 1;
xen/include/asm-x86/hvm/svm/asid.h +1 -4
diff -r d1677c0438fb -r 5458a9862db2 xen/include/asm-x86/hvm/svm/asid.h
@@ -34,10 +34,7 @@ static inline void svm_asid_g_invlpg(str
 {
 #if 0
     /* Optimization? */
-    asm volatile (".byte 0x0F,0x01,0xDF    \n"
-                  : /* output */
-                  : /* input */
-                  "a" (g_vaddr), "c"(v->arch.hvm_svm.vmcb->guest_asid) );
+    svm_invlpga(g_vaddr, v->arch.hvm_svm.vmcb->guest_asid);
 #endif
 
     /* Safe fallback. Take a new ASID. */
xen/include/asm-x86/hvm/svm/svm.h +9 -0
diff -r d1677c0438fb -r 5458a9862db2 xen/include/asm-x86/hvm/svm/svm.h
@@ -60,6 +60,15 @@ static inline void svm_vmsave(void *vmcb
         : : "a" (__pa(vmcb)) : "memory" );
 }
 
+static inline void svm_invlpga(unsigned long vaddr, uint32_t asid)
+{
+    asm volatile (
+        ".byte 0x0f,0x01,0xdf"
+        : /* output */
+        : /* input */
+        "a" (vaddr), "c" (asid));
+}
+
 unsigned long *svm_msrbit(unsigned long *msr_bitmap, uint32_t msr);
 void __update_guest_eip(struct cpu_user_regs *regs, unsigned int inst_len);
xen/include/asm-x86/hvm/vcpu.h +2 -2
diff -r d1677c0438fb -r 5458a9862db2 xen/include/asm-x86/hvm/vcpu.h
@@ -100,8 +100,8 @@ struct hvm_vcpu {
     bool_t              hcall_preempted;
     bool_t              hcall_64bit;
 
-    u64                 asid_generation;
-    u32                 asid;
+    uint64_t            asid_generation;
+    uint32_t            asid;
 
     u32                 msr_tsc_aux;

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

author
#4 lore raw
In reply to Keir Fraser
Quoted 14 lines · Keir Fraser
Quoted 12 lines · Christoph Egger
Quoted 10 lines · On Wednesday 13 April 2011 15:06:43 Keir Fraser wrote:
Quoted 4 lines · Christoph Egger

Cleanup for ASID emulation:
- Use C99 integer types for asid numbers

- asid.c: consistently use 'v' instead of 'curr'

On what planet is that an improvement??

We use 'curr' as a convenient idiom to represent cached current. Whereas 'v' is supposed to represent an arbitrary vcpu.

Quoted 7 lines
Quoted 4 lines

Oh, that's the difference. I thought 'curr' is just another random name for an arbitrary vcpu.

I will resend the patch w/o that hunk.

I already applied the two cleanups that I didn't mind, as two separate changesets.

Thanks.

Quoted 2 lines

And I've nacked your main patch so you can go take a look at that. :-)

Just sent my comments.

Christoph

Signature · 6 lines
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632