Hi all,
This set adds supports for SMCA changes in future AMD systems.
Patch 1 adds an "unknown" bank type so that sysfs initialization issues
can be avoided on systems with new bank types.
Patch 2 adds new bank types and error descriptions used in future AMD
systems.
Patch 3 adjusts how SMCA bank information is cached. Future AMD systems
will have different bank type layouts between logical CPUs. So having a
single system-wide cache of the layout won't be correct.
Thanks,
Yazen
Yazen Ghannam (3):
x86/MCE/AMD: Provide an "Unknown" MCA bank type
x86/MCE/AMD, EDAC/mce_amd: Add new SMCA Bank Types
x86/MCE/AMD, EDAC/mce_amd: Support non-uniform MCA bank type
enumeration
arch/x86/include/asm/mce.h | 26 ++---
arch/x86/kernel/cpu/mce/amd.c | 114 +++++++++++++-----
drivers/edac/mce_amd.c | 148 +++++++++++++++++++++---
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 2 +-
4 files changed, 228 insertions(+), 62 deletions(-)
--
2.25.1
The AMD MCA Thresholding sysfs interface populates directories for each
bank and thresholding block. The name used for each directory is looked
up in a table of known bank types. However, new bank types won't match
in this list and will return NULL for the name. This will cause the
machinecheck sysfs interface to fail to be populated.
Set new and unknown MCA bank types to the "unknown" type. Also,
ensure that the bank's thresholding block directories have unique names.
This will ensure that the machinecheck sysfs interface can be
initialized.
Signed-off-by: Yazen Ghannam <[email protected]>
---
arch/x86/include/asm/mce.h | 1 +
arch/x86/kernel/cpu/mce/amd.c | 34 ++++++++++++++++++++++++++++------
drivers/edac/mce_amd.c | 3 +++
3 files changed, 32 insertions(+), 6 deletions(-)
diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
index d58f4f2e006f..7c1c35909946 100644
--- a/arch/x86/include/asm/mce.h
+++ b/arch/x86/include/asm/mce.h
@@ -319,6 +319,7 @@ enum smca_bank_types {
SMCA_XGMI_PCS, /* xGMI PCS Unit */
SMCA_XGMI_PHY, /* xGMI PHY Unit */
SMCA_WAFL_PHY, /* WAFL PHY Unit */
+ SMCA_UNKNOWN, /* Unknown type */
N_SMCA_BANK_TYPES
};
diff --git a/arch/x86/kernel/cpu/mce/amd.c b/arch/x86/kernel/cpu/mce/amd.c
index 2f351838d5f7..b9a5a94914a9 100644
--- a/arch/x86/kernel/cpu/mce/amd.c
+++ b/arch/x86/kernel/cpu/mce/amd.c
@@ -100,6 +100,7 @@ static struct smca_bank_name smca_names[] = {
[SMCA_XGMI_PCS] = { "xgmi_pcs", "Ext Global Memory Interconnect PCS Unit" },
[SMCA_XGMI_PHY] = { "xgmi_phy", "Ext Global Memory Interconnect PHY Unit" },
[SMCA_WAFL_PHY] = { "wafl_phy", "WAFL PHY Unit" },
+ [SMCA_UNKNOWN] = { "unknown", "Unrecognized Bank Type" },
};
static const char *smca_get_name(enum smca_bank_types t)
@@ -189,6 +190,9 @@ static struct smca_hwid smca_hwid_mcatypes[] = {
/* WAFL PHY MCA type */
{ SMCA_WAFL_PHY, HWID_MCATYPE(0x267, 0x0) },
+
+ /* Unknown type - this must be last in the list */
+ { SMCA_UNKNOWN, HWID_MCATYPE(0xFFF, 0xFFFF) },
};
struct smca_bank smca_banks[MAX_NR_BANKS];
@@ -300,7 +304,9 @@ static void smca_configure(unsigned int bank, unsigned int cpu)
for (i = 0; i < ARRAY_SIZE(smca_hwid_mcatypes); i++) {
s_hwid = &smca_hwid_mcatypes[i];
- if (hwid_mcatype == s_hwid->hwid_mcatype) {
+
+ if (hwid_mcatype == s_hwid->hwid_mcatype ||
+ s_hwid->bank_type == SMCA_UNKNOWN) {
smca_banks[bank].hwid = s_hwid;
smca_banks[bank].id = low;
smca_banks[bank].sysfs_id = s_hwid->count++;
@@ -1032,12 +1038,28 @@ static const char *get_name(unsigned int bank, struct threshold_block *b)
return NULL;
}
- if (smca_banks[bank].hwid->count == 1)
- return smca_get_name(bank_type);
+ if (smca_banks[bank].hwid->count == 1) {
+ if (bank_type == SMCA_UNKNOWN) {
+ snprintf(buf_mcatype, MAX_MCATYPE_NAME_LEN,
+ "%s_%x", smca_get_name(bank_type),
+ smca_banks[bank].id);
+
+ return buf_mcatype;
+ } else {
+ return smca_get_name(bank_type);
+ }
+ }
+
+ if (b && bank_type == SMCA_UNKNOWN) {
+ snprintf(buf_mcatype, MAX_MCATYPE_NAME_LEN,
+ "%s_%x_block_%u", smca_get_name(bank_type),
+ smca_banks[bank].id, b->block);
+ } else {
+ snprintf(buf_mcatype, MAX_MCATYPE_NAME_LEN,
+ "%s_%u", smca_get_name(bank_type),
+ smca_banks[bank].sysfs_id);
+ }
- snprintf(buf_mcatype, MAX_MCATYPE_NAME_LEN,
- "%s_%x", smca_get_name(bank_type),
- smca_banks[bank].sysfs_id);
return buf_mcatype;
}
diff --git a/drivers/edac/mce_amd.c b/drivers/edac/mce_amd.c
index 67dbf4c31271..5ccc09db0a51 100644
--- a/drivers/edac/mce_amd.c
+++ b/drivers/edac/mce_amd.c
@@ -1068,6 +1068,9 @@ static void decode_smca_error(struct mce *m)
pr_emerg(HW_ERR "%s Ext. Error Code: %d", ip_name, xec);
+ if (bank_type == SMCA_UNKNOWN)
+ return;
+
/* Only print the decode of valid error codes */
if (xec < smca_mce_descs[bank_type].num_descs)
pr_cont(", %s.\n", smca_mce_descs[bank_type].descs[xec]);
--
2.25.1
Add HWID and McaType values for new SMCA bank types, and add their error
descriptions to edac_mce_amd.
The "PHY" bank types all have the same error descriptions, and the NBIF
and SHUB bank types have the same error descriptions. So reuse the same
arrays where appropriate.
Signed-off-by: Yazen Ghannam <[email protected]>
---
arch/x86/include/asm/mce.h | 7 ++
arch/x86/kernel/cpu/mce/amd.c | 28 +++++++
drivers/edac/mce_amd.c | 133 ++++++++++++++++++++++++++++++++--
3 files changed, 162 insertions(+), 6 deletions(-)
diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
index 7c1c35909946..d6834e8fbb6a 100644
--- a/arch/x86/include/asm/mce.h
+++ b/arch/x86/include/asm/mce.h
@@ -313,12 +313,19 @@ enum smca_bank_types {
SMCA_SMU, /* System Management Unit */
SMCA_SMU_V2,
SMCA_MP5, /* Microprocessor 5 Unit */
+ SMCA_MPDMA, /* MPDMA Unit */
SMCA_NBIO, /* Northbridge IO Unit */
SMCA_PCIE, /* PCI Express Unit */
SMCA_PCIE_V2,
SMCA_XGMI_PCS, /* xGMI PCS Unit */
+ SMCA_NBIF, /* NBIF Unit */
+ SMCA_SHUB, /* System HUB Unit */
+ SMCA_SATA, /* SATA Unit */
+ SMCA_USB, /* USB Unit */
+ SMCA_GMI_PCS, /* GMI PCS Unit */
SMCA_XGMI_PHY, /* xGMI PHY Unit */
SMCA_WAFL_PHY, /* WAFL PHY Unit */
+ SMCA_GMI_PHY, /* GMI PHY Unit */
SMCA_UNKNOWN, /* Unknown type */
N_SMCA_BANK_TYPES
};
diff --git a/arch/x86/kernel/cpu/mce/amd.c b/arch/x86/kernel/cpu/mce/amd.c
index b9a5a94914a9..e12a8f3414f5 100644
--- a/arch/x86/kernel/cpu/mce/amd.c
+++ b/arch/x86/kernel/cpu/mce/amd.c
@@ -95,11 +95,18 @@ static struct smca_bank_name smca_names[] = {
[SMCA_PSP ... SMCA_PSP_V2] = { "psp", "Platform Security Processor" },
[SMCA_SMU ... SMCA_SMU_V2] = { "smu", "System Management Unit" },
[SMCA_MP5] = { "mp5", "Microprocessor 5 Unit" },
+ [SMCA_MPDMA] = { "mpdma", "MPDMA Unit" },
[SMCA_NBIO] = { "nbio", "Northbridge IO Unit" },
[SMCA_PCIE ... SMCA_PCIE_V2] = { "pcie", "PCI Express Unit" },
[SMCA_XGMI_PCS] = { "xgmi_pcs", "Ext Global Memory Interconnect PCS Unit" },
+ [SMCA_NBIF] = { "nbif", "NBIF Unit" },
+ [SMCA_SHUB] = { "shub", "System Hub Unit" },
+ [SMCA_SATA] = { "sata", "SATA Unit" },
+ [SMCA_USB] = { "usb", "USB Unit" },
+ [SMCA_GMI_PCS] = { "gmi_pcs", "Global Memory Interconnect PCS Unit" },
[SMCA_XGMI_PHY] = { "xgmi_phy", "Ext Global Memory Interconnect PHY Unit" },
[SMCA_WAFL_PHY] = { "wafl_phy", "WAFL PHY Unit" },
+ [SMCA_GMI_PHY] = { "gmi_phy", "Global Memory Interconnect PHY Unit" },
[SMCA_UNKNOWN] = { "unknown", "Unrecognized Bank Type" },
};
@@ -175,6 +182,9 @@ static struct smca_hwid smca_hwid_mcatypes[] = {
/* Microprocessor 5 Unit MCA type */
{ SMCA_MP5, HWID_MCATYPE(0x01, 0x2) },
+ /* MPDMA MCA type */
+ { SMCA_MPDMA, HWID_MCATYPE(0x01, 0x3) },
+
/* Northbridge IO Unit MCA type */
{ SMCA_NBIO, HWID_MCATYPE(0x18, 0x0) },
@@ -185,12 +195,30 @@ static struct smca_hwid smca_hwid_mcatypes[] = {
/* xGMI PCS MCA type */
{ SMCA_XGMI_PCS, HWID_MCATYPE(0x50, 0x0) },
+ /* NBIF MCA type */
+ { SMCA_NBIF, HWID_MCATYPE(0x6C, 0x0) },
+
+ /* SHUB MCA type */
+ { SMCA_SHUB, HWID_MCATYPE(0x80, 0x0) },
+
+ /* SATA MCA type */
+ { SMCA_SATA, HWID_MCATYPE(0xA8, 0x0) },
+
+ /* USB MCA type */
+ { SMCA_USB, HWID_MCATYPE(0xAA, 0x0) },
+
+ /* GMI PCS MCA type */
+ { SMCA_GMI_PCS, HWID_MCATYPE(0x241, 0x0) },
+
/* xGMI PHY MCA type */
{ SMCA_XGMI_PHY, HWID_MCATYPE(0x259, 0x0) },
/* WAFL PHY MCA type */
{ SMCA_WAFL_PHY, HWID_MCATYPE(0x267, 0x0) },
+ /* GMI PHY MCA type */
+ { SMCA_GMI_PHY, HWID_MCATYPE(0x269, 0x0) },
+
/* Unknown type - this must be last in the list */
{ SMCA_UNKNOWN, HWID_MCATYPE(0xFFF, 0xFFFF) },
};
diff --git a/drivers/edac/mce_amd.c b/drivers/edac/mce_amd.c
index 5ccc09db0a51..720df7b4d6ab 100644
--- a/drivers/edac/mce_amd.c
+++ b/drivers/edac/mce_amd.c
@@ -399,6 +399,63 @@ static const char * const smca_mp5_mce_desc[] = {
"Instruction Tag Cache Bank B ECC or parity error",
};
+static const char * const smca_mpdma_mce_desc[] = {
+ "Main SRAM [31:0] bank ECC or parity error",
+ "Main SRAM [63:32] bank ECC or parity error",
+ "Main SRAM [95:64] bank ECC or parity error",
+ "Main SRAM [127:96] bank ECC or parity error",
+ "Data Cache Bank A ECC or parity error",
+ "Data Cache Bank B ECC or parity error",
+ "Data Tag Cache Bank A ECC or parity error",
+ "Data Tag Cache Bank B ECC or parity error",
+ "Instruction Cache Bank A ECC or parity error",
+ "Instruction Cache Bank B ECC or parity error",
+ "Instruction Tag Cache Bank A ECC or parity error",
+ "Instruction Tag Cache Bank B ECC or parity error",
+ "Data Cache Bank A ECC or parity error",
+ "Data Cache Bank B ECC or parity error",
+ "Data Tag Cache Bank A ECC or parity error",
+ "Data Tag Cache Bank B ECC or parity error",
+ "Instruction Cache Bank A ECC or parity error",
+ "Instruction Cache Bank B ECC or parity error",
+ "Instruction Tag Cache Bank A ECC or parity error",
+ "Instruction Tag Cache Bank B ECC or parity error",
+ "Data Cache Bank A ECC or parity error",
+ "Data Cache Bank B ECC or parity error",
+ "Data Tag Cache Bank A ECC or parity error",
+ "Data Tag Cache Bank B ECC or parity error",
+ "Instruction Cache Bank A ECC or parity error",
+ "Instruction Cache Bank B ECC or parity error",
+ "Instruction Tag Cache Bank A ECC or parity error",
+ "Instruction Tag Cache Bank B ECC or parity error",
+ "System Hub Read Buffer ECC or parity error",
+ "MPDMA TVF DVSEC Memory ECC or parity error",
+ "MPDMA TVF MMIO Mailbox0 ECC or parity error",
+ "MPDMA TVF MMIO Mailbox1 ECC or parity error",
+ "MPDMA TVF Doorbell Memory ECC or parity error",
+ "MPDMA TVF SDP Slave Memory 0 ECC or parity error",
+ "MPDMA TVF SDP Slave Memory 1 ECC or parity error",
+ "MPDMA TVF SDP Slave Memory 2 ECC or parity error",
+ "MPDMA TVF SDP Master Memory 0 ECC or parity error",
+ "MPDMA TVF SDP Master Memory 1 ECC or parity error",
+ "MPDMA TVF SDP Master Memory 2 ECC or parity error",
+ "MPDMA TVF SDP Master Memory 3 ECC or parity error",
+ "MPDMA TVF SDP Master Memory 4 ECC or parity error",
+ "MPDMA TVF SDP Master Memory 5 ECC or parity error",
+ "MPDMA TVF SDP Master Memory 6 ECC or parity error",
+ "MPDMA PTE Command FIFO ECC or parity error",
+ "MPDMA PTE Hub Data FIFO ECC or parity error",
+ "MPDMA PTE Internal Data FIFO ECC or parity error",
+ "MPDMA PTE Command Memory DMA ECC or parity error",
+ "MPDMA PTE Command Memory Internal ECC or parity error",
+ "MPDMA PTE DMA Completion FIFO ECC or parity error",
+ "MPDMA PTE Tablewalk Completion FIFO ECC or parity error",
+ "MPDMA PTE Descriptor Completion FIFO ECC or parity error",
+ "MPDMA PTE ReadOnly Completion FIFO ECC or parity error",
+ "MPDMA PTE DirectWrite Completion FIFO ECC or parity error",
+ "SDP Watchdog Timer expired",
+};
+
static const char * const smca_nbio_mce_desc[] = {
"ECC or Parity error",
"PCIE error",
@@ -458,11 +515,66 @@ static const char * const smca_xgmiphy_mce_desc[] = {
"PHY APB error",
};
-static const char * const smca_waflphy_mce_desc[] = {
- "RAM ECC Error",
- "ARC instruction buffer parity error",
- "ARC data buffer parity error",
- "PHY APB error",
+static const char * const smca_nbif_mce_desc[] = {
+ "Timeout error from GMI",
+ "SRAM ECC error",
+ "NTB Error Event",
+ "SDP Parity error",
+};
+
+static const char * const smca_sata_mce_desc[] = {
+ "Parity error for port 0",
+ "Parity error for port 1",
+ "Parity error for port 2",
+ "Parity error for port 3",
+ "Parity error for port 4",
+ "Parity error for port 5",
+ "Parity error for port 6",
+ "Parity error for port 7",
+};
+
+static const char * const smca_usb_mce_desc[] = {
+ "Parity error or ECC error for S0 RAM0",
+ "Parity error or ECC error for S0 RAM1",
+ "Parity error or ECC error for S0 RAM2",
+ "Parity error for PHY RAM0",
+ "Parity error for PHY RAM1",
+ "AXI Slave Response error",
+};
+
+static const char * const smca_gmipcs_mce_desc[] = {
+ "Data Loss Error",
+ "Training Error",
+ "Replay Parity Error",
+ "Rx Fifo Underflow Error",
+ "Rx Fifo Overflow Error",
+ "CRC Error",
+ "BER Exceeded Error",
+ "Tx Fifo Underflow Error",
+ "Replay Buffer Parity Error",
+ "Tx Overflow Error",
+ "Replay Fifo Overflow Error",
+ "Replay Fifo Underflow Error",
+ "Elastic Fifo Overflow Error",
+ "Deskew Error",
+ "Offline Error",
+ "Data Startup Limit Error",
+ "FC Init Timeout Error",
+ "Recovery Timeout Error",
+ "Ready Serial Timeout Error",
+ "Ready Serial Attempt Error",
+ "Recovery Attempt Error",
+ "Recovery Relock Attempt Error",
+ "Deskew Abort Error",
+ "Rx Buffer Error",
+ "Rx LFDS Fifo Overflow Error",
+ "Rx LFDS Fifo Underflow Error",
+ "LinkSub Tx Timeout Error",
+ "LinkSub Rx Timeout Error",
+ "Rx CMD Pocket Error",
+ "LFDS Training Timeout Error",
+ "LFDS FC Init Timeout Error",
+ "Data Loss Error",
};
struct smca_mce_desc {
@@ -490,12 +602,21 @@ static struct smca_mce_desc smca_mce_descs[] = {
[SMCA_SMU] = { smca_smu_mce_desc, ARRAY_SIZE(smca_smu_mce_desc) },
[SMCA_SMU_V2] = { smca_smu2_mce_desc, ARRAY_SIZE(smca_smu2_mce_desc) },
[SMCA_MP5] = { smca_mp5_mce_desc, ARRAY_SIZE(smca_mp5_mce_desc) },
+ [SMCA_MPDMA] = { smca_mpdma_mce_desc, ARRAY_SIZE(smca_mpdma_mce_desc) },
[SMCA_NBIO] = { smca_nbio_mce_desc, ARRAY_SIZE(smca_nbio_mce_desc) },
[SMCA_PCIE] = { smca_pcie_mce_desc, ARRAY_SIZE(smca_pcie_mce_desc) },
[SMCA_PCIE_V2] = { smca_pcie2_mce_desc, ARRAY_SIZE(smca_pcie2_mce_desc) },
[SMCA_XGMI_PCS] = { smca_xgmipcs_mce_desc, ARRAY_SIZE(smca_xgmipcs_mce_desc) },
+ /* NBIF and SHUB have the same error descriptions, for now. */
+ [SMCA_NBIF] = { smca_nbif_mce_desc, ARRAY_SIZE(smca_nbif_mce_desc) },
+ [SMCA_SHUB] = { smca_nbif_mce_desc, ARRAY_SIZE(smca_nbif_mce_desc) },
+ [SMCA_SATA] = { smca_sata_mce_desc, ARRAY_SIZE(smca_sata_mce_desc) },
+ [SMCA_USB] = { smca_usb_mce_desc, ARRAY_SIZE(smca_usb_mce_desc) },
+ [SMCA_GMI_PCS] = { smca_gmipcs_mce_desc, ARRAY_SIZE(smca_gmipcs_mce_desc) },
+ /* All the PHY bank types have the same error descriptions, for now. */
[SMCA_XGMI_PHY] = { smca_xgmiphy_mce_desc, ARRAY_SIZE(smca_xgmiphy_mce_desc) },
- [SMCA_WAFL_PHY] = { smca_waflphy_mce_desc, ARRAY_SIZE(smca_waflphy_mce_desc) },
+ [SMCA_WAFL_PHY] = { smca_xgmiphy_mce_desc, ARRAY_SIZE(smca_xgmiphy_mce_desc) },
+ [SMCA_GMI_PHY] = { smca_xgmiphy_mce_desc, ARRAY_SIZE(smca_xgmiphy_mce_desc) },
};
static bool f12h_mc0_mce(u16 ec, u8 xec)
--
2.25.1
AMD systems currently lay out MCA bank types such that the type of bank
number "i" is either the same across all CPUs or is Reserved/Read-as-Zero.
For example:
Bank # | CPUx | CPUy
0 LS LS
1 RAZ UMC
2 CS CS
3 SMU RAZ
Future AMD systems will lay out MCA bank types such that the type of
bank number "i" may be different across CPUs.
For example:
Bank # | CPUx | CPUy
0 LS LS
1 RAZ UMC
2 CS NBIO
3 SMU RAZ
Change the structures that cache MCA bank types to be per-CPU and update
smca_get_bank_type() to handle this change.
Move some SMCA-specific structures to amd.c from mce.h, since they no
longer need to be global.
Break out the "count" for bank types from struct smca_hwid, since this
should provide a per-CPU count rather than a system-wide count.
Apply the "const" qualifier to the struct smca_hwid_mcatypes array. The
values in this array should not change at runtime.
Signed-off-by: Yazen Ghannam <[email protected]>
---
arch/x86/include/asm/mce.h | 18 +-------
arch/x86/kernel/cpu/mce/amd.c | 60 ++++++++++++++-----------
drivers/edac/mce_amd.c | 12 +----
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 2 +-
4 files changed, 38 insertions(+), 54 deletions(-)
diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
index d6834e8fbb6a..a0b3cdd8e6ec 100644
--- a/arch/x86/include/asm/mce.h
+++ b/arch/x86/include/asm/mce.h
@@ -330,22 +330,6 @@ enum smca_bank_types {
N_SMCA_BANK_TYPES
};
-#define HWID_MCATYPE(hwid, mcatype) (((hwid) << 16) | (mcatype))
-
-struct smca_hwid {
- unsigned int bank_type; /* Use with smca_bank_types for easy indexing. */
- u32 hwid_mcatype; /* (hwid,mcatype) tuple */
- u8 count; /* Number of instances. */
-};
-
-struct smca_bank {
- struct smca_hwid *hwid;
- u32 id; /* Value of MCA_IPID[InstanceId]. */
- u8 sysfs_id; /* Value used for sysfs name. */
-};
-
-extern struct smca_bank smca_banks[MAX_NR_BANKS];
-
extern const char *smca_get_long_name(enum smca_bank_types t);
extern bool amd_mce_is_memory_error(struct mce *m);
@@ -353,7 +337,7 @@ extern int mce_threshold_create_device(unsigned int cpu);
extern int mce_threshold_remove_device(unsigned int cpu);
void mce_amd_feature_init(struct cpuinfo_x86 *c);
-enum smca_bank_types smca_get_bank_type(unsigned int bank);
+enum smca_bank_types smca_get_bank_type(unsigned int cpu, unsigned int bank);
#else
static inline int mce_threshold_create_device(unsigned int cpu) { return 0; };
diff --git a/arch/x86/kernel/cpu/mce/amd.c b/arch/x86/kernel/cpu/mce/amd.c
index e12a8f3414f5..6ec87e580145 100644
--- a/arch/x86/kernel/cpu/mce/amd.c
+++ b/arch/x86/kernel/cpu/mce/amd.c
@@ -71,6 +71,22 @@ static const char * const smca_umc_block_names[] = {
"misc_umc"
};
+#define HWID_MCATYPE(hwid, mcatype) (((hwid) << 16) | (mcatype))
+
+struct smca_hwid {
+ unsigned int bank_type; /* Use with smca_bank_types for easy indexing. */
+ u32 hwid_mcatype; /* (hwid,mcatype) tuple */
+};
+
+struct smca_bank {
+ const struct smca_hwid *hwid;
+ u32 id; /* Value of MCA_IPID[InstanceId]. */
+ u8 sysfs_id; /* Value used for sysfs name. */
+};
+
+static DEFINE_PER_CPU_READ_MOSTLY(struct smca_bank[MAX_NR_BANKS], smca_banks);
+static DEFINE_PER_CPU_READ_MOSTLY(u8[N_SMCA_BANK_TYPES], smca_bank_counts);
+
struct smca_bank_name {
const char *name; /* Short name for sysfs */
const char *long_name; /* Long name for pretty-printing */
@@ -127,14 +143,14 @@ const char *smca_get_long_name(enum smca_bank_types t)
}
EXPORT_SYMBOL_GPL(smca_get_long_name);
-enum smca_bank_types smca_get_bank_type(unsigned int bank)
+enum smca_bank_types smca_get_bank_type(unsigned int cpu, unsigned int bank)
{
struct smca_bank *b;
if (bank >= MAX_NR_BANKS)
return N_SMCA_BANK_TYPES;
- b = &smca_banks[bank];
+ b = &per_cpu(smca_banks, cpu)[bank];
if (!b->hwid)
return N_SMCA_BANK_TYPES;
@@ -142,7 +158,7 @@ enum smca_bank_types smca_get_bank_type(unsigned int bank)
}
EXPORT_SYMBOL_GPL(smca_get_bank_type);
-static struct smca_hwid smca_hwid_mcatypes[] = {
+static const struct smca_hwid smca_hwid_mcatypes[] = {
/* { bank_type, hwid_mcatype } */
/* Reserved type */
@@ -223,9 +239,6 @@ static struct smca_hwid smca_hwid_mcatypes[] = {
{ SMCA_UNKNOWN, HWID_MCATYPE(0xFFF, 0xFFFF) },
};
-struct smca_bank smca_banks[MAX_NR_BANKS];
-EXPORT_SYMBOL_GPL(smca_banks);
-
/*
* In SMCA enabled processors, we can have multiple banks for a given IP type.
* So to define a unique name for each bank, we use a temp c-string to append
@@ -281,8 +294,9 @@ static void smca_set_misc_banks_map(unsigned int bank, unsigned int cpu)
static void smca_configure(unsigned int bank, unsigned int cpu)
{
+ u8 *bank_counts = this_cpu_ptr(smca_bank_counts);
+ const struct smca_hwid *s_hwid;
unsigned int i, hwid_mcatype;
- struct smca_hwid *s_hwid;
u32 high, low;
u32 smca_config = MSR_AMD64_SMCA_MCx_CONFIG(bank);
@@ -318,10 +332,6 @@ static void smca_configure(unsigned int bank, unsigned int cpu)
smca_set_misc_banks_map(bank, cpu);
- /* Return early if this bank was already initialized. */
- if (smca_banks[bank].hwid && smca_banks[bank].hwid->hwid_mcatype != 0)
- return;
-
if (rdmsr_safe(MSR_AMD64_SMCA_MCx_IPID(bank), &low, &high)) {
pr_warn("Failed to read MCA_IPID for bank %d\n", bank);
return;
@@ -335,9 +345,9 @@ static void smca_configure(unsigned int bank, unsigned int cpu)
if (hwid_mcatype == s_hwid->hwid_mcatype ||
s_hwid->bank_type == SMCA_UNKNOWN) {
- smca_banks[bank].hwid = s_hwid;
- smca_banks[bank].id = low;
- smca_banks[bank].sysfs_id = s_hwid->count++;
+ this_cpu_ptr(smca_banks)[bank].hwid = s_hwid;
+ this_cpu_ptr(smca_banks)[bank].id = low;
+ this_cpu_ptr(smca_banks)[bank].sysfs_id = bank_counts[s_hwid->bank_type]++;
break;
}
}
@@ -623,7 +633,7 @@ prepare_threshold_block(unsigned int bank, unsigned int block, u32 addr,
bool amd_filter_mce(struct mce *m)
{
- enum smca_bank_types bank_type = smca_get_bank_type(m->bank);
+ enum smca_bank_types bank_type = smca_get_bank_type(m->extcpu, m->bank);
struct cpuinfo_x86 *c = &boot_cpu_data;
/* See Family 17h Models 10h-2Fh Erratum #1114. */
@@ -661,7 +671,7 @@ static void disable_err_thresholding(struct cpuinfo_x86 *c, unsigned int bank)
} else if (c->x86 == 0x17 &&
(c->x86_model >= 0x10 && c->x86_model <= 0x2F)) {
- if (smca_get_bank_type(bank) != SMCA_IF)
+ if (smca_get_bank_type(smp_processor_id(), bank) != SMCA_IF)
return;
msrs[0] = MSR_AMD64_SMCA_MCx_MISC(bank);
@@ -729,7 +739,7 @@ bool amd_mce_is_memory_error(struct mce *m)
u8 xec = (m->status >> 16) & 0x1f;
if (mce_flags.smca)
- return smca_get_bank_type(m->bank) == SMCA_UMC && xec == 0x0;
+ return smca_get_bank_type(m->extcpu, m->bank) == SMCA_UMC && xec == 0x0;
return m->bank == 4 && xec == 0x8;
}
@@ -1045,7 +1055,7 @@ static struct kobj_type threshold_ktype = {
.release = threshold_block_release,
};
-static const char *get_name(unsigned int bank, struct threshold_block *b)
+static const char *get_name(unsigned int cpu, unsigned int bank, struct threshold_block *b)
{
enum smca_bank_types bank_type;
@@ -1056,7 +1066,7 @@ static const char *get_name(unsigned int bank, struct threshold_block *b)
return th_names[bank];
}
- bank_type = smca_get_bank_type(bank);
+ bank_type = smca_get_bank_type(cpu, bank);
if (bank_type >= N_SMCA_BANK_TYPES)
return NULL;
@@ -1066,11 +1076,11 @@ static const char *get_name(unsigned int bank, struct threshold_block *b)
return NULL;
}
- if (smca_banks[bank].hwid->count == 1) {
+ if (per_cpu(smca_bank_counts, cpu)[bank_type] == 1) {
if (bank_type == SMCA_UNKNOWN) {
snprintf(buf_mcatype, MAX_MCATYPE_NAME_LEN,
"%s_%x", smca_get_name(bank_type),
- smca_banks[bank].id);
+ per_cpu(smca_banks, cpu)[bank].id);
return buf_mcatype;
} else {
@@ -1081,11 +1091,11 @@ static const char *get_name(unsigned int bank, struct threshold_block *b)
if (b && bank_type == SMCA_UNKNOWN) {
snprintf(buf_mcatype, MAX_MCATYPE_NAME_LEN,
"%s_%x_block_%u", smca_get_name(bank_type),
- smca_banks[bank].id, b->block);
+ per_cpu(smca_banks, cpu)[bank].id, b->block);
} else {
snprintf(buf_mcatype, MAX_MCATYPE_NAME_LEN,
"%s_%u", smca_get_name(bank_type),
- smca_banks[bank].sysfs_id);
+ per_cpu(smca_banks, cpu)[bank].sysfs_id);
}
return buf_mcatype;
@@ -1143,7 +1153,7 @@ static int allocate_threshold_blocks(unsigned int cpu, struct threshold_bank *tb
else
tb->blocks = b;
- err = kobject_init_and_add(&b->kobj, &threshold_ktype, tb->kobj, get_name(bank, b));
+ err = kobject_init_and_add(&b->kobj, &threshold_ktype, tb->kobj, get_name(cpu, bank, b));
if (err)
goto out_free;
recurse:
@@ -1198,7 +1208,7 @@ static int threshold_create_bank(struct threshold_bank **bp, unsigned int cpu,
struct device *dev = this_cpu_read(mce_device);
struct amd_northbridge *nb = NULL;
struct threshold_bank *b = NULL;
- const char *name = get_name(bank, NULL);
+ const char *name = get_name(cpu, bank, NULL);
int err = 0;
if (!dev)
diff --git a/drivers/edac/mce_amd.c b/drivers/edac/mce_amd.c
index 720df7b4d6ab..39ffde96a343 100644
--- a/drivers/edac/mce_amd.c
+++ b/drivers/edac/mce_amd.c
@@ -1166,20 +1166,10 @@ static void decode_mc6_mce(struct mce *m)
/* Decode errors according to Scalable MCA specification */
static void decode_smca_error(struct mce *m)
{
- struct smca_hwid *hwid;
- enum smca_bank_types bank_type;
+ enum smca_bank_types bank_type = smca_get_bank_type(m->extcpu, m->bank);
const char *ip_name;
u8 xec = XEC(m->status, xec_mask);
- if (m->bank >= ARRAY_SIZE(smca_banks))
- return;
-
- hwid = smca_banks[m->bank].hwid;
- if (!hwid)
- return;
-
- bank_type = hwid->bank_type;
-
if (bank_type == SMCA_RESERVED) {
pr_emerg(HW_ERR "Bank %d is reserved.\n", m->bank);
return;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
index 08133de21fdd..75dad0214dc7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
@@ -2647,7 +2647,7 @@ static int amdgpu_bad_page_notifier(struct notifier_block *nb,
* and error occurred in DramECC (Extended error code = 0) then only
* process the error, else bail out.
*/
- if (!m || !((smca_get_bank_type(m->bank) == SMCA_UMC_V2) &&
+ if (!m || !((smca_get_bank_type(m->extcpu, m->bank) == SMCA_UMC_V2) &&
(XEC(m->status, 0x3f) == 0x0)))
return NOTIFY_DONE;
--
2.25.1
On Fri, Dec 03, 2021 at 02:00:15AM +0000, Yazen Ghannam wrote:
> The AMD MCA Thresholding sysfs interface populates directories for each
> bank and thresholding block. The name used for each directory is looked
> up in a table of known bank types. However, new bank types won't match
> in this list and will return NULL for the name. This will cause the
> machinecheck sysfs interface to fail to be populated.
>
> Set new and unknown MCA bank types to the "unknown" type. Also,
> ensure that the bank's thresholding block directories have unique names.
> This will ensure that the machinecheck sysfs interface can be
> initialized.
What is the advantage of having a sysfs directory structure headed with
an "unknown" entry vs not having that structure at all when the kernel
runs on a machine for which it has not been enabled yet?
IOW, if those new banks would need additional enablement, what's the
point of having "unknown" on older kernels which do not have any
functionality?
IOW, how does this:
/sys/devices/system/machinecheck/machinecheck0/unknown/unknown/
├── error_count
├── interrupt_enable
└── threshold_limit
help a user?
Btw, looking at the current layout:
...
├── insn_fetch
│ └── insn_fetch
│ ├── error_count
│ ├── interrupt_enable
│ └── threshold_limit
├── l2_cache
│ └── l2_cache
│ ├── error_count
│ ├── interrupt_enable
│ └── threshold_limit
...
we have those names repeated which looks wonky and useless too. I'd
expect them to be:
...
├── insn_fetch
│ ├── error_count
│ ├── interrupt_enable
│ └── threshold_limit
├── l2_cache
│ ├── error_count
│ ├── interrupt_enable
│ └── threshold_limit
...
Can we fix that too pls?
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Fri, Dec 03, 2021 at 11:17:45PM +0100, Borislav Petkov wrote:
> On Fri, Dec 03, 2021 at 02:00:15AM +0000, Yazen Ghannam wrote:
> > The AMD MCA Thresholding sysfs interface populates directories for each
> > bank and thresholding block. The name used for each directory is looked
> > up in a table of known bank types. However, new bank types won't match
> > in this list and will return NULL for the name. This will cause the
> > machinecheck sysfs interface to fail to be populated.
> >
> > Set new and unknown MCA bank types to the "unknown" type. Also,
> > ensure that the bank's thresholding block directories have unique names.
> > This will ensure that the machinecheck sysfs interface can be
> > initialized.
>
> What is the advantage of having a sysfs directory structure headed with
> an "unknown" entry vs not having that structure at all when the kernel
> runs on a machine for which it has not been enabled yet?
>
> IOW, if those new banks would need additional enablement, what's the
> point of having "unknown" on older kernels which do not have any
> functionality?
>
> IOW, how does this:
>
> /sys/devices/system/machinecheck/machinecheck0/unknown/unknown/
> ├── error_count
> ├── interrupt_enable
> └── threshold_limit
>
> help a user?
Yeah, I see your point.
>
> Btw, looking at the current layout:
>
> ...
> ├── insn_fetch
> │ └── insn_fetch
> │ ├── error_count
> │ ├── interrupt_enable
> │ └── threshold_limit
> ├── l2_cache
> │ └── l2_cache
> │ ├── error_count
> │ ├── interrupt_enable
> │ └── threshold_limit
> ...
>
> we have those names repeated which looks wonky and useless too. I'd
> expect them to be:
>
> ...
> ├── insn_fetch
> │ ├── error_count
> │ ├── interrupt_enable
> │ └── threshold_limit
> ├── l2_cache
> │ ├── error_count
> │ ├── interrupt_enable
> │ └── threshold_limit
> ...
>
> Can we fix that too pls?
>
Sure thing. But I don't think removing the second directory will be okay. The
layout is "bank"/"block". If the "block" has special use like DRAM ECC, or L3
Cache on older systems, then it'll have a unique name. Otherwise, the block
will take the name of the bank.
I think the more robust solution is to drop the unique names and use generic
names like "bank"/"block". A new file called "type" can be introduced into the
directory structure, and this can return the name of the bank/block. New bank
types will return "<null>" for the "type", but the directory structure should
remain the same and functional.
I've seen this in other sysfs interfaces like cpuidle,
e.g. /sys/devices/system/cpu/cpu0/cpuidle/stateX
The "blockX/type" file is like the "stateX/desc" file. Or the "type" file can
be called "desc", since it's a description of what the bank or block
represent.
Here are a couple of examples:
/sys/devices/system/machinecheck/machinecheck0/
├── th_bank0
│ ├── type ("Instruction Fetch")
│ └── th_block0
│ ├── type ("All Errors")
│ ├── error_count
│ ├── interrupt_enable
│ └── threshold_limit
├── th_bank1
│ ├── type ("Northbridge")
│ ├── th_block0
│ │ ├── type ("DRAM Errors")
│ │ ├── error_count
│ │ ├── interrupt_enable
│ │ └── threshold_limit
│ └── th_block1
│ ├── type ("Link Errors")
│ ├── error_count
│ ├── interrupt_enable
│ └── threshold_limit
...
OR
/sys/devices/system/machinecheck/machinecheck0/thresholding
├── bank0
│ ├── desc ("Instruction Fetch")
│ └── block0
│ ├── desc ("All Errors")
│ ├── error_count
│ ├── interrupt_enable
│ └── threshold_limit
├── bank1
│ ├── desc ("Northbridge")
│ ├── block0
│ │ ├── desc ("DRAM Errors")
│ │ ├── error_count
│ │ ├── interrupt_enable
│ │ └── threshold_limit
│ └── block1
│ ├── desc ("Link Errors")
│ ├── error_count
│ ├── interrupt_enable
│ └── threshold_limit
...
I'm inclined to the second option, since it keeps all the thresholding
functionality under a single directory.
What do you think?
Thanks,
Yazen
On Tue, Dec 07, 2021 at 04:28:42PM +0000, Yazen Ghannam wrote:
> Sure thing. But I don't think removing the second directory will be okay. The
> layout is "bank"/"block". If the "block" has special use like DRAM ECC, or L3
> Cache on older systems, then it'll have a unique name. Otherwise, the block
> will take the name of the bank.
Ah, there was something... and I found a good example on my zen1 box:
├── umc_0
│ ├── dram_ecc
│ │ ├── error_count
│ │ ├── interrupt_enable
│ │ └── threshold_limit
│ └── misc_umc
│ ├── error_count
│ ├── interrupt_enable
│ └── threshold_limit
but yeah, that still doesn't make it clear how the hierarchy is...
> /sys/devices/system/machinecheck/machinecheck0/thresholding
> ├── bank0
> │ ├── desc ("Instruction Fetch")
> │ └── block0
> │ ├── desc ("All Errors")
> │ ├── error_count
> │ ├── interrupt_enable
> │ └── threshold_limit
> ├── bank1
> │ ├── desc ("Northbridge")
> │ ├── block0
> │ │ ├── desc ("DRAM Errors")
> │ │ ├── error_count
> │ │ ├── interrupt_enable
> │ │ └── threshold_limit
> │ └── block1
> │ ├── desc ("Link Errors")
> │ ├── error_count
> │ ├── interrupt_enable
> │ └── threshold_limit
> ...
>
> I'm inclined to the second option, since it keeps all the thresholding
> functionality under a single directory.
Yeah, that makes it explicit and one can see that a bank can have
multiple blocks.
Renaming will change the ABI but we can always do symlinks later if
people complain. Which I doubt because I've yet to hear of someone using
that thresholding thing at all...
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette