| Message ID | 20221107082727.1355797-1-yu.c.zhang@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: VMX: Do not trap VMFUNC instructions for L1 guests. | expand |
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@linux.intel.com> Is this for TDX or similar? The reason for a patch should be mentioned in the commit message. Paolo
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@linux.intel.com> > > 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
On Mon, Nov 07, 2022 at 05:36:21PM +0000, Sean Christopherson wrote: > 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@linux.intel.com> > > > > 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(). > Got it. Will do in V2. And thanks! B.R. Yu
On Mon, Nov 07, 2022 at 06:20:23PM +0100, 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@linux.intel.com> > > Is this for TDX or similar? The reason for a patch should be mentioned in > the commit message. Thanks for your quick reply, Paolo. It is not a new feature. Just a clean up for VMFUNC, which is not supported by KVM for L1 guest. According to Intel SDM 25.5.6.2 - "General Operation of the VMFUNC Instruction", The VMFUNC instruction causes an invalid-opcode exception (#UD) if the “enable VM functions” VM-execution controls is 0 or the value of EAX is greater than 63 (only VM functions 0–63 can be enable). Otherwise, the instruction causes a VM exit if the bit at position EAX is 0 in the VM-function controls (the selected VM function is not enabled) And since KVM only provides emulation of VMFUNC for nested guests, it is uncessary for KVM to intercept it and reinject a #UD. So just disable VMFUNC in VM-execution control for L1 guests. But please feel free to educate me if I missed some backgrounds about why this is enabled in the first place. Thanks! B.R. Yu
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c index 0c62352dda6a..8858c6c0979f 100644 --- a/arch/x86/kvm/vmx/nested.c +++ b/arch/x86/kvm/vmx/nested.c @@ -5793,11 +5793,11 @@ static int handle_vmfunc(struct kvm_vcpu *vcpu) 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. + * VMFUNC is only supported for nested guests, instead of triggering + * a VM Exit, non-nested guests shall receive #UD directly. */ if (!is_guest_mode(vcpu)) { + vcpu_unimpl(vcpu, "vmx: unexpected vm exit EXIT_REASON_VMFUNC.\n"); kvm_queue_exception(vcpu, UD_VECTOR); return 1; } @@ -6808,6 +6808,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; @@ -6839,16 +6840,8 @@ 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 - */ - if (enable_ept) + if (cpu_has_vmx_vmfunc()) msrs->vmfunc_controls = VMX_VMFUNC_EPTP_SWITCHING; } diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index 65f092e4a81b..9e17de62eb37 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -4483,6 +4483,9 @@ static u32 vmx_secondary_exec_control(struct vcpu_vmx *vmx) SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY); exec_control &= ~SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE; + /* VMFUNC is not supported for L1 guest, just disable it. */ + 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; @@ -6000,7 +6003,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,
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@linux.intel.com> --- arch/x86/kvm/vmx/nested.c | 17 +++++------------ arch/x86/kvm/vmx/vmx.c | 4 +++- 2 files changed, 8 insertions(+), 13 deletions(-)