| 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
| ||
|
Message-ID: <000701d9e481$f70738a0$e515a9e0$@samsung.com>
Date: Mon, 11 Sep 2023 16:31:22 +0900
From: "Kiwoong Kim" <kwmad.kim@...sung.com>
To: <linux-scsi@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<alim.akhtar@...sung.com>, <avri.altman@....com>,
<bvanassche@....org>, <jejb@...ux.ibm.com>,
<martin.petersen@...cle.com>, <beanhuo@...ron.com>,
<adrian.hunter@...el.com>, <sc.suh@...sung.com>,
<hy50.seo@...sung.com>, <sh425.lee@...sung.com>,
<kwangwon.min@...sung.com>, <junwoo80.lee@...sung.com>,
<wkon.kim@...sung.com>
Subject: RE: [PATCH v4] ufs: poll pmc until another pa request is completed
@ , 안녕하세요.
Thanks.
Kiwoong Kim
> -----Original Message-----
> From: Kiwoong Kim <kwmad.kim@...sung.com>
> Sent: Thursday, September 7, 2023 10:48 AM
> To: linux-scsi@...r.kernel.org; linux-kernel@...r.kernel.org;
> alim.akhtar@...sung.com; avri.altman@....com; bvanassche@....org;
> jejb@...ux.ibm.com; martin.petersen@...cle.com; beanhuo@...ron.com;
> adrian.hunter@...el.com; sc.suh@...sung.com; hy50.seo@...sung.com;
> sh425.lee@...sung.com; kwangwon.min@...sung.com; junwoo80.lee@...sung.com;
> wkon.kim@...sung.com
> Cc: Kiwoong Kim <kwmad.kim@...sung.com>
> Subject: [PATCH v4] ufs: poll pmc until another pa request is completed
>
> Regarding 5.7.12.1.1 in Unipro v1.8, PA rejects sebsequent requests
> following the first request from upper layer or remote.
> In this situation, PA responds w/ BUSY in cases when they come from upper
> layer and does nothing for the requests. So HCI doesn't receive ind, a.k.a.
> indicator for its requests and an interrupt, IS.UPMS isn't generated.
>
> When LINERESET occurs, the error handler issues PMC which is recognized as
> a request for PA. If a host's PA gets or raises LINERESET, and a request
> for PMC, this could be a concurrent situation mentioned above. And I found
> that situation w/ my environment.
>
> [ 222.929539]I[0:DefaultDispatch:20410] exynos-ufs 13500000.ufs:
> ufshcd_update_uic_error: uecdl : 0x80000002 [ 222.999009]I[0:
> arch_disk_io_1:20413] exynos-ufs 13500000.ufs: ufshcd_update_uic_error:
> uecpa : 0x80000010 [ 222.999200] [6: kworker/u16:2: 132] exynos-ufs
> 13500000.ufs: ufs_pwr_mode_restore_needed : mode = 0x15, pwr_rx = 1,
> pwr_tx = 1 [ 223.002876]I[0: arch_disk_io_3:20422] exynos-ufs
> 13500000.ufs: ufshcd_update_uic_error: uecpa : 0x80000010 [ 223.501050]
> [4: kworker/u16:2: 132] exynos-ufs 13500000.ufs: pwr ctrl cmd 0x2 with
> mode 0x11 completion timeout [ 223.502305] [4: kworker/u16:2: 132]
> exynos-ufs 13500000.ufs: ufshcd_change_power_mode: power mode change
> failed -110 [ 223.502312] [4: kworker/u16:2: 132] exynos-ufs
> 13500000.ufs: ufshcd_err_handler: Failed to restore power mode, err = -110
> [ 223.502392] [4: kworker/u16:2: 132] exynos-ufs 13500000.ufs:
> ufshcd_is_pwr_mode_restore_needed : mode = 0x11, pwr_rx = 1, pwr_tx = 1
>
> This patch is to poll PMC's result w/o checking its ind until the result
> is not busy, i.e. 09h, to avoid the rejection.
>
> And this patch requires the following patch because it assumes
> __ufshcd_send_uic_cmd doesn't require host_lock.
> https://lore.kernel.org/linux-
> scsi/bec84ee1ce8f5c5971b98d8e501aeabb9b5b26d1.1686716811.git.kwmad.kim@sam
> sung.com/
>
> Signed-off-by: Kiwoong Kim <kwmad.kim@...sung.com>
>
> ---
> v3 -> v4
> 1) change description
>
> v2 -> v3
> 1) check time in the loop with jiffies, instead of miliseconds.
> 2) change a variable's name and fix a typo and wrong alignment.
>
> v1 -> v2
> 1) remove clearing hba->active_uic_cmd at the end of __ufshcd_poll_uic_pwr
> 2) change commit message on the cited clause: 5.7.12.11 -> 5.7.12.1.1
> 3) add mdelay to avoid too many issueing
> 4) change the timeout for the polling to 100ms because I found it
> sometimes takes much longer than expected.
>
> Signed-off-by: Kiwoong Kim <kwmad.kim@...sung.com>
> ---
> drivers/ufs/core/ufshcd.c | 93 +++++++++++++++++++++++++++++++++---------
> -----
> 1 file changed, 65 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index
> 7bc3fc4..f6b8659 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -98,6 +98,9 @@
> /* Polling time to wait for fDeviceInit */ #define
> FDEVICEINIT_COMPL_TIMEOUT 1500 /* millisecs */
>
> +/* Polling time to wait until PA is ready */
> +#define UIC_PA_RDY_TIMEOUT 100 /* millisecs */
> +
> /* UFSHC 4.0 compliant HC support this mode. */ static bool use_mcq_mode
> = true;
>
> @@ -4120,6 +4123,62 @@ int ufshcd_dme_get_attr(struct ufs_hba *hba, u32
> attr_sel, } EXPORT_SYMBOL_GPL(ufshcd_dme_get_attr);
>
> +static int __ufshcd_poll_uic_pwr(struct ufs_hba *hba, struct uic_command
> *cmd,
> + struct completion *cnf)
> +{
> + unsigned long flags;
> + int ret;
> + u32 mode = cmd->argument3;
> + unsigned long deadline = jiffies +
> + msecs_to_jiffies(UIC_PA_RDY_TIMEOUT);
> +
> + do {
> + spin_lock_irqsave(hba->host->host_lock, flags);
> + hba->active_uic_cmd = NULL;
> + if (ufshcd_is_link_broken(hba)) {
> + spin_unlock_irqrestore(hba->host->host_lock, flags);
> + ret = -ENOLINK;
> + goto out;
> + }
> + hba->uic_async_done = cnf;
> + spin_unlock_irqrestore(hba->host->host_lock, flags);
> +
> + cmd->argument2 = 0;
> + cmd->argument3 = mode;
> + ret = __ufshcd_send_uic_cmd(hba, cmd, true);
> + if (ret) {
> + dev_err(hba->dev,
> + "pwr ctrl cmd 0x%x with mode 0x%x uic
> error %d\n",
> + cmd->command, cmd->argument3, ret);
> + goto out;
> + }
> +
> + /* This value is heuristic */
> + if (!wait_for_completion_timeout(&cmd->done,
> + msecs_to_jiffies(5))) {
> + ret = -ETIMEDOUT;
> + dev_err(hba->dev,
> + "pwr ctrl cmd 0x%x with mode 0x%x timeout\n",
> + cmd->command, cmd->argument3);
> + if (cmd->cmd_active)
> + goto out;
> +
> + dev_info(hba->dev, "%s: pwr ctrl cmd has already been
> completed\n", __func__);
> + }
> +
> + /* retry only for busy cases */
> + ret = cmd->argument2 & MASK_UIC_COMMAND_RESULT;
> + if (ret != UIC_CMD_RESULT_BUSY)
> + break;
> +
> + dev_info(hba->dev, "%s: PA is busy and can't handle a
> requeest, %d\n", __func__, ret);
> + mdelay(1);
> +
> + } while (time_before(jiffies, deadline));
> +out:
> + return ret;
> +}
> +
> /**
> * ufshcd_uic_pwr_ctrl - executes UIC commands (which affects the link
> power
> * state) and waits for it to take effect.
> @@ -4142,33 +4201,13 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba *hba,
> struct uic_command *cmd)
> unsigned long flags;
> u8 status;
> int ret;
> - bool reenable_intr = false;
>
> - mutex_lock(&hba->uic_cmd_mutex);
> - ufshcd_add_delay_before_dme_cmd(hba);
> -
> - spin_lock_irqsave(hba->host->host_lock, flags);
> - if (ufshcd_is_link_broken(hba)) {
> - ret = -ENOLINK;
> - goto out_unlock;
> - }
> - hba->uic_async_done = &uic_async_done;
> - if (ufshcd_readl(hba, REG_INTERRUPT_ENABLE) & UIC_COMMAND_COMPL) {
> - ufshcd_disable_intr(hba, UIC_COMMAND_COMPL);
> - /*
> - * Make sure UIC command completion interrupt is disabled
> before
> - * issuing UIC command.
> - */
> - wmb();
> - reenable_intr = true;
> - }
> - spin_unlock_irqrestore(hba->host->host_lock, flags);
> - ret = __ufshcd_send_uic_cmd(hba, cmd, false);
> + ret = __ufshcd_poll_uic_pwr(hba, cmd, &uic_async_done);
> if (ret) {
> - dev_err(hba->dev,
> - "pwr ctrl cmd 0x%x with mode 0x%x uic error %d\n",
> - cmd->command, cmd->argument3, ret);
> - goto out;
> + if (ret == -ENOLINK)
> + goto out_unlock;
> + else
> + goto out;
> }
>
> if (!wait_for_completion_timeout(hba->uic_async_done,
> @@ -4205,14 +4244,12 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba *hba,
> struct uic_command *cmd)
> spin_lock_irqsave(hba->host->host_lock, flags);
> hba->active_uic_cmd = NULL;
> hba->uic_async_done = NULL;
> - if (reenable_intr)
> - ufshcd_enable_intr(hba, UIC_COMMAND_COMPL);
> if (ret) {
> ufshcd_set_link_broken(hba);
> ufshcd_schedule_eh_work(hba);
> }
> -out_unlock:
> spin_unlock_irqrestore(hba->host->host_lock, flags);
> +out_unlock:
> mutex_unlock(&hba->uic_cmd_mutex);
>
> return ret;
> --
> 2.7.4
Powered by blists - more mailing lists