| Message ID | 4d775047675e9f8ae2a9db9cb8a0cc8216a309b8.1685963081.git.christophe.leroy@csgroup.eu (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/4] powerpc/kuap: Avoid unnecessary reads of MD_AP | expand |
On Mon Jun 5, 2023 at 9:04 PM AEST, Christophe Leroy wrote: > All but book3s/64 use a static branch key for disabling kuap. > book3s/64 uses a memory feature. > > Refactor all targets except book3s/64. > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > --- > arch/powerpc/include/asm/book3s/32/kup.h | 7 ------- > arch/powerpc/include/asm/book3s/64/kup.h | 1 + > arch/powerpc/include/asm/kup.h | 15 +++++++++++++++ > arch/powerpc/include/asm/nohash/32/kup-8xx.h | 7 ------- > arch/powerpc/include/asm/nohash/kup-booke.h | 7 ------- > arch/powerpc/mm/book3s32/kuap.c | 3 --- > arch/powerpc/mm/init-common.c | 3 +++ > arch/powerpc/mm/nohash/kup.c | 3 --- > 8 files changed, 19 insertions(+), 27 deletions(-) > > diff --git a/arch/powerpc/include/asm/book3s/32/kup.h b/arch/powerpc/include/asm/book3s/32/kup.h > index 466a19cfb4df..8da9997a67ba 100644 > --- a/arch/powerpc/include/asm/book3s/32/kup.h > +++ b/arch/powerpc/include/asm/book3s/32/kup.h > @@ -11,8 +11,6 @@ > > #include <linux/jump_label.h> > > -extern struct static_key_false disable_kuap_key; > - > static __always_inline bool kuep_is_disabled(void) > { > return !IS_ENABLED(CONFIG_PPC_KUEP); > @@ -25,11 +23,6 @@ static __always_inline bool kuep_is_disabled(void) > #define KUAP_NONE (~0UL) > #define KUAP_ALL (~1UL) > > -static __always_inline bool kuap_is_disabled(void) > -{ > - return static_branch_unlikely(&disable_kuap_key); > -} > - > static inline void kuap_lock_one(unsigned long addr) > { > mtsr(mfsr(addr) | SR_KS, addr); > diff --git a/arch/powerpc/include/asm/book3s/64/kup.h b/arch/powerpc/include/asm/book3s/64/kup.h > index 1b0215ff3710..f8b8e93c488c 100644 > --- a/arch/powerpc/include/asm/book3s/64/kup.h > +++ b/arch/powerpc/include/asm/book3s/64/kup.h > @@ -233,6 +233,7 @@ static __always_inline bool kuap_is_disabled(void) > { > return !mmu_has_feature(MMU_FTR_BOOK3S_KUAP); > } > +#define kuap_is_disabled kuap_is_disabled Is there any point to doing this pattern since the code is in places that have ifdef PPC6 S etc? > diff --git a/arch/powerpc/mm/init-common.c b/arch/powerpc/mm/init-common.c > index 119ef491f797..74e140b1efef 100644 > --- a/arch/powerpc/mm/init-common.c > +++ b/arch/powerpc/mm/init-common.c > @@ -32,6 +32,9 @@ EXPORT_SYMBOL_GPL(kernstart_virt_addr); > bool disable_kuep = !IS_ENABLED(CONFIG_PPC_KUEP); > bool disable_kuap = !IS_ENABLED(CONFIG_PPC_KUAP); > > +struct static_key_false disable_kuap_key; > +EXPORT_SYMBOL(disable_kuap_key); > + That's going to define it on 64s? Nice refactoring though. Thanks, Nick
Le 06/06/2023 à 11:16, Nicholas Piggin a écrit : > On Mon Jun 5, 2023 at 9:04 PM AEST, Christophe Leroy wrote: >> All but book3s/64 use a static branch key for disabling kuap. >> book3s/64 uses a memory feature. >> >> Refactor all targets except book3s/64. >> >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >> --- >> arch/powerpc/include/asm/book3s/32/kup.h | 7 ------- >> arch/powerpc/include/asm/book3s/64/kup.h | 1 + >> arch/powerpc/include/asm/kup.h | 15 +++++++++++++++ >> arch/powerpc/include/asm/nohash/32/kup-8xx.h | 7 ------- >> arch/powerpc/include/asm/nohash/kup-booke.h | 7 ------- >> arch/powerpc/mm/book3s32/kuap.c | 3 --- >> arch/powerpc/mm/init-common.c | 3 +++ >> arch/powerpc/mm/nohash/kup.c | 3 --- >> 8 files changed, 19 insertions(+), 27 deletions(-) >> >> diff --git a/arch/powerpc/include/asm/book3s/32/kup.h b/arch/powerpc/include/asm/book3s/32/kup.h >> index 466a19cfb4df..8da9997a67ba 100644 >> --- a/arch/powerpc/include/asm/book3s/32/kup.h >> +++ b/arch/powerpc/include/asm/book3s/32/kup.h >> @@ -11,8 +11,6 @@ >> >> #include <linux/jump_label.h> >> >> -extern struct static_key_false disable_kuap_key; >> - >> static __always_inline bool kuep_is_disabled(void) >> { >> return !IS_ENABLED(CONFIG_PPC_KUEP); >> @@ -25,11 +23,6 @@ static __always_inline bool kuep_is_disabled(void) >> #define KUAP_NONE (~0UL) >> #define KUAP_ALL (~1UL) >> >> -static __always_inline bool kuap_is_disabled(void) >> -{ >> - return static_branch_unlikely(&disable_kuap_key); >> -} >> - >> static inline void kuap_lock_one(unsigned long addr) >> { >> mtsr(mfsr(addr) | SR_KS, addr); >> diff --git a/arch/powerpc/include/asm/book3s/64/kup.h b/arch/powerpc/include/asm/book3s/64/kup.h >> index 1b0215ff3710..f8b8e93c488c 100644 >> --- a/arch/powerpc/include/asm/book3s/64/kup.h >> +++ b/arch/powerpc/include/asm/book3s/64/kup.h >> @@ -233,6 +233,7 @@ static __always_inline bool kuap_is_disabled(void) >> { >> return !mmu_has_feature(MMU_FTR_BOOK3S_KUAP); >> } >> +#define kuap_is_disabled kuap_is_disabled > > Is there any point to doing this pattern since the code is in places > that have ifdef PPC6 S etc? I'm not sure what you have in mind. There is a default kuap_is_disabled() in arch/powerpc/include/asm/kup.h For 64s we want the version from arch/powerpc/include/asm/book3s/64/kup.h instead of the default one. And kuap_is_disabled() is used in some common code in arch/powerpc/include/asm/kup.h, not only in the code encloded by #ifndef 64s > >> diff --git a/arch/powerpc/mm/init-common.c b/arch/powerpc/mm/init-common.c >> index 119ef491f797..74e140b1efef 100644 >> --- a/arch/powerpc/mm/init-common.c >> +++ b/arch/powerpc/mm/init-common.c >> @@ -32,6 +32,9 @@ EXPORT_SYMBOL_GPL(kernstart_virt_addr); >> bool disable_kuep = !IS_ENABLED(CONFIG_PPC_KUEP); >> bool disable_kuap = !IS_ENABLED(CONFIG_PPC_KUAP); >> >> +struct static_key_false disable_kuap_key; >> +EXPORT_SYMBOL(disable_kuap_key); >> + > > That's going to define it on 64s? Is that a problem at all ? By the way I was thinking about also using that key on 64s, will look at it as a second step. Thanks for the review Christophe > > Nice refactoring though. > > Thanks, > Nick
diff --git a/arch/powerpc/include/asm/book3s/32/kup.h b/arch/powerpc/include/asm/book3s/32/kup.h index 466a19cfb4df..8da9997a67ba 100644 --- a/arch/powerpc/include/asm/book3s/32/kup.h +++ b/arch/powerpc/include/asm/book3s/32/kup.h @@ -11,8 +11,6 @@ #include <linux/jump_label.h> -extern struct static_key_false disable_kuap_key; - static __always_inline bool kuep_is_disabled(void) { return !IS_ENABLED(CONFIG_PPC_KUEP); @@ -25,11 +23,6 @@ static __always_inline bool kuep_is_disabled(void) #define KUAP_NONE (~0UL) #define KUAP_ALL (~1UL) -static __always_inline bool kuap_is_disabled(void) -{ - return static_branch_unlikely(&disable_kuap_key); -} - static inline void kuap_lock_one(unsigned long addr) { mtsr(mfsr(addr) | SR_KS, addr); diff --git a/arch/powerpc/include/asm/book3s/64/kup.h b/arch/powerpc/include/asm/book3s/64/kup.h index 1b0215ff3710..f8b8e93c488c 100644 --- a/arch/powerpc/include/asm/book3s/64/kup.h +++ b/arch/powerpc/include/asm/book3s/64/kup.h @@ -233,6 +233,7 @@ static __always_inline bool kuap_is_disabled(void) { return !mmu_has_feature(MMU_FTR_BOOK3S_KUAP); } +#define kuap_is_disabled kuap_is_disabled static inline void kuap_user_restore(struct pt_regs *regs) { diff --git a/arch/powerpc/include/asm/kup.h b/arch/powerpc/include/asm/kup.h index 0a4e07175612..74b7f4cee2ed 100644 --- a/arch/powerpc/include/asm/kup.h +++ b/arch/powerpc/include/asm/kup.h @@ -6,6 +6,12 @@ #define KUAP_WRITE 2 #define KUAP_READ_WRITE (KUAP_READ | KUAP_WRITE) +#ifndef __ASSEMBLY__ +#include <linux/types.h> + +static __always_inline bool kuap_is_disabled(void); +#endif + #ifdef CONFIG_PPC_BOOK3S_64 #include <asm/book3s/64/kup.h> #endif @@ -41,6 +47,15 @@ void setup_kuep(bool disabled); #ifdef CONFIG_PPC_KUAP void setup_kuap(bool disabled); + +#ifndef kuap_is_disabled +extern struct static_key_false disable_kuap_key; + +static __always_inline bool kuap_is_disabled(void) +{ + return static_branch_unlikely(&disable_kuap_key); +} +#endif #else static inline void setup_kuap(bool disabled) { } diff --git a/arch/powerpc/include/asm/nohash/32/kup-8xx.h b/arch/powerpc/include/asm/nohash/32/kup-8xx.h index a372cd822887..1d53f38c5cd5 100644 --- a/arch/powerpc/include/asm/nohash/32/kup-8xx.h +++ b/arch/powerpc/include/asm/nohash/32/kup-8xx.h @@ -13,13 +13,6 @@ #include <asm/reg.h> -extern struct static_key_false disable_kuap_key; - -static __always_inline bool kuap_is_disabled(void) -{ - return static_branch_unlikely(&disable_kuap_key); -} - static inline void __kuap_save_and_lock(struct pt_regs *regs) { regs->kuap = mfspr(SPRN_MD_AP); diff --git a/arch/powerpc/include/asm/nohash/kup-booke.h b/arch/powerpc/include/asm/nohash/kup-booke.h index 71182cbe20c3..07759ae9117b 100644 --- a/arch/powerpc/include/asm/nohash/kup-booke.h +++ b/arch/powerpc/include/asm/nohash/kup-booke.h @@ -18,13 +18,6 @@ #include <asm/reg.h> -extern struct static_key_false disable_kuap_key; - -static __always_inline bool kuap_is_disabled(void) -{ - return static_branch_unlikely(&disable_kuap_key); -} - static inline void __kuap_lock(void) { mtspr(SPRN_PID, 0); diff --git a/arch/powerpc/mm/book3s32/kuap.c b/arch/powerpc/mm/book3s32/kuap.c index 28676cabb005..c5484729b595 100644 --- a/arch/powerpc/mm/book3s32/kuap.c +++ b/arch/powerpc/mm/book3s32/kuap.c @@ -3,9 +3,6 @@ #include <asm/kup.h> #include <asm/smp.h> -struct static_key_false disable_kuap_key; -EXPORT_SYMBOL(disable_kuap_key); - void kuap_lock_all_ool(void) { kuap_lock_all(); diff --git a/arch/powerpc/mm/init-common.c b/arch/powerpc/mm/init-common.c index 119ef491f797..74e140b1efef 100644 --- a/arch/powerpc/mm/init-common.c +++ b/arch/powerpc/mm/init-common.c @@ -32,6 +32,9 @@ EXPORT_SYMBOL_GPL(kernstart_virt_addr); bool disable_kuep = !IS_ENABLED(CONFIG_PPC_KUEP); bool disable_kuap = !IS_ENABLED(CONFIG_PPC_KUAP); +struct static_key_false disable_kuap_key; +EXPORT_SYMBOL(disable_kuap_key); + static int __init parse_nosmep(char *p) { if (!IS_ENABLED(CONFIG_PPC_BOOK3S_64)) diff --git a/arch/powerpc/mm/nohash/kup.c b/arch/powerpc/mm/nohash/kup.c index 552becf90e97..4e22adfa2aa8 100644 --- a/arch/powerpc/mm/nohash/kup.c +++ b/arch/powerpc/mm/nohash/kup.c @@ -13,9 +13,6 @@ #include <asm/smp.h> #ifdef CONFIG_PPC_KUAP -struct static_key_false disable_kuap_key; -EXPORT_SYMBOL(disable_kuap_key); - void setup_kuap(bool disabled) { if (disabled) {
All but book3s/64 use a static branch key for disabling kuap. book3s/64 uses a memory feature. Refactor all targets except book3s/64. Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> --- arch/powerpc/include/asm/book3s/32/kup.h | 7 ------- arch/powerpc/include/asm/book3s/64/kup.h | 1 + arch/powerpc/include/asm/kup.h | 15 +++++++++++++++ arch/powerpc/include/asm/nohash/32/kup-8xx.h | 7 ------- arch/powerpc/include/asm/nohash/kup-booke.h | 7 ------- arch/powerpc/mm/book3s32/kuap.c | 3 --- arch/powerpc/mm/init-common.c | 3 +++ arch/powerpc/mm/nohash/kup.c | 3 --- 8 files changed, 19 insertions(+), 27 deletions(-)