linux-kernel - Re: [PATCH v2] bitfield.h: add FIELD_MAX() and field

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: <20200310212938.GA17565@ubuntu-m2-xlarge-x86>
Date:   Tue, 10 Mar 2020 14:29:38 -0700
From:   Nathan Chancellor <natechancellor@...il.com>
To:     Alex Elder <elder@...aro.org>
Cc:     David Miller <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Al Viro <viro@...iv.linux.org.uk>,
        Johannes Berg <johannes@...solutions.net>,
        Arnd Bergmann <arnd@...db.de>,
        Masahiro Yamada <masahiroy@...nel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        lkml <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] bitfield.h: add FIELD_MAX() and field_max()

On Thu, Mar 05, 2020 at 10:23:02PM -0600, Alex Elder wrote:
> Define FIELD_MAX(), which supplies the maximum value that can be
> represented by a bitfield.  Define field_max() as well, to go
> along with the lower-case forms of the field mask functions.
> 
> Signed-off-by: Alex Elder <elder@...aro.org>
> Acked-by: Jakub Kicinski <kuba@...nel.org>
> ---
> v2: Added Acked-by from Jakub.
> 
> David, I added you to this because it's probably easiest to take
> this change in through your tree with the rest of the IPA code
> (which uses field_max(), defined here).
> 
> 					-Alex
> 
>  include/linux/bitfield.h | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
> index 4bbb5f1c8b5b..48ea093ff04c 100644
> --- a/include/linux/bitfield.h
> +++ b/include/linux/bitfield.h
> @@ -55,6 +55,19 @@
>  					      (1ULL << __bf_shf(_mask))); \
>  	})
>  
> +/**
> + * FIELD_MAX() - produce the maximum value representable by a field
> + * @_mask: shifted mask defining the field's length and position
> + *
> + * FIELD_MAX() returns the maximum value that can be held in the field
> + * specified by @_mask.
> + */
> +#define FIELD_MAX(_mask)						\
> +	({								\
> +		__BF_FIELD_CHECK(_mask, 0ULL, 0ULL, "FIELD_MAX: ");	\
> +		(typeof(_mask))((_mask) >> __bf_shf(_mask));		\
> +	})
> +
>  /**
>   * FIELD_FIT() - check if value fits in the field
>   * @_mask: shifted mask defining the field's length and position
> @@ -110,6 +123,7 @@ static __always_inline u64 field_mask(u64 field)
>  {
>  	return field / field_multiplier(field);
>  }
> +#define field_max(field)	((typeof(field))field_mask(field))
>  #define ____MAKE_OP(type,base,to,from)					\
>  static __always_inline __##type type##_encode_bits(base v, base field)	\
>  {									\
> -- 
> 2.20.1
> 

Without this patch, the IPA driver that was picked up a couple of days
ago does not build...

$ make -j$(nproc) -s ARCH=arm64 CC=clang CROSS_COMPILE=aarch64-linux-gnu- O=out.arm32 distclean allyesconfig drivers/net/ipa/
../drivers/net/ipa/ipa_cmd.c:182:28: error: implicit declaration of function 'field_max' [-Werror,-Wimplicit-function-declaration]
        BUILD_BUG_ON(TABLE_SIZE > field_max(IP_FLTRT_FLAGS_HASH_SIZE_FMASK));
                                  ^
../drivers/net/ipa/ipa_cmd.c:182:28: note: did you mean 'field_mask'?
../include/linux/bitfield.h:109:28: note: 'field_mask' declared here
static __always_inline u64 field_mask(u64 field)
                           ^
../drivers/net/ipa/ipa_cmd.c:183:28: error: implicit declaration of function 'field_max' [-Werror,-Wimplicit-function-declaration]
        BUILD_BUG_ON(TABLE_SIZE > field_max(IP_FLTRT_FLAGS_NHASH_SIZE_FMASK));
                                  ^
../drivers/net/ipa/gsi.c:220:39: error: implicit declaration of function 'field_max' [-Werror,-Wimplicit-function-declaration]
        BUILD_BUG_ON(GSI_RING_ELEMENT_SIZE > field_max(ELEMENT_SIZE_FMASK));
                                             ^
2 errors generated.
../drivers/net/ipa/gsi.c:220:39: note: did you mean 'field_mask'?
../include/linux/bitfield.h:109:28: note: 'field_mask' declared here
static __always_inline u64 field_mask(u64 field)
                           ^
../drivers/net/ipa/gsi.c:223:39: error: implicit declaration of function 'field_max' [-Werror,-Wimplicit-function-declaration]
        BUILD_BUG_ON(GSI_RING_ELEMENT_SIZE > field_max(EV_ELEMENT_SIZE_FMASK));
                                             ^
make[5]: *** [../scripts/Makefile.build:267: drivers/net/ipa/ipa_cmd.o] Error 1
make[5]: *** Waiting for unfinished jobs....
../drivers/net/ipa/gsi.c:710:16: error: implicit declaration of function 'field_max' [-Werror,-Wimplicit-function-declaration]
                wrr_weight = field_max(WRR_WEIGHT_FMASK);
                             ^
3 errors generated.
make[5]: *** [../scripts/Makefile.build:267: drivers/net/ipa/gsi.o] Error 1
../drivers/net/ipa/ipa_endpoint.c:584:14: error: implicit declaration of function 'field_max' [-Werror,-Wimplicit-function-declaration]
        if (scale > field_max(SCALE_FMASK))
                    ^
../drivers/net/ipa/ipa_endpoint.c:584:14: note: did you mean 'field_mask'?
../include/linux/bitfield.h:109:28: note: 'field_mask' declared here
static __always_inline u64 field_mask(u64 field)
                           ^
../drivers/net/ipa/ipa_endpoint.c:965:16: error: implicit declaration of function 'field_max' [-Werror,-Wimplicit-function-declaration]
        return val == field_max(IPA_STATUS_FLAGS1_RT_RULE_ID_FMASK);
                      ^
2 errors generated.

This probably should go through the net-next tree to avoid that build
breakage.

Cheers,
Nathan