[PATCH v2 0/2] KVM: nVMX: Disallow access to unsupported vmcs12 fields

[PATCH v2 0/2] KVM: nVMX: Disallow access to unsupported vmcs12 fields

[Sean Christopherson]

Disallow accesses to vmcs12 fields that are defined by KVM, but are unsupported
in the current incarnation of KVM, e.g. due to lack of hardware support for the
underlying VMCS fields.

The primary motivation is to avoid having to carry the same logic for shadowed
VMCS fields, which can't play nice with unsupported fields since VMREAD/VMWRITE
will fail when attempting to transfer state between vmcs12 and the shadow VMCS.

v2:
 - Name the array of KVM-defined fields kvm_supported_vmcs12_field_offsets,
   e.g. so that it's no confused with what's supported by hardware. [Xin]
 - Combine encodings in switch statements for fields shared fate. [Xin]
 - Drop the extern declaration of supported_vmcs12_field_offsets. [Chao]
 - Handle GUEST_INTR_STATUS in cpu_has_vmcs12_field() and add a patch to
   drop the custom handling from init_vmcs_shadow_fields(). [Chao]

v1: https://lore.kernel.org/all/20251216012918.1707681-1-seanjc@google.com

Sean Christopherson (2):
  KVM: nVMX: Disallow access to vmcs12 fields that aren't supported by
    "hardware"
  KVM: nVMX: Remove explicit filtering of GUEST_INTR_STATUS from shadow
    VMCS fields

 arch/x86/kvm/vmx/nested.c | 17 +++-------
 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, 86 insertions(+), 17 deletions(-)


base-commit: 9448598b22c50c8a5bb77a9103e2d49f134c9578
-- 
2.52.0.351.gbe84eed79e-goog

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

[Sean Christopherson]

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

[PATCH v2 2/2] KVM: nVMX: Remove explicit filtering of GUEST_INTR_STATUS from shadow VMCS fields

[Sean Christopherson]

Drop KVM's filtering of GUEST_INTR_STATUS when generating the shadow VMCS
bitmap now that KVM drops GUEST_INTR_STATUS from the set of supported
vmcs12 fields if the field isn't supported by hardware.

Note, there is technically a small functional change here, as the vmcs12
filtering only requires support for Virtual Interrupt Delivery, whereas
the shadow VMCS code being removed required "full" APICv support, i.e.
required Virtual Interrupt Delivery *and* APIC Register Virtualizaton *and*
Posted Interrupt support.

Opportunistically tweak the comment to more precisely explain why the
PML and VMX preemption timer fields need to be explicitly checked.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/vmx/nested.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 9d8f84e3f2da..f50d21a6a2d7 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -112,9 +112,10 @@ static void init_vmcs_shadow_fields(void)
 			  "Update vmcs12_write_any() to drop reserved bits from AR_BYTES");
 
 		/*
-		 * PML and the preemption timer can be emulated, but the
-		 * processor cannot vmwrite to fields that don't exist
-		 * on bare metal.
+		 * KVM emulates PML and the VMX preemption timer irrespective
+		 * of hardware support, but shadowing their related VMCS fields
+		 * requires hardware support as the CPU will reject VMWRITEs to
+		 * fields that don't exist.
 		 */
 		switch (field) {
 		case GUEST_PML_INDEX:
@@ -125,10 +126,6 @@ static void init_vmcs_shadow_fields(void)
 			if (!cpu_has_vmx_preemption_timer())
 				continue;
 			break;
-		case GUEST_INTR_STATUS:
-			if (!cpu_has_vmx_apicv())
-				continue;
-			break;
 		default:
 			break;
 		}
-- 
2.52.0.351.gbe84eed79e-goog

Re: [PATCH v2 2/2] KVM: nVMX: Remove explicit filtering of GUEST_INTR_STATUS from shadow VMCS fields

[Chao Gao]

On Tue, Dec 30, 2025 at 02:02:20PM -0800, Sean Christopherson wrote:
>Drop KVM's filtering of GUEST_INTR_STATUS when generating the shadow VMCS
>bitmap now that KVM drops GUEST_INTR_STATUS from the set of supported
>vmcs12 fields if the field isn't supported by hardware.

IIUC, the construction of the shadow VMCS bitmap and fields doesn't reference
"the set of supported vmcs12 fields".

So, with the filtering dropped, copy_shadow_to_vmcs12() and
copy_vmcs12_to_shadow() may access GUEST_INTR_STATUS on unsupported hardware.

Do we need something like this (i.e., don't shadow unsupported vmcs12 fields)

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index f50d21a6a2d7..08433b3713d2 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -127,6 +127,8 @@ static void init_vmcs_shadow_fields(void)
				continue;
			break;
		default:
+			if (!cpu_has_vmcs12_field(field))
+				continue;
			break;
		}

>
>Note, there is technically a small functional change here, as the vmcs12
>filtering only requires support for Virtual Interrupt Delivery, whereas
>the shadow VMCS code being removed required "full" APICv support, i.e.
>required Virtual Interrupt Delivery *and* APIC Register Virtualizaton *and*
>Posted Interrupt support.
>
>Opportunistically tweak the comment to more precisely explain why the
>PML and VMX preemption timer fields need to be explicitly checked.
>
>Signed-off-by: Sean Christopherson <seanjc@google.com>
>---
> arch/x86/kvm/vmx/nested.c | 11 ++++-------
> 1 file changed, 4 insertions(+), 7 deletions(-)
>
>diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
>index 9d8f84e3f2da..f50d21a6a2d7 100644
>--- a/arch/x86/kvm/vmx/nested.c
>+++ b/arch/x86/kvm/vmx/nested.c
>@@ -112,9 +112,10 @@ static void init_vmcs_shadow_fields(void)
> 			  "Update vmcs12_write_any() to drop reserved bits from AR_BYTES");
> 
> 		/*
>-		 * PML and the preemption timer can be emulated, but the
>-		 * processor cannot vmwrite to fields that don't exist
>-		 * on bare metal.
>+		 * KVM emulates PML and the VMX preemption timer irrespective
>+		 * of hardware support, but shadowing their related VMCS fields
>+		 * requires hardware support as the CPU will reject VMWRITEs to
>+		 * fields that don't exist.
> 		 */
> 		switch (field) {
> 		case GUEST_PML_INDEX:
>@@ -125,10 +126,6 @@ static void init_vmcs_shadow_fields(void)
> 			if (!cpu_has_vmx_preemption_timer())
> 				continue;
> 			break;
>-		case GUEST_INTR_STATUS:
>-			if (!cpu_has_vmx_apicv())
>-				continue;
>-			break;
> 		default:
> 			break;
> 		}
>-- 
>2.52.0.351.gbe84eed79e-goog
>

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

[Chao Gao]

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>

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

[Xiaoyao Li]

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"

[Xin Li]

> 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 2/2] KVM: nVMX: Remove explicit filtering of GUEST_INTR_STATUS from shadow VMCS fields

[Sean Christopherson]

On Wed, Dec 31, 2025, Chao Gao wrote:
> On Tue, Dec 30, 2025 at 02:02:20PM -0800, Sean Christopherson wrote:
> >Drop KVM's filtering of GUEST_INTR_STATUS when generating the shadow VMCS
> >bitmap now that KVM drops GUEST_INTR_STATUS from the set of supported
> >vmcs12 fields if the field isn't supported by hardware.
> 
> IIUC, the construction of the shadow VMCS bitmap and fields doesn't reference
> "the set of supported vmcs12 fields".

Argh, right you are.  I assumed init_vmcs_shadow_fields() would already verify
the field is a valid vmcs12 field, at least as a sanity check, but it doesn't.

> So, with the filtering dropped, copy_shadow_to_vmcs12() and
> copy_vmcs12_to_shadow() may access GUEST_INTR_STATUS on unsupported hardware.
> 
> Do we need something like this (i.e., don't shadow unsupported vmcs12 fields)
> 
> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> index f50d21a6a2d7..08433b3713d2 100644
> --- a/arch/x86/kvm/vmx/nested.c
> +++ b/arch/x86/kvm/vmx/nested.c
> @@ -127,6 +127,8 @@ static void init_vmcs_shadow_fields(void)
> 				continue;
> 			break;
> 		default:
> +			if (!cpu_has_vmcs12_field(field))

This can be

			if (get_vmcs12_field_offset(field) < 0)

And I think I'll put it outside the switch statement, because the requirement
applies to all fields, even those that have additional restrictions.

I also think it makes sense to have patch 1 call nested_vmx_setup_vmcs12_fields()
from nested_vmx_hardware_setup(), so that the ordering and dependency between
configuring vmcs12 fields and shadow VMCS fields can be explicitly documented.

Re: [PATCH v2 2/2] KVM: nVMX: Remove explicit filtering of GUEST_INTR_STATUS from shadow VMCS fields

[Chao Gao]

>> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
>> index f50d21a6a2d7..08433b3713d2 100644
>> --- a/arch/x86/kvm/vmx/nested.c
>> +++ b/arch/x86/kvm/vmx/nested.c
>> @@ -127,6 +127,8 @@ static void init_vmcs_shadow_fields(void)
>> 				continue;
>> 			break;
>> 		default:
>> +			if (!cpu_has_vmcs12_field(field))
>
>This can be
>
>			if (get_vmcs12_field_offset(field) < 0)
>
>And I think I'll put it outside the switch statement, because the requirement
>applies to all fields, even those that have additional restrictions.

Agree.

>
>I also think it makes sense to have patch 1 call nested_vmx_setup_vmcs12_fields()
>from nested_vmx_hardware_setup(), so that the ordering and dependency between
>configuring vmcs12 fields and shadow VMCS fields can be explicitly documented.

Looks good to me.