[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] Tidy up the viridian code a little and flesh out the APIC assist MSR


  • To: Paul Durrant <paul.durrant@xxxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxx>
  • From: Keir Fraser <keir.xen@xxxxxxxxx>
  • Date: Thu, 15 Sep 2011 07:38:01 +0100
  • Cc:
  • Delivery-date: Wed, 14 Sep 2011 23:38:50 -0700
  • List-id: Xen developer discussion <xen-devel.lists.xensource.com>
  • Thread-index: AcxzcgONtakPWFHMdk29U12pniwCEw==
  • Thread-topic: [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@xxxxxxxxxx> wrote:

> # HG changeset patch
> # User Paul Durrant <paul.durrant@xxxxxxxxxx>
> # 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@xxxxxxxxxx>
> 
> 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@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel



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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.