linux-kernel - [PATCH v3 06/13] EDAC/synopsys: Add data poisoning disable support

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]
Date:   Fri, 30 Sep 2022 02:41:14 +0300
From:   Serge Semin <Sergey.Semin@...kalelectronics.ru>
To:     Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Michal Simek <michal.simek@...inx.com>,
        Borislav Petkov <bp@...en8.de>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Tony Luck <tony.luck@...el.com>,
        James Morse <james.morse@....com>,
        Robert Richter <rric@...nel.org>
CC:     Serge Semin <Sergey.Semin@...kalelectronics.ru>,
        Serge Semin <fancer.lancer@...il.com>,
        Alexey Malahov <Alexey.Malahov@...kalelectronics.ru>,
        Michail Ivanov <Michail.Ivanov@...kalelectronics.ru>,
        Pavel Parkhomenko <Pavel.Parkhomenko@...kalelectronics.ru>,
        Punnaiah Choudary Kalluri 
        <punnaiah.choudary.kalluri@...inx.com>,
        Manish Narani <manish.narani@...inx.com>,
        Dinh Nguyen <dinguyen@...nel.org>,
        Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        <devicetree@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-edac@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: [PATCH v3 06/13] EDAC/synopsys: Add data poisoning disable support

Even though being a pure-debug feature currently the data poison can't be
disabled once it has been initialized and enabled. Irrespective to the way
the feature has been implemented it doesn't seem right since the system
may print false ECC errors in case if the poisoned address is accessed by
the kernel or by the user-space applications. It's possible since the
poisoned address isn't reserved in any kernel mm subsystems. Even though
that doesn't seem right either at least it's tolerable since the ECC data
poison is supposed to be utilized in the framework of the EDAC driver
debugging, but having the feature unswitchable can't be justified that
easy especially seeing it's not that hard to implement.

So in order to have the ECC data poison switchable we suggest to define
three possible values acceptable by the "inject_data_poison" DebugFS node:
1. "CE" - emit correctable error (as before).
2. "UE" - emit uncorrectable error (used to be any non-"CE" value).
3. Any other value - disable data poison feature.

Note we have to redefine the macros describing the data poison-related
fields of the ECC_CFG0 register in a way so they would be used to
separately switch the feature on/off and to select the type of the ECC
error. As a result the suggest solution turns into a proper ECC_CFG0 CSRs
fields setup based on the value written to the "inject_data_poison"
DebugFS node.

Signed-off-by: Serge Semin <Sergey.Semin@...kalelectronics.ru>
---
 drivers/edac/synopsys_edac.c | 29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/drivers/edac/synopsys_edac.c b/drivers/edac/synopsys_edac.c
index 8771d69f6c5a..1df5be2af1de 100644
--- a/drivers/edac/synopsys_edac.c
+++ b/drivers/edac/synopsys_edac.c
@@ -118,6 +118,10 @@
 #define ECC_CFG0_DIS_SCRUB		BIT(4)
 #define ECC_CFG0_MODE_MASK		GENMASK(2, 0)
 
+/* ECC CFG1 register definitions */
+#define ECC_CFG1_POISON_BIT		BIT(1)
+#define ECC_CFG1_POISON_EN		BIT(0)
+
 /* ECC status register definitions */
 #define ECC_STAT_UE_MASK		GENMASK(23, 16)
 #define ECC_STAT_CE_MASK		GENMASK(15, 8)
@@ -157,10 +161,6 @@
 #define ECC_POISON1_BANK_MASK		GENMASK(26, 24)
 #define ECC_POISON1_ROW_MASK		GENMASK(17, 0)
 
-/* DDRC ECC CE & UE poison mask */
-#define ECC_CEPOISON_MASK		GENMASK(1, 0)
-#define ECC_UEPOISON_MASK		BIT(0)
-
 /* DDRC address mapping parameters */
 #define DDR_ADDRMAP_NREGS		12
 
@@ -1781,10 +1781,14 @@ static ssize_t snps_inject_data_poison_read(struct file *filep, char __user *ubu
 	int pos;
 
 	regval = readl(priv->baseaddr + ECC_CFG1_OFST);
-	errstr = FIELD_GET(ECC_CEPOISON_MASK, regval) == ECC_CEPOISON_MASK ?
-		 "Correctable Error" : "UnCorrectable Error";
+	if (!(regval & ECC_CFG1_POISON_EN))
+		errstr = "Off";
+	else if (regval & ECC_CFG1_POISON_BIT)
+		errstr = "CE";
+	else
+		errstr = "UE";
 
-	pos = scnprintf(buf, sizeof(buf), "Data Poisoning: %s\n\r", errstr);
+	pos = scnprintf(buf, sizeof(buf), "%s\n", errstr);
 
 	return simple_read_from_buffer(ubuf, size, offp, buf, pos);
 }
@@ -1795,6 +1799,7 @@ static ssize_t snps_inject_data_poison_write(struct file *filep, const char __us
 	struct mem_ctl_info *mci = filep->private_data;
 	struct snps_edac_priv *priv = mci->pvt_info;
 	char buf[SNPS_DBGFS_BUF_LEN];
+	u32 regval;
 	int rc;
 
 	rc = simple_write_to_buffer(buf, sizeof(buf), offp, ubuf, size);
@@ -1802,10 +1807,16 @@ static ssize_t snps_inject_data_poison_write(struct file *filep, const char __us
 		return rc;
 
 	writel(0, priv->baseaddr + DDR_SWCTL);
+
+	regval = readl(priv->baseaddr + ECC_CFG1_OFST);
 	if (strncmp(buf, "CE", 2) == 0)
-		writel(ECC_CEPOISON_MASK, priv->baseaddr + ECC_CFG1_OFST);
+		regval |= ECC_CFG1_POISON_BIT | ECC_CFG1_POISON_EN;
+	else if (strncmp(buf, "UE", 2) == 0)
+		regval = (regval & ~ECC_CFG1_POISON_BIT) | ECC_CFG1_POISON_EN;
 	else
-		writel(ECC_UEPOISON_MASK, priv->baseaddr + ECC_CFG1_OFST);
+		regval &= ~ECC_CFG1_POISON_EN;
+	writel(regval, priv->baseaddr + ECC_CFG1_OFST);
+
 	writel(1, priv->baseaddr + DDR_SWCTL);
 
 	return size;
-- 
2.37.3