2022-06-14 20:49:21

by Sean Christopherson

[permalink] [raw]
Subject: [PATCH v2 03/21] KVM: x86: Don't check for code breakpoints when emulating on exception

Don't check for code breakpoints during instruction emulation if the
emulation was triggered by exception interception. Code breakpoints are
the highest priority fault-like exception, and KVM only emulates on
exceptions that are fault-like. Thus, if hardware signaled a different
exception, then the vCPU is already passed the stage of checking for
hardware breakpoints.

This is likely a glorified nop in terms of functionality, and is more for
clarification and is technically an optimization. Intel's SDM explicitly
states vmcs.GUEST_RFLAGS.RF on exception interception is the same as the
value that would have been saved on the stack had the exception not been
intercepted, i.e. will be '1' due to all fault-like exceptions setting RF
to '1'. AMD says "guest state saved ... is the processor state as of the
moment the intercept triggers", but that begs the question, "when does
the intercept trigger?".

Signed-off-by: Sean Christopherson <[email protected]>
---
arch/x86/kvm/x86.c | 21 ++++++++++++++++++---
1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 2318a99139fa..c5db31b4bd6f 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -8364,8 +8364,24 @@ int kvm_skip_emulated_instruction(struct kvm_vcpu *vcpu)
}
EXPORT_SYMBOL_GPL(kvm_skip_emulated_instruction);

-static bool kvm_vcpu_check_code_breakpoint(struct kvm_vcpu *vcpu, int *r)
+static bool kvm_vcpu_check_code_breakpoint(struct kvm_vcpu *vcpu,
+ int emulation_type, int *r)
{
+ WARN_ON_ONCE(emulation_type & EMULTYPE_NO_DECODE);
+
+ /*
+ * Do not check for code breakpoints if hardware has already done the
+ * checks, as inferred from the emulation type. On NO_DECODE and SKIP,
+ * the instruction has passed all exception checks, and all intercepted
+ * exceptions that trigger emulation have lower priority than code
+ * breakpoints, i.e. the fact that the intercepted exception occurred
+ * means any code breakpoints have already been serviced.
+ */
+ if (emulation_type & (EMULTYPE_NO_DECODE | EMULTYPE_SKIP |
+ EMULTYPE_TRAP_UD | EMULTYPE_TRAP_UD_FORCED |
+ EMULTYPE_VMWARE_GP | EMULTYPE_PF))
+ return false;
+
if (unlikely(vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP) &&
(vcpu->arch.guest_debug_dr7 & DR7_BP_EN_MASK)) {
struct kvm_run *kvm_run = vcpu->run;
@@ -8487,8 +8503,7 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
* are fault-like and are higher priority than any faults on
* the code fetch itself.
*/
- if (!(emulation_type & EMULTYPE_SKIP) &&
- kvm_vcpu_check_code_breakpoint(vcpu, &r))
+ if (kvm_vcpu_check_code_breakpoint(vcpu, emulation_type, &r))
return r;

r = x86_decode_emulated_instruction(vcpu, emulation_type,
--
2.36.1.476.g0c4daa206d-goog


2022-07-06 12:04:41

by Maxim Levitsky

[permalink] [raw]
Subject: Re: [PATCH v2 03/21] KVM: x86: Don't check for code breakpoints when emulating on exception

On Tue, 2022-06-14 at 20:47 +0000, Sean Christopherson wrote:
> Don't check for code breakpoints during instruction emulation if the
> emulation was triggered by exception interception. Code breakpoints are
> the highest priority fault-like exception, and KVM only emulates on
> exceptions that are fault-like. Thus, if hardware signaled a different
> exception, then the vCPU is already passed the stage of checking for
> hardware breakpoints.
>
> This is likely a glorified nop in terms of functionality, and is more for
> clarification and is technically an optimization. Intel's SDM explicitly
> states vmcs.GUEST_RFLAGS.RF on exception interception is the same as the
> value that would have been saved on the stack had the exception not been
> intercepted, i.e. will be '1' due to all fault-like exceptions setting RF
> to '1'. AMD says "guest state saved ... is the processor state as of the
> moment the intercept triggers", but that begs the question, "when does
> the intercept trigger?".
>
> Signed-off-by: Sean Christopherson <[email protected]>
> ---
> arch/x86/kvm/x86.c | 21 ++++++++++++++++++---
> 1 file changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 2318a99139fa..c5db31b4bd6f 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -8364,8 +8364,24 @@ int kvm_skip_emulated_instruction(struct kvm_vcpu *vcpu)
> }
> EXPORT_SYMBOL_GPL(kvm_skip_emulated_instruction);
>
> -static bool kvm_vcpu_check_code_breakpoint(struct kvm_vcpu *vcpu, int *r)
> +static bool kvm_vcpu_check_code_breakpoint(struct kvm_vcpu *vcpu,
> + int emulation_type, int *r)
> {
> + WARN_ON_ONCE(emulation_type & EMULTYPE_NO_DECODE);
> +
> + /*
> + * Do not check for code breakpoints if hardware has already done the
> + * checks, as inferred from the emulation type. On NO_DECODE and SKIP,
> + * the instruction has passed all exception checks, and all intercepted
> + * exceptions that trigger emulation have lower priority than code
> + * breakpoints, i.e. the fact that the intercepted exception occurred
> + * means any code breakpoints have already been serviced.
> + */
> + if (emulation_type & (EMULTYPE_NO_DECODE | EMULTYPE_SKIP |
> + EMULTYPE_TRAP_UD | EMULTYPE_TRAP_UD_FORCED |
> + EMULTYPE_VMWARE_GP | EMULTYPE_PF))
> + return false;
> +
> if (unlikely(vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP) &&
> (vcpu->arch.guest_debug_dr7 & DR7_BP_EN_MASK)) {
> struct kvm_run *kvm_run = vcpu->run;
> @@ -8487,8 +8503,7 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
> * are fault-like and are higher priority than any faults on
> * the code fetch itself.
> */
> - if (!(emulation_type & EMULTYPE_SKIP) &&
> - kvm_vcpu_check_code_breakpoint(vcpu, &r))
> + if (kvm_vcpu_check_code_breakpoint(vcpu, emulation_type, &r))
> return r;
>
> r = x86_decode_emulated_instruction(vcpu, emulation_type,


Reviewed-by: Maxim Levitsky <[email protected]>

Best regards,
Maxim Levitsky

2022-07-06 22:20:09

by Jim Mattson

[permalink] [raw]
Subject: Re: [PATCH v2 03/21] KVM: x86: Don't check for code breakpoints when emulating on exception

On Tue, Jun 14, 2022 at 1:47 PM Sean Christopherson <[email protected]> wrote:
>
> Don't check for code breakpoints during instruction emulation if the
> emulation was triggered by exception interception. Code breakpoints are
> the highest priority fault-like exception, and KVM only emulates on
> exceptions that are fault-like. Thus, if hardware signaled a different
> exception, then the vCPU is already passed the stage of checking for
> hardware breakpoints.
>
> This is likely a glorified nop in terms of functionality, and is more for
> clarification and is technically an optimization. Intel's SDM explicitly
> states vmcs.GUEST_RFLAGS.RF on exception interception is the same as the
> value that would have been saved on the stack had the exception not been
> intercepted, i.e. will be '1' due to all fault-like exceptions setting RF
> to '1'. AMD says "guest state saved ... is the processor state as of the
> moment the intercept triggers", but that begs the question, "when does
> the intercept trigger?".

IIRC, AMD does not prematurely clobber EFLAGS.RF on an intercepted exception.

This is actually a big deal with shadow paging. On Intel, the
hypervisor can't fully squash a #PF and restart the guest instruction
after filling in the shadow page table entry...not easily, anyway.

(OTOH, AMD does prematurely clobber DR6 and DR7 on an intercepted #DB.
So, no one should be celebrating!)