Skip to content

Commit b47b414

Browse files
sean-jcSasha Levin
authored andcommitted
KVM: x86/mmu: Resolve nx_huge_pages when kvm.ko is loaded
commit 1d0e848 upstream. Resolve nx_huge_pages to true/false when kvm.ko is loaded, leaving it as -1 is technically undefined behavior when its value is read out by param_get_bool(), as boolean values are supposed to be '0' or '1'. Alternatively, KVM could define a custom getter for the param, but the auto value doesn't depend on the vendor module in any way, and printing "auto" would be unnecessarily unfriendly to the user. In addition to fixing the undefined behavior, resolving the auto value also fixes the scenario where the auto value resolves to N and no vendor module is loaded. Previously, -1 would result in Y being printed even though KVM would ultimately disable the mitigation. Rename the existing MMU module init/exit helpers to clarify that they're invoked with respect to the vendor module, and add comments to document why KVM has two separate "module init" flows. ========================================================================= UBSAN: invalid-load in kernel/params.c:320:33 load of value 255 is not a valid value for type '_Bool' CPU: 6 PID: 892 Comm: tail Not tainted 5.17.0-rc3+ torvalds#799 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015 Call Trace: <TASK> dump_stack_lvl+0x34/0x44 ubsan_epilogue+0x5/0x40 __ubsan_handle_load_invalid_value.cold+0x43/0x48 param_get_bool.cold+0xf/0x14 param_attr_show+0x55/0x80 module_attr_show+0x1c/0x30 sysfs_kf_seq_show+0x93/0xc0 seq_read_iter+0x11c/0x450 new_sync_read+0x11b/0x1a0 vfs_read+0xf0/0x190 ksys_read+0x5f/0xe0 do_syscall_64+0x3b/0xc0 entry_SYSCALL_64_after_hwframe+0x44/0xae </TASK> ========================================================================= Fixes: b8e8c83 ("kvm: mmu: ITLB_MULTIHIT mitigation") Cc: [email protected] Reported-by: Bruno Goncalves <[email protected]> Reported-by: Jan Stancek <[email protected]> Signed-off-by: Sean Christopherson <[email protected]> Message-Id: <[email protected]> Signed-off-by: Paolo Bonzini <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent e5314cf commit b47b414

File tree

3 files changed

+37
-8
lines changed

3 files changed

+37
-8
lines changed

arch/x86/include/asm/kvm_host.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1574,8 +1574,9 @@ static inline int kvm_arch_flush_remote_tlb(struct kvm *kvm)
15741574
#define kvm_arch_pmi_in_guest(vcpu) \
15751575
((vcpu) && (vcpu)->arch.handling_intr_from_guest)
15761576

1577-
int kvm_mmu_module_init(void);
1578-
void kvm_mmu_module_exit(void);
1577+
void kvm_mmu_x86_module_init(void);
1578+
int kvm_mmu_vendor_module_init(void);
1579+
void kvm_mmu_vendor_module_exit(void);
15791580

15801581
void kvm_mmu_destroy(struct kvm_vcpu *vcpu);
15811582
int kvm_mmu_create(struct kvm_vcpu *vcpu);

arch/x86/kvm/mmu/mmu.c

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6144,12 +6144,24 @@ static int set_nx_huge_pages(const char *val, const struct kernel_param *kp)
61446144
return 0;
61456145
}
61466146

6147-
int kvm_mmu_module_init(void)
6147+
/*
6148+
* nx_huge_pages needs to be resolved to true/false when kvm.ko is loaded, as
6149+
* its default value of -1 is technically undefined behavior for a boolean.
6150+
*/
6151+
void kvm_mmu_x86_module_init(void)
61486152
{
6149-
int ret = -ENOMEM;
6150-
61516153
if (nx_huge_pages == -1)
61526154
__set_nx_huge_pages(get_nx_auto_mode());
6155+
}
6156+
6157+
/*
6158+
* The bulk of the MMU initialization is deferred until the vendor module is
6159+
* loaded as many of the masks/values may be modified by VMX or SVM, i.e. need
6160+
* to be reset when a potentially different vendor module is loaded.
6161+
*/
6162+
int kvm_mmu_vendor_module_init(void)
6163+
{
6164+
int ret = -ENOMEM;
61536165

61546166
/*
61556167
* MMU roles use union aliasing which is, generally speaking, an
@@ -6197,7 +6209,7 @@ void kvm_mmu_destroy(struct kvm_vcpu *vcpu)
61976209
mmu_free_memory_caches(vcpu);
61986210
}
61996211

6200-
void kvm_mmu_module_exit(void)
6212+
void kvm_mmu_vendor_module_exit(void)
62016213
{
62026214
mmu_destroy_caches();
62036215
percpu_counter_destroy(&kvm_total_used_mmu_pages);

arch/x86/kvm/x86.c

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8846,7 +8846,7 @@ int kvm_arch_init(void *opaque)
88468846
}
88478847
kvm_nr_uret_msrs = 0;
88488848

8849-
r = kvm_mmu_module_init();
8849+
r = kvm_mmu_vendor_module_init();
88508850
if (r)
88518851
goto out_free_percpu;
88528852

@@ -8894,7 +8894,7 @@ void kvm_arch_exit(void)
88948894
cancel_work_sync(&pvclock_gtod_work);
88958895
#endif
88968896
kvm_x86_ops.hardware_enable = NULL;
8897-
kvm_mmu_module_exit();
8897+
kvm_mmu_vendor_module_exit();
88988898
free_percpu(user_return_msrs);
88998899
kmem_cache_destroy(x86_emulator_cache);
89008900
#ifdef CONFIG_KVM_XEN
@@ -12887,3 +12887,19 @@ EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_vmgexit_enter);
1288712887
EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_vmgexit_exit);
1288812888
EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_vmgexit_msr_protocol_enter);
1288912889
EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_vmgexit_msr_protocol_exit);
12890+
12891+
static int __init kvm_x86_init(void)
12892+
{
12893+
kvm_mmu_x86_module_init();
12894+
return 0;
12895+
}
12896+
module_init(kvm_x86_init);
12897+
12898+
static void __exit kvm_x86_exit(void)
12899+
{
12900+
/*
12901+
* If module_init() is implemented, module_exit() must also be
12902+
* implemented to allow module unload.
12903+
*/
12904+
}
12905+
module_exit(kvm_x86_exit);

0 commit comments

Comments
 (0)