Re: [PATCH] qemu/bitops.h: Locate changed bits
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 Sent: Wednesday, May 29, 2024 6:44 AM To: Ho, Tong Cc: qemu-devel@nongnu.org Subject: Re: [PATCH] qemu/bitops.h: Locate changed bits On Wed, 29 May 2024 at 06:05, 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. 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
Re: [PATCH] qemu/bitops.h: Locate changed bits
On Wed, 29 May 2024 at 06:05, 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. 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
Re: [PATCH] qemu/bitops.h: Locate changed bits
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
> ---
> 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
[PATCH] qemu/bitops.h: Locate changed bits
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
---
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
