* [PATCH 1/3] bits: split the definition of the asm and non-asm GENMASK*()
2025-03-22 10:39 [PATCH 0/3] bits: Split asm and non-asm GENMASK*() and unify definitions Vincent Mailhol via B4 Relay
@ 2025-03-22 10:39 ` Vincent Mailhol via B4 Relay
2025-03-22 10:39 ` [PATCH 2/3] bits: unify the " Vincent Mailhol via B4 Relay
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Vincent Mailhol via B4 Relay @ 2025-03-22 10:39 UTC (permalink / raw)
To: Yury Norov, Lucas De Marchi, Rasmus Villemoes, Jani Nikula,
Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin, David Airlie,
Simona Vetter, Andrew Morton
Cc: linux-kernel, intel-gfx, dri-devel, Andi Shyti, David Laight,
Andy Shevchenko, Vincent Mailhol
From: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
In an upcoming change, the non-asm GENMASK*() will all be unified to
depend on GENMASK_TYPE() which indirectly depend on sizeof(), something
not available in asm.
Instead of adding further complexity to GENMASK_TYPE() to make it work
for both asm and non asm, just split the definition of the two variants.
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
This patch previously belonged to this series:
bits: Fixed-type GENMASK_U*() and BIT_U*()
Link: https://lore.kernel.org/r/20250308-fixed-type-genmasks-v6-0-f59315e73c29@wanadoo.fr
Below changelog also contains the history from the previous series.
Changelog:
v6 (previous series) -> v1 (new series):
- Do not rephrase the comment saying that BUILD_BUG_ON() is not
available in asm code. Leave it as it is.
- Add Lucas reviewed-by tag.
v5 -> v6:
- Restore the comment saying that BUILD_BUG_ON() is not available in asm
code.
v4 -> v5:
- Use tab indentations instead of single space to separate the
macro name from its body.
v3 -> v4:
- New patch in the series
---
include/linux/bits.h | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/include/linux/bits.h b/include/linux/bits.h
index 6a942ea9ab380d3bd0e521916caa1d59db8031c0..9dd2d244c1f37fde995ed65c1eed879bb2a994a8 100644
--- a/include/linux/bits.h
+++ b/include/linux/bits.h
@@ -35,6 +35,11 @@
#define GENMASK_INPUT_CHECK(h, l) BUILD_BUG_ON_ZERO(const_true((l) > (h)))
+#define GENMASK(h, l) \
+ (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
+#define GENMASK_ULL(h, l) \
+ (GENMASK_INPUT_CHECK(h, l) + __GENMASK_ULL(h, l))
+
/*
* Generate a mask for the specified type @t. Additional checks are made to
* guarantee the value returned fits in that type, relying on
@@ -79,15 +84,11 @@
* BUILD_BUG_ON_ZERO is not available in h files included from asm files,
* disable the input check if that is the case.
*/
-#define GENMASK_INPUT_CHECK(h, l) 0
+#define GENMASK(h, l) __GENMASK(h, l)
+#define GENMASK_ULL(h, l) __GENMASK_ULL(h, l)
#endif /* !defined(__ASSEMBLY__) */
-#define GENMASK(h, l) \
- (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
-#define GENMASK_ULL(h, l) \
- (GENMASK_INPUT_CHECK(h, l) + __GENMASK_ULL(h, l))
-
#if !defined(__ASSEMBLY__)
/*
* Missing asm support
--
2.48.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/3] bits: unify the non-asm GENMASK*()
2025-03-22 10:39 [PATCH 0/3] bits: Split asm and non-asm GENMASK*() and unify definitions Vincent Mailhol via B4 Relay
2025-03-22 10:39 ` [PATCH 1/3] bits: split the definition of the asm and non-asm GENMASK*() Vincent Mailhol via B4 Relay
@ 2025-03-22 10:39 ` Vincent Mailhol via B4 Relay
2025-03-22 10:39 ` [PATCH 3/3] test_bits: add tests for __GENMASK() and __GENMASK_ULL() Vincent Mailhol via B4 Relay
2025-03-24 16:11 ` [PATCH 0/3] bits: Split asm and non-asm GENMASK*() and unify definitions Yury Norov
3 siblings, 0 replies; 6+ messages in thread
From: Vincent Mailhol via B4 Relay @ 2025-03-22 10:39 UTC (permalink / raw)
To: Yury Norov, Lucas De Marchi, Rasmus Villemoes, Jani Nikula,
Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin, David Airlie,
Simona Vetter, Andrew Morton
Cc: linux-kernel, intel-gfx, dri-devel, Andi Shyti, David Laight,
Andy Shevchenko, Vincent Mailhol
From: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
The newly introduced GENMASK_TYPE() macro can also be used to generate
the pre-existing non-asm GENMASK*() variants.
Apply GENMASK_TYPE() to GENMASK(), GENMASK_ULL() and GENMASK_U128().
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
include/linux/bits.h | 26 ++++----------------------
1 file changed, 4 insertions(+), 22 deletions(-)
diff --git a/include/linux/bits.h b/include/linux/bits.h
index 9dd2d244c1f37fde995ed65c1eed879bb2a994a8..025c9cb994b148f8d2e50e69be929d356c6c4322 100644
--- a/include/linux/bits.h
+++ b/include/linux/bits.h
@@ -2,10 +2,8 @@
#ifndef __LINUX_BITS_H
#define __LINUX_BITS_H
-#include <linux/const.h>
#include <vdso/bits.h>
#include <uapi/linux/bits.h>
-#include <asm/bitsperlong.h>
#define BIT_MASK(nr) (UL(1) << ((nr) % BITS_PER_LONG))
#define BIT_WORD(nr) ((nr) / BITS_PER_LONG)
@@ -35,11 +33,6 @@
#define GENMASK_INPUT_CHECK(h, l) BUILD_BUG_ON_ZERO(const_true((l) > (h)))
-#define GENMASK(h, l) \
- (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
-#define GENMASK_ULL(h, l) \
- (GENMASK_INPUT_CHECK(h, l) + __GENMASK_ULL(h, l))
-
/*
* Generate a mask for the specified type @t. Additional checks are made to
* guarantee the value returned fits in that type, relying on
@@ -55,10 +48,14 @@
(type_max(t) << (l) & \
type_max(t) >> (BITS_PER_TYPE(t) - 1 - (h)))))
+#define GENMASK(h, l) GENMASK_TYPE(unsigned long, h, l)
+#define GENMASK_ULL(h, l) GENMASK_TYPE(unsigned long long, h, l)
+
#define GENMASK_U8(h, l) GENMASK_TYPE(u8, h, l)
#define GENMASK_U16(h, l) GENMASK_TYPE(u16, h, l)
#define GENMASK_U32(h, l) GENMASK_TYPE(u32, h, l)
#define GENMASK_U64(h, l) GENMASK_TYPE(u64, h, l)
+#define GENMASK_U128(h, l) GENMASK_TYPE(u128, h, l)
/*
* Fixed-type variants of BIT(), with additional checks like GENMASK_TYPE(). The
@@ -89,19 +86,4 @@
#endif /* !defined(__ASSEMBLY__) */
-#if !defined(__ASSEMBLY__)
-/*
- * Missing asm support
- *
- * __GENMASK_U128() depends on _BIT128() which would not work
- * in the asm code, as it shifts an 'unsigned __int128' 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
-
#endif /* __LINUX_BITS_H */
--
2.48.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] test_bits: add tests for __GENMASK() and __GENMASK_ULL()
2025-03-22 10:39 [PATCH 0/3] bits: Split asm and non-asm GENMASK*() and unify definitions Vincent Mailhol via B4 Relay
2025-03-22 10:39 ` [PATCH 1/3] bits: split the definition of the asm and non-asm GENMASK*() Vincent Mailhol via B4 Relay
2025-03-22 10:39 ` [PATCH 2/3] bits: unify the " Vincent Mailhol via B4 Relay
@ 2025-03-22 10:39 ` Vincent Mailhol via B4 Relay
2025-03-24 16:11 ` [PATCH 0/3] bits: Split asm and non-asm GENMASK*() and unify definitions Yury Norov
3 siblings, 0 replies; 6+ messages in thread
From: Vincent Mailhol via B4 Relay @ 2025-03-22 10:39 UTC (permalink / raw)
To: Yury Norov, Lucas De Marchi, Rasmus Villemoes, Jani Nikula,
Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin, David Airlie,
Simona Vetter, Andrew Morton
Cc: linux-kernel, intel-gfx, dri-devel, Andi Shyti, David Laight,
Andy Shevchenko, Vincent Mailhol
From: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
The definitions of GENMASK() and GENMASK_ULL() do not depend any more
on __GENMASK() and __GENMASK_ULL(). Duplicate the existing unit tests
so that __GENMASK{,ULL}() are still covered.
Because __GENMASK() and __GENMASK_ULL() do use GENMASK_INPUT_CHECK(),
drop the TEST_GENMASK_FAILURES negative tests.
It would be good to have a small assembly test case for GENMASK*() in
case somebody decides to unify both in the future. However, I lack
expertise in assembly to do so. Instead add a FIXME message to
highlight the absence of the asm unit test.
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
This patch previously belonged to this series:
bits: Fixed-type GENMASK_U*() and BIT_U*()
Link: https://lore.kernel.org/r/20250308-fixed-type-genmasks-v6-0-f59315e73c29@wanadoo.fr
Below changelog also contains the history from the previous series.
Changelog:
v6 (previous series) -> v1 (new series):
- Add a message in the commit description to explain why the
negative tests are dropped.
- Add Lucas's Reviewed-by tag.
v5 -> v6:
- Add a FIXME message to highlight the absence of the asm
GENMASK*() unit tests.
v4 -> v5:
- No changes.
v3 -> v4:
- New patch.
---
lib/test_bits.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/lib/test_bits.c b/lib/test_bits.c
index 47325b41515fde2c3ed434ed6f4094925c98886b..ab88e50d2edfa2b011f07d50460ac8ea6ff99923 100644
--- a/lib/test_bits.c
+++ b/lib/test_bits.c
@@ -26,6 +26,23 @@ static_assert(assert_type(u16, GENMASK_U16(15, 0)) == U16_MAX);
static_assert(assert_type(u32, GENMASK_U32(31, 0)) == U32_MAX);
static_assert(assert_type(u64, GENMASK_U64(63, 0)) == U64_MAX);
+/* FIXME: add a test case written in asm for GENMASK() and GENMASK_ULL() */
+
+static void __genmask_test(struct kunit *test)
+{
+ KUNIT_EXPECT_EQ(test, 1ul, __GENMASK(0, 0));
+ KUNIT_EXPECT_EQ(test, 3ul, __GENMASK(1, 0));
+ KUNIT_EXPECT_EQ(test, 6ul, __GENMASK(2, 1));
+ KUNIT_EXPECT_EQ(test, 0xFFFFFFFFul, __GENMASK(31, 0));
+}
+
+static void __genmask_ull_test(struct kunit *test)
+{
+ KUNIT_EXPECT_EQ(test, 1ull, __GENMASK_ULL(0, 0));
+ KUNIT_EXPECT_EQ(test, 3ull, __GENMASK_ULL(1, 0));
+ KUNIT_EXPECT_EQ(test, 0x000000ffffe00000ull, __GENMASK_ULL(39, 21));
+ KUNIT_EXPECT_EQ(test, 0xffffffffffffffffull, __GENMASK_ULL(63, 0));
+}
static void genmask_test(struct kunit *test)
{
@@ -123,6 +140,8 @@ static void genmask_input_check_test(struct kunit *test)
static struct kunit_case bits_test_cases[] = {
+ KUNIT_CASE(__genmask_test),
+ KUNIT_CASE(__genmask_ull_test),
KUNIT_CASE(genmask_test),
KUNIT_CASE(genmask_ull_test),
KUNIT_CASE(genmask_u128_test),
--
2.48.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 0/3] bits: Split asm and non-asm GENMASK*() and unify definitions
2025-03-22 10:39 [PATCH 0/3] bits: Split asm and non-asm GENMASK*() and unify definitions Vincent Mailhol via B4 Relay
` (2 preceding siblings ...)
2025-03-22 10:39 ` [PATCH 3/3] test_bits: add tests for __GENMASK() and __GENMASK_ULL() Vincent Mailhol via B4 Relay
@ 2025-03-24 16:11 ` Yury Norov
2025-03-24 16:39 ` Vincent Mailhol
3 siblings, 1 reply; 6+ messages in thread
From: Yury Norov @ 2025-03-24 16:11 UTC (permalink / raw)
To: mailhol.vincent
Cc: Lucas De Marchi, Rasmus Villemoes, Jani Nikula, Joonas Lahtinen,
Rodrigo Vivi, Tvrtko Ursulin, David Airlie, Simona Vetter,
Andrew Morton, linux-kernel, intel-gfx, dri-devel, Andi Shyti,
David Laight, Andy Shevchenko, Anshuman Khandual,
linux-arm-kernel, Catalin Marinas
+ Anshuman Khandual, Catalin Marinas, linux-arm-kernel@lists.infradead.org
This series moves GENMASK_U128 out of uapi. ARM is the only proposed
user. Add ARM people for visibility.
Thanks,
Yury
On Sat, Mar 22, 2025 at 07:39:35PM +0900, Vincent Mailhol via B4 Relay wrote:
> This is a subset of below series:
>
> bits: Fixed-type GENMASK_U*() and BIT_U*()
> Link: https://lore.kernel.org/r/20250308-fixed-type-genmasks-v6-0-f59315e73c29@wanadoo.fr
>
> Yury suggested to split the above series in two steps:
>
> #1 Introduce the new fixed type GENMASK_U*()
> #2 Consolidate the existing GENMASK*()
>
> This new series is the resulting step #2 following the split.
>
> And thus, this series consolidate all the non-asm GENMASK*() so that
> they now all depend on GENMASK_TYPE() which was introduced in step #1.
>
> To do so, I had to split the definition of the asm and non-asm
> GENMASK(). I think this is controversial. So I initially implemented a
> first draft in which both the asm and non-asm version would rely on
> the same helper macro, i.e. adding this:
>
> #define __GENMASK_TYPE(t, w, h, l) \
> (((t)~_ULL(0) << (l)) & \
> ((t)~_ULL(0) >> (w - 1 - (h))))
>
> to uapi/bits.h. And then, the different GENMASK()s would look like
> this:
>
> #define __GENMASK(h, l) __GENMASK_TYPE(unsigned long, __BITS_PER_LONG, h, l)
>
> and so on.
>
> I implemented it, and the final result looked quite ugly. Not only do
> we need to manually provide the width each time, the biggest concern
> is that adding this to the uapi is asking for trouble. Who knows how
> people are going to use this? And once it is in the uapi, there is
> virtually no way back.
>
> Adding to this, that macro can not even be generalized to u128
> integers, whereas after the split, it can.
>
> And so, after implementing both, the asm and non-asm split seems way
> more clean and I think this is the best compromise.
>
> Aside from the split, the asm's GENMASK() and GENMASK_ULL() are left
> untouched. While there are some strong incentives to also simplify
> these as pointed by David Laight in this thread:
>
> https://lore.kernel.org/all/20250309102312.4ff08576@pumpkin/
>
> this series deliberately limit its scope to the non-asm variants.
>
> Here are the bloat-o-meter stats:
>
> $ ./scripts/bloat-o-meter vmlinux_before.o vmlinux_after.o
> add/remove: 0/0 grow/shrink: 4/2 up/down: 5/-4 (1)
> Function old new delta
> intel_psr_invalidate 666 668 +2
> mst_stream_compute_config 1652 1653 +1
> intel_psr_flush 977 978 +1
> intel_dp_compute_link_config 1327 1328 +1
> cfg80211_inform_bss_data 5109 5108 -1
> intel_drrs_activate 379 376 -3
> Total: Before=22723481, After=22723482, chg +0.00%
>
> (done with GCC 12.4.1 on an x86_64 defconfig)
>
> --
> 2.43.0
>
> Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> ---
> Vincent Mailhol (3):
> bits: split the definition of the asm and non-asm GENMASK*()
> bits: unify the non-asm GENMASK*()
> test_bits: add tests for __GENMASK() and __GENMASK_ULL()
>
> include/linux/bits.h | 29 ++++++-----------------------
> lib/test_bits.c | 19 +++++++++++++++++++
> 2 files changed, 25 insertions(+), 23 deletions(-)
> ---
> base-commit: e3f42c436d7e0cb432935fe3ae275dd8d9b60f71
> change-id: 20250320-consolidate-genmask-6cd02abadf82
> prerequisite-change-id: 20250228-fixed-type-genmasks-8d1a555f34e8:v7
> prerequisite-patch-id: 572c05165229640db7dd8fe4d53e1a33ee5dd586
> prerequisite-patch-id: c16d122a487f83e2866a9a669259db097ef46a70
> prerequisite-patch-id: 35f115c0f1b327f1516cfc38b3076e07713df6cd
> prerequisite-patch-id: 5fe7058f6ea73b37df75d5c39ad69a4da928058d
> prerequisite-patch-id: 82fb628d052ce9f1efac7f3b61eafb2749f95847
>
> Best regards,
> --
> Vincent Mailhol <mailhol.vincent@wanadoo.fr>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 0/3] bits: Split asm and non-asm GENMASK*() and unify definitions
2025-03-24 16:11 ` [PATCH 0/3] bits: Split asm and non-asm GENMASK*() and unify definitions Yury Norov
@ 2025-03-24 16:39 ` Vincent Mailhol
0 siblings, 0 replies; 6+ messages in thread
From: Vincent Mailhol @ 2025-03-24 16:39 UTC (permalink / raw)
To: Yury Norov
Cc: Lucas De Marchi, Rasmus Villemoes, Jani Nikula, Joonas Lahtinen,
Rodrigo Vivi, Tvrtko Ursulin, David Airlie, Simona Vetter,
Andrew Morton, linux-kernel, intel-gfx, dri-devel, Andi Shyti,
David Laight, Andy Shevchenko, Anshuman Khandual,
linux-arm-kernel, Catalin Marinas
On 25/03/2025 at 01:11, Yury Norov wrote:
> + Anshuman Khandual, Catalin Marinas, linux-arm-kernel@lists.infradead.org
>
> This series moves GENMASK_U128 out of uapi. ARM is the only proposed
> user. Add ARM people for visibility.
Actually, not yet. Here, I am decoupling GENMASK_U128() from
__GENMASK_U128(), but I did not touch the uapi.
After this series, __GENMASK_U128() is not used anymore in the kernel,
but I am not brave enough to remove it myself because there is always a
risk that some userland code somewhere is relying on it…
(...)
Yours sincerely,
Vincent Mailhol
^ permalink raw reply [flat|nested] 6+ messages in thread