Re: [RFC PATCH 3/4] x86/speculation: Use IBRS if available before calling into firmware

[David Woodhouse <dwmw2@infradead.org>]

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



On Wed, 2018-02-07 at 12:17 +0100, Borislav Petkov wrote:
> Just some random thoughts:
> 
> On Wed, Feb 07, 2018 at 12:03:13AM +0000, David Woodhouse wrote:
> > 
> > +#define alternative_msr_write(_msr, _val, _feature)		\
> > +	asm volatile(ALTERNATIVE("",				\
> > +				 "movl %[msr], %%ecx\n\t"	\
> > +				 "movl %[val], %%eax\n\t"	\
> > +				 "movl $0, %%edx\n\t"		\
> We'll never write anything except 0 in %edx?

For these uses, no.

> > 
> > +				 "wrmsr",			\
> > +				 _feature)			\
> > +		     : : [msr] "i" (_msr), [val] "i" (_val)	\
> > +		     : "eax", "ecx", "edx", "memory")
>
> So I'm not crazy about making it a separate macro because TBH it doesn't
> look too generic to do that but then again what do I know, considering
> recent history. :-)

It was mostly split out so that if you want to keep playing silly
buggers with it, you can do so in only one place and not three.

If you don't want to keep messing with it, I'm happy to not bother ;)

> 	 [ Maybe I need to not look at the spectral meltdown for a
> 	   couple of weeks and simply take a break. ]
> 
> I guess it is fine if it is in nospec-branch.h and not prefix it with
> alternative_ so that it doesn't give people ideas.

Right. It wasn't really being proposed as a generic alternative.

> Oh and firmware_restrict_branch_speculation_start/end() is just too long
> a name.

Yeah... Thomas wanted descriptive names; I'm not really convinced.

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