| Message ID | 20180124151234.32329-1-vkuznets@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2018-01-24 23:12 GMT+08:00 Vitaly Kuznetsov <vkuznets@redhat.com>: > I was investigating an issue with seabios >= 1.10 which stopped working > for nested KVM on Hyper-V. The problem appears to be in > handle_ept_violation() function: when we do fast mmio we need to skip > the instruction so we do kvm_skip_emulated_instruction(). This, however, > depends on VM_EXIT_INSTRUCTION_LEN field being set correctly in VMCS. > However, this is not the case. > > Intel's manual doesn't mandate VM_EXIT_INSTRUCTION_LEN to be set when > EPT MISCONFIG occurs. While on real hardware it was observed to be set, > some hypervisors follow the spec and don't set it; we end up advancing > IP with some random value. > > I checked with Microsoft and they confirmed they don't fill > VM_EXIT_INSTRUCTION_LEN on EPT MISCONFIG. > > Fix the issue by disabling fast mmio when running nested. > > Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> Reviewed-by: Wanpeng Li <wanpeng.li@hotmail.com> > --- > arch/x86/kvm/vmx.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index c829d89e2e63..54afb446f38e 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -6558,9 +6558,16 @@ static int handle_ept_misconfig(struct kvm_vcpu *vcpu) > /* > * A nested guest cannot optimize MMIO vmexits, because we have an > * nGPA here instead of the required GPA. > + * Skipping instruction below depends on undefined behavior: Intel's > + * manual doesn't mandate VM_EXIT_INSTRUCTION_LEN to be set in VMCS > + * when EPT MISCONFIG occurs and while on real hardware it was observed > + * to be set, other hypervisors (namely Hyper-V) don't set it, we end > + * up advancing IP with some random value. Disable fast mmio when > + * running nested and keep it for real hardware in hope that > + * VM_EXIT_INSTRUCTION_LEN will always be set correctly. > */ > gpa = vmcs_read64(GUEST_PHYSICAL_ADDRESS); > - if (!is_guest_mode(vcpu) && > + if (!static_cpu_has(X86_FEATURE_HYPERVISOR) && !is_guest_mode(vcpu) && > !kvm_io_bus_write(vcpu, KVM_FAST_MMIO_BUS, gpa, 0, NULL)) { > trace_kvm_fast_mmio(gpa); > return kvm_skip_emulated_instruction(vcpu); > -- > 2.14.3 >
2018-01-24 16:12+0100, Vitaly Kuznetsov: > I was investigating an issue with seabios >= 1.10 which stopped working > for nested KVM on Hyper-V. The problem appears to be in > handle_ept_violation() function: when we do fast mmio we need to skip > the instruction so we do kvm_skip_emulated_instruction(). This, however, > depends on VM_EXIT_INSTRUCTION_LEN field being set correctly in VMCS. > However, this is not the case. > > Intel's manual doesn't mandate VM_EXIT_INSTRUCTION_LEN to be set when > EPT MISCONFIG occurs. While on real hardware it was observed to be set, > some hypervisors follow the spec and don't set it; we end up advancing > IP with some random value. > > I checked with Microsoft and they confirmed they don't fill > VM_EXIT_INSTRUCTION_LEN on EPT MISCONFIG. > > Fix the issue by disabling fast mmio when running nested. > > Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> > --- > arch/x86/kvm/vmx.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index c829d89e2e63..54afb446f38e 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -6558,9 +6558,16 @@ static int handle_ept_misconfig(struct kvm_vcpu *vcpu) > /* > * A nested guest cannot optimize MMIO vmexits, because we have an > * nGPA here instead of the required GPA. > + * Skipping instruction below depends on undefined behavior: Intel's > + * manual doesn't mandate VM_EXIT_INSTRUCTION_LEN to be set in VMCS > + * when EPT MISCONFIG occurs and while on real hardware it was observed > + * to be set, other hypervisors (namely Hyper-V) don't set it, we end > + * up advancing IP with some random value. Disable fast mmio when > + * running nested and keep it for real hardware in hope that > + * VM_EXIT_INSTRUCTION_LEN will always be set correctly. > */ > gpa = vmcs_read64(GUEST_PHYSICAL_ADDRESS); > - if (!is_guest_mode(vcpu) && > + if (!static_cpu_has(X86_FEATURE_HYPERVISOR) && !is_guest_mode(vcpu) && I realized that Paolo kept a minor optimization while getting rid of the undefined behavior (https://patchwork.kernel.org/patch/9903811/). Please do the same trick that signals kvm_io_bus_write() before going to x86_emulate_instruction(... EMULTYPE_SKIP ...), but add a branch to use kvm_skip_emulated_instruction() for bare-metal, thanks. > !kvm_io_bus_write(vcpu, KVM_FAST_MMIO_BUS, gpa, 0, NULL)) { > trace_kvm_fast_mmio(gpa); > return kvm_skip_emulated_instruction(vcpu); > -- > 2.14.3 >
----- Original Message ----- > From: "Vitaly Kuznetsov" <vkuznets@redhat.com> > To: kvm@vger.kernel.org > Cc: x86@kernel.org, linux-kernel@vger.kernel.org, "Paolo Bonzini" <pbonzini@redhat.com>, "Radim Krčmář" > <rkrcmar@redhat.com> > Sent: Wednesday, January 24, 2018 4:12:34 PM > Subject: [PATCH] x86/kvm: disable fast MMIO when running nested > > I was investigating an issue with seabios >= 1.10 which stopped working > for nested KVM on Hyper-V. The problem appears to be in > handle_ept_violation() function: when we do fast mmio we need to skip > the instruction so we do kvm_skip_emulated_instruction(). This, however, > depends on VM_EXIT_INSTRUCTION_LEN field being set correctly in VMCS. > However, this is not the case. > > Intel's manual doesn't mandate VM_EXIT_INSTRUCTION_LEN to be set when > EPT MISCONFIG occurs. While on real hardware it was observed to be set, > some hypervisors follow the spec and don't set it; we end up advancing > IP with some random value. > > I checked with Microsoft and they confirmed they don't fill > VM_EXIT_INSTRUCTION_LEN on EPT MISCONFIG. > > Fix the issue by disabling fast mmio when running nested. > > Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> > --- > arch/x86/kvm/vmx.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index c829d89e2e63..54afb446f38e 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -6558,9 +6558,16 @@ static int handle_ept_misconfig(struct kvm_vcpu *vcpu) > /* > * A nested guest cannot optimize MMIO vmexits, because we have an > * nGPA here instead of the required GPA. > + * Skipping instruction below depends on undefined behavior: Intel's > + * manual doesn't mandate VM_EXIT_INSTRUCTION_LEN to be set in VMCS > + * when EPT MISCONFIG occurs and while on real hardware it was observed > + * to be set, other hypervisors (namely Hyper-V) don't set it, we end > + * up advancing IP with some random value. Disable fast mmio when > + * running nested and keep it for real hardware in hope that > + * VM_EXIT_INSTRUCTION_LEN will always be set correctly. > */ > gpa = vmcs_read64(GUEST_PHYSICAL_ADDRESS); > - if (!is_guest_mode(vcpu) && > + if (!static_cpu_has(X86_FEATURE_HYPERVISOR) && !is_guest_mode(vcpu) && > !kvm_io_bus_write(vcpu, KVM_FAST_MMIO_BUS, gpa, 0, NULL)) { > trace_kvm_fast_mmio(gpa); > return kvm_skip_emulated_instruction(vcpu); > -- > 2.14.3 Vitaly, could you base the X86_FEATURE_HYPERVISOR case on the patch at https://patchwork.kernel.org/patch/9903811/? By using EMULTYPE_SKIP, the eventfd is signaled before entering the emulator and the impact on latency is small. Thanks, Paolo
Paolo Bonzini <pbonzini@redhat.com> writes: > ----- Original Message ----- >> From: "Vitaly Kuznetsov" <vkuznets@redhat.com> >> To: kvm@vger.kernel.org >> Cc: x86@kernel.org, linux-kernel@vger.kernel.org, "Paolo Bonzini" <pbonzini@redhat.com>, "Radim Krčmář" >> <rkrcmar@redhat.com> >> Sent: Wednesday, January 24, 2018 4:12:34 PM >> Subject: [PATCH] x86/kvm: disable fast MMIO when running nested >> >> I was investigating an issue with seabios >= 1.10 which stopped working >> for nested KVM on Hyper-V. The problem appears to be in >> handle_ept_violation() function: when we do fast mmio we need to skip >> the instruction so we do kvm_skip_emulated_instruction(). This, however, >> depends on VM_EXIT_INSTRUCTION_LEN field being set correctly in VMCS. >> However, this is not the case. >> >> Intel's manual doesn't mandate VM_EXIT_INSTRUCTION_LEN to be set when >> EPT MISCONFIG occurs. While on real hardware it was observed to be set, >> some hypervisors follow the spec and don't set it; we end up advancing >> IP with some random value. >> >> I checked with Microsoft and they confirmed they don't fill >> VM_EXIT_INSTRUCTION_LEN on EPT MISCONFIG. >> >> Fix the issue by disabling fast mmio when running nested. >> >> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> >> --- >> arch/x86/kvm/vmx.c | 9 ++++++++- >> 1 file changed, 8 insertions(+), 1 deletion(-) >> >> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >> index c829d89e2e63..54afb446f38e 100644 >> --- a/arch/x86/kvm/vmx.c >> +++ b/arch/x86/kvm/vmx.c >> @@ -6558,9 +6558,16 @@ static int handle_ept_misconfig(struct kvm_vcpu *vcpu) >> /* >> * A nested guest cannot optimize MMIO vmexits, because we have an >> * nGPA here instead of the required GPA. >> + * Skipping instruction below depends on undefined behavior: Intel's >> + * manual doesn't mandate VM_EXIT_INSTRUCTION_LEN to be set in VMCS >> + * when EPT MISCONFIG occurs and while on real hardware it was observed >> + * to be set, other hypervisors (namely Hyper-V) don't set it, we end >> + * up advancing IP with some random value. Disable fast mmio when >> + * running nested and keep it for real hardware in hope that >> + * VM_EXIT_INSTRUCTION_LEN will always be set correctly. >> */ >> gpa = vmcs_read64(GUEST_PHYSICAL_ADDRESS); >> - if (!is_guest_mode(vcpu) && >> + if (!static_cpu_has(X86_FEATURE_HYPERVISOR) && !is_guest_mode(vcpu) && >> !kvm_io_bus_write(vcpu, KVM_FAST_MMIO_BUS, gpa, 0, NULL)) { >> trace_kvm_fast_mmio(gpa); >> return kvm_skip_emulated_instruction(vcpu); >> -- >> 2.14.3 > > Vitaly, > > could you base the X86_FEATURE_HYPERVISOR case on the patch at > https://patchwork.kernel.org/patch/9903811/? > > By using EMULTYPE_SKIP, the eventfd is signaled before entering the > emulator and the impact on latency is small. > Oh, I didn't know there was a story! I'll try EMULTYPE_SKIP, thanks!
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index c829d89e2e63..54afb446f38e 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -6558,9 +6558,16 @@ static int handle_ept_misconfig(struct kvm_vcpu *vcpu) /* * A nested guest cannot optimize MMIO vmexits, because we have an * nGPA here instead of the required GPA. + * Skipping instruction below depends on undefined behavior: Intel's + * manual doesn't mandate VM_EXIT_INSTRUCTION_LEN to be set in VMCS + * when EPT MISCONFIG occurs and while on real hardware it was observed + * to be set, other hypervisors (namely Hyper-V) don't set it, we end + * up advancing IP with some random value. Disable fast mmio when + * running nested and keep it for real hardware in hope that + * VM_EXIT_INSTRUCTION_LEN will always be set correctly. */ gpa = vmcs_read64(GUEST_PHYSICAL_ADDRESS); - if (!is_guest_mode(vcpu) && + if (!static_cpu_has(X86_FEATURE_HYPERVISOR) && !is_guest_mode(vcpu) && !kvm_io_bus_write(vcpu, KVM_FAST_MMIO_BUS, gpa, 0, NULL)) { trace_kvm_fast_mmio(gpa); return kvm_skip_emulated_instruction(vcpu);
I was investigating an issue with seabios >= 1.10 which stopped working for nested KVM on Hyper-V. The problem appears to be in handle_ept_violation() function: when we do fast mmio we need to skip the instruction so we do kvm_skip_emulated_instruction(). This, however, depends on VM_EXIT_INSTRUCTION_LEN field being set correctly in VMCS. However, this is not the case. Intel's manual doesn't mandate VM_EXIT_INSTRUCTION_LEN to be set when EPT MISCONFIG occurs. While on real hardware it was observed to be set, some hypervisors follow the spec and don't set it; we end up advancing IP with some random value. I checked with Microsoft and they confirmed they don't fill VM_EXIT_INSTRUCTION_LEN on EPT MISCONFIG. Fix the issue by disabling fast mmio when running nested. Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> --- arch/x86/kvm/vmx.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)