2023-09-13 17:48:08

by Andrew Cooper

[permalink] [raw]
Subject: Re: [PATCH 1/4] x86/srso: Rename srso_alias_*() to srso_fam19_*()

On 13/09/2023 2:46 pm, Borislav Petkov wrote:
> On Mon, Aug 21, 2023 at 12:27:20PM +0100, Andrew Cooper wrote:
>> The 'alias' name name is an internal detail of how the logic works. Rename it
>> to state which microarchitecture is is applicable to.
> Sorry, no. Hardcoding the family into some function is a backwards. The
> moment you need to apply this to some other family, it becomes wrong.
>
> And I prefer much more "srso" and "srso_alias".

You literally have one set of functions which is not safe to use on
anything other than fam17, and a different set of functions which is not
safe to use on anything other than fam19.  Neither are safe to use under
virt, which is an outstanding security vulnerability in the SRSO work.

Given the clustermess that is SRSO, it's not as if the fam1a BTB is
going to be reverted back to look like a fam19 one, so "different
families" isn't going to happen.  The most likely thing to happen is
that you'll have to invent a $FOO_different_alias when a 3rd BTB
structure is shown to have related problems.

I know you may like $FOO and $FOO_alias, but an alias infix on one of a
pair implies they're related when in fact they are not.  It takes a the
already-insanely-complicated logic and makes even harder to follow.

Naming is very important for clarity/understanding, and the current
naming here is doing it's damn hardest to make the logic impossible to
follow, edit, and crucially, fix.

~Andrew