RE: [PATCH v12 10/18] x86/fsgsbase/64: Enable FSGSBASE instructions in helper functions

[David Laight]

From: Sasha Levin
> Sent: 18 May 2020 21:25
> Thank you for taking the time to review this.
> 
> On Mon, May 18, 2020 at 08:20:08PM +0200, Thomas Gleixner wrote:
> >Sasha Levin <sashal@xxxxxxxxxx> writes:
> >> +unsigned long x86_gsbase_read_cpu_inactive(void)
> >> +{
> >> +	unsigned long gsbase;
> >> +
> >> +	if (static_cpu_has(X86_FEATURE_FSGSBASE)) {
> >> +		bool need_restore = false;
> >> +		unsigned long flags;
> >> +
> >> +		/*
> >> +		 * We read the inactive GS base value by swapping
> >> +		 * to make it the active one. But we cannot allow
> >> +		 * an interrupt while we switch to and from.
> >> +		 */
> >> +		if (!irqs_disabled()) {
> >> +			local_irq_save(flags);
> >> +			need_restore = true;
> >> +		}
> >> +
> >> +		native_swapgs();
> >> +		gsbase = rdgsbase();
> >> +		native_swapgs();

Does local_irq_save() even do anything useful here.
You need to actually execute CLI, not just set a
flag that indicates interrupts shouldn't happen.
(Which is what I think local_irq_save() might do.)

You also (probably) need to disable NMIs.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)