KVM: nVMX: L0 should emulate instructions for L2
diff mbox series

Message ID 20180917190720.156906-1-jmattson@google.com
State New
Headers show
Series
  • KVM: nVMX: L0 should emulate instructions for L2
Related show

Commit Message

Jim Mattson Sept. 17, 2018, 7:07 p.m. UTC
When L0 enumerates support for an instruction not supported by the
physical CPU, it must emulate that instruction for L2 as well as for
L1. Take MOVBE, for example. If hardware reports that
CPUID.1:ECX.MOVBE[bit 22] is clear, but L0 sets the bit when emulating
CPUID for L1, then L0 is obligated to intercept #UD and emulate the
MOVBE instruction for L1. Furthermore, L0 is also obligated to
intercept #UD and emulate the MOVBE instruction for L2. Even if L1
chooses to intercept #UD, L0 must be prepared to decode the
instruction and emulate MOVBE before deciding to reflect the #UD
VM-exit to L1.

Rewriting the wrong hypercall instruction with the correct hypercall
instruction is not as clear-cut. One could argue that this is a
property of a kvm virtual CPU, and that L0 should always do
it. However, to avoid surprises, I've preserved the existing behavior
(right or wrong).

Signed-off-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/kvm/vmx.c | 2 ++
 arch/x86/kvm/x86.c | 6 ++++++
 2 files changed, 8 insertions(+)

Comments

Jim Mattson Sept. 19, 2018, 4:19 p.m. UTC | #1
On Mon, Sep 17, 2018 at 12:07 PM, Jim Mattson <jmattson@google.com> wrote:
> When L0 enumerates support for an instruction not supported by the
> physical CPU, it must emulate that instruction for L2 as well as for
> L1. Take MOVBE, for example. If hardware reports that
> CPUID.1:ECX.MOVBE[bit 22] is clear, but L0 sets the bit when emulating
> CPUID for L1, then L0 is obligated to intercept #UD and emulate the
> MOVBE instruction for L1. Furthermore, L0 is also obligated to
> intercept #UD and emulate the MOVBE instruction for L2. Even if L1
> chooses to intercept #UD, L0 must be prepared to decode the
> instruction and emulate MOVBE before deciding to reflect the #UD
> VM-exit to L1.
>
> Rewriting the wrong hypercall instruction with the correct hypercall
> instruction is not as clear-cut. One could argue that this is a
> property of a kvm virtual CPU, and that L0 should always do
> it. However, to avoid surprises, I've preserved the existing behavior
> (right or wrong).

Actually, I haven't completely preserved the existing behavior.

Currently, if L1 intercepts #UD, then the wrong hypercall instruction
will result in the #UD VM-exit being reflected to L1. However, if L1
does not intercept #UD, then L0 will try to rewrite the hypercall
instruction with the correct one. It will try, but it will most likely
fail, since the ewrite operation obeys the guest's page protections. A
failure will result in a synthesized #PF being delivered to L2.

With this change, if L1 doesn't intercept #UD, a synthesized #UD is
delivered to L2. L0 will never try to rewrite the wrong hypercall
instruction in L2, regardless of whether or not L1 intercepts #UD.
Jim Mattson Oct. 12, 2018, 4:41 p.m. UTC | #2
On Mon, Sep 17, 2018 at 12:07 PM, Jim Mattson <jmattson@google.com> wrote:
> When L0 enumerates support for an instruction not supported by the
> physical CPU, it must emulate that instruction for L2 as well as for
> L1. Take MOVBE, for example. If hardware reports that
> CPUID.1:ECX.MOVBE[bit 22] is clear, but L0 sets the bit when emulating
> CPUID for L1, then L0 is obligated to intercept #UD and emulate the
> MOVBE instruction for L1. Furthermore, L0 is also obligated to
> intercept #UD and emulate the MOVBE instruction for L2. Even if L1
> chooses to intercept #UD, L0 must be prepared to decode the
> instruction and emulate MOVBE before deciding to reflect the #UD
> VM-exit to L1.
>
> Rewriting the wrong hypercall instruction with the correct hypercall
> instruction is not as clear-cut. One could argue that this is a
> property of a kvm virtual CPU, and that L0 should always do
> it. However, to avoid surprises, I've preserved the existing behavior
> (right or wrong).
>
> Signed-off-by: Jim Mattson <jmattson@google.com>
> ---
>  arch/x86/kvm/vmx.c | 2 ++
>  arch/x86/kvm/x86.c | 6 ++++++
>  2 files changed, 8 insertions(+)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 533a327372c8..c9d8ab3ce56e 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -9649,6 +9649,8 @@ static bool nested_vmx_exit_reflected(struct kvm_vcpu *vcpu, u32 exit_reason)
>                 else if (is_breakpoint(intr_info) &&
>                          vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP)
>                         return false;
> +               else if (is_invalid_opcode(intr_info))
> +                       return false;
>                 return vmcs12->exception_bitmap &
>                                 (1u << (intr_info & INTR_INFO_VECTOR_MASK));
>         case EXIT_REASON_EXTERNAL_INTERRUPT:
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 542f6315444d..212067531050 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -6876,6 +6876,12 @@ static int emulator_fix_hypercall(struct x86_emulate_ctxt *ctxt)
>         char instruction[3];
>         unsigned long rip = kvm_rip_read(vcpu);
>
> +       if (is_guest_mode(vcpu)) {
> +               ctxt->exception.vector = UD_VECTOR;
> +               ctxt->exception.error_code_valid = false;
> +               return X86EMUL_PROPAGATE_FAULT;
> +       }
> +
>         kvm_x86_ops->patch_hypercall(vcpu, instruction);
>
>         return emulator_write_emulated(ctxt, rip, instruction, 3,
> --
> 2.19.0.397.gdd90340f6a-goog
>

Ping?

Patch
diff mbox series

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 533a327372c8..c9d8ab3ce56e 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -9649,6 +9649,8 @@  static bool nested_vmx_exit_reflected(struct kvm_vcpu *vcpu, u32 exit_reason)
 		else if (is_breakpoint(intr_info) &&
 			 vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP)
 			return false;
+		else if (is_invalid_opcode(intr_info))
+			return false;
 		return vmcs12->exception_bitmap &
 				(1u << (intr_info & INTR_INFO_VECTOR_MASK));
 	case EXIT_REASON_EXTERNAL_INTERRUPT:
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 542f6315444d..212067531050 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -6876,6 +6876,12 @@  static int emulator_fix_hypercall(struct x86_emulate_ctxt *ctxt)
 	char instruction[3];
 	unsigned long rip = kvm_rip_read(vcpu);
 
+	if (is_guest_mode(vcpu)) {
+		ctxt->exception.vector = UD_VECTOR;
+		ctxt->exception.error_code_valid = false;
+		return X86EMUL_PROPAGATE_FAULT;
+	}
+
 	kvm_x86_ops->patch_hypercall(vcpu, instruction);
 
 	return emulator_write_emulated(ctxt, rip, instruction, 3,