| Message ID | 1504910713-7094-15-git-send-email-linuxram@us.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | None | expand |
On Fri, 8 Sep 2017 15:44:54 -0700 Ram Pai <linuxram@us.ibm.com> wrote: > cleanup the bits corresponding to a key in the AMR, and IAMR > register, when the key is newly allocated/activated or is freed. > We dont want some residual bits cause the hardware enforce > unintended behavior when the key is activated or freed. > > Signed-off-by: Ram Pai <linuxram@us.ibm.com> > --- > arch/powerpc/include/asm/pkeys.h | 12 ++++++++++++ > 1 files changed, 12 insertions(+), 0 deletions(-) > > diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h > index 5a83ed7..53bf13b 100644 > --- a/arch/powerpc/include/asm/pkeys.h > +++ b/arch/powerpc/include/asm/pkeys.h > @@ -54,6 +54,8 @@ static inline bool mm_pkey_is_allocated(struct mm_struct *mm, int pkey) > mm_set_pkey_is_allocated(mm, pkey)); > } > > +extern void __arch_activate_pkey(int pkey); > +extern void __arch_deactivate_pkey(int pkey); > /* > * Returns a positive, 5-bit key on success, or -1 on failure. > */ > @@ -80,6 +82,12 @@ static inline int mm_pkey_alloc(struct mm_struct *mm) > > ret = ffz((u32)mm_pkey_allocation_map(mm)); > mm_set_pkey_allocated(mm, ret); > + > + /* > + * enable the key in the hardware > + */ > + if (ret > 0) > + __arch_activate_pkey(ret); > return ret; > } > > @@ -91,6 +99,10 @@ static inline int mm_pkey_free(struct mm_struct *mm, int pkey) > if (!mm_pkey_is_allocated(mm, pkey)) > return -EINVAL; > > + /* > + * Disable the key in the hardware > + */ > + __arch_deactivate_pkey(pkey); > mm_set_pkey_free(mm, pkey); > > return 0; I think some of these patches can be merged, too much fine granularity is hurting my ability to see the larger function/implementation. Balbir Singh.
Ram Pai <linuxram@us.ibm.com> writes: > cleanup the bits corresponding to a key in the AMR, and IAMR > register, when the key is newly allocated/activated or is freed. > We dont want some residual bits cause the hardware enforce > unintended behavior when the key is activated or freed. > > Signed-off-by: Ram Pai <linuxram@us.ibm.com> > --- > arch/powerpc/include/asm/pkeys.h | 12 ++++++++++++ > 1 files changed, 12 insertions(+), 0 deletions(-) > > diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h > index 5a83ed7..53bf13b 100644 > --- a/arch/powerpc/include/asm/pkeys.h > +++ b/arch/powerpc/include/asm/pkeys.h > @@ -54,6 +54,8 @@ static inline bool mm_pkey_is_allocated(struct mm_struct *mm, int pkey) > mm_set_pkey_is_allocated(mm, pkey)); > } > > +extern void __arch_activate_pkey(int pkey); > +extern void __arch_deactivate_pkey(int pkey); > /* > * Returns a positive, 5-bit key on success, or -1 on failure. > */ > @@ -80,6 +82,12 @@ static inline int mm_pkey_alloc(struct mm_struct *mm) > > ret = ffz((u32)mm_pkey_allocation_map(mm)); > mm_set_pkey_allocated(mm, ret); > + > + /* > + * enable the key in the hardware > + */ > + if (ret > 0) > + __arch_activate_pkey(ret); > return ret; > } We are already arch specific because we are defining them in arch/powerpc/include/asm/, then why __arch_activate_pkey() ? > > @@ -91,6 +99,10 @@ static inline int mm_pkey_free(struct mm_struct *mm, int pkey) > if (!mm_pkey_is_allocated(mm, pkey)) > return -EINVAL; > > + /* > + * Disable the key in the hardware > + */ > + __arch_deactivate_pkey(pkey); > mm_set_pkey_free(mm, pkey); > > return 0; > -- > 1.7.1
Balbir Singh <bsingharora@gmail.com> writes: > On Fri, 8 Sep 2017 15:44:54 -0700 > Ram Pai <linuxram@us.ibm.com> wrote: > >> cleanup the bits corresponding to a key in the AMR, and IAMR >> register, when the key is newly allocated/activated or is freed. >> We dont want some residual bits cause the hardware enforce >> unintended behavior when the key is activated or freed. >> >> Signed-off-by: Ram Pai <linuxram@us.ibm.com> >> --- >> arch/powerpc/include/asm/pkeys.h | 12 ++++++++++++ >> 1 files changed, 12 insertions(+), 0 deletions(-) >> >> diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h >> index 5a83ed7..53bf13b 100644 >> --- a/arch/powerpc/include/asm/pkeys.h >> +++ b/arch/powerpc/include/asm/pkeys.h >> @@ -54,6 +54,8 @@ static inline bool mm_pkey_is_allocated(struct mm_struct *mm, int pkey) >> mm_set_pkey_is_allocated(mm, pkey)); >> } >> >> +extern void __arch_activate_pkey(int pkey); >> +extern void __arch_deactivate_pkey(int pkey); >> /* >> * Returns a positive, 5-bit key on success, or -1 on failure. >> */ >> @@ -80,6 +82,12 @@ static inline int mm_pkey_alloc(struct mm_struct *mm) >> >> ret = ffz((u32)mm_pkey_allocation_map(mm)); >> mm_set_pkey_allocated(mm, ret); >> + >> + /* >> + * enable the key in the hardware >> + */ >> + if (ret > 0) >> + __arch_activate_pkey(ret); >> return ret; >> } >> >> @@ -91,6 +99,10 @@ static inline int mm_pkey_free(struct mm_struct *mm, int pkey) >> if (!mm_pkey_is_allocated(mm, pkey)) >> return -EINVAL; >> >> + /* >> + * Disable the key in the hardware >> + */ >> + __arch_deactivate_pkey(pkey); >> mm_set_pkey_free(mm, pkey); >> >> return 0; > > I think some of these patches can be merged, too much fine granularity > is hurting my ability to see the larger function/implementation. Completely agree -aneesh
On Mon, Oct 23, 2017 at 03:13:33PM +0530, Aneesh Kumar K.V wrote: > Ram Pai <linuxram@us.ibm.com> writes: > > > cleanup the bits corresponding to a key in the AMR, and IAMR > > register, when the key is newly allocated/activated or is freed. > > We dont want some residual bits cause the hardware enforce > > unintended behavior when the key is activated or freed. > > > > Signed-off-by: Ram Pai <linuxram@us.ibm.com> > > --- > > arch/powerpc/include/asm/pkeys.h | 12 ++++++++++++ > > 1 files changed, 12 insertions(+), 0 deletions(-) > > > > diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h > > index 5a83ed7..53bf13b 100644 > > --- a/arch/powerpc/include/asm/pkeys.h > > +++ b/arch/powerpc/include/asm/pkeys.h > > @@ -54,6 +54,8 @@ static inline bool mm_pkey_is_allocated(struct mm_struct *mm, int pkey) > > mm_set_pkey_is_allocated(mm, pkey)); > > } > > > > +extern void __arch_activate_pkey(int pkey); > > +extern void __arch_deactivate_pkey(int pkey); > > /* > > * Returns a positive, 5-bit key on success, or -1 on failure. > > */ > > @@ -80,6 +82,12 @@ static inline int mm_pkey_alloc(struct mm_struct *mm) > > > > ret = ffz((u32)mm_pkey_allocation_map(mm)); > > mm_set_pkey_allocated(mm, ret); > > + > > + /* > > + * enable the key in the hardware > > + */ > > + if (ret > 0) > > + __arch_activate_pkey(ret); > > return ret; > > } > > We are already arch specific because we are defining them in > arch/powerpc/include/asm/, then why __arch_activate_pkey() ? almost all the memory-key internal functions are named with a two leading underbars. So just *loosely* following a convention within that file. The corresponding functions without the two underbars are the one exposed to the arch-neutral code. RP
On Mon, Oct 23, 2017 at 03:13:45PM +0530, Aneesh Kumar K.V wrote: > Balbir Singh <bsingharora@gmail.com> writes: > > > On Fri, 8 Sep 2017 15:44:54 -0700 > > Ram Pai <linuxram@us.ibm.com> wrote: > > > >> cleanup the bits corresponding to a key in the AMR, and IAMR > >> register, when the key is newly allocated/activated or is freed. > >> We dont want some residual bits cause the hardware enforce > >> unintended behavior when the key is activated or freed. > >> > >> Signed-off-by: Ram Pai <linuxram@us.ibm.com> > >> --- > >> arch/powerpc/include/asm/pkeys.h | 12 ++++++++++++ > >> 1 files changed, 12 insertions(+), 0 deletions(-) > >> > >> diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h > >> index 5a83ed7..53bf13b 100644 > >> --- a/arch/powerpc/include/asm/pkeys.h > >> +++ b/arch/powerpc/include/asm/pkeys.h > >> @@ -54,6 +54,8 @@ static inline bool mm_pkey_is_allocated(struct mm_struct *mm, int pkey) > >> mm_set_pkey_is_allocated(mm, pkey)); > >> } > >> > >> +extern void __arch_activate_pkey(int pkey); > >> +extern void __arch_deactivate_pkey(int pkey); > >> /* > >> * Returns a positive, 5-bit key on success, or -1 on failure. > >> */ > >> @@ -80,6 +82,12 @@ static inline int mm_pkey_alloc(struct mm_struct *mm) > >> > >> ret = ffz((u32)mm_pkey_allocation_map(mm)); > >> mm_set_pkey_allocated(mm, ret); > >> + > >> + /* > >> + * enable the key in the hardware > >> + */ > >> + if (ret > 0) > >> + __arch_activate_pkey(ret); > >> return ret; > >> } > >> > >> @@ -91,6 +99,10 @@ static inline int mm_pkey_free(struct mm_struct *mm, int pkey) > >> if (!mm_pkey_is_allocated(mm, pkey)) > >> return -EINVAL; > >> > >> + /* > >> + * Disable the key in the hardware > >> + */ > >> + __arch_deactivate_pkey(pkey); > >> mm_set_pkey_free(mm, pkey); > >> > >> return 0; > > > > I think some of these patches can be merged, too much fine granularity > > is hurting my ability to see the larger function/implementation. > > > Completely agree Me agree too :) Had to fine-grain it to satisfy comments received during the initial versions. I had about 10-12 patches in total. Thanks, will merge a couple of these patches. RP
diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h index 5a83ed7..53bf13b 100644 --- a/arch/powerpc/include/asm/pkeys.h +++ b/arch/powerpc/include/asm/pkeys.h @@ -54,6 +54,8 @@ static inline bool mm_pkey_is_allocated(struct mm_struct *mm, int pkey) mm_set_pkey_is_allocated(mm, pkey)); } +extern void __arch_activate_pkey(int pkey); +extern void __arch_deactivate_pkey(int pkey); /* * Returns a positive, 5-bit key on success, or -1 on failure. */ @@ -80,6 +82,12 @@ static inline int mm_pkey_alloc(struct mm_struct *mm) ret = ffz((u32)mm_pkey_allocation_map(mm)); mm_set_pkey_allocated(mm, ret); + + /* + * enable the key in the hardware + */ + if (ret > 0) + __arch_activate_pkey(ret); return ret; } @@ -91,6 +99,10 @@ static inline int mm_pkey_free(struct mm_struct *mm, int pkey) if (!mm_pkey_is_allocated(mm, pkey)) return -EINVAL; + /* + * Disable the key in the hardware + */ + __arch_deactivate_pkey(pkey); mm_set_pkey_free(mm, pkey); return 0;
cleanup the bits corresponding to a key in the AMR, and IAMR register, when the key is newly allocated/activated or is freed. We dont want some residual bits cause the hardware enforce unintended behavior when the key is activated or freed. Signed-off-by: Ram Pai <linuxram@us.ibm.com> --- arch/powerpc/include/asm/pkeys.h | 12 ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-)