* [PATCH] WHPX: Use QEMU values for trapped CPUID
@ 2020-02-27 21:01 Sunil Muthuswamy
2020-02-27 21:09 ` Eduardo Habkost
2020-02-28 10:59 ` Paolo Bonzini
0 siblings, 2 replies; 4+ messages in thread
From: Sunil Muthuswamy @ 2020-02-27 21:01 UTC (permalink / raw)
To: Paolo Bonzini, Richard Henderson, Eduardo Habkost
Cc: qemu-devel@nongnu.org, Stefan Weil
Currently, WHPX is using some default values for the trapped CPUID
functions. These were not in sync with the QEMU values because the
CPUID values were never set with WHPX during VCPU initialization.
Additionally, at the moment, WHPX doesn't support setting CPUID
values in the hypervisor at runtime (i.e. after the partition has
been setup). That is needed to be able to set the CPUID values in
the hypervisor during VCPU init.
Until that support comes, use the QEMU values for the trapped CPUIDs.
Signed-off-by: Sunil Muthuswamy <sunilmut@microsoft.com>
---
target/i386/whpx-all.c | 42 ++++++++++++++++++------------------------
1 file changed, 18 insertions(+), 24 deletions(-)
diff --git a/target/i386/whpx-all.c b/target/i386/whpx-all.c
index 35601b8176..4fe5a78b29 100644
--- a/target/i386/whpx-all.c
+++ b/target/i386/whpx-all.c
@@ -980,38 +980,32 @@ static int whpx_vcpu_run(CPUState *cpu)
WHV_REGISTER_VALUE reg_values[5];
WHV_REGISTER_NAME reg_names[5];
UINT32 reg_count = 5;
- UINT64 rip, rax, rcx, rdx, rbx;
+ UINT64 cpuid_fn, rip = 0, rax = 0, rcx = 0, rdx = 0, rbx = 0;
+ X86CPU *x86_cpu = X86_CPU(cpu);
+ CPUX86State *env = &x86_cpu->env;
memset(reg_values, 0, sizeof(reg_values));
rip = vcpu->exit_ctx.VpContext.Rip +
vcpu->exit_ctx.VpContext.InstructionLength;
- switch (vcpu->exit_ctx.CpuidAccess.Rax) {
- case 1:
- rax = vcpu->exit_ctx.CpuidAccess.DefaultResultRax;
- /* Advertise that we are running on a hypervisor */
- rcx =
- vcpu->exit_ctx.CpuidAccess.DefaultResultRcx |
- CPUID_EXT_HYPERVISOR;
-
- rdx = vcpu->exit_ctx.CpuidAccess.DefaultResultRdx;
- rbx = vcpu->exit_ctx.CpuidAccess.DefaultResultRbx;
- break;
+ cpuid_fn = vcpu->exit_ctx.CpuidAccess.Rax;
+
+ /*
+ * Ideally, these should be supplied to the hypervisor during VCPU
+ * initialization and it should be able to satisfy this request.
+ * But, currently, WHPX doesn't support setting CPUID values in the
+ * hypervisor once the partition has been setup, which is too late
+ * since VCPUs are realized later. For now, use the values from
+ * QEMU to satisfy these requests, until WHPX adds support for
+ * being able to set these values in the hypervisor at runtime.
+ */
+ cpu_x86_cpuid(env, cpuid_fn, 0, (UINT32 *)&rax, (UINT32 *)&rbx,
+ (UINT32 *)&rcx, (UINT32 *)&rdx);
+ switch (cpuid_fn) {
case 0x80000001:
- rax = vcpu->exit_ctx.CpuidAccess.DefaultResultRax;
/* Remove any support of OSVW */
- rcx =
- vcpu->exit_ctx.CpuidAccess.DefaultResultRcx &
- ~CPUID_EXT3_OSVW;
-
- rdx = vcpu->exit_ctx.CpuidAccess.DefaultResultRdx;
- rbx = vcpu->exit_ctx.CpuidAccess.DefaultResultRbx;
+ rcx &= ~CPUID_EXT3_OSVW;
break;
- default:
- rax = vcpu->exit_ctx.CpuidAccess.DefaultResultRax;
- rcx = vcpu->exit_ctx.CpuidAccess.DefaultResultRcx;
- rdx = vcpu->exit_ctx.CpuidAccess.DefaultResultRdx;
- rbx = vcpu->exit_ctx.CpuidAccess.DefaultResultRbx;
}
reg_names[0] = WHvX64RegisterRip;
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] WHPX: Use QEMU values for trapped CPUID
2020-02-27 21:01 [PATCH] WHPX: Use QEMU values for trapped CPUID Sunil Muthuswamy
@ 2020-02-27 21:09 ` Eduardo Habkost
2020-02-27 21:29 ` [EXTERNAL] " Sunil Muthuswamy
2020-02-28 10:59 ` Paolo Bonzini
1 sibling, 1 reply; 4+ messages in thread
From: Eduardo Habkost @ 2020-02-27 21:09 UTC (permalink / raw)
To: Sunil Muthuswamy
Cc: Paolo Bonzini, Stefan Weil, qemu-devel@nongnu.org,
Richard Henderson
On Thu, Feb 27, 2020 at 09:01:04PM +0000, Sunil Muthuswamy wrote:
> Currently, WHPX is using some default values for the trapped CPUID
> functions. These were not in sync with the QEMU values because the
> CPUID values were never set with WHPX during VCPU initialization.
> Additionally, at the moment, WHPX doesn't support setting CPUID
> values in the hypervisor at runtime (i.e. after the partition has
> been setup). That is needed to be able to set the CPUID values in
> the hypervisor during VCPU init.
> Until that support comes, use the QEMU values for the trapped CPUIDs.
>
> Signed-off-by: Sunil Muthuswamy <sunilmut@microsoft.com>
I like the change, but I wonder if any if your users would still
prefer to use the default result chosen by WHPX instead of the
ones chosen by QEMU.
On the KVM side I have always wondered if we should have a mode
where all CPUID leaves are the ones chosen by KVM, making no
KVM_SET_CPUID calls. It would be useful for experimentation and
debugging of KVM/QEMU defaults.
> ---
> target/i386/whpx-all.c | 42 ++++++++++++++++++------------------------
> 1 file changed, 18 insertions(+), 24 deletions(-)
>
> diff --git a/target/i386/whpx-all.c b/target/i386/whpx-all.c
> index 35601b8176..4fe5a78b29 100644
> --- a/target/i386/whpx-all.c
> +++ b/target/i386/whpx-all.c
> @@ -980,38 +980,32 @@ static int whpx_vcpu_run(CPUState *cpu)
> WHV_REGISTER_VALUE reg_values[5];
> WHV_REGISTER_NAME reg_names[5];
> UINT32 reg_count = 5;
> - UINT64 rip, rax, rcx, rdx, rbx;
> + UINT64 cpuid_fn, rip = 0, rax = 0, rcx = 0, rdx = 0, rbx = 0;
> + X86CPU *x86_cpu = X86_CPU(cpu);
> + CPUX86State *env = &x86_cpu->env;
>
> memset(reg_values, 0, sizeof(reg_values));
>
> rip = vcpu->exit_ctx.VpContext.Rip +
> vcpu->exit_ctx.VpContext.InstructionLength;
> - switch (vcpu->exit_ctx.CpuidAccess.Rax) {
> - case 1:
> - rax = vcpu->exit_ctx.CpuidAccess.DefaultResultRax;
> - /* Advertise that we are running on a hypervisor */
> - rcx =
> - vcpu->exit_ctx.CpuidAccess.DefaultResultRcx |
> - CPUID_EXT_HYPERVISOR;
> -
> - rdx = vcpu->exit_ctx.CpuidAccess.DefaultResultRdx;
> - rbx = vcpu->exit_ctx.CpuidAccess.DefaultResultRbx;
> - break;
> + cpuid_fn = vcpu->exit_ctx.CpuidAccess.Rax;
> +
> + /*
> + * Ideally, these should be supplied to the hypervisor during VCPU
> + * initialization and it should be able to satisfy this request.
> + * But, currently, WHPX doesn't support setting CPUID values in the
> + * hypervisor once the partition has been setup, which is too late
> + * since VCPUs are realized later. For now, use the values from
> + * QEMU to satisfy these requests, until WHPX adds support for
> + * being able to set these values in the hypervisor at runtime.
> + */
> + cpu_x86_cpuid(env, cpuid_fn, 0, (UINT32 *)&rax, (UINT32 *)&rbx,
> + (UINT32 *)&rcx, (UINT32 *)&rdx);
> + switch (cpuid_fn) {
> case 0x80000001:
> - rax = vcpu->exit_ctx.CpuidAccess.DefaultResultRax;
> /* Remove any support of OSVW */
> - rcx =
> - vcpu->exit_ctx.CpuidAccess.DefaultResultRcx &
> - ~CPUID_EXT3_OSVW;
> -
> - rdx = vcpu->exit_ctx.CpuidAccess.DefaultResultRdx;
> - rbx = vcpu->exit_ctx.CpuidAccess.DefaultResultRbx;
> + rcx &= ~CPUID_EXT3_OSVW;
> break;
> - default:
> - rax = vcpu->exit_ctx.CpuidAccess.DefaultResultRax;
> - rcx = vcpu->exit_ctx.CpuidAccess.DefaultResultRcx;
> - rdx = vcpu->exit_ctx.CpuidAccess.DefaultResultRdx;
> - rbx = vcpu->exit_ctx.CpuidAccess.DefaultResultRbx;
> }
>
> reg_names[0] = WHvX64RegisterRip;
> --
> 2.17.1
>
--
Eduardo
^ permalink raw reply [flat|nested] 4+ messages in thread* RE: [EXTERNAL] Re: [PATCH] WHPX: Use QEMU values for trapped CPUID
2020-02-27 21:09 ` Eduardo Habkost
@ 2020-02-27 21:29 ` Sunil Muthuswamy
0 siblings, 0 replies; 4+ messages in thread
From: Sunil Muthuswamy @ 2020-02-27 21:29 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Paolo Bonzini, Richard Henderson, qemu-devel@nongnu.org,
Stefan Weil
> -----Original Message-----
> From: Eduardo Habkost <ehabkost@redhat.com>
> Sent: Thursday, February 27, 2020 1:10 PM
> To: Sunil Muthuswamy <sunilmut@microsoft.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>; Richard Henderson <rth@twiddle.net>; qemu-devel@nongnu.org; Stefan Weil
> <sw@weilnetz.de>
> Subject: [EXTERNAL] Re: [PATCH] WHPX: Use QEMU values for trapped CPUID
>
> On Thu, Feb 27, 2020 at 09:01:04PM +0000, Sunil Muthuswamy wrote:
> > Currently, WHPX is using some default values for the trapped CPUID
> > functions. These were not in sync with the QEMU values because the
> > CPUID values were never set with WHPX during VCPU initialization.
> > Additionally, at the moment, WHPX doesn't support setting CPUID
> > values in the hypervisor at runtime (i.e. after the partition has
> > been setup). That is needed to be able to set the CPUID values in
> > the hypervisor during VCPU init.
> > Until that support comes, use the QEMU values for the trapped CPUIDs.
> >
> > Signed-off-by: Sunil Muthuswamy <sunilmut@microsoft.com>
>
> I like the change, but I wonder if any if your users would still
> prefer to use the default result chosen by WHPX instead of the
> ones chosen by QEMU.
>
Note that the current patch only applies to the trapped CPUIDs, which for
WHPX are currently only {1, 0x80000001}. WHPX will still provide most
of the values.
> On the KVM side I have always wondered if we should have a mode
> where all CPUID leaves are the ones chosen by KVM, making no
> KVM_SET_CPUID calls. It would be useful for experimentation and
> debugging of KVM/QEMU defaults.
>
Agreed. I think such an option could be useful debugging tool.
>
> > ---
> > target/i386/whpx-all.c | 42 ++++++++++++++++++------------------------
> > 1 file changed, 18 insertions(+), 24 deletions(-)
> >
> > diff --git a/target/i386/whpx-all.c b/target/i386/whpx-all.c
> > index 35601b8176..4fe5a78b29 100644
> > --- a/target/i386/whpx-all.c
> > +++ b/target/i386/whpx-all.c
> > @@ -980,38 +980,32 @@ static int whpx_vcpu_run(CPUState *cpu)
> > WHV_REGISTER_VALUE reg_values[5];
> > WHV_REGISTER_NAME reg_names[5];
> > UINT32 reg_count = 5;
> > - UINT64 rip, rax, rcx, rdx, rbx;
> > + UINT64 cpuid_fn, rip = 0, rax = 0, rcx = 0, rdx = 0, rbx = 0;
> > + X86CPU *x86_cpu = X86_CPU(cpu);
> > + CPUX86State *env = &x86_cpu->env;
> >
> > memset(reg_values, 0, sizeof(reg_values));
> >
> > rip = vcpu->exit_ctx.VpContext.Rip +
> > vcpu->exit_ctx.VpContext.InstructionLength;
> > - switch (vcpu->exit_ctx.CpuidAccess.Rax) {
> > - case 1:
> > - rax = vcpu->exit_ctx.CpuidAccess.DefaultResultRax;
> > - /* Advertise that we are running on a hypervisor */
> > - rcx =
> > - vcpu->exit_ctx.CpuidAccess.DefaultResultRcx |
> > - CPUID_EXT_HYPERVISOR;
> > -
> > - rdx = vcpu->exit_ctx.CpuidAccess.DefaultResultRdx;
> > - rbx = vcpu->exit_ctx.CpuidAccess.DefaultResultRbx;
> > - break;
> > + cpuid_fn = vcpu->exit_ctx.CpuidAccess.Rax;
> > +
> > + /*
> > + * Ideally, these should be supplied to the hypervisor during VCPU
> > + * initialization and it should be able to satisfy this request.
> > + * But, currently, WHPX doesn't support setting CPUID values in the
> > + * hypervisor once the partition has been setup, which is too late
> > + * since VCPUs are realized later. For now, use the values from
> > + * QEMU to satisfy these requests, until WHPX adds support for
> > + * being able to set these values in the hypervisor at runtime.
> > + */
> > + cpu_x86_cpuid(env, cpuid_fn, 0, (UINT32 *)&rax, (UINT32 *)&rbx,
> > + (UINT32 *)&rcx, (UINT32 *)&rdx);
> > + switch (cpuid_fn) {
> > case 0x80000001:
> > - rax = vcpu->exit_ctx.CpuidAccess.DefaultResultRax;
> > /* Remove any support of OSVW */
> > - rcx =
> > - vcpu->exit_ctx.CpuidAccess.DefaultResultRcx &
> > - ~CPUID_EXT3_OSVW;
> > -
> > - rdx = vcpu->exit_ctx.CpuidAccess.DefaultResultRdx;
> > - rbx = vcpu->exit_ctx.CpuidAccess.DefaultResultRbx;
> > + rcx &= ~CPUID_EXT3_OSVW;
> > break;
> > - default:
> > - rax = vcpu->exit_ctx.CpuidAccess.DefaultResultRax;
> > - rcx = vcpu->exit_ctx.CpuidAccess.DefaultResultRcx;
> > - rdx = vcpu->exit_ctx.CpuidAccess.DefaultResultRdx;
> > - rbx = vcpu->exit_ctx.CpuidAccess.DefaultResultRbx;
> > }
> >
> > reg_names[0] = WHvX64RegisterRip;
> > --
> > 2.17.1
> >
>
> --
> Eduardo
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] WHPX: Use QEMU values for trapped CPUID
2020-02-27 21:01 [PATCH] WHPX: Use QEMU values for trapped CPUID Sunil Muthuswamy
2020-02-27 21:09 ` Eduardo Habkost
@ 2020-02-28 10:59 ` Paolo Bonzini
1 sibling, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2020-02-28 10:59 UTC (permalink / raw)
To: Sunil Muthuswamy, Richard Henderson, Eduardo Habkost
Cc: Stefan Weil, qemu-devel@nongnu.org
On 27/02/20 22:01, Sunil Muthuswamy wrote:
> Currently, WHPX is using some default values for the trapped CPUID
> functions. These were not in sync with the QEMU values because the
> CPUID values were never set with WHPX during VCPU initialization.
> Additionally, at the moment, WHPX doesn't support setting CPUID
> values in the hypervisor at runtime (i.e. after the partition has
> been setup). That is needed to be able to set the CPUID values in
> the hypervisor during VCPU init.
> Until that support comes, use the QEMU values for the trapped CPUIDs.
>
> Signed-off-by: Sunil Muthuswamy <sunilmut@microsoft.com>
> ---
> target/i386/whpx-all.c | 42 ++++++++++++++++++------------------------
> 1 file changed, 18 insertions(+), 24 deletions(-)
>
> diff --git a/target/i386/whpx-all.c b/target/i386/whpx-all.c
> index 35601b8176..4fe5a78b29 100644
> --- a/target/i386/whpx-all.c
> +++ b/target/i386/whpx-all.c
> @@ -980,38 +980,32 @@ static int whpx_vcpu_run(CPUState *cpu)
> WHV_REGISTER_VALUE reg_values[5];
> WHV_REGISTER_NAME reg_names[5];
> UINT32 reg_count = 5;
> - UINT64 rip, rax, rcx, rdx, rbx;
> + UINT64 cpuid_fn, rip = 0, rax = 0, rcx = 0, rdx = 0, rbx = 0;
> + X86CPU *x86_cpu = X86_CPU(cpu);
> + CPUX86State *env = &x86_cpu->env;
>
> memset(reg_values, 0, sizeof(reg_values));
>
> rip = vcpu->exit_ctx.VpContext.Rip +
> vcpu->exit_ctx.VpContext.InstructionLength;
> - switch (vcpu->exit_ctx.CpuidAccess.Rax) {
> - case 1:
> - rax = vcpu->exit_ctx.CpuidAccess.DefaultResultRax;
> - /* Advertise that we are running on a hypervisor */
> - rcx =
> - vcpu->exit_ctx.CpuidAccess.DefaultResultRcx |
> - CPUID_EXT_HYPERVISOR;
> -
> - rdx = vcpu->exit_ctx.CpuidAccess.DefaultResultRdx;
> - rbx = vcpu->exit_ctx.CpuidAccess.DefaultResultRbx;
> - break;
> + cpuid_fn = vcpu->exit_ctx.CpuidAccess.Rax;
> +
> + /*
> + * Ideally, these should be supplied to the hypervisor during VCPU
> + * initialization and it should be able to satisfy this request.
> + * But, currently, WHPX doesn't support setting CPUID values in the
> + * hypervisor once the partition has been setup, which is too late
> + * since VCPUs are realized later. For now, use the values from
> + * QEMU to satisfy these requests, until WHPX adds support for
> + * being able to set these values in the hypervisor at runtime.
> + */
> + cpu_x86_cpuid(env, cpuid_fn, 0, (UINT32 *)&rax, (UINT32 *)&rbx,
> + (UINT32 *)&rcx, (UINT32 *)&rdx);
> + switch (cpuid_fn) {
> case 0x80000001:
> - rax = vcpu->exit_ctx.CpuidAccess.DefaultResultRax;
> /* Remove any support of OSVW */
> - rcx =
> - vcpu->exit_ctx.CpuidAccess.DefaultResultRcx &
> - ~CPUID_EXT3_OSVW;
> -
> - rdx = vcpu->exit_ctx.CpuidAccess.DefaultResultRdx;
> - rbx = vcpu->exit_ctx.CpuidAccess.DefaultResultRbx;
> + rcx &= ~CPUID_EXT3_OSVW;
> break;
> - default:
> - rax = vcpu->exit_ctx.CpuidAccess.DefaultResultRax;
> - rcx = vcpu->exit_ctx.CpuidAccess.DefaultResultRcx;
> - rdx = vcpu->exit_ctx.CpuidAccess.DefaultResultRdx;
> - rbx = vcpu->exit_ctx.CpuidAccess.DefaultResultRbx;
> }
>
> reg_names[0] = WHvX64RegisterRip;
>
Queued if you need that, thanks.
Paolo
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-02-28 11:00 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-02-27 21:01 [PATCH] WHPX: Use QEMU values for trapped CPUID Sunil Muthuswamy
2020-02-27 21:09 ` Eduardo Habkost
2020-02-27 21:29 ` [EXTERNAL] " Sunil Muthuswamy
2020-02-28 10:59 ` Paolo Bonzini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).