[PATCH 1/1] nvme: check for Identify CNS 06h opcode support

[PATCH 1/1] nvme: check for Identify CNS 06h opcode support

[Aviv Heller]

nvme_init_non_mdts_limits() issues Identify CNS 06h command, and
silently fails in case of command failure.

The code checks whether CNS values greater than 1 are supported,
however, CNS 06h is only supported since 2.0.

Modify the check such that the command is only issued for 2.0
compliant devices.

Fixes: 5befc7c26e5a ("nvme: implement non-mdts command limits")
Signed-off-by: Aviv Heller <aviv.heller@kioxia.com>
---
 drivers/nvme/host/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index e486845d2c7e..8cfb4ebe1279 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2823,7 +2823,7 @@ static int nvme_init_non_mdts_limits(struct nvme_ctrl *ctrl)
 	else
 		ctrl->max_zeroes_sectors = 0;
 
-	if (nvme_ctrl_limited_cns(ctrl))
+	if (ctrl->vs < NVME_VS(2, 0, 0))
 		return 0;
 
 	id = kzalloc(sizeof(*id), GFP_KERNEL);
-- 
2.30.2

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH 1/1] nvme: check for Identify CNS 06h opcode support

[Keith Busch]

On Thu, Sep 30, 2021 at 12:14:26PM +0000, Aviv Heller wrote:
> nvme_init_non_mdts_limits() issues Identify CNS 06h command, and
> silently fails in case of command failure.
> 
> The code checks whether CNS values greater than 1 are supported,
> however, CNS 06h is only supported since 2.0.
> 
> Modify the check such that the command is only issued for 2.0
> compliant devices.
>
> Fixes: 5befc7c26e5a ("nvme: implement non-mdts command limits")

There's no requirement that a controller report a particular version in
order to implement a TP, though. It's perfectly valid for a controller
that reports version 1.4 may implement this feature. We just don't have
a way to konw if a controller implements this CNS (TPAR 4135 aims to fix
that), so we just try it and look for success.

The driver doesn't care if the controller doesn't support this
identification. If the controller doesn't support it, we should get an
Invalid Field status. The driver will carry on as before, so it should
be harmless. What problem is this actually fixing?

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

RE: [PATCH 1/1] nvme: check for Identify CNS 06h opcode support

[Aviv Heller]

Thanks for your feedback.

> There's no requirement that a controller report a particular version in order
> to implement a TP, though. It's perfectly valid for a controller that reports
> version 1.4 may implement this feature. We just don't have a way to konw if
> a controller implements this CNS (TPAR 4135 aims to fix that), so we just try it
> and look for success.

That's good to know. I didn't notice that this CNS is defined in a TP.

> The driver doesn't care if the controller doesn't support this identification. If
> the controller doesn't support it, we should get an Invalid Field status. The
> driver will carry on as before, so it should be harmless. What problem is this
> actually fixing?

It was noticed when we got an invalid CNS error print from our NVMeoF target.
We then thought that if support could be tested, avoiding issuing the command
is the right way to go.

This is not the case, though.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme