Add inlined functions to obtain a mask of changed bits.  3 flavors
are added: toggled, changed to 1, changed to 0.

These newly added utilities aid common device behaviors where
actions are taken only when a register's bit(s) are changed.

Signed-off-by: Tong Ho <tong.ho@amd.com>
---
 include/qemu/bitops.h | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h
index 2c0a2fe751..7a701474ea 100644
--- a/include/qemu/bitops.h
+++ b/include/qemu/bitops.h
@@ -148,6 +148,39 @@ static inline int test_bit(long nr, const unsigned long *addr)
     return 1UL & (addr[BIT_WORD(nr)] >> (nr & (BITS_PER_LONG-1)));
 }
 
+/**
+ * find_bits_changed - Returns a mask of bits changed.
+ * @ref_bits: the reference bits against which the test is made.
+ * @chk_bits: the bits to be checked.
+ */
+static inline unsigned long find_bits_changed(unsigned long ref_bits,
+                                              unsigned long chk_bits)
+{
+    return ref_bits ^ chk_bits;
+}
+
+/**
+ * find_bits_to_1 - Returns a mask of bits changed from 0 to 1.
+ * @ref_bits: the reference bits against which the test is made.
+ * @chk_bits: the bits to be checked.
+ */
+static inline unsigned long find_bits_to_1(unsigned long ref_bits,
+                                           unsigned long chk_bits)
+{
+    return find_bits_changed(ref_bits, chk_bits) & chk_bits;
+}
+
+/**
+ * find_bits_to_0 - Returns a mask of bits changed from 1 to 0.
+ * @ref_bits: the reference bits against which the test is made.
+ * @chk_bits: the bits to be checked.
+ */
+static inline unsigned long find_bits_to_0(unsigned long ref_bits,
+                                           unsigned long chk_bits)
+{
+    return find_bits_to_1(chk_bits, ref_bits);
+}
+
 /**
  * find_last_bit - find the last set bit in a memory region
  * @addr: The address to start the search at
-- 
2.25.1

On Wed, 29 May 2024 at 06:05, Tong Ho <tong.ho@amd.com> wrote:
>
> Add inlined functions to obtain a mask of changed bits.  3 flavors
> are added: toggled, changed to 1, changed to 0.
>
> These newly added utilities aid common device behaviors where
> actions are taken only when a register's bit(s) are changed.

Generally we would expect this kind of "add new utility functions"
patch to appear in a series together with some patches which
actually use the new functions. Otherwise this is all dead code.

More generally:
 * the other bit operations in this file work on bit arrays
   which are arbitrary-length arrays of unsigned long, so
   these new functions don't fit the pattern
 * we have the bitops functions partly because they're inherited
   from the Linux kernel. The use of unsigned long works quite
   badly in QEMU, because for us 'long' is a type that is almost
   always wrong. QEMU devices usually want a type of a known
   length, which is either 'uint32_t' or 'uint64_t'. So I'm
   dubious about adding more functions that work on unsigned long.

thanks
-- PMM

Point taken, and I am withdrawing this patch.

I will post a new implementation following the arbitrary-length array pattern
in a later date, and only as part of a series including the consuming code.

Thanks,
Tong Ho

________________________________
From: Peter Maydell <peter.maydell@linaro.org>
Sent: Wednesday, May 29, 2024 6:44 AM
To: Ho, Tong <tong.ho@amd.com>
Cc: qemu-devel@nongnu.org <qemu-devel@nongnu.org>
Subject: Re: [PATCH] qemu/bitops.h: Locate changed bits

On Wed, 29 May 2024 at 06:05, Tong Ho <tong.ho@amd.com> wrote:
>
> Add inlined functions to obtain a mask of changed bits.  3 flavors
> are added: toggled, changed to 1, changed to 0.
>
> These newly added utilities aid common device behaviors where
> actions are taken only when a register's bit(s) are changed.

Generally we would expect this kind of "add new utility functions"
patch to appear in a series together with some patches which
actually use the new functions. Otherwise this is all dead code.

More generally:
 * the other bit operations in this file work on bit arrays
   which are arbitrary-length arrays of unsigned long, so
   these new functions don't fit the pattern
 * we have the bitops functions partly because they're inherited
   from the Linux kernel. The use of unsigned long works quite
   badly in QEMU, because for us 'long' is a type that is almost
   always wrong. QEMU devices usually want a type of a known
   length, which is either 'uint32_t' or 'uint64_t'. So I'm
   dubious about adding more functions that work on unsigned long.

thanks
-- PMM

On 5/29/2024 12:59, Tong Ho wrote:> Add inlined functions to obtain a mask of
changed bits.  3 flavors
> are added: toggled, changed to 1, changed to 0.
> 
> These newly added utilities aid common device behaviors where
> actions are taken only when a register's bit(s) are changed.
> 
> Signed-off-by: Tong Ho <tong.ho@amd.com>
> ---
>  include/qemu/bitops.h | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h
> index 2c0a2fe751..7a701474ea 100644
> --- a/include/qemu/bitops.h
> +++ b/include/qemu/bitops.h
> @@ -148,6 +148,39 @@ static inline int test_bit(long nr, const unsigned long *addr)
>      return 1UL & (addr[BIT_WORD(nr)] >> (nr & (BITS_PER_LONG-1)));
>  }
>  
> +/**
> + * find_bits_changed - Returns a mask of bits changed.
> + * @ref_bits: the reference bits against which the test is made.
> + * @chk_bits: the bits to be checked.
> + */
> +static inline unsigned long find_bits_changed(unsigned long ref_bits,
> +                                              unsigned long chk_bits)
> +{
> +    return ref_bits ^ chk_bits;
> +}
> +
> +/**
> + * find_bits_to_1 - Returns a mask of bits changed from 0 to 1.
> + * @ref_bits: the reference bits against which the test is made.
> + * @chk_bits: the bits to be checked.
> + */
> +static inline unsigned long find_bits_to_1(unsigned long ref_bits,
> +                                           unsigned long chk_bits)
> +{
> +    return find_bits_changed(ref_bits, chk_bits) & chk_bits;
> +}
> +
> +/**
> + * find_bits_to_0 - Returns a mask of bits changed from 1 to 0.
> + * @ref_bits: the reference bits against which the test is made.
> + * @chk_bits: the bits to be checked.
> + */
> +static inline unsigned long find_bits_to_0(unsigned long ref_bits,
> +                                           unsigned long chk_bits)
> +{
> +    return find_bits_to_1(chk_bits, ref_bits);
> +}
> +
>  /**
>   * find_last_bit - find the last set bit in a memory region
>   * @addr: The address to start the search at

Reviewed-by: Lei Wang <lei4.wang@intel.com>