Messages in this thread | | | Date | Wed, 15 Dec 2021 17:32:27 +0100 | Subject | Re: [PATCH v2 2/2] EDAC/amd64: Add new register offset support and related changes | From | William Roche <> |
| |
On 15/12/2021 16:53, Yazen Ghannam wrote: > 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. > > Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com> > --- > Link: > https://lkml.kernel.org/r/20211208174356.1997855-5-yazen.ghannam@amd.com > > 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 | 59 +++++++++++++++++++++++++++++++++++---- > drivers/edac/amd64_edac.h | 14 ++++++++++ > 2 files changed, 67 insertions(+), 6 deletions(-) > > diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c > index 1df763128483..b7dd87636155 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]; > @@ -1628,6 +1655,20 @@ static void determine_memory_type(struct amd64_pvt *pvt) > dimm_cfg |= pvt->umc[i].dimm_cfg; > } > > + /* > + * 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_cfg & GENMASK(2, 0)) == 0x1)) { > + if (dimm_cfg & BIT(5)) > + pvt->dram_type = MEM_LRDDR5; > + else if (dimm_cfg & BIT(4)) > + pvt->dram_type = MEM_RDDR5; > + else > + pvt->dram_type = MEM_DDR5; > + return; > + } > + > if (dimm_cfg & BIT(5)) > pvt->dram_type = MEM_LRDDR4; > else if (dimm_cfg & BIT(4)) > @@ -2174,8 +2215,13 @@ static int f17_addr_mask_to_cs_size(struct amd64_pvt *pvt, u8 umc, > * There is one mask per DIMM, and two Chip Selects per DIMM. > * CS0 and CS1 -> DIMM0 > * CS2 and CS3 -> DIMM1 > + * > + * Systems with newer register layout have one mask per Chip Select.
Just a question about this comment: Can it be translated into this ?
+ * Except on systems with newer register layout where we have one Chip Select per DIMM.
> */ > - dimm = csrow_nr >> 1; > + if (fam_type->flags.zn_regs_v2) > + dimm = csrow_nr; > + else > + dimm = csrow_nr >> 1; > > /* Asymmetric dual-rank DIMM support. */ > if ((csrow_nr & 1) && (cs_mode & CS_ODD_SECONDARY)) > @@ -2937,6 +2983,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, > @@ -3365,7 +3412,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 650cab401e21..39ecb77873db 100644 > --- a/drivers/edac/amd64_edac.h > +++ b/drivers/edac/amd64_edac.h > @@ -271,8 +271,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 > @@ -477,11 +480,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; > }; > Thanks in advance,
William.
| |