On Tue, Feb 18, 2025 at 12:13:33PM +0100, Borislav Petkov wrote: > So, > > in the interest of finally making some progress here I'd like to commit this > below (will test it one more time just in case but it should work :-P). It is > simple and straight-forward and doesn't need an IBPB when the bit gets > cleared. > > A potential future improvement is David's suggestion that there could be a way > for tracking when the first guest gets started, we set the bit then, we make > sure the bit gets set on each logical CPU when the guests migrate across the > machine and when the *last* guest exists, that bit gets cleared again. Well, that "simplicity" was short-lived: https://www.phoronix.com/review/linux-615-amd-regression Sean, how about this below? It is hacky and RFC-ish - i.e., don't look too hard at it - but basically I'm pushing down into arch code the decision whether to enable virt on load. And it has no effects on anything else but machines which have this SRSO_MSR_FIX (Zen5). And it seems to work here - the MSR is set only when I create a VM - i.e., as expected. Thoughts? Better ideas? Thx. --- diff --git a/arch/x86/include/asm/kvm-x86-ops.h b/arch/x86/include/asm/kvm-x86-ops.h index 823c0434bbad..6cc8698df1a5 100644 --- a/arch/x86/include/asm/kvm-x86-ops.h +++ b/arch/x86/include/asm/kvm-x86-ops.h @@ -16,6 +16,7 @@ BUILD_BUG_ON(1) KVM_X86_OP(check_processor_compatibility) KVM_X86_OP(enable_virtualization_cpu) KVM_X86_OP(disable_virtualization_cpu) +KVM_X86_OP_OPTIONAL(enable_virt_on_load) KVM_X86_OP(hardware_unsetup) KVM_X86_OP(has_emulated_msr) KVM_X86_OP(vcpu_after_set_cpuid) diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index 3131abcac4f1..c1a29d7fee45 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -1664,6 +1664,7 @@ struct kvm_x86_ops { int (*enable_virtualization_cpu)(void); void (*disable_virtualization_cpu)(void); + bool (*enable_virt_on_load)(void); cpu_emergency_virt_cb *emergency_disable_virtualization_cpu; void (*hardware_unsetup)(void); diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c index 67657b3a36ce..dcbba55cb949 100644 --- a/arch/x86/kvm/svm/svm.c +++ b/arch/x86/kvm/svm/svm.c @@ -693,6 +693,16 @@ static int svm_enable_virtualization_cpu(void) return 0; } +static bool svm_enable_virt_on_load(void) +{ + bool ret = true; + + if (cpu_feature_enabled(X86_FEATURE_SRSO_BP_SPEC_REDUCE)) + ret = false; + + return ret; +} + static void svm_cpu_uninit(int cpu) { struct svm_cpu_data *sd = per_cpu_ptr(&svm_data, cpu); @@ -5082,6 +5092,7 @@ static struct kvm_x86_ops svm_x86_ops __initdata = { .hardware_unsetup = svm_hardware_unsetup, .enable_virtualization_cpu = svm_enable_virtualization_cpu, .disable_virtualization_cpu = svm_disable_virtualization_cpu, + .enable_virt_on_load = svm_enable_virt_on_load, .emergency_disable_virtualization_cpu = svm_emergency_disable_virtualization_cpu, .has_emulated_msr = svm_has_emulated_msr, diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 4c6553985e75..a09dc8cbd59f 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -12576,9 +12576,15 @@ void kvm_vcpu_deliver_sipi_vector(struct kvm_vcpu *vcpu, u8 vector) } EXPORT_SYMBOL_GPL(kvm_vcpu_deliver_sipi_vector); -void kvm_arch_enable_virtualization(void) +bool kvm_arch_enable_virtualization(bool allow_arch_override) { + if (allow_arch_override) + if (!kvm_x86_call(enable_virt_on_load)()) + return false; + cpu_emergency_register_virt_callback(kvm_x86_ops.emergency_disable_virtualization_cpu); + + return true; } void kvm_arch_disable_virtualization(void) diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index 291d49b9bf05..4353ef54d45d 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -1599,7 +1599,7 @@ static inline void kvm_create_vcpu_debugfs(struct kvm_vcpu *vcpu) {} * kvm_usage_count, i.e. at the beginning of the generic hardware enabling * sequence, and at the end of the generic hardware disabling sequence. */ -void kvm_arch_enable_virtualization(void); +bool kvm_arch_enable_virtualization(bool); void kvm_arch_disable_virtualization(void); /* * kvm_arch_{enable,disable}_virtualization_cpu() are called on "every" CPU to diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index e85b33a92624..0009661dee1d 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -143,8 +143,8 @@ static int kvm_no_compat_open(struct inode *inode, struct file *file) #define KVM_COMPAT(c) .compat_ioctl = kvm_no_compat_ioctl, \ .open = kvm_no_compat_open #endif -static int kvm_enable_virtualization(void); -static void kvm_disable_virtualization(void); +static int kvm_enable_virtualization(bool allow_arch_override); +static void kvm_disable_virtualization(bool allow_arch_override); static void kvm_io_bus_destroy(struct kvm_io_bus *bus); @@ -1187,7 +1187,7 @@ static struct kvm *kvm_create_vm(unsigned long type, const char *fdname) if (r) goto out_err_no_arch_destroy_vm; - r = kvm_enable_virtualization(); + r = kvm_enable_virtualization(false); if (r) goto out_err_no_disable; @@ -1224,7 +1224,7 @@ static struct kvm *kvm_create_vm(unsigned long type, const char *fdname) mmu_notifier_unregister(&kvm->mmu_notifier, current->mm); #endif out_err_no_mmu_notifier: - kvm_disable_virtualization(); + kvm_disable_virtualization(false); out_err_no_disable: kvm_arch_destroy_vm(kvm); out_err_no_arch_destroy_vm: @@ -1320,7 +1320,7 @@ static void kvm_destroy_vm(struct kvm *kvm) #endif kvm_arch_free_vm(kvm); preempt_notifier_dec(); - kvm_disable_virtualization(); + kvm_disable_virtualization(false); mmdrop(mm); } @@ -5489,9 +5489,9 @@ static DEFINE_PER_CPU(bool, virtualization_enabled); static DEFINE_MUTEX(kvm_usage_lock); static int kvm_usage_count; -__weak void kvm_arch_enable_virtualization(void) +__weak bool kvm_arch_enable_virtualization(bool) { - + return false; } __weak void kvm_arch_disable_virtualization(void) @@ -5589,8 +5589,9 @@ static struct syscore_ops kvm_syscore_ops = { .shutdown = kvm_shutdown, }; -static int kvm_enable_virtualization(void) +static int kvm_enable_virtualization(bool allow_arch_override) { + bool do_init; int r; guard(mutex)(&kvm_usage_lock); @@ -5598,7 +5599,9 @@ static int kvm_enable_virtualization(void) if (kvm_usage_count++) return 0; - kvm_arch_enable_virtualization(); + do_init = kvm_arch_enable_virtualization(allow_arch_override); + if (!do_init) + goto out; r = cpuhp_setup_state(CPUHP_AP_KVM_ONLINE, "kvm/cpu:online", kvm_online_cpu, kvm_offline_cpu); @@ -5631,11 +5634,13 @@ static int kvm_enable_virtualization(void) cpuhp_remove_state(CPUHP_AP_KVM_ONLINE); err_cpuhp: kvm_arch_disable_virtualization(); + +out: --kvm_usage_count; return r; } -static void kvm_disable_virtualization(void) +static void kvm_disable_virtualization(bool allow_arch_override) { guard(mutex)(&kvm_usage_lock); @@ -5650,7 +5655,7 @@ static void kvm_disable_virtualization(void) static int kvm_init_virtualization(void) { if (enable_virt_at_load) - return kvm_enable_virtualization(); + return kvm_enable_virtualization(true); return 0; } @@ -5658,10 +5663,10 @@ static int kvm_init_virtualization(void) static void kvm_uninit_virtualization(void) { if (enable_virt_at_load) - kvm_disable_virtualization(); + kvm_disable_virtualization(true); } #else /* CONFIG_KVM_GENERIC_HARDWARE_ENABLING */ -static int kvm_enable_virtualization(void) +static int kvm_enable_virtualization(bool allow_arch_override) { return 0; } @@ -5671,7 +5676,7 @@ static int kvm_init_virtualization(void) return 0; } -static void kvm_disable_virtualization(void) +static void kvm_disable_virtualization(bool allow_arch_override) { } -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
