On Mon, Nov 07, 2022, Paolo Bonzini wrote:
> On 11/7/22 09:27, Yu Zhang wrote:
> > VMFUNC is not supported for L1 guests, and executing VMFUNC in
> > L1 shall generate a #UD directly. Just disable it in secondary
> > proc-based execution control for L1, instead of intercepting it
> > and inject the #UD again.
> >
> > Signed-off-by: Yu Zhang<yu.c.zhang@xxxxxxxxxxxxxxx>
>
> Is this for TDX or similar? The reason for a patch should be mentioned in
> the commit message.
It's just a cleanup, but (a) it should be split over two patches as disabling
VMFUNC for L1 is technically a functional change, where as the changes to
nested_vmx_setup_ctls_msrs() are pure cleanups, and (b) the !guest_mode path in
handle_vmfunc() should either be removed or turned into a KVM_BUG_ON().
E.g.
---
arch/x86/kvm/vmx/nested.c | 11 ++---------
arch/x86/kvm/vmx/vmx.c | 7 ++++++-
2 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 0c62352dda6a..fa4130361187 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -5792,15 +5792,8 @@ static int handle_vmfunc(struct kvm_vcpu *vcpu)
struct vmcs12 *vmcs12;
u32 function = kvm_rax_read(vcpu);
- /*
- * VMFUNC is only supported for nested guests, but we always enable the
- * secondary control for simplicity; for non-nested mode, fake that we
- * didn't by injecting #UD.
- */
- if (!is_guest_mode(vcpu)) {
- kvm_queue_exception(vcpu, UD_VECTOR);
- return 1;
- }
+ if (KVM_BUG_ON(!is_guest_mode(vcpu), vcpu->kvm))
+ return -EIO;
vmcs12 = get_vmcs12(vcpu);
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 63247c57c72c..5a66c3c16c2d 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -4487,6 +4487,12 @@ static u32 vmx_secondary_exec_control(struct vcpu_vmx *vmx)
SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY);
exec_control &= ~SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE;
+ /*
+ * KVM doesn't support VMFUNC for L1, but the control is set in KVM's
+ * base configuration as KVM emulates VMFUNC[EPTP_SWITCHING] for L2.
+ */
+ exec_control &= ~SECONDARY_EXEC_ENABLE_VMFUNC;
+
/* SECONDARY_EXEC_DESC is enabled/disabled on writes to CR4.UMIP,
* in vmx_set_cr4. */
exec_control &= ~SECONDARY_EXEC_DESC;
@@ -6004,7 +6010,6 @@ static int (*kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu) = {
[EXIT_REASON_RDSEED] = kvm_handle_invalid_op,
[EXIT_REASON_PML_FULL] = handle_pml_full,
[EXIT_REASON_INVPCID] = handle_invpcid,
- [EXIT_REASON_VMFUNC] = handle_vmx_instruction,
[EXIT_REASON_PREEMPTION_TIMER] = handle_preemption_timer,
[EXIT_REASON_ENCLS] = handle_encls,
[EXIT_REASON_BUS_LOCK] = handle_bus_lock_vmexit,
base-commit: 07341b10fcbd5a7ef18225e0e9a8a40d91e3a2cc
--
and then the pure cleanup that is made possible because KVM now does:
msrs->secondary_ctls_high = vmcs_conf->cpu_based_2nd_exec_ctrl;
---
arch/x86/kvm/vmx/nested.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index fa4130361187..981bf5b3a319 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -6801,6 +6801,7 @@ void nested_vmx_setup_ctls_msrs(struct vmcs_config *vmcs_conf, u32 ept_caps)
SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY |
SECONDARY_EXEC_RDRAND_EXITING |
SECONDARY_EXEC_ENABLE_INVPCID |
+ SECONDARY_EXEC_ENABLE_VMFUNC |
SECONDARY_EXEC_RDSEED_EXITING |
SECONDARY_EXEC_XSAVES |
SECONDARY_EXEC_TSC_SCALING;
@@ -6832,18 +6833,13 @@ void nested_vmx_setup_ctls_msrs(struct vmcs_config *vmcs_conf, u32 ept_caps)
SECONDARY_EXEC_ENABLE_PML;
msrs->ept_caps |= VMX_EPT_AD_BIT;
}
- }
- if (cpu_has_vmx_vmfunc()) {
- msrs->secondary_ctls_high |=
- SECONDARY_EXEC_ENABLE_VMFUNC;
/*
- * Advertise EPTP switching unconditionally
- * since we emulate it
+ * Advertise EPTP switching irrespective of hardware support,
+ * KVM emulates it in software so long as VMFUNC is supported.
*/
- if (enable_ept)
- msrs->vmfunc_controls =
- VMX_VMFUNC_EPTP_SWITCHING;
+ if (cpu_has_vmx_vmfunc())
+ msrs->vmfunc_controls = VMX_VMFUNC_EPTP_SWITCHING;
}
/*
base-commit: 777dde94dd5e4328b419dcc5cb7118b39588eab1
--
