Currently, the unhandleable vectoring (e.g. when guest accesses MMIO during vectoring) is handled differently on VMX and SVM: on VMX KVM returns internal error, when SVM goes into infinite loop trying to deliver an event again and again.
This patch series eliminates this difference by returning a KVM internal error when KVM can't emulate during vectoring for both VMX and SVM.
Also, introduce a selftest test case which covers the error handling mentioned above.
V1 -> V2: - Make commit messages more brief, avoid using pronouns - Extract SVM error handling into a separate commit - Introduce a new X86EMUL_ return type and detect the unhandleable vectoring error in vendor-specific check_emulate_instruction instead of handling it in the common MMU code (which is specific for cached MMIO)
V2 -> V3: - Make the new X86EMUL_ code more generic - Prohibit any emulation during vectoring if it is due to an intercepted #PF - Add a new patch for checking whether unprotect & retry is possible before exiting to userspace due to unhandleable vectoring - Codestyle fixes
Ivan Orlov (7): KVM: x86: Add function for vectoring error generation KVM: x86: Add emulation status for unhandleable vectoring KVM: x86: Unprotect & retry before unhandleable vectoring check KVM: VMX: Handle vectoring error in check_emulate_instruction KVM: SVM: Handle vectoring error in check_emulate_instruction selftests: KVM: extract lidt into helper function selftests: KVM: Add test case for MMIO during vectoring
arch/x86/include/asm/kvm_host.h | 11 +++- arch/x86/kvm/kvm_emulate.h | 2 + arch/x86/kvm/svm/svm.c | 6 +++ arch/x86/kvm/vmx/vmx.c | 30 ++++------- arch/x86/kvm/x86.c | 31 +++++++++++ .../selftests/kvm/include/x86_64/processor.h | 7 +++ .../selftests/kvm/set_memory_region_test.c | 53 ++++++++++++++++++- .../selftests/kvm/x86_64/sev_smoke_test.c | 2 +- 8 files changed, 117 insertions(+), 25 deletions(-)
Extract VMX code for unhandleable VM-Exit during vectoring into vendor-agnostic function so that boiler-plate code can be shared by SVM.
Report an actual GPA for EPT misconfig or invalid GPA for any other exit code in internal.data[3].
Signed-off-by: Ivan Orlov iorlov@amazon.com --- V1 -> V2: - Return GPA for any exit reason, using reported GPA when it is valid or INVALID_GPA otherwise. - Rename the error preparation function - Fix indentation V2 -> V3: - Move 'gpa' declaration into block it is used in - Use 'if' instead of ternary operator when reporting GPA - Remove redundant cast
arch/x86/include/asm/kvm_host.h | 2 ++ arch/x86/kvm/vmx/vmx.c | 17 +++++------------ arch/x86/kvm/x86.c | 22 ++++++++++++++++++++++ 3 files changed, 29 insertions(+), 12 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index e159e44a6a1b..de8fb1ab230c 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -2062,6 +2062,8 @@ void __kvm_prepare_emulation_failure_exit(struct kvm_vcpu *vcpu, u64 *data, u8 ndata); void kvm_prepare_emulation_failure_exit(struct kvm_vcpu *vcpu);
+void kvm_prepare_event_vectoring_exit(struct kvm_vcpu *vcpu, gpa_t gpa); + void kvm_enable_efer_bits(u64); bool kvm_valid_efer(struct kvm_vcpu *vcpu, u64 efer); int kvm_get_msr_with_filter(struct kvm_vcpu *vcpu, u32 index, u64 *data); diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index 893366e53732..acc2f0e0a339 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -6557,19 +6557,12 @@ static int __vmx_handle_exit(struct kvm_vcpu *vcpu, fastpath_t exit_fastpath) exit_reason.basic != EXIT_REASON_APIC_ACCESS && exit_reason.basic != EXIT_REASON_TASK_SWITCH && exit_reason.basic != EXIT_REASON_NOTIFY)) { - int ndata = 3; + gpa_t gpa = INVALID_GPA;
- vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR; - vcpu->run->internal.suberror = KVM_INTERNAL_ERROR_DELIVERY_EV; - vcpu->run->internal.data[0] = vectoring_info; - vcpu->run->internal.data[1] = exit_reason.full; - vcpu->run->internal.data[2] = vmx_get_exit_qual(vcpu); - if (exit_reason.basic == EXIT_REASON_EPT_MISCONFIG) { - vcpu->run->internal.data[ndata++] = - vmcs_read64(GUEST_PHYSICAL_ADDRESS); - } - vcpu->run->internal.data[ndata++] = vcpu->arch.last_vmentry_cpu; - vcpu->run->internal.ndata = ndata; + if (exit_reason.basic == EXIT_REASON_EPT_MISCONFIG) + gpa = vmcs_read64(GUEST_PHYSICAL_ADDRESS); + + kvm_prepare_event_vectoring_exit(vcpu, gpa); return 0; }
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 2e713480933a..7ce9cdb66f19 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -8813,6 +8813,28 @@ void kvm_prepare_emulation_failure_exit(struct kvm_vcpu *vcpu) } EXPORT_SYMBOL_GPL(kvm_prepare_emulation_failure_exit);
+void kvm_prepare_event_vectoring_exit(struct kvm_vcpu *vcpu, gpa_t gpa) +{ + u32 reason, intr_info, error_code; + struct kvm_run *run = vcpu->run; + u64 info1, info2; + int ndata = 0; + + kvm_x86_call(get_exit_info)(vcpu, &reason, &info1, &info2, + &intr_info, &error_code); + + run->internal.data[ndata++] = info2; + run->internal.data[ndata++] = reason; + run->internal.data[ndata++] = info1; + run->internal.data[ndata++] = gpa; + run->internal.data[ndata++] = vcpu->arch.last_vmentry_cpu; + + run->exit_reason = KVM_EXIT_INTERNAL_ERROR; + run->internal.suberror = KVM_INTERNAL_ERROR_DELIVERY_EV; + run->internal.ndata = ndata; +} +EXPORT_SYMBOL_GPL(kvm_prepare_event_vectoring_exit); + static int handle_emulation_failure(struct kvm_vcpu *vcpu, int emulation_type) { struct kvm *kvm = vcpu->kvm;
Add emulation status for unhandleable vectoring, i.e. when KVM can't emulate an instruction during vectoring. Such a situation can occur if guest sets the IDT descriptor base to point to MMIO region, and triggers an exception after that.
Exit to userspace with event delivery error when KVM can't emulate an instruction when vectoring an event.
Signed-off-by: Ivan Orlov iorlov@amazon.com --- V1 -> V2: - This patch wasn't included in V1. V2 -> V3: - Make new X86EMUL_ code more generic to allow using it for any type of unhandleable vectoring
arch/x86/kvm/kvm_emulate.h | 2 ++ arch/x86/kvm/x86.c | 5 +++++ 2 files changed, 7 insertions(+)
diff --git a/arch/x86/kvm/kvm_emulate.h b/arch/x86/kvm/kvm_emulate.h index 10495fffb890..5bcf50ffc3de 100644 --- a/arch/x86/kvm/kvm_emulate.h +++ b/arch/x86/kvm/kvm_emulate.h @@ -88,6 +88,8 @@ struct x86_instruction_info { #define X86EMUL_CMPXCHG_FAILED 4 /* cmpxchg did not see expected value */ #define X86EMUL_IO_NEEDED 5 /* IO is needed to complete emulation */ #define X86EMUL_INTERCEPTED 6 /* Intercepted by nested VMCB/VMCS */ +/* Vectroing can't be emulated */ +#define X86EMUL_UNHANDLEABLE_VECTORING 7
/* x86-specific emulation flags */ #define X86EMUL_F_WRITE BIT(0) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 7ce9cdb66f19..849a6fc364b3 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -9107,6 +9107,11 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa, if (r == X86EMUL_RETRY_INSTR || r == X86EMUL_PROPAGATE_FAULT) return 1;
+ if (r == X86EMUL_UNHANDLEABLE_VECTORING) { + kvm_prepare_event_vectoring_exit(vcpu, cr2_or_gpa); + return 0; + } + WARN_ON_ONCE(r != X86EMUL_UNHANDLEABLE); return handle_emulation_failure(vcpu, emulation_type); }
On Tue, Dec 17, 2024, Ivan Orlov wrote:
Add emulation status for unhandleable vectoring, i.e. when KVM can't emulate an instruction during vectoring. Such a situation can occur if guest sets the IDT descriptor base to point to MMIO region, and triggers an exception after that.
Exit to userspace with event delivery error when KVM can't emulate an instruction when vectoring an event.
Signed-off-by: Ivan Orlov iorlov@amazon.com
V1 -> V2:
- This patch wasn't included in V1.
V2 -> V3:
- Make new X86EMUL_ code more generic to allow using it for any type
of unhandleable vectoring
arch/x86/kvm/kvm_emulate.h | 2 ++ arch/x86/kvm/x86.c | 5 +++++ 2 files changed, 7 insertions(+)
diff --git a/arch/x86/kvm/kvm_emulate.h b/arch/x86/kvm/kvm_emulate.h index 10495fffb890..5bcf50ffc3de 100644 --- a/arch/x86/kvm/kvm_emulate.h +++ b/arch/x86/kvm/kvm_emulate.h @@ -88,6 +88,8 @@ struct x86_instruction_info { #define X86EMUL_CMPXCHG_FAILED 4 /* cmpxchg did not see expected value */ #define X86EMUL_IO_NEEDED 5 /* IO is needed to complete emulation */ #define X86EMUL_INTERCEPTED 6 /* Intercepted by nested VMCB/VMCS */ +/* Vectroing can't be emulated */
Typo. I think it's also worth elaborating a bit, e.g.
/* Emulation during event vectoring is unsupported */
+#define X86EMUL_UNHANDLEABLE_VECTORING 7 /* x86-specific emulation flags */ #define X86EMUL_F_WRITE BIT(0) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 7ce9cdb66f19..849a6fc364b3 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -9107,6 +9107,11 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa, if (r == X86EMUL_RETRY_INSTR || r == X86EMUL_PROPAGATE_FAULT) return 1;
if (r == X86EMUL_UNHANDLEABLE_VECTORING) {kvm_prepare_event_vectoring_exit(vcpu, cr2_or_gpa);return 0;}- WARN_ON_ONCE(r != X86EMUL_UNHANDLEABLE); return handle_emulation_failure(vcpu, emulation_type); }
-- 2.43.0
Try to unprotect and retry the instruction execution before checking for unhandleable vectoring. If there is a write to a shadowed page table when vectoring an event, KVM should be able to unprotect the gfn and retry the instruction execution without returning an error to userspace.
This ensures that the subsequent patches won't make KVM exit to userspace when handling an intercepted #PF during vectoring without checking whether unprotect & retry is possible.
Suggested-by: Sean Christopherson seanjc@google.com Signed-off-by: Ivan Orlov iorlov@amazon.com --- V1 -> V2: - This patch wasn't included in V1. V2 -> V3: - This patch wasn't included in V2.
arch/x86/kvm/x86.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 849a6fc364b3..26faacc99c4c 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -9107,6 +9107,10 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa, if (r == X86EMUL_RETRY_INSTR || r == X86EMUL_PROPAGATE_FAULT) return 1;
+ if (kvm_unprotect_and_retry_on_failure(vcpu, cr2_or_gpa, + emulation_type)) + return 1; + if (r == X86EMUL_UNHANDLEABLE_VECTORING) { kvm_prepare_event_vectoring_exit(vcpu, cr2_or_gpa); return 0;
Move unhandleable vmexit during vectoring error detection into check_emulate_instruction. Implement the function which prohibits the emulation if EMULTYPE_PF is set when vectoring, otherwise such a situation may occur:
1. CPU executes an instruction and hits a #GP 2. While vectoring the #GP, a shadow #PF occurs 3. On vmexit, KVM re-injects #GP 4. KVM emulates because of the write-protected page 5. KVM "successfully" emulates and also detects the #GP 6. KVM synthesizes a #GP, and since #GP has already been injected, incorrectly escalates to a #DF.
Fix the comment about EMULTYPE_PF as this flag doesn't necessarily mean MMIO anymore: it can also be set due to the write protection violation.
Suggested-by: Sean Christopherson seanjc@google.com Signed-off-by: Ivan Orlov iorlov@amazon.com --- V1 -> V2: - Detect the unhandleable vectoring error in vmx_check_emulate_instruction instead of handling it in the common MMU code (which is specific for cached MMIO) V2 -> V3: - Prohibit any emulation during vectoring if it happens due to an intercepted #PF.
arch/x86/include/asm/kvm_host.h | 9 +++++++-- arch/x86/kvm/vmx/vmx.c | 23 +++++++++-------------- 2 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index de8fb1ab230c..f3a1d050e1d6 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -2019,8 +2019,8 @@ u64 vcpu_tsc_khz(struct kvm_vcpu *vcpu); * VMware backdoor emulation handles select instructions * and reinjects the #GP for all other cases. * - * EMULTYPE_PF - Set when emulating MMIO by way of an intercepted #PF, in which - * case the CR2/GPA value pass on the stack is valid. + * EMULTYPE_PF - Set when an intercepted #PF triggers the emulation, in which case + * the CR2/GPA value pass on the stack is valid. * * EMULTYPE_COMPLETE_USER_EXIT - Set when the emulator should update interruptibility * state and inject single-step #DBs after skipping @@ -2055,6 +2055,11 @@ u64 vcpu_tsc_khz(struct kvm_vcpu *vcpu); #define EMULTYPE_COMPLETE_USER_EXIT (1 << 7) #define EMULTYPE_WRITE_PF_TO_SP (1 << 8)
+static inline bool kvm_can_emulate_event_vectoring(int emul_type) +{ + return !(emul_type & EMULTYPE_PF); +} + int kvm_emulate_instruction(struct kvm_vcpu *vcpu, int emulation_type); int kvm_emulate_instruction_from_buffer(struct kvm_vcpu *vcpu, void *insn, int insn_len); diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index acc2f0e0a339..89ddbe1175c7 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -1705,6 +1705,12 @@ int vmx_check_emulate_instruction(struct kvm_vcpu *vcpu, int emul_type, kvm_queue_exception(vcpu, UD_VECTOR); return X86EMUL_PROPAGATE_FAULT; } + + /* Check that emulation is possible during event vectoring */ + if ((to_vmx(vcpu)->idt_vectoring_info & VECTORING_INFO_VALID_MASK) && + !kvm_can_emulate_event_vectoring(emul_type)) + return X86EMUL_UNHANDLEABLE_VECTORING; + return X86EMUL_CONTINUE; }
@@ -6543,26 +6549,15 @@ static int __vmx_handle_exit(struct kvm_vcpu *vcpu, fastpath_t exit_fastpath) return 0; }
- /* - * Note: - * Do not try to fix EXIT_REASON_EPT_MISCONFIG if it caused by - * delivery event since it indicates guest is accessing MMIO. - * The vm-exit can be triggered again after return to guest that - * will cause infinite loop. - */ if ((vectoring_info & VECTORING_INFO_VALID_MASK) && (exit_reason.basic != EXIT_REASON_EXCEPTION_NMI && exit_reason.basic != EXIT_REASON_EPT_VIOLATION && exit_reason.basic != EXIT_REASON_PML_FULL && exit_reason.basic != EXIT_REASON_APIC_ACCESS && exit_reason.basic != EXIT_REASON_TASK_SWITCH && - exit_reason.basic != EXIT_REASON_NOTIFY)) { - gpa_t gpa = INVALID_GPA; - - if (exit_reason.basic == EXIT_REASON_EPT_MISCONFIG) - gpa = vmcs_read64(GUEST_PHYSICAL_ADDRESS); - - kvm_prepare_event_vectoring_exit(vcpu, gpa); + exit_reason.basic != EXIT_REASON_NOTIFY && + exit_reason.basic != EXIT_REASON_EPT_MISCONFIG)) { + kvm_prepare_event_vectoring_exit(vcpu, INVALID_GPA); return 0; }
On Tue, Dec 17, 2024, Ivan Orlov wrote:
Move unhandleable vmexit during vectoring error detection into check_emulate_instruction. Implement the function which prohibits the emulation if EMULTYPE_PF is set when vectoring, otherwise such a situation may occur:
I definitely think it's worth explaining that moving the detection covers new emulation cases, and also calling out that handle_ept_misconfig() consults vmx_check_emulate_instruction(), i.e. that moving the detection shouldn't affect KVM's overall handlng of EPT Misconfig.
--
Move handling of emulation during event vectoring, which KVM doesn't support, into VMX's check_emulate_instruction(), so that KVM detects all unsupported emulation, not just cached emulated MMIO (EPT misconfig). E.g. on emulated MMIO that isn't cached (EPT Violation) or occurs with legacy shadow paging (#PF).
Rejecting emulation on other sources of emulation also fixes a largely theoretical flaw (thanks to the "unprotect and retry" logic), where KVM could incorrectly inject a #DF:
1. CPU executes an instruction and hits a #GP 2. While vectoring the #GP, a shadow #PF occurs 3. On the #PF VM-Exit, KVM re-injects #GP 4. KVM emulates because of the write-protected page 5. KVM "successfully" emulates and also detects the #GP 6. KVM synthesizes a #GP, and since #GP has already been injected, incorrectly escalates to a #DF.
Fix the comment about EMULTYPE_PF as this flag doesn't necessarily mean MMIO anymore: it can also be set due to the write protection violation.
Note, handle_ept_misconfig() checks vmx_check_emulate_instruction() before attempting emulation of any kind.
On 12/18/24 18:39, Sean Christopherson wrote:
I definitely think it's worth explaining that moving the detection covers new emulation cases, and also calling out that handle_ept_misconfig() consults vmx_check_emulate_instruction(), i.e. that moving the detection shouldn't affect KVM's overall handlng of EPT Misconfig.
--
Move handling of emulation during event vectoring, which KVM doesn't support, into VMX's check_emulate_instruction(), so that KVM detects all unsupported emulation, not just cached emulated MMIO (EPT misconfig). E.g. on emulated MMIO that isn't cached (EPT Violation) or occurs with legacy shadow paging (#PF).
Rejecting emulation on other sources of emulation also fixes a largely theoretical flaw (thanks to the "unprotect and retry" logic), where KVM could incorrectly inject a #DF:
- CPU executes an instruction and hits a #GP
- While vectoring the #GP, a shadow #PF occurs
- On the #PF VM-Exit, KVM re-injects #GP
- KVM emulates because of the write-protected page
- KVM "successfully" emulates and also detects the #GP
- KVM synthesizes a #GP, and since #GP has already been injected, incorrectly escalates to a #DF.
Fix the comment about EMULTYPE_PF as this flag doesn't necessarily mean MMIO anymore: it can also be set due to the write protection violation.
Note, handle_ept_misconfig() checks vmx_check_emulate_instruction() before attempting emulation of any kind.
Yeah, I thought that covering the change in non-cacheable MMIO / shadow paged #PF handling, but forgot to include it into the commit message :( Could you please fix the message when applying? The message you suggested looks good to me.
Thanks!
Detect unhandleable vectoring in check_emulate_instruction to prevent infinite loop on SVM and eliminate the difference in how intercepted #PF during vectoring is handled on SVM and VMX.
Signed-off-by: Ivan Orlov iorlov@amazon.com --- V1 -> V2: - Detect the unhandleable vectoring error in svm_check_emulate_instruction instead of handling it in the common MMU code (which is specific for cached MMIO) V2 -> V3: - Use more generic function to check if emulation is allowed when vectoring
arch/x86/kvm/svm/svm.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c index dd15cc635655..e89c6fc2c4e6 100644 --- a/arch/x86/kvm/svm/svm.c +++ b/arch/x86/kvm/svm/svm.c @@ -4802,6 +4802,12 @@ static int svm_check_emulate_instruction(struct kvm_vcpu *vcpu, int emul_type, bool smep, smap, is_user; u64 error_code;
+ /* Check that emulation is possible during event vectoring */ + if ((to_svm(vcpu)->vmcb->control.exit_int_info & + SVM_EXITINTINFO_TYPE_MASK) && + !kvm_can_emulate_event_vectoring(emul_type)) + return X86EMUL_UNHANDLEABLE_VECTORING; + /* Emulation is always possible when KVM has access to all guest state. */ if (!sev_guest(vcpu->kvm)) return X86EMUL_CONTINUE;
On Tue, Dec 17, 2024, Ivan Orlov wrote:
Detect unhandleable vectoring in check_emulate_instruction to prevent infinite loop on SVM and eliminate the difference in how intercepted #PF during vectoring is handled on SVM and VMX.
Signed-off-by: Ivan Orlov iorlov@amazon.com
V1 -> V2:
- Detect the unhandleable vectoring error in svm_check_emulate_instruction
instead of handling it in the common MMU code (which is specific for cached MMIO) V2 -> V3:
- Use more generic function to check if emulation is allowed when
vectoring
arch/x86/kvm/svm/svm.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c index dd15cc635655..e89c6fc2c4e6 100644 --- a/arch/x86/kvm/svm/svm.c +++ b/arch/x86/kvm/svm/svm.c @@ -4802,6 +4802,12 @@ static int svm_check_emulate_instruction(struct kvm_vcpu *vcpu, int emul_type, bool smep, smap, is_user; u64 error_code;
- /* Check that emulation is possible during event vectoring */
- if ((to_svm(vcpu)->vmcb->control.exit_int_info &
SVM_EXITINTINFO_TYPE_MASK) &&
Let this poke out. Alternatively, and probably preferably, capture "svm" locally and it fits nicely on one line (there's an existing user of to_svm() in this helper). My objection to a local variable was specifically about a local "is_event_vectoring", not about local variables in general.
!kvm_can_emulate_event_vectoring(emul_type))return X86EMUL_UNHANDLEABLE_VECTORING;- /* Emulation is always possible when KVM has access to all guest state. */ if (!sev_guest(vcpu->kvm)) return X86EMUL_CONTINUE;
-- 2.43.0
Implement a function for setting the IDT descriptor from the guest code. Replace the existing lidt occurrences with calls to this function as `lidt` is used in multiple places.
Signed-off-by: Ivan Orlov iorlov@amazon.com --- V1 -> V2: - This patch wasn't included in V1. V2 -> V3: - No changes
tools/testing/selftests/kvm/include/x86_64/processor.h | 5 +++++ tools/testing/selftests/kvm/set_memory_region_test.c | 2 +- tools/testing/selftests/kvm/x86_64/sev_smoke_test.c | 2 +- 3 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h index 645200e95f89..69938c649a5e 100644 --- a/tools/testing/selftests/kvm/include/x86_64/processor.h +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h @@ -571,6 +571,11 @@ static inline void set_cr4(uint64_t val) __asm__ __volatile__("mov %0, %%cr4" : : "r" (val) : "memory"); }
+static inline void set_idt(const struct desc_ptr *idt_desc) +{ + __asm__ __volatile__("lidt %0"::"m"(*idt_desc)); +} + static inline u64 xgetbv(u32 index) { u32 eax, edx; diff --git a/tools/testing/selftests/kvm/set_memory_region_test.c b/tools/testing/selftests/kvm/set_memory_region_test.c index a8267628e9ed..a1c53cc854a5 100644 --- a/tools/testing/selftests/kvm/set_memory_region_test.c +++ b/tools/testing/selftests/kvm/set_memory_region_test.c @@ -235,7 +235,7 @@ static void guest_code_delete_memory_region(void) * in the guest will never succeed, and so isn't an option. */ memset(&idt, 0, sizeof(idt)); - __asm__ __volatile__("lidt %0" :: "m"(idt)); + set_idt(&idt);
GUEST_SYNC(0);
diff --git a/tools/testing/selftests/kvm/x86_64/sev_smoke_test.c b/tools/testing/selftests/kvm/x86_64/sev_smoke_test.c index ae77698e6e97..a1a688e75266 100644 --- a/tools/testing/selftests/kvm/x86_64/sev_smoke_test.c +++ b/tools/testing/selftests/kvm/x86_64/sev_smoke_test.c @@ -155,7 +155,7 @@ static void guest_shutdown_code(void)
/* Clobber the IDT so that #UD is guaranteed to trigger SHUTDOWN. */ memset(&idt, 0, sizeof(idt)); - __asm__ __volatile__("lidt %0" :: "m"(idt)); + set_idt(&idt);
__asm__ __volatile__("ud2"); }
KVM: selftests: is the preferred scope.
Extend the 'set_memory_region_test' with a test case which covers the MMIO during vectoring error handling. The test case
1) Sets an IDT descriptor base to point to an MMIO address 2) Generates a #GP in the guest 3) Verifies that we got a correct exit reason and suberror code 4) Verifies that we got a corrent reported GPA in internal.data[3]
Also, add a definition of non-canonical address to processor.h
Signed-off-by: Ivan Orlov iorlov@amazon.com --- V1 -> V2: - Get rid of pronouns, redundant comments and incorrect wording - Define noncanonical address in processor.h - Fix indentation and wrap lines at 80 columns V2 -> V3: - Move "NONCANONICAL" definition to the beginning of the file
.../selftests/kvm/include/x86_64/processor.h | 2 + .../selftests/kvm/set_memory_region_test.c | 51 +++++++++++++++++++ 2 files changed, 53 insertions(+)
diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h index 69938c649a5e..6b8d12f042a8 100644 --- a/tools/testing/selftests/kvm/include/x86_64/processor.h +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h @@ -29,6 +29,8 @@ extern uint64_t guest_tsc_khz; #define MAX_NR_CPUID_ENTRIES 100 #endif
+#define NONCANONICAL 0xaaaaaaaaaaaaaaaaull + /* Forced emulation prefix, used to invoke the emulator unconditionally. */ #define KVM_FEP "ud2; .byte 'k', 'v', 'm';"
diff --git a/tools/testing/selftests/kvm/set_memory_region_test.c b/tools/testing/selftests/kvm/set_memory_region_test.c index a1c53cc854a5..d65a9f20aa1a 100644 --- a/tools/testing/selftests/kvm/set_memory_region_test.c +++ b/tools/testing/selftests/kvm/set_memory_region_test.c @@ -553,6 +553,56 @@ static void test_add_overlapping_private_memory_regions(void) close(memfd); kvm_vm_free(vm); } + +static void guest_code_mmio_during_vectoring(void) +{ + const struct desc_ptr idt_desc = { + .address = MEM_REGION_GPA, + .size = 0xFFF, + }; + + set_idt(&idt_desc); + + /* Generate a #GP by dereferencing a non-canonical address */ + *((uint8_t *)NONCANONICAL) = 0x1; + + GUEST_ASSERT(0); +} + +/* + * This test points the IDT descriptor base to an MMIO address. It should cause + * a KVM internal error when an event occurs in the guest. + */ +static void test_mmio_during_vectoring(void) +{ + struct kvm_vcpu *vcpu; + struct kvm_run *run; + struct kvm_vm *vm; + u64 expected_gpa; + + pr_info("Testing MMIO during vectoring error handling\n"); + + vm = vm_create_with_one_vcpu(&vcpu, guest_code_mmio_during_vectoring); + virt_map(vm, MEM_REGION_GPA, MEM_REGION_GPA, 1); + + run = vcpu->run; + + vcpu_run(vcpu); + TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_INTERNAL_ERROR); + TEST_ASSERT(run->internal.suberror == KVM_INTERNAL_ERROR_DELIVERY_EV, + "Unexpected suberror = %d", vcpu->run->internal.suberror); + TEST_ASSERT(run->internal.ndata != 4, "Unexpected internal error data array size = %d", + run->internal.ndata); + + /* The reported GPA should be IDT base + offset of the GP vector */ + expected_gpa = MEM_REGION_GPA + GP_VECTOR * sizeof(struct idt_entry); + + TEST_ASSERT(run->internal.data[3] == expected_gpa, + "Unexpected GPA = %llx (expected %lx)", + vcpu->run->internal.data[3], expected_gpa); + + kvm_vm_free(vm); +} #endif
int main(int argc, char *argv[]) @@ -568,6 +618,7 @@ int main(int argc, char *argv[]) * KVM_RUN fails with ENOEXEC or EFAULT. */ test_zero_memory_regions(); + test_mmio_during_vectoring(); #endif
test_invalid_memory_region_flags();
KVM: selftests:
On Tue, Dec 17, 2024, Ivan Orlov wrote:
Extend the 'set_memory_region_test' with a test case which covers the MMIO during vectoring error handling. The test case
Probably a good idea to explicitly state this is x86-only (hard to see that from the diff alone).
- Sets an IDT descriptor base to point to an MMIO address
- Generates a #GP in the guest
- Verifies that we got a correct exit reason and suberror code
No "we". It's very specifically userspace that needs to see the exit.
- Verifies that we got a corrent reported GPA in internal.data[3]
s/corrent/correct?
And #4 can be combined with #3:
3) Verifies userspace gets the correct exit reason, suberror code, and GPA in internal.data[3]
On Tue, Dec 17, 2024, Ivan Orlov wrote:
Currently, the unhandleable vectoring (e.g. when guest accesses MMIO during vectoring) is handled differently on VMX and SVM: on VMX KVM returns internal error, when SVM goes into infinite loop trying to deliver an event again and again.
This patch series eliminates this difference by returning a KVM internal error when KVM can't emulate during vectoring for both VMX and SVM.
Also, introduce a selftest test case which covers the error handling mentioned above.
A few nits throughout, but I'll address them when applying. Thanks!
On 12/18/24 18:44, Sean Christopherson wrote:
On Tue, Dec 17, 2024, Ivan Orlov wrote:
Currently, the unhandleable vectoring (e.g. when guest accesses MMIO during vectoring) is handled differently on VMX and SVM: on VMX KVM returns internal error, when SVM goes into infinite loop trying to deliver an event again and again.
This patch series eliminates this difference by returning a KVM internal error when KVM can't emulate during vectoring for both VMX and SVM.
Also, introduce a selftest test case which covers the error handling mentioned above.
A few nits throughout, but I'll address them when applying. Thanks!
Hi Sean,
Awesome, thanks a lot for fixing the commits and for the review.
On Tue, 17 Dec 2024 18:14:51 +0000, Ivan Orlov wrote:
Currently, the unhandleable vectoring (e.g. when guest accesses MMIO during vectoring) is handled differently on VMX and SVM: on VMX KVM returns internal error, when SVM goes into infinite loop trying to deliver an event again and again.
This patch series eliminates this difference by returning a KVM internal error when KVM can't emulate during vectoring for both VMX and SVM.
[...]
Applied to kvm-x86 misc, thanks! If you get a chance, please double check that I didn't fat-finger anything.
[1/7] KVM: x86: Add function for vectoring error generation https://github.com/kvm-x86/linux/commit/11c98fa07a79 [2/7] KVM: x86: Add emulation status for unhandleable vectoring https://github.com/kvm-x86/linux/commit/5c9cfc486636 [3/7] KVM: x86: Unprotect & retry before unhandleable vectoring check https://github.com/kvm-x86/linux/commit/704fc6021b9e [4/7] KVM: VMX: Handle vectoring error in check_emulate_instruction https://github.com/kvm-x86/linux/commit/47ef3ef843c0 [5/7] KVM: SVM: Handle vectoring error in check_emulate_instruction https://github.com/kvm-x86/linux/commit/7bd7ff99110a [6/7] selftests: KVM: extract lidt into helper function https://github.com/kvm-x86/linux/commit/4e9427aeb957 [7/7] selftests: KVM: Add test case for MMIO during vectoring https://github.com/kvm-x86/linux/commit/62e41f6b4f36
On Wed, Dec 18, 2024 at 06:40:46PM -0800, Sean Christopherson wrote:
CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
On Tue, 17 Dec 2024 18:14:51 +0000, Ivan Orlov wrote:
Currently, the unhandleable vectoring (e.g. when guest accesses MMIO during vectoring) is handled differently on VMX and SVM: on VMX KVM returns internal error, when SVM goes into infinite loop trying to deliver an event again and again.
This patch series eliminates this difference by returning a KVM internal error when KVM can't emulate during vectoring for both VMX and SVM.
[...]
Applied to kvm-x86 misc, thanks! If you get a chance, please double check that I didn't fat-finger anything.
[1/7] KVM: x86: Add function for vectoring error generation https://github.com/kvm-x86/linux/commit/11c98fa07a79 [2/7] KVM: x86: Add emulation status for unhandleable vectoring https://github.com/kvm-x86/linux/commit/5c9cfc486636 [3/7] KVM: x86: Unprotect & retry before unhandleable vectoring check https://github.com/kvm-x86/linux/commit/704fc6021b9e [4/7] KVM: VMX: Handle vectoring error in check_emulate_instruction https://github.com/kvm-x86/linux/commit/47ef3ef843c0 [5/7] KVM: SVM: Handle vectoring error in check_emulate_instruction https://github.com/kvm-x86/linux/commit/7bd7ff99110a [6/7] selftests: KVM: extract lidt into helper function https://github.com/kvm-x86/linux/commit/4e9427aeb957 [7/7] selftests: KVM: Add test case for MMIO during vectoring https://github.com/kvm-x86/linux/commit/62e41f6b4f36
Hi Sean,
The commits (and the messages specifically) look good to me, thanks a lot for making the changelogs better! :)
Also, I ran the selftests for the `next` branch on both Intel and AMD platforms, and all of them seem to pass.
-- Kind regards, Ivan Orlov
linux-kselftest-mirror@lists.linaro.org
-
Ivan Orlov -
Ivan Orlov -
Sean Christopherson