[PATCH] KVM: VMX: Correct *intr_info content and *info2 for EPT_VIOLATION in get_exit_info()

[PATCH] KVM: VMX: Correct *intr_info content and *info2 for EPT_VIOLATION in get_exit_info()

[Robert Hoo]

Fill vmx::idt_vectoring_info in *intr_info, to align with
svm_get_exit_info(), where *intr_info is for complement information about
intercepts occurring during event delivery through IDT (APM 15.7.2
Intercepts During IDT Interrupt Delivery), whose counterpart in
VMX is IDT_VECTORING_INFO_FIELD (SDM 25.9.3 Information for VM Exits
That Occur During Event Delivery), rather than VM_EXIT_INTR_INFO.

Fill *info2 with GUEST_PHYSICAL_ADDRESS in case of EPT_VIOLATION, also
to align with SVM. It can be filled with other info for different exit
reasons, like SVM's EXITINFO2.

Fixes: 235ba74f008d ("KVM: x86: Add intr/vectoring info and error code to kvm_exit tracepoint")
Signed-off-by: Robert Hoo <robert.hoo.linux@gmail.com>
---
 arch/x86/kvm/vmx/vmx.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index d21f55f323ea..f1bf9f1fc561 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -6141,14 +6141,26 @@ static void vmx_get_exit_info(struct kvm_vcpu *vcpu, u32 *reason,
 
 	*reason = vmx->exit_reason.full;
 	*info1 = vmx_get_exit_qual(vcpu);
+
 	if (!(vmx->exit_reason.failed_vmentry)) {
-		*info2 = vmx->idt_vectoring_info;
-		*intr_info = vmx_get_intr_info(vcpu);
+		*intr_info = vmx->idt_vectoring_info;
 		if (is_exception_with_error_code(*intr_info))
-			*error_code = vmcs_read32(VM_EXIT_INTR_ERROR_CODE);
+			*error_code = vmcs_read32(IDT_VECTORING_ERROR_CODE);
 		else
 			*error_code = 0;
-	} else {
+
+		/* various *info2 semantics according to exit reason */
+		switch (vmx->exit_reason.basic) {
+		case EXIT_REASON_EPT_VIOLATION:
+			*info2 = vmcs_read64(GUEST_PHYSICAL_ADDRESS);
+			break;
+		/* To do: *info2 for other exit reasons */
+		default:
+			*info2 = 0;
+			break;
+		}
+
+	} else {
 		*info2 = 0;
 		*intr_info = 0;
 		*error_code = 0;

base-commit: 1c6d984f523f67ecfad1083bb04c55d91977bb15
-- 
2.39.3

Re: [PATCH] KVM: VMX: Correct *intr_info content and *info2 for EPT_VIOLATION in get_exit_info()

[Sean Christopherson]

On Fri, Jan 12, 2024, Robert Hoo wrote:
> Fill vmx::idt_vectoring_info in *intr_info, to align with
> svm_get_exit_info(), where *intr_info is for complement information about
> intercepts occurring during event delivery through IDT (APM 15.7.2
> Intercepts During IDT Interrupt Delivery), whose counterpart in
> VMX is IDT_VECTORING_INFO_FIELD (SDM 25.9.3 Information for VM Exits
> That Occur During Event Delivery), rather than VM_EXIT_INTR_INFO.
> 
> Fill *info2 with GUEST_PHYSICAL_ADDRESS in case of EPT_VIOLATION, also
> to align with SVM. It can be filled with other info for different exit
> reasons, like SVM's EXITINFO2.

Nothing in here says *why*.

> Fixes: 235ba74f008d ("KVM: x86: Add intr/vectoring info and error code to kvm_exit tracepoint")
> Signed-off-by: Robert Hoo <robert.hoo.linux@gmail.com>
> ---
>  arch/x86/kvm/vmx/vmx.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index d21f55f323ea..f1bf9f1fc561 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -6141,14 +6141,26 @@ static void vmx_get_exit_info(struct kvm_vcpu *vcpu, u32 *reason,
>  
>  	*reason = vmx->exit_reason.full;
>  	*info1 = vmx_get_exit_qual(vcpu);
> +
>  	if (!(vmx->exit_reason.failed_vmentry)) {
> -		*info2 = vmx->idt_vectoring_info;
> -		*intr_info = vmx_get_intr_info(vcpu);
> +		*intr_info = vmx->idt_vectoring_info;
>  		if (is_exception_with_error_code(*intr_info))
> -			*error_code = vmcs_read32(VM_EXIT_INTR_ERROR_CODE);
> +			*error_code = vmcs_read32(IDT_VECTORING_ERROR_CODE);
>  		else
>  			*error_code = 0;
> -	} else {
> +
> +		/* various *info2 semantics according to exit reason */
> +		switch (vmx->exit_reason.basic) {
> +		case EXIT_REASON_EPT_VIOLATION:
> +			*info2 = vmcs_read64(GUEST_PHYSICAL_ADDRESS);
> +			break;
> +		/* To do: *info2 for other exit reasons */

I really, really don't want to go down this road.  As I stated in a different,
similar thread[*], I don't think this approach is sustainable.  I would much
rather people put time and effort into (a) developing BPF programs to extract info
from the VM-Entry and VM-Exit paths, (b) enhancing KVM in the areas where it's
painful or impossible for a BPF program to get at interesting data, and (c) start
upstreaming the BPF programs, e.g. somewhere in tools/.

[*] https://lore.kernel.org/all/ZRNC5IKXDq1yv0v3@google.com