Re: [PATCH v6 1/5] x86/amd_nb: Add support for northbridges on Aldebaran
From: Chatradhi, Naveen Krishna
Date: Mon Nov 08 2021 - 11:54:08 EST
Hi Boris,
On 11/8/2021 7:04 PM, Borislav Petkov wrote:
[CAUTION: External Email]
On Thu, Nov 04, 2021 at 06:48:29PM +0530, Chatradhi, Naveen Krishna wrote:
I know, this is confusion. we will try to give a meaning for definition
here.
Well, not that - you will need to keep adding PCI device IDs for the
future GPUs supporting this stuff.
Creating a pci_device_id list for GPUs will be needed.
How about, defining a new struct
+struct system_topology {
+ const struct pci_device_id *misc_ids;
+ const struct pci_device_id *link_ids;
+ const struct pci_device_id *root_ids;
+ u16 roots_per_misc;
+ u16 misc_count;
+ u16 root_count;
+};
Well, how does having a single struct help make things easier?
Northbridges on CPUs and GPUs can be described using the elements in the
above structure.
IOW, if you use accessors to get the information you need, it doesn't
really matter what the underlying organization of the data is. And if
it helps to keep 'em separate because stuff is simple altogether, then,
they should be separate.
I thought organizing the data in a structure would simplify the
initialization of cpus and gpus.
So, before you ask "How about", think of answering the question "Why
should it be done this way? What are the advantages?"
I will modify the patch to enumerate gpu northbridge info only if there are
gpu nodes with pci_device to access the node_map registers.
This way, creating appropriate number MCs under EDAC and existing exported
APIs can remain the same.
Why does that matter?
Also, have you verified in what order the init_amd_nbs() fs initcall and
amd64_edac_init() get executed?
I'm going to venture a pretty sure guess that the initcall runs first
and that amd_cache_northbridges() call in amd64_edac_init() is probably
not even needed anymore...
Yes, fs_initcall come before module_init (which inturn is device_init
level 6). We can replace the amd_cache_northbridges() call in
amd64_edac_init() with if(amd_nb_num() < 0).
Thx.
--
Regards/Gruss,
Boris.
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpeople.kernel.org%2Ftglx%2Fnotes-about-netiquette&data=04%7C01%7CNaveenKrishna.Chatradhi%40amd.com%7C45b6d75b206849ab022808d9a2bc7d4f%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637719752712242190%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=WW26EMOnu%2FBazap9njmnQIvY9hVSZxVpNol4uptGcec%3D&reserved=0