2013-08-09 16:55:04

by Aravind Gopalakrishnan

[permalink] [raw]
Subject: [PATCH 3/3 V4] EDAC, AMD64_EDAC: Add ECC decoding support for newer F15h models.

Adding support for handling ECC error decoding for new F15 models.
On newer models, support has been included for upto 4 DCT's, however,
only DCT0 and DCT3 are currently configured. (Refer BKDG Section 2.10)
Routing DRAM Requests algorithm is different for F15h M30h.
It is cleaner to use a brand new function rather than
adding quirks in the more generic 'f1x_match_to_this_node'
Refer "2.10.5 DRAM Routing Requests" in BKDG for info.

Tested on Fam15h M30h with ECC turned on using mce_amd_inj facility and
verified to be functionally correct.

Changes from V3:
- Remove compiler warnings
- Recheck models that need erratum workaround for E505:
- From looking up history of the bug, we find that
the workaround holds true only for Fam15 models upto 1h
and only upto stepping 0h. CPU revisions after these
do not need the workaround.

Signed-off-by: Aravind Gopalakrishnan <[email protected]>

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 8b6a034..bf07ae7 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -123,7 +123,7 @@ static void f15h_select_dct(struct amd64_pvt *pvt, u8 dct)
u32 reg = 0;

amd64_read_pci_cfg(pvt->F1, DCT_CFG_SEL, &reg);
- reg &= 0xfffffffe;
+ reg &= (pvt->model >= 0x30) ? ~3 : ~1;
reg |= dct;
amd64_write_pci_cfg(pvt->F1, DCT_CFG_SEL, reg);
}
@@ -133,8 +133,12 @@ static int f15_read_dct_pci_cfg(struct amd64_pvt *pvt, int addr, u32 *val,
{
u8 dct = 0;

+ /*
+ * For F15 M30h, the second dct is DCT 3.
+ * Refer BKDG Section 2.10
+ */
if (addr >= 0x140 && addr <= 0x1a0) {
- dct = 1;
+ dct = (pvt->model >= 0x30) ? 3 : 1;
addr -= 0x100;
}

@@ -205,8 +209,10 @@ static int amd64_set_scrub_rate(struct mem_ctl_info *mci, u32 bw)
if (boot_cpu_data.x86 == 0xf)
min_scrubrate = 0x0;

- /* F15h Erratum #505 */
- if (boot_cpu_data.x86 == 0x15)
+ /* Erratum #505 for F15h Model 0x00 - Model 0x01, Stepping 0 */
+ if (boot_cpu_data.x86 == 0x15 &&
+ boot_cpu_data.x86_model <= 0x01 &&
+ boot_cpu_data.x86_mask < 0x1)
f15h_select_dct(pvt, 0);

return __amd64_set_scrub_rate(pvt->F3, bw, min_scrubrate);
@@ -218,8 +224,10 @@ static int amd64_get_scrub_rate(struct mem_ctl_info *mci)
u32 scrubval = 0;
int i, retval = -EINVAL;

- /* F15h Erratum #505 */
- if (boot_cpu_data.x86 == 0x15)
+ /* Erratum #505 for F15h Model 0x00 - Model 0x01, Stepping 0 */
+ if (boot_cpu_data.x86 == 0x15 &&
+ boot_cpu_data.x86_model <= 0x01 &&
+ boot_cpu_data.x86_mask < 0x1)
f15h_select_dct(pvt, 0);

amd64_read_pci_cfg(pvt->F3, SCRCTRL, &scrubval);
@@ -343,10 +351,11 @@ static void get_cs_base_and_mask(struct amd64_pvt *pvt, int csrow, u8 dct,
addr_shift = 4;

/*
- * F16h needs two addr_shift values: 8 for high and 6 for low
- * (cf. F16h BKDG).
+ * F16h and F15h(model 30h) needs two addr_shift values:
+ * 8 for high and 6 for low (cf. F16h BKDG).
*/
- } else if (boot_cpu_data.x86 == 0x16) {
+ } else if (pvt->fam == 0x16 ||
+ (pvt->fam == 0x15 && pvt->model >= 0x30)) {
csbase = pvt->csels[dct].csbases[csrow];
csmask = pvt->csels[dct].csmasks[csrow >> 1];

@@ -743,6 +752,10 @@ static void prep_chip_selects(struct amd64_pvt *pvt)
if (boot_cpu_data.x86 == 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) {
+ pvt->csels[0].b_cnt = pvt->csels[1].b_cnt = 4;
+ pvt->csels[0].m_cnt = pvt->csels[1].m_cnt = 2;
} else {
pvt->csels[0].b_cnt = pvt->csels[1].b_cnt = 8;
pvt->csels[0].m_cnt = pvt->csels[1].m_cnt = 4;
@@ -916,8 +929,9 @@ static struct pci_dev *pci_get_related_function(unsigned int vendor,
static void read_dram_base_limit_regs(struct amd64_pvt *pvt, unsigned range)
{
struct amd_northbridge *nb;
- struct pci_dev *misc, *f1 = NULL;
+ struct pci_dev *f1 = NULL;
struct cpuinfo_x86 *c = &boot_cpu_data;
+ unsigned int pci_func;
int off = range << 3;
u32 llim;

@@ -941,8 +955,11 @@ static void read_dram_base_limit_regs(struct amd64_pvt *pvt, unsigned range)
if (WARN_ON(!nb))
return;

- misc = nb->misc;
- f1 = pci_get_related_function(misc->vendor, PCI_DEVICE_ID_AMD_15H_NB_F1, misc);
+ pci_func = (c->x86_model == 0x30) ? PCI_DEVICE_ID_AMD_15H_M30H_NB_F1
+ : PCI_DEVICE_ID_AMD_15H_NB_F1;
+
+ f1 = pci_get_related_function(nb->misc->vendor, pci_func, nb->misc);
+
if (WARN_ON(!f1))
return;

@@ -1173,7 +1190,8 @@ static int f15_dbam_to_chip_select(struct amd64_pvt *pvt, u8 dct,
}

/*
- * F16h has only limited cs_modes
+ * F16h has only limited cs_modes.
+ * F15 M30h follows the same pattern.
*/
static int f16_dbam_to_chip_select(struct amd64_pvt *pvt, u8 dct,
unsigned cs_mode)
@@ -1218,6 +1236,29 @@ static void read_dram_ctl_register(struct amd64_pvt *pvt)
}

/*
+ * Determine channel (DCT) based on the interleaving mode:
+ * F15h M30h BKDG, 2.10.12 Memory Interleaving Modes.
+ */
+static u8 f15_m30h_determine_channel(struct amd64_pvt *pvt, u64 sys_addr,
+ u8 intlv_en, int num_dcts_intlv,
+ u32 dct_sel)
+{
+ u8 channel = 0;
+ u8 select;
+
+ if (!(intlv_en))
+ return (u8)(dct_sel);
+
+ if (num_dcts_intlv == 2) {
+ select = (sys_addr >> 8) & 0x3;
+ channel = select ? 0x3 : 0;
+ } else if (num_dcts_intlv == 4)
+ channel = (sys_addr >> 8) & 0x7;
+
+ return channel;
+}
+
+/*
* Determine channel (DCT) based on the interleaving mode: F10h BKDG, 2.8.9 Memory
* Interleaving Modes.
*/
@@ -1366,6 +1407,10 @@ static int f1x_lookup_addr_in_dct(u64 in_addr, u8 nid, u8 dct)
(in_addr & cs_mask), (cs_base & cs_mask));

if ((in_addr & cs_mask) == (cs_base & cs_mask)) {
+ if (pvt->fam == 0x15 && pvt->model >= 0x30) {
+ cs_found = csrow;
+ break;
+ }
cs_found = f10_process_possible_spare(pvt, dct, csrow);

edac_dbg(1, " MATCH csrow=%d\n", cs_found);
@@ -1492,20 +1537,140 @@ static int f1x_match_to_this_node(struct amd64_pvt *pvt, unsigned range,
return cs_found;
}

-static int f1x_translate_sysaddr_to_cs(struct amd64_pvt *pvt, u64 sys_addr,
- int *chan_sel)
+static int f15_m30h_match_to_this_node(struct amd64_pvt *pvt, unsigned range,
+ u64 sys_addr, int *chan_sel)
+{
+ int cs_found = -EINVAL;
+ int num_dcts_intlv = 0;
+ u64 chan_addr, chan_offset;
+ u64 dct_base, dct_limit;
+ u32 dct_cont_base_reg, dct_cont_limit_reg, tmp;
+ u8 channel, alias_channel, leg_mmio_hole, dct_sel, dct_offset_en;
+
+ u64 dhar_offset = f10_dhar_offset(pvt);
+ u8 intlv_addr = dct_sel_interleave_addr(pvt);
+ u8 node_id = dram_dst_node(pvt, range);
+ u8 intlv_en = dram_intlv_en(pvt, range);
+
+ amd64_read_pci_cfg(pvt->F1, DRAM_CONT_BASE, &dct_cont_base_reg);
+ amd64_read_pci_cfg(pvt->F1, DRAM_CONT_LIMIT, &dct_cont_limit_reg);
+
+ dct_offset_en = (u8) ((dct_cont_base_reg >> 3) & BIT(0));
+ dct_sel = (u8) ((dct_cont_base_reg >> 4) & 0x7);
+
+ edac_dbg(1, "(range %d) SystemAddr= 0x%llx Limit=0x%llx\n",
+ range, sys_addr, get_dram_limit(pvt, range));
+
+ if (!(get_dram_base(pvt, range) <= sys_addr) &&
+ !(get_dram_limit(pvt, range) >= sys_addr))
+ return -EINVAL;
+
+ if (dhar_valid(pvt) &&
+ dhar_base(pvt) <= sys_addr &&
+ sys_addr < BIT_64(32)) {
+ amd64_warn("Huh? Address is in the MMIO hole: 0x%016llx\n",
+ sys_addr);
+ return -EINVAL;
+ }
+
+ /* Verify sys_addr is within DCT Range. */
+ dct_base = (dct_sel_baseaddr(pvt) << 27);
+ dct_limit = (((dct_cont_limit_reg >> 11) & 0x1FFF) << 27) | 0x7FFFFFF;
+ if (!(dct_cont_base_reg & BIT(0)) &&
+ !(dct_base <= sys_addr && dct_limit >= sys_addr)) {
+ return -EINVAL;
+ }
+
+ /* Verify number of dct's that participate in channel interleaving. */
+ num_dcts_intlv = (int) hweight8(intlv_en);
+
+ if (!(num_dcts_intlv % 2 == 0) || (num_dcts_intlv > 4))
+ return -EINVAL;
+
+ channel = f15_m30h_determine_channel(pvt, sys_addr, intlv_en,
+ num_dcts_intlv, dct_sel);
+
+ /* Verify we stay within the MAX number of channels allowed */
+ if (channel > 4 || channel < 0)
+ return -EINVAL;
+
+ leg_mmio_hole = (u8) (dct_cont_base_reg >> 1 & BIT(0));
+
+ /* Get normalized DCT addr */
+ if (leg_mmio_hole && (sys_addr >= BIT_64(32)))
+ chan_offset = dhar_offset;
+ else
+ chan_offset = dct_base;
+
+ chan_addr = sys_addr - chan_offset;
+
+ /* remove channel interleave */
+ if (num_dcts_intlv == 2) {
+ if (intlv_addr == 0x4)
+ chan_addr = ((chan_addr >> 9) << 8) |
+ (chan_addr & 0xff);
+ else if (intlv_addr == 0x5)
+ chan_addr = ((chan_addr >> 10) << 9) |
+ (chan_addr & 0x1ff);
+ else
+ return -EINVAL;
+
+ } else if (num_dcts_intlv == 4) {
+ if (intlv_addr == 0x4)
+ chan_addr = ((chan_addr >> 10) << 8) |
+ (chan_addr & 0xff);
+ else if (intlv_addr == 0x5)
+ chan_addr = ((chan_addr >> 11) << 9) |
+ (chan_addr & 0x1ff);
+ else
+ return -EINVAL;
+ }
+
+ if (dct_offset_en) {
+ amd64_read_pci_cfg(pvt->F1,
+ DRAM_CONT_HIGH_OFF + (int) channel * 4,
+ &tmp);
+ chan_addr += ((tmp >> 11) & 0xfff) << 27;
+ }
+
+ f15h_select_dct(pvt, channel);
+
+ edac_dbg(1, " Normalized DCT addr: 0x%llx\n", chan_addr);
+ /*
+ * Find Chip select:
+ * if channel = 3, then alias it to 1. This is because, in F15 M30h,
+ * there is support for 4 DCT's, but only 2 are currently functional.
+ * They are DCT0 and DCT3. But we have read all registers of DCT3 into
+ * pvt->csels[1]. So we need to use '1' here to get correct info.
+ * Refer F15 M30h BKDG Section 2.10 and 2.10.3 for clarifications.
+ */
+
+ alias_channel = (channel == 3) ? 1 : channel;
+
+ cs_found = f1x_lookup_addr_in_dct(chan_addr, node_id, alias_channel);
+
+ if (cs_found >= 0)
+ *chan_sel = alias_channel;
+
+ return cs_found;
+}
+
+static int f1x_translate_sysaddr_to_cs(struct amd64_pvt *pvt,
+ u64 sys_addr,
+ int *chan_sel)
{
int cs_found = -EINVAL;
unsigned range;

for (range = 0; range < DRAM_RANGES; range++) {
-
if (!dram_rw(pvt, range))
continue;
-
- if ((get_dram_base(pvt, range) <= sys_addr) &&
- (get_dram_limit(pvt, range) >= sys_addr)) {
-
+ if (pvt->fam == 0x15 && pvt->model >= 0x30)
+ cs_found = f15_m30h_match_to_this_node(pvt, range,
+ sys_addr,
+ chan_sel);
+ else if ((get_dram_base(pvt, range) <= sys_addr) &&
+ (get_dram_limit(pvt, range) >= sys_addr)) {
cs_found = f1x_match_to_this_node(pvt, range,
sys_addr, chan_sel);
if (cs_found >= 0)
@@ -1624,6 +1789,17 @@ static struct amd64_family_type amd64_family_types[] = {
.read_dct_pci_cfg = f15_read_dct_pci_cfg,
}
},
+ [F15_M30H_CPUS] = {
+ .ctl_name = "F15h_M30h",
+ .f1_id = PCI_DEVICE_ID_AMD_15H_M30H_NB_F1,
+ .f3_id = PCI_DEVICE_ID_AMD_15H_M30H_NB_F3,
+ .ops = {
+ .early_channel_count = f1x_early_channel_count,
+ .map_sysaddr_to_csrow = f1x_map_sysaddr_to_csrow,
+ .dbam_to_cs = f16_dbam_to_chip_select,
+ .read_dct_pci_cfg = f15_read_dct_pci_cfg,
+ }
+ },
[F16_CPUS] = {
.ctl_name = "F16h",
.f1_id = PCI_DEVICE_ID_AMD_16H_NB_F1,
@@ -2388,6 +2564,7 @@ static void setup_mci_misc_attrs(struct mem_ctl_info *mci,
static struct amd64_family_type *amd64_per_family_init(struct amd64_pvt *pvt)
{
u8 fam = boot_cpu_data.x86;
+ u8 model = boot_cpu_data.x86_model;
struct amd64_family_type *fam_type = NULL;

switch (fam) {
@@ -2402,6 +2579,12 @@ static struct amd64_family_type *amd64_per_family_init(struct amd64_pvt *pvt)
break;

case 0x15:
+ if (model == 0x30) {
+ fam_type = &amd64_family_types[F15_M30H_CPUS];
+ pvt->ops = &amd64_family_types[F15_M30H_CPUS].ops;
+ break;
+ }
+
fam_type = &amd64_family_types[F15_CPUS];
pvt->ops = &amd64_family_types[F15_CPUS].ops;
break;
@@ -2417,6 +2600,8 @@ static struct amd64_family_type *amd64_per_family_init(struct amd64_pvt *pvt)
}

pvt->ext_model = boot_cpu_data.x86_model >> 4;
+ pvt->fam = fam;
+ pvt->model = model;

amd64_info("%s %sdetected (node %d).\n", fam_type->ctl_name,
(fam == 0xf ?
@@ -2638,6 +2823,14 @@ static DEFINE_PCI_DEVICE_TABLE(amd64_pci_table) = {
},
{
.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,
diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
index 2c6f113..ffd42bf 100644
--- a/drivers/edac/amd64_edac.h
+++ b/drivers/edac/amd64_edac.h
@@ -170,6 +170,8 @@
/*
* 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_16H_NB_F1 0x1531
@@ -181,13 +183,24 @@
#define DRAM_BASE_LO 0x40
#define DRAM_LIMIT_LO 0x44

-#define dram_intlv_en(pvt, i) ((u8)((pvt->ranges[i].base.lo >> 8) & 0x7))
+/*
+ * F15 M30h DRAM Controller Base/Limit
+ * D18F1x2[1C:00]
+ */
+#define DRAM_CONT_BASE 0x200
+#define DRAM_CONT_LIMIT 0x204
+
+/*
+ * F15 M30h DRAM Controller High Addre Offset
+ * D18F1x2[4C:40]
+ */
+#define DRAM_CONT_HIGH_OFF 0x240
+
#define dram_rw(pvt, i) ((u8)(pvt->ranges[i].base.lo & 0x3))
#define dram_intlv_sel(pvt, i) ((u8)((pvt->ranges[i].lim.lo >> 8) & 0x7))
#define dram_dst_node(pvt, i) ((u8)(pvt->ranges[i].lim.lo & 0x7))

#define DHAR 0xf0
-#define dhar_valid(pvt) ((pvt)->dhar & BIT(0))
#define dhar_mem_hoist_valid(pvt) ((pvt)->dhar & BIT(1))
#define dhar_base(pvt) ((pvt)->dhar & 0xff000000)
#define k8_dhar_offset(pvt) (((pvt)->dhar & 0x0000ff00) << 16)
@@ -234,8 +247,6 @@
#define DDR3_MODE BIT(8)

#define DCT_SEL_LO 0x110
-#define dct_sel_baseaddr(pvt) ((pvt)->dct_sel_lo & 0xFFFFF800)
-#define dct_sel_interleave_addr(pvt) (((pvt)->dct_sel_lo >> 6) & 0x3)
#define dct_high_range_enabled(pvt) ((pvt)->dct_sel_lo & BIT(0))
#define dct_interleave_enabled(pvt) ((pvt)->dct_sel_lo & BIT(2))

@@ -297,6 +308,7 @@ enum amd_families {
K8_CPUS = 0,
F10_CPUS,
F15_CPUS,
+ F15_M30H_CPUS,
F16_CPUS,
NUM_FAMILIES,
};
@@ -337,6 +349,8 @@ struct amd64_pvt {
struct pci_dev *F1, *F2, *F3;

u16 mc_node_id; /* MC index of this MC node */
+ u8 fam; /* CPU family */
+ u8 model; /* CPU model */
int ext_model; /* extended model value of this node */
int channel_count;

@@ -414,6 +428,14 @@ static inline u16 extract_syndrome(u64 status)
return ((status >> 47) & 0xff) | ((status >> 16) & 0xff00);
}

+static inline u8 dct_sel_interleave_addr(struct amd64_pvt *pvt)
+{
+ if (pvt->fam == 0x15 && pvt->model >= 0x30)
+ return (((pvt->dct_sel_hi >> 9) & 0x1) << 2) |
+ ((pvt->dct_sel_lo >> 6) & 0x3);
+
+ return ((pvt)->dct_sel_lo >> 6) & 0x3;
+}
/*
* per-node ECC settings descriptor
*/
@@ -504,3 +526,33 @@ static inline void enable_caches(void *dummy)
{
write_cr0(read_cr0() & ~X86_CR0_CD);
}
+
+static inline u8 dram_intlv_en(struct amd64_pvt *pvt, unsigned int i)
+{
+ if (pvt->fam == 0x15 && pvt->model >= 0x30) {
+ u32 tmp;
+ amd64_read_pci_cfg(pvt->F1, DRAM_CONT_LIMIT, &tmp);
+ return (u8) tmp & 0xF;
+ }
+ return (u8) (pvt->ranges[i].base.lo >> 8) & 0x7;
+}
+
+static inline u8 dhar_valid(struct amd64_pvt *pvt)
+{
+ if (pvt->fam == 0x15 && pvt->model >= 0x30) {
+ u32 tmp;
+ amd64_read_pci_cfg(pvt->F1, DRAM_CONT_BASE, &tmp);
+ return (tmp >> 1) & BIT(0);
+ }
+ return (pvt)->dhar & BIT(0);
+}
+
+static inline u32 dct_sel_baseaddr(struct amd64_pvt *pvt)
+{
+ if (pvt->fam == 0x15 && pvt->model >= 0x30) {
+ u32 tmp;
+ amd64_read_pci_cfg(pvt->F1, DRAM_CONT_BASE, &tmp);
+ return (tmp >> 11) & 0x1FFF;
+ }
+ return (pvt)->dct_sel_lo & 0xFFFFF800;
+}
--
1.7.10.4


2013-08-10 15:15:35

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 3/3 V4] EDAC, AMD64_EDAC: Add ECC decoding support for newer F15h models.

On Fri, Aug 09, 2013 at 11:54:49AM -0500, Aravind Gopalakrishnan wrote:
> Adding support for handling ECC error decoding for new F15 models.
> On newer models, support has been included for upto 4 DCT's, however,
> only DCT0 and DCT3 are currently configured. (Refer BKDG Section 2.10)
> Routing DRAM Requests algorithm is different for F15h M30h.
> It is cleaner to use a brand new function rather than
> adding quirks in the more generic 'f1x_match_to_this_node'
> Refer "2.10.5 DRAM Routing Requests" in BKDG for info.
>
> Tested on Fam15h M30h with ECC turned on using mce_amd_inj facility and
> verified to be functionally correct.
>
> Changes from V3:
> - Remove compiler warnings
> - Recheck models that need erratum workaround for E505:
> - From looking up history of the bug, we find that
> the workaround holds true only for Fam15 models upto 1h
> and only upto stepping 0h. CPU revisions after these
> do not need the workaround.
>
> Signed-off-by: Aravind Gopalakrishnan <[email protected]>

Ok, I've taken it and applied a cleanup ontop which uses the locally
cached family, model, stepping (sending it as a reply to this message).

I'd appreciate it if you tested the branch here

git://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git amd-f15-m30

just to make sure everything is still kosher.

Thanks.

--
Regards/Gruss,
Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

2013-08-10 15:16:51

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH] amd64_edac: Get rid of boot_cpu_data accesses

From: Borislav Petkov <[email protected]>
Date: Sat, 10 Aug 2013 13:54:48 +0200
Subject: [PATCH] amd64_edac: Get rid of boot_cpu_data accesses

Now that we cache (family, model, stepping) locally, use them instead of
boot_cpu_data.

No functionality change.

Signed-off-by: Borislav Petkov <[email protected]>
---
drivers/edac/amd64_edac.c | 90 ++++++++++++++++++++++-------------------------
drivers/edac/amd64_edac.h | 4 ++-
2 files changed, 46 insertions(+), 48 deletions(-)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 650d30c271b9..b86228cce672 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -203,13 +203,11 @@ static int amd64_set_scrub_rate(struct mem_ctl_info *mci, u32 bw)
struct amd64_pvt *pvt = mci->pvt_info;
u32 min_scrubrate = 0x5;

- if (boot_cpu_data.x86 == 0xf)
+ if (pvt->fam == 0xf)
min_scrubrate = 0x0;

/* Erratum #505 for F15h Model 0x00 - Model 0x01, Stepping 0 */
- if (boot_cpu_data.x86 == 0x15 &&
- boot_cpu_data.x86_model <= 0x01 &&
- boot_cpu_data.x86_mask < 0x1)
+ if (pvt->fam == 0x15 && pvt->model <= 0x01 && pvt->stepping < 0x1)
f15h_select_dct(pvt, 0);

return __amd64_set_scrub_rate(pvt->F3, bw, min_scrubrate);
@@ -222,9 +220,7 @@ static int amd64_get_scrub_rate(struct mem_ctl_info *mci)
int i, retval = -EINVAL;

/* Erratum #505 for F15h Model 0x00 - Model 0x01, Stepping 0 */
- if (boot_cpu_data.x86 == 0x15 &&
- boot_cpu_data.x86_model <= 0x01 &&
- boot_cpu_data.x86_mask < 0x1)
+ if (pvt->fam == 0x15 && pvt->model <= 0x01 && pvt->stepping < 0x1)
f15h_select_dct(pvt, 0);

amd64_read_pci_cfg(pvt->F3, SCRCTRL, &scrubval);
@@ -373,7 +369,7 @@ static void get_cs_base_and_mask(struct amd64_pvt *pvt, int csrow, u8 dct,
csmask = pvt->csels[dct].csmasks[csrow >> 1];
addr_shift = 8;

- if (boot_cpu_data.x86 == 0x15)
+ if (pvt->fam == 0x15)
base_bits = mask_bits = GENMASK(19,30) | GENMASK(5,13);
else
base_bits = mask_bits = GENMASK(19,28) | GENMASK(5,13);
@@ -453,14 +449,14 @@ int amd64_get_dram_hole_info(struct mem_ctl_info *mci, u64 *hole_base,
struct amd64_pvt *pvt = mci->pvt_info;

/* only revE and later have the DRAM Hole Address Register */
- if (boot_cpu_data.x86 == 0xf && pvt->ext_model < K8_REV_E) {
+ if (pvt->fam == 0xf && pvt->ext_model < K8_REV_E) {
edac_dbg(1, " revision %d for node %d does not support DHAR\n",
pvt->ext_model, pvt->mc_node_id);
return 1;
}

/* valid for Fam10h and above */
- if (boot_cpu_data.x86 >= 0x10 && !dhar_mem_hoist_valid(pvt)) {
+ if (pvt->fam >= 0x10 && !dhar_mem_hoist_valid(pvt)) {
edac_dbg(1, " Dram Memory Hoisting is DISABLED on this system\n");
return 1;
}
@@ -492,10 +488,8 @@ int amd64_get_dram_hole_info(struct mem_ctl_info *mci, u64 *hole_base,
*hole_base = dhar_base(pvt);
*hole_size = (1ULL << 32) - *hole_base;

- if (boot_cpu_data.x86 > 0xf)
- *hole_offset = f10_dhar_offset(pvt);
- else
- *hole_offset = k8_dhar_offset(pvt);
+ *hole_offset = (pvt->fam > 0xf) ? f10_dhar_offset(pvt)
+ : k8_dhar_offset(pvt);

edac_dbg(1, " DHAR info for node %d base 0x%lx offset 0x%lx size 0x%lx\n",
pvt->mc_node_id, (unsigned long)*hole_base,
@@ -669,7 +663,7 @@ static unsigned long amd64_determine_edac_cap(struct amd64_pvt *pvt)
u8 bit;
unsigned long edac_cap = EDAC_FLAG_NONE;

- bit = (boot_cpu_data.x86 > 0xf || pvt->ext_model >= K8_REV_F)
+ bit = (pvt->fam > 0xf || pvt->ext_model >= K8_REV_F)
? 19
: 17;

@@ -681,7 +675,7 @@ static unsigned long amd64_determine_edac_cap(struct amd64_pvt *pvt)

static void amd64_debug_display_dimm_sizes(struct amd64_pvt *, u8);

-static void amd64_dump_dramcfg_low(u32 dclr, int chan)
+static void amd64_dump_dramcfg_low(struct amd64_pvt *pvt, u32 dclr, int chan)
{
edac_dbg(1, "F2x%d90 (DRAM Cfg Low): 0x%08x\n", chan, dclr);

@@ -692,7 +686,7 @@ static void amd64_dump_dramcfg_low(u32 dclr, int chan)
edac_dbg(1, " PAR/ERR parity: %s\n",
(dclr & BIT(8)) ? "enabled" : "disabled");

- if (boot_cpu_data.x86 == 0x10)
+ if (pvt->fam == 0x10)
edac_dbg(1, " DCT 128bit mode width: %s\n",
(dclr & BIT(11)) ? "128b" : "64b");

@@ -715,21 +709,21 @@ static void dump_misc_regs(struct amd64_pvt *pvt)
(pvt->nbcap & NBCAP_SECDED) ? "yes" : "no",
(pvt->nbcap & NBCAP_CHIPKILL) ? "yes" : "no");

- amd64_dump_dramcfg_low(pvt->dclr0, 0);
+ amd64_dump_dramcfg_low(pvt, pvt->dclr0, 0);

edac_dbg(1, "F3xB0 (Online Spare): 0x%08x\n", pvt->online_spare);

edac_dbg(1, "F1xF0 (DRAM Hole Address): 0x%08x, base: 0x%08x, offset: 0x%08x\n",
pvt->dhar, dhar_base(pvt),
- (boot_cpu_data.x86 == 0xf) ? k8_dhar_offset(pvt)
- : f10_dhar_offset(pvt));
+ (pvt->fam == 0xf) ? k8_dhar_offset(pvt)
+ : f10_dhar_offset(pvt));

edac_dbg(1, " DramHoleValid: %s\n", dhar_valid(pvt) ? "yes" : "no");

amd64_debug_display_dimm_sizes(pvt, 0);

/* everything below this point is Fam10h and above */
- if (boot_cpu_data.x86 == 0xf)
+ if (pvt->fam == 0xf)
return;

amd64_debug_display_dimm_sizes(pvt, 1);
@@ -738,7 +732,7 @@ static void dump_misc_regs(struct amd64_pvt *pvt)

/* Only if NOT ganged does dclr1 have valid info */
if (!dct_ganging_enabled(pvt))
- amd64_dump_dramcfg_low(pvt->dclr1, 1);
+ amd64_dump_dramcfg_low(pvt, pvt->dclr1, 1);
}

/*
@@ -777,7 +771,7 @@ static void read_dct_base_mask(struct amd64_pvt *pvt)
edac_dbg(0, " DCSB0[%d]=0x%08x reg: F2x%x\n",
cs, *base0, reg0);

- if (boot_cpu_data.x86 == 0xf || dct_ganging_enabled(pvt))
+ if (pvt->fam == 0xf || dct_ganging_enabled(pvt))
continue;

if (!amd64_read_dct_pci_cfg(pvt, reg1, base1))
@@ -795,7 +789,7 @@ static void read_dct_base_mask(struct amd64_pvt *pvt)
edac_dbg(0, " DCSM0[%d]=0x%08x reg: F2x%x\n",
cs, *mask0, reg0);

- if (boot_cpu_data.x86 == 0xf || dct_ganging_enabled(pvt))
+ if (pvt->fam == 0xf || dct_ganging_enabled(pvt))
continue;

if (!amd64_read_dct_pci_cfg(pvt, reg1, mask1))
@@ -809,9 +803,9 @@ static enum mem_type amd64_determine_memory_type(struct amd64_pvt *pvt, int cs)
enum mem_type type;

/* F15h supports only DDR3 */
- if (boot_cpu_data.x86 >= 0x15)
+ if (pvt->fam >= 0x15)
type = (pvt->dclr0 & BIT(16)) ? MEM_DDR3 : MEM_RDDR3;
- else if (boot_cpu_data.x86 == 0x10 || pvt->ext_model >= K8_REV_F) {
+ 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
@@ -844,14 +838,13 @@ static int k8_early_channel_count(struct amd64_pvt *pvt)
}

/* On F10h and later ErrAddr is MC4_ADDR[47:1] */
-static u64 get_error_address(struct mce *m)
+static u64 get_error_address(struct amd64_pvt *pvt, struct mce *m)
{
- struct cpuinfo_x86 *c = &boot_cpu_data;
u64 addr;
u8 start_bit = 1;
u8 end_bit = 47;

- if (c->x86 == 0xf) {
+ if (pvt->fam == 0xf) {
start_bit = 3;
end_bit = 39;
}
@@ -861,7 +854,7 @@ static u64 get_error_address(struct mce *m)
/*
* Erratum 637 workaround
*/
- if (c->x86 == 0x15) {
+ if (pvt->fam == 0x15) {
struct amd64_pvt *pvt;
u64 cc6_base, tmp_addr;
u32 tmp;
@@ -1100,7 +1093,7 @@ static int f1x_early_channel_count(struct amd64_pvt *pvt)
int i, j, channels = 0;

/* On F10h, if we are in 128 bit mode, then we are using 2 channels */
- if (boot_cpu_data.x86 == 0x10 && (pvt->dclr0 & WIDTH_128))
+ if (pvt->fam == 0x10 && (pvt->dclr0 & WIDTH_128))
return 2;

/*
@@ -1201,7 +1194,7 @@ static int f16_dbam_to_chip_select(struct amd64_pvt *pvt, u8 dct,
static void read_dram_ctl_register(struct amd64_pvt *pvt)
{

- if (boot_cpu_data.x86 == 0xf)
+ if (pvt->fam == 0xf)
return;

if (!amd64_read_dct_pci_cfg(pvt, DCT_SEL_LO, &pvt->dct_sel_lo)) {
@@ -1422,11 +1415,9 @@ static u64 f1x_swap_interleaved_region(struct amd64_pvt *pvt, u64 sys_addr)
{
u32 swap_reg, swap_base, swap_limit, rgn_size, tmp_addr;

- if (boot_cpu_data.x86 == 0x10) {
+ if (pvt->fam == 0x10) {
/* only revC3 and revE have that feature */
- if (boot_cpu_data.x86_model < 4 ||
- (boot_cpu_data.x86_model < 0xa &&
- boot_cpu_data.x86_mask < 3))
+ if (pvt->model < 4 || (pvt->model < 0xa && pvt->stepping < 3))
return sys_addr;
}

@@ -1714,7 +1705,7 @@ static void amd64_debug_display_dimm_sizes(struct amd64_pvt *pvt, u8 ctrl)
u32 *dcsb = ctrl ? pvt->csels[1].csbases : pvt->csels[0].csbases;
u32 dbam = ctrl ? pvt->dbam1 : pvt->dbam0;

- if (boot_cpu_data.x86 == 0xf) {
+ if (pvt->fam == 0xf) {
/* K8 families < revF not supported yet */
if (pvt->ext_model < K8_REV_F)
return;
@@ -2031,7 +2022,7 @@ static inline void __amd64_decode_bus_error(struct mem_ctl_info *mci,

memset(&err, 0, sizeof(err));

- sys_addr = get_error_address(m);
+ sys_addr = get_error_address(pvt, m);

if (ecc_type == 2)
err.syndrome = extract_syndrome(m->status);
@@ -2092,10 +2083,9 @@ static void free_mc_sibling_devs(struct amd64_pvt *pvt)
*/
static void read_mc_regs(struct amd64_pvt *pvt)
{
- struct cpuinfo_x86 *c = &boot_cpu_data;
+ unsigned range;
u64 msr_val;
u32 tmp;
- unsigned range;

/*
* Retrieve TOP_MEM and TOP_MEM2; no masking off of reserved bits since
@@ -2156,14 +2146,14 @@ static void read_mc_regs(struct amd64_pvt *pvt)

pvt->ecc_sym_sz = 4;

- if (c->x86 >= 0x10) {
+ if (pvt->fam >= 0x10) {
amd64_read_pci_cfg(pvt->F3, EXT_NB_MCA_CFG, &tmp);
- if (c->x86 != 0x16)
+ if (pvt->fam != 0x16)
/* F16h has only DCT0 */
amd64_read_dct_pci_cfg(pvt, DBAM1, &pvt->dbam1);

/* F10h, revD and later can do x8 ECC too */
- if ((c->x86 > 0x10 || c->x86_model > 7) && tmp & BIT(25))
+ if ((pvt->fam > 0x10 || pvt->model > 7) && tmp & BIT(25))
pvt->ecc_sym_sz = 8;
}
dump_misc_regs(pvt);
@@ -2257,7 +2247,7 @@ static int init_csrows(struct mem_ctl_info *mci)
bool row_dct0 = !!csrow_enabled(i, 0, pvt);
bool row_dct1 = false;

- if (boot_cpu_data.x86 != 0xf)
+ if (pvt->fam != 0xf)
row_dct1 = !!csrow_enabled(i, 1, pvt);

if (!row_dct0 && !row_dct1)
@@ -2275,7 +2265,7 @@ static int init_csrows(struct mem_ctl_info *mci)
}

/* K8 has only one DCT */
- if (boot_cpu_data.x86 != 0xf && row_dct1) {
+ if (pvt->fam != 0xf && row_dct1) {
int row_dct1_pages = amd64_csrow_nr_pages(pvt, 1, i);

csrow->channels[1]->dimm->nr_pages = row_dct1_pages;
@@ -2504,13 +2494,14 @@ static bool ecc_enabled(struct pci_dev *F3, u16 nid)

static int set_mc_sysfs_attrs(struct mem_ctl_info *mci)
{
+ struct amd64_pvt *pvt = mci->pvt_info;
int rc;

rc = amd64_create_sysfs_dbg_files(mci);
if (rc < 0)
return rc;

- if (boot_cpu_data.x86 >= 0x10) {
+ if (pvt->fam >= 0x10) {
rc = amd64_create_sysfs_inject_files(mci);
if (rc < 0)
return rc;
@@ -2521,9 +2512,11 @@ static int set_mc_sysfs_attrs(struct mem_ctl_info *mci)

static void del_mc_sysfs_attrs(struct mem_ctl_info *mci)
{
+ struct amd64_pvt *pvt = mci->pvt_info;
+
amd64_remove_sysfs_dbg_files(mci);

- if (boot_cpu_data.x86 >= 0x10)
+ if (pvt->fam >= 0x10)
amd64_remove_sysfs_inject_files(mci);
}

@@ -2561,6 +2554,7 @@ static struct amd64_family_type *amd64_per_family_init(struct amd64_pvt *pvt)
struct amd64_family_type *fam_type = NULL;

pvt->ext_model = boot_cpu_data.x86_model >> 4;
+ pvt->stepping = boot_cpu_data.x86_mask;
pvt->model = boot_cpu_data.x86_model;
pvt->fam = boot_cpu_data.x86;

@@ -2764,6 +2758,8 @@ static void amd64_remove_one_instance(struct pci_dev *pdev)
struct ecc_settings *s = ecc_stngs[nid];

mci = find_mci_by_dev(&pdev->dev);
+ WARN_ON(!mci);
+
del_mc_sysfs_attrs(mci);
/* Remove from EDAC CORE tracking list */
mci = edac_mc_del_mc(&pdev->dev);
diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
index 8fddad7b3b95..d2443cfa0698 100644
--- a/drivers/edac/amd64_edac.h
+++ b/drivers/edac/amd64_edac.h
@@ -348,7 +348,9 @@ struct amd64_pvt {

u16 mc_node_id; /* MC index of this MC node */
u8 fam; /* CPU family */
- u8 model; /* CPU model */
+ u8 model; /* ... model */
+ u8 stepping; /* ... stepping */
+
int ext_model; /* extended model value of this node */
int channel_count;

--
1.8.3

--
Regards/Gruss,
Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

2013-08-10 18:28:58

by Aravind Gopalakrishnan

[permalink] [raw]
Subject: Re: [PATCH 3/3 V4] EDAC, AMD64_EDAC: Add ECC decoding support for newer F15h models.


On Aug 10, 2013, at 10:15 AM, Borislav Petkov <[email protected]>
wrote:

> On Fri, Aug 09, 2013 at 11:54:49AM -0500, Aravind Gopalakrishnan wrote:
>> Adding support for handling ECC error decoding for new F15 models.
>> On newer models, support has been included for upto 4 DCT's, however,
>> only DCT0 and DCT3 are currently configured. (Refer BKDG Section 2.10)
>> Routing DRAM Requests algorithm is different for F15h M30h.
>> It is cleaner to use a brand new function rather than
>> adding quirks in the more generic 'f1x_match_to_this_node'
>> Refer "2.10.5 DRAM Routing Requests" in BKDG for info.
>>
>> Tested on Fam15h M30h with ECC turned on using mce_amd_inj facility and
>> verified to be functionally correct.
>>
>> Changes from V3:
>> - Remove compiler warnings
>> - Recheck models that need erratum workaround for E505:
>> - From looking up history of the bug, we find that
>> the workaround holds true only for Fam15 models upto 1h
>> and only upto stepping 0h. CPU revisions after these
>> do not need the workaround.
>>
>> Signed-off-by: Aravind Gopalakrishnan <[email protected]>
>
> Ok, I've taken it and applied a cleanup ontop which uses the locally
> cached family, model, stepping (sending it as a reply to this message).
>
> I'd appreciate it if you tested the branch here
>
> git://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git amd-f15-m30
>
> just to make sure everything is still kosher.
>

Thanks!
I have tested the branch and it works fine.

-Aravind.

> Thanks.
>
> --
> Regards/Gruss,
> Boris.
>
> Sent from a fat crate under my desk. Formatting is fine.
> --
>

2013-08-10 18:35:24

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 3/3 V4] EDAC, AMD64_EDAC: Add ECC decoding support for newer F15h models.

On Sat, Aug 10, 2013 at 01:28:44PM -0500, Aravind Gopalakrishnan wrote:
> I have tested the branch and it works fine.

Cool, thanks!

--
Regards/Gruss,
Boris.

Sent from a fat crate under my desk. Formatting is fine.
--