[PATCH 0/3] EDAC: add DDR4 flag, misc improvements

[PATCH 0/3] EDAC: add DDR4 flag, misc improvements

[lukasz.anaczkowski]

From: Lukasz Anaczkowski <lukasz.anaczkowski@intel.com>

Series of patches that makes sb_edac more flexible for adding
support for future platforms.

Jim Snow (3):
  EDAC: add DDR4 flag
  sb_edac: virtualize several hard-coded functions
  sb_edac: support for duplicate device IDs

 drivers/edac/sb_edac.c | 95 ++++++++++++++++++++++++++++++++++++++++----------
 include/linux/edac.h   |  6 ++--
 2 files changed, 80 insertions(+), 21 deletions(-)

-- 
1.8.3.1

--------------------------------------------------------------------

Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.

[PATCH 1/3] EDAC: add DDR4 flag

[lukasz.anaczkowski]

From: Jim Snow <jim.m.snow@intel.com>

Signed-off-by: Jim Snow <jim.m.snow@intel.com>
Signed-off-by: Lukasz Anaczkowski <lukasz.anaczkowski@intel.com>
---
 include/linux/edac.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/linux/edac.h b/include/linux/edac.h
index da3b72e..bb305b17 100644
--- a/include/linux/edac.h
+++ b/include/linux/edac.h
@@ -237,8 +237,10 @@ enum mem_type {
 #define MEM_FLAG_FB_DDR2        BIT(MEM_FB_DDR2)
 #define MEM_FLAG_RDDR2          BIT(MEM_RDDR2)
 #define MEM_FLAG_XDR            BIT(MEM_XDR)
-#define MEM_FLAG_DDR3		 BIT(MEM_DDR3)
-#define MEM_FLAG_RDDR3		 BIT(MEM_RDDR3)
+#define MEM_FLAG_DDR3           BIT(MEM_DDR3)
+#define MEM_FLAG_RDDR3          BIT(MEM_RDDR3)
+#define MEM_FLAG_DDR4           BIT(MEM_DDR4)
+#define MEM_FLAG_RDDR4          BIT(MEM_RDDR4)
 
 /**
  * enum edac-type - Error Detection and Correction capabilities and mode
-- 
1.8.3.1

--------------------------------------------------------------------

Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.

[PATCH 2/3] sb_edac: virtualize several hard-coded functions

[lukasz.anaczkowski]

From: Jim Snow <jim.m.snow@intel.com>

Signed-off-by: Jim Snow <jim.m.snow@intel.com>
Signed-off-by: Lukasz Anaczkowski <lukasz.anaczkowski@intel.com>
---
 drivers/edac/sb_edac.c | 55 ++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 44 insertions(+), 11 deletions(-)

diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c
index 1acf57b..6e35405 100644
--- a/drivers/edac/sb_edac.c
+++ b/drivers/edac/sb_edac.c
@@ -65,15 +65,12 @@ static const u32 ibridge_dram_rule[] = {
 	0xd8, 0xe0, 0xe8, 0xf0, 0xf8,
 };
 
-#define SAD_LIMIT(reg)		((GET_BITFIELD(reg, 6, 25) << 26) | 0x3ffffff)
-#define DRAM_ATTR(reg)		GET_BITFIELD(reg, 2,  3)
-#define INTERLEAVE_MODE(reg)	GET_BITFIELD(reg, 1,  1)
 #define DRAM_RULE_ENABLE(reg)	GET_BITFIELD(reg, 0,  0)
 #define A7MODE(reg)		GET_BITFIELD(reg, 26, 26)
 
-static char *get_dram_attr(u32 reg)
+static char *show_dram_attr(u32 attr)
 {
-	switch(DRAM_ATTR(reg)) {
+	switch (attr) {
 		case 0:
 			return "DRAM";
 		case 1:
@@ -273,6 +270,10 @@ struct sbridge_info {
 	u64		(*get_tolm)(struct sbridge_pvt *pvt);
 	u64		(*get_tohm)(struct sbridge_pvt *pvt);
 	u64		(*rir_limit)(u32 reg);
+	u64		(*sad_limit)(u32 reg);
+	u32		(*interleave_mode)(u32 reg);
+	char*		(*show_interleave_mode)(u32 reg);
+	u32		(*dram_attr)(u32 reg);
 	const u32	*dram_rule;
 	const u32	*interleave_list;
 	const struct interleave_pkg *interleave_pkg;
@@ -679,6 +680,26 @@ static u64 rir_limit(u32 reg)
 	return ((u64)GET_BITFIELD(reg,  1, 10) << 29) | 0x1fffffff;
 }
 
+static u64 sad_limit(u32 reg)
+{
+	return (GET_BITFIELD(reg, 6, 25) << 26) | 0x3ffffff;
+}
+
+static u32 interleave_mode(u32 reg)
+{
+	return GET_BITFIELD(reg, 1, 1);
+}
+
+char *show_interleave_mode(u32 reg)
+{
+	return interleave_mode(reg) ? "8:6" : "[8:6]XOR[18:16]";
+}
+
+static u32 dram_attr(u32 reg)
+{
+	return GET_BITFIELD(reg, 2, 3);
+}
+
 static enum mem_type get_memory_type(struct sbridge_pvt *pvt)
 {
 	u32 reg;
@@ -995,7 +1016,7 @@ static void get_memory_layout(const struct mem_ctl_info *mci)
 		/* SAD_LIMIT Address range is 45:26 */
 		pci_read_config_dword(pvt->pci_sad0, pvt->info.dram_rule[n_sads],
 				      &reg);
-		limit = SAD_LIMIT(reg);
+		limit = pvt->info.sad_limit(reg);
 
 		if (!DRAM_RULE_ENABLE(reg))
 			continue;
@@ -1007,10 +1028,10 @@ static void get_memory_layout(const struct mem_ctl_info *mci)
 		gb = div_u64_rem(tmp_mb, 1024, &mb);
 		edac_dbg(0, "SAD#%d %s up to %u.%03u GB (0x%016Lx) Interleave: %s reg=0x%08x\n",
 			 n_sads,
-			 get_dram_attr(reg),
+			 show_dram_attr(pvt->info.dram_attr(reg)),
 			 gb, (mb*1000)/1024,
 			 ((u64)tmp_mb) << 20L,
-			 INTERLEAVE_MODE(reg) ? "8:6" : "[8:6]XOR[18:16]",
+			 pvt->info.show_interleave_mode(reg),
 			 reg);
 		prv = limit;
 
@@ -1174,7 +1195,7 @@ static int get_memory_error_data(struct mem_ctl_info *mci,
 		if (!DRAM_RULE_ENABLE(reg))
 			continue;
 
-		limit = SAD_LIMIT(reg);
+		limit = pvt->info.sad_limit(reg);
 		if (limit <= prv) {
 			sprintf(msg, "Can't discover the memory socket");
 			return -EINVAL;
@@ -1188,8 +1209,8 @@ static int get_memory_error_data(struct mem_ctl_info *mci,
 		return -EINVAL;
 	}
 	dram_rule = reg;
-	*area_type = get_dram_attr(dram_rule);
-	interleave_mode = INTERLEAVE_MODE(dram_rule);
+	*area_type = show_dram_attr(pvt->info.dram_attr(dram_rule));
+	interleave_mode = pvt->info.interleave_mode(dram_rule);
 
 	pci_read_config_dword(pvt->pci_sad0, pvt->info.interleave_list[n_sads],
 			      &reg);
@@ -2322,6 +2343,10 @@ static int sbridge_register_mci(struct sbridge_dev *sbridge_dev, enum type type)
 		pvt->info.get_memory_type = get_memory_type;
 		pvt->info.get_node_id = get_node_id;
 		pvt->info.rir_limit = rir_limit;
+		pvt->info.sad_limit = sad_limit;
+		pvt->info.interleave_mode = interleave_mode;
+		pvt->info.show_interleave_mode = show_interleave_mode;
+		pvt->info.dram_attr = dram_attr;
 		pvt->info.max_sad = ARRAY_SIZE(ibridge_dram_rule);
 		pvt->info.interleave_list = ibridge_interleave_list;
 		pvt->info.max_interleave = ARRAY_SIZE(ibridge_interleave_list);
@@ -2341,6 +2366,10 @@ static int sbridge_register_mci(struct sbridge_dev *sbridge_dev, enum type type)
 		pvt->info.get_memory_type = get_memory_type;
 		pvt->info.get_node_id = get_node_id;
 		pvt->info.rir_limit = rir_limit;
+		pvt->info.sad_limit = sad_limit;
+		pvt->info.interleave_mode = interleave_mode;
+		pvt->info.show_interleave_mode = show_interleave_mode;
+		pvt->info.dram_attr = dram_attr;
 		pvt->info.max_sad = ARRAY_SIZE(sbridge_dram_rule);
 		pvt->info.interleave_list = sbridge_interleave_list;
 		pvt->info.max_interleave = ARRAY_SIZE(sbridge_interleave_list);
@@ -2360,6 +2389,10 @@ static int sbridge_register_mci(struct sbridge_dev *sbridge_dev, enum type type)
 		pvt->info.get_memory_type = haswell_get_memory_type;
 		pvt->info.get_node_id = haswell_get_node_id;
 		pvt->info.rir_limit = haswell_rir_limit;
+		pvt->info.sad_limit = sad_limit;
+		pvt->info.interleave_mode = interleave_mode;
+		pvt->info.show_interleave_mode = show_interleave_mode;
+		pvt->info.dram_attr = dram_attr;
 		pvt->info.max_sad = ARRAY_SIZE(ibridge_dram_rule);
 		pvt->info.interleave_list = ibridge_interleave_list;
 		pvt->info.max_interleave = ARRAY_SIZE(ibridge_interleave_list);
-- 
1.8.3.1

--------------------------------------------------------------------

Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.

Re: [PATCH 2/3] sb_edac: virtualize several hard-coded functions

[Aristeu Rozanski]

Hi Jim, Lukasz,
On Tue, Jun 09, 2015 at 01:43:11PM +0200, lukasz.anaczkowski@intel.com wrote:
> @@ -273,6 +270,10 @@ struct sbridge_info {
>  	u64		(*get_tolm)(struct sbridge_pvt *pvt);
>  	u64		(*get_tohm)(struct sbridge_pvt *pvt);
>  	u64		(*rir_limit)(u32 reg);
> +	u64		(*sad_limit)(u32 reg);
> +	u32		(*interleave_mode)(u32 reg);
> +	char*		(*show_interleave_mode)(u32 reg);
> +	u32		(*dram_attr)(u32 reg);
>  	const u32	*dram_rule;
>  	const u32	*interleave_list;
>  	const struct interleave_pkg *interleave_pkg;

the only reason these exist is for when there's a difference between
memory controllers:
>  		pvt->info.get_memory_type = get_memory_type;

>  		pvt->info.get_memory_type = get_memory_type;

>  		pvt->info.get_memory_type = haswell_get_memory_type;

until then such change isn't really necessary.

-- 
Aristeu

RE: [PATCH 2/3] sb_edac: virtualize several hard-coded functions

[Anaczkowski, Lukasz]

> From: Aristeu Rozanski [mailto:aris@redhat.com] 

> the only reason these exist is for when there's a difference between memory controllers:
> >  		pvt->info.get_memory_type = get_memory_type;
> >  		pvt->info.get_memory_type = get_memory_type;
> >  		pvt->info.get_memory_type = haswell_get_memory_type;
> 

Well, this is similar to what's gonna happen when KNL patches are submitted, but currently, I'm not allowed to disclose them, so, please ignore this change for now.

Cheers,
Lukasz


--------------------------------------------------------------------

Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.

[PATCH 3/3] sb_edac: support for duplicate device IDs

[lukasz.anaczkowski]

From: Jim Snow <jim.m.snow@intel.com>

Add options to sbridge_get_all_devices to allow
for duplicate device IDs and devices that are scattered
across mulitple PCI buses.

Signed-off-by: Jim Snow <jim.m.snow@intel.com>
Signed-off-by: Lukasz Anaczkowski <lukasz.anaczkowski@intel.com>
---
 drivers/edac/sb_edac.c | 40 ++++++++++++++++++++++++++++++++--------
 1 file changed, 32 insertions(+), 8 deletions(-)

diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c
index 6e35405..4b51019 100644
--- a/drivers/edac/sb_edac.c
+++ b/drivers/edac/sb_edac.c
@@ -598,10 +598,19 @@ static inline int numcol(u32 mtr)
 	return 1 << cols;
 }
 
-static struct sbridge_dev *get_sbridge_dev(u8 bus)
+static struct sbridge_dev *get_sbridge_dev(u8 bus, int multi_bus)
 {
 	struct sbridge_dev *sbridge_dev;
 
+	/*
+	 * If we have devices scattered across several busses that pertain
+	 * to the same memory controller, we'll lump them all together.
+	 */
+	if (multi_bus) {
+		return list_first_entry_or_null(&sbridge_edac_list,
+				struct sbridge_dev, list);
+	}
+
 	list_for_each_entry(sbridge_dev, &sbridge_edac_list, list) {
 		if (sbridge_dev->bus == bus)
 			return sbridge_dev;
@@ -1508,7 +1517,8 @@ static void sbridge_put_all_devices(void)
 static int sbridge_get_onedevice(struct pci_dev **prev,
 				 u8 *num_mc,
 				 const struct pci_id_table *table,
-				 const unsigned devno)
+				 const unsigned devno,
+				 const int multi_bus)
 {
 	struct sbridge_dev *sbridge_dev;
 	const struct pci_id_descr *dev_descr = &table->descr[devno];
@@ -1544,7 +1554,7 @@ static int sbridge_get_onedevice(struct pci_dev **prev,
 	}
 	bus = pdev->bus->number;
 
-	sbridge_dev = get_sbridge_dev(bus);
+	sbridge_dev = get_sbridge_dev(bus, multi_bus);
 	if (!sbridge_dev) {
 		sbridge_dev = alloc_sbridge_dev(bus, table);
 		if (!sbridge_dev) {
@@ -1593,21 +1603,32 @@ static int sbridge_get_onedevice(struct pci_dev **prev,
  * @num_mc: pointer to the memory controllers count, to be incremented in case
  *	    of success.
  * @table: model specific table
+ * @allow_dups: allow for multiple devices to exist with the same device id
+ *              (as implemented, this isn't expected to work correctly in the
+ *              multi-socket case).
+ * @multi_bus: don't assume devices on different buses belong to different
+ *             memory controllers.
  *
  * returns 0 in case of success or error code
  */
-static int sbridge_get_all_devices(u8 *num_mc,
-				   const struct pci_id_table *table)
+static int sbridge_get_all_devices_full(u8 *num_mc,
+					const struct pci_id_table *table,
+					int allow_dups,
+					int multi_bus)
 {
 	int i, rc;
 	struct pci_dev *pdev = NULL;
 
 	while (table && table->descr) {
 		for (i = 0; i < table->n_devs; i++) {
-			pdev = NULL;
+			if (!allow_dups || i == 0 ||
+					table->descr[i].dev_id !=
+						table->descr[i-1].dev_id) {
+				pdev = NULL;
+			}
 			do {
 				rc = sbridge_get_onedevice(&pdev, num_mc,
-							   table, i);
+							   table, i, multi_bus);
 				if (rc < 0) {
 					if (i == 0) {
 						i = table->n_devs;
@@ -1616,7 +1637,7 @@ static int sbridge_get_all_devices(u8 *num_mc,
 					sbridge_put_all_devices();
 					return -ENODEV;
 				}
-			} while (pdev);
+			} while (pdev && !allow_dups);
 		}
 		table++;
 	}
@@ -1624,6 +1645,9 @@ static int sbridge_get_all_devices(u8 *num_mc,
 	return 0;
 }
 
+#define sbridge_get_all_devices(num_mc, table) \
+		sbridge_get_all_devices_full(num_mc, table, 0, 0)
+
 static int sbridge_mci_bind_devs(struct mem_ctl_info *mci,
 				 struct sbridge_dev *sbridge_dev)
 {
-- 
1.8.3.1

--------------------------------------------------------------------

Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.

Re: [PATCH 0/3] EDAC: add DDR4 flag, misc improvements

[Borislav Petkov]

On Tue, Jun 09, 2015 at 01:43:09PM +0200, lukasz.anaczkowski@intel.com wrote:
> From: Lukasz Anaczkowski <lukasz.anaczkowski@intel.com>
> 
> Series of patches that makes sb_edac more flexible for adding
> support for future platforms.
> 
> Jim Snow (3):
>   EDAC: add DDR4 flag
>   sb_edac: virtualize several hard-coded functions

Please add commit messages to the first two patches.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [PATCH 0/3] EDAC: add DDR4 flag, misc improvements

[Mauro Carvalho Chehab]

Hi Lukasz,

Em Tue,  9 Jun 2015 13:43:09 +0200
lukasz.anaczkowski@intel.com escreveu:

> From: Lukasz Anaczkowski <lukasz.anaczkowski@intel.com>
> 
> Series of patches that makes sb_edac more flexible for adding
> support for future platforms.
> 
> Jim Snow (3):
>   EDAC: add DDR4 flag
>   sb_edac: virtualize several hard-coded functions
>   sb_edac: support for duplicate device IDs
> 
>  drivers/edac/sb_edac.c | 95 ++++++++++++++++++++++++++++++++++++++++----------
>  include/linux/edac.h   |  6 ++--
>  2 files changed, 80 insertions(+), 21 deletions(-)

The patches look interesting, but, specially in the case of patch 3/3,
it is hard to review them without the patches that add support for
future platforms that require multi-bus. I can't, for example,
understand, so far, the need of "allow_dups" function parameter for
sbridge_get_all_devices().

So, I'd like to see this patch series together with the patches
adding support for those new "multi-bus" platforms.

Thanks,
Mauro