WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

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

To: Paul Durrant <paul.durrant@xxxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] Tidy up the viridian code a little and flesh out the APIC assist MSR
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
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=user-agent:date:subject:from:to:message-id:thread-topic :thread-index:in-reply-to:mime-version:content-type :content-transfer-encoding; bh=wnD+n6GKXpwRqgnzqdywJbYRDjksbUrPE4skhCvwUk8=; b=wILVaSMurC9VV9cKz/FnXFUL0xinnroSA22HwR+8HTSJRGFQondSjNmjjkimnwgKYE OLem/J4rUnRTl68oHT69/ijLHjBTJQAlCYf5SutCWj6ieJeru5NyE2CKIKbWslTmSMhc rNsnEvshGNF55KurBkV9K5XSO4nEtJVdOZe6E=
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <a08073c5cf28ab768931.1316063586@xxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AcxzcgONtakPWFHMdk29U12pniwCEw==
Thread-topic: [Xen-devel] [PATCH] Tidy up the viridian code a little and flesh out the APIC assist MSR
User-agent: Microsoft-Entourage/12.30.0.110427
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