Re: [PATCH V2 1/2] uapi: Define GENMASK_U128

Yury Norov
2024-07-30T18:16:01Z

On Thu, Jul 25, 2024 at 11:18:07AM +0530, Anshuman Khandual wrote:
> This adds GENMASK_U128() and __GENMASK_U128() macros using __BITS_PER_U128
> and __int128 data types. These macros will be used in providing support for
> generating 128 bit masks.
>
> Cc: Yury Norov <yury.norov@gmail.com>
> Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Cc: Arnd Bergmann <arnd@arndb.de>>
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-arch@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
> include/linux/bits.h | 2 ++
> include/uapi/asm-generic/bitsperlong.h | 2 ++
> include/uapi/linux/bits.h | 3 +++
> include/uapi/linux/const.h | 1 +
> 4 files changed, 8 insertions(+)
>
> diff --git a/include/linux/bits.h b/include/linux/bits.h
> index 0eb24d21aac2..0a174cce09d2 100644
> --- a/include/linux/bits.h
> +++ b/include/linux/bits.h
> @@ -35,5 +35,7 @@
> (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
> #define GENMASK_ULL(h, l) \
> (GENMASK_INPUT_CHECK(h, l) + __GENMASK_ULL(h, l))
> +#define GENMASK_U128(h, l) \
> + (GENMASK_INPUT_CHECK(h, l) + __GENMASK_U128(h, l))
>
> #endif /* __LINUX_BITS_H */
> diff --git a/include/uapi/asm-generic/bitsperlong.h b/include/uapi/asm-generic/bitsperlong.h
> index fadb3f857f28..6275367b17bb 100644
> --- a/include/uapi/asm-generic/bitsperlong.h
> +++ b/include/uapi/asm-generic/bitsperlong.h
> @@ -28,4 +28,6 @@
> #define __BITS_PER_LONG_LONG 64
> #endif
>
> +#define __BITS_PER_U128 128

Do we need such a macro for a fixed-width type? Even if we do, I'm not
sure that a header named bitsperlong.h is a good place to host it.

> +
> #endif /* _UAPI__ASM_GENERIC_BITS_PER_LONG */
> diff --git a/include/uapi/linux/bits.h b/include/uapi/linux/bits.h
> index 3c2a101986a3..4d4b7b08003c 100644
> --- a/include/uapi/linux/bits.h
> +++ b/include/uapi/linux/bits.h
> @@ -12,4 +12,7 @@
> (((~_ULL(0)) - (_ULL(1) << (l)) + 1) & \
> (~_ULL(0) >> (__BITS_PER_LONG_LONG - 1 - (h))))
>
> +#define __GENMASK_U128(h, l) \
> + ((_BIT128((h) + 1)) - (_BIT128(l)))
> +
> #endif /* _UAPI_LINUX_BITS_H */
> diff --git a/include/uapi/linux/const.h b/include/uapi/linux/const.h
> index a429381e7ca5..a0211136dfd8 100644
> --- a/include/uapi/linux/const.h
> +++ b/include/uapi/linux/const.h
> @@ -27,6 +27,7 @@
>
> #define _BITUL(x) (_UL(1) << (x))
> #define _BITULL(x) (_ULL(1) << (x))
> +#define _BIT128(x) ((unsigned __int128)(1) << (x))

GENMASK() macros may be used in assembly code. This is not the case
for GENMASK_128 at this time, of course, but I think we'd introduce
assembly glue at this point to simplify things in future. Can you
check the include/uapi/linux/const.h and add something like _U128()
in there?

>
> #define __ALIGN_KERNEL(x, a) __ALIGN_KERNEL_MASK(x, (__typeof__(x))(a) - 1)
> #define __ALIGN_KERNEL_MASK(x, mask) (((x) + (mask)) & ~(mask))
> --
> 2.30.2