LKML Archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/16] amd64_edac stuff for .38
@ 2010-11-26 19:04 Borislav Petkov
  2010-11-26 19:04 ` [PATCH 01/16] amd64_edac: Remove F11h support Borislav Petkov
                   ` (15 more replies)
  0 siblings, 16 replies; 17+ messages in thread
From: Borislav Petkov @ 2010-11-26 19:04 UTC (permalink / raw
  To: norsk5; +Cc: linux-edac, linux-kernel, Borislav Petkov

From: Borislav Petkov <borislav.petkov@amd.com>

Hi,

here's my patchqueue with amd64_edac improvements/fixes for .38. What
stands out is the removal of the two-staged driver initialization.
Also, the scrubrate interface is a bit more sane now. The rest is a lot
of dead/unused code removal (diffstat says it all) in preparation for
Bulldozer support.

Also, it needs tip:amd-nb to be present on the branch before merging the
series ontop.

Borislav Petkov (16):
  amd64_edac: Remove F11h support
  amd64_edac: Use cached extended CPU model
  amd64_edac: Add per-family init function
  amd64_edac: Simplify CPU family detection
  amd64_edac: Cleanup the CPU PCI device reservation
  amd64_edac: Concentrate per-family init even more
  amd64_edac: Rename CPU PCI devices
  amd64_edac: Rework printk macros
  amd64_edac: Allocate driver instances dynamically
  amd64_edac: Remove explicit Kconfig PCI dependency
  amd64_edac: Remove PCI ECS enabling functions
  amd64_edac: Carve out ECC-related hw settings
  amd64_edac: Check ECC capabilities initially
  amd64_edac: Remove two-stage initialization
  EDAC: Fixup scrubrate manipulation
  amd64_edac: Disable DRAM ECC injection on K8

 drivers/edac/Kconfig          |    8 +-
 drivers/edac/amd64_edac.c     |  823 +++++++++++++++++------------------------
 drivers/edac/amd64_edac.h     |   86 ++---
 drivers/edac/amd64_edac_inj.c |   25 +-
 drivers/edac/cpc925_edac.c    |    9 +-
 drivers/edac/e752x_edac.c     |    8 +-
 drivers/edac/edac_core.h      |    5 +-
 drivers/edac/edac_mc.c        |    4 +-
 drivers/edac/edac_mc_sysfs.c  |   57 ++--
 drivers/edac/i5100_edac.c     |    9 +-
 10 files changed, 436 insertions(+), 598 deletions(-)

-- 
1.7.3.1.50.g1e633


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 01/16] amd64_edac: Remove F11h support
  2010-11-26 19:04 [PATCH 00/16] amd64_edac stuff for .38 Borislav Petkov
@ 2010-11-26 19:04 ` Borislav Petkov
  2010-11-26 19:04 ` [PATCH 02/16] amd64_edac: Use cached extended CPU model Borislav Petkov
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Borislav Petkov @ 2010-11-26 19:04 UTC (permalink / raw
  To: norsk5; +Cc: linux-edac, linux-kernel, Borislav Petkov

From: Borislav Petkov <borislav.petkov@amd.com>

F11h doesn't support DRAM ECC so whack it away.

Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
---
 drivers/edac/amd64_edac.c |   50 ++------------------------------------------
 drivers/edac/amd64_edac.h |    2 -
 2 files changed, 3 insertions(+), 49 deletions(-)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 774f950..b7bde66 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -172,9 +172,6 @@ static int amd64_set_scrub_rate(struct mem_ctl_info *mci, u32 bandwidth)
 	case 0x10:
 		min_scrubrate = F10_MIN_SCRUB_RATE_BITS;
 		break;
-	case 0x11:
-		min_scrubrate = F11_MIN_SCRUB_RATE_BITS;
-		break;
 
 	default:
 		amd64_printk(KERN_ERR, "Unsupported family!\n");
@@ -803,9 +800,7 @@ static u16 extract_syndrome(struct err_regs *err)
 
 static void amd64_cpu_display_info(struct amd64_pvt *pvt)
 {
-	if (boot_cpu_data.x86 == 0x11)
-		edac_printk(KERN_DEBUG, EDAC_MC, "F11h CPU detected\n");
-	else if (boot_cpu_data.x86 == 0x10)
+	if (boot_cpu_data.x86 == 0x10)
 		edac_printk(KERN_DEBUG, EDAC_MC, "F10h CPU detected\n");
 	else if (boot_cpu_data.x86 == 0xf)
 		edac_printk(KERN_DEBUG, EDAC_MC, "%s detected\n",
@@ -965,14 +960,8 @@ static void amd64_set_dct_base_and_mask(struct amd64_pvt *pvt)
 		pvt->dcsm_mask		= REV_F_F1Xh_DCSM_MASK_BITS;
 		pvt->dcs_mask_notused	= REV_F_F1Xh_DCS_NOTUSED_BITS;
 		pvt->dcs_shift		= REV_F_F1Xh_DCS_SHIFT;
-
-		if (boot_cpu_data.x86 == 0x11) {
-			pvt->cs_count = 4;
-			pvt->num_dcsm = 2;
-		} else {
-			pvt->cs_count = 8;
-			pvt->num_dcsm = 4;
-		}
+		pvt->cs_count		= 8;
+		pvt->num_dcsm		= 4;
 	}
 }
 
@@ -1744,17 +1733,6 @@ static void amd64_debug_display_dimm_sizes(int ctrl, struct amd64_pvt *pvt)
 	}
 }
 
-/*
- * There currently are 3 types type of MC devices for AMD Athlon/Opterons
- * (as per PCI DEVICE_IDs):
- *
- * Family K8: That is the Athlon64 and Opteron CPUs. They all have the same PCI
- * DEVICE ID, even though there is differences between the different Revisions
- * (CG,D,E,F).
- *
- * Family F10h and F11h.
- *
- */
 static struct amd64_family_type amd64_family_types[] = {
 	[K8_CPUS] = {
 		.ctl_name = "RevF",
@@ -1781,19 +1759,6 @@ static struct amd64_family_type amd64_family_types[] = {
 			.dbam_to_cs		= f10_dbam_to_chip_select,
 		}
 	},
-	[F11_CPUS] = {
-		.ctl_name = "Family 11h",
-		.addr_f1_ctl = PCI_DEVICE_ID_AMD_11H_NB_MAP,
-		.misc_f3_ctl = PCI_DEVICE_ID_AMD_11H_NB_MISC,
-		.ops = {
-			.early_channel_count	= f10_early_channel_count,
-			.get_error_address	= f10_get_error_address,
-			.read_dram_base_limit	= f10_read_dram_base_limit,
-			.read_dram_ctl_register	= f10_read_dram_ctl_register,
-			.map_sysaddr_to_csrow	= f10_map_sysaddr_to_csrow,
-			.dbam_to_cs		= f10_dbam_to_chip_select,
-		}
-	},
 };
 
 static struct pci_dev *pci_get_related_function(unsigned int vendor,
@@ -2862,15 +2827,6 @@ static const struct pci_device_id amd64_pci_table[] __devinitdata = {
 		.class_mask	= 0,
 		.driver_data	= F10_CPUS
 	},
-	{
-		.vendor		= PCI_VENDOR_ID_AMD,
-		.device		= PCI_DEVICE_ID_AMD_11H_NB_DRAM,
-		.subvendor	= PCI_ANY_ID,
-		.subdevice	= PCI_ANY_ID,
-		.class		= 0,
-		.class_mask	= 0,
-		.driver_data	= F11_CPUS
-	},
 	{0, }
 };
 MODULE_DEVICE_TABLE(pci, amd64_pci_table);
diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
index 044aee4..c8f2734 100644
--- a/drivers/edac/amd64_edac.h
+++ b/drivers/edac/amd64_edac.h
@@ -373,7 +373,6 @@ static inline int get_node_id(struct pci_dev *pdev)
 enum amd64_chipset_families {
 	K8_CPUS = 0,
 	F10_CPUS,
-	F11_CPUS,
 };
 
 /* Error injection control structure */
@@ -556,7 +555,6 @@ static inline int amd64_read_pci_cfg_dword(struct pci_dev *pdev, int offset,
  */
 #define K8_MIN_SCRUB_RATE_BITS	0x0
 #define F10_MIN_SCRUB_RATE_BITS	0x5
-#define F11_MIN_SCRUB_RATE_BITS	0x6
 
 int amd64_get_dram_hole_info(struct mem_ctl_info *mci, u64 *hole_base,
 			     u64 *hole_offset, u64 *hole_size);
-- 
1.7.3.1.50.g1e633


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 02/16] amd64_edac: Use cached extended CPU model
  2010-11-26 19:04 [PATCH 00/16] amd64_edac stuff for .38 Borislav Petkov
  2010-11-26 19:04 ` [PATCH 01/16] amd64_edac: Remove F11h support Borislav Petkov
@ 2010-11-26 19:04 ` Borislav Petkov
  2010-11-26 19:04 ` [PATCH 03/16] amd64_edac: Add per-family init function Borislav Petkov
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Borislav Petkov @ 2010-11-26 19:04 UTC (permalink / raw
  To: norsk5; +Cc: linux-edac, linux-kernel, Borislav Petkov

From: Borislav Petkov <borislav.petkov@amd.com>

... instead of computing it needlessly again.

Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
---
 drivers/edac/amd64_edac.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index b7bde66..7f3609d 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -1046,13 +1046,12 @@ static int k8_early_channel_count(struct amd64_pvt *pvt)
 	if (err)
 		return err;
 
-	if ((boot_cpu_data.x86_model >> 4) >= K8_REV_F) {
+	if (pvt->ext_model >= K8_REV_F)
 		/* RevF (NPT) and later */
 		flag = pvt->dclr0 & F10_WIDTH_128;
-	} else {
+	else
 		/* RevE and earlier */
 		flag = pvt->dclr0 & REVE_WIDTH_128;
-	}
 
 	/* not used */
 	pvt->dclr1 = 0;
-- 
1.7.3.1.50.g1e633


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 03/16] amd64_edac: Add per-family init function
  2010-11-26 19:04 [PATCH 00/16] amd64_edac stuff for .38 Borislav Petkov
  2010-11-26 19:04 ` [PATCH 01/16] amd64_edac: Remove F11h support Borislav Petkov
  2010-11-26 19:04 ` [PATCH 02/16] amd64_edac: Use cached extended CPU model Borislav Petkov
@ 2010-11-26 19:04 ` Borislav Petkov
  2010-11-26 19:04 ` [PATCH 04/16] amd64_edac: Simplify CPU family detection Borislav Petkov
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Borislav Petkov @ 2010-11-26 19:04 UTC (permalink / raw
  To: norsk5; +Cc: linux-edac, linux-kernel, Borislav Petkov

From: Borislav Petkov <borislav.petkov@amd.com>

Run a per-family init function which does all the settings based on
the family this driver instance is running on. Move the scrubrate
calculation in it and simplify code.

Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
---
 drivers/edac/amd64_edac.c |   44 +++++++++++++++++++++++++-------------------
 drivers/edac/amd64_edac.h |    3 +++
 2 files changed, 28 insertions(+), 19 deletions(-)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 7f3609d..bb9a342 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -117,8 +117,7 @@ struct scrubrate scrubrates[] = {
  * scan the scrub rate mapping table for a close or matching bandwidth value to
  * issue. If requested is too big, then use last maximum value found.
  */
-static int amd64_search_set_scrub_rate(struct pci_dev *ctl, u32 new_bw,
-				       u32 min_scrubrate)
+static int __amd64_set_scrub_rate(struct pci_dev *ctl, u32 new_bw, u32 min_rate)
 {
 	u32 scrubval;
 	int i;
@@ -134,7 +133,7 @@ static int amd64_search_set_scrub_rate(struct pci_dev *ctl, u32 new_bw,
 		 * skip scrub rates which aren't recommended
 		 * (see F10 BKDG, F3x58)
 		 */
-		if (scrubrates[i].scrubval < min_scrubrate)
+		if (scrubrates[i].scrubval < min_rate)
 			continue;
 
 		if (scrubrates[i].bandwidth <= new_bw)
@@ -160,25 +159,11 @@ static int amd64_search_set_scrub_rate(struct pci_dev *ctl, u32 new_bw,
 	return 0;
 }
 
-static int amd64_set_scrub_rate(struct mem_ctl_info *mci, u32 bandwidth)
+static int amd64_set_scrub_rate(struct mem_ctl_info *mci, u32 bw)
 {
 	struct amd64_pvt *pvt = mci->pvt_info;
-	u32 min_scrubrate = 0x0;
 
-	switch (boot_cpu_data.x86) {
-	case 0xf:
-		min_scrubrate = K8_MIN_SCRUB_RATE_BITS;
-		break;
-	case 0x10:
-		min_scrubrate = F10_MIN_SCRUB_RATE_BITS;
-		break;
-
-	default:
-		amd64_printk(KERN_ERR, "Unsupported family!\n");
-		return -EINVAL;
-	}
-	return amd64_search_set_scrub_rate(pvt->misc_f3_ctl, bandwidth,
-					   min_scrubrate);
+	return __amd64_set_scrub_rate(pvt->misc_f3_ctl, bw, pvt->min_scrubrate);
 }
 
 static int amd64_get_scrub_rate(struct mem_ctl_info *mci, u32 *bw)
@@ -2607,6 +2592,23 @@ static void amd64_setup_mci_misc_attributes(struct mem_ctl_info *mci)
 	mci->get_sdram_scrub_rate = amd64_get_scrub_rate;
 }
 
+static int amd64_per_family_init(struct amd64_pvt *pvt)
+{
+	switch (boot_cpu_data.x86) {
+	case 0xf:
+		pvt->min_scrubrate = K8_MIN_SCRUB_RATE_BITS;
+		break;
+	case 0x10:
+		pvt->min_scrubrate = F10_MIN_SCRUB_RATE_BITS;
+		break;
+
+	default:
+		amd64_printk(KERN_ERR, "Unsupported family!\n");
+		return -EINVAL;
+	}
+	return 0;
+}
+
 /*
  * Init stuff for this DRAM Controller device.
  *
@@ -2637,6 +2639,10 @@ static int amd64_probe_one_instance(struct pci_dev *dram_f2_ctl,
 	pvt->mc_type_index	= mc_type_index;
 	pvt->ops		= family_ops(mc_type_index);
 
+	ret = -EINVAL;
+	if (amd64_per_family_init(pvt))
+		goto err_free;
+
 	/*
 	 * We have the dram_f2_ctl device as an argument, now go reserve its
 	 * sibling devices from the PCI system.
diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
index c8f2734..e5204fe 100644
--- a/drivers/edac/amd64_edac.h
+++ b/drivers/edac/amd64_edac.h
@@ -461,6 +461,9 @@ struct amd64_pvt {
 	/* MC Type Index value: socket F vs Family 10h */
 	u32 mc_type_index;
 
+	/* DCT per-family scrubrate setting */
+	u32 min_scrubrate;
+
 	/* misc settings */
 	struct flags {
 		unsigned long cf8_extcfg:1;
-- 
1.7.3.1.50.g1e633


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 04/16] amd64_edac: Simplify CPU family detection
  2010-11-26 19:04 [PATCH 00/16] amd64_edac stuff for .38 Borislav Petkov
                   ` (2 preceding siblings ...)
  2010-11-26 19:04 ` [PATCH 03/16] amd64_edac: Add per-family init function Borislav Petkov
@ 2010-11-26 19:04 ` Borislav Petkov
  2010-11-26 19:04 ` [PATCH 05/16] amd64_edac: Cleanup the CPU PCI device reservation Borislav Petkov
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Borislav Petkov @ 2010-11-26 19:04 UTC (permalink / raw
  To: norsk5; +Cc: linux-edac, linux-kernel, Borislav Petkov

From: Borislav Petkov <borislav.petkov@amd.com>

Concentrate CPU family detection in the per-family init function.

Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
---
 drivers/edac/amd64_edac.c |   56 +++++++++++++++++++++++---------------------
 drivers/edac/amd64_edac.h |    8 ++----
 2 files changed, 32 insertions(+), 32 deletions(-)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index bb9a342..a59c1da 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -783,19 +783,6 @@ static u16 extract_syndrome(struct err_regs *err)
 	return ((err->nbsh >> 15) & 0xff) | ((err->nbsl >> 16) & 0xff00);
 }
 
-static void amd64_cpu_display_info(struct amd64_pvt *pvt)
-{
-	if (boot_cpu_data.x86 == 0x10)
-		edac_printk(KERN_DEBUG, EDAC_MC, "F10h CPU detected\n");
-	else if (boot_cpu_data.x86 == 0xf)
-		edac_printk(KERN_DEBUG, EDAC_MC, "%s detected\n",
-			(pvt->ext_model >= K8_REV_F) ?
-			"Rev F or later" : "Rev E or earlier");
-	else
-		/* we'll hardly ever ever get here */
-		edac_printk(KERN_ERR, EDAC_MC, "Unknown cpu!\n");
-}
-
 /*
  * Determine if the DIMMs have ECC enabled. ECC is enabled ONLY if all the DIMMs
  * are ECC capable.
@@ -1719,7 +1706,7 @@ static void amd64_debug_display_dimm_sizes(int ctrl, struct amd64_pvt *pvt)
 
 static struct amd64_family_type amd64_family_types[] = {
 	[K8_CPUS] = {
-		.ctl_name = "RevF",
+		.ctl_name = "K8",
 		.addr_f1_ctl = PCI_DEVICE_ID_AMD_K8_NB_ADDRMAP,
 		.misc_f3_ctl = PCI_DEVICE_ID_AMD_K8_NB_MISC,
 		.ops = {
@@ -1731,7 +1718,7 @@ static struct amd64_family_type amd64_family_types[] = {
 		}
 	},
 	[F10_CPUS] = {
-		.ctl_name = "Family 10h",
+		.ctl_name = "F10h",
 		.addr_f1_ctl = PCI_DEVICE_ID_AMD_10H_NB_MAP,
 		.misc_f3_ctl = PCI_DEVICE_ID_AMD_10H_NB_MISC,
 		.ops = {
@@ -2137,8 +2124,6 @@ static void amd64_read_mc_registers(struct amd64_pvt *pvt)
 	} else
 		debugf0("  TOP_MEM2 disabled.\n");
 
-	amd64_cpu_display_info(pvt);
-
 	amd64_read_pci_cfg(pvt->misc_f3_ctl, K8_NBCAP, &pvt->nbcap);
 
 	if (pvt->ops->read_dram_ctl_register)
@@ -2583,7 +2568,7 @@ static void amd64_setup_mci_misc_attributes(struct mem_ctl_info *mci)
 	mci->edac_cap		= amd64_determine_edac_cap(pvt);
 	mci->mod_name		= EDAC_MOD_STR;
 	mci->mod_ver		= EDAC_AMD64_VERSION;
-	mci->ctl_name		= get_amd_family_name(pvt->mc_type_index);
+	mci->ctl_name		= pvt->ctl_name;
 	mci->dev_name		= pci_name(pvt->dram_f2_ctl);
 	mci->ctl_page_to_phys	= NULL;
 
@@ -2592,21 +2577,37 @@ static void amd64_setup_mci_misc_attributes(struct mem_ctl_info *mci)
 	mci->get_sdram_scrub_rate = amd64_get_scrub_rate;
 }
 
-static int amd64_per_family_init(struct amd64_pvt *pvt)
+/*
+ * returns a pointer to the family descriptor on success, NULL otherwise.
+ */
+static struct amd64_family_type *amd64_per_family_init(struct amd64_pvt *pvt)
 {
-	switch (boot_cpu_data.x86) {
+	u8 fam = boot_cpu_data.x86;
+	struct amd64_family_type *fam_type = NULL;
+
+	switch (fam) {
 	case 0xf:
-		pvt->min_scrubrate = K8_MIN_SCRUB_RATE_BITS;
+		fam_type		= &amd64_family_types[K8_CPUS];
+		pvt->ctl_name		= fam_type->ctl_name;
+		pvt->min_scrubrate	= K8_MIN_SCRUB_RATE_BITS;
 		break;
 	case 0x10:
-		pvt->min_scrubrate = F10_MIN_SCRUB_RATE_BITS;
+		fam_type		= &amd64_family_types[F10_CPUS];
+		pvt->ctl_name		= fam_type->ctl_name;
+		pvt->min_scrubrate	= F10_MIN_SCRUB_RATE_BITS;
 		break;
 
 	default:
 		amd64_printk(KERN_ERR, "Unsupported family!\n");
-		return -EINVAL;
+		return NULL;
 	}
-	return 0;
+
+	amd64_printk(KERN_INFO, "%s %s detected.\n", pvt->ctl_name,
+		     (fam == 0xf ?
+				(pvt->ext_model >= K8_REV_F  ? "revF or later"
+							     : "revE or earlier")
+				 : ""));
+	return fam_type;
 }
 
 /*
@@ -2625,6 +2626,7 @@ static int amd64_probe_one_instance(struct pci_dev *dram_f2_ctl,
 				    int mc_type_index)
 {
 	struct amd64_pvt *pvt = NULL;
+	struct amd64_family_type *fam_type = NULL;
 	int err = 0, ret;
 
 	ret = -ENOMEM;
@@ -2640,7 +2642,8 @@ static int amd64_probe_one_instance(struct pci_dev *dram_f2_ctl,
 	pvt->ops		= family_ops(mc_type_index);
 
 	ret = -EINVAL;
-	if (amd64_per_family_init(pvt))
+	fam_type = amd64_per_family_init(pvt);
+	if (!fam_type)
 		goto err_free;
 
 	/*
@@ -2762,8 +2765,7 @@ static int __devinit amd64_init_one_instance(struct pci_dev *pdev,
 {
 	int ret = 0;
 
-	debugf0("(MC node=%d,mc_type='%s')\n", get_node_id(pdev),
-		get_amd_family_name(mc_type->driver_data));
+	debugf0("(MC node=%d)\n", get_node_id(pdev));
 
 	ret = pci_enable_device(pdev);
 	if (ret < 0)
diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
index e5204fe..064e0d6 100644
--- a/drivers/edac/amd64_edac.h
+++ b/drivers/edac/amd64_edac.h
@@ -464,6 +464,9 @@ struct amd64_pvt {
 	/* DCT per-family scrubrate setting */
 	u32 min_scrubrate;
 
+	/* family name this instance is running on */
+	const char *ctl_name;
+
 	/* misc settings */
 	struct flags {
 		unsigned long cf8_extcfg:1;
@@ -526,11 +529,6 @@ struct amd64_family_type {
 
 static struct amd64_family_type amd64_family_types[];
 
-static inline const char *get_amd_family_name(int index)
-{
-	return amd64_family_types[index].ctl_name;
-}
-
 static inline struct low_ops *family_ops(int index)
 {
 	return &amd64_family_types[index].ops;
-- 
1.7.3.1.50.g1e633


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 05/16] amd64_edac: Cleanup the CPU PCI device reservation
  2010-11-26 19:04 [PATCH 00/16] amd64_edac stuff for .38 Borislav Petkov
                   ` (3 preceding siblings ...)
  2010-11-26 19:04 ` [PATCH 04/16] amd64_edac: Simplify CPU family detection Borislav Petkov
@ 2010-11-26 19:04 ` Borislav Petkov
  2010-11-26 19:04 ` [PATCH 06/16] amd64_edac: Concentrate per-family init even more Borislav Petkov
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Borislav Petkov @ 2010-11-26 19:04 UTC (permalink / raw
  To: norsk5; +Cc: linux-edac, linux-kernel, Borislav Petkov

From: Borislav Petkov <borislav.petkov@amd.com>

Shorten code and clarify comments, return proper -E* values on error.

Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
---
 drivers/edac/amd64_edac.c |   42 ++++++++++++------------------------------
 1 files changed, 12 insertions(+), 30 deletions(-)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index a59c1da..3edaf4d 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -2035,42 +2035,27 @@ void amd64_decode_bus_error(int node_id, struct mce *m, u32 nbcfg)
 }
 
 /*
- * Input:
- *	1) struct amd64_pvt which contains pvt->dram_f2_ctl pointer
- *	2) AMD Family index value
- *
- * Ouput:
- *	Upon return of 0, the following filled in:
- *
- *		struct pvt->addr_f1_ctl
- *		struct pvt->misc_f3_ctl
- *
- *	Filled in with related device funcitions of 'dram_f2_ctl'
- *	These devices are "reserved" via the pci_get_device()
- *
- *	Upon return of 1 (error status):
- *
- *		Nothing reserved
+ * Use pvt->dram_f2_ctl which contains the F2 CPU PCI device to get the related
+ * F1 (AddrMap) and F3 (Misc) devices. Return negative value on error.
  */
-static int amd64_reserve_mc_sibling_devices(struct amd64_pvt *pvt, int mc_idx)
+static int amd64_reserve_mc_sibling_devices(struct amd64_pvt *pvt, u16 f1_id,
+					    u16 f3_id)
 {
-	const struct amd64_family_type *amd64_dev = &amd64_family_types[mc_idx];
-
 	/* Reserve the ADDRESS MAP Device */
 	pvt->addr_f1_ctl = pci_get_related_function(pvt->dram_f2_ctl->vendor,
-						    amd64_dev->addr_f1_ctl,
+						    f1_id,
 						    pvt->dram_f2_ctl);
 
 	if (!pvt->addr_f1_ctl) {
 		amd64_printk(KERN_ERR, "error address map device not found: "
 			     "vendor %x device 0x%x (broken BIOS?)\n",
-			     PCI_VENDOR_ID_AMD, amd64_dev->addr_f1_ctl);
-		return 1;
+			     PCI_VENDOR_ID_AMD, f1_id);
+		return -ENODEV;
 	}
 
 	/* Reserve the MISC Device */
 	pvt->misc_f3_ctl = pci_get_related_function(pvt->dram_f2_ctl->vendor,
-						    amd64_dev->misc_f3_ctl,
+						    f3_id,
 						    pvt->dram_f2_ctl);
 
 	if (!pvt->misc_f3_ctl) {
@@ -2079,8 +2064,8 @@ static int amd64_reserve_mc_sibling_devices(struct amd64_pvt *pvt, int mc_idx)
 
 		amd64_printk(KERN_ERR, "error miscellaneous device not found: "
 			     "vendor %x device 0x%x (broken BIOS?)\n",
-			     PCI_VENDOR_ID_AMD, amd64_dev->misc_f3_ctl);
-		return 1;
+			     PCI_VENDOR_ID_AMD, f3_id);
+		return -ENODEV;
 	}
 
 	debugf1("    Addr Map device PCI Bus ID:\t%s\n",
@@ -2646,12 +2631,9 @@ static int amd64_probe_one_instance(struct pci_dev *dram_f2_ctl,
 	if (!fam_type)
 		goto err_free;
 
-	/*
-	 * We have the dram_f2_ctl device as an argument, now go reserve its
-	 * sibling devices from the PCI system.
-	 */
 	ret = -ENODEV;
-	err = amd64_reserve_mc_sibling_devices(pvt, mc_type_index);
+	err = amd64_reserve_mc_sibling_devices(pvt, fam_type->addr_f1_ctl,
+					       fam_type->misc_f3_ctl);
 	if (err)
 		goto err_free;
 
-- 
1.7.3.1.50.g1e633


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 06/16] amd64_edac: Concentrate per-family init even more
  2010-11-26 19:04 [PATCH 00/16] amd64_edac stuff for .38 Borislav Petkov
                   ` (4 preceding siblings ...)
  2010-11-26 19:04 ` [PATCH 05/16] amd64_edac: Cleanup the CPU PCI device reservation Borislav Petkov
@ 2010-11-26 19:04 ` Borislav Petkov
  2010-11-26 19:04 ` [PATCH 07/16] amd64_edac: Rename CPU PCI devices Borislav Petkov
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Borislav Petkov @ 2010-11-26 19:04 UTC (permalink / raw
  To: norsk5; +Cc: linux-edac, linux-kernel, Borislav Petkov

From: Borislav Petkov <borislav.petkov@amd.com>

Move the remaining per-family init code into the proper place and
simplify the rest of the initialization. Reorganize error handling in
amd64_init_one_instance().

Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
---
 drivers/edac/amd64_edac.c |   31 +++++++++++++++----------------
 drivers/edac/amd64_edac.h |   15 ++-------------
 2 files changed, 17 insertions(+), 29 deletions(-)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 3edaf4d..9370812 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -2573,11 +2573,13 @@ static struct amd64_family_type *amd64_per_family_init(struct amd64_pvt *pvt)
 	switch (fam) {
 	case 0xf:
 		fam_type		= &amd64_family_types[K8_CPUS];
+		pvt->ops		= &amd64_family_types[K8_CPUS].ops;
 		pvt->ctl_name		= fam_type->ctl_name;
 		pvt->min_scrubrate	= K8_MIN_SCRUB_RATE_BITS;
 		break;
 	case 0x10:
 		fam_type		= &amd64_family_types[F10_CPUS];
+		pvt->ops		= &amd64_family_types[F10_CPUS].ops;
 		pvt->ctl_name		= fam_type->ctl_name;
 		pvt->min_scrubrate	= F10_MIN_SCRUB_RATE_BITS;
 		break;
@@ -2587,6 +2589,8 @@ static struct amd64_family_type *amd64_per_family_init(struct amd64_pvt *pvt)
 		return NULL;
 	}
 
+	pvt->ext_model = boot_cpu_data.x86_model >> 4;
+
 	amd64_printk(KERN_INFO, "%s %s detected.\n", pvt->ctl_name,
 		     (fam == 0xf ?
 				(pvt->ext_model >= K8_REV_F  ? "revF or later"
@@ -2607,8 +2611,7 @@ static struct amd64_family_type *amd64_per_family_init(struct amd64_pvt *pvt)
  * later come back in a finish-setup function to perform that final
  * initialization. See also amd64_init_2nd_stage() for that.
  */
-static int amd64_probe_one_instance(struct pci_dev *dram_f2_ctl,
-				    int mc_type_index)
+static int amd64_probe_one_instance(struct pci_dev *dram_f2_ctl)
 {
 	struct amd64_pvt *pvt = NULL;
 	struct amd64_family_type *fam_type = NULL;
@@ -2619,12 +2622,8 @@ static int amd64_probe_one_instance(struct pci_dev *dram_f2_ctl,
 	if (!pvt)
 		goto err_exit;
 
-	pvt->mc_node_id = get_node_id(dram_f2_ctl);
-
-	pvt->dram_f2_ctl	= dram_f2_ctl;
-	pvt->ext_model		= boot_cpu_data.x86_model >> 4;
-	pvt->mc_type_index	= mc_type_index;
-	pvt->ops		= family_ops(mc_type_index);
+	pvt->mc_node_id	 = get_node_id(dram_f2_ctl);
+	pvt->dram_f2_ctl = dram_f2_ctl;
 
 	ret = -EINVAL;
 	fam_type = amd64_per_family_init(pvt);
@@ -2743,20 +2742,22 @@ err_exit:
 
 
 static int __devinit amd64_init_one_instance(struct pci_dev *pdev,
-				 const struct pci_device_id *mc_type)
+					     const struct pci_device_id *mc_type)
 {
 	int ret = 0;
 
 	debugf0("(MC node=%d)\n", get_node_id(pdev));
 
 	ret = pci_enable_device(pdev);
-	if (ret < 0)
-		ret = -EIO;
-	else
-		ret = amd64_probe_one_instance(pdev, mc_type->driver_data);
+	if (ret < 0) {
+		debugf0("ret=%d\n", ret);
+		return -EIO;
+	}
 
+	ret = amd64_probe_one_instance(pdev);
 	if (ret < 0)
-		debugf0("ret=%d\n", ret);
+		amd64_printk(KERN_ERR, "Error probing instance: %d\n",
+					get_node_id(pdev));
 
 	return ret;
 }
@@ -2805,7 +2806,6 @@ static const struct pci_device_id amd64_pci_table[] __devinitdata = {
 		.subdevice	= PCI_ANY_ID,
 		.class		= 0,
 		.class_mask	= 0,
-		.driver_data	= K8_CPUS
 	},
 	{
 		.vendor		= PCI_VENDOR_ID_AMD,
@@ -2814,7 +2814,6 @@ static const struct pci_device_id amd64_pci_table[] __devinitdata = {
 		.subdevice	= PCI_ANY_ID,
 		.class		= 0,
 		.class_mask	= 0,
-		.driver_data	= F10_CPUS
 	},
 	{0, }
 };
diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
index 064e0d6..007b68a 100644
--- a/drivers/edac/amd64_edac.h
+++ b/drivers/edac/amd64_edac.h
@@ -383,6 +383,8 @@ struct error_injection {
 };
 
 struct amd64_pvt {
+	struct low_ops *ops;
+
 	/* pci_device handles which we utilize */
 	struct pci_dev *addr_f1_ctl;
 	struct pci_dev *dram_f2_ctl;
@@ -390,9 +392,6 @@ struct amd64_pvt {
 
 	int mc_node_id;		/* MC index of this MC node */
 	int ext_model;		/* extended model value of this node */
-
-	struct low_ops *ops;	/* pointer to per PCI Device ID func table */
-
 	int channel_count;
 
 	/* Raw registers */
@@ -458,9 +457,6 @@ struct amd64_pvt {
 	u32 nbctl_mcgctl_saved;		/* When true, following 2 are valid */
 	u32 old_nbctl;
 
-	/* MC Type Index value: socket F vs Family 10h */
-	u32 mc_type_index;
-
 	/* DCT per-family scrubrate setting */
 	u32 min_scrubrate;
 
@@ -527,13 +523,6 @@ struct amd64_family_type {
 	struct low_ops ops;
 };
 
-static struct amd64_family_type amd64_family_types[];
-
-static inline struct low_ops *family_ops(int index)
-{
-	return &amd64_family_types[index].ops;
-}
-
 static inline int amd64_read_pci_cfg_dword(struct pci_dev *pdev, int offset,
 					   u32 *val, const char *func)
 {
-- 
1.7.3.1.50.g1e633


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 07/16] amd64_edac: Rename CPU PCI devices
  2010-11-26 19:04 [PATCH 00/16] amd64_edac stuff for .38 Borislav Petkov
                   ` (5 preceding siblings ...)
  2010-11-26 19:04 ` [PATCH 06/16] amd64_edac: Concentrate per-family init even more Borislav Petkov
@ 2010-11-26 19:04 ` Borislav Petkov
  2010-11-26 19:04 ` [PATCH 08/16] amd64_edac: Rework printk macros Borislav Petkov
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Borislav Petkov @ 2010-11-26 19:04 UTC (permalink / raw
  To: norsk5; +Cc: linux-edac, linux-kernel, Borislav Petkov

From: Borislav Petkov <borislav.petkov@amd.com>

Rename variables representing PCI devices to their BKDG names for faster
search and shorter, clearer code.

Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
---
 drivers/edac/amd64_edac.c     |  153 +++++++++++++++++++----------------------
 drivers/edac/amd64_edac.h     |    7 +--
 drivers/edac/amd64_edac_inj.c |   12 +--
 3 files changed, 77 insertions(+), 95 deletions(-)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 9370812..aa57853 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -163,7 +163,7 @@ static int amd64_set_scrub_rate(struct mem_ctl_info *mci, u32 bw)
 {
 	struct amd64_pvt *pvt = mci->pvt_info;
 
-	return __amd64_set_scrub_rate(pvt->misc_f3_ctl, bw, pvt->min_scrubrate);
+	return __amd64_set_scrub_rate(pvt->F3, bw, pvt->min_scrubrate);
 }
 
 static int amd64_get_scrub_rate(struct mem_ctl_info *mci, u32 *bw)
@@ -172,7 +172,7 @@ static int amd64_get_scrub_rate(struct mem_ctl_info *mci, u32 *bw)
 	u32 scrubval = 0;
 	int status = -1, i;
 
-	amd64_read_pci_cfg(pvt->misc_f3_ctl, K8_SCRCTRL, &scrubval);
+	amd64_read_pci_cfg(pvt->F3, K8_SCRCTRL, &scrubval);
 
 	scrubval = scrubval & 0x001F;
 
@@ -882,10 +882,10 @@ static void amd64_dump_misc_regs(struct amd64_pvt *pvt)
 /* Read in both of DBAM registers */
 static void amd64_read_dbam_reg(struct amd64_pvt *pvt)
 {
-	amd64_read_pci_cfg(pvt->dram_f2_ctl, DBAM0, &pvt->dbam0);
+	amd64_read_pci_cfg(pvt->F2, DBAM0, &pvt->dbam0);
 
 	if (boot_cpu_data.x86 >= 0x10)
-		amd64_read_pci_cfg(pvt->dram_f2_ctl, DBAM1, &pvt->dbam1);
+		amd64_read_pci_cfg(pvt->F2, DBAM1, &pvt->dbam1);
 }
 
 /*
@@ -948,14 +948,14 @@ static void amd64_read_dct_base_mask(struct amd64_pvt *pvt)
 
 	for (cs = 0; cs < pvt->cs_count; cs++) {
 		reg = K8_DCSB0 + (cs * 4);
-		if (!amd64_read_pci_cfg(pvt->dram_f2_ctl, reg, &pvt->dcsb0[cs]))
+		if (!amd64_read_pci_cfg(pvt->F2, reg, &pvt->dcsb0[cs]))
 			debugf0("  DCSB0[%d]=0x%08x reg: F2x%x\n",
 				cs, pvt->dcsb0[cs], reg);
 
 		/* If DCT are NOT ganged, then read in DCT1's base */
 		if (boot_cpu_data.x86 >= 0x10 && !dct_ganging_enabled(pvt)) {
 			reg = F10_DCSB1 + (cs * 4);
-			if (!amd64_read_pci_cfg(pvt->dram_f2_ctl, reg,
+			if (!amd64_read_pci_cfg(pvt->F2, reg,
 						&pvt->dcsb1[cs]))
 				debugf0("  DCSB1[%d]=0x%08x reg: F2x%x\n",
 					cs, pvt->dcsb1[cs], reg);
@@ -966,14 +966,14 @@ static void amd64_read_dct_base_mask(struct amd64_pvt *pvt)
 
 	for (cs = 0; cs < pvt->num_dcsm; cs++) {
 		reg = K8_DCSM0 + (cs * 4);
-		if (!amd64_read_pci_cfg(pvt->dram_f2_ctl, reg, &pvt->dcsm0[cs]))
+		if (!amd64_read_pci_cfg(pvt->F2, reg, &pvt->dcsm0[cs]))
 			debugf0("    DCSM0[%d]=0x%08x reg: F2x%x\n",
 				cs, pvt->dcsm0[cs], reg);
 
 		/* If DCT are NOT ganged, then read in DCT1's mask */
 		if (boot_cpu_data.x86 >= 0x10 && !dct_ganging_enabled(pvt)) {
 			reg = F10_DCSM1 + (cs * 4);
-			if (!amd64_read_pci_cfg(pvt->dram_f2_ctl, reg,
+			if (!amd64_read_pci_cfg(pvt->F2, reg,
 						&pvt->dcsm1[cs]))
 				debugf0("    DCSM1[%d]=0x%08x reg: F2x%x\n",
 					cs, pvt->dcsm1[cs], reg);
@@ -1014,7 +1014,7 @@ static int k8_early_channel_count(struct amd64_pvt *pvt)
 {
 	int flag, err = 0;
 
-	err = amd64_read_pci_cfg(pvt->dram_f2_ctl, F10_DCLR_0, &pvt->dclr0);
+	err = amd64_read_pci_cfg(pvt->F2, F10_DCLR_0, &pvt->dclr0);
 	if (err)
 		return err;
 
@@ -1050,14 +1050,14 @@ static void k8_read_dram_base_limit(struct amd64_pvt *pvt, int dram)
 	u32 low;
 	u32 off = dram << 3;	/* 8 bytes between DRAM entries */
 
-	amd64_read_pci_cfg(pvt->addr_f1_ctl, K8_DRAM_BASE_LOW + off, &low);
+	amd64_read_pci_cfg(pvt->F1, K8_DRAM_BASE_LOW + off, &low);
 
 	/* Extract parts into separate data entries */
 	pvt->dram_base[dram] = ((u64) low & 0xFFFF0000) << 8;
 	pvt->dram_IntlvEn[dram] = (low >> 8) & 0x7;
 	pvt->dram_rw_en[dram] = (low & 0x3);
 
-	amd64_read_pci_cfg(pvt->addr_f1_ctl, K8_DRAM_LIMIT_LOW + off, &low);
+	amd64_read_pci_cfg(pvt->F1, K8_DRAM_LIMIT_LOW + off, &low);
 
 	/*
 	 * Extract parts into separate data entries. Limit is the HIGHEST memory
@@ -1180,7 +1180,7 @@ static int f10_early_channel_count(struct amd64_pvt *pvt)
 	 * both controllers since DIMMs can be placed in either one.
 	 */
 	for (i = 0; i < ARRAY_SIZE(dbams); i++) {
-		if (amd64_read_pci_cfg(pvt->dram_f2_ctl, dbams[i], &dbam))
+		if (amd64_read_pci_cfg(pvt->F2, dbams[i], &dbam))
 			goto err_reg;
 
 		for (j = 0; j < 4; j++) {
@@ -1220,11 +1220,11 @@ static void amd64_setup(struct amd64_pvt *pvt)
 {
 	u32 reg;
 
-	amd64_read_pci_cfg(pvt->misc_f3_ctl, F10_NB_CFG_HIGH, &reg);
+	amd64_read_pci_cfg(pvt->F3, F10_NB_CFG_HIGH, &reg);
 
 	pvt->flags.cf8_extcfg = !!(reg & F10_NB_CFG_LOW_ENABLE_EXT_CFG);
 	reg |= F10_NB_CFG_LOW_ENABLE_EXT_CFG;
-	pci_write_config_dword(pvt->misc_f3_ctl, F10_NB_CFG_HIGH, reg);
+	pci_write_config_dword(pvt->F3, F10_NB_CFG_HIGH, reg);
 }
 
 /* Restore the extended configuration access via 0xCF8 feature */
@@ -1232,12 +1232,12 @@ static void amd64_teardown(struct amd64_pvt *pvt)
 {
 	u32 reg;
 
-	amd64_read_pci_cfg(pvt->misc_f3_ctl, F10_NB_CFG_HIGH, &reg);
+	amd64_read_pci_cfg(pvt->F3, F10_NB_CFG_HIGH, &reg);
 
 	reg &= ~F10_NB_CFG_LOW_ENABLE_EXT_CFG;
 	if (pvt->flags.cf8_extcfg)
 		reg |= F10_NB_CFG_LOW_ENABLE_EXT_CFG;
-	pci_write_config_dword(pvt->misc_f3_ctl, F10_NB_CFG_HIGH, reg);
+	pci_write_config_dword(pvt->F3, F10_NB_CFG_HIGH, reg);
 }
 
 static u64 f10_get_error_address(struct mem_ctl_info *mci,
@@ -1261,10 +1261,10 @@ static void f10_read_dram_base_limit(struct amd64_pvt *pvt, int dram)
 	high_offset = F10_DRAM_BASE_HIGH + (dram << 3);
 
 	/* read the 'raw' DRAM BASE Address register */
-	amd64_read_pci_cfg(pvt->addr_f1_ctl, low_offset, &low_base);
+	amd64_read_pci_cfg(pvt->F1, low_offset, &low_base);
 
 	/* Read from the ECS data register */
-	amd64_read_pci_cfg(pvt->addr_f1_ctl, high_offset, &high_base);
+	amd64_read_pci_cfg(pvt->F1, high_offset, &high_base);
 
 	/* Extract parts into separate data entries */
 	pvt->dram_rw_en[dram] = (low_base & 0x3);
@@ -1281,10 +1281,10 @@ static void f10_read_dram_base_limit(struct amd64_pvt *pvt, int dram)
 	high_offset = F10_DRAM_LIMIT_HIGH + (dram << 3);
 
 	/* read the 'raw' LIMIT registers */
-	amd64_read_pci_cfg(pvt->addr_f1_ctl, low_offset, &low_limit);
+	amd64_read_pci_cfg(pvt->F1, low_offset, &low_limit);
 
 	/* Read from the ECS data register for the HIGH portion */
-	amd64_read_pci_cfg(pvt->addr_f1_ctl, high_offset, &high_limit);
+	amd64_read_pci_cfg(pvt->F1, high_offset, &high_limit);
 
 	pvt->dram_DstNode[dram] = (low_limit & 0x7);
 	pvt->dram_IntlvSel[dram] = (low_limit >> 8) & 0x7;
@@ -1301,7 +1301,7 @@ static void f10_read_dram_base_limit(struct amd64_pvt *pvt, int dram)
 static void f10_read_dram_ctl_register(struct amd64_pvt *pvt)
 {
 
-	if (!amd64_read_pci_cfg(pvt->dram_f2_ctl, F10_DCTL_SEL_LOW,
+	if (!amd64_read_pci_cfg(pvt->F2, F10_DCTL_SEL_LOW,
 				&pvt->dram_ctl_select_low)) {
 		debugf0("F2x110 (DCTL Sel. Low): 0x%08x, "
 			"High range addresses at: 0x%x\n",
@@ -1327,7 +1327,7 @@ static void f10_read_dram_ctl_register(struct amd64_pvt *pvt)
 			dct_sel_interleave_addr(pvt));
 	}
 
-	amd64_read_pci_cfg(pvt->dram_f2_ctl, F10_DCTL_SEL_HIGH,
+	amd64_read_pci_cfg(pvt->F2, F10_DCTL_SEL_HIGH,
 			   &pvt->dram_ctl_select_high);
 }
 
@@ -1707,8 +1707,8 @@ static void amd64_debug_display_dimm_sizes(int ctrl, struct amd64_pvt *pvt)
 static struct amd64_family_type amd64_family_types[] = {
 	[K8_CPUS] = {
 		.ctl_name = "K8",
-		.addr_f1_ctl = PCI_DEVICE_ID_AMD_K8_NB_ADDRMAP,
-		.misc_f3_ctl = PCI_DEVICE_ID_AMD_K8_NB_MISC,
+		.f1_id = PCI_DEVICE_ID_AMD_K8_NB_ADDRMAP,
+		.f3_id = PCI_DEVICE_ID_AMD_K8_NB_MISC,
 		.ops = {
 			.early_channel_count	= k8_early_channel_count,
 			.get_error_address	= k8_get_error_address,
@@ -1719,8 +1719,8 @@ static struct amd64_family_type amd64_family_types[] = {
 	},
 	[F10_CPUS] = {
 		.ctl_name = "F10h",
-		.addr_f1_ctl = PCI_DEVICE_ID_AMD_10H_NB_MAP,
-		.misc_f3_ctl = PCI_DEVICE_ID_AMD_10H_NB_MISC,
+		.f1_id = PCI_DEVICE_ID_AMD_10H_NB_MAP,
+		.f3_id = PCI_DEVICE_ID_AMD_10H_NB_MISC,
 		.ops = {
 			.early_channel_count	= f10_early_channel_count,
 			.get_error_address	= f10_get_error_address,
@@ -2035,53 +2035,44 @@ void amd64_decode_bus_error(int node_id, struct mce *m, u32 nbcfg)
 }
 
 /*
- * Use pvt->dram_f2_ctl which contains the F2 CPU PCI device to get the related
+ * Use pvt->F2 which contains the F2 CPU PCI device to get the related
  * F1 (AddrMap) and F3 (Misc) devices. Return negative value on error.
  */
 static int amd64_reserve_mc_sibling_devices(struct amd64_pvt *pvt, u16 f1_id,
 					    u16 f3_id)
 {
 	/* Reserve the ADDRESS MAP Device */
-	pvt->addr_f1_ctl = pci_get_related_function(pvt->dram_f2_ctl->vendor,
-						    f1_id,
-						    pvt->dram_f2_ctl);
-
-	if (!pvt->addr_f1_ctl) {
+	pvt->F1 = pci_get_related_function(pvt->F2->vendor, f1_id, pvt->F2);
+	if (!pvt->F1) {
 		amd64_printk(KERN_ERR, "error address map device not found: "
-			     "vendor %x device 0x%x (broken BIOS?)\n",
-			     PCI_VENDOR_ID_AMD, f1_id);
+				       "vendor %x device 0x%x (broken BIOS?)\n",
+				       PCI_VENDOR_ID_AMD, f1_id);
 		return -ENODEV;
 	}
 
 	/* Reserve the MISC Device */
-	pvt->misc_f3_ctl = pci_get_related_function(pvt->dram_f2_ctl->vendor,
-						    f3_id,
-						    pvt->dram_f2_ctl);
+	pvt->F3 = pci_get_related_function(pvt->F2->vendor, f3_id, pvt->F2);
+	if (!pvt->F3) {
+		pci_dev_put(pvt->F1);
+		pvt->F1 = NULL;
 
-	if (!pvt->misc_f3_ctl) {
-		pci_dev_put(pvt->addr_f1_ctl);
-		pvt->addr_f1_ctl = NULL;
+		amd64_printk(KERN_ERR, "error F3 device not found: "
+				       "vendor %x device 0x%x (broken BIOS?)\n",
+				       PCI_VENDOR_ID_AMD, f3_id);
 
-		amd64_printk(KERN_ERR, "error miscellaneous device not found: "
-			     "vendor %x device 0x%x (broken BIOS?)\n",
-			     PCI_VENDOR_ID_AMD, f3_id);
 		return -ENODEV;
 	}
-
-	debugf1("    Addr Map device PCI Bus ID:\t%s\n",
-		pci_name(pvt->addr_f1_ctl));
-	debugf1("    DRAM MEM-CTL PCI Bus ID:\t%s\n",
-		pci_name(pvt->dram_f2_ctl));
-	debugf1("    Misc device PCI Bus ID:\t%s\n",
-		pci_name(pvt->misc_f3_ctl));
+	debugf1("F1: %s\n", pci_name(pvt->F1));
+	debugf1("F2: %s\n", pci_name(pvt->F2));
+	debugf1("F3: %s\n", pci_name(pvt->F3));
 
 	return 0;
 }
 
 static void amd64_free_mc_sibling_devices(struct amd64_pvt *pvt)
 {
-	pci_dev_put(pvt->addr_f1_ctl);
-	pci_dev_put(pvt->misc_f3_ctl);
+	pci_dev_put(pvt->F1);
+	pci_dev_put(pvt->F3);
 }
 
 /*
@@ -2109,7 +2100,7 @@ static void amd64_read_mc_registers(struct amd64_pvt *pvt)
 	} else
 		debugf0("  TOP_MEM2 disabled.\n");
 
-	amd64_read_pci_cfg(pvt->misc_f3_ctl, K8_NBCAP, &pvt->nbcap);
+	amd64_read_pci_cfg(pvt->F3, K8_NBCAP, &pvt->nbcap);
 
 	if (pvt->ops->read_dram_ctl_register)
 		pvt->ops->read_dram_ctl_register(pvt);
@@ -2146,21 +2137,20 @@ static void amd64_read_mc_registers(struct amd64_pvt *pvt)
 
 	amd64_read_dct_base_mask(pvt);
 
-	amd64_read_pci_cfg(pvt->addr_f1_ctl, K8_DHAR, &pvt->dhar);
+	amd64_read_pci_cfg(pvt->F1, K8_DHAR, &pvt->dhar);
 	amd64_read_dbam_reg(pvt);
 
-	amd64_read_pci_cfg(pvt->misc_f3_ctl,
-			   F10_ONLINE_SPARE, &pvt->online_spare);
+	amd64_read_pci_cfg(pvt->F3, F10_ONLINE_SPARE, &pvt->online_spare);
 
-	amd64_read_pci_cfg(pvt->dram_f2_ctl, F10_DCLR_0, &pvt->dclr0);
-	amd64_read_pci_cfg(pvt->dram_f2_ctl, F10_DCHR_0, &pvt->dchr0);
+	amd64_read_pci_cfg(pvt->F2, F10_DCLR_0, &pvt->dclr0);
+	amd64_read_pci_cfg(pvt->F2, F10_DCHR_0, &pvt->dchr0);
 
 	if (boot_cpu_data.x86 >= 0x10) {
 		if (!dct_ganging_enabled(pvt)) {
-			amd64_read_pci_cfg(pvt->dram_f2_ctl, F10_DCLR_1, &pvt->dclr1);
-			amd64_read_pci_cfg(pvt->dram_f2_ctl, F10_DCHR_1, &pvt->dchr1);
+			amd64_read_pci_cfg(pvt->F2, F10_DCLR_1, &pvt->dclr1);
+			amd64_read_pci_cfg(pvt->F2, F10_DCHR_1, &pvt->dchr1);
 		}
-		amd64_read_pci_cfg(pvt->misc_f3_ctl, EXT_NB_MCA_CFG, &tmp);
+		amd64_read_pci_cfg(pvt->F3, EXT_NB_MCA_CFG, &tmp);
 	}
 
 	if (boot_cpu_data.x86 == 0x10 &&
@@ -2249,7 +2239,7 @@ static int amd64_init_csrows(struct mem_ctl_info *mci)
 
 	pvt = mci->pvt_info;
 
-	amd64_read_pci_cfg(pvt->misc_f3_ctl, K8_NBCFG, &pvt->nbcfg);
+	amd64_read_pci_cfg(pvt->F3, K8_NBCFG, &pvt->nbcfg);
 
 	debugf0("NBCFG= 0x%x  CHIPKILL= %s DRAM ECC= %s\n", pvt->nbcfg,
 		(pvt->nbcfg & K8_NBCFG_CHIPKILL) ? "Enabled" : "Disabled",
@@ -2394,20 +2384,20 @@ static void amd64_enable_ecc_error_reporting(struct mem_ctl_info *mci)
 	struct amd64_pvt *pvt = mci->pvt_info;
 	u32 value, mask = K8_NBCTL_CECCEn | K8_NBCTL_UECCEn;
 
-	amd64_read_pci_cfg(pvt->misc_f3_ctl, K8_NBCTL, &value);
+	amd64_read_pci_cfg(pvt->F3, K8_NBCTL, &value);
 
 	/* turn on UECCn and CECCEn bits */
 	pvt->old_nbctl = value & mask;
 	pvt->nbctl_mcgctl_saved = 1;
 
 	value |= mask;
-	pci_write_config_dword(pvt->misc_f3_ctl, K8_NBCTL, value);
+	pci_write_config_dword(pvt->F3, K8_NBCTL, value);
 
 	if (amd64_toggle_ecc_err_reporting(pvt, ON))
 		amd64_printk(KERN_WARNING, "Error enabling ECC reporting over "
 					   "MCGCTL!\n");
 
-	amd64_read_pci_cfg(pvt->misc_f3_ctl, K8_NBCFG, &value);
+	amd64_read_pci_cfg(pvt->F3, K8_NBCFG, &value);
 
 	debugf0("NBCFG(1)= 0x%x  CHIPKILL= %s ECC_ENABLE= %s\n", value,
 		(value & K8_NBCFG_CHIPKILL) ? "Enabled" : "Disabled",
@@ -2422,9 +2412,9 @@ static void amd64_enable_ecc_error_reporting(struct mem_ctl_info *mci)
 
 		/* Attempt to turn on DRAM ECC Enable */
 		value |= K8_NBCFG_ECC_ENABLE;
-		pci_write_config_dword(pvt->misc_f3_ctl, K8_NBCFG, value);
+		pci_write_config_dword(pvt->F3, K8_NBCFG, value);
 
-		amd64_read_pci_cfg(pvt->misc_f3_ctl, K8_NBCFG, &value);
+		amd64_read_pci_cfg(pvt->F3, K8_NBCFG, &value);
 
 		if (!(value & K8_NBCFG_ECC_ENABLE)) {
 			amd64_printk(KERN_WARNING,
@@ -2452,17 +2442,17 @@ static void amd64_restore_ecc_error_reporting(struct amd64_pvt *pvt)
 	if (!pvt->nbctl_mcgctl_saved)
 		return;
 
-	amd64_read_pci_cfg(pvt->misc_f3_ctl, K8_NBCTL, &value);
+	amd64_read_pci_cfg(pvt->F3, K8_NBCTL, &value);
 	value &= ~mask;
 	value |= pvt->old_nbctl;
 
-	pci_write_config_dword(pvt->misc_f3_ctl, K8_NBCTL, value);
+	pci_write_config_dword(pvt->F3, K8_NBCTL, value);
 
 	/* restore previous BIOS DRAM ECC "off" setting which we force-enabled */
 	if (!pvt->flags.nb_ecc_prev) {
-		amd64_read_pci_cfg(pvt->misc_f3_ctl, K8_NBCFG, &value);
+		amd64_read_pci_cfg(pvt->F3, K8_NBCFG, &value);
 		value &= ~K8_NBCFG_ECC_ENABLE;
-		pci_write_config_dword(pvt->misc_f3_ctl, K8_NBCFG, value);
+		pci_write_config_dword(pvt->F3, K8_NBCFG, value);
 	}
 
 	/* restore the NB Enable MCGCTL bit */
@@ -2488,13 +2478,13 @@ static int amd64_check_ecc_enabled(struct amd64_pvt *pvt)
 	u8 ecc_enabled = 0;
 	bool nb_mce_en = false;
 
-	amd64_read_pci_cfg(pvt->misc_f3_ctl, K8_NBCFG, &value);
+	amd64_read_pci_cfg(pvt->F3, K8_NBCFG, &value);
 
 	ecc_enabled = !!(value & K8_NBCFG_ECC_ENABLE);
 	if (!ecc_enabled)
 		amd64_printk(KERN_NOTICE, "This node reports that Memory ECC "
 			     "is currently disabled, set F3x%x[22] (%s).\n",
-			     K8_NBCFG, pci_name(pvt->misc_f3_ctl));
+			     K8_NBCFG, pci_name(pvt->F3));
 	else
 		amd64_printk(KERN_INFO, "ECC is enabled by BIOS.\n");
 
@@ -2554,7 +2544,7 @@ static void amd64_setup_mci_misc_attributes(struct mem_ctl_info *mci)
 	mci->mod_name		= EDAC_MOD_STR;
 	mci->mod_ver		= EDAC_AMD64_VERSION;
 	mci->ctl_name		= pvt->ctl_name;
-	mci->dev_name		= pci_name(pvt->dram_f2_ctl);
+	mci->dev_name		= pci_name(pvt->F2);
 	mci->ctl_page_to_phys	= NULL;
 
 	/* memory scrubber interface */
@@ -2611,7 +2601,7 @@ static struct amd64_family_type *amd64_per_family_init(struct amd64_pvt *pvt)
  * later come back in a finish-setup function to perform that final
  * initialization. See also amd64_init_2nd_stage() for that.
  */
-static int amd64_probe_one_instance(struct pci_dev *dram_f2_ctl)
+static int amd64_probe_one_instance(struct pci_dev *F2)
 {
 	struct amd64_pvt *pvt = NULL;
 	struct amd64_family_type *fam_type = NULL;
@@ -2622,8 +2612,8 @@ static int amd64_probe_one_instance(struct pci_dev *dram_f2_ctl)
 	if (!pvt)
 		goto err_exit;
 
-	pvt->mc_node_id	 = get_node_id(dram_f2_ctl);
-	pvt->dram_f2_ctl = dram_f2_ctl;
+	pvt->mc_node_id	 = get_node_id(F2);
+	pvt->F2 = F2;
 
 	ret = -EINVAL;
 	fam_type = amd64_per_family_init(pvt);
@@ -2631,8 +2621,8 @@ static int amd64_probe_one_instance(struct pci_dev *dram_f2_ctl)
 		goto err_free;
 
 	ret = -ENODEV;
-	err = amd64_reserve_mc_sibling_devices(pvt, fam_type->addr_f1_ctl,
-					       fam_type->misc_f3_ctl);
+	err = amd64_reserve_mc_sibling_devices(pvt, fam_type->f1_id,
+						fam_type->f3_id);
 	if (err)
 		goto err_free;
 
@@ -2695,7 +2685,7 @@ static int amd64_init_2nd_stage(struct amd64_pvt *pvt)
 
 	mci->pvt_info = pvt;
 
-	mci->dev = &pvt->dram_f2_ctl->dev;
+	mci->dev = &pvt->F2->dev;
 	amd64_setup_mci_misc_attributes(mci);
 
 	if (amd64_init_csrows(mci))
@@ -2839,8 +2829,7 @@ static void amd64_setup_pci_device(void)
 
 		pvt = mci->pvt_info;
 		amd64_ctl_pci =
-			edac_pci_create_generic_ctl(&pvt->dram_f2_ctl->dev,
-						    EDAC_MOD_STR);
+			edac_pci_create_generic_ctl(&pvt->F2->dev, EDAC_MOD_STR);
 
 		if (!amd64_ctl_pci) {
 			pr_warning("%s(): Unable to create PCI control\n",
diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
index 007b68a..76760a8 100644
--- a/drivers/edac/amd64_edac.h
+++ b/drivers/edac/amd64_edac.h
@@ -386,9 +386,7 @@ struct amd64_pvt {
 	struct low_ops *ops;
 
 	/* pci_device handles which we utilize */
-	struct pci_dev *addr_f1_ctl;
-	struct pci_dev *dram_f2_ctl;
-	struct pci_dev *misc_f3_ctl;
+	struct pci_dev *F1, *F2, *F3;
 
 	int mc_node_id;		/* MC index of this MC node */
 	int ext_model;		/* extended model value of this node */
@@ -518,8 +516,7 @@ struct low_ops {
 
 struct amd64_family_type {
 	const char *ctl_name;
-	u16 addr_f1_ctl;
-	u16 misc_f3_ctl;
+	u16 f1_id, f3_id;
 	struct low_ops ops;
 };
 
diff --git a/drivers/edac/amd64_edac_inj.c b/drivers/edac/amd64_edac_inj.c
index 29f1f7a..523ce4a 100644
--- a/drivers/edac/amd64_edac_inj.c
+++ b/drivers/edac/amd64_edac_inj.c
@@ -122,15 +122,13 @@ static ssize_t amd64_inject_read_store(struct mem_ctl_info *mci,
 		/* Form value to choose 16-byte section of cacheline */
 		section = F10_NB_ARRAY_DRAM_ECC |
 				SET_NB_ARRAY_ADDRESS(pvt->injection.section);
-		pci_write_config_dword(pvt->misc_f3_ctl,
-					F10_NB_ARRAY_ADDR, section);
+		pci_write_config_dword(pvt->F3, F10_NB_ARRAY_ADDR, section);
 
 		word_bits = SET_NB_DRAM_INJECTION_READ(pvt->injection.word,
 						pvt->injection.bit_map);
 
 		/* Issue 'word' and 'bit' along with the READ request */
-		pci_write_config_dword(pvt->misc_f3_ctl,
-					F10_NB_ARRAY_DATA, word_bits);
+		pci_write_config_dword(pvt->F3, F10_NB_ARRAY_DATA, word_bits);
 
 		debugf0("section=0x%x word_bits=0x%x\n", section, word_bits);
 
@@ -157,15 +155,13 @@ static ssize_t amd64_inject_write_store(struct mem_ctl_info *mci,
 		/* Form value to choose 16-byte section of cacheline */
 		section = F10_NB_ARRAY_DRAM_ECC |
 				SET_NB_ARRAY_ADDRESS(pvt->injection.section);
-		pci_write_config_dword(pvt->misc_f3_ctl,
-					F10_NB_ARRAY_ADDR, section);
+		pci_write_config_dword(pvt->F3, F10_NB_ARRAY_ADDR, section);
 
 		word_bits = SET_NB_DRAM_INJECTION_WRITE(pvt->injection.word,
 						pvt->injection.bit_map);
 
 		/* Issue 'word' and 'bit' along with the READ request */
-		pci_write_config_dword(pvt->misc_f3_ctl,
-					F10_NB_ARRAY_DATA, word_bits);
+		pci_write_config_dword(pvt->F3, F10_NB_ARRAY_DATA, word_bits);
 
 		debugf0("section=0x%x word_bits=0x%x\n", section, word_bits);
 
-- 
1.7.3.1.50.g1e633


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 08/16] amd64_edac: Rework printk macros
  2010-11-26 19:04 [PATCH 00/16] amd64_edac stuff for .38 Borislav Petkov
                   ` (6 preceding siblings ...)
  2010-11-26 19:04 ` [PATCH 07/16] amd64_edac: Rename CPU PCI devices Borislav Petkov
@ 2010-11-26 19:04 ` Borislav Petkov
  2010-11-26 19:04 ` [PATCH 09/16] amd64_edac: Allocate driver instances dynamically Borislav Petkov
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Borislav Petkov @ 2010-11-26 19:04 UTC (permalink / raw
  To: norsk5; +Cc: linux-edac, linux-kernel, Borislav Petkov

From: Borislav Petkov <borislav.petkov@amd.com>

Add a macro per printk level, shorten up error messages. Add relevant
information to KERN_INFO level. No functional change.

Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
---
 drivers/edac/amd64_edac.c     |  145 ++++++++++++++++-------------------------
 drivers/edac/amd64_edac.h     |   29 ++++++--
 drivers/edac/amd64_edac_inj.c |   13 +---
 drivers/edac/edac_core.h      |    3 +-
 drivers/edac/edac_mc.c        |    4 +-
 5 files changed, 87 insertions(+), 107 deletions(-)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index aa57853..d42d6d1 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -62,7 +62,7 @@ static int ddr3_dbam[] = { [0]		= -1,
 			   [5 ... 6]	= 1024,
 			   [7 ... 8]	= 2048,
 			   [9 ... 10]	= 4096,
-			   [11]	= 8192,
+			   [11]		= 8192,
 };
 
 /*
@@ -148,11 +148,10 @@ static int __amd64_set_scrub_rate(struct pci_dev *ctl, u32 new_bw, u32 min_rate)
 
 	scrubval = scrubrates[i].scrubval;
 	if (scrubval)
-		edac_printk(KERN_DEBUG, EDAC_MC,
-			    "Setting scrub rate bandwidth: %u\n",
-			    scrubrates[i].bandwidth);
+		amd64_info("Setting scrub rate bandwidth: %u\n",
+			   scrubrates[i].bandwidth);
 	else
-		edac_printk(KERN_DEBUG, EDAC_MC, "Turning scrubbing off.\n");
+		amd64_info("Turning scrubbing off.\n");
 
 	pci_write_bits32(ctl, K8_SCRCTRL, scrubval, 0x001F);
 
@@ -176,8 +175,7 @@ static int amd64_get_scrub_rate(struct mem_ctl_info *mci, u32 *bw)
 
 	scrubval = scrubval & 0x001F;
 
-	edac_printk(KERN_DEBUG, EDAC_MC,
-		    "pci-read, sdram scrub control value: %d \n", scrubval);
+	amd64_debug("pci-read, sdram scrub control value: %d\n", scrubval);
 
 	for (i = 0; i < ARRAY_SIZE(scrubrates); i++) {
 		if (scrubrates[i].scrubval == scrubval) {
@@ -296,9 +294,7 @@ static struct mem_ctl_info *find_mc_by_sys_addr(struct mem_ctl_info *mci,
 	if (unlikely((intlv_en != 0x01) &&
 		     (intlv_en != 0x03) &&
 		     (intlv_en != 0x07))) {
-		amd64_printk(KERN_WARNING, "junk value of 0x%x extracted from "
-			     "IntlvEn field of DRAM Base Register for node 0: "
-			     "this probably indicates a BIOS bug.\n", intlv_en);
+		amd64_warn("DRAM Base[IntlvEn] junk value: 0x%x, BIOS bug?\n", intlv_en);
 		return NULL;
 	}
 
@@ -314,11 +310,9 @@ static struct mem_ctl_info *find_mc_by_sys_addr(struct mem_ctl_info *mci,
 
 	/* sanity test for sys_addr */
 	if (unlikely(!amd64_base_limit_match(pvt, sys_addr, node_id))) {
-		amd64_printk(KERN_WARNING,
-			     "%s(): sys_addr 0x%llx falls outside base/limit "
-			     "address range for node %d with node interleaving "
-			     "enabled.\n",
-			     __func__, sys_addr, node_id);
+		amd64_warn("%s: sys_addr 0x%llx falls outside base/limit address"
+			   "range for node %d with node interleaving enabled.\n",
+			   __func__, sys_addr, node_id);
 		return NULL;
 	}
 
@@ -770,9 +764,8 @@ static int sys_addr_to_csrow(struct mem_ctl_info *mci, u64 sys_addr)
 	csrow = input_addr_to_csrow(mci, sys_addr_to_input_addr(mci, sys_addr));
 
 	if (csrow == -1)
-		amd64_mc_printk(mci, KERN_ERR,
-			     "Failed to translate InputAddr to csrow for "
-			     "address 0x%lx\n", (unsigned long)sys_addr);
+		amd64_mc_err(mci, "Failed to translate InputAddr to csrow for "
+				  "address 0x%lx\n", (unsigned long)sys_addr);
 	return csrow;
 }
 
@@ -860,8 +853,7 @@ static void amd64_dump_misc_regs(struct amd64_pvt *pvt)
 		return;
 	}
 
-	amd64_printk(KERN_INFO, "using %s syndromes.\n",
-		     ((pvt->syn_type == 8) ? "x8" : "x4"));
+	amd64_info("using %s syndromes.\n", ((pvt->syn_type == 8) ? "x8" : "x4"));
 
 	/* Only if NOT ganged does dclr1 have valid info */
 	if (!dct_ganging_enabled(pvt))
@@ -983,7 +975,7 @@ static void amd64_read_dct_base_mask(struct amd64_pvt *pvt)
 	}
 }
 
-static enum mem_type amd64_determine_memory_type(struct amd64_pvt *pvt)
+static enum mem_type amd64_determine_memory_type(struct amd64_pvt *pvt, int cs)
 {
 	enum mem_type type;
 
@@ -996,7 +988,7 @@ static enum mem_type amd64_determine_memory_type(struct amd64_pvt *pvt)
 		type = (pvt->dclr0 & BIT(18)) ? MEM_DDR : MEM_RDDR;
 	}
 
-	debugf1("  Memory type is: %s\n", edac_mem_types[type]);
+	amd64_info("CS%d: %s\n", cs, edac_mem_types[type]);
 
 	return type;
 }
@@ -1087,9 +1079,8 @@ static void k8_map_sysaddr_to_csrow(struct mem_ctl_info *mci,
 			 * 2 DIMMs is in error. So we need to ID 'both' of them
 			 * as suspect.
 			 */
-			amd64_mc_printk(mci, KERN_WARNING,
-					"unknown syndrome 0x%04x - possible "
-					"error reporting race\n", syndrome);
+			amd64_mc_warn(mci, "unknown syndrome 0x%04x - possible "
+					   "error reporting race\n", syndrome);
 			edac_mc_handle_ce_no_info(mci, EDAC_MOD_STR);
 			return;
 		}
@@ -1111,8 +1102,7 @@ static void k8_map_sysaddr_to_csrow(struct mem_ctl_info *mci,
 	 */
 	src_mci = find_mc_by_sys_addr(mci, sys_addr);
 	if (!src_mci) {
-		amd64_mc_printk(mci, KERN_ERR,
-			     "failed to map error address 0x%lx to a node\n",
+		amd64_mc_err(mci, "failed to map error addr 0x%lx to a node\n",
 			     (unsigned long)sys_addr);
 		edac_mc_handle_ce_no_info(mci, EDAC_MOD_STR);
 		return;
@@ -1194,7 +1184,7 @@ static int f10_early_channel_count(struct amd64_pvt *pvt)
 	if (channels > 2)
 		channels = 2;
 
-	debugf0("MCT channel count: %d\n", channels);
+	amd64_info("MCT channel count: %d\n", channels);
 
 	return channels;
 
@@ -1698,9 +1688,9 @@ static void amd64_debug_display_dimm_sizes(int ctrl, struct amd64_pvt *pvt)
 		if (dcsb[dimm*2 + 1] & K8_DCSB_CS_ENABLE)
 			size1 = pvt->ops->dbam_to_cs(pvt, DBAM_DIMM(dimm, dbam));
 
-		edac_printk(KERN_DEBUG, EDAC_MC, " %d: %5dMB %d: %5dMB\n",
-			    dimm * 2,     size0 << factor,
-			    dimm * 2 + 1, size1 << factor);
+		amd64_info(EDAC_MC ": %d: %5dMB %d: %5dMB\n",
+				dimm * 2,     size0 << factor,
+				dimm * 2 + 1, size1 << factor);
 	}
 }
 
@@ -1906,8 +1896,7 @@ static int get_channel_from_ecc_syndrome(struct mem_ctl_info *mci, u16 syndrome)
 					  ARRAY_SIZE(x4_vectors),
 					  pvt->syn_type);
 	else {
-		amd64_printk(KERN_WARNING, "%s: Illegal syndrome type: %u\n",
-					   __func__, pvt->syn_type);
+		amd64_warn("Illegal syndrome type: %u\n", pvt->syn_type);
 		return err_sym;
 	}
 
@@ -1925,17 +1914,15 @@ static void amd64_handle_ce(struct mem_ctl_info *mci,
 	u64 sys_addr;
 
 	/* Ensure that the Error Address is VALID */
-	if ((info->nbsh & K8_NBSH_VALID_ERROR_ADDR) == 0) {
-		amd64_mc_printk(mci, KERN_ERR,
-			"HW has no ERROR_ADDRESS available\n");
+	if (!(info->nbsh & K8_NBSH_VALID_ERROR_ADDR)) {
+		amd64_mc_err(mci, "HW has no ERROR_ADDRESS available\n");
 		edac_mc_handle_ce_no_info(mci, EDAC_MOD_STR);
 		return;
 	}
 
 	sys_addr = pvt->ops->get_error_address(mci, info);
 
-	amd64_mc_printk(mci, KERN_ERR,
-		"CE ERROR_ADDRESS= 0x%llx\n", sys_addr);
+	amd64_mc_err(mci, "CE ERROR_ADDRESS= 0x%llx\n", sys_addr);
 
 	pvt->ops->map_sysaddr_to_csrow(mci, info, sys_addr);
 }
@@ -1952,9 +1939,8 @@ static void amd64_handle_ue(struct mem_ctl_info *mci,
 
 	log_mci = mci;
 
-	if ((info->nbsh & K8_NBSH_VALID_ERROR_ADDR) == 0) {
-		amd64_mc_printk(mci, KERN_CRIT,
-			"HW has no ERROR_ADDRESS available\n");
+	if (!(info->nbsh & K8_NBSH_VALID_ERROR_ADDR)) {
+		amd64_mc_err(mci, "HW has no ERROR_ADDRESS available\n");
 		edac_mc_handle_ue_no_info(log_mci, EDAC_MOD_STR);
 		return;
 	}
@@ -1967,9 +1953,8 @@ static void amd64_handle_ue(struct mem_ctl_info *mci,
 	 */
 	src_mci = find_mc_by_sys_addr(mci, sys_addr);
 	if (!src_mci) {
-		amd64_mc_printk(mci, KERN_CRIT,
-			"ERROR ADDRESS (0x%lx) value NOT mapped to a MC\n",
-			(unsigned long)sys_addr);
+		amd64_mc_err(mci, "ERROR ADDRESS (0x%lx) NOT mapped to a MC\n",
+				  (unsigned long)sys_addr);
 		edac_mc_handle_ue_no_info(log_mci, EDAC_MOD_STR);
 		return;
 	}
@@ -1978,9 +1963,8 @@ static void amd64_handle_ue(struct mem_ctl_info *mci,
 
 	csrow = sys_addr_to_csrow(log_mci, sys_addr);
 	if (csrow < 0) {
-		amd64_mc_printk(mci, KERN_CRIT,
-			"ERROR_ADDRESS (0x%lx) value NOT mapped to 'csrow'\n",
-			(unsigned long)sys_addr);
+		amd64_mc_err(mci, "ERROR_ADDRESS (0x%lx) NOT mapped to CS\n",
+				  (unsigned long)sys_addr);
 		edac_mc_handle_ue_no_info(log_mci, EDAC_MOD_STR);
 	} else {
 		error_address_to_page_and_offset(sys_addr, &page, &offset);
@@ -2044,9 +2028,9 @@ static int amd64_reserve_mc_sibling_devices(struct amd64_pvt *pvt, u16 f1_id,
 	/* Reserve the ADDRESS MAP Device */
 	pvt->F1 = pci_get_related_function(pvt->F2->vendor, f1_id, pvt->F2);
 	if (!pvt->F1) {
-		amd64_printk(KERN_ERR, "error address map device not found: "
-				       "vendor %x device 0x%x (broken BIOS?)\n",
-				       PCI_VENDOR_ID_AMD, f1_id);
+		amd64_err("error address map device not found: "
+			  "vendor %x device 0x%x (broken BIOS?)\n",
+			  PCI_VENDOR_ID_AMD, f1_id);
 		return -ENODEV;
 	}
 
@@ -2056,9 +2040,9 @@ static int amd64_reserve_mc_sibling_devices(struct amd64_pvt *pvt, u16 f1_id,
 		pci_dev_put(pvt->F1);
 		pvt->F1 = NULL;
 
-		amd64_printk(KERN_ERR, "error F3 device not found: "
-				       "vendor %x device 0x%x (broken BIOS?)\n",
-				       PCI_VENDOR_ID_AMD, f3_id);
+		amd64_err("error F3 device not found: "
+			  "vendor %x device 0x%x (broken BIOS?)\n",
+			  PCI_VENDOR_ID_AMD, f3_id);
 
 		return -ENODEV;
 	}
@@ -2268,7 +2252,7 @@ static int amd64_init_csrows(struct mem_ctl_info *mci)
 		csrow->page_mask = ~mask_from_dct_mask(pvt, i);
 		/* 8 bytes of resolution */
 
-		csrow->mtype = amd64_determine_memory_type(pvt);
+		csrow->mtype = amd64_determine_memory_type(pvt, i);
 
 		debugf1("  for MC node %d csrow %d:\n", pvt->mc_node_id, i);
 		debugf1("    input_addr_min: 0x%lx input_addr_max: 0x%lx\n",
@@ -2313,8 +2297,7 @@ static bool amd64_nb_mce_bank_enabled_on_node(int nid)
 	bool ret = false;
 
 	if (!zalloc_cpumask_var(&mask, GFP_KERNEL)) {
-		amd64_printk(KERN_WARNING, "%s: error allocating mask\n",
-			     __func__);
+		amd64_warn("%s: Error allocating mask\n", __func__);
 		return false;
 	}
 
@@ -2346,8 +2329,7 @@ static int amd64_toggle_ecc_err_reporting(struct amd64_pvt *pvt, bool on)
 	int cpu;
 
 	if (!zalloc_cpumask_var(&cmask, GFP_KERNEL)) {
-		amd64_printk(KERN_WARNING, "%s: error allocating mask\n",
-			     __func__);
+		amd64_warn("%s: error allocating mask\n", __func__);
 		return false;
 	}
 
@@ -2394,8 +2376,7 @@ static void amd64_enable_ecc_error_reporting(struct mem_ctl_info *mci)
 	pci_write_config_dword(pvt->F3, K8_NBCTL, value);
 
 	if (amd64_toggle_ecc_err_reporting(pvt, ON))
-		amd64_printk(KERN_WARNING, "Error enabling ECC reporting over "
-					   "MCGCTL!\n");
+		amd64_warn("Error enabling ECC reporting over MCGCTL!\n");
 
 	amd64_read_pci_cfg(pvt->F3, K8_NBCFG, &value);
 
@@ -2404,9 +2385,7 @@ static void amd64_enable_ecc_error_reporting(struct mem_ctl_info *mci)
 		(value & K8_NBCFG_ECC_ENABLE) ? "Enabled" : "Disabled");
 
 	if (!(value & K8_NBCFG_ECC_ENABLE)) {
-		amd64_printk(KERN_WARNING,
-			"This node reports that DRAM ECC is "
-			"currently Disabled; ENABLING now\n");
+		amd64_warn("DRAM ECC disabled on this node, enabling...\n");
 
 		pvt->flags.nb_ecc_prev = 0;
 
@@ -2417,12 +2396,10 @@ static void amd64_enable_ecc_error_reporting(struct mem_ctl_info *mci)
 		amd64_read_pci_cfg(pvt->F3, K8_NBCFG, &value);
 
 		if (!(value & K8_NBCFG_ECC_ENABLE)) {
-			amd64_printk(KERN_WARNING,
-				"Hardware rejects Enabling DRAM ECC checking\n"
-				"Check memory DIMM configuration\n");
+			amd64_warn("Hardware rejected DRAM ECC enable,"
+				   "check memory DIMM configuration.\n");
 		} else {
-			amd64_printk(KERN_DEBUG,
-				"Hardware accepted DRAM ECC Enable\n");
+			amd64_info("Hardware accepted DRAM ECC Enable\n");
 		}
 	} else {
 		pvt->flags.nb_ecc_prev = 1;
@@ -2457,7 +2434,7 @@ static void amd64_restore_ecc_error_reporting(struct amd64_pvt *pvt)
 
 	/* restore the NB Enable MCGCTL bit */
 	if (amd64_toggle_ecc_err_reporting(pvt, OFF))
-		amd64_printk(KERN_WARNING, "Error restoring NB MCGCTL settings!\n");
+		amd64_warn("Error restoring NB MCGCTL settings!\n");
 }
 
 /*
@@ -2481,25 +2458,20 @@ static int amd64_check_ecc_enabled(struct amd64_pvt *pvt)
 	amd64_read_pci_cfg(pvt->F3, K8_NBCFG, &value);
 
 	ecc_enabled = !!(value & K8_NBCFG_ECC_ENABLE);
-	if (!ecc_enabled)
-		amd64_printk(KERN_NOTICE, "This node reports that Memory ECC "
-			     "is currently disabled, set F3x%x[22] (%s).\n",
-			     K8_NBCFG, pci_name(pvt->F3));
-	else
-		amd64_printk(KERN_INFO, "ECC is enabled by BIOS.\n");
+	amd64_info("DRAM ECC %s.\n", (ecc_enabled ? "enabled" : "disabled"));
 
 	nb_mce_en = amd64_nb_mce_bank_enabled_on_node(pvt->mc_node_id);
 	if (!nb_mce_en)
-		amd64_printk(KERN_NOTICE, "NB MCE bank disabled, set MSR "
-			     "0x%08x[4] on node %d to enable.\n",
+		amd64_notice("NB MCE bank disabled, "
+			     "set MSR 0x%08x[4] on node %d to enable.\n",
 			     MSR_IA32_MCG_CTL, pvt->mc_node_id);
 
 	if (!ecc_enabled || !nb_mce_en) {
 		if (!ecc_enable_override) {
-			amd64_printk(KERN_NOTICE, "%s", ecc_msg);
+			amd64_notice("%s", ecc_msg);
 			return -ENODEV;
 		} else {
-			amd64_printk(KERN_WARNING, "Forcing ECC checking on!\n");
+			amd64_warn("Forcing ECC on!\n");
 		}
 	}
 
@@ -2575,17 +2547,17 @@ static struct amd64_family_type *amd64_per_family_init(struct amd64_pvt *pvt)
 		break;
 
 	default:
-		amd64_printk(KERN_ERR, "Unsupported family!\n");
+		amd64_err("Unsupported family!\n");
 		return NULL;
 	}
 
 	pvt->ext_model = boot_cpu_data.x86_model >> 4;
 
-	amd64_printk(KERN_INFO, "%s %s detected.\n", pvt->ctl_name,
+	amd64_info("%s %sdetected (node %d).\n", pvt->ctl_name,
 		     (fam == 0xf ?
-				(pvt->ext_model >= K8_REV_F  ? "revF or later"
-							     : "revE or earlier")
-				 : ""));
+				(pvt->ext_model >= K8_REV_F  ? "revF or later "
+							     : "revE or earlier ")
+				 : ""), pvt->mc_node_id);
 	return fam_type;
 }
 
@@ -2736,8 +2708,6 @@ static int __devinit amd64_init_one_instance(struct pci_dev *pdev,
 {
 	int ret = 0;
 
-	debugf0("(MC node=%d)\n", get_node_id(pdev));
-
 	ret = pci_enable_device(pdev);
 	if (ret < 0) {
 		debugf0("ret=%d\n", ret);
@@ -2746,8 +2716,7 @@ static int __devinit amd64_init_one_instance(struct pci_dev *pdev,
 
 	ret = amd64_probe_one_instance(pdev);
 	if (ret < 0)
-		amd64_printk(KERN_ERR, "Error probing instance: %d\n",
-					get_node_id(pdev));
+		amd64_err("Error probing instance: %d\n", get_node_id(pdev));
 
 	return ret;
 }
diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
index 76760a8..f15e2b2 100644
--- a/drivers/edac/amd64_edac.h
+++ b/drivers/edac/amd64_edac.h
@@ -74,11 +74,26 @@
 #include "edac_core.h"
 #include "mce_amd.h"
 
-#define amd64_printk(level, fmt, arg...) \
-	edac_printk(level, "amd64", fmt, ##arg)
+#define amd64_debug(fmt, arg...) \
+	edac_printk(KERN_DEBUG, "amd64", fmt, ##arg)
 
-#define amd64_mc_printk(mci, level, fmt, arg...) \
-	edac_mc_chipset_printk(mci, level, "amd64", fmt, ##arg)
+#define amd64_info(fmt, arg...) \
+	edac_printk(KERN_INFO, "amd64", fmt, ##arg)
+
+#define amd64_notice(fmt, arg...) \
+	edac_printk(KERN_NOTICE, "amd64", fmt, ##arg)
+
+#define amd64_warn(fmt, arg...) \
+	edac_printk(KERN_WARNING, "amd64", fmt, ##arg)
+
+#define amd64_err(fmt, arg...) \
+	edac_printk(KERN_ERR, "amd64", fmt, ##arg)
+
+#define amd64_mc_warn(mci, fmt, arg...) \
+	edac_mc_chipset_printk(mci, KERN_WARNING, "amd64", fmt, ##arg)
+
+#define amd64_mc_err(mci, fmt, arg...) \
+	edac_mc_chipset_printk(mci, KERN_ERR, "amd64", fmt, ##arg)
 
 /*
  * Throughout the comments in this code, the following terms are used:
@@ -129,7 +144,7 @@
  *         sections 3.5.4 and 3.5.5 for more information.
  */
 
-#define EDAC_AMD64_VERSION		" Ver: 3.3.0 " __DATE__
+#define EDAC_AMD64_VERSION		"v3.3.0"
 #define EDAC_MOD_STR			"amd64_edac"
 
 #define EDAC_MAX_NUMNODES		8
@@ -527,8 +542,8 @@ static inline int amd64_read_pci_cfg_dword(struct pci_dev *pdev, int offset,
 
 	err = pci_read_config_dword(pdev, offset, val);
 	if (err)
-		amd64_printk(KERN_WARNING, "%s: error reading F%dx%x.\n",
-			     func, PCI_FUNC(pdev->devfn), offset);
+		amd64_warn("%s: error reading F%dx%x.\n",
+			   func, PCI_FUNC(pdev->devfn), offset);
 
 	return err;
 }
diff --git a/drivers/edac/amd64_edac_inj.c b/drivers/edac/amd64_edac_inj.c
index 523ce4a..688478d 100644
--- a/drivers/edac/amd64_edac_inj.c
+++ b/drivers/edac/amd64_edac_inj.c
@@ -23,9 +23,7 @@ static ssize_t amd64_inject_section_store(struct mem_ctl_info *mci,
 	if (ret != -EINVAL) {
 
 		if (value > 3) {
-			amd64_printk(KERN_WARNING,
-				     "%s: invalid section 0x%lx\n",
-				     __func__, value);
+			amd64_warn("%s: invalid section 0x%lx\n", __func__, value);
 			return -EINVAL;
 		}
 
@@ -58,9 +56,7 @@ static ssize_t amd64_inject_word_store(struct mem_ctl_info *mci,
 	if (ret != -EINVAL) {
 
 		if (value > 8) {
-			amd64_printk(KERN_WARNING,
-				     "%s: invalid word 0x%lx\n",
-				     __func__, value);
+			amd64_warn("%s: invalid word 0x%lx\n", __func__, value);
 			return -EINVAL;
 		}
 
@@ -92,9 +88,8 @@ static ssize_t amd64_inject_ecc_vector_store(struct mem_ctl_info *mci,
 	if (ret != -EINVAL) {
 
 		if (value & 0xFFFF0000) {
-			amd64_printk(KERN_WARNING,
-				     "%s: invalid EccVector: 0x%lx\n",
-				     __func__, value);
+			amd64_warn("%s: invalid EccVector: 0x%lx\n",
+				   __func__, value);
 			return -EINVAL;
 		}
 
diff --git a/drivers/edac/edac_core.h b/drivers/edac/edac_core.h
index d7ca43a..050f9ee 100644
--- a/drivers/edac/edac_core.h
+++ b/drivers/edac/edac_core.h
@@ -68,9 +68,10 @@
 #define EDAC_PCI "PCI"
 #define EDAC_DEBUG "DEBUG"
 
+extern const char *edac_mem_types[];
+
 #ifdef CONFIG_EDAC_DEBUG
 extern int edac_debug_level;
-extern const char *edac_mem_types[];
 
 #define edac_debug_printk(level, fmt, arg...)                           \
 	do {                                                            \
diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c
index ba6586a..4ab9480 100644
--- a/drivers/edac/edac_mc.c
+++ b/drivers/edac/edac_mc.c
@@ -76,6 +76,8 @@ static void edac_mc_dump_mci(struct mem_ctl_info *mci)
 	debugf3("\tpvt_info = %p\n\n", mci->pvt_info);
 }
 
+#endif				/* CONFIG_EDAC_DEBUG */
+
 /*
  * keep those in sync with the enum mem_type
  */
@@ -100,8 +102,6 @@ const char *edac_mem_types[] = {
 };
 EXPORT_SYMBOL_GPL(edac_mem_types);
 
-#endif				/* CONFIG_EDAC_DEBUG */
-
 /* 'ptr' points to a possibly unaligned item X such that sizeof(X) is 'size'.
  * Adjust 'ptr' so that its alignment is at least as stringent as what the
  * compiler would provide for X and return the aligned result.
-- 
1.7.3.1.50.g1e633


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 09/16] amd64_edac: Allocate driver instances dynamically
  2010-11-26 19:04 [PATCH 00/16] amd64_edac stuff for .38 Borislav Petkov
                   ` (7 preceding siblings ...)
  2010-11-26 19:04 ` [PATCH 08/16] amd64_edac: Rework printk macros Borislav Petkov
@ 2010-11-26 19:04 ` Borislav Petkov
  2010-11-26 19:04 ` [PATCH 10/16] amd64_edac: Remove explicit Kconfig PCI dependency Borislav Petkov
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Borislav Petkov @ 2010-11-26 19:04 UTC (permalink / raw
  To: norsk5; +Cc: linux-edac, linux-kernel, Borislav Petkov

From: Borislav Petkov <borislav.petkov@amd.com>

Remove static allocation in favor of dynamically allocating space for as
many driver instances as northbridges present on the system.

There should be no functional change resulting from this patch.

Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
---
 drivers/edac/amd64_edac.c |   45 +++++++++++++++++++++++++++++----------------
 drivers/edac/amd64_edac.h |    2 --
 2 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index d42d6d1..a7922da 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -15,10 +15,9 @@ module_param(ecc_enable_override, int, 0644);
 
 static struct msr __percpu *msrs;
 
-/* Lookup table for all possible MC control instances */
-struct amd64_pvt;
-static struct mem_ctl_info *mci_lookup[EDAC_MAX_NUMNODES];
-static struct amd64_pvt *pvt_lookup[EDAC_MAX_NUMNODES];
+/* Per-node driver instances */
+static struct mem_ctl_info **mcis;
+static struct amd64_pvt **pvts;
 
 /*
  * Address to DRAM bank mapping: see F2x80 for K8 and F2x[1,0]80 for Fam10 and
@@ -1446,7 +1445,7 @@ static int f10_lookup_addr_in_dct(u32 in_addr, u32 nid, u32 cs)
 	int cs_found = -EINVAL;
 	int csrow;
 
-	mci = mci_lookup[nid];
+	mci = mcis[nid];
 	if (!mci)
 		return cs_found;
 
@@ -1995,7 +1994,7 @@ static inline void __amd64_decode_bus_error(struct mem_ctl_info *mci,
 
 void amd64_decode_bus_error(int node_id, struct mce *m, u32 nbcfg)
 {
-	struct mem_ctl_info *mci = mci_lookup[node_id];
+	struct mem_ctl_info *mci = mcis[node_id];
 	struct err_regs regs;
 
 	regs.nbsl  = (u32) m->status;
@@ -2615,7 +2614,7 @@ static int amd64_probe_one_instance(struct pci_dev *F2)
 	 * Save the pointer to the private data for use in 2nd initialization
 	 * stage
 	 */
-	pvt_lookup[pvt->mc_node_id] = pvt;
+	pvts[pvt->mc_node_id] = pvt;
 
 	return 0;
 
@@ -2672,8 +2671,8 @@ static int amd64_init_2nd_stage(struct amd64_pvt *pvt)
 		goto err_add_mc;
 	}
 
-	mci_lookup[node_id] = mci;
-	pvt_lookup[node_id] = NULL;
+	mcis[node_id] = mci;
+	pvts[node_id] = NULL;
 
 	/* register stuff with EDAC MCE */
 	if (report_gart_errors)
@@ -2696,8 +2695,8 @@ err_exit:
 
 	amd64_free_mc_sibling_devices(pvt);
 
-	kfree(pvt_lookup[pvt->mc_node_id]);
-	pvt_lookup[node_id] = NULL;
+	kfree(pvts[pvt->mc_node_id]);
+	pvts[node_id] = NULL;
 
 	return ret;
 }
@@ -2746,7 +2745,7 @@ static void __devexit amd64_remove_one_instance(struct pci_dev *pdev)
 
 	/* Free the EDAC CORE resources */
 	mci->pvt_info = NULL;
-	mci_lookup[pvt->mc_node_id] = NULL;
+	mcis[pvt->mc_node_id] = NULL;
 
 	kfree(pvt);
 	edac_mc_free(mci);
@@ -2793,7 +2792,7 @@ static void amd64_setup_pci_device(void)
 	if (amd64_ctl_pci)
 		return;
 
-	mci = mci_lookup[0];
+	mci = mcis[0];
 	if (mci) {
 
 		pvt = mci->pvt_info;
@@ -2822,6 +2821,12 @@ static int __init amd64_edac_init(void)
 	if (amd_cache_northbridges() < 0)
 		goto err_ret;
 
+	err = -ENOMEM;
+	pvts = kzalloc(amd_nb_num() * sizeof(pvts[0]), GFP_KERNEL);
+	mcis = kzalloc(amd_nb_num() * sizeof(mcis[0]), GFP_KERNEL);
+	if (!(pvts && mcis))
+		goto err_ret;
+
 	msrs = msrs_alloc();
 	if (!msrs)
 		goto err_ret;
@@ -2831,16 +2836,16 @@ static int __init amd64_edac_init(void)
 		goto err_pci;
 
 	/*
-	 * At this point, the array 'pvt_lookup[]' contains pointers to alloc'd
+	 * At this point, the array 'pvts[]' contains pointers to alloc'd
 	 * amd64_pvt structs. These will be used in the 2nd stage init function
 	 * to finish initialization of the MC instances.
 	 */
 	err = -ENODEV;
 	for (nb = 0; nb < amd_nb_num(); nb++) {
-		if (!pvt_lookup[nb])
+		if (!pvts[nb])
 			continue;
 
-		err = amd64_init_2nd_stage(pvt_lookup[nb]);
+		err = amd64_init_2nd_stage(pvts[nb]);
 		if (err)
 			goto err_2nd_stage;
 
@@ -2854,9 +2859,11 @@ static int __init amd64_edac_init(void)
 
 err_2nd_stage:
 	pci_unregister_driver(&amd64_pci_driver);
+
 err_pci:
 	msrs_free(msrs);
 	msrs = NULL;
+
 err_ret:
 	return err;
 }
@@ -2868,6 +2875,12 @@ static void __exit amd64_edac_exit(void)
 
 	pci_unregister_driver(&amd64_pci_driver);
 
+	kfree(mcis);
+	mcis = NULL;
+
+	kfree(pvts);
+	pvts = NULL;
+
 	msrs_free(msrs);
 	msrs = NULL;
 }
diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
index f15e2b2..5538cc1 100644
--- a/drivers/edac/amd64_edac.h
+++ b/drivers/edac/amd64_edac.h
@@ -147,8 +147,6 @@
 #define EDAC_AMD64_VERSION		"v3.3.0"
 #define EDAC_MOD_STR			"amd64_edac"
 
-#define EDAC_MAX_NUMNODES		8
-
 /* Extended Model from CPUID, for CPU Revision numbers */
 #define K8_REV_D			1
 #define K8_REV_E			2
-- 
1.7.3.1.50.g1e633


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 10/16] amd64_edac: Remove explicit Kconfig PCI dependency
  2010-11-26 19:04 [PATCH 00/16] amd64_edac stuff for .38 Borislav Petkov
                   ` (8 preceding siblings ...)
  2010-11-26 19:04 ` [PATCH 09/16] amd64_edac: Allocate driver instances dynamically Borislav Petkov
@ 2010-11-26 19:04 ` Borislav Petkov
  2010-11-26 19:04 ` [PATCH 11/16] amd64_edac: Remove PCI ECS enabling functions Borislav Petkov
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Borislav Petkov @ 2010-11-26 19:04 UTC (permalink / raw
  To: norsk5; +Cc: linux-edac, linux-kernel, Borislav Petkov

From: Borislav Petkov <borislav.petkov@amd.com>

AMD_NB pulls in the dependency on PCI. Clarify/fix help text while at it.

Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
---
 drivers/edac/Kconfig |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
index f436a2f..fe70a34 100644
--- a/drivers/edac/Kconfig
+++ b/drivers/edac/Kconfig
@@ -75,11 +75,11 @@ config EDAC_MCE
 	bool
 
 config EDAC_AMD64
-	tristate "AMD64 (Opteron, Athlon64) K8, F10h, F11h"
-	depends on EDAC_MM_EDAC && AMD_NB && X86_64 && PCI && EDAC_DECODE_MCE
+	tristate "AMD64 (Opteron, Athlon64) K8, F10h"
+	depends on EDAC_MM_EDAC && AMD_NB && X86_64 && EDAC_DECODE_MCE
 	help
-	  Support for error detection and correction on the AMD 64
-	  Families of Memory Controllers (K8, F10h and F11h)
+	  Support for error detection and correction of DRAM ECC errors on
+	  the AMD64 families of memory controllers (K8 and F10h)
 
 config EDAC_AMD64_ERROR_INJECTION
 	bool "Sysfs HW Error injection facilities"
-- 
1.7.3.1.50.g1e633


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 11/16] amd64_edac: Remove PCI ECS enabling functions
  2010-11-26 19:04 [PATCH 00/16] amd64_edac stuff for .38 Borislav Petkov
                   ` (9 preceding siblings ...)
  2010-11-26 19:04 ` [PATCH 10/16] amd64_edac: Remove explicit Kconfig PCI dependency Borislav Petkov
@ 2010-11-26 19:04 ` Borislav Petkov
  2010-11-26 19:04 ` [PATCH 12/16] amd64_edac: Carve out ECC-related hw settings Borislav Petkov
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Borislav Petkov @ 2010-11-26 19:04 UTC (permalink / raw
  To: norsk5; +Cc: linux-edac, linux-kernel, Borislav Petkov

From: Borislav Petkov <borislav.petkov@amd.com>

PCI ECS is being enabled by default since 2.6.26 on AMD so this code is
just superfluous now, remove it.

Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
---
 drivers/edac/amd64_edac.c |   55 ---------------------------------------------
 drivers/edac/amd64_edac.h |    4 ---
 2 files changed, 0 insertions(+), 59 deletions(-)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index a7922da..99d3334 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -1204,31 +1204,6 @@ static int f10_dbam_to_chip_select(struct amd64_pvt *pvt, int cs_mode)
 	return dbam_map[cs_mode];
 }
 
-/* Enable extended configuration access via 0xCF8 feature */
-static void amd64_setup(struct amd64_pvt *pvt)
-{
-	u32 reg;
-
-	amd64_read_pci_cfg(pvt->F3, F10_NB_CFG_HIGH, &reg);
-
-	pvt->flags.cf8_extcfg = !!(reg & F10_NB_CFG_LOW_ENABLE_EXT_CFG);
-	reg |= F10_NB_CFG_LOW_ENABLE_EXT_CFG;
-	pci_write_config_dword(pvt->F3, F10_NB_CFG_HIGH, reg);
-}
-
-/* Restore the extended configuration access via 0xCF8 feature */
-static void amd64_teardown(struct amd64_pvt *pvt)
-{
-	u32 reg;
-
-	amd64_read_pci_cfg(pvt->F3, F10_NB_CFG_HIGH, &reg);
-
-	reg &= ~F10_NB_CFG_LOW_ENABLE_EXT_CFG;
-	if (pvt->flags.cf8_extcfg)
-		reg |= F10_NB_CFG_LOW_ENABLE_EXT_CFG;
-	pci_write_config_dword(pvt->F3, F10_NB_CFG_HIGH, reg);
-}
-
 static u64 f10_get_error_address(struct mem_ctl_info *mci,
 			struct err_regs *info)
 {
@@ -1251,8 +1226,6 @@ static void f10_read_dram_base_limit(struct amd64_pvt *pvt, int dram)
 
 	/* read the 'raw' DRAM BASE Address register */
 	amd64_read_pci_cfg(pvt->F1, low_offset, &low_base);
-
-	/* Read from the ECS data register */
 	amd64_read_pci_cfg(pvt->F1, high_offset, &high_base);
 
 	/* Extract parts into separate data entries */
@@ -1271,8 +1244,6 @@ static void f10_read_dram_base_limit(struct amd64_pvt *pvt, int dram)
 
 	/* read the 'raw' LIMIT registers */
 	amd64_read_pci_cfg(pvt->F1, low_offset, &low_limit);
-
-	/* Read from the ECS data register for the HIGH portion */
 	amd64_read_pci_cfg(pvt->F1, high_offset, &high_limit);
 
 	pvt->dram_DstNode[dram] = (low_limit & 0x7);
@@ -2560,18 +2531,6 @@ static struct amd64_family_type *amd64_per_family_init(struct amd64_pvt *pvt)
 	return fam_type;
 }
 
-/*
- * Init stuff for this DRAM Controller device.
- *
- * Due to a hardware feature on Fam10h CPUs, the Enable Extended Configuration
- * Space feature MUST be enabled on ALL Processors prior to actually reading
- * from the ECS registers. Since the loading of the module can occur on any
- * 'core', and cores don't 'see' all the other processors ECS data when the
- * others are NOT enabled. Our solution is to first enable ECS access in this
- * routine on all processors, gather some data in a amd64_pvt structure and
- * later come back in a finish-setup function to perform that final
- * initialization. See also amd64_init_2nd_stage() for that.
- */
 static int amd64_probe_one_instance(struct pci_dev *F2)
 {
 	struct amd64_pvt *pvt = NULL;
@@ -2603,14 +2562,6 @@ static int amd64_probe_one_instance(struct pci_dev *F2)
 		goto err_put;
 
 	/*
-	 * Key operation here: setup of HW prior to performing ops on it. Some
-	 * setup is required to access ECS data. After this is performed, the
-	 * 'teardown' function must be called upon error and normal exit paths.
-	 */
-	if (boot_cpu_data.x86 >= 0x10)
-		amd64_setup(pvt);
-
-	/*
 	 * Save the pointer to the private data for use in 2nd initialization
 	 * stage
 	 */
@@ -2690,9 +2641,6 @@ err_exit:
 
 	amd64_restore_ecc_error_reporting(pvt);
 
-	if (boot_cpu_data.x86 > 0xf)
-		amd64_teardown(pvt);
-
 	amd64_free_mc_sibling_devices(pvt);
 
 	kfree(pvts[pvt->mc_node_id]);
@@ -2734,9 +2682,6 @@ static void __devexit amd64_remove_one_instance(struct pci_dev *pdev)
 
 	amd64_restore_ecc_error_reporting(pvt);
 
-	if (boot_cpu_data.x86 > 0xf)
-		amd64_teardown(pvt);
-
 	amd64_free_mc_sibling_devices(pvt);
 
 	/* unregister from EDAC MCE */
diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
index 5538cc1..4bc6f18 100644
--- a/drivers/edac/amd64_edac.h
+++ b/drivers/edac/amd64_edac.h
@@ -335,9 +335,6 @@
 #define K8_SCRCTRL			0x58
 
 #define F10_NB_CFG_LOW			0x88
-#define	F10_NB_CFG_LOW_ENABLE_EXT_CFG	BIT(14)
-
-#define F10_NB_CFG_HIGH			0x8C
 
 #define F10_ONLINE_SPARE		0xB0
 #define F10_ONLINE_SPARE_SWAPDONE0(x)	((x) & BIT(1))
@@ -476,7 +473,6 @@ struct amd64_pvt {
 
 	/* misc settings */
 	struct flags {
-		unsigned long cf8_extcfg:1;
 		unsigned long nb_mce_enable:1;
 		unsigned long nb_ecc_prev:1;
 	} flags;
-- 
1.7.3.1.50.g1e633


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 12/16] amd64_edac: Carve out ECC-related hw settings
  2010-11-26 19:04 [PATCH 00/16] amd64_edac stuff for .38 Borislav Petkov
                   ` (10 preceding siblings ...)
  2010-11-26 19:04 ` [PATCH 11/16] amd64_edac: Remove PCI ECS enabling functions Borislav Petkov
@ 2010-11-26 19:04 ` Borislav Petkov
  2010-11-26 19:04 ` [PATCH 13/16] amd64_edac: Check ECC capabilities initially Borislav Petkov
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Borislav Petkov @ 2010-11-26 19:04 UTC (permalink / raw
  To: norsk5; +Cc: linux-edac, linux-kernel, Borislav Petkov

From: Borislav Petkov <borislav.petkov@amd.com>

This is in preparation for the init path reorganization where we want
only to

1) test whether a particular node supports ECC
2) can it be enabled

and only then do the necessary allocation/initialization. For that,
we need to decouple the ECC settings of the node from the instance's
descriptor.

The should be no functional change introduced by this patch.

Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
---
 drivers/edac/amd64_edac.c |   59 ++++++++++++++++++++++++++++++--------------
 drivers/edac/amd64_edac.h |   14 +++++++----
 2 files changed, 49 insertions(+), 24 deletions(-)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 99d3334..2388bb9 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -18,6 +18,7 @@ static struct msr __percpu *msrs;
 /* Per-node driver instances */
 static struct mem_ctl_info **mcis;
 static struct amd64_pvt **pvts;
+static struct ecc_settings **ecc_stngs;
 
 /*
  * Address to DRAM bank mapping: see F2x80 for K8 and F2x[1,0]80 for Fam10 and
@@ -2293,7 +2294,7 @@ out:
 	return ret;
 }
 
-static int amd64_toggle_ecc_err_reporting(struct amd64_pvt *pvt, bool on)
+static int amd64_toggle_ecc_err_reporting(struct ecc_settings *s, u8 nid, bool on)
 {
 	cpumask_var_t cmask;
 	int cpu;
@@ -2303,7 +2304,7 @@ static int amd64_toggle_ecc_err_reporting(struct amd64_pvt *pvt, bool on)
 		return false;
 	}
 
-	get_cpus_on_this_dct_cpumask(cmask, pvt->mc_node_id);
+	get_cpus_on_this_dct_cpumask(cmask, nid);
 
 	rdmsr_on_cpus(cmask, MSR_IA32_MCG_CTL, msrs);
 
@@ -2313,14 +2314,14 @@ static int amd64_toggle_ecc_err_reporting(struct amd64_pvt *pvt, bool on)
 
 		if (on) {
 			if (reg->l & K8_MSR_MCGCTL_NBE)
-				pvt->flags.nb_mce_enable = 1;
+				s->flags.nb_mce_enable = 1;
 
 			reg->l |= K8_MSR_MCGCTL_NBE;
 		} else {
 			/*
 			 * Turn off NB MCE reporting only when it was off before
 			 */
-			if (!pvt->flags.nb_mce_enable)
+			if (!s->flags.nb_mce_enable)
 				reg->l &= ~K8_MSR_MCGCTL_NBE;
 		}
 	}
@@ -2334,18 +2335,20 @@ static int amd64_toggle_ecc_err_reporting(struct amd64_pvt *pvt, bool on)
 static void amd64_enable_ecc_error_reporting(struct mem_ctl_info *mci)
 {
 	struct amd64_pvt *pvt = mci->pvt_info;
+	u8 nid = pvt->mc_node_id;
+	struct ecc_settings *s = ecc_stngs[nid];
 	u32 value, mask = K8_NBCTL_CECCEn | K8_NBCTL_UECCEn;
 
 	amd64_read_pci_cfg(pvt->F3, K8_NBCTL, &value);
 
-	/* turn on UECCn and CECCEn bits */
-	pvt->old_nbctl = value & mask;
-	pvt->nbctl_mcgctl_saved = 1;
+	/* turn on UECCEn and CECCEn bits */
+	s->old_nbctl   = value & mask;
+	s->nbctl_valid = true;
 
 	value |= mask;
 	pci_write_config_dword(pvt->F3, K8_NBCTL, value);
 
-	if (amd64_toggle_ecc_err_reporting(pvt, ON))
+	if (amd64_toggle_ecc_err_reporting(s, nid, ON))
 		amd64_warn("Error enabling ECC reporting over MCGCTL!\n");
 
 	amd64_read_pci_cfg(pvt->F3, K8_NBCFG, &value);
@@ -2357,7 +2360,7 @@ static void amd64_enable_ecc_error_reporting(struct mem_ctl_info *mci)
 	if (!(value & K8_NBCFG_ECC_ENABLE)) {
 		amd64_warn("DRAM ECC disabled on this node, enabling...\n");
 
-		pvt->flags.nb_ecc_prev = 0;
+		s->flags.nb_ecc_prev = 0;
 
 		/* Attempt to turn on DRAM ECC Enable */
 		value |= K8_NBCFG_ECC_ENABLE;
@@ -2372,7 +2375,7 @@ static void amd64_enable_ecc_error_reporting(struct mem_ctl_info *mci)
 			amd64_info("Hardware accepted DRAM ECC Enable\n");
 		}
 	} else {
-		pvt->flags.nb_ecc_prev = 1;
+		s->flags.nb_ecc_prev = 1;
 	}
 
 	debugf0("NBCFG(2)= 0x%x  CHIPKILL= %s ECC_ENABLE= %s\n", value,
@@ -2384,26 +2387,28 @@ static void amd64_enable_ecc_error_reporting(struct mem_ctl_info *mci)
 
 static void amd64_restore_ecc_error_reporting(struct amd64_pvt *pvt)
 {
+	u8 nid = pvt->mc_node_id;
+	struct ecc_settings *s = ecc_stngs[nid];
 	u32 value, mask = K8_NBCTL_CECCEn | K8_NBCTL_UECCEn;
 
-	if (!pvt->nbctl_mcgctl_saved)
+	if (!s->nbctl_valid)
 		return;
 
 	amd64_read_pci_cfg(pvt->F3, K8_NBCTL, &value);
 	value &= ~mask;
-	value |= pvt->old_nbctl;
+	value |= s->old_nbctl;
 
 	pci_write_config_dword(pvt->F3, K8_NBCTL, value);
 
-	/* restore previous BIOS DRAM ECC "off" setting which we force-enabled */
-	if (!pvt->flags.nb_ecc_prev) {
+	/* restore previous BIOS DRAM ECC "off" setting we force-enabled */
+	if (!s->flags.nb_ecc_prev) {
 		amd64_read_pci_cfg(pvt->F3, K8_NBCFG, &value);
 		value &= ~K8_NBCFG_ECC_ENABLE;
 		pci_write_config_dword(pvt->F3, K8_NBCFG, value);
 	}
 
 	/* restore the NB Enable MCGCTL bit */
-	if (amd64_toggle_ecc_err_reporting(pvt, OFF))
+	if (amd64_toggle_ecc_err_reporting(s, nid, OFF))
 		amd64_warn("Error restoring NB MCGCTL settings!\n");
 }
 
@@ -2654,6 +2659,8 @@ static int __devinit amd64_init_one_instance(struct pci_dev *pdev,
 					     const struct pci_device_id *mc_type)
 {
 	int ret = 0;
+	u8 nid = get_node_id(pdev);
+	struct ecc_settings *s;
 
 	ret = pci_enable_device(pdev);
 	if (ret < 0) {
@@ -2661,9 +2668,16 @@ static int __devinit amd64_init_one_instance(struct pci_dev *pdev,
 		return -EIO;
 	}
 
+	ret = -ENOMEM;
+	s = kzalloc(sizeof(struct ecc_settings), GFP_KERNEL);
+	if (!s)
+		return ret;
+
+	ecc_stngs[nid] = s;
+
 	ret = amd64_probe_one_instance(pdev);
 	if (ret < 0)
-		amd64_err("Error probing instance: %d\n", get_node_id(pdev));
+		amd64_err("Error probing instance: %d\n", nid);
 
 	return ret;
 }
@@ -2688,6 +2702,9 @@ static void __devexit amd64_remove_one_instance(struct pci_dev *pdev)
 	amd_report_gart_errors(false);
 	amd_unregister_ecc_decoder(amd64_decode_bus_error);
 
+	kfree(ecc_stngs[pvt->mc_node_id]);
+	ecc_stngs[pvt->mc_node_id] = NULL;
+
 	/* Free the EDAC CORE resources */
 	mci->pvt_info = NULL;
 	mcis[pvt->mc_node_id] = NULL;
@@ -2767,9 +2784,10 @@ static int __init amd64_edac_init(void)
 		goto err_ret;
 
 	err = -ENOMEM;
-	pvts = kzalloc(amd_nb_num() * sizeof(pvts[0]), GFP_KERNEL);
-	mcis = kzalloc(amd_nb_num() * sizeof(mcis[0]), GFP_KERNEL);
-	if (!(pvts && mcis))
+	pvts	  = kzalloc(amd_nb_num() * sizeof(pvts[0]), GFP_KERNEL);
+	mcis	  = kzalloc(amd_nb_num() * sizeof(mcis[0]), GFP_KERNEL);
+	ecc_stngs = kzalloc(amd_nb_num() * sizeof(ecc_stngs[0]), GFP_KERNEL);
+	if (!(pvts && mcis && ecc_stngs))
 		goto err_ret;
 
 	msrs = msrs_alloc();
@@ -2820,6 +2838,9 @@ static void __exit amd64_edac_exit(void)
 
 	pci_unregister_driver(&amd64_pci_driver);
 
+	kfree(ecc_stngs);
+	ecc_stngs = NULL;
+
 	kfree(mcis);
 	mcis = NULL;
 
diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
index 4bc6f18..b76dce9 100644
--- a/drivers/edac/amd64_edac.h
+++ b/drivers/edac/amd64_edac.h
@@ -461,17 +461,21 @@ struct amd64_pvt {
 	/* place to store error injection parameters prior to issue */
 	struct error_injection injection;
 
-	/* Save old hw registers' values before we modified them */
-	u32 nbctl_mcgctl_saved;		/* When true, following 2 are valid */
-	u32 old_nbctl;
-
 	/* DCT per-family scrubrate setting */
 	u32 min_scrubrate;
 
 	/* family name this instance is running on */
 	const char *ctl_name;
 
-	/* misc settings */
+};
+
+/*
+ * per-node ECC settings descriptor
+ */
+struct ecc_settings {
+	u32 old_nbctl;
+	bool nbctl_valid;
+
 	struct flags {
 		unsigned long nb_mce_enable:1;
 		unsigned long nb_ecc_prev:1;
-- 
1.7.3.1.50.g1e633


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 13/16] amd64_edac: Check ECC capabilities initially
  2010-11-26 19:04 [PATCH 00/16] amd64_edac stuff for .38 Borislav Petkov
                   ` (11 preceding siblings ...)
  2010-11-26 19:04 ` [PATCH 12/16] amd64_edac: Carve out ECC-related hw settings Borislav Petkov
@ 2010-11-26 19:04 ` Borislav Petkov
  2010-11-26 19:04 ` [PATCH 14/16] amd64_edac: Remove two-stage initialization Borislav Petkov
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: Borislav Petkov @ 2010-11-26 19:04 UTC (permalink / raw
  To: norsk5; +Cc: linux-edac, linux-kernel, Borislav Petkov

From: Borislav Petkov <borislav.petkov@amd.com>

Rework the code to check the hardware ECC capabilities at PCI probing
time. We do all further initialization only if we actually can/have ECC
enabled.

While at it:
0. Fix function naming.
1. Simplify/clarify debug output.
2. Remove amd64_ prefix from the static functions
3. Reorganize code.

Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
---
 drivers/edac/amd64_edac.c |  141 ++++++++++++++++++++++++---------------------
 1 files changed, 75 insertions(+), 66 deletions(-)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 2388bb9..2fe7725 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -2188,18 +2188,19 @@ static u32 amd64_csrow_nr_pages(int csrow_nr, struct amd64_pvt *pvt)
 static int amd64_init_csrows(struct mem_ctl_info *mci)
 {
 	struct csrow_info *csrow;
-	struct amd64_pvt *pvt;
+	struct amd64_pvt *pvt = mci->pvt_info;
 	u64 input_addr_min, input_addr_max, sys_addr;
+	u32 val;
 	int i, empty = 1;
 
-	pvt = mci->pvt_info;
+	amd64_read_pci_cfg(pvt->F3, K8_NBCFG, &val);
 
-	amd64_read_pci_cfg(pvt->F3, K8_NBCFG, &pvt->nbcfg);
+	pvt->nbcfg = val;
+	pvt->ctl_error_info.nbcfg = val;
 
-	debugf0("NBCFG= 0x%x  CHIPKILL= %s DRAM ECC= %s\n", pvt->nbcfg,
-		(pvt->nbcfg & K8_NBCFG_CHIPKILL) ? "Enabled" : "Disabled",
-		(pvt->nbcfg & K8_NBCFG_ECC_ENABLE) ? "Enabled" : "Disabled"
-		);
+	debugf0("node %d, NBCFG=0x%08x[ChipKillEccCap: %d|DramEccEn: %d]\n",
+		pvt->mc_node_id, val,
+		!!(val & K8_NBCFG_CHIPKILL), !!(val & K8_NBCFG_ECC_ENABLE));
 
 	for (i = 0; i < pvt->cs_count; i++) {
 		csrow = &mci->csrows[i];
@@ -2294,7 +2295,7 @@ out:
 	return ret;
 }
 
-static int amd64_toggle_ecc_err_reporting(struct ecc_settings *s, u8 nid, bool on)
+static int toggle_ecc_err_reporting(struct ecc_settings *s, u8 nid, bool on)
 {
 	cpumask_var_t cmask;
 	int cpu;
@@ -2332,30 +2333,31 @@ static int amd64_toggle_ecc_err_reporting(struct ecc_settings *s, u8 nid, bool o
 	return 0;
 }
 
-static void amd64_enable_ecc_error_reporting(struct mem_ctl_info *mci)
+static bool enable_ecc_error_reporting(struct ecc_settings *s, u8 nid,
+				       struct pci_dev *F3)
 {
-	struct amd64_pvt *pvt = mci->pvt_info;
-	u8 nid = pvt->mc_node_id;
-	struct ecc_settings *s = ecc_stngs[nid];
+	bool ret = true;
 	u32 value, mask = K8_NBCTL_CECCEn | K8_NBCTL_UECCEn;
 
-	amd64_read_pci_cfg(pvt->F3, K8_NBCTL, &value);
+	if (toggle_ecc_err_reporting(s, nid, ON)) {
+		amd64_warn("Error enabling ECC reporting over MCGCTL!\n");
+		return false;
+	}
+
+	amd64_read_pci_cfg(F3, K8_NBCTL, &value);
 
 	/* turn on UECCEn and CECCEn bits */
 	s->old_nbctl   = value & mask;
 	s->nbctl_valid = true;
 
 	value |= mask;
-	pci_write_config_dword(pvt->F3, K8_NBCTL, value);
+	pci_write_config_dword(F3, K8_NBCTL, value);
 
-	if (amd64_toggle_ecc_err_reporting(s, nid, ON))
-		amd64_warn("Error enabling ECC reporting over MCGCTL!\n");
+	amd64_read_pci_cfg(F3, K8_NBCFG, &value);
 
-	amd64_read_pci_cfg(pvt->F3, K8_NBCFG, &value);
-
-	debugf0("NBCFG(1)= 0x%x  CHIPKILL= %s ECC_ENABLE= %s\n", value,
-		(value & K8_NBCFG_CHIPKILL) ? "Enabled" : "Disabled",
-		(value & K8_NBCFG_ECC_ENABLE) ? "Enabled" : "Disabled");
+	debugf0("1: node %d, NBCFG=0x%08x[ChipKillEccCap: %d|DramEccEn: %d]\n",
+		nid, value,
+		!!(value & K8_NBCFG_CHIPKILL), !!(value & K8_NBCFG_ECC_ENABLE));
 
 	if (!(value & K8_NBCFG_ECC_ENABLE)) {
 		amd64_warn("DRAM ECC disabled on this node, enabling...\n");
@@ -2364,13 +2366,14 @@ static void amd64_enable_ecc_error_reporting(struct mem_ctl_info *mci)
 
 		/* Attempt to turn on DRAM ECC Enable */
 		value |= K8_NBCFG_ECC_ENABLE;
-		pci_write_config_dword(pvt->F3, K8_NBCFG, value);
+		pci_write_config_dword(F3, K8_NBCFG, value);
 
-		amd64_read_pci_cfg(pvt->F3, K8_NBCFG, &value);
+		amd64_read_pci_cfg(F3, K8_NBCFG, &value);
 
 		if (!(value & K8_NBCFG_ECC_ENABLE)) {
 			amd64_warn("Hardware rejected DRAM ECC enable,"
 				   "check memory DIMM configuration.\n");
+			ret = false;
 		} else {
 			amd64_info("Hardware accepted DRAM ECC Enable\n");
 		}
@@ -2378,11 +2381,11 @@ static void amd64_enable_ecc_error_reporting(struct mem_ctl_info *mci)
 		s->flags.nb_ecc_prev = 1;
 	}
 
-	debugf0("NBCFG(2)= 0x%x  CHIPKILL= %s ECC_ENABLE= %s\n", value,
-		(value & K8_NBCFG_CHIPKILL) ? "Enabled" : "Disabled",
-		(value & K8_NBCFG_ECC_ENABLE) ? "Enabled" : "Disabled");
+	debugf0("2: node %d, NBCFG=0x%08x[ChipKillEccCap: %d|DramEccEn: %d]\n",
+		nid, value,
+		!!(value & K8_NBCFG_CHIPKILL), !!(value & K8_NBCFG_ECC_ENABLE));
 
-	pvt->ctl_error_info.nbcfg = value;
+	return ret;
 }
 
 static void amd64_restore_ecc_error_reporting(struct amd64_pvt *pvt)
@@ -2408,15 +2411,15 @@ static void amd64_restore_ecc_error_reporting(struct amd64_pvt *pvt)
 	}
 
 	/* restore the NB Enable MCGCTL bit */
-	if (amd64_toggle_ecc_err_reporting(s, nid, OFF))
+	if (toggle_ecc_err_reporting(s, nid, OFF))
 		amd64_warn("Error restoring NB MCGCTL settings!\n");
 }
 
 /*
- * EDAC requires that the BIOS have ECC enabled before taking over the
- * processing of ECC errors. This is because the BIOS can properly initialize
- * the memory system completely. A command line option allows to force-enable
- * hardware ECC later in amd64_enable_ecc_error_reporting().
+ * EDAC requires that the BIOS have ECC enabled before
+ * taking over the processing of ECC errors. A command line
+ * option allows to force-enable hardware ECC later in
+ * enable_ecc_error_reporting().
  */
 static const char *ecc_msg =
 	"ECC disabled in the BIOS or no ECC capability, module will not load.\n"
@@ -2424,33 +2427,28 @@ static const char *ecc_msg =
 	"'ecc_enable_override'.\n"
 	" (Note that use of the override may cause unknown side effects.)\n";
 
-static int amd64_check_ecc_enabled(struct amd64_pvt *pvt)
+static bool ecc_enabled(struct pci_dev *F3, u8 nid)
 {
 	u32 value;
-	u8 ecc_enabled = 0;
+	u8 ecc_en = 0;
 	bool nb_mce_en = false;
 
-	amd64_read_pci_cfg(pvt->F3, K8_NBCFG, &value);
+	amd64_read_pci_cfg(F3, K8_NBCFG, &value);
 
-	ecc_enabled = !!(value & K8_NBCFG_ECC_ENABLE);
-	amd64_info("DRAM ECC %s.\n", (ecc_enabled ? "enabled" : "disabled"));
+	ecc_en = !!(value & K8_NBCFG_ECC_ENABLE);
+	amd64_info("DRAM ECC %s.\n", (ecc_en ? "enabled" : "disabled"));
 
-	nb_mce_en = amd64_nb_mce_bank_enabled_on_node(pvt->mc_node_id);
+	nb_mce_en = amd64_nb_mce_bank_enabled_on_node(nid);
 	if (!nb_mce_en)
-		amd64_notice("NB MCE bank disabled, "
-			     "set MSR 0x%08x[4] on node %d to enable.\n",
-			     MSR_IA32_MCG_CTL, pvt->mc_node_id);
-
-	if (!ecc_enabled || !nb_mce_en) {
-		if (!ecc_enable_override) {
-			amd64_notice("%s", ecc_msg);
-			return -ENODEV;
-		} else {
-			amd64_warn("Forcing ECC on!\n");
-		}
-	}
+		amd64_notice("NB MCE bank disabled, set MSR "
+			     "0x%08x[4] on node %d to enable.\n",
+			     MSR_IA32_MCG_CTL, nid);
 
-	return 0;
+	if (!ecc_en || !nb_mce_en) {
+		amd64_notice("%s", ecc_msg);
+		return false;
+	}
+	return true;
 }
 
 struct mcidev_sysfs_attribute sysfs_attrs[ARRAY_SIZE(amd64_dbg_attrs) +
@@ -2536,7 +2534,7 @@ static struct amd64_family_type *amd64_per_family_init(struct amd64_pvt *pvt)
 	return fam_type;
 }
 
-static int amd64_probe_one_instance(struct pci_dev *F2)
+static int amd64_init_one_instance(struct pci_dev *F2)
 {
 	struct amd64_pvt *pvt = NULL;
 	struct amd64_family_type *fam_type = NULL;
@@ -2561,11 +2559,6 @@ static int amd64_probe_one_instance(struct pci_dev *F2)
 	if (err)
 		goto err_free;
 
-	ret = -EINVAL;
-	err = amd64_check_ecc_enabled(pvt);
-	if (err)
-		goto err_put;
-
 	/*
 	 * Save the pointer to the private data for use in 2nd initialization
 	 * stage
@@ -2574,9 +2567,6 @@ static int amd64_probe_one_instance(struct pci_dev *F2)
 
 	return 0;
 
-err_put:
-	amd64_free_mc_sibling_devices(pvt);
-
 err_free:
 	kfree(pvt);
 
@@ -2618,7 +2608,6 @@ static int amd64_init_2nd_stage(struct amd64_pvt *pvt)
 	if (amd64_init_csrows(mci))
 		mci->edac_cap = EDAC_FLAG_NONE;
 
-	amd64_enable_ecc_error_reporting(mci);
 	amd64_set_mc_sysfs_attributes(mci);
 
 	ret = -ENODEV;
@@ -2655,12 +2644,13 @@ err_exit:
 }
 
 
-static int __devinit amd64_init_one_instance(struct pci_dev *pdev,
+static int __devinit amd64_probe_one_instance(struct pci_dev *pdev,
 					     const struct pci_device_id *mc_type)
 {
-	int ret = 0;
 	u8 nid = get_node_id(pdev);
+	struct pci_dev *F3 = node_to_amd_nb(nid)->misc;
 	struct ecc_settings *s;
+	int ret = 0;
 
 	ret = pci_enable_device(pdev);
 	if (ret < 0) {
@@ -2671,15 +2661,34 @@ static int __devinit amd64_init_one_instance(struct pci_dev *pdev,
 	ret = -ENOMEM;
 	s = kzalloc(sizeof(struct ecc_settings), GFP_KERNEL);
 	if (!s)
-		return ret;
+		goto err_out;
 
 	ecc_stngs[nid] = s;
 
-	ret = amd64_probe_one_instance(pdev);
+	if (!ecc_enabled(F3, nid)) {
+		ret = -ENODEV;
+
+		if (!ecc_enable_override)
+			goto err_enable;
+
+		amd64_warn("Forcing ECC on!\n");
+
+		if (!enable_ecc_error_reporting(s, nid, F3))
+			goto err_enable;
+	}
+
+	ret = amd64_init_one_instance(pdev);
 	if (ret < 0)
 		amd64_err("Error probing instance: %d\n", nid);
 
 	return ret;
+
+err_enable:
+	kfree(s);
+	ecc_stngs[nid] = NULL;
+
+err_out:
+	return ret;
 }
 
 static void __devexit amd64_remove_one_instance(struct pci_dev *pdev)
@@ -2741,7 +2750,7 @@ MODULE_DEVICE_TABLE(pci, amd64_pci_table);
 
 static struct pci_driver amd64_pci_driver = {
 	.name		= EDAC_MOD_STR,
-	.probe		= amd64_init_one_instance,
+	.probe		= amd64_probe_one_instance,
 	.remove		= __devexit_p(amd64_remove_one_instance),
 	.id_table	= amd64_pci_table,
 };
-- 
1.7.3.1.50.g1e633


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 14/16] amd64_edac: Remove two-stage initialization
  2010-11-26 19:04 [PATCH 00/16] amd64_edac stuff for .38 Borislav Petkov
                   ` (12 preceding siblings ...)
  2010-11-26 19:04 ` [PATCH 13/16] amd64_edac: Check ECC capabilities initially Borislav Petkov
@ 2010-11-26 19:04 ` Borislav Petkov
  2010-11-26 19:04 ` [PATCH 15/16] EDAC: Fixup scrubrate manipulation Borislav Petkov
  2010-11-26 19:04 ` [PATCH 16/16] amd64_edac: Disable DRAM ECC injection on K8 Borislav Petkov
  15 siblings, 0 replies; 17+ messages in thread
From: Borislav Petkov @ 2010-11-26 19:04 UTC (permalink / raw
  To: norsk5; +Cc: linux-edac, linux-kernel, Borislav Petkov

From: Borislav Petkov <borislav.petkov@amd.com>

Now that all prerequisites are in place, drop the two-stage driver
instances initialization in favor of the following simple init sequence:

1. Probe PCI device: we only test ECC capabilities here and if none exit
early.

2. If the hw supports ECC and it is/can be enabled, we init the per-node
instance.

Remove "amd64_" prefix from static functions touched, while at it.

There actually should be no visible functional change resulting from
this patch.

Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
---
 drivers/edac/amd64_edac.c |  168 ++++++++++++++++++---------------------------
 1 files changed, 68 insertions(+), 100 deletions(-)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 2fe7725..4dcad97 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -15,9 +15,13 @@ module_param(ecc_enable_override, int, 0644);
 
 static struct msr __percpu *msrs;
 
+/*
+ * count successfully initialized driver instances for setup_pci_device()
+ */
+static atomic_t drv_instances = ATOMIC_INIT(0);
+
 /* Per-node driver instances */
 static struct mem_ctl_info **mcis;
-static struct amd64_pvt **pvts;
 static struct ecc_settings **ecc_stngs;
 
 /*
@@ -1993,8 +1997,7 @@ void amd64_decode_bus_error(int node_id, struct mce *m, u32 nbcfg)
  * Use pvt->F2 which contains the F2 CPU PCI device to get the related
  * F1 (AddrMap) and F3 (Misc) devices. Return negative value on error.
  */
-static int amd64_reserve_mc_sibling_devices(struct amd64_pvt *pvt, u16 f1_id,
-					    u16 f3_id)
+static int reserve_mc_sibling_devs(struct amd64_pvt *pvt, u16 f1_id, u16 f3_id)
 {
 	/* Reserve the ADDRESS MAP Device */
 	pvt->F1 = pci_get_related_function(pvt->F2->vendor, f1_id, pvt->F2);
@@ -2024,7 +2027,7 @@ static int amd64_reserve_mc_sibling_devices(struct amd64_pvt *pvt, u16 f1_id,
 	return 0;
 }
 
-static void amd64_free_mc_sibling_devices(struct amd64_pvt *pvt)
+static void free_mc_sibling_devs(struct amd64_pvt *pvt)
 {
 	pci_dev_put(pvt->F1);
 	pci_dev_put(pvt->F3);
@@ -2034,7 +2037,7 @@ static void amd64_free_mc_sibling_devices(struct amd64_pvt *pvt)
  * Retrieve the hardware registers of the memory controller (this includes the
  * 'Address Map' and 'Misc' device regs)
  */
-static void amd64_read_mc_registers(struct amd64_pvt *pvt)
+static void read_mc_regs(struct amd64_pvt *pvt)
 {
 	u64 msr_val;
 	u32 tmp;
@@ -2185,7 +2188,7 @@ static u32 amd64_csrow_nr_pages(int csrow_nr, struct amd64_pvt *pvt)
  * Initialize the array of csrow attribute instances, based on the values
  * from pci config hardware registers.
  */
-static int amd64_init_csrows(struct mem_ctl_info *mci)
+static int init_csrows(struct mem_ctl_info *mci)
 {
 	struct csrow_info *csrow;
 	struct amd64_pvt *pvt = mci->pvt_info;
@@ -2388,26 +2391,25 @@ static bool enable_ecc_error_reporting(struct ecc_settings *s, u8 nid,
 	return ret;
 }
 
-static void amd64_restore_ecc_error_reporting(struct amd64_pvt *pvt)
+static void restore_ecc_error_reporting(struct ecc_settings *s, u8 nid,
+					struct pci_dev *F3)
 {
-	u8 nid = pvt->mc_node_id;
-	struct ecc_settings *s = ecc_stngs[nid];
 	u32 value, mask = K8_NBCTL_CECCEn | K8_NBCTL_UECCEn;
 
 	if (!s->nbctl_valid)
 		return;
 
-	amd64_read_pci_cfg(pvt->F3, K8_NBCTL, &value);
+	amd64_read_pci_cfg(F3, K8_NBCTL, &value);
 	value &= ~mask;
 	value |= s->old_nbctl;
 
-	pci_write_config_dword(pvt->F3, K8_NBCTL, value);
+	pci_write_config_dword(F3, K8_NBCTL, value);
 
 	/* restore previous BIOS DRAM ECC "off" setting we force-enabled */
 	if (!s->flags.nb_ecc_prev) {
-		amd64_read_pci_cfg(pvt->F3, K8_NBCFG, &value);
+		amd64_read_pci_cfg(F3, K8_NBCFG, &value);
 		value &= ~K8_NBCFG_ECC_ENABLE;
-		pci_write_config_dword(pvt->F3, K8_NBCFG, value);
+		pci_write_config_dword(F3, K8_NBCFG, value);
 	}
 
 	/* restore the NB Enable MCGCTL bit */
@@ -2457,7 +2459,7 @@ struct mcidev_sysfs_attribute sysfs_attrs[ARRAY_SIZE(amd64_dbg_attrs) +
 
 struct mcidev_sysfs_attribute terminator = { .attr = { .name = NULL } };
 
-static void amd64_set_mc_sysfs_attributes(struct mem_ctl_info *mci)
+static void set_mc_sysfs_attrs(struct mem_ctl_info *mci)
 {
 	unsigned int i = 0, j = 0;
 
@@ -2472,7 +2474,7 @@ static void amd64_set_mc_sysfs_attributes(struct mem_ctl_info *mci)
 	mci->mc_driver_sysfs_attributes = sysfs_attrs;
 }
 
-static void amd64_setup_mci_misc_attributes(struct mem_ctl_info *mci)
+static void setup_mci_misc_attrs(struct mem_ctl_info *mci)
 {
 	struct amd64_pvt *pvt = mci->pvt_info;
 
@@ -2538,14 +2540,16 @@ static int amd64_init_one_instance(struct pci_dev *F2)
 {
 	struct amd64_pvt *pvt = NULL;
 	struct amd64_family_type *fam_type = NULL;
+	struct mem_ctl_info *mci = NULL;
 	int err = 0, ret;
+	u8 nid = get_node_id(F2);
 
 	ret = -ENOMEM;
 	pvt = kzalloc(sizeof(struct amd64_pvt), GFP_KERNEL);
 	if (!pvt)
-		goto err_exit;
+		goto err_ret;
 
-	pvt->mc_node_id	 = get_node_id(F2);
+	pvt->mc_node_id	= nid;
 	pvt->F2 = F2;
 
 	ret = -EINVAL;
@@ -2554,61 +2558,36 @@ static int amd64_init_one_instance(struct pci_dev *F2)
 		goto err_free;
 
 	ret = -ENODEV;
-	err = amd64_reserve_mc_sibling_devices(pvt, fam_type->f1_id,
-						fam_type->f3_id);
+	err = reserve_mc_sibling_devs(pvt, fam_type->f1_id, fam_type->f3_id);
 	if (err)
 		goto err_free;
 
-	/*
-	 * Save the pointer to the private data for use in 2nd initialization
-	 * stage
-	 */
-	pvts[pvt->mc_node_id] = pvt;
-
-	return 0;
-
-err_free:
-	kfree(pvt);
-
-err_exit:
-	return ret;
-}
-
-/*
- * This is the finishing stage of the init code. Needs to be performed after all
- * MCs' hardware have been prepped for accessing extended config space.
- */
-static int amd64_init_2nd_stage(struct amd64_pvt *pvt)
-{
-	int node_id = pvt->mc_node_id;
-	struct mem_ctl_info *mci;
-	int ret = -ENODEV;
-
-	amd64_read_mc_registers(pvt);
+	read_mc_regs(pvt);
 
 	/*
 	 * We need to determine how many memory channels there are. Then use
 	 * that information for calculating the size of the dynamic instance
-	 * tables in the 'mci' structure
+	 * tables in the 'mci' structure.
 	 */
+	ret = -EINVAL;
 	pvt->channel_count = pvt->ops->early_channel_count(pvt);
 	if (pvt->channel_count < 0)
-		goto err_exit;
+		goto err_siblings;
 
 	ret = -ENOMEM;
-	mci = edac_mc_alloc(0, pvt->cs_count, pvt->channel_count, node_id);
+	mci = edac_mc_alloc(0, pvt->cs_count, pvt->channel_count, nid);
 	if (!mci)
-		goto err_exit;
+		goto err_siblings;
 
 	mci->pvt_info = pvt;
-
 	mci->dev = &pvt->F2->dev;
-	amd64_setup_mci_misc_attributes(mci);
 
-	if (amd64_init_csrows(mci))
+	setup_mci_misc_attrs(mci);
+
+	if (init_csrows(mci))
 		mci->edac_cap = EDAC_FLAG_NONE;
 
-	amd64_set_mc_sysfs_attributes(mci);
+	set_mc_sysfs_attrs(mci);
 
 	ret = -ENODEV;
 	if (edac_mc_add_mc(mci)) {
@@ -2616,34 +2595,31 @@ static int amd64_init_2nd_stage(struct amd64_pvt *pvt)
 		goto err_add_mc;
 	}
 
-	mcis[node_id] = mci;
-	pvts[node_id] = NULL;
-
 	/* register stuff with EDAC MCE */
 	if (report_gart_errors)
 		amd_report_gart_errors(true);
 
 	amd_register_ecc_decoder(amd64_decode_bus_error);
 
+	mcis[nid] = mci;
+
+	atomic_inc(&drv_instances);
+
 	return 0;
 
 err_add_mc:
 	edac_mc_free(mci);
 
-err_exit:
-	debugf0("failure to init 2nd stage: ret=%d\n", ret);
+err_siblings:
+	free_mc_sibling_devs(pvt);
 
-	amd64_restore_ecc_error_reporting(pvt);
-
-	amd64_free_mc_sibling_devices(pvt);
-
-	kfree(pvts[pvt->mc_node_id]);
-	pvts[node_id] = NULL;
+err_free:
+	kfree(pvt);
 
+err_ret:
 	return ret;
 }
 
-
 static int __devinit amd64_probe_one_instance(struct pci_dev *pdev,
 					     const struct pci_device_id *mc_type)
 {
@@ -2678,8 +2654,10 @@ static int __devinit amd64_probe_one_instance(struct pci_dev *pdev,
 	}
 
 	ret = amd64_init_one_instance(pdev);
-	if (ret < 0)
+	if (ret < 0) {
 		amd64_err("Error probing instance: %d\n", nid);
+		restore_ecc_error_reporting(s, nid, F3);
+	}
 
 	return ret;
 
@@ -2695,6 +2673,9 @@ static void __devexit amd64_remove_one_instance(struct pci_dev *pdev)
 {
 	struct mem_ctl_info *mci;
 	struct amd64_pvt *pvt;
+	u8 nid = get_node_id(pdev);
+	struct pci_dev *F3 = node_to_amd_nb(nid)->misc;
+	struct ecc_settings *s = ecc_stngs[nid];
 
 	/* Remove from EDAC CORE tracking list */
 	mci = edac_mc_del_mc(&pdev->dev);
@@ -2703,20 +2684,20 @@ static void __devexit amd64_remove_one_instance(struct pci_dev *pdev)
 
 	pvt = mci->pvt_info;
 
-	amd64_restore_ecc_error_reporting(pvt);
+	restore_ecc_error_reporting(s, nid, F3);
 
-	amd64_free_mc_sibling_devices(pvt);
+	free_mc_sibling_devs(pvt);
 
 	/* unregister from EDAC MCE */
 	amd_report_gart_errors(false);
 	amd_unregister_ecc_decoder(amd64_decode_bus_error);
 
-	kfree(ecc_stngs[pvt->mc_node_id]);
-	ecc_stngs[pvt->mc_node_id] = NULL;
+	kfree(ecc_stngs[nid]);
+	ecc_stngs[nid] = NULL;
 
 	/* Free the EDAC CORE resources */
 	mci->pvt_info = NULL;
-	mcis[pvt->mc_node_id] = NULL;
+	mcis[nid] = NULL;
 
 	kfree(pvt);
 	edac_mc_free(mci);
@@ -2755,7 +2736,7 @@ static struct pci_driver amd64_pci_driver = {
 	.id_table	= amd64_pci_table,
 };
 
-static void amd64_setup_pci_device(void)
+static void setup_pci_device(void)
 {
 	struct mem_ctl_info *mci;
 	struct amd64_pvt *pvt;
@@ -2782,8 +2763,7 @@ static void amd64_setup_pci_device(void)
 
 static int __init amd64_edac_init(void)
 {
-	int nb, err = -ENODEV;
-	bool load_ok = false;
+	int err = -ENODEV;
 
 	edac_printk(KERN_INFO, EDAC_MOD_STR, EDAC_AMD64_VERSION "\n");
 
@@ -2793,49 +2773,40 @@ static int __init amd64_edac_init(void)
 		goto err_ret;
 
 	err = -ENOMEM;
-	pvts	  = kzalloc(amd_nb_num() * sizeof(pvts[0]), GFP_KERNEL);
 	mcis	  = kzalloc(amd_nb_num() * sizeof(mcis[0]), GFP_KERNEL);
 	ecc_stngs = kzalloc(amd_nb_num() * sizeof(ecc_stngs[0]), GFP_KERNEL);
-	if (!(pvts && mcis && ecc_stngs))
+	if (!(mcis && ecc_stngs))
 		goto err_ret;
 
 	msrs = msrs_alloc();
 	if (!msrs)
-		goto err_ret;
+		goto err_free;
 
 	err = pci_register_driver(&amd64_pci_driver);
 	if (err)
 		goto err_pci;
 
-	/*
-	 * At this point, the array 'pvts[]' contains pointers to alloc'd
-	 * amd64_pvt structs. These will be used in the 2nd stage init function
-	 * to finish initialization of the MC instances.
-	 */
 	err = -ENODEV;
-	for (nb = 0; nb < amd_nb_num(); nb++) {
-		if (!pvts[nb])
-			continue;
-
-		err = amd64_init_2nd_stage(pvts[nb]);
-		if (err)
-			goto err_2nd_stage;
-
-		load_ok = true;
-	}
+	if (!atomic_read(&drv_instances))
+		goto err_no_instances;
 
-	if (load_ok) {
-		amd64_setup_pci_device();
-		return 0;
-	}
+	setup_pci_device();
+	return 0;
 
-err_2nd_stage:
+err_no_instances:
 	pci_unregister_driver(&amd64_pci_driver);
 
 err_pci:
 	msrs_free(msrs);
 	msrs = NULL;
 
+err_free:
+	kfree(mcis);
+	mcis = NULL;
+
+	kfree(ecc_stngs);
+	ecc_stngs = NULL;
+
 err_ret:
 	return err;
 }
@@ -2853,9 +2824,6 @@ static void __exit amd64_edac_exit(void)
 	kfree(mcis);
 	mcis = NULL;
 
-	kfree(pvts);
-	pvts = NULL;
-
 	msrs_free(msrs);
 	msrs = NULL;
 }
-- 
1.7.3.1.50.g1e633


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 15/16] EDAC: Fixup scrubrate manipulation
  2010-11-26 19:04 [PATCH 00/16] amd64_edac stuff for .38 Borislav Petkov
                   ` (13 preceding siblings ...)
  2010-11-26 19:04 ` [PATCH 14/16] amd64_edac: Remove two-stage initialization Borislav Petkov
@ 2010-11-26 19:04 ` Borislav Petkov
  2010-11-26 19:04 ` [PATCH 16/16] amd64_edac: Disable DRAM ECC injection on K8 Borislav Petkov
  15 siblings, 0 replies; 17+ messages in thread
From: Borislav Petkov @ 2010-11-26 19:04 UTC (permalink / raw
  To: norsk5; +Cc: linux-edac, linux-kernel, Borislav Petkov

From: Borislav Petkov <borislav.petkov@amd.com>

Make the ->{get|set}_sdram_scrub_rate return the actual scrub rate
bandwidth it succeded setting and remove superfluous arg pointer used
for that. A negative value returned still means that an error occurred
while setting the scrubrate. Document this for future reference.

Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
---
 drivers/edac/amd64_edac.c    |   24 +++++++++---------
 drivers/edac/amd64_edac.h    |    6 ----
 drivers/edac/cpc925_edac.c   |    9 +++---
 drivers/edac/e752x_edac.c    |    8 ++---
 drivers/edac/edac_core.h     |    2 +-
 drivers/edac/edac_mc_sysfs.c |   57 ++++++++++++++++++++---------------------
 drivers/edac/i5100_edac.c    |    9 ++----
 7 files changed, 52 insertions(+), 63 deletions(-)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 4dcad97..f19c34e 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -77,7 +77,11 @@ static int ddr3_dbam[] = { [0]		= -1,
  *FIXME: Produce a better mapping/linearisation.
  */
 
-struct scrubrate scrubrates[] = {
+
+struct scrubrate {
+       u32 scrubval;           /* bit pattern for scrub rate */
+       u32 bandwidth;          /* bandwidth consumed (bytes/sec) */
+} scrubrates[] = {
 	{ 0x01, 1600000000UL},
 	{ 0x02, 800000000UL},
 	{ 0x03, 400000000UL},
@@ -151,14 +155,12 @@ static int __amd64_set_scrub_rate(struct pci_dev *ctl, u32 new_bw, u32 min_rate)
 	}
 
 	scrubval = scrubrates[i].scrubval;
-	if (scrubval)
-		amd64_info("Setting scrub rate bandwidth: %u\n",
-			   scrubrates[i].bandwidth);
-	else
-		amd64_info("Turning scrubbing off.\n");
 
 	pci_write_bits32(ctl, K8_SCRCTRL, scrubval, 0x001F);
 
+	if (scrubval)
+		return scrubrates[i].bandwidth;
+
 	return 0;
 }
 
@@ -169,11 +171,11 @@ static int amd64_set_scrub_rate(struct mem_ctl_info *mci, u32 bw)
 	return __amd64_set_scrub_rate(pvt->F3, bw, pvt->min_scrubrate);
 }
 
-static int amd64_get_scrub_rate(struct mem_ctl_info *mci, u32 *bw)
+static int amd64_get_scrub_rate(struct mem_ctl_info *mci)
 {
 	struct amd64_pvt *pvt = mci->pvt_info;
 	u32 scrubval = 0;
-	int status = -1, i;
+	int i, retval = -EINVAL;
 
 	amd64_read_pci_cfg(pvt->F3, K8_SCRCTRL, &scrubval);
 
@@ -183,13 +185,11 @@ static int amd64_get_scrub_rate(struct mem_ctl_info *mci, u32 *bw)
 
 	for (i = 0; i < ARRAY_SIZE(scrubrates); i++) {
 		if (scrubrates[i].scrubval == scrubval) {
-			*bw = scrubrates[i].bandwidth;
-			status = 0;
+			retval = scrubrates[i].bandwidth;
 			break;
 		}
 	}
-
-	return status;
+	return retval;
 }
 
 /* Map from a CSROW entry to the mask entry that operates on it */
diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
index b76dce9..613ec72 100644
--- a/drivers/edac/amd64_edac.h
+++ b/drivers/edac/amd64_edac.h
@@ -482,12 +482,6 @@ struct ecc_settings {
 	} flags;
 };
 
-struct scrubrate {
-       u32 scrubval;           /* bit pattern for scrub rate */
-       u32 bandwidth;          /* bandwidth consumed (bytes/sec) */
-};
-
-extern struct scrubrate scrubrates[23];
 extern const char *tt_msgs[4];
 extern const char *ll_msgs[4];
 extern const char *rrrr_msgs[16];
diff --git a/drivers/edac/cpc925_edac.c b/drivers/edac/cpc925_edac.c
index 1609a19..b9a781c 100644
--- a/drivers/edac/cpc925_edac.c
+++ b/drivers/edac/cpc925_edac.c
@@ -818,9 +818,10 @@ static void cpc925_del_edac_devices(void)
 }
 
 /* Convert current back-ground scrub rate into byte/sec bandwith */
-static int cpc925_get_sdram_scrub_rate(struct mem_ctl_info *mci, u32 *bw)
+static int cpc925_get_sdram_scrub_rate(struct mem_ctl_info *mci)
 {
 	struct cpc925_mc_pdata *pdata = mci->pvt_info;
+	int bw;
 	u32 mscr;
 	u8 si;
 
@@ -832,11 +833,11 @@ static int cpc925_get_sdram_scrub_rate(struct mem_ctl_info *mci, u32 *bw)
 	if (((mscr & MSCR_SCRUB_MOD_MASK) != MSCR_BACKGR_SCRUB) ||
 	    (si == 0)) {
 		cpc925_mc_printk(mci, KERN_INFO, "Scrub mode not enabled\n");
-		*bw = 0;
+		bw = 0;
 	} else
-		*bw = CPC925_SCRUB_BLOCK_SIZE * 0xFA67 / si;
+		bw = CPC925_SCRUB_BLOCK_SIZE * 0xFA67 / si;
 
-	return 0;
+	return bw;
 }
 
 /* Return 0 for single channel; 1 for dual channel */
diff --git a/drivers/edac/e752x_edac.c b/drivers/edac/e752x_edac.c
index 073f5a0..ec302d4 100644
--- a/drivers/edac/e752x_edac.c
+++ b/drivers/edac/e752x_edac.c
@@ -983,11 +983,11 @@ static int set_sdram_scrub_rate(struct mem_ctl_info *mci, u32 new_bw)
 
 	pci_write_config_word(pdev, E752X_MCHSCRB, scrubrates[i].scrubval);
 
-	return 0;
+	return scrubrates[i].bandwidth;
 }
 
 /* Convert current scrub rate value into byte/sec bandwidth */
-static int get_sdram_scrub_rate(struct mem_ctl_info *mci, u32 *bw)
+static int get_sdram_scrub_rate(struct mem_ctl_info *mci)
 {
 	const struct scrubrate *scrubrates;
 	struct e752x_pvt *pvt = (struct e752x_pvt *) mci->pvt_info;
@@ -1013,10 +1013,8 @@ static int get_sdram_scrub_rate(struct mem_ctl_info *mci, u32 *bw)
 			"Invalid sdram scrub control value: 0x%x\n", scrubval);
 		return -1;
 	}
+	return scrubrates[i].bandwidth;
 
-	*bw = scrubrates[i].bandwidth;
-
-	return 0;
 }
 
 /* Return 1 if dual channel mode is active.  Else return 0. */
diff --git a/drivers/edac/edac_core.h b/drivers/edac/edac_core.h
index 050f9ee..25d8f97 100644
--- a/drivers/edac/edac_core.h
+++ b/drivers/edac/edac_core.h
@@ -387,7 +387,7 @@ struct mem_ctl_info {
 	   representation and converts it to the closest matching
 	   bandwith in bytes/sec.
 	 */
-	int (*get_sdram_scrub_rate) (struct mem_ctl_info * mci, u32 * bw);
+	int (*get_sdram_scrub_rate) (struct mem_ctl_info * mci);
 
 
 	/* pointer to edac checking routine */
diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c
index dce61f7..39d97cf 100644
--- a/drivers/edac/edac_mc_sysfs.c
+++ b/drivers/edac/edac_mc_sysfs.c
@@ -436,56 +436,55 @@ static ssize_t mci_reset_counters_store(struct mem_ctl_info *mci,
 	return count;
 }
 
-/* memory scrubbing */
+/* Memory scrubbing interface:
+ *
+ * A MC driver can limit the scrubbing bandwidth based on the CPU type.
+ * Therefore, ->set_sdram_scrub_rate should be made to return the actual
+ * bandwidth that is accepted or 0 when scrubbing is to be disabled.
+ *
+ * Negative value still means that an error has occurred while setting
+ * the scrub rate.
+ */
 static ssize_t mci_sdram_scrub_rate_store(struct mem_ctl_info *mci,
 					  const char *data, size_t count)
 {
 	unsigned long bandwidth = 0;
-	int err;
+	int new_bw = 0;
 
-	if (!mci->set_sdram_scrub_rate) {
-		edac_printk(KERN_WARNING, EDAC_MC,
-			    "Memory scrub rate setting not implemented!\n");
+	if (!mci->set_sdram_scrub_rate)
 		return -EINVAL;
-	}
 
 	if (strict_strtoul(data, 10, &bandwidth) < 0)
 		return -EINVAL;
 
-	err = mci->set_sdram_scrub_rate(mci, (u32)bandwidth);
-	if (err) {
-		edac_printk(KERN_DEBUG, EDAC_MC,
-			    "Failed setting scrub rate to %lu\n", bandwidth);
-		return -EINVAL;
-	}
-	else {
-		edac_printk(KERN_DEBUG, EDAC_MC,
-			    "Scrub rate set to: %lu\n", bandwidth);
+	new_bw = mci->set_sdram_scrub_rate(mci, bandwidth);
+	if (new_bw >= 0) {
+		edac_printk(KERN_DEBUG, EDAC_MC, "Scrub rate set to %d\n", new_bw);
 		return count;
 	}
+
+	edac_printk(KERN_DEBUG, EDAC_MC, "Error setting scrub rate to: %lu\n", bandwidth);
+	return -EINVAL;
 }
 
+/*
+ * ->get_sdram_scrub_rate() return value semantics same as above.
+ */
 static ssize_t mci_sdram_scrub_rate_show(struct mem_ctl_info *mci, char *data)
 {
-	u32 bandwidth = 0;
-	int err;
+	int bandwidth = 0;
 
-	if (!mci->get_sdram_scrub_rate) {
-		edac_printk(KERN_WARNING, EDAC_MC,
-			    "Memory scrub rate reading not implemented\n");
+	if (!mci->get_sdram_scrub_rate)
 		return -EINVAL;
-	}
 
-	err = mci->get_sdram_scrub_rate(mci, &bandwidth);
-	if (err) {
+	bandwidth = mci->get_sdram_scrub_rate(mci);
+	if (bandwidth < 0) {
 		edac_printk(KERN_DEBUG, EDAC_MC, "Error reading scrub rate\n");
-		return err;
-	}
-	else {
-		edac_printk(KERN_DEBUG, EDAC_MC,
-			    "Read scrub rate: %d\n", bandwidth);
-		return sprintf(data, "%d\n", bandwidth);
+		return bandwidth;
 	}
+
+	edac_printk(KERN_DEBUG, EDAC_MC, "Read scrub rate: %d\n", bandwidth);
+	return sprintf(data, "%d\n", bandwidth);
 }
 
 /* default attribute files for the MCI object */
diff --git a/drivers/edac/i5100_edac.c b/drivers/edac/i5100_edac.c
index f459a6c..0448da0 100644
--- a/drivers/edac/i5100_edac.c
+++ b/drivers/edac/i5100_edac.c
@@ -611,20 +611,17 @@ static int i5100_set_scrub_rate(struct mem_ctl_info *mci, u32 bandwidth)
 
 	bandwidth = 5900000 * i5100_mc_scrben(dw);
 
-	return 0;
+	return bandwidth;
 }
 
-static int i5100_get_scrub_rate(struct mem_ctl_info *mci,
-				u32 *bandwidth)
+static int i5100_get_scrub_rate(struct mem_ctl_info *mci)
 {
 	struct i5100_priv *priv = mci->pvt_info;
 	u32 dw;
 
 	pci_read_config_dword(priv->mc, I5100_MC, &dw);
 
-	*bandwidth = 5900000 * i5100_mc_scrben(dw);
-
-	return 0;
+	return 5900000 * i5100_mc_scrben(dw);
 }
 
 static struct pci_dev *pci_get_device_func(unsigned vendor,
-- 
1.7.3.1.50.g1e633


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 16/16] amd64_edac: Disable DRAM ECC injection on K8
  2010-11-26 19:04 [PATCH 00/16] amd64_edac stuff for .38 Borislav Petkov
                   ` (14 preceding siblings ...)
  2010-11-26 19:04 ` [PATCH 15/16] EDAC: Fixup scrubrate manipulation Borislav Petkov
@ 2010-11-26 19:04 ` Borislav Petkov
  15 siblings, 0 replies; 17+ messages in thread
From: Borislav Petkov @ 2010-11-26 19:04 UTC (permalink / raw
  To: norsk5; +Cc: linux-edac, linux-kernel, Borislav Petkov

From: Borislav Petkov <borislav.petkov@amd.com>

K8 does not allow for an atomic RMW to a cacheline as GH does so disable
the error injection interface for it.

Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
---
 drivers/edac/amd64_edac.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index f19c34e..3f7e886 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -2466,8 +2466,9 @@ static void set_mc_sysfs_attrs(struct mem_ctl_info *mci)
 	for (; i < ARRAY_SIZE(amd64_dbg_attrs); i++)
 		sysfs_attrs[i] = amd64_dbg_attrs[i];
 
-	for (j = 0; j < ARRAY_SIZE(amd64_inj_attrs); j++, i++)
-		sysfs_attrs[i] = amd64_inj_attrs[j];
+	if (boot_cpu_data.x86 >= 0x10)
+		for (j = 0; j < ARRAY_SIZE(amd64_inj_attrs); j++, i++)
+			sysfs_attrs[i] = amd64_inj_attrs[j];
 
 	sysfs_attrs[i] = terminator;
 
-- 
1.7.3.1.50.g1e633


^ permalink raw reply related	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2010-11-26 19:08 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-11-26 19:04 [PATCH 00/16] amd64_edac stuff for .38 Borislav Petkov
2010-11-26 19:04 ` [PATCH 01/16] amd64_edac: Remove F11h support Borislav Petkov
2010-11-26 19:04 ` [PATCH 02/16] amd64_edac: Use cached extended CPU model Borislav Petkov
2010-11-26 19:04 ` [PATCH 03/16] amd64_edac: Add per-family init function Borislav Petkov
2010-11-26 19:04 ` [PATCH 04/16] amd64_edac: Simplify CPU family detection Borislav Petkov
2010-11-26 19:04 ` [PATCH 05/16] amd64_edac: Cleanup the CPU PCI device reservation Borislav Petkov
2010-11-26 19:04 ` [PATCH 06/16] amd64_edac: Concentrate per-family init even more Borislav Petkov
2010-11-26 19:04 ` [PATCH 07/16] amd64_edac: Rename CPU PCI devices Borislav Petkov
2010-11-26 19:04 ` [PATCH 08/16] amd64_edac: Rework printk macros Borislav Petkov
2010-11-26 19:04 ` [PATCH 09/16] amd64_edac: Allocate driver instances dynamically Borislav Petkov
2010-11-26 19:04 ` [PATCH 10/16] amd64_edac: Remove explicit Kconfig PCI dependency Borislav Petkov
2010-11-26 19:04 ` [PATCH 11/16] amd64_edac: Remove PCI ECS enabling functions Borislav Petkov
2010-11-26 19:04 ` [PATCH 12/16] amd64_edac: Carve out ECC-related hw settings Borislav Petkov
2010-11-26 19:04 ` [PATCH 13/16] amd64_edac: Check ECC capabilities initially Borislav Petkov
2010-11-26 19:04 ` [PATCH 14/16] amd64_edac: Remove two-stage initialization Borislav Petkov
2010-11-26 19:04 ` [PATCH 15/16] EDAC: Fixup scrubrate manipulation Borislav Petkov
2010-11-26 19:04 ` [PATCH 16/16] amd64_edac: Disable DRAM ECC injection on K8 Borislav Petkov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).