On 24.05.2024 22:03, Andrew Cooper wrote:
> --- a/xen/arch/x86/hvm/hpet.c
> +++ b/xen/arch/x86/hvm/hpet.c
> @@ -335,7 +335,7 @@ static void timer_sanitize_int_route(HPETState *h,
> unsigned int tn)
> * enabled pick the first irq.
> */
> timer_config(h, tn) |=
> - MASK_INSR(find_first_set_bit(timer_int_route_cap(h, tn)),
> + MASK_INSR(ffsl(timer_int_route_cap(h, tn)) - 1,
> HPET_TN_ROUTE);
> }
This can be just ffs().
> @@ -409,7 +409,7 @@ static int cf_check hpet_write(
> {
> bool active;
>
> - i = find_first_set_bit(new_val);
> + i = ffsl(new_val) - 1;
> if ( i >= HPET_TIMER_NUM )
> break;
This in principle can be, too, but would require a little further care.
> @@ -535,14 +535,14 @@ static int cf_check hpet_write(
> /* stop/start timers whos state was changed by this write. */
> while (stop_timers)
> {
> - i = find_first_set_bit(stop_timers);
> + i = ffsl(stop_timers) - 1;
> __clear_bit(i, &stop_timers);
> hpet_stop_timer(h, i, guest_time);
> }
>
> while (start_timers)
> {
> - i = find_first_set_bit(start_timers);
> + i = ffsl(start_timers) - 1;
> __clear_bit(i, &start_timers);
> hpet_set_timer(h, i, guest_time);
> }
Same here; in fact {start,stop}_timers are needlessly unsigned long in
the first place.
> --- a/xen/arch/x86/include/asm/pt-contig-markers.h
> +++ b/xen/arch/x86/include/asm/pt-contig-markers.h
> @@ -60,7 +60,7 @@ static bool pt_update_contig_markers(uint64_t *pt, unsigned
> int idx,
> /* Step 1: Reduce markers in lower numbered entries. */
> while ( i )
> {
> - b = find_first_set_bit(i);
> + b = ffsl(i) - 1;
> i &= ~(1U << b);
Considering i's type and the immediately following expression, this again
can easily be just ffs().
> --- a/xen/drivers/passthrough/amd/iommu_map.c
> +++ b/xen/drivers/passthrough/amd/iommu_map.c
> @@ -137,7 +137,7 @@ static void set_iommu_ptes_present(unsigned long pt_mfn,
> ASSERT(!pde->u);
>
> if ( pde > table )
> - ASSERT(pde->ign0 == find_first_set_bit(pde - table));
> + ASSERT(pde->ign0 == ffsl(pde - table) - 1);
pde pointing into the page starting at table, this can be ffs(), too.
Preferably with at least the easy adjustments done:
Reviewed-by: Jan Beulich <jbeul...@suse.com>
Jan
