[PATCH 0/1] Subject: Support deposit8 in include/qemu/bitops.h

[PATCH 0/1] Subject: Support deposit8 in include/qemu/bitops.h

[Jason Fan]

Jason Fan (1):
  include/qemu/bitops.h: Add deposit8 for uint8_t bit operation

 include/qemu/bitops.h | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

-- 
2.46.0.295.g3b9ea8a38a-goog

[PATCH 1/1] include/qemu/bitops.h: Add deposit8 for uint8_t bit operation

[Jason Fan]

Signed-off-by: Jason Fan <fanjason@google.com>
---
 include/qemu/bitops.h | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h
index 2c0a2fe751..d01c4b42f2 100644
--- a/include/qemu/bitops.h
+++ b/include/qemu/bitops.h
@@ -459,6 +459,32 @@ static inline int64_t sextract64(uint64_t value, int start, int length)
     return ((int64_t)(value << (64 - length - start))) >> (64 - length);
 }
 
+/**
+ * deposit8:
+ * @value: initial value to insert bit field into
+ * @start: the lowest bit in the bit field (numbered from 0)
+ * @length: the length of the bit field
+ * @fieldval: the value to insert into the bit field
+ *
+ * Deposit @fieldval into the 8 bit @value at the bit field specified
+ * by the @start and @length parameters, and return the modified
+ * @value. Bits of @value outside the bit field are not modified.
+ * Bits of @fieldval above the least significant @length bits are
+ * ignored. The bit field must lie entirely within the 8 bit word.
+ * It is valid to request that all 8 bits are modified (ie @length
+ * 8 and @start 0).
+ *
+ * Returns: the modified @value.
+ */
+static inline uint8_t deposit8(uint8_t value, int start, int length,
+                               uint8_t fieldval)
+{
+    uint8_t mask = 0xFF;
+    assert(start >= 0 && length > 0 && length <= 8 - start);
+    mask = (mask >> (8 - length)) << start;
+    return (value & ~mask) | ((fieldval << start) & mask);
+}
+
 /**
  * deposit32:
  * @value: initial value to insert bit field into
-- 
2.46.0.295.g3b9ea8a38a-goog

Re: [PATCH 1/1] include/qemu/bitops.h: Add deposit8 for uint8_t bit operation

[Peter Maydell]

On Mon, 26 Aug 2024 at 22:01, Jason Fan <fanjason@google.com> wrote:
>
> Signed-off-by: Jason Fan <fanjason@google.com>
> ---
>  include/qemu/bitops.h | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
>
> diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h
> index 2c0a2fe751..d01c4b42f2 100644
> --- a/include/qemu/bitops.h
> +++ b/include/qemu/bitops.h
> @@ -459,6 +459,32 @@ static inline int64_t sextract64(uint64_t value, int start, int length)
>      return ((int64_t)(value << (64 - length - start))) >> (64 - length);
>  }
>
> +/**
> + * deposit8:
> + * @value: initial value to insert bit field into
> + * @start: the lowest bit in the bit field (numbered from 0)
> + * @length: the length of the bit field
> + * @fieldval: the value to insert into the bit field
> + *
> + * Deposit @fieldval into the 8 bit @value at the bit field specified
> + * by the @start and @length parameters, and return the modified
> + * @value. Bits of @value outside the bit field are not modified.
> + * Bits of @fieldval above the least significant @length bits are
> + * ignored. The bit field must lie entirely within the 8 bit word.
> + * It is valid to request that all 8 bits are modified (ie @length
> + * 8 and @start 0).
> + *
> + * Returns: the modified @value.
> + */
> +static inline uint8_t deposit8(uint8_t value, int start, int length,
> +                               uint8_t fieldval)
> +{
> +    uint8_t mask = 0xFF;
> +    assert(start >= 0 && length > 0 && length <= 8 - start);
> +    mask = (mask >> (8 - length)) << start;
> +    return (value & ~mask) | ((fieldval << start) & mask);
> +}

What's the use case for this? Where would you need this
and not be able to use "deposit32" instead?

thanks
-- PMM

Re: [PATCH 1/1] include/qemu/bitops.h: Add deposit8 for uint8_t bit operation

[Jason Fan]

[-- Attachment #1: Type: text/plain, Size: 2391 bytes --]

Re-send this to include the original mail-list.

Hi Peter,
I am working on a i3c target model which requires bitops on the uint8_t
registers.
deposit8 can help to check incorrect length or start input for 8 bit value.
You are right that desposit32 should also work if we always pass the
correct arguments, but since the implementation seems trivial enough, I
just go ahead and create the patch.
Thank you.

On Tue, Aug 27, 2024 at 2:23 AM Peter Maydell <peter.maydell@linaro.org>
wrote:

> On Mon, 26 Aug 2024 at 22:01, Jason Fan <fanjason@google.com> wrote:
> >
> > Signed-off-by: Jason Fan <fanjason@google.com>
> > ---
> >  include/qemu/bitops.h | 26 ++++++++++++++++++++++++++
> >  1 file changed, 26 insertions(+)
> >
> > diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h
> > index 2c0a2fe751..d01c4b42f2 100644
> > --- a/include/qemu/bitops.h
> > +++ b/include/qemu/bitops.h
> > @@ -459,6 +459,32 @@ static inline int64_t sextract64(uint64_t value,
> int start, int length)
> >      return ((int64_t)(value << (64 - length - start))) >> (64 - length);
> >  }
> >
> > +/**
> > + * deposit8:
> > + * @value: initial value to insert bit field into
> > + * @start: the lowest bit in the bit field (numbered from 0)
> > + * @length: the length of the bit field
> > + * @fieldval: the value to insert into the bit field
> > + *
> > + * Deposit @fieldval into the 8 bit @value at the bit field specified
> > + * by the @start and @length parameters, and return the modified
> > + * @value. Bits of @value outside the bit field are not modified.
> > + * Bits of @fieldval above the least significant @length bits are
> > + * ignored. The bit field must lie entirely within the 8 bit word.
> > + * It is valid to request that all 8 bits are modified (ie @length
> > + * 8 and @start 0).
> > + *
> > + * Returns: the modified @value.
> > + */
> > +static inline uint8_t deposit8(uint8_t value, int start, int length,
> > +                               uint8_t fieldval)
> > +{
> > +    uint8_t mask = 0xFF;
> > +    assert(start >= 0 && length > 0 && length <= 8 - start);
> > +    mask = (mask >> (8 - length)) << start;
> > +    return (value & ~mask) | ((fieldval << start) & mask);
> > +}
>
> What's the use case for this? Where would you need this
> and not be able to use "deposit32" instead?
>
> thanks
> -- PMM
>

[-- Attachment #2: Type: text/html, Size: 3172 bytes --]

Re: [PATCH 1/1] include/qemu/bitops.h: Add deposit8 for uint8_t bit operation

[Peter Maydell]

On Wed, 28 Aug 2024 at 16:59, Jason Fan <fanjason@google.com> wrote:
>
> Re-send this to include the original mail-list.
>
> Hi Peter,
> I am working on a i3c target model which requires bitops on the uint8_t registers.
> deposit8 can help to check incorrect length or start input for 8 bit value.
> You are right that desposit32 should also work if we always pass the correct arguments, but since the implementation seems trivial enough, I just go ahead and create the patch.

Thanks. However I think by this argument we would want
also deposit16, deposit24, deposit4 for 4-bit registers,
and so on indefinitely. And your start and length arguments
should be #defined constants, preferably done with the
FIELD macros, not raw numbers, so I don't think that
kind of bug is very likely. (Besides, even with a
deposit8() it could protect against, say, a wrong
start value that was 8 or greater, but not a wrong
start value that happened to still be in range for a
uint8_t.)

-- PMM