[PATCH] KVM: update depracated jump label API

[PATCH] KVM: update depracated jump label API

[Cun Li]

The use of 'struct static_key' and 'static_key_false' is
deprecated. Use the new API.

Signed-off-by: Cun Li <cun.jia.li@gmail.com>
---
 arch/x86/kvm/lapic.h         | 6 +++---
 arch/x86/kvm/mmu/mmu_audit.c | 4 ++--
 arch/x86/kvm/x86.c           | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
index 4fb86e3a9dd3..b7aa76e2678e 100644
--- a/arch/x86/kvm/lapic.h
+++ b/arch/x86/kvm/lapic.h
@@ -176,7 +176,7 @@ extern struct static_key kvm_no_apic_vcpu;
 
 static inline bool lapic_in_kernel(struct kvm_vcpu *vcpu)
 {
-	if (static_key_false(&kvm_no_apic_vcpu))
+	if (static_branch_unlikely(&kvm_no_apic_vcpu))
 		return vcpu->arch.apic;
 	return true;
 }
@@ -185,7 +185,7 @@ extern struct static_key_deferred apic_hw_disabled;
 
 static inline int kvm_apic_hw_enabled(struct kvm_lapic *apic)
 {
-	if (static_key_false(&apic_hw_disabled.key))
+	if (static_branch_unlikely(&apic_hw_disabled.key))
 		return apic->vcpu->arch.apic_base & MSR_IA32_APICBASE_ENABLE;
 	return MSR_IA32_APICBASE_ENABLE;
 }
@@ -194,7 +194,7 @@ extern struct static_key_deferred apic_sw_disabled;
 
 static inline bool kvm_apic_sw_enabled(struct kvm_lapic *apic)
 {
-	if (static_key_false(&apic_sw_disabled.key))
+	if (static_branch_unlikely(&apic_sw_disabled.key))
 		return apic->sw_enabled;
 	return true;
 }
diff --git a/arch/x86/kvm/mmu/mmu_audit.c b/arch/x86/kvm/mmu/mmu_audit.c
index c8d51a37e2ce..8a4b3510151a 100644
--- a/arch/x86/kvm/mmu/mmu_audit.c
+++ b/arch/x86/kvm/mmu/mmu_audit.c
@@ -234,7 +234,7 @@ static void audit_vcpu_spte(struct kvm_vcpu *vcpu)
 }
 
 static bool mmu_audit;
-static struct static_key mmu_audit_key;
+static DEFINE_STATIC_KEY_FALSE(mmu_audit_key);
 
 static void __kvm_mmu_audit(struct kvm_vcpu *vcpu, int point)
 {
@@ -250,7 +250,7 @@ static void __kvm_mmu_audit(struct kvm_vcpu *vcpu, int point)
 
 static inline void kvm_mmu_audit(struct kvm_vcpu *vcpu, int point)
 {
-	if (static_key_false((&mmu_audit_key)))
+	if (static_branch_unlikely((&mmu_audit_key)))
 		__kvm_mmu_audit(vcpu, point);
 }
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 9a8969a6dd06..b8c05ef26942 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -10339,7 +10339,7 @@ bool kvm_vcpu_is_bsp(struct kvm_vcpu *vcpu)
 	return (vcpu->arch.apic_base & MSR_IA32_APICBASE_BSP) != 0;
 }
 
-struct static_key kvm_no_apic_vcpu __read_mostly;
+__read_mostly DEFINE_STATIC_KEY_FALSE(kvm_no_apic_vcpu);
 EXPORT_SYMBOL_GPL(kvm_no_apic_vcpu);
 
 void kvm_arch_sched_in(struct kvm_vcpu *vcpu, int cpu)
-- 
2.25.1

Re: [PATCH] KVM: update depracated jump label API

[Vitaly Kuznetsov]

Cun Li <cun.jia.li@gmail.com> writes:

> The use of 'struct static_key' and 'static_key_false' is
> deprecated. Use the new API.
>
> Signed-off-by: Cun Li <cun.jia.li@gmail.com>
> ---
>  arch/x86/kvm/lapic.h         | 6 +++---
>  arch/x86/kvm/mmu/mmu_audit.c | 4 ++--
>  arch/x86/kvm/x86.c           | 2 +-
>  3 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
> index 4fb86e3a9dd3..b7aa76e2678e 100644
> --- a/arch/x86/kvm/lapic.h
> +++ b/arch/x86/kvm/lapic.h
> @@ -176,7 +176,7 @@ extern struct static_key kvm_no_apic_vcpu;
>  
>  static inline bool lapic_in_kernel(struct kvm_vcpu *vcpu)
>  {
> -	if (static_key_false(&kvm_no_apic_vcpu))
> +	if (static_branch_unlikely(&kvm_no_apic_vcpu))
>  		return vcpu->arch.apic;
>  	return true;
>  }
> @@ -185,7 +185,7 @@ extern struct static_key_deferred apic_hw_disabled;
>  
>  static inline int kvm_apic_hw_enabled(struct kvm_lapic *apic)
>  {
> -	if (static_key_false(&apic_hw_disabled.key))
> +	if (static_branch_unlikely(&apic_hw_disabled.key))
>  		return apic->vcpu->arch.apic_base & MSR_IA32_APICBASE_ENABLE;
>  	return MSR_IA32_APICBASE_ENABLE;
>  }
> @@ -194,7 +194,7 @@ extern struct static_key_deferred apic_sw_disabled;
>  
>  static inline bool kvm_apic_sw_enabled(struct kvm_lapic *apic)
>  {
> -	if (static_key_false(&apic_sw_disabled.key))
> +	if (static_branch_unlikely(&apic_sw_disabled.key))
>  		return apic->sw_enabled;
>  	return true;
>  }
> diff --git a/arch/x86/kvm/mmu/mmu_audit.c b/arch/x86/kvm/mmu/mmu_audit.c
> index c8d51a37e2ce..8a4b3510151a 100644
> --- a/arch/x86/kvm/mmu/mmu_audit.c
> +++ b/arch/x86/kvm/mmu/mmu_audit.c
> @@ -234,7 +234,7 @@ static void audit_vcpu_spte(struct kvm_vcpu *vcpu)
>  }
>  
>  static bool mmu_audit;
> -static struct static_key mmu_audit_key;
> +static DEFINE_STATIC_KEY_FALSE(mmu_audit_key);
>  
>  static void __kvm_mmu_audit(struct kvm_vcpu *vcpu, int point)
>  {
> @@ -250,7 +250,7 @@ static void __kvm_mmu_audit(struct kvm_vcpu *vcpu, int point)
>  
>  static inline void kvm_mmu_audit(struct kvm_vcpu *vcpu, int point)
>  {
> -	if (static_key_false((&mmu_audit_key)))
> +	if (static_branch_unlikely((&mmu_audit_key)))
>  		__kvm_mmu_audit(vcpu, point);
>  }
>  
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 9a8969a6dd06..b8c05ef26942 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -10339,7 +10339,7 @@ bool kvm_vcpu_is_bsp(struct kvm_vcpu *vcpu)
>  	return (vcpu->arch.apic_base & MSR_IA32_APICBASE_BSP) != 0;
>  }
>  
> -struct static_key kvm_no_apic_vcpu __read_mostly;
> +__read_mostly DEFINE_STATIC_KEY_FALSE(kvm_no_apic_vcpu);
>  EXPORT_SYMBOL_GPL(kvm_no_apic_vcpu);
>  
>  void kvm_arch_sched_in(struct kvm_vcpu *vcpu, int cpu)

mmu_audit_key can only be true or false so it would also be nice to use 
static_branch_enable()/static_branch_disable() for it and not
static_key_slow_inc()/static_key_slow_dec() we currently use (as it
sounds weird to increment 'false').

kvm_no_apic_vcpu is different, we actually need to increase it with
every vCPU which doesn't have LAPIC but maybe we can at least switch to
static_branch_inc()/static_branch_dec(). It is still weird we initialize
it to 'false' but it seems to be a documented behavior. From
include/linux/jump_label.h:

"... Thus, static_branch_inc() can be thought of as a 'make more true'
 and static_branch_dec() as a 'make more false'"

so .. oh well.

-- 
Vitaly

Re: [PATCH] KVM: update depracated jump label API

[Cun Li]

Sorry for my late reply. Several final exams have delayed me these days.

Thank you for your detailed advice!

[PATCH] KVM: update depracated and inappropriate jump label API

[Cun Li]

The use of 'struct static_key' and 'static_key_false' is
deprecated. Use the new API.

mmu_audit_key can only be true or false so it would be nice to use
static_branch_enable()/static_branch_disable() for it and not
static_key_slow_inc()/static_key_slow_dec().

Signed-off-by: Cun Li <cun.jia.li@gmail.com>
---
 arch/x86/kvm/lapic.h         | 6 +++---
 arch/x86/kvm/mmu/mmu_audit.c | 8 ++++----
 arch/x86/kvm/x86.c           | 6 +++---
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
index 4fb86e3a9dd3..b7aa76e2678e 100644
--- a/arch/x86/kvm/lapic.h
+++ b/arch/x86/kvm/lapic.h
@@ -176,7 +176,7 @@ extern struct static_key kvm_no_apic_vcpu;
 
 static inline bool lapic_in_kernel(struct kvm_vcpu *vcpu)
 {
-	if (static_key_false(&kvm_no_apic_vcpu))
+	if (static_branch_unlikely(&kvm_no_apic_vcpu))
 		return vcpu->arch.apic;
 	return true;
 }
@@ -185,7 +185,7 @@ extern struct static_key_deferred apic_hw_disabled;
 
 static inline int kvm_apic_hw_enabled(struct kvm_lapic *apic)
 {
-	if (static_key_false(&apic_hw_disabled.key))
+	if (static_branch_unlikely(&apic_hw_disabled.key))
 		return apic->vcpu->arch.apic_base & MSR_IA32_APICBASE_ENABLE;
 	return MSR_IA32_APICBASE_ENABLE;
 }
@@ -194,7 +194,7 @@ extern struct static_key_deferred apic_sw_disabled;
 
 static inline bool kvm_apic_sw_enabled(struct kvm_lapic *apic)
 {
-	if (static_key_false(&apic_sw_disabled.key))
+	if (static_branch_unlikely(&apic_sw_disabled.key))
 		return apic->sw_enabled;
 	return true;
 }
diff --git a/arch/x86/kvm/mmu/mmu_audit.c b/arch/x86/kvm/mmu/mmu_audit.c
index c8d51a37e2ce..df3482784aa5 100644
--- a/arch/x86/kvm/mmu/mmu_audit.c
+++ b/arch/x86/kvm/mmu/mmu_audit.c
@@ -234,7 +234,7 @@ static void audit_vcpu_spte(struct kvm_vcpu *vcpu)
 }
 
 static bool mmu_audit;
-static struct static_key mmu_audit_key;
+static DEFINE_STATIC_KEY_FALSE(mmu_audit_key);
 
 static void __kvm_mmu_audit(struct kvm_vcpu *vcpu, int point)
 {
@@ -250,7 +250,7 @@ static void __kvm_mmu_audit(struct kvm_vcpu *vcpu, int point)
 
 static inline void kvm_mmu_audit(struct kvm_vcpu *vcpu, int point)
 {
-	if (static_key_false((&mmu_audit_key)))
+	if (static_branch_unlikely((&mmu_audit_key)))
 		__kvm_mmu_audit(vcpu, point);
 }
 
@@ -259,7 +259,7 @@ static void mmu_audit_enable(void)
 	if (mmu_audit)
 		return;
 
-	static_key_slow_inc(&mmu_audit_key);
+	static_branch_enable(&mmu_audit_key);
 	mmu_audit = true;
 }
 
@@ -268,7 +268,7 @@ static void mmu_audit_disable(void)
 	if (!mmu_audit)
 		return;
 
-	static_key_slow_dec(&mmu_audit_key);
+	static_branch_disable(&mmu_audit_key);
 	mmu_audit = false;
 }
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 9a8969a6dd06..6f460c3b8fb8 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -9955,7 +9955,7 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
 		if (kvm_apicv_activated(vcpu->kvm))
 			vcpu->arch.apicv_active = true;
 	} else
-		static_key_slow_inc(&kvm_no_apic_vcpu);
+		static_branch_inc(&kvm_no_apic_vcpu);
 
 	r = -ENOMEM;
 
@@ -10084,7 +10084,7 @@ void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
 	free_page((unsigned long)vcpu->arch.pio_data);
 	kvfree(vcpu->arch.cpuid_entries);
 	if (!lapic_in_kernel(vcpu))
-		static_key_slow_dec(&kvm_no_apic_vcpu);
+		static_branch_dec(&kvm_no_apic_vcpu);
 }
 
 void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
@@ -10339,7 +10339,7 @@ bool kvm_vcpu_is_bsp(struct kvm_vcpu *vcpu)
 	return (vcpu->arch.apic_base & MSR_IA32_APICBASE_BSP) != 0;
 }
 
-struct static_key kvm_no_apic_vcpu __read_mostly;
+__read_mostly DEFINE_STATIC_KEY_FALSE(kvm_no_apic_vcpu);
 EXPORT_SYMBOL_GPL(kvm_no_apic_vcpu);
 
 void kvm_arch_sched_in(struct kvm_vcpu *vcpu, int cpu)
-- 
2.25.1

Re: [PATCH] KVM: update depracated and inappropriate jump label API

[Sean Christopherson]

s/depracated/deprecated

And maybe reword the shortlog to be more explicit?  E.g.

  KVM: Stop using deprecated jump label APIs

On Sat, Jan 16, 2021, Cun Li wrote:
> The use of 'struct static_key' and 'static_key_false' is
> deprecated. Use the new API.
> 
> mmu_audit_key can only be true or false so it would be nice to use
> static_branch_enable()/static_branch_disable() for it and not
> static_key_slow_inc()/static_key_slow_dec().

Does anyone even build with CONFIG_KVM_MMU_AUDIT=y, let alone actually use the
auditing capabilities?

Re: [PATCH] KVM: update depracated jump label API

[Paolo Bonzini]

On 11/01/21 18:15, Vitaly Kuznetsov wrote:
> kvm_no_apic_vcpu is different, we actually need to increase it with
> every vCPU which doesn't have LAPIC but maybe we can at least switch to
> static_branch_inc()/static_branch_dec(). It is still weird we initialize
> it to 'false'

"kvm_no_apic_vcpu" is badly named.  It reads as "true if no vCPU has 
APIC" but it means "true if some vCPU has no APIC".  The latter is 
obviously false in the beginning, because there is no vCPUs at all.

Perhaps a better name would be "kvm_has_noapic_vcpu" (for once, 
smashingwordstogether is more readable than the alternative).

Paolo

  but it seems to be a documented behavior. From
> include/linux/jump_label.h:
> 
> "... Thus, static_branch_inc() can be thought of as a 'make more true'
>   and static_branch_dec() as a 'make more false'"

Re: [PATCH] KVM: update depracated jump label API

[Sean Christopherson]

On Tue, Jan 26, 2021, Paolo Bonzini wrote:
> On 11/01/21 18:15, Vitaly Kuznetsov wrote:
> > kvm_no_apic_vcpu is different, we actually need to increase it with
> > every vCPU which doesn't have LAPIC but maybe we can at least switch to
> > static_branch_inc()/static_branch_dec(). It is still weird we initialize
> > it to 'false'
> 
> "kvm_no_apic_vcpu" is badly named.  It reads as "true if no vCPU has APIC"
> but it means "true if some vCPU has no APIC".  The latter is obviously false
> in the beginning, because there is no vCPUs at all.
> 
> Perhaps a better name would be "kvm_has_noapic_vcpu" (for once,
> smashingwordstogether is more readable than the alternative).

+1