Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Kiwoong Kim <kwmad.kim@samsung.com>
To:     linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org,
        alim.akhtar@samsung.com, avri.altman@wdc.com, jejb@linux.ibm.com,
        martin.petersen@oracle.com, beanhuo@micron.com,
        cang@codeaurora.org, adrian.hunter@intel.com, sc.suh@samsung.com,
        hy50.seo@samsung.com, sh425.lee@samsung.com,
        bhoon95.kim@samsung.com
Cc:     Kiwoong Kim <kwmad.kim@samsung.com>
Subject: [PATCH v2 3/3] scsi: ufs: ufs-exynos: implement exynos isr
Date:   Mon, 13 Sep 2021 16:55:55 +0900
Message-Id: <746e059782953fca6c21945297151d2bb73d3370.1631519695.git.kwmad.kim@samsung.com>
In-Reply-To: <cover.1631519695.git.kwmad.kim@samsung.com>
In-Reply-To: <cover.1631519695.git.kwmad.kim@samsung.com>
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 102P
DLP-Filter: Pass
References: <cover.1631519695.git.kwmad.kim@samsung.com>
        <CGME20210913081152epcas2p2eac4a8dbef33164a150dccf2e282dcce@epcas2p2.samsung.com>
Precedence: bulk

This patch is to raise recovery in some abnormal
conditions using an vendor specific interrupt
for some cases, such as a situation that some
contexts of a pending request in the host isn't
the same with those of its corresponding UPIUs
if they should have been the same exactly.

The representative case is shown like below.
In the case, a broken UTRD entry, for internal
coherent problem or whatever, that had smaller value
of PRDT length than expected was transferred to the host.
So, the host raised an interrupt of transfer complete
even if device didn't finish its data transfer because
the host sees a fetched version of UTRD to determine
if data tranfer is over or not. Then the application level
seemed to recogize this as a sort of corruption and this
symptom led to boot failure.

[5:   init:    1] init: Failed to copy /avb into /metadata/gsi/dsu/avb/: No such file or directory
[5:   init:    1] init: [libfs_mgr]Created logical partition system on device /dev/block/dm-0
[4:   init:    1] init: [libfs_mgr]Created logical partition vendor on device /dev/block/dm-1
[4:   init:    1] init: [libfs_avb]total vbmeta size mismatch: 13504 (expected: 15616)
[4:   init:    1] init: [libfs_avb]VerifyVbmetaImages failed
[4:   init:    1] init: Failed to open AvbHandle: No such file or directory

If the issued command would be write, it could be a
disaster because some parts of its entire data chunk
may be not written or more than the chunk may be written.

Using the VENDOR_SPECIFIC_IS.RX_UPIU_HIT_ERROR, the host
can detect the abnormal condition right when an
DATA IN for offset bigger than expected for the broken value
(i.e. PRDT length in this case) is received, do something for
recovery and system will keep going safe.

Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com>
---
 drivers/scsi/ufs/ufs-exynos.c | 84 ++++++++++++++++++++++++++++++++++++-------
 1 file changed, 72 insertions(+), 12 deletions(-)

diff --git a/drivers/scsi/ufs/ufs-exynos.c b/drivers/scsi/ufs/ufs-exynos.c
index a14dd8c..b0c8843 100644
--- a/drivers/scsi/ufs/ufs-exynos.c
+++ b/drivers/scsi/ufs/ufs-exynos.c
@@ -21,6 +21,7 @@
 #include "unipro.h"
 
 #include "ufs-exynos.h"
+#include "../scsi_transport_api.h"
 
 /*
  * Exynos's Vendor specific registers for UFSHCI
@@ -31,6 +32,8 @@
 #define HCI_RXPRDT_ENTRY_SIZE	0x04
 #define HCI_1US_TO_CNT_VAL	0x0C
 #define CNT_VAL_1US_MASK	0x3FF
+#define VENDOR_SPECIFIC_IS	0x38
+#define VENDOR_SPECIFIC_IE	0x3C
 #define HCI_UTRL_NEXUS_TYPE	0x40
 #define HCI_UTMRL_NEXUS_TYPE	0x44
 #define HCI_SW_RST		0x50
@@ -74,6 +77,18 @@
 				 UIC_TRANSPORT_NO_CONNECTION_RX |\
 				 UIC_TRANSPORT_BAD_TC)
 
+enum exynos_ufs_vs_interrupt {
+	/*
+	 * This occurs when information of a pending request isn't
+	 * the same with incoming UPIU for the request. For example,
+	 * if UFS driver rings with task tag #1, subsequential UPIUs
+	 * for this must have one as the value of task tag. But if
+	 * it's corrutped until the host receives it or incoming UPIUs
+	 * has an unexpected value for task tag, this raises.
+	 */
+	RX_UPIU_HIT_ERROR	= 1 << 19,
+};
+
 enum {
 	UNIPRO_L1_5 = 0,/* PHY Adapter */
 	UNIPRO_L2,	/* Data Link */
@@ -996,6 +1011,10 @@ static int exynos_ufs_init(struct ufs_hba *hba)
 		goto out;
 	exynos_ufs_specify_phy_time_attr(ufs);
 	exynos_ufs_config_smu(ufs);
+
+	/* enable vendor interrupts */
+	hci_writel(ufs, VENDOR_SPECIFIC_IE, RX_UPIU_HIT_ERROR);
+
 	return 0;
 
 phy_off:
@@ -1005,26 +1024,34 @@ static int exynos_ufs_init(struct ufs_hba *hba)
 	return ret;
 }
 
-static int exynos_ufs_host_reset(struct ufs_hba *hba)
+static int exynos_ufs_host_reset(struct exynos_ufs *ufs)
 {
-	struct exynos_ufs *ufs = ufshcd_get_variant(hba);
 	unsigned long timeout = jiffies + msecs_to_jiffies(1);
-	u32 val;
-	int ret = 0;
-
-	exynos_ufs_disable_auto_ctrl_hcc_save(ufs, &val);
 
+	/* host reset */
 	hci_writel(ufs, UFS_SW_RST_MASK, HCI_SW_RST);
-
 	do {
 		if (!(hci_readl(ufs, HCI_SW_RST) & UFS_SW_RST_MASK))
-			goto out;
+			return 0;
+		cpu_relax();
 	} while (time_before(jiffies, timeout));
 
-	dev_err(hba->dev, "timeout host sw-reset\n");
-	ret = -ETIMEDOUT;
+	pr_err("timeout host sw-reset\n");
+	return -ETIMEDOUT;
+}
+
+static int exynos_ufs_host_init(struct ufs_hba *hba)
+{
+	struct exynos_ufs *ufs = ufshcd_get_variant(hba);
+	u32 val;
+	int ret;
+
+	exynos_ufs_disable_auto_ctrl_hcc_save(ufs, &val);
+
+	ret = exynos_ufs_host_reset(ufs);
+	if (ret)
+		return ret;
 
-out:
 	exynos_ufs_auto_ctrl_hcc_restore(ufs, &val);
 	return ret;
 }
@@ -1110,7 +1137,7 @@ static int exynos_ufs_hce_enable_notify(struct ufs_hba *hba,
 
 	switch (status) {
 	case PRE_CHANGE:
-		ret = exynos_ufs_host_reset(hba);
+		ret = exynos_ufs_host_init(hba);
 		if (ret)
 			return ret;
 		exynos_ufs_dev_hw_reset(hba);
@@ -1198,6 +1225,38 @@ static int exynos_ufs_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
 	return 0;
 }
 
+static irqreturn_t exynos_ufs_isr(struct ufs_hba *hba)
+{
+	struct exynos_ufs *ufs = ufshcd_get_variant(hba);
+	u32 status;
+	unsigned long flags;
+
+	if (!hba->priv) return IRQ_HANDLED;
+	status = hci_readl(ufs, VENDOR_SPECIFIC_IS);
+	hci_writel(ufs, status, VENDOR_SPECIFIC_IS);
+	/*
+	 * If host doesn't guarantee integrity of UTP transmission,
+	 * it needs to be reset immediately to make it unable to
+	 * proceed requests. Because w/o this, if UTP functions
+	 * in host doesn't work properly for such system power margins,
+	 * DATA IN from broken devices or whatever in the real world,
+	 * some unexpected events could happen, such as transferring
+	 * a broken DATA IN to a device. It could be various types of
+	 * problems on the level of file system. In this case, I think
+	 * blocking the host's functionality is the best strategy.
+	 * Perhaps, if its root cause is temporary, system could recover.
+	 */
+	if (status & RX_UPIU_HIT_ERROR) {
+		pr_err("%s: status: 0x%08x\n", __func__, status);
+		hba->force_reset = true;
+		hba->force_requeue = true;
+		scsi_schedule_eh(hba->host);
+		spin_unlock_irqrestore(hba->host->host_lock, flags);
+		return IRQ_HANDLED;
+	}
+	return IRQ_NONE;
+}
+
 static struct ufs_hba_variant_ops ufs_hba_exynos_ops = {
 	.name				= "exynos_ufs",
 	.init				= exynos_ufs_init,
@@ -1209,6 +1268,7 @@ static struct ufs_hba_variant_ops ufs_hba_exynos_ops = {
 	.hibern8_notify			= exynos_ufs_hibern8_notify,
 	.suspend			= exynos_ufs_suspend,
 	.resume				= exynos_ufs_resume,
+	.intr				= exynos_ufs_isr,
 };
 
 static int exynos_ufs_probe(struct platform_device *pdev)
-- 
2.7.4