On Wed, 15 Jul 2026 at 16:30, Pierrick Bouvier
<[email protected]> wrote:
>
> On 7/15/26 3:01 AM, Peter Maydell wrote:
> > On Wed, 15 Jul 2026 at 02:18, Pierrick Bouvier
> > <[email protected]> wrote:
> >>
> >> On 7/14/2026 8:51 AM, Brian Cain wrote:
> >>> set_bit32()/test_bit32()/etc already let devices operate on
> >>> guest-visible uint32_t register arrays without depending on the
> >>> host's 'unsigned long' size. find_first_bit() has no such
> >>> equivalent, which pushes callers towards casting a uint32_t array
> >>> to 'unsigned long *'.
> >>>
> >>> Add find_first_bit32(), implemented the same way as find_first_bit().
> >>>
> >>> Signed-off-by: Brian Cain <[email protected]>
> >
> >>> +/**
> >>> + * find_first_bit32 - find the first set bit in a memory region
> >>> + * @addr: The address to start the search at
> >>> + * @size: The maximum size to search
> >>> + *
> >>> + * Returns the bit number of the first set bit,
> >>> + * or @size if there is no set bit in the bitmap.
> >>> + */
> >>> +static inline uint32_t find_first_bit32(const uint32_t *addr, uint32_t 
> >>> size)
> >>> +{
> >>> +    uint32_t result;
> >>> +
> >>> +    for (result = 0; result < size; result += 32) {
> >>> +        uint32_t tmp = *addr++;
> >>> +        if (tmp) {
> >>> +            result += ctz32(tmp);
> >>> +            return result < size ? result : size;
> >>> +        }
> >>> +    }
> >>> +    /* Not found */
> >>> +    return size;
> >>> +}
> >>
> >> Either we should express size in terms of number of elements, or add an
> >> explicit check if size is not a multiple of 32.
> >
> > I think we should follow the API of the existing find_first_bit()
> > function here, which defines the size in terms of number of bits.
> >
> > Sizes not a multiple of 32 should work fine -- the representation
> > is "bit 0 in LSB of first 32 bit word, working upwards". So e.g.
> > an bitmap of 16 bits has the representation "a single 32 bit word,
> > and the bits are in bits 0..15". The code correctly handles this
> > with the "return result < size ? result : size" logic: even if
> > bit 17 is set, we will return 16 to indicate "no set bit in bitmap".
> > (This also follows the existing find_first_bit() handling.)
> >
>
> What I had in mind with 'add an explicit check', was to make sure we
> don't read the last entry if size is not a multiple of 32.
> In case addr is a buffer, we might read past its last entry.
> But maybe it's not a concern.

I think if we didn't need to check it for the existing find_first_bit()
we aren't likely to need to check it here. If the caller has taken
something that's not actually an array of uint32_t and passed it
to this function then the code at the callsite probably looks suspicious
enough to be caught in code review.

-- PMM

Reply via email to