[PATCH V3 0/2] uapi: Add support for GENMASK_U128()

[PATCH V3 0/2] uapi: Add support for GENMASK_U128()

[Anshuman Khandual]

This adds support for GENMASK_U128() and some corresponding tests as well.
GENMASK_U128() generated 128 bit masks will be required later on the arm64
platform for enabling FEAT_SYSREG128 and FEAT_D128 features.

Because GENMAKS_U128() depends on __int128 data type being supported in the
compiler, its usage needs to be protected with CONFIG_ARCH_SUPPORTS_INT128.

Cc: Andrew Morton <akpm@linux-foundation.org>
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

Changes in V3:

- Dropped unused __BITS_PER_U128
- Moved #ifdef CONFIG_ARCH_SUPPORTS_INT128 inside genmask_u128_test()
- Added asm unsupported comments for GENMASK_U128() and __BIT128()
- Added reviewed tag from Arnd

Changes in V2:

https://lore.kernel.org/all/20240725054808.286708-1-anshuman.khandual@arm.com/

- Wrapped genmask_u128_test() with CONFIG_ARCH_SUPPORTS_INT128
- Defined __BITS_PER_U128 unconditionally as 128
- Defined __GENMASK_U128() via new _BIT128()
- Dropped _U128() and _AC128()

Changes in V1:

https://lore.kernel.org/lkml/20240724103142.165693-1-anshuman.khandual@arm.com/

Anshuman Khandual (2):
  uapi: Define GENMASK_U128
  lib/test_bits.c: Add tests for GENMASK_U128()

 include/linux/bits.h       | 13 +++++++++++++
 include/uapi/linux/bits.h  |  3 +++
 include/uapi/linux/const.h | 15 +++++++++++++++
 lib/test_bits.c            | 23 +++++++++++++++++++++++
 4 files changed, 54 insertions(+)

-- 
2.30.2

[PATCH V3 1/2] uapi: Define GENMASK_U128

[Anshuman Khandual]

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
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 include/linux/bits.h       | 13 +++++++++++++
 include/uapi/linux/bits.h  |  3 +++
 include/uapi/linux/const.h | 15 +++++++++++++++
 3 files changed, 31 insertions(+)

diff --git a/include/linux/bits.h b/include/linux/bits.h
index 0eb24d21aac2..bf99feb5570e 100644
--- a/include/linux/bits.h
+++ b/include/linux/bits.h
@@ -36,4 +36,17 @@
 #define GENMASK_ULL(h, l) \
 	(GENMASK_INPUT_CHECK(h, l) + __GENMASK_ULL(h, l))
 
+/*
+ * Missing asm support
+ *
+ * __GENMASK_U128() depends on _BIT128() which would not work
+ * in the asm code, as it shifts an 'unsigned __init128' data
+ * type instead of direct representation of 128 bit constants
+ * such as long and unsigned long. The fundamental problem is
+ * that a 128 bit constant will get silently truncated by the
+ * gcc compiler.
+ */
+#define GENMASK_U128(h, l) \
+	(GENMASK_INPUT_CHECK(h, l) + __GENMASK_U128(h, l))
+
 #endif	/* __LINUX_BITS_H */
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..5be12e8f8f9c 100644
--- a/include/uapi/linux/const.h
+++ b/include/uapi/linux/const.h
@@ -28,6 +28,21 @@
 #define _BITUL(x)	(_UL(1) << (x))
 #define _BITULL(x)	(_ULL(1) << (x))
 
+/*
+ * Missing asm support
+ *
+ * __BIT128() would not work in the asm code, as it shifts an
+ * 'unsigned __init128' data type as direct representation of
+ * 128 bit constants is not supported in the gcc compiler, as
+ * they get silently truncated.
+ *
+ * TODO: Please revisit this implementation when gcc compiler
+ * starts representing 128 bit constants directly like long
+ * and unsigned long etc. Subsequently drop the comment for
+ * GENMASK_U128() which would then start supporting asm code.
+ */
+#define _BIT128(x)	((unsigned __int128)(1) << (x))
+
 #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

[PATCH V3 2/2] lib/test_bits.c: Add tests for GENMASK_U128()

[Anshuman Khandual]

This adds GENMASK_U128() tests although currently only 64 bit wide masks
are being tested.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 lib/test_bits.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/lib/test_bits.c b/lib/test_bits.c
index 01313980f175..d3d858b24e02 100644
--- a/lib/test_bits.c
+++ b/lib/test_bits.c
@@ -39,6 +39,26 @@ static void genmask_ull_test(struct kunit *test)
 #endif
 }
 
+static void genmask_u128_test(struct kunit *test)
+{
+#ifdef CONFIG_ARCH_SUPPORTS_INT128
+	/* Tests mask generation only when the mask width is within 64 bits */
+	KUNIT_EXPECT_EQ(test, 0x0000000000ff0000ULL, GENMASK_U128(87, 80) >> 64);
+	KUNIT_EXPECT_EQ(test, 0x0000000000ffffffULL, GENMASK_U128(87, 64) >> 64);
+	KUNIT_EXPECT_EQ(test, 0x0000000000000001ULL, GENMASK_U128(0, 0));
+	KUNIT_EXPECT_EQ(test, 0xffffffffffffffffULL, GENMASK_U128(63, 0));
+	KUNIT_EXPECT_EQ(test, 0xffffffffffffffffULL, GENMASK_U128(64, 0) >> 1);
+	KUNIT_EXPECT_EQ(test, 0x00000000ffffffffULL, GENMASK_U128(81, 50) >> 50);
+
+#ifdef TEST_GENMASK_FAILURES
+	/* these should fail compilation */
+	GENMASK_U128(0, 1);
+	GENMASK_U128(0, 10);
+	GENMASK_U128(9, 10);
+#endif /* TEST_GENMASK_FAILURES */
+#endif /* CONFIG_ARCH_SUPPORTS_INT128 */
+}
+
 static void genmask_input_check_test(struct kunit *test)
 {
 	unsigned int x, y;
@@ -56,12 +76,15 @@ static void genmask_input_check_test(struct kunit *test)
 	/* Valid input */
 	KUNIT_EXPECT_EQ(test, 0, GENMASK_INPUT_CHECK(1, 1));
 	KUNIT_EXPECT_EQ(test, 0, GENMASK_INPUT_CHECK(39, 21));
+	KUNIT_EXPECT_EQ(test, 0, GENMASK_INPUT_CHECK(100, 80));
+	KUNIT_EXPECT_EQ(test, 0, GENMASK_INPUT_CHECK(110, 65));
 }
 
 
 static struct kunit_case bits_test_cases[] = {
 	KUNIT_CASE(genmask_test),
 	KUNIT_CASE(genmask_ull_test),
+	KUNIT_CASE(genmask_u128_test),
 	KUNIT_CASE(genmask_input_check_test),
 	{}
 };
-- 
2.30.2

Re: [PATCH V3 0/2] uapi: Add support for GENMASK_U128()

[Yury Norov]

On Thu, Aug 1, 2024 at 12:16 AM Anshuman Khandual
<anshuman.khandual@arm.com> wrote:
>
> This adds support for GENMASK_U128() and some corresponding tests as well.
> GENMASK_U128() generated 128 bit masks will be required later on the arm64
> platform for enabling FEAT_SYSREG128 and FEAT_D128 features.
>
> Because GENMAKS_U128() depends on __int128 data type being supported in the
> compiler, its usage needs to be protected with CONFIG_ARCH_SUPPORTS_INT128.
>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> 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

For the patches:

Reviewed-by: Yury Norov <yury.norov@gmail.com>

This series doesn't include a real use-case for the new macros. Do you
have some?
I can take it via my branch, but I need at least one use-case to not
merge dead code.

Thanks,
Yury

Re: [PATCH V3 0/2] uapi: Add support for GENMASK_U128()

[Anshuman Khandual]

On 8/1/24 20:13, Yury Norov wrote:
> On Thu, Aug 1, 2024 at 12:16 AM Anshuman Khandual
> <anshuman.khandual@arm.com> wrote:
>>
>> This adds support for GENMASK_U128() and some corresponding tests as well.
>> GENMASK_U128() generated 128 bit masks will be required later on the arm64
>> platform for enabling FEAT_SYSREG128 and FEAT_D128 features.
>>
>> Because GENMAKS_U128() depends on __int128 data type being supported in the
>> compiler, its usage needs to be protected with CONFIG_ARCH_SUPPORTS_INT128.
>>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> 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
> 
> For the patches:
> 
> Reviewed-by: Yury Norov <yury.norov@gmail.com>

Thanks Yury.

> 
> This series doesn't include a real use-case for the new macros. Do you
> have some?
> I can take it via my branch, but I need at least one use-case to not
> merge dead code.

I have recently posted the following patch for arm64 platform although
most of the subsequent work is still in progress. But for now there
are some corresponding tests for this new GENMASK_U128() ABI as well.
Hence it will be really great to have these two patches merged first.
Thank you.

https://lore.kernel.org/all/20240801054436.612024-1-anshuman.khandual@arm.com/

Re: [PATCH V3 0/2] uapi: Add support for GENMASK_U128()

[Yury Norov]

On Fri, Aug 02, 2024 at 07:00:43AM +0530, Anshuman Khandual wrote:
> 
> 
> On 8/1/24 20:13, Yury Norov wrote:
> > On Thu, Aug 1, 2024 at 12:16 AM Anshuman Khandual
> > <anshuman.khandual@arm.com> wrote:
> >>
> >> This adds support for GENMASK_U128() and some corresponding tests as well.
> >> GENMASK_U128() generated 128 bit masks will be required later on the arm64
> >> platform for enabling FEAT_SYSREG128 and FEAT_D128 features.
> >>
> >> Because GENMAKS_U128() depends on __int128 data type being supported in the
> >> compiler, its usage needs to be protected with CONFIG_ARCH_SUPPORTS_INT128.
> >>
> >> Cc: Andrew Morton <akpm@linux-foundation.org>
> >> 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
> > 
> > For the patches:
> > 
> > Reviewed-by: Yury Norov <yury.norov@gmail.com>
> 
> Thanks Yury.
> 
> > 
> > This series doesn't include a real use-case for the new macros. Do you
> > have some?
> > I can take it via my branch, but I need at least one use-case to not
> > merge dead code.
> 
> I have recently posted the following patch for arm64 platform although
> most of the subsequent work is still in progress. But for now there
> are some corresponding tests for this new GENMASK_U128() ABI as well.
> Hence it will be really great to have these two patches merged first.
> Thank you.
> 
> https://lore.kernel.org/all/20240801054436.612024-1-anshuman.khandual@arm.com/

If you're going to merge the above patch in 6.12, you'd normally send
it together with GENMASK_U128 preparation series, get an ACK from me
and Rasmus for bitops part and move through arm64 tree.

Whatever, I've merged it in bitmap-for-next for testing. Please keep
me posted for any following patches.

Thanks,
Yury

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

[Anshuman Khandual]

On 8/1/24 12:46, 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
> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
>  include/linux/bits.h       | 13 +++++++++++++
>  include/uapi/linux/bits.h  |  3 +++
>  include/uapi/linux/const.h | 15 +++++++++++++++
>  3 files changed, 31 insertions(+)
> 
> diff --git a/include/linux/bits.h b/include/linux/bits.h
> index 0eb24d21aac2..bf99feb5570e 100644
> --- a/include/linux/bits.h
> +++ b/include/linux/bits.h
> @@ -36,4 +36,17 @@
>  #define GENMASK_ULL(h, l) \
>  	(GENMASK_INPUT_CHECK(h, l) + __GENMASK_ULL(h, l))
>  
> +/*
> + * Missing asm support
> + *
> + * __GENMASK_U128() depends on _BIT128() which would not work
> + * in the asm code, as it shifts an 'unsigned __init128' data
> + * type instead of direct representation of 128 bit constants
> + * such as long and unsigned long. The fundamental problem is
> + * that a 128 bit constant will get silently truncated by the
> + * gcc compiler.
> + */
> +#define GENMASK_U128(h, l) \
> +	(GENMASK_INPUT_CHECK(h, l) + __GENMASK_U128(h, l))
> +
>  #endif	/* __LINUX_BITS_H */
> 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..5be12e8f8f9c 100644
> --- a/include/uapi/linux/const.h
> +++ b/include/uapi/linux/const.h
> @@ -28,6 +28,21 @@
>  #define _BITUL(x)	(_UL(1) << (x))
>  #define _BITULL(x)	(_ULL(1) << (x))
>  
> +/*
> + * Missing asm support
> + *
> + * __BIT128() would not work in the asm code, as it shifts an
> + * 'unsigned __init128' data type as direct representation of
> + * 128 bit constants is not supported in the gcc compiler, as
> + * they get silently truncated.
> + *
> + * TODO: Please revisit this implementation when gcc compiler
> + * starts representing 128 bit constants directly like long
> + * and unsigned long etc. Subsequently drop the comment for
> + * GENMASK_U128() which would then start supporting asm code.
> + */
> +#define _BIT128(x)	((unsigned __int128)(1) << (x))
> +
>  #define __ALIGN_KERNEL(x, a)		__ALIGN_KERNEL_MASK(x, (__typeof__(x))(a) - 1)
>  #define __ALIGN_KERNEL_MASK(x, mask)	(((x) + (mask)) & ~(mask))
>  

Hello Yuri/Arnd,

This proposed GENMASK_U128(h, l) warns during build when the higher end
bit is 127 (which in itself is a valid input).

./include/uapi/linux/const.h:45:44: warning: left shift count >= width of type [-Wshift-count-overflow]
   45 | #define _BIT128(x) ((unsigned __int128)(1) << (x))
      |                                            ^~
./include/asm-generic/bug.h:123:25: note: in definition of macro ‘WARN_ON’
  123 |  int __ret_warn_on = !!(condition);    \
      |                         ^~~~~~~~~
./include/uapi/linux/bits.h:16:4: note: in expansion of macro ‘_BIT128’
   16 |  ((_BIT128((h) + 1)) - (_BIT128(l)))
      |    ^~~~~~~
./include/linux/bits.h:51:31: note: in expansion of macro ‘__GENMASK_U128’
   51 |  (GENMASK_INPUT_CHECK(h, l) + __GENMASK_U128(h, l))
      |                               ^~~~~~~~~~~~~~

This is caused by ((unsigned __int128)(1) << (128)) which is generated
via (h + 1) element in __GENMASK_U128().

#define _BIT128(x)	((unsigned __int128)(1) << (x))
#define __GENMASK_U128(h, l) \
	((_BIT128((h) + 1)) - (_BIT128(l)))

Adding some extra tests in lib/test_bits.c exposes this build problem,
although it does not fail these new tests.

[    1.719221]     # Subtest: bits-test
[    1.719291]     # module: test_bits
[    1.720522]     ok 1 genmask_test
[    1.721570]     ok 2 genmask_ull_test
[    1.722668]     ok 3 genmask_u128_test
[    1.723760]     ok 4 genmask_input_check_test
[    1.723909] # bits-test: pass:4 fail:0 skip:0 total:4
[    1.724101] ok 1 bits-test

diff --git a/lib/test_bits.c b/lib/test_bits.c
index d3d858b24e02..7a972edc7122 100644
--- a/lib/test_bits.c
+++ b/lib/test_bits.c
@@ -49,6 +49,8 @@ static void genmask_u128_test(struct kunit *test)
        KUNIT_EXPECT_EQ(test, 0xffffffffffffffffULL, GENMASK_U128(63, 0));
        KUNIT_EXPECT_EQ(test, 0xffffffffffffffffULL, GENMASK_U128(64, 0) >> 1);
        KUNIT_EXPECT_EQ(test, 0x00000000ffffffffULL, GENMASK_U128(81, 50) >> 50);
+       KUNIT_EXPECT_EQ(test, 0x0000000000000003ULL, GENMASK_U128(127, 126) >> 126);
+       KUNIT_EXPECT_EQ(test, 0x0000000000000001ULL, GENMASK_U128(127, 127) >> 127);

The most significant bit in the generate mask can be added separately
, thus voiding that extra shift. The following patch solves the build
problem.

diff --git a/include/uapi/linux/bits.h b/include/uapi/linux/bits.h
index 4d4b7b08003c..4e50f635c6d9 100644
--- a/include/uapi/linux/bits.h
+++ b/include/uapi/linux/bits.h
@@ -13,6 +13,6 @@
          (~_ULL(0) >> (__BITS_PER_LONG_LONG - 1 - (h))))
 
 #define __GENMASK_U128(h, l) \
-       ((_BIT128((h) + 1)) - (_BIT128(l)))
+       (((_BIT128(h)) - (_BIT128(l))) | (_BIT128(h)))

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

[Yury Norov]

On Fri, Aug 16, 2024 at 11:58:04AM +0530, Anshuman Khandual wrote:
> 
> 
> On 8/1/24 12:46, 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
> > Reviewed-by: Arnd Bergmann <arnd@arndb.de>
> > Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> > ---
> >  include/linux/bits.h       | 13 +++++++++++++
> >  include/uapi/linux/bits.h  |  3 +++
> >  include/uapi/linux/const.h | 15 +++++++++++++++
> >  3 files changed, 31 insertions(+)
> > 
> > diff --git a/include/linux/bits.h b/include/linux/bits.h
> > index 0eb24d21aac2..bf99feb5570e 100644
> > --- a/include/linux/bits.h
> > +++ b/include/linux/bits.h
> > @@ -36,4 +36,17 @@
> >  #define GENMASK_ULL(h, l) \
> >  	(GENMASK_INPUT_CHECK(h, l) + __GENMASK_ULL(h, l))
> >  
> > +/*
> > + * Missing asm support
> > + *
> > + * __GENMASK_U128() depends on _BIT128() which would not work
> > + * in the asm code, as it shifts an 'unsigned __init128' data
> > + * type instead of direct representation of 128 bit constants
> > + * such as long and unsigned long. The fundamental problem is
> > + * that a 128 bit constant will get silently truncated by the
> > + * gcc compiler.
> > + */
> > +#define GENMASK_U128(h, l) \
> > +	(GENMASK_INPUT_CHECK(h, l) + __GENMASK_U128(h, l))
> > +
> >  #endif	/* __LINUX_BITS_H */
> > 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..5be12e8f8f9c 100644
> > --- a/include/uapi/linux/const.h
> > +++ b/include/uapi/linux/const.h
> > @@ -28,6 +28,21 @@
> >  #define _BITUL(x)	(_UL(1) << (x))
> >  #define _BITULL(x)	(_ULL(1) << (x))
> >  
> > +/*
> > + * Missing asm support
> > + *
> > + * __BIT128() would not work in the asm code, as it shifts an
> > + * 'unsigned __init128' data type as direct representation of
> > + * 128 bit constants is not supported in the gcc compiler, as
> > + * they get silently truncated.
> > + *
> > + * TODO: Please revisit this implementation when gcc compiler
> > + * starts representing 128 bit constants directly like long
> > + * and unsigned long etc. Subsequently drop the comment for
> > + * GENMASK_U128() which would then start supporting asm code.
> > + */
> > +#define _BIT128(x)	((unsigned __int128)(1) << (x))
> > +
> >  #define __ALIGN_KERNEL(x, a)		__ALIGN_KERNEL_MASK(x, (__typeof__(x))(a) - 1)
> >  #define __ALIGN_KERNEL_MASK(x, mask)	(((x) + (mask)) & ~(mask))
> >  
> 
> Hello Yuri/Arnd,
> 
> This proposed GENMASK_U128(h, l) warns during build when the higher end
> bit is 127 (which in itself is a valid input).
> 
> ./include/uapi/linux/const.h:45:44: warning: left shift count >= width of type [-Wshift-count-overflow]
>    45 | #define _BIT128(x) ((unsigned __int128)(1) << (x))
>       |                                            ^~
> ./include/asm-generic/bug.h:123:25: note: in definition of macro ‘WARN_ON’
>   123 |  int __ret_warn_on = !!(condition);    \
>       |                         ^~~~~~~~~
> ./include/uapi/linux/bits.h:16:4: note: in expansion of macro ‘_BIT128’
>    16 |  ((_BIT128((h) + 1)) - (_BIT128(l)))
>       |    ^~~~~~~
> ./include/linux/bits.h:51:31: note: in expansion of macro ‘__GENMASK_U128’
>    51 |  (GENMASK_INPUT_CHECK(h, l) + __GENMASK_U128(h, l))
>       |                               ^~~~~~~~~~~~~~
> 
> This is caused by ((unsigned __int128)(1) << (128)) which is generated
> via (h + 1) element in __GENMASK_U128().
> 
> #define _BIT128(x)	((unsigned __int128)(1) << (x))
> #define __GENMASK_U128(h, l) \
> 	((_BIT128((h) + 1)) - (_BIT128(l)))
> 
> Adding some extra tests in lib/test_bits.c exposes this build problem,
> although it does not fail these new tests.
> 
> [    1.719221]     # Subtest: bits-test
> [    1.719291]     # module: test_bits
> [    1.720522]     ok 1 genmask_test
> [    1.721570]     ok 2 genmask_ull_test
> [    1.722668]     ok 3 genmask_u128_test
> [    1.723760]     ok 4 genmask_input_check_test
> [    1.723909] # bits-test: pass:4 fail:0 skip:0 total:4
> [    1.724101] ok 1 bits-test
> 
> diff --git a/lib/test_bits.c b/lib/test_bits.c
> index d3d858b24e02..7a972edc7122 100644
> --- a/lib/test_bits.c
> +++ b/lib/test_bits.c
> @@ -49,6 +49,8 @@ static void genmask_u128_test(struct kunit *test)
>         KUNIT_EXPECT_EQ(test, 0xffffffffffffffffULL, GENMASK_U128(63, 0));
>         KUNIT_EXPECT_EQ(test, 0xffffffffffffffffULL, GENMASK_U128(64, 0) >> 1);
>         KUNIT_EXPECT_EQ(test, 0x00000000ffffffffULL, GENMASK_U128(81, 50) >> 50);
> +       KUNIT_EXPECT_EQ(test, 0x0000000000000003ULL, GENMASK_U128(127, 126) >> 126);
> +       KUNIT_EXPECT_EQ(test, 0x0000000000000001ULL, GENMASK_U128(127, 127) >> 127);
> 
> The most significant bit in the generate mask can be added separately
> , thus voiding that extra shift. The following patch solves the build
> problem.
> 
> diff --git a/include/uapi/linux/bits.h b/include/uapi/linux/bits.h
> index 4d4b7b08003c..4e50f635c6d9 100644
> --- a/include/uapi/linux/bits.h
> +++ b/include/uapi/linux/bits.h
> @@ -13,6 +13,6 @@
>           (~_ULL(0) >> (__BITS_PER_LONG_LONG - 1 - (h))))
>  
>  #define __GENMASK_U128(h, l) \
> -       ((_BIT128((h) + 1)) - (_BIT128(l)))
> +       (((_BIT128(h)) - (_BIT128(l))) | (_BIT128(h)))

Can you send v3 with the fix? I will drop this series from bitmap-for-next
meanwhile.

Can you also extend the test for more? I'd like to check for example
the (127, 0) range. Also, can you please check both HI and LO parts 
the mask?

For the v3, please share the link to the following series that
actually uses new API.

Thanks,
Yury

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

[Anshuman Khandual]

On 8/17/24 19:27, Yury Norov wrote:
> On Fri, Aug 16, 2024 at 11:58:04AM +0530, Anshuman Khandual wrote:
>>
>>
>> On 8/1/24 12:46, 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
>>> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>>> ---
>>>  include/linux/bits.h       | 13 +++++++++++++
>>>  include/uapi/linux/bits.h  |  3 +++
>>>  include/uapi/linux/const.h | 15 +++++++++++++++
>>>  3 files changed, 31 insertions(+)
>>>
>>> diff --git a/include/linux/bits.h b/include/linux/bits.h
>>> index 0eb24d21aac2..bf99feb5570e 100644
>>> --- a/include/linux/bits.h
>>> +++ b/include/linux/bits.h
>>> @@ -36,4 +36,17 @@
>>>  #define GENMASK_ULL(h, l) \
>>>  	(GENMASK_INPUT_CHECK(h, l) + __GENMASK_ULL(h, l))
>>>  
>>> +/*
>>> + * Missing asm support
>>> + *
>>> + * __GENMASK_U128() depends on _BIT128() which would not work
>>> + * in the asm code, as it shifts an 'unsigned __init128' data
>>> + * type instead of direct representation of 128 bit constants
>>> + * such as long and unsigned long. The fundamental problem is
>>> + * that a 128 bit constant will get silently truncated by the
>>> + * gcc compiler.
>>> + */
>>> +#define GENMASK_U128(h, l) \
>>> +	(GENMASK_INPUT_CHECK(h, l) + __GENMASK_U128(h, l))
>>> +
>>>  #endif	/* __LINUX_BITS_H */
>>> 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..5be12e8f8f9c 100644
>>> --- a/include/uapi/linux/const.h
>>> +++ b/include/uapi/linux/const.h
>>> @@ -28,6 +28,21 @@
>>>  #define _BITUL(x)	(_UL(1) << (x))
>>>  #define _BITULL(x)	(_ULL(1) << (x))
>>>  
>>> +/*
>>> + * Missing asm support
>>> + *
>>> + * __BIT128() would not work in the asm code, as it shifts an
>>> + * 'unsigned __init128' data type as direct representation of
>>> + * 128 bit constants is not supported in the gcc compiler, as
>>> + * they get silently truncated.
>>> + *
>>> + * TODO: Please revisit this implementation when gcc compiler
>>> + * starts representing 128 bit constants directly like long
>>> + * and unsigned long etc. Subsequently drop the comment for
>>> + * GENMASK_U128() which would then start supporting asm code.
>>> + */
>>> +#define _BIT128(x)	((unsigned __int128)(1) << (x))
>>> +
>>>  #define __ALIGN_KERNEL(x, a)		__ALIGN_KERNEL_MASK(x, (__typeof__(x))(a) - 1)
>>>  #define __ALIGN_KERNEL_MASK(x, mask)	(((x) + (mask)) & ~(mask))
>>>  
>>
>> Hello Yuri/Arnd,
>>
>> This proposed GENMASK_U128(h, l) warns during build when the higher end
>> bit is 127 (which in itself is a valid input).
>>
>> ./include/uapi/linux/const.h:45:44: warning: left shift count >= width of type [-Wshift-count-overflow]
>>    45 | #define _BIT128(x) ((unsigned __int128)(1) << (x))
>>       |                                            ^~
>> ./include/asm-generic/bug.h:123:25: note: in definition of macro ‘WARN_ON’
>>   123 |  int __ret_warn_on = !!(condition);    \
>>       |                         ^~~~~~~~~
>> ./include/uapi/linux/bits.h:16:4: note: in expansion of macro ‘_BIT128’
>>    16 |  ((_BIT128((h) + 1)) - (_BIT128(l)))
>>       |    ^~~~~~~
>> ./include/linux/bits.h:51:31: note: in expansion of macro ‘__GENMASK_U128’
>>    51 |  (GENMASK_INPUT_CHECK(h, l) + __GENMASK_U128(h, l))
>>       |                               ^~~~~~~~~~~~~~
>>
>> This is caused by ((unsigned __int128)(1) << (128)) which is generated
>> via (h + 1) element in __GENMASK_U128().
>>
>> #define _BIT128(x)	((unsigned __int128)(1) << (x))
>> #define __GENMASK_U128(h, l) \
>> 	((_BIT128((h) + 1)) - (_BIT128(l)))
>>
>> Adding some extra tests in lib/test_bits.c exposes this build problem,
>> although it does not fail these new tests.
>>
>> [    1.719221]     # Subtest: bits-test
>> [    1.719291]     # module: test_bits
>> [    1.720522]     ok 1 genmask_test
>> [    1.721570]     ok 2 genmask_ull_test
>> [    1.722668]     ok 3 genmask_u128_test
>> [    1.723760]     ok 4 genmask_input_check_test
>> [    1.723909] # bits-test: pass:4 fail:0 skip:0 total:4
>> [    1.724101] ok 1 bits-test
>>
>> diff --git a/lib/test_bits.c b/lib/test_bits.c
>> index d3d858b24e02..7a972edc7122 100644
>> --- a/lib/test_bits.c
>> +++ b/lib/test_bits.c
>> @@ -49,6 +49,8 @@ static void genmask_u128_test(struct kunit *test)
>>         KUNIT_EXPECT_EQ(test, 0xffffffffffffffffULL, GENMASK_U128(63, 0));
>>         KUNIT_EXPECT_EQ(test, 0xffffffffffffffffULL, GENMASK_U128(64, 0) >> 1);
>>         KUNIT_EXPECT_EQ(test, 0x00000000ffffffffULL, GENMASK_U128(81, 50) >> 50);
>> +       KUNIT_EXPECT_EQ(test, 0x0000000000000003ULL, GENMASK_U128(127, 126) >> 126);
>> +       KUNIT_EXPECT_EQ(test, 0x0000000000000001ULL, GENMASK_U128(127, 127) >> 127);
>>
>> The most significant bit in the generate mask can be added separately
>> , thus voiding that extra shift. The following patch solves the build
>> problem.
>>
>> diff --git a/include/uapi/linux/bits.h b/include/uapi/linux/bits.h
>> index 4d4b7b08003c..4e50f635c6d9 100644
>> --- a/include/uapi/linux/bits.h
>> +++ b/include/uapi/linux/bits.h
>> @@ -13,6 +13,6 @@
>>           (~_ULL(0) >> (__BITS_PER_LONG_LONG - 1 - (h))))
>>  
>>  #define __GENMASK_U128(h, l) \
>> -       ((_BIT128((h) + 1)) - (_BIT128(l)))
>> +       (((_BIT128(h)) - (_BIT128(l))) | (_BIT128(h)))
> 
> Can you send v3 with the fix? I will drop this series from bitmap-for-next
> meanwhile.

Sure, will send out V4 (current series being V3).

> 
> Can you also extend the test for more? I'd like to check for example
> the (127, 0) range. Also, can you please check both HI and LO parts 
> the mask?

Following tests form the complete set on GENMASK_U128(). The last four tests
here will be added in V4. Please also note that only 64 bit mask portion can
be tested at once.

        KUNIT_EXPECT_EQ(test, 0x0000000000ff0000ULL, GENMASK_U128(87, 80) >> 64);
        KUNIT_EXPECT_EQ(test, 0x0000000000ffffffULL, GENMASK_U128(87, 64) >> 64);
        KUNIT_EXPECT_EQ(test, 0x0000000000000001ULL, GENMASK_U128(0, 0));
        KUNIT_EXPECT_EQ(test, 0xffffffffffffffffULL, GENMASK_U128(63, 0));
        KUNIT_EXPECT_EQ(test, 0xffffffffffffffffULL, GENMASK_U128(64, 0) >> 1);
        KUNIT_EXPECT_EQ(test, 0x00000000ffffffffULL, GENMASK_U128(81, 50) >> 50);
        KUNIT_EXPECT_EQ(test, 0x0000000000000003ULL, GENMASK_U128(127, 126) >> 126);
        KUNIT_EXPECT_EQ(test, 0x0000000000000001ULL, GENMASK_U128(127, 127) >> 127);
        KUNIT_EXPECT_EQ(test, 0xffffffffffffffffULL, GENMASK_U128(127, 0) >> 64);
        KUNIT_EXPECT_EQ(test, 0xffffffffffffffffULL, GENMASK_U128(127, 0) & ~GENMASK_U128(127, 64));

Although, please do let me know if you would like to add some more tests.

> 
> For the v3, please share the link to the following series that
> actually uses new API.

Sure, will add the following link pointing to the work in progress on arm64.

https://lore.kernel.org/all/20240801054436.612024-1-anshuman.khandual@arm.com/

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

[Arnd Bergmann]

On Fri, Aug 16, 2024, at 08:28, Anshuman Khandual wrote:
>
> This is caused by ((unsigned __int128)(1) << (128)) which is generated
> via (h + 1) element in __GENMASK_U128().
>
> #define _BIT128(x)	((unsigned __int128)(1) << (x))
> #define __GENMASK_U128(h, l) \
> 	((_BIT128((h) + 1)) - (_BIT128(l)))

Right, makes sense.

>
> The most significant bit in the generate mask can be added separately
> , thus voiding that extra shift. The following patch solves the build
> problem.
>
> diff --git a/include/uapi/linux/bits.h b/include/uapi/linux/bits.h
> index 4d4b7b08003c..4e50f635c6d9 100644
> --- a/include/uapi/linux/bits.h
> +++ b/include/uapi/linux/bits.h
> @@ -13,6 +13,6 @@
>           (~_ULL(0) >> (__BITS_PER_LONG_LONG - 1 - (h))))
> 
>  #define __GENMASK_U128(h, l) \
> -       ((_BIT128((h) + 1)) - (_BIT128(l)))
> +       (((_BIT128(h)) - (_BIT128(l))) | (_BIT128(h)))

This could probably use a comment then, as it's less intuitive.

Another solution might be to use a double shift, as in

#define __GENMASK_U128(h, l) \
       ((_BIT128((h)) << 1) - (_BIT128(l)))

but I have not checked if this is correct for all inputs
or if it avoids the warning. Your version looks fine to
me otherwise.

    Arnd

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

[Anshuman Khandual]

On 8/19/24 12:43, Arnd Bergmann wrote:
> On Fri, Aug 16, 2024, at 08:28, Anshuman Khandual wrote:
>>
>> This is caused by ((unsigned __int128)(1) << (128)) which is generated
>> via (h + 1) element in __GENMASK_U128().
>>
>> #define _BIT128(x)	((unsigned __int128)(1) << (x))
>> #define __GENMASK_U128(h, l) \
>> 	((_BIT128((h) + 1)) - (_BIT128(l)))
> 
> Right, makes sense.
> 
>>
>> The most significant bit in the generate mask can be added separately
>> , thus voiding that extra shift. The following patch solves the build
>> problem.
>>
>> diff --git a/include/uapi/linux/bits.h b/include/uapi/linux/bits.h
>> index 4d4b7b08003c..4e50f635c6d9 100644
>> --- a/include/uapi/linux/bits.h
>> +++ b/include/uapi/linux/bits.h
>> @@ -13,6 +13,6 @@
>>           (~_ULL(0) >> (__BITS_PER_LONG_LONG - 1 - (h))))
>>
>>  #define __GENMASK_U128(h, l) \
>> -       ((_BIT128((h) + 1)) - (_BIT128(l)))
>> +       (((_BIT128(h)) - (_BIT128(l))) | (_BIT128(h)))
> 
> This could probably use a comment then, as it's less intuitive.

Right, a comment explaining the need for this additional bit to
cover the corner 127 bit case could be added for reference.

> 
> Another solution might be to use a double shift, as in
> 
> #define __GENMASK_U128(h, l) \
>        ((_BIT128((h)) << 1) - (_BIT128(l)))

This looks much cleaner, passed all the tests without warning.
But for the above 127 bit case, wondering how the bit position
is managed after the second shift operation because it still
goes beyond __int128 element's 128 bit representation.

(_BIT128((127)) << 1)
(((unsigned __int128)(1) << (127)) << 1)

Should not the second shift operation warn about the possible
overflow scenario ? But actually it does not. Or the compiler
is too smart in detecting what's happening next in the overall
equation and do the needful while creating the mask below the
highest bit.

> 
> but I have not checked if this is correct for all inputs
> or if it avoids the warning. Your version looks fine to
> me otherwise.

This approach is much cleaner, passes all tests without warning,
unless something else shows up, will fold this in instead.

Re: [PATCH V3 2/2] lib/test_bits.c: Add tests for GENMASK_U128()

[Anshuman Khandual]

On 8/1/24 12:46, Anshuman Khandual wrote:
> This adds GENMASK_U128() tests although currently only 64 bit wide masks
> are being tested.
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: linux-kernel@vger.kernel.org
> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
>  lib/test_bits.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/lib/test_bits.c b/lib/test_bits.c
> index 01313980f175..d3d858b24e02 100644
> --- a/lib/test_bits.c
> +++ b/lib/test_bits.c
> @@ -39,6 +39,26 @@ static void genmask_ull_test(struct kunit *test)
>  #endif
>  }
>  
> +static void genmask_u128_test(struct kunit *test)
> +{
> +#ifdef CONFIG_ARCH_SUPPORTS_INT128
> +	/* Tests mask generation only when the mask width is within 64 bits */
> +	KUNIT_EXPECT_EQ(test, 0x0000000000ff0000ULL, GENMASK_U128(87, 80) >> 64);
> +	KUNIT_EXPECT_EQ(test, 0x0000000000ffffffULL, GENMASK_U128(87, 64) >> 64);
> +	KUNIT_EXPECT_EQ(test, 0x0000000000000001ULL, GENMASK_U128(0, 0));
> +	KUNIT_EXPECT_EQ(test, 0xffffffffffffffffULL, GENMASK_U128(63, 0));
> +	KUNIT_EXPECT_EQ(test, 0xffffffffffffffffULL, GENMASK_U128(64, 0) >> 1);
> +	KUNIT_EXPECT_EQ(test, 0x00000000ffffffffULL, GENMASK_U128(81, 50) >> 50);
> +
> +#ifdef TEST_GENMASK_FAILURES
> +	/* these should fail compilation */
> +	GENMASK_U128(0, 1);
> +	GENMASK_U128(0, 10);
> +	GENMASK_U128(9, 10);
> +#endif /* TEST_GENMASK_FAILURES */
> +#endif /* CONFIG_ARCH_SUPPORTS_INT128 */
> +}
> +
>  static void genmask_input_check_test(struct kunit *test)
>  {
>  	unsigned int x, y;
> @@ -56,12 +76,15 @@ static void genmask_input_check_test(struct kunit *test)
>  	/* Valid input */
>  	KUNIT_EXPECT_EQ(test, 0, GENMASK_INPUT_CHECK(1, 1));
>  	KUNIT_EXPECT_EQ(test, 0, GENMASK_INPUT_CHECK(39, 21));
> +	KUNIT_EXPECT_EQ(test, 0, GENMASK_INPUT_CHECK(100, 80));
> +	KUNIT_EXPECT_EQ(test, 0, GENMASK_INPUT_CHECK(110, 65));
>  }
>  
>  
>  static struct kunit_case bits_test_cases[] = {
>  	KUNIT_CASE(genmask_test),
>  	KUNIT_CASE(genmask_ull_test),
> +	KUNIT_CASE(genmask_u128_test),
>  	KUNIT_CASE(genmask_input_check_test),
>  	{}
>  };

Here is the updated test case to cover some more corner cases. Please
do let me know if something more can be added here to the list.

--- a/lib/test_bits.c
+++ b/lib/test_bits.c
@@ -39,6 +39,36 @@ static void genmask_ull_test(struct kunit *test)
 #endif
 }
 
+static void genmask_u128_test(struct kunit *test)
+{
+#ifdef CONFIG_ARCH_SUPPORTS_INT128
+       /* Below 64 bit masks */
+       KUNIT_EXPECT_EQ(test, 0x0000000000000001ull, GENMASK_U128(0, 0));
+       KUNIT_EXPECT_EQ(test, 0x0000000000000003ull, GENMASK_U128(1, 0));
+       KUNIT_EXPECT_EQ(test, 0x0000000000000006ull, GENMASK_U128(2, 1));
+       KUNIT_EXPECT_EQ(test, 0x00000000ffffffffull, GENMASK_U128(31, 0));
+       KUNIT_EXPECT_EQ(test, 0x000000ffffe00000ull, GENMASK_U128(39, 21));
+       KUNIT_EXPECT_EQ(test, 0xffffffffffffffffull, GENMASK_U128(63, 0));
+
+       /* Above 64 bit masks - only 64 bit portion can be validated once */
+       KUNIT_EXPECT_EQ(test, 0xffffffffffffffffull, GENMASK_U128(64, 0) >> 1);
+       KUNIT_EXPECT_EQ(test, 0x00000000ffffffffull, GENMASK_U128(81, 50) >> 50);
+       KUNIT_EXPECT_EQ(test, 0x0000000000ffffffull, GENMASK_U128(87, 64) >> 64);
+       KUNIT_EXPECT_EQ(test, 0x0000000000ff0000ull, GENMASK_U128(87, 80) >> 64);
+
+       KUNIT_EXPECT_EQ(test, 0xffffffffffffffffull, GENMASK_U128(127, 0) >> 64);
+       KUNIT_EXPECT_EQ(test, 0xffffffffffffffffull, (u64)GENMASK_U128(127, 0));
+       KUNIT_EXPECT_EQ(test, 0x0000000000000003ull, GENMASK_U128(127, 126) >> 126);
+       KUNIT_EXPECT_EQ(test, 0x0000000000000001ull, GENMASK_U128(127, 127) >> 127);
+#ifdef TEST_GENMASK_FAILURES
+       /* these should fail compilation */
+       GENMASK_U128(0, 1);
+       GENMASK_U128(0, 10);
+       GENMASK_U128(9, 10);
+#endif /* TEST_GENMASK_FAILURES */
+#endif /* CONFIG_ARCH_SUPPORTS_INT128 */
+}
+
 static void genmask_input_check_test(struct kunit *test)
 {
        unsigned int x, y;
@@ -56,12 +86,16 @@ static void genmask_input_check_test(struct kunit *test)
        /* Valid input */
        KUNIT_EXPECT_EQ(test, 0, GENMASK_INPUT_CHECK(1, 1));
        KUNIT_EXPECT_EQ(test, 0, GENMASK_INPUT_CHECK(39, 21));
+       KUNIT_EXPECT_EQ(test, 0, GENMASK_INPUT_CHECK(100, 80));
+       KUNIT_EXPECT_EQ(test, 0, GENMASK_INPUT_CHECK(110, 65));
+       KUNIT_EXPECT_EQ(test, 0, GENMASK_INPUT_CHECK(127, 0));
 }
 
 
 static struct kunit_case bits_test_cases[] = {
        KUNIT_CASE(genmask_test),
        KUNIT_CASE(genmask_ull_test),
+       KUNIT_CASE(genmask_u128_test),
        KUNIT_CASE(genmask_input_check_test),
        {}
 };
(END)
 

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

[Arnd Bergmann]

On Tue, Aug 20, 2024, at 03:25, Anshuman Khandual wrote:
> On 8/19/24 12:43, Arnd Bergmann wrote:
>
> Should not the second shift operation warn about the possible
> overflow scenario ? But actually it does not. Or the compiler
> is too smart in detecting what's happening next in the overall
> equation and do the needful while creating the mask below the
> highest bit.

Not sure about the reasoning behind the compiler warning for
one but not the other, but I know that we rely on similar
behavior in places like:

#define upper_32_bits(n) ((u32)(((n) >> 16) >> 16))

which is intended to return a zero without a compiler
warning when passing an 'unsigned long' input on 32-bit
architectures.

      Arnd