Re: [PATCH] KVM: drop the kvm_has_noapic_vcpu optimization

[Chao Gao <chao.gao@intel.com>]

The shortlog should be:

KVM: x86: Drop the kvm_has_noapic_vcpu optimization

see https://docs.kernel.org/process/maintainer-kvm-x86.html#shortlog

On Fri, Oct 18, 2024 at 12:08:45PM +0200, Bernhard Kauer wrote:
>It used a static key to avoid loading the lapic pointer from
>the vcpu->arch structure.  However, in the common case the load
>is from a hot cacheline and the CPU should be able to perfectly
>predict it. Thus there is no upside of this premature optimization.
>
>The downside is that code patching including an IPI to all CPUs
>is required whenever the first VM without an lapic is created or
>the last is destroyed.
>
>Signed-off-by: Bernhard Kauer <bk@alpico.io>
>---
> arch/x86/kvm/lapic.c | 4 ----
> arch/x86/kvm/lapic.h | 6 +-----
> arch/x86/kvm/x86.c   | 8 --------
> 3 files changed, 1 insertion(+), 17 deletions(-)
>
>diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
>index 2098dc689088..0c75b3cc01f2 100644
>--- a/arch/x86/kvm/lapic.c
>+++ b/arch/x86/kvm/lapic.c
>@@ -135,8 +135,6 @@ static inline int __apic_test_and_clear_vector(int vec, void *bitmap)
> 	return __test_and_clear_bit(VEC_POS(vec), (bitmap) + REG_POS(vec));
> }
> 
>-__read_mostly DEFINE_STATIC_KEY_FALSE(kvm_has_noapic_vcpu);
>-EXPORT_SYMBOL_GPL(kvm_has_noapic_vcpu);
> 
> __read_mostly DEFINE_STATIC_KEY_DEFERRED_FALSE(apic_hw_disabled, HZ);
> __read_mostly DEFINE_STATIC_KEY_DEFERRED_FALSE(apic_sw_disabled, HZ);
>@@ -2518,7 +2516,6 @@ void kvm_free_lapic(struct kvm_vcpu *vcpu)
> 	struct kvm_lapic *apic = vcpu->arch.apic;
> 
> 	if (!vcpu->arch.apic) {
>-		static_branch_dec(&kvm_has_noapic_vcpu);
> 		return;
> 	}

remove the curly brackets, i.e., just

	if (!vcpu->arch.apic)
		return;

> 
>@@ -2864,7 +2861,6 @@ int kvm_create_lapic(struct kvm_vcpu *vcpu)
> 	ASSERT(vcpu != NULL);
> 
> 	if (!irqchip_in_kernel(vcpu->kvm)) {
>-		static_branch_inc(&kvm_has_noapic_vcpu);
> 		return 0;
> 	}

ditto

> 
>diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
>index 1b8ef9856422..157af18c9fc8 100644
>--- a/arch/x86/kvm/lapic.h
>+++ b/arch/x86/kvm/lapic.h
>@@ -179,13 +179,9 @@ static inline u32 kvm_lapic_get_reg(struct kvm_lapic *apic, int reg_off)
> 	return __kvm_lapic_get_reg(apic->regs, reg_off);
> }
> 
>-DECLARE_STATIC_KEY_FALSE(kvm_has_noapic_vcpu);
>-
> static inline bool lapic_in_kernel(struct kvm_vcpu *vcpu)
> {
>-	if (static_branch_unlikely(&kvm_has_noapic_vcpu))
>-		return vcpu->arch.apic;
>-	return true;
>+	return vcpu->arch.apic;
> }
> 
> extern struct static_key_false_deferred apic_hw_disabled;
>diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>index 83fe0a78146f..ca73cae31f53 100644
>--- a/arch/x86/kvm/x86.c
>+++ b/arch/x86/kvm/x86.c
>@@ -9828,8 +9828,6 @@ void kvm_x86_vendor_exit(void)
> 	if (hypervisor_is_type(X86_HYPER_MS_HYPERV))
> 		clear_hv_tscchange_cb();
> #endif
>-	kvm_lapic_exit();
>-

why is this call removed?

> 	if (!boot_cpu_has(X86_FEATURE_CONSTANT_TSC)) {
> 		cpufreq_unregister_notifier(&kvmclock_cpufreq_notifier_block,
> 					    CPUFREQ_TRANSITION_NOTIFIER);
>@@ -14015,9 +14013,3 @@ static int __init kvm_x86_init(void)
> 	return 0;
> }
> module_init(kvm_x86_init);
>-
>-static void __exit kvm_x86_exit(void)
>-{
>-	WARN_ON_ONCE(static_branch_unlikely(&kvm_has_noapic_vcpu));
>-}
>-module_exit(kvm_x86_exit);
>-- 
>2.45.2
>
>