Re: [PATCH] x86/cpufeatures: Cleanup AMD speculation feature bits

[David Woodhouse <dwmw2@infradead.org>]

[-- Attachment #1: Type: text/plain, Size: 2287 bytes --]

On Sat, 2018-01-27 at 10:37 +0100, Borislav Petkov wrote:
> On Sat, Jan 27, 2018 at 09:27:48AM +0000, David Woodhouse wrote:
> > 
> > http://david.woodhou.se/cleanup-feature-bits.patch on top of my full
> > tree?
> @@ -223,7 +223,7 @@ static inline void indirect_branch_prediction_barrier(void)
>  				 "movl %[val], %%eax\n\t"
>  				 "movl $0, %%edx\n\t"
>  				 "wrmsr",
> -				 X86_FEATURE_IBPB)
> +				 X86_FEATURE_USE_IBPB)
> 
> I still don't think that's the right approach: I'd call the
> software-defined, synthetic features
> 
> X86_FEATURE_IBPB
> X86_FEATURE_IBRS
> X86_FEATURE_STIBP
> 
> then make *them* visible in /proc/cpuinfo and use them everywhere in the
> code.

No because cpuinfo should be information about the CPU. For details of
what mitigations are *actually* in use on this kernel, you want
/sys/…/cpu/vulnerabilities, which might not even be readable by a non-
root user.

That's why I've used the names that we want to see in cpuinfo, for the
basic CPU functionality.

> Only the vendor-specific detection code will set the synthetic ones when
> it detects a corresponding vendor-specific one.
> 
> This way one *only* concentrates on the three above everywhere and
> only low-level, early, vendor-specific code takes care to set the
> corresponding synthetic features based on the actual hardware bits it
> detects.
> 
> I think that unifies the view both to the user *and* to the rest of the
> kernel which should not care about the actual name of a hardware feature
> bit.
> 
> And then you avoid coders scratching heads, asking, so what should I
> use, X86_FEATURE_IBPB or X86_FEATURE_USE_IBPB.

Does it exist, vs. whether the kernel is *using* it. The latter being a
little bit of a hack because alternatives *only* let us do this stuff
based on "CPU features", which is why X86_FEATURE_PTI exists.

That one probably shouldn't be user-visible in /proc/cpuinfo *either*,
should it?

> Instead you call IBPB the synthetic one and the hardware feature name is
> something different like PRED_CMD or so. This will drop the confusion
> additionally.

I think I covered this, but for clarity: No, because the *hardware*
feature is the one we want to be called just "ibpb" in /proc/cpuinfo.

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5213 bytes --]