[PATCH] nvme: avoid bogus CRTO values
Christoph Hellwig
hch at lst.de
Wed Sep 13 03:50:15 PDT 2023
On Tue, Sep 12, 2023 at 02:47:33PM -0700, Keith Busch wrote:
> From: Keith Busch <kbusch at kernel.org>
>
> Some devices are reporting controller ready mode support, but return 0
> for CRTO. These devices require a much higher time to ready than that,
> so they are failing to initialize after the driver starter preferring
> that value over CAP.TO.
>
> The spec requires that CAP.TO match the appropritate CRTO value, or be
> set to 0xff if CRTO is larger than that. This means that CAP.TO can be
> used to validate if CRTO is reliable, and provides an appropriate
> fallback for setting the timeout value if not. Use whichever is larger.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217863
> Reported-by: Cláudio Sampaio <patola at gmail.com>
> Reported-by: Felix Yan <felixonmars at archlinux.org>
> Based-on-a-patch-by: Felix Yan <felixonmars at archlinux.org>
> Cc: stable at vger.kernel.org
> Signed-off-by: Keith Busch <kbusch at kernel.org>
> ---
> drivers/nvme/host/core.c | 48 ++++++++++++++++++++++++----------------
> 1 file changed, 29 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> + if (ctrl->cap & NVME_CAP_CRMS_CRWMS && ctrl->cap & NVME_CAP_CRMS_CRIMS)
I don't think the NVME_CAP_CRMS_CRWMS check here makes sense, this
should only need the NVME_CAP_CRMS_CRIMS one.
> + timeout = NVME_CAP_TIMEOUT(ctrl->cap);
> + if (ctrl->cap & NVME_CAP_CRMS_CRWMS) {
> + u32 crto;
> +
> + ret = ctrl->ops->reg_read32(ctrl, NVME_REG_CRTO, &crto);
> + if (ret) {
> + dev_err(ctrl->device, "Reading CRTO failed (%d)\n",
> + ret);
> + return ret;
> + }
> +
> + /*
> + * CRTO should always be greater or equal to CAP.TO, but some
> + * devices are known to get this wrong. Use the larger of the
> + * two values.
> + */
> + if (ctrl->ctrl_config & NVME_CC_CRIME)
> + timeout = max(timeout, NVME_CRTO_CRIMT(crto));
> + else
> + timeout = max(timeout, NVME_CRTO_CRWMT(crto));
Should we at least log a harmless one-liner warning if the new timeouts
are too small?
More information about the Linux-nvme
mailing list