[PATCH] KVM: x86: use assign_bit() where applicable

[PATCH] KVM: x86: use assign_bit() where applicable

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

Convert open-coded if/else with set_bit/clear_bit and their
non-atomic __set_bit/__clear_bit variants to the assign_bit/__assign_bit
API.

Done with Coccinelle semantic patch:
    // set_bit -> clear_bit => assign_bit

    @@
    expression cond, bit, addr;
    @@

    -if (cond)
    -        set_bit(bit, addr);
    -else
    -        clear_bit(bit, addr);
    +assign_bit(bit, addr, cond);

    // clear_bit -> set_bit => assign_bit

    @@
    expression cond, bit, addr;
    @@

    -if (cond)
    -        clear_bit(bit, addr);
    -else
    -        set_bit(bit, addr);
    +assign_bit(bit, addr, !cond);

    // __set_bit -> __clear_bit => __assign_bit

    @@
    expression cond, bit, addr;
    @@

    -if (cond)
    -        __set_bit(bit, addr);
    -else
    -        __clear_bit(bit, addr);
    +__assign_bit(bit, addr, cond);

    // __clear_bit -> __set_bit => __assign_bit

    @@
    expression cond, bit, addr;
    @@

    -if (cond)
    -        __clear_bit(bit, addr);
    -else
    -        __set_bit(bit, addr);
    +__assign_bit(bit, addr, !cond);

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 arch/x86/kvm/hyperv.c  | 12 ++++--------
 arch/x86/kvm/svm/pmu.c |  6 ++----
 arch/x86/kvm/x86.c     | 11 +++--------
 3 files changed, 9 insertions(+), 20 deletions(-)

diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 8d2669d8ef34..c131d9a3c550 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -114,17 +114,13 @@ static void synic_update_vector(struct kvm_vcpu_hv_synic *synic,
 	if (vector < HV_SYNIC_FIRST_VALID_VECTOR)
 		return;
 
-	if (synic_has_vector_connected(synic, vector))
-		__set_bit(vector, synic->vec_bitmap);
-	else
-		__clear_bit(vector, synic->vec_bitmap);
+	__assign_bit(vector, synic->vec_bitmap,
+		     synic_has_vector_connected(synic, vector));
 
 	auto_eoi_old = !bitmap_empty(synic->auto_eoi_bitmap, 256);
 
-	if (synic_has_vector_auto_eoi(synic, vector))
-		__set_bit(vector, synic->auto_eoi_bitmap);
-	else
-		__clear_bit(vector, synic->auto_eoi_bitmap);
+	__assign_bit(vector, synic->auto_eoi_bitmap,
+		     synic_has_vector_auto_eoi(synic, vector));
 
 	auto_eoi_new = !bitmap_empty(synic->auto_eoi_bitmap, 256);
 
diff --git a/arch/x86/kvm/svm/pmu.c b/arch/x86/kvm/svm/pmu.c
index c18286545a7a..4c13a6345277 100644
--- a/arch/x86/kvm/svm/pmu.c
+++ b/arch/x86/kvm/svm/pmu.c
@@ -169,10 +169,8 @@ static int amd_pmu_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 			pmc->eventsel_hw = (data & ~AMD64_EVENTSEL_HOSTONLY) |
 					   AMD64_EVENTSEL_GUESTONLY;
 
-			if (data & AMD64_EVENTSEL_HOST_GUEST_MASK)
-				__set_bit(pmc->idx, pmu->pmc_has_mode_specific_enables);
-			else
-				__clear_bit(pmc->idx, pmu->pmc_has_mode_specific_enables);
+			__assign_bit(pmc->idx, pmu->pmc_has_mode_specific_enables,
+				     data & AMD64_EVENTSEL_HOST_GUEST_MASK);
 
 			kvm_pmu_request_counter_reprogram(pmc);
 		}
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index af3ceee714c9..a33ef4ab4e60 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3074,10 +3074,8 @@ static int kvm_vcpu_ioctl_x86_set_vcpu_events(struct kvm_vcpu *vcpu,
 #endif
 
 		if (lapic_in_kernel(vcpu)) {
-			if (events->smi.latched_init)
-				set_bit(KVM_APIC_INIT, &vcpu->arch.apic->pending_events);
-			else
-				clear_bit(KVM_APIC_INIT, &vcpu->arch.apic->pending_events);
+			assign_bit(KVM_APIC_INIT, &vcpu->arch.apic->pending_events,
+				   events->smi.latched_init);
 		}
 	}
 
@@ -7224,10 +7222,7 @@ static void set_or_clear_apicv_inhibit(unsigned long *inhibits,
 
 	BUILD_BUG_ON(ARRAY_SIZE(apicv_inhibits) != NR_APICV_INHIBIT_REASONS);
 
-	if (set)
-		__set_bit(reason, inhibits);
-	else
-		__clear_bit(reason, inhibits);
+	__assign_bit(reason, inhibits, set);
 
 	trace_kvm_apicv_inhibit_changed(reason, set, *inhibits);
 }
-- 
2.51.0

Re: [PATCH] KVM: x86: use assign_bit() where applicable

[Sean Christopherson]

On Sun, Sep 20, 2026, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> Convert open-coded if/else with set_bit/clear_bit and their
> non-atomic __set_bit/__clear_bit variants to the assign_bit/__assign_bit
> API.

...

> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  arch/x86/kvm/hyperv.c  | 12 ++++--------
>  arch/x86/kvm/svm/pmu.c |  6 ++----
>  arch/x86/kvm/x86.c     | 11 +++--------
>  3 files changed, 9 insertions(+), 20 deletions(-)
> 
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index 8d2669d8ef34..c131d9a3c550 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -114,17 +114,13 @@ static void synic_update_vector(struct kvm_vcpu_hv_synic *synic,
>  	if (vector < HV_SYNIC_FIRST_VALID_VECTOR)
>  		return;
>  
> -	if (synic_has_vector_connected(synic, vector))
> -		__set_bit(vector, synic->vec_bitmap);
> -	else
> -		__clear_bit(vector, synic->vec_bitmap);
> +	__assign_bit(vector, synic->vec_bitmap,
> +		     synic_has_vector_connected(synic, vector));
>  
>  	auto_eoi_old = !bitmap_empty(synic->auto_eoi_bitmap, 256);
>  
> -	if (synic_has_vector_auto_eoi(synic, vector))
> -		__set_bit(vector, synic->auto_eoi_bitmap);
> -	else
> -		__clear_bit(vector, synic->auto_eoi_bitmap);
> +	__assign_bit(vector, synic->auto_eoi_bitmap,
> +		     synic_has_vector_auto_eoi(synic, vector));

Am I the only one that finds the assign_bit() code signficantly harder to follow?
Maybe it's just that I haven't seen assign_bit() much, but I've come back to this
patch several times, and I've had the same reaction every time.  IMO, this is a
solution looking for a problem.

Re: [PATCH] KVM: x86: use assign_bit() where applicable

[Peng Fan]

Hi Sean,

On Mon, Sep 28, 2026 at 05:58:57PM -0700, Sean Christopherson wrote:
>On Sun, Sep 20, 2026, Peng Fan (OSS) wrote:
>> From: Peng Fan <peng.fan@nxp.com>
>> 
>> Convert open-coded if/else with set_bit/clear_bit and their
>> non-atomic __set_bit/__clear_bit variants to the assign_bit/__assign_bit
>> API.
>
>...
>
...
>>  
>> -	if (synic_has_vector_auto_eoi(synic, vector))
>> -		__set_bit(vector, synic->auto_eoi_bitmap);
>> -	else
>> -		__clear_bit(vector, synic->auto_eoi_bitmap);
>> +	__assign_bit(vector, synic->auto_eoi_bitmap,
>> +		     synic_has_vector_auto_eoi(synic, vector));
>
>Am I the only one that finds the assign_bit() code signficantly harder to follow?

No :) S390 maintainers also not like this.

>Maybe it's just that I haven't seen assign_bit() much, but I've come back to this
>patch several times, and I've had the same reaction every time.  IMO, this is a
>solution looking for a problem.
>

It might be easy to read if the test condition is just a simple value, not
a function call.

Free to drop this patch.

Thanks
Peng

>

Re: [PATCH] KVM: x86: use assign_bit() where applicable

[David Laight]

On Mon, 28 Sep 2026 17:58:57 -0700
Sean Christopherson <seanjc@google.com> wrote:

> On Sun, Sep 20, 2026, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> > 
> > Convert open-coded if/else with set_bit/clear_bit and their
> > non-atomic __set_bit/__clear_bit variants to the assign_bit/__assign_bit
> > API.  
> 
> ...
> 
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >  arch/x86/kvm/hyperv.c  | 12 ++++--------
> >  arch/x86/kvm/svm/pmu.c |  6 ++----
> >  arch/x86/kvm/x86.c     | 11 +++--------
> >  3 files changed, 9 insertions(+), 20 deletions(-)
> > 
> > diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> > index 8d2669d8ef34..c131d9a3c550 100644
> > --- a/arch/x86/kvm/hyperv.c
> > +++ b/arch/x86/kvm/hyperv.c
> > @@ -114,17 +114,13 @@ static void synic_update_vector(struct kvm_vcpu_hv_synic *synic,
> >  	if (vector < HV_SYNIC_FIRST_VALID_VECTOR)
> >  		return;
> >  
> > -	if (synic_has_vector_connected(synic, vector))
> > -		__set_bit(vector, synic->vec_bitmap);
> > -	else
> > -		__clear_bit(vector, synic->vec_bitmap);
> > +	__assign_bit(vector, synic->vec_bitmap,
> > +		     synic_has_vector_connected(synic, vector));
> >  
> >  	auto_eoi_old = !bitmap_empty(synic->auto_eoi_bitmap, 256);
> >  
> > -	if (synic_has_vector_auto_eoi(synic, vector))
> > -		__set_bit(vector, synic->auto_eoi_bitmap);
> > -	else
> > -		__clear_bit(vector, synic->auto_eoi_bitmap);
> > +	__assign_bit(vector, synic->auto_eoi_bitmap,
> > +		     synic_has_vector_auto_eoi(synic, vector));  
> 
> Am I the only one that finds the assign_bit() code signficantly harder to follow?
> Maybe it's just that I haven't seen assign_bit() much, but I've come back to this
> patch several times, and I've had the same reaction every time.  IMO, this is a
> solution looking for a problem.
> 

The same is pretty much true of __set_bit() and even BIT().
What is wrong with:
	if (synic_has_vector_auto_eoi(synic, vector))
		vector |= 1u << synic->auto_eoi_bitmap;
	else
		vector &= ~(1u << synic->auto_eoi_bitmap);
'Does what is sway on the tin'.

David