Re: [PATCH 4/6] powerpc/64s: Enable barrier_nospec based on firmware settings

["Michal Suchánek" <msuchanek@suse.de>]

Hello,

On Tue, 24 Apr 2018 14:15:57 +1000
Michael Ellerman <mpe@ellerman.id.au> wrote:

> From: Michal Suchanek <msuchanek@suse.de>
> 
> Check what firmware told us and enable/disable the barrier_nospec as
> appropriate.
> 
> We err on the side of enabling the barrier, as it's no-op on older
> systems, see the comment for more detail.
> 
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
>  arch/powerpc/include/asm/setup.h       |  1 +
>  arch/powerpc/kernel/security.c         | 60
> ++++++++++++++++++++++++++++++++++
> arch/powerpc/platforms/powernv/setup.c |  1 +
> arch/powerpc/platforms/pseries/setup.c |  1 + 4 files changed, 63
> insertions(+)
> 
> diff --git a/arch/powerpc/include/asm/setup.h
> b/arch/powerpc/include/asm/setup.h index 4335cddc1cf2..aeb175e8a525
> 100644 --- a/arch/powerpc/include/asm/setup.h
> +++ b/arch/powerpc/include/asm/setup.h
> @@ -52,6 +52,7 @@ enum l1d_flush_type {
>  
>  void setup_rfi_flush(enum l1d_flush_type, bool enable);
>  void do_rfi_flush_fixups(enum l1d_flush_type types);
> +void setup_barrier_nospec(void);
>  void do_barrier_nospec_fixups(bool enable);
>  
>  #ifdef CONFIG_PPC_BOOK3S_64
> diff --git a/arch/powerpc/kernel/security.c
> b/arch/powerpc/kernel/security.c index b963eae0b0a0..d1b9639e5e24
> 100644 --- a/arch/powerpc/kernel/security.c
> +++ b/arch/powerpc/kernel/security.c
> @@ -8,6 +8,7 @@
>  #include <linux/device.h>
>  #include <linux/seq_buf.h>
>  
> +#include <asm/debugfs.h>
>  #include <asm/security_features.h>
>  #include <asm/setup.h>
>  
> @@ -22,6 +23,65 @@ static void enable_barrier_nospec(bool enable)
>  	do_barrier_nospec_fixups(enable);
>  }
>  
> +void setup_barrier_nospec(void)
> +{
> +	bool enable;
> +
> +	/*
> +	 * It would make sense to check SEC_FTR_SPEC_BAR_ORI31 below
> as well.
> +	 * But there's a good reason not to. The two flags we check
> below are
> +	 * both are enabled by default in the kernel, so if the
> hcall is not
> +	 * functional they will be enabled.
> +	 * On a system where the host firmware has been updated (so
> the ori
> +	 * functions as a barrier), but on which the hypervisor
> (KVM/Qemu) has
> +	 * not been updated, we would like to enable the barrier.
> Dropping the
> +	 * check for SEC_FTR_SPEC_BAR_ORI31 achieves that. The only
> downside is
> +	 * we potentially enable the barrier on systems where the
> host firmware
> +	 * is not updated, but that's harmless as it's a no-op.
> +	 */
> +	enable = security_ftr_enabled(SEC_FTR_FAVOUR_SECURITY) &&
> +		 security_ftr_enabled(SEC_FTR_BNDS_CHK_SPEC_BAR);
> +
> +	enable_barrier_nospec(enable);
> +}

I am missing the option for the barrier to be disabled by a kernel
commandline argument here.

It does make sense to add a kernel parameter that is checked on boot to
be compatible with other platforms that implement one.

Thanks

Michal