Re: [PATCH v3 1/3] RAS: Introduce AMD Address Translation Library

[Yazen Ghannam]

On 12/12/2023 10:34 AM, Borislav Petkov wrote:
> On Tue, Dec 12, 2023 at 09:23:44AM -0500, Yazen Ghannam wrote:
> > I'm thinking that the warning only happens if the "assert" condition above
> > is hit.
>
> assert usually means "assert - abort the program if assertion is false"
> - from assert(3).

Right, agreed. In this context, the program is the translation method. 
But yeah, it doesn't make much sense describing the kernel. I'll change 
the wording if I don't drop the macros completely.

> > In older revisions, I had all these messages as "debug" loglevel. I don't
> > think there's anything a user can do to fix these issues. They're either
> > coding bugs in the library or system configuration.
> >
> > I'd rather go back to the debug messages if you don't mind. It's not
> > difficult to enable dynamic debug messages compared to DEBUG Kconfig
> > options. So I think it'd be okay to work with users on this if they
> > encounter an issue.
>
> Makes sense.
>
> > > > +static const struct x86_cpu_id amd_atl_cpuids[] = {
> > > > +	X86_MATCH_FEATURE(X86_FEATURE_SMCA, NULL),
> > >
> > > I'd expect for only this one to be needed, but not those below.
> >
> > Me too. Those below are to workaround a current module loading issue. I'll
> > add a code comment for that.
>
> You mean the systemdoofus crap?
>
> Fget it - we don't fix the kernel because luserspace is nuts.
>
> > > > +	X86_MATCH_FEATURE(X86_FEATURE_ZEN, NULL),
>
> ...and those are influx - this is called X86_FEATURE_ZEN1 now and
> X86_FEATURE_ZEN is set on all Zens. So you might as well match on
> X86_FEATURE_ZEN only.
>
> But you should not need it - if SMCA doesn't match then we have another
> problem. ATL should load on SMCA systems only.

I agree in principle. But I don't think it hurts to include an 
additional line to avoid the confusion when the module doesn't load.

Also, the SMCA feature is used here as a short-cut to match on systems 
with a Data Fabric. We could use the Zen feature in the same way.

Thanks,
Yazen