From: Michal Suchanek <hidden> Date: 2018-03-13 18:32:58
Hello,
this is patchset adding barrier_nospec on powerpc. It is based on the
out-of-tree gmb() patch and the existing rfi patches.
I do not have the tests for the Spectre/Meltdown issues available so this is
untested.
Feedback on the general approach as well as actual effectivity is welcome.
Thanks
Michal
Michal Suchanek (8):
powerpc: Add barrier_nospec
powerpc: Use barrier_nospec in copy_from_user
powerpc/64: Use barrier_nospec in syscall entry
powerpc/64s: Add support for ori barrier_nospec
powerpc/64: Patch barrier_nospec in modules
powerpc/64: barrier_nospec: Add debugfs trigger
powerpc/64s: barrier_nospec: Add hcall triggerr
powerpc/64: barrier_nospec: Add commandline trigger
arch/powerpc/include/asm/barrier.h | 9 ++++
arch/powerpc/include/asm/feature-fixups.h | 9 ++++
arch/powerpc/include/asm/setup.h | 11 +++++
arch/powerpc/include/asm/uaccess.h | 11 ++++-
arch/powerpc/kernel/entry_64.S | 3 ++
arch/powerpc/kernel/module.c | 6 +++
arch/powerpc/kernel/setup_64.c | 72 +++++++++++++++++++++++++++++++
arch/powerpc/kernel/vmlinux.lds.S | 7 +++
arch/powerpc/lib/feature-fixups.c | 38 ++++++++++++++++
arch/powerpc/platforms/pseries/setup.c | 38 ++++++++++------
10 files changed, 190 insertions(+), 14 deletions(-)
--
2.13.6
@@ -75,6 +75,15 @@ do { \___p1;\})+/* TODO: add patching so this can be disabled */+/* Prevent speculative execution past this barrier. */+#define barrier_nospec_asm ori 31,31,0+#ifdef __ASSEMBLY__+#define barrier_nospec barrier_nospec_asm+#else+#define barrier_nospec() __asm__ __volatile__ (stringify_in_c(barrier_nospec_asm) : : :)+#endif+#include<asm-generic/barrier.h>#endif /* _ASM_POWERPC_BARRIER_H */
@@ -75,9 +75,9 @@ do { \___p1;\})-/* TODO: add patching so this can be disabled *//* Prevent speculative execution past this barrier. */-#define barrier_nospec_asm ori 31,31,0+#define barrier_nospec_asm SPEC_BARRIER_FIXUP_SECTION; \+nop#ifdef __ASSEMBLY__#define barrier_nospec barrier_nospec_asm#else
@@ -49,8 +49,16 @@ enum l1d_flush_type {L1D_FLUSH_MTTRIG=0x8,};+/* These are bit flags */+enumspec_barrier_type{+SPEC_BARRIER_NONE=0x1,+SPEC_BARRIER_ORI=0x2,+};+void__initsetup_rfi_flush(enuml1d_flush_type,boolenable);voiddo_rfi_flush_fixups(enuml1d_flush_typetypes);+void__initsetup_barrier_nospec(enumspec_barrier_type,boolenable);+voiddo_barrier_nospec_fixups(enumspec_barrier_typetype);#endif /* !__ASSEMBLY__ */
From: Michal Suchanek <hidden> Date: 2018-03-13 18:33:03
Copypasta from lwsync patching.
Note that unlike RFI which is patched only in kernel the nospec state
reflects settings at the time the module was loaded.
Iterating all modules and re-patching every time the settings change is
not implemented.
Signed-off-by: Michal Suchanek <redacted>
---
arch/powerpc/include/asm/setup.h | 5 ++++-
arch/powerpc/kernel/module.c | 6 ++++++
arch/powerpc/kernel/setup_64.c | 4 ++--
arch/powerpc/lib/feature-fixups.c | 17 ++++++++++++++---
4 files changed, 26 insertions(+), 6 deletions(-)
@@ -459,38 +459,50 @@ static void __init find_and_init_phbs(void)of_pci_check_probe_only();}-staticvoidpseries_setup_rfi_flush(void)+staticvoidpseries_setup_rfi_nospec(void){structh_cpu_char_resultresult;-enuml1d_flush_typetypes;-boolenable;+enuml1d_flush_typeflush_types;+enumspec_barrier_typebarrier_type;+boolflush_enable;+boolbarrier_enable;longrc;/* Enable by default */-enable=true;+flush_enable=true;+barrier_enable=true;+/* no fallback if the firmware does not tell us */+barrier_type=SPEC_BARRIER_NONE;rc=plpar_get_cpu_characteristics(&result);if(rc==H_SUCCESS){-types=L1D_FLUSH_NONE;+flush_types=L1D_FLUSH_NONE;if(result.character&H_CPU_CHAR_L1D_FLUSH_TRIG2)-types|=L1D_FLUSH_MTTRIG;+flush_types|=L1D_FLUSH_MTTRIG;if(result.character&H_CPU_CHAR_L1D_FLUSH_ORI30)-types|=L1D_FLUSH_ORI;+flush_types|=L1D_FLUSH_ORI;+if(result.character&H_CPU_CHAR_SPEC_BAR_ORI31)+barrier_type|=SPEC_BARRIER_ORI;/* Use fallback if nothing set in hcall */-if(types==L1D_FLUSH_NONE)-types=L1D_FLUSH_FALLBACK;+if(flush_types==L1D_FLUSH_NONE)+flush_types=L1D_FLUSH_FALLBACK;if((!(result.behaviour&H_CPU_BEHAV_L1D_FLUSH_PR))||(!(result.behaviour&H_CPU_BEHAV_FAVOUR_SECURITY)))-enable=false;+flush_enable=false;++if((!(result.behaviour&H_CPU_BEHAV_BNDS_CHK_SPEC_BAR))||+(!(result.behaviour&H_CPU_BEHAV_FAVOUR_SECURITY)))+barrier_enable=false;}else{/* Default to fallback if case hcall is not available */-types=L1D_FLUSH_FALLBACK;+flush_types=L1D_FLUSH_FALLBACK;}-setup_rfi_flush(types,enable);+setup_barrier_nospec(barrier_type,barrier_enable);+setup_rfi_flush(flush_types,flush_enable);}#ifdef CONFIG_PCI_IOV
@@ -666,7 +678,7 @@ static void __init pSeries_setup_arch(void)fwnmi_init();-pseries_setup_rfi_flush();+pseries_setup_rfi_nospec();/* By default, only probe PCI (can be overridden by rtas_pci) */pci_add_flags(PCI_PROBE_ONLY);
From: Michal Suchanek <hidden> Date: 2018-03-15 19:15:50
When the firmware supports it an otherwise useless combination of ORI
instruction arguments is interpreted as speculation barrier. Implement
barrier_nospec using this instruction.
Based on the out-of-tree gmb() implementation.
Signed-off-by: Michal Suchanek <redacted>
---
arch/powerpc/include/asm/barrier.h | 9 +++++++++
1 file changed, 9 insertions(+)
@@ -75,6 +75,15 @@ do { \___p1;\})+/* TODO: add patching so this can be disabled */+/* Prevent speculative execution past this barrier. */+#define barrier_nospec_asm ori 31,31,0+#ifdef __ASSEMBLY__+#define barrier_nospec barrier_nospec_asm+#else+#define barrier_nospec() __asm__ __volatile__ (stringify_in_c(barrier_nospec_asm) : : :)+#endif+#include<asm-generic/barrier.h>#endif /* _ASM_POWERPC_BARRIER_H */
From: Michal Suchanek <hidden> Date: 2018-03-15 19:15:51
This is based on x86 patch doing the same.
Signed-off-by: Michal Suchanek <redacted>
---
arch/powerpc/include/asm/uaccess.h | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
From: Michal Suchanek <hidden> Date: 2018-03-15 19:15:52
On powerpc syscall entry is done in assembly so patch in an explicit
barrier_nospec.
Signed-off-by: Michal Suchanek <redacted>
---
arch/powerpc/kernel/entry_64.S | 3 +++
1 file changed, 3 insertions(+)
From: Michal Suchanek <hidden> Date: 2018-03-15 19:15:53
The RFI flush support patches the speculation barrier into
RFI_FLUSH_SLOT as part of the RFI flush. Use separate barrier_nospec
instead.
Signed-off-by: Michal Suchanek <redacted>
---
arch/powerpc/include/asm/exception-64s.h | 2 +-
arch/powerpc/lib/feature-fixups.c | 9 +++------
2 files changed, 4 insertions(+), 7 deletions(-)
From: Michal Suchanek <hidden> Date: 2018-03-15 19:15:54
Based on the RFI patching. This is required to be able to disable the
speculation barrier.
Only one barrier type is supported and it does nothing when the firmware
does not enable it. Also re-patching modules is not supported So the
only meaningful thing that can be done is patching out the speculation
barrier at boot when the user says it is not wanted.
Signed-off-by: Michal Suchanek <redacted>
---
arch/powerpc/include/asm/barrier.h | 4 ++--
arch/powerpc/include/asm/feature-fixups.h | 9 +++++++++
arch/powerpc/include/asm/setup.h | 8 ++++++++
arch/powerpc/kernel/setup_64.c | 30 ++++++++++++++++++++++++++++++
arch/powerpc/kernel/vmlinux.lds.S | 7 +++++++
arch/powerpc/lib/feature-fixups.c | 27 +++++++++++++++++++++++++++
6 files changed, 83 insertions(+), 2 deletions(-)
@@ -75,9 +75,9 @@ do { \___p1;\})-/* TODO: add patching so this can be disabled *//* Prevent speculative execution past this barrier. */-#define barrier_nospec_asm ori 31,31,0+#define barrier_nospec_asm SPEC_BARRIER_FIXUP_SECTION; \+nop#ifdef __ASSEMBLY__#define barrier_nospec barrier_nospec_asm#else
@@ -49,8 +49,16 @@ enum l1d_flush_type {L1D_FLUSH_MTTRIG=0x8,};+/* These are bit flags */+enumspec_barrier_type{+SPEC_BARRIER_NONE=0x1,+SPEC_BARRIER_ORI=0x2,+};+voidsetup_rfi_flush(enuml1d_flush_type,boolenable);voiddo_rfi_flush_fixups(enuml1d_flush_typetypes);+voidsetup_barrier_nospec(enumspec_barrier_type,boolenable);+voiddo_barrier_nospec_fixups(enumspec_barrier_typetype);#endif /* !__ASSEMBLY__ */
From: Michal Suchanek <hidden> Date: 2018-03-15 19:15:55
Note that unlike RFI which is patched only in kernel the nospec state
reflects settings at the time the module was loaded.
Iterating all modules and re-patching every time the settings change is
not implemented.
Based on lwsync patching.
Signed-off-by: Michal Suchanek <redacted>
---
arch/powerpc/include/asm/setup.h | 5 ++++-
arch/powerpc/kernel/module.c | 6 ++++++
arch/powerpc/kernel/setup_64.c | 4 ++--
arch/powerpc/lib/feature-fixups.c | 17 ++++++++++++++---
4 files changed, 26 insertions(+), 6 deletions(-)
@@ -349,7 +349,7 @@ void post_mobility_fixup(void)"failed: %d\n",rc);/* Possibly switch to a new RFI flush type */-pseries_setup_rfi_flush();+pseries_setup_rfi_nospec();return;}
@@ -459,30 +459,47 @@ static void __init find_and_init_phbs(void)of_pci_check_probe_only();}-voidpseries_setup_rfi_flush(void)+voidpseries_setup_rfi_nospec(void){structh_cpu_char_resultresult;-enuml1d_flush_typetypes;-boolenable;+enuml1d_flush_typeflush_types;+enumspec_barrier_typebarrier_type;+boolflush_enable;+boolbarrier_enable;longrc;/* Enable by default */-enable=true;-types=L1D_FLUSH_FALLBACK;+flush_enable=true;+flush_types=L1D_FLUSH_FALLBACK;+barrier_enable=true;+/* no fallback available if the firmware does not tell us */+barrier_type=SPEC_BARRIER_NONE;rc=plpar_get_cpu_characteristics(&result);if(rc==H_SUCCESS){if(result.character&H_CPU_CHAR_L1D_FLUSH_TRIG2)-types|=L1D_FLUSH_MTTRIG;+flush_types|=L1D_FLUSH_MTTRIG;if(result.character&H_CPU_CHAR_L1D_FLUSH_ORI30)-types|=L1D_FLUSH_ORI;+flush_types|=L1D_FLUSH_ORI;+if(result.character&H_CPU_CHAR_SPEC_BAR_ORI31)+barrier_type|=SPEC_BARRIER_ORI;if((!(result.behaviour&H_CPU_BEHAV_L1D_FLUSH_PR))||(!(result.behaviour&H_CPU_BEHAV_FAVOUR_SECURITY)))-enable=false;+flush_enable=false;+/*+*DonotcheckH_CPU_BEHAV_BNDS_CHK_SPEC_BAR-theORIdoes+*nothinganywaywhennotsupported.+*/+if((!(result.behaviour&H_CPU_BEHAV_FAVOUR_SECURITY)))+barrier_enable=false;+}else{+/* Default to fallback if case hcall is not available */+flush_types=L1D_FLUSH_FALLBACK;}-setup_rfi_flush(types,enable);+setup_barrier_nospec(barrier_type,barrier_enable);+setup_rfi_flush(flush_types,flush_enable);}#ifdef CONFIG_PCI_IOV
@@ -658,7 +675,7 @@ static void __init pSeries_setup_arch(void)fwnmi_init();-pseries_setup_rfi_flush();+pseries_setup_rfi_nospec();/* By default, only probe PCI (can be overridden by rtas_pci) */pci_add_flags(PCI_PROBE_ONLY);
From: Michal Suchanek <hidden> Date: 2018-03-15 19:15:58
Add commandline options spectre_v2 and nospectre_v2
These are named same as similar x86 options regardless of actual effect
to not require platform-specific configuration.
Supported options:
nospectre_v2 or spectre_v2=off - speculation barrier not used
spectre_v2=on or spectre_v2=auto - speculation barrier used
Changing the settings after boot is not supported and VM migration may
change requirements so auto is same as on.
Based on s390 implementation
Signed-off-by: Michal Suchanek <redacted>
---
arch/powerpc/kernel/setup_64.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
@@ -258,8 +259,10 @@ do { \long__gu_err=-EFAULT;\unsignedlong__gu_val=0;\const__typeof__(*(ptr))__user*__gu_addr=(ptr);\+intcan_access=access_ok(VERIFY_READ,__gu_addr,(size));\might_fault();\-if(access_ok(VERIFY_READ,__gu_addr,(size)))\+barrier_nospec();\+if(can_access)\__get_user_size(__gu_val,__gu_addr,(size),__gu_err);\(x)=(__force__typeof__(*(ptr)))__gu_val;\__gu_err;\
Is the above really correct? The barrier is *before* the conditional
branch that might be mis-predicted.
I don't know how the ppc barrier works, but that sounds completely bogus.
Linus
From: Nicholas Piggin <npiggin@gmail.com> Date: 2018-03-16 05:18:23
On Thu, 15 Mar 2018 20:15:52 +0100
Michal Suchanek [off-list ref] wrote:
On powerpc syscall entry is done in assembly so patch in an explicit
barrier_nospec.
Same comment as Linus for this -- the barriers are before the branch here,
so is it possible the branch instruction can be speculative while the index
is used to load the syscall table?
Thanks,
Nick
From: Michal Suchánek <hidden> Date: 2018-03-16 09:15:49
Hello,
On Fri, 16 Mar 2018 15:18:23 +1000
Nicholas Piggin [off-list ref] wrote:
On Thu, 15 Mar 2018 20:15:52 +0100
Michal Suchanek [off-list ref] wrote:
quoted
On powerpc syscall entry is done in assembly so patch in an explicit
barrier_nospec.
Same comment as Linus for this -- the barriers are before the branch
here, so is it possible the branch instruction can be speculative
while the index is used to load the syscall table?
As far as I understand barriers they separate code before the barrier
and code after the barrier.
So inserting barrier_nospec after cmpldi means that the result of the
cmpldi has to be known before any instruction following barrier_nospec
that depends on the result can be executed.
In many cases it is useful to put the barrier after a branch. It allows
the compiler to speculate on the computed value at compile time and if
it is constrained optimize out the branch. It may also result in the
need to include many barriers and less readable code.
However, you have probably knowledge of the powerpc implementation of
the barrier so if the semantic is actually different then please
enlighten me.
Thanks
Michal
From: Nicholas Piggin <npiggin@gmail.com> Date: 2018-03-16 10:46:47
On Fri, 16 Mar 2018 10:15:49 +0100
Michal Suchánek [off-list ref] wrote:
Hello,
On Fri, 16 Mar 2018 15:18:23 +1000
Nicholas Piggin [off-list ref] wrote:
quoted
On Thu, 15 Mar 2018 20:15:52 +0100
Michal Suchanek [off-list ref] wrote:
quoted
On powerpc syscall entry is done in assembly so patch in an explicit
barrier_nospec.
Same comment as Linus for this -- the barriers are before the branch
here, so is it possible the branch instruction can be speculative
while the index is used to load the syscall table?
As far as I understand barriers they separate code before the barrier
and code after the barrier.
So inserting barrier_nospec after cmpldi means that the result of the
cmpldi has to be known before any instruction following barrier_nospec
that depends on the result can be executed.
In many cases it is useful to put the barrier after a branch. It allows
the compiler to speculate on the computed value at compile time and if
it is constrained optimize out the branch. It may also result in the
need to include many barriers and less readable code.
However, you have probably knowledge of the powerpc implementation of
the barrier so if the semantic is actually different then please
enlighten me.
I actually don't. I'm assuming we should be able to say that no previous
instruction is speculative when a subsequent one is executed.
But the branch instruction itself that is speculated, not the compare.
Usually even if all sources are ready, the pipeline may take in some
cycles after a branch, before that branch can finish executing and
squash speculation if it was wrong. Perhaps there is only a couple of
cycles of instructions that get a chance to reach execution units and
disturb any caches, but still there could be some window and I don't
think we would have architectural gurantees on that.
I'll try to ask around and see if there's any documentation we can
give you yet.
Thanks,
Nick
@@ -258,8 +259,10 @@ do { \long__gu_err=-EFAULT;\unsignedlong__gu_val=0;\const__typeof__(*(ptr))__user*__gu_addr=(ptr);\+intcan_access=access_ok(VERIFY_READ,__gu_addr,(size));\might_fault();\-if(access_ok(VERIFY_READ,__gu_addr,(size)))\+barrier_nospec();\+if(can_access)\__get_user_size(__gu_val,__gu_addr,(size),__gu_err);\(x)=(__force__typeof__(*(ptr)))__gu_val;\__gu_err;\
Is the above really correct? The barrier is *before* the conditional
branch that might be mis-predicted.
I don't know how the ppc barrier works, but that sounds completely bogus.
Yeah it should be after the branch.
I don't have a formal spec for the barrier yet, it should be defined in
a hopefully soon to be released revision of the ISA.
But the gist is it will stall execution until any older branches are no
longer speculating.
It doesn't order any two arbitrary instructions, such as a comparison
and a branch, which I suspect is how Michal was interpreting it.
cheers
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2018-03-16 13:28:42
Hi Michal,
Thanks for working on this series in the absence of any documentation.
Michal Such=C3=A1nek [off-list ref] writes:
On Fri, 16 Mar 2018 15:18:23 +1000
Nicholas Piggin [off-list ref] wrote:
quoted
On Thu, 15 Mar 2018 20:15:52 +0100
Michal Suchanek [off-list ref] wrote:
=20
quoted
On powerpc syscall entry is done in assembly so patch in an explicit
barrier_nospec.=20=20
=20
Same comment as Linus for this -- the barriers are before the branch
here, so is it possible the branch instruction can be speculative
while the index is used to load the syscall table?
As far as I understand barriers they separate code before the barrier
and code after the barrier.
So inserting barrier_nospec after cmpldi means that the result of the
cmpldi has to be known before any instruction following barrier_nospec
that depends on the result can be executed.=20
That would make sense, but I don't think that's how the barrier's been
defined.
I don't have a formal spec for it (yet), but what I do have indicates it
only orders older branches vs future instructions.
However, you have probably knowledge of the powerpc implementation of
the barrier so if the semantic is actually different then please
enlighten me.
We have some knowledge, but only some :)
It's not necessarily implemented the same way on each chip revision, so
it's not entirely clear what the formal semantics will be vs what we are
seeing in current implementations. But I think it's safe to say it
should always go after the branch that might be speculatively executed.
Will try and get some better documentation for you.
cheers
On Fri, Mar 16, 2018 at 2:15 AM, Michal Such=C3=A1nek [off-list ref] w=
rote:
As far as I understand barriers they separate code before the barrier
and code after the barrier.
Almost certainly not. Even if you were to do an expensive
serialization before the branch, the branch will still predict after
the serialization.
The thing is, it doesn't make sense to insert a barrier before a
conditional branch for Spectre mitigation.
The problem is not that the data isn't ready for the branch - the
problem is that the branch is predicted _regardless_ of the data.
Sure, some micro-architecture might not predict branches at all if
they have a stable conditional, so a barrier _can_ make sense.
But fundamentally, good branch prediction - in order to be optimal -
has to happen before instructions have even been parsed, much less
things like "stable conditional register state" having been decided
on. You'll want to do I$ prefetching etc.
So the problem is that even if the data is ready, the branch will be
predicted according to some unrelated historical data, and a barrier
to make the branch conditional be stable is pointless.
A barrier *after* the branch, making sure that you don't actually
start executing instructions past it (even if you might have predicted
and fetched stuff past it) *if* you have mis-predicted the previous
branch, is what a sane architecture would specify.
Of course, on x86, we mostly tried to avoid branch prediction being
the critical problem and having to have barriers by just making it an
address generation dependency instead. That should presumably work on
powerpc too, since address generation is part of the memory ordering
definition. But obviously a microarchitecture *could* end up
speculating and just redoing even for memory ordering, and maybe the
ppc architects prefer the barrier since they are already used to crazy
and not very well architected barriers elsewhere.
Linus
Add commandline options spectre_v2 and nospectre_v2=0A=
=0A=
These are named same as similar x86 options regardless of actual effect=
=0A=
to not require platform-specific configuration.=0A=
=0A=
Supported options:=0A=
nospectre_v2 or spectre_v2=3Doff - speculation barrier not used=0A=
spectre_v2=3Don or spectre_v2=3Dauto - speculation barrier used=0A=
=0A=
Why the barrier is enabled only for spectre variant 2 mitigations? It=0A=
can be used as well for variant 1 mitigations. In fact I am not sure=0A=
that the places where it is used fall under spectre 2 at all.=0A=
=0A=
quoted hunk
=0A=
Changing the settings after boot is not supported and VM migration may=0A=
change requirements so auto is same as on.=0A=
=0A=
Based on s390 implementation=0A=
=0A=
Signed-off-by: Michal Suchanek <redacted>=0A=
---=0A=
arch/powerpc/kernel/setup_64.c | 22 ++++++++++++++++++++++=0A=
1 file changed, 22 insertions(+)=0A=
=0A=