linux-nvme.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: James Smart <jsmart2021@gmail.com>
To: linux-nvme@lists.infradead.org
Cc: James Smart <jsmart2021@gmail.com>
Subject: [RFC 4/6] nvme: Add t_flags and pi_blkszs ctrl and ops attributes
Date: Mon, 24 Feb 2020 10:48:57 -0800	[thread overview]
Message-ID: <20200224184859.20995-5-jsmart2021@gmail.com> (raw)
In-Reply-To: <20200224184859.20995-1-jsmart2021@gmail.com>

To prepare for fabric PI support, the ops->flags handling needed to be
reworked and the addition of a new field, pi_blkszs is needed.

The nvme_ctrl_ops flags defines were moved to an enum, and values were
added to cover PI types that the host transport supports, as well as a
flag on whether the transport can insert/strip PI data for payload
received without pi data. A new field, pi_blkszs, was added. The field
parallels ns->lba_shift - each bit is a power of 2 block size, and if set,
is a block size the transport knows how to do pi operations on.

The new fields were also added to nvme_ctrl. This allows the default
values to be loaded from the ops via nvme_init_ctrl, but the transport
is allowed to tune the value on a per-controller basis if needed (directly
writes the field after nvme_init_ctrl). For example, the same transport
may have two host ports that have hw with different capabilities.

The PCI driver's ops struct was updated for the new pi flags and the
new pi_blkszs field. As all pci devices will behave the same, the ops
structure is all that needed to change.

The core code was updated to take the transport flags from the controller
rather than the ops structure.  nvme_update_disk_info() was revised to
better check the support of the transport when there is metadata.

Signed-off-by: James Smart <jsmart2021@gmail.com>

---
 I don't believe the following check, added by this patch, is necessary
 with the code in this new form. Meaning: in all cases now, if we reach this
 point, nvme_init_integrity() is called. So I don't think we need to check
 blk_get_integrity() any more.
 Please review:
  @@ -1805,12 +1806,32 @@ static void nvme_update_disk_info(struct gendisk *disk,
  ...
  +               if (!nvme_ns_has_pi(ns) && !blk_get_integrity(disk))
  +                       capacity = 0;
---
 drivers/nvme/host/core.c | 41 ++++++++++++++++++++++++++++++++---------
 drivers/nvme/host/nvme.h | 33 +++++++++++++++++++++++++++++----
 drivers/nvme/host/pci.c  |  6 +++++-
 3 files changed, 66 insertions(+), 14 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 8421eafa81c6..ab03311105c3 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1762,6 +1762,7 @@ static bool nvme_ns_ids_equal(struct nvme_ns_ids *a, struct nvme_ns_ids *b)
 static void nvme_update_disk_info(struct gendisk *disk,
 		struct nvme_ns *ns, struct nvme_id_ns *id)
 {
+	struct nvme_ctrl *ctrl = ns->ctrl;
 	sector_t capacity = nvme_lba_to_sect(ns, le64_to_cpu(id->nsze));
 	unsigned short bs = 1 << ns->lba_shift;
 	u32 atomic_bs, phys_bs, io_opt;
@@ -1782,7 +1783,7 @@ static void nvme_update_disk_info(struct gendisk *disk,
 		if (id->nsfeat & (1 << 1) && id->nawupf)
 			atomic_bs = (1 + le16_to_cpu(id->nawupf)) * bs;
 		else
-			atomic_bs = (1 + ns->ctrl->subsys->awupf) * bs;
+			atomic_bs = (1 + ctrl->subsys->awupf) * bs;
 	} else {
 		atomic_bs = bs;
 	}
@@ -1805,12 +1806,32 @@ static void nvme_update_disk_info(struct gendisk *disk,
 	blk_queue_io_min(disk->queue, phys_bs);
 	blk_queue_io_opt(disk->queue, io_opt);
 
-	if (ns->ms && !ns->ext &&
-	    (ns->ctrl->ops->flags & NVME_F_METADATA_SUPPORTED))
+	if (ns->ms) {
+		/*
+		 * Ensure transport supports separate metadata. If not,
+		 * only allowable configuration is with pi enabled and
+		 * using PRACT=1 or the host transport to insert/strip.
+		 */
+		if (!(ctrl->t_flags & NVME_F_METADATA_SUPPORTED) &&
+		    !nvme_ns_has_pi(ns))
+			goto set_0_capacity;
+
+		/* Check controller requirements:
+		 *  PCI requires separate metadata support.
+		 *  Fabrics do not support separate metadata.
+		 */
+		if ((!ns->ext && ctrl->t_flags & NVME_F_FABRICS) ||
+		    (ns->ext && !(ctrl->t_flags & NVME_F_FABRICS)))
+			goto set_0_capacity;
+
 		nvme_init_integrity(disk, ns->ms, ns->pi_type,
-				    ns->ctrl->max_meta_segments);
-	if ((ns->ms && !nvme_ns_has_pi(ns) && !blk_get_integrity(disk)) ||
-	    ns->lba_shift > PAGE_SHIFT)
+				    ctrl->max_meta_segments);
+
+		if (!nvme_ns_has_pi(ns) && !blk_get_integrity(disk))
+			capacity = 0;
+	}
+	if (ns->lba_shift > PAGE_SHIFT)
+set_0_capacity:
 		capacity = 0;
 
 	set_capacity(disk, capacity);
@@ -2764,7 +2785,7 @@ int nvme_init_identify(struct nvme_ctrl *ctrl)
 			goto out_free;
 	}
 
-	if (!(ctrl->ops->flags & NVME_F_FABRICS))
+	if (!(ctrl->t_flags & NVME_F_FABRICS))
 		ctrl->cntlid = le16_to_cpu(id->cntlid);
 
 	if (!ctrl->identified) {
@@ -2848,7 +2869,7 @@ int nvme_init_identify(struct nvme_ctrl *ctrl)
 	}
 	memcpy(ctrl->psd, id->psd, sizeof(ctrl->psd));
 
-	if (ctrl->ops->flags & NVME_F_FABRICS) {
+	if (ctrl->t_flags & NVME_F_FABRICS) {
 		ctrl->icdoff = le16_to_cpu(id->icdoff);
 		ctrl->ioccsz = le32_to_cpu(id->ioccsz);
 		ctrl->iorcsz = le32_to_cpu(id->iorcsz);
@@ -3530,7 +3551,7 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid)
 			|= BDI_CAP_STABLE_WRITES;
 
 	blk_queue_flag_set(QUEUE_FLAG_NONROT, ns->queue);
-	if (ctrl->ops->flags & NVME_F_PCI_P2PDMA)
+	if (ctrl->t_flags & NVME_F_PCI_P2PDMA)
 		blk_queue_flag_set(QUEUE_FLAG_PCI_P2PDMA, ns->queue);
 
 	ns->queue->queuedata = ns;
@@ -4061,7 +4082,9 @@ int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct device *dev,
 	init_rwsem(&ctrl->namespaces_rwsem);
 	ctrl->dev = dev;
 	ctrl->ops = ops;
+	ctrl->t_flags = ops->flags;
 	ctrl->max_meta_segments = ops->max_meta_segments;
+	ctrl->pi_blkszs = ops->pi_blkszs;
 	ctrl->quirks = quirks;
 	INIT_WORK(&ctrl->scan_work, nvme_scan_work);
 	INIT_WORK(&ctrl->async_event_work, nvme_async_event_work);
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 4c6b6fc18560..88e82250bb3d 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -262,7 +262,10 @@ struct nvme_ctrl {
 	struct work_struct fw_act_work;
 	unsigned long events;
 
+	/* host transport attributes */
+	u32 t_flags;	/* transport flags: see enum nvme_transport_flags */
 	u32 max_meta_segments;
+	u32 pi_blkszs;	/* each bit is a power of 2. See ns->lba_shift */
 
 #ifdef CONFIG_NVME_MULTIPATH
 	/* asymmetric namespace access: */
@@ -402,13 +405,34 @@ static inline bool nvme_ns_has_pi(struct nvme_ns *ns)
 	return ns->pi_type && ns->ms == sizeof(struct t10_pi_tuple);
 }
 
+enum nvme_transport_flags {
+	NVME_F_FABRICS			= 1 << 0,
+	NVME_F_PCI_P2PDMA		= 1 << 1,
+
+	/* Metadata requires use of a separate blk_integrity_rq buffer */
+	NVME_F_METADATA_SUPPORTED	= 1 << 8,
+
+	/*
+	 * The PI types supported by the host transport, if it participates
+	 * in PI checking or can insert/strip PI on behalf of the OS.
+	 */
+	NVME_F_PI_TYPE_MASK		= 0x7 << 9,
+	NVME_F_PI_TYPE1_PROTECTION	= 1 << 9,
+	NVME_F_PI_TYPE2_PROTECTION	= 1 << 10,
+	NVME_F_PI_TYPE3_PROTECTION	= 1 << 11,
+
+	/* Indicates that the host transport can insert/strip PI for
+	 * presentation to the controller. I.E. OS does not have metadata/PI.
+	 * Transport will insert PI/meta on TX and strip PI/meta on RX.
+	 * Similar to PRACT=1.
+	 */
+	NVME_F_PI_TXINSERT_RXSTRIP	= 1 << 12,
+};
+
 struct nvme_ctrl_ops {
 	const char *name;
 	struct module *module;
-	unsigned int flags;
-#define NVME_F_FABRICS			(1 << 0)
-#define NVME_F_METADATA_SUPPORTED	(1 << 1)
-#define NVME_F_PCI_P2PDMA		(1 << 2)
+	unsigned int flags;		/* see enum nvme_transport_flags */
 	int (*reg_read32)(struct nvme_ctrl *ctrl, u32 off, u32 *val);
 	int (*reg_write32)(struct nvme_ctrl *ctrl, u32 off, u32 val);
 	int (*reg_read64)(struct nvme_ctrl *ctrl, u32 off, u64 *val);
@@ -417,6 +441,7 @@ struct nvme_ctrl_ops {
 	void (*delete_ctrl)(struct nvme_ctrl *ctrl);
 	int (*get_address)(struct nvme_ctrl *ctrl, char *buf, int size);
 	u32 max_meta_segments;
+	u32 pi_blkszs;	/* each bit is a power of 2. See ns->lba_shift */
 };
 
 #ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 7cbd2fbda743..f56038294c23 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -2683,6 +2683,9 @@ static const struct nvme_ctrl_ops nvme_pci_ctrl_ops = {
 	.name			= "pcie",
 	.module			= THIS_MODULE,
 	.flags			= NVME_F_METADATA_SUPPORTED |
+				  NVME_F_PI_TYPE_MASK | /* all types presented
+							 * by the controller
+							 */
 				  NVME_F_PCI_P2PDMA,
 	.reg_read32		= nvme_pci_reg_read32,
 	.reg_write32		= nvme_pci_reg_write32,
@@ -2692,7 +2695,8 @@ static const struct nvme_ctrl_ops nvme_pci_ctrl_ops = {
 	.get_address		= nvme_pci_get_address,
 	/* PCI supports metadata via single segment separate buffer only */
 	.max_meta_segments	= 1,
-
+	/* Support is whatever the controller presents */
+	.pi_blkszs		= 0xFFFFFFFF,
 };
 
 static int nvme_dev_map(struct nvme_dev *dev)
-- 
2.16.4


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

  parent reply	other threads:[~2020-02-24 18:49 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-24 18:48 [RFC 0/6] nvme: core layer metadata/pi support James Smart
2020-02-24 18:48 ` [RFC 1/6] nvme: Add ns to nvme_request struct James Smart
2020-04-21 18:03   ` Christoph Hellwig
2020-02-24 18:48 ` [RFC 2/6] nvme: make nvme_ns_has_pi accessible to transports James Smart
2020-04-21 18:03   ` Christoph Hellwig
2020-02-24 18:48 ` [RFC 3/6] nvme: Introduce max_meta_segments ctrl and ops attribute James Smart
2020-02-25  9:49   ` Max Gurtovoy
2020-04-21 18:04     ` Christoph Hellwig
2020-02-24 18:48 ` James Smart [this message]
2020-04-21 18:10   ` [RFC 4/6] nvme: Add t_flags and pi_blkszs ctrl and ops attributes Christoph Hellwig
2020-02-24 18:48 ` [RFC 5/6] nvme: Add pi_flags to nvme_request for transport pi support James Smart
2020-04-21 18:11   ` Christoph Hellwig
2020-02-24 18:48 ` [RFC 6/6] nvme: Introduce NVME_INLINE_PROT_SG_CNT James Smart

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=20200224184859.20995-5-jsmart2021@gmail.com \
    --to=jsmart2021@gmail.com \
    --cc=linux-nvme@lists.infradead.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).