All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Tidy up the viridian code a little and flesh out the APIC assist MSR
@ 2011-09-15  5:13 Paul Durrant
  2011-09-15  6:38 ` Keir Fraser
  0 siblings, 1 reply; 5+ messages in thread
From: Paul Durrant @ 2011-09-15  5:13 UTC (permalink / raw)
  To: xen-devel

# HG changeset patch
# User Paul Durrant <paul.durrant@citrix.com>
# Date 1316063461 -3600
# Node ID a08073c5cf28ab76893102cc7a8d20bf3e5e15ae
# Parent  e90438f6e6d1585a71b18784a99c162b5d95f390
Tidy up the viridian code a little and flesh out the APIC assist MSR
handling code.

We don't say we that handle that MSR but Windows assumes it. In Windows 7 it
just wrote to the MSR and we used to handle that ok. Windows 8 also reads
from the MSR so we need to keep a record of the contents.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>

diff -r e90438f6e6d1 -r a08073c5cf28 xen/arch/x86/hvm/viridian.c
--- a/xen/arch/x86/hvm/viridian.c	Wed Sep 14 11:38:13 2011 +0100
+++ b/xen/arch/x86/hvm/viridian.c	Thu Sep 15 06:11:01 2011 +0100
@@ -96,9 +96,43 @@ int cpuid_viridian_leaves(unsigned int l
     return 1;
 }
 
-static void enable_hypercall_page(void)
+void dump_guest_os_id(struct domain *d)
 {
-    struct domain *d = current->domain;
+    gdprintk(XENLOG_INFO, "GUEST_OS_ID:\n");
+    gdprintk(XENLOG_INFO, "\tvendor: %x\n",
+            d->arch.hvm_domain.viridian.guest_os_id.fields.vendor);
+    gdprintk(XENLOG_INFO, "\tos: %x\n",
+            d->arch.hvm_domain.viridian.guest_os_id.fields.os);
+    gdprintk(XENLOG_INFO, "\tmajor: %x\n",
+            d->arch.hvm_domain.viridian.guest_os_id.fields.major);
+    gdprintk(XENLOG_INFO, "\tminor: %x\n",
+            d->arch.hvm_domain.viridian.guest_os_id.fields.minor);
+    gdprintk(XENLOG_INFO, "\tsp: %x\n",
+            d->arch.hvm_domain.viridian.guest_os_id.fields.service_pack);
+    gdprintk(XENLOG_INFO, "\tbuild: %x\n",
+            d->arch.hvm_domain.viridian.guest_os_id.fields.build_number);
+}
+
+void dump_hypercall(struct domain *d)
+{
+    gdprintk(XENLOG_INFO, "HYPERCALL:\n");
+    gdprintk(XENLOG_INFO, "\tenabled: %x\n",
+            d->arch.hvm_domain.viridian.hypercall_gpa.fields.enabled);
+    gdprintk(XENLOG_INFO, "\tpfn: %lx\n",
+            (unsigned long)d->arch.hvm_domain.viridian.hypercall_gpa.fields.pfn);
+}
+
+void dump_apic_assist(struct vcpu *v)
+{
+    gdprintk(XENLOG_INFO, "APIC_ASSIST[%d]:\n", v->vcpu_id);
+    gdprintk(XENLOG_INFO, "\tenabled: %x\n",
+            v->arch.hvm_vcpu.viridian.apic_assist.fields.enabled);
+    gdprintk(XENLOG_INFO, "\tpfn: %lx\n",
+            (unsigned long)v->arch.hvm_vcpu.viridian.apic_assist.fields.pfn);
+}
+
+static void enable_hypercall_page(struct domain *d)
+{
     unsigned long gmfn = d->arch.hvm_domain.viridian.hypercall_gpa.fields.pfn;
     unsigned long mfn = gmfn_to_mfn(d, gmfn);
     uint8_t *p;
@@ -130,9 +164,43 @@ static void enable_hypercall_page(void)
     put_page_and_type(mfn_to_page(mfn));
 }
 
+void initialize_apic_assist(struct vcpu *v)
+{
+    struct domain *d = v->domain;
+    unsigned long gmfn = v->arch.hvm_vcpu.viridian.apic_assist.fields.pfn;
+    unsigned long mfn = gmfn_to_mfn(d, gmfn);
+    uint8_t *p;
+
+    /*
+     * We don't support the APIC assist page, and that fact is reflected in
+     * our CPUID flags. However, Newer versions of Windows have a bug which
+     * means that they don't recognise that, and tries to use the page
+     * anyway. We therefore have to fake up just enough to keep windows happy.
+     *
+     * See http://msdn.microsoft.com/en-us/library/ff538657%28VS.85%29.aspx for
+     * details of how Windows uses the page.
+     */
+
+    if ( !mfn_valid(mfn) ||
+         !get_page_and_type(mfn_to_page(mfn), d, PGT_writable_page) )
+    {
+        gdprintk(XENLOG_WARNING, "Bad GMFN %lx (MFN %lx)\n", gmfn, mfn);
+        return;
+    }
+
+    p = map_domain_page(mfn);
+
+    *(u32 *)p = 0;
+
+    unmap_domain_page(p);
+
+    put_page_and_type(mfn_to_page(mfn));
+}
+
 int wrmsr_viridian_regs(uint32_t idx, uint64_t val)
 {
-    struct domain *d = current->domain;
+    struct vcpu *v = current;
+    struct domain *d = v->domain;
 
     if ( !is_viridian_domain(d) )
         return 0;
@@ -142,44 +210,29 @@ int wrmsr_viridian_regs(uint32_t idx, ui
     case VIRIDIAN_MSR_GUEST_OS_ID:
         perfc_incr(mshv_wrmsr_osid);
         d->arch.hvm_domain.viridian.guest_os_id.raw = val;
-        gdprintk(XENLOG_INFO, "Guest os:\n");
-        gdprintk(XENLOG_INFO, "\tvendor: %x\n",
-               d->arch.hvm_domain.viridian.guest_os_id.fields.vendor);
-        gdprintk(XENLOG_INFO, "\tos: %x\n",
-               d->arch.hvm_domain.viridian.guest_os_id.fields.os);
-        gdprintk(XENLOG_INFO, "\tmajor: %x\n",
-               d->arch.hvm_domain.viridian.guest_os_id.fields.major);
-        gdprintk(XENLOG_INFO, "\tminor: %x\n",
-               d->arch.hvm_domain.viridian.guest_os_id.fields.minor);
-        gdprintk(XENLOG_INFO, "\tsp: %x\n",
-               d->arch.hvm_domain.viridian.guest_os_id.fields.service_pack);
-        gdprintk(XENLOG_INFO, "\tbuild: %x\n",
-               d->arch.hvm_domain.viridian.guest_os_id.fields.build_number);
+        dump_guest_os_id(d);
         break;
 
     case VIRIDIAN_MSR_HYPERCALL:
         perfc_incr(mshv_wrmsr_hc_page);
-        gdprintk(XENLOG_INFO, "Set hypercall page %"PRIx64".\n", val);
-        if ( d->arch.hvm_domain.viridian.guest_os_id.raw == 0 )
-            break;
         d->arch.hvm_domain.viridian.hypercall_gpa.raw = val;
+        dump_hypercall(d);
         if ( d->arch.hvm_domain.viridian.hypercall_gpa.fields.enabled )
-            enable_hypercall_page();
+            enable_hypercall_page(d);
         break;
 
     case VIRIDIAN_MSR_VP_INDEX:
         perfc_incr(mshv_wrmsr_vp_index);
-        gdprintk(XENLOG_INFO, "Set VP index %"PRIu64".\n", val);
         break;
 
     case VIRIDIAN_MSR_EOI:
         perfc_incr(mshv_wrmsr_eoi);
-        vlapic_EOI_set(vcpu_vlapic(current));
+        vlapic_EOI_set(vcpu_vlapic(v));
         break;
 
     case VIRIDIAN_MSR_ICR: {
         u32 eax = (u32)val, edx = (u32)(val >> 32);
-        struct vlapic *vlapic = vcpu_vlapic(current);
+        struct vlapic *vlapic = vcpu_vlapic(v);
         perfc_incr(mshv_wrmsr_icr);
         eax &= ~(1 << 12);
         edx &= 0xff000000;
@@ -191,31 +244,15 @@ int wrmsr_viridian_regs(uint32_t idx, ui
 
     case VIRIDIAN_MSR_TPR:
         perfc_incr(mshv_wrmsr_tpr);
-        vlapic_set_reg(vcpu_vlapic(current), APIC_TASKPRI, (uint8_t)val);
+        vlapic_set_reg(vcpu_vlapic(v), APIC_TASKPRI, (uint8_t)val);
         break;
 
     case VIRIDIAN_MSR_APIC_ASSIST:
-        /*
-         * We don't support the APIC assist page, and that fact is reflected in
-         * our CPUID flags. However, Windows 7 build 7000 has a bug which means
-         * that it doesn't recognise that, and tries to use the page anyway. We
-         * therefore have to fake up just enough to keep win7 happy.
-         * Fortunately, that's really easy: just setting the first four bytes
-         * in the page to zero effectively disables the page again, so that's
-         * what we do. Semantically, the first four bytes are supposed to be a
-         * flag saying whether the guest really needs to issue an EOI. Setting
-         * that flag to zero means that it must always issue one, which is what
-         * we want. Once a page has been repurposed as an APIC assist page the
-         * guest isn't allowed to set anything in it, so the flag remains zero
-         * and all is fine. The guest is allowed to clear flags in the page,
-         * but that doesn't cause us any problems.
-         */
-        if ( val & 1 ) /* APIC assist page enabled? */
-        {
-            uint32_t word = 0;
-            paddr_t page_start = val & ~1ul;
-            (void)hvm_copy_to_guest_phys(page_start, &word, sizeof(word));
-        }
+        perfc_incr(mshv_wrmsr_apic_msr);
+        v->arch.hvm_vcpu.viridian.apic_assist.raw = val;
+        dump_apic_assist(v);
+        if (v->arch.hvm_vcpu.viridian.apic_assist.fields.enabled)
+            initialize_apic_assist(v);
         break;
 
     default:
@@ -228,20 +265,21 @@ int wrmsr_viridian_regs(uint32_t idx, ui
 int rdmsr_viridian_regs(uint32_t idx, uint64_t *val)
 {
     struct vcpu *v = current;
+    struct domain *d = v->domain;
     
-    if ( !is_viridian_domain(v->domain) )
+    if ( !is_viridian_domain(d) )
         return 0;
 
     switch ( idx )
     {
     case VIRIDIAN_MSR_GUEST_OS_ID:
         perfc_incr(mshv_rdmsr_osid);
-        *val = v->domain->arch.hvm_domain.viridian.guest_os_id.raw;
+        *val = d->arch.hvm_domain.viridian.guest_os_id.raw;
         break;
 
     case VIRIDIAN_MSR_HYPERCALL:
         perfc_incr(mshv_rdmsr_hc_page);
-        *val = v->domain->arch.hvm_domain.viridian.hypercall_gpa.raw;
+        *val = d->arch.hvm_domain.viridian.hypercall_gpa.raw;
         break;
 
     case VIRIDIAN_MSR_VP_INDEX:
@@ -260,6 +298,11 @@ int rdmsr_viridian_regs(uint32_t idx, ui
         *val = vlapic_get_reg(vcpu_vlapic(v), APIC_TASKPRI);
         break;
 
+    case VIRIDIAN_MSR_APIC_ASSIST:
+        perfc_incr(mshv_rdmsr_apic_msr);
+        *val = v->arch.hvm_vcpu.viridian.apic_assist.raw;
+        break;
+
     default:
         return 0;
     }
diff -r e90438f6e6d1 -r a08073c5cf28 xen/include/asm-x86/hvm/vcpu.h
--- a/xen/include/asm-x86/hvm/vcpu.h	Wed Sep 14 11:38:13 2011 +0100
+++ b/xen/include/asm-x86/hvm/vcpu.h	Thu Sep 15 06:11:01 2011 +0100
@@ -23,6 +23,7 @@
 #include <xen/tasklet.h>
 #include <asm/hvm/io.h>
 #include <asm/hvm/vlapic.h>
+#include <asm/hvm/viridian.h>
 #include <asm/hvm/vmx/vmcs.h>
 #include <asm/hvm/vmx/vvmx.h>
 #include <asm/hvm/svm/vmcb.h>
@@ -163,6 +164,8 @@ struct hvm_vcpu {
     int           inject_trap;       /* -1 for nothing to inject */
     int           inject_error_code;
     unsigned long inject_cr2;
+
+    struct viridian_vcpu viridian;
 };
 
 #endif /* __ASM_X86_HVM_VCPU_H__ */
diff -r e90438f6e6d1 -r a08073c5cf28 xen/include/asm-x86/hvm/viridian.h
--- a/xen/include/asm-x86/hvm/viridian.h	Wed Sep 14 11:38:13 2011 +0100
+++ b/xen/include/asm-x86/hvm/viridian.h	Thu Sep 15 06:11:01 2011 +0100
@@ -9,6 +9,21 @@
 #ifndef __ASM_X86_HVM_VIRIDIAN_H__
 #define __ASM_X86_HVM_VIRIDIAN_H__
 
+union viridian_apic_assist
+{   uint64_t raw;
+    struct
+    {
+        uint64_t enabled:1;
+        uint64_t reserved_preserved:11;
+        uint64_t pfn:48;
+    } fields;
+};
+
+struct viridian_vcpu
+{
+    union viridian_apic_assist apic_assist;
+};
+
 union viridian_guest_os_id
 {
     uint64_t raw;
diff -r e90438f6e6d1 -r a08073c5cf28 xen/include/asm-x86/perfc_defn.h
--- a/xen/include/asm-x86/perfc_defn.h	Wed Sep 14 11:38:13 2011 +0100
+++ b/xen/include/asm-x86/perfc_defn.h	Thu Sep 15 06:11:01 2011 +0100
@@ -120,12 +120,14 @@ PERFCOUNTER(mshv_rdmsr_hc_page,         
 PERFCOUNTER(mshv_rdmsr_vp_index,        "MS Hv rdmsr vp index")
 PERFCOUNTER(mshv_rdmsr_icr,             "MS Hv rdmsr icr")
 PERFCOUNTER(mshv_rdmsr_tpr,             "MS Hv rdmsr tpr")
+PERFCOUNTER(mshv_rdmsr_apic_assist,     "MS Hv rdmsr APIC assist")
 PERFCOUNTER(mshv_wrmsr_osid,            "MS Hv wrmsr Guest OS ID")
 PERFCOUNTER(mshv_wrmsr_hc_page,         "MS Hv wrmsr hypercall page")
 PERFCOUNTER(mshv_wrmsr_vp_index,        "MS Hv wrmsr vp index")
 PERFCOUNTER(mshv_wrmsr_icr,             "MS Hv wrmsr icr")
 PERFCOUNTER(mshv_wrmsr_tpr,             "MS Hv wrmsr tpr")
 PERFCOUNTER(mshv_wrmsr_eoi,             "MS Hv wrmsr eoi")
+PERFCOUNTER(mshv_wrmsr_apic_assist,     "MS Hv wrmsr APIC assist")
 
 PERFCOUNTER(realmode_emulations, "realmode instructions emulated")
 PERFCOUNTER(realmode_exits,      "vmexits from realmode")

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Tidy up the viridian code a little and flesh out the APIC assist MSR
  2011-09-15  5:13 [PATCH] Tidy up the viridian code a little and flesh out the APIC assist MSR Paul Durrant
@ 2011-09-15  6:38 ` Keir Fraser
  2011-09-15 14:19   ` Paul Durrant
  0 siblings, 1 reply; 5+ messages in thread
From: Keir Fraser @ 2011-09-15  6:38 UTC (permalink / raw)
  To: Paul Durrant, xen-devel

On 15/09/2011 06:13, "Paul Durrant" <paul.durrant@citrix.com> wrote:

> # HG changeset patch
> # User Paul Durrant <paul.durrant@citrix.com>
> # Date 1316063461 -3600
> # Node ID a08073c5cf28ab76893102cc7a8d20bf3e5e15ae
> # Parent  e90438f6e6d1585a71b18784a99c162b5d95f390
> Tidy up the viridian code a little and flesh out the APIC assist MSR
> handling code.
> 
> We don't say we that handle that MSR but Windows assumes it. In Windows 7 it
> just wrote to the MSR and we used to handle that ok. Windows 8 also reads
> from the MSR so we need to keep a record of the contents.

Does this work properly across save/restore?

 -- Keir

> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> 
> diff -r e90438f6e6d1 -r a08073c5cf28 xen/arch/x86/hvm/viridian.c
> --- a/xen/arch/x86/hvm/viridian.c Wed Sep 14 11:38:13 2011 +0100
> +++ b/xen/arch/x86/hvm/viridian.c Thu Sep 15 06:11:01 2011 +0100
> @@ -96,9 +96,43 @@ int cpuid_viridian_leaves(unsigned int l
>      return 1;
>  }
>  
> -static void enable_hypercall_page(void)
> +void dump_guest_os_id(struct domain *d)
>  {
> -    struct domain *d = current->domain;
> +    gdprintk(XENLOG_INFO, "GUEST_OS_ID:\n");
> +    gdprintk(XENLOG_INFO, "\tvendor: %x\n",
> +            d->arch.hvm_domain.viridian.guest_os_id.fields.vendor);
> +    gdprintk(XENLOG_INFO, "\tos: %x\n",
> +            d->arch.hvm_domain.viridian.guest_os_id.fields.os);
> +    gdprintk(XENLOG_INFO, "\tmajor: %x\n",
> +            d->arch.hvm_domain.viridian.guest_os_id.fields.major);
> +    gdprintk(XENLOG_INFO, "\tminor: %x\n",
> +            d->arch.hvm_domain.viridian.guest_os_id.fields.minor);
> +    gdprintk(XENLOG_INFO, "\tsp: %x\n",
> +            d->arch.hvm_domain.viridian.guest_os_id.fields.service_pack);
> +    gdprintk(XENLOG_INFO, "\tbuild: %x\n",
> +            d->arch.hvm_domain.viridian.guest_os_id.fields.build_number);
> +}
> +
> +void dump_hypercall(struct domain *d)
> +{
> +    gdprintk(XENLOG_INFO, "HYPERCALL:\n");
> +    gdprintk(XENLOG_INFO, "\tenabled: %x\n",
> +            d->arch.hvm_domain.viridian.hypercall_gpa.fields.enabled);
> +    gdprintk(XENLOG_INFO, "\tpfn: %lx\n",
> +            (unsigned
> long)d->arch.hvm_domain.viridian.hypercall_gpa.fields.pfn);
> +}
> +
> +void dump_apic_assist(struct vcpu *v)
> +{
> +    gdprintk(XENLOG_INFO, "APIC_ASSIST[%d]:\n", v->vcpu_id);
> +    gdprintk(XENLOG_INFO, "\tenabled: %x\n",
> +            v->arch.hvm_vcpu.viridian.apic_assist.fields.enabled);
> +    gdprintk(XENLOG_INFO, "\tpfn: %lx\n",
> +            (unsigned long)v->arch.hvm_vcpu.viridian.apic_assist.fields.pfn);
> +}
> +
> +static void enable_hypercall_page(struct domain *d)
> +{
>      unsigned long gmfn =
> d->arch.hvm_domain.viridian.hypercall_gpa.fields.pfn;
>      unsigned long mfn = gmfn_to_mfn(d, gmfn);
>      uint8_t *p;
> @@ -130,9 +164,43 @@ static void enable_hypercall_page(void)
>      put_page_and_type(mfn_to_page(mfn));
>  }
>  
> +void initialize_apic_assist(struct vcpu *v)
> +{
> +    struct domain *d = v->domain;
> +    unsigned long gmfn = v->arch.hvm_vcpu.viridian.apic_assist.fields.pfn;
> +    unsigned long mfn = gmfn_to_mfn(d, gmfn);
> +    uint8_t *p;
> +
> +    /*
> +     * We don't support the APIC assist page, and that fact is reflected in
> +     * our CPUID flags. However, Newer versions of Windows have a bug which
> +     * means that they don't recognise that, and tries to use the page
> +     * anyway. We therefore have to fake up just enough to keep windows
> happy.
> +     *
> +     * See http://msdn.microsoft.com/en-us/library/ff538657%28VS.85%29.aspx
> for
> +     * details of how Windows uses the page.
> +     */
> +
> +    if ( !mfn_valid(mfn) ||
> +         !get_page_and_type(mfn_to_page(mfn), d, PGT_writable_page) )
> +    {
> +        gdprintk(XENLOG_WARNING, "Bad GMFN %lx (MFN %lx)\n", gmfn, mfn);
> +        return;
> +    }
> +
> +    p = map_domain_page(mfn);
> +
> +    *(u32 *)p = 0;
> +
> +    unmap_domain_page(p);
> +
> +    put_page_and_type(mfn_to_page(mfn));
> +}
> +
>  int wrmsr_viridian_regs(uint32_t idx, uint64_t val)
>  {
> -    struct domain *d = current->domain;
> +    struct vcpu *v = current;
> +    struct domain *d = v->domain;
>  
>      if ( !is_viridian_domain(d) )
>          return 0;
> @@ -142,44 +210,29 @@ int wrmsr_viridian_regs(uint32_t idx, ui
>      case VIRIDIAN_MSR_GUEST_OS_ID:
>          perfc_incr(mshv_wrmsr_osid);
>          d->arch.hvm_domain.viridian.guest_os_id.raw = val;
> -        gdprintk(XENLOG_INFO, "Guest os:\n");
> -        gdprintk(XENLOG_INFO, "\tvendor: %x\n",
> -               d->arch.hvm_domain.viridian.guest_os_id.fields.vendor);
> -        gdprintk(XENLOG_INFO, "\tos: %x\n",
> -               d->arch.hvm_domain.viridian.guest_os_id.fields.os);
> -        gdprintk(XENLOG_INFO, "\tmajor: %x\n",
> -               d->arch.hvm_domain.viridian.guest_os_id.fields.major);
> -        gdprintk(XENLOG_INFO, "\tminor: %x\n",
> -               d->arch.hvm_domain.viridian.guest_os_id.fields.minor);
> -        gdprintk(XENLOG_INFO, "\tsp: %x\n",
> -               d->arch.hvm_domain.viridian.guest_os_id.fields.service_pack);
> -        gdprintk(XENLOG_INFO, "\tbuild: %x\n",
> -               d->arch.hvm_domain.viridian.guest_os_id.fields.build_number);
> +        dump_guest_os_id(d);
>          break;
>  
>      case VIRIDIAN_MSR_HYPERCALL:
>          perfc_incr(mshv_wrmsr_hc_page);
> -        gdprintk(XENLOG_INFO, "Set hypercall page %"PRIx64".\n", val);
> -        if ( d->arch.hvm_domain.viridian.guest_os_id.raw == 0 )
> -            break;
>          d->arch.hvm_domain.viridian.hypercall_gpa.raw = val;
> +        dump_hypercall(d);
>          if ( d->arch.hvm_domain.viridian.hypercall_gpa.fields.enabled )
> -            enable_hypercall_page();
> +            enable_hypercall_page(d);
>          break;
>  
>      case VIRIDIAN_MSR_VP_INDEX:
>          perfc_incr(mshv_wrmsr_vp_index);
> -        gdprintk(XENLOG_INFO, "Set VP index %"PRIu64".\n", val);
>          break;
>  
>      case VIRIDIAN_MSR_EOI:
>          perfc_incr(mshv_wrmsr_eoi);
> -        vlapic_EOI_set(vcpu_vlapic(current));
> +        vlapic_EOI_set(vcpu_vlapic(v));
>          break;
>  
>      case VIRIDIAN_MSR_ICR: {
>          u32 eax = (u32)val, edx = (u32)(val >> 32);
> -        struct vlapic *vlapic = vcpu_vlapic(current);
> +        struct vlapic *vlapic = vcpu_vlapic(v);
>          perfc_incr(mshv_wrmsr_icr);
>          eax &= ~(1 << 12);
>          edx &= 0xff000000;
> @@ -191,31 +244,15 @@ int wrmsr_viridian_regs(uint32_t idx, ui
>  
>      case VIRIDIAN_MSR_TPR:
>          perfc_incr(mshv_wrmsr_tpr);
> -        vlapic_set_reg(vcpu_vlapic(current), APIC_TASKPRI, (uint8_t)val);
> +        vlapic_set_reg(vcpu_vlapic(v), APIC_TASKPRI, (uint8_t)val);
>          break;
>  
>      case VIRIDIAN_MSR_APIC_ASSIST:
> -        /*
> -         * We don't support the APIC assist page, and that fact is reflected
> in
> -         * our CPUID flags. However, Windows 7 build 7000 has a bug which
> means
> -         * that it doesn't recognise that, and tries to use the page anyway.
> We
> -         * therefore have to fake up just enough to keep win7 happy.
> -         * Fortunately, that's really easy: just setting the first four bytes
> -         * in the page to zero effectively disables the page again, so that's
> -         * what we do. Semantically, the first four bytes are supposed to be
> a
> -         * flag saying whether the guest really needs to issue an EOI.
> Setting
> -         * that flag to zero means that it must always issue one, which is
> what
> -         * we want. Once a page has been repurposed as an APIC assist page
> the
> -         * guest isn't allowed to set anything in it, so the flag remains
> zero
> -         * and all is fine. The guest is allowed to clear flags in the page,
> -         * but that doesn't cause us any problems.
> -         */
> -        if ( val & 1 ) /* APIC assist page enabled? */
> -        {
> -            uint32_t word = 0;
> -            paddr_t page_start = val & ~1ul;
> -            (void)hvm_copy_to_guest_phys(page_start, &word, sizeof(word));
> -        }
> +        perfc_incr(mshv_wrmsr_apic_msr);
> +        v->arch.hvm_vcpu.viridian.apic_assist.raw = val;
> +        dump_apic_assist(v);
> +        if (v->arch.hvm_vcpu.viridian.apic_assist.fields.enabled)
> +            initialize_apic_assist(v);
>          break;
>  
>      default:
> @@ -228,20 +265,21 @@ int wrmsr_viridian_regs(uint32_t idx, ui
>  int rdmsr_viridian_regs(uint32_t idx, uint64_t *val)
>  {
>      struct vcpu *v = current;
> +    struct domain *d = v->domain;
>      
> -    if ( !is_viridian_domain(v->domain) )
> +    if ( !is_viridian_domain(d) )
>          return 0;
>  
>      switch ( idx )
>      {
>      case VIRIDIAN_MSR_GUEST_OS_ID:
>          perfc_incr(mshv_rdmsr_osid);
> -        *val = v->domain->arch.hvm_domain.viridian.guest_os_id.raw;
> +        *val = d->arch.hvm_domain.viridian.guest_os_id.raw;
>          break;
>  
>      case VIRIDIAN_MSR_HYPERCALL:
>          perfc_incr(mshv_rdmsr_hc_page);
> -        *val = v->domain->arch.hvm_domain.viridian.hypercall_gpa.raw;
> +        *val = d->arch.hvm_domain.viridian.hypercall_gpa.raw;
>          break;
>  
>      case VIRIDIAN_MSR_VP_INDEX:
> @@ -260,6 +298,11 @@ int rdmsr_viridian_regs(uint32_t idx, ui
>          *val = vlapic_get_reg(vcpu_vlapic(v), APIC_TASKPRI);
>          break;
>  
> +    case VIRIDIAN_MSR_APIC_ASSIST:
> +        perfc_incr(mshv_rdmsr_apic_msr);
> +        *val = v->arch.hvm_vcpu.viridian.apic_assist.raw;
> +        break;
> +
>      default:
>          return 0;
>      }
> diff -r e90438f6e6d1 -r a08073c5cf28 xen/include/asm-x86/hvm/vcpu.h
> --- a/xen/include/asm-x86/hvm/vcpu.h Wed Sep 14 11:38:13 2011 +0100
> +++ b/xen/include/asm-x86/hvm/vcpu.h Thu Sep 15 06:11:01 2011 +0100
> @@ -23,6 +23,7 @@
>  #include <xen/tasklet.h>
>  #include <asm/hvm/io.h>
>  #include <asm/hvm/vlapic.h>
> +#include <asm/hvm/viridian.h>
>  #include <asm/hvm/vmx/vmcs.h>
>  #include <asm/hvm/vmx/vvmx.h>
>  #include <asm/hvm/svm/vmcb.h>
> @@ -163,6 +164,8 @@ struct hvm_vcpu {
>      int           inject_trap;       /* -1 for nothing to inject */
>      int           inject_error_code;
>      unsigned long inject_cr2;
> +
> +    struct viridian_vcpu viridian;
>  };
>  
>  #endif /* __ASM_X86_HVM_VCPU_H__ */
> diff -r e90438f6e6d1 -r a08073c5cf28 xen/include/asm-x86/hvm/viridian.h
> --- a/xen/include/asm-x86/hvm/viridian.h Wed Sep 14 11:38:13 2011 +0100
> +++ b/xen/include/asm-x86/hvm/viridian.h Thu Sep 15 06:11:01 2011 +0100
> @@ -9,6 +9,21 @@
>  #ifndef __ASM_X86_HVM_VIRIDIAN_H__
>  #define __ASM_X86_HVM_VIRIDIAN_H__
>  
> +union viridian_apic_assist
> +{   uint64_t raw;
> +    struct
> +    {
> +        uint64_t enabled:1;
> +        uint64_t reserved_preserved:11;
> +        uint64_t pfn:48;
> +    } fields;
> +};
> +
> +struct viridian_vcpu
> +{
> +    union viridian_apic_assist apic_assist;
> +};
> +
>  union viridian_guest_os_id
>  {
>      uint64_t raw;
> diff -r e90438f6e6d1 -r a08073c5cf28 xen/include/asm-x86/perfc_defn.h
> --- a/xen/include/asm-x86/perfc_defn.h Wed Sep 14 11:38:13 2011 +0100
> +++ b/xen/include/asm-x86/perfc_defn.h Thu Sep 15 06:11:01 2011 +0100
> @@ -120,12 +120,14 @@ PERFCOUNTER(mshv_rdmsr_hc_page,
>  PERFCOUNTER(mshv_rdmsr_vp_index,        "MS Hv rdmsr vp index")
>  PERFCOUNTER(mshv_rdmsr_icr,             "MS Hv rdmsr icr")
>  PERFCOUNTER(mshv_rdmsr_tpr,             "MS Hv rdmsr tpr")
> +PERFCOUNTER(mshv_rdmsr_apic_assist,     "MS Hv rdmsr APIC assist")
>  PERFCOUNTER(mshv_wrmsr_osid,            "MS Hv wrmsr Guest OS ID")
>  PERFCOUNTER(mshv_wrmsr_hc_page,         "MS Hv wrmsr hypercall page")
>  PERFCOUNTER(mshv_wrmsr_vp_index,        "MS Hv wrmsr vp index")
>  PERFCOUNTER(mshv_wrmsr_icr,             "MS Hv wrmsr icr")
>  PERFCOUNTER(mshv_wrmsr_tpr,             "MS Hv wrmsr tpr")
>  PERFCOUNTER(mshv_wrmsr_eoi,             "MS Hv wrmsr eoi")
> +PERFCOUNTER(mshv_wrmsr_apic_assist,     "MS Hv wrmsr APIC assist")
>  
>  PERFCOUNTER(realmode_emulations, "realmode instructions emulated")
>  PERFCOUNTER(realmode_exits,      "vmexits from realmode")
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH] Tidy up the viridian code a little and flesh out the APIC assist MSR
  2011-09-15  6:38 ` Keir Fraser
@ 2011-09-15 14:19   ` Paul Durrant
  2011-09-15 14:31     ` Keir Fraser
  0 siblings, 1 reply; 5+ messages in thread
From: Paul Durrant @ 2011-09-15 14:19 UTC (permalink / raw)
  To: Keir Fraser, xen-devel

Good point. I guess I may need to explicitly save/restore the apic assist pfn. I'll check.

  Paul

> -----Original Message-----
> From: Keir Fraser [mailto:keir.xen@gmail.com]
> Sent: 14 September 2011 23:38
> To: Paul Durrant; xen-devel@lists.xensource.com
> Subject: Re: [Xen-devel] [PATCH] Tidy up the viridian code a little
> and flesh out the APIC assist MSR
> 
> On 15/09/2011 06:13, "Paul Durrant" <paul.durrant@citrix.com> wrote:
> 
> > # HG changeset patch
> > # User Paul Durrant <paul.durrant@citrix.com> # Date 1316063461 -
> 3600
> > # Node ID a08073c5cf28ab76893102cc7a8d20bf3e5e15ae
> > # Parent  e90438f6e6d1585a71b18784a99c162b5d95f390
> > Tidy up the viridian code a little and flesh out the APIC assist
> MSR
> > handling code.
> >
> > We don't say we that handle that MSR but Windows assumes it. In
> > Windows 7 it just wrote to the MSR and we used to handle that ok.
> > Windows 8 also reads from the MSR so we need to keep a record of
> the contents.
> 
> Does this work properly across save/restore?
> 
>  -- Keir
> 
> > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> >
> > diff -r e90438f6e6d1 -r a08073c5cf28 xen/arch/x86/hvm/viridian.c
> > --- a/xen/arch/x86/hvm/viridian.c Wed Sep 14 11:38:13 2011 +0100
> > +++ b/xen/arch/x86/hvm/viridian.c Thu Sep 15 06:11:01 2011 +0100
> > @@ -96,9 +96,43 @@ int cpuid_viridian_leaves(unsigned int l
> >      return 1;
> >  }
> >
> > -static void enable_hypercall_page(void)
> > +void dump_guest_os_id(struct domain *d)
> >  {
> > -    struct domain *d = current->domain;
> > +    gdprintk(XENLOG_INFO, "GUEST_OS_ID:\n");
> > +    gdprintk(XENLOG_INFO, "\tvendor: %x\n",
> > +            d-
> >arch.hvm_domain.viridian.guest_os_id.fields.vendor);
> > +    gdprintk(XENLOG_INFO, "\tos: %x\n",
> > +            d->arch.hvm_domain.viridian.guest_os_id.fields.os);
> > +    gdprintk(XENLOG_INFO, "\tmajor: %x\n",
> > +            d-
> >arch.hvm_domain.viridian.guest_os_id.fields.major);
> > +    gdprintk(XENLOG_INFO, "\tminor: %x\n",
> > +            d-
> >arch.hvm_domain.viridian.guest_os_id.fields.minor);
> > +    gdprintk(XENLOG_INFO, "\tsp: %x\n",
> > +            d-
> >arch.hvm_domain.viridian.guest_os_id.fields.service_pack);
> > +    gdprintk(XENLOG_INFO, "\tbuild: %x\n",
> > +
> > +d->arch.hvm_domain.viridian.guest_os_id.fields.build_number);
> > +}
> > +
> > +void dump_hypercall(struct domain *d) {
> > +    gdprintk(XENLOG_INFO, "HYPERCALL:\n");
> > +    gdprintk(XENLOG_INFO, "\tenabled: %x\n",
> > +            d-
> >arch.hvm_domain.viridian.hypercall_gpa.fields.enabled);
> > +    gdprintk(XENLOG_INFO, "\tpfn: %lx\n",
> > +            (unsigned
> > long)d->arch.hvm_domain.viridian.hypercall_gpa.fields.pfn);
> > +}
> > +
> > +void dump_apic_assist(struct vcpu *v) {
> > +    gdprintk(XENLOG_INFO, "APIC_ASSIST[%d]:\n", v->vcpu_id);
> > +    gdprintk(XENLOG_INFO, "\tenabled: %x\n",
> > +            v-
> >arch.hvm_vcpu.viridian.apic_assist.fields.enabled);
> > +    gdprintk(XENLOG_INFO, "\tpfn: %lx\n",
> > +            (unsigned
> > +long)v->arch.hvm_vcpu.viridian.apic_assist.fields.pfn);
> > +}
> > +
> > +static void enable_hypercall_page(struct domain *d) {
> >      unsigned long gmfn =
> > d->arch.hvm_domain.viridian.hypercall_gpa.fields.pfn;
> >      unsigned long mfn = gmfn_to_mfn(d, gmfn);
> >      uint8_t *p;
> > @@ -130,9 +164,43 @@ static void enable_hypercall_page(void)
> >      put_page_and_type(mfn_to_page(mfn));
> >  }
> >
> > +void initialize_apic_assist(struct vcpu *v) {
> > +    struct domain *d = v->domain;
> > +    unsigned long gmfn = v-
> >arch.hvm_vcpu.viridian.apic_assist.fields.pfn;
> > +    unsigned long mfn = gmfn_to_mfn(d, gmfn);
> > +    uint8_t *p;
> > +
> > +    /*
> > +     * We don't support the APIC assist page, and that fact is
> reflected in
> > +     * our CPUID flags. However, Newer versions of Windows have a
> bug which
> > +     * means that they don't recognise that, and tries to use the
> page
> > +     * anyway. We therefore have to fake up just enough to keep
> > + windows
> > happy.
> > +     *
> > +     * See
> > + http://msdn.microsoft.com/en-us/library/ff538657%28VS.85%29.aspx
> > for
> > +     * details of how Windows uses the page.
> > +     */
> > +
> > +    if ( !mfn_valid(mfn) ||
> > +         !get_page_and_type(mfn_to_page(mfn), d,
> PGT_writable_page) )
> > +    {
> > +        gdprintk(XENLOG_WARNING, "Bad GMFN %lx (MFN %lx)\n",
> gmfn, mfn);
> > +        return;
> > +    }
> > +
> > +    p = map_domain_page(mfn);
> > +
> > +    *(u32 *)p = 0;
> > +
> > +    unmap_domain_page(p);
> > +
> > +    put_page_and_type(mfn_to_page(mfn));
> > +}
> > +
> >  int wrmsr_viridian_regs(uint32_t idx, uint64_t val)  {
> > -    struct domain *d = current->domain;
> > +    struct vcpu *v = current;
> > +    struct domain *d = v->domain;
> >
> >      if ( !is_viridian_domain(d) )
> >          return 0;
> > @@ -142,44 +210,29 @@ int wrmsr_viridian_regs(uint32_t idx, ui
> >      case VIRIDIAN_MSR_GUEST_OS_ID:
> >          perfc_incr(mshv_wrmsr_osid);
> >          d->arch.hvm_domain.viridian.guest_os_id.raw = val;
> > -        gdprintk(XENLOG_INFO, "Guest os:\n");
> > -        gdprintk(XENLOG_INFO, "\tvendor: %x\n",
> > -               d-
> >arch.hvm_domain.viridian.guest_os_id.fields.vendor);
> > -        gdprintk(XENLOG_INFO, "\tos: %x\n",
> > -               d-
> >arch.hvm_domain.viridian.guest_os_id.fields.os);
> > -        gdprintk(XENLOG_INFO, "\tmajor: %x\n",
> > -               d-
> >arch.hvm_domain.viridian.guest_os_id.fields.major);
> > -        gdprintk(XENLOG_INFO, "\tminor: %x\n",
> > -               d-
> >arch.hvm_domain.viridian.guest_os_id.fields.minor);
> > -        gdprintk(XENLOG_INFO, "\tsp: %x\n",
> > -               d-
> >arch.hvm_domain.viridian.guest_os_id.fields.service_pack);
> > -        gdprintk(XENLOG_INFO, "\tbuild: %x\n",
> > -               d-
> >arch.hvm_domain.viridian.guest_os_id.fields.build_number);
> > +        dump_guest_os_id(d);
> >          break;
> >
> >      case VIRIDIAN_MSR_HYPERCALL:
> >          perfc_incr(mshv_wrmsr_hc_page);
> > -        gdprintk(XENLOG_INFO, "Set hypercall page %"PRIx64".\n",
> val);
> > -        if ( d->arch.hvm_domain.viridian.guest_os_id.raw == 0 )
> > -            break;
> >          d->arch.hvm_domain.viridian.hypercall_gpa.raw = val;
> > +        dump_hypercall(d);
> >          if ( d-
> >arch.hvm_domain.viridian.hypercall_gpa.fields.enabled )
> > -            enable_hypercall_page();
> > +            enable_hypercall_page(d);
> >          break;
> >
> >      case VIRIDIAN_MSR_VP_INDEX:
> >          perfc_incr(mshv_wrmsr_vp_index);
> > -        gdprintk(XENLOG_INFO, "Set VP index %"PRIu64".\n", val);
> >          break;
> >
> >      case VIRIDIAN_MSR_EOI:
> >          perfc_incr(mshv_wrmsr_eoi);
> > -        vlapic_EOI_set(vcpu_vlapic(current));
> > +        vlapic_EOI_set(vcpu_vlapic(v));
> >          break;
> >
> >      case VIRIDIAN_MSR_ICR: {
> >          u32 eax = (u32)val, edx = (u32)(val >> 32);
> > -        struct vlapic *vlapic = vcpu_vlapic(current);
> > +        struct vlapic *vlapic = vcpu_vlapic(v);
> >          perfc_incr(mshv_wrmsr_icr);
> >          eax &= ~(1 << 12);
> >          edx &= 0xff000000;
> > @@ -191,31 +244,15 @@ int wrmsr_viridian_regs(uint32_t idx, ui
> >
> >      case VIRIDIAN_MSR_TPR:
> >          perfc_incr(mshv_wrmsr_tpr);
> > -        vlapic_set_reg(vcpu_vlapic(current), APIC_TASKPRI,
> (uint8_t)val);
> > +        vlapic_set_reg(vcpu_vlapic(v), APIC_TASKPRI,
> (uint8_t)val);
> >          break;
> >
> >      case VIRIDIAN_MSR_APIC_ASSIST:
> > -        /*
> > -         * We don't support the APIC assist page, and that fact
> is reflected
> > in
> > -         * our CPUID flags. However, Windows 7 build 7000 has a
> bug which
> > means
> > -         * that it doesn't recognise that, and tries to use the
> page anyway.
> > We
> > -         * therefore have to fake up just enough to keep win7
> happy.
> > -         * Fortunately, that's really easy: just setting the
> first four bytes
> > -         * in the page to zero effectively disables the page
> again, so that's
> > -         * what we do. Semantically, the first four bytes are
> supposed to be
> > a
> > -         * flag saying whether the guest really needs to issue an
> EOI.
> > Setting
> > -         * that flag to zero means that it must always issue one,
> which is
> > what
> > -         * we want. Once a page has been repurposed as an APIC
> assist page
> > the
> > -         * guest isn't allowed to set anything in it, so the flag
> remains
> > zero
> > -         * and all is fine. The guest is allowed to clear flags
> in the page,
> > -         * but that doesn't cause us any problems.
> > -         */
> > -        if ( val & 1 ) /* APIC assist page enabled? */
> > -        {
> > -            uint32_t word = 0;
> > -            paddr_t page_start = val & ~1ul;
> > -            (void)hvm_copy_to_guest_phys(page_start, &word,
> sizeof(word));
> > -        }
> > +        perfc_incr(mshv_wrmsr_apic_msr);
> > +        v->arch.hvm_vcpu.viridian.apic_assist.raw = val;
> > +        dump_apic_assist(v);
> > +        if (v->arch.hvm_vcpu.viridian.apic_assist.fields.enabled)
> > +            initialize_apic_assist(v);
> >          break;
> >
> >      default:
> > @@ -228,20 +265,21 @@ int wrmsr_viridian_regs(uint32_t idx, ui
> int
> > rdmsr_viridian_regs(uint32_t idx, uint64_t *val)  {
> >      struct vcpu *v = current;
> > +    struct domain *d = v->domain;
> >
> > -    if ( !is_viridian_domain(v->domain) )
> > +    if ( !is_viridian_domain(d) )
> >          return 0;
> >
> >      switch ( idx )
> >      {
> >      case VIRIDIAN_MSR_GUEST_OS_ID:
> >          perfc_incr(mshv_rdmsr_osid);
> > -        *val = v->domain-
> >arch.hvm_domain.viridian.guest_os_id.raw;
> > +        *val = d->arch.hvm_domain.viridian.guest_os_id.raw;
> >          break;
> >
> >      case VIRIDIAN_MSR_HYPERCALL:
> >          perfc_incr(mshv_rdmsr_hc_page);
> > -        *val = v->domain-
> >arch.hvm_domain.viridian.hypercall_gpa.raw;
> > +        *val = d->arch.hvm_domain.viridian.hypercall_gpa.raw;
> >          break;
> >
> >      case VIRIDIAN_MSR_VP_INDEX:
> > @@ -260,6 +298,11 @@ int rdmsr_viridian_regs(uint32_t idx, ui
> >          *val = vlapic_get_reg(vcpu_vlapic(v), APIC_TASKPRI);
> >          break;
> >
> > +    case VIRIDIAN_MSR_APIC_ASSIST:
> > +        perfc_incr(mshv_rdmsr_apic_msr);
> > +        *val = v->arch.hvm_vcpu.viridian.apic_assist.raw;
> > +        break;
> > +
> >      default:
> >          return 0;
> >      }
> > diff -r e90438f6e6d1 -r a08073c5cf28 xen/include/asm-
> x86/hvm/vcpu.h
> > --- a/xen/include/asm-x86/hvm/vcpu.h Wed Sep 14 11:38:13 2011
> +0100
> > +++ b/xen/include/asm-x86/hvm/vcpu.h Thu Sep 15 06:11:01 2011
> +0100
> > @@ -23,6 +23,7 @@
> >  #include <xen/tasklet.h>
> >  #include <asm/hvm/io.h>
> >  #include <asm/hvm/vlapic.h>
> > +#include <asm/hvm/viridian.h>
> >  #include <asm/hvm/vmx/vmcs.h>
> >  #include <asm/hvm/vmx/vvmx.h>
> >  #include <asm/hvm/svm/vmcb.h>
> > @@ -163,6 +164,8 @@ struct hvm_vcpu {
> >      int           inject_trap;       /* -1 for nothing to inject
> */
> >      int           inject_error_code;
> >      unsigned long inject_cr2;
> > +
> > +    struct viridian_vcpu viridian;
> >  };
> >
> >  #endif /* __ASM_X86_HVM_VCPU_H__ */
> > diff -r e90438f6e6d1 -r a08073c5cf28
> > xen/include/asm-x86/hvm/viridian.h
> > --- a/xen/include/asm-x86/hvm/viridian.h Wed Sep 14 11:38:13 2011
> > +0100
> > +++ b/xen/include/asm-x86/hvm/viridian.h Thu Sep 15 06:11:01 2011
> > +++ +0100
> > @@ -9,6 +9,21 @@
> >  #ifndef __ASM_X86_HVM_VIRIDIAN_H__
> >  #define __ASM_X86_HVM_VIRIDIAN_H__
> >
> > +union viridian_apic_assist
> > +{   uint64_t raw;
> > +    struct
> > +    {
> > +        uint64_t enabled:1;
> > +        uint64_t reserved_preserved:11;
> > +        uint64_t pfn:48;
> > +    } fields;
> > +};
> > +
> > +struct viridian_vcpu
> > +{
> > +    union viridian_apic_assist apic_assist; };
> > +
> >  union viridian_guest_os_id
> >  {
> >      uint64_t raw;
> > diff -r e90438f6e6d1 -r a08073c5cf28 xen/include/asm-
> x86/perfc_defn.h
> > --- a/xen/include/asm-x86/perfc_defn.h Wed Sep 14 11:38:13 2011
> +0100
> > +++ b/xen/include/asm-x86/perfc_defn.h Thu Sep 15 06:11:01 2011
> +0100
> > @@ -120,12 +120,14 @@ PERFCOUNTER(mshv_rdmsr_hc_page,
> >  PERFCOUNTER(mshv_rdmsr_vp_index,        "MS Hv rdmsr vp index")
> >  PERFCOUNTER(mshv_rdmsr_icr,             "MS Hv rdmsr icr")
> >  PERFCOUNTER(mshv_rdmsr_tpr,             "MS Hv rdmsr tpr")
> > +PERFCOUNTER(mshv_rdmsr_apic_assist,     "MS Hv rdmsr APIC
> assist")
> >  PERFCOUNTER(mshv_wrmsr_osid,            "MS Hv wrmsr Guest OS
> ID")
> >  PERFCOUNTER(mshv_wrmsr_hc_page,         "MS Hv wrmsr hypercall
> page")
> >  PERFCOUNTER(mshv_wrmsr_vp_index,        "MS Hv wrmsr vp index")
> >  PERFCOUNTER(mshv_wrmsr_icr,             "MS Hv wrmsr icr")
> >  PERFCOUNTER(mshv_wrmsr_tpr,             "MS Hv wrmsr tpr")
> >  PERFCOUNTER(mshv_wrmsr_eoi,             "MS Hv wrmsr eoi")
> > +PERFCOUNTER(mshv_wrmsr_apic_assist,     "MS Hv wrmsr APIC
> assist")
> >
> >  PERFCOUNTER(realmode_emulations, "realmode instructions
> emulated")
> >  PERFCOUNTER(realmode_exits,      "vmexits from realmode")
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xensource.com
> > http://lists.xensource.com/xen-devel
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Tidy up the viridian code a little and flesh out the APIC assist MSR
  2011-09-15 14:19   ` Paul Durrant
@ 2011-09-15 14:31     ` Keir Fraser
  2011-09-15 14:47       ` Paul Durrant
  0 siblings, 1 reply; 5+ messages in thread
From: Keir Fraser @ 2011-09-15 14:31 UTC (permalink / raw)
  To: Paul Durrant, xen-devel

Might need a shiny new Viridian struct type for our save format, which can
then be extended with more Viridian-y stuff as required.

 -- Keir

On 15/09/2011 15:19, "Paul Durrant" <Paul.Durrant@citrix.com> wrote:

> Good point. I guess I may need to explicitly save/restore the apic assist pfn.
> I'll check.
> 
>   Paul
> 
>> -----Original Message-----
>> From: Keir Fraser [mailto:keir.xen@gmail.com]
>> Sent: 14 September 2011 23:38
>> To: Paul Durrant; xen-devel@lists.xensource.com
>> Subject: Re: [Xen-devel] [PATCH] Tidy up the viridian code a little
>> and flesh out the APIC assist MSR
>> 
>> On 15/09/2011 06:13, "Paul Durrant" <paul.durrant@citrix.com> wrote:
>> 
>>> # HG changeset patch
>>> # User Paul Durrant <paul.durrant@citrix.com> # Date 1316063461 -
>> 3600
>>> # Node ID a08073c5cf28ab76893102cc7a8d20bf3e5e15ae
>>> # Parent  e90438f6e6d1585a71b18784a99c162b5d95f390
>>> Tidy up the viridian code a little and flesh out the APIC assist
>> MSR
>>> handling code.
>>> 
>>> We don't say we that handle that MSR but Windows assumes it. In
>>> Windows 7 it just wrote to the MSR and we used to handle that ok.
>>> Windows 8 also reads from the MSR so we need to keep a record of
>> the contents.
>> 
>> Does this work properly across save/restore?
>> 
>>  -- Keir
>> 
>>> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
>>> 
>>> diff -r e90438f6e6d1 -r a08073c5cf28 xen/arch/x86/hvm/viridian.c
>>> --- a/xen/arch/x86/hvm/viridian.c Wed Sep 14 11:38:13 2011 +0100
>>> +++ b/xen/arch/x86/hvm/viridian.c Thu Sep 15 06:11:01 2011 +0100
>>> @@ -96,9 +96,43 @@ int cpuid_viridian_leaves(unsigned int l
>>>      return 1;
>>>  }
>>> 
>>> -static void enable_hypercall_page(void)
>>> +void dump_guest_os_id(struct domain *d)
>>>  {
>>> -    struct domain *d = current->domain;
>>> +    gdprintk(XENLOG_INFO, "GUEST_OS_ID:\n");
>>> +    gdprintk(XENLOG_INFO, "\tvendor: %x\n",
>>> +            d-
>>> arch.hvm_domain.viridian.guest_os_id.fields.vendor);
>>> +    gdprintk(XENLOG_INFO, "\tos: %x\n",
>>> +            d->arch.hvm_domain.viridian.guest_os_id.fields.os);
>>> +    gdprintk(XENLOG_INFO, "\tmajor: %x\n",
>>> +            d-
>>> arch.hvm_domain.viridian.guest_os_id.fields.major);
>>> +    gdprintk(XENLOG_INFO, "\tminor: %x\n",
>>> +            d-
>>> arch.hvm_domain.viridian.guest_os_id.fields.minor);
>>> +    gdprintk(XENLOG_INFO, "\tsp: %x\n",
>>> +            d-
>>> arch.hvm_domain.viridian.guest_os_id.fields.service_pack);
>>> +    gdprintk(XENLOG_INFO, "\tbuild: %x\n",
>>> +
>>> +d->arch.hvm_domain.viridian.guest_os_id.fields.build_number);
>>> +}
>>> +
>>> +void dump_hypercall(struct domain *d) {
>>> +    gdprintk(XENLOG_INFO, "HYPERCALL:\n");
>>> +    gdprintk(XENLOG_INFO, "\tenabled: %x\n",
>>> +            d-
>>> arch.hvm_domain.viridian.hypercall_gpa.fields.enabled);
>>> +    gdprintk(XENLOG_INFO, "\tpfn: %lx\n",
>>> +            (unsigned
>>> long)d->arch.hvm_domain.viridian.hypercall_gpa.fields.pfn);
>>> +}
>>> +
>>> +void dump_apic_assist(struct vcpu *v) {
>>> +    gdprintk(XENLOG_INFO, "APIC_ASSIST[%d]:\n", v->vcpu_id);
>>> +    gdprintk(XENLOG_INFO, "\tenabled: %x\n",
>>> +            v-
>>> arch.hvm_vcpu.viridian.apic_assist.fields.enabled);
>>> +    gdprintk(XENLOG_INFO, "\tpfn: %lx\n",
>>> +            (unsigned
>>> +long)v->arch.hvm_vcpu.viridian.apic_assist.fields.pfn);
>>> +}
>>> +
>>> +static void enable_hypercall_page(struct domain *d) {
>>>      unsigned long gmfn =
>>> d->arch.hvm_domain.viridian.hypercall_gpa.fields.pfn;
>>>      unsigned long mfn = gmfn_to_mfn(d, gmfn);
>>>      uint8_t *p;
>>> @@ -130,9 +164,43 @@ static void enable_hypercall_page(void)
>>>      put_page_and_type(mfn_to_page(mfn));
>>>  }
>>> 
>>> +void initialize_apic_assist(struct vcpu *v) {
>>> +    struct domain *d = v->domain;
>>> +    unsigned long gmfn = v-
>>> arch.hvm_vcpu.viridian.apic_assist.fields.pfn;
>>> +    unsigned long mfn = gmfn_to_mfn(d, gmfn);
>>> +    uint8_t *p;
>>> +
>>> +    /*
>>> +     * We don't support the APIC assist page, and that fact is
>> reflected in
>>> +     * our CPUID flags. However, Newer versions of Windows have a
>> bug which
>>> +     * means that they don't recognise that, and tries to use the
>> page
>>> +     * anyway. We therefore have to fake up just enough to keep
>>> + windows
>>> happy.
>>> +     *
>>> +     * See
>>> + http://msdn.microsoft.com/en-us/library/ff538657%28VS.85%29.aspx
>>> for
>>> +     * details of how Windows uses the page.
>>> +     */
>>> +
>>> +    if ( !mfn_valid(mfn) ||
>>> +         !get_page_and_type(mfn_to_page(mfn), d,
>> PGT_writable_page) )
>>> +    {
>>> +        gdprintk(XENLOG_WARNING, "Bad GMFN %lx (MFN %lx)\n",
>> gmfn, mfn);
>>> +        return;
>>> +    }
>>> +
>>> +    p = map_domain_page(mfn);
>>> +
>>> +    *(u32 *)p = 0;
>>> +
>>> +    unmap_domain_page(p);
>>> +
>>> +    put_page_and_type(mfn_to_page(mfn));
>>> +}
>>> +
>>>  int wrmsr_viridian_regs(uint32_t idx, uint64_t val)  {
>>> -    struct domain *d = current->domain;
>>> +    struct vcpu *v = current;
>>> +    struct domain *d = v->domain;
>>> 
>>>      if ( !is_viridian_domain(d) )
>>>          return 0;
>>> @@ -142,44 +210,29 @@ int wrmsr_viridian_regs(uint32_t idx, ui
>>>      case VIRIDIAN_MSR_GUEST_OS_ID:
>>>          perfc_incr(mshv_wrmsr_osid);
>>>          d->arch.hvm_domain.viridian.guest_os_id.raw = val;
>>> -        gdprintk(XENLOG_INFO, "Guest os:\n");
>>> -        gdprintk(XENLOG_INFO, "\tvendor: %x\n",
>>> -               d-
>>> arch.hvm_domain.viridian.guest_os_id.fields.vendor);
>>> -        gdprintk(XENLOG_INFO, "\tos: %x\n",
>>> -               d-
>>> arch.hvm_domain.viridian.guest_os_id.fields.os);
>>> -        gdprintk(XENLOG_INFO, "\tmajor: %x\n",
>>> -               d-
>>> arch.hvm_domain.viridian.guest_os_id.fields.major);
>>> -        gdprintk(XENLOG_INFO, "\tminor: %x\n",
>>> -               d-
>>> arch.hvm_domain.viridian.guest_os_id.fields.minor);
>>> -        gdprintk(XENLOG_INFO, "\tsp: %x\n",
>>> -               d-
>>> arch.hvm_domain.viridian.guest_os_id.fields.service_pack);
>>> -        gdprintk(XENLOG_INFO, "\tbuild: %x\n",
>>> -               d-
>>> arch.hvm_domain.viridian.guest_os_id.fields.build_number);
>>> +        dump_guest_os_id(d);
>>>          break;
>>> 
>>>      case VIRIDIAN_MSR_HYPERCALL:
>>>          perfc_incr(mshv_wrmsr_hc_page);
>>> -        gdprintk(XENLOG_INFO, "Set hypercall page %"PRIx64".\n",
>> val);
>>> -        if ( d->arch.hvm_domain.viridian.guest_os_id.raw == 0 )
>>> -            break;
>>>          d->arch.hvm_domain.viridian.hypercall_gpa.raw = val;
>>> +        dump_hypercall(d);
>>>          if ( d-
>>> arch.hvm_domain.viridian.hypercall_gpa.fields.enabled )
>>> -            enable_hypercall_page();
>>> +            enable_hypercall_page(d);
>>>          break;
>>> 
>>>      case VIRIDIAN_MSR_VP_INDEX:
>>>          perfc_incr(mshv_wrmsr_vp_index);
>>> -        gdprintk(XENLOG_INFO, "Set VP index %"PRIu64".\n", val);
>>>          break;
>>> 
>>>      case VIRIDIAN_MSR_EOI:
>>>          perfc_incr(mshv_wrmsr_eoi);
>>> -        vlapic_EOI_set(vcpu_vlapic(current));
>>> +        vlapic_EOI_set(vcpu_vlapic(v));
>>>          break;
>>> 
>>>      case VIRIDIAN_MSR_ICR: {
>>>          u32 eax = (u32)val, edx = (u32)(val >> 32);
>>> -        struct vlapic *vlapic = vcpu_vlapic(current);
>>> +        struct vlapic *vlapic = vcpu_vlapic(v);
>>>          perfc_incr(mshv_wrmsr_icr);
>>>          eax &= ~(1 << 12);
>>>          edx &= 0xff000000;
>>> @@ -191,31 +244,15 @@ int wrmsr_viridian_regs(uint32_t idx, ui
>>> 
>>>      case VIRIDIAN_MSR_TPR:
>>>          perfc_incr(mshv_wrmsr_tpr);
>>> -        vlapic_set_reg(vcpu_vlapic(current), APIC_TASKPRI,
>> (uint8_t)val);
>>> +        vlapic_set_reg(vcpu_vlapic(v), APIC_TASKPRI,
>> (uint8_t)val);
>>>          break;
>>> 
>>>      case VIRIDIAN_MSR_APIC_ASSIST:
>>> -        /*
>>> -         * We don't support the APIC assist page, and that fact
>> is reflected
>>> in
>>> -         * our CPUID flags. However, Windows 7 build 7000 has a
>> bug which
>>> means
>>> -         * that it doesn't recognise that, and tries to use the
>> page anyway.
>>> We
>>> -         * therefore have to fake up just enough to keep win7
>> happy.
>>> -         * Fortunately, that's really easy: just setting the
>> first four bytes
>>> -         * in the page to zero effectively disables the page
>> again, so that's
>>> -         * what we do. Semantically, the first four bytes are
>> supposed to be
>>> a
>>> -         * flag saying whether the guest really needs to issue an
>> EOI.
>>> Setting
>>> -         * that flag to zero means that it must always issue one,
>> which is
>>> what
>>> -         * we want. Once a page has been repurposed as an APIC
>> assist page
>>> the
>>> -         * guest isn't allowed to set anything in it, so the flag
>> remains
>>> zero
>>> -         * and all is fine. The guest is allowed to clear flags
>> in the page,
>>> -         * but that doesn't cause us any problems.
>>> -         */
>>> -        if ( val & 1 ) /* APIC assist page enabled? */
>>> -        {
>>> -            uint32_t word = 0;
>>> -            paddr_t page_start = val & ~1ul;
>>> -            (void)hvm_copy_to_guest_phys(page_start, &word,
>> sizeof(word));
>>> -        }
>>> +        perfc_incr(mshv_wrmsr_apic_msr);
>>> +        v->arch.hvm_vcpu.viridian.apic_assist.raw = val;
>>> +        dump_apic_assist(v);
>>> +        if (v->arch.hvm_vcpu.viridian.apic_assist.fields.enabled)
>>> +            initialize_apic_assist(v);
>>>          break;
>>> 
>>>      default:
>>> @@ -228,20 +265,21 @@ int wrmsr_viridian_regs(uint32_t idx, ui
>> int
>>> rdmsr_viridian_regs(uint32_t idx, uint64_t *val)  {
>>>      struct vcpu *v = current;
>>> +    struct domain *d = v->domain;
>>> 
>>> -    if ( !is_viridian_domain(v->domain) )
>>> +    if ( !is_viridian_domain(d) )
>>>          return 0;
>>> 
>>>      switch ( idx )
>>>      {
>>>      case VIRIDIAN_MSR_GUEST_OS_ID:
>>>          perfc_incr(mshv_rdmsr_osid);
>>> -        *val = v->domain-
>>> arch.hvm_domain.viridian.guest_os_id.raw;
>>> +        *val = d->arch.hvm_domain.viridian.guest_os_id.raw;
>>>          break;
>>> 
>>>      case VIRIDIAN_MSR_HYPERCALL:
>>>          perfc_incr(mshv_rdmsr_hc_page);
>>> -        *val = v->domain-
>>> arch.hvm_domain.viridian.hypercall_gpa.raw;
>>> +        *val = d->arch.hvm_domain.viridian.hypercall_gpa.raw;
>>>          break;
>>> 
>>>      case VIRIDIAN_MSR_VP_INDEX:
>>> @@ -260,6 +298,11 @@ int rdmsr_viridian_regs(uint32_t idx, ui
>>>          *val = vlapic_get_reg(vcpu_vlapic(v), APIC_TASKPRI);
>>>          break;
>>> 
>>> +    case VIRIDIAN_MSR_APIC_ASSIST:
>>> +        perfc_incr(mshv_rdmsr_apic_msr);
>>> +        *val = v->arch.hvm_vcpu.viridian.apic_assist.raw;
>>> +        break;
>>> +
>>>      default:
>>>          return 0;
>>>      }
>>> diff -r e90438f6e6d1 -r a08073c5cf28 xen/include/asm-
>> x86/hvm/vcpu.h
>>> --- a/xen/include/asm-x86/hvm/vcpu.h Wed Sep 14 11:38:13 2011
>> +0100
>>> +++ b/xen/include/asm-x86/hvm/vcpu.h Thu Sep 15 06:11:01 2011
>> +0100
>>> @@ -23,6 +23,7 @@
>>>  #include <xen/tasklet.h>
>>>  #include <asm/hvm/io.h>
>>>  #include <asm/hvm/vlapic.h>
>>> +#include <asm/hvm/viridian.h>
>>>  #include <asm/hvm/vmx/vmcs.h>
>>>  #include <asm/hvm/vmx/vvmx.h>
>>>  #include <asm/hvm/svm/vmcb.h>
>>> @@ -163,6 +164,8 @@ struct hvm_vcpu {
>>>      int           inject_trap;       /* -1 for nothing to inject
>> */
>>>      int           inject_error_code;
>>>      unsigned long inject_cr2;
>>> +
>>> +    struct viridian_vcpu viridian;
>>>  };
>>> 
>>>  #endif /* __ASM_X86_HVM_VCPU_H__ */
>>> diff -r e90438f6e6d1 -r a08073c5cf28
>>> xen/include/asm-x86/hvm/viridian.h
>>> --- a/xen/include/asm-x86/hvm/viridian.h Wed Sep 14 11:38:13 2011
>>> +0100
>>> +++ b/xen/include/asm-x86/hvm/viridian.h Thu Sep 15 06:11:01 2011
>>> +++ +0100
>>> @@ -9,6 +9,21 @@
>>>  #ifndef __ASM_X86_HVM_VIRIDIAN_H__
>>>  #define __ASM_X86_HVM_VIRIDIAN_H__
>>> 
>>> +union viridian_apic_assist
>>> +{   uint64_t raw;
>>> +    struct
>>> +    {
>>> +        uint64_t enabled:1;
>>> +        uint64_t reserved_preserved:11;
>>> +        uint64_t pfn:48;
>>> +    } fields;
>>> +};
>>> +
>>> +struct viridian_vcpu
>>> +{
>>> +    union viridian_apic_assist apic_assist; };
>>> +
>>>  union viridian_guest_os_id
>>>  {
>>>      uint64_t raw;
>>> diff -r e90438f6e6d1 -r a08073c5cf28 xen/include/asm-
>> x86/perfc_defn.h
>>> --- a/xen/include/asm-x86/perfc_defn.h Wed Sep 14 11:38:13 2011
>> +0100
>>> +++ b/xen/include/asm-x86/perfc_defn.h Thu Sep 15 06:11:01 2011
>> +0100
>>> @@ -120,12 +120,14 @@ PERFCOUNTER(mshv_rdmsr_hc_page,
>>>  PERFCOUNTER(mshv_rdmsr_vp_index,        "MS Hv rdmsr vp index")
>>>  PERFCOUNTER(mshv_rdmsr_icr,             "MS Hv rdmsr icr")
>>>  PERFCOUNTER(mshv_rdmsr_tpr,             "MS Hv rdmsr tpr")
>>> +PERFCOUNTER(mshv_rdmsr_apic_assist,     "MS Hv rdmsr APIC
>> assist")
>>>  PERFCOUNTER(mshv_wrmsr_osid,            "MS Hv wrmsr Guest OS
>> ID")
>>>  PERFCOUNTER(mshv_wrmsr_hc_page,         "MS Hv wrmsr hypercall
>> page")
>>>  PERFCOUNTER(mshv_wrmsr_vp_index,        "MS Hv wrmsr vp index")
>>>  PERFCOUNTER(mshv_wrmsr_icr,             "MS Hv wrmsr icr")
>>>  PERFCOUNTER(mshv_wrmsr_tpr,             "MS Hv wrmsr tpr")
>>>  PERFCOUNTER(mshv_wrmsr_eoi,             "MS Hv wrmsr eoi")
>>> +PERFCOUNTER(mshv_wrmsr_apic_assist,     "MS Hv wrmsr APIC
>> assist")
>>> 
>>>  PERFCOUNTER(realmode_emulations, "realmode instructions
>> emulated")
>>>  PERFCOUNTER(realmode_exits,      "vmexits from realmode")
>>> 
>>> _______________________________________________
>>> Xen-devel mailing list
>>> Xen-devel@lists.xensource.com
>>> http://lists.xensource.com/xen-devel
>> 
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH] Tidy up the viridian code a little and flesh out the APIC assist MSR
  2011-09-15 14:31     ` Keir Fraser
@ 2011-09-15 14:47       ` Paul Durrant
  0 siblings, 0 replies; 5+ messages in thread
From: Paul Durrant @ 2011-09-15 14:47 UTC (permalink / raw)
  To: Keir Fraser, xen-devel

Yep. I'm hoping to start making use of apic assist once I've got specific evtchn -> hvm irq mappings working. I can get windows to give me edge triggered interrupts so hopefully I can persuade it to skip the eoi by setting the apic assist bit prior to injection. Keeping track of the pfn will, of course, become quite important at this point :-) For now, I don't think windows reads the msr again after boot so losing the value across a save/restore probably doesn't matter much.

  Paul

> -----Original Message-----
> From: Keir Fraser [mailto:keir.xen@gmail.com]
> Sent: 15 September 2011 07:32
> To: Paul Durrant; xen-devel@lists.xensource.com
> Subject: Re: [Xen-devel] [PATCH] Tidy up the viridian code a little
> and flesh out the APIC assist MSR
>
> Might need a shiny new Viridian struct type for our save format,
> which can then be extended with more Viridian-y stuff as required.
>
>  -- Keir
>
> On 15/09/2011 15:19, "Paul Durrant" <Paul.Durrant@citrix.com> wrote:
>
> > Good point. I guess I may need to explicitly save/restore the apic
> assist pfn.
> > I'll check.
> >
> >   Paul
> >
> >> -----Original Message-----
> >> From: Keir Fraser [mailto:keir.xen@gmail.com]
> >> Sent: 14 September 2011 23:38
> >> To: Paul Durrant; xen-devel@lists.xensource.com
> >> Subject: Re: [Xen-devel] [PATCH] Tidy up the viridian code a
> little
> >> and flesh out the APIC assist MSR
> >>
> >> On 15/09/2011 06:13, "Paul Durrant" <paul.durrant@citrix.com>
> wrote:
> >>
> >>> # HG changeset patch
> >>> # User Paul Durrant <paul.durrant@citrix.com> # Date 1316063461
> -
> >> 3600
> >>> # Node ID a08073c5cf28ab76893102cc7a8d20bf3e5e15ae
> >>> # Parent  e90438f6e6d1585a71b18784a99c162b5d95f390
> >>> Tidy up the viridian code a little and flesh out the APIC assist
> >> MSR
> >>> handling code.
> >>>
> >>> We don't say we that handle that MSR but Windows assumes it. In
> >>> Windows 7 it just wrote to the MSR and we used to handle that
> ok.
> >>> Windows 8 also reads from the MSR so we need to keep a record of
> >> the contents.
> >>
> >> Does this work properly across save/restore?
> >>
> >>  -- Keir
> >>
> >>> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> >>>
> >>> diff -r e90438f6e6d1 -r a08073c5cf28 xen/arch/x86/hvm/viridian.c
> >>> --- a/xen/arch/x86/hvm/viridian.c Wed Sep 14 11:38:13 2011 +0100
> >>> +++ b/xen/arch/x86/hvm/viridian.c Thu Sep 15 06:11:01 2011 +0100
> >>> @@ -96,9 +96,43 @@ int cpuid_viridian_leaves(unsigned int l
> >>>      return 1;
> >>>  }
> >>>
> >>> -static void enable_hypercall_page(void)
> >>> +void dump_guest_os_id(struct domain *d)
> >>>  {
> >>> -    struct domain *d = current->domain;
> >>> +    gdprintk(XENLOG_INFO, "GUEST_OS_ID:\n");
> >>> +    gdprintk(XENLOG_INFO, "\tvendor: %x\n",
> >>> +            d-
> >>> arch.hvm_domain.viridian.guest_os_id.fields.vendor);
> >>> +    gdprintk(XENLOG_INFO, "\tos: %x\n",
> >>> +            d->arch.hvm_domain.viridian.guest_os_id.fields.os);
> >>> +    gdprintk(XENLOG_INFO, "\tmajor: %x\n",
> >>> +            d-
> >>> arch.hvm_domain.viridian.guest_os_id.fields.major);
> >>> +    gdprintk(XENLOG_INFO, "\tminor: %x\n",
> >>> +            d-
> >>> arch.hvm_domain.viridian.guest_os_id.fields.minor);
> >>> +    gdprintk(XENLOG_INFO, "\tsp: %x\n",
> >>> +            d-
> >>> arch.hvm_domain.viridian.guest_os_id.fields.service_pack);
> >>> +    gdprintk(XENLOG_INFO, "\tbuild: %x\n",
> >>> +
> >>> +d->arch.hvm_domain.viridian.guest_os_id.fields.build_number);
> >>> +}
> >>> +
> >>> +void dump_hypercall(struct domain *d) {
> >>> +    gdprintk(XENLOG_INFO, "HYPERCALL:\n");
> >>> +    gdprintk(XENLOG_INFO, "\tenabled: %x\n",
> >>> +            d-
> >>> arch.hvm_domain.viridian.hypercall_gpa.fields.enabled);
> >>> +    gdprintk(XENLOG_INFO, "\tpfn: %lx\n",
> >>> +            (unsigned
> >>> long)d->arch.hvm_domain.viridian.hypercall_gpa.fields.pfn);
> >>> +}
> >>> +
> >>> +void dump_apic_assist(struct vcpu *v) {
> >>> +    gdprintk(XENLOG_INFO, "APIC_ASSIST[%d]:\n", v->vcpu_id);
> >>> +    gdprintk(XENLOG_INFO, "\tenabled: %x\n",
> >>> +            v-
> >>> arch.hvm_vcpu.viridian.apic_assist.fields.enabled);
> >>> +    gdprintk(XENLOG_INFO, "\tpfn: %lx\n",
> >>> +            (unsigned
> >>> +long)v->arch.hvm_vcpu.viridian.apic_assist.fields.pfn);
> >>> +}
> >>> +
> >>> +static void enable_hypercall_page(struct domain *d) {
> >>>      unsigned long gmfn =
> >>> d->arch.hvm_domain.viridian.hypercall_gpa.fields.pfn;
> >>>      unsigned long mfn = gmfn_to_mfn(d, gmfn);
> >>>      uint8_t *p;
> >>> @@ -130,9 +164,43 @@ static void enable_hypercall_page(void)
> >>>      put_page_and_type(mfn_to_page(mfn));
> >>>  }
> >>>
> >>> +void initialize_apic_assist(struct vcpu *v) {
> >>> +    struct domain *d = v->domain;
> >>> +    unsigned long gmfn = v-
> >>> arch.hvm_vcpu.viridian.apic_assist.fields.pfn;
> >>> +    unsigned long mfn = gmfn_to_mfn(d, gmfn);
> >>> +    uint8_t *p;
> >>> +
> >>> +    /*
> >>> +     * We don't support the APIC assist page, and that fact is
> >> reflected in
> >>> +     * our CPUID flags. However, Newer versions of Windows have
> a
> >> bug which
> >>> +     * means that they don't recognise that, and tries to use
> the
> >> page
> >>> +     * anyway. We therefore have to fake up just enough to keep
> >>> + windows
> >>> happy.
> >>> +     *
> >>> +     * See
> >>> + http://msdn.microsoft.com/en-
> us/library/ff538657%28VS.85%29.aspx
> >>> for
> >>> +     * details of how Windows uses the page.
> >>> +     */
> >>> +
> >>> +    if ( !mfn_valid(mfn) ||
> >>> +         !get_page_and_type(mfn_to_page(mfn), d,
> >> PGT_writable_page) )
> >>> +    {
> >>> +        gdprintk(XENLOG_WARNING, "Bad GMFN %lx (MFN %lx)\n",
> >> gmfn, mfn);
> >>> +        return;
> >>> +    }
> >>> +
> >>> +    p = map_domain_page(mfn);
> >>> +
> >>> +    *(u32 *)p = 0;
> >>> +
> >>> +    unmap_domain_page(p);
> >>> +
> >>> +    put_page_and_type(mfn_to_page(mfn));
> >>> +}
> >>> +
> >>>  int wrmsr_viridian_regs(uint32_t idx, uint64_t val)  {
> >>> -    struct domain *d = current->domain;
> >>> +    struct vcpu *v = current;
> >>> +    struct domain *d = v->domain;
> >>>
> >>>      if ( !is_viridian_domain(d) )
> >>>          return 0;
> >>> @@ -142,44 +210,29 @@ int wrmsr_viridian_regs(uint32_t idx, ui
> >>>      case VIRIDIAN_MSR_GUEST_OS_ID:
> >>>          perfc_incr(mshv_wrmsr_osid);
> >>>          d->arch.hvm_domain.viridian.guest_os_id.raw = val;
> >>> -        gdprintk(XENLOG_INFO, "Guest os:\n");
> >>> -        gdprintk(XENLOG_INFO, "\tvendor: %x\n",
> >>> -               d-
> >>> arch.hvm_domain.viridian.guest_os_id.fields.vendor);
> >>> -        gdprintk(XENLOG_INFO, "\tos: %x\n",
> >>> -               d-
> >>> arch.hvm_domain.viridian.guest_os_id.fields.os);
> >>> -        gdprintk(XENLOG_INFO, "\tmajor: %x\n",
> >>> -               d-
> >>> arch.hvm_domain.viridian.guest_os_id.fields.major);
> >>> -        gdprintk(XENLOG_INFO, "\tminor: %x\n",
> >>> -               d-
> >>> arch.hvm_domain.viridian.guest_os_id.fields.minor);
> >>> -        gdprintk(XENLOG_INFO, "\tsp: %x\n",
> >>> -               d-
> >>> arch.hvm_domain.viridian.guest_os_id.fields.service_pack);
> >>> -        gdprintk(XENLOG_INFO, "\tbuild: %x\n",
> >>> -               d-
> >>> arch.hvm_domain.viridian.guest_os_id.fields.build_number);
> >>> +        dump_guest_os_id(d);
> >>>          break;
> >>>
> >>>      case VIRIDIAN_MSR_HYPERCALL:
> >>>          perfc_incr(mshv_wrmsr_hc_page);
> >>> -        gdprintk(XENLOG_INFO, "Set hypercall page
> %"PRIx64".\n",
> >> val);
> >>> -        if ( d->arch.hvm_domain.viridian.guest_os_id.raw == 0 )
> >>> -            break;
> >>>          d->arch.hvm_domain.viridian.hypercall_gpa.raw = val;
> >>> +        dump_hypercall(d);
> >>>          if ( d-
> >>> arch.hvm_domain.viridian.hypercall_gpa.fields.enabled )
> >>> -            enable_hypercall_page();
> >>> +            enable_hypercall_page(d);
> >>>          break;
> >>>
> >>>      case VIRIDIAN_MSR_VP_INDEX:
> >>>          perfc_incr(mshv_wrmsr_vp_index);
> >>> -        gdprintk(XENLOG_INFO, "Set VP index %"PRIu64".\n",
> val);
> >>>          break;
> >>>
> >>>      case VIRIDIAN_MSR_EOI:
> >>>          perfc_incr(mshv_wrmsr_eoi);
> >>> -        vlapic_EOI_set(vcpu_vlapic(current));
> >>> +        vlapic_EOI_set(vcpu_vlapic(v));
> >>>          break;
> >>>
> >>>      case VIRIDIAN_MSR_ICR: {
> >>>          u32 eax = (u32)val, edx = (u32)(val >> 32);
> >>> -        struct vlapic *vlapic = vcpu_vlapic(current);
> >>> +        struct vlapic *vlapic = vcpu_vlapic(v);
> >>>          perfc_incr(mshv_wrmsr_icr);
> >>>          eax &= ~(1 << 12);
> >>>          edx &= 0xff000000;
> >>> @@ -191,31 +244,15 @@ int wrmsr_viridian_regs(uint32_t idx, ui
> >>>
> >>>      case VIRIDIAN_MSR_TPR:
> >>>          perfc_incr(mshv_wrmsr_tpr);
> >>> -        vlapic_set_reg(vcpu_vlapic(current), APIC_TASKPRI,
> >> (uint8_t)val);
> >>> +        vlapic_set_reg(vcpu_vlapic(v), APIC_TASKPRI,
> >> (uint8_t)val);
> >>>          break;
> >>>
> >>>      case VIRIDIAN_MSR_APIC_ASSIST:
> >>> -        /*
> >>> -         * We don't support the APIC assist page, and that fact
> >> is reflected
> >>> in
> >>> -         * our CPUID flags. However, Windows 7 build 7000 has a
> >> bug which
> >>> means
> >>> -         * that it doesn't recognise that, and tries to use the
> >> page anyway.
> >>> We
> >>> -         * therefore have to fake up just enough to keep win7
> >> happy.
> >>> -         * Fortunately, that's really easy: just setting the
> >> first four bytes
> >>> -         * in the page to zero effectively disables the page
> >> again, so that's
> >>> -         * what we do. Semantically, the first four bytes are
> >> supposed to be
> >>> a
> >>> -         * flag saying whether the guest really needs to issue
> an
> >> EOI.
> >>> Setting
> >>> -         * that flag to zero means that it must always issue
> one,
> >> which is
> >>> what
> >>> -         * we want. Once a page has been repurposed as an APIC
> >> assist page
> >>> the
> >>> -         * guest isn't allowed to set anything in it, so the
> flag
> >> remains
> >>> zero
> >>> -         * and all is fine. The guest is allowed to clear flags
> >> in the page,
> >>> -         * but that doesn't cause us any problems.
> >>> -         */
> >>> -        if ( val & 1 ) /* APIC assist page enabled? */
> >>> -        {
> >>> -            uint32_t word = 0;
> >>> -            paddr_t page_start = val & ~1ul;
> >>> -            (void)hvm_copy_to_guest_phys(page_start, &word,
> >> sizeof(word));
> >>> -        }
> >>> +        perfc_incr(mshv_wrmsr_apic_msr);
> >>> +        v->arch.hvm_vcpu.viridian.apic_assist.raw = val;
> >>> +        dump_apic_assist(v);
> >>> +        if (v-
> >arch.hvm_vcpu.viridian.apic_assist.fields.enabled)
> >>> +            initialize_apic_assist(v);
> >>>          break;
> >>>
> >>>      default:
> >>> @@ -228,20 +265,21 @@ int wrmsr_viridian_regs(uint32_t idx, ui
> >> int
> >>> rdmsr_viridian_regs(uint32_t idx, uint64_t *val)  {
> >>>      struct vcpu *v = current;
> >>> +    struct domain *d = v->domain;
> >>>
> >>> -    if ( !is_viridian_domain(v->domain) )
> >>> +    if ( !is_viridian_domain(d) )
> >>>          return 0;
> >>>
> >>>      switch ( idx )
> >>>      {
> >>>      case VIRIDIAN_MSR_GUEST_OS_ID:
> >>>          perfc_incr(mshv_rdmsr_osid);
> >>> -        *val = v->domain-
> >>> arch.hvm_domain.viridian.guest_os_id.raw;
> >>> +        *val = d->arch.hvm_domain.viridian.guest_os_id.raw;
> >>>          break;
> >>>
> >>>      case VIRIDIAN_MSR_HYPERCALL:
> >>>          perfc_incr(mshv_rdmsr_hc_page);
> >>> -        *val = v->domain-
> >>> arch.hvm_domain.viridian.hypercall_gpa.raw;
> >>> +        *val = d->arch.hvm_domain.viridian.hypercall_gpa.raw;
> >>>          break;
> >>>
> >>>      case VIRIDIAN_MSR_VP_INDEX:
> >>> @@ -260,6 +298,11 @@ int rdmsr_viridian_regs(uint32_t idx, ui
> >>>          *val = vlapic_get_reg(vcpu_vlapic(v), APIC_TASKPRI);
> >>>          break;
> >>>
> >>> +    case VIRIDIAN_MSR_APIC_ASSIST:
> >>> +        perfc_incr(mshv_rdmsr_apic_msr);
> >>> +        *val = v->arch.hvm_vcpu.viridian.apic_assist.raw;
> >>> +        break;
> >>> +
> >>>      default:
> >>>          return 0;
> >>>      }
> >>> diff -r e90438f6e6d1 -r a08073c5cf28 xen/include/asm-
> >> x86/hvm/vcpu.h
> >>> --- a/xen/include/asm-x86/hvm/vcpu.h Wed Sep 14 11:38:13 2011
> >> +0100
> >>> +++ b/xen/include/asm-x86/hvm/vcpu.h Thu Sep 15 06:11:01 2011
> >> +0100
> >>> @@ -23,6 +23,7 @@
> >>>  #include <xen/tasklet.h>
> >>>  #include <asm/hvm/io.h>
> >>>  #include <asm/hvm/vlapic.h>
> >>> +#include <asm/hvm/viridian.h>
> >>>  #include <asm/hvm/vmx/vmcs.h>
> >>>  #include <asm/hvm/vmx/vvmx.h>
> >>>  #include <asm/hvm/svm/vmcb.h>
> >>> @@ -163,6 +164,8 @@ struct hvm_vcpu {
> >>>      int           inject_trap;       /* -1 for nothing to
> inject
> >> */
> >>>      int           inject_error_code;
> >>>      unsigned long inject_cr2;
> >>> +
> >>> +    struct viridian_vcpu viridian;
> >>>  };
> >>>
> >>>  #endif /* __ASM_X86_HVM_VCPU_H__ */ diff -r e90438f6e6d1 -r
> >>> a08073c5cf28 xen/include/asm-x86/hvm/viridian.h
> >>> --- a/xen/include/asm-x86/hvm/viridian.h Wed Sep 14 11:38:13
> 2011
> >>> +0100
> >>> +++ b/xen/include/asm-x86/hvm/viridian.h Thu Sep 15 06:11:01
> 2011
> >>> +++ +0100
> >>> @@ -9,6 +9,21 @@
> >>>  #ifndef __ASM_X86_HVM_VIRIDIAN_H__
> >>>  #define __ASM_X86_HVM_VIRIDIAN_H__
> >>>
> >>> +union viridian_apic_assist
> >>> +{   uint64_t raw;
> >>> +    struct
> >>> +    {
> >>> +        uint64_t enabled:1;
> >>> +        uint64_t reserved_preserved:11;
> >>> +        uint64_t pfn:48;
> >>> +    } fields;
> >>> +};
> >>> +
> >>> +struct viridian_vcpu
> >>> +{
> >>> +    union viridian_apic_assist apic_assist; };
> >>> +
> >>>  union viridian_guest_os_id
> >>>  {
> >>>      uint64_t raw;
> >>> diff -r e90438f6e6d1 -r a08073c5cf28 xen/include/asm-
> >> x86/perfc_defn.h
> >>> --- a/xen/include/asm-x86/perfc_defn.h Wed Sep 14 11:38:13 2011
> >> +0100
> >>> +++ b/xen/include/asm-x86/perfc_defn.h Thu Sep 15 06:11:01 2011
> >> +0100
> >>> @@ -120,12 +120,14 @@ PERFCOUNTER(mshv_rdmsr_hc_page,
> >>>  PERFCOUNTER(mshv_rdmsr_vp_index,        "MS Hv rdmsr vp index")
> >>>  PERFCOUNTER(mshv_rdmsr_icr,             "MS Hv rdmsr icr")
> >>>  PERFCOUNTER(mshv_rdmsr_tpr,             "MS Hv rdmsr tpr")
> >>> +PERFCOUNTER(mshv_rdmsr_apic_assist,     "MS Hv rdmsr APIC
> >> assist")
> >>>  PERFCOUNTER(mshv_wrmsr_osid,            "MS Hv wrmsr Guest OS
> >> ID")
> >>>  PERFCOUNTER(mshv_wrmsr_hc_page,         "MS Hv wrmsr hypercall
> >> page")
> >>>  PERFCOUNTER(mshv_wrmsr_vp_index,        "MS Hv wrmsr vp index")
> >>>  PERFCOUNTER(mshv_wrmsr_icr,             "MS Hv wrmsr icr")
> >>>  PERFCOUNTER(mshv_wrmsr_tpr,             "MS Hv wrmsr tpr")
> >>>  PERFCOUNTER(mshv_wrmsr_eoi,             "MS Hv wrmsr eoi")
> >>> +PERFCOUNTER(mshv_wrmsr_apic_assist,     "MS Hv wrmsr APIC
> >> assist")
> >>>
> >>>  PERFCOUNTER(realmode_emulations, "realmode instructions
> >> emulated")
> >>>  PERFCOUNTER(realmode_exits,      "vmexits from realmode")
> >>>
> >>> _______________________________________________
> >>> Xen-devel mailing list
> >>> Xen-devel@lists.xensource.com
> >>> http://lists.xensource.com/xen-devel
> >>
> >
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2011-09-15 14:47 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-15  5:13 [PATCH] Tidy up the viridian code a little and flesh out the APIC assist MSR Paul Durrant
2011-09-15  6:38 ` Keir Fraser
2011-09-15 14:19   ` Paul Durrant
2011-09-15 14:31     ` Keir Fraser
2011-09-15 14:47       ` Paul Durrant

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.