[PATCH v7 00/12] x86/edac/amd64: Add support for GPU nodes

[PATCH v7 00/12] x86/edac/amd64: Add support for GPU nodes

[Naveen Krishna Chatradhi]

From: Muralidhara M K <muralimk@amd.com>

On heterogeneous systems made up of AMD CPUs and GPUs, where the
data fabrics of CPUs and GPUs are connected directly via custom links.
UMC MCA banks on GPUs can be viewed similar to the UMCs banks on the CPUs.
Hence, memory errors on GPU UMCs can be reported via edac framework.

This patchset applies on top of the following series
[v4,00/24] AMD MCA Address Translation Updates
https://patchwork.kernel.org/project/linux-edac/cover/20220127204115.384161-1-yazen.ghannam@amd.com/

Each patch was build tested individually. The entire set was
tested for address translation and error counts on GPU
memory.

This patchset does the following
1. edac.rst:
   a. Add Documentation support for heterogeneous systems

2. amd_nb.c:
   a. Add support for northbridges on Aldebaran GPU nodes
   b. export AMD node map details to be used by edac and mce modules
	
3. mce_amd module:
   a. Identify the node ID where the error is and map the node id
      to linux enumerated node id.

4. Modifies the amd64_edac module
   a. Refactor the code, define new family op routines and use
      struct amd64_pvt. Making struct fam_type obsolete.
   b. Enumerate UMCs and HBMs on the GPU nodes

5. DF3.5 Address translation support
   a. Support Data Fabric 3.5 Address translation
   b. Fixed UMC to CS mapping for errors


Muralidhara M K (6):
  EDAC/amd64: edac.rst: Add Doc support for heterogeneous systems
  x86/amd_nb: Add support for northbridges on Aldebaran
  EDAC/amd64: Move struct fam_type variables into amd64_pvt structure
  EDAC/amd64: Define dynamic family ops routines
  EDAC/amd64: Add AMD heterogeneous family 19h Model 30h-3fh
  EDAC/amd64: Add address translation support for DF3.5

Naveen Krishna Chatradhi (3):
  EDAC/mce_amd: Extract node id from MCA_IPID
  EDAC/amd64: Enumerate Aldebaran GPU nodes by adding family ops
  EDAC/amd64: Add Family ops to update GPU csrow and channel info

Yazen Ghannam (3):
  EDAC/amd64: Add check for when to add DRAM base and hole
  EDAC/amd64: Save the number of block instances
  EDAC/amd64: Add fixed UMC to CS mapping

 Documentation/driver-api/edac.rst |    9 +
 arch/x86/include/asm/amd_nb.h     |    9 +
 arch/x86/kernel/amd_nb.c          |  149 ++-
 drivers/edac/amd64_edac.c         | 1450 ++++++++++++++++++++---------
 drivers/edac/amd64_edac.h         |  203 +++-
 drivers/edac/mce_amd.c            |   23 +-
 include/linux/pci_ids.h           |    1 +
 7 files changed, 1345 insertions(+), 499 deletions(-)

-- 
2.25.1

[PATCH v7 01/12] EDAC/amd64: Document heterogeneous enumeration

[Naveen Krishna Chatradhi]

From: Muralidhara M K <muralimk@amd.com>

The Documentation notes have been added in amd64_edac.h and will be
referring to driver-api wherever needed.

Explains how the physical topology is enumerated in the software and
edac module populates the sysfs ABIs.

Signed-off-by: Muralidhara M K <muralimk@amd.com>
Signed-off-by: Naveen Krishna Chatradhi <nchatrad@amd.com>
---
v6->v7:
* New in v7

 Documentation/driver-api/edac.rst |   9 +++
 drivers/edac/amd64_edac.h         | 101 ++++++++++++++++++++++++++++++
 2 files changed, 110 insertions(+)

diff --git a/Documentation/driver-api/edac.rst b/Documentation/driver-api/edac.rst
index b8c742aa0a71..0dd07d0d0e47 100644
--- a/Documentation/driver-api/edac.rst
+++ b/Documentation/driver-api/edac.rst
@@ -106,6 +106,15 @@ will occupy those chip-select rows.
 This term is avoided because it is unclear when needing to distinguish
 between chip-select rows and socket sets.
 
+* High Bandwidth Memory (HBM)
+
+HBM is a new type of memory chip with low power consumption and ultra-wide
+communication lanes. It uses vertically stacked memory chips (DRAM dies)
+interconnected by microscopic wires called "through-silicon vias," or TSVs.
+
+Several stacks of HBM chips connect to the CPU or GPU through an ultra-fast
+interconnect called the “interposer". So that HBM’s characteristics are
+nearly indistinguishable from on-chip integrated RAM.
 
 Memory Controllers
 ------------------
diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
index 6f8147abfa71..6a112270a84b 100644
--- a/drivers/edac/amd64_edac.h
+++ b/drivers/edac/amd64_edac.h
@@ -559,3 +559,104 @@ static inline u32 dct_sel_baseaddr(struct amd64_pvt *pvt)
 	}
 	return (pvt)->dct_sel_lo & 0xFFFFF800;
 }
+
+/*
+ * AMD Heterogeneous system support on EDAC subsystem
+ * --------------------------------------------------
+ *
+ * An AMD heterogeneous system built by connecting the data fabrics of both CPUs
+ * and GPUs via custom xGMI links. So, the Data Fabric on the GPU nodes can be
+ * accessed the same way as the Data Fabric on CPU nodes.
+ *
+ * An Aldebaran GPUs has 2 Data Fabrics, each GPU DF contains four Unified
+ * Memory Controllers (UMC). Each UMC contains eight Channels. Each UMC Channel
+ * controls one 128-bit HBM2e (2GB) channel (equivalent to 8 X 2GB ranks),
+ * this creates a total of 4096-bits of DRAM data bus.
+ *
+ * While UMC is interfacing a 16GB (8H X 2GB DRAM) HBM stack, each UMC channel is
+ * interfacing 2GB of DRAM (represented as rank).
+ *
+ * Memory controllers on AMD GPU nodes can be represented in EDAC is as below:
+ *       GPU DF / GPU Node -> EDAC MC
+ *       GPU UMC           -> EDAC CSROW
+ *       GPU UMC channel   -> EDAC CHANNEL
+ *
+ * Eg: An heterogeneous system with 1 AMD CPU is connected to 4 Aldebaran GPUs using xGMI.
+ *
+ * AMD GPU Nodes are enumerated in sequential order based on the PCI hierarchy, and the
+ * first GPU node is assumed to have an "Node ID" value after CPU Nodes are fully
+ * populated.
+ *
+ * $ ls /sys/devices/system/edac/mc/
+ *	mc0   - CPU MC node 0
+ *	mc1  |
+ *	mc2  |- GPU card[0] => node 0(mc1), node 1(mc2)
+ *	mc3  |
+ *	mc4  |- GPU card[1] => node 0(mc3), node 1(mc4)
+ *	mc5  |
+ *	mc6  |- GPU card[2] => node 0(mc5), node 1(mc6)
+ *	mc7  |
+ *	mc8  |- GPU card[3] => node 0(mc7), node 1(mc8)
+ *
+ * sysfs entries will be populated as below:
+ *
+ *	CPU			# CPU node
+ *	├── mc 0
+ *
+ *	GPU Nodes are enumerated sequentially after CPU nodes are populated
+ *	GPU card 1		# Each Aldebaran GPU has 2 nodes/mcs
+ *	├── mc 1		# GPU node 0 == mc1, Each MC node has 4 UMCs/CSROWs
+ *	│   ├── csrow 0		# UMC 0
+ *	│   │   ├── channel 0	# Each UMC has 8 channels
+ *	│   │   ├── channel 1   # size of each channel is 2 GB, so each UMC has 16 GB
+ *	│   │   ├── channel 2
+ *	│   │   ├── channel 3
+ *	│   │   ├── channel 4
+ *	│   │   ├── channel 5
+ *	│   │   ├── channel 6
+ *	│   │   ├── channel 7
+ *	│   ├── csrow 1		# UMC 1
+ *	│   │   ├── channel 0
+ *	│   │   ├── ..
+ *	│   │   ├── channel 7
+ *	│   ├── ..		..
+ *	│   ├── csrow 3		# UMC 3
+ *	│   │   ├── channel 0
+ *	│   │   ├── ..
+ *	│   │   ├── channel 7
+ *	│   ├── rank 0
+ *	│   ├── ..		..
+ *	│   ├── rank 31		# total 32 ranks/dimms from 4 UMCs
+ *	├
+ *	├── mc 2		# GPU node 1 == mc2
+ *	│   ├── ..		# each GPU has total 64 GB
+ *
+ *	GPU card 2
+ *	├── mc 3
+ *	│   ├── ..
+ *	├── mc 4
+ *	│   ├── ..
+ *
+ *	GPU card 3
+ *	├── mc 5
+ *	│   ├── ..
+ *	├── mc 6
+ *	│   ├── ..
+ *
+ *	GPU card 4
+ *	├── mc 7
+ *	│   ├── ..
+ *	├── mc 8
+ *	│   ├── ..
+ *
+ *
+ * Heterogeneous hardware details for above context as below:
+ * - The CPU UMC (Unified Memory Controller) is mostly the same as the GPU UMC.
+ *   They have chip selects (csrows) and channels. However, the layouts are different
+ *   for performance, physical layout, or other reasons.
+ * - CPU UMCs use 1 channel. So we say UMC = EDAC Channel. This follows the
+ *   marketing speak, example. CPU has X memory channels, etc.
+ * - CPU UMCs use up to 4 chip selects. So we say UMC chip select = EDAC CSROW.
+ * - GPU UMCs use 1 chip select. So we say UMC = EDAC CSROW.
+ * - GPU UMCs use 8 channels. So we say UMC Channel = EDAC Channel.
+ */
-- 
2.25.1

[PATCH v7 02/12] x86/amd_nb: Add support for northbridges on Aldebaran

[Naveen Krishna Chatradhi]

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
in init_amd_nbs with completely separate data structures.

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;
+};
+
 #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
-- 
2.25.1

[PATCH v7 03/12] EDAC/mce_amd: Extract node id from MCA_IPID

[Naveen Krishna Chatradhi]

On SMCA banks of the GPU nodes, the 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.

Co-developed-by: Muralidhara M K <muralimk@amd.com>
Signed-off-by: Muralidhara M K <muralimk@amd.com>
Signed-off-by: Naveen Krishna Chatradhi <nchatrad@amd.com>
---
Link:
https://lkml.kernel.org/r/20211028130106.15701-3-nchatrad@amd.com

v6->v7
* None

v5->v6:
* Called amd_get_gpu_node_id function to get node_id

v4->v5:
* None

v3->v4:
* Add reviewed by Yazen

v2->v3:
* Use APIs from amd_nb to identify the gpu_node_start_id and cpu_node_count.
  Which is required to map the hardware node id to node id enumerated by Linux.

v1->v2:
* Modified subject and commit message
* Added Reviewed by Yazen Ghannam

v0->v1:
* Modified the commit message
* Rearranged the conditions before calling decode_dram_ecc()


 drivers/edac/mce_amd.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/edac/mce_amd.c b/drivers/edac/mce_amd.c
index cc5c63feb26a..865a925ccef0 100644
--- a/drivers/edac/mce_amd.c
+++ b/drivers/edac/mce_amd.c
@@ -2,6 +2,7 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 
+#include <asm/amd_nb.h>
 #include <asm/cpu.h>
 
 #include "mce_amd.h"
@@ -1186,8 +1187,26 @@ static void decode_smca_error(struct mce *m)
 	if (xec < smca_mce_descs[bank_type].num_descs)
 		pr_cont(", %s.\n", smca_mce_descs[bank_type].descs[xec]);
 
-	if (bank_type == SMCA_UMC && xec == 0 && decode_dram_ecc)
-		decode_dram_ecc(topology_die_id(m->extcpu), m);
+	if (xec == 0 && decode_dram_ecc) {
+		int node_id = 0;
+
+		if (bank_type == SMCA_UMC) {
+			node_id = topology_die_id(m->extcpu);
+		} else if (bank_type == SMCA_UMC_V2) {
+			/*
+			 * SMCA_UMC_V2 exists on GPU nodes, extract the node id
+			 * from register MCA_IPID[47:44](InstanceIdHi).
+			 * The InstanceIdHi field represents the instance ID of the GPU.
+			 * Which needs to be mapped to a value used by Linux,
+			 * where GPU nodes are simply numerically after the CPU nodes.
+			 */
+			node_id = amd_get_gpu_node_system_id(m->ipid);
+		} else {
+			return;
+		}
+
+		decode_dram_ecc(node_id, m);
+	}
 }
 
 static inline void amd_decode_err_code(u16 ec)
-- 
2.25.1

[PATCH v7 04/12] EDAC/amd64: Move struct fam_type variables into amd64_pvt structure

[Naveen Krishna Chatradhi]

From: Muralidhara M K <muralimk@amd.com>

On heterogeneous systems, the GPU nodes are probed after the CPU
nodes and will overwrites the family type set by CPU nodes.

Removed struct fam_type and made all family-specific assignments
dynamic and get rid of static definitions of family_types and ops,
This would simplify adding support for future platforms.

Signed-off-by: Muralidhara M K <muralimk@amd.com>
---
Link:
https://lkml.kernel.org/r/20211028130106.15701-5-nchatrad@amd.com

v6->v7:
* rebased on top of\
  https://lore.kernel.org/all/20220202144307.2678405-1-yazen.ghannam@amd.com/

v4->v5:
* Added reviewed by Yazen

v1->v4:
* New change in v4


 drivers/edac/amd64_edac.c | 384 +++++++++++++-------------------------
 drivers/edac/amd64_edac.h |  64 +++----
 2 files changed, 153 insertions(+), 295 deletions(-)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 782e286d5390..4cac43840ccc 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -13,11 +13,9 @@ module_param(ecc_enable_override, int, 0644);
 
 static struct msr __percpu *msrs;
 
-static struct amd64_family_type *fam_type;
-
-static inline u32 get_umc_reg(u32 reg)
+static inline u32 get_umc_reg(u32 reg, struct amd64_pvt *pvt)
 {
-	if (!fam_type->flags.zn_regs_v2)
+	if (!pvt->flags.zn_regs_v2)
 		return reg;
 
 	switch (reg) {
@@ -463,7 +461,7 @@ static void get_cs_base_and_mask(struct amd64_pvt *pvt, int csrow, u8 dct,
 	for (i = 0; i < pvt->csels[dct].m_cnt; i++)
 
 #define for_each_umc(i) \
-	for (i = 0; i < fam_type->max_mcs; i++)
+	for (i = 0; i < pvt->max_mcs; i++)
 
 /*
  * @input_addr is an InputAddr associated with the node given by mci. Return the
@@ -1859,7 +1857,7 @@ static void __dump_misc_regs_df(struct amd64_pvt *pvt)
 
 		if (umc->dram_type == MEM_LRDDR4 || umc->dram_type == MEM_LRDDR5) {
 			amd_smn_read(pvt->mc_node_id,
-				     umc_base + get_umc_reg(UMCCH_ADDR_CFG),
+				     umc_base + get_umc_reg(UMCCH_ADDR_CFG, pvt),
 				     &tmp);
 			edac_dbg(1, "UMC%d LRDIMM %dx rank multiply\n",
 					i, 1 << ((tmp >> 4) & 0x3));
@@ -1935,7 +1933,7 @@ static void prep_chip_selects(struct amd64_pvt *pvt)
 
 		for_each_umc(umc) {
 			pvt->csels[umc].b_cnt = 4;
-			pvt->csels[umc].m_cnt = fam_type->flags.zn_regs_v2 ? 4 : 2;
+			pvt->csels[umc].m_cnt = pvt->flags.zn_regs_v2 ? 4 : 2;
 		}
 
 	} else {
@@ -1975,7 +1973,7 @@ static void read_umc_base_mask(struct amd64_pvt *pvt)
 		}
 
 		umc_mask_reg = get_umc_base(umc) + UMCCH_ADDR_MASK;
-		umc_mask_reg_sec = get_umc_base(umc) + get_umc_reg(UMCCH_ADDR_MASK_SEC);
+		umc_mask_reg_sec = get_umc_base(umc) + get_umc_reg(UMCCH_ADDR_MASK_SEC, pvt);
 
 		for_each_chip_select_mask(cs, umc, pvt) {
 			mask = &pvt->csels[umc].csmasks[cs];
@@ -2046,7 +2044,7 @@ static void read_dct_base_mask(struct amd64_pvt *pvt)
 	}
 }
 
-static void _determine_memory_type_df(struct amd64_umc *umc)
+static void _determine_memory_type_df(struct amd64_pvt *pvt, struct amd64_umc *umc)
 {
 	if (!(umc->sdp_ctrl & UMC_SDP_INIT)) {
 		umc->dram_type = MEM_EMPTY;
@@ -2057,7 +2055,7 @@ static void _determine_memory_type_df(struct amd64_umc *umc)
 	 * Check if the system supports the "DDR Type" field in UMC Config
 	 * and has DDR5 DIMMs in use.
 	 */
-	if (fam_type->flags.zn_regs_v2 && ((umc->umc_cfg & GENMASK(2, 0)) == 0x1)) {
+	if (pvt->flags.zn_regs_v2 && ((umc->umc_cfg & GENMASK(2, 0)) == 0x1)) {
 		if (umc->dimm_cfg & BIT(5))
 			umc->dram_type = MEM_LRDDR5;
 		else if (umc->dimm_cfg & BIT(4))
@@ -2082,7 +2080,7 @@ static void determine_memory_type_df(struct amd64_pvt *pvt)
 	for_each_umc(i) {
 		umc = &pvt->umc[i];
 
-		_determine_memory_type_df(umc);
+		_determine_memory_type_df(pvt, umc);
 		edac_dbg(1, "  UMC%d DIMM type: %s\n", i, edac_mem_types[umc->dram_type]);
 	}
 }
@@ -2648,7 +2646,7 @@ static int f17_addr_mask_to_cs_size(struct amd64_pvt *pvt, u8 umc,
 	 */
 	dimm = csrow_nr >> 1;
 
-	if (!fam_type->flags.zn_regs_v2)
+	if (!pvt->flags.zn_regs_v2)
 		cs_mask_nr >>= 1;
 
 	/* Asymmetric dual-rank DIMM support. */
@@ -3268,167 +3266,6 @@ static void debug_display_dimm_sizes(struct amd64_pvt *pvt, u8 ctrl)
 	}
 }
 
-static struct amd64_family_type family_types[] = {
-	[K8_CPUS] = {
-		.ctl_name = "K8",
-		.f1_id = PCI_DEVICE_ID_AMD_K8_NB_ADDRMAP,
-		.f2_id = PCI_DEVICE_ID_AMD_K8_NB_MEMCTL,
-		.max_mcs = 2,
-		.ops = {
-			.early_channel_count	= k8_early_channel_count,
-			.map_sysaddr_to_csrow	= k8_map_sysaddr_to_csrow,
-			.dbam_to_cs		= k8_dbam_to_chip_select,
-		}
-	},
-	[F10_CPUS] = {
-		.ctl_name = "F10h",
-		.f1_id = PCI_DEVICE_ID_AMD_10H_NB_MAP,
-		.f2_id = PCI_DEVICE_ID_AMD_10H_NB_DRAM,
-		.max_mcs = 2,
-		.ops = {
-			.early_channel_count	= f1x_early_channel_count,
-			.map_sysaddr_to_csrow	= f1x_map_sysaddr_to_csrow,
-			.dbam_to_cs		= f10_dbam_to_chip_select,
-		}
-	},
-	[F15_CPUS] = {
-		.ctl_name = "F15h",
-		.f1_id = PCI_DEVICE_ID_AMD_15H_NB_F1,
-		.f2_id = PCI_DEVICE_ID_AMD_15H_NB_F2,
-		.max_mcs = 2,
-		.ops = {
-			.early_channel_count	= f1x_early_channel_count,
-			.map_sysaddr_to_csrow	= f1x_map_sysaddr_to_csrow,
-			.dbam_to_cs		= f15_dbam_to_chip_select,
-		}
-	},
-	[F15_M30H_CPUS] = {
-		.ctl_name = "F15h_M30h",
-		.f1_id = PCI_DEVICE_ID_AMD_15H_M30H_NB_F1,
-		.f2_id = PCI_DEVICE_ID_AMD_15H_M30H_NB_F2,
-		.max_mcs = 2,
-		.ops = {
-			.early_channel_count	= f1x_early_channel_count,
-			.map_sysaddr_to_csrow	= f1x_map_sysaddr_to_csrow,
-			.dbam_to_cs		= f16_dbam_to_chip_select,
-		}
-	},
-	[F15_M60H_CPUS] = {
-		.ctl_name = "F15h_M60h",
-		.f1_id = PCI_DEVICE_ID_AMD_15H_M60H_NB_F1,
-		.f2_id = PCI_DEVICE_ID_AMD_15H_M60H_NB_F2,
-		.max_mcs = 2,
-		.ops = {
-			.early_channel_count	= f1x_early_channel_count,
-			.map_sysaddr_to_csrow	= f1x_map_sysaddr_to_csrow,
-			.dbam_to_cs		= f15_m60h_dbam_to_chip_select,
-		}
-	},
-	[F16_CPUS] = {
-		.ctl_name = "F16h",
-		.f1_id = PCI_DEVICE_ID_AMD_16H_NB_F1,
-		.f2_id = PCI_DEVICE_ID_AMD_16H_NB_F2,
-		.max_mcs = 2,
-		.ops = {
-			.early_channel_count	= f1x_early_channel_count,
-			.map_sysaddr_to_csrow	= f1x_map_sysaddr_to_csrow,
-			.dbam_to_cs		= f16_dbam_to_chip_select,
-		}
-	},
-	[F16_M30H_CPUS] = {
-		.ctl_name = "F16h_M30h",
-		.f1_id = PCI_DEVICE_ID_AMD_16H_M30H_NB_F1,
-		.f2_id = PCI_DEVICE_ID_AMD_16H_M30H_NB_F2,
-		.max_mcs = 2,
-		.ops = {
-			.early_channel_count	= f1x_early_channel_count,
-			.map_sysaddr_to_csrow	= f1x_map_sysaddr_to_csrow,
-			.dbam_to_cs		= f16_dbam_to_chip_select,
-		}
-	},
-	[F17_CPUS] = {
-		.ctl_name = "F17h",
-		.f0_id = PCI_DEVICE_ID_AMD_17H_DF_F0,
-		.f6_id = PCI_DEVICE_ID_AMD_17H_DF_F6,
-		.max_mcs = 2,
-		.ops = {
-			.early_channel_count	= f17_early_channel_count,
-			.dbam_to_cs		= f17_addr_mask_to_cs_size,
-		}
-	},
-	[F17_M10H_CPUS] = {
-		.ctl_name = "F17h_M10h",
-		.f0_id = PCI_DEVICE_ID_AMD_17H_M10H_DF_F0,
-		.f6_id = PCI_DEVICE_ID_AMD_17H_M10H_DF_F6,
-		.max_mcs = 2,
-		.ops = {
-			.early_channel_count	= f17_early_channel_count,
-			.dbam_to_cs		= f17_addr_mask_to_cs_size,
-		}
-	},
-	[F17_M30H_CPUS] = {
-		.ctl_name = "F17h_M30h",
-		.f0_id = PCI_DEVICE_ID_AMD_17H_M30H_DF_F0,
-		.f6_id = PCI_DEVICE_ID_AMD_17H_M30H_DF_F6,
-		.max_mcs = 8,
-		.ops = {
-			.early_channel_count	= f17_early_channel_count,
-			.dbam_to_cs		= f17_addr_mask_to_cs_size,
-		}
-	},
-	[F17_M60H_CPUS] = {
-		.ctl_name = "F17h_M60h",
-		.f0_id = PCI_DEVICE_ID_AMD_17H_M60H_DF_F0,
-		.f6_id = PCI_DEVICE_ID_AMD_17H_M60H_DF_F6,
-		.max_mcs = 2,
-		.ops = {
-			.early_channel_count	= f17_early_channel_count,
-			.dbam_to_cs		= f17_addr_mask_to_cs_size,
-		}
-	},
-	[F17_M70H_CPUS] = {
-		.ctl_name = "F17h_M70h",
-		.f0_id = PCI_DEVICE_ID_AMD_17H_M70H_DF_F0,
-		.f6_id = PCI_DEVICE_ID_AMD_17H_M70H_DF_F6,
-		.max_mcs = 2,
-		.ops = {
-			.early_channel_count	= f17_early_channel_count,
-			.dbam_to_cs		= f17_addr_mask_to_cs_size,
-		}
-	},
-	[F19_CPUS] = {
-		.ctl_name = "F19h",
-		.f0_id = PCI_DEVICE_ID_AMD_19H_DF_F0,
-		.f6_id = PCI_DEVICE_ID_AMD_19H_DF_F6,
-		.max_mcs = 8,
-		.ops = {
-			.early_channel_count	= f17_early_channel_count,
-			.dbam_to_cs		= f17_addr_mask_to_cs_size,
-		}
-	},
-	[F19_M10H_CPUS] = {
-		.ctl_name = "F19h_M10h",
-		.f0_id = PCI_DEVICE_ID_AMD_19H_M10H_DF_F0,
-		.f6_id = PCI_DEVICE_ID_AMD_19H_M10H_DF_F6,
-		.max_mcs = 12,
-		.flags.zn_regs_v2 = 1,
-		.ops = {
-			.early_channel_count	= f17_early_channel_count,
-			.dbam_to_cs		= f17_addr_mask_to_cs_size,
-		}
-	},
-	[F19_M50H_CPUS] = {
-		.ctl_name = "F19h_M50h",
-		.f0_id = PCI_DEVICE_ID_AMD_19H_M50H_DF_F0,
-		.f6_id = PCI_DEVICE_ID_AMD_19H_M50H_DF_F6,
-		.max_mcs = 2,
-		.ops = {
-			.early_channel_count	= f17_early_channel_count,
-			.dbam_to_cs		= f17_addr_mask_to_cs_size,
-		}
-	},
-};
-
 /*
  * These are tables of eigenvectors (one per line) which can be used for the
  * construction of the syndrome tables. The modified syndrome search algorithm
@@ -3850,7 +3687,7 @@ static void __read_mc_regs_df(struct amd64_pvt *pvt)
 		umc_base = get_umc_base(i);
 		umc = &pvt->umc[i];
 
-		amd_smn_read(nid, umc_base + get_umc_reg(UMCCH_DIMM_CFG), &umc->dimm_cfg);
+		amd_smn_read(nid, umc_base + get_umc_reg(UMCCH_DIMM_CFG, pvt), &umc->dimm_cfg);
 		amd_smn_read(nid, umc_base + UMCCH_UMC_CFG, &umc->umc_cfg);
 		amd_smn_read(nid, umc_base + UMCCH_SDP_CTRL, &umc->sdp_ctrl);
 		amd_smn_read(nid, umc_base + UMCCH_ECC_CTRL, &umc->ecc_ctrl);
@@ -4380,7 +4217,7 @@ static void setup_mci_misc_attrs(struct mem_ctl_info *mci)
 
 	mci->edac_cap		= determine_edac_cap(pvt);
 	mci->mod_name		= EDAC_MOD_STR;
-	mci->ctl_name		= fam_type->ctl_name;
+	mci->ctl_name		= pvt->ctl_name;
 	mci->dev_name		= pci_name(pvt->F3);
 	mci->ctl_page_to_phys	= NULL;
 
@@ -4392,7 +4229,7 @@ static void setup_mci_misc_attrs(struct mem_ctl_info *mci)
 /*
  * returns a pointer to the family descriptor on success, NULL otherwise.
  */
-static struct amd64_family_type *per_family_init(struct amd64_pvt *pvt)
+static void per_family_init(struct amd64_pvt *pvt)
 {
 	pvt->ext_model  = boot_cpu_data.x86_model >> 4;
 	pvt->stepping	= boot_cpu_data.x86_stepping;
@@ -4401,109 +4238,150 @@ static struct amd64_family_type *per_family_init(struct amd64_pvt *pvt)
 
 	switch (pvt->fam) {
 	case 0xf:
-		fam_type	= &family_types[K8_CPUS];
-		pvt->ops	= &family_types[K8_CPUS].ops;
+		pvt->ctl_name				= "K8";
+		pvt->f1_id				= PCI_DEVICE_ID_AMD_K8_NB_ADDRMAP;
+		pvt->f2_id				= PCI_DEVICE_ID_AMD_K8_NB_MEMCTL;
+		pvt->max_mcs				= 2;
+		pvt->ops->early_channel_count		= k8_early_channel_count;
+		pvt->ops->map_sysaddr_to_csrow		= k8_map_sysaddr_to_csrow;
+		pvt->ops->dbam_to_cs			= k8_dbam_to_chip_select;
 		break;
 
 	case 0x10:
-		fam_type	= &family_types[F10_CPUS];
-		pvt->ops	= &family_types[F10_CPUS].ops;
+		pvt->ctl_name				= "F10h";
+		pvt->f1_id				= PCI_DEVICE_ID_AMD_10H_NB_MAP;
+		pvt->f2_id				= PCI_DEVICE_ID_AMD_10H_NB_DRAM;
+		pvt->max_mcs				= 2;
+		pvt->ops->early_channel_count		= f1x_early_channel_count;
+		pvt->ops->map_sysaddr_to_csrow		= f1x_map_sysaddr_to_csrow;
+		pvt->ops->dbam_to_cs			= f10_dbam_to_chip_select;
 		break;
 
 	case 0x15:
 		if (pvt->model == 0x30) {
-			fam_type = &family_types[F15_M30H_CPUS];
-			pvt->ops = &family_types[F15_M30H_CPUS].ops;
-			break;
+			pvt->ctl_name			= "F15h_M30h";
+			pvt->f1_id			= PCI_DEVICE_ID_AMD_15H_M30H_NB_F1;
+			pvt->f2_id			= PCI_DEVICE_ID_AMD_15H_M30H_NB_F2;
+			pvt->ops->dbam_to_cs		= f16_dbam_to_chip_select;
 		} else if (pvt->model == 0x60) {
-			fam_type = &family_types[F15_M60H_CPUS];
-			pvt->ops = &family_types[F15_M60H_CPUS].ops;
-			break;
-		/* Richland is only client */
+			pvt->ctl_name			= "F15h_M60h";
+			pvt->f1_id			= PCI_DEVICE_ID_AMD_15H_M60H_NB_F1;
+			pvt->f2_id			= PCI_DEVICE_ID_AMD_15H_M60H_NB_F2;
+			pvt->ops->dbam_to_cs		= f15_m60h_dbam_to_chip_select;
 		} else if (pvt->model == 0x13) {
-			return NULL;
+		/* Richland is only client */
+			return;
 		} else {
-			fam_type	= &family_types[F15_CPUS];
-			pvt->ops	= &family_types[F15_CPUS].ops;
+			pvt->ctl_name			= "F15h";
+			pvt->f1_id			= PCI_DEVICE_ID_AMD_15H_NB_F1;
+			pvt->f2_id			= PCI_DEVICE_ID_AMD_15H_NB_F2;
+			pvt->ops->dbam_to_cs		= f15_dbam_to_chip_select;
 		}
+		pvt->max_mcs				= 2;
+		pvt->ops->early_channel_count		= f1x_early_channel_count;
+		pvt->ops->map_sysaddr_to_csrow		= f1x_map_sysaddr_to_csrow;
 		break;
 
 	case 0x16:
 		if (pvt->model == 0x30) {
-			fam_type = &family_types[F16_M30H_CPUS];
-			pvt->ops = &family_types[F16_M30H_CPUS].ops;
-			break;
+			pvt->ctl_name			= "F16h_M30h";
+			pvt->f1_id			= PCI_DEVICE_ID_AMD_16H_M30H_NB_F1;
+			pvt->f2_id			= PCI_DEVICE_ID_AMD_16H_M30H_NB_F2;
+		} else {
+			pvt->ctl_name			= "F16h";
+			pvt->f1_id			= PCI_DEVICE_ID_AMD_16H_NB_F1;
+			pvt->f2_id			= PCI_DEVICE_ID_AMD_16H_NB_F2;
 		}
-		fam_type	= &family_types[F16_CPUS];
-		pvt->ops	= &family_types[F16_CPUS].ops;
+		pvt->max_mcs				= 2;
+		pvt->ops->early_channel_count		= f1x_early_channel_count;
+		pvt->ops->map_sysaddr_to_csrow		= f1x_map_sysaddr_to_csrow;
+		pvt->ops->dbam_to_cs			= f16_dbam_to_chip_select;
 		break;
 
 	case 0x17:
 		if (pvt->model >= 0x10 && pvt->model <= 0x2f) {
-			fam_type = &family_types[F17_M10H_CPUS];
-			pvt->ops = &family_types[F17_M10H_CPUS].ops;
-			df_ops	 = &df2_ops;
-			break;
+			pvt->ctl_name			= "F17h_M10h";
+			pvt->f0_id			= PCI_DEVICE_ID_AMD_17H_M10H_DF_F0;
+			pvt->f6_id			= PCI_DEVICE_ID_AMD_17H_M10H_DF_F6;
+			pvt->max_mcs			= 2;
+			df_ops				= &df2_ops;
 		} else if (pvt->model >= 0x30 && pvt->model <= 0x3f) {
-			fam_type = &family_types[F17_M30H_CPUS];
-			pvt->ops = &family_types[F17_M30H_CPUS].ops;
-			df_ops	 = &df3_ops;
-			break;
+			pvt->ctl_name			= "F17h_M30h";
+			pvt->f0_id			= PCI_DEVICE_ID_AMD_17H_M30H_DF_F0;
+			pvt->f6_id			= PCI_DEVICE_ID_AMD_17H_M30H_DF_F6;
+			pvt->max_mcs			= 8;
+			df_ops				= &df3_ops;
 		} else if (pvt->model >= 0x60 && pvt->model <= 0x6f) {
-			fam_type = &family_types[F17_M60H_CPUS];
-			pvt->ops = &family_types[F17_M60H_CPUS].ops;
-			df_ops	 = &df3_ops;
-			break;
+			pvt->ctl_name			= "F17h_M60h";
+			pvt->f0_id			= PCI_DEVICE_ID_AMD_17H_M60H_DF_F0;
+			pvt->f6_id			= PCI_DEVICE_ID_AMD_17H_M60H_DF_F6;
+			pvt->max_mcs			= 2;
+			df_ops				= &df3_ops;
 		} else if (pvt->model >= 0x70 && pvt->model <= 0x7f) {
-			fam_type = &family_types[F17_M70H_CPUS];
-			pvt->ops = &family_types[F17_M70H_CPUS].ops;
-			df_ops	 = &df3_ops;
-			break;
+			pvt->ctl_name			= "F17h_M70h";
+			pvt->f0_id			= PCI_DEVICE_ID_AMD_17H_M70H_DF_F0;
+			pvt->f6_id			= PCI_DEVICE_ID_AMD_17H_M70H_DF_F6;
+			pvt->max_mcs			= 2;
+			df_ops				= &df3_ops;
+		} else {
+			pvt->ctl_name			= "F17h";
+			pvt->f0_id			= PCI_DEVICE_ID_AMD_17H_DF_F0;
+			pvt->f6_id			= PCI_DEVICE_ID_AMD_17H_DF_F6;
+			pvt->max_mcs			= 2;
+			df_ops				= &df2_ops;
 		}
 		fallthrough;
 	case 0x18:
-		fam_type	= &family_types[F17_CPUS];
-		pvt->ops	= &family_types[F17_CPUS].ops;
-		df_ops		= &df2_ops;
-
-		if (pvt->fam == 0x18)
-			family_types[F17_CPUS].ctl_name = "F18h";
+		pvt->ops->early_channel_count		= f17_early_channel_count;
+		pvt->ops->dbam_to_cs			= f17_addr_mask_to_cs_size;
+
+		if (pvt->fam == 0x18) {
+			pvt->ctl_name			= "F18h";
+			pvt->f0_id			= PCI_DEVICE_ID_AMD_17H_DF_F0;
+			pvt->f6_id			= PCI_DEVICE_ID_AMD_17H_DF_F6;
+			pvt->max_mcs			= 2;
+			df_ops				= &df2_ops;
+		}
 		break;
 
 	case 0x19:
 		if (pvt->model >= 0x10 && pvt->model <= 0x1f) {
-			fam_type = &family_types[F19_M10H_CPUS];
-			pvt->ops = &family_types[F19_M10H_CPUS].ops;
-			break;
+			pvt->ctl_name			= "F19h_M10h";
+			pvt->f0_id			= PCI_DEVICE_ID_AMD_19H_M10H_DF_F0;
+			pvt->f6_id			= PCI_DEVICE_ID_AMD_19H_M10H_DF_F6;
+			pvt->max_mcs			= 12;
+			pvt->flags.zn_regs_v2		= 1;
 		} else if (pvt->model >= 0x20 && pvt->model <= 0x2f) {
-			fam_type = &family_types[F17_M70H_CPUS];
-			pvt->ops = &family_types[F17_M70H_CPUS].ops;
-			fam_type->ctl_name = "F19h_M20h";
-			df_ops	 = &df3_ops;
-			break;
+			pvt->ctl_name			= "F19h_M20h";
+			pvt->f0_id			= PCI_DEVICE_ID_AMD_17H_M70H_DF_F0;
+			pvt->f6_id			= PCI_DEVICE_ID_AMD_17H_M70H_DF_F6;
+			pvt->max_mcs			= 2;
+			df_ops				= &df3_ops;
 		} else if (pvt->model >= 0x50 && pvt->model <= 0x5f) {
-			fam_type = &family_types[F19_M50H_CPUS];
-			pvt->ops = &family_types[F19_M50H_CPUS].ops;
-			fam_type->ctl_name = "F19h_M50h";
-			break;
+			pvt->ctl_name			= "F19h_M50h";
+			pvt->f0_id			= PCI_DEVICE_ID_AMD_19H_M50H_DF_F0;
+			pvt->f6_id			= PCI_DEVICE_ID_AMD_19H_M50H_DF_F6;
+			pvt->max_mcs			= 2;
 		} else if (pvt->model >= 0xa0 && pvt->model <= 0xaf) {
-			fam_type = &family_types[F19_M10H_CPUS];
-			pvt->ops = &family_types[F19_M10H_CPUS].ops;
-			fam_type->ctl_name = "F19h_MA0h";
-			break;
+			pvt->ctl_name			= "F19h_M10h";
+			pvt->f0_id			= PCI_DEVICE_ID_AMD_19H_M10H_DF_F0;
+			pvt->f6_id			= PCI_DEVICE_ID_AMD_19H_M10H_DF_F6;
+			pvt->max_mcs			= 2;
+		} else {
+			pvt->ctl_name			= "F19h";
+			pvt->f0_id			= PCI_DEVICE_ID_AMD_19H_DF_F0;
+			pvt->f6_id			= PCI_DEVICE_ID_AMD_19H_DF_F6;
+			pvt->max_mcs			= 8;
+			df_ops				= &df3_ops;
 		}
-		fam_type	= &family_types[F19_CPUS];
-		pvt->ops	= &family_types[F19_CPUS].ops;
-		family_types[F19_CPUS].ctl_name = "F19h";
-		df_ops		= &df3_ops;
+		pvt->ops->early_channel_count		= f17_early_channel_count;
+		pvt->ops->dbam_to_cs			= f17_addr_mask_to_cs_size;
 		break;
 
 	default:
 		amd64_err("Unsupported family!\n");
-		return NULL;
+		return;
 	}
-
-	return fam_type;
 }
 
 static const struct attribute_group *amd64_edac_attr_groups[] = {
@@ -4520,15 +4398,15 @@ static int hw_info_get(struct amd64_pvt *pvt)
 	int ret;
 
 	if (pvt->fam >= 0x17) {
-		pvt->umc = kcalloc(fam_type->max_mcs, sizeof(struct amd64_umc), GFP_KERNEL);
+		pvt->umc = kcalloc(pvt->max_mcs, sizeof(struct amd64_umc), GFP_KERNEL);
 		if (!pvt->umc)
 			return -ENOMEM;
 
-		pci_id1 = fam_type->f0_id;
-		pci_id2 = fam_type->f6_id;
+		pci_id1 = pvt->f0_id;
+		pci_id2 = pvt->f6_id;
 	} else {
-		pci_id1 = fam_type->f1_id;
-		pci_id2 = fam_type->f2_id;
+		pci_id1 = pvt->f1_id;
+		pci_id2 = pvt->f2_id;
 	}
 
 	ret = reserve_mc_sibling_devs(pvt, pci_id1, pci_id2);
@@ -4574,7 +4452,7 @@ static int init_one_instance(struct amd64_pvt *pvt)
 	 * only one channel. Also, this simplifies handling later for the price
 	 * of a couple of KBs tops.
 	 */
-	layers[1].size = fam_type->max_mcs;
+	layers[1].size = pvt->max_mcs;
 	layers[1].is_virt_csrow = false;
 
 	mci = edac_mc_alloc(pvt->mc_node_id, ARRAY_SIZE(layers), layers, 0);
@@ -4604,7 +4482,7 @@ static bool instance_has_memory(struct amd64_pvt *pvt)
 	bool cs_enabled = false;
 	int cs = 0, dct = 0;
 
-	for (dct = 0; dct < fam_type->max_mcs; dct++) {
+	for (dct = 0; dct < pvt->max_mcs; dct++) {
 		for_each_chip_select(cs, dct, pvt)
 			cs_enabled |= csrow_enabled(cs, dct, pvt);
 	}
@@ -4633,10 +4511,12 @@ static int probe_one_instance(unsigned int nid)
 	pvt->mc_node_id	= nid;
 	pvt->F3 = F3;
 
+	pvt->ops = kzalloc(sizeof(*pvt->ops), GFP_KERNEL);
+	if (!pvt->ops)
+		goto err_out;
+
 	ret = -ENODEV;
-	fam_type = per_family_init(pvt);
-	if (!fam_type)
-		goto err_enable;
+	per_family_init(pvt);
 
 	ret = hw_info_get(pvt);
 	if (ret < 0)
@@ -4674,8 +4554,8 @@ static int probe_one_instance(unsigned int nid)
 		goto err_enable;
 	}
 
-	amd64_info("%s %sdetected (node %d).\n", fam_type->ctl_name,
-		     (pvt->fam == 0xf ?
+	amd64_info("%s %sdetected (node %d).\n", pvt->ctl_name,
+		   (pvt->fam == 0xf ?
 				(pvt->ext_model >= K8_REV_F  ? "revF or later "
 							     : "revE or earlier ")
 				 : ""), pvt->mc_node_id);
diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
index 6a112270a84b..4e3f9755bc73 100644
--- a/drivers/edac/amd64_edac.h
+++ b/drivers/edac/amd64_edac.h
@@ -291,25 +291,6 @@
 
 #define UMC_SDP_INIT			BIT(31)
 
-enum amd_families {
-	K8_CPUS = 0,
-	F10_CPUS,
-	F15_CPUS,
-	F15_M30H_CPUS,
-	F15_M60H_CPUS,
-	F16_CPUS,
-	F16_M30H_CPUS,
-	F17_CPUS,
-	F17_M10H_CPUS,
-	F17_M30H_CPUS,
-	F17_M60H_CPUS,
-	F17_M70H_CPUS,
-	F19_CPUS,
-	F19_M10H_CPUS,
-	F19_M50H_CPUS,
-	NUM_FAMILIES,
-};
-
 /* Error injection control structure */
 struct error_injection {
 	u32	 section;
@@ -352,6 +333,16 @@ struct amd64_umc {
 	enum mem_type dram_type;
 };
 
+struct amd64_family_flags {
+	/*
+	 * Indicates that the system supports the new register offsets, etc.
+	 * first introduced with Family 19h Model 10h.
+	 */
+	__u64 zn_regs_v2	: 1,
+
+	      __reserved	: 63;
+};
+
 struct amd64_pvt {
 	struct low_ops *ops;
 
@@ -394,6 +385,12 @@ struct amd64_pvt {
 	/* x4, x8, or x16 syndromes in use */
 	u8 ecc_sym_sz;
 
+	const char *ctl_name;
+	u16 f0_id, f1_id, f2_id, f6_id;
+	/* Maximum number of memory controllers per die/node. */
+	u8 max_mcs;
+
+	struct amd64_family_flags flags;
 	/* place to store error injection parameters prior to issue */
 	struct error_injection injection;
 
@@ -479,30 +476,11 @@ struct ecc_settings {
  * functions and per device encoding/decoding logic.
  */
 struct low_ops {
-	int (*early_channel_count)	(struct amd64_pvt *pvt);
-	void (*map_sysaddr_to_csrow)	(struct mem_ctl_info *mci, u64 sys_addr,
-					 struct err_info *);
-	int (*dbam_to_cs)		(struct amd64_pvt *pvt, u8 dct,
-					 unsigned cs_mode, int cs_mask_nr);
-};
-
-struct amd64_family_flags {
-	/*
-	 * Indicates that the system supports the new register offsets, etc.
-	 * first introduced with Family 19h Model 10h.
-	 */
-	__u64 zn_regs_v2	: 1,
-
-	      __reserved	: 63;
-};
-
-struct amd64_family_type {
-	const char *ctl_name;
-	u16 f0_id, f1_id, f2_id, f6_id;
-	/* Maximum number of memory controllers per die/node. */
-	u8 max_mcs;
-	struct amd64_family_flags flags;
-	struct low_ops ops;
+	int  (*early_channel_count)(struct amd64_pvt *pvt);
+	void (*map_sysaddr_to_csrow)(struct mem_ctl_info *mci, u64 sys_addr,
+				     struct err_info *err);
+	int  (*dbam_to_cs)(struct amd64_pvt *pvt, u8 dct,
+			   unsigned int cs_mode, int cs_mask_nr);
 };
 
 int __amd64_read_pci_cfg_dword(struct pci_dev *pdev, int offset,
-- 
2.25.1

[PATCH v7 05/12] EDAC/amd64: Define dynamic family ops routines

[Naveen Krishna Chatradhi]

From: Muralidhara M K <muralimk@amd.com>

Extending family-specific assignments dynamic.
This would simplify adding support for future platforms.

Signed-off-by: Muralidhara M K <muralimk@amd.com>
Signed-off-by: Naveen Krishna Chatradhi <nchatrad@amd.com>
---
Link:
https://lkml.kernel.org/r/20211028130106.15701-4-nchatrad@amd.com

v5->v7:
* None

v4->v5:
* split read_mc_regs for per family ops
* Adjusted and Called dump_misc_regs for family ops

v3->v4:
* Modified k8_prep_chip_selects for ext_model checks
* Add read_dct_base_mask to ops
* Renamed find_umc_channel and addressed minor comments

v2->v3:
* Defined new family operation routines

v1->v2:
* new change


 drivers/edac/amd64_edac.c | 442 ++++++++++++++++++++++++--------------
 drivers/edac/amd64_edac.h |  13 ++
 2 files changed, 297 insertions(+), 158 deletions(-)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 4cac43840ccc..babd25f29845 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -1695,36 +1695,40 @@ static int get_channel_from_ecc_syndrome(struct mem_ctl_info *, u16);
  * Determine if the DIMMs have ECC enabled. ECC is enabled ONLY if all the DIMMs
  * are ECC capable.
  */
-static unsigned long determine_edac_cap(struct amd64_pvt *pvt)
+static unsigned long f1x_determine_edac_cap(struct amd64_pvt *pvt)
 {
 	unsigned long edac_cap = EDAC_FLAG_NONE;
 	u8 bit;
 
-	if (pvt->umc) {
-		u8 i, umc_en_mask = 0, dimm_ecc_en_mask = 0;
+	bit = (pvt->fam > 0xf || pvt->ext_model >= K8_REV_F)
+		? 19
+		: 17;
 
-		for_each_umc(i) {
-			if (!(pvt->umc[i].sdp_ctrl & UMC_SDP_INIT))
-				continue;
+	if (pvt->dclr0 & BIT(bit))
+		edac_cap = EDAC_FLAG_SECDED;
 
-			umc_en_mask |= BIT(i);
+	return edac_cap;
+}
 
-			/* UMC Configuration bit 12 (DimmEccEn) */
-			if (pvt->umc[i].umc_cfg & BIT(12))
-				dimm_ecc_en_mask |= BIT(i);
-		}
+static unsigned long f17_determine_edac_cap(struct amd64_pvt *pvt)
+{
+	u8 i, umc_en_mask = 0, dimm_ecc_en_mask = 0;
+	unsigned long edac_cap = EDAC_FLAG_NONE;
 
-		if (umc_en_mask == dimm_ecc_en_mask)
-			edac_cap = EDAC_FLAG_SECDED;
-	} else {
-		bit = (pvt->fam > 0xf || pvt->ext_model >= K8_REV_F)
-			? 19
-			: 17;
+	for_each_umc(i) {
+		if (!(pvt->umc[i].sdp_ctrl & UMC_SDP_INIT))
+			continue;
 
-		if (pvt->dclr0 & BIT(bit))
-			edac_cap = EDAC_FLAG_SECDED;
+		umc_en_mask |= BIT(i);
+
+		/* UMC Configuration bit 12 (DimmEccEn) */
+		if (pvt->umc[i].umc_cfg & BIT(12))
+			dimm_ecc_en_mask |= BIT(i);
 	}
 
+	if (umc_en_mask == dimm_ecc_en_mask)
+		edac_cap = EDAC_FLAG_SECDED;
+
 	return edac_cap;
 }
 
@@ -1771,6 +1775,13 @@ static void debug_dump_dramcfg_low(struct amd64_pvt *pvt, u32 dclr, int chan)
 #define CS_EVEN			(CS_EVEN_PRIMARY | CS_EVEN_SECONDARY)
 #define CS_ODD			(CS_ODD_PRIMARY | CS_ODD_SECONDARY)
 
+static int f1x_get_cs_mode(int dimm, u8 ctrl, struct amd64_pvt *pvt)
+{
+	u32 dbam = ctrl ? pvt->dbam1 : pvt->dbam0;
+
+	return DBAM_DIMM(dimm, dbam);
+}
+
 static int f17_get_cs_mode(int dimm, u8 ctrl, struct amd64_pvt *pvt)
 {
 	u8 base, count = 0;
@@ -1907,10 +1918,7 @@ static void __dump_misc_regs(struct amd64_pvt *pvt)
 /* Display and decode various NB registers for debug purposes. */
 static void dump_misc_regs(struct amd64_pvt *pvt)
 {
-	if (pvt->umc)
-		__dump_misc_regs_df(pvt);
-	else
-		__dump_misc_regs(pvt);
+	pvt->ops->dump_misc_regs(pvt);
 
 	edac_dbg(1, "  DramHoleValid: %s\n", dhar_valid(pvt) ? "yes" : "no");
 
@@ -1920,28 +1928,51 @@ static void dump_misc_regs(struct amd64_pvt *pvt)
 /*
  * See BKDG, F2x[1,0][5C:40], F2[1,0][6C:60]
  */
-static void prep_chip_selects(struct amd64_pvt *pvt)
+static void k8_prep_chip_selects(struct amd64_pvt *pvt)
 {
-	if (pvt->fam == 0xf && pvt->ext_model < K8_REV_F) {
-		pvt->csels[0].b_cnt = pvt->csels[1].b_cnt = 8;
-		pvt->csels[0].m_cnt = pvt->csels[1].m_cnt = 8;
-	} else if (pvt->fam == 0x15 && pvt->model == 0x30) {
-		pvt->csels[0].b_cnt = pvt->csels[1].b_cnt = 4;
-		pvt->csels[0].m_cnt = pvt->csels[1].m_cnt = 2;
-	} else if (pvt->fam >= 0x17) {
-		int umc;
-
-		for_each_umc(umc) {
-			pvt->csels[umc].b_cnt = 4;
-			pvt->csels[umc].m_cnt = pvt->flags.zn_regs_v2 ? 4 : 2;
-		}
+	if (pvt->ext_model < K8_REV_F) {
+		pvt->csels[0].b_cnt = 8;
+		pvt->csels[1].b_cnt = 8;
 
-	} else {
-		pvt->csels[0].b_cnt = pvt->csels[1].b_cnt = 8;
-		pvt->csels[0].m_cnt = pvt->csels[1].m_cnt = 4;
+		pvt->csels[0].m_cnt = 8;
+		pvt->csels[1].m_cnt = 8;
+	} else if (pvt->ext_model >= K8_REV_F) {
+		pvt->csels[0].b_cnt = 8;
+		pvt->csels[1].b_cnt = 8;
+
+		pvt->csels[0].m_cnt = 4;
+		pvt->csels[1].m_cnt = 4;
 	}
 }
 
+static void f15_m30h_prep_chip_selects(struct amd64_pvt *pvt)
+{
+	pvt->csels[0].b_cnt = 4;
+	pvt->csels[1].b_cnt = 4;
+
+	pvt->csels[0].m_cnt = 2;
+	pvt->csels[1].m_cnt = 2;
+}
+
+static void f17_prep_chip_selects(struct amd64_pvt *pvt)
+{
+	int umc;
+
+	for_each_umc(umc) {
+		pvt->csels[umc].b_cnt = 4;
+		pvt->csels[umc].m_cnt = pvt->flags.zn_regs_v2 ? 4 : 2;
+	}
+}
+
+static void default_prep_chip_selects(struct amd64_pvt *pvt)
+{
+	pvt->csels[0].b_cnt = 8;
+	pvt->csels[1].b_cnt = 8;
+
+	pvt->csels[0].m_cnt = 4;
+	pvt->csels[1].m_cnt = 4;
+}
+
 static void read_umc_base_mask(struct amd64_pvt *pvt)
 {
 	u32 umc_base_reg, umc_base_reg_sec;
@@ -2000,11 +2031,6 @@ static void read_dct_base_mask(struct amd64_pvt *pvt)
 {
 	int cs;
 
-	prep_chip_selects(pvt);
-
-	if (pvt->umc)
-		return read_umc_base_mask(pvt);
-
 	for_each_chip_select(cs, 0, pvt) {
 		int reg0   = DCSB0 + (cs * 4);
 		int reg1   = DCSB1 + (cs * 4);
@@ -2089,9 +2115,6 @@ static void determine_memory_type(struct amd64_pvt *pvt)
 {
 	u32 dram_ctrl, dcsm;
 
-	if (pvt->umc)
-		return determine_memory_type_df(pvt);
-
 	switch (pvt->fam) {
 	case 0xf:
 		if (pvt->ext_model >= K8_REV_F)
@@ -2141,6 +2164,8 @@ static void determine_memory_type(struct amd64_pvt *pvt)
 		WARN(1, KERN_ERR "%s: Family??? 0x%x\n", __func__, pvt->fam);
 		pvt->dram_type = MEM_EMPTY;
 	}
+
+	edac_dbg(1, "  DIMM type: %s\n", edac_mem_types[pvt->dram_type]);
 	return;
 
 ddr3:
@@ -3513,10 +3538,13 @@ static inline void decode_bus_error(int node_id, struct mce *m)
  * Currently, we can derive the channel number by looking at the 6th nibble in
  * the instance_id. For example, instance_id=0xYXXXXX where Y is the channel
  * number.
+ *
+ * csrow can be derived from the lower 3 bits of MCA_SYND value.
  */
-static int find_umc_channel(struct mce *m)
+static void update_umc_err_info(struct mce *m, struct err_info *err)
 {
-	return (m->ipid & GENMASK(31, 0)) >> 20;
+	err->channel = (m->ipid & GENMASK(31, 0)) >> 20;
+	err->csrow = m->synd & 0x7;
 }
 
 static void decode_umc_error(int node_id, struct mce *m)
@@ -3538,8 +3566,6 @@ static void decode_umc_error(int node_id, struct mce *m)
 	if (m->status & MCI_STATUS_DEFERRED)
 		ecc_type = 3;
 
-	err.channel = find_umc_channel(m);
-
 	if (!(m->status & MCI_STATUS_SYNDV)) {
 		err.err_code = ERR_SYND;
 		goto log_error;
@@ -3554,7 +3580,7 @@ static void decode_umc_error(int node_id, struct mce *m)
 			err.err_code = ERR_CHANNEL;
 	}
 
-	err.csrow = m->synd & 0x7;
+	pvt->ops->get_umc_err_info(m, &err);
 
 	if (umc_normaddr_to_sysaddr(m->addr, pvt->mc_node_id, err.channel, &sys_addr)) {
 		err.err_code = ERR_NORM_ADDR;
@@ -3639,26 +3665,11 @@ static void free_mc_sibling_devs(struct amd64_pvt *pvt)
 	}
 }
 
-static void determine_ecc_sym_sz(struct amd64_pvt *pvt)
+static void f1x_determine_ecc_sym_sz(struct amd64_pvt *pvt)
 {
 	pvt->ecc_sym_sz = 4;
 
-	if (pvt->umc) {
-		u8 i;
-
-		for_each_umc(i) {
-			/* Check enabled channels only: */
-			if (pvt->umc[i].sdp_ctrl & UMC_SDP_INIT) {
-				if (pvt->umc[i].ecc_ctrl & BIT(9)) {
-					pvt->ecc_sym_sz = 16;
-					return;
-				} else if (pvt->umc[i].ecc_ctrl & BIT(7)) {
-					pvt->ecc_sym_sz = 8;
-					return;
-				}
-			}
-		}
-	} else if (pvt->fam >= 0x10) {
+	if (pvt->fam >= 0x10) {
 		u32 tmp;
 
 		amd64_read_pci_cfg(pvt->F3, EXT_NB_MCA_CFG, &tmp);
@@ -3672,6 +3683,47 @@ static void determine_ecc_sym_sz(struct amd64_pvt *pvt)
 	}
 }
 
+static void f17_determine_ecc_sym_sz(struct amd64_pvt *pvt)
+{
+	u8 i;
+
+	pvt->ecc_sym_sz = 4;
+
+	for_each_umc(i) {
+		/* Check enabled channels only: */
+		if (pvt->umc[i].sdp_ctrl & UMC_SDP_INIT) {
+			if (pvt->umc[i].ecc_ctrl & BIT(9)) {
+				pvt->ecc_sym_sz = 16;
+				return;
+			} else if (pvt->umc[i].ecc_ctrl & BIT(7)) {
+				pvt->ecc_sym_sz = 8;
+				return;
+			}
+		}
+	}
+}
+
+static void read_top_mem_registers(struct amd64_pvt *pvt)
+{
+	u64 msr_val;
+
+	/*
+	 * Retrieve TOP_MEM and TOP_MEM2; no masking off of reserved bits since
+	 * those are Read-As-Zero.
+	 */
+	rdmsrl(MSR_K8_TOP_MEM1, pvt->top_mem);
+	edac_dbg(0, "  TOP_MEM:  0x%016llx\n", pvt->top_mem);
+
+	/* Check first whether TOP_MEM2 is enabled: */
+	rdmsrl(MSR_AMD64_SYSCFG, msr_val);
+	if (msr_val & BIT(21)) {
+		rdmsrl(MSR_K8_TOP_MEM2, pvt->top_mem2);
+		edac_dbg(0, "  TOP_MEM2: 0x%016llx\n", pvt->top_mem2);
+	} else {
+		edac_dbg(0, "  TOP_MEM2 disabled\n");
+	}
+}
+
 /*
  * Retrieve the hardware registers of the memory controller.
  */
@@ -3693,6 +3745,8 @@ static void __read_mc_regs_df(struct amd64_pvt *pvt)
 		amd_smn_read(nid, umc_base + UMCCH_ECC_CTRL, &umc->ecc_ctrl);
 		amd_smn_read(nid, umc_base + UMCCH_UMC_CAP_HI, &umc->umc_cap_hi);
 	}
+
+	amd64_read_pci_cfg(pvt->F0, DF_DHAR, &pvt->dhar);
 }
 
 /*
@@ -3702,30 +3756,8 @@ static void __read_mc_regs_df(struct amd64_pvt *pvt)
 static void read_mc_regs(struct amd64_pvt *pvt)
 {
 	unsigned int range;
-	u64 msr_val;
 
-	/*
-	 * Retrieve TOP_MEM and TOP_MEM2; no masking off of reserved bits since
-	 * those are Read-As-Zero.
-	 */
-	rdmsrl(MSR_K8_TOP_MEM1, pvt->top_mem);
-	edac_dbg(0, "  TOP_MEM:  0x%016llx\n", pvt->top_mem);
-
-	/* Check first whether TOP_MEM2 is enabled: */
-	rdmsrl(MSR_AMD64_SYSCFG, msr_val);
-	if (msr_val & BIT(21)) {
-		rdmsrl(MSR_K8_TOP_MEM2, pvt->top_mem2);
-		edac_dbg(0, "  TOP_MEM2: 0x%016llx\n", pvt->top_mem2);
-	} else {
-		edac_dbg(0, "  TOP_MEM2 disabled\n");
-	}
-
-	if (pvt->umc) {
-		__read_mc_regs_df(pvt);
-		amd64_read_pci_cfg(pvt->F0, DF_DHAR, &pvt->dhar);
-
-		goto skip;
-	}
+	read_top_mem_registers(pvt);
 
 	amd64_read_pci_cfg(pvt->F3, NBCAP, &pvt->nbcap);
 
@@ -3766,16 +3798,6 @@ static void read_mc_regs(struct amd64_pvt *pvt)
 		amd64_read_dct_pci_cfg(pvt, 1, DCLR0, &pvt->dclr1);
 		amd64_read_dct_pci_cfg(pvt, 1, DCHR0, &pvt->dchr1);
 	}
-
-skip:
-	read_dct_base_mask(pvt);
-
-	determine_memory_type(pvt);
-
-	if (!pvt->umc)
-		edac_dbg(1, "  DIMM type: %s\n", edac_mem_types[pvt->dram_type]);
-
-	determine_ecc_sym_sz(pvt);
 }
 
 /*
@@ -3814,17 +3836,10 @@ static void read_mc_regs(struct amd64_pvt *pvt)
  */
 static u32 get_csrow_nr_pages(struct amd64_pvt *pvt, u8 dct, int csrow_nr_orig)
 {
-	u32 dbam = dct ? pvt->dbam1 : pvt->dbam0;
 	int csrow_nr = csrow_nr_orig;
 	u32 cs_mode, nr_pages;
 
-	if (!pvt->umc) {
-		csrow_nr >>= 1;
-		cs_mode = DBAM_DIMM(csrow_nr, dbam);
-	} else {
-		cs_mode = f17_get_cs_mode(csrow_nr >> 1, dct, pvt);
-	}
-
+	cs_mode = pvt->ops->get_cs_mode(csrow_nr >> 1, dct, pvt);
 	nr_pages   = pvt->ops->dbam_to_cs(pvt, dct, cs_mode, csrow_nr);
 	nr_pages <<= 20 - PAGE_SHIFT;
 
@@ -3893,9 +3908,6 @@ static int init_csrows(struct mem_ctl_info *mci)
 	int nr_pages = 0;
 	u32 val;
 
-	if (pvt->umc)
-		return init_csrows_df(mci);
-
 	amd64_read_pci_cfg(pvt->F3, NBCFG, &val);
 
 	pvt->nbcfg = val;
@@ -4116,49 +4128,60 @@ static void restore_ecc_error_reporting(struct ecc_settings *s, u16 nid,
 		amd64_warn("Error restoring NB MCGCTL settings!\n");
 }
 
-static bool ecc_enabled(struct amd64_pvt *pvt)
+static bool f1x_check_ecc_enabled(struct amd64_pvt *pvt)
 {
 	u16 nid = pvt->mc_node_id;
 	bool nb_mce_en = false;
-	u8 ecc_en = 0, i;
+	u8 ecc_en = 0;
 	u32 value;
 
-	if (boot_cpu_data.x86 >= 0x17) {
-		u8 umc_en_mask = 0, ecc_en_mask = 0;
-		struct amd64_umc *umc;
+	amd64_read_pci_cfg(pvt->F3, NBCFG, &value);
 
-		for_each_umc(i) {
-			umc = &pvt->umc[i];
+	ecc_en = !!(value & NBCFG_ECC_ENABLE);
 
-			/* Only check enabled UMCs. */
-			if (!(umc->sdp_ctrl & UMC_SDP_INIT))
-				continue;
+	nb_mce_en = nb_mce_bank_enabled_on_node(nid);
+	if (!nb_mce_en)
+		edac_dbg(0, "NB MCE bank disabled, set MSR 0x%08x[4] on node %d to enable.\n",
+			 MSR_IA32_MCG_CTL, nid);
 
-			umc_en_mask |= BIT(i);
+	edac_dbg(3, "Node %d: DRAM ECC %s.\n", nid, (ecc_en ? "enabled" : "disabled"));
 
-			if (umc->umc_cap_hi & UMC_ECC_ENABLED)
-				ecc_en_mask |= BIT(i);
-		}
+	if (!ecc_en || !nb_mce_en)
+		return false;
+	else
+		return true;
+}
 
-		/* Check whether at least one UMC is enabled: */
-		if (umc_en_mask)
-			ecc_en = umc_en_mask == ecc_en_mask;
-		else
-			edac_dbg(0, "Node %d: No enabled UMCs.\n", nid);
+static bool f17_check_ecc_enabled(struct amd64_pvt *pvt)
+{
+	u16 nid = pvt->mc_node_id;
+	struct amd64_umc *umc;
+	u8 umc_en_mask = 0, ecc_en_mask = 0;
+	bool nb_mce_en = false;
+	u8 ecc_en = 0, i;
 
-		/* Assume UMC MCA banks are enabled. */
-		nb_mce_en = true;
-	} else {
-		amd64_read_pci_cfg(pvt->F3, NBCFG, &value);
+	for_each_umc(i) {
+		umc = &pvt->umc[i];
+
+		/* Only check enabled UMCs. */
+		if (!(umc->sdp_ctrl & UMC_SDP_INIT))
+			continue;
 
-		ecc_en = !!(value & NBCFG_ECC_ENABLE);
+		umc_en_mask |= BIT(i);
 
-		nb_mce_en = nb_mce_bank_enabled_on_node(nid);
-		if (!nb_mce_en)
-			edac_dbg(0, "NB MCE bank disabled, set MSR 0x%08x[4] on node %d to enable.\n",
-				     MSR_IA32_MCG_CTL, nid);
+		if (umc->umc_cap_hi & UMC_ECC_ENABLED)
+			ecc_en_mask |= BIT(i);
 	}
 
+	/* Check whether at least one UMC is enabled: */
+	if (umc_en_mask)
+		ecc_en = umc_en_mask == ecc_en_mask;
+	else
+		edac_dbg(0, "Node %d: No enabled UMCs.\n", nid);
+
+	/* Assume UMC MCA banks are enabled. */
+	nb_mce_en = true;
+
 	edac_dbg(3, "Node %d: DRAM ECC %s.\n", nid, (ecc_en ? "enabled" : "disabled"));
 
 	if (!ecc_en || !nb_mce_en)
@@ -4168,7 +4191,17 @@ static bool ecc_enabled(struct amd64_pvt *pvt)
 }
 
 static inline void
-f17h_determine_edac_ctl_cap(struct mem_ctl_info *mci, struct amd64_pvt *pvt)
+f1x_determine_edac_ctl_cap(struct mem_ctl_info *mci, struct amd64_pvt *pvt)
+{
+	if (pvt->nbcap & NBCAP_SECDED)
+		mci->edac_ctl_cap |= EDAC_FLAG_SECDED;
+
+	if (pvt->nbcap & NBCAP_CHIPKILL)
+		mci->edac_ctl_cap |= EDAC_FLAG_S4ECD4ED;
+}
+
+static inline void
+f17_determine_edac_ctl_cap(struct mem_ctl_info *mci, struct amd64_pvt *pvt)
 {
 	u8 i, ecc_en = 1, cpk_en = 1, dev_x4 = 1, dev_x16 = 1;
 
@@ -4198,24 +4231,16 @@ f17h_determine_edac_ctl_cap(struct mem_ctl_info *mci, struct amd64_pvt *pvt)
 	}
 }
 
-static void setup_mci_misc_attrs(struct mem_ctl_info *mci)
+static void f1x_setup_mci_misc_attrs(struct mem_ctl_info *mci)
 {
 	struct amd64_pvt *pvt = mci->pvt_info;
 
 	mci->mtype_cap		= MEM_FLAG_DDR2 | MEM_FLAG_RDDR2;
 	mci->edac_ctl_cap	= EDAC_FLAG_NONE;
 
-	if (pvt->umc) {
-		f17h_determine_edac_ctl_cap(mci, pvt);
-	} else {
-		if (pvt->nbcap & NBCAP_SECDED)
-			mci->edac_ctl_cap |= EDAC_FLAG_SECDED;
+	pvt->ops->determine_edac_ctl_cap(mci, pvt);
 
-		if (pvt->nbcap & NBCAP_CHIPKILL)
-			mci->edac_ctl_cap |= EDAC_FLAG_S4ECD4ED;
-	}
-
-	mci->edac_cap		= determine_edac_cap(pvt);
+	mci->edac_cap		= pvt->ops->determine_edac_cap(pvt);
 	mci->mod_name		= EDAC_MOD_STR;
 	mci->ctl_name		= pvt->ctl_name;
 	mci->dev_name		= pci_name(pvt->F3);
@@ -4245,6 +4270,18 @@ static void per_family_init(struct amd64_pvt *pvt)
 		pvt->ops->early_channel_count		= k8_early_channel_count;
 		pvt->ops->map_sysaddr_to_csrow		= k8_map_sysaddr_to_csrow;
 		pvt->ops->dbam_to_cs			= k8_dbam_to_chip_select;
+		pvt->ops->prep_chip_selects		= k8_prep_chip_selects;
+		pvt->ops->determine_memory_type		= determine_memory_type;
+		pvt->ops->determine_ecc_sym_sz		= f1x_determine_ecc_sym_sz;
+		pvt->ops->ecc_enabled			= f1x_check_ecc_enabled;
+		pvt->ops->determine_edac_ctl_cap	= f1x_determine_edac_ctl_cap;
+		pvt->ops->determine_edac_cap		= f1x_determine_edac_cap;
+		pvt->ops->setup_mci_misc_attrs		= f1x_setup_mci_misc_attrs;
+		pvt->ops->get_cs_mode			= f1x_get_cs_mode;
+		pvt->ops->get_base_mask			= read_dct_base_mask;
+		pvt->ops->dump_misc_regs		= __dump_misc_regs;
+		pvt->ops->get_mc_regs			= read_mc_regs;
+		pvt->ops->populate_csrows		= init_csrows;
 		break;
 
 	case 0x10:
@@ -4255,6 +4292,18 @@ static void per_family_init(struct amd64_pvt *pvt)
 		pvt->ops->early_channel_count		= f1x_early_channel_count;
 		pvt->ops->map_sysaddr_to_csrow		= f1x_map_sysaddr_to_csrow;
 		pvt->ops->dbam_to_cs			= f10_dbam_to_chip_select;
+		pvt->ops->prep_chip_selects		= default_prep_chip_selects;
+		pvt->ops->determine_memory_type		= determine_memory_type;
+		pvt->ops->determine_ecc_sym_sz		= f1x_determine_ecc_sym_sz;
+		pvt->ops->ecc_enabled			= f1x_check_ecc_enabled;
+		pvt->ops->determine_edac_ctl_cap	= f1x_determine_edac_ctl_cap;
+		pvt->ops->determine_edac_cap		= f1x_determine_edac_cap;
+		pvt->ops->setup_mci_misc_attrs		= f1x_setup_mci_misc_attrs;
+		pvt->ops->get_cs_mode			= f1x_get_cs_mode;
+		pvt->ops->get_base_mask			= read_dct_base_mask;
+		pvt->ops->dump_misc_regs		= __dump_misc_regs;
+		pvt->ops->get_mc_regs			= read_mc_regs;
+		pvt->ops->populate_csrows		= init_csrows;
 		break;
 
 	case 0x15:
@@ -4263,11 +4312,13 @@ static void per_family_init(struct amd64_pvt *pvt)
 			pvt->f1_id			= PCI_DEVICE_ID_AMD_15H_M30H_NB_F1;
 			pvt->f2_id			= PCI_DEVICE_ID_AMD_15H_M30H_NB_F2;
 			pvt->ops->dbam_to_cs		= f16_dbam_to_chip_select;
+			pvt->ops->prep_chip_selects	= f15_m30h_prep_chip_selects;
 		} else if (pvt->model == 0x60) {
 			pvt->ctl_name			= "F15h_M60h";
 			pvt->f1_id			= PCI_DEVICE_ID_AMD_15H_M60H_NB_F1;
 			pvt->f2_id			= PCI_DEVICE_ID_AMD_15H_M60H_NB_F2;
 			pvt->ops->dbam_to_cs		= f15_m60h_dbam_to_chip_select;
+			pvt->ops->prep_chip_selects	= default_prep_chip_selects;
 		} else if (pvt->model == 0x13) {
 		/* Richland is only client */
 			return;
@@ -4276,10 +4327,22 @@ static void per_family_init(struct amd64_pvt *pvt)
 			pvt->f1_id			= PCI_DEVICE_ID_AMD_15H_NB_F1;
 			pvt->f2_id			= PCI_DEVICE_ID_AMD_15H_NB_F2;
 			pvt->ops->dbam_to_cs		= f15_dbam_to_chip_select;
+			pvt->ops->prep_chip_selects	= default_prep_chip_selects;
 		}
 		pvt->max_mcs				= 2;
 		pvt->ops->early_channel_count		= f1x_early_channel_count;
 		pvt->ops->map_sysaddr_to_csrow		= f1x_map_sysaddr_to_csrow;
+		pvt->ops->determine_memory_type		= determine_memory_type;
+		pvt->ops->determine_ecc_sym_sz		= f1x_determine_ecc_sym_sz;
+		pvt->ops->ecc_enabled			= f1x_check_ecc_enabled;
+		pvt->ops->determine_edac_ctl_cap	= f1x_determine_edac_ctl_cap;
+		pvt->ops->determine_edac_cap		= f1x_determine_edac_cap;
+		pvt->ops->setup_mci_misc_attrs		= f1x_setup_mci_misc_attrs;
+		pvt->ops->get_cs_mode			= f1x_get_cs_mode;
+		pvt->ops->get_base_mask			= read_dct_base_mask;
+		pvt->ops->dump_misc_regs		= __dump_misc_regs;
+		pvt->ops->get_mc_regs			= read_mc_regs;
+		pvt->ops->populate_csrows		= init_csrows;
 		break;
 
 	case 0x16:
@@ -4296,6 +4359,17 @@ static void per_family_init(struct amd64_pvt *pvt)
 		pvt->ops->early_channel_count		= f1x_early_channel_count;
 		pvt->ops->map_sysaddr_to_csrow		= f1x_map_sysaddr_to_csrow;
 		pvt->ops->dbam_to_cs			= f16_dbam_to_chip_select;
+		pvt->ops->prep_chip_selects		= default_prep_chip_selects;
+		pvt->ops->determine_memory_type		= determine_memory_type;
+		pvt->ops->determine_ecc_sym_sz		= f1x_determine_ecc_sym_sz;
+		pvt->ops->ecc_enabled			= f1x_check_ecc_enabled;
+		pvt->ops->determine_edac_ctl_cap	= f1x_determine_edac_ctl_cap;
+		pvt->ops->determine_edac_cap		= f1x_determine_edac_cap;
+		pvt->ops->get_cs_mode			= f1x_get_cs_mode;
+		pvt->ops->get_base_mask			= read_dct_base_mask;
+		pvt->ops->dump_misc_regs		= __dump_misc_regs;
+		pvt->ops->get_mc_regs			= read_mc_regs;
+		pvt->ops->populate_csrows		= init_csrows;
 		break;
 
 	case 0x17:
@@ -4334,6 +4408,19 @@ static void per_family_init(struct amd64_pvt *pvt)
 	case 0x18:
 		pvt->ops->early_channel_count		= f17_early_channel_count;
 		pvt->ops->dbam_to_cs			= f17_addr_mask_to_cs_size;
+		pvt->ops->prep_chip_selects		= f17_prep_chip_selects;
+		pvt->ops->determine_memory_type		= determine_memory_type_df;
+		pvt->ops->determine_ecc_sym_sz		= f17_determine_ecc_sym_sz;
+		pvt->ops->ecc_enabled			= f17_check_ecc_enabled;
+		pvt->ops->determine_edac_ctl_cap	= f17_determine_edac_ctl_cap;
+		pvt->ops->determine_edac_cap		= f17_determine_edac_cap;
+		pvt->ops->setup_mci_misc_attrs		= f1x_setup_mci_misc_attrs;
+		pvt->ops->get_cs_mode			= f17_get_cs_mode;
+		pvt->ops->get_base_mask			= read_umc_base_mask;
+		pvt->ops->dump_misc_regs		= __dump_misc_regs_df;
+		pvt->ops->get_mc_regs			= __read_mc_regs_df;
+		pvt->ops->populate_csrows		= init_csrows_df;
+		pvt->ops->get_umc_err_info		= update_umc_err_info;
 
 		if (pvt->fam == 0x18) {
 			pvt->ctl_name			= "F18h";
@@ -4376,12 +4463,43 @@ static void per_family_init(struct amd64_pvt *pvt)
 		}
 		pvt->ops->early_channel_count		= f17_early_channel_count;
 		pvt->ops->dbam_to_cs			= f17_addr_mask_to_cs_size;
+		pvt->ops->prep_chip_selects		= f17_prep_chip_selects;
+		pvt->ops->determine_memory_type		= determine_memory_type_df;
+		pvt->ops->determine_ecc_sym_sz		= f17_determine_ecc_sym_sz;
+		pvt->ops->ecc_enabled			= f17_check_ecc_enabled;
+		pvt->ops->determine_edac_ctl_cap	= f17_determine_edac_ctl_cap;
+		pvt->ops->determine_edac_cap		= f17_determine_edac_cap;
+		pvt->ops->setup_mci_misc_attrs		= f1x_setup_mci_misc_attrs;
+		pvt->ops->get_cs_mode			= f17_get_cs_mode;
+		pvt->ops->get_base_mask			= read_umc_base_mask;
+		pvt->ops->dump_misc_regs		= __dump_misc_regs_df;
+		pvt->ops->get_mc_regs			= __read_mc_regs_df;
+		pvt->ops->populate_csrows		= init_csrows_df;
+		pvt->ops->get_umc_err_info		= update_umc_err_info;
 		break;
 
 	default:
 		amd64_err("Unsupported family!\n");
 		return;
 	}
+
+	/* ops required for all the families */
+	if (!pvt->ops->early_channel_count || !pvt->ops->dbam_to_cs ||
+	    !pvt->ops->prep_chip_selects || !pvt->ops->determine_memory_type ||
+	    !pvt->ops->ecc_enabled || !pvt->ops->determine_edac_ctl_cap ||
+	    !pvt->ops->determine_edac_cap || !pvt->ops->setup_mci_misc_attrs ||
+	    !pvt->ops->get_cs_mode || !pvt->ops->get_base_mask ||
+	    !pvt->ops->dump_misc_regs || !pvt->ops->get_mc_regs ||
+	    !pvt->ops->populate_csrows) {
+		edac_dbg(1, "Common helper routines not defined.\n");
+		return;
+	}
+
+	/* ops required for families 17h and later */
+	if (pvt->fam >= 0x17 && !pvt->ops->get_umc_err_info) {
+		edac_dbg(1, "Platform specific helper routines not defined.\n");
+		return;
+	}
 }
 
 static const struct attribute_group *amd64_edac_attr_groups[] = {
@@ -4413,7 +4531,15 @@ static int hw_info_get(struct amd64_pvt *pvt)
 	if (ret)
 		return ret;
 
-	read_mc_regs(pvt);
+	pvt->ops->get_mc_regs(pvt);
+
+	pvt->ops->prep_chip_selects(pvt);
+
+	pvt->ops->get_base_mask(pvt);
+
+	pvt->ops->determine_memory_type(pvt);
+
+	pvt->ops->determine_ecc_sym_sz(pvt);
 
 	return 0;
 }
@@ -4462,9 +4588,9 @@ static int init_one_instance(struct amd64_pvt *pvt)
 	mci->pvt_info = pvt;
 	mci->pdev = &pvt->F3->dev;
 
-	setup_mci_misc_attrs(mci);
+	pvt->ops->setup_mci_misc_attrs(mci);
 
-	if (init_csrows(mci))
+	if (pvt->ops->populate_csrows(mci))
 		mci->edac_cap = EDAC_FLAG_NONE;
 
 	ret = -ENODEV;
@@ -4528,7 +4654,7 @@ static int probe_one_instance(unsigned int nid)
 		goto err_enable;
 	}
 
-	if (!ecc_enabled(pvt)) {
+	if (!pvt->ops->ecc_enabled(pvt)) {
 		ret = -ENODEV;
 
 		if (!ecc_enable_override)
diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
index 4e3f9755bc73..07ff2c6c17c5 100644
--- a/drivers/edac/amd64_edac.h
+++ b/drivers/edac/amd64_edac.h
@@ -481,6 +481,19 @@ struct low_ops {
 				     struct err_info *err);
 	int  (*dbam_to_cs)(struct amd64_pvt *pvt, u8 dct,
 			   unsigned int cs_mode, int cs_mask_nr);
+	void (*prep_chip_selects)(struct amd64_pvt *pvt);
+	void (*determine_memory_type)(struct amd64_pvt *pvt);
+	void (*determine_ecc_sym_sz)(struct amd64_pvt *pvt);
+	bool (*ecc_enabled)(struct amd64_pvt *pvt);
+	void (*determine_edac_ctl_cap)(struct mem_ctl_info *mci, struct amd64_pvt *pvt);
+	unsigned long (*determine_edac_cap)(struct amd64_pvt *pvt);
+	int  (*get_cs_mode)(int dimm, u8 ctrl, struct amd64_pvt *pvt);
+	void (*get_base_mask)(struct amd64_pvt *pvt);
+	void (*dump_misc_regs)(struct amd64_pvt *pvt);
+	void (*get_mc_regs)(struct amd64_pvt *pvt);
+	void (*setup_mci_misc_attrs)(struct mem_ctl_info *mci);
+	int  (*populate_csrows)(struct mem_ctl_info *mci);
+	void (*get_umc_err_info)(struct mce *m, struct err_info *err);
 };
 
 int __amd64_read_pci_cfg_dword(struct pci_dev *pdev, int offset,
-- 
2.25.1

[PATCH v7 06/12] EDAC/amd64: Add AMD heterogeneous family 19h Model 30h-3fh

[Naveen Krishna Chatradhi]

From: Muralidhara M K <muralimk@amd.com>

On heterogeneous systems with AMD CPUs, the data fabrics of the GPUs
are connected directly via custom links.

One such system, where Aldebaran GPU nodes are connected to the
Family 19h, model 30h family of CPU nodes.

Aldebaran GPU support was added to DRM framework
https://lists.freedesktop.org/archives/amd-gfx/2021-February/059694.html

Signed-off-by: Muralidhara M K <muralimk@amd.com>
Signed-off-by: Naveen Krishna Chatradhi <nchatrad@amd.com>
---
Link:
https://lkml.kernel.org/r/20211028130106.15701-6-nchatrad@amd.com

v6->v7:
* split the model specific assignments in patch 5 of v6 series
  


 drivers/edac/amd64_edac.c | 14 ++++++++++++++
 drivers/edac/amd64_edac.h |  2 ++
 2 files changed, 16 insertions(+)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index babd25f29845..54af7e38d26c 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -4454,6 +4454,19 @@ static void per_family_init(struct amd64_pvt *pvt)
 			pvt->f0_id			= PCI_DEVICE_ID_AMD_19H_M10H_DF_F0;
 			pvt->f6_id			= PCI_DEVICE_ID_AMD_19H_M10H_DF_F6;
 			pvt->max_mcs			= 2;
+		} else if (pvt->model >= 0x30 && pvt->model <= 0x3f) {
+			if (pvt->mc_node_id >= amd_nb_num()) {
+				pvt->ctl_name		= "ALDEBARAN";
+				pvt->f0_id		= PCI_DEVICE_ID_AMD_ALDEBARAN_DF_F0;
+				pvt->f6_id		= PCI_DEVICE_ID_AMD_ALDEBARAN_DF_F6;
+				pvt->max_mcs		= 4;
+				goto end_fam;
+			} else {
+				pvt->ctl_name		= "F19h_M30h";
+				pvt->f0_id		= PCI_DEVICE_ID_AMD_19H_DF_F0;
+				pvt->f6_id		= PCI_DEVICE_ID_AMD_19H_DF_F6;
+				pvt->max_mcs		= 8;
+			}
 		} else {
 			pvt->ctl_name			= "F19h";
 			pvt->f0_id			= PCI_DEVICE_ID_AMD_19H_DF_F0;
@@ -4476,6 +4489,7 @@ static void per_family_init(struct amd64_pvt *pvt)
 		pvt->ops->get_mc_regs			= __read_mc_regs_df;
 		pvt->ops->populate_csrows		= init_csrows_df;
 		pvt->ops->get_umc_err_info		= update_umc_err_info;
+ end_fam:
 		break;
 
 	default:
diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
index 07ff2c6c17c5..66f7b5d45a37 100644
--- a/drivers/edac/amd64_edac.h
+++ b/drivers/edac/amd64_edac.h
@@ -130,6 +130,8 @@
 #define PCI_DEVICE_ID_AMD_19H_M10H_DF_F6 0x14b3
 #define PCI_DEVICE_ID_AMD_19H_M50H_DF_F0 0x166a
 #define PCI_DEVICE_ID_AMD_19H_M50H_DF_F6 0x1670
+#define PCI_DEVICE_ID_AMD_ALDEBARAN_DF_F0 0x14d0
+#define PCI_DEVICE_ID_AMD_ALDEBARAN_DF_F6 0x14d6
 
 /*
  * Function 1 - Address Map
-- 
2.25.1

[PATCH v7 07/12] EDAC/amd64: Enumerate Aldebaran GPU nodes by adding family ops

[Naveen Krishna Chatradhi]

On newer heterogeneous systems with AMD CPUs, the data fabrics of the GPUs
are connected directly via custom links.

One such system, where Aldebaran GPU nodes are connected to the
Family 19h, model 30h family of CPU nodes, the Aldebaran GPUs can report
memory errors via SMCA banks.

Aldebaran GPU support was added to DRM framework
https://lists.freedesktop.org/archives/amd-gfx/2021-February/059694.html

The GPU nodes comes with HBM2 memory in-built, ECC support is enabled by
default and the UMCs on GPU node are different from the UMCs on CPU nodes.

GPU specific ops routines are defined to extend the amd64_edac
module to enumerate HBM memory leveraging the existing edac and the
amd64 specific data structures.

The UMC Phys on GPU nodes are enumerated as csrows and the UMC
channels connected to HBM banks are enumerated as ranks.

Cc: Yazen Ghannam <yazen.ghannam@amd.com>
Co-developed-by: Muralidhara M K <muralimk@amd.com>
Signed-off-by: Muralidhara M K <muralimk@amd.com>
Signed-off-by: Naveen Krishna Chatradhi <nchatrad@amd.com>
---
Link:
https://lkml.kernel.org/r/20210823185437.94417-4-nchatrad@amd.com

v6->v7:
* Added GPU specific ops function definitions, based on the refactor.

v5->v6:
* Added to support number of GPU northbridges with amd_gpu_nb_num()

v4->v5:
* Removed else condition in per_family_init for 19h family

v3->v4:
* Split "f17_addr_mask_to_cs_size" instead as did in 3rd patch earlier

v2->v3:
* Bifurcated the GPU code from v2

v1->v2:
* Restored line deletions and handled minor comments
* Modified commit message and some of the function comments
* variable df_inst_id is introduced instead of umc_num

v0->v1:
* Modifed the commit message
* Change the edac_cap
* kept sizes of both cpu and noncpu together
* return success if the !F3 condition true and remove unnecessary validation


 drivers/edac/amd64_edac.c | 285 +++++++++++++++++++++++++++++++++-----
 drivers/edac/amd64_edac.h |  21 +++
 2 files changed, 273 insertions(+), 33 deletions(-)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 54af7e38d26c..10efe726a959 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -1012,6 +1012,12 @@ static int sys_addr_to_csrow(struct mem_ctl_info *mci, u64 sys_addr)
 /* Protect the PCI config register pairs used for DF indirect access. */
 static DEFINE_MUTEX(df_indirect_mutex);
 
+/* Total number of northbridges if in case of heterogeneous systems */
+static int amd_total_nb_num(void)
+{
+	return amd_nb_num() + amd_gpu_nb_num();
+}
+
 /*
  * Data Fabric Indirect Access uses FICAA/FICAD.
  *
@@ -1031,7 +1037,7 @@ static int __df_indirect_read(u16 node, u8 func, u16 reg, u8 instance_id, u32 *l
 	u32 ficaa;
 	int err = -ENODEV;
 
-	if (node >= amd_nb_num())
+	if (node >= amd_total_nb_num())
 		goto out;
 
 	F4 = node_to_amd_nb(node)->link;
@@ -1732,6 +1738,11 @@ static unsigned long f17_determine_edac_cap(struct amd64_pvt *pvt)
 	return edac_cap;
 }
 
+static unsigned long gpu_determine_edac_cap(struct amd64_pvt *pvt)
+{
+	return EDAC_FLAG_EC;
+}
+
 static void debug_display_dimm_sizes(struct amd64_pvt *, u8);
 
 static void debug_dump_dramcfg_low(struct amd64_pvt *pvt, u32 dclr, int chan)
@@ -1814,6 +1825,25 @@ static int f17_get_cs_mode(int dimm, u8 ctrl, struct amd64_pvt *pvt)
 	return cs_mode;
 }
 
+static int gpu_get_cs_mode(int dimm, u8 ctrl, struct amd64_pvt *pvt)
+{
+	return CS_EVEN_PRIMARY | CS_ODD_PRIMARY;
+}
+
+static void gpu_debug_display_dimm_sizes(struct amd64_pvt *pvt, u8 ctrl)
+{
+	int size, cs = 0, cs_mode;
+
+	edac_printk(KERN_DEBUG, EDAC_MC, "UMC%d chip selects:\n", ctrl);
+
+	cs_mode = CS_EVEN_PRIMARY | CS_ODD_PRIMARY;
+
+	for_each_chip_select(cs, ctrl, pvt) {
+		size = pvt->ops->dbam_to_cs(pvt, ctrl, cs_mode, cs);
+		amd64_info(EDAC_MC ": %d: %5dMB\n", cs, size);
+	}
+}
+
 static void debug_display_dimm_sizes_df(struct amd64_pvt *pvt, u8 ctrl)
 {
 	int dimm, size0, size1, cs0, cs1, cs_mode;
@@ -1835,6 +1865,27 @@ static void debug_display_dimm_sizes_df(struct amd64_pvt *pvt, u8 ctrl)
 	}
 }
 
+static void gpu_dump_misc_regs(struct amd64_pvt *pvt)
+{
+	struct amd64_umc *umc;
+	u32 i, umc_base;
+
+	for_each_umc(i) {
+		umc_base = gpu_get_umc_base(i, 0);
+		umc = &pvt->umc[i];
+
+		edac_dbg(1, "UMC%d UMC cfg: 0x%x\n", i, umc->umc_cfg);
+		edac_dbg(1, "UMC%d SDP ctrl: 0x%x\n", i, umc->sdp_ctrl);
+		edac_dbg(1, "UMC%d ECC ctrl: 0x%x\n", i, umc->ecc_ctrl);
+		edac_dbg(1, "UMC%d All HBMs support ECC: yes\n", i);
+
+		gpu_debug_display_dimm_sizes(pvt, i);
+	}
+
+	edac_dbg(1, "F0x104 (DRAM Hole Address): 0x%08x, base: 0x%08x\n",
+		 pvt->dhar, dhar_base(pvt));
+}
+
 static void __dump_misc_regs_df(struct amd64_pvt *pvt)
 {
 	struct amd64_umc *umc;
@@ -1973,6 +2024,43 @@ static void default_prep_chip_selects(struct amd64_pvt *pvt)
 	pvt->csels[1].m_cnt = 4;
 }
 
+static void gpu_prep_chip_selects(struct amd64_pvt *pvt)
+{
+	int umc;
+
+	for_each_umc(umc) {
+		pvt->csels[umc].b_cnt = 8;
+		pvt->csels[umc].m_cnt = 8;
+	}
+}
+
+static void gpu_read_umc_base_mask(struct amd64_pvt *pvt)
+{
+	u32 base_reg, mask_reg;
+	u32 *base, *mask;
+	int umc, cs;
+
+	for_each_umc(umc) {
+		for_each_chip_select(cs, umc, pvt) {
+			base_reg = gpu_get_umc_base(umc, cs) + UMCCH_BASE_ADDR;
+			base = &pvt->csels[umc].csbases[cs];
+
+			if (!amd_smn_read(pvt->mc_node_id, base_reg, base)) {
+				edac_dbg(0, "  DCSB%d[%d]=0x%08x reg: 0x%x\n",
+					 umc, cs, *base, base_reg);
+			}
+
+			mask_reg = gpu_get_umc_base(umc, cs) + UMCCH_ADDR_MASK;
+			mask = &pvt->csels[umc].csmasks[cs];
+
+			if (!amd_smn_read(pvt->mc_node_id, mask_reg, mask)) {
+				edac_dbg(0, "  DCSM%d[%d]=0x%08x reg: 0x%x\n",
+					 umc, cs, *mask, mask_reg);
+			}
+		}
+	}
+}
+
 static void read_umc_base_mask(struct amd64_pvt *pvt)
 {
 	u32 umc_base_reg, umc_base_reg_sec;
@@ -2172,6 +2260,11 @@ static void determine_memory_type(struct amd64_pvt *pvt)
 	pvt->dram_type = (pvt->dclr0 & BIT(16)) ? MEM_DDR3 : MEM_RDDR3;
 }
 
+static void gpu_determine_memory_type(struct amd64_pvt *pvt)
+{
+	pvt->dram_type = MEM_HBM2;
+}
+
 /* Get the number of DCT channels the memory controller is using. */
 static int k8_early_channel_count(struct amd64_pvt *pvt)
 {
@@ -2504,6 +2597,19 @@ static int f17_early_channel_count(struct amd64_pvt *pvt)
 	return channels;
 }
 
+static int gpu_early_channel_count(struct amd64_pvt *pvt)
+{
+	int i, channels = 0;
+
+	/* The memory channels in case of GPUs are fully populated */
+	for_each_umc(i)
+		channels += pvt->csels[i].b_cnt;
+
+	amd64_info("MCT channel count: %d\n", channels);
+
+	return channels;
+}
+
 static int ddr3_cs_size(unsigned i, bool dct_width)
 {
 	unsigned shift = 0;
@@ -2631,11 +2737,46 @@ static int f16_dbam_to_chip_select(struct amd64_pvt *pvt, u8 dct,
 		return ddr3_cs_size(cs_mode, false);
 }
 
+static int __addr_mask_to_cs_size(u32 addr_mask_orig, unsigned int cs_mode,
+				  int csrow_nr, int dimm)
+{
+	u32 msb, weight, num_zero_bits;
+	u32 addr_mask_deinterleaved;
+	int size = 0;
+
+	/*
+	 * The number of zero bits in the mask is equal to the number of bits
+	 * in a full mask minus the number of bits in the current mask.
+	 *
+	 * The MSB is the number of bits in the full mask because BIT[0] is
+	 * always 0.
+	 *
+	 * In the special 3 Rank interleaving case, a single bit is flipped
+	 * without swapping with the most significant bit. This can be handled
+	 * by keeping the MSB where it is and ignoring the single zero bit.
+	 */
+	msb = fls(addr_mask_orig) - 1;
+	weight = hweight_long(addr_mask_orig);
+	num_zero_bits = msb - weight - !!(cs_mode & CS_3R_INTERLEAVE);
+
+	/* Take the number of zero bits off from the top of the mask. */
+	addr_mask_deinterleaved = GENMASK_ULL(msb - num_zero_bits, 1);
+
+	edac_dbg(1, "CS%d DIMM%d AddrMasks:\n", csrow_nr, dimm);
+	edac_dbg(1, "  Original AddrMask: 0x%x\n", addr_mask_orig);
+	edac_dbg(1, "  Deinterleaved AddrMask: 0x%x\n", addr_mask_deinterleaved);
+
+	/* Register [31:1] = Address [39:9]. Size is in kBs here. */
+	size = (addr_mask_deinterleaved >> 2) + 1;
+
+	/* Return size in MBs. */
+	return size >> 10;
+}
+
 static int f17_addr_mask_to_cs_size(struct amd64_pvt *pvt, u8 umc,
 				    unsigned int cs_mode, int csrow_nr)
 {
-	u32 addr_mask_orig, addr_mask_deinterleaved;
-	u32 msb, weight, num_zero_bits;
+	u32 addr_mask_orig;
 	int cs_mask_nr = csrow_nr;
 	int dimm, size = 0;
 
@@ -2680,33 +2821,15 @@ static int f17_addr_mask_to_cs_size(struct amd64_pvt *pvt, u8 umc,
 	else
 		addr_mask_orig = pvt->csels[umc].csmasks[cs_mask_nr];
 
-	/*
-	 * The number of zero bits in the mask is equal to the number of bits
-	 * in a full mask minus the number of bits in the current mask.
-	 *
-	 * The MSB is the number of bits in the full mask because BIT[0] is
-	 * always 0.
-	 *
-	 * In the special 3 Rank interleaving case, a single bit is flipped
-	 * without swapping with the most significant bit. This can be handled
-	 * by keeping the MSB where it is and ignoring the single zero bit.
-	 */
-	msb = fls(addr_mask_orig) - 1;
-	weight = hweight_long(addr_mask_orig);
-	num_zero_bits = msb - weight - !!(cs_mode & CS_3R_INTERLEAVE);
-
-	/* Take the number of zero bits off from the top of the mask. */
-	addr_mask_deinterleaved = GENMASK_ULL(msb - num_zero_bits, 1);
-
-	edac_dbg(1, "CS%d DIMM%d AddrMasks:\n", csrow_nr, dimm);
-	edac_dbg(1, "  Original AddrMask: 0x%x\n", addr_mask_orig);
-	edac_dbg(1, "  Deinterleaved AddrMask: 0x%x\n", addr_mask_deinterleaved);
+	return __addr_mask_to_cs_size(addr_mask_orig, cs_mode, cs_mask_nr, dimm);
+}
 
-	/* Register [31:1] = Address [39:9]. Size is in kBs here. */
-	size = (addr_mask_deinterleaved >> 2) + 1;
+static int gpu_addr_mask_to_cs_size(struct amd64_pvt *pvt, u8 umc,
+				    unsigned int cs_mode, int csrow_nr)
+{
+	u32 addr_mask_orig = pvt->csels[umc].csmasks[csrow_nr];
 
-	/* Return size in MBs. */
-	return size >> 10;
+	return __addr_mask_to_cs_size(addr_mask_orig, cs_mode, csrow_nr, csrow_nr >> 1);
 }
 
 static void read_dram_ctl_register(struct amd64_pvt *pvt)
@@ -3703,6 +3826,11 @@ static void f17_determine_ecc_sym_sz(struct amd64_pvt *pvt)
 	}
 }
 
+/* ECC symbol size is not available on Aldebaran nodes */
+static void gpu_determine_ecc_sym_sz(struct amd64_pvt *pvt)
+{
+}
+
 static void read_top_mem_registers(struct amd64_pvt *pvt)
 {
 	u64 msr_val;
@@ -3724,6 +3852,25 @@ static void read_top_mem_registers(struct amd64_pvt *pvt)
 	}
 }
 
+static void gpu_read_mc_regs(struct amd64_pvt *pvt)
+{
+	u8 nid = pvt->mc_node_id;
+	struct amd64_umc *umc;
+	u32 i, umc_base;
+
+	/* Read registers from each UMC */
+	for_each_umc(i) {
+		umc_base = gpu_get_umc_base(i, 0);
+		umc = &pvt->umc[i];
+
+		amd_smn_read(nid, umc_base + UMCCH_UMC_CFG, &umc->umc_cfg);
+		amd_smn_read(nid, umc_base + UMCCH_SDP_CTRL, &umc->sdp_ctrl);
+		amd_smn_read(nid, umc_base + UMCCH_ECC_CTRL, &umc->ecc_ctrl);
+	}
+
+	amd64_read_pci_cfg(pvt->F0, DF_DHAR, &pvt->dhar);
+}
+
 /*
  * Retrieve the hardware registers of the memory controller.
  */
@@ -3850,6 +3997,35 @@ static u32 get_csrow_nr_pages(struct amd64_pvt *pvt, u8 dct, int csrow_nr_orig)
 	return nr_pages;
 }
 
+static int gpu_init_csrows(struct mem_ctl_info *mci)
+{
+	struct amd64_pvt *pvt = mci->pvt_info;
+	struct dimm_info *dimm;
+	int empty = 1;
+	u8 umc, cs;
+
+	for_each_umc(umc) {
+		for_each_chip_select(cs, umc, pvt) {
+			if (!csrow_enabled(cs, umc, pvt))
+				continue;
+
+			empty = 0;
+			dimm = mci->csrows[umc]->channels[cs]->dimm;
+
+			edac_dbg(1, "MC node: %d, csrow: %d\n",
+				 pvt->mc_node_id, cs);
+
+			dimm->nr_pages = get_csrow_nr_pages(pvt, umc, cs);
+			dimm->mtype = pvt->dram_type;
+			dimm->edac_mode = EDAC_SECDED;
+			dimm->dtype = DEV_X16;
+			dimm->grain = 64;
+		}
+	}
+
+	return empty;
+}
+
 static int init_csrows_df(struct mem_ctl_info *mci)
 {
 	struct amd64_pvt *pvt = mci->pvt_info;
@@ -4190,6 +4366,12 @@ static bool f17_check_ecc_enabled(struct amd64_pvt *pvt)
 		return true;
 }
 
+/* ECC is enabled by default on GPU nodes */
+static bool gpu_check_ecc_enabled(struct amd64_pvt *pvt)
+{
+	return true;
+}
+
 static inline void
 f1x_determine_edac_ctl_cap(struct mem_ctl_info *mci, struct amd64_pvt *pvt)
 {
@@ -4231,6 +4413,12 @@ f17_determine_edac_ctl_cap(struct mem_ctl_info *mci, struct amd64_pvt *pvt)
 	}
 }
 
+static inline void
+gpu_determine_edac_ctl_cap(struct mem_ctl_info *mci, struct amd64_pvt *pvt)
+{
+	mci->edac_ctl_cap |= EDAC_FLAG_SECDED;
+}
+
 static void f1x_setup_mci_misc_attrs(struct mem_ctl_info *mci)
 {
 	struct amd64_pvt *pvt = mci->pvt_info;
@@ -4251,6 +4439,22 @@ static void f1x_setup_mci_misc_attrs(struct mem_ctl_info *mci)
 	mci->get_sdram_scrub_rate = get_scrub_rate;
 }
 
+static void gpu_setup_mci_misc_attrs(struct mem_ctl_info *mci)
+{
+	struct amd64_pvt *pvt = mci->pvt_info;
+
+	mci->mtype_cap		= MEM_FLAG_HBM2;
+	mci->edac_ctl_cap	= EDAC_FLAG_NONE;
+
+	pvt->ops->determine_edac_ctl_cap(mci, pvt);
+
+	mci->edac_cap		= pvt->ops->determine_edac_cap(pvt);
+	mci->mod_name		= EDAC_MOD_STR;
+	mci->ctl_name		= pvt->ctl_name;
+	mci->dev_name		= pci_name(pvt->F3);
+	mci->ctl_page_to_phys	= NULL;
+}
+
 /*
  * returns a pointer to the family descriptor on success, NULL otherwise.
  */
@@ -4460,6 +4664,20 @@ static void per_family_init(struct amd64_pvt *pvt)
 				pvt->f0_id		= PCI_DEVICE_ID_AMD_ALDEBARAN_DF_F0;
 				pvt->f6_id		= PCI_DEVICE_ID_AMD_ALDEBARAN_DF_F6;
 				pvt->max_mcs		= 4;
+				pvt->ops->early_channel_count	= gpu_early_channel_count;
+				pvt->ops->dbam_to_cs		= gpu_addr_mask_to_cs_size;
+				pvt->ops->prep_chip_selects	= gpu_prep_chip_selects;
+				pvt->ops->determine_memory_type	= gpu_determine_memory_type;
+				pvt->ops->determine_ecc_sym_sz	= gpu_determine_ecc_sym_sz;
+				pvt->ops->determine_edac_ctl_cap = gpu_determine_edac_ctl_cap;
+				pvt->ops->determine_edac_cap	= gpu_determine_edac_cap;
+				pvt->ops->setup_mci_misc_attrs	= gpu_setup_mci_misc_attrs;
+				pvt->ops->get_cs_mode		= gpu_get_cs_mode;
+				pvt->ops->ecc_enabled		= gpu_check_ecc_enabled;
+				pvt->ops->get_base_mask		= gpu_read_umc_base_mask;
+				pvt->ops->dump_misc_regs	= gpu_dump_misc_regs;
+				pvt->ops->get_mc_regs		= gpu_read_mc_regs;
+				pvt->ops->populate_csrows	= gpu_init_csrows;
 				goto end_fam;
 			} else {
 				pvt->ctl_name		= "F19h_M30h";
@@ -4581,9 +4799,10 @@ static int init_one_instance(struct amd64_pvt *pvt)
 	if (pvt->channel_count < 0)
 		return ret;
 
+	/* Define layers for CPU and GPU nodes */
 	ret = -ENOMEM;
 	layers[0].type = EDAC_MC_LAYER_CHIP_SELECT;
-	layers[0].size = pvt->csels[0].b_cnt;
+	layers[0].size = amd_gpu_nb_num() ? pvt->max_mcs : pvt->csels[0].b_cnt;
 	layers[0].is_virt_csrow = true;
 	layers[1].type = EDAC_MC_LAYER_CHANNEL;
 
@@ -4592,7 +4811,7 @@ static int init_one_instance(struct amd64_pvt *pvt)
 	 * only one channel. Also, this simplifies handling later for the price
 	 * of a couple of KBs tops.
 	 */
-	layers[1].size = pvt->max_mcs;
+	layers[1].size = amd_gpu_nb_num() ? pvt->csels[0].b_cnt : pvt->max_mcs;
 	layers[1].is_virt_csrow = false;
 
 	mci = edac_mc_alloc(pvt->mc_node_id, ARRAY_SIZE(layers), layers, 0);
@@ -4786,7 +5005,7 @@ static int __init amd64_edac_init(void)
 	opstate_init();
 
 	err = -ENOMEM;
-	ecc_stngs = kcalloc(amd_nb_num(), sizeof(ecc_stngs[0]), GFP_KERNEL);
+	ecc_stngs = kcalloc(amd_total_nb_num(), sizeof(ecc_stngs[0]), GFP_KERNEL);
 	if (!ecc_stngs)
 		goto err_free;
 
@@ -4794,7 +5013,7 @@ static int __init amd64_edac_init(void)
 	if (!msrs)
 		goto err_free;
 
-	for (i = 0; i < amd_nb_num(); i++) {
+	for (i = 0; i < amd_total_nb_num(); i++) {
 		err = probe_one_instance(i);
 		if (err) {
 			/* unwind properly */
@@ -4852,7 +5071,7 @@ static void __exit amd64_edac_exit(void)
 	else
 		amd_unregister_ecc_decoder(decode_bus_error);
 
-	for (i = 0; i < amd_nb_num(); i++)
+	for (i = 0; i < amd_total_nb_num(); i++)
 		remove_one_instance(i);
 
 	kfree(ecc_stngs);
diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
index 66f7b5d45a37..71df08a496d2 100644
--- a/drivers/edac/amd64_edac.h
+++ b/drivers/edac/amd64_edac.h
@@ -653,3 +653,24 @@ static inline u32 dct_sel_baseaddr(struct amd64_pvt *pvt)
  * - GPU UMCs use 1 chip select. So we say UMC = EDAC CSROW.
  * - GPU UMCs use 8 channels. So we say UMC Channel = EDAC Channel.
  */
+static inline u32 gpu_get_umc_base(u8 umc, u8 channel)
+{
+	/*
+	 * On CPUs, there is one channel per UMC, so UMC numbering equals
+	 * channel numbering. On GPUs, there are eight channels per UMC,
+	 * so the channel numbering is different from UMC numbering.
+	 *
+	 * On CPU nodes channels are selected in 6th nibble
+	 * UMC chY[3:0]= [(chY*2 + 1) : (chY*2)]50000;
+	 *
+	 * On GPU nodes channels are selected in 3rd nibble
+	 * HBM chX[3:0]= [Y  ]5X[3:0]000;
+	 * HBM chX[7:4]= [Y+1]5X[3:0]000
+	 */
+	umc *= 2;
+
+	if (channel >= 4)
+		umc++;
+
+	return 0x50000 + (umc << 20) + ((channel % 4) << 12);
+}
-- 
2.25.1

[PATCH v7 08/12] EDAC/amd64: Add Family ops to update GPU csrow and channel info

[Naveen Krishna Chatradhi]

GPU node has 'X' number of PHYs and 'Y' number of channels.
This results in 'X*Y' number of instances in the Data Fabric.
Therefore the Data Fabric ID of an instance in GPU as below:
  df_inst_id = 'X' * number of channels per PHY + 'Y'

On CPUs the Data Fabric ID of an instance on a CPU is equal to the
UMC number. since the UMC number and channel are equal in CPU nodes,
the channel can be used as the Data Fabric ID of the instance.

Cc: Yazen Ghannam <yazen.ghannam@amd.com>
Co-developed-by: Muralidhara M K <muralimk@amd.com>
Signed-off-by: Muralidhara M K <muralimk@amd.com>
Signed-off-by: Naveen Krishna Chatradhi <nchatrad@amd.com>
---
v1->v7:
* New change in v7

 drivers/edac/amd64_edac.c | 60 +++++++++++++++++++++++++++++++++++++--
 drivers/edac/amd64_edac.h |  2 ++
 2 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 10efe726a959..241419a0be93 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -3653,6 +3653,30 @@ static inline void decode_bus_error(int node_id, struct mce *m)
 	__log_ecc_error(mci, &err, ecc_type);
 }
 
+/*
+ * On CPUs, The Data Fabric ID of an instance is equal to the UMC number.
+ * And since the UMC number and channel are equal in CPU nodes, the channel can be used
+ * as the Data Fabric ID of the instance.
+ */
+static int f17_df_inst_id(struct mem_ctl_info *mci, struct amd64_pvt *pvt,
+			  struct err_info *err)
+{
+	return err->channel;
+}
+
+/*
+ * A GPU node has 'X' number of PHYs and 'Y' number of channels.
+ * This results in 'X*Y' number of instances in the Data Fabric.
+ * Therefore the Data Fabric ID of an instance can be found with the following formula:
+ * df_inst_id = 'X' * number of channels per PHY + 'Y'
+ *
+ */
+static int gpu_df_inst_id(struct mem_ctl_info *mci, struct amd64_pvt *pvt,
+			  struct err_info *err)
+{
+	return (err->csrow * pvt->channel_count / mci->nr_csrows) + err->channel;
+}
+
 /*
  * To find the UMC channel represented by this bank we need to match on its
  * instance_id. The instance_id of a bank is held in the lower 32 bits of its
@@ -3670,12 +3694,38 @@ static void update_umc_err_info(struct mce *m, struct err_info *err)
 	err->csrow = m->synd & 0x7;
 }
 
+/*
+ * The CPUs have one channel per UMC, So  UMC number is equivalent to a
+ * channel number. The GPUs have 8 channels per UMC, so the UMC number no
+ * longer works as a channel number.
+ * The channel number within a GPU UMC is given in MCA_IPID[15:12].
+ * However, the IDs are split such that two UMC values go to one UMC, and
+ * the channel numbers are split in two groups of four.
+ *
+ * Refer comment on get_umc_base_gpu() from amd64_edac.h
+ *
+ * For example,
+ * UMC0 CH[3:0] = 0x0005[3:0]000
+ * UMC0 CH[7:4] = 0x0015[3:0]000
+ * UMC1 CH[3:0] = 0x0025[3:0]000
+ * UMC1 CH[7:4] = 0x0035[3:0]000
+ */
+static void gpu_update_umc_err_info(struct mce *m, struct err_info *err)
+{
+	u8 ch = (m->ipid & GENMASK(31, 0)) >> 20;
+	u8 phy = ((m->ipid >> 12) & 0xf);
+
+	err->channel = ch % 2 ? phy + 4 : phy;
+	err->csrow = phy;
+}
+
 static void decode_umc_error(int node_id, struct mce *m)
 {
 	u8 ecc_type = (m->status >> 45) & 0x3;
 	struct mem_ctl_info *mci;
 	struct amd64_pvt *pvt;
 	struct err_info err;
+	u8 df_inst_id;
 	u64 sys_addr;
 
 	mci = edac_mc_find(node_id);
@@ -3705,7 +3755,9 @@ static void decode_umc_error(int node_id, struct mce *m)
 
 	pvt->ops->get_umc_err_info(m, &err);
 
-	if (umc_normaddr_to_sysaddr(m->addr, pvt->mc_node_id, err.channel, &sys_addr)) {
+	df_inst_id = pvt->ops->find_umc_inst_id(mci, pvt, &err);
+
+	if (umc_normaddr_to_sysaddr(m->addr, pvt->mc_node_id, df_inst_id, &sys_addr)) {
 		err.err_code = ERR_NORM_ADDR;
 		goto log_error;
 	}
@@ -4625,6 +4677,7 @@ static void per_family_init(struct amd64_pvt *pvt)
 		pvt->ops->get_mc_regs			= __read_mc_regs_df;
 		pvt->ops->populate_csrows		= init_csrows_df;
 		pvt->ops->get_umc_err_info		= update_umc_err_info;
+		pvt->ops->find_umc_inst_id		= f17_df_inst_id;
 
 		if (pvt->fam == 0x18) {
 			pvt->ctl_name			= "F18h";
@@ -4678,6 +4731,8 @@ static void per_family_init(struct amd64_pvt *pvt)
 				pvt->ops->dump_misc_regs	= gpu_dump_misc_regs;
 				pvt->ops->get_mc_regs		= gpu_read_mc_regs;
 				pvt->ops->populate_csrows	= gpu_init_csrows;
+				pvt->ops->get_umc_err_info	= gpu_update_umc_err_info;
+				pvt->ops->find_umc_inst_id	= gpu_df_inst_id;
 				goto end_fam;
 			} else {
 				pvt->ctl_name		= "F19h_M30h";
@@ -4707,6 +4762,7 @@ static void per_family_init(struct amd64_pvt *pvt)
 		pvt->ops->get_mc_regs			= __read_mc_regs_df;
 		pvt->ops->populate_csrows		= init_csrows_df;
 		pvt->ops->get_umc_err_info		= update_umc_err_info;
+		pvt->ops->find_umc_inst_id		= f17_df_inst_id;
  end_fam:
 		break;
 
@@ -4728,7 +4784,7 @@ static void per_family_init(struct amd64_pvt *pvt)
 	}
 
 	/* ops required for families 17h and later */
-	if (pvt->fam >= 0x17 && !pvt->ops->get_umc_err_info) {
+	if (pvt->fam >= 0x17 && (!pvt->ops->get_umc_err_info || !pvt->ops->find_umc_inst_id)) {
 		edac_dbg(1, "Platform specific helper routines not defined.\n");
 		return;
 	}
diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
index 71df08a496d2..b6177bd5d5ba 100644
--- a/drivers/edac/amd64_edac.h
+++ b/drivers/edac/amd64_edac.h
@@ -496,6 +496,8 @@ struct low_ops {
 	void (*setup_mci_misc_attrs)(struct mem_ctl_info *mci);
 	int  (*populate_csrows)(struct mem_ctl_info *mci);
 	void (*get_umc_err_info)(struct mce *m, struct err_info *err);
+	int  (*find_umc_inst_id)(struct mem_ctl_info *mci, struct amd64_pvt *pvt,
+				 struct err_info *err);
 };
 
 int __amd64_read_pci_cfg_dword(struct pci_dev *pdev, int offset,
-- 
2.25.1

[PATCH v7 09/12] EDAC/amd64: Add check for when to add DRAM base and hole

[Naveen Krishna Chatradhi]

From: Yazen Ghannam <yazen.ghannam@amd.com>

Some DF versions and interleaving modes require the DRAM base address
and hole adjustments to be done before accounting for hashing. Others
require this to be done after.

Add a check for this behavior. Save a boolean in the ctx struct which
can be appropriately set based on DF version or interleaving mode.

Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
Signed-off-by: Naveen Krishna Chatradhi <nchatrad@amd.com>
---
Link:
https://lkml.kernel.org/r/20211028175728.121452-32-yazen.ghannam@amd.com

v4->v7:
* Was patch 31 in v3.

v2->v3:
* New in v3.


 drivers/edac/amd64_edac.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 241419a0be93..0f35c8519555 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -1112,6 +1112,7 @@ struct addr_ctx {
 	u8 intlv_num_sockets;
 	u8 cs_id;
 	u8 node_id_shift;
+	bool late_hole_remove;
 	int (*dehash_addr)(struct addr_ctx *ctx);
 	void (*make_space_for_cs_id)(struct addr_ctx *ctx);
 	void (*insert_cs_id)(struct addr_ctx *ctx);
@@ -1676,7 +1677,7 @@ static int umc_normaddr_to_sysaddr(u64 norm_addr, u16 nid, u8 umc, u64 *sys_addr
 		return -EINVAL;
 	}
 
-	if (add_base_and_hole(&ctx)) {
+	if (!ctx.late_hole_remove && add_base_and_hole(&ctx)) {
 		pr_debug("Failed to add DRAM base address and hole");
 		return -EINVAL;
 	}
@@ -1686,6 +1687,11 @@ static int umc_normaddr_to_sysaddr(u64 norm_addr, u16 nid, u8 umc, u64 *sys_addr
 		return -EINVAL;
 	}
 
+	if (ctx.late_hole_remove && add_base_and_hole(&ctx)) {
+		pr_debug("Failed to add DRAM base address and hole");
+		return -EINVAL;
+	}
+
 	if (addr_over_limit(&ctx)) {
 		pr_debug("Calculated address is over limit");
 		return -EINVAL;
-- 
2.25.1

[PATCH v7 10/12] EDAC/amd64: Save the number of block instances

[Naveen Krishna Chatradhi]

From: Yazen Ghannam <yazen.ghannam@amd.com>

Cache the number of block instances. This value is needed by future DF
versions.

Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
Signed-off-by: Naveen Krishna Chatradhi <nchatrad@amd.com>
---
Link:
https://lkml.kernel.org/r/20211028175728.121452-33-yazen.ghannam@amd.com

v3->v7:
* Was in patch 32 in v3.

v2->v3:
* New in v3.


 drivers/edac/amd64_edac.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 0f35c8519555..8f2b5c8be651 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -1112,6 +1112,7 @@ struct addr_ctx {
 	u8 intlv_num_sockets;
 	u8 cs_id;
 	u8 node_id_shift;
+	u8 num_blk_instances;
 	bool late_hole_remove;
 	int (*dehash_addr)(struct addr_ctx *ctx);
 	void (*make_space_for_cs_id)(struct addr_ctx *ctx);
@@ -1437,6 +1438,17 @@ struct data_fabric_ops df3_ops = {
 
 struct data_fabric_ops *df_ops;
 
+static int get_blk_inst_cnt(struct addr_ctx *ctx)
+{
+	/* Read D18F0x40 (FabricBlockInstanceCount). */
+	if (df_indirect_read_broadcast(0, 0, 0x40, &ctx->tmp))
+		return -EINVAL;
+
+	ctx->num_blk_instances = ctx->tmp & 0xFF;
+
+	return 0;
+}
+
 static int get_dram_offset_reg(struct addr_ctx *ctx)
 {
 	/* Read D18F0x1B4 (DramOffset) */
@@ -1647,6 +1659,11 @@ static int umc_normaddr_to_sysaddr(u64 norm_addr, u16 nid, u8 umc, u64 *sys_addr
 	ctx.nid = nid;
 	ctx.inst_id = umc;
 
+	if (get_blk_inst_cnt(&ctx)) {
+		pr_debug("Failed to get Block Instance Count");
+		return -EINVAL;
+	}
+
 	if (df_ops->get_masks(&ctx)) {
 		pr_debug("Failed to get masks");
 		return -EINVAL;
-- 
2.25.1

[PATCH v7 11/12] EDAC/amd64: Add address translation support for DF3.5

[Naveen Krishna Chatradhi]

From: Muralidhara M K <muralimk@amd.com>

Add support for address translation on Data Fabric version 3.5.

Add new data fabric ops and interleaving modes. Also, adjust how the
DRAM address maps are found early in the translation for certain cases.

Signed-off-by: Muralidhara M K <muralimk@amd.com>
Co-developed-by: Yazen Ghannam <yazen.ghannam@amd.com>
Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
Signed-off-by: Naveen Krishna Chatradhi <nchatrad@amd.com>
---
Link:
https://lkml.kernel.org/r/20211028175728.121452-34-yazen.ghannam@amd.com

v3->v7:
* Was in patch 33 in v3.

v2->v3:
* New in v3. Original version at link above.
* Squashed the following into this patch:
  https://lkml.kernel.org/r/20210630152828.162659-8-nchatrad@amd.com
* Dropped "df_regs" use.
* Set "df_ops" during module init.
* Dropped hard-coded Node ID values.


 drivers/edac/amd64_edac.c | 216 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 209 insertions(+), 7 deletions(-)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 8f2b5c8be651..6e0d617fd95f 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -1004,6 +1004,7 @@ static int sys_addr_to_csrow(struct mem_ctl_info *mci, u64 sys_addr)
 /*
  * Glossary of acronyms used in address translation for Zen-based systems
  *
+ * CCM         =       Cache Coherent Master
  * COD         =       Cluster-on-Die
  * CS          =       Coherent Slave
  * DF          =       Data Fabric
@@ -1084,9 +1085,14 @@ enum intlv_modes {
 	NOHASH_2CH	= 0x01,
 	NOHASH_4CH	= 0x03,
 	NOHASH_8CH	= 0x05,
+	NOHASH_16CH	= 0x07,
+	NOHASH_32CH	= 0x08,
 	HASH_COD4_2CH	= 0x0C,
 	HASH_COD2_4CH	= 0x0D,
 	HASH_COD1_8CH	= 0x0E,
+	HASH_8CH	= 0x1C,
+	HASH_16CH	= 0x1D,
+	HASH_32CH	= 0x1E,
 	DF2_HASH_2CH	= 0x21,
 };
 
@@ -1100,6 +1106,7 @@ struct addr_ctx {
 	u32 reg_limit_addr;
 	u32 reg_fab_id_mask0;
 	u32 reg_fab_id_mask1;
+	u32 reg_fab_id_mask2;
 	u16 cs_fabric_id;
 	u16 die_id_mask;
 	u16 socket_id_mask;
@@ -1436,6 +1443,141 @@ struct data_fabric_ops df3_ops = {
 	.get_component_id_mask		=	get_component_id_mask_df3,
 };
 
+static int dehash_addr_df35(struct addr_ctx *ctx)
+{
+	u8 hashed_bit, intlv_ctl_64k, intlv_ctl_2M, intlv_ctl_1G;
+	u8 num_intlv_bits = ctx->intlv_num_chan;
+	u32 i;
+
+	/* Read D18F0x3F8 (DfGlobalCtrl). */
+	if (df_indirect_read_broadcast(0, 0, 0x3F8, &ctx->tmp))
+		return -EINVAL;
+
+	intlv_ctl_64k = !!((ctx->tmp >> 20) & 0x1);
+	intlv_ctl_2M  = !!((ctx->tmp >> 21) & 0x1);
+	intlv_ctl_1G  = !!((ctx->tmp >> 22) & 0x1);
+
+	/*
+	 * CSSelect[0] = XOR of addr{8,  16, 21, 30};
+	 * CSSelect[1] = XOR of addr{9,  17, 22, 31};
+	 * CSSelect[2] = XOR of addr{10, 18, 23, 32};
+	 * CSSelect[3] = XOR of addr{11, 19, 24, 33}; - 16 and 32 channel only
+	 * CSSelect[4] = XOR of addr{12, 20, 25, 34}; - 32 channel only
+	 */
+	for (i = 0; i < num_intlv_bits; i++) {
+		hashed_bit =	((ctx->ret_addr >> (8 + i)) ^
+				((ctx->ret_addr >> (16 + i)) & intlv_ctl_64k) ^
+				((ctx->ret_addr >> (21 + i)) & intlv_ctl_2M) ^
+				((ctx->ret_addr >> (30 + i)) & intlv_ctl_1G));
+
+		hashed_bit &= BIT(0);
+		if (hashed_bit != ((ctx->ret_addr >> (8 + i)) & BIT(0)))
+			ctx->ret_addr ^= BIT(8 + i);
+	}
+
+	return 0;
+}
+
+static int get_intlv_mode_df35(struct addr_ctx *ctx)
+{
+	ctx->intlv_mode = (ctx->reg_base_addr >> 2) & 0x1F;
+
+	if (ctx->intlv_mode == HASH_COD4_2CH ||
+	    ctx->intlv_mode == HASH_COD2_4CH ||
+	    ctx->intlv_mode == HASH_COD1_8CH) {
+		ctx->make_space_for_cs_id	= make_space_for_cs_id_cod_hash;
+		ctx->insert_cs_id		= insert_cs_id_cod_hash;
+		ctx->dehash_addr		= dehash_addr_df3;
+	} else {
+		ctx->make_space_for_cs_id	= make_space_for_cs_id_simple;
+		ctx->insert_cs_id		= insert_cs_id_simple;
+		ctx->dehash_addr		= dehash_addr_df35;
+	}
+
+	return 0;
+}
+
+static void get_intlv_num_dies_df35(struct addr_ctx *ctx)
+{
+	ctx->intlv_num_dies  = (ctx->reg_base_addr >> 7) & 0x1;
+}
+
+static u8 get_die_id_shift_df35(struct addr_ctx *ctx)
+{
+	return ctx->node_id_shift;
+}
+
+static u8 get_socket_id_shift_df35(struct addr_ctx *ctx)
+{
+	return (ctx->reg_fab_id_mask1 >> 8) & 0xF;
+}
+
+static int get_masks_df35(struct addr_ctx *ctx)
+{
+	/* Read D18F1x150 (SystemFabricIdMask0). */
+	if (df_indirect_read_broadcast(0, 1, 0x150, &ctx->reg_fab_id_mask0))
+		return -EINVAL;
+
+	/* Read D18F1x154 (SystemFabricIdMask1) */
+	if (df_indirect_read_broadcast(0, 1, 0x154, &ctx->reg_fab_id_mask1))
+		return -EINVAL;
+
+	/* Read D18F1x158 (SystemFabricIdMask2) */
+	if (df_indirect_read_broadcast(0, 1, 0x158, &ctx->reg_fab_id_mask2))
+		return -EINVAL;
+
+	ctx->node_id_shift = ctx->reg_fab_id_mask1 & 0xF;
+
+	ctx->die_id_mask = ctx->reg_fab_id_mask2 & 0xFFFF;
+
+	ctx->socket_id_mask = (ctx->reg_fab_id_mask2 >> 16) & 0xFFFF;
+
+	return 0;
+}
+
+static u16 get_dst_fabric_id_df35(struct addr_ctx *ctx)
+{
+	return ctx->reg_limit_addr & 0xFFF;
+}
+
+static int get_cs_fabric_id_df35(struct addr_ctx *ctx)
+{
+	u16 nid = ctx->nid;
+
+	/* Special handling for GPU nodes.*/
+	if (nid >= amd_nb_num()) {
+		if (get_umc_to_cs_mapping(ctx))
+			return -EINVAL;
+
+		/* Need to convert back to the hardware-provided Node ID. */
+		nid -= amd_nb_num();
+		nid += amd_gpu_node_start_id();
+	}
+
+	ctx->cs_fabric_id = ctx->inst_id | (nid << ctx->node_id_shift);
+
+	return 0;
+}
+
+static u16 get_component_id_mask_df35(struct addr_ctx *ctx)
+{
+	return ctx->reg_fab_id_mask0 & 0xFFFF;
+}
+
+struct data_fabric_ops df3point5_ops = {
+	.get_hi_addr_offset		=	get_hi_addr_offset_df3,
+	.get_intlv_mode			=	get_intlv_mode_df35,
+	.get_intlv_addr_sel		=	get_intlv_addr_sel_df3,
+	.get_intlv_num_dies		=	get_intlv_num_dies_df35,
+	.get_intlv_num_sockets		=	get_intlv_num_sockets_df3,
+	.get_masks			=	get_masks_df35,
+	.get_die_id_shift		=	get_die_id_shift_df35,
+	.get_socket_id_shift		=	get_socket_id_shift_df35,
+	.get_dst_fabric_id		=	get_dst_fabric_id_df35,
+	.get_cs_fabric_id		=	get_cs_fabric_id_df35,
+	.get_component_id_mask		=	get_component_id_mask_df35,
+};
+
 struct data_fabric_ops *df_ops;
 
 static int get_blk_inst_cnt(struct addr_ctx *ctx)
@@ -1534,8 +1676,17 @@ static void get_intlv_num_chan(struct addr_ctx *ctx)
 		break;
 	case NOHASH_8CH:
 	case HASH_COD1_8CH:
+	case HASH_8CH:
 		ctx->intlv_num_chan = 3;
 		break;
+	case NOHASH_16CH:
+	case HASH_16CH:
+		ctx->intlv_num_chan = 4;
+		break;
+	case NOHASH_32CH:
+	case HASH_32CH:
+		ctx->intlv_num_chan = 5;
+		break;
 	default:
 		/* Valid interleaving modes where checked earlier. */
 		break;
@@ -1642,6 +1793,44 @@ static int addr_over_limit(struct addr_ctx *ctx)
 	return 0;
 }
 
+static int find_ccm_instance_id(struct addr_ctx *ctx)
+{
+	u32 temp;
+
+	for (ctx->inst_id = 0; ctx->inst_id < ctx->num_blk_instances; ctx->inst_id++) {
+		/* Read D18F0x44 (FabricBlockInstanceInformation0). */
+		if (df_indirect_read_instance(0, 0, 0x44, ctx->inst_id, &temp))
+			return -EINVAL;
+
+		if (temp == 0)
+			continue;
+
+		if ((temp & 0xF) == 0)
+			return 0;
+	}
+
+	return -EINVAL;
+}
+
+#define DF_NUM_DRAM_MAPS_AVAILABLE  16
+static int find_map_reg_by_dstfabricid(struct addr_ctx *ctx)
+{
+	u16 node_id_mask = (ctx->reg_fab_id_mask0 >> 16) & 0xFFFF;
+	u16 dst_fabric_id;
+
+	for (ctx->map_num = 0; ctx->map_num < DF_NUM_DRAM_MAPS_AVAILABLE ; ctx->map_num++) {
+		if (get_dram_addr_map(ctx))
+			continue;
+
+		dst_fabric_id = df_ops->get_dst_fabric_id(ctx);
+
+		if ((dst_fabric_id & node_id_mask) == (ctx->cs_fabric_id & node_id_mask))
+			return 0;
+	}
+
+	return -EINVAL;
+}
+
 static int umc_normaddr_to_sysaddr(u64 norm_addr, u16 nid, u8 umc, u64 *sys_addr)
 {
 	struct addr_ctx ctx;
@@ -1659,6 +1848,9 @@ static int umc_normaddr_to_sysaddr(u64 norm_addr, u16 nid, u8 umc, u64 *sys_addr
 	ctx.nid = nid;
 	ctx.inst_id = umc;
 
+	if (df_ops == &df3point5_ops)
+		ctx.late_hole_remove = true;
+
 	if (get_blk_inst_cnt(&ctx)) {
 		pr_debug("Failed to get Block Instance Count");
 		return -EINVAL;
@@ -1674,14 +1866,22 @@ static int umc_normaddr_to_sysaddr(u64 norm_addr, u16 nid, u8 umc, u64 *sys_addr
 		return -EINVAL;
 	}
 
-	if (remove_dram_offset(&ctx)) {
-		pr_debug("Failed to remove DRAM offset");
-		return -EINVAL;
-	}
+	if (ctx.nid >= amd_nb_num()) {
+		if (find_ccm_instance_id(&ctx))
+			return -EINVAL;
 
-	if (get_dram_addr_map(&ctx)) {
-		pr_debug("Failed to get DRAM address map");
-		return -EINVAL;
+		if (find_map_reg_by_dstfabricid(&ctx))
+			return -EINVAL;
+	} else {
+		if (remove_dram_offset(&ctx)) {
+			pr_debug("Failed to remove DRAM offset");
+			return -EINVAL;
+		}
+
+		if (get_dram_addr_map(&ctx)) {
+			pr_debug("Failed to get DRAM address map");
+			return -EINVAL;
+		}
 	}
 
 	if (df_ops->get_intlv_mode(&ctx)) {
@@ -4756,12 +4956,14 @@ static void per_family_init(struct amd64_pvt *pvt)
 				pvt->ops->populate_csrows	= gpu_init_csrows;
 				pvt->ops->get_umc_err_info	= gpu_update_umc_err_info;
 				pvt->ops->find_umc_inst_id	= gpu_df_inst_id;
+				df_ops				= &df3point5_ops;
 				goto end_fam;
 			} else {
 				pvt->ctl_name		= "F19h_M30h";
 				pvt->f0_id		= PCI_DEVICE_ID_AMD_19H_DF_F0;
 				pvt->f6_id		= PCI_DEVICE_ID_AMD_19H_DF_F6;
 				pvt->max_mcs		= 8;
+				df_ops			= &df3point5_ops;
 			}
 		} else {
 			pvt->ctl_name			= "F19h";
-- 
2.25.1

[PATCH v7 12/12] EDAC/amd64: Add fixed UMC to CS mapping

[Naveen Krishna Chatradhi]

From: Yazen Ghannam <yazen.ghannam@amd.com>

GPU memory address mapping entries in Aldebaran will enable on which
channel the error occurred.

Aldebaran has 2 dies and are enumerated alternatively
 * die0's are enumerated as node 2, 4, 6 and 8
 * die1's are enumerated as node 1, 3, 5 and 7

Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
Signed-off-by: Naveen Krishna Chatradhi <nchatrad@amd.com>
---
Link:
v3->v7:
* Split and fixed UMC to CS mapping from patch 33 in v3.
  https://patchwork.kernel.org/project/linux-edac/patch/20211028175728.121452-34-yazen.ghannam@amd.com/

 drivers/edac/amd64_edac.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 6e0d617fd95f..e0f9f3a4fff8 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -1540,6 +1540,36 @@ static u16 get_dst_fabric_id_df35(struct addr_ctx *ctx)
 	return ctx->reg_limit_addr & 0xFFF;
 }
 
+/* UMC to CS mapping for Aldebaran die[0]s */
+u8 umc_to_cs_mapping_aldebaran_die0[] = { 28, 20, 24, 16, 12, 4, 8, 0,
+					 6, 30, 2, 26, 22, 14, 18, 10,
+					 19, 11, 15, 7, 3, 27, 31, 23,
+					 9, 1, 5, 29, 25, 17, 21, 13};
+
+/* UMC to CS mapping for Aldebaran die[1]s */
+u8 umc_to_cs_mapping_aldebaran_die1[] = { 19, 11, 15, 7, 3, 27, 31, 23,
+					9, 1, 5, 29, 25, 17, 21, 13,
+					28, 20, 24, 16, 12, 4, 8, 0,
+					6, 30, 2, 26, 22, 14, 18, 10};
+
+int get_umc_to_cs_mapping(struct addr_ctx *ctx)
+{
+	if (ctx->inst_id >= sizeof(umc_to_cs_mapping_aldebaran_die0))
+		return -EINVAL;
+
+	/*
+	 * Aldebaran has 2 dies and are enumerated alternatively
+	 * die0's are enumerated as node 2, 4, 6 and 8
+	 * die1's are enumerated as node 1, 3, 5 and 7
+	 */
+	if (ctx->nid % 2)
+		ctx->inst_id = umc_to_cs_mapping_aldebaran_die1[ctx->inst_id];
+	else
+		ctx->inst_id = umc_to_cs_mapping_aldebaran_die0[ctx->inst_id];
+
+	return 0;
+}
+
 static int get_cs_fabric_id_df35(struct addr_ctx *ctx)
 {
 	u16 nid = ctx->nid;
-- 
2.25.1

Re: [PATCH v7 01/12] EDAC/amd64: Document heterogeneous enumeration

[Yazen Ghannam]

On Thu, Feb 03, 2022 at 11:49:31AM -0600, Naveen Krishna Chatradhi wrote:
> From: Muralidhara M K <muralimk@amd.com>
> 
> The Documentation notes have been added in amd64_edac.h and will be
> referring to driver-api wherever needed.

I don't see the comment in amd64_edac.h referring to driver-api/edac.rst. So
I'm not sure what this sentence is saying.

> 
> Explains how the physical topology is enumerated in the software and
> edac module populates the sysfs ABIs.
>

Also, please make sure the message is imperative, e.g "Add...", "Explain...",
etc.
 
> Signed-off-by: Muralidhara M K <muralimk@amd.com>
> Signed-off-by: Naveen Krishna Chatradhi <nchatrad@amd.com>
> ---
> v6->v7:
> * New in v7
> 
>  Documentation/driver-api/edac.rst |   9 +++
>  drivers/edac/amd64_edac.h         | 101 ++++++++++++++++++++++++++++++
>  2 files changed, 110 insertions(+)
> 
> diff --git a/Documentation/driver-api/edac.rst b/Documentation/driver-api/edac.rst
> index b8c742aa0a71..0dd07d0d0e47 100644
> --- a/Documentation/driver-api/edac.rst
> +++ b/Documentation/driver-api/edac.rst
> @@ -106,6 +106,15 @@ will occupy those chip-select rows.
>  This term is avoided because it is unclear when needing to distinguish
>  between chip-select rows and socket sets.
>  
> +* High Bandwidth Memory (HBM)
> +
> +HBM is a new type of memory chip with low power consumption and ultra-wide
> +communication lanes. It uses vertically stacked memory chips (DRAM dies)
> +interconnected by microscopic wires called "through-silicon vias," or TSVs.
> +
> +Several stacks of HBM chips connect to the CPU or GPU through an ultra-fast
> +interconnect called the “interposer". So that HBM’s characteristics are
> +nearly indistinguishable from on-chip integrated RAM.
> 

I think this makes sense.
 
>  Memory Controllers
>  ------------------
> diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
> index 6f8147abfa71..6a112270a84b 100644
> --- a/drivers/edac/amd64_edac.h
> +++ b/drivers/edac/amd64_edac.h
> @@ -559,3 +559,104 @@ static inline u32 dct_sel_baseaddr(struct amd64_pvt *pvt)
>  	}
>  	return (pvt)->dct_sel_lo & 0xFFFFF800;
>  }
> +
> +/*
> + * AMD Heterogeneous system support on EDAC subsystem
> + * --------------------------------------------------
> + *
> + * An AMD heterogeneous system built by connecting the data fabrics of both CPUs
> + * and GPUs via custom xGMI links. So, the Data Fabric on the GPU nodes can be
> + * accessed the same way as the Data Fabric on CPU nodes.
> + *
> + * An Aldebaran GPUs has 2 Data Fabrics, each GPU DF contains four Unified
> + * Memory Controllers (UMC). Each UMC contains eight Channels. Each UMC Channel
> + * controls one 128-bit HBM2e (2GB) channel (equivalent to 8 X 2GB ranks),
> + * this creates a total of 4096-bits of DRAM data bus.
> + *
> + * While UMC is interfacing a 16GB (8H X 2GB DRAM) HBM stack, each UMC channel is

What is "8H"? Is that 8 "high"?

> + * interfacing 2GB of DRAM (represented as rank).
> + *
> + * Memory controllers on AMD GPU nodes can be represented in EDAC is as below:
> + *       GPU DF / GPU Node -> EDAC MC
> + *       GPU UMC           -> EDAC CSROW
> + *       GPU UMC channel   -> EDAC CHANNEL
> + *
> + * Eg: An heterogeneous system with 1 AMD CPU is connected to 4 Aldebaran GPUs using xGMI.
> + *
> + * AMD GPU Nodes are enumerated in sequential order based on the PCI hierarchy, and the
> + * first GPU node is assumed to have an "Node ID" value after CPU Nodes are fully
> + * populated.
> + *
> + * $ ls /sys/devices/system/edac/mc/
> + *	mc0   - CPU MC node 0
> + *	mc1  |
> + *	mc2  |- GPU card[0] => node 0(mc1), node 1(mc2)
> + *	mc3  |
> + *	mc4  |- GPU card[1] => node 0(mc3), node 1(mc4)
> + *	mc5  |
> + *	mc6  |- GPU card[2] => node 0(mc5), node 1(mc6)
> + *	mc7  |
> + *	mc8  |- GPU card[3] => node 0(mc7), node 1(mc8)
> + *
> + * sysfs entries will be populated as below:
> + *
> + *	CPU			# CPU node
> + *	├── mc 0
> + *
> + *	GPU Nodes are enumerated sequentially after CPU nodes are populated
> + *	GPU card 1		# Each Aldebaran GPU has 2 nodes/mcs
> + *	├── mc 1		# GPU node 0 == mc1, Each MC node has 4 UMCs/CSROWs
> + *	│   ├── csrow 0		# UMC 0
> + *	│   │   ├── channel 0	# Each UMC has 8 channels
> + *	│   │   ├── channel 1   # size of each channel is 2 GB, so each UMC has 16 GB
> + *	│   │   ├── channel 2
> + *	│   │   ├── channel 3
> + *	│   │   ├── channel 4
> + *	│   │   ├── channel 5
> + *	│   │   ├── channel 6
> + *	│   │   ├── channel 7
> + *	│   ├── csrow 1		# UMC 1
> + *	│   │   ├── channel 0
> + *	│   │   ├── ..
> + *	│   │   ├── channel 7
> + *	│   ├── ..		..
> + *	│   ├── csrow 3		# UMC 3
> + *	│   │   ├── channel 0
> + *	│   │   ├── ..
> + *	│   │   ├── channel 7
> + *	│   ├── rank 0
> + *	│   ├── ..		..
> + *	│   ├── rank 31		# total 32 ranks/dimms from 4 UMCs
> + *	├
> + *	├── mc 2		# GPU node 1 == mc2
> + *	│   ├── ..		# each GPU has total 64 GB
> + *
> + *	GPU card 2
> + *	├── mc 3
> + *	│   ├── ..
> + *	├── mc 4
> + *	│   ├── ..
> + *
> + *	GPU card 3
> + *	├── mc 5
> + *	│   ├── ..
> + *	├── mc 6
> + *	│   ├── ..
> + *
> + *	GPU card 4
> + *	├── mc 7
> + *	│   ├── ..
> + *	├── mc 8
> + *	│   ├── ..
> + *
> + *
> + * Heterogeneous hardware details for above context as below:
> + * - The CPU UMC (Unified Memory Controller) is mostly the same as the GPU UMC.
> + *   They have chip selects (csrows) and channels. However, the layouts are different
> + *   for performance, physical layout, or other reasons.
> + * - CPU UMCs use 1 channel. So we say UMC = EDAC Channel. This follows the
> + *   marketing speak, example. CPU has X memory channels, etc.
> + * - CPU UMCs use up to 4 chip selects. So we say UMC chip select = EDAC CSROW.
> + * - GPU UMCs use 1 chip select. So we say UMC = EDAC CSROW.
> + * - GPU UMCs use 8 channels. So we say UMC Channel = EDAC Channel.
> + */
> --

This makes sense to me. I'm interested to see if there's any feedback from
others though.

Please fix up the commit message. Otherwise, I think this looks good.

Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>

Thanks,
Yazen 

Re: [PATCH v7 02/12] x86/amd_nb: Add support for northbridges on Aldebaran

[Yazen Ghannam]

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  

Re: [PATCH v7 03/12] EDAC/mce_amd: Extract node id from MCA_IPID

[Yazen Ghannam]

On Thu, Feb 03, 2022 at 11:49:33AM -0600, Naveen Krishna Chatradhi wrote:
> On SMCA banks of the GPU nodes, the 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.
>

Terminology should be consistent. I see "node id" and "node ID" here.
 
...

> +		} else if (bank_type == SMCA_UMC_V2) {
> +			/*
> +			 * SMCA_UMC_V2 exists on GPU nodes, extract the node id
> +			 * from register MCA_IPID[47:44](InstanceIdHi).
> +			 * The InstanceIdHi field represents the instance ID of the GPU.
> +			 * Which needs to be mapped to a value used by Linux,
> +			 * where GPU nodes are simply numerically after the CPU nodes.
> +			 */
> +			node_id = amd_get_gpu_node_system_id(m->ipid);

As mentioned for the previous patch, why not define this function in EDAC?

Thanks,
Yazen

Re: [PATCH v7 03/12] EDAC/mce_amd: Extract node id from MCA_IPID

[Chatradhi, Naveen Krishna]

Hi Yazen

On 2/10/2022 5:01 AM, Yazen Ghannam wrote:
> On Thu, Feb 03, 2022 at 11:49:33AM -0600, Naveen Krishna Chatradhi wrote:
>> On SMCA banks of the GPU nodes, the 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.
>>
> Terminology should be consistent. I see "node id" and "node ID" here.
Will keep it consistent.
>   
> ...
>
>> +		} else if (bank_type == SMCA_UMC_V2) {
>> +			/*
>> +			 * SMCA_UMC_V2 exists on GPU nodes, extract the node id
>> +			 * from register MCA_IPID[47:44](InstanceIdHi).
>> +			 * The InstanceIdHi field represents the instance ID of the GPU.
>> +			 * Which needs to be mapped to a value used by Linux,
>> +			 * where GPU nodes are simply numerically after the CPU nodes.
>> +			 */
>> +			node_id = amd_get_gpu_node_system_id(m->ipid);
> As mentioned for the previous patch, why not define this function in EDAC?

Sure, with recent changes we can move this function to edac. Will wait 
for comments on other patches

in the series and submit next version with feedback addressed.

Regards,

Naveenk

>
> Thanks,
> Yazen

Re: [PATCH v7 04/12] EDAC/amd64: Move struct fam_type variables into amd64_pvt structure

[Yazen Ghannam]

On Thu, Feb 03, 2022 at 11:49:34AM -0600, Naveen Krishna Chatradhi wrote:
> From: Muralidhara M K <muralimk@amd.com>
> 
> On heterogeneous systems, the GPU nodes are probed after the CPU
> nodes and will overwrites the family type set by CPU nodes.
>

s/overwrites/overwrite
 
> Removed struct fam_type and made all family-specific assignments

Remove...make...

> dynamic and get rid of static definitions of family_types and ops,

Should the comma (,) be a period (.)?

> This would simplify adding support for future platforms.
> 
> Signed-off-by: Muralidhara M K <muralimk@amd.com>
> ---
> Link:
> https://lkml.kernel.org/r/20211028130106.15701-5-nchatrad@amd.com
> 
> v6->v7:
> * rebased on top of\
>   https://lore.kernel.org/all/20220202144307.2678405-1-yazen.ghannam@amd.com/
> 
> v4->v5:
> * Added reviewed by Yazen
> 
> v1->v4:
> * New change in v4
> 
> 
>  drivers/edac/amd64_edac.c | 384 +++++++++++++-------------------------
>  drivers/edac/amd64_edac.h |  64 +++----
>  2 files changed, 153 insertions(+), 295 deletions(-)
> 
> diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
> index 782e286d5390..4cac43840ccc 100644
> --- a/drivers/edac/amd64_edac.c
> +++ b/drivers/edac/amd64_edac.c
> @@ -13,11 +13,9 @@ module_param(ecc_enable_override, int, 0644);
>  
>  static struct msr __percpu *msrs;
>  
> -static struct amd64_family_type *fam_type;
> -
> -static inline u32 get_umc_reg(u32 reg)
> +static inline u32 get_umc_reg(u32 reg, struct amd64_pvt *pvt)

Can you please switch the parameters? I think having "pvt" first is more
consistent across this file.

>  {
> -	if (!fam_type->flags.zn_regs_v2)
> +	if (!pvt->flags.zn_regs_v2)
>  		return reg;
>  
>  	switch (reg) {
> @@ -463,7 +461,7 @@ static void get_cs_base_and_mask(struct amd64_pvt *pvt, int csrow, u8 dct,
>  	for (i = 0; i < pvt->csels[dct].m_cnt; i++)
>  
>  #define for_each_umc(i) \
> -	for (i = 0; i < fam_type->max_mcs; i++)
> +	for (i = 0; i < pvt->max_mcs; i++)
>  
>  /*
>   * @input_addr is an InputAddr associated with the node given by mci. Return the
> @@ -1859,7 +1857,7 @@ static void __dump_misc_regs_df(struct amd64_pvt *pvt)
>  
>  		if (umc->dram_type == MEM_LRDDR4 || umc->dram_type == MEM_LRDDR5) {
>  			amd_smn_read(pvt->mc_node_id,
> -				     umc_base + get_umc_reg(UMCCH_ADDR_CFG),
> +				     umc_base + get_umc_reg(UMCCH_ADDR_CFG, pvt),
>  				     &tmp);
>  			edac_dbg(1, "UMC%d LRDIMM %dx rank multiply\n",
>  					i, 1 << ((tmp >> 4) & 0x3));
> @@ -1935,7 +1933,7 @@ static void prep_chip_selects(struct amd64_pvt *pvt)
>  
>  		for_each_umc(umc) {
>  			pvt->csels[umc].b_cnt = 4;
> -			pvt->csels[umc].m_cnt = fam_type->flags.zn_regs_v2 ? 4 : 2;
> +			pvt->csels[umc].m_cnt = pvt->flags.zn_regs_v2 ? 4 : 2;
>  		}
>  
>  	} else {
> @@ -1975,7 +1973,7 @@ static void read_umc_base_mask(struct amd64_pvt *pvt)
>  		}
>  
>  		umc_mask_reg = get_umc_base(umc) + UMCCH_ADDR_MASK;
> -		umc_mask_reg_sec = get_umc_base(umc) + get_umc_reg(UMCCH_ADDR_MASK_SEC);
> +		umc_mask_reg_sec = get_umc_base(umc) + get_umc_reg(UMCCH_ADDR_MASK_SEC, pvt);
>  
>  		for_each_chip_select_mask(cs, umc, pvt) {
>  			mask = &pvt->csels[umc].csmasks[cs];
> @@ -2046,7 +2044,7 @@ static void read_dct_base_mask(struct amd64_pvt *pvt)
>  	}
>  }
>  
> -static void _determine_memory_type_df(struct amd64_umc *umc)
> +static void _determine_memory_type_df(struct amd64_pvt *pvt, struct amd64_umc *umc)
>  {
>  	if (!(umc->sdp_ctrl & UMC_SDP_INIT)) {
>  		umc->dram_type = MEM_EMPTY;
> @@ -2057,7 +2055,7 @@ static void _determine_memory_type_df(struct amd64_umc *umc)
>  	 * Check if the system supports the "DDR Type" field in UMC Config
>  	 * and has DDR5 DIMMs in use.
>  	 */
> -	if (fam_type->flags.zn_regs_v2 && ((umc->umc_cfg & GENMASK(2, 0)) == 0x1)) {
> +	if (pvt->flags.zn_regs_v2 && ((umc->umc_cfg & GENMASK(2, 0)) == 0x1)) {
>  		if (umc->dimm_cfg & BIT(5))
>  			umc->dram_type = MEM_LRDDR5;
>  		else if (umc->dimm_cfg & BIT(4))
> @@ -2082,7 +2080,7 @@ static void determine_memory_type_df(struct amd64_pvt *pvt)
>  	for_each_umc(i) {
>  		umc = &pvt->umc[i];
>  
> -		_determine_memory_type_df(umc);
> +		_determine_memory_type_df(pvt, umc);
>  		edac_dbg(1, "  UMC%d DIMM type: %s\n", i, edac_mem_types[umc->dram_type]);
>  	}
>  }
> @@ -2648,7 +2646,7 @@ static int f17_addr_mask_to_cs_size(struct amd64_pvt *pvt, u8 umc,
>  	 */
>  	dimm = csrow_nr >> 1;
>  
> -	if (!fam_type->flags.zn_regs_v2)
> +	if (!pvt->flags.zn_regs_v2)
>  		cs_mask_nr >>= 1;
>  
>  	/* Asymmetric dual-rank DIMM support. */
> @@ -3268,167 +3266,6 @@ static void debug_display_dimm_sizes(struct amd64_pvt *pvt, u8 ctrl)
>  	}
>  }
>  
> -static struct amd64_family_type family_types[] = {
> -	[K8_CPUS] = {
> -		.ctl_name = "K8",
> -		.f1_id = PCI_DEVICE_ID_AMD_K8_NB_ADDRMAP,
> -		.f2_id = PCI_DEVICE_ID_AMD_K8_NB_MEMCTL,
> -		.max_mcs = 2,
> -		.ops = {
> -			.early_channel_count	= k8_early_channel_count,
> -			.map_sysaddr_to_csrow	= k8_map_sysaddr_to_csrow,
> -			.dbam_to_cs		= k8_dbam_to_chip_select,
> -		}
> -	},
> -	[F10_CPUS] = {
> -		.ctl_name = "F10h",
> -		.f1_id = PCI_DEVICE_ID_AMD_10H_NB_MAP,
> -		.f2_id = PCI_DEVICE_ID_AMD_10H_NB_DRAM,
> -		.max_mcs = 2,
> -		.ops = {
> -			.early_channel_count	= f1x_early_channel_count,
> -			.map_sysaddr_to_csrow	= f1x_map_sysaddr_to_csrow,
> -			.dbam_to_cs		= f10_dbam_to_chip_select,
> -		}
> -	},
> -	[F15_CPUS] = {
> -		.ctl_name = "F15h",
> -		.f1_id = PCI_DEVICE_ID_AMD_15H_NB_F1,
> -		.f2_id = PCI_DEVICE_ID_AMD_15H_NB_F2,
> -		.max_mcs = 2,
> -		.ops = {
> -			.early_channel_count	= f1x_early_channel_count,
> -			.map_sysaddr_to_csrow	= f1x_map_sysaddr_to_csrow,
> -			.dbam_to_cs		= f15_dbam_to_chip_select,
> -		}
> -	},
> -	[F15_M30H_CPUS] = {
> -		.ctl_name = "F15h_M30h",
> -		.f1_id = PCI_DEVICE_ID_AMD_15H_M30H_NB_F1,
> -		.f2_id = PCI_DEVICE_ID_AMD_15H_M30H_NB_F2,
> -		.max_mcs = 2,
> -		.ops = {
> -			.early_channel_count	= f1x_early_channel_count,
> -			.map_sysaddr_to_csrow	= f1x_map_sysaddr_to_csrow,
> -			.dbam_to_cs		= f16_dbam_to_chip_select,
> -		}
> -	},
> -	[F15_M60H_CPUS] = {
> -		.ctl_name = "F15h_M60h",
> -		.f1_id = PCI_DEVICE_ID_AMD_15H_M60H_NB_F1,
> -		.f2_id = PCI_DEVICE_ID_AMD_15H_M60H_NB_F2,
> -		.max_mcs = 2,
> -		.ops = {
> -			.early_channel_count	= f1x_early_channel_count,
> -			.map_sysaddr_to_csrow	= f1x_map_sysaddr_to_csrow,
> -			.dbam_to_cs		= f15_m60h_dbam_to_chip_select,
> -		}
> -	},
> -	[F16_CPUS] = {
> -		.ctl_name = "F16h",
> -		.f1_id = PCI_DEVICE_ID_AMD_16H_NB_F1,
> -		.f2_id = PCI_DEVICE_ID_AMD_16H_NB_F2,
> -		.max_mcs = 2,
> -		.ops = {
> -			.early_channel_count	= f1x_early_channel_count,
> -			.map_sysaddr_to_csrow	= f1x_map_sysaddr_to_csrow,
> -			.dbam_to_cs		= f16_dbam_to_chip_select,
> -		}
> -	},
> -	[F16_M30H_CPUS] = {
> -		.ctl_name = "F16h_M30h",
> -		.f1_id = PCI_DEVICE_ID_AMD_16H_M30H_NB_F1,
> -		.f2_id = PCI_DEVICE_ID_AMD_16H_M30H_NB_F2,
> -		.max_mcs = 2,
> -		.ops = {
> -			.early_channel_count	= f1x_early_channel_count,
> -			.map_sysaddr_to_csrow	= f1x_map_sysaddr_to_csrow,
> -			.dbam_to_cs		= f16_dbam_to_chip_select,
> -		}
> -	},
> -	[F17_CPUS] = {
> -		.ctl_name = "F17h",
> -		.f0_id = PCI_DEVICE_ID_AMD_17H_DF_F0,
> -		.f6_id = PCI_DEVICE_ID_AMD_17H_DF_F6,
> -		.max_mcs = 2,
> -		.ops = {
> -			.early_channel_count	= f17_early_channel_count,
> -			.dbam_to_cs		= f17_addr_mask_to_cs_size,
> -		}
> -	},
> -	[F17_M10H_CPUS] = {
> -		.ctl_name = "F17h_M10h",
> -		.f0_id = PCI_DEVICE_ID_AMD_17H_M10H_DF_F0,
> -		.f6_id = PCI_DEVICE_ID_AMD_17H_M10H_DF_F6,
> -		.max_mcs = 2,
> -		.ops = {
> -			.early_channel_count	= f17_early_channel_count,
> -			.dbam_to_cs		= f17_addr_mask_to_cs_size,
> -		}
> -	},
> -	[F17_M30H_CPUS] = {
> -		.ctl_name = "F17h_M30h",
> -		.f0_id = PCI_DEVICE_ID_AMD_17H_M30H_DF_F0,
> -		.f6_id = PCI_DEVICE_ID_AMD_17H_M30H_DF_F6,
> -		.max_mcs = 8,
> -		.ops = {
> -			.early_channel_count	= f17_early_channel_count,
> -			.dbam_to_cs		= f17_addr_mask_to_cs_size,
> -		}
> -	},
> -	[F17_M60H_CPUS] = {
> -		.ctl_name = "F17h_M60h",
> -		.f0_id = PCI_DEVICE_ID_AMD_17H_M60H_DF_F0,
> -		.f6_id = PCI_DEVICE_ID_AMD_17H_M60H_DF_F6,
> -		.max_mcs = 2,
> -		.ops = {
> -			.early_channel_count	= f17_early_channel_count,
> -			.dbam_to_cs		= f17_addr_mask_to_cs_size,
> -		}
> -	},
> -	[F17_M70H_CPUS] = {
> -		.ctl_name = "F17h_M70h",
> -		.f0_id = PCI_DEVICE_ID_AMD_17H_M70H_DF_F0,
> -		.f6_id = PCI_DEVICE_ID_AMD_17H_M70H_DF_F6,
> -		.max_mcs = 2,
> -		.ops = {
> -			.early_channel_count	= f17_early_channel_count,
> -			.dbam_to_cs		= f17_addr_mask_to_cs_size,
> -		}
> -	},
> -	[F19_CPUS] = {
> -		.ctl_name = "F19h",
> -		.f0_id = PCI_DEVICE_ID_AMD_19H_DF_F0,
> -		.f6_id = PCI_DEVICE_ID_AMD_19H_DF_F6,
> -		.max_mcs = 8,
> -		.ops = {
> -			.early_channel_count	= f17_early_channel_count,
> -			.dbam_to_cs		= f17_addr_mask_to_cs_size,
> -		}
> -	},
> -	[F19_M10H_CPUS] = {
> -		.ctl_name = "F19h_M10h",
> -		.f0_id = PCI_DEVICE_ID_AMD_19H_M10H_DF_F0,
> -		.f6_id = PCI_DEVICE_ID_AMD_19H_M10H_DF_F6,
> -		.max_mcs = 12,
> -		.flags.zn_regs_v2 = 1,
> -		.ops = {
> -			.early_channel_count	= f17_early_channel_count,
> -			.dbam_to_cs		= f17_addr_mask_to_cs_size,
> -		}
> -	},
> -	[F19_M50H_CPUS] = {
> -		.ctl_name = "F19h_M50h",
> -		.f0_id = PCI_DEVICE_ID_AMD_19H_M50H_DF_F0,
> -		.f6_id = PCI_DEVICE_ID_AMD_19H_M50H_DF_F6,
> -		.max_mcs = 2,
> -		.ops = {
> -			.early_channel_count	= f17_early_channel_count,
> -			.dbam_to_cs		= f17_addr_mask_to_cs_size,
> -		}
> -	},
> -};
> -
>  /*
>   * These are tables of eigenvectors (one per line) which can be used for the
>   * construction of the syndrome tables. The modified syndrome search algorithm
> @@ -3850,7 +3687,7 @@ static void __read_mc_regs_df(struct amd64_pvt *pvt)
>  		umc_base = get_umc_base(i);
>  		umc = &pvt->umc[i];
>  
> -		amd_smn_read(nid, umc_base + get_umc_reg(UMCCH_DIMM_CFG), &umc->dimm_cfg);
> +		amd_smn_read(nid, umc_base + get_umc_reg(UMCCH_DIMM_CFG, pvt), &umc->dimm_cfg);
>  		amd_smn_read(nid, umc_base + UMCCH_UMC_CFG, &umc->umc_cfg);
>  		amd_smn_read(nid, umc_base + UMCCH_SDP_CTRL, &umc->sdp_ctrl);
>  		amd_smn_read(nid, umc_base + UMCCH_ECC_CTRL, &umc->ecc_ctrl);
> @@ -4380,7 +4217,7 @@ static void setup_mci_misc_attrs(struct mem_ctl_info *mci)
>  
>  	mci->edac_cap		= determine_edac_cap(pvt);
>  	mci->mod_name		= EDAC_MOD_STR;
> -	mci->ctl_name		= fam_type->ctl_name;
> +	mci->ctl_name		= pvt->ctl_name;
>  	mci->dev_name		= pci_name(pvt->F3);
>  	mci->ctl_page_to_phys	= NULL;
>  
> @@ -4392,7 +4229,7 @@ static void setup_mci_misc_attrs(struct mem_ctl_info *mci)
>  /*
>   * returns a pointer to the family descriptor on success, NULL otherwise.
>   */
> -static struct amd64_family_type *per_family_init(struct amd64_pvt *pvt)
> +static void per_family_init(struct amd64_pvt *pvt)
>  {
>  	pvt->ext_model  = boot_cpu_data.x86_model >> 4;
>  	pvt->stepping	= boot_cpu_data.x86_stepping;
> @@ -4401,109 +4238,150 @@ static struct amd64_family_type *per_family_init(struct amd64_pvt *pvt)
>  
>  	switch (pvt->fam) {
>  	case 0xf:
> -		fam_type	= &family_types[K8_CPUS];
> -		pvt->ops	= &family_types[K8_CPUS].ops;
> +		pvt->ctl_name				= "K8";
> +		pvt->f1_id				= PCI_DEVICE_ID_AMD_K8_NB_ADDRMAP;
> +		pvt->f2_id				= PCI_DEVICE_ID_AMD_K8_NB_MEMCTL;
> +		pvt->max_mcs				= 2;

Most systems have "max_mcs = 2", so this can be the default. It can be set
before the switch statement. Then any systems that have a different value can
overwrite it.

> +		pvt->ops->early_channel_count		= k8_early_channel_count;
> +		pvt->ops->map_sysaddr_to_csrow		= k8_map_sysaddr_to_csrow;
> +		pvt->ops->dbam_to_cs			= k8_dbam_to_chip_select;

I think an argument can be made that the "low_ops" can also be removed. That
way the the "pvt" is flatten even more. But I think that'd make the diff even
bigger without any immediate benefit. I think this module needs a bit of
spring cleaning, so removing "low_ops" can be done then once the current
changes settle down.

>  		break;
>  
>  	case 0x10:
> -		fam_type	= &family_types[F10_CPUS];
> -		pvt->ops	= &family_types[F10_CPUS].ops;
> +		pvt->ctl_name				= "F10h";
> +		pvt->f1_id				= PCI_DEVICE_ID_AMD_10H_NB_MAP;
> +		pvt->f2_id				= PCI_DEVICE_ID_AMD_10H_NB_DRAM;
> +		pvt->max_mcs				= 2;
> +		pvt->ops->early_channel_count		= f1x_early_channel_count;
> +		pvt->ops->map_sysaddr_to_csrow		= f1x_map_sysaddr_to_csrow;
> +		pvt->ops->dbam_to_cs			= f10_dbam_to_chip_select;
>  		break;
>  
>  	case 0x15:
>  		if (pvt->model == 0x30) {
> -			fam_type = &family_types[F15_M30H_CPUS];
> -			pvt->ops = &family_types[F15_M30H_CPUS].ops;
> -			break;
> +			pvt->ctl_name			= "F15h_M30h";
> +			pvt->f1_id			= PCI_DEVICE_ID_AMD_15H_M30H_NB_F1;
> +			pvt->f2_id			= PCI_DEVICE_ID_AMD_15H_M30H_NB_F2;
> +			pvt->ops->dbam_to_cs		= f16_dbam_to_chip_select;
>  		} else if (pvt->model == 0x60) {
> -			fam_type = &family_types[F15_M60H_CPUS];
> -			pvt->ops = &family_types[F15_M60H_CPUS].ops;
> -			break;
> -		/* Richland is only client */
> +			pvt->ctl_name			= "F15h_M60h";
> +			pvt->f1_id			= PCI_DEVICE_ID_AMD_15H_M60H_NB_F1;
> +			pvt->f2_id			= PCI_DEVICE_ID_AMD_15H_M60H_NB_F2;
> +			pvt->ops->dbam_to_cs		= f15_m60h_dbam_to_chip_select;
>  		} else if (pvt->model == 0x13) {
> -			return NULL;
> +		/* Richland is only client */
> +			return;
>  		} else {
> -			fam_type	= &family_types[F15_CPUS];
> -			pvt->ops	= &family_types[F15_CPUS].ops;
> +			pvt->ctl_name			= "F15h";
> +			pvt->f1_id			= PCI_DEVICE_ID_AMD_15H_NB_F1;
> +			pvt->f2_id			= PCI_DEVICE_ID_AMD_15H_NB_F2;
> +			pvt->ops->dbam_to_cs		= f15_dbam_to_chip_select;
>  		}
> +		pvt->max_mcs				= 2;
> +		pvt->ops->early_channel_count		= f1x_early_channel_count;
> +		pvt->ops->map_sysaddr_to_csrow		= f1x_map_sysaddr_to_csrow;
>  		break;
>  
>  	case 0x16:
>  		if (pvt->model == 0x30) {
> -			fam_type = &family_types[F16_M30H_CPUS];
> -			pvt->ops = &family_types[F16_M30H_CPUS].ops;
> -			break;
> +			pvt->ctl_name			= "F16h_M30h";
> +			pvt->f1_id			= PCI_DEVICE_ID_AMD_16H_M30H_NB_F1;
> +			pvt->f2_id			= PCI_DEVICE_ID_AMD_16H_M30H_NB_F2;
> +		} else {
> +			pvt->ctl_name			= "F16h";
> +			pvt->f1_id			= PCI_DEVICE_ID_AMD_16H_NB_F1;
> +			pvt->f2_id			= PCI_DEVICE_ID_AMD_16H_NB_F2;
>  		}
> -		fam_type	= &family_types[F16_CPUS];
> -		pvt->ops	= &family_types[F16_CPUS].ops;
> +		pvt->max_mcs				= 2;
> +		pvt->ops->early_channel_count		= f1x_early_channel_count;
> +		pvt->ops->map_sysaddr_to_csrow		= f1x_map_sysaddr_to_csrow;
> +		pvt->ops->dbam_to_cs			= f16_dbam_to_chip_select;
>  		break;
>  
>  	case 0x17:
>  		if (pvt->model >= 0x10 && pvt->model <= 0x2f) {
> -			fam_type = &family_types[F17_M10H_CPUS];
> -			pvt->ops = &family_types[F17_M10H_CPUS].ops;
> -			df_ops	 = &df2_ops;
> -			break;
> +			pvt->ctl_name			= "F17h_M10h";
> +			pvt->f0_id			= PCI_DEVICE_ID_AMD_17H_M10H_DF_F0;
> +			pvt->f6_id			= PCI_DEVICE_ID_AMD_17H_M10H_DF_F6;
> +			pvt->max_mcs			= 2;
> +			df_ops				= &df2_ops;
>  		} else if (pvt->model >= 0x30 && pvt->model <= 0x3f) {
> -			fam_type = &family_types[F17_M30H_CPUS];
> -			pvt->ops = &family_types[F17_M30H_CPUS].ops;
> -			df_ops	 = &df3_ops;
> -			break;
> +			pvt->ctl_name			= "F17h_M30h";
> +			pvt->f0_id			= PCI_DEVICE_ID_AMD_17H_M30H_DF_F0;
> +			pvt->f6_id			= PCI_DEVICE_ID_AMD_17H_M30H_DF_F6;
> +			pvt->max_mcs			= 8;
> +			df_ops				= &df3_ops;
>  		} else if (pvt->model >= 0x60 && pvt->model <= 0x6f) {
> -			fam_type = &family_types[F17_M60H_CPUS];
> -			pvt->ops = &family_types[F17_M60H_CPUS].ops;
> -			df_ops	 = &df3_ops;
> -			break;
> +			pvt->ctl_name			= "F17h_M60h";
> +			pvt->f0_id			= PCI_DEVICE_ID_AMD_17H_M60H_DF_F0;
> +			pvt->f6_id			= PCI_DEVICE_ID_AMD_17H_M60H_DF_F6;
> +			pvt->max_mcs			= 2;
> +			df_ops				= &df3_ops;
>  		} else if (pvt->model >= 0x70 && pvt->model <= 0x7f) {
> -			fam_type = &family_types[F17_M70H_CPUS];
> -			pvt->ops = &family_types[F17_M70H_CPUS].ops;
> -			df_ops	 = &df3_ops;
> -			break;
> +			pvt->ctl_name			= "F17h_M70h";
> +			pvt->f0_id			= PCI_DEVICE_ID_AMD_17H_M70H_DF_F0;
> +			pvt->f6_id			= PCI_DEVICE_ID_AMD_17H_M70H_DF_F6;
> +			pvt->max_mcs			= 2;
> +			df_ops				= &df3_ops;
> +		} else {
> +			pvt->ctl_name			= "F17h";
> +			pvt->f0_id			= PCI_DEVICE_ID_AMD_17H_DF_F0;
> +			pvt->f6_id			= PCI_DEVICE_ID_AMD_17H_DF_F6;
> +			pvt->max_mcs			= 2;
> +			df_ops				= &df2_ops;
>  		}
>  		fallthrough;
>  	case 0x18:
> -		fam_type	= &family_types[F17_CPUS];
> -		pvt->ops	= &family_types[F17_CPUS].ops;
> -		df_ops		= &df2_ops;
> -
> -		if (pvt->fam == 0x18)
> -			family_types[F17_CPUS].ctl_name = "F18h";
> +		pvt->ops->early_channel_count		= f17_early_channel_count;
> +		pvt->ops->dbam_to_cs			= f17_addr_mask_to_cs_size;
> +
> +		if (pvt->fam == 0x18) {
> +			pvt->ctl_name			= "F18h";
> +			pvt->f0_id			= PCI_DEVICE_ID_AMD_17H_DF_F0;
> +			pvt->f6_id			= PCI_DEVICE_ID_AMD_17H_DF_F6;
> +			pvt->max_mcs			= 2;
> +			df_ops				= &df2_ops;
> +		}
>  		break;
>  
>  	case 0x19:
>  		if (pvt->model >= 0x10 && pvt->model <= 0x1f) {
> -			fam_type = &family_types[F19_M10H_CPUS];
> -			pvt->ops = &family_types[F19_M10H_CPUS].ops;
> -			break;
> +			pvt->ctl_name			= "F19h_M10h";
> +			pvt->f0_id			= PCI_DEVICE_ID_AMD_19H_M10H_DF_F0;
> +			pvt->f6_id			= PCI_DEVICE_ID_AMD_19H_M10H_DF_F6;
> +			pvt->max_mcs			= 12;
> +			pvt->flags.zn_regs_v2		= 1;
>  		} else if (pvt->model >= 0x20 && pvt->model <= 0x2f) {
> -			fam_type = &family_types[F17_M70H_CPUS];
> -			pvt->ops = &family_types[F17_M70H_CPUS].ops;
> -			fam_type->ctl_name = "F19h_M20h";
> -			df_ops	 = &df3_ops;
> -			break;
> +			pvt->ctl_name			= "F19h_M20h";
> +			pvt->f0_id			= PCI_DEVICE_ID_AMD_17H_M70H_DF_F0;
> +			pvt->f6_id			= PCI_DEVICE_ID_AMD_17H_M70H_DF_F6;
> +			pvt->max_mcs			= 2;
> +			df_ops				= &df3_ops;
>  		} else if (pvt->model >= 0x50 && pvt->model <= 0x5f) {
> -			fam_type = &family_types[F19_M50H_CPUS];
> -			pvt->ops = &family_types[F19_M50H_CPUS].ops;
> -			fam_type->ctl_name = "F19h_M50h";
> -			break;
> +			pvt->ctl_name			= "F19h_M50h";
> +			pvt->f0_id			= PCI_DEVICE_ID_AMD_19H_M50H_DF_F0;
> +			pvt->f6_id			= PCI_DEVICE_ID_AMD_19H_M50H_DF_F6;
> +			pvt->max_mcs			= 2;
>  		} else if (pvt->model >= 0xa0 && pvt->model <= 0xaf) {
> -			fam_type = &family_types[F19_M10H_CPUS];
> -			pvt->ops = &family_types[F19_M10H_CPUS].ops;
> -			fam_type->ctl_name = "F19h_MA0h";
> -			break;
> +			pvt->ctl_name			= "F19h_M10h";

Should be "F19h_MA0h".

> +			pvt->f0_id			= PCI_DEVICE_ID_AMD_19H_M10H_DF_F0;
> +			pvt->f6_id			= PCI_DEVICE_ID_AMD_19H_M10H_DF_F6;
> +			pvt->max_mcs			= 2;

Should be "12".

> +		} else {
> +			pvt->ctl_name			= "F19h";
> +			pvt->f0_id			= PCI_DEVICE_ID_AMD_19H_DF_F0;
> +			pvt->f6_id			= PCI_DEVICE_ID_AMD_19H_DF_F6;
> +			pvt->max_mcs			= 8;
> +			df_ops				= &df3_ops;
>  		}
> -		fam_type	= &family_types[F19_CPUS];
> -		pvt->ops	= &family_types[F19_CPUS].ops;
> -		family_types[F19_CPUS].ctl_name = "F19h";
> -		df_ops		= &df3_ops;
> +		pvt->ops->early_channel_count		= f17_early_channel_count;
> +		pvt->ops->dbam_to_cs			= f17_addr_mask_to_cs_size;
>  		break;
>  
>  	default:
>  		amd64_err("Unsupported family!\n");
> -		return NULL;
> +		return;

This should return an error code.

>  	}
> -
> -	return fam_type;
>  }
>  
>  static const struct attribute_group *amd64_edac_attr_groups[] = {
> @@ -4520,15 +4398,15 @@ static int hw_info_get(struct amd64_pvt *pvt)
>  	int ret;
>  
>  	if (pvt->fam >= 0x17) {
> -		pvt->umc = kcalloc(fam_type->max_mcs, sizeof(struct amd64_umc), GFP_KERNEL);
> +		pvt->umc = kcalloc(pvt->max_mcs, sizeof(struct amd64_umc), GFP_KERNEL);
>  		if (!pvt->umc)
>  			return -ENOMEM;
>  
> -		pci_id1 = fam_type->f0_id;
> -		pci_id2 = fam_type->f6_id;
> +		pci_id1 = pvt->f0_id;
> +		pci_id2 = pvt->f6_id;
>  	} else {
> -		pci_id1 = fam_type->f1_id;
> -		pci_id2 = fam_type->f2_id;
> +		pci_id1 = pvt->f1_id;
> +		pci_id2 = pvt->f2_id;
>  	}
>  
>  	ret = reserve_mc_sibling_devs(pvt, pci_id1, pci_id2);
> @@ -4574,7 +4452,7 @@ static int init_one_instance(struct amd64_pvt *pvt)
>  	 * only one channel. Also, this simplifies handling later for the price
>  	 * of a couple of KBs tops.
>  	 */
> -	layers[1].size = fam_type->max_mcs;
> +	layers[1].size = pvt->max_mcs;
>  	layers[1].is_virt_csrow = false;
>  
>  	mci = edac_mc_alloc(pvt->mc_node_id, ARRAY_SIZE(layers), layers, 0);
> @@ -4604,7 +4482,7 @@ static bool instance_has_memory(struct amd64_pvt *pvt)
>  	bool cs_enabled = false;
>  	int cs = 0, dct = 0;
>  
> -	for (dct = 0; dct < fam_type->max_mcs; dct++) {
> +	for (dct = 0; dct < pvt->max_mcs; dct++) {
>  		for_each_chip_select(cs, dct, pvt)
>  			cs_enabled |= csrow_enabled(cs, dct, pvt);
>  	}
> @@ -4633,10 +4511,12 @@ static int probe_one_instance(unsigned int nid)
>  	pvt->mc_node_id	= nid;
>  	pvt->F3 = F3;
>  
> +	pvt->ops = kzalloc(sizeof(*pvt->ops), GFP_KERNEL);
> +	if (!pvt->ops)
> +		goto err_out;
> +
>  	ret = -ENODEV;
> -	fam_type = per_family_init(pvt);
> -	if (!fam_type)
> -		goto err_enable;
> +	per_family_init(pvt);

This should check for an error code. The module should not continue to load if
it doesn't have the necessary information for the system.

>  
>  	ret = hw_info_get(pvt);
>  	if (ret < 0)
> @@ -4674,8 +4554,8 @@ static int probe_one_instance(unsigned int nid)
>  		goto err_enable;
>  	}
>  
> -	amd64_info("%s %sdetected (node %d).\n", fam_type->ctl_name,
> -		     (pvt->fam == 0xf ?
> +	amd64_info("%s %sdetected (node %d).\n", pvt->ctl_name,
> +		   (pvt->fam == 0xf ?
>  				(pvt->ext_model >= K8_REV_F  ? "revF or later "
>  							     : "revE or earlier ")
>  				 : ""), pvt->mc_node_id);

This "ext_model" check can be baked into the ctl_name during
per_family_init().

Thanks,
Yazen

Re: [PATCH v7 05/12] EDAC/amd64: Define dynamic family ops routines

[Yazen Ghannam]

On Thu, Feb 03, 2022 at 11:49:35AM -0600, Naveen Krishna Chatradhi wrote:
> From: Muralidhara M K <muralimk@amd.com>
> 
> Extending family-specific assignments dynamic.

The commit message doesn't clearly describe what the patch is about.

> This would simplify adding support for future platforms.
> 
> Signed-off-by: Muralidhara M K <muralimk@amd.com>
> Signed-off-by: Naveen Krishna Chatradhi <nchatrad@amd.com>
> ---

...

> --- a/drivers/edac/amd64_edac.h
> +++ b/drivers/edac/amd64_edac.h
> @@ -481,6 +481,19 @@ struct low_ops {
>  				     struct err_info *err);
>  	int  (*dbam_to_cs)(struct amd64_pvt *pvt, u8 dct,
>  			   unsigned int cs_mode, int cs_mask_nr);
> +	void (*prep_chip_selects)(struct amd64_pvt *pvt);
> +	void (*determine_memory_type)(struct amd64_pvt *pvt);
> +	void (*determine_ecc_sym_sz)(struct amd64_pvt *pvt);
> +	bool (*ecc_enabled)(struct amd64_pvt *pvt);
> +	void (*determine_edac_ctl_cap)(struct mem_ctl_info *mci, struct amd64_pvt *pvt);
> +	unsigned long (*determine_edac_cap)(struct amd64_pvt *pvt);
> +	int  (*get_cs_mode)(int dimm, u8 ctrl, struct amd64_pvt *pvt);
> +	void (*get_base_mask)(struct amd64_pvt *pvt);
> +	void (*dump_misc_regs)(struct amd64_pvt *pvt);
> +	void (*get_mc_regs)(struct amd64_pvt *pvt);
> +	void (*setup_mci_misc_attrs)(struct mem_ctl_info *mci);
> +	int  (*populate_csrows)(struct mem_ctl_info *mci);
> +	void (*get_umc_err_info)(struct mce *m, struct err_info *err);
>  };

I think there should be a patch for breaking out each of these functions.

Thanks,
Yazen

Re: [PATCH v7 06/12] EDAC/amd64: Add AMD heterogeneous family 19h Model 30h-3fh

[Yazen Ghannam]

On Thu, Feb 03, 2022 at 11:49:36AM -0600, Naveen Krishna Chatradhi wrote:
> From: Muralidhara M K <muralimk@amd.com>
> 
> On heterogeneous systems with AMD CPUs, the data fabrics of the GPUs
> are connected directly via custom links.
> 
> One such system, where Aldebaran GPU nodes are connected to the
> Family 19h, model 30h family of CPU nodes.
> 
> Aldebaran GPU support was added to DRM framework
> https://lists.freedesktop.org/archives/amd-gfx/2021-February/059694.html
>

This message doesn't describe the patch.
 
> Signed-off-by: Muralidhara M K <muralimk@amd.com>
> Signed-off-by: Naveen Krishna Chatradhi <nchatrad@amd.com>
> ---
> Link:
> https://lkml.kernel.org/r/20211028130106.15701-6-nchatrad@amd.com
> 
> v6->v7:
> * split the model specific assignments in patch 5 of v6 series
>   
> 
> 
>  drivers/edac/amd64_edac.c | 14 ++++++++++++++
>  drivers/edac/amd64_edac.h |  2 ++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
> index babd25f29845..54af7e38d26c 100644
> --- a/drivers/edac/amd64_edac.c
> +++ b/drivers/edac/amd64_edac.c
> @@ -4454,6 +4454,19 @@ static void per_family_init(struct amd64_pvt *pvt)
>  			pvt->f0_id			= PCI_DEVICE_ID_AMD_19H_M10H_DF_F0;
>  			pvt->f6_id			= PCI_DEVICE_ID_AMD_19H_M10H_DF_F6;
>  			pvt->max_mcs			= 2;
> +		} else if (pvt->model >= 0x30 && pvt->model <= 0x3f) {
> +			if (pvt->mc_node_id >= amd_nb_num()) {

"ALDEBARAN" is a specific device with unique PCI IDs. So this dependency on
amd_nb.c can be removed.

For example,
			if (pvt->F3->device == PCI_DEVICE_ID_AMD_ALDEBARAN_DF_F3)
				...

> +				pvt->ctl_name		= "ALDEBARAN";
> +				pvt->f0_id		= PCI_DEVICE_ID_AMD_ALDEBARAN_DF_F0;
> +				pvt->f6_id		= PCI_DEVICE_ID_AMD_ALDEBARAN_DF_F6;
> +				pvt->max_mcs		= 4;
> +				goto end_fam;

Why not just "break" here instead of jumping to the break below?

> +			} else {
> +				pvt->ctl_name		= "F19h_M30h";
> +				pvt->f0_id		= PCI_DEVICE_ID_AMD_19H_DF_F0;
> +				pvt->f6_id		= PCI_DEVICE_ID_AMD_19H_DF_F6;
> +				pvt->max_mcs		= 8;
> +			}
>  		} else {
>  			pvt->ctl_name			= "F19h";
>  			pvt->f0_id			= PCI_DEVICE_ID_AMD_19H_DF_F0;
> @@ -4476,6 +4489,7 @@ static void per_family_init(struct amd64_pvt *pvt)
>  		pvt->ops->get_mc_regs			= __read_mc_regs_df;
>  		pvt->ops->populate_csrows		= init_csrows_df;
>  		pvt->ops->get_umc_err_info		= update_umc_err_info;
> + end_fam:
>  		break;
>  
>  	default:

Thanks,
Yazen

Re: [PATCH v7 07/12] EDAC/amd64: Enumerate Aldebaran GPU nodes by adding family ops

[Yazen Ghannam]

On Thu, Feb 03, 2022 at 11:49:37AM -0600, Naveen Krishna Chatradhi wrote:
> On newer heterogeneous systems with AMD CPUs, the data fabrics of the GPUs
> are connected directly via custom links.
> 
> One such system, where Aldebaran GPU nodes are connected to the
> Family 19h, model 30h family of CPU nodes, the Aldebaran GPUs can report
> memory errors via SMCA banks.
> 
> Aldebaran GPU support was added to DRM framework
> https://lists.freedesktop.org/archives/amd-gfx/2021-February/059694.html
> 
> The GPU nodes comes with HBM2 memory in-built, ECC support is enabled by
> default and the UMCs on GPU node are different from the UMCs on CPU nodes.
> 
> GPU specific ops routines are defined to extend the amd64_edac

This should be imperative, i.e. "Define GPU-specific ops..."

> module to enumerate HBM memory leveraging the existing edac and the
> amd64 specific data structures.
> 
> The UMC Phys on GPU nodes are enumerated as csrows and the UMC
> channels connected to HBM banks are enumerated as ranks.
> 
> Cc: Yazen Ghannam <yazen.ghannam@amd.com>
> Co-developed-by: Muralidhara M K <muralimk@amd.com>
> Signed-off-by: Muralidhara M K <muralimk@amd.com>
> Signed-off-by: Naveen Krishna Chatradhi <nchatrad@amd.com>
> ---
> Link:
> https://lkml.kernel.org/r/20210823185437.94417-4-nchatrad@amd.com
> 
> v6->v7:
> * Added GPU specific ops function definitions, based on the refactor.
> 
> v5->v6:
> * Added to support number of GPU northbridges with amd_gpu_nb_num()
> 
> v4->v5:
> * Removed else condition in per_family_init for 19h family
> 
> v3->v4:
> * Split "f17_addr_mask_to_cs_size" instead as did in 3rd patch earlier
> 
> v2->v3:
> * Bifurcated the GPU code from v2
> 
> v1->v2:
> * Restored line deletions and handled minor comments
> * Modified commit message and some of the function comments
> * variable df_inst_id is introduced instead of umc_num
> 
> v0->v1:
> * Modifed the commit message
> * Change the edac_cap
> * kept sizes of both cpu and noncpu together
> * return success if the !F3 condition true and remove unnecessary validation
> 
> 
>  drivers/edac/amd64_edac.c | 285 +++++++++++++++++++++++++++++++++-----
>  drivers/edac/amd64_edac.h |  21 +++
>  2 files changed, 273 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
> index 54af7e38d26c..10efe726a959 100644
> --- a/drivers/edac/amd64_edac.c
> +++ b/drivers/edac/amd64_edac.c
> @@ -1012,6 +1012,12 @@ static int sys_addr_to_csrow(struct mem_ctl_info *mci, u64 sys_addr)
>  /* Protect the PCI config register pairs used for DF indirect access. */
>  static DEFINE_MUTEX(df_indirect_mutex);
>  
> +/* Total number of northbridges if in case of heterogeneous systems */
> +static int amd_total_nb_num(void)
> +{
> +	return amd_nb_num() + amd_gpu_nb_num();
> +}
> +
>  /*
>   * Data Fabric Indirect Access uses FICAA/FICAD.
>   *
> @@ -1031,7 +1037,7 @@ static int __df_indirect_read(u16 node, u8 func, u16 reg, u8 instance_id, u32 *l
>  	u32 ficaa;
>  	int err = -ENODEV;
>  
> -	if (node >= amd_nb_num())
> +	if (node >= amd_total_nb_num())
>  		goto out;
>  
>  	F4 = node_to_amd_nb(node)->link;
> @@ -1732,6 +1738,11 @@ static unsigned long f17_determine_edac_cap(struct amd64_pvt *pvt)
>  	return edac_cap;
>  }
>  
> +static unsigned long gpu_determine_edac_cap(struct amd64_pvt *pvt)
> +{
> +	return EDAC_FLAG_EC;
> +}
> +
>  static void debug_display_dimm_sizes(struct amd64_pvt *, u8);
>  
>  static void debug_dump_dramcfg_low(struct amd64_pvt *pvt, u32 dclr, int chan)
> @@ -1814,6 +1825,25 @@ static int f17_get_cs_mode(int dimm, u8 ctrl, struct amd64_pvt *pvt)
>  	return cs_mode;
>  }
>  
> +static int gpu_get_cs_mode(int dimm, u8 ctrl, struct amd64_pvt *pvt)
> +{
> +	return CS_EVEN_PRIMARY | CS_ODD_PRIMARY;
> +}
> +
> +static void gpu_debug_display_dimm_sizes(struct amd64_pvt *pvt, u8 ctrl)
> +{
> +	int size, cs = 0, cs_mode;
> +
> +	edac_printk(KERN_DEBUG, EDAC_MC, "UMC%d chip selects:\n", ctrl);
> +
> +	cs_mode = CS_EVEN_PRIMARY | CS_ODD_PRIMARY;

Why not call gpu_get_cs_mode() here?

> +
> +	for_each_chip_select(cs, ctrl, pvt) {
> +		size = pvt->ops->dbam_to_cs(pvt, ctrl, cs_mode, cs);
> +		amd64_info(EDAC_MC ": %d: %5dMB\n", cs, size);
> +	}
> +}
> +
>  static void debug_display_dimm_sizes_df(struct amd64_pvt *pvt, u8 ctrl)
>  {
>  	int dimm, size0, size1, cs0, cs1, cs_mode;
> @@ -1835,6 +1865,27 @@ static void debug_display_dimm_sizes_df(struct amd64_pvt *pvt, u8 ctrl)
>  	}
>  }
>  
> +static void gpu_dump_misc_regs(struct amd64_pvt *pvt)
> +{
> +	struct amd64_umc *umc;
> +	u32 i, umc_base;
> +
> +	for_each_umc(i) {
> +		umc_base = gpu_get_umc_base(i, 0);
> +		umc = &pvt->umc[i];
> +
> +		edac_dbg(1, "UMC%d UMC cfg: 0x%x\n", i, umc->umc_cfg);
> +		edac_dbg(1, "UMC%d SDP ctrl: 0x%x\n", i, umc->sdp_ctrl);
> +		edac_dbg(1, "UMC%d ECC ctrl: 0x%x\n", i, umc->ecc_ctrl);
> +		edac_dbg(1, "UMC%d All HBMs support ECC: yes\n", i);
> +
> +		gpu_debug_display_dimm_sizes(pvt, i);
> +	}
> +
> +	edac_dbg(1, "F0x104 (DRAM Hole Address): 0x%08x, base: 0x%08x\n",
> +		 pvt->dhar, dhar_base(pvt));
> +}
> +
>  static void __dump_misc_regs_df(struct amd64_pvt *pvt)
>  {
>  	struct amd64_umc *umc;
> @@ -1973,6 +2024,43 @@ static void default_prep_chip_selects(struct amd64_pvt *pvt)
>  	pvt->csels[1].m_cnt = 4;
>  }
>  
> +static void gpu_prep_chip_selects(struct amd64_pvt *pvt)
> +{
> +	int umc;
> +
> +	for_each_umc(umc) {
> +		pvt->csels[umc].b_cnt = 8;
> +		pvt->csels[umc].m_cnt = 8;
> +	}
> +}
> +
> +static void gpu_read_umc_base_mask(struct amd64_pvt *pvt)
> +{
> +	u32 base_reg, mask_reg;
> +	u32 *base, *mask;
> +	int umc, cs;
> +
> +	for_each_umc(umc) {
> +		for_each_chip_select(cs, umc, pvt) {
> +			base_reg = gpu_get_umc_base(umc, cs) + UMCCH_BASE_ADDR;
> +			base = &pvt->csels[umc].csbases[cs];
> +
> +			if (!amd_smn_read(pvt->mc_node_id, base_reg, base)) {
> +				edac_dbg(0, "  DCSB%d[%d]=0x%08x reg: 0x%x\n",
> +					 umc, cs, *base, base_reg);
> +			}
> +
> +			mask_reg = gpu_get_umc_base(umc, cs) + UMCCH_ADDR_MASK;
> +			mask = &pvt->csels[umc].csmasks[cs];
> +
> +			if (!amd_smn_read(pvt->mc_node_id, mask_reg, mask)) {
> +				edac_dbg(0, "  DCSM%d[%d]=0x%08x reg: 0x%x\n",
> +					 umc, cs, *mask, mask_reg);
> +			}
> +		}
> +	}
> +}
> +
>  static void read_umc_base_mask(struct amd64_pvt *pvt)
>  {
>  	u32 umc_base_reg, umc_base_reg_sec;
> @@ -2172,6 +2260,11 @@ static void determine_memory_type(struct amd64_pvt *pvt)
>  	pvt->dram_type = (pvt->dclr0 & BIT(16)) ? MEM_DDR3 : MEM_RDDR3;
>  }
>  
> +static void gpu_determine_memory_type(struct amd64_pvt *pvt)
> +{
> +	pvt->dram_type = MEM_HBM2;
> +}
> +
>  /* Get the number of DCT channels the memory controller is using. */
>  static int k8_early_channel_count(struct amd64_pvt *pvt)
>  {
> @@ -2504,6 +2597,19 @@ static int f17_early_channel_count(struct amd64_pvt *pvt)
>  	return channels;
>  }
>  
> +static int gpu_early_channel_count(struct amd64_pvt *pvt)
> +{
> +	int i, channels = 0;
> +
> +	/* The memory channels in case of GPUs are fully populated */
> +	for_each_umc(i)
> +		channels += pvt->csels[i].b_cnt;
> +
> +	amd64_info("MCT channel count: %d\n", channels);
> +
> +	return channels;
> +}
> +
>  static int ddr3_cs_size(unsigned i, bool dct_width)
>  {
>  	unsigned shift = 0;
> @@ -2631,11 +2737,46 @@ static int f16_dbam_to_chip_select(struct amd64_pvt *pvt, u8 dct,
>  		return ddr3_cs_size(cs_mode, false);
>  }
>  
> +static int __addr_mask_to_cs_size(u32 addr_mask_orig, unsigned int cs_mode,
> +				  int csrow_nr, int dimm)
> +{
> +	u32 msb, weight, num_zero_bits;
> +	u32 addr_mask_deinterleaved;
> +	int size = 0;
> +
> +	/*
> +	 * The number of zero bits in the mask is equal to the number of bits
> +	 * in a full mask minus the number of bits in the current mask.
> +	 *
> +	 * The MSB is the number of bits in the full mask because BIT[0] is
> +	 * always 0.
> +	 *
> +	 * In the special 3 Rank interleaving case, a single bit is flipped
> +	 * without swapping with the most significant bit. This can be handled
> +	 * by keeping the MSB where it is and ignoring the single zero bit.
> +	 */
> +	msb = fls(addr_mask_orig) - 1;
> +	weight = hweight_long(addr_mask_orig);
> +	num_zero_bits = msb - weight - !!(cs_mode & CS_3R_INTERLEAVE);
> +
> +	/* Take the number of zero bits off from the top of the mask. */
> +	addr_mask_deinterleaved = GENMASK_ULL(msb - num_zero_bits, 1);
> +
> +	edac_dbg(1, "CS%d DIMM%d AddrMasks:\n", csrow_nr, dimm);
> +	edac_dbg(1, "  Original AddrMask: 0x%x\n", addr_mask_orig);
> +	edac_dbg(1, "  Deinterleaved AddrMask: 0x%x\n", addr_mask_deinterleaved);
> +
> +	/* Register [31:1] = Address [39:9]. Size is in kBs here. */
> +	size = (addr_mask_deinterleaved >> 2) + 1;
> +
> +	/* Return size in MBs. */
> +	return size >> 10;
> +}
> +
>  static int f17_addr_mask_to_cs_size(struct amd64_pvt *pvt, u8 umc,
>  				    unsigned int cs_mode, int csrow_nr)
>  {
> -	u32 addr_mask_orig, addr_mask_deinterleaved;
> -	u32 msb, weight, num_zero_bits;
> +	u32 addr_mask_orig;

Please keep the lines from longest to shortest.

>  	int cs_mask_nr = csrow_nr;
>  	int dimm, size = 0;
>  
> @@ -2680,33 +2821,15 @@ static int f17_addr_mask_to_cs_size(struct amd64_pvt *pvt, u8 umc,
>  	else
>  		addr_mask_orig = pvt->csels[umc].csmasks[cs_mask_nr];
>  
> -	/*
> -	 * The number of zero bits in the mask is equal to the number of bits
> -	 * in a full mask minus the number of bits in the current mask.
> -	 *
> -	 * The MSB is the number of bits in the full mask because BIT[0] is
> -	 * always 0.
> -	 *
> -	 * In the special 3 Rank interleaving case, a single bit is flipped
> -	 * without swapping with the most significant bit. This can be handled
> -	 * by keeping the MSB where it is and ignoring the single zero bit.
> -	 */
> -	msb = fls(addr_mask_orig) - 1;
> -	weight = hweight_long(addr_mask_orig);
> -	num_zero_bits = msb - weight - !!(cs_mode & CS_3R_INTERLEAVE);
> -
> -	/* Take the number of zero bits off from the top of the mask. */
> -	addr_mask_deinterleaved = GENMASK_ULL(msb - num_zero_bits, 1);
> -
> -	edac_dbg(1, "CS%d DIMM%d AddrMasks:\n", csrow_nr, dimm);
> -	edac_dbg(1, "  Original AddrMask: 0x%x\n", addr_mask_orig);
> -	edac_dbg(1, "  Deinterleaved AddrMask: 0x%x\n", addr_mask_deinterleaved);
> +	return __addr_mask_to_cs_size(addr_mask_orig, cs_mode, cs_mask_nr, dimm);
> +}
>  
> -	/* Register [31:1] = Address [39:9]. Size is in kBs here. */
> -	size = (addr_mask_deinterleaved >> 2) + 1;
> +static int gpu_addr_mask_to_cs_size(struct amd64_pvt *pvt, u8 umc,
> +				    unsigned int cs_mode, int csrow_nr)
> +{
> +	u32 addr_mask_orig = pvt->csels[umc].csmasks[csrow_nr];
>  
> -	/* Return size in MBs. */
> -	return size >> 10;
> +	return __addr_mask_to_cs_size(addr_mask_orig, cs_mode, csrow_nr, csrow_nr >> 1);
>  }
>  
>  static void read_dram_ctl_register(struct amd64_pvt *pvt)
> @@ -3703,6 +3826,11 @@ static void f17_determine_ecc_sym_sz(struct amd64_pvt *pvt)
>  	}
>  }
>  
> +/* ECC symbol size is not available on Aldebaran nodes */
> +static void gpu_determine_ecc_sym_sz(struct amd64_pvt *pvt)
> +{
> +}
> +
>  static void read_top_mem_registers(struct amd64_pvt *pvt)
>  {
>  	u64 msr_val;
> @@ -3724,6 +3852,25 @@ static void read_top_mem_registers(struct amd64_pvt *pvt)
>  	}
>  }
>  
> +static void gpu_read_mc_regs(struct amd64_pvt *pvt)
> +{
> +	u8 nid = pvt->mc_node_id;
> +	struct amd64_umc *umc;
> +	u32 i, umc_base;
> +
> +	/* Read registers from each UMC */
> +	for_each_umc(i) {
> +		umc_base = gpu_get_umc_base(i, 0);
> +		umc = &pvt->umc[i];
> +
> +		amd_smn_read(nid, umc_base + UMCCH_UMC_CFG, &umc->umc_cfg);
> +		amd_smn_read(nid, umc_base + UMCCH_SDP_CTRL, &umc->sdp_ctrl);
> +		amd_smn_read(nid, umc_base + UMCCH_ECC_CTRL, &umc->ecc_ctrl);
> +	}
> +
> +	amd64_read_pci_cfg(pvt->F0, DF_DHAR, &pvt->dhar);
> +}
> +
>  /*
>   * Retrieve the hardware registers of the memory controller.
>   */
> @@ -3850,6 +3997,35 @@ static u32 get_csrow_nr_pages(struct amd64_pvt *pvt, u8 dct, int csrow_nr_orig)
>  	return nr_pages;
>  }
>  
> +static int gpu_init_csrows(struct mem_ctl_info *mci)
> +{
> +	struct amd64_pvt *pvt = mci->pvt_info;
> +	struct dimm_info *dimm;
> +	int empty = 1;
> +	u8 umc, cs;
> +
> +	for_each_umc(umc) {
> +		for_each_chip_select(cs, umc, pvt) {
> +			if (!csrow_enabled(cs, umc, pvt))
> +				continue;
> +
> +			empty = 0;
> +			dimm = mci->csrows[umc]->channels[cs]->dimm;
> +
> +			edac_dbg(1, "MC node: %d, csrow: %d\n",
> +				 pvt->mc_node_id, cs);
> +
> +			dimm->nr_pages = get_csrow_nr_pages(pvt, umc, cs);
> +			dimm->mtype = pvt->dram_type;
> +			dimm->edac_mode = EDAC_SECDED;
> +			dimm->dtype = DEV_X16;
> +			dimm->grain = 64;
> +		}
> +	}
> +
> +	return empty;
> +}
> +
>  static int init_csrows_df(struct mem_ctl_info *mci)
>  {
>  	struct amd64_pvt *pvt = mci->pvt_info;
> @@ -4190,6 +4366,12 @@ static bool f17_check_ecc_enabled(struct amd64_pvt *pvt)
>  		return true;
>  }
>  
> +/* ECC is enabled by default on GPU nodes */
> +static bool gpu_check_ecc_enabled(struct amd64_pvt *pvt)
> +{
> +	return true;
> +}
> +
>  static inline void
>  f1x_determine_edac_ctl_cap(struct mem_ctl_info *mci, struct amd64_pvt *pvt)
>  {
> @@ -4231,6 +4413,12 @@ f17_determine_edac_ctl_cap(struct mem_ctl_info *mci, struct amd64_pvt *pvt)
>  	}
>  }
>  
> +static inline void
> +gpu_determine_edac_ctl_cap(struct mem_ctl_info *mci, struct amd64_pvt *pvt)
> +{
> +	mci->edac_ctl_cap |= EDAC_FLAG_SECDED;
> +}
> +
>  static void f1x_setup_mci_misc_attrs(struct mem_ctl_info *mci)
>  {
>  	struct amd64_pvt *pvt = mci->pvt_info;
> @@ -4251,6 +4439,22 @@ static void f1x_setup_mci_misc_attrs(struct mem_ctl_info *mci)
>  	mci->get_sdram_scrub_rate = get_scrub_rate;
>  }
>  
> +static void gpu_setup_mci_misc_attrs(struct mem_ctl_info *mci)
> +{
> +	struct amd64_pvt *pvt = mci->pvt_info;
> +
> +	mci->mtype_cap		= MEM_FLAG_HBM2;
> +	mci->edac_ctl_cap	= EDAC_FLAG_NONE;
> +
> +	pvt->ops->determine_edac_ctl_cap(mci, pvt);
> +
> +	mci->edac_cap		= pvt->ops->determine_edac_cap(pvt);
> +	mci->mod_name		= EDAC_MOD_STR;
> +	mci->ctl_name		= pvt->ctl_name;
> +	mci->dev_name		= pci_name(pvt->F3);
> +	mci->ctl_page_to_phys	= NULL;
> +}
> +
>  /*
>   * returns a pointer to the family descriptor on success, NULL otherwise.
>   */
> @@ -4460,6 +4664,20 @@ static void per_family_init(struct amd64_pvt *pvt)
>  				pvt->f0_id		= PCI_DEVICE_ID_AMD_ALDEBARAN_DF_F0;
>  				pvt->f6_id		= PCI_DEVICE_ID_AMD_ALDEBARAN_DF_F6;
>  				pvt->max_mcs		= 4;
> +				pvt->ops->early_channel_count	= gpu_early_channel_count;
> +				pvt->ops->dbam_to_cs		= gpu_addr_mask_to_cs_size;
> +				pvt->ops->prep_chip_selects	= gpu_prep_chip_selects;
> +				pvt->ops->determine_memory_type	= gpu_determine_memory_type;
> +				pvt->ops->determine_ecc_sym_sz	= gpu_determine_ecc_sym_sz;
> +				pvt->ops->determine_edac_ctl_cap = gpu_determine_edac_ctl_cap;
> +				pvt->ops->determine_edac_cap	= gpu_determine_edac_cap;
> +				pvt->ops->setup_mci_misc_attrs	= gpu_setup_mci_misc_attrs;
> +				pvt->ops->get_cs_mode		= gpu_get_cs_mode;
> +				pvt->ops->ecc_enabled		= gpu_check_ecc_enabled;
> +				pvt->ops->get_base_mask		= gpu_read_umc_base_mask;
> +				pvt->ops->dump_misc_regs	= gpu_dump_misc_regs;
> +				pvt->ops->get_mc_regs		= gpu_read_mc_regs;
> +				pvt->ops->populate_csrows	= gpu_init_csrows;
>  				goto end_fam;
>  			} else {
>  				pvt->ctl_name		= "F19h_M30h";
> @@ -4581,9 +4799,10 @@ static int init_one_instance(struct amd64_pvt *pvt)
>  	if (pvt->channel_count < 0)
>  		return ret;
>  
> +	/* Define layers for CPU and GPU nodes */
>  	ret = -ENOMEM;
>  	layers[0].type = EDAC_MC_LAYER_CHIP_SELECT;
> -	layers[0].size = pvt->csels[0].b_cnt;
> +	layers[0].size = amd_gpu_nb_num() ? pvt->max_mcs : pvt->csels[0].b_cnt;

I think a flag in pvt->flags could be used here.

>  	layers[0].is_virt_csrow = true;
>  	layers[1].type = EDAC_MC_LAYER_CHANNEL;
>  
> @@ -4592,7 +4811,7 @@ static int init_one_instance(struct amd64_pvt *pvt)
>  	 * only one channel. Also, this simplifies handling later for the price
>  	 * of a couple of KBs tops.
>  	 */
> -	layers[1].size = pvt->max_mcs;
> +	layers[1].size = amd_gpu_nb_num() ? pvt->csels[0].b_cnt : pvt->max_mcs;
>  	layers[1].is_virt_csrow = false;
>

Thanks,
Yazen  

Re: [PATCH v7 08/12] EDAC/amd64: Add Family ops to update GPU csrow and channel info

[Yazen Ghannam]

On Thu, Feb 03, 2022 at 11:49:38AM -0600, Naveen Krishna Chatradhi wrote:
> GPU node has 'X' number of PHYs and 'Y' number of channels.
> This results in 'X*Y' number of instances in the Data Fabric.
> Therefore the Data Fabric ID of an instance in GPU as below:
>   df_inst_id = 'X' * number of channels per PHY + 'Y'
> 
> On CPUs the Data Fabric ID of an instance on a CPU is equal to the
> UMC number. since the UMC number and channel are equal in CPU nodes,
> the channel can be used as the Data Fabric ID of the instance.
> 
> Cc: Yazen Ghannam <yazen.ghannam@amd.com>
> Co-developed-by: Muralidhara M K <muralimk@amd.com>
> Signed-off-by: Muralidhara M K <muralimk@amd.com>
> Signed-off-by: Naveen Krishna Chatradhi <nchatrad@amd.com>
> ---
> v1->v7:
> * New change in v7
> 
>  drivers/edac/amd64_edac.c | 60 +++++++++++++++++++++++++++++++++++++--
>  drivers/edac/amd64_edac.h |  2 ++
>  2 files changed, 60 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
> index 10efe726a959..241419a0be93 100644
> --- a/drivers/edac/amd64_edac.c
> +++ b/drivers/edac/amd64_edac.c
> @@ -3653,6 +3653,30 @@ static inline void decode_bus_error(int node_id, struct mce *m)
>  	__log_ecc_error(mci, &err, ecc_type);
>  }
>  
> +/*
> + * On CPUs, The Data Fabric ID of an instance is equal to the UMC number.
> + * And since the UMC number and channel are equal in CPU nodes, the channel can be used
> + * as the Data Fabric ID of the instance.
> + */
> +static int f17_df_inst_id(struct mem_ctl_info *mci, struct amd64_pvt *pvt,
> +			  struct err_info *err)
> +{
> +	return err->channel;
> +}
> +
> +/*
> + * A GPU node has 'X' number of PHYs and 'Y' number of channels.
> + * This results in 'X*Y' number of instances in the Data Fabric.
> + * Therefore the Data Fabric ID of an instance can be found with the following formula:
> + * df_inst_id = 'X' * number of channels per PHY + 'Y'
> + *
> + */
> +static int gpu_df_inst_id(struct mem_ctl_info *mci, struct amd64_pvt *pvt,
> +			  struct err_info *err)
> +{
> +	return (err->csrow * pvt->channel_count / mci->nr_csrows) + err->channel;
> +}
> +

The DF Instance ID needs to get adjusted again later in the translation code
due to the fixed mapping of CSes to UMCs. Can that be done here instead? Also,
I assume that fixed mapping is unique to each product, so that would make it a
good fit for the family/pvt ops.

Thanks,
Yazen