Re: [PATCH v4 4/5] x86/amd_node: Skip SMN setup on Hygon Fam18h models 0x04-0x08

[Lin Wang]

Hi Yazen,

Thanks for your feedback!

On 9/4/2026 1:34 AM, Yazen Ghannam wrote:
> On Thu, Sep 03, 2026 at 09:49:58AM +0800, Lin Wang wrote:
> > My proposed approach is to use explicit vendor checks to make
> > init_amd_nbs() and amd_smn_init() AMD-only. Hygon node enumeration and
> > SMN setup would remain in Hygon code.
> >
> > Sashiko's concern with v3 was that the AMD initializers were restricted
> > while Hygon call sites remained. However, as currently implemented, the
> > AMD NB and SMN paths are based on AMD topology assumptions and cannot
> > provide the Hygon node and SMN mappings.
>
> The "vendor != HYGON" restriction would be fair.
>
> I don't understand how the "model-specific" restriction is better.
> Wouldn't it cause similar problems?

It was intended only to limit the scope of the change and address the
Sashiko finding, but it does not provide the AMD SMN code with a valid
Hygon node-to-root mapping. I will drop the model-specific check.

> > I would first stop the affected Hygon call sites from entering those AMD
> > paths, and then make the two initializers AMD-only in the same series.
> > Later driver support would use the Hygon interfaces.
> >
> > Would this ordering address the Sashiko concern?
>
> Right, so first you provide a Hygon-only option for all affected call
> sites. Then you add the vendor check. This ensures that the affected
> callers don't break in the middle of the patch series.
>
> Is this your intent?

Not quite. By stopping the affected call sites, I meant removing the
Hygon match entries from the drivers which currently route them through
the AMD NB or SMN mapping. I did not mean adding complete Hygon EDAC,
ATL and k10temp support in this series. These paths do not provide
usable Hygon support with the current AMD node mapping.

Those implementations depend on the Hygon node and SMN interfaces. The
intended order is:

  1. add the Hygon node interfaces;
  2. stop the affected Hygon drivers from entering the AMD paths;
  3. make AMD NB and SMN initialization AMD-only;
  4. add the Hygon SMN implementation.

EDAC, ATL and temperature monitoring support can then be submitted
separately using the Hygon interfaces. This ordering addresses the v3
Sashiko concern without making those drivers a prerequisite for the
node layer.

> Regarding the Zen feature flag, I think it may be overloaded.
>
> Hygon systems are Zen systems. The same Zen1 architecture was used
> orginally, though I don't know how much things have diverged since then.
>
> All Hygon systems are Zen systems, but not all AMD systems are Zen
> systems.
>
> So we have three diverging Platforms:
> 1) AMD legacy (Northbridge / !Zen)
> 2) AMD modern (Zen)
> 3) Hygon (Zen by default)
>
> (2) and (3) where the same until recently.
>
> Maybe we define new feature flags that indicate a Platform/Interconnect
> group? That way we won't need to rely on PCI IDs or vendor+core
> combinations.

I agree that X86_FEATURE_ZEN describes the core architecture and should
not be used to select an interconnect implementation. A separate
interconnect capability may be useful, but defining it and converting
its users would be broader than this node series. For this series, I
would prefer to use explicit vendor checks and keep the interconnect
capability work separate.

That is the ordering I intended.

Thanks & Br;

Lin
> FYI, that was the intent to use the Zen feature flag here. It was a way
> to distinguish (1) and (2/3) above. It worked until it didn't. :)
>
> Thanks,
> Yazen