[PATCH] nestedhvm: ASID emulation (cleanup)

[PATCH] nestedhvm: ASID emulation (cleanup)

[Christoph Egger]

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


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 <Christoph.Egger@amd.com>

-- 
---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

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

diff -r d1677c0438fb -r 5cd1060fb63e xen/arch/x86/hvm/asid.c
--- a/xen/arch/x86/hvm/asid.c
+++ b/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;
 }
 
diff -r d1677c0438fb -r 5cd1060fb63e xen/include/asm-x86/hvm/svm/asid.h
--- a/xen/include/asm-x86/hvm/svm/asid.h
+++ b/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. */
diff -r d1677c0438fb -r 5cd1060fb63e xen/include/asm-x86/hvm/svm/svm.h
--- a/xen/include/asm-x86/hvm/svm/svm.h
+++ b/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);
 
diff -r d1677c0438fb -r 5cd1060fb63e xen/include/asm-x86/hvm/vcpu.h
--- a/xen/include/asm-x86/hvm/vcpu.h
+++ b/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;
 

[-- 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] nestedhvm: ASID emulation (cleanup)

[Keir Fraser]

On 13/04/2011 11:35, "Christoph Egger" <Christoph.Egger@amd.com> wrote:

> 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

> - Introduce svm_invlpga() used in ASID emulation patch
> 
> Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>

Re: [PATCH] nestedhvm: ASID emulation (cleanup)

[Christoph Egger]

On Wednesday 13 April 2011 15:06:43 Keir Fraser wrote:
> On 13/04/2011 11:35, "Christoph Egger" <Christoph.Egger@amd.com> wrote:
> > 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.

>
> I know it costs three extra characters per use, but it does make code
> easier to understand.
>
>  -- Keir
>
> > - Introduce svm_invlpga() used in ASID emulation patch
> >
> > Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel



-- 
---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

Re: [PATCH] nestedhvm: ASID emulation (cleanup)

[Keir Fraser]

On 13/04/2011 14:20, "Christoph Egger" <Christoph.Egger@amd.com> wrote:

> On Wednesday 13 April 2011 15:06:43 Keir Fraser wrote:
>> On 13/04/2011 11:35, "Christoph Egger" <Christoph.Egger@amd.com> wrote:
>>> 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.

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

>> 
>> I know it costs three extra characters per use, but it does make code
>> easier to understand.
>> 
>>  -- Keir
>> 
>>> - Introduce svm_invlpga() used in ASID emulation patch
>>> 
>>> Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>
>> 
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel
> 
> 

Re: [PATCH] nestedhvm: ASID emulation (cleanup)

[Christoph Egger]

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

On 04/13/11 15:20, Christoph Egger wrote:
> On Wednesday 13 April 2011 15:06:43 Keir Fraser wrote:
>> On 13/04/2011 11:35, "Christoph Egger"<Christoph.Egger@amd.com>  wrote:
>>> 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.
>
>>
>> I know it costs three extra characters per use, but it does make code
>> easier to understand.
>>
>>   -- Keir
>>
>>> - Introduce svm_invlpga() used in ASID emulation patch
>>>
>>> Signed-off-by: Christoph Egger<Christoph.Egger@amd.com>


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 <Christoph.Egger@amd.com>


-- 
---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

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

diff -r d1677c0438fb -r 5458a9862db2 xen/arch/x86/hvm/asid.c
--- a/xen/arch/x86/hvm/asid.c
+++ b/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;
diff -r d1677c0438fb -r 5458a9862db2 xen/include/asm-x86/hvm/svm/asid.h
--- a/xen/include/asm-x86/hvm/svm/asid.h
+++ b/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. */
diff -r d1677c0438fb -r 5458a9862db2 xen/include/asm-x86/hvm/svm/svm.h
--- a/xen/include/asm-x86/hvm/svm/svm.h
+++ b/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);
 
diff -r d1677c0438fb -r 5458a9862db2 xen/include/asm-x86/hvm/vcpu.h
--- a/xen/include/asm-x86/hvm/vcpu.h
+++ b/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;
 

[-- 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] nestedhvm: ASID emulation (cleanup)

[Christoph Egger]

On 04/13/11 15:28, Keir Fraser wrote:
> On 13/04/2011 14:20, "Christoph Egger"<Christoph.Egger@amd.com>  wrote:
>
>> On Wednesday 13 April 2011 15:06:43 Keir Fraser wrote:
>>> On 13/04/2011 11:35, "Christoph Egger"<Christoph.Egger@amd.com>  wrote:
>>>> 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.
>
> I already applied the two cleanups that I didn't mind, as two separate
> changesets.

Thanks.

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

Just sent my comments.

Christoph


-- 
---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