LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 2/2] nvmet: use passthru cntrl in nvmet_init_cap
       [not found]   ` <CGME20210826211546uscas1p1e181ca820e506c7c195b933168301dd0@uscas1p1.samsung.com>
@ 2021-08-26 21:15     ` Adam Manzanares
  2021-08-27  6:17       ` hch
  0 siblings, 1 reply; 6+ messages in thread
From: Adam Manzanares @ 2021-08-26 21:15 UTC (permalink / raw)
  To: kbusch, axboe, hch, sagi, chaitanya.kulkarni, linux-nvme, open list
  Cc: linux-kernel, Adam Manzanares

For a passthru controller make cap initialization dependent on the cap of
the passthru controller.

Fixes: ab5d0b38c047 (nvmet: add Command Set Identifier support)
Signed-off-by: Adam Manzanares <a.manzanares@samsung.com>
---
 drivers/nvme/target/core.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index 66d05eecc2a9..220ba5ed5f3a 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -9,6 +9,7 @@
 #include <linux/rculist.h>
 #include <linux/pci-p2pdma.h>
 #include <linux/scatterlist.h>
+#include "../host/nvme.h"
 
 #define CREATE_TRACE_POINTS
 #include "trace.h"
@@ -1198,10 +1199,13 @@ void nvmet_update_cc(struct nvmet_ctrl *ctrl, u32 new)
 
 static void nvmet_init_cap(struct nvmet_ctrl *ctrl)
 {
+	struct nvme_ctrl *ptctrl = nvmet_passthru_ctrl(ctrl->subsys);
+
 	/* command sets supported: NVMe command set: */
 	ctrl->cap = (1ULL << 37);
 	/* Controller supports one or more I/O Command Sets */
-	ctrl->cap |= (1ULL << 43);
+	if ((ptctrl && nvme_multi_css(ptctrl)) || !ptctrl)
+		ctrl->cap |= (1ULL << 43);
 	/* CC.EN timeout in 500msec units: */
 	ctrl->cap |= (15ULL << 24);
 	/* maximum queue entries supported: */
@@ -1363,8 +1367,6 @@ u16 nvmet_alloc_ctrl(const char *subsysnqn, const char *hostnqn,
 		goto out_put_subsystem;
 	mutex_init(&ctrl->lock);
 
-	nvmet_init_cap(ctrl);
-
 	ctrl->port = req->port;
 
 	INIT_WORK(&ctrl->async_event_work, nvmet_async_event_work);
@@ -1378,6 +1380,7 @@ u16 nvmet_alloc_ctrl(const char *subsysnqn, const char *hostnqn,
 
 	kref_init(&ctrl->ref);
 	ctrl->subsys = subsys;
+	nvmet_init_cap(ctrl);
 	WRITE_ONCE(ctrl->aen_enabled, NVMET_AEN_CFG_OPTIONAL);
 
 	ctrl->changed_ns_list = kmalloc_array(NVME_MAX_CHANGED_NAMESPACES,
-- 
2.25.1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 0/2] Make nvmet_init_cap dependent on passthru controller
       [not found] <CGME20210826211546uscas1p1fc876392e5aaf990eeb480cc74508852@uscas1p1.samsung.com>
@ 2021-08-26 21:15 ` Adam Manzanares
       [not found]   ` <CGME20210826211546uscas1p2488ecd33269aa6f71f1fe8271fad3cec@uscas1p2.samsung.com>
                     ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Adam Manzanares @ 2021-08-26 21:15 UTC (permalink / raw)
  To: kbusch, axboe, hch, sagi, chaitanya.kulkarni, linux-nvme
  Cc: linux-kernel, Adam Manzanares

nvme_init_cap unconditionally sets support for one or more command sets. When
using a passthru controller this may cause a conventional namespace to be
ignored when checking the namespace identification descriptors which must
include the command set identifier when the cap is set to support one or more
command sets. Since the namespace identification descriptors come from the
passthru controller they may not include the command set identifier causing the
namespace to be ignored.

Adam Manzanares (2):
  nvme: move nvme_multi_css into nvme.h
  nvmet: use passthru cntrl in nvmet_init_cap

 drivers/nvme/host/core.c   | 5 -----
 drivers/nvme/host/nvme.h   | 5 +++++
 drivers/nvme/target/core.c | 9 ++++++---
 3 files changed, 11 insertions(+), 8 deletions(-)

-- 
2.25.1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] nvme: move nvme_multi_css into nvme.h
       [not found]   ` <CGME20210826211546uscas1p2488ecd33269aa6f71f1fe8271fad3cec@uscas1p2.samsung.com>
@ 2021-08-26 21:15     ` Adam Manzanares
  0 siblings, 0 replies; 6+ messages in thread
From: Adam Manzanares @ 2021-08-26 21:15 UTC (permalink / raw)
  To: kbusch, axboe, hch, sagi, chaitanya.kulkarni, linux-nvme, open list
  Cc: linux-kernel, Adam Manzanares

Preparatory patch in order to reuse nvme_multi_css in the nvme target code

Signed-off-by: Adam Manzanares <a.manzanares@samsung.com>
---
 drivers/nvme/host/core.c | 5 -----
 drivers/nvme/host/nvme.h | 5 +++++
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 42b69f3c6e20..cd5dc104d4b1 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1303,11 +1303,6 @@ static int nvme_identify_ctrl(struct nvme_ctrl *dev, struct nvme_id_ctrl **id)
 	return error;
 }
 
-static bool nvme_multi_css(struct nvme_ctrl *ctrl)
-{
-	return (ctrl->ctrl_config & NVME_CC_CSS_MASK) == NVME_CC_CSS_CSI;
-}
-
 static int nvme_process_ns_desc(struct nvme_ctrl *ctrl, struct nvme_ns_ids *ids,
 		struct nvme_ns_id_desc *cur, bool *csi_seen)
 {
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 8fd30ef19757..9871c0c9374c 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -892,4 +892,9 @@ struct nvme_ctrl *nvme_ctrl_from_file(struct file *file);
 struct nvme_ns *nvme_find_get_ns(struct nvme_ctrl *ctrl, unsigned nsid);
 void nvme_put_ns(struct nvme_ns *ns);
 
+static inline bool nvme_multi_css(struct nvme_ctrl *ctrl)
+{
+	return (ctrl->ctrl_config & NVME_CC_CSS_MASK) == NVME_CC_CSS_CSI;
+}
+
 #endif /* _NVME_H */
-- 
2.25.1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/2] Make nvmet_init_cap dependent on passthru controller
  2021-08-26 21:15 ` [PATCH 0/2] Make nvmet_init_cap dependent on passthru controller Adam Manzanares
       [not found]   ` <CGME20210826211546uscas1p2488ecd33269aa6f71f1fe8271fad3cec@uscas1p2.samsung.com>
       [not found]   ` <CGME20210826211546uscas1p1e181ca820e506c7c195b933168301dd0@uscas1p1.samsung.com>
@ 2021-08-26 21:50   ` Keith Busch
  2 siblings, 0 replies; 6+ messages in thread
From: Keith Busch @ 2021-08-26 21:50 UTC (permalink / raw)
  To: Adam Manzanares
  Cc: axboe, hch, sagi, chaitanya.kulkarni, linux-nvme, linux-kernel

On Thu, Aug 26, 2021 at 09:15:45PM +0000, Adam Manzanares wrote:
> nvme_init_cap unconditionally sets support for one or more command sets. When
> using a passthru controller this may cause a conventional namespace to be
> ignored when checking the namespace identification descriptors which must
> include the command set identifier when the cap is set to support one or more
> command sets. Since the namespace identification descriptors come from the
> passthru controller they may not include the command set identifier causing the
> namespace to be ignored.
> 
> Adam Manzanares (2):
>   nvme: move nvme_multi_css into nvme.h
>   nvmet: use passthru cntrl in nvmet_init_cap

Looks good to me.

Reviewed-by: Keith Busch <kbusch@kernel.org>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] nvmet: use passthru cntrl in nvmet_init_cap
  2021-08-26 21:15     ` [PATCH 2/2] nvmet: use passthru cntrl in nvmet_init_cap Adam Manzanares
@ 2021-08-27  6:17       ` hch
  2021-08-27 16:19         ` Adam Manzanares
  0 siblings, 1 reply; 6+ messages in thread
From: hch @ 2021-08-27  6:17 UTC (permalink / raw)
  To: Adam Manzanares
  Cc: kbusch, axboe, hch, sagi, chaitanya.kulkarni, linux-nvme, open list

Technically this looks good, but the core target code should not have
to poke into the host side data structures.  Does this version still look
good to you?

---
From c5777caf1562df35150a71e5c91c5b272956beee Mon Sep 17 00:00:00 2001
From: Adam Manzanares <a.manzanares@samsung.com>
Date: Thu, 26 Aug 2021 21:15:45 +0000
Subject: nvmet: looks at the passthrough controller when initializing CAP

For a passthru controller make cap initialization dependent on the cap of
the passthru controller, given that multiple Command Set support needs
to be supported by the underlying controller.  For that move the
initialization of CAP later so that it can use the fully initialized
nvmet_ctrl structure.

Fixes: ab5d0b38c047 (nvmet: add Command Set Identifier support)
Signed-off-by: Adam Manzanares <a.manzanares@samsung.com>
Reviewed-by: Keith Busch <kbusch@kernel.org>
[hch: refactored the code a bit to keep it more contained in passthru.c]
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/nvme/target/core.c     |  6 ++++--
 drivers/nvme/target/nvmet.h    |  2 ++
 drivers/nvme/target/passthru.c | 10 ++++++++++
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index 66d05eecc2a9..11c44706dc2d 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -1206,6 +1206,9 @@ static void nvmet_init_cap(struct nvmet_ctrl *ctrl)
 	ctrl->cap |= (15ULL << 24);
 	/* maximum queue entries supported: */
 	ctrl->cap |= NVMET_QUEUE_SIZE - 1;
+
+	if (nvmet_passthru_ctrl(ctrl->subsys))
+		nvmet_passthrough_override_cap(ctrl);
 }
 
 struct nvmet_ctrl *nvmet_ctrl_find_get(const char *subsysnqn,
@@ -1363,8 +1366,6 @@ u16 nvmet_alloc_ctrl(const char *subsysnqn, const char *hostnqn,
 		goto out_put_subsystem;
 	mutex_init(&ctrl->lock);
 
-	nvmet_init_cap(ctrl);
-
 	ctrl->port = req->port;
 
 	INIT_WORK(&ctrl->async_event_work, nvmet_async_event_work);
@@ -1378,6 +1379,7 @@ u16 nvmet_alloc_ctrl(const char *subsysnqn, const char *hostnqn,
 
 	kref_init(&ctrl->ref);
 	ctrl->subsys = subsys;
+	nvmet_init_cap(ctrl);
 	WRITE_ONCE(ctrl->aen_enabled, NVMET_AEN_CFG_OPTIONAL);
 
 	ctrl->changed_ns_list = kmalloc_array(NVME_MAX_CHANGED_NAMESPACES,
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index 06dd3d537f07..183119607968 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -613,6 +613,8 @@ nvmet_req_passthru_ctrl(struct nvmet_req *req)
 	return nvmet_passthru_ctrl(nvmet_req_subsys(req));
 }
 
+void nvmet_passthrough_override_cap(struct nvmet_ctrl *ctrl);
+
 u16 errno_to_nvme_status(struct nvmet_req *req, int errno);
 u16 nvmet_report_invalid_opcode(struct nvmet_req *req);
 
diff --git a/drivers/nvme/target/passthru.c b/drivers/nvme/target/passthru.c
index 225cd1ffbe45..8784c487e462 100644
--- a/drivers/nvme/target/passthru.c
+++ b/drivers/nvme/target/passthru.c
@@ -20,6 +20,16 @@ MODULE_IMPORT_NS(NVME_TARGET_PASSTHRU);
  */
 static DEFINE_XARRAY(passthru_subsystems);
 
+void nvmet_passthrough_override_cap(struct nvmet_ctrl *ctrl)
+{
+	/*
+	 * Multiple command set support can only be declared if the underlying
+	 * controller actually supports it.
+	 */
+	if (!nvme_multi_css(ctrl->subsys->passthru_ctrl))
+		ctrl->cap &= ~(1ULL << 43);
+}
+
 static u16 nvmet_passthru_override_id_ctrl(struct nvmet_req *req)
 {
 	struct nvmet_ctrl *ctrl = req->sq->ctrl;
-- 
2.30.2


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] nvmet: use passthru cntrl in nvmet_init_cap
  2021-08-27  6:17       ` hch
@ 2021-08-27 16:19         ` Adam Manzanares
  0 siblings, 0 replies; 6+ messages in thread
From: Adam Manzanares @ 2021-08-27 16:19 UTC (permalink / raw)
  To: hch; +Cc: kbusch, axboe, sagi, chaitanya.kulkarni, linux-nvme, open list

On Fri, Aug 27, 2021 at 08:17:37AM +0200, hch@lst.de wrote:
> Technically this looks good, but the core target code should not have
> to poke into the host side data structures.  Does this version still look
> good to you?

Yup looks good, thanks for the help cleaning it up.

> 
> ---
> From c5777caf1562df35150a71e5c91c5b272956beee Mon Sep 17 00:00:00 2001
> From: Adam Manzanares <a.manzanares@samsung.com>
> Date: Thu, 26 Aug 2021 21:15:45 +0000
> Subject: nvmet: looks at the passthrough controller when initializing CAP
> 
> For a passthru controller make cap initialization dependent on the cap of
> the passthru controller, given that multiple Command Set support needs
> to be supported by the underlying controller.  For that move the
> initialization of CAP later so that it can use the fully initialized
> nvmet_ctrl structure.
> 
> Fixes: ab5d0b38c047 (nvmet: add Command Set Identifier support)
> Signed-off-by: Adam Manzanares <a.manzanares@samsung.com>
> Reviewed-by: Keith Busch <kbusch@kernel.org>
> [hch: refactored the code a bit to keep it more contained in passthru.c]
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/nvme/target/core.c     |  6 ++++--
>  drivers/nvme/target/nvmet.h    |  2 ++
>  drivers/nvme/target/passthru.c | 10 ++++++++++
>  3 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
> index 66d05eecc2a9..11c44706dc2d 100644
> --- a/drivers/nvme/target/core.c
> +++ b/drivers/nvme/target/core.c
> @@ -1206,6 +1206,9 @@ static void nvmet_init_cap(struct nvmet_ctrl *ctrl)
>  	ctrl->cap |= (15ULL << 24);
>  	/* maximum queue entries supported: */
>  	ctrl->cap |= NVMET_QUEUE_SIZE - 1;
> +
> +	if (nvmet_passthru_ctrl(ctrl->subsys))
> +		nvmet_passthrough_override_cap(ctrl);
>  }
>  
>  struct nvmet_ctrl *nvmet_ctrl_find_get(const char *subsysnqn,
> @@ -1363,8 +1366,6 @@ u16 nvmet_alloc_ctrl(const char *subsysnqn, const char *hostnqn,
>  		goto out_put_subsystem;
>  	mutex_init(&ctrl->lock);
>  
> -	nvmet_init_cap(ctrl);
> -
>  	ctrl->port = req->port;
>  
>  	INIT_WORK(&ctrl->async_event_work, nvmet_async_event_work);
> @@ -1378,6 +1379,7 @@ u16 nvmet_alloc_ctrl(const char *subsysnqn, const char *hostnqn,
>  
>  	kref_init(&ctrl->ref);
>  	ctrl->subsys = subsys;
> +	nvmet_init_cap(ctrl);
>  	WRITE_ONCE(ctrl->aen_enabled, NVMET_AEN_CFG_OPTIONAL);
>  
>  	ctrl->changed_ns_list = kmalloc_array(NVME_MAX_CHANGED_NAMESPACES,
> diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
> index 06dd3d537f07..183119607968 100644
> --- a/drivers/nvme/target/nvmet.h
> +++ b/drivers/nvme/target/nvmet.h
> @@ -613,6 +613,8 @@ nvmet_req_passthru_ctrl(struct nvmet_req *req)
>  	return nvmet_passthru_ctrl(nvmet_req_subsys(req));
>  }
>  
> +void nvmet_passthrough_override_cap(struct nvmet_ctrl *ctrl);
> +
>  u16 errno_to_nvme_status(struct nvmet_req *req, int errno);
>  u16 nvmet_report_invalid_opcode(struct nvmet_req *req);
>  
> diff --git a/drivers/nvme/target/passthru.c b/drivers/nvme/target/passthru.c
> index 225cd1ffbe45..8784c487e462 100644
> --- a/drivers/nvme/target/passthru.c
> +++ b/drivers/nvme/target/passthru.c
> @@ -20,6 +20,16 @@ MODULE_IMPORT_NS(NVME_TARGET_PASSTHRU);
>   */
>  static DEFINE_XARRAY(passthru_subsystems);
>  
> +void nvmet_passthrough_override_cap(struct nvmet_ctrl *ctrl)
> +{
> +	/*
> +	 * Multiple command set support can only be declared if the underlying
> +	 * controller actually supports it.
> +	 */
> +	if (!nvme_multi_css(ctrl->subsys->passthru_ctrl))
> +		ctrl->cap &= ~(1ULL << 43);
> +}
> +
>  static u16 nvmet_passthru_override_id_ctrl(struct nvmet_req *req)
>  {
>  	struct nvmet_ctrl *ctrl = req->sq->ctrl;
> -- 
> 2.30.2
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-08-27 16:19 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20210826211546uscas1p1fc876392e5aaf990eeb480cc74508852@uscas1p1.samsung.com>
2021-08-26 21:15 ` [PATCH 0/2] Make nvmet_init_cap dependent on passthru controller Adam Manzanares
     [not found]   ` <CGME20210826211546uscas1p2488ecd33269aa6f71f1fe8271fad3cec@uscas1p2.samsung.com>
2021-08-26 21:15     ` [PATCH 1/2] nvme: move nvme_multi_css into nvme.h Adam Manzanares
     [not found]   ` <CGME20210826211546uscas1p1e181ca820e506c7c195b933168301dd0@uscas1p1.samsung.com>
2021-08-26 21:15     ` [PATCH 2/2] nvmet: use passthru cntrl in nvmet_init_cap Adam Manzanares
2021-08-27  6:17       ` hch
2021-08-27 16:19         ` Adam Manzanares
2021-08-26 21:50   ` [PATCH 0/2] Make nvmet_init_cap dependent on passthru controller Keith Busch

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).