linux-kernel - Re: Re: [PATCH v3 1/3] bits: introduce fixed-type genmasks

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Zde9Je1C0LLbXE6z@yury-ThinkPad>
Date: Thu, 22 Feb 2024 13:31:17 -0800
From: Yury Norov <yury.norov@...il.com>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc: Lucas De Marchi <lucas.demarchi@...el.com>,
	Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
	linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
	Jani Nikula <jani.nikula@...ux.intel.com>,
	intel-xe@...ts.freedesktop.org, intel-gfx@...ts.freedesktop.org,
	Jani Nikula <jani.nikula@...el.com>
Subject: Re: Re: [PATCH v3 1/3] bits: introduce fixed-type genmasks

On Thu, Feb 22, 2024 at 05:04:10PM +0200, Andy Shevchenko wrote:
> On Thu, Feb 22, 2024 at 06:49:59AM -0800, Yury Norov wrote:
> > On Wed, Feb 21, 2024 at 03:59:06PM -0600, Lucas De Marchi wrote:
> 
> ...
> 
> > +#define __GENMASK(t, h, l) \
> > +	((~0 - (1 << (l)) + 1) & (~0 >> (BITS_PER_LONG - 1 - (h))))
> 
> What's wrong on using the UL/ULL() macros?

Nothing wrong except that in the !__ASSEMBLY section they all are
useless. And having that in mind, useless macros may hurt readability.

> Also it would be really good to avoid bifurcation of the implementations of
> __GENMASK() for both cases.

Not exactly. It would be really good if GENMASK_XX() will share the
implementation (and they do). The underscored helper macro is not
intended to be used directly, and I think nobody cares.

> ...
> 
> > -#define __GENMASK(h, l) \
> > -	(((~UL(0)) - (UL(1) << (l)) + 1) & \
> > -	 (~UL(0) >> (BITS_PER_LONG - 1 - (h))))
> 
> This at bare minimum can be left untouched for asm case, no?

As soon as we have to have different versions for the macro depending
on __ASSEMBLY__, I would prefer to remove all compatibility black
magic. After all, the <linux/bits.h> machinery to me is about the same
level of abstraction as the stuff in <linux/const.h>, and in future we
can try to remove dependency on it.

This all is not a big deal to me. I can keep the old implementation
for the asm, if you think it's really important.

What are you thinking guys?

Thanks,
Yury