All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 1/2] WHPX workaround bug in OSVW handling
@ 2018-06-05 22:15 Justin Terry (VM)
  2018-06-05 22:15 ` [Qemu-devel] [PATCH 2/2] WHPX: register for unrecognized MSR exits Justin Terry (VM)
  0 siblings, 1 reply; 5+ messages in thread
From: Justin Terry (VM) @ 2018-06-05 22:15 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, rth, ehabkost, Justin Terry (VM)

Adds a workaround to an incorrect value setting
CPUID Fn8000_0001_ECX[bit 9 OSVW] = 1. This can cause a guest linux kernel
to panic when an issue to rdmsr C001_0140h returns 0. Disabling this feature
correctly allows the guest to boot without accessing the osv workarounds.

Signed-off-by: Justin Terry (VM) <juterry@microsoft.com>
---
 target/i386/whpx-all.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/target/i386/whpx-all.c b/target/i386/whpx-all.c
index 6b42096698..99501bac57 100644
--- a/target/i386/whpx-all.c
+++ b/target/i386/whpx-all.c
@@ -964,6 +964,16 @@ static int whpx_vcpu_run(CPUState *cpu)
                 rdx = vcpu->exit_ctx.CpuidAccess.DefaultResultRdx;
                 rbx = vcpu->exit_ctx.CpuidAccess.DefaultResultRbx;
                 break;
+            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;
+                break;
             default:
                 rax = vcpu->exit_ctx.CpuidAccess.DefaultResultRax;
                 rcx = vcpu->exit_ctx.CpuidAccess.DefaultResultRcx;
@@ -1382,12 +1392,13 @@ static int whpx_accel_init(MachineState *ms)
         goto error;
     }
 
-    UINT32 cpuidExitList[] = {1};
+    UINT32 cpuidExitList[] = {1, 0x80000001};
     hr = whp_dispatch.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);
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [Qemu-devel] [PATCH 2/2] WHPX: register for unrecognized MSR exits
  2018-06-05 22:15 [Qemu-devel] [PATCH 1/2] WHPX workaround bug in OSVW handling Justin Terry (VM)
@ 2018-06-05 22:15 ` Justin Terry (VM)
  2018-06-13 16:27   ` Paolo Bonzini
  0 siblings, 1 reply; 5+ messages in thread
From: Justin Terry (VM) @ 2018-06-05 22:15 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, rth, ehabkost, Justin Terry (VM)

Some variations of Linux kernels end up accessing MSR's that the Windows
Hypervisor doesn't implement which causes a GP to be returned to the guest.
This fix registers QEMU for unimplemented MSR access and globally returns 0 on
reads and ignores writes. This behavior is allows the Linux kernel to probe the
MSR with a write/read/check sequence it does often without failing the access.

Signed-off-by: Justin Terry (VM) <juterry@microsoft.com>
---
 target/i386/whpx-all.c | 41 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 38 insertions(+), 3 deletions(-)

diff --git a/target/i386/whpx-all.c b/target/i386/whpx-all.c
index 99501bac57..57e53e1f1f 100644
--- a/target/i386/whpx-all.c
+++ b/target/i386/whpx-all.c
@@ -932,6 +932,7 @@ static int whpx_vcpu_run(CPUState *cpu)
 
         case WHvRunVpExitReasonX64InterruptWindow:
             vcpu->window_registered = 0;
+            ret = 0;
             break;
 
         case WHvRunVpExitReasonX64Halt:
@@ -943,6 +944,40 @@ static int whpx_vcpu_run(CPUState *cpu)
             ret = 1;
             break;
 
+        case WHvRunVpExitReasonX64MsrAccess: {
+            WHV_REGISTER_VALUE reg_values[3] = {0};
+            WHV_REGISTER_NAME reg_names[3];
+            UINT32 reg_count;
+
+            reg_names[0] = WHvX64RegisterRip;
+            reg_names[1] = WHvX64RegisterRax;
+            reg_names[2] = WHvX64RegisterRdx;
+
+            reg_values[0].Reg64 =
+                vcpu->exit_ctx.VpContext.Rip +
+                vcpu->exit_ctx.VpContext.InstructionLength;
+
+            /*
+             * For all unsupported MSR access we:
+             *     ignore writes
+             *     return 0 on read.
+             */
+            reg_count = vcpu->exit_ctx.MsrAccess.AccessInfo.IsWrite ?
+                        1 : 3;
+
+            hr = whp_dispatch.WHvSetVirtualProcessorRegisters(
+                whpx->partition,
+                cpu->cpu_index,
+                reg_names, reg_count,
+                reg_values);
+
+            if (FAILED(hr)) {
+                error_report("WHPX: Failed to set MsrAccess state "
+                             " registers, hr=%08lx", hr);
+            }
+            ret = 0;
+            break;
+        }
         case WHvRunVpExitReasonX64Cpuid: {
             WHV_REGISTER_VALUE reg_values[5];
             WHV_REGISTER_NAME reg_names[5];
@@ -1010,7 +1045,6 @@ static int whpx_vcpu_run(CPUState *cpu)
         case WHvRunVpExitReasonUnrecoverableException:
         case WHvRunVpExitReasonInvalidVpRegisterValue:
         case WHvRunVpExitReasonUnsupportedFeature:
-        case WHvRunVpExitReasonX64MsrAccess:
         case WHvRunVpExitReasonException:
         default:
             error_report("WHPX: Unexpected VP exit code %d",
@@ -1378,6 +1412,7 @@ static int whpx_accel_init(MachineState *ms)
     }
 
     memset(&prop, 0, sizeof(WHV_PARTITION_PROPERTY));
+    prop.ExtendedVmExits.X64MsrExit = 1;
     prop.ExtendedVmExits.X64CpuidExit = 1;
     hr = whp_dispatch.WHvSetPartitionProperty(
         whpx->partition,
@@ -1386,8 +1421,8 @@ static int whpx_accel_init(MachineState *ms)
         sizeof(WHV_PARTITION_PROPERTY));
 
     if (FAILED(hr)) {
-        error_report("WHPX: Failed to enable partition extended X64CpuidExit"
-                     " hr=%08lx", hr);
+        error_report("WHPX: Failed to enable partition extended X64MsrExit and"
+                     " X64CpuidExit hr=%08lx", hr);
         ret = -EINVAL;
         goto error;
     }
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] WHPX: register for unrecognized MSR exits
  2018-06-05 22:15 ` [Qemu-devel] [PATCH 2/2] WHPX: register for unrecognized MSR exits Justin Terry (VM)
@ 2018-06-13 16:27   ` Paolo Bonzini
  2018-06-18 22:01     ` Justin Terry (VM)
  0 siblings, 1 reply; 5+ messages in thread
From: Paolo Bonzini @ 2018-06-13 16:27 UTC (permalink / raw)
  To: Justin Terry (VM), qemu-devel; +Cc: rth, ehabkost

On 06/06/2018 00:15, Justin Terry (VM) wrote:
> Some variations of Linux kernels end up accessing MSR's that the Windows
> Hypervisor doesn't implement which causes a GP to be returned to the guest.
> This fix registers QEMU for unimplemented MSR access and globally returns 0 on
> reads and ignores writes. This behavior is allows the Linux kernel to probe the
> MSR with a write/read/check sequence it does often without failing the access.
> 
> Signed-off-by: Justin Terry (VM) <juterry@microsoft.com>
> ---
>  target/i386/whpx-all.c | 41 ++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 38 insertions(+), 3 deletions(-)

Hmm, KVM tries to list the MSRs that Linux (or Windows :)) use.  It can
do the full whitelist, but it's opt-in.

Recent Linux kernels also are generally less picky about #GPs from MSRs,
so I don't think a generic whitelist is a good idea.  If the
"non-hosted" Hyper-V is doing the same that would be fine I guess, but
then there should probably be a comment about it in the code.

While this is discussed a bit more, I've queued patch 1.

Thanks,

Paolo

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] WHPX: register for unrecognized MSR exits
  2018-06-13 16:27   ` Paolo Bonzini
@ 2018-06-18 22:01     ` Justin Terry (VM)
  2018-06-19 12:30       ` Paolo Bonzini
  0 siblings, 1 reply; 5+ messages in thread
From: Justin Terry (VM) @ 2018-06-18 22:01 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: rth, ehabkost

Hey Paolo,

Thanks for the reply.

I am certainly open for suggestions if you have any here. This was originally reported when running the android kernel which I believe is Linux 4.4. I agree that newer kernels do seem to handle #GP more gracefully than others but it doesn’t help down level kernels to boot.

The issue is that the Windows Hypervisor Platform will return #GP for any rdmsr/wrmsr that is not a virtualized MSR in the hypervisor by default. A virt stack (QEMU) can override this default behavior by registering for MSR exits. In this configuration the virt stack will receive any non-virtualized MSR exit and from the hypervisors perspective this is now effectively handled. I could certainly list each MSR individually but it seems overly prone to test-matrix errors. For example, each kernel that I try might succeed but a single option difference in another kernel might fail (which was the case here because the 4.4 kernel is different from the 4.14+ it seems). But, I will make any change you see fit to handle this in the way QEMU prefers.

As FYI, this is the same logic that Hyper-V uses in its virt stack for any hypervisor exits that are not handled in the hypervisor itself.

-Justin

> -----Original Message-----
> From: Paolo Bonzini <pbonzini@redhat.com>
> Sent: Wednesday, June 13, 2018 9:28 AM
> To: Justin Terry (VM) <juterry@microsoft.com>; qemu-devel@nongnu.org
> Cc: rth@twiddle.net; ehabkost@redhat.com
> Subject: Re: [PATCH 2/2] WHPX: register for unrecognized MSR exits
> 
> On 06/06/2018 00:15, Justin Terry (VM) wrote:
> > Some variations of Linux kernels end up accessing MSR's that the
> > Windows Hypervisor doesn't implement which causes a GP to be returned
> to the guest.
> > This fix registers QEMU for unimplemented MSR access and globally
> > returns 0 on reads and ignores writes. This behavior is allows the
> > Linux kernel to probe the MSR with a write/read/check sequence it does
> often without failing the access.
> >
> > Signed-off-by: Justin Terry (VM) <juterry@microsoft.com>
> > ---
> >  target/i386/whpx-all.c | 41
> ++++++++++++++++++++++++++++++++++++++---
> >  1 file changed, 38 insertions(+), 3 deletions(-)
> 
> Hmm, KVM tries to list the MSRs that Linux (or Windows :)) use.  It can do the
> full whitelist, but it's opt-in.
> 
> Recent Linux kernels also are generally less picky about #GPs from MSRs, so I
> don't think a generic whitelist is a good idea.  If the "non-hosted" Hyper-V is
> doing the same that would be fine I guess, but then there should probably
> be a comment about it in the code.
> 
> While this is discussed a bit more, I've queued patch 1.
> 
> Thanks,
> 
> Paolo

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] WHPX: register for unrecognized MSR exits
  2018-06-18 22:01     ` Justin Terry (VM)
@ 2018-06-19 12:30       ` Paolo Bonzini
  0 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2018-06-19 12:30 UTC (permalink / raw)
  To: Justin Terry (VM), qemu-devel; +Cc: rth, ehabkost

On 19/06/2018 00:01, Justin Terry (VM) wrote:
> The issue is that the Windows Hypervisor Platform will return #GP for
> any rdmsr/wrmsr that is not a virtualized MSR in the hypervisor by
> default. A virt stack (QEMU) can override this default behavior by
> registering for MSR exits. In this configuration the virt stack will
> receive any non-virtualized MSR exit and from the hypervisors
> perspective this is now effectively handled. I could certainly list
> each MSR individually but it seems overly prone to test-matrix
> errors. For example, each kernel that I try might succeed but a
> single option difference in another kernel might fail (which was the
> case here because the 4.4 kernel is different from the 4.14+ it
> seems). But, I will make any change you see fit to handle this in the
> way QEMU prefers.
> 
> As FYI, this is the same logic that Hyper-V uses in its virt stack
> for any hypervisor exits that are not handled in the hypervisor
> itself.

If by "the same logic" you mean that Hyper-V does
read-as-zero/write-to-nowhere, then the patch is okay with a comment
that says so.

Thanks,

Paolo

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-06-19 12:30 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-05 22:15 [Qemu-devel] [PATCH 1/2] WHPX workaround bug in OSVW handling Justin Terry (VM)
2018-06-05 22:15 ` [Qemu-devel] [PATCH 2/2] WHPX: register for unrecognized MSR exits Justin Terry (VM)
2018-06-13 16:27   ` Paolo Bonzini
2018-06-18 22:01     ` Justin Terry (VM)
2018-06-19 12:30       ` Paolo Bonzini

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.