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

Reply via email to