Skip to content

Commit 5fdf86e

Browse files
committed
KVM: nVMX: Disallow access to vmcs12 fields that aren't supported by "hardware"
Disallow access (VMREAD/VMWRITE), both emulated and via a shadow VMCS, to VMCS 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 functionally correct, but very 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 ba1f824 ("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". Reviewed-by: Chao Gao <chao.gao@intel.com> Reviewed-by: Xin Li <xin@zytor.com> Cc: Xiaoyao Li <xiaoyao.li@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 Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com> Link: https://patch.msgid.link/20260115173427.716021-4-seanjc@google.com Signed-off-by: Sean Christopherson <seanjc@google.com>
1 parent c68feb6 commit 5fdf86e

4 files changed

Lines changed: 92 additions & 10 deletions

File tree

arch/x86/kvm/vmx/nested.c

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,9 @@ static void init_vmcs_shadow_fields(void)
8686
pr_err("Missing field from shadow_read_only_field %x\n",
8787
field + 1);
8888

89+
if (get_vmcs12_field_offset(field) < 0)
90+
continue;
91+
8992
clear_bit(field, vmx_vmread_bitmap);
9093
if (field & 1)
9194
#ifdef CONFIG_X86_64
@@ -111,6 +114,9 @@ static void init_vmcs_shadow_fields(void)
111114
field <= GUEST_TR_AR_BYTES,
112115
"Update vmcs12_write_any() to drop reserved bits from AR_BYTES");
113116

117+
if (get_vmcs12_field_offset(field) < 0)
118+
continue;
119+
114120
/*
115121
* PML and the preemption timer can be emulated, but the
116122
* processor cannot vmwrite to fields that don't exist
@@ -7074,12 +7080,6 @@ void nested_vmx_set_vmcs_shadowing_bitmap(void)
70747080
}
70757081
}
70767082

7077-
/*
7078-
* Indexing into the vmcs12 uses the VMCS encoding rotated left by 6. Undo
7079-
* that madness to get the encoding for comparison.
7080-
*/
7081-
#define VMCS12_IDX_TO_ENC(idx) ((u16)(((u16)(idx) >> 6) | ((u16)(idx) << 10)))
7082-
70837083
static u64 nested_vmx_calc_vmcs_enum_msr(void)
70847084
{
70857085
/*
@@ -7407,6 +7407,12 @@ __init int nested_vmx_hardware_setup(int (*exit_handlers[])(struct kvm_vcpu *))
74077407
{
74087408
int i;
74097409

7410+
/*
7411+
* Note! The set of supported vmcs12 fields is consumed by both VMX
7412+
* MSR and shadow VMCS setup.
7413+
*/
7414+
nested_vmx_setup_vmcs12_fields();
7415+
74107416
nested_vmx_setup_ctls_msrs(&vmcs_config, vmx_capability.ept);
74117417

74127418
if (!cpu_has_vmx_shadow_vmcs())

arch/x86/kvm/vmx/vmcs.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,15 @@
1111

1212
#include "capabilities.h"
1313

14+
/*
15+
* Indexing into the vmcs12 uses the VMCS encoding rotated left by 6 as a very
16+
* rudimentary compression of the range of indices. The compression ratio is
17+
* good enough to allow KVM to use a (very sparsely populated) array without
18+
* wasting too much memory, while the "algorithm" is fast enough to be used to
19+
* lookup vmcs12 fields on-demand, e.g. for emulation.
20+
*/
1421
#define ROL16(val, n) ((u16)(((u16)(val) << (n)) | ((u16)(val) >> (16 - (n)))))
22+
#define VMCS12_IDX_TO_ENC(idx) ROL16(idx, 10)
1523
#define ENC_TO_VMCS12_IDX(enc) ROL16(enc, 6)
1624

1725
struct vmcs_hdr {

arch/x86/kvm/vmx/vmcs12.c

Lines changed: 68 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
FIELD(number, name), \
1010
[ENC_TO_VMCS12_IDX(number##_HIGH)] = VMCS12_OFFSET(name) + sizeof(u32)
1111

12-
const unsigned short vmcs12_field_offsets[] = {
12+
static const u16 kvm_supported_vmcs12_field_offsets[] __initconst = {
1313
FIELD(VIRTUAL_PROCESSOR_ID, virtual_processor_id),
1414
FIELD(POSTED_INTR_NV, posted_intr_nv),
1515
FIELD(GUEST_ES_SELECTOR, guest_es_selector),
@@ -158,4 +158,70 @@ const unsigned short vmcs12_field_offsets[] = {
158158
FIELD(HOST_SSP, host_ssp),
159159
FIELD(HOST_INTR_SSP_TABLE, host_ssp_tbl),
160160
};
161-
const unsigned int nr_vmcs12_fields = ARRAY_SIZE(vmcs12_field_offsets);
161+
162+
u16 vmcs12_field_offsets[ARRAY_SIZE(kvm_supported_vmcs12_field_offsets)] __ro_after_init;
163+
unsigned int nr_vmcs12_fields __ro_after_init;
164+
165+
#define VMCS12_CASE64(enc) case enc##_HIGH: case enc
166+
167+
static __init bool cpu_has_vmcs12_field(unsigned int idx)
168+
{
169+
switch (VMCS12_IDX_TO_ENC(idx)) {
170+
case VIRTUAL_PROCESSOR_ID:
171+
return cpu_has_vmx_vpid();
172+
case POSTED_INTR_NV:
173+
return cpu_has_vmx_posted_intr();
174+
VMCS12_CASE64(TSC_MULTIPLIER):
175+
return cpu_has_vmx_tsc_scaling();
176+
case TPR_THRESHOLD:
177+
VMCS12_CASE64(VIRTUAL_APIC_PAGE_ADDR):
178+
return cpu_has_vmx_tpr_shadow();
179+
VMCS12_CASE64(APIC_ACCESS_ADDR):
180+
return cpu_has_vmx_virtualize_apic_accesses();
181+
VMCS12_CASE64(POSTED_INTR_DESC_ADDR):
182+
return cpu_has_vmx_posted_intr();
183+
case GUEST_INTR_STATUS:
184+
return cpu_has_vmx_virtual_intr_delivery();
185+
VMCS12_CASE64(VM_FUNCTION_CONTROL):
186+
VMCS12_CASE64(EPTP_LIST_ADDRESS):
187+
return cpu_has_vmx_vmfunc();
188+
VMCS12_CASE64(EPT_POINTER):
189+
return cpu_has_vmx_ept();
190+
VMCS12_CASE64(XSS_EXIT_BITMAP):
191+
return cpu_has_vmx_xsaves();
192+
VMCS12_CASE64(ENCLS_EXITING_BITMAP):
193+
return cpu_has_vmx_encls_vmexit();
194+
VMCS12_CASE64(GUEST_IA32_PERF_GLOBAL_CTRL):
195+
VMCS12_CASE64(HOST_IA32_PERF_GLOBAL_CTRL):
196+
return cpu_has_load_perf_global_ctrl();
197+
case SECONDARY_VM_EXEC_CONTROL:
198+
return cpu_has_secondary_exec_ctrls();
199+
case GUEST_S_CET:
200+
case GUEST_SSP:
201+
case GUEST_INTR_SSP_TABLE:
202+
case HOST_S_CET:
203+
case HOST_SSP:
204+
case HOST_INTR_SSP_TABLE:
205+
return cpu_has_load_cet_ctrl();
206+
207+
/* KVM always emulates PML and the VMX preemption timer in software. */
208+
case GUEST_PML_INDEX:
209+
case VMX_PREEMPTION_TIMER_VALUE:
210+
default:
211+
return true;
212+
}
213+
}
214+
215+
void __init nested_vmx_setup_vmcs12_fields(void)
216+
{
217+
unsigned int i;
218+
219+
for (i = 0; i < ARRAY_SIZE(kvm_supported_vmcs12_field_offsets); i++) {
220+
if (!kvm_supported_vmcs12_field_offsets[i] ||
221+
!cpu_has_vmcs12_field(i))
222+
continue;
223+
224+
vmcs12_field_offsets[i] = kvm_supported_vmcs12_field_offsets[i];
225+
nr_vmcs12_fields = i + 1;
226+
}
227+
}

arch/x86/kvm/vmx/vmcs12.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -374,8 +374,10 @@ static inline void vmx_check_vmcs12_offsets(void)
374374
CHECK_OFFSET(guest_pml_index, 996);
375375
}
376376

377-
extern const unsigned short vmcs12_field_offsets[];
378-
extern const unsigned int nr_vmcs12_fields;
377+
extern u16 vmcs12_field_offsets[] __ro_after_init;
378+
extern unsigned int nr_vmcs12_fields __ro_after_init;
379+
380+
void __init nested_vmx_setup_vmcs12_fields(void);
379381

380382
static inline short get_vmcs12_field_offset(unsigned long field)
381383
{

0 commit comments

Comments
 (0)