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

[Yury Norov]

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@xxxxxxxxx>
> Cc: Rasmus Villemoes <linux@xxxxxxxxxxxxxxxxxx>
> Cc: Arnd Bergmann <arnd@xxxxxxxx>>
> Cc: linux-kernel@xxxxxxxxxxxxxxx
> Cc: linux-arch@xxxxxxxxxxxxxxx
> Signed-off-by: Anshuman Khandual <anshuman.khandual@xxxxxxx>
> ---
>  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