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