On Fri, Jul 17, 2020 at 12:20:41AM -0700, ira.weiny@xxxxxxxxx wrote: > +/* > + * Get a new pkey register value from the user values specified. > + * > + * Kernel users use the same flags as user space: > + * PKEY_DISABLE_ACCESS > + * PKEY_DISABLE_WRITE > + */ > +u32 get_new_pkr(u32 old_pkr, int pkey, unsigned long init_val) > +{ > + int pkey_shift = (pkey * PKR_BITS_PER_PKEY); > + u32 new_pkr_bits = 0; > + > + /* Set the bits we need in the register: */ > + if (init_val & PKEY_DISABLE_ACCESS) > + new_pkr_bits |= PKR_AD_BIT; > + if (init_val & PKEY_DISABLE_WRITE) > + new_pkr_bits |= PKR_WD_BIT; > + > + /* Shift the bits in to the correct place: */ > + new_pkr_bits <<= pkey_shift; > + > + /* Mask off any old bits in place: */ > + old_pkr &= ~((PKR_AD_BIT | PKR_WD_BIT) << pkey_shift); > + > + /* Return the old part along with the new part: */ > + return old_pkr | new_pkr_bits; > +} This is unbelievable junk... 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.