[PATCH v2 1/2] KVM: nVMX: Disallow access to vmcs12 fields that aren't supported by "hardware"

Sean Christopherson posted 2 patches 1 month, 1 week ago
[PATCH v2 1/2] KVM: nVMX: Disallow access to vmcs12 fields that aren't supported by "hardware"
Posted by Sean Christopherson 1 month, 1 week ago
Disallow access (VMREAD/VMWRITE) to fields that the loaded incarnation of
KVM doesn't support, e.g. due to lack of hardware support, as a middle
ground between allowing access to any vmcs12 field defined by KVM (current
behavior) and gating access based on the userspace-defined vCPU model (the
most correct, but costly, implementation).

Disallowing access to unsupported fields helps a tiny bit in terms of
closing the virtualization hole (see below), but the main motivation is to
avoid having to weed out unsupported fields when synchronizing between
vmcs12 and a shadow VMCS.  Because shadow VMCS accesses are done via
VMREAD and VMWRITE, KVM _must_ filter out unsupported fields (or eat
VMREAD/VMWRITE failures), and filtering out just shadow VMCS fields is
about the same amount of effort, and arguably much more confusing.

As a bonus, this also fixes a KVM-Unit-Test failure bug when running on
_hardware_ without support for TSC Scaling, which fails with the same
signature as the bug fixed by commit ba1f82456ba8 ("KVM: nVMX: Dynamically
compute max VMCS index for vmcs12"):

  FAIL: VMX_VMCS_ENUM.MAX_INDEX expected: 19, actual: 17

Dynamically computing the max VMCS index only resolved the issue where KVM
was hardcoding max index, but for CPUs with TSC Scaling, that was "good
enough".

Cc: Xin Li <xin@zytor.com>
Cc: Chao Gao <chao.gao@intel.com>
Cc: Yosry Ahmed <yosry.ahmed@linux.dev>
Link: https://lore.kernel.org/all/20251026201911.505204-22-xin@zytor.com
Link: https://lore.kernel.org/all/YR2Tf9WPNEzrE7Xg@google.com
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/vmx/nested.c |  6 ----
 arch/x86/kvm/vmx/vmcs.h   |  8 +++++
 arch/x86/kvm/vmx/vmcs12.c | 70 +++++++++++++++++++++++++++++++++++++--
 arch/x86/kvm/vmx/vmcs12.h |  6 ++--
 arch/x86/kvm/vmx/vmx.c    |  2 ++
 5 files changed, 82 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 6137e5307d0f..9d8f84e3f2da 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -7074,12 +7074,6 @@ void nested_vmx_set_vmcs_shadowing_bitmap(void)
 	}
 }
 
-/*
- * Indexing into the vmcs12 uses the VMCS encoding rotated left by 6.  Undo
- * that madness to get the encoding for comparison.
- */
-#define VMCS12_IDX_TO_ENC(idx) ((u16)(((u16)(idx) >> 6) | ((u16)(idx) << 10)))
-
 static u64 nested_vmx_calc_vmcs_enum_msr(void)
 {
 	/*
diff --git a/arch/x86/kvm/vmx/vmcs.h b/arch/x86/kvm/vmx/vmcs.h
index b25625314658..98281e019e38 100644
--- a/arch/x86/kvm/vmx/vmcs.h
+++ b/arch/x86/kvm/vmx/vmcs.h
@@ -11,7 +11,15 @@
 
 #include "capabilities.h"
 
+/*
+ * Indexing into the vmcs12 uses the VMCS encoding rotated left by 6 as a very
+ * rudimentary compression of the range of indices.  The compression ratio is
+ * good enough to allow KVM to use a (very sparsely populated) array without
+ * wasting too much memory, while the "algorithm" is fast enough to be used to
+ * lookup vmcs12 fields on-demand, e.g. for emulation.
+ */
 #define ROL16(val, n) ((u16)(((u16)(val) << (n)) | ((u16)(val) >> (16 - (n)))))
+#define VMCS12_IDX_TO_ENC(idx) ((u16)(((u16)(idx) >> 6) | ((u16)(idx) << 10)))
 
 struct vmcs_hdr {
 	u32 revision_id:31;
diff --git a/arch/x86/kvm/vmx/vmcs12.c b/arch/x86/kvm/vmx/vmcs12.c
index 4233b5ca9461..b92db4768346 100644
--- a/arch/x86/kvm/vmx/vmcs12.c
+++ b/arch/x86/kvm/vmx/vmcs12.c
@@ -9,7 +9,7 @@
 	FIELD(number, name),						\
 	[ROL16(number##_HIGH, 6)] = VMCS12_OFFSET(name) + sizeof(u32)
 
-const unsigned short vmcs12_field_offsets[] = {
+static const u16 kvm_supported_vmcs12_field_offsets[] __initconst = {
 	FIELD(VIRTUAL_PROCESSOR_ID, virtual_processor_id),
 	FIELD(POSTED_INTR_NV, posted_intr_nv),
 	FIELD(GUEST_ES_SELECTOR, guest_es_selector),
@@ -158,4 +158,70 @@ const unsigned short vmcs12_field_offsets[] = {
 	FIELD(HOST_SSP, host_ssp),
 	FIELD(HOST_INTR_SSP_TABLE, host_ssp_tbl),
 };
-const unsigned int nr_vmcs12_fields = ARRAY_SIZE(vmcs12_field_offsets);
+
+u16 vmcs12_field_offsets[ARRAY_SIZE(kvm_supported_vmcs12_field_offsets)] __ro_after_init;
+unsigned int nr_vmcs12_fields __ro_after_init;
+
+#define VMCS12_CASE64(enc) case enc##_HIGH: case enc
+
+static __init bool cpu_has_vmcs12_field(unsigned int idx)
+{
+	switch (VMCS12_IDX_TO_ENC(idx)) {
+	case VIRTUAL_PROCESSOR_ID:
+		return cpu_has_vmx_vpid();
+	case POSTED_INTR_NV:
+		return cpu_has_vmx_posted_intr();
+	VMCS12_CASE64(TSC_MULTIPLIER):
+		return cpu_has_vmx_tsc_scaling();
+	case TPR_THRESHOLD:
+	VMCS12_CASE64(VIRTUAL_APIC_PAGE_ADDR):
+		return cpu_has_vmx_tpr_shadow();
+	VMCS12_CASE64(APIC_ACCESS_ADDR):
+		return cpu_has_vmx_virtualize_apic_accesses();
+	VMCS12_CASE64(POSTED_INTR_DESC_ADDR):
+		return cpu_has_vmx_posted_intr();
+	case GUEST_INTR_STATUS:
+		return cpu_has_vmx_virtual_intr_delivery();
+	VMCS12_CASE64(VM_FUNCTION_CONTROL):
+	VMCS12_CASE64(EPTP_LIST_ADDRESS):
+		return cpu_has_vmx_vmfunc();
+	VMCS12_CASE64(EPT_POINTER):
+		return cpu_has_vmx_ept();
+	VMCS12_CASE64(XSS_EXIT_BITMAP):
+		return cpu_has_vmx_xsaves();
+	VMCS12_CASE64(ENCLS_EXITING_BITMAP):
+		return cpu_has_vmx_encls_vmexit();
+	VMCS12_CASE64(GUEST_IA32_PERF_GLOBAL_CTRL):
+	VMCS12_CASE64(HOST_IA32_PERF_GLOBAL_CTRL):
+		return cpu_has_load_perf_global_ctrl();
+	case SECONDARY_VM_EXEC_CONTROL:
+		return cpu_has_secondary_exec_ctrls();
+	case GUEST_S_CET:
+	case GUEST_SSP:
+	case GUEST_INTR_SSP_TABLE:
+	case HOST_S_CET:
+	case HOST_SSP:
+	case HOST_INTR_SSP_TABLE:
+		return cpu_has_load_cet_ctrl();
+
+	/* KVM always emulates PML and the VMX preemption timer in software. */
+	case GUEST_PML_INDEX:
+	case VMX_PREEMPTION_TIMER_VALUE:
+	default:
+		return true;
+	}
+}
+
+void __init nested_vmx_setup_vmcs12_fields(void)
+{
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(kvm_supported_vmcs12_field_offsets); i++) {
+		if (!kvm_supported_vmcs12_field_offsets[i] ||
+		    !cpu_has_vmcs12_field(i))
+			continue;
+
+		vmcs12_field_offsets[i] = kvm_supported_vmcs12_field_offsets[i];
+		nr_vmcs12_fields = i + 1;
+	}
+}
diff --git a/arch/x86/kvm/vmx/vmcs12.h b/arch/x86/kvm/vmx/vmcs12.h
index 4ad6b16525b9..f2c0721fe3e2 100644
--- a/arch/x86/kvm/vmx/vmcs12.h
+++ b/arch/x86/kvm/vmx/vmcs12.h
@@ -374,8 +374,10 @@ static inline void vmx_check_vmcs12_offsets(void)
 	CHECK_OFFSET(guest_pml_index, 996);
 }
 
-extern const unsigned short vmcs12_field_offsets[];
-extern const unsigned int nr_vmcs12_fields;
+extern u16 vmcs12_field_offsets[] __ro_after_init;
+extern unsigned int nr_vmcs12_fields __ro_after_init;
+
+void __init nested_vmx_setup_vmcs12_fields(void);
 
 static inline short get_vmcs12_field_offset(unsigned long field)
 {
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 6b96f7aea20b..e5ad3853f51d 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -8670,6 +8670,8 @@ __init int vmx_hardware_setup(void)
 	 * can hide/show features based on kvm_cpu_cap_has().
 	 */
 	if (nested) {
+		nested_vmx_setup_vmcs12_fields();
+
 		nested_vmx_setup_ctls_msrs(&vmcs_config, vmx_capability.ept);
 
 		r = nested_vmx_hardware_setup(kvm_vmx_exit_handlers);
-- 
2.52.0.351.gbe84eed79e-goog
Re: [PATCH v2 1/2] KVM: nVMX: Disallow access to vmcs12 fields that aren't supported by "hardware"
Posted by Xin Li 1 month, 1 week ago
> On Dec 30, 2025, at 2:02 PM, Sean Christopherson <seanjc@google.com> wrote:
> 
> Disallow access (VMREAD/VMWRITE) to fields that the loaded incarnation of
> KVM doesn't support, e.g. due to lack of hardware support, as a middle
> ground between allowing access to any vmcs12 field defined by KVM (current
> behavior) and gating access based on the userspace-defined vCPU model (the
> most correct, but costly, implementation).
> 
> Disallowing access to unsupported fields helps a tiny bit in terms of
> closing the virtualization hole (see below), but the main motivation is to
> avoid having to weed out unsupported fields when synchronizing between
> vmcs12 and a shadow VMCS.  Because shadow VMCS accesses are done via
> VMREAD and VMWRITE, KVM _must_ filter out unsupported fields (or eat
> VMREAD/VMWRITE failures), and filtering out just shadow VMCS fields is
> about the same amount of effort, and arguably much more confusing.
> 
> As a bonus, this also fixes a KVM-Unit-Test failure bug when running on
> _hardware_ without support for TSC Scaling, which fails with the same
> signature as the bug fixed by commit ba1f82456ba8 ("KVM: nVMX: Dynamically
> compute max VMCS index for vmcs12"):
> 
>  FAIL: VMX_VMCS_ENUM.MAX_INDEX expected: 19, actual: 17
> 
> Dynamically computing the max VMCS index only resolved the issue where KVM
> was hardcoding max index, but for CPUs with TSC Scaling, that was "good
> enough".
> 
> Cc: Xin Li <xin@zytor.com>
> Cc: Chao Gao <chao.gao@intel.com>
> Cc: Yosry Ahmed <yosry.ahmed@linux.dev>
> Link: https://lore.kernel.org/all/20251026201911.505204-22-xin@zytor.com
> Link: https://lore.kernel.org/all/YR2Tf9WPNEzrE7Xg@google.com
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Reviewed-by: Xin Li <xin@zytor.com <mailto:xin@zytor.com>>
Re: [PATCH v2 1/2] KVM: nVMX: Disallow access to vmcs12 fields that aren't supported by "hardware"
Posted by Xiaoyao Li 1 month, 1 week ago
On 12/31/2025 6:02 AM, Sean Christopherson wrote:
> +/*
> + * Indexing into the vmcs12 uses the VMCS encoding rotated left by 6 as a very
> + * rudimentary compression of the range of indices.  The compression ratio is
> + * good enough to allow KVM to use a (very sparsely populated) array without
> + * wasting too much memory, while the "algorithm" is fast enough to be used to
> + * lookup vmcs12 fields on-demand, e.g. for emulation.
> + */
>   #define ROL16(val, n) ((u16)(((u16)(val) << (n)) | ((u16)(val) >> (16 - (n)))))
> +#define VMCS12_IDX_TO_ENC(idx) ((u16)(((u16)(idx) >> 6) | ((u16)(idx) << 10)))

Put them together is really good.

And ROL16(val, 16-n) is exactly undoing ROL16(val, n), so that we can 
further do

---8<---
diff --git a/arch/x86/kvm/vmx/vmcs.h b/arch/x86/kvm/vmx/vmcs.h
index 98281e019e38..be2f410f82cd 100644
--- a/arch/x86/kvm/vmx/vmcs.h
+++ b/arch/x86/kvm/vmx/vmcs.h
@@ -19,7 +19,8 @@
   * lookup vmcs12 fields on-demand, e.g. for emulation.
   */
  #define ROL16(val, n) ((u16)(((u16)(val) << (n)) | ((u16)(val) >> (16 
- (n)))))
-#define VMCS12_IDX_TO_ENC(idx) ((u16)(((u16)(idx) >> 6) | ((u16)(idx) 
<< 10)))
+#define ENC_TO_VMCS12_IDX(enc) ROL16(enc, 6)
+#define VMCS12_IDX_TO_ENC(idx) ROL16(idx,10)

  struct vmcs_hdr {
         u32 revision_id:31;
diff --git a/arch/x86/kvm/vmx/vmcs12.c b/arch/x86/kvm/vmx/vmcs12.c
index b92db4768346..1ebe67c384ad 100644
--- a/arch/x86/kvm/vmx/vmcs12.c
+++ b/arch/x86/kvm/vmx/vmcs12.c
@@ -4,10 +4,10 @@
  #include "vmcs12.h"

  #define VMCS12_OFFSET(x) offsetof(struct vmcs12, x)
-#define FIELD(number, name)    [ROL16(number, 6)] = VMCS12_OFFSET(name)
+#define FIELD(number, name)    [ENC_TO_VMCS12_IDX(number)] = 
VMCS12_OFFSET(name)
  #define FIELD64(number, name)                                          \
         FIELD(number, name),                                            \
-       [ROL16(number##_HIGH, 6)] = VMCS12_OFFSET(name) + sizeof(u32)
+       [ENC_TO_VMCS12_IDX(number##_HIGH)] = VMCS12_OFFSET(name) + 
sizeof(u32)

  static const u16 kvm_supported_vmcs12_field_offsets[] __initconst = {
         FIELD(VIRTUAL_PROCESSOR_ID, virtual_processor_id),
Re: [PATCH v2 1/2] KVM: nVMX: Disallow access to vmcs12 fields that aren't supported by "hardware"
Posted by Chao Gao 1 month, 1 week ago
On Tue, Dec 30, 2025 at 02:02:19PM -0800, Sean Christopherson wrote:
>Disallow access (VMREAD/VMWRITE) to fields that the loaded incarnation of
>KVM doesn't support, e.g. due to lack of hardware support, as a middle
>ground between allowing access to any vmcs12 field defined by KVM (current
>behavior) and gating access based on the userspace-defined vCPU model (the
>most correct, but costly, implementation).
>
>Disallowing access to unsupported fields helps a tiny bit in terms of
>closing the virtualization hole (see below), but the main motivation is to
>avoid having to weed out unsupported fields when synchronizing between
>vmcs12 and a shadow VMCS.  Because shadow VMCS accesses are done via
>VMREAD and VMWRITE, KVM _must_ filter out unsupported fields (or eat
>VMREAD/VMWRITE failures), and filtering out just shadow VMCS fields is
>about the same amount of effort, and arguably much more confusing.
>
>As a bonus, this also fixes a KVM-Unit-Test failure bug when running on
>_hardware_ without support for TSC Scaling, which fails with the same
>signature as the bug fixed by commit ba1f82456ba8 ("KVM: nVMX: Dynamically
>compute max VMCS index for vmcs12"):
>
>  FAIL: VMX_VMCS_ENUM.MAX_INDEX expected: 19, actual: 17
>
>Dynamically computing the max VMCS index only resolved the issue where KVM
>was hardcoding max index, but for CPUs with TSC Scaling, that was "good
>enough".
>
>Cc: Xin Li <xin@zytor.com>
>Cc: Chao Gao <chao.gao@intel.com>
>Cc: Yosry Ahmed <yosry.ahmed@linux.dev>
>Link: https://lore.kernel.org/all/20251026201911.505204-22-xin@zytor.com
>Link: https://lore.kernel.org/all/YR2Tf9WPNEzrE7Xg@google.com
>Signed-off-by: Sean Christopherson <seanjc@google.com>

Reviewed-by: Chao Gao <chao.gao@intel.com>