Linux PCI subsystem development
 help / color / mirror / Atom feed
From: Yazen Ghannam <yazen.ghannam@amd.com>
To: Lin Wang <wanglin@open-hieco.net>
Cc: Borislav Petkov <bp@alien8.de>,
	mario.limonciello@amd.com, tglx@linutronix.de, mingo@redhat.com,
	dave.hansen@linux.intel.com, "H. Peter Anvin" <hpa@zytor.com>,
	bhelgaas@google.com, x86@kernel.org, linux-pci@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 4/5] x86/amd_node: Skip SMN setup on Hygon Fam18h models 0x04-0x08
Date: Thu, 3 Sep 2026 13:34:44 -0400	[thread overview]
Message-ID: <20260903173444.GA27025@yaz-khff2.amd.com> (raw)
In-Reply-To: <20724518-2bf0-40ac-a5a9-1b2fe9df2b0c@open-hieco.net>

On Thu, Sep 03, 2026 at 09:49:58AM +0800, Lin Wang wrote:
> 
> 
> On 9/2/2026 11:13 PM, Borislav Petkov wrote:
> > On Wed, Sep 02, 2026 at 02:22:13PM +0800, Lin Wang wrote:
> > > Please let me know how you would prefer this to be handled.
> > 
> > Well, X86_FEATURE_ZEN isn't Hygon apparently. Otherwise, we wouldn't be having
> > this conversation. And we wouldn't need ugly workarounds like that.
> > 
> > Which means, you cannot set it on Hygon. Instead, you should check vendor or
> > define your own X86_FEATURE_HYGON if you think it would be useful to denote
> > common set of features between all Hygon cores.
> > 
> > Reusing an AMD-specific flag but then having different hardware which needs
> > hacks and workarounds to the code is not what you do. Hygon is a different
> > vendor and it should be treated as such - not piggyback on AMD code.
> > 
> > Unless you wanna piggyback. But then you have to extract it properly and make
> > it generic so that it can be shared. I have been asking all people submitting
> > Hygon code to do that and you are starting to do it but then you need to stick
> > to it and do only that.
> > 
> 
> Hi Boris,
> 
> Thanks for clarifying the direction.
> 
> 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?

> 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?

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.

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

  reply	other threads:[~2026-09-03 17:34 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-01  7:16 [PATCH v4 0/5] x86/hygon: Add Family 0x18 DF node enumeration and SMN access Lin Wang
2026-09-01  7:16 ` [PATCH v4 1/5] pci_ids: Add Hygon Family 0x18 DF F3/F4 device IDs Lin Wang
2026-09-01  7:34   ` sashiko-bot
2026-09-03 22:10   ` Bjorn Helgaas
2026-09-04  3:18     ` Lin Wang
2026-09-01  7:16 ` [PATCH v4 2/5] x86/hygon: Add Family 0x18 DF node enumeration Lin Wang
2026-09-01  7:29   ` sashiko-bot
2026-09-01  7:16 ` [PATCH v4 3/5] x86/hygon: Map CPU NodeIds to DF nodes Lin Wang
2026-09-01  7:32   ` sashiko-bot
2026-09-01  7:16 ` [PATCH v4 4/5] x86/amd_node: Skip SMN setup on Hygon Fam18h models 0x04-0x08 Lin Wang
2026-09-01  7:34   ` sashiko-bot
2026-09-02  2:48   ` Borislav Petkov
2026-09-02  6:22     ` Lin Wang
2026-09-02 15:13       ` Borislav Petkov
2026-09-03  1:49         ` Lin Wang
2026-09-03 17:34           ` Yazen Ghannam [this message]
2026-09-04  2:43             ` Lin Wang
2026-09-04  5:36           ` Borislav Petkov
2026-09-04  6:26             ` Lin Wang
2026-09-01  7:17 ` [PATCH v4 5/5] x86/hygon: Add Family 0x18 SMN access Lin Wang
2026-09-01  7:28   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260903173444.GA27025@yaz-khff2.amd.com \
    --to=yazen.ghannam@amd.com \
    --cc=bhelgaas@google.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=mario.limonciello@amd.com \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=wanglin@open-hieco.net \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox