Re: [PATCH] x86/CPU/AMD: Make sure EFER[AIBRSE] is set

[Borislav Petkov]

On Sat, Feb 25, 2023 at 03:43:30PM -0800, Josh Poimboeuf wrote:
> Hm?  So code in bugs.c is not vendor-specific?  That seems circular and
> I don't get your point.

Lemme try that again...

So there's an obvious benefit of keeping vendor-specific CPU code in one
place: Intel stuff in cpu/intel*, AMD stuff in cpu/amd.c

The sekjority stuff is still vendor-specific CPU code.

Now, if you wanna add a function pointer ->bugs_init or so, say, to

	struct cpu_dev

and keep the respective code in amd.c or intel.c, then we get the best
of both worlds:

- vendor-specific code remains in the respective file

- you have a vendor-specific function which does hw vuln-specific work
  *without* vendor checks and so on

> Right, so rather than spreading all the bug-related MSR logic around,
> just do it in one spot.

It is all CPU init code and I'm wondering if splitting stuff by vendor
wouldn't make all that maze in bugs.c a lot more palatable. And get rid
of

$ git grep VENDOR arch/x86/kernel/cpu/bugs.c | wc -l
11

those, for starters.

There's this trade-off of

1. keeping bugs setup code in one place - but then you need to do vendor
   checks and the other CPU setup code is somewhere else and it is
   probably related, MSR_AMD64_DE_CFG_LFENCE_SERIALIZE_BIT in amd.c for
   example.

or

2. separating it into their respective files. Then the respective vendor
   code is simple because you don't need vendor checks. It would need to
   be done in a slick way, though, so that it remains maintainable.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette