2022-06-27 22:24:26

by Isaku Yamahata

[permalink] [raw]
Subject: [PATCH v7 050/102] KVM: VMX: Split out guts of EPT violation to common/exposed function

From: Sean Christopherson <[email protected]>

The difference of TDX EPT violation is how to retrieve information, GPA,
and exit qualification. To share the code to handle EPT violation, split
out the guts of EPT violation handler so that VMX/TDX exit handler can call
it after retrieving GPA and exit qualification.

Signed-off-by: Sean Christopherson <[email protected]>
Signed-off-by: Isaku Yamahata <[email protected]>
Reviewed-by: Paolo Bonzini <[email protected]>
---
arch/x86/kvm/vmx/common.h | 33 +++++++++++++++++++++++++++++++++
arch/x86/kvm/vmx/vmx.c | 32 ++++++--------------------------
2 files changed, 39 insertions(+), 26 deletions(-)
create mode 100644 arch/x86/kvm/vmx/common.h

diff --git a/arch/x86/kvm/vmx/common.h b/arch/x86/kvm/vmx/common.h
new file mode 100644
index 000000000000..235908f3e044
--- /dev/null
+++ b/arch/x86/kvm/vmx/common.h
@@ -0,0 +1,33 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __KVM_X86_VMX_COMMON_H
+#define __KVM_X86_VMX_COMMON_H
+
+#include <linux/kvm_host.h>
+
+#include "mmu.h"
+
+static inline int __vmx_handle_ept_violation(struct kvm_vcpu *vcpu, gpa_t gpa,
+ unsigned long exit_qualification)
+{
+ u64 error_code;
+
+ /* Is it a read fault? */
+ error_code = (exit_qualification & EPT_VIOLATION_ACC_READ)
+ ? PFERR_USER_MASK : 0;
+ /* Is it a write fault? */
+ error_code |= (exit_qualification & EPT_VIOLATION_ACC_WRITE)
+ ? PFERR_WRITE_MASK : 0;
+ /* Is it a fetch fault? */
+ error_code |= (exit_qualification & EPT_VIOLATION_ACC_INSTR)
+ ? PFERR_FETCH_MASK : 0;
+ /* ept page table entry is present? */
+ error_code |= (exit_qualification & EPT_VIOLATION_RWX_MASK)
+ ? PFERR_PRESENT_MASK : 0;
+
+ error_code |= (exit_qualification & EPT_VIOLATION_GVA_TRANSLATED) != 0 ?
+ PFERR_GUEST_FINAL_MASK : PFERR_GUEST_PAGE_MASK;
+
+ return kvm_mmu_page_fault(vcpu, gpa, error_code, NULL, 0);
+}
+
+#endif /* __KVM_X86_VMX_COMMON_H */
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index e3d304b14df0..2f1dc06aec3c 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -50,6 +50,7 @@
#include <asm/vmx.h>

#include "capabilities.h"
+#include "common.h"
#include "cpuid.h"
#include "evmcs.h"
#include "hyperv.h"
@@ -5578,11 +5579,10 @@ static int handle_task_switch(struct kvm_vcpu *vcpu)

static int handle_ept_violation(struct kvm_vcpu *vcpu)
{
- unsigned long exit_qualification;
- gpa_t gpa;
- u64 error_code;
+ unsigned long exit_qualification = vmx_get_exit_qual(vcpu);
+ gpa_t gpa = vmcs_read64(GUEST_PHYSICAL_ADDRESS);

- exit_qualification = vmx_get_exit_qual(vcpu);
+ trace_kvm_page_fault(gpa, exit_qualification);

/*
* EPT violation happened while executing iret from NMI,
@@ -5591,29 +5591,9 @@ static int handle_ept_violation(struct kvm_vcpu *vcpu)
* AAK134, BY25.
*/
if (!(to_vmx(vcpu)->idt_vectoring_info & VECTORING_INFO_VALID_MASK) &&
- enable_vnmi &&
- (exit_qualification & INTR_INFO_UNBLOCK_NMI))
+ enable_vnmi && (exit_qualification & INTR_INFO_UNBLOCK_NMI))
vmcs_set_bits(GUEST_INTERRUPTIBILITY_INFO, GUEST_INTR_STATE_NMI);

- gpa = vmcs_read64(GUEST_PHYSICAL_ADDRESS);
- trace_kvm_page_fault(gpa, exit_qualification);
-
- /* Is it a read fault? */
- error_code = (exit_qualification & EPT_VIOLATION_ACC_READ)
- ? PFERR_USER_MASK : 0;
- /* Is it a write fault? */
- error_code |= (exit_qualification & EPT_VIOLATION_ACC_WRITE)
- ? PFERR_WRITE_MASK : 0;
- /* Is it a fetch fault? */
- error_code |= (exit_qualification & EPT_VIOLATION_ACC_INSTR)
- ? PFERR_FETCH_MASK : 0;
- /* ept page table entry is present? */
- error_code |= (exit_qualification & EPT_VIOLATION_RWX_MASK)
- ? PFERR_PRESENT_MASK : 0;
-
- error_code |= (exit_qualification & EPT_VIOLATION_GVA_TRANSLATED) != 0 ?
- PFERR_GUEST_FINAL_MASK : PFERR_GUEST_PAGE_MASK;
-
vcpu->arch.exit_qualification = exit_qualification;

/*
@@ -5627,7 +5607,7 @@ static int handle_ept_violation(struct kvm_vcpu *vcpu)
if (unlikely(allow_smaller_maxphyaddr && kvm_vcpu_is_illegal_gpa(vcpu, gpa)))
return kvm_emulate_instruction(vcpu, 0);

- return kvm_mmu_page_fault(vcpu, gpa, error_code, NULL, 0);
+ return __vmx_handle_ept_violation(vcpu, gpa, exit_qualification);
}

static int handle_ept_misconfig(struct kvm_vcpu *vcpu)
--
2.25.1


2022-07-08 10:44:09

by Kai Huang

[permalink] [raw]
Subject: Re: [PATCH v7 050/102] KVM: VMX: Split out guts of EPT violation to common/exposed function

On Mon, 2022-06-27 at 14:53 -0700, [email protected] wrote:
> From: Sean Christopherson <[email protected]>
>
> The difference of TDX EPT violation is how to retrieve information, GPA,
> and exit qualification. To share the code to handle EPT violation, split
> out the guts of EPT violation handler so that VMX/TDX exit handler can call
> it after retrieving GPA and exit qualification.
>
> Signed-off-by: Sean Christopherson <[email protected]>
> Signed-off-by: Isaku Yamahata <[email protected]>
> Reviewed-by: Paolo Bonzini <[email protected]>
> ---
> arch/x86/kvm/vmx/common.h | 33 +++++++++++++++++++++++++++++++++
> arch/x86/kvm/vmx/vmx.c | 32 ++++++--------------------------
> 2 files changed, 39 insertions(+), 26 deletions(-)
> create mode 100644 arch/x86/kvm/vmx/common.h
>
> diff --git a/arch/x86/kvm/vmx/common.h b/arch/x86/kvm/vmx/common.h
> new file mode 100644
> index 000000000000..235908f3e044
> --- /dev/null
> +++ b/arch/x86/kvm/vmx/common.h
> @@ -0,0 +1,33 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef __KVM_X86_VMX_COMMON_H
> +#define __KVM_X86_VMX_COMMON_H
> +
> +#include <linux/kvm_host.h>
> +
> +#include "mmu.h"
> +
> +static inline int __vmx_handle_ept_violation(struct kvm_vcpu *vcpu, gpa_t gpa,
> + unsigned long exit_qualification)
> +{
> + u64 error_code;
> +
> + /* Is it a read fault? */
> + error_code = (exit_qualification & EPT_VIOLATION_ACC_READ)
> + ? PFERR_USER_MASK : 0;
> + /* Is it a write fault? */
> + error_code |= (exit_qualification & EPT_VIOLATION_ACC_WRITE)
> + ? PFERR_WRITE_MASK : 0;
> + /* Is it a fetch fault? */
> + error_code |= (exit_qualification & EPT_VIOLATION_ACC_INSTR)
> + ? PFERR_FETCH_MASK : 0;
> + /* ept page table entry is present? */
> + error_code |= (exit_qualification & EPT_VIOLATION_RWX_MASK)
> + ? PFERR_PRESENT_MASK : 0;
> +
> + error_code |= (exit_qualification & EPT_VIOLATION_GVA_TRANSLATED) != 0 ?
> + PFERR_GUEST_FINAL_MASK : PFERR_GUEST_PAGE_MASK;
> +
> + return kvm_mmu_page_fault(vcpu, gpa, error_code, NULL, 0);
> +}
> +
> +#endif /* __KVM_X86_VMX_COMMON_H */
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index e3d304b14df0..2f1dc06aec3c 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -50,6 +50,7 @@
> #include <asm/vmx.h>
>
> #include "capabilities.h"
> +#include "common.h"
> #include "cpuid.h"
> #include "evmcs.h"
> #include "hyperv.h"
> @@ -5578,11 +5579,10 @@ static int handle_task_switch(struct kvm_vcpu *vcpu)
>
> static int handle_ept_violation(struct kvm_vcpu *vcpu)
> {
> - unsigned long exit_qualification;
> - gpa_t gpa;
> - u64 error_code;
> + unsigned long exit_qualification = vmx_get_exit_qual(vcpu);
> + gpa_t gpa = vmcs_read64(GUEST_PHYSICAL_ADDRESS);
>
> - exit_qualification = vmx_get_exit_qual(vcpu);
> + trace_kvm_page_fault(gpa, exit_qualification);
>
> /*
> * EPT violation happened while executing iret from NMI,
> @@ -5591,29 +5591,9 @@ static int handle_ept_violation(struct kvm_vcpu *vcpu)
> * AAK134, BY25.
> */
> if (!(to_vmx(vcpu)->idt_vectoring_info & VECTORING_INFO_VALID_MASK) &&
> - enable_vnmi &&
> - (exit_qualification & INTR_INFO_UNBLOCK_NMI))
> + enable_vnmi && (exit_qualification & INTR_INFO_UNBLOCK_NMI))

Why this code change?

With this removed:

Reviewed-by: Kai Huang <[email protected]>

> vmcs_set_bits(GUEST_INTERRUPTIBILITY_INFO, GUEST_INTR_STATE_NMI);
>
> - gpa = vmcs_read64(GUEST_PHYSICAL_ADDRESS);
> - trace_kvm_page_fault(gpa, exit_qualification);
> -
> - /* Is it a read fault? */
> - error_code = (exit_qualification & EPT_VIOLATION_ACC_READ)
> - ? PFERR_USER_MASK : 0;
> - /* Is it a write fault? */
> - error_code |= (exit_qualification & EPT_VIOLATION_ACC_WRITE)
> - ? PFERR_WRITE_MASK : 0;
> - /* Is it a fetch fault? */
> - error_code |= (exit_qualification & EPT_VIOLATION_ACC_INSTR)
> - ? PFERR_FETCH_MASK : 0;
> - /* ept page table entry is present? */
> - error_code |= (exit_qualification & EPT_VIOLATION_RWX_MASK)
> - ? PFERR_PRESENT_MASK : 0;
> -
> - error_code |= (exit_qualification & EPT_VIOLATION_GVA_TRANSLATED) != 0 ?
> - PFERR_GUEST_FINAL_MASK : PFERR_GUEST_PAGE_MASK;
> -
> vcpu->arch.exit_qualification = exit_qualification;
>
> /*
> @@ -5627,7 +5607,7 @@ static int handle_ept_violation(struct kvm_vcpu *vcpu)
> if (unlikely(allow_smaller_maxphyaddr && kvm_vcpu_is_illegal_gpa(vcpu, gpa)))
> return kvm_emulate_instruction(vcpu, 0);
>
> - return kvm_mmu_page_fault(vcpu, gpa, error_code, NULL, 0);
> + return __vmx_handle_ept_violation(vcpu, gpa, exit_qualification);
> }
>
> static int handle_ept_misconfig(struct kvm_vcpu *vcpu)