linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/4] nvme-pci: Support for Apple 201+ (T2 chip)
@ 2019-08-07  7:51 Benjamin Herrenschmidt
  2019-08-07  7:51 ` [PATCH v4 1/4] nvme-pci: Pass the queue to SQ_SIZE/CQ_SIZE macros Benjamin Herrenschmidt
                   ` (4 more replies)
  0 siblings, 5 replies; 14+ messages in thread
From: Benjamin Herrenschmidt @ 2019-08-07  7:51 UTC (permalink / raw)
  To: linux-nvme
  Cc: Sagi Grimberg, Jens Axboe, Keith Busch, Christoph Hellwig,
	linux-kernel, Paul Pawlowski

This series combines the original series and an updated version of the
shared tags patch, and is rebased on nvme-5.4.

This adds support for the controller found in recent Apple machines
which is basically a SW emulated NVME controller in the T2 chip.

The original reverse engineering work was done by
Paul Pawlowski <paul@mrarm.io>.



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

* [PATCH v4 1/4] nvme-pci: Pass the queue to SQ_SIZE/CQ_SIZE macros
  2019-08-07  7:51 [PATCH v4 0/4] nvme-pci: Support for Apple 201+ (T2 chip) Benjamin Herrenschmidt
@ 2019-08-07  7:51 ` Benjamin Herrenschmidt
  2019-08-07  7:51 ` [PATCH v4 2/4] nvme-pci: Add support for variable IO SQ element size Benjamin Herrenschmidt
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 14+ messages in thread
From: Benjamin Herrenschmidt @ 2019-08-07  7:51 UTC (permalink / raw)
  To: linux-nvme
  Cc: Sagi Grimberg, Jens Axboe, Keith Busch, Christoph Hellwig,
	linux-kernel, Paul Pawlowski, Benjamin Herrenschmidt

This will make it easier to handle variable queue entry sizes
later. No functional change.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Minwoo Im <minwoo.im.dev@gmail.com>
---
 drivers/nvme/host/pci.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 362a1a9ced36..b5b296984aa1 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -28,8 +28,8 @@
 #include "trace.h"
 #include "nvme.h"
 
-#define SQ_SIZE(depth)		(depth * sizeof(struct nvme_command))
-#define CQ_SIZE(depth)		(depth * sizeof(struct nvme_completion))
+#define SQ_SIZE(q)	((q)->q_depth * sizeof(struct nvme_command))
+#define CQ_SIZE(q)	((q)->q_depth * sizeof(struct nvme_completion))
 
 #define SGES_PER_PAGE	(PAGE_SIZE / sizeof(struct nvme_sgl_desc))
 
@@ -1344,16 +1344,16 @@ static enum blk_eh_timer_return nvme_timeout(struct request *req, bool reserved)
 
 static void nvme_free_queue(struct nvme_queue *nvmeq)
 {
-	dma_free_coherent(nvmeq->dev->dev, CQ_SIZE(nvmeq->q_depth),
+	dma_free_coherent(nvmeq->dev->dev, CQ_SIZE(nvmeq),
 				(void *)nvmeq->cqes, nvmeq->cq_dma_addr);
 	if (!nvmeq->sq_cmds)
 		return;
 
 	if (test_and_clear_bit(NVMEQ_SQ_CMB, &nvmeq->flags)) {
 		pci_free_p2pmem(to_pci_dev(nvmeq->dev->dev),
-				nvmeq->sq_cmds, SQ_SIZE(nvmeq->q_depth));
+				nvmeq->sq_cmds, SQ_SIZE(nvmeq));
 	} else {
-		dma_free_coherent(nvmeq->dev->dev, SQ_SIZE(nvmeq->q_depth),
+		dma_free_coherent(nvmeq->dev->dev, SQ_SIZE(nvmeq),
 				nvmeq->sq_cmds, nvmeq->sq_dma_addr);
 	}
 }
@@ -1433,12 +1433,12 @@ static int nvme_cmb_qdepth(struct nvme_dev *dev, int nr_io_queues,
 }
 
 static int nvme_alloc_sq_cmds(struct nvme_dev *dev, struct nvme_queue *nvmeq,
-				int qid, int depth)
+				int qid)
 {
 	struct pci_dev *pdev = to_pci_dev(dev->dev);
 
 	if (qid && dev->cmb_use_sqes && (dev->cmbsz & NVME_CMBSZ_SQS)) {
-		nvmeq->sq_cmds = pci_alloc_p2pmem(pdev, SQ_SIZE(depth));
+		nvmeq->sq_cmds = pci_alloc_p2pmem(pdev, SQ_SIZE(nvmeq));
 		if (nvmeq->sq_cmds) {
 			nvmeq->sq_dma_addr = pci_p2pmem_virt_to_bus(pdev,
 							nvmeq->sq_cmds);
@@ -1447,11 +1447,11 @@ static int nvme_alloc_sq_cmds(struct nvme_dev *dev, struct nvme_queue *nvmeq,
 				return 0;
 			}
 
-			pci_free_p2pmem(pdev, nvmeq->sq_cmds, SQ_SIZE(depth));
+			pci_free_p2pmem(pdev, nvmeq->sq_cmds, SQ_SIZE(nvmeq));
 		}
 	}
 
-	nvmeq->sq_cmds = dma_alloc_coherent(dev->dev, SQ_SIZE(depth),
+	nvmeq->sq_cmds = dma_alloc_coherent(dev->dev, SQ_SIZE(nvmeq),
 				&nvmeq->sq_dma_addr, GFP_KERNEL);
 	if (!nvmeq->sq_cmds)
 		return -ENOMEM;
@@ -1465,12 +1465,13 @@ static int nvme_alloc_queue(struct nvme_dev *dev, int qid, int depth)
 	if (dev->ctrl.queue_count > qid)
 		return 0;
 
-	nvmeq->cqes = dma_alloc_coherent(dev->dev, CQ_SIZE(depth),
+	nvmeq->q_depth = depth;
+	nvmeq->cqes = dma_alloc_coherent(dev->dev, CQ_SIZE(nvmeq),
 					 &nvmeq->cq_dma_addr, GFP_KERNEL);
 	if (!nvmeq->cqes)
 		goto free_nvmeq;
 
-	if (nvme_alloc_sq_cmds(dev, nvmeq, qid, depth))
+	if (nvme_alloc_sq_cmds(dev, nvmeq, qid))
 		goto free_cqdma;
 
 	nvmeq->dev = dev;
@@ -1479,15 +1480,14 @@ static int nvme_alloc_queue(struct nvme_dev *dev, int qid, int depth)
 	nvmeq->cq_head = 0;
 	nvmeq->cq_phase = 1;
 	nvmeq->q_db = &dev->dbs[qid * 2 * dev->db_stride];
-	nvmeq->q_depth = depth;
 	nvmeq->qid = qid;
 	dev->ctrl.queue_count++;
 
 	return 0;
 
  free_cqdma:
-	dma_free_coherent(dev->dev, CQ_SIZE(depth), (void *)nvmeq->cqes,
-							nvmeq->cq_dma_addr);
+	dma_free_coherent(dev->dev, CQ_SIZE(nvmeq), (void *)nvmeq->cqes,
+			  nvmeq->cq_dma_addr);
  free_nvmeq:
 	return -ENOMEM;
 }
@@ -1515,7 +1515,7 @@ static void nvme_init_queue(struct nvme_queue *nvmeq, u16 qid)
 	nvmeq->cq_head = 0;
 	nvmeq->cq_phase = 1;
 	nvmeq->q_db = &dev->dbs[qid * 2 * dev->db_stride];
-	memset((void *)nvmeq->cqes, 0, CQ_SIZE(nvmeq->q_depth));
+	memset((void *)nvmeq->cqes, 0, CQ_SIZE(nvmeq));
 	nvme_dbbuf_init(dev, nvmeq, qid);
 	dev->online_queues++;
 	wmb(); /* ensure the first interrupt sees the initialization */
-- 
2.17.1


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

* [PATCH v4 2/4] nvme-pci: Add support for variable IO SQ element size
  2019-08-07  7:51 [PATCH v4 0/4] nvme-pci: Support for Apple 201+ (T2 chip) Benjamin Herrenschmidt
  2019-08-07  7:51 ` [PATCH v4 1/4] nvme-pci: Pass the queue to SQ_SIZE/CQ_SIZE macros Benjamin Herrenschmidt
@ 2019-08-07  7:51 ` Benjamin Herrenschmidt
  2019-08-22  0:28   ` Christoph Hellwig
  2019-08-07  7:51 ` [PATCH v4 3/4] nvme-pci: Add support for Apple 2018+ models Benjamin Herrenschmidt
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: Benjamin Herrenschmidt @ 2019-08-07  7:51 UTC (permalink / raw)
  To: linux-nvme
  Cc: Sagi Grimberg, Jens Axboe, Keith Busch, Christoph Hellwig,
	linux-kernel, Paul Pawlowski, Benjamin Herrenschmidt

The size of a submission queue element should always be 6 (64 bytes)
by spec.

However some controllers such as Apple's are not properly implementing
the standard and require a different size.

This provides the ground work for the subsequent quirks for these
controllers.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Reviewed-by: Minwoo Im <minwoo.im.dev@gmail.com>
---
 drivers/nvme/host/pci.c | 11 ++++++++---
 include/linux/nvme.h    |  1 +
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index b5b296984aa1..78a660e229d9 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -28,7 +28,7 @@
 #include "trace.h"
 #include "nvme.h"
 
-#define SQ_SIZE(q)	((q)->q_depth * sizeof(struct nvme_command))
+#define SQ_SIZE(q)	((q)->q_depth << (q)->sqes)
 #define CQ_SIZE(q)	((q)->q_depth * sizeof(struct nvme_completion))
 
 #define SGES_PER_PAGE	(PAGE_SIZE / sizeof(struct nvme_sgl_desc))
@@ -100,6 +100,7 @@ struct nvme_dev {
 	unsigned io_queues[HCTX_MAX_TYPES];
 	unsigned int num_vecs;
 	int q_depth;
+	int io_sqes;
 	u32 db_stride;
 	void __iomem *bar;
 	unsigned long bar_mapped_size;
@@ -162,7 +163,7 @@ static inline struct nvme_dev *to_nvme_dev(struct nvme_ctrl *ctrl)
 struct nvme_queue {
 	struct nvme_dev *dev;
 	spinlock_t sq_lock;
-	struct nvme_command *sq_cmds;
+	void *sq_cmds;
 	 /* only used for poll queues: */
 	spinlock_t cq_poll_lock ____cacheline_aligned_in_smp;
 	volatile struct nvme_completion *cqes;
@@ -178,6 +179,7 @@ struct nvme_queue {
 	u16 last_cq_head;
 	u16 qid;
 	u8 cq_phase;
+	u8 sqes;
 	unsigned long flags;
 #define NVMEQ_ENABLED		0
 #define NVMEQ_SQ_CMB		1
@@ -488,7 +490,8 @@ static void nvme_submit_cmd(struct nvme_queue *nvmeq, struct nvme_command *cmd,
 			    bool write_sq)
 {
 	spin_lock(&nvmeq->sq_lock);
-	memcpy(&nvmeq->sq_cmds[nvmeq->sq_tail], cmd, sizeof(*cmd));
+	memcpy(nvmeq->sq_cmds + (nvmeq->sq_tail << nvmeq->sqes),
+	       cmd, sizeof(*cmd));
 	if (++nvmeq->sq_tail == nvmeq->q_depth)
 		nvmeq->sq_tail = 0;
 	nvme_write_sq_db(nvmeq, write_sq);
@@ -1465,6 +1468,7 @@ static int nvme_alloc_queue(struct nvme_dev *dev, int qid, int depth)
 	if (dev->ctrl.queue_count > qid)
 		return 0;
 
+	nvmeq->sqes = qid ? dev->io_sqes : NVME_NVM_ADMSQES;
 	nvmeq->q_depth = depth;
 	nvmeq->cqes = dma_alloc_coherent(dev->dev, CQ_SIZE(nvmeq),
 					 &nvmeq->cq_dma_addr, GFP_KERNEL);
@@ -2317,6 +2321,7 @@ static int nvme_pci_enable(struct nvme_dev *dev)
 	dev->ctrl.sqsize = dev->q_depth - 1; /* 0's based queue depth */
 	dev->db_stride = 1 << NVME_CAP_STRIDE(dev->ctrl.cap);
 	dev->dbs = dev->bar + 4096;
+	dev->io_sqes = NVME_NVM_IOSQES;
 
 	/*
 	 * Temporary fix for the Apple controller found in the MacBook8,1 and
diff --git a/include/linux/nvme.h b/include/linux/nvme.h
index 01aa6a6c241d..d5a4bc21f36b 100644
--- a/include/linux/nvme.h
+++ b/include/linux/nvme.h
@@ -140,6 +140,7 @@ enum {
  * Submission and Completion Queue Entry Sizes for the NVM command set.
  * (In bytes and specified as a power of two (2^n)).
  */
+#define NVME_NVM_ADMSQES	6
 #define NVME_NVM_IOSQES		6
 #define NVME_NVM_IOCQES		4
 
-- 
2.17.1


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

* [PATCH v4 3/4] nvme-pci: Add support for Apple 2018+ models
  2019-08-07  7:51 [PATCH v4 0/4] nvme-pci: Support for Apple 201+ (T2 chip) Benjamin Herrenschmidt
  2019-08-07  7:51 ` [PATCH v4 1/4] nvme-pci: Pass the queue to SQ_SIZE/CQ_SIZE macros Benjamin Herrenschmidt
  2019-08-07  7:51 ` [PATCH v4 2/4] nvme-pci: Add support for variable IO SQ element size Benjamin Herrenschmidt
@ 2019-08-07  7:51 ` Benjamin Herrenschmidt
  2019-08-22  0:28   ` Christoph Hellwig
  2019-08-07  7:51 ` [PATCH v4 4/4] nvme-pci: Support shared tags across queues for Apple 2018 controllers Benjamin Herrenschmidt
  2019-08-08 23:52 ` [PATCH v4 0/4] nvme-pci: Support for Apple 201+ (T2 chip) Sagi Grimberg
  4 siblings, 1 reply; 14+ messages in thread
From: Benjamin Herrenschmidt @ 2019-08-07  7:51 UTC (permalink / raw)
  To: linux-nvme
  Cc: Sagi Grimberg, Jens Axboe, Keith Busch, Christoph Hellwig,
	linux-kernel, Paul Pawlowski, Benjamin Herrenschmidt

Based on reverse engineering and original patch by

Paul Pawlowski <paul@mrarm.io>

This adds support for Apple weird implementation of NVME in their
2018 or later machines. It accounts for the twice-as-big SQ entries
for the IO queues, and the fact that only interrupt vector 0 appears
to function properly.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Reviewed-by: Minwoo Im <minwoo.im.dev@gmail.com>
---
 drivers/nvme/host/nvme.h | 10 ++++++++++
 drivers/nvme/host/pci.c  | 21 ++++++++++++++++++++-
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 8dc010ca30e5..0925f7fc13ff 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -92,6 +92,16 @@ enum nvme_quirks {
 	 * Broken Write Zeroes.
 	 */
 	NVME_QUIRK_DISABLE_WRITE_ZEROES		= (1 << 9),
+
+	/*
+	 * Use only one interrupt vector for all queues
+	 */
+	NVME_QUIRK_SINGLE_VECTOR		= (1 << 10),
+
+	/*
+	 * Use non-standard 128 bytes SQEs.
+	 */
+	NVME_QUIRK_128_BYTES_SQES		= (1 << 11),
 };
 
 /*
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 78a660e229d9..c683263cdf60 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -2081,6 +2081,13 @@ static int nvme_setup_irqs(struct nvme_dev *dev, unsigned int nr_io_queues)
 	dev->io_queues[HCTX_TYPE_DEFAULT] = 1;
 	dev->io_queues[HCTX_TYPE_READ] = 0;
 
+	/*
+	 * Some Apple controllers require all queues to use the
+	 * first vector.
+	 */
+	if (dev->ctrl.quirks & NVME_QUIRK_SINGLE_VECTOR)
+		irq_queues = 1;
+
 	return pci_alloc_irq_vectors_affinity(pdev, 1, irq_queues,
 			      PCI_IRQ_ALL_TYPES | PCI_IRQ_AFFINITY, &affd);
 }
@@ -2321,7 +2328,16 @@ static int nvme_pci_enable(struct nvme_dev *dev)
 	dev->ctrl.sqsize = dev->q_depth - 1; /* 0's based queue depth */
 	dev->db_stride = 1 << NVME_CAP_STRIDE(dev->ctrl.cap);
 	dev->dbs = dev->bar + 4096;
-	dev->io_sqes = NVME_NVM_IOSQES;
+
+	/*
+	 * Some Apple controllers require a non-standard SQE size.
+	 * Interestingly they also seem to ignore the CC:IOSQES register
+	 * so we don't bother updating it here.
+	 */
+	if (dev->ctrl.quirks & NVME_QUIRK_128_BYTES_SQES)
+		dev->io_sqes = 7;
+	else
+		dev->io_sqes = NVME_NVM_IOSQES;
 
 	/*
 	 * Temporary fix for the Apple controller found in the MacBook8,1 and
@@ -3039,6 +3055,9 @@ static const struct pci_device_id nvme_id_table[] = {
 	{ PCI_DEVICE_CLASS(PCI_CLASS_STORAGE_EXPRESS, 0xffffff) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_APPLE, 0x2001) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_APPLE, 0x2003) },
+	{ PCI_DEVICE(PCI_VENDOR_ID_APPLE, 0x2005),
+		.driver_data = NVME_QUIRK_SINGLE_VECTOR |
+				NVME_QUIRK_128_BYTES_SQES },
 	{ 0, }
 };
 MODULE_DEVICE_TABLE(pci, nvme_id_table);
-- 
2.17.1


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

* [PATCH v4 4/4] nvme-pci: Support shared tags across queues for Apple 2018 controllers
  2019-08-07  7:51 [PATCH v4 0/4] nvme-pci: Support for Apple 201+ (T2 chip) Benjamin Herrenschmidt
                   ` (2 preceding siblings ...)
  2019-08-07  7:51 ` [PATCH v4 3/4] nvme-pci: Add support for Apple 2018+ models Benjamin Herrenschmidt
@ 2019-08-07  7:51 ` Benjamin Herrenschmidt
  2019-08-22  0:29   ` Christoph Hellwig
  2019-08-08 23:52 ` [PATCH v4 0/4] nvme-pci: Support for Apple 201+ (T2 chip) Sagi Grimberg
  4 siblings, 1 reply; 14+ messages in thread
From: Benjamin Herrenschmidt @ 2019-08-07  7:51 UTC (permalink / raw)
  To: linux-nvme
  Cc: Sagi Grimberg, Jens Axboe, Keith Busch, Christoph Hellwig,
	linux-kernel, Paul Pawlowski, Benjamin Herrenschmidt

Another issue with the Apple T2 based 2018 controllers seem to be
that they blow up (and shut the machine down) if there's a tag
collision between the IO queue and the Admin queue.

My suspicion is that they use our tags for their internal tracking
and don't mix them with the queue id. They also seem to not like
when tags go beyond the IO queue depth, ie 128 tags.

This adds a quirk that marks tags 0..31 of the IO queue reserved

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Reviewed-by: Ming Lei <ming.lei@redhat.com>
Acked-by: Keith Busch <keith.busch@intel.com>
---
 drivers/nvme/host/nvme.h |  5 +++++
 drivers/nvme/host/pci.c  | 31 ++++++++++++++++++++++++++++++-
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 0925f7fc13ff..3e64f7187e70 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -102,6 +102,11 @@ enum nvme_quirks {
 	 * Use non-standard 128 bytes SQEs.
 	 */
 	NVME_QUIRK_128_BYTES_SQES		= (1 << 11),
+
+	/*
+	 * Prevent tag overlap between queues
+	 */
+	NVME_QUIRK_SHARED_TAGS                  = (1 << 12),
 };
 
 /*
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index c683263cdf60..de8c170d5abc 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -2106,6 +2106,14 @@ static int nvme_setup_io_queues(struct nvme_dev *dev)
 	unsigned long size;
 
 	nr_io_queues = max_io_queues();
+
+	/*
+	 * If tags are shared with admin queue (Apple bug), then
+	 * make sure we only use one IO queue.
+	 */
+	if (dev->ctrl.quirks & NVME_QUIRK_SHARED_TAGS)
+		nr_io_queues = 1;
+
 	result = nvme_set_queue_count(&dev->ctrl, &nr_io_queues);
 	if (result < 0)
 		return result;
@@ -2276,6 +2284,14 @@ static int nvme_dev_add(struct nvme_dev *dev)
 		dev->tagset.flags = BLK_MQ_F_SHOULD_MERGE;
 		dev->tagset.driver_data = dev;
 
+		/*
+		 * Some Apple controllers requires tags to be unique
+		 * across admin and IO queue, so reserve the first 32
+		 * tags of the IO queue.
+		 */
+		if (dev->ctrl.quirks & NVME_QUIRK_SHARED_TAGS)
+			dev->tagset.reserved_tags = NVME_AQ_DEPTH;
+
 		ret = blk_mq_alloc_tag_set(&dev->tagset);
 		if (ret) {
 			dev_warn(dev->ctrl.device,
@@ -2356,6 +2372,18 @@ static int nvme_pci_enable(struct nvme_dev *dev)
                         "set queue depth=%u\n", dev->q_depth);
 	}
 
+	/*
+	 * Controllers with the shared tags quirk need the IO queue to be
+	 * big enough so that we get 32 tags for the admin queue
+	 */
+	if ((dev->ctrl.quirks & NVME_QUIRK_SHARED_TAGS) &&
+	    (dev->q_depth < (NVME_AQ_DEPTH + 2))) {
+		dev->q_depth = NVME_AQ_DEPTH + 2;
+		dev_warn(dev->ctrl.device, "IO queue depth clamped to %d\n",
+			 dev->q_depth);
+	}
+
+
 	nvme_map_cmb(dev);
 
 	pci_enable_pcie_error_reporting(pdev);
@@ -3057,7 +3085,8 @@ static const struct pci_device_id nvme_id_table[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_APPLE, 0x2003) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_APPLE, 0x2005),
 		.driver_data = NVME_QUIRK_SINGLE_VECTOR |
-				NVME_QUIRK_128_BYTES_SQES },
+				NVME_QUIRK_128_BYTES_SQES |
+				NVME_QUIRK_SHARED_TAGS },
 	{ 0, }
 };
 MODULE_DEVICE_TABLE(pci, nvme_id_table);
-- 
2.17.1


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

* Re: [PATCH v4 0/4] nvme-pci: Support for Apple 201+ (T2 chip)
  2019-08-07  7:51 [PATCH v4 0/4] nvme-pci: Support for Apple 201+ (T2 chip) Benjamin Herrenschmidt
                   ` (3 preceding siblings ...)
  2019-08-07  7:51 ` [PATCH v4 4/4] nvme-pci: Support shared tags across queues for Apple 2018 controllers Benjamin Herrenschmidt
@ 2019-08-08 23:52 ` Sagi Grimberg
  4 siblings, 0 replies; 14+ messages in thread
From: Sagi Grimberg @ 2019-08-08 23:52 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, linux-nvme
  Cc: Jens Axboe, Keith Busch, Christoph Hellwig, linux-kernel, Paul Pawlowski


> This series combines the original series and an updated version of the
> shared tags patch, and is rebased on nvme-5.4.
> 
> This adds support for the controller found in recent Apple machines
> which is basically a SW emulated NVME controller in the T2 chip.
> 
> The original reverse engineering work was done by
> Paul Pawlowski <paul@mrarm.io>.

Thanks, pulled to nvme-5.4

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

* Re: [PATCH v4 2/4] nvme-pci: Add support for variable IO SQ element size
  2019-08-07  7:51 ` [PATCH v4 2/4] nvme-pci: Add support for variable IO SQ element size Benjamin Herrenschmidt
@ 2019-08-22  0:28   ` Christoph Hellwig
  2019-08-22  0:31     ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 14+ messages in thread
From: Christoph Hellwig @ 2019-08-22  0:28 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: linux-nvme, Sagi Grimberg, Jens Axboe, Keith Busch,
	Christoph Hellwig, linux-kernel, Paul Pawlowski

On Wed, Aug 07, 2019 at 05:51:20PM +1000, Benjamin Herrenschmidt wrote:
> +#define NVME_NVM_ADMSQES	6
>  #define NVME_NVM_IOSQES		6
>  #define NVME_NVM_IOCQES		4

The NVM in the two defines here stands for the NVM command set,
so this should just be named NVME_ADM_SQES or so.  But except for this
the patch looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

So maybe Sagi can just fix this up in the tree.

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

* Re: [PATCH v4 3/4] nvme-pci: Add support for Apple 2018+ models
  2019-08-07  7:51 ` [PATCH v4 3/4] nvme-pci: Add support for Apple 2018+ models Benjamin Herrenschmidt
@ 2019-08-22  0:28   ` Christoph Hellwig
  0 siblings, 0 replies; 14+ messages in thread
From: Christoph Hellwig @ 2019-08-22  0:28 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: linux-nvme, Sagi Grimberg, Jens Axboe, Keith Busch,
	Christoph Hellwig, linux-kernel, Paul Pawlowski

On Wed, Aug 07, 2019 at 05:51:21PM +1000, Benjamin Herrenschmidt wrote:
> Based on reverse engineering and original patch by
> 
> Paul Pawlowski <paul@mrarm.io>
> 
> This adds support for Apple weird implementation of NVME in their
> 2018 or later machines. It accounts for the twice-as-big SQ entries
> for the IO queues, and the fact that only interrupt vector 0 appears
> to function properly.
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Reviewed-by: Minwoo Im <minwoo.im.dev@gmail.com>

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

* Re: [PATCH v4 4/4] nvme-pci: Support shared tags across queues for Apple 2018 controllers
  2019-08-07  7:51 ` [PATCH v4 4/4] nvme-pci: Support shared tags across queues for Apple 2018 controllers Benjamin Herrenschmidt
@ 2019-08-22  0:29   ` Christoph Hellwig
  0 siblings, 0 replies; 14+ messages in thread
From: Christoph Hellwig @ 2019-08-22  0:29 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: linux-nvme, Sagi Grimberg, Jens Axboe, Keith Busch,
	Christoph Hellwig, linux-kernel, Paul Pawlowski

On Wed, Aug 07, 2019 at 05:51:22PM +1000, Benjamin Herrenschmidt wrote:
> Another issue with the Apple T2 based 2018 controllers seem to be
> that they blow up (and shut the machine down) if there's a tag
> collision between the IO queue and the Admin queue.
> 
> My suspicion is that they use our tags for their internal tracking
> and don't mix them with the queue id. They also seem to not like
> when tags go beyond the IO queue depth, ie 128 tags.
> 
> This adds a quirk that marks tags 0..31 of the IO queue reserved

What a mess.  But given how widely available the macbooks are supporting
them makes sense:

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

* Re: [PATCH v4 2/4] nvme-pci: Add support for variable IO SQ element size
  2019-08-22  0:28   ` Christoph Hellwig
@ 2019-08-22  0:31     ` Benjamin Herrenschmidt
  2019-08-22 18:02       ` Sagi Grimberg
  0 siblings, 1 reply; 14+ messages in thread
From: Benjamin Herrenschmidt @ 2019-08-22  0:31 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-nvme, Sagi Grimberg, Jens Axboe, Keith Busch, linux-kernel,
	Paul Pawlowski

On Thu, 2019-08-22 at 02:28 +0200, Christoph Hellwig wrote:
> On Wed, Aug 07, 2019 at 05:51:20PM +1000, Benjamin Herrenschmidt
> wrote:
> > +#define NVME_NVM_ADMSQES	6
> >  #define NVME_NVM_IOSQES		6
> >  #define NVME_NVM_IOCQES		4
> 
> The NVM in the two defines here stands for the NVM command set,
> so this should just be named NVME_ADM_SQES or so.  But except for
> this
> the patch looks good:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> So maybe Sagi can just fix this up in the tree.

Ah ok I missed the meaning. Thanks. Sagi, can you fix that up or do you
need me to resubmit ?

Cheers,
Ben.



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

* Re: [PATCH v4 2/4] nvme-pci: Add support for variable IO SQ element size
  2019-08-22  0:31     ` Benjamin Herrenschmidt
@ 2019-08-22 18:02       ` Sagi Grimberg
  2019-08-23  0:41         ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 14+ messages in thread
From: Sagi Grimberg @ 2019-08-22 18:02 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Christoph Hellwig
  Cc: linux-nvme, Jens Axboe, Keith Busch, linux-kernel, Paul Pawlowski


>> wrote:
>>> +#define NVME_NVM_ADMSQES	6
>>>   #define NVME_NVM_IOSQES		6
>>>   #define NVME_NVM_IOCQES		4
>>
>> The NVM in the two defines here stands for the NVM command set,
>> so this should just be named NVME_ADM_SQES or so.  But except for
>> this
>> the patch looks good:
>>
>> Reviewed-by: Christoph Hellwig <hch@lst.de>
>>
>> So maybe Sagi can just fix this up in the tree.
> 
> Ah ok I missed the meaning. Thanks. Sagi, can you fix that up or do you
> need me to resubmit ?

I'll fix it. Note that I'm going to take it out of the tree soon
because it will have conflicts with Jens for-5.4/block, so we
will send it to Jens after the initial merge window, after he
rebases off of Linus.

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

* Re: [PATCH v4 2/4] nvme-pci: Add support for variable IO SQ element size
  2019-08-22 18:02       ` Sagi Grimberg
@ 2019-08-23  0:41         ` Benjamin Herrenschmidt
  2019-08-23  2:52           ` Sagi Grimberg
  0 siblings, 1 reply; 14+ messages in thread
From: Benjamin Herrenschmidt @ 2019-08-23  0:41 UTC (permalink / raw)
  To: Sagi Grimberg, Christoph Hellwig
  Cc: linux-nvme, Jens Axboe, Keith Busch, linux-kernel, Paul Pawlowski

On Thu, 2019-08-22 at 11:02 -0700, Sagi Grimberg wrote:
> > > 
> I'll fix it. Note that I'm going to take it out of the tree soon
> because it will have conflicts with Jens for-5.4/block, so we
> will send it to Jens after the initial merge window, after he
> rebases off of Linus.

Conflicts too hard to fixup at merge time ? Otherwise I could just
rebase on top of Jens and put in a topic branch...

Whatever works for you.

Cheers,
Ben.



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

* Re: [PATCH v4 2/4] nvme-pci: Add support for variable IO SQ element size
  2019-08-23  0:41         ` Benjamin Herrenschmidt
@ 2019-08-23  2:52           ` Sagi Grimberg
  2019-08-23  3:50             ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 14+ messages in thread
From: Sagi Grimberg @ 2019-08-23  2:52 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Christoph Hellwig
  Cc: linux-nvme, Jens Axboe, Keith Busch, linux-kernel, Paul Pawlowski


>> I'll fix it. Note that I'm going to take it out of the tree soon
>> because it will have conflicts with Jens for-5.4/block, so we
>> will send it to Jens after the initial merge window, after he
>> rebases off of Linus.
> 
> Conflicts too hard to fixup at merge time ? Otherwise I could just
> rebase on top of Jens and put in a topic branch...

The quirk enumeration conflicts with 5.3-rc. Not a big deal, just
thought it'd be easier to handle that way.

Rebasing on top of Jens won't help because his for-5.4/block which
does not have the rc code yet.

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

* Re: [PATCH v4 2/4] nvme-pci: Add support for variable IO SQ element size
  2019-08-23  2:52           ` Sagi Grimberg
@ 2019-08-23  3:50             ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 14+ messages in thread
From: Benjamin Herrenschmidt @ 2019-08-23  3:50 UTC (permalink / raw)
  To: Sagi Grimberg, Christoph Hellwig
  Cc: linux-nvme, Jens Axboe, Keith Busch, linux-kernel, Paul Pawlowski

On Thu, 2019-08-22 at 19:52 -0700, Sagi Grimberg wrote:
> > > I'll fix it. Note that I'm going to take it out of the tree soon
> > > because it will have conflicts with Jens for-5.4/block, so we
> > > will send it to Jens after the initial merge window, after he
> > > rebases off of Linus.
> > 
> > Conflicts too hard to fixup at merge time ? Otherwise I could just
> > rebase on top of Jens and put in a topic branch...
> 
> The quirk enumeration conflicts with 5.3-rc. Not a big deal, just
> thought it'd be easier to handle that way.
> 
> Rebasing on top of Jens won't help because his for-5.4/block which
> does not have the rc code yet.

Ok sure, do as you prefer. Let me know if you want me to do anything.

Cheers,
Ben.



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

end of thread, other threads:[~2019-08-23  3:51 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-07  7:51 [PATCH v4 0/4] nvme-pci: Support for Apple 201+ (T2 chip) Benjamin Herrenschmidt
2019-08-07  7:51 ` [PATCH v4 1/4] nvme-pci: Pass the queue to SQ_SIZE/CQ_SIZE macros Benjamin Herrenschmidt
2019-08-07  7:51 ` [PATCH v4 2/4] nvme-pci: Add support for variable IO SQ element size Benjamin Herrenschmidt
2019-08-22  0:28   ` Christoph Hellwig
2019-08-22  0:31     ` Benjamin Herrenschmidt
2019-08-22 18:02       ` Sagi Grimberg
2019-08-23  0:41         ` Benjamin Herrenschmidt
2019-08-23  2:52           ` Sagi Grimberg
2019-08-23  3:50             ` Benjamin Herrenschmidt
2019-08-07  7:51 ` [PATCH v4 3/4] nvme-pci: Add support for Apple 2018+ models Benjamin Herrenschmidt
2019-08-22  0:28   ` Christoph Hellwig
2019-08-07  7:51 ` [PATCH v4 4/4] nvme-pci: Support shared tags across queues for Apple 2018 controllers Benjamin Herrenschmidt
2019-08-22  0:29   ` Christoph Hellwig
2019-08-08 23:52 ` [PATCH v4 0/4] nvme-pci: Support for Apple 201+ (T2 chip) Sagi Grimberg

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).