| Message ID | 20241111102749.82761-2-iorlov@amazon.com |
|---|---|
| State | New |
| Headers | show |
| Series | Enhance event delivery error handling | expand |
On Mon, Nov 11, 2024, Ivan Orlov wrote: > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > index f6900bec4874..f92740e7e107 100644 > --- a/arch/x86/kvm/vmx/vmx.c > +++ b/arch/x86/kvm/vmx/vmx.c > @@ -6452,6 +6452,7 @@ static int __vmx_handle_exit(struct kvm_vcpu *vcpu, fastpath_t exit_fastpath) > union vmx_exit_reason exit_reason = vmx->exit_reason; > u32 vectoring_info = vmx->idt_vectoring_info; > u16 exit_handler_index; > + gpa_t gpa; I've gone back and forth on where to declare scoped varaibles, but in this case, I think it makes sense to declare "gpa" inside the if-statement. Making it visible at the function scope when it's valid in a _super_ limited case is bound to cause issues. Of course, this code goes away by the end of the series, so that point is moot. But on the other hand, declaring the variable in the if-statement is desirable as the churn is precisely limited to the code that's being changed. > /* > * Flush logged GPAs PML buffer, this will make dirty_bitmap more > @@ -6550,19 +6551,10 @@ 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 = exit_reason.basic == EXIT_REASON_EPT_MISCONFIG > + ? vmcs_read64(GUEST_PHYSICAL_ADDRESS) : INVALID_GPA; Again a moot point, but IMO using a ternary operator here makes it unnecessarily difficult to see that gpa is valid if and only if the exit was an EPT misconfig. gpa_t gpa = INVALID_GPA; if (exit_reason.basic == EXIT_REASON_EPT_MISCONFIG) gpa = vmcs_read64(GUEST_PHYSICAL_ADDRESS); > - 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; > + kvm_prepare_event_vectoring_exit(vcpu, gpa); > return 0; > } > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 83fe0a78146f..e338d583f48f 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -8828,6 +8828,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++] = (u64)gpa; No need for the cast. > + 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; > -- > 2.43.0 >
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index 6d9f763a7bb9..eb413079b7c6 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -2060,6 +2060,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 f6900bec4874..f92740e7e107 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -6452,6 +6452,7 @@ static int __vmx_handle_exit(struct kvm_vcpu *vcpu, fastpath_t exit_fastpath) union vmx_exit_reason exit_reason = vmx->exit_reason; u32 vectoring_info = vmx->idt_vectoring_info; u16 exit_handler_index; + gpa_t gpa; /* * Flush logged GPAs PML buffer, this will make dirty_bitmap more @@ -6550,19 +6551,10 @@ 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 = exit_reason.basic == EXIT_REASON_EPT_MISCONFIG + ? vmcs_read64(GUEST_PHYSICAL_ADDRESS) : 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; + kvm_prepare_event_vectoring_exit(vcpu, gpa); return 0; } diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 83fe0a78146f..e338d583f48f 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -8828,6 +8828,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++] = (u64)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;
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 arch/x86/include/asm/kvm_host.h | 2 ++ arch/x86/kvm/vmx/vmx.c | 16 ++++------------ arch/x86/kvm/x86.c | 22 ++++++++++++++++++++++ 3 files changed, 28 insertions(+), 12 deletions(-)