Message ID | 20180424041559.32410-4-mpe@ellerman.id.au (mailing list archive) |
---|---|
State | Accepted |
Commit | cb3d6759a93c6d0aea1c10deb6d00e111c29c19c |
Headers | show |
Series | [1/6] powerpc/64s: Add barrier_nospec | expand |
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
Michal Suchánek <msuchanek@suse.de> writes: > 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> ... > > 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. No other platforms have an option to disable variant 1 mitigations, so there isn't an existing parameter we can use. Which is not to say we can't add one, but I wasn't sure if it was really worth it. But I guess we should add one, I might do it as a follow-up patch. cheers
On Tue, 01 May 2018 21:11:06 +1000 Michael Ellerman <mpe@ellerman.id.au> wrote: > Michal Suchánek <msuchanek@suse.de> writes: > > 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> > ... > > > > 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. > > No other platforms have an option to disable variant 1 mitigations, so > there isn't an existing parameter we can use. Right, I was looking at an older implementation which turned off both v1 and v2 with same parameter. In current kernel the v1 mitigation is not turned off at all. > > Which is not to say we can't add one, but I wasn't sure if it was > really worth it. The current thinking is that most performance relevant cases are covered with array_nospec which has little overhead. The less code we have for this the better ;-) Thanks Michal
Michal Suchánek <msuchanek@suse.de> writes: > On Tue, 01 May 2018 21:11:06 +1000 > Michael Ellerman <mpe@ellerman.id.au> wrote: >> Michal Suchánek <msuchanek@suse.de> writes: >> > 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> >> ... >> > >> > 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. >> >> No other platforms have an option to disable variant 1 mitigations, so >> there isn't an existing parameter we can use. > > Right, I was looking at an older implementation which turned off both > v1 and v2 with same parameter. In current kernel the v1 mitigation is > not turned off at all. Ah OK. >> Which is not to say we can't add one, but I wasn't sure if it was >> really worth it. > > The current thinking is that most performance relevant cases are > covered with array_nospec which has little overhead. The less code we > have for this the better ;-) Amen to that :) cheers
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); +} + +#ifdef CONFIG_DEBUG_FS +static int barrier_nospec_set(void *data, u64 val) +{ + switch (val) { + case 0: + case 1: + break; + default: + return -EINVAL; + } + + if (!!val == !!barrier_nospec_enabled) + return 0; + + enable_barrier_nospec(!!val); + + return 0; +} + +static int barrier_nospec_get(void *data, u64 *val) +{ + *val = barrier_nospec_enabled ? 1 : 0; + return 0; +} + +DEFINE_SIMPLE_ATTRIBUTE(fops_barrier_nospec, + barrier_nospec_get, barrier_nospec_set, "%llu\n"); + +static __init int barrier_nospec_debugfs_init(void) +{ + debugfs_create_file("barrier_nospec", 0600, powerpc_debugfs_root, NULL, + &fops_barrier_nospec); + return 0; +} +device_initcall(barrier_nospec_debugfs_init); +#endif /* CONFIG_DEBUG_FS */ + ssize_t cpu_show_meltdown(struct device *dev, struct device_attribute *attr, char *buf) { bool thread_priv; diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c index ef8c9ce53a61..e2ca5f77a55f 100644 --- a/arch/powerpc/platforms/powernv/setup.c +++ b/arch/powerpc/platforms/powernv/setup.c @@ -124,6 +124,7 @@ static void pnv_setup_rfi_flush(void) security_ftr_enabled(SEC_FTR_L1D_FLUSH_HV)); setup_rfi_flush(type, enable); + setup_barrier_nospec(); } static void __init pnv_setup_arch(void) diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c index b55ad4286dc7..63b1f0d10ef0 100644 --- a/arch/powerpc/platforms/pseries/setup.c +++ b/arch/powerpc/platforms/pseries/setup.c @@ -534,6 +534,7 @@ void pseries_setup_rfi_flush(void) security_ftr_enabled(SEC_FTR_L1D_FLUSH_PR); setup_rfi_flush(types, enable); + setup_barrier_nospec(); } #ifdef CONFIG_PCI_IOV