linux-kernel - [PATCH v2 0/3] bits: Split asm and non-asm GENMASK*() and unify definitions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20250609-consolidate-genmask-v2-0-b8cce8107e49@wanadoo.fr>
Date: Mon, 09 Jun 2025 11:45:44 +0900
From: Vincent Mailhol via B4 Relay <devnull+mailhol.vincent.wanadoo.fr@...nel.org>
To: Yury Norov <yury.norov@...il.com>, 
 Lucas De Marchi <lucas.demarchi@...el.com>, 
 Rasmus Villemoes <linux@...musvillemoes.dk>, 
 Jani Nikula <jani.nikula@...ux.intel.com>, 
 Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>, 
 Rodrigo Vivi <rodrigo.vivi@...el.com>, 
 Tvrtko Ursulin <tursulin@...ulin.net>, David Airlie <airlied@...il.com>, 
 Simona Vetter <simona@...ll.ch>, Andrew Morton <akpm@...ux-foundation.org>
Cc: linux-kernel@...r.kernel.org, intel-gfx@...ts.freedesktop.org, 
 dri-devel@...ts.freedesktop.org, Andi Shyti <andi.shyti@...ux.intel.com>, 
 David Laight <David.Laight@...LAB.COM>, 
 Andy Shevchenko <andriy.shevchenko@...ux.intel.com>, 
 Catalin Marinas <catalin.marinas@....com>, 
 Anshuman Khandual <anshuman.khandual@....com>, 
 linux-arm-kernel@...ts.infradead.org, 
 Vincent Mailhol <mailhol.vincent@...adoo.fr>
Subject: [PATCH v2 0/3] bits: Split asm and non-asm GENMASK*() and unify
 definitions

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*() (already merged upstream)
  #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 generalised to u128
integers, whereas after the split, it can.

And so, after implementing both, the asm seems way cleaner than the
non-asm split and is, I think, 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/-9 (-4)
  Function                                     old     new   delta
  intel_psr_invalidate                         352     354      +2
  mst_stream_compute_config                   1589    1590      +1
  intel_psr_flush                              707     708      +1
  intel_dp_compute_link_config                1338    1339      +1
  intel_drrs_activate                          398     395      -3
  cfg80211_inform_bss_data                    5137    5131      -6
  Total: Before=23333846, After=23333842, chg -0.00%

(done with GCC 12.4.1 on an x86_64 defconfig)

--
2.43.0

Signed-off-by: Vincent Mailhol <mailhol.vincent@...adoo.fr>
---
Changes from v1:

  - Meanwhile, in commit db6fe4d61ece ("lib: Move KUnit tests into
    tests/ subdirectory"), lib/test_bits.c was moved to
    lib/tests/test_bits.c.
    Rebase onto: 6.16-rc1

  - Minor editorial changes to the cover letter.

  - Aside from the above, this is just a resend.

  - Link to v1: https://lore.kernel.org/r/20250322-consolidate-genmask-v1-0-54bfd36c5643@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/tests/test_bits.c | 19 +++++++++++++++++++
 2 files changed, 25 insertions(+), 23 deletions(-)
---
base-commit: d9946fe286439c2aeaa7953b8c316efe5b83d515
change-id: 20250320-consolidate-genmask-6cd02abadf82

Best regards,
-- 
Vincent Mailhol <mailhol.vincent@...adoo.fr>