Hi all,
This set adds support for AMD Family 19h Models 10h-1Fh and A0h-AFh.
This set is based on the following branches:
- tip/master
- ras/edac-for-next
- groeck/linux-staging/hwmon-next
The following commit in hwmon-next is needed for functional support of
this set.
49e90c39d0be ("x86/amd_nb: Add AMD Family 19h Models (10h-1Fh) and (A0h-AFh) PCI IDs")
Patch 1 has been completely reworked into a new patch to support setting
the "memory type" per DIMM rather than assuming all DIMMs are the same.
Patch 2 adds register offset and other minor changes introduced with
these new models.
Thanks,
Yazen
Link:
https://lkml.kernel.org/r/[email protected]
v2->v3:
* Patch 1 completely reworked.
* Patch 2 updated based on comments from William.
Yazen Ghannam (2):
EDAC/amd64: Set memory type per DIMM
EDAC/amd64: Add new register offset support and related changes
drivers/edac/amd64_edac.c | 101 ++++++++++++++++++++++++++++++--------
drivers/edac/amd64_edac.h | 14 ++++++
2 files changed, 95 insertions(+), 20 deletions(-)
--
2.25.1
Current AMD systems allow mixing of DIMM types within a system. However,
DIMMs within a channel, i.e. managed by a single Unified Memory
Controller (UMC), must be of the same type.
Handle this possible configuration by checking and setting the memory
type for each individual EDAC "DIMM" structure.
Signed-off-by: Yazen Ghannam <[email protected]>
---
Link:
https://lkml.kernel.org/r/[email protected]
v2->v3:
* Change patch to properly handle systems with different DIMM types.
v1->v2:
* Was patch 3 in v1.
* Update commit message.
drivers/edac/amd64_edac.c | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index fba609ada0e6..4db92c77276f 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -1616,19 +1616,23 @@ static void read_dct_base_mask(struct amd64_pvt *pvt)
}
}
+static enum mem_type determine_memory_type_df(struct amd64_umc *umc)
+{
+ if (umc->dimm_cfg & BIT(5))
+ return MEM_LRDDR4;
+
+ if (umc->dimm_cfg & BIT(4))
+ return MEM_RDDR4;
+
+ return MEM_DDR4;
+}
+
static void determine_memory_type(struct amd64_pvt *pvt)
{
u32 dram_ctrl, dcsm;
- if (pvt->umc) {
- if ((pvt->umc[0].dimm_cfg | pvt->umc[1].dimm_cfg) & BIT(5))
- pvt->dram_type = MEM_LRDDR4;
- else if ((pvt->umc[0].dimm_cfg | pvt->umc[1].dimm_cfg) & BIT(4))
- pvt->dram_type = MEM_RDDR4;
- else
- pvt->dram_type = MEM_DDR4;
+ if (pvt->umc)
return;
- }
switch (pvt->fam) {
case 0xf:
@@ -3547,8 +3551,8 @@ static int init_csrows_df(struct mem_ctl_info *mci)
edac_dbg(1, "MC node: %d, csrow: %d\n",
pvt->mc_node_id, cs);
+ dimm->mtype = determine_memory_type_df(&pvt->umc[umc]);
dimm->nr_pages = get_csrow_nr_pages(pvt, umc, cs);
- dimm->mtype = pvt->dram_type;
dimm->edac_mode = edac_mode;
dimm->dtype = dev_type;
dimm->grain = 64;
--
2.25.1
Introduce a "family flags" bitmask that can be used to indicate any
special behavior needed on a per-family basis.
Add a flag to indicate a system uses the new register offsets introduced
with Family 19h Model 10h.
Use this flag to account for register offset changes, a new bitfield
indicating DDR5 use on a memory controller, and to set the proper number
of chip select masks.
Rework f17_addr_mask_to_cs_size() to properly handle the change in chip
select masks. And update code comments to reflect the updated Chip
Select, DIMM, and Mask relationships.
Signed-off-by: Yazen Ghannam <[email protected]>
---
Link:
https://lkml.kernel.org/r/[email protected]
v2->v3:
* Adjust variable names to explicitly show what they represent.
* Update code comment to give more detail on CS/MASK/DIMM layout.
v1->v2:
* Was patch 4 in v1.
* Change "has_ddr5" flag to "zn_regs_v2".
* Drop flag check helper function.
* Update determine_memory_type() to check bitfield for DDR5.
* Update code comments.
drivers/edac/amd64_edac.c | 79 +++++++++++++++++++++++++++++++++------
drivers/edac/amd64_edac.h | 14 +++++++
2 files changed, 82 insertions(+), 11 deletions(-)
diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 4db92c77276f..a299c361a904 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -15,6 +15,31 @@ static struct msr __percpu *msrs;
static struct amd64_family_type *fam_type;
+/* Family flag helpers */
+static inline u64 get_addr_cfg(void)
+{
+ if (fam_type->flags.zn_regs_v2)
+ return UMCCH_ADDR_CFG_DDR5;
+
+ return UMCCH_ADDR_CFG;
+}
+
+static inline u64 get_addr_mask_sec(void)
+{
+ if (fam_type->flags.zn_regs_v2)
+ return UMCCH_ADDR_MASK_SEC_DDR5;
+
+ return UMCCH_ADDR_MASK_SEC;
+}
+
+static inline u64 get_dimm_cfg(void)
+{
+ if (fam_type->flags.zn_regs_v2)
+ return UMCCH_DIMM_CFG_DDR5;
+
+ return UMCCH_DIMM_CFG;
+}
+
/* Per-node stuff */
static struct ecc_settings **ecc_stngs;
@@ -1429,8 +1454,10 @@ static void __dump_misc_regs_df(struct amd64_pvt *pvt)
edac_dbg(1, "UMC%d x16 DIMMs present: %s\n",
i, (umc->dimm_cfg & BIT(7)) ? "yes" : "no");
- if (pvt->dram_type == MEM_LRDDR4) {
- amd_smn_read(pvt->mc_node_id, umc_base + UMCCH_ADDR_CFG, &tmp);
+ if (pvt->dram_type == MEM_LRDDR4 || pvt->dram_type == MEM_LRDDR5) {
+ amd_smn_read(pvt->mc_node_id,
+ umc_base + get_addr_cfg(),
+ &tmp);
edac_dbg(1, "UMC%d LRDIMM %dx rank multiply\n",
i, 1 << ((tmp >> 4) & 0x3));
}
@@ -1505,7 +1532,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 = 2;
+ pvt->csels[umc].m_cnt = fam_type->flags.zn_regs_v2 ? 4 : 2;
}
} else {
@@ -1545,7 +1572,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) + UMCCH_ADDR_MASK_SEC;
+ umc_mask_reg_sec = get_umc_base(umc) + get_addr_mask_sec();
for_each_chip_select_mask(cs, umc, pvt) {
mask = &pvt->csels[umc].csmasks[cs];
@@ -1618,6 +1645,20 @@ static void read_dct_base_mask(struct amd64_pvt *pvt)
static enum mem_type 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 (umc->dimm_cfg & BIT(5))
+ return MEM_LRDDR5;
+
+ if (umc->dimm_cfg & BIT(4))
+ return MEM_RDDR5;
+
+ return MEM_DDR5;
+ }
+
if (umc->dimm_cfg & BIT(5))
return MEM_LRDDR4;
@@ -2153,6 +2194,7 @@ static int f17_addr_mask_to_cs_size(struct amd64_pvt *pvt, u8 umc,
{
u32 addr_mask_orig, addr_mask_deinterleaved;
u32 msb, weight, num_zero_bits;
+ int cs_mask_nr = csrow_nr;
int dimm, size = 0;
/* No Chip Selects are enabled. */
@@ -2168,17 +2210,31 @@ static int f17_addr_mask_to_cs_size(struct amd64_pvt *pvt, u8 umc,
return size;
/*
- * There is one mask per DIMM, and two Chip Selects per DIMM.
- * CS0 and CS1 -> DIMM0
- * CS2 and CS3 -> DIMM1
+ * Family 17h introduced systems with one mask per DIMM,
+ * and two Chip Selects per DIMM.
+ *
+ * CS0 and CS1 -> MASK0 / DIMM0
+ * CS2 and CS3 -> MASK1 / DIMM1
+ *
+ * Family 19h Model 10h introduced systems with one mask per Chip Select,
+ * and two Chip Selects per DIMM.
+ *
+ * CS0 -> MASK0 -> DIMM0
+ * CS1 -> MASK1 -> DIMM0
+ * CS2 -> MASK2 -> DIMM1
+ * CS3 -> MASK3 -> DIMM1
+ *
+ * Keep the mask number equal to the Chip Select number for newer systems,
+ * and shift the mask number for older systems.
*/
- dimm = csrow_nr >> 1;
+ if (!fam_type->flags.zn_regs_v2)
+ cs_mask_nr >>= 1;
/* Asymmetric dual-rank DIMM support. */
if ((csrow_nr & 1) && (cs_mode & CS_ODD_SECONDARY))
- addr_mask_orig = pvt->csels[umc].csmasks_sec[dimm];
+ addr_mask_orig = pvt->csels[umc].csmasks_sec[cs_mask_nr];
else
- addr_mask_orig = pvt->csels[umc].csmasks[dimm];
+ 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
@@ -2934,6 +2990,7 @@ static struct amd64_family_type family_types[] = {
.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,
@@ -3372,7 +3429,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 + UMCCH_DIMM_CFG, &umc->dimm_cfg);
+ amd_smn_read(nid, umc_base + get_dimm_cfg(), &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);
diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
index 352bda9803f6..4598a5cd0361 100644
--- a/drivers/edac/amd64_edac.h
+++ b/drivers/edac/amd64_edac.h
@@ -273,8 +273,11 @@
#define UMCCH_BASE_ADDR_SEC 0x10
#define UMCCH_ADDR_MASK 0x20
#define UMCCH_ADDR_MASK_SEC 0x28
+#define UMCCH_ADDR_MASK_SEC_DDR5 0x30
#define UMCCH_ADDR_CFG 0x30
+#define UMCCH_ADDR_CFG_DDR5 0x40
#define UMCCH_DIMM_CFG 0x80
+#define UMCCH_DIMM_CFG_DDR5 0x90
#define UMCCH_UMC_CFG 0x100
#define UMCCH_SDP_CTRL 0x104
#define UMCCH_ECC_CTRL 0x14C
@@ -480,11 +483,22 @@ struct low_ops {
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;
};
--
2.25.1
Hi Yazen,
I love your patch! Perhaps something to improve:
[auto build test WARNING on ras/edac-for-next]
[also build test WARNING on next-20211224]
[cannot apply to linux/master linus/master v5.16-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Yazen-Ghannam/AMD-Family-19h-Models-10h-1Fh-Updates/20211229-040749
base: https://git.kernel.org/pub/scm/linux/kernel/git/ras/ras.git edac-for-next
config: x86_64-randconfig-c007-20211231 (https://download.01.org/0day-ci/archive/20220101/[email protected]/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 7cd109b92c72855937273a6c8ab19016fbe27d33)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/dfcd741f577d123f8b488cf88979d6bac6dca5da
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Yazen-Ghannam/AMD-Family-19h-Models-10h-1Fh-Updates/20211229-040749
git checkout dfcd741f577d123f8b488cf88979d6bac6dca5da
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/edac/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
>> drivers/edac/amd64_edac.c:1982:52: warning: variable 'dimm' is uninitialized when used here [-Wuninitialized]
edac_dbg(1, "CS%d DIMM%d AddrMasks:\n", csrow_nr, dimm);
^~~~
drivers/edac/edac_mc.h:77:32: note: expanded from macro 'edac_dbg'
"%s: " fmt, __func__, ##__VA_ARGS__); \
^~~~~~~~~~~
drivers/edac/edac_mc.h:49:42: note: expanded from macro 'edac_printk'
printk(level "EDAC " prefix ": " fmt, ##arg)
^~~
include/linux/printk.h:450:60: note: expanded from macro 'printk'
#define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)
^~~~~~~~~~~
include/linux/printk.h:422:19: note: expanded from macro 'printk_index_wrap'
_p_func(_fmt, ##__VA_ARGS__); \
^~~~~~~~~~~
drivers/edac/amd64_edac.c:1923:10: note: initialize the variable 'dimm' to silence this warning
int dimm, size = 0;
^
= 0
1 warning generated.
vim +/dimm +1982 drivers/edac/amd64_edac.c
94c1acf2c85b03 Aravind Gopalakrishnan 2013-04-17 1916
e53a3b267fb0a7 Yazen Ghannam 2019-08-21 1917 static int f17_addr_mask_to_cs_size(struct amd64_pvt *pvt, u8 umc,
f1cbbec9fce958 Yazen Ghannam 2016-11-17 1918 unsigned int cs_mode, int csrow_nr)
f1cbbec9fce958 Yazen Ghannam 2016-11-17 1919 {
e53a3b267fb0a7 Yazen Ghannam 2019-08-21 1920 u32 addr_mask_orig, addr_mask_deinterleaved;
e53a3b267fb0a7 Yazen Ghannam 2019-08-21 1921 u32 msb, weight, num_zero_bits;
dfcd741f577d12 Yazen Ghannam 2021-12-28 1922 int cs_mask_nr = csrow_nr;
e53a3b267fb0a7 Yazen Ghannam 2019-08-21 1923 int dimm, size = 0;
f1cbbec9fce958 Yazen Ghannam 2016-11-17 1924
e53a3b267fb0a7 Yazen Ghannam 2019-08-21 1925 /* No Chip Selects are enabled. */
e53a3b267fb0a7 Yazen Ghannam 2019-08-21 1926 if (!cs_mode)
e53a3b267fb0a7 Yazen Ghannam 2019-08-21 1927 return size;
f1cbbec9fce958 Yazen Ghannam 2016-11-17 1928
e53a3b267fb0a7 Yazen Ghannam 2019-08-21 1929 /* Requested size of an even CS but none are enabled. */
e53a3b267fb0a7 Yazen Ghannam 2019-08-21 1930 if (!(cs_mode & CS_EVEN) && !(csrow_nr & 1))
e53a3b267fb0a7 Yazen Ghannam 2019-08-21 1931 return size;
f1cbbec9fce958 Yazen Ghannam 2016-11-17 1932
e53a3b267fb0a7 Yazen Ghannam 2019-08-21 1933 /* Requested size of an odd CS but none are enabled. */
e53a3b267fb0a7 Yazen Ghannam 2019-08-21 1934 if (!(cs_mode & CS_ODD) && (csrow_nr & 1))
e53a3b267fb0a7 Yazen Ghannam 2019-08-21 1935 return size;
e53a3b267fb0a7 Yazen Ghannam 2019-08-21 1936
e53a3b267fb0a7 Yazen Ghannam 2019-08-21 1937 /*
dfcd741f577d12 Yazen Ghannam 2021-12-28 1938 * Family 17h introduced systems with one mask per DIMM,
dfcd741f577d12 Yazen Ghannam 2021-12-28 1939 * and two Chip Selects per DIMM.
dfcd741f577d12 Yazen Ghannam 2021-12-28 1940 *
dfcd741f577d12 Yazen Ghannam 2021-12-28 1941 * CS0 and CS1 -> MASK0 / DIMM0
dfcd741f577d12 Yazen Ghannam 2021-12-28 1942 * CS2 and CS3 -> MASK1 / DIMM1
dfcd741f577d12 Yazen Ghannam 2021-12-28 1943 *
dfcd741f577d12 Yazen Ghannam 2021-12-28 1944 * Family 19h Model 10h introduced systems with one mask per Chip Select,
dfcd741f577d12 Yazen Ghannam 2021-12-28 1945 * and two Chip Selects per DIMM.
dfcd741f577d12 Yazen Ghannam 2021-12-28 1946 *
dfcd741f577d12 Yazen Ghannam 2021-12-28 1947 * CS0 -> MASK0 -> DIMM0
dfcd741f577d12 Yazen Ghannam 2021-12-28 1948 * CS1 -> MASK1 -> DIMM0
dfcd741f577d12 Yazen Ghannam 2021-12-28 1949 * CS2 -> MASK2 -> DIMM1
dfcd741f577d12 Yazen Ghannam 2021-12-28 1950 * CS3 -> MASK3 -> DIMM1
dfcd741f577d12 Yazen Ghannam 2021-12-28 1951 *
dfcd741f577d12 Yazen Ghannam 2021-12-28 1952 * Keep the mask number equal to the Chip Select number for newer systems,
dfcd741f577d12 Yazen Ghannam 2021-12-28 1953 * and shift the mask number for older systems.
e53a3b267fb0a7 Yazen Ghannam 2019-08-21 1954 */
dfcd741f577d12 Yazen Ghannam 2021-12-28 1955 if (!fam_type->flags.zn_regs_v2)
dfcd741f577d12 Yazen Ghannam 2021-12-28 1956 cs_mask_nr >>= 1;
e53a3b267fb0a7 Yazen Ghannam 2019-08-21 1957
81f5090db843be Yazen Ghannam 2019-08-22 1958 /* Asymmetric dual-rank DIMM support. */
81f5090db843be Yazen Ghannam 2019-08-22 1959 if ((csrow_nr & 1) && (cs_mode & CS_ODD_SECONDARY))
dfcd741f577d12 Yazen Ghannam 2021-12-28 1960 addr_mask_orig = pvt->csels[umc].csmasks_sec[cs_mask_nr];
81f5090db843be Yazen Ghannam 2019-08-22 1961 else
dfcd741f577d12 Yazen Ghannam 2021-12-28 1962 addr_mask_orig = pvt->csels[umc].csmasks[cs_mask_nr];
e53a3b267fb0a7 Yazen Ghannam 2019-08-21 1963
e53a3b267fb0a7 Yazen Ghannam 2019-08-21 1964 /*
e53a3b267fb0a7 Yazen Ghannam 2019-08-21 1965 * The number of zero bits in the mask is equal to the number of bits
e53a3b267fb0a7 Yazen Ghannam 2019-08-21 1966 * in a full mask minus the number of bits in the current mask.
e53a3b267fb0a7 Yazen Ghannam 2019-08-21 1967 *
e53a3b267fb0a7 Yazen Ghannam 2019-08-21 1968 * The MSB is the number of bits in the full mask because BIT[0] is
e53a3b267fb0a7 Yazen Ghannam 2019-08-21 1969 * always 0.
9f4873fb6af796 Yazen Ghannam 2021-10-05 1970 *
9f4873fb6af796 Yazen Ghannam 2021-10-05 1971 * In the special 3 Rank interleaving case, a single bit is flipped
9f4873fb6af796 Yazen Ghannam 2021-10-05 1972 * without swapping with the most significant bit. This can be handled
9f4873fb6af796 Yazen Ghannam 2021-10-05 1973 * by keeping the MSB where it is and ignoring the single zero bit.
e53a3b267fb0a7 Yazen Ghannam 2019-08-21 1974 */
e53a3b267fb0a7 Yazen Ghannam 2019-08-21 1975 msb = fls(addr_mask_orig) - 1;
e53a3b267fb0a7 Yazen Ghannam 2019-08-21 1976 weight = hweight_long(addr_mask_orig);
9f4873fb6af796 Yazen Ghannam 2021-10-05 1977 num_zero_bits = msb - weight - !!(cs_mode & CS_3R_INTERLEAVE);
e53a3b267fb0a7 Yazen Ghannam 2019-08-21 1978
e53a3b267fb0a7 Yazen Ghannam 2019-08-21 1979 /* Take the number of zero bits off from the top of the mask. */
e53a3b267fb0a7 Yazen Ghannam 2019-08-21 1980 addr_mask_deinterleaved = GENMASK_ULL(msb - num_zero_bits, 1);
e53a3b267fb0a7 Yazen Ghannam 2019-08-21 1981
e53a3b267fb0a7 Yazen Ghannam 2019-08-21 @1982 edac_dbg(1, "CS%d DIMM%d AddrMasks:\n", csrow_nr, dimm);
e53a3b267fb0a7 Yazen Ghannam 2019-08-21 1983 edac_dbg(1, " Original AddrMask: 0x%x\n", addr_mask_orig);
e53a3b267fb0a7 Yazen Ghannam 2019-08-21 1984 edac_dbg(1, " Deinterleaved AddrMask: 0x%x\n", addr_mask_deinterleaved);
e53a3b267fb0a7 Yazen Ghannam 2019-08-21 1985
e53a3b267fb0a7 Yazen Ghannam 2019-08-21 1986 /* Register [31:1] = Address [39:9]. Size is in kBs here. */
e53a3b267fb0a7 Yazen Ghannam 2019-08-21 1987 size = (addr_mask_deinterleaved >> 2) + 1;
f1cbbec9fce958 Yazen Ghannam 2016-11-17 1988
f1cbbec9fce958 Yazen Ghannam 2016-11-17 1989 /* Return size in MBs. */
f1cbbec9fce958 Yazen Ghannam 2016-11-17 1990 return size >> 10;
f1cbbec9fce958 Yazen Ghannam 2016-11-17 1991 }
f1cbbec9fce958 Yazen Ghannam 2016-11-17 1992
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
On Tue, Dec 28, 2021 at 08:06:15PM +0000, Yazen Ghannam wrote:
> diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
> index 4db92c77276f..a299c361a904 100644
> --- a/drivers/edac/amd64_edac.c
> +++ b/drivers/edac/amd64_edac.c
> @@ -15,6 +15,31 @@ static struct msr __percpu *msrs;
>
> static struct amd64_family_type *fam_type;
>
> +/* Family flag helpers */
> +static inline u64 get_addr_cfg(void)
> +{
> + if (fam_type->flags.zn_regs_v2)
> + return UMCCH_ADDR_CFG_DDR5;
> +
> + return UMCCH_ADDR_CFG;
> +}
> +
> +static inline u64 get_addr_mask_sec(void)
> +{
> + if (fam_type->flags.zn_regs_v2)
> + return UMCCH_ADDR_MASK_SEC_DDR5;
> +
> + return UMCCH_ADDR_MASK_SEC;
> +}
> +
> +static inline u64 get_dimm_cfg(void)
> +{
> + if (fam_type->flags.zn_regs_v2)
> + return UMCCH_DIMM_CFG_DDR5;
> +
> + return UMCCH_DIMM_CFG;
> +}
Yeah, you can do it either this way and have a lot of small functions
or you can do what I did with mca_msr_reg() which is a single mapping
function you then use everywhere.
Your call.
> +
> /* Per-node stuff */
> static struct ecc_settings **ecc_stngs;
>
> @@ -1429,8 +1454,10 @@ static void __dump_misc_regs_df(struct amd64_pvt *pvt)
> edac_dbg(1, "UMC%d x16 DIMMs present: %s\n",
> i, (umc->dimm_cfg & BIT(7)) ? "yes" : "no");
>
> - if (pvt->dram_type == MEM_LRDDR4) {
> - amd_smn_read(pvt->mc_node_id, umc_base + UMCCH_ADDR_CFG, &tmp);
> + if (pvt->dram_type == MEM_LRDDR4 || pvt->dram_type == MEM_LRDDR5) {
This still keeps the ->dram_type per pvt, which is per memory controller
in amd64_edac nomenclature.
But AFAIR, we said last time that the DRAM type is per UMC now, as you
do in the previous patch.
Which means, you either have to test umc->dimm_cfg to get the DRAM type
here or push ->dram_type into the umc struct...
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Fri, Jan 21, 2022 at 01:25:21PM +0100, Borislav Petkov wrote:
> On Tue, Dec 28, 2021 at 08:06:15PM +0000, Yazen Ghannam wrote:
> > diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
> > index 4db92c77276f..a299c361a904 100644
> > --- a/drivers/edac/amd64_edac.c
> > +++ b/drivers/edac/amd64_edac.c
> > @@ -15,6 +15,31 @@ static struct msr __percpu *msrs;
> >
> > static struct amd64_family_type *fam_type;
> >
> > +/* Family flag helpers */
> > +static inline u64 get_addr_cfg(void)
> > +{
> > + if (fam_type->flags.zn_regs_v2)
> > + return UMCCH_ADDR_CFG_DDR5;
> > +
> > + return UMCCH_ADDR_CFG;
> > +}
> > +
> > +static inline u64 get_addr_mask_sec(void)
> > +{
> > + if (fam_type->flags.zn_regs_v2)
> > + return UMCCH_ADDR_MASK_SEC_DDR5;
> > +
> > + return UMCCH_ADDR_MASK_SEC;
> > +}
> > +
> > +static inline u64 get_dimm_cfg(void)
> > +{
> > + if (fam_type->flags.zn_regs_v2)
> > + return UMCCH_DIMM_CFG_DDR5;
> > +
> > + return UMCCH_DIMM_CFG;
> > +}
>
> Yeah, you can do it either this way and have a lot of small functions
> or you can do what I did with mca_msr_reg() which is a single mapping
> function you then use everywhere.
>
> Your call.
>
Yes, I'll do this.
> > +
> > /* Per-node stuff */
> > static struct ecc_settings **ecc_stngs;
> >
> > @@ -1429,8 +1454,10 @@ static void __dump_misc_regs_df(struct amd64_pvt *pvt)
> > edac_dbg(1, "UMC%d x16 DIMMs present: %s\n",
> > i, (umc->dimm_cfg & BIT(7)) ? "yes" : "no");
> >
> > - if (pvt->dram_type == MEM_LRDDR4) {
> > - amd_smn_read(pvt->mc_node_id, umc_base + UMCCH_ADDR_CFG, &tmp);
> > + if (pvt->dram_type == MEM_LRDDR4 || pvt->dram_type == MEM_LRDDR5) {
>
> This still keeps the ->dram_type per pvt, which is per memory controller
> in amd64_edac nomenclature.
>
> But AFAIR, we said last time that the DRAM type is per UMC now, as you
> do in the previous patch.
>
> Which means, you either have to test umc->dimm_cfg to get the DRAM type
> here or push ->dram_type into the umc struct...
>
Yep, you're right. I'll cache the dram_type in the umc struct.
Thanks,
Yazen