Skip to contentSkip to conversation

WHPX fixes an issue with CPUID 1 not returning CPUID_EXT_HYPERVISOR

PATCH In review Justin Terry (VM) 4 participants 8 messages Mar 26, 2018 – Apr 16, 2018

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 WHPX fixes an issue with CPUID 1 not returning CPUID_EXT_HYPERVISOR

Implements the CPUID trap for CPUID 1 to include the CPUID_EXT_HYPERVISOR flag in the ECX results. This was preventing some older linux kernels from booting when trying to access MSR's that dont make sense when virtualized.

---
 target/i386/whpx-all.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 78 insertions(+), 1 deletion(-)
target/i386/whpx-all.c +78 -1
index bf33d320bf..58435178a4 100644
@@ -911,12 +911,62 @@ static int whpx_vcpu_run(CPUState *cpu)
             ret = 1;
             break;
 
+        case WHvRunVpExitReasonX64Cpuid: {
+            WHV_REGISTER_VALUE reg_values[5] = {0};
+            WHV_REGISTER_NAME reg_names[5];
+            UINT32 reg_count = 5;
+            UINT64 rip, rax, rcx, rdx, rbx;
+
+            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;
+            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;
+            reg_names[1] = WHvX64RegisterRax;
+            reg_names[2] = WHvX64RegisterRcx;
+            reg_names[3] = WHvX64RegisterRdx;
+            reg_names[4] = WHvX64RegisterRbx;
+
+            reg_values[0].Reg64 = rip;
+            reg_values[1].Reg64 = rax;
+            reg_values[2].Reg64 = rcx;
+            reg_values[3].Reg64 = rdx;
+            reg_values[4].Reg64 = rbx;
+
+            hr = WHvSetVirtualProcessorRegisters(whpx->partition,
+                                                 cpu->cpu_index,
+                                                 reg_names,
+                                                 reg_count,
+                                                 reg_values);
+
+            if (FAILED(hr)) {
+                error_report("WHPX: Failed to set CpuidAccess state registers,"
+                             " hr=%08lx", hr);
+            }
+            ret = 0;
+            break;
+        }
         case WHvRunVpExitReasonNone:
         case WHvRunVpExitReasonUnrecoverableException:
         case WHvRunVpExitReasonInvalidVpRegisterValue:
         case WHvRunVpExitReasonUnsupportedFeature:
         case WHvRunVpExitReasonX64MsrAccess:
-        case WHvRunVpExitReasonX64Cpuid:
         case WHvRunVpExitReasonException:
         default:
             error_report("WHPX: Unexpected VP exit code %d",
@@ -1272,6 +1322,33 @@ static int whpx_accel_init(MachineState *ms)
         goto error;
     }
 
+    memset(&prop, 0, sizeof(WHV_PARTITION_PROPERTY));
+    prop.ExtendedVmExits.X64CpuidExit = 1;
+    hr = WHvSetPartitionProperty(whpx->partition,
+                                 WHvPartitionPropertyCodeExtendedVmExits,
+                                 &prop,
+                                 sizeof(WHV_PARTITION_PROPERTY));
+
+    if (FAILED(hr)) {
+        error_report("WHPX: Failed to enable partition extended X64CpuidExit"
+                     " hr=%08lx", hr);
+        ret = -EINVAL;
+        goto error;
+    }
+
+    UINT32 cpuidExitList[] = {1};
+    hr = WHvSetPartitionProperty(whpx->partition,
+                                 WHvPartitionPropertyCodeCpuidExitList,
+                                 cpuidExitList,
+                                 RTL_NUMBER_OF(cpuidExitList) * sizeof(UINT32));
+
+    if (FAILED(hr)) {
+        error_report("WHPX: Failed to set partition CpuidExitList hr=%08lx",
+                     hr);
+        ret = -EINVAL;
+        goto error;
+    }
+
     hr = WHvSetupPartition(whpx->partition);
     if (FAILED(hr)) {
         error_report("WHPX: Failed to setup partition, hr=%08lx", hr);
Signature · 1 line
2.11.0
#1 lore raw

On 03/26/2018 12:06 PM, Justin Terry (VM) via Qemu-devel wrote:

[meta-comment]

Since we're already discussing this today, I noticed the list rewrites your headers due to SPF policies at microsoft.com, to the point that a blind 'git am' on the list message will attempt to attribute your patch to 'Justin Terry (VM) via Qemu-devel <[email protected]>' if the maintainer is not careful.

You may want to configure git on your end to emit an explicit 'From: Justin Terry (VM) <[email protected]>' in the body of each message you send, to override the mailman overwrite of your messages to the list. I don't know whether 'git config format.from "Your Name <[email protected]>"' is sufficient for this (the help for git format-patch mentions that it adds the From: line to the body only when it differs textually from the From: header, but doesn't mention a way to force the From: line in the body) even when the two start out the same.

Quoted 3 lines

Implements the CPUID trap for CPUID 1 to include the CPUID_EXT_HYPERVISOR flag in the ECX results. This was preventing some older linux kernels from booting when trying to access MSR's that dont

s/dont/don't/

Quoted 4 lines

make sense when virtualized.

---

Signature · 3 lines
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org
#2 lore raw
In reply to Justin Terry (VM)
Quoted 32 lines · Justin Terry (VM)

Implements the CPUID trap for CPUID 1 to include the CPUID_EXT_HYPERVISOR flag in the ECX results. This was preventing some older linux kernels from booting when trying to access MSR's that dont make sense when virtualized.

---
 target/i386/whpx-all.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 78 insertions(+), 1 deletion(-)
target/i386/whpx-all.c +14 -0
index bf33d320bf..58435178a4 100644
@@ -911,12 +911,62 @@ static int whpx_vcpu_run(CPUState *cpu)
             ret = 1;
             break;
 
+        case WHvRunVpExitReasonX64Cpuid: {
+            WHV_REGISTER_VALUE reg_values[5] = {0};
+            WHV_REGISTER_NAME reg_names[5];
+            UINT32 reg_count = 5;
+            UINT64 rip, rax, rcx, rdx, rbx;
+
+            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 |

target/i386/whpx-all.c static int whpx_vcpu_run(CPUState *cpu) · line 937

Quoted 10 lines
target/i386/whpx-all.c +10 -0
+                    CPUID_EXT_HYPERVISOR;
+
+                rdx = vcpu->exit_ctx.CpuidAccess.DefaultResultRdx;
+                rbx = vcpu->exit_ctx.CpuidAccess.DefaultResultRbx;
+                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;

Interesting, so the WHPX API already tries to provide default values for the CPUID leaves. Would it make sense to try and use the values returned by cpu_x86_cpuid() in the future?

Is there a way to get the default CPUID results from the WHPX API without calling WHvRunVirtualProcessor(), so QEMU can be aware of what exactly the guest is seeing on CPUID?

Quoted 73 lines
target/i386/whpx-all.c +54 -1
+            }
+
+            reg_names[0] = WHvX64RegisterRip;
+            reg_names[1] = WHvX64RegisterRax;
+            reg_names[2] = WHvX64RegisterRcx;
+            reg_names[3] = WHvX64RegisterRdx;
+            reg_names[4] = WHvX64RegisterRbx;
+
+            reg_values[0].Reg64 = rip;
+            reg_values[1].Reg64 = rax;
+            reg_values[2].Reg64 = rcx;
+            reg_values[3].Reg64 = rdx;
+            reg_values[4].Reg64 = rbx;
+
+            hr = WHvSetVirtualProcessorRegisters(whpx->partition,
+                                                 cpu->cpu_index,
+                                                 reg_names,
+                                                 reg_count,
+                                                 reg_values);
+
+            if (FAILED(hr)) {
+                error_report("WHPX: Failed to set CpuidAccess state registers,"
+                             " hr=%08lx", hr);
+            }
+            ret = 0;
+            break;
+        }
         case WHvRunVpExitReasonNone:
         case WHvRunVpExitReasonUnrecoverableException:
         case WHvRunVpExitReasonInvalidVpRegisterValue:
         case WHvRunVpExitReasonUnsupportedFeature:
         case WHvRunVpExitReasonX64MsrAccess:
-        case WHvRunVpExitReasonX64Cpuid:
         case WHvRunVpExitReasonException:
         default:
             error_report("WHPX: Unexpected VP exit code %d",
@@ -1272,6 +1322,33 @@ static int whpx_accel_init(MachineState *ms)
         goto error;
     }
 
+    memset(&prop, 0, sizeof(WHV_PARTITION_PROPERTY));
+    prop.ExtendedVmExits.X64CpuidExit = 1;
+    hr = WHvSetPartitionProperty(whpx->partition,
+                                 WHvPartitionPropertyCodeExtendedVmExits,
+                                 &prop,
+                                 sizeof(WHV_PARTITION_PROPERTY));
+
+    if (FAILED(hr)) {
+        error_report("WHPX: Failed to enable partition extended X64CpuidExit"
+                     " hr=%08lx", hr);
+        ret = -EINVAL;
+        goto error;
+    }
+
+    UINT32 cpuidExitList[] = {1};
+    hr = WHvSetPartitionProperty(whpx->partition,
+                                 WHvPartitionPropertyCodeCpuidExitList,
+                                 cpuidExitList,
+                                 RTL_NUMBER_OF(cpuidExitList) * sizeof(UINT32));
+
+    if (FAILED(hr)) {
+        error_report("WHPX: Failed to set partition CpuidExitList hr=%08lx",
+                     hr);
+        ret = -EINVAL;
+        goto error;
+    }
+
     hr = WHvSetupPartition(whpx->partition);
     if (FAILED(hr)) {
         error_report("WHPX: Failed to setup partition, hr=%08lx", hr);
Signature · 1 line
2.11.0
Signature · 1 line
Eduardo
#3 lore raw

Hey Eduardo

Responses inline. Thanks!

Quoted 57 lines

-----Original Message-----
From: Eduardo Habkost <[email protected]>
Sent: Wednesday, March 28, 2018 10:51 AM
To: Justin Terry (VM) <[email protected]>
Cc: [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH] WHPX fixes an issue with CPUID 1 not returning CPUID_EXT_HYPERVISOR

Quoted 33 lines · Justin Terry (VM)

Implements the CPUID trap for CPUID 1 to include the CPUID_EXT_HYPERVISOR flag in the ECX results. This was preventing some older linux kernels from booting when trying to access MSR's that dont make sense when virtualized.

---
 target/i386/whpx-all.c | 79
+++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 78 insertions(+), 1 deletion(-)
target/i386/whpx-all.c index renamed from target/i386/whpx-all.c +0 -0

bf33d320bf..58435178a4 100644

target/i386/whpx-all.c +14 -0
@@ -911,12 +911,62 @@ static int whpx_vcpu_run(CPUState *cpu)
             ret = 1;
             break;
 
+        case WHvRunVpExitReasonX64Cpuid: {
+            WHV_REGISTER_VALUE reg_values[5] = {0};
+            WHV_REGISTER_NAME reg_names[5];
+            UINT32 reg_count = 5;
+            UINT64 rip, rax, rcx, rdx, rbx;
+
+            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 |

target/i386/whpx-all.c static int whpx_vcpu_run(CPUState *cpu) · line 937

Quoted 10 lines
target/i386/whpx-all.c +10 -0
+                    CPUID_EXT_HYPERVISOR;
+
+                rdx = vcpu->exit_ctx.CpuidAccess.DefaultResultRdx;
+                rbx = vcpu->exit_ctx.CpuidAccess.DefaultResultRbx;
+                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;

Interesting, so the WHPX API already tries to provide default values for the CPUID leaves. Would it make sense to try and use the values returned by cpu_x86_cpuid() in the future?

Quoted 3 lines

Is there a way to get the default CPUID results from the WHPX API without calling WHvRunVirtualProcessor(), so QEMU can be aware of what exactly the guest is seeing on CPUID?

The platform now has two ways to interact with CPUID.

1. (As the code is doing now). At partition creation time you can register for specific CPUID exits and then respond to the CPUID with your custom answer or with the Hypervisor defaults that were forwarded to you. Unfortunately, QEMU has no way to know the Hypervisor default ahead of time but QEMU can make at least make a runtime decision about how to respond.
2. At partition creation time the platform allows QEMU to inject (set) the default responses for specific CPUID exits. This can now be done by setting the WHV_X64_CPUID_RESULT in the CpuidResultList of WHV_PARTITION_PROPERTY to the exit values QEMU wants. So effectively you can know the answers ahead of time for any that you set but the answers are not dynamic.

The only issues/questions I have there are:

If we use [1] (like the code is now) I don't see any way to keep the exits in cpu_x86_cpuid() matched up with the registered exits to WHPX. This means that WHPX would not be called in these cases and would instead get the Hypervisor default rather than the answer from cpu_x86_cpuid().

If we use [2] to inject the answers at creation time WHPX needs access to the CPUX86State at accel init which also doesn't seem to be possible in QEMU today. WHPX could basically just call cpu_x86_cpuid() for each CPUID QEMU cares about and plumb the answer before start. This has the best performance as we avoid the additional exits but has an issue in that the results must be known ahead of time.

And, we could obviously use a hybrid of the two for cases we know. Do you have any ideas that I could try out here on how you would like to see this work?

Thanks,
Justin

Quoted 81 lines
Quoted 41 lines
target/i386/whpx-all.c +27 -2
+            }
+
+            reg_names[0] = WHvX64RegisterRip;
+            reg_names[1] = WHvX64RegisterRax;
+            reg_names[2] = WHvX64RegisterRcx;
+            reg_names[3] = WHvX64RegisterRdx;
+            reg_names[4] = WHvX64RegisterRbx;
+
+            reg_values[0].Reg64 = rip;
+            reg_values[1].Reg64 = rax;
+            reg_values[2].Reg64 = rcx;
+            reg_values[3].Reg64 = rdx;
+            reg_values[4].Reg64 = rbx;
+
+            hr = WHvSetVirtualProcessorRegisters(whpx->partition,
+                                                 cpu->cpu_index,
+                                                 reg_names,
+                                                 reg_count,
+                                                 reg_values);
+
+            if (FAILED(hr)) {
+                error_report("WHPX: Failed to set CpuidAccess state registers,"
+                             " hr=%08lx", hr);
+            }
+            ret = 0;
+            break;
+        }
         case WHvRunVpExitReasonNone:
         case WHvRunVpExitReasonUnrecoverableException:
         case WHvRunVpExitReasonInvalidVpRegisterValue:
         case WHvRunVpExitReasonUnsupportedFeature:
         case WHvRunVpExitReasonX64MsrAccess:
-        case WHvRunVpExitReasonX64Cpuid:
         case WHvRunVpExitReasonException:
         default:
             error_report("WHPX: Unexpected VP exit code %d", @@
-1272,6 +1322,33 @@ static int whpx_accel_init(MachineState *ms)
         goto error;
     }

target/i386/whpx-all.c static int whpx_vcpu_run(CPUState *cpu)

Quoted 10 lines
target/i386/whpx-all.c +9 -0
 
+    memset(&prop, 0, sizeof(WHV_PARTITION_PROPERTY));
+    prop.ExtendedVmExits.X64CpuidExit = 1;
+    hr = WHvSetPartitionProperty(whpx->partition,
+                                 WHvPartitionPropertyCodeExtendedVmExits,
+                                 &prop,
+                                 sizeof(WHV_PARTITION_PROPERTY));
+
+    if (FAILED(hr)) {
+        error_report("WHPX: Failed to enable partition extended

X64CpuidExit"

Quoted 22 lines
target/i386/whpx-all.c +19 -0
+                     " hr=%08lx", hr);
+        ret = -EINVAL;
+        goto error;
+    }
+
+    UINT32 cpuidExitList[] = {1};
+    hr = WHvSetPartitionProperty(whpx->partition,
+                                 WHvPartitionPropertyCodeCpuidExitList,
+                                 cpuidExitList,
+                                 RTL_NUMBER_OF(cpuidExitList) *
+ sizeof(UINT32));
+
+    if (FAILED(hr)) {
+        error_report("WHPX: Failed to set partition CpuidExitList hr=%08lx",
+                     hr);
+        ret = -EINVAL;
+        goto error;
+    }
+
     hr = WHvSetupPartition(whpx->partition);
     if (FAILED(hr)) {
         error_report("WHPX: Failed to setup partition, hr=%08lx",
Quoted 4 lines

hr);
--
2.11.0

--
Eduardo

#4 lore raw
Quoted 81 lines · Justin Terry (VM)

Hey Eduardo

Responses inline. Thanks!

Quoted 57 lines

-----Original Message-----
From: Eduardo Habkost <[email protected]>
Sent: Wednesday, March 28, 2018 10:51 AM
To: Justin Terry (VM) <[email protected]>
Cc: [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH] WHPX fixes an issue with CPUID 1 not returning CPUID_EXT_HYPERVISOR

Quoted 33 lines · Justin Terry (VM)

Implements the CPUID trap for CPUID 1 to include the CPUID_EXT_HYPERVISOR flag in the ECX results. This was preventing some older linux kernels from booting when trying to access MSR's that dont make sense when virtualized.

---
 target/i386/whpx-all.c | 79
+++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 78 insertions(+), 1 deletion(-)
target/i386/whpx-all.c index renamed from target/i386/whpx-all.c +0 -0

bf33d320bf..58435178a4 100644

target/i386/whpx-all.c +14 -0
@@ -911,12 +911,62 @@ static int whpx_vcpu_run(CPUState *cpu)
             ret = 1;
             break;
 
+        case WHvRunVpExitReasonX64Cpuid: {
+            WHV_REGISTER_VALUE reg_values[5] = {0};
+            WHV_REGISTER_NAME reg_names[5];
+            UINT32 reg_count = 5;
+            UINT64 rip, rax, rcx, rdx, rbx;
+
+            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 |

target/i386/whpx-all.c static int whpx_vcpu_run(CPUState *cpu) · line 937

Quoted 10 lines
target/i386/whpx-all.c +10 -0
+                    CPUID_EXT_HYPERVISOR;
+
+                rdx = vcpu->exit_ctx.CpuidAccess.DefaultResultRdx;
+                rbx = vcpu->exit_ctx.CpuidAccess.DefaultResultRbx;
+                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;

Interesting, so the WHPX API already tries to provide default values for the CPUID leaves. Would it make sense to try and use the values returned by cpu_x86_cpuid() in the future?

Quoted 3 lines

Is there a way to get the default CPUID results from the WHPX API without calling WHvRunVirtualProcessor(), so QEMU can be aware of what exactly the guest is seeing on CPUID?

The platform now has two ways to interact with CPUID.

1. (As the code is doing now). At partition creation time you can register for specific CPUID exits and then respond to the CPUID with your custom answer or with the Hypervisor defaults that were forwarded to you. Unfortunately, QEMU has no way to know the Hypervisor default ahead of time but QEMU can make at least make a runtime decision about how to respond.
2. At partition creation time the platform allows QEMU to inject (set) the default responses for specific CPUID exits. This can now be done by setting the WHV_X64_CPUID_RESULT in the CpuidResultList of WHV_PARTITION_PROPERTY to the exit values QEMU wants. So effectively you can know the answers ahead of time for any that you set but the answers are not dynamic.

Quoted 7 lines

The only issues/questions I have there are:

If we use [1] (like the code is now) I don't see any way to keep the exits in cpu_x86_cpuid() matched up with the registered exits to WHPX. This means that WHPX would not be called in these cases and would instead get the Hypervisor default rather than the answer from cpu_x86_cpuid().

I assume you could call cpu_x86_cpuid() on every CPUID exit and override the hypervisor default completely. Not a very efficient solution, but seems simple to implement.

But I'm still not sure if you really want to override the hypervisor defaults completely (see below).

Quoted 8 lines

If we use [2] to inject the answers at creation time WHPX needs access to the CPUX86State at accel init which also doesn't seem to be possible in QEMU today. WHPX could basically just call cpu_x86_cpuid() for each CPUID QEMU cares about and plumb the answer before start. This has the best performance as we avoid the additional exits but has an issue in that the results must be known ahead of time.

You already have a hook into CPU initialization at whpx_init_vcpu(), wouldn't it work for you?

Quoted 4 lines

And, we could obviously use a hybrid of the two for cases we know. Do you have any ideas that I could try out here on how you would like to see this work?

(2) matches very closely how it works on KVM, and using the contents of cpu_x86_cpuid() (using either method) seems necessary if you want to safely support live migration between different host CPUs.

But there's one thing I would like to understand better, before giving any advice: how important/useful are the hypervisor defaults for your users? The cost of making QEMU manage every single CPUID bit is extra complexity. The benefit you get in return is letting management software + QEMU ensure guest ABI is stable and nothing breaks on live migration. So it depends on the use cases you have in mind.

Signature · 1 line
Eduardo
#5 lore raw
In reply to Justin Terry (VM)
Quoted 9 lines · Justin Terry (VM)

1. (As the code is doing now). At partition creation time you can register for specific CPUID exits and then respond to the CPUID with your custom answer or with the Hypervisor defaults that were forwarded to you. Unfortunately, QEMU has no way to know the Hypervisor default ahead of time but QEMU can make at least make a runtime decision about how to respond.

2. At partition creation time the platform allows QEMU to inject (set) the default responses for specific CPUID exits.

... but it still cannot access the hypervisor defaults, right?

Quoted 10 lines

This can now be done by
setting the WHV_X64_CPUID_RESULT in the CpuidResultList of WHV_PARTITION_PROPERTY to the exit values QEMU wants. So effectively you can know the answers ahead of time for any that you set but the answers are not dynamic.

The only issues/questions I have there are:

If we use [1] (like the code is now) I don't see any way to keep the exits in cpu_x86_cpuid() matched up with the registered exits to WHPX.

You'd have to do that on a case-by-case basis. For example, the number of leaves can be the minimum of the hypervisor and QEMU values, or just the QEMU value; for "feature" leaves the results will be the AND of the QEMU and WHPX features; for the XSAVE/XSAVEC/XSAVES (size, offset) tuples you have to use WHPX's; for family/model/stepping/name/vendor your mileage may vary but I suppose you can just use WHPX's; and so on.

You can take a look at target/i386/cpu.c's array feature_word_info and add a function like

void x86_is_feature_word(uint32_t eax, uint32_t ecx, int reg)

{
    for (w = 0; w < FEATURE_WORDS; w++) {
        FeatureWordInfo *wi = &feature_word_info[w];
        if (eax == wi->cpuid_eax &&
            (ecx == wi->cpuid_ecx || wi->cpuid_needs_ecx) &&
            reg == wi->cpuid_reg) {
            return true;
        }
    }
    return false;
}

so that the code in the end is

    switch (eax) {
        /* yadda yadda... code to special case leaves whose value
         * comes from WHPX.
         */
        ...
    default:
        if (x86_is_feature_word(eax, ecx, R_EAX)) {
            rax &= vcpu->exit_ctx.CpuidAccess.DefaultResultRax;
        }
        if (x86_is_feature_word(eax, ecx, R_EBX)) {
            rax &= vcpu->exit_ctx.CpuidAccess.DefaultResultRbx;
        }
        if (x86_is_feature_word(eax, ecx, R_ECX)) {
            rax &= vcpu->exit_ctx.CpuidAccess.DefaultResultRcx;
        }
        if (x86_is_feature_word(eax, ecx, R_EDX)) {
            rax &= vcpu->exit_ctx.CpuidAccess.DefaultResultRdx;
        }
    }
Quoted 6 lines

If we use [2] to inject the answers at creation time WHPX needs access to the CPUX86State at accel init which also doesn't seem to be possible in QEMU today. WHPX could basically just call cpu_x86_cpuid() for each CPUID QEMU cares about and plumb the answer before start. This has the best performance as we avoid the additional exits but has an issue in that the results must be known ahead of time.

The earliest where you have access to that is x86_cpu_initfn.

Paolo

#6 lore raw
Quoted 1 line · Paolo Bonzini

On 28/03/2018 22:48, Justin Terry (VM) wrote:

[...]

Quoted 8 lines
Quoted 6 lines

If we use [2] to inject the answers at creation time WHPX needs access to the CPUX86State at accel init which also doesn't seem to be possible in QEMU today. WHPX could basically just call cpu_x86_cpuid() for each CPUID QEMU cares about and plumb the answer before start. This has the best performance as we avoid the additional exits but has an issue in that the results must be known ahead of time.

The earliest where you have access to that is x86_cpu_initfn.

x86_cpu_initfn() is the earliest you have access to the CPU object, but note that the final CPUID bits (based on -cpu options, accel data, and possibly other input) are known only when x86_cpu_realizefn() is called.

Signature · 1 line
Eduardo
#7 lore raw

Hey Eduardo/Paolo,

I have not forgotten about your responses. I am working out how best to do this in our platform and will send a follow up patch (this one is already merged) to fully support the -cpu flag. It looks like all the pieces are in place between the two and we just need a bit of translation between the QEMU state at partition creation/start/execution to handle the various exits either by pre-setting the value or via the actual CPUID trap at runtime. Thank you for all your insights/input up to here. It has been really helpful. More to come.

Thanks,
Justin

Quoted 29 lines

-----Original Message-----
From: Eduardo Habkost <[email protected]>
Sent: Monday, April 16, 2018 9:33 AM
To: Paolo Bonzini <[email protected]>
Cc: Justin Terry (VM) <[email protected]>; [email protected]; [email protected]
Subject: Re: [PATCH] WHPX fixes an issue with CPUID 1 not returning CPUID_EXT_HYPERVISOR

Quoted 1 line · Paolo Bonzini

On 28/03/2018 22:48, Justin Terry (VM) wrote:

[...]

Quoted 4 lines
Quoted 4 lines

If we use [2] to inject the answers at creation time WHPX needs access to the CPUX86State at accel init which also doesn't seem to be possible in QEMU today. WHPX could basically just call cpu_x86_cpuid() for each CPUID QEMU cares about and plumb the

answer

Quoted 2 lines
Quoted 2 lines

before start. This has the best performance as we avoid the additional exits but has an issue in that the results must be known ahead

of time.

Quoted 2 lines

The earliest where you have access to that is x86_cpu_initfn.

x86_cpu_initfn() is the earliest you have access to the CPU object, but note that the final CPUID bits (based on -cpu options, accel data, and possibly other input) are known only when x86_cpu_realizefn() is called.

--
Eduardo