Re: [RFC PATCH] powerpc/fsl: Add barrier_nospec implementation for NXP PowerPC Book E

On Tue, 2018-05-29 at 15:22 +0000, Diana Madalina Craciun wrote:

Hi Scott,

Thanks for the review.

On 05/22/2018 11:31 PM, Scott Wood wrote:

quoted

On Tue, 2018-05-22 at 10:10 +0300, Diana Craciun wrote:

quoted

Implement the barrier_nospec as a isync;sync instruction sequence.
The implementation uses the infrastructure built for BOOK3S 64
with the difference that for NXP platforms there is no firmware involved
and the need for a speculation barrier is read from the device tree.
I have used the same name for the property:
fsl,needs-spec-barrier-for-bounds-check

Using the device tree this way means that anyone without an updated device
tree won't get the protection.  I also don't see any device tree updates
--
which chips are affected?

I was planning to have the device tree changes in a different patch-set.
The affected cores are e500, e500mc, e5500, e6500.

So, all supported FSL/NXP book E chips.  Why not just enable the workaround
unconditionally (and revisit if NXP ever produces a book E chip that doesn't
need it and/or e200 is ever supported if that's simple enough to be immune)?

quoted

Why patch nops in if not enabled?  Aren't those locations already
nops?  For
that matter, how can this function even be called on FSL_BOOK3E with
enable !=
true?

There is some code in arch/powerpc/kernel/security.c which allows
control of barrier_nospec via debugfs.

OK.

quoted

Should there be a way for the user to choose not to enable this (editing
the
device tree doesn't count), for a use case that is not sufficiently
security
sensitive to justify the performance loss?  What is the performance impact
of
this patch?

My reason was that on the other architectures Spectre variant 1
mitigations are not disabled either. But I think that it might be a good
idea to add a bootarg parameter to disable the barrier.

Is there a specific policy reason why they allow spectre v2 to be disabled but
not v1, or just a matter of not having a mechanism to disable it, or the parts
which could practically be disabled not impacting performance much?

-Scott