On 3/2/2016 4:53 AM, Borislav Petkov wrote:
Merge all IP blocks into a single enum. This allows for easier block
name use later. Drop superfluous "_BLOCK" from the enum names.
Signed-off-by: Borislav Petkov <b...@suse.de>
Cc: Aravind Gopalakrishnan <aravind.gopalakrish...@amd.com>
enum amd_ip_types {
- SMCA_F17H_CORE_BLOCK = 0, /* Core errors */
- SMCA_DF_BLOCK, /* Data Fabric */
- SMCA_UMC_BLOCK, /* Unified Memory Controller */
- SMCA_PB_BLOCK, /* Parameter Block */
- SMCA_PSP_BLOCK, /* Platform Security Processor */
- SMCA_SMU_BLOCK, /* System Management Unit */
+ SMCA_F17H_CORE = 0, /* Core errors */
+ SMCA_LS, /* - Load Store */
+ SMCA_IF, /* - Instruction Fetch */
+ SMCA_L2_CACHE, /* - L2 cache */
+ SMCA_DE, /* - Decoder unit */
+ RES, /* - Reserved */
+ SMCA_EX, /* - Execution unit */
+ SMCA_FP, /* - Floating Point */
+ SMCA_L3_CACHE, /* - L3 cache */
+
+ SMCA_DF, /* Data Fabric */
+ SMCA_CS, /* - Coherent Slave */
+ SMCA_PIE, /* - Power management, Interrupts, etc */
+
+ SMCA_UMC, /* Unified Memory Controller */
+ SMCA_PB, /* Parameter Block */
+ SMCA_PSP, /* Platform Security Processor */
+ SMCA_SMU, /* System Management Unit */
N_AMD_IP_TYPES
};
No, this would break the logic in EDAC.
The main reason I placed it in separate enums is because the "mca_type"
values map to the enum.
These blocks-
+ SMCA_LS, /* - Load Store */
+ SMCA_IF, /* - Instruction Fetch */
+ SMCA_L2_CACHE, /* - L2 cache */
+ SMCA_DE, /* - Decoder unit */
+ RES, /* - Reserved */
+ SMCA_EX, /* - Execution unit */
+ SMCA_FP, /* - Floating Point */
+ SMCA_L3_CACHE, /* - L3 cache */
have the same hwid value (0xb0). But they differ in mca_type values. And
in exactly that order.
(LS is mca_type 0, IF is mca_type 1 and so on..)
So, after we get mca_type value from the MSR (mca_type = (high &
MCI_IPID_MCATYPE) >> 16),
We check for LS (=0) or IF (=1) ...
With this change, LS block is assigned 1 due to the ordering in enum.
And this logic applies to "DF" block as well. (whose component blocks
are "coherent slave" and "pie" which have mca_type values of 0 and 1
respectively)
"DF" and "F17h_core" are essentially parent blocks and CS, PIE, LS, IF
etc are children. hwid indicates the parent, mca_type indicates the child..
/* Define HWID to IP type mappings for Scalable MCA */
-struct amd_hwid amd_hwid_mappings[] =
-{
- [SMCA_F17H_CORE_BLOCK] = { "f17h_core", 0xB0 },
- [SMCA_DF_BLOCK] = { "data fabric", 0x2E },
- [SMCA_UMC_BLOCK] = { "UMC", 0x96 },
- [SMCA_PB_BLOCK] = { "param block", 0x5 },
- [SMCA_PSP_BLOCK] = { "PSP", 0xFF },
- [SMCA_SMU_BLOCK] = { "SMU", 0x1 },
+struct amd_hwid amd_hwids[] =
+{
+ [SMCA_F17H_CORE] = { "F17h core", 0xB0 },
+ [SMCA_LS] = { "Load-Store", 0x0 },
+ [SMCA_IF] = { "IFetch", 0x0 },
+ [SMCA_L2_CACHE] = { "L2 Cache", 0x0 },
+ [SMCA_DE] = { "Decoder", 0x0 },
+ [SMCA_EX] = { "Execution", 0x0 },
+ [SMCA_FP] = { "Floating Point", 0x0 },
+ [SMCA_L3_CACHE] = { "L3 Cache", 0x0 },
+ [SMCA_DF] = { "Data Fabric", 0x2E },
+ [SMCA_CS] = { "Coherent Slave", 0x0 },
+ [SMCA_PIE] = { "PwrMan/Intr", 0x0 },
+ [SMCA_UMC] = { "UMC", 0x96 },
+ [SMCA_PB] = { "Param Block", 0x5 },
+ [SMCA_PSP] = { "PSP", 0xFF },
+ [SMCA_SMU] = { "SMU", 0x1 },
};
-EXPORT_SYMBOL_GPL(amd_hwid_mappings);
+EXPORT_SYMBOL_GPL(amd_hwids);
These strings are what I intend to use for the sysfs interface.
So, I am not sure if "PwrMan/Intr" would work when I need to create the
kobj..
Also, the legacy names use snake_case-
static const char * const th_names[] = {
"load_store",
"insn_fetch",
"combined_unit",
"",
"northbridge",
"execution_unit",
};
So, I think we should continue this approach and have something like this-
static const char * const amd_core_mcablock_names[] = {
[SMCA_LS] = "load_store",
[SMCA_IF] = "insn_fetch",
[SMCA_L2_CACHE] = "l2_cache",
[SMCA_DE] = "decode_unit",
[RES] = "",
[SMCA_EX] = "execution_unit",
[SMCA_FP] = "floating_point",
[SMCA_L3_CACHE] = "l3_cache",
};
static const char * const amd_df_mcablock_names[] = {
[SMCA_CS] = "coherent_slave",
[SMCA_PIE] = "pie",
};
(Split arrays again because I feel it'd be better to have arrays ordered
according to mca_type values)
Expanding "df" to "data_fabric" and "pb" to "param_block" is fine with me.
if (xec > len) {
- pr_cont("Unrecognized error code from %s MCA bank\n", ip_name);
+ pr_cont("Unrecognized error code from %s MCA bank\n",
amd_hwids[mca_type].name);
This wouldn't work because of the mca_type ordering as mentioned above.
(Or it should be amd_core_mcablock_names[mca_type])
if (xec > len) {
- pr_cont("Unrecognized error code from %s MCA bank\n", ip_name);
+ pr_cont("Unrecognized error code from %s MCA bank\n",
amd_hwids[mca_type].name);
return;
}
Ditto.
- pr_emerg(HW_ERR "%s Error: ", ip_name);
+ ip_name = amd_hwids[mca_type].name;
This should be amd_hwids[i].name
We shouldn't be using mca_type value as index..
Thanks,
-Aravind.
