> On May 6, 2019 at 9:29 PM Michael Ellerman <m...@ellerman.id.au> wrote: > > > Christopher M Riedl <c...@informatik.wtf> writes: > >> On May 5, 2019 at 9:32 PM Andrew Donnellan <a...@linux.ibm.com> wrote: > >> On 6/5/19 8:10 am, Christopher M. Riedl wrote: > >> > Add support for disabling the kernel implemented spectre v2 mitigation > >> > (count cache flush on context switch) via the nospectre_v2 cmdline > >> > option. > >> > > >> > Suggested-by: Michael Ellerman <m...@ellerman.id.au> > >> > Signed-off-by: Christopher M. Riedl <c...@informatik.wtf> > >> > > >> > diff --git a/arch/powerpc/kernel/security.c > >> > b/arch/powerpc/kernel/security.c > >> > index b33bafb8fcea..f7c34745cd0f 100644 > >> > --- a/arch/powerpc/kernel/security.c > >> > +++ b/arch/powerpc/kernel/security.c > >> > @@ -391,6 +394,13 @@ static void toggle_count_cache_flush(bool enable) > >> > > >> > void setup_count_cache_flush(void) > >> > { > >> > + if (no_spectrev2) { > >> > + if (security_ftr_enabled(SEC_FTR_BCCTRL_SERIALISED) > >> > + || > >> > security_ftr_enabled(SEC_FTR_COUNT_CACHE_DISABLED)) > >> > + pr_warn("Spectre v2 mitigations not under > >> > software control, can't disable\n"); > >> > >> If one of those ftrs is set, what's the impact of not calling > >> toggle_count_cache_flush()? > >> > > > > The patchsite/callsite (kernel/entry_64.S:597) for flush_count_cache > > inside _switch remains a nop. > > > > Disassembly of vmlinux after build: > > c00000000000e260: 00 00 23 f8 std r1,0(r3) > > c00000000000e264: 00 00 00 60 nop > > c00000000000e268: 00 60 c0 3c lis r6,24576 > > > > Disassembly (xmon) after boot/during runtime in qemu: > > c00000000000e260 f8230000 std r1,0(r3) > > c00000000000e264 4bffdb7d bl c00000000000bde0 # > > flush_count_cache+0x0/0x2420 > > c00000000000e268 3cc06000 lis r6,24576 > > > > Disassembly (xmon) after boot/during runtime in qemu w/ nospectre_v2: > > c00000000000e260 f8230000 std r1,0(r3) > > c00000000000e264 60000000 nop > > c00000000000e268 3cc06000 lis r6,24576 > > Yes you're right, in this case the initial state is deactivated. > > > toggle_count_cache_flush() well uhh "toggles" the patchsite to either > > contain a branch to the flush procedure or a nop. > > Sort of. It doesn't actually know the initial state, so calling it at > boot with false will still nop out the nops. > > I think I'd probably prefer it if we still call > toggle_count_cache_flush(false) when no_spectrev2 is true. The main > reason being that it means we'll print to dmesg, but it would also avoid > problems if we ever change the initial state of the count cache flush. > > cheers
Sounds good, I will add this to the next version. Thanks!