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().
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com>
Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com>
---
include/qemu/bitops.h | 30 ++++++++++++++++++++++++++----
1 file changed, 26 insertions(+), 4 deletions(-)
diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h
index c7b838a6283..f7363a5479f 100644
--- a/include/qemu/bitops.h
+++ b/include/qemu/bitops.h
@@ -43,10 +43,9 @@
* be some guest-visible register view of the bit array.
*
* We do not currently implement uint32_t versions of find_last_bit(),
- * find_next_bit(), find_next_zero_bit(), find_first_bit() or
- * find_first_zero_bit(), because we haven't yet needed them. If you
- * need them you should implement them similarly to the 'unsigned long'
- * versions.
+ * find_next_bit(), find_next_zero_bit() or find_first_zero_bit(),
+ * because we haven't yet needed them. If you need them you should
+ * implement them similarly to the 'unsigned long' versions.
*
* You can declare a bitmap to be used with these functions via the
* DECLARE_BITMAP and DECLARE_BITMAP32 macros in bitmap.h.
@@ -382,6 +381,29 @@ static inline int test_bit32(long nr, const uint32_t *addr)
return 1U & (addr[BIT32_WORD(nr)] >> (nr & 31));
}
+/**
+ * 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;
+}
+
/**
* DOC: Miscellaneous bit operations on single values
*
--
2.34.1
Hi Brian,
some random thoughts...
On 15/7/26 20:56, 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().
>
> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com>
> Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com>
> ---
> include/qemu/bitops.h | 30 ++++++++++++++++++++++++++----
> 1 file changed, 26 insertions(+), 4 deletions(-)
> +/**
> + * find_first_bit32 - find the first set bit in a memory region
> + * @addr: The address to start the search at
s/addr/ptr/?
> + * @size: The maximum size to search
Pre-existing, it is not clear this is expressed in bits. Neither if
we expect it to be a multiple of 32, or @addr to be 32-bit aligbed.
Maybe rename as @last_searched_bit?
> + *
> + * Returns the bit number of the first set bit,
"Returns the index of the first 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)
Could we simply return a plain unsigned? Also take a size_t?
> +{
> + 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;
> +}
On Mon, 20 Jul 2026 at 08:49, Philippe Mathieu-Daudé <philmd@oss.qualcomm.com> wrote: > > Hi Brian, > > some random thoughts... > > On 15/7/26 20:56, 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(). > > > > Reviewed-by: Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com> > > Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com> > > --- > > include/qemu/bitops.h | 30 ++++++++++++++++++++++++++---- > > 1 file changed, 26 insertions(+), 4 deletions(-) > > > > +/** > > + * find_first_bit32 - find the first set bit in a memory region > > + * @addr: The address to start the search at > > s/addr/ptr/? > > > + * @size: The maximum size to search > > Pre-existing, it is not clear this is expressed in bits. Neither if > we expect it to be a multiple of 32, or @addr to be 32-bit aligbed. > > Maybe rename as @last_searched_bit? Could we please stop bikeshedding this function? It's a simple copy of the existing find_first_bit() but with the type changed from 'unsigned long' to 'uint32_t'. The comments and parameter names on that existing function have been fine for years, and having the new function pick different names and comment phrasing seems unnecessary and obscures the parallels between the two functions. thanks -- PMM
On 20/7/26 10:31, Peter Maydell wrote: > On Mon, 20 Jul 2026 at 08:49, Philippe Mathieu-Daudé > <philmd@oss.qualcomm.com> wrote: >> >> Hi Brian, >> >> some random thoughts... >> >> On 15/7/26 20:56, 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(). >>> >>> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com> >>> Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com> >>> --- >>> include/qemu/bitops.h | 30 ++++++++++++++++++++++++++---- >>> 1 file changed, 26 insertions(+), 4 deletions(-) >> >> >>> +/** >>> + * find_first_bit32 - find the first set bit in a memory region >>> + * @addr: The address to start the search at >> >> s/addr/ptr/? >> >>> + * @size: The maximum size to search >> >> Pre-existing, it is not clear this is expressed in bits. Neither if >> we expect it to be a multiple of 32, or @addr to be 32-bit aligbed. >> >> Maybe rename as @last_searched_bit? > > Could we please stop bikeshedding this function? It's > a simple copy of the existing find_first_bit() but > with the type changed from 'unsigned long' to 'uint32_t'. > The comments and parameter names on that existing function > have been fine for years, and having the new function > pick different names and comment phrasing seems unnecessary > and obscures the parallels between the two functions. Argument renaming of course implies renaming the other companion methods. You taught me to add public APIs with good descriptions when possible, I thought your advice was worth to other contributors. I'm not considering this review comment as bikeshedding. Anyway, thanks for your review and advice. I don't object if Brian keeps the current names or rename as @a, @b, @foo, @bar. Thanks.
On Mon, 20 Jul 2026 at 12:46, Philippe Mathieu-Daudé <philmd@oss.qualcomm.com> wrote: > > On 20/7/26 10:31, Peter Maydell wrote: > > On Mon, 20 Jul 2026 at 08:49, Philippe Mathieu-Daudé > > <philmd@oss.qualcomm.com> wrote: > >> > >> Hi Brian, > >> > >> some random thoughts... > >> > >> On 15/7/26 20:56, 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(). > >>> > >>> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com> > >>> Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com> > >>> --- > >>> include/qemu/bitops.h | 30 ++++++++++++++++++++++++++---- > >>> 1 file changed, 26 insertions(+), 4 deletions(-) > >> > >> > >>> +/** > >>> + * find_first_bit32 - find the first set bit in a memory region > >>> + * @addr: The address to start the search at > >> > >> s/addr/ptr/? > >> > >>> + * @size: The maximum size to search > >> > >> Pre-existing, it is not clear this is expressed in bits. Neither if > >> we expect it to be a multiple of 32, or @addr to be 32-bit aligbed. > >> > >> Maybe rename as @last_searched_bit? > > > > Could we please stop bikeshedding this function? It's > > a simple copy of the existing find_first_bit() but > > with the type changed from 'unsigned long' to 'uint32_t'. > > The comments and parameter names on that existing function > > have been fine for years, and having the new function > > pick different names and comment phrasing seems unnecessary > > and obscures the parallels between the two functions. > > Argument renaming of course implies renaming the other companion > methods. You taught me to add public APIs with good descriptions > when possible, I thought your advice was worth to other contributors. > I'm not considering this review comment as bikeshedding. Anyway, > thanks for your review and advice. I don't object if Brian keeps > the current names or rename as @a, @b, @foo, @bar. Sorry for being a bit grumpy -- it's just that this is the second time around of somebody suggesting a change here to something that's already present in the existing function this is patterned off (and which we borrowed from the Linux kernel originally). -- PMM
On 20/7/26 13:51, Peter Maydell wrote: > On Mon, 20 Jul 2026 at 12:46, Philippe Mathieu-Daudé > <philmd@oss.qualcomm.com> wrote: >> >> On 20/7/26 10:31, Peter Maydell wrote: >>> On Mon, 20 Jul 2026 at 08:49, Philippe Mathieu-Daudé >>> <philmd@oss.qualcomm.com> wrote: >>>> >>>> Hi Brian, >>>> >>>> some random thoughts... >>>> >>>> On 15/7/26 20:56, 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(). >>>>> >>>>> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com> >>>>> Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com> >>>>> --- >>>>> include/qemu/bitops.h | 30 ++++++++++++++++++++++++++---- >>>>> 1 file changed, 26 insertions(+), 4 deletions(-) >>>> >>>> >>>>> +/** >>>>> + * find_first_bit32 - find the first set bit in a memory region >>>>> + * @addr: The address to start the search at >>>> >>>> s/addr/ptr/? >>>> >>>>> + * @size: The maximum size to search >>>> >>>> Pre-existing, it is not clear this is expressed in bits. Neither if >>>> we expect it to be a multiple of 32, or @addr to be 32-bit aligbed. >>>> >>>> Maybe rename as @last_searched_bit? >>> >>> Could we please stop bikeshedding this function? It's >>> a simple copy of the existing find_first_bit() but >>> with the type changed from 'unsigned long' to 'uint32_t'. >>> The comments and parameter names on that existing function >>> have been fine for years, and having the new function >>> pick different names and comment phrasing seems unnecessary >>> and obscures the parallels between the two functions. >> >> Argument renaming of course implies renaming the other companion >> methods. You taught me to add public APIs with good descriptions >> when possible, I thought your advice was worth to other contributors. >> I'm not considering this review comment as bikeshedding. Anyway, >> thanks for your review and advice. I don't object if Brian keeps >> the current names or rename as @a, @b, @foo, @bar. > > Sorry for being a bit grumpy -- it's just that this is the > second time around of somebody suggesting a change here > to something that's already present in the existing > function this is patterned off (and which we borrowed from > the Linux kernel originally). Ah, I missed the previous suggestion and didn't know about the historical origin. Thanks Peter.
© 2016 - 2026 Red Hat, Inc.