Re: [PATCH 6/7] xen: Swap find_first_set_bit() for ffsl() - 1

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Thu, 2024-03-14 at 15:30 +0100, Jan Beulich wrote:
> On 13.03.2024 18:27, Andrew Cooper wrote:
> > --- a/xen/drivers/passthrough/x86/iommu.c
> > +++ b/xen/drivers/passthrough/x86/iommu.c
> > @@ -641,7 +641,7 @@ struct page_info *iommu_alloc_pgtable(struct
> > domain_iommu *hd,
> >      if ( contig_mask )
> >      {
> >          /* See pt-contig-markers.h for a description of the marker
> > scheme. */
> > -        unsigned int i, shift = find_first_set_bit(contig_mask);
> > +        unsigned int i, shift = ffsl(contig_mask) - 1;
> 
> The need for subtracting 1 is why personally I dislike ffs() / ffsl()
> (and
> why I think find_first_set_bit() and __ffs() (but no __ffsl()) were
> introduced).
> 
> But what I first of all would like to have clarification on is what
> your
> (perhaps just abstract at this point) plans are wrt ffz() / ffzl().
> Potential side-by-side uses would be odd now, and would continue to
> be odd
> if the difference in bit labeling was retained. Since we're switching
> to
> a consolidated set of basic helpers, such an anomaly would better not
> survive imo.
Right now, ffz() is defined as __ffs(~(x)), so and it seems to me
__ffs()/ffz() exist only as a Linux compatible, so I wanted as a part
of RISC-V patch series put into xen/linux-compat.h and just include
this header where it will be necessary:

#define __ffs(x) (ffs(~(x)) - 1)
#define ffz(x) __ffs(~(x))

Why should we care about ffzl()? It is not used in Xen, is it?

~ Oleksii