nvme: avoid bogus CRTO values
authorKeith Busch <kbusch@kernel.org>
Tue, 12 Sep 2023 21:38:58 +0000 (14:38 -0700)
committerKeith Busch <kbusch@kernel.org>
Thu, 14 Sep 2023 20:09:52 +0000 (13:09 -0700)
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@gmail.com>
Reported-by: Felix Yan <felixonmars@archlinux.org>
Tested-by: Felix Yan <felixonmars@archlinux.org>
Based-on-a-patch-by: Felix Yan <felixonmars@archlinux.org>
Cc: stable@vger.kernel.org
Signed-off-by: Keith Busch <kbusch@kernel.org>
drivers/nvme/host/core.c

index 37b6fa7466620436b2ab23cd00eb0de91309bb30..0685ed4f2dc4994751a37e8ad360a8b7acd1c3f7 100644 (file)
@@ -2245,25 +2245,8 @@ int nvme_enable_ctrl(struct nvme_ctrl *ctrl)
        else
                ctrl->ctrl_config = NVME_CC_CSS_NVM;
 
-       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;
-               }
-
-               if (ctrl->cap & NVME_CAP_CRMS_CRIMS) {
-                       ctrl->ctrl_config |= NVME_CC_CRIME;
-                       timeout = NVME_CRTO_CRIMT(crto);
-               } else {
-                       timeout = NVME_CRTO_CRWMT(crto);
-               }
-       } else {
-               timeout = NVME_CAP_TIMEOUT(ctrl->cap);
-       }
+       if (ctrl->cap & NVME_CAP_CRMS_CRWMS && ctrl->cap & NVME_CAP_CRMS_CRIMS)
+               ctrl->ctrl_config |= NVME_CC_CRIME;
 
        ctrl->ctrl_config |= (NVME_CTRL_PAGE_SHIFT - 12) << NVME_CC_MPS_SHIFT;
        ctrl->ctrl_config |= NVME_CC_AMS_RR | NVME_CC_SHN_NONE;
@@ -2277,6 +2260,39 @@ int nvme_enable_ctrl(struct nvme_ctrl *ctrl)
        if (ret)
                return ret;
 
+       /* CAP value may change after initial CC write */
+       ret = ctrl->ops->reg_read64(ctrl, NVME_REG_CAP, &ctrl->cap);
+       if (ret)
+               return ret;
+
+       timeout = NVME_CAP_TIMEOUT(ctrl->cap);
+       if (ctrl->cap & NVME_CAP_CRMS_CRWMS) {
+               u32 crto, ready_timeout;
+
+               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)
+                       ready_timeout = NVME_CRTO_CRIMT(crto);
+               else
+                       ready_timeout = NVME_CRTO_CRWMT(crto);
+
+               if (ready_timeout < timeout)
+                       dev_warn_once(ctrl->device, "bad crto:%x cap:%llx\n",
+                                     crto, ctrl->cap);
+               else
+                       timeout = ready_timeout;
+       }
+
        ctrl->ctrl_config |= NVME_CC_ENABLE;
        ret = ctrl->ops->reg_write32(ctrl, NVME_REG_CC, ctrl->ctrl_config);
        if (ret)