Le 06/06/2023 à 11:16, Nicholas Piggin a écrit :

On Mon Jun 5, 2023 at 9:04 PM AEST, Christophe Leroy wrote:

quoted

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 <redacted>
---
  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

quoted

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