| 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: <DM6PR04MB65758403CC6D31654A98BB43FC669@DM6PR04MB6575.namprd04.prod.outlook.com>
Date: Sun, 23 Apr 2023 10:12:31 +0000
From: Avri Altman <Avri.Altman@....com>
To: Kiwoong Kim <kwmad.kim@...sung.com>,
"linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"alim.akhtar@...sung.com" <alim.akhtar@...sung.com>,
"jejb@...ux.ibm.com" <jejb@...ux.ibm.com>,
"martin.petersen@...cle.com" <martin.petersen@...cle.com>,
"beanhuo@...ron.com" <beanhuo@...ron.com>,
"adrian.hunter@...el.com" <adrian.hunter@...el.com>,
"sc.suh@...sung.com" <sc.suh@...sung.com>,
"hy50.seo@...sung.com" <hy50.seo@...sung.com>,
"sh425.lee@...sung.com" <sh425.lee@...sung.com>,
"bhoon95.kim@...sung.com" <bhoon95.kim@...sung.com>,
"kwangwon.min@...sung.com" <kwangwon.min@...sung.com>
Subject: RE: [RFC PATCH v1] ufs: poll pmc until another pa request is
completed
> Regarding 5.7.12.11 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.
Can you please elaborate on how this concurrency can happen?
My understanding is that both line reset indication and uic command are protected by host_lock?
>
> [ 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.
>
> Signed-off-by: Kiwoong Kim <kwmad.kim@...sung.com>
> ---
> drivers/ufs/core/ufshcd.c | 92 ++++++++++++++++++++++++++++++++++---------
> ----
> 1 file changed, 67 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 9434328..3fa58d9 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 30 /* millisecs */
Is this something that is common to all hosts?
Thanks,
Avri
> +
> /* UFSHC 4.0 compliant HC support this mode, refer param_set_mcq_mode()
> */
> static bool use_mcq_mode = true;
>
> @@ -4138,6 +4141,64 @@ 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;
> + ktime_t timeout;
> + u32 mode = cmd->argument3;
> +
> + timeout = ktime_add_ms(ktime_get(), 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;
> + cmd->argument2 = 0;
> + cmd->argument3 = mode;
> + ret = __ufshcd_send_uic_cmd(hba, cmd, true);
> + spin_unlock_irqrestore(hba->host->host_lock, flags);
> + 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 for only 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\n",
> __func__);
> +
> + } while (ktime_before(ktime_get(), timeout));
> +out:
> + spin_lock_irqsave(hba->host->host_lock, flags);
> + hba->active_uic_cmd = NULL;
> + spin_unlock_irqrestore(hba->host->host_lock, flags);
> +
> + return ret;
> +}
> +
> /**
> * ufshcd_uic_pwr_ctrl - executes UIC commands (which affects the link
> power
> * state) and waits for it to take effect.
> @@ -4160,33 +4221,16 @@ 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;
> - }
> - ret = __ufshcd_send_uic_cmd(hba, cmd, false);
> - spin_unlock_irqrestore(hba->host->host_lock, flags);
> + 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,
> @@ -4223,14 +4267,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