Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Mime-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=UTF-8
Date:   Tue, 06 Jun 2023 19:16:36 +1000
Message-Id: <CT5GCZLAL3QC.URC3KKG0M0WR@wheely>
Cc:     <linux-kernel@vger.kernel.org>, <linuxppc-dev@lists.ozlabs.org>
Subject: Re: [PATCH 3/4] powerpc/kuap: Refactor static branch for disabling
 kuap
From:   "Nicholas Piggin" <npiggin@gmail.com>
To:     "Christophe Leroy" <christophe.leroy@csgroup.eu>,
        "Michael Ellerman" <mpe@ellerman.id.au>
References: <1a3c69e38349b687c6c65240d7c09a7817a797d8.1685963081.git.christophe.leroy@csgroup.eu> <4d775047675e9f8ae2a9db9cb8a0cc8216a309b8.1685963081.git.christophe.leroy@csgroup.eu>
In-Reply-To: <4d775047675e9f8ae2a9db9cb8a0cc8216a309b8.1685963081.git.christophe.leroy@csgroup.eu>
Precedence: bulk

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/incl=
ude/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 @@
> =20
>  #include <linux/jump_label.h>
> =20
> -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)
> =20
> -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/incl=
ude/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 =3D !IS_ENABLED(CONFIG_PPC_KUEP);
>  bool disable_kuap =3D !IS_ENABLED(CONFIG_PPC_KUAP);
> =20
> +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