Re: [PATCH v3 1/3] x86/amd_nb: Add support for northbridges on Aldebaran

From: Borislav Petkov <bp@alien8.de>
To: Yazen Ghannam <yazen.ghannam@amd.com>
Cc: Naveen Krishna Chatradhi <nchatrad@amd.com>,
	linux-edac@vger.kernel.org, x86@kernel.org,
	linux-kernel@vger.kernel.org, mchehab@kernel.org,
	Muralidhara M K <muralimk@amd.com>
Subject: Re: [PATCH v3 1/3] x86/amd_nb: Add support for northbridges on Aldebaran
Date: Thu, 16 Sep 2021 18:36:29 +0200	[thread overview]
Message-ID: <YUNyjSfTxIT7VY2H@zn.tnic> (raw)
In-Reply-To: <YT+TYrOZpe1IEv28@yaz-ubuntu>

On Mon, Sep 13, 2021 at 06:07:30PM +0000, Yazen Ghannam wrote:
> I really like this idea. I've gone over the current and future code a few
> times to make sure things are okay. As far as I can tell, this idea should
> work most of the time, since the "node_id" value is mostly used to look up the
> right devices in the nb array. But there is one case so far where the "real"
> hardware node_id is needed during address translation.

Yap, I figured as much as this is kinda like the only place where you'd
care about the actual node id.

> This case is in the new code in review for Data Fabric v3.5, and it
> only applies to the GPU devices.
>
> What do you think about having a couple of helper functions to go between the
> hardware and Linux index IDs? Most cases will use "hardware -> Linux index",
> and when needed there can be a "Linux index -> hardware".

That's fine as long as it is properly documented what it does.

> I think we still need some piece of info to indicate a device is a GPU based
> on its node_id. The AMD NB code doesn't need to know, but the address
> translation code does. The AMD NB enumeration can be mostly generic. I think
> it may be enough to save an "id offset" value and also a "first special index"
> value. Then we can go back and forth between the appropriate values without
> having to allocate a bunch of unused memory or hardcoding certain values.

Well, since we're going to need this in the translation logic and that
is part of amd64_edac and there we said we'll move the family type up
into amd64_pvt so that you can have a family descriptor per node, then I
guess you're all set. :-)

> Thanks for the idea!

Sure, np.

Thx.

-- 
Regards/Gruss,
    Boris.

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