linux-kernel - [PATCH v2 11/22] EDAC/amd64: Rework hw_info

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230127170419.1824692-12-yazen.ghannam@amd.com>
Date:   Fri, 27 Jan 2023 17:04:08 +0000
From:   Yazen Ghannam <yazen.ghannam@....com>
To:     <bp@...en8.de>, <linux-edac@...r.kernel.org>
CC:     <linux-kernel@...r.kernel.org>, <muralidhara.mk@....com>,
        <naveenkrishna.chatradhi@....com>,
        Yazen Ghannam <yazen.ghannam@....com>
Subject: [PATCH v2 11/22] EDAC/amd64: Rework hw_info_{get,put}

The bulk of system-specific information is gathered at init time with
hw_info_get(). This function calls a number of helper functions, and
many of these helper functions are split between a modern UMC/DF path
and a legacy DCT path.

Split hw_info_get() into legacy and modern versions. This creates two
separate code paths early on, and legacy and modern helper functions can
be called directly in the appropriate code path.

Also, simplify hw_info_put() and share it between legacy and modern
systems. NULL pointer checks are done in pci_dev_put() and kfree(), so
they can be called unconditionally.

Signed-off-by: Yazen Ghannam <yazen.ghannam@....com>
---
Link:
https://lore.kernel.org/r/Yn0a9T9xqAkWnPWt@yaz-fattaah

v1->v2:
* New in v2.

 drivers/edac/amd64_edac.c | 78 +++++++++++++++++----------------------
 drivers/edac/amd64_edac.h |  1 +
 2 files changed, 34 insertions(+), 45 deletions(-)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index a9b20bc413af..3830b0a4b5dc 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -3045,9 +3045,6 @@ static void decode_umc_error(int node_id, struct mce *m)
 static int
 reserve_mc_sibling_devs(struct amd64_pvt *pvt, u16 pci_id1, u16 pci_id2)
 {
-	if (pvt->umc)
-		return 0;
-
 	/* Reserve the ADDRESS MAP Device */
 	pvt->F1 = pci_get_related_function(pvt->F3->vendor, pci_id1, pvt->F3);
 	if (!pvt->F1) {
@@ -3075,16 +3072,6 @@ reserve_mc_sibling_devs(struct amd64_pvt *pvt, u16 pci_id1, u16 pci_id2)
 	return 0;
 }
 
-static void free_mc_sibling_devs(struct amd64_pvt *pvt)
-{
-	if (pvt->umc) {
-		return;
-	} else {
-		pci_dev_put(pvt->F1);
-		pci_dev_put(pvt->F2);
-	}
-}
-
 static void determine_ecc_sym_sz(struct amd64_pvt *pvt)
 {
 	pvt->ecc_sym_sz = 4;
@@ -3671,13 +3658,45 @@ static void setup_mci_misc_attrs(struct mem_ctl_info *mci)
 	mci->get_sdram_scrub_rate = get_scrub_rate;
 }
 
+static int dct_hw_info_get(struct amd64_pvt *pvt)
+{
+	int ret = reserve_mc_sibling_devs(pvt, pvt->f1_id, pvt->f2_id);
+
+	if (ret)
+		return ret;
+
+	read_mc_regs(pvt);
+
+	return 0;
+}
+
+static int umc_hw_info_get(struct amd64_pvt *pvt)
+{
+	pvt->umc = kcalloc(pvt->max_mcs, sizeof(struct amd64_umc), GFP_KERNEL);
+	if (!pvt->umc)
+		return -ENOMEM;
+
+	read_mc_regs(pvt);
+
+	return 0;
+}
+
+static void hw_info_put(struct amd64_pvt *pvt)
+{
+	pci_dev_put(pvt->F1);
+	pci_dev_put(pvt->F2);
+	kfree(pvt->umc);
+}
+
 static struct low_ops umc_ops = {
+	.hw_info_get			= umc_hw_info_get,
 };
 
 /* Use Family 16h versions for defaults and adjust as needed below. */
 static struct low_ops dct_ops = {
 	.map_sysaddr_to_csrow		= f1x_map_sysaddr_to_csrow,
 	.dbam_to_cs			= f16_dbam_to_chip_select,
+	.hw_info_get			= dct_hw_info_get,
 };
 
 static int per_family_init(struct amd64_pvt *pvt)
@@ -3820,37 +3839,6 @@ static const struct attribute_group *amd64_edac_attr_groups[] = {
 	NULL
 };
 
-static int hw_info_get(struct amd64_pvt *pvt)
-{
-	u16 pci_id1, pci_id2;
-	int ret;
-
-	if (pvt->fam >= 0x17) {
-		pvt->umc = kcalloc(pvt->max_mcs, sizeof(struct amd64_umc), GFP_KERNEL);
-		if (!pvt->umc)
-			return -ENOMEM;
-	} else {
-		pci_id1 = pvt->f1_id;
-		pci_id2 = pvt->f2_id;
-	}
-
-	ret = reserve_mc_sibling_devs(pvt, pci_id1, pci_id2);
-	if (ret)
-		return ret;
-
-	read_mc_regs(pvt);
-
-	return 0;
-}
-
-static void hw_info_put(struct amd64_pvt *pvt)
-{
-	if (pvt->F1)
-		free_mc_sibling_devs(pvt);
-
-	kfree(pvt->umc);
-}
-
 static int init_one_instance(struct amd64_pvt *pvt)
 {
 	struct mem_ctl_info *mci = NULL;
@@ -3924,7 +3912,7 @@ static int probe_one_instance(unsigned int nid)
 	if (ret < 0)
 		goto err_enable;
 
-	ret = hw_info_get(pvt);
+	ret = pvt->ops->hw_info_get(pvt);
 	if (ret < 0)
 		goto err_enable;
 
diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
index 8eea15546b9f..00b3f32e3cbb 100644
--- a/drivers/edac/amd64_edac.h
+++ b/drivers/edac/amd64_edac.h
@@ -466,6 +466,7 @@ struct low_ops {
 				     struct err_info *err);
 	int  (*dbam_to_cs)(struct amd64_pvt *pvt, u8 dct,
 			   unsigned int cs_mode, int cs_mask_nr);
+	int (*hw_info_get)(struct amd64_pvt *pvt);
 };
 
 int __amd64_read_pci_cfg_dword(struct pci_dev *pdev, int offset,
-- 
2.25.1