[PATCH v2 0/2] Cleanup VMFUNC handling in KVM.

[PATCH v2 0/2] Cleanup VMFUNC handling in KVM.

[Yu Zhang]

Since VMFUNC is not supported for non-nested guests, and executing VMFUNC
can cause a #UD directly, if the “enable VM functions” VM-execution control
is 0, KVM can just disable it in VM-exectution control, instead of taking
pains to trap it and emulate the #UD for L1 guests.

Also, simplified the process of setting SECONDARY_EXEC_ENABLE_VMFUNC for
nested VMX MSR configurations.

Change log:
==========
v1->v2
- Split the patch into two pieces.
- Use KVM_BUG_ON() for unexpected VM Exits.
- Comments changes.
- Commit message changes, trying to better illustrate the reason.

Yu Zhang (2):
  KVM: VMX: Do not trap VMFUNC instructions for L1 guests.
  KVM: nVMX: Simplify the setting of SECONDARY_EXEC_ENABLE_VMFUNC for
    nested.

 arch/x86/kvm/vmx/nested.c | 26 ++++++++++----------------
 arch/x86/kvm/vmx/vmx.c    |  7 ++++++-
 2 files changed, 16 insertions(+), 17 deletions(-)

-- 
2.17.1

[PATCH v2 1/2] KVM: VMX: Do not trap VMFUNC instructions for L1 guests.

[Yu Zhang]

Currently, although KVM does not support VMFUNC for L1 guests,
it still traps the VMFUNC instructions. This is done by setting
SECONDARY_EXEC_ENABLE_VMFUNC in the VM-execution control and 0
to VM-function control. And then in the VM exit handler, a #UD
is injected to L1 guest.

But for non-nested, KVM do not need to trap VMFUNC at all. According
to Intel SDM Volume3 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.

So just disable SECONDARY_EXEC_ENABLE_VMFUNC in VM-execution
control for L1 guests.

Signed-off-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Yu Zhang <yu.c.zhang@linux.intel.com>
---
 arch/x86/kvm/vmx/nested.c | 12 +++++-------
 arch/x86/kvm/vmx/vmx.c    |  7 ++++++-
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 0c62352dda6a..1acb81c2be11 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -5793,14 +5793,12 @@ 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. Executing VMFUNC
+	 * in non-nested guests shall receive #UD directly, instead of
+	 * trigerring a VM-Exit.
 	 */
-	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,
-- 
2.17.1

[PATCH v2 2/2] KVM: nVMX: Simplify the setting of SECONDARY_EXEC_ENABLE_VMFUNC for nested.

[Yu Zhang]

Values of base settings for nested proc-based VM-Execution control MSR come
from the ones for non-nested. And for SECONDARY_EXEC_ENABLE_VMFUNC flag,
KVM currently a) first mask off it from vmcs_conf->cpu_based_2nd_exec_ctrl;
b) then check it against the same source; c) and reset it again if host has
it.

So just simplify this, by not masking off SECONDARY_EXEC_ENABLE_VMFUNC in
the first place.

No functional change.

Signed-off-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Yu Zhang <yu.c.zhang@linux.intel.com>
---
 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 1acb81c2be11..2bad79985c10 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -6806,6 +6806,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;
@@ -6837,18 +6838,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;
 	}
 
 	/*
-- 
2.17.1

Re: [PATCH v2 0/2] Cleanup VMFUNC handling in KVM.

[Yu Zhang]

On Wed, Nov 09, 2022 at 03:54:11PM +0800, Yu Zhang wrote:

Ping... Thanks!

B.R.
Yu

Re: [PATCH v2 0/2] Cleanup VMFUNC handling in KVM.

[Sean Christopherson]

On Wed, 09 Nov 2022 15:54:11 +0800, Yu Zhang wrote:
> Since VMFUNC is not supported for non-nested guests, and executing VMFUNC
> can cause a #UD directly, if the “enable VM functions” VM-execution control
> is 0, KVM can just disable it in VM-exectution control, instead of taking
> pains to trap it and emulate the #UD for L1 guests.
> 
> Also, simplified the process of setting SECONDARY_EXEC_ENABLE_VMFUNC for
> nested VMX MSR configurations.
> 
> [...]

After much waffling, applied to kvm-x86 vmx.  I ended up keeping the logic to
inject #UD on now-unexpected VMFUNC exits from L1, i.e. patch one does nothing
more than clear the control bit.  I like the idea of clearing the control bit as
it more explicitly documents what's going on, but killing the VM on an unexpected
exit that KVM can gracefully handle seemed unnecessary.

Thanks!

[1/2] KVM: VMX: Do not trap VMFUNC instructions for L1 guests.
      https://github.com/kvm-x86/linux/commit/41acdd419735
[2/2] KVM: nVMX: Simplify the setting of SECONDARY_EXEC_ENABLE_VMFUNC for nested.
      https://github.com/kvm-x86/linux/commit/496c917b0989

--
https://github.com/kvm-x86/linux/tree/next
https://github.com/kvm-x86/linux/tree/fixes

Re: [PATCH v2 0/2] Cleanup VMFUNC handling in KVM.

[Yu Zhang]

> 
> After much waffling, applied to kvm-x86 vmx.  I ended up keeping the logic to
> inject #UD on now-unexpected VMFUNC exits from L1, i.e. patch one does nothing
> more than clear the control bit.  I like the idea of clearing the control bit as
> it more explicitly documents what's going on, but killing the VM on an unexpected
> exit that KVM can gracefully handle seemed unnecessary.

Glad to know that. Thanks!

B.R.
Yu