| Message ID | 1411070230-10298-1-git-send-email-Aravind.Gopalakrishnan@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Thu, Sep 18, 2014 at 02:57:10PM -0500, Aravind Gopalakrishnan wrote: > This patch adds support for ECC error decoding for F15h M60h processor. > Aside from the usual changes, the patch adds support for some new features > in the processor: > - DDR4(unbuffered, registered); LRDIMM DDR3 support > - relevant debug messages have been modified/added to report these > memory types > - new dbam_to_cs mappers > - if (F15h M60h && LRDIMM); we need a 'multiplier' value to find > cs_size. This multiplier value is obtained from the per-dimm > DCSM register. So, change the interface to accept a 'cs_mask_nr' > value to facilitate this calculation > Misc cleanup: > - amd64_pci_table[] is condensed by using PCI_VDEVICE macro. > > Testing details: > Tested the patch by injecting 'ECC' type errors using mce_amd_inj > and error decoding works fine. > > Signed-off-by: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com> > --- > drivers/edac/amd64_edac.c | 226 ++++++++++++++++++++++++++++++++-------------- > drivers/edac/amd64_edac.h | 12 ++- > 2 files changed, 169 insertions(+), 69 deletions(-) > > diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c > index bbd6514..3e265e0 100644 > --- a/drivers/edac/amd64_edac.c > +++ b/drivers/edac/amd64_edac.c > @@ -690,11 +690,32 @@ static void debug_display_dimm_sizes(struct amd64_pvt *, u8); > > static void debug_dump_dramcfg_low(struct amd64_pvt *pvt, u32 dclr, int chan) > { > + int cs; > + > edac_dbg(1, "F2x%d90 (DRAM Cfg Low): 0x%08x\n", chan, dclr); > > - edac_dbg(1, " DIMM type: %sbuffered; all DIMMs support ECC: %s\n", > - (dclr & BIT(16)) ? "un" : "", > - (dclr & BIT(19)) ? "yes" : "no"); > + if (pvt->fam == 0x15 && pvt->model == 0x60) { > + for_each_chip_select_mask(cs, chan, pvt) { > + u32 dcsm = pvt->csels[chan].csmasks[cs]; > + > + if (dcsm & 0x3) { > + /* LRDIMMs */ > + edac_dbg(1, " DIMM type: LRDIMM %dx rank multiply;" > + "CS = %d; all DIMMs support ECC: %s\n", > + (dcsm & 0x3), cs, > + (dclr & BIT(19)) ? "yes" : "no"); Why do we need to iterate over the DRAM CS sets? Just for the rank multiplier, apparently. We dump those normally in read_dct_base_mask(), though. > + } else { > + edac_dbg(1, " DIMM type: %sbuffered; CS = %d;" > + "all DIMMs support ECC: %s\n", > + (dclr & BIT(16)) ? "un" : "", cs, > + (dclr & BIT(19)) ? "yes" : "no"); > + } > + } > + } else { > + edac_dbg(1, " DIMM type: %sbuffered; all DIMMs support ECC:" > + "%s\n", (dclr & BIT(16)) ? "un" : "", > + (dclr & BIT(19)) ? "yes" : "no"); > + } Single if-else statements don't need {} braces. > > edac_dbg(1, " PAR/ERR parity: %s\n", > (dclr & BIT(8)) ? "enabled" : "disabled"); > @@ -756,7 +777,7 @@ static void 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) { > + } 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 { > @@ -817,10 +838,26 @@ static enum mem_type determine_memory_type(struct amd64_pvt *pvt, int cs) > { > enum mem_type type; > > - /* F15h supports only DDR3 */ > - if (pvt->fam >= 0x15) > - type = (pvt->dclr0 & BIT(16)) ? MEM_DDR3 : MEM_RDDR3; > - else if (pvt->fam == 0x10 || pvt->ext_model >= K8_REV_F) { > + /* F15h, M60h supports DDR4 too*/ > + if (pvt->fam >= 0x15) { > + if (pvt->model == 0x60) { > + /* > + * Since in init_csrow we iterate over just DCT0 > + * use '0' for dct values here when necessary. > + */ > + u32 dram_ctrl; > + u32 dcsm = pvt->csels[0].csmasks[cs]; > + > + amd64_read_dct_pci_cfg(pvt, 0, DRAM_CONTROL, > + &dram_ctrl); We're reading DRAM_CONTROL at two locations, maybe we should cache it in pvt->dram_ctl ? > + type = (((dram_ctrl >> 8) & 0x7) == 0x2) ? MEM_DDR4 : > + (pvt->dclr0 & BIT(16)) ? MEM_DDR3 : > + (dcsm & 0x3) ? MEM_LRDDR3 : MEM_RDDR3; Doesn't D18F2x78_dct[3:0][DramType] define all possible types or do you have to fallback to DCLR for DDR3 types? > + } else { > + type = (pvt->dclr0 & BIT(16)) ? MEM_DDR3 : MEM_RDDR3; > + } Single if-else statements don't need {} braces. Please read Documentation/CodingStyle. > + > + } else if (pvt->fam == 0x10 || pvt->ext_model >= K8_REV_F) { > if (pvt->dchr0 & DDR3_MODE) > type = (pvt->dclr0 & BIT(16)) ? MEM_DDR3 : MEM_RDDR3; > else > @@ -958,8 +995,12 @@ static void read_dram_base_limit_regs(struct amd64_pvt *pvt, unsigned range) > if (WARN_ON(!nb)) > return; > > - pci_func = (pvt->model == 0x30) ? PCI_DEVICE_ID_AMD_15H_M30H_NB_F1 > - : PCI_DEVICE_ID_AMD_15H_NB_F1; > + if (pvt->model == 0x60) > + pci_func = PCI_DEVICE_ID_AMD_15H_M60H_NB_F1; > + else if (pvt->model == 0x30) > + pci_func = PCI_DEVICE_ID_AMD_15H_M30H_NB_F1; > + else > + pci_func = PCI_DEVICE_ID_AMD_15H_NB_F1; > > f1 = pci_get_related_function(nb->misc->vendor, pci_func, nb->misc); > if (WARN_ON(!f1)) > @@ -1049,7 +1090,7 @@ static int ddr2_cs_size(unsigned i, bool dct_width) > } > > static int k8_dbam_to_chip_select(struct amd64_pvt *pvt, u8 dct, > - unsigned cs_mode) > + unsigned cs_mode, int cs_mask_nr) > { > u32 dclr = dct ? pvt->dclr1 : pvt->dclr0; > > @@ -1167,8 +1208,43 @@ static int ddr3_cs_size(unsigned i, bool dct_width) > return cs_size; > } > > +static int ddr3_lrdimm_cs_size(unsigned i, unsigned rank_multiply) > +{ > + unsigned shift = 0; > + int cs_size = 0; > + > + if (i < 4 || i == 6) > + cs_size = -1; > + else if (i == 12) > + shift = 7; > + else if (!(i & 0x1)) > + shift = i >> 1; > + else > + shift = (i + 1) >> 1; > + > + if (cs_size != -1) > + cs_size = rank_multiply * (128 << shift); > + > + return cs_size; > +} > + > +static int ddr4_cs_size(unsigned i) > +{ > + int cs_size = 0; > + > + if (i == 0) > + cs_size = -1; > + else if (i == 1) > + cs_size = 1024; > + else > + /* Min cs_size = 1G */ > + cs_size = 1024 * (1 << (i >> 1)); > + > + return cs_size; > +} > + > static int f10_dbam_to_chip_select(struct amd64_pvt *pvt, u8 dct, > - unsigned cs_mode) > + unsigned cs_mode, int cs_mask_nr) > { > u32 dclr = dct ? pvt->dclr1 : pvt->dclr0; > > @@ -1184,18 +1260,56 @@ static int f10_dbam_to_chip_select(struct amd64_pvt *pvt, u8 dct, > * F15h supports only 64bit DCT interfaces > */ > static int f15_dbam_to_chip_select(struct amd64_pvt *pvt, u8 dct, > - unsigned cs_mode) > + unsigned cs_mode, int cs_mask_nr) > { > WARN_ON(cs_mode > 12); > > return ddr3_cs_size(cs_mode, false); > } > > +/* F15h M60h supports DDR4 mapping as well.. */ > +static int f15_m60h_dbam_to_chip_select(struct amd64_pvt *pvt, u8 dct, > + unsigned cs_mode, int cs_mask_nr) > +{ > + int cs_size; > + enum mem_type type; > + u32 dram_ctrl; > + u32 dcsm = pvt->csels[dct].csmasks[cs_mask_nr]; > + > + WARN_ON(cs_mode > 12); > + > + amd64_read_dct_pci_cfg(pvt, dct, DRAM_CONTROL, &dram_ctrl); > + type = (((dram_ctrl >> 8) & 0x7) == 0x2) ? MEM_DDR4 : > + (pvt->dclr0 & BIT(16)) ? MEM_DDR3 : > + (dcsm & 0x3) ? MEM_LRDDR3 : MEM_RDDR3; This is the second time we're determining memory type, maybe we should cache that too into pvt->dram_type...? > + > + if (type == MEM_DDR4) { > + if (cs_mode > 9) > + return -1; > + > + cs_size = ddr4_cs_size(cs_mode); > + } else if (type == MEM_LRDDR3) { > + unsigned rank_multiply = dcsm & 0xf; > + > + if (rank_multiply == 3) > + rank_multiply = 4; > + cs_size = ddr3_lrdimm_cs_size(cs_mode, rank_multiply); > + } else { > + /* Minimum cs size is 512mb for F15hM60h*/ > + if (cs_mode == 0x1) > + return -1; > + > + cs_size = ddr3_cs_size(cs_mode, false); > + } > + > + return cs_size; > +}
On 10/1/2014 6:32 AM, Borislav Petkov wrote: > On Thu, Sep 18, 2014 at 02:57:10PM -0500, Aravind Gopalakrishnan wrote: >> This patch adds support for ECC error decoding for F15h M60h processor. >> Aside from the usual changes, the patch adds support for some new features >> in the processor: >> - DDR4(unbuffered, registered); LRDIMM DDR3 support >> - relevant debug messages have been modified/added to report these >> memory types >> - new dbam_to_cs mappers >> - if (F15h M60h && LRDIMM); we need a 'multiplier' value to find >> cs_size. This multiplier value is obtained from the per-dimm >> DCSM register. So, change the interface to accept a 'cs_mask_nr' >> value to facilitate this calculation >> Misc cleanup: >> - amd64_pci_table[] is condensed by using PCI_VDEVICE macro. >> >> Testing details: >> Tested the patch by injecting 'ECC' type errors using mce_amd_inj >> and error decoding works fine. >> >> Signed-off-by: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com> >> --- >> drivers/edac/amd64_edac.c | 226 ++++++++++++++++++++++++++++++++-------------- >> drivers/edac/amd64_edac.h | 12 ++- >> 2 files changed, 169 insertions(+), 69 deletions(-) >> >> diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c >> index bbd6514..3e265e0 100644 >> --- a/drivers/edac/amd64_edac.c >> +++ b/drivers/edac/amd64_edac.c >> @@ -690,11 +690,32 @@ static void debug_display_dimm_sizes(struct amd64_pvt *, u8); >> >> static void debug_dump_dramcfg_low(struct amd64_pvt *pvt, u32 dclr, int chan) >> { >> + int cs; >> + >> edac_dbg(1, "F2x%d90 (DRAM Cfg Low): 0x%08x\n", chan, dclr); >> >> - edac_dbg(1, " DIMM type: %sbuffered; all DIMMs support ECC: %s\n", >> - (dclr & BIT(16)) ? "un" : "", >> - (dclr & BIT(19)) ? "yes" : "no"); >> + if (pvt->fam == 0x15 && pvt->model == 0x60) { >> + for_each_chip_select_mask(cs, chan, pvt) { >> + u32 dcsm = pvt->csels[chan].csmasks[cs]; >> + >> + if (dcsm & 0x3) { >> + /* LRDIMMs */ >> + edac_dbg(1, " DIMM type: LRDIMM %dx rank multiply;" >> + "CS = %d; all DIMMs support ECC: %s\n", >> + (dcsm & 0x3), cs, >> + (dclr & BIT(19)) ? "yes" : "no"); > Why do we need to iterate over the DRAM CS sets? Just for the rank > multiplier, apparently. We dump those normally in read_dct_base_mask(), > though. It's not just for rank multiplier.. we find that it's LRDIMM only by examining dcsm. Hence the iteration here.. Not sure if we should move it to read_dct_base_mask() as traditionally we report the DIMM type in this function. >> + } else { >> + edac_dbg(1, " DIMM type: %sbuffered; CS = %d;" >> + "all DIMMs support ECC: %s\n", >> + (dclr & BIT(16)) ? "un" : "", cs, >> + (dclr & BIT(19)) ? "yes" : "no"); >> + } >> + } >> + } else { >> + edac_dbg(1, " DIMM type: %sbuffered; all DIMMs support ECC:" >> + "%s\n", (dclr & BIT(16)) ? "un" : "", >> + (dclr & BIT(19)) ? "yes" : "no"); >> + } > Single if-else statements don't need {} braces. Ok, will fix this. >> >> edac_dbg(1, " PAR/ERR parity: %s\n", >> (dclr & BIT(8)) ? "enabled" : "disabled"); >> @@ -756,7 +777,7 @@ static void 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) { >> + } 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 { >> @@ -817,10 +838,26 @@ static enum mem_type determine_memory_type(struct amd64_pvt *pvt, int cs) >> { >> enum mem_type type; >> >> - /* F15h supports only DDR3 */ >> - if (pvt->fam >= 0x15) >> - type = (pvt->dclr0 & BIT(16)) ? MEM_DDR3 : MEM_RDDR3; >> - else if (pvt->fam == 0x10 || pvt->ext_model >= K8_REV_F) { >> + /* F15h, M60h supports DDR4 too*/ >> + if (pvt->fam >= 0x15) { >> + if (pvt->model == 0x60) { >> + /* >> + * Since in init_csrow we iterate over just DCT0 >> + * use '0' for dct values here when necessary. >> + */ >> + u32 dram_ctrl; >> + u32 dcsm = pvt->csels[0].csmasks[cs]; >> + >> + amd64_read_dct_pci_cfg(pvt, 0, DRAM_CONTROL, >> + &dram_ctrl); > We're reading DRAM_CONTROL at two locations, maybe we should cache it in > pvt->dram_ctl ? Makes sense. Will do this. >> + type = (((dram_ctrl >> 8) & 0x7) == 0x2) ? MEM_DDR4 : >> + (pvt->dclr0 & BIT(16)) ? MEM_DDR3 : >> + (dcsm & 0x3) ? MEM_LRDDR3 : MEM_RDDR3; > Doesn't D18F2x78_dct[3:0][DramType] define all possible types or do you > have to fallback to DCLR for DDR3 types? No, we need to fall back to DCLR to realize if it's Unbuffered DIMM or not. >> + } else { >> + type = (pvt->dclr0 & BIT(16)) ? MEM_DDR3 : MEM_RDDR3; >> + } > Single if-else statements don't need {} braces. Please read > Documentation/CodingStyle. Yeah. Sorry about that. Will fix. >> + >> + } else if (pvt->fam == 0x10 || pvt->ext_model >= K8_REV_F) { >> if (pvt->dchr0 & DDR3_MODE) >> type = (pvt->dclr0 & BIT(16)) ? MEM_DDR3 : MEM_RDDR3; >> else >> @@ -958,8 +995,12 @@ static void read_dram_base_limit_regs(struct amd64_pvt *pvt, unsigned range) >> if (WARN_ON(!nb)) >> return; >> >> - pci_func = (pvt->model == 0x30) ? PCI_DEVICE_ID_AMD_15H_M30H_NB_F1 >> - : PCI_DEVICE_ID_AMD_15H_NB_F1; >> + if (pvt->model == 0x60) >> + pci_func = PCI_DEVICE_ID_AMD_15H_M60H_NB_F1; >> + else if (pvt->model == 0x30) >> + pci_func = PCI_DEVICE_ID_AMD_15H_M30H_NB_F1; >> + else >> + pci_func = PCI_DEVICE_ID_AMD_15H_NB_F1; >> >> f1 = pci_get_related_function(nb->misc->vendor, pci_func, nb->misc); >> if (WARN_ON(!f1)) >> @@ -1049,7 +1090,7 @@ static int ddr2_cs_size(unsigned i, bool dct_width) >> } >> >> static int k8_dbam_to_chip_select(struct amd64_pvt *pvt, u8 dct, >> - unsigned cs_mode) >> + unsigned cs_mode, int cs_mask_nr) >> { >> u32 dclr = dct ? pvt->dclr1 : pvt->dclr0; >> >> @@ -1167,8 +1208,43 @@ static int ddr3_cs_size(unsigned i, bool dct_width) >> return cs_size; >> } >> >> +static int ddr3_lrdimm_cs_size(unsigned i, unsigned rank_multiply) >> +{ >> + unsigned shift = 0; >> + int cs_size = 0; >> + >> + if (i < 4 || i == 6) >> + cs_size = -1; >> + else if (i == 12) >> + shift = 7; >> + else if (!(i & 0x1)) >> + shift = i >> 1; >> + else >> + shift = (i + 1) >> 1; >> + >> + if (cs_size != -1) >> + cs_size = rank_multiply * (128 << shift); >> + >> + return cs_size; >> +} >> + >> +static int ddr4_cs_size(unsigned i) >> +{ >> + int cs_size = 0; >> + >> + if (i == 0) >> + cs_size = -1; >> + else if (i == 1) >> + cs_size = 1024; >> + else >> + /* Min cs_size = 1G */ >> + cs_size = 1024 * (1 << (i >> 1)); >> + >> + return cs_size; >> +} >> + >> static int f10_dbam_to_chip_select(struct amd64_pvt *pvt, u8 dct, >> - unsigned cs_mode) >> + unsigned cs_mode, int cs_mask_nr) >> { >> u32 dclr = dct ? pvt->dclr1 : pvt->dclr0; >> >> @@ -1184,18 +1260,56 @@ static int f10_dbam_to_chip_select(struct amd64_pvt *pvt, u8 dct, >> * F15h supports only 64bit DCT interfaces >> */ >> static int f15_dbam_to_chip_select(struct amd64_pvt *pvt, u8 dct, >> - unsigned cs_mode) >> + unsigned cs_mode, int cs_mask_nr) >> { >> WARN_ON(cs_mode > 12); >> >> return ddr3_cs_size(cs_mode, false); >> } >> >> +/* F15h M60h supports DDR4 mapping as well.. */ >> +static int f15_m60h_dbam_to_chip_select(struct amd64_pvt *pvt, u8 dct, >> + unsigned cs_mode, int cs_mask_nr) >> +{ >> + int cs_size; >> + enum mem_type type; >> + u32 dram_ctrl; >> + u32 dcsm = pvt->csels[dct].csmasks[cs_mask_nr]; >> + >> + WARN_ON(cs_mode > 12); >> + >> + amd64_read_dct_pci_cfg(pvt, dct, DRAM_CONTROL, &dram_ctrl); >> + type = (((dram_ctrl >> 8) & 0x7) == 0x2) ? MEM_DDR4 : >> + (pvt->dclr0 & BIT(16)) ? MEM_DDR3 : >> + (dcsm & 0x3) ? MEM_LRDDR3 : MEM_RDDR3; > This is the second time we're determining memory type, maybe we should > cache that too into pvt->dram_type...? Yes, Will do. >> + >> + if (type == MEM_DDR4) { >> + if (cs_mode > 9) >> + return -1; >> + >> + cs_size = ddr4_cs_size(cs_mode); >> + } else if (type == MEM_LRDDR3) { >> + unsigned rank_multiply = dcsm & 0xf; >> + >> + if (rank_multiply == 3) >> + rank_multiply = 4; >> + cs_size = ddr3_lrdimm_cs_size(cs_mode, rank_multiply); >> + } else { >> + /* Minimum cs size is 512mb for F15hM60h*/ >> + if (cs_mode == 0x1) >> + return -1; >> + >> + cs_size = ddr3_cs_size(cs_mode, false); >> + } >> + >> + return cs_size; >> +} Shall resend as V2. Thanks, -Aravind. -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Oct 01, 2014 at 10:32:58AM -0500, Aravind Gopalakrishnan wrote: > >>+ if (dcsm & 0x3) { > >>+ /* LRDIMMs */ > >>+ edac_dbg(1, " DIMM type: LRDIMM %dx rank multiply;" > >>+ "CS = %d; all DIMMs support ECC: %s\n", > >>+ (dcsm & 0x3), cs, > >>+ (dclr & BIT(19)) ? "yes" : "no"); > >Why do we need to iterate over the DRAM CS sets? Just for the rank > >multiplier, apparently. We dump those normally in read_dct_base_mask(), > >though. > > It's not just for rank multiplier.. we find that it's LRDIMM only by > examining dcsm. Hence the iteration here.. So we can look only at the first DCSM, no? Or are there systems with different types of LRDIMMs on one DCT?
On 10/1/2014 10:45 AM, Borislav Petkov wrote: > On Wed, Oct 01, 2014 at 10:32:58AM -0500, Aravind Gopalakrishnan wrote: >>>> + if (dcsm & 0x3) { >>>> + /* LRDIMMs */ >>>> + edac_dbg(1, " DIMM type: LRDIMM %dx rank multiply;" >>>> + "CS = %d; all DIMMs support ECC: %s\n", >>>> + (dcsm & 0x3), cs, >>>> + (dclr & BIT(19)) ? "yes" : "no"); >>> Why do we need to iterate over the DRAM CS sets? Just for the rank >>> multiplier, apparently. We dump those normally in read_dct_base_mask(), >>> though. >> It's not just for rank multiplier.. we find that it's LRDIMM only by >> examining dcsm. Hence the iteration here.. > So we can look only at the first DCSM, no? Or are there systems with > different types of LRDIMMs on one DCT? > Not AFAIK, But I'll ask to make sure. If different LRDIMMs on one DCT don't exist, then sure, I'll modify the code to use first DCSM. Regarding resend- I'm not sure how you'll want the patches.. I just noticed a 'edac-for-3.19' branch. Would you prefer I based changes on top of this for merging purposes? Or just resend the original 4 patches based off tip as usual? Thanks, -Aravind. -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Oct 01, 2014 at 11:02:16AM -0500, Aravind Gopalakrishnan wrote: > Not AFAIK, But I'll ask to make sure. Yes please. > If different LRDIMMs on one DCT don't exist, then sure, I'll modify the code > to use first DCSM. Right. > Regarding resend- > I'm not sure how you'll want the patches.. I just noticed a 'edac-for-3.19' > branch. I uploaded it for the 0day bot so that the patches can be build-tested a little with different configs. Don't pay attention to any other branches except to "for-next" which is what will go to Linus come next merge window. The other branches might get rebased and you shouldn't base any work ontop. > Would you prefer I based changes on top of this for merging purposes? > Or just resend the original 4 patches based off tip as usual? Nah, just resend the last one - 4/4 - the other three are fine. Thanks.
On 10/1/2014 6:32 AM, Borislav Petkov wrote: > On Thu, Sep 18, 2014 at 02:57:10PM -0500, Aravind Gopalakrishnan wrote: >> This patch adds support for ECC error decoding for F15h M60h processor. >> Aside from the usual changes, the patch adds support for some new features >> in the processor: >> - DDR4(unbuffered, registered); LRDIMM DDR3 support >> - relevant debug messages have been modified/added to report these >> memory types >> - new dbam_to_cs mappers >> - if (F15h M60h && LRDIMM); we need a 'multiplier' value to find >> cs_size. This multiplier value is obtained from the per-dimm >> DCSM register. So, change the interface to accept a 'cs_mask_nr' >> value to facilitate this calculation >> Misc cleanup: >> - amd64_pci_table[] is condensed by using PCI_VDEVICE macro. >> >> Testing details: >> Tested the patch by injecting 'ECC' type errors using mce_amd_inj >> and error decoding works fine. >> >> Signed-off-by: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com> >> --- >> drivers/edac/amd64_edac.c | 226 ++++++++++++++++++++++++++++++++-------------- >> drivers/edac/amd64_edac.h | 12 ++- >> 2 files changed, 169 insertions(+), 69 deletions(-) >> >> @@ -817,10 +838,26 @@ static enum mem_type determine_memory_type(struct amd64_pvt *pvt, int cs) >> { >> enum mem_type type; >> >> - /* F15h supports only DDR3 */ >> - if (pvt->fam >= 0x15) >> - type = (pvt->dclr0 & BIT(16)) ? MEM_DDR3 : MEM_RDDR3; >> - else if (pvt->fam == 0x10 || pvt->ext_model >= K8_REV_F) { >> + /* F15h, M60h supports DDR4 too*/ >> + if (pvt->fam >= 0x15) { >> + if (pvt->model == 0x60) { >> + /* >> + * Since in init_csrow we iterate over just DCT0 >> + * use '0' for dct values here when necessary. >> + */ >> + u32 dram_ctrl; >> + u32 dcsm = pvt->csels[0].csmasks[cs]; >> + >> + amd64_read_dct_pci_cfg(pvt, 0, DRAM_CONTROL, >> + &dram_ctrl); > We're reading DRAM_CONTROL at two locations, maybe we should cache it in > pvt->dram_ctl ? > >> + type = (((dram_ctrl >> 8) & 0x7) == 0x2) ? MEM_DDR4 : >> + (pvt->dclr0 & BIT(16)) ? MEM_DDR3 : >> + (dcsm & 0x3) ? MEM_LRDDR3 : MEM_RDDR3; > > > @@ -1184,18 +1260,56 @@ static int f10_dbam_to_chip_select(struct amd64_pvt *pvt, u8 dct, > * F15h supports only 64bit DCT interfaces > */ > static int f15_dbam_to_chip_select(struct amd64_pvt *pvt, u8 dct, > - unsigned cs_mode) > + unsigned cs_mode, int cs_mask_nr) > { > WARN_ON(cs_mode > 12); > > return ddr3_cs_size(cs_mode, false); > } > > +/* F15h M60h supports DDR4 mapping as well.. */ > +static int f15_m60h_dbam_to_chip_select(struct amd64_pvt *pvt, u8 dct, > + unsigned cs_mode, int cs_mask_nr) > +{ > + int cs_size; > + enum mem_type type; > + u32 dram_ctrl; > + u32 dcsm = pvt->csels[dct].csmasks[cs_mask_nr]; > + > + WARN_ON(cs_mode > 12); > + > + amd64_read_dct_pci_cfg(pvt, dct, DRAM_CONTROL, &dram_ctrl); > + type = (((dram_ctrl >> 8) & 0x7) == 0x2) ? MEM_DDR4 : > + (pvt->dclr0 & BIT(16)) ? MEM_DDR3 : > + (dcsm & 0x3) ? MEM_LRDDR3 : MEM_RDDR3; > This is the second time we're determining memory type, maybe we should > cache that too into pvt->dram_type...? > > The more I think about this, I'm finding it's hard to do this cleanly. I initially thought I'd just cache this in pvt->dram_type the first time I'm doing this. But, the pvt->ops->dbam_to_cs() mappers get called first before determine_memory_type(). So, If I look for dram_type in f15_m60h_dbam_to_chip_select() it's ugly as that's really the point of having a determine_memory_type(). Also, there's just a lot of if-else statements in determine_memory_type() now. This could benefit from having a per-family low_ops function. And, we can call this early... somewhere in read_mc_regs() so that we have information ready to use in f15_m60h_dbam_to_chip_select() and in init_csrows() which needs dram_type too. Oh, btw- We can do away with a pvt->dram_ctrl as f15_m60h_dbam_to_chip_select() really just needs the dram_type. Thoughts? -Aravind. -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Oct 01, 2014 at 02:44:21PM -0500, Aravind Gopalakrishnan wrote: > The more I think about this, I'm finding it's hard to do this cleanly. > I initially thought I'd just cache this in pvt->dram_type the first time I'm > doing this. > But, the pvt->ops->dbam_to_cs() mappers get called first before > determine_memory_type(). > > So, If I look for dram_type in f15_m60h_dbam_to_chip_select() it's ugly as > that's really the point of > having a determine_memory_type(). > > Also, there's just a lot of if-else statements in determine_memory_type() > now. > This could benefit from having a per-family low_ops function. > And, we can call this early... somewhere in read_mc_regs() so that we have > information ready to use in > f15_m60h_dbam_to_chip_select() and in init_csrows() which needs dram_type > too. Right, this is what I was thinking too: somewhere in read_mc_regs(), after having collected ->dclr0, you call determine_memory_type() and store it into pvt->dram_type. > Oh, btw- We can do away with a pvt->dram_ctrl as > f15_m60h_dbam_to_chip_select() really just needs the dram_type. Yes, you make the read of DRAM_CONTROL inside determine_memory_type() as we don't need it anywhere else. If we do, all of a sudden, we'll move it up to read_mc_regs(). IOW, I'm trying to centralize all reg reads in read_mc_regs() and use locally cached info later so I don't have to access the hardware each time needlessly, if it can be helped. Thanks.
On 10/2/2014 9:52 AM, Borislav Petkov wrote: > On Wed, Oct 01, 2014 at 02:44:21PM -0500, Aravind Gopalakrishnan wrote: >> The more I think about this, I'm finding it's hard to do this cleanly. >> I initially thought I'd just cache this in pvt->dram_type the first time I'm >> doing this. >> But, the pvt->ops->dbam_to_cs() mappers get called first before >> determine_memory_type(). >> >> So, If I look for dram_type in f15_m60h_dbam_to_chip_select() it's ugly as >> that's really the point of >> having a determine_memory_type(). >> >> Also, there's just a lot of if-else statements in determine_memory_type() >> now. >> This could benefit from having a per-family low_ops function. >> And, we can call this early... somewhere in read_mc_regs() so that we have >> information ready to use in >> f15_m60h_dbam_to_chip_select() and in init_csrows() which needs dram_type >> too. > Right, this is what I was thinking too: somewhere in read_mc_regs(), > after having collected ->dclr0, you call determine_memory_type() and > store it into pvt->dram_type. Yep. So, let me go ahead and make these changes. Shall send out a V2 once I have an answer on the LRDIMM question.. Thanks, -Aravind. >> Oh, btw- We can do away with a pvt->dram_ctrl as >> f15_m60h_dbam_to_chip_select() really just needs the dram_type. > Yes, you make the read of DRAM_CONTROL inside determine_memory_type() as > we don't need it anywhere else. > > If we do, all of a sudden, we'll move it up to read_mc_regs(). IOW, I'm > trying to centralize all reg reads in read_mc_regs() and use locally > cached info later so I don't have to access the hardware each time > needlessly, if it can be helped. > > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/1/2014 10:45 AM, Borislav Petkov wrote: > On Wed, Oct 01, 2014 at 10:32:58AM -0500, Aravind Gopalakrishnan wrote: >>>> + if (dcsm & 0x3) { >>>> + /* LRDIMMs */ >>>> + edac_dbg(1, " DIMM type: LRDIMM %dx rank multiply;" >>>> + "CS = %d; all DIMMs support ECC: %s\n", >>>> + (dcsm & 0x3), cs, >>>> + (dclr & BIT(19)) ? "yes" : "no"); >>> Why do we need to iterate over the DRAM CS sets? Just for the rank >>> multiplier, apparently. We dump those normally in read_dct_base_mask(), >>> though. >> It's not just for rank multiplier.. we find that it's LRDIMM only by >> examining dcsm. Hence the iteration here.. > So we can look only at the first DCSM, no? Or are there systems with > different types of LRDIMMs on one DCT? > So it seems we can theoretically have systems in this manner, but that is not a common case.. So, shall I just use first DCSM to keep it simple for now? And find rank multiplier iteratively as and when need arises? Thanks, -Aravind. -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Oct 03, 2014 at 09:39:58AM -0500, Aravind Gopalakrishnan wrote: > So it seems we can theoretically have systems in this manner, but that is > not a common case.. > > So, shall I just use first DCSM to keep it simple for now? > And find rank multiplier iteratively as and when need arises? Yep, sounds nicely conservative. Thanks.
diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c index bbd6514..3e265e0 100644 --- a/drivers/edac/amd64_edac.c +++ b/drivers/edac/amd64_edac.c @@ -690,11 +690,32 @@ static void debug_display_dimm_sizes(struct amd64_pvt *, u8); static void debug_dump_dramcfg_low(struct amd64_pvt *pvt, u32 dclr, int chan) { + int cs; + edac_dbg(1, "F2x%d90 (DRAM Cfg Low): 0x%08x\n", chan, dclr); - edac_dbg(1, " DIMM type: %sbuffered; all DIMMs support ECC: %s\n", - (dclr & BIT(16)) ? "un" : "", - (dclr & BIT(19)) ? "yes" : "no"); + if (pvt->fam == 0x15 && pvt->model == 0x60) { + for_each_chip_select_mask(cs, chan, pvt) { + u32 dcsm = pvt->csels[chan].csmasks[cs]; + + if (dcsm & 0x3) { + /* LRDIMMs */ + edac_dbg(1, " DIMM type: LRDIMM %dx rank multiply;" + "CS = %d; all DIMMs support ECC: %s\n", + (dcsm & 0x3), cs, + (dclr & BIT(19)) ? "yes" : "no"); + } else { + edac_dbg(1, " DIMM type: %sbuffered; CS = %d;" + "all DIMMs support ECC: %s\n", + (dclr & BIT(16)) ? "un" : "", cs, + (dclr & BIT(19)) ? "yes" : "no"); + } + } + } else { + edac_dbg(1, " DIMM type: %sbuffered; all DIMMs support ECC:" + "%s\n", (dclr & BIT(16)) ? "un" : "", + (dclr & BIT(19)) ? "yes" : "no"); + } edac_dbg(1, " PAR/ERR parity: %s\n", (dclr & BIT(8)) ? "enabled" : "disabled"); @@ -756,7 +777,7 @@ static void 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) { + } 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 { @@ -817,10 +838,26 @@ static enum mem_type determine_memory_type(struct amd64_pvt *pvt, int cs) { enum mem_type type; - /* F15h supports only DDR3 */ - if (pvt->fam >= 0x15) - type = (pvt->dclr0 & BIT(16)) ? MEM_DDR3 : MEM_RDDR3; - else if (pvt->fam == 0x10 || pvt->ext_model >= K8_REV_F) { + /* F15h, M60h supports DDR4 too*/ + if (pvt->fam >= 0x15) { + if (pvt->model == 0x60) { + /* + * Since in init_csrow we iterate over just DCT0 + * use '0' for dct values here when necessary. + */ + u32 dram_ctrl; + u32 dcsm = pvt->csels[0].csmasks[cs]; + + amd64_read_dct_pci_cfg(pvt, 0, DRAM_CONTROL, + &dram_ctrl); + type = (((dram_ctrl >> 8) & 0x7) == 0x2) ? MEM_DDR4 : + (pvt->dclr0 & BIT(16)) ? MEM_DDR3 : + (dcsm & 0x3) ? MEM_LRDDR3 : MEM_RDDR3; + } else { + type = (pvt->dclr0 & BIT(16)) ? MEM_DDR3 : MEM_RDDR3; + } + + } else if (pvt->fam == 0x10 || pvt->ext_model >= K8_REV_F) { if (pvt->dchr0 & DDR3_MODE) type = (pvt->dclr0 & BIT(16)) ? MEM_DDR3 : MEM_RDDR3; else @@ -958,8 +995,12 @@ static void read_dram_base_limit_regs(struct amd64_pvt *pvt, unsigned range) if (WARN_ON(!nb)) return; - pci_func = (pvt->model == 0x30) ? PCI_DEVICE_ID_AMD_15H_M30H_NB_F1 - : PCI_DEVICE_ID_AMD_15H_NB_F1; + if (pvt->model == 0x60) + pci_func = PCI_DEVICE_ID_AMD_15H_M60H_NB_F1; + else if (pvt->model == 0x30) + pci_func = PCI_DEVICE_ID_AMD_15H_M30H_NB_F1; + else + pci_func = PCI_DEVICE_ID_AMD_15H_NB_F1; f1 = pci_get_related_function(nb->misc->vendor, pci_func, nb->misc); if (WARN_ON(!f1)) @@ -1049,7 +1090,7 @@ static int ddr2_cs_size(unsigned i, bool dct_width) } static int k8_dbam_to_chip_select(struct amd64_pvt *pvt, u8 dct, - unsigned cs_mode) + unsigned cs_mode, int cs_mask_nr) { u32 dclr = dct ? pvt->dclr1 : pvt->dclr0; @@ -1167,8 +1208,43 @@ static int ddr3_cs_size(unsigned i, bool dct_width) return cs_size; } +static int ddr3_lrdimm_cs_size(unsigned i, unsigned rank_multiply) +{ + unsigned shift = 0; + int cs_size = 0; + + if (i < 4 || i == 6) + cs_size = -1; + else if (i == 12) + shift = 7; + else if (!(i & 0x1)) + shift = i >> 1; + else + shift = (i + 1) >> 1; + + if (cs_size != -1) + cs_size = rank_multiply * (128 << shift); + + return cs_size; +} + +static int ddr4_cs_size(unsigned i) +{ + int cs_size = 0; + + if (i == 0) + cs_size = -1; + else if (i == 1) + cs_size = 1024; + else + /* Min cs_size = 1G */ + cs_size = 1024 * (1 << (i >> 1)); + + return cs_size; +} + static int f10_dbam_to_chip_select(struct amd64_pvt *pvt, u8 dct, - unsigned cs_mode) + unsigned cs_mode, int cs_mask_nr) { u32 dclr = dct ? pvt->dclr1 : pvt->dclr0; @@ -1184,18 +1260,56 @@ static int f10_dbam_to_chip_select(struct amd64_pvt *pvt, u8 dct, * F15h supports only 64bit DCT interfaces */ static int f15_dbam_to_chip_select(struct amd64_pvt *pvt, u8 dct, - unsigned cs_mode) + unsigned cs_mode, int cs_mask_nr) { WARN_ON(cs_mode > 12); return ddr3_cs_size(cs_mode, false); } +/* F15h M60h supports DDR4 mapping as well.. */ +static int f15_m60h_dbam_to_chip_select(struct amd64_pvt *pvt, u8 dct, + unsigned cs_mode, int cs_mask_nr) +{ + int cs_size; + enum mem_type type; + u32 dram_ctrl; + u32 dcsm = pvt->csels[dct].csmasks[cs_mask_nr]; + + WARN_ON(cs_mode > 12); + + amd64_read_dct_pci_cfg(pvt, dct, DRAM_CONTROL, &dram_ctrl); + type = (((dram_ctrl >> 8) & 0x7) == 0x2) ? MEM_DDR4 : + (pvt->dclr0 & BIT(16)) ? MEM_DDR3 : + (dcsm & 0x3) ? MEM_LRDDR3 : MEM_RDDR3; + + if (type == MEM_DDR4) { + if (cs_mode > 9) + return -1; + + cs_size = ddr4_cs_size(cs_mode); + } else if (type == MEM_LRDDR3) { + unsigned rank_multiply = dcsm & 0xf; + + if (rank_multiply == 3) + rank_multiply = 4; + cs_size = ddr3_lrdimm_cs_size(cs_mode, rank_multiply); + } else { + /* Minimum cs size is 512mb for F15hM60h*/ + if (cs_mode == 0x1) + return -1; + + cs_size = ddr3_cs_size(cs_mode, false); + } + + return cs_size; +} + /* * F16h and F15h model 30h have only limited cs_modes. */ static int f16_dbam_to_chip_select(struct amd64_pvt *pvt, u8 dct, - unsigned cs_mode) + unsigned cs_mode, int cs_mask_nr) { WARN_ON(cs_mode > 12); @@ -1757,13 +1871,20 @@ static void debug_display_dimm_sizes(struct amd64_pvt *pvt, u8 ctrl) size0 = 0; if (dcsb[dimm*2] & DCSB_CS_ENABLE) + /* For f15m60h, need multiplier for LRDIMM cs_size + * calculation. We pass 'dimm' value to the dbam_to_cs + * mapper so we can find the multiplier from the + * corresponding DCSM. + */ size0 = pvt->ops->dbam_to_cs(pvt, ctrl, - DBAM_DIMM(dimm, dbam)); + DBAM_DIMM(dimm, dbam), + dimm); size1 = 0; if (dcsb[dimm*2 + 1] & DCSB_CS_ENABLE) size1 = pvt->ops->dbam_to_cs(pvt, ctrl, - DBAM_DIMM(dimm, dbam)); + DBAM_DIMM(dimm, dbam), + dimm); amd64_info(EDAC_MC ": %d: %5dMB %d: %5dMB\n", dimm * 2, size0, @@ -1812,6 +1933,16 @@ static struct amd64_family_type family_types[] = { .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, + .f3_id = PCI_DEVICE_ID_AMD_15H_M60H_NB_F3, + .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, @@ -2238,7 +2369,8 @@ static u32 get_csrow_nr_pages(struct amd64_pvt *pvt, u8 dct, int csrow_nr) */ cs_mode = DBAM_DIMM(csrow_nr / 2, dbam); - nr_pages = pvt->ops->dbam_to_cs(pvt, dct, cs_mode) << (20 - PAGE_SHIFT); + nr_pages = pvt->ops->dbam_to_cs(pvt, dct, cs_mode, (csrow_nr / 2)) + << (20 - PAGE_SHIFT); edac_dbg(0, "csrow: %d, channel: %d, DBAM idx: %d\n", csrow_nr, dct, cs_mode); @@ -2604,6 +2736,10 @@ static struct amd64_family_type *per_family_init(struct amd64_pvt *pvt) fam_type = &family_types[F15_M30H_CPUS]; pvt->ops = &family_types[F15_M30H_CPUS].ops; break; + } else if (pvt->model == 0x60) { + fam_type = &family_types[F15_M60H_CPUS]; + pvt->ops = &family_types[F15_M60H_CPUS].ops; + break; } fam_type = &family_types[F15_CPUS]; @@ -2828,55 +2964,13 @@ static void remove_one_instance(struct pci_dev *pdev) * inquiry this table to see if this driver is for a given device found. */ static const struct pci_device_id amd64_pci_table[] = { - { - .vendor = PCI_VENDOR_ID_AMD, - .device = PCI_DEVICE_ID_AMD_K8_NB_MEMCTL, - .subvendor = PCI_ANY_ID, - .subdevice = PCI_ANY_ID, - .class = 0, - .class_mask = 0, - }, - { - .vendor = PCI_VENDOR_ID_AMD, - .device = PCI_DEVICE_ID_AMD_10H_NB_DRAM, - .subvendor = PCI_ANY_ID, - .subdevice = PCI_ANY_ID, - .class = 0, - .class_mask = 0, - }, - { - .vendor = PCI_VENDOR_ID_AMD, - .device = PCI_DEVICE_ID_AMD_15H_NB_F2, - .subvendor = PCI_ANY_ID, - .subdevice = PCI_ANY_ID, - .class = 0, - .class_mask = 0, - }, - { - .vendor = PCI_VENDOR_ID_AMD, - .device = PCI_DEVICE_ID_AMD_15H_M30H_NB_F2, - .subvendor = PCI_ANY_ID, - .subdevice = PCI_ANY_ID, - .class = 0, - .class_mask = 0, - }, - { - .vendor = PCI_VENDOR_ID_AMD, - .device = PCI_DEVICE_ID_AMD_16H_NB_F2, - .subvendor = PCI_ANY_ID, - .subdevice = PCI_ANY_ID, - .class = 0, - .class_mask = 0, - }, - { - .vendor = PCI_VENDOR_ID_AMD, - .device = PCI_DEVICE_ID_AMD_16H_M30H_NB_F2, - .subvendor = PCI_ANY_ID, - .subdevice = PCI_ANY_ID, - .class = 0, - .class_mask = 0, - }, - + { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_K8_NB_MEMCTL) }, + { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_10H_NB_DRAM) }, + { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_15H_NB_F2) }, + { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_15H_M30H_NB_F2) }, + { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_15H_M60H_NB_F2) }, + { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_16H_NB_F2) }, + { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_16H_M30H_NB_F2) }, {0, } }; MODULE_DEVICE_TABLE(pci, amd64_pci_table); diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h index 55fb594..dbbae84 100644 --- a/drivers/edac/amd64_edac.h +++ b/drivers/edac/amd64_edac.h @@ -162,10 +162,12 @@ /* * PCI-defined configuration space registers */ -#define PCI_DEVICE_ID_AMD_15H_M30H_NB_F1 0x141b -#define PCI_DEVICE_ID_AMD_15H_M30H_NB_F2 0x141c #define PCI_DEVICE_ID_AMD_15H_NB_F1 0x1601 #define PCI_DEVICE_ID_AMD_15H_NB_F2 0x1602 +#define PCI_DEVICE_ID_AMD_15H_M30H_NB_F1 0x141b +#define PCI_DEVICE_ID_AMD_15H_M30H_NB_F2 0x141c +#define PCI_DEVICE_ID_AMD_15H_M60H_NB_F1 0x1571 +#define PCI_DEVICE_ID_AMD_15H_M60H_NB_F2 0x1572 #define PCI_DEVICE_ID_AMD_16H_NB_F1 0x1531 #define PCI_DEVICE_ID_AMD_16H_NB_F2 0x1532 #define PCI_DEVICE_ID_AMD_16H_M30H_NB_F1 0x1581 @@ -221,6 +223,8 @@ #define csrow_enabled(i, dct, pvt) ((pvt)->csels[(dct)].csbases[(i)] & DCSB_CS_ENABLE) +#define DRAM_CONTROL 0x78 + #define DBAM0 0x80 #define DBAM1 0x180 @@ -301,6 +305,7 @@ enum amd_families { F10_CPUS, F15_CPUS, F15_M30H_CPUS, + F15_M60H_CPUS, F16_CPUS, F16_M30H_CPUS, NUM_FAMILIES, @@ -480,7 +485,8 @@ 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 (*dbam_to_cs) (struct amd64_pvt *pvt, u8 dct, + unsigned cs_mode, int cs_mask_nr); }; struct amd64_family_type {
This patch adds support for ECC error decoding for F15h M60h processor. Aside from the usual changes, the patch adds support for some new features in the processor: - DDR4(unbuffered, registered); LRDIMM DDR3 support - relevant debug messages have been modified/added to report these memory types - new dbam_to_cs mappers - if (F15h M60h && LRDIMM); we need a 'multiplier' value to find cs_size. This multiplier value is obtained from the per-dimm DCSM register. So, change the interface to accept a 'cs_mask_nr' value to facilitate this calculation Misc cleanup: - amd64_pci_table[] is condensed by using PCI_VDEVICE macro. Testing details: Tested the patch by injecting 'ECC' type errors using mce_amd_inj and error decoding works fine. Signed-off-by: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com> --- drivers/edac/amd64_edac.c | 226 ++++++++++++++++++++++++++++++++-------------- drivers/edac/amd64_edac.h | 12 ++- 2 files changed, 169 insertions(+), 69 deletions(-)