On 8/2/23 10:04, Sean Christopherson wrote:
On Wed, Aug 02, 2023, Tom Lendacky wrote:
On 8/2/23 09:25, Tom Lendacky wrote:
On 8/2/23 09:01, Sean Christopherson wrote:
You're right. The #UD is injected by KVM.
The path I found is:
svm_vcpu_run
svm_complete_interrupts
kvm_requeue_exception // vector = 3
kvm_make_request
vcpu_enter_guest
kvm_check_and_inject_events
svm_inject_exception
svm_update_soft_interrupt_rip
__svm_skip_emulated_instruction
x86_emulate_instruction
svm_can_emulate_instruction
kvm_queue_exception(vcpu, UD_VECTOR)
Does this mean a #PF intercept occur when the guest try to deliver a
#BP through the IDT? But why?
I doubt it's a #PF. A #NPF is much more likely, though it could be
something
else entirely, but I'm pretty sure that would require bugs in both
the host and
guest.
What is the last exit recorded by trace_kvm_exit() before the #UD is
injected?
I'm guessing it was a #NPF, too. Could it be related to the changes that
went in around svm_update_soft_interrupt_rip()?
6ef88d6e36c2 ("KVM: SVM: Re-inject INT3/INTO instead of retrying the
instruction")
Sorry, that should have been:
7e5b5ef8dca3 ("KVM: SVM: Re-inject INTn instead of retrying the insn on "failure"")
Before this the !nrips check would prevent the call into
svm_skip_emulated_instruction(). But now, there is a call to:
svm_update_soft_interrupt_rip()
__svm_skip_emulated_instruction()
kvm_emulate_instruction()
x86_emulate_instruction() (passed a NULL insn pointer)
kvm_can_emulate_insn() (passed a NULL insn pointer)
svm_can_emulate_instruction() (passed NULL insn pointer)
Because it is an SEV guest, it ends up in the "if (unlikely(!insn))" path
and injects the #UD.
Yeah, my money is on that too. I believe this is the least awful solution:
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index d381ad424554..2eace114a934 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -385,6 +385,9 @@ static int __svm_skip_emulated_instruction(struct kvm_vcpu *vcpu,
}
if (!svm->next_rip) {
+ if (sev_guest(vcpu->kvm))
+ return 0;
+
if (unlikely(!commit_side_effects))
old_rflags = svm->vmcb->save.rflags;
I'll send a formal patch (with a comment) if that solves the problem.
Side topic, KVM should require nrips for SEV and beyond, I don't see how SEV can
possibly work if KVM doesn't utilize nrips. E.g. this
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 2eace114a934..43e500503d48 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -5111,9 +5111,11 @@ static __init int svm_hardware_setup(void)
svm_adjust_mmio_mask();
+ nrips = nrips && boot_cpu_has(X86_FEATURE_NRIPS);
+
/*
* Note, SEV setup consumes npt_enabled and enable_mmio_caching (which
- * may be modified by svm_adjust_mmio_mask()).
+ * may be modified by svm_adjust_mmio_mask()), as well as nrips.
*/
sev_hardware_setup();
You moved the setting of nrips up, I'm assuming you then want to add a
check in sev_hardware_setup() for nrips?
Thanks,
Tom
@@ -5125,11 +5127,6 @@ static __init int svm_hardware_setup(void)
goto err;
}
- if (nrips) {
- if (!boot_cpu_has(X86_FEATURE_NRIPS))
- nrips = false;
- }
-
enable_apicv = avic = avic && avic_hardware_setup();
if (!enable_apicv) {
