Re: [PATCH 2/2] crypto: ccree - add custom cache params from DT file

[Gilad Ben-Yossef]

Hi,

On Fri, Sep 18, 2020 at 10:39 PM Nick Desaulniers
<ndesaulniers@xxxxxxxxxx> wrote:
>
> On Thu, Sep 17, 2020 at 12:20 AM Gilad Ben-Yossef <gilad@xxxxxxxxxxxxx> wrote:
> >
...
> >
> > I am unable to understand this warning. It looks like it is
> > complaining about a FIELD_GET sanity check that is always false, which
> > makes sense since we're using a constant.
> >
> > Anyone can enlighten me if I've missed something?
>
> Looked at some of this code recently.  I think it may have an issue
> for masks where sizeof(mask) < sizeof(unsigned long long).
>
> In your code, via 0day bot:
>
>    107          u32 cache_params, ace_const, val, mask;
> ...
> > 120          cache_params |= FIELD_PREP(mask, val);
>
> then in include/linux/bitfield.h, we have:
>
>  92 #define FIELD_PREP(_mask, _val)           \
>  93   ({                \
>  94     __BF_FIELD_CHECK(_mask, 0ULL, _val, "FIELD_PREP: ");  \
>
>  44 #define __BF_FIELD_CHECK(_mask, _reg, _val, _pfx)     \
> ...
>  52     BUILD_BUG_ON_MSG((_mask) > (typeof(_reg))~0ull,   \
>  53          _pfx "type of reg too small for mask"); \
>
> so the 0ULL in FIELD_PREP is important.  In __BF_FIELD_CHECK, the
> typeof(_reg) is unsigned long long (because 0ULL was passed).  So we
> have a comparison between a u32 and a u64; indeed any u32 can never be
> greater than a u64 that we know has the value of ULLONG_MAX.
>
> I did send a series splitting these up, I wonder if they'd help here:
> https://lore.kernel.org/lkml/20200708230402.1644819-3-ndesaulniers@xxxxxxxxxx/
> --

Thanks!

This indeed explains this. It seems there is nothing for me to do
about it in the driver code though, as it seems the issue is in the
macro and you have already posted a fix for it.

Thanks again,
Gilad

-- 
Gilad Ben-Yossef
Chief Coffee Drinker

values of β will give rise to dom!