[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aAKH37xa1brIAXfs@yury>
Date: Fri, 18 Apr 2025 13:11:59 -0400
From: Yury Norov <yury.norov@...il.com>
To: Luo Jie <quic_luoj@...cinc.com>
Cc: Rasmus Villemoes <linux@...musvillemoes.dk>,
Julia Lawall <Julia.Lawall@...ia.fr>,
Nicolas Palix <nicolas.palix@...g.fr>,
Catalin Marinas <catalin.marinas@....com>,
Will Deacon <will@...nel.org>, Marc Zyngier <maz@...nel.org>,
Oliver Upton <oliver.upton@...ux.dev>,
Joey Gouly <joey.gouly@....com>,
Suzuki K Poulose <suzuki.poulose@....com>,
Zenghui Yu <yuzenghui@...wei.com>, linux-kernel@...r.kernel.org,
cocci@...ia.fr, linux-arm-kernel@...ts.infradead.org,
kvmarm@...ts.linux.dev, andrew@...n.ch, quic_kkumarcs@...cinc.com,
quic_linchen@...cinc.com, quic_leiwei@...cinc.com,
quic_suruchia@...cinc.com, quic_pavir@...cinc.com
Subject: Re: [PATCH v3 1/6] bitfield: Add FIELD_MODIFY() helper
On Thu, Apr 17, 2025 at 06:47:08PM +0800, Luo Jie wrote:
> Add a helper for replacing the contents of bitfield in memory
> with the specified value.
>
> Even though a helper xxx_replace_bits() is available, it is not
> well documented, and only reports errors at the run time, which
> will not be helpful to catch possible overflow errors due to
> incorrect parameter types used.
>
> Add the helper FIELD_MODIFY() to the FIELD_XXX family of bitfield
> macros. It is functionally similar as xxx_replace_bits(), and in
> addition adds the compile time type checking.
This paragraph duplicates the above. I'll drop it and take this
patch to bitmap-for-next. Regarding the rest of the series - it's up
to ARM64 and Cocci maintainers if they want them or not.
Thanks for the work!
Thanks,
Yury
> FIELD_MODIFY(REG_FIELD_C, ®, c) is the wrapper of the code below.
> reg &= ~REG_FIELD_C;
> reg |= FIELD_PREP(REG_FIELD_C, c);
>
> Signed-off-by: Luo Jie <quic_luoj@...cinc.com>
> ---
> include/linux/bitfield.h | 21 +++++++++++++++++++--
> 1 file changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
> index 63928f173223..2eaefa76f759 100644
> --- a/include/linux/bitfield.h
> +++ b/include/linux/bitfield.h
> @@ -8,6 +8,7 @@
> #define _LINUX_BITFIELD_H
>
> #include <linux/build_bug.h>
> +#include <linux/typecheck.h>
> #include <asm/byteorder.h>
>
> /*
> @@ -38,8 +39,7 @@
> * FIELD_PREP(REG_FIELD_D, 0x40);
> *
> * Modify:
> - * reg &= ~REG_FIELD_C;
> - * reg |= FIELD_PREP(REG_FIELD_C, c);
> + * FIELD_MODIFY(REG_FIELD_C, ®, c);
> */
>
> #define __bf_shf(x) (__builtin_ffsll(x) - 1)
> @@ -156,6 +156,23 @@
> (typeof(_mask))(((_reg) & (_mask)) >> __bf_shf(_mask)); \
> })
>
> +/**
> + * FIELD_MODIFY() - modify a bitfield element
> + * @_mask: shifted mask defining the field's length and position
> + * @_reg_p: pointer to the memory that should be updated
> + * @_val: value to store in the bitfield
> + *
> + * FIELD_MODIFY() modifies the set of bits in @_reg_p specified by @_mask,
> + * by replacing them with the bitfield value passed in as @_val.
> + */
> +#define FIELD_MODIFY(_mask, _reg_p, _val) \
> + ({ \
> + typecheck_pointer(_reg_p); \
> + __BF_FIELD_CHECK(_mask, *(_reg_p), _val, "FIELD_MODIFY: "); \
> + *(_reg_p) &= ~(_mask); \
> + *(_reg_p) |= (((typeof(_mask))(_val) << __bf_shf(_mask)) & (_mask)); \
> + })
> +
> extern void __compiletime_error("value doesn't fit into mask")
> __field_overflow(void);
> extern void __compiletime_error("bad bitfield mask")
>
> --
> 2.34.1
Powered by blists - more mailing lists