Linux-NVME Archive on lore.kernel.org
 help / color / Atom feed
From: Himanshu Madhani <himanshu.madhani@oracle.com>
To: Keith Busch <kbusch@kernel.org>
Cc: axboe@kernel.dk, "Niklas Cassel" <niklas.cassel@wdc.com>,
	sagi@grimberg.me,
	"Martin K . Petersen" <martin.petersen@oracle.com>,
	"Johannes Thumshirn" <johannes.thumshirn@wdc.com>,
	"Daniel Wagner" <dwagner@suse.de>,
	linux-nvme@lists.infradead.org, linux-block@vger.kernel.org,
	"Javier González" <javier.gonz@samsung.com>,
	hch@lst.de, "Matias Bjørling" <matias.bjorling@wdc.com>
Subject: Re: [PATCHv4 3/5] nvme: implement I/O Command Sets Command Set support
Date: Tue, 30 Jun 2020 10:45:31 -0500
Message-ID: <50ECB21A-E2DA-4B2D-B5CE-603694DFB224@oracle.com> (raw)
In-Reply-To: <20200629190641.1986462-4-kbusch@kernel.org>



> On Jun 29, 2020, at 2:06 PM, Keith Busch <kbusch@kernel.org> wrote:
> 
> From: Niklas Cassel <niklas.cassel@wdc.com>
> 
> Implements support for the I/O Command Sets command set. The command set
> introduces a method to enumerate multiple command sets per namespace. If
> the command set is exposed, this method for enumeration will be used
> instead of the traditional method that uses the CC.CSS register command
> set register for command set identification.
> 
> For namespaces where the Command Set Identifier is not supported or
> recognized, the specific namespace will not be created.
> 
> Reviewed-by: Javier González <javier.gonz@samsung.com>
> Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
> Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> Reviewed-by: Matias Bjørling <matias.bjorling@wdc.com>
> Reviewed-by: Daniel Wagner <dwagner@suse.de>
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> ---
> drivers/nvme/host/core.c | 53 ++++++++++++++++++++++++++++++++--------
> drivers/nvme/host/nvme.h |  1 +
> include/linux/nvme.h     | 19 ++++++++++++--
> 3 files changed, 61 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 701763910e48..b7d12eb42fd8 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -1056,8 +1056,13 @@ 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)
> +		struct nvme_ns_id_desc *cur, bool *csi_seen)
> {
> 	const char *warn_str = "ctrl returned bogus length:";
> 	void *data = cur;
> @@ -1087,6 +1092,15 @@ static int nvme_process_ns_desc(struct nvme_ctrl *ctrl, struct nvme_ns_ids *ids,
> 		}
> 		uuid_copy(&ids->uuid, data + sizeof(*cur));
> 		return NVME_NIDT_UUID_LEN;
> +	case NVME_NIDT_CSI:
> +		if (cur->nidl != NVME_NIDT_CSI_LEN) {
> +			dev_warn(ctrl->device, "%s %d for NVME_NIDT_CSI\n",
> +				 warn_str, cur->nidl);
> +			return -1;
> +		}
> +		memcpy(&ids->csi, data + sizeof(*cur), NVME_NIDT_CSI_LEN);
> +		*csi_seen = true;
> +		return NVME_NIDT_CSI_LEN;
> 	default:
> 		/* Skip unknown types */
> 		return cur->nidl;
> @@ -1097,10 +1111,9 @@ static int nvme_identify_ns_descs(struct nvme_ctrl *ctrl, unsigned nsid,
> 		struct nvme_ns_ids *ids)
> {
> 	struct nvme_command c = { };
> -	int status;
> +	bool csi_seen = false;
> +	int status, pos, len;
> 	void *data;
> -	int pos;
> -	int len;
> 
> 	c.identify.opcode = nvme_admin_identify;
> 	c.identify.nsid = cpu_to_le32(nsid);
> @@ -1125,7 +1138,7 @@ static int nvme_identify_ns_descs(struct nvme_ctrl *ctrl, unsigned nsid,
> 		  * device just because of a temporal retry-able error (such
> 		  * as path of transport errors).
> 		  */
> -		if (status > 0 && (status & NVME_SC_DNR))
> +		if (status > 0 && (status & NVME_SC_DNR) && !nvme_multi_css(ctrl))
> 			status = 0;
> 		goto free_data;
> 	}
> @@ -1136,12 +1149,19 @@ static int nvme_identify_ns_descs(struct nvme_ctrl *ctrl, unsigned nsid,
> 		if (cur->nidl == 0)
> 			break;
> 
> -		len = nvme_process_ns_desc(ctrl, ids, cur);
> +		len = nvme_process_ns_desc(ctrl, ids, cur, &csi_seen);
> 		if (len < 0)
> -			goto free_data;
> +			break;
> 
> 		len += sizeof(*cur);
> 	}
> +
> +	if (nvme_multi_css(ctrl) && !csi_seen) {
> +		dev_warn(ctrl->device, "Command set not reported for nsid:%d\n",
> +			 nsid);
> +		status = -EINVAL;
> +	}
> +
> free_data:
> 	kfree(data);
> 	return status;
> @@ -1798,7 +1818,7 @@ static int nvme_report_ns_ids(struct nvme_ctrl *ctrl, unsigned int nsid,
> 		memcpy(ids->eui64, id->eui64, sizeof(id->eui64));
> 	if (ctrl->vs >= NVME_VS(1, 2, 0))
> 		memcpy(ids->nguid, id->nguid, sizeof(id->nguid));
> -	if (ctrl->vs >= NVME_VS(1, 3, 0))
> +	if (ctrl->vs >= NVME_VS(1, 3, 0) || nvme_multi_css(ctrl))
> 		return nvme_identify_ns_descs(ctrl, nsid, ids);
> 	return 0;
> }
> @@ -1814,7 +1834,8 @@ static bool nvme_ns_ids_equal(struct nvme_ns_ids *a, struct nvme_ns_ids *b)
> {
> 	return uuid_equal(&a->uuid, &b->uuid) &&
> 		memcmp(&a->nguid, &b->nguid, sizeof(a->nguid)) == 0 &&
> -		memcmp(&a->eui64, &b->eui64, sizeof(a->eui64)) == 0;
> +		memcmp(&a->eui64, &b->eui64, sizeof(a->eui64)) == 0 &&
> +		a->csi == b->csi;
> }
> 
> static int nvme_setup_streams_ns(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
> @@ -1936,6 +1957,15 @@ static int __nvme_revalidate_disk(struct gendisk *disk, struct nvme_id_ns *id)
> 	if (ns->lba_shift == 0)
> 		ns->lba_shift = 9;
> 
> +	switch (ns->head->ids.csi) {
> +	case NVME_CSI_NVM:
> +		break;
> +	default:
> +		dev_warn(ctrl->device, "unknown csi:%d ns:%d\n",
> +			ns->head->ids.csi, ns->head->ns_id);
> +		return -ENODEV;
> +	}
> +
> 	if ((ctrl->quirks & NVME_QUIRK_STRIPE_SIZE) &&
> 	    is_power_of_2(ctrl->max_hw_sectors))
> 		iob = ctrl->max_hw_sectors;
> @@ -2269,7 +2299,10 @@ int nvme_enable_ctrl(struct nvme_ctrl *ctrl)
> 
> 	ctrl->page_size = 1 << page_shift;
> 
> -	ctrl->ctrl_config = NVME_CC_CSS_NVM;
> +	if (NVME_CAP_CSS(ctrl->cap) & NVME_CAP_CSS_CSI)
> +		ctrl->ctrl_config = NVME_CC_CSS_CSI;
> +	else
> +		ctrl->ctrl_config = NVME_CC_CSS_NVM;
> 	ctrl->ctrl_config |= (page_shift - 12) << NVME_CC_MPS_SHIFT;
> 	ctrl->ctrl_config |= NVME_CC_AMS_RR | NVME_CC_SHN_NONE;
> 	ctrl->ctrl_config |= NVME_CC_IOSQES | NVME_CC_IOCQES;
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index 919ebaf3fdef..4aa321b16eaa 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -339,6 +339,7 @@ struct nvme_ns_ids {
> 	u8	eui64[8];
> 	u8	nguid[16];
> 	uuid_t	uuid;
> +	u8	csi;
> };
> 
> /*
> diff --git a/include/linux/nvme.h b/include/linux/nvme.h
> index 5ce51ab4c50e..81ffe5247505 100644
> --- a/include/linux/nvme.h
> +++ b/include/linux/nvme.h
> @@ -132,6 +132,7 @@ enum {
> #define NVME_CAP_TIMEOUT(cap)	(((cap) >> 24) & 0xff)
> #define NVME_CAP_STRIDE(cap)	(((cap) >> 32) & 0xf)
> #define NVME_CAP_NSSRC(cap)	(((cap) >> 36) & 0x1)
> +#define NVME_CAP_CSS(cap)	(((cap) >> 37) & 0xff)
> #define NVME_CAP_MPSMIN(cap)	(((cap) >> 48) & 0xf)
> #define NVME_CAP_MPSMAX(cap)	(((cap) >> 52) & 0xf)
> 
> @@ -162,7 +163,6 @@ enum {
> 
> enum {
> 	NVME_CC_ENABLE		= 1 << 0,
> -	NVME_CC_CSS_NVM		= 0 << 4,
> 	NVME_CC_EN_SHIFT	= 0,
> 	NVME_CC_CSS_SHIFT	= 4,
> 	NVME_CC_MPS_SHIFT	= 7,
> @@ -170,6 +170,9 @@ enum {
> 	NVME_CC_SHN_SHIFT	= 14,
> 	NVME_CC_IOSQES_SHIFT	= 16,
> 	NVME_CC_IOCQES_SHIFT	= 20,
> +	NVME_CC_CSS_NVM		= 0 << NVME_CC_CSS_SHIFT,
> +	NVME_CC_CSS_CSI		= 6 << NVME_CC_CSS_SHIFT,
> +	NVME_CC_CSS_MASK	= 7 << NVME_CC_CSS_SHIFT,
> 	NVME_CC_AMS_RR		= 0 << NVME_CC_AMS_SHIFT,
> 	NVME_CC_AMS_WRRU	= 1 << NVME_CC_AMS_SHIFT,
> 	NVME_CC_AMS_VS		= 7 << NVME_CC_AMS_SHIFT,
> @@ -179,6 +182,8 @@ enum {
> 	NVME_CC_SHN_MASK	= 3 << NVME_CC_SHN_SHIFT,
> 	NVME_CC_IOSQES		= NVME_NVM_IOSQES << NVME_CC_IOSQES_SHIFT,
> 	NVME_CC_IOCQES		= NVME_NVM_IOCQES << NVME_CC_IOCQES_SHIFT,
> +	NVME_CAP_CSS_NVM	= 1 << 0,
> +	NVME_CAP_CSS_CSI	= 1 << 6,
> 	NVME_CSTS_RDY		= 1 << 0,
> 	NVME_CSTS_CFS		= 1 << 1,
> 	NVME_CSTS_NSSRO		= 1 << 4,
> @@ -374,6 +379,8 @@ enum {
> 	NVME_ID_CNS_CTRL		= 0x01,
> 	NVME_ID_CNS_NS_ACTIVE_LIST	= 0x02,
> 	NVME_ID_CNS_NS_DESC_LIST	= 0x03,
> +	NVME_ID_CNS_CS_NS		= 0x05,
> +	NVME_ID_CNS_CS_CTRL		= 0x06,
> 	NVME_ID_CNS_NS_PRESENT_LIST	= 0x10,
> 	NVME_ID_CNS_NS_PRESENT		= 0x11,
> 	NVME_ID_CNS_CTRL_NS_LIST	= 0x12,
> @@ -383,6 +390,10 @@ enum {
> 	NVME_ID_CNS_UUID_LIST		= 0x17,
> };
> 
> +enum {
> +	NVME_CSI_NVM			= 0,
> +};
> +
> enum {
> 	NVME_DIR_IDENTIFY		= 0x00,
> 	NVME_DIR_STREAMS		= 0x01,
> @@ -435,11 +446,13 @@ struct nvme_ns_id_desc {
> #define NVME_NIDT_EUI64_LEN	8
> #define NVME_NIDT_NGUID_LEN	16
> #define NVME_NIDT_UUID_LEN	16
> +#define NVME_NIDT_CSI_LEN	1
> 
> enum {
> 	NVME_NIDT_EUI64		= 0x01,
> 	NVME_NIDT_NGUID		= 0x02,
> 	NVME_NIDT_UUID		= 0x03,
> +	NVME_NIDT_CSI		= 0x04,
> };
> 
> struct nvme_smart_log {
> @@ -972,7 +985,9 @@ struct nvme_identify {
> 	__u8			cns;
> 	__u8			rsvd3;
> 	__le16			ctrlid;
> -	__u32			rsvd11[5];
> +	__u8			rsvd11[3];
> +	__u8			csi;
> +	__u32			rsvd12[4];
> };
> 
> #define NVME_IDENTIFY_DATA_SIZE 4096
> -- 
> 2.24.1
> 

Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>

--
Himanshu Madhani	Oracle Linux Engineering






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

  parent reply index

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-29 19:06 [PATCHv4 0/5] nvme support for zoned namespace command set Keith Busch
2020-06-29 19:06 ` [PATCHv4 1/5] block: add capacity field to zone descriptors Keith Busch
2020-06-30 15:34   ` Himanshu Madhani
2020-06-29 19:06 ` [PATCHv4 2/5] null_blk: introduce zone capacity for zoned device Keith Busch
2020-06-30 15:34   ` Himanshu Madhani
2020-06-29 19:06 ` [PATCHv4 3/5] nvme: implement I/O Command Sets Command Set support Keith Busch
2020-06-30  6:01   ` Hannes Reinecke
2020-06-30 15:45   ` Himanshu Madhani [this message]
2020-06-29 19:06 ` [PATCHv4 4/5] nvme: support for multi-command set effects Keith Busch
2020-06-30 15:51   ` Himanshu Madhani
2020-06-29 19:06 ` [PATCHv4 5/5] nvme: support for zoned namespaces Keith Busch
2020-06-30 16:36   ` Himanshu Madhani
2020-07-01  6:37 ` [PATCHv4 0/5] nvme support for zoned namespace command set Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=50ECB21A-E2DA-4B2D-B5CE-603694DFB224@oracle.com \
    --to=himanshu.madhani@oracle.com \
    --cc=axboe@kernel.dk \
    --cc=dwagner@suse.de \
    --cc=hch@lst.de \
    --cc=javier.gonz@samsung.com \
    --cc=johannes.thumshirn@wdc.com \
    --cc=kbusch@kernel.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=martin.petersen@oracle.com \
    --cc=matias.bjorling@wdc.com \
    --cc=niklas.cassel@wdc.com \
    --cc=sagi@grimberg.me \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-NVME Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-nvme/0 linux-nvme/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-nvme linux-nvme/ https://lore.kernel.org/linux-nvme \
		linux-nvme@lists.infradead.org
	public-inbox-index linux-nvme

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-nvme


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git