From: Keir Fraser <keir.xen@gmail.com>
To: Paul Durrant <Paul.Durrant@citrix.com>,
"xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>
Subject: Re: [PATCH] Tidy up the viridian code a little and flesh out the APIC assist MSR
Date: Thu, 15 Sep 2011 15:31:39 +0100 [thread overview]
Message-ID: <CA97CADB.20F20%keir.xen@gmail.com> (raw)
In-Reply-To: <291EDFCB1E9E224A99088639C4762022B44F04717B@LONPMAILBOX01.citrite.net>
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
>>
>
next prev parent reply other threads:[~2011-09-15 14:31 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2011-09-15 14:47 ` Paul Durrant
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CA97CADB.20F20%keir.xen@gmail.com \
--to=keir.xen@gmail.com \
--cc=Paul.Durrant@citrix.com \
--cc=xen-devel@lists.xensource.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.