[02/25] powerpc: define an additional vma bit for protection keys.

Message ID 1504910713-7094-11-git-send-email-linuxram@us.ibm.com
State Changes Requested
Headers show
Series
  • Untitled series #2307
Related show

Commit Message

Ram Pai Sept. 8, 2017, 10:44 p.m.
powerpc needs an additional vma bit to support 32 keys.
Till the additional vma bit lands in include/linux/mm.h
we have to define  it  in powerpc specific header file.
This is  needed to get pkeys working on power.

Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
 arch/powerpc/include/asm/pkeys.h |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

Comments

Balbir Singh Sept. 14, 2017, 4:38 a.m. | #1
On Fri,  8 Sep 2017 15:44:50 -0700
Ram Pai <linuxram@us.ibm.com> wrote:

> powerpc needs an additional vma bit to support 32 keys.
> Till the additional vma bit lands in include/linux/mm.h
> we have to define  it  in powerpc specific header file.
> This is  needed to get pkeys working on power.
> 
> Signed-off-by: Ram Pai <linuxram@us.ibm.com>
> ---

"This" being an arch specific hack for the additional bit?

Balbir
Benjamin Herrenschmidt Sept. 14, 2017, 8:11 a.m. | #2
On Thu, 2017-09-14 at 14:38 +1000, Balbir Singh wrote:
> On Fri,  8 Sep 2017 15:44:50 -0700
> Ram Pai <linuxram@us.ibm.com> wrote:
> 
> > powerpc needs an additional vma bit to support 32 keys.
> > Till the additional vma bit lands in include/linux/mm.h
> > we have to define  it  in powerpc specific header file.
> > This is  needed to get pkeys working on power.
> > 
> > Signed-off-by: Ram Pai <linuxram@us.ibm.com>
> > ---
> 
> "This" being an arch specific hack for the additional bit?

Arch VMA bits ? really ? I'd rather we limit ourselves to 16 keys first
then push for adding the extra bit to the generic code.

Ben.
Ram Pai Sept. 14, 2017, 4:15 p.m. | #3
On Thu, Sep 14, 2017 at 02:38:07PM +1000, Balbir Singh wrote:
> On Fri,  8 Sep 2017 15:44:50 -0700
> Ram Pai <linuxram@us.ibm.com> wrote:
> 
> > powerpc needs an additional vma bit to support 32 keys.
> > Till the additional vma bit lands in include/linux/mm.h
> > we have to define  it  in powerpc specific header file.
> > This is  needed to get pkeys working on power.
> > 
> > Signed-off-by: Ram Pai <linuxram@us.ibm.com>
> > ---
> 
> "This" being an arch specific hack for the additional bit?


Yes. arch-specific hack.  I am trying to get the arch specific
changes merged parallelly, along with these patches. Don't know
which one will merge first. Regardless of which patch-set
lands-in first; I have organized the code such that nothing
breaks.

RP
Aneesh Kumar K.V Oct. 23, 2017, 9:25 a.m. | #4
Ram Pai <linuxram@us.ibm.com> writes:

> powerpc needs an additional vma bit to support 32 keys.
> Till the additional vma bit lands in include/linux/mm.h
> we have to define  it  in powerpc specific header file.
> This is  needed to get pkeys working on power.
>
> Signed-off-by: Ram Pai <linuxram@us.ibm.com>
> ---
>  arch/powerpc/include/asm/pkeys.h |   18 ++++++++++++++++++
>  1 files changed, 18 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
> index c02305a..44e01a2 100644
> --- a/arch/powerpc/include/asm/pkeys.h
> +++ b/arch/powerpc/include/asm/pkeys.h
> @@ -3,6 +3,24 @@
>
>  extern bool pkey_inited;
>  extern bool pkey_execute_disable_support;
> +
> +/*
> + * powerpc needs an additional vma bit to support 32 keys.
> + * Till the additional vma bit lands in include/linux/mm.h
> + * we have to carry the hunk below. This is  needed to get
> + * pkeys working on power. -- Ram
> + */
> +#ifndef VM_HIGH_ARCH_BIT_4
> +#define VM_HIGH_ARCH_BIT_4	36
> +#define VM_HIGH_ARCH_4	BIT(VM_HIGH_ARCH_BIT_4)
> +#define VM_PKEY_SHIFT VM_HIGH_ARCH_BIT_0
> +#define VM_PKEY_BIT0	VM_HIGH_ARCH_0
> +#define VM_PKEY_BIT1	VM_HIGH_ARCH_1
> +#define VM_PKEY_BIT2	VM_HIGH_ARCH_2
> +#define VM_PKEY_BIT3	VM_HIGH_ARCH_3
> +#define VM_PKEY_BIT4	VM_HIGH_ARCH_4
> +#endif
> +
>  #define ARCH_VM_PKEY_FLAGS 0

Do we want them in pkeys.h ? Even if they are arch specific for the
existing ones we have them in include/linux/mm.h. IIUC, vmflags details
are always in mm.h? This will be the first exception to that?


>
>  static inline bool mm_pkey_is_allocated(struct mm_struct *mm, int pkey)
> -- 
> 1.7.1
Aneesh Kumar K.V Oct. 23, 2017, 9:28 a.m. | #5
"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> writes:

> Ram Pai <linuxram@us.ibm.com> writes:
>
>> powerpc needs an additional vma bit to support 32 keys.
>> Till the additional vma bit lands in include/linux/mm.h
>> we have to define  it  in powerpc specific header file.
>> This is  needed to get pkeys working on power.
>>
>> Signed-off-by: Ram Pai <linuxram@us.ibm.com>
>> ---
>>  arch/powerpc/include/asm/pkeys.h |   18 ++++++++++++++++++
>>  1 files changed, 18 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
>> index c02305a..44e01a2 100644
>> --- a/arch/powerpc/include/asm/pkeys.h
>> +++ b/arch/powerpc/include/asm/pkeys.h
>> @@ -3,6 +3,24 @@
>>
>>  extern bool pkey_inited;
>>  extern bool pkey_execute_disable_support;
>> +
>> +/*
>> + * powerpc needs an additional vma bit to support 32 keys.
>> + * Till the additional vma bit lands in include/linux/mm.h
>> + * we have to carry the hunk below. This is  needed to get
>> + * pkeys working on power. -- Ram
>> + */
>> +#ifndef VM_HIGH_ARCH_BIT_4
>> +#define VM_HIGH_ARCH_BIT_4	36
>> +#define VM_HIGH_ARCH_4	BIT(VM_HIGH_ARCH_BIT_4)
>> +#define VM_PKEY_SHIFT VM_HIGH_ARCH_BIT_0
>> +#define VM_PKEY_BIT0	VM_HIGH_ARCH_0
>> +#define VM_PKEY_BIT1	VM_HIGH_ARCH_1
>> +#define VM_PKEY_BIT2	VM_HIGH_ARCH_2
>> +#define VM_PKEY_BIT3	VM_HIGH_ARCH_3
>> +#define VM_PKEY_BIT4	VM_HIGH_ARCH_4
>> +#endif
>> +
>>  #define ARCH_VM_PKEY_FLAGS 0
>
> Do we want them in pkeys.h ? Even if they are arch specific for the
> existing ones we have them in include/linux/mm.h. IIUC, vmflags details
> are always in mm.h? This will be the first exception to that?


Also can we move that 

#if defined (CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS)
# define VM_PKEY_SHIFT	VM_HIGH_ARCH_BIT_0
# define VM_PKEY_BIT0	VM_HIGH_ARCH_0	/* A protection key is a 4-bit value */
# define VM_PKEY_BIT1	VM_HIGH_ARCH_1
# define VM_PKEY_BIT2	VM_HIGH_ARCH_2
# define VM_PKEY_BIT3	VM_HIGH_ARCH_3
#endif

to

#if defined (CONFIG_ARCH_HAS_PKEYS)
# define VM_PKEY_SHIFT	VM_HIGH_ARCH_BIT_0
# define VM_PKEY_BIT0	VM_HIGH_ARCH_0	/* A protection key is a 4-bit value */
# define VM_PKEY_BIT1	VM_HIGH_ARCH_1
# define VM_PKEY_BIT2	VM_HIGH_ARCH_2
# define VM_PKEY_BIT3	VM_HIGH_ARCH_3
#endif


And then later update the generic to handle PKEY_BIT4?

-aneesh
Ram Pai Oct. 23, 2017, 5:43 p.m. | #6
On Mon, Oct 23, 2017 at 02:55:51PM +0530, Aneesh Kumar K.V wrote:
> Ram Pai <linuxram@us.ibm.com> writes:
> 
> > powerpc needs an additional vma bit to support 32 keys.
> > Till the additional vma bit lands in include/linux/mm.h
> > we have to define  it  in powerpc specific header file.
> > This is  needed to get pkeys working on power.
> >
> > Signed-off-by: Ram Pai <linuxram@us.ibm.com>
> > ---
> >  arch/powerpc/include/asm/pkeys.h |   18 ++++++++++++++++++
> >  1 files changed, 18 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
> > index c02305a..44e01a2 100644
> > --- a/arch/powerpc/include/asm/pkeys.h
> > +++ b/arch/powerpc/include/asm/pkeys.h
> > @@ -3,6 +3,24 @@
> >
> >  extern bool pkey_inited;
> >  extern bool pkey_execute_disable_support;
> > +
> > +/*
> > + * powerpc needs an additional vma bit to support 32 keys.
> > + * Till the additional vma bit lands in include/linux/mm.h
> > + * we have to carry the hunk below. This is  needed to get
> > + * pkeys working on power. -- Ram
> > + */
> > +#ifndef VM_HIGH_ARCH_BIT_4
> > +#define VM_HIGH_ARCH_BIT_4	36
> > +#define VM_HIGH_ARCH_4	BIT(VM_HIGH_ARCH_BIT_4)
> > +#define VM_PKEY_SHIFT VM_HIGH_ARCH_BIT_0
> > +#define VM_PKEY_BIT0	VM_HIGH_ARCH_0
> > +#define VM_PKEY_BIT1	VM_HIGH_ARCH_1
> > +#define VM_PKEY_BIT2	VM_HIGH_ARCH_2
> > +#define VM_PKEY_BIT3	VM_HIGH_ARCH_3
> > +#define VM_PKEY_BIT4	VM_HIGH_ARCH_4
> > +#endif
> > +
> >  #define ARCH_VM_PKEY_FLAGS 0
> 
> Do we want them in pkeys.h ? Even if they are arch specific for the
> existing ones we have them in include/linux/mm.h. IIUC, vmflags details
> are always in mm.h? This will be the first exception to that?

I am trying to get the real fix in include/linux/mm.h  
If that happens sooner than this hunk is not needed. 
Till then this is an exception, but it is the **ONLY** exception.

I think your point is to have this hunk in include/linux/mm.h  ?

If yes, than it would be easier to push the real fix in
include/linux/mm.h  instead of pushing this hunk in the there.

RP
Ram Pai Oct. 23, 2017, 5:57 p.m. | #7
On Mon, Oct 23, 2017 at 02:58:55PM +0530, Aneesh Kumar K.V wrote:
> "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> writes:
> 
> > Ram Pai <linuxram@us.ibm.com> writes:
> >
> >> powerpc needs an additional vma bit to support 32 keys.
> >> Till the additional vma bit lands in include/linux/mm.h
> >> we have to define  it  in powerpc specific header file.
> >> This is  needed to get pkeys working on power.
> >>
> >> Signed-off-by: Ram Pai <linuxram@us.ibm.com>
> >> ---
> >>  arch/powerpc/include/asm/pkeys.h |   18 ++++++++++++++++++
> >>  1 files changed, 18 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
> >> index c02305a..44e01a2 100644
> >> --- a/arch/powerpc/include/asm/pkeys.h
> >> +++ b/arch/powerpc/include/asm/pkeys.h
> >> @@ -3,6 +3,24 @@
> >>
> >>  extern bool pkey_inited;
> >>  extern bool pkey_execute_disable_support;
> >> +
> >> +/*
> >> + * powerpc needs an additional vma bit to support 32 keys.
> >> + * Till the additional vma bit lands in include/linux/mm.h
> >> + * we have to carry the hunk below. This is  needed to get
> >> + * pkeys working on power. -- Ram
> >> + */
> >> +#ifndef VM_HIGH_ARCH_BIT_4
> >> +#define VM_HIGH_ARCH_BIT_4	36
> >> +#define VM_HIGH_ARCH_4	BIT(VM_HIGH_ARCH_BIT_4)
> >> +#define VM_PKEY_SHIFT VM_HIGH_ARCH_BIT_0
> >> +#define VM_PKEY_BIT0	VM_HIGH_ARCH_0
> >> +#define VM_PKEY_BIT1	VM_HIGH_ARCH_1
> >> +#define VM_PKEY_BIT2	VM_HIGH_ARCH_2
> >> +#define VM_PKEY_BIT3	VM_HIGH_ARCH_3
> >> +#define VM_PKEY_BIT4	VM_HIGH_ARCH_4
> >> +#endif
> >> +
> >>  #define ARCH_VM_PKEY_FLAGS 0
> >
> > Do we want them in pkeys.h ? Even if they are arch specific for the
> > existing ones we have them in include/linux/mm.h. IIUC, vmflags details
> > are always in mm.h? This will be the first exception to that?
> 
> 
> Also can we move that 
> 
> #if defined (CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS)
> # define VM_PKEY_SHIFT	VM_HIGH_ARCH_BIT_0
> # define VM_PKEY_BIT0	VM_HIGH_ARCH_0	/* A protection key is a 4-bit value */
> # define VM_PKEY_BIT1	VM_HIGH_ARCH_1
> # define VM_PKEY_BIT2	VM_HIGH_ARCH_2
> # define VM_PKEY_BIT3	VM_HIGH_ARCH_3
> #endif
> 
> to
> 
> #if defined (CONFIG_ARCH_HAS_PKEYS)
> # define VM_PKEY_SHIFT	VM_HIGH_ARCH_BIT_0
> # define VM_PKEY_BIT0	VM_HIGH_ARCH_0	/* A protection key is a 4-bit value */
> # define VM_PKEY_BIT1	VM_HIGH_ARCH_1
> # define VM_PKEY_BIT2	VM_HIGH_ARCH_2
> # define VM_PKEY_BIT3	VM_HIGH_ARCH_3
> #endif
> 
> 
> And then later update the generic to handle PKEY_BIT4?


Yes. The above changes have been implemented in a patch sent to the mm
mailing list as well as to lkml.

https://lkml.org/lkml/2017/9/15/504

RP
Ram Pai Oct. 23, 2017, 9:06 p.m. | #8
On Thu, Sep 14, 2017 at 06:11:32PM +1000, Benjamin Herrenschmidt wrote:
> On Thu, 2017-09-14 at 14:38 +1000, Balbir Singh wrote:
> > On Fri,  8 Sep 2017 15:44:50 -0700
> > Ram Pai <linuxram@us.ibm.com> wrote:
> > 
> > > powerpc needs an additional vma bit to support 32 keys.
> > > Till the additional vma bit lands in include/linux/mm.h
> > > we have to define  it  in powerpc specific header file.
> > > This is  needed to get pkeys working on power.
> > > 
> > > Signed-off-by: Ram Pai <linuxram@us.ibm.com>
> > > ---
> > 
> > "This" being an arch specific hack for the additional bit?
> 
> Arch VMA bits ? really ? I'd rather we limit ourselves to 16 keys first
> then push for adding the extra bit to the generic code.

(hmm.. this mail did not land in my mailbox :( Sorry for the delay. Just
saw it in the mailing list.)

I think this is good idea. I can code it such a way that we
support 16 or 32 keys depending on the availability of the vma bit.

No more hunks in the code.


RP

Patch

diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
index c02305a..44e01a2 100644
--- a/arch/powerpc/include/asm/pkeys.h
+++ b/arch/powerpc/include/asm/pkeys.h
@@ -3,6 +3,24 @@ 
 
 extern bool pkey_inited;
 extern bool pkey_execute_disable_support;
+
+/*
+ * powerpc needs an additional vma bit to support 32 keys.
+ * Till the additional vma bit lands in include/linux/mm.h
+ * we have to carry the hunk below. This is  needed to get
+ * pkeys working on power. -- Ram
+ */
+#ifndef VM_HIGH_ARCH_BIT_4
+#define VM_HIGH_ARCH_BIT_4	36
+#define VM_HIGH_ARCH_4	BIT(VM_HIGH_ARCH_BIT_4)
+#define VM_PKEY_SHIFT VM_HIGH_ARCH_BIT_0
+#define VM_PKEY_BIT0	VM_HIGH_ARCH_0
+#define VM_PKEY_BIT1	VM_HIGH_ARCH_1
+#define VM_PKEY_BIT2	VM_HIGH_ARCH_2
+#define VM_PKEY_BIT3	VM_HIGH_ARCH_3
+#define VM_PKEY_BIT4	VM_HIGH_ARCH_4
+#endif
+
 #define ARCH_VM_PKEY_FLAGS 0
 
 static inline bool mm_pkey_is_allocated(struct mm_struct *mm, int pkey)