| Message ID | 20220203174942.31630-3-nchatrad@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/edac/amd64: Add support for GPU nodes | expand |
On Thu, Feb 03, 2022 at 11:49:32AM -0600, Naveen Krishna Chatradhi wrote: > From: Muralidhara M K <muralimk@amd.com> > > On newer systems the CPUs manage MCA errors reported from the GPUs. > Enumerate the GPU nodes with the AMD NB framework to support EDAC. > > GPU nodes are enumerated in sequential order based on the PCI hierarchy, > and the first GPU node is assumed to have an "AMD Node ID" value after > CPU Nodes are fully populated. > > Aldebaran is an AMD GPU, GPU drivers are part of the DRM framework > https://lists.freedesktop.org/archives/amd-gfx/2021-February/059694.html > > Each Aldebaran GPU has 2 Data Fabrics, which are enumerated as 2 nodes. > With this implementation detail, the Data Fabric on the GPU nodes can be > accessed the same way as the Data Fabric on CPU nodes. > > Handled new device support and enumeration by calling separate function s/Handled/handle/ > in init_amd_nbs with completely separate data structures. s/init_amd_nbs/init_amd_nbs()/ Function names should have "()" to clearly show they are functions. > > Signed-off-by: Muralidhara M K <muralimk@amd.com> > Co-developed-by: Naveen Krishna Chatradhi <nchatrad@amd.com> > Signed-off-by: Naveen Krishna Chatradhi <nchatrad@amd.com> > --- > Link: > https://lkml.kernel.org/r/20211028130106.15701-2-nchatrad@amd.com > > v6->v7: > * API amd_gpu_node_start_id() introduced to reuse in future patches. > > v5->v6: > * Defined seperate structure for GPU NBs and handled GPu enumearation > seperately by defining new function amd_cache_gpu_devices > * Defined amd_get_gpu_node_id which will be used by mce module > > v4->v5: > * Modified amd_get_node_map() and checking return value > > v3->v4: > * renamed struct name from nmap to nodemap > * split amd_get_node_map and addressed minor comments > > v2->v3: > * Use word "gpu" instead of "noncpu" in the patch > * Do not create pci_dev_ids arrays for gpu nodes > * Identify the gpu node start index from DF18F1 registers on the GPU nodes. > Export cpu node count and gpu start node id > > v1->v2: > * Added Reviewed-by Yazen Ghannam > > v0->v1 > * Modified the commit message and comments in the code > * Squashed patch 1/7: "x86/amd_nb: Add Aldebaran device to PCI IDs" > > > arch/x86/include/asm/amd_nb.h | 9 ++ > arch/x86/kernel/amd_nb.c | 149 +++++++++++++++++++++++++++++++++- > include/linux/pci_ids.h | 1 + > 3 files changed, 155 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/include/asm/amd_nb.h b/arch/x86/include/asm/amd_nb.h > index 00d1a400b7a1..cb8bc59d17a0 100644 > --- a/arch/x86/include/asm/amd_nb.h > +++ b/arch/x86/include/asm/amd_nb.h > @@ -73,6 +73,12 @@ struct amd_northbridge_info { > struct amd_northbridge *nb; > }; > > +struct amd_gpu_nb_info { > + u16 gpu_num; > + struct amd_northbridge *gpu_nb; > + u16 gpu_node_start_id; The members should be aligned with tabs. Is the "gpu" prefix needed for the members since the struct type indicates this? Also, I think grouping members with similar types and leaving pointers to the end may help avoid padding, etc. > +}; > + > #define AMD_NB_GART BIT(0) > #define AMD_NB_L3_INDEX_DISABLE BIT(1) > #define AMD_NB_L3_PARTITIONING BIT(2) > @@ -80,8 +86,11 @@ struct amd_northbridge_info { > #ifdef CONFIG_AMD_NB > > u16 amd_nb_num(void); > +u16 amd_gpu_nb_num(void); > +u16 amd_gpu_node_start_id(void); > bool amd_nb_has_feature(unsigned int feature); > struct amd_northbridge *node_to_amd_nb(int node); > +int amd_get_gpu_node_system_id(u64 ipid); > > static inline u16 amd_pci_dev_to_node_id(struct pci_dev *pdev) > { > diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c > index 020c906f7934..dfa7c7516321 100644 > --- a/arch/x86/kernel/amd_nb.c > +++ b/arch/x86/kernel/amd_nb.c > @@ -20,6 +20,7 @@ > #define PCI_DEVICE_ID_AMD_17H_M30H_ROOT 0x1480 > #define PCI_DEVICE_ID_AMD_17H_M60H_ROOT 0x1630 > #define PCI_DEVICE_ID_AMD_19H_M10H_ROOT 0x14a4 > +#define PCI_DEVICE_ID_AMD_ALDEBARAN_ROOT 0x14bb > #define PCI_DEVICE_ID_AMD_17H_DF_F4 0x1464 > #define PCI_DEVICE_ID_AMD_17H_M10H_DF_F4 0x15ec > #define PCI_DEVICE_ID_AMD_17H_M30H_DF_F4 0x1494 > @@ -30,6 +31,14 @@ > #define PCI_DEVICE_ID_AMD_19H_M40H_ROOT 0x14b5 > #define PCI_DEVICE_ID_AMD_19H_M40H_DF_F4 0x167d > #define PCI_DEVICE_ID_AMD_19H_M50H_DF_F4 0x166e > +#define PCI_DEVICE_ID_AMD_ALDEBARAN_DF_F4 0x14d4 > + > +/* GPU Data Fabric ID Device 24 Function 1 */ > +#define PCI_DEVICE_ID_AMD_ALDEBARAN_DF_F1 0x14d1 > + > +/* DF18xF1 register on Aldebaran GPU */ > +#define REG_LOCAL_NODE_TYPE_MAP 0x144 > + > > /* Protect the PCI config register pairs used for SMN. */ > static DEFINE_MUTEX(smn_mutex); > @@ -104,6 +113,21 @@ static const struct pci_device_id hygon_nb_link_ids[] = { > {} > }; > > +static const struct pci_device_id amd_gpu_root_ids[] = { > + { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_ALDEBARAN_ROOT) }, > + {} > +}; > + > +static const struct pci_device_id amd_gpu_misc_ids[] = { > + { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_ALDEBARAN_DF_F3) }, > + {} > +}; > + > +static const struct pci_device_id amd_gpu_link_ids[] = { > + { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_ALDEBARAN_DF_F4) }, > + {} > +}; > + > const struct amd_nb_bus_dev_range amd_nb_bus_dev_ranges[] __initconst = { > { 0x00, 0x18, 0x20 }, > { 0xff, 0x00, 0x20 }, > @@ -112,6 +136,8 @@ const struct amd_nb_bus_dev_range amd_nb_bus_dev_ranges[] __initconst = { > }; > > static struct amd_northbridge_info amd_northbridges; > +/* GPU specific structure declaration */ > +static struct amd_gpu_nb_info amd_gpu_nbs; > > u16 amd_nb_num(void) > { > @@ -119,6 +145,20 @@ u16 amd_nb_num(void) > } > EXPORT_SYMBOL_GPL(amd_nb_num); > > +/* Total number of GPU nbs in a system */ > +u16 amd_gpu_nb_num(void) > +{ > + return amd_gpu_nbs.gpu_num; > +} > +EXPORT_SYMBOL_GPL(amd_gpu_nb_num); > + > +/* Start index of hardware provided GPU node ID */ > +u16 amd_gpu_node_start_id(void) > +{ > + return amd_gpu_nbs.gpu_node_start_id; > +} > +EXPORT_SYMBOL_GPL(amd_gpu_node_start_id); > + > bool amd_nb_has_feature(unsigned int feature) > { > return ((amd_northbridges.flags & feature) == feature); > @@ -127,10 +167,60 @@ EXPORT_SYMBOL_GPL(amd_nb_has_feature); > > struct amd_northbridge *node_to_amd_nb(int node) > { > - return (node < amd_northbridges.num) ? &amd_northbridges.nb[node] : NULL; > + if (node < amd_northbridges.num) > + return &amd_northbridges.nb[node]; > + else if (node >= amd_northbridges.num && > + node < amd_gpu_nbs.gpu_num + amd_northbridges.num) > + return &amd_gpu_nbs.gpu_nb[node - amd_northbridges.num]; > + else > + return NULL; > } > EXPORT_SYMBOL_GPL(node_to_amd_nb); > > +/* > + * On SMCA banks of the GPU nodes, the hardware node id information is > + * available in register MCA_IPID[47:44](InstanceIdHi). > + * > + * Convert the hardware node ID to a value used by linux where > + * GPU nodes are sequentially after the CPU nodes. > + */ > +int amd_get_gpu_node_system_id(u64 ipid) > +{ > + return ((ipid >> 44 & 0xF) - amd_gpu_node_start_id() > + + amd_northbridges.num); > +} > +EXPORT_SYMBOL_GPL(amd_get_gpu_node_system_id); Why does this function need to be in amd_nb.c? It seems like it's only needed in EDAC. And the necessary info is already being exported. > + > +/* > + * AMD CPUs and GPUs whose data fabrics can be connected via custom xGMI > + * links, come with registers to gather local and remote node type map info. > + * > + * This function, reads the register in DF function 1 from "Local Node Type" > + * which refers to GPU node. > + */ > +static int amd_gpu_df_nodemap(void) The purpose of this function is to find the "gpu_node_start_id", so I think the function name should reflect that. > +{ > + struct pci_dev *pdev; > + u32 tmp; > + > + pdev = pci_get_device(PCI_VENDOR_ID_AMD, > + PCI_DEVICE_ID_AMD_ALDEBARAN_DF_F1, NULL); > + if (!pdev) { > + pr_debug("DF Func1 PCI device not found on this node.\n"); > + return -ENODEV; > + } > + > + if (pci_read_config_dword(pdev, REG_LOCAL_NODE_TYPE_MAP, &tmp)) > + goto out; > + amd_gpu_nbs.gpu_node_start_id = tmp & 0xFFF; > + > + return 0; > +out: > + pr_warn("PCI config access failed\n"); > + pci_dev_put(pdev); Shouldn't this "put" also happen on the success path? > + return -ENODEV; > +} > + > static struct pci_dev *next_northbridge(struct pci_dev *dev, > const struct pci_device_id *ids) > { > @@ -147,7 +237,7 @@ static int __amd_smn_rw(u16 node, u32 address, u32 *value, bool write) > struct pci_dev *root; > int err = -ENODEV; > > - if (node >= amd_northbridges.num) > + if (node >= amd_northbridges.num + amd_gpu_nbs.gpu_num) > goto out; > > root = node_to_amd_nb(node)->root; > @@ -210,14 +300,14 @@ int amd_cache_northbridges(void) > } > > misc = NULL; > - while ((misc = next_northbridge(misc, misc_ids)) != NULL) > + while ((misc = next_northbridge(misc, misc_ids))) > misc_count++; > > if (!misc_count) > return -ENODEV; > > root = NULL; > - while ((root = next_northbridge(root, root_ids)) != NULL) > + while ((root = next_northbridge(root, root_ids))) I think these last two changes aren't necessary for this feature. > root_count++; > > if (root_count) { > @@ -292,6 +382,56 @@ int amd_cache_northbridges(void) > } > EXPORT_SYMBOL_GPL(amd_cache_northbridges); > > +int amd_cache_gpu_devices(void) > +{ > + const struct pci_device_id *misc_ids = amd_gpu_misc_ids; > + const struct pci_device_id *link_ids = amd_gpu_link_ids; > + const struct pci_device_id *root_ids = amd_gpu_root_ids; > + struct pci_dev *root, *misc, *link; > + struct amd_northbridge *gpu_nb; > + u16 misc_count = 0; > + u16 root_count = 0; > + int ret; > + u16 i; > + > + if (amd_gpu_nbs.gpu_num) > + return 0; > + > + ret = amd_gpu_df_nodemap(); > + if (ret) > + return ret; > + > + misc = NULL; > + while ((misc = next_northbridge(misc, misc_ids))) > + misc_count++; > + > + if (!misc_count) > + return -ENODEV; > + > + root = NULL; > + while ((root = next_northbridge(root, root_ids))) > + root_count++; > + > + gpu_nb = kcalloc(misc_count, sizeof(struct amd_northbridge), GFP_KERNEL); > + if (!gpu_nb) > + return -ENOMEM; > + > + amd_gpu_nbs.gpu_nb = gpu_nb; > + amd_gpu_nbs.gpu_num = misc_count; > + > + link = NULL, misc = NULL, root = NULL; > + for (i = amd_northbridges.num; i < (amd_northbridges.num + amd_gpu_nbs.gpu_num); i++) { > + node_to_amd_nb(i)->root = root = > + next_northbridge(root, root_ids); > + node_to_amd_nb(i)->misc = misc = > + next_northbridge(misc, misc_ids); > + node_to_amd_nb(i)->link = link = > + next_northbridge(link, link_ids); > + } > + return 0; > +} > +EXPORT_SYMBOL_GPL(amd_cache_gpu_devices); I don't think this needs to be exported since it's called in init_amd_nbs() below. A module can use amd_nb_num() to see if anything was initialized rather than calling amd_cache_*() again. The existing callers of amd_cache_*() functions need to be cleaned up. > + > /* > * Ignores subdevice/subvendor but as far as I can figure out > * they're useless anyways > @@ -497,6 +637,7 @@ static __init int init_amd_nbs(void) > { > amd_cache_northbridges(); > amd_cache_gart(); > + amd_cache_gpu_devices(); > > fix_erratum_688(); > ... Thanks, Yazen
diff --git a/arch/x86/include/asm/amd_nb.h b/arch/x86/include/asm/amd_nb.h index 00d1a400b7a1..cb8bc59d17a0 100644 --- a/arch/x86/include/asm/amd_nb.h +++ b/arch/x86/include/asm/amd_nb.h @@ -73,6 +73,12 @@ struct amd_northbridge_info { struct amd_northbridge *nb; }; +struct amd_gpu_nb_info { + u16 gpu_num; + struct amd_northbridge *gpu_nb; + u16 gpu_node_start_id; +}; + #define AMD_NB_GART BIT(0) #define AMD_NB_L3_INDEX_DISABLE BIT(1) #define AMD_NB_L3_PARTITIONING BIT(2) @@ -80,8 +86,11 @@ struct amd_northbridge_info { #ifdef CONFIG_AMD_NB u16 amd_nb_num(void); +u16 amd_gpu_nb_num(void); +u16 amd_gpu_node_start_id(void); bool amd_nb_has_feature(unsigned int feature); struct amd_northbridge *node_to_amd_nb(int node); +int amd_get_gpu_node_system_id(u64 ipid); static inline u16 amd_pci_dev_to_node_id(struct pci_dev *pdev) { diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c index 020c906f7934..dfa7c7516321 100644 --- a/arch/x86/kernel/amd_nb.c +++ b/arch/x86/kernel/amd_nb.c @@ -20,6 +20,7 @@ #define PCI_DEVICE_ID_AMD_17H_M30H_ROOT 0x1480 #define PCI_DEVICE_ID_AMD_17H_M60H_ROOT 0x1630 #define PCI_DEVICE_ID_AMD_19H_M10H_ROOT 0x14a4 +#define PCI_DEVICE_ID_AMD_ALDEBARAN_ROOT 0x14bb #define PCI_DEVICE_ID_AMD_17H_DF_F4 0x1464 #define PCI_DEVICE_ID_AMD_17H_M10H_DF_F4 0x15ec #define PCI_DEVICE_ID_AMD_17H_M30H_DF_F4 0x1494 @@ -30,6 +31,14 @@ #define PCI_DEVICE_ID_AMD_19H_M40H_ROOT 0x14b5 #define PCI_DEVICE_ID_AMD_19H_M40H_DF_F4 0x167d #define PCI_DEVICE_ID_AMD_19H_M50H_DF_F4 0x166e +#define PCI_DEVICE_ID_AMD_ALDEBARAN_DF_F4 0x14d4 + +/* GPU Data Fabric ID Device 24 Function 1 */ +#define PCI_DEVICE_ID_AMD_ALDEBARAN_DF_F1 0x14d1 + +/* DF18xF1 register on Aldebaran GPU */ +#define REG_LOCAL_NODE_TYPE_MAP 0x144 + /* Protect the PCI config register pairs used for SMN. */ static DEFINE_MUTEX(smn_mutex); @@ -104,6 +113,21 @@ static const struct pci_device_id hygon_nb_link_ids[] = { {} }; +static const struct pci_device_id amd_gpu_root_ids[] = { + { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_ALDEBARAN_ROOT) }, + {} +}; + +static const struct pci_device_id amd_gpu_misc_ids[] = { + { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_ALDEBARAN_DF_F3) }, + {} +}; + +static const struct pci_device_id amd_gpu_link_ids[] = { + { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_ALDEBARAN_DF_F4) }, + {} +}; + const struct amd_nb_bus_dev_range amd_nb_bus_dev_ranges[] __initconst = { { 0x00, 0x18, 0x20 }, { 0xff, 0x00, 0x20 }, @@ -112,6 +136,8 @@ const struct amd_nb_bus_dev_range amd_nb_bus_dev_ranges[] __initconst = { }; static struct amd_northbridge_info amd_northbridges; +/* GPU specific structure declaration */ +static struct amd_gpu_nb_info amd_gpu_nbs; u16 amd_nb_num(void) { @@ -119,6 +145,20 @@ u16 amd_nb_num(void) } EXPORT_SYMBOL_GPL(amd_nb_num); +/* Total number of GPU nbs in a system */ +u16 amd_gpu_nb_num(void) +{ + return amd_gpu_nbs.gpu_num; +} +EXPORT_SYMBOL_GPL(amd_gpu_nb_num); + +/* Start index of hardware provided GPU node ID */ +u16 amd_gpu_node_start_id(void) +{ + return amd_gpu_nbs.gpu_node_start_id; +} +EXPORT_SYMBOL_GPL(amd_gpu_node_start_id); + bool amd_nb_has_feature(unsigned int feature) { return ((amd_northbridges.flags & feature) == feature); @@ -127,10 +167,60 @@ EXPORT_SYMBOL_GPL(amd_nb_has_feature); struct amd_northbridge *node_to_amd_nb(int node) { - return (node < amd_northbridges.num) ? &amd_northbridges.nb[node] : NULL; + if (node < amd_northbridges.num) + return &amd_northbridges.nb[node]; + else if (node >= amd_northbridges.num && + node < amd_gpu_nbs.gpu_num + amd_northbridges.num) + return &amd_gpu_nbs.gpu_nb[node - amd_northbridges.num]; + else + return NULL; } EXPORT_SYMBOL_GPL(node_to_amd_nb); +/* + * On SMCA banks of the GPU nodes, the hardware node id information is + * available in register MCA_IPID[47:44](InstanceIdHi). + * + * Convert the hardware node ID to a value used by linux where + * GPU nodes are sequentially after the CPU nodes. + */ +int amd_get_gpu_node_system_id(u64 ipid) +{ + return ((ipid >> 44 & 0xF) - amd_gpu_node_start_id() + + amd_northbridges.num); +} +EXPORT_SYMBOL_GPL(amd_get_gpu_node_system_id); + +/* + * AMD CPUs and GPUs whose data fabrics can be connected via custom xGMI + * links, come with registers to gather local and remote node type map info. + * + * This function, reads the register in DF function 1 from "Local Node Type" + * which refers to GPU node. + */ +static int amd_gpu_df_nodemap(void) +{ + struct pci_dev *pdev; + u32 tmp; + + pdev = pci_get_device(PCI_VENDOR_ID_AMD, + PCI_DEVICE_ID_AMD_ALDEBARAN_DF_F1, NULL); + if (!pdev) { + pr_debug("DF Func1 PCI device not found on this node.\n"); + return -ENODEV; + } + + if (pci_read_config_dword(pdev, REG_LOCAL_NODE_TYPE_MAP, &tmp)) + goto out; + amd_gpu_nbs.gpu_node_start_id = tmp & 0xFFF; + + return 0; +out: + pr_warn("PCI config access failed\n"); + pci_dev_put(pdev); + return -ENODEV; +} + static struct pci_dev *next_northbridge(struct pci_dev *dev, const struct pci_device_id *ids) { @@ -147,7 +237,7 @@ static int __amd_smn_rw(u16 node, u32 address, u32 *value, bool write) struct pci_dev *root; int err = -ENODEV; - if (node >= amd_northbridges.num) + if (node >= amd_northbridges.num + amd_gpu_nbs.gpu_num) goto out; root = node_to_amd_nb(node)->root; @@ -210,14 +300,14 @@ int amd_cache_northbridges(void) } misc = NULL; - while ((misc = next_northbridge(misc, misc_ids)) != NULL) + while ((misc = next_northbridge(misc, misc_ids))) misc_count++; if (!misc_count) return -ENODEV; root = NULL; - while ((root = next_northbridge(root, root_ids)) != NULL) + while ((root = next_northbridge(root, root_ids))) root_count++; if (root_count) { @@ -292,6 +382,56 @@ int amd_cache_northbridges(void) } EXPORT_SYMBOL_GPL(amd_cache_northbridges); +int amd_cache_gpu_devices(void) +{ + const struct pci_device_id *misc_ids = amd_gpu_misc_ids; + const struct pci_device_id *link_ids = amd_gpu_link_ids; + const struct pci_device_id *root_ids = amd_gpu_root_ids; + struct pci_dev *root, *misc, *link; + struct amd_northbridge *gpu_nb; + u16 misc_count = 0; + u16 root_count = 0; + int ret; + u16 i; + + if (amd_gpu_nbs.gpu_num) + return 0; + + ret = amd_gpu_df_nodemap(); + if (ret) + return ret; + + misc = NULL; + while ((misc = next_northbridge(misc, misc_ids))) + misc_count++; + + if (!misc_count) + return -ENODEV; + + root = NULL; + while ((root = next_northbridge(root, root_ids))) + root_count++; + + gpu_nb = kcalloc(misc_count, sizeof(struct amd_northbridge), GFP_KERNEL); + if (!gpu_nb) + return -ENOMEM; + + amd_gpu_nbs.gpu_nb = gpu_nb; + amd_gpu_nbs.gpu_num = misc_count; + + link = NULL, misc = NULL, root = NULL; + for (i = amd_northbridges.num; i < (amd_northbridges.num + amd_gpu_nbs.gpu_num); i++) { + node_to_amd_nb(i)->root = root = + next_northbridge(root, root_ids); + node_to_amd_nb(i)->misc = misc = + next_northbridge(misc, misc_ids); + node_to_amd_nb(i)->link = link = + next_northbridge(link, link_ids); + } + return 0; +} +EXPORT_SYMBOL_GPL(amd_cache_gpu_devices); + /* * Ignores subdevice/subvendor but as far as I can figure out * they're useless anyways @@ -497,6 +637,7 @@ static __init int init_amd_nbs(void) { amd_cache_northbridges(); amd_cache_gart(); + amd_cache_gpu_devices(); fix_erratum_688(); diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h index aad54c666407..27fad5e1bf80 100644 --- a/include/linux/pci_ids.h +++ b/include/linux/pci_ids.h @@ -558,6 +558,7 @@ #define PCI_DEVICE_ID_AMD_19H_M10H_DF_F3 0x14b0 #define PCI_DEVICE_ID_AMD_19H_M40H_DF_F3 0x167c #define PCI_DEVICE_ID_AMD_19H_M50H_DF_F3 0x166d +#define PCI_DEVICE_ID_AMD_ALDEBARAN_DF_F3 0x14d3 #define PCI_DEVICE_ID_AMD_CNB17H_F3 0x1703 #define PCI_DEVICE_ID_AMD_LANCE 0x2000 #define PCI_DEVICE_ID_AMD_LANCE_HOME 0x2001