On Sat, 8 Feb 2025 12:03:09 -0800
Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
> On Sat, 8 Feb 2025 at 11:06, David Laight <david.laight.linux@xxxxxxxxx> wrote:
> >
> > Can the 'alternatives' be flipped so the .o doesn't contain loads of nops?
>
> Sadly, no. The instructions generate #UD if the CPU doesn't support SMAP.
>
> Now, arguably the alternatives *should* be fixed up before the first
> user space access and thus it would be safe to switch, but honestly, I
> don't want to risk some early code doing odd things. The potential
> pain would be too damn high.
>
> > It'd be nice to see the clac and lfence.
>
> Heh. I agree 100%, which is why my personal tree has a patch like this:
>
> -#define LOCK_PREFIX_HERE \
> - ".pushsection .smp_locks,\"a\"\n" \
> - ".balign 4\n" \
> - ".long 671f - .\n" /* offset */ \
> - ".popsection\n" \
> - "671:"
> +#define LOCK_PREFIX_HERE
That one is just hidden bloat. 40k on the kernel I'm building.
Is it really worth the effort for single-cpu systems.
I can't remember if you can still build a kernel with max-cpus of 1.
But a minor change of removing lock prefix at compile time might make sense.
> ...
> -#define barrier_nospec() alternative("", "lfence", X86_FEATURE_LFENCE_RDTSC)
> +#define barrier_nospec() asm volatile("lfence":::"memory")
Isn't that testing the wrong feature bit?
LFENCE_RDTSC indicates that lfence serialises rdtsc.
lfence itself is predicated by XMM2 (see higher up the file) and is always
present for 64bit.
I'm not sure whether old 32bit cpu need a speculation barrier.
But I'm sure I remember something from a very old (p-pro era?) document
that mentioned avoiding interleaving data and code because the data might
be something like atan and the cpu would have to wait for it to finish
(and that would have been after a ret/jmp as well).
So that might have to be rmb() even on old cpu.
In any case it doesn't need to be an alternative on 64bit.
> ...
> -#define ASM_CLAC \
> - ALTERNATIVE "", "clac", X86_FEATURE_SMAP
> -
> -#define ASM_STAC \
> - ALTERNATIVE "", "stac", X86_FEATURE_SMAP
> +#define ASM_CLAC clac
> +#define ASM_STAC stac
> ...
> - /* Note: a barrier is implicit in alternative() */
s/barrier/memory clobber/
> - alternative("", "clac", X86_FEATURE_SMAP);
> + asm volatile("clac":::"memory");
> ...
> - /* Note: a barrier is implicit in alternative() */
> - alternative("", "stac", X86_FEATURE_SMAP);
> + asm volatile("stac":::"memory");
Is there any reason why we get three 0x90 bytes instead of a three-byte nop?
Indeed, since the flags can be changed there are a lot of choices.
Compare %rsp imm8 (83:fc:xx) would probably not affect anything.
It might even decode faster than three nop bytes.
> ...
> -#define ASM_BARRIER_NOSPEC ALTERNATIVE "", "lfence", X86_FEATURE_LFENCE_RDTSC
> +#define ASM_BARRIER_NOSPEC lfence
>
> but I've never made it a real usable patch for others. It would have
> to be some "modern CPU only" config variable, and nobody else has ever
> complained about this until now.
Defaulting to SMAP just requires a bit of bravery!
After all (as you said) it ought to get patched out before userspace exists.
It is still fairly new (Broadwell ix-5xxx).
I'm sure a lot of people are still running Linux on Sandy bridge cpu.
I've not used mine for a few years (since I stopped being an active NetBSD
developer), but might dig it back out for testing if I retire.
(Or just 'borrow' a slightly newer system from work.)
David
>
> Linus
