On 7/15/2026 9:05 AM, Peter Maydell wrote:
> 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.
>

Works for me as long as you're happy with it also.
@Brian: you can ignore my original comment.

Reviewed-by: Pierrick Bouvier <[email protected]>

> -- PMM

Regards,
Pierrick

Reply via email to