Messages in this thread | | | Date | Mon, 8 Nov 2021 22:23:49 +0530 | Subject | Re: [PATCH v6 1/5] x86/amd_nb: Add support for northbridges on Aldebaran | From | "Chatradhi, Naveen Krishna" <> |
| |
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
| |