On Wed, 18 Oct 2017 14:10:41 -0700
Ram Pai <linux...@us.ibm.com> wrote:
> On Wed, Oct 18, 2017 at 03:36:35PM +1100, Balbir Singh wrote:
> > On Fri, 8 Sep 2017 15:45:01 -0700
> > Ram Pai <linux...@us.ibm.com> wrote:
> >
> > > arch independent code calls arch_override_mprotect_pkey()
> > > to return a pkey that best matches the requested protection.
> > >
> > > This patch provides the implementation.
> > >
> > > Signed-off-by: Ram Pai <linux...@us.ibm.com>
> > > ---
> > > arch/powerpc/include/asm/mmu_context.h | 5 +++
> > > arch/powerpc/include/asm/pkeys.h | 17 ++++++++++-
> > > arch/powerpc/mm/pkeys.c | 47
> > > ++++++++++++++++++++++++++++++++
> > > 3 files changed, 67 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/arch/powerpc/include/asm/mmu_context.h
> > > b/arch/powerpc/include/asm/mmu_context.h
> > > index c705a5d..8e5a87e 100644
> > > --- a/arch/powerpc/include/asm/mmu_context.h
> > > +++ b/arch/powerpc/include/asm/mmu_context.h
> > > @@ -145,6 +145,11 @@ static inline bool arch_vma_access_permitted(struct
> > > vm_area_struct *vma,
> > > #ifndef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
> > > #define pkey_initialize()
> > > #define pkey_mm_init(mm)
> > > +
> > > +static inline int vma_pkey(struct vm_area_struct *vma)
> > > +{
> > > + return 0;
> > > +}
> > > #endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
> > >
> > > #endif /* __KERNEL__ */
> > > diff --git a/arch/powerpc/include/asm/pkeys.h
> > > b/arch/powerpc/include/asm/pkeys.h
> > > index f13e913..d2fffef 100644
> > > --- a/arch/powerpc/include/asm/pkeys.h
> > > +++ b/arch/powerpc/include/asm/pkeys.h
> > > @@ -41,6 +41,16 @@ static inline u64 pkey_to_vmflag_bits(u16 pkey)
> > > ((pkey & 0x10UL) ? VM_PKEY_BIT4 : 0x0UL));
> > > }
> > >
> > > +#define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 |
> > > \
> > > + VM_PKEY_BIT3 | VM_PKEY_BIT4)
> > > +
> > > +static inline int vma_pkey(struct vm_area_struct *vma)
> > > +{
> > > + if (!pkey_inited)
> > > + return 0;
> >
> > We don't want pkey_inited to be present in all functions, why do we need
> > a conditional branch for all functions. Even if we do, it should be a jump
> > label. I would rather we just removed !pkey_inited unless really really
> > required.
>
> No. we really really need it. For example when we build a kernel with
> PROTECTION_KEYS config enabled and run that kernel on a older processor
> or on a system where the key feature is not enabled in the device tree,
> we have fail all the calls that get called-in by the arch-neutral code.
>
> Hence we need this check.
>
Use a mmu_feature then, it's already designed and optimized for that
purpose
> BTW: jump labels are awkward IMHO, unless absolutely needed.
>
The if checks across the place will hurt performance and we want to have
this enabled by default, we may need a mmu feature or jump labels
Balbir Singh.
