diff mbox series

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

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

Commit Message

Michael Ellerman April 24, 2018, 4:15 a.m. UTC 
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(+)

Comments

Michal Suchánek April 26, 2018, 4:02 p.m. UTC | #1 
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
Michael Ellerman May 1, 2018, 11:11 a.m. UTC | #2 
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
Michal Suchánek May 2, 2018, 11:41 a.m. UTC | #3 
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
Michael Ellerman May 4, 2018, 12:58 a.m. UTC | #4 
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 mbox series

Patch

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