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: Keir Fraser <keir.xen@xxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: RE: [Xen-devel] [PATCH] Tidy up the viridian code a little and flesh out the APIC assist MSR
From: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
Date: Thu, 15 Sep 2011 15:47:22 +0100
Accept-language: en-US
Acceptlanguage: en-US
Cc:
Delivery-date: Thu, 15 Sep 2011 07:49:01 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <CA97CADB.20F20%keir.xen@xxxxxxxxx>
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>
References: <291EDFCB1E9E224A99088639C4762022B44F04717B@xxxxxxxxxxxxxxxxxxxxxxxxx> <CA97CADB.20F20%keir.xen@xxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AcxzcgONtakPWFHMdk29U12pniwCEwAQBjXQAACEZ70AAGXvAA==
Thread-topic: [Xen-devel] [PATCH] Tidy up the viridian code a little and flesh out the APIC assist MSR
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@xxxxxxxxx]
> Sent: 15 September 2011 07:32
> To: Paul Durrant; xen-devel@xxxxxxxxxxxxxxxxxxx
> 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@xxxxxxxxxx> 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@xxxxxxxxx]
> >> Sent: 14 September 2011 23:38
> >> To: Paul Durrant; xen-devel@xxxxxxxxxxxxxxxxxxx
> >> 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@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