[PATCH] powerpc/64s: support nospectre_v2 cmdline option

[Christopher M Riedl]

> On May 6, 2019 at 9:29 PM Michael Ellerman <mpe at ellerman.id.au> wrote:
> 
> 
> Christopher M Riedl <cmr at informatik.wtf> writes:
> >> On May 5, 2019 at 9:32 PM Andrew Donnellan <ajd at 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 <mpe at ellerman.id.au>
> >> > Signed-off-by: Christopher M. Riedl <cmr at 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!