Re: [PATCH v2 05/13] xen/bitops: Implement generic_f?sl() in lib/

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 27 May 2024, Jan Beulich wrote:
> On 24.05.2024 22:03, Andrew Cooper wrote:
> > generic_f?s() being static inline is the cause of lots of the complexity
> > between the common and arch-specific bitops.h
> > 
> > They appear to be static inline for constant-folding reasons (ARM uses them
> > for this), but there are better ways to achieve the same effect.
> > 
> > It is presumptuous that an unrolled binary search is the right algorithm to
> > use on all microarchitectures.  Indeed, it's not for the eventual users, but
> > that can be addressed at a later point.
> > 
> > It is also nonsense to implement the int form as the base primitive and
> > construct the long form from 2x int in 64-bit builds, when it's just one 
> > extra
> > step to operate at the native register width.
> > 
> > Therefore, implement generic_f?sl() in lib/.  They're not actually needed in
> > x86/ARM/PPC by the end of the cleanup (i.e. the functions will be dropped by
> > the linker), and they're only expected be needed by RISC-V on hardware which
> > lacks the Zbb extension.
> > 
> > Implement generic_fls() in terms of generic_flsl() for now, but this will be
> > cleaned up in due course.
> > 
> > Provide basic runtime testing using __constructor inside the lib/ file.  
> > This
> > is important, as it means testing runs if and only if generic_f?sl() are 
> > used
> > elsewhere in Xen.
> > 
> > Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> 
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> with a suggestion and a question.

Acked-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


> > I suspect we want to swap CONFIG_DEBUG for CONFIG_BOOT_UNIT_TESTS in due
> > course.  These ought to be able to be used in a release build too.
> 
> +1

+1