On 7/17/20 1:54 AM, Peter Zijlstra wrote:
> This is unbelievable junk...
Ouch!
This is from the original user pkeys implementation.
> How about something like:
>
> u32 update_pkey_reg(u32 pk_reg, int pkey, unsigned int flags)
> {
> int pkey_shift = pkey * PKR_BITS_PER_PKEY;
>
> pk_reg &= ~(((1 << PKR_BITS_PER_PKEY) - 1) << pkey_shift);
>
> if (flags & PKEY_DISABLE_ACCESS)
> pk_reg |= PKR_AD_BIT << pkey_shift;
> if (flags & PKEY_DISABLE_WRITE)
> pk_reg |= PKR_WD_BIT << pkey_shift;
>
> return pk_reg;
> }
>
> Then we at least have a little clue wtf the thing does.. Yes I started
> with a rename and then got annoyed at the implementation too.
That's fine, if some comments get added. It looks correct to me but
probably compiles down to pretty much the same thing as what was there.
FWIW, I prefer the explicit masking off of two bit values to implicit
masking off with a mask generated from PKR_BITS_PER_PKEY. It's
certainly more compact, but I usually don't fret over the lines of code.
