On 5/26/26 13:15, Kevin Brodsky wrote: > Introduce a helper that sets the permissions of a given pkey > (POIndex) in the POR_ELx format, and make use of it in > arch_set_user_pkey_access(). > > Signed-off-by: Kevin Brodsky <[email protected]> > --- > arch/arm64/include/asm/por.h | 7 +++++++ > arch/arm64/mm/mmu.c | 26 ++++++++++---------------- > 2 files changed, 17 insertions(+), 16 deletions(-) > > diff --git a/arch/arm64/include/asm/por.h b/arch/arm64/include/asm/por.h > index d913d5b529e4..bffb4d2b1246 100644 > --- a/arch/arm64/include/asm/por.h > +++ b/arch/arm64/include/asm/por.h > @@ -31,4 +31,11 @@ static inline bool por_elx_allows_exec(u64 por, u8 pkey) > return perm & POE_X; > } > > +static inline u64 por_elx_set_pkey_perms(u64 por, u8 pkey, u64 perms) > +{ > + u64 shift = POR_ELx_PERM_SHIFT(pkey); > + > + return (por & ~(POE_MASK << shift)) | (perms << shift); > +} > + > #endif /* _ASM_ARM64_POR_H */ > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c > index dd85e093ffdb..493310cf0486 100644 > --- a/arch/arm64/mm/mmu.c > +++ b/arch/arm64/mm/mmu.c > @@ -2339,8 +2339,8 @@ void __cpu_replace_ttbr1(pgd_t *pgdp, bool cnp) > #ifdef CONFIG_ARCH_HAS_PKEYS > int arch_set_user_pkey_access(int pkey, unsigned long init_val) > { > - u64 new_por; > - u64 old_por; > + u64 new_perms;
You should spell out the renaming of the variable. Given that perms is 4bit per key, should we use a u8 for it instead? > + u64 por; > > if (!system_supports_poe()) > return -ENOSPC; > @@ -2354,25 +2354,19 @@ int arch_set_user_pkey_access(int pkey, unsigned long > init_val) > return -EINVAL; > > /* Set the bits we need in POR: */ > - new_por = POE_RWX; > + new_perms = POE_RWX; > if (init_val & PKEY_DISABLE_WRITE) > - new_por &= ~POE_W; > + new_perms &= ~POE_W; > if (init_val & PKEY_DISABLE_ACCESS) > - new_por &= ~POE_RW; > + new_perms &= ~POE_RW; > if (init_val & PKEY_DISABLE_READ) > - new_por &= ~POE_R; > + new_perms &= ~POE_R; > if (init_val & PKEY_DISABLE_EXECUTE) > - new_por &= ~POE_X; > + new_perms &= ~POE_X; > > - /* Shift the bits in to the correct place in POR for pkey: */ > - new_por = POR_ELx_PERM_PREP(pkey, new_por); > - > - /* Get old POR and mask off any old bits in place: */ > - old_por = read_sysreg_s(SYS_POR_EL0); > - old_por &= ~(POE_MASK << POR_ELx_PERM_SHIFT(pkey)); > - > - /* Write old part along with new part: */ > - write_sysreg_s(old_por | new_por, SYS_POR_EL0); > + por = read_sysreg_s(SYS_POR_EL0); > + por = por_elx_set_pkey_perms(por, pkey, new_perms); > + write_sysreg_s(por, SYS_POR_EL0); Was wondering whether to move reading+writing of the register into the same helper, but you cannot reuse this exactly the same way for SYS_POR_EL1 as it seems. Reviewed-by: David Hildenbrand (Arm) <[email protected]> -- Cheers, David
