[PATCH] nvme: Add 48-bit DMA address quirk

[PATCH] nvme: Add 48-bit DMA address quirk

[Filippo Sironi]

Certain NVMe controllers don't support 64-bit DMA addresses.  Instead,
they are limited to 48-bit DMA addresses.  Let's add a quirk to use them
properly.

Signed-off-by: Filippo Sironi <sironi@amazon.de>
---
 drivers/nvme/host/nvme.h |  5 +++++
 drivers/nvme/host/pci.c  | 12 +++++++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 88a6b97247f5..dae747b4ac35 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -144,6 +144,11 @@ enum nvme_quirks {
 	 * NVMe 1.3 compliance.
 	 */
 	NVME_QUIRK_NO_NS_DESC_LIST		= (1 << 15),
+
+	/*
+	 * The controller supports up to 48-bit DMA address.
+	 */
+	NVME_QUIRK_DMA_ADDRESS_BITS_48		= (1 << 16),
 };
 
 /*
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 81e6389b2042..5716ae16c7a7 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -2362,13 +2362,16 @@ static int nvme_pci_enable(struct nvme_dev *dev)
 {
 	int result = -ENOMEM;
 	struct pci_dev *pdev = to_pci_dev(dev->dev);
+	int dma_address_bits = 64;
 
 	if (pci_enable_device_mem(pdev))
 		return result;
 
 	pci_set_master(pdev);
 
-	if (dma_set_mask_and_coherent(dev->dev, DMA_BIT_MASK(64)))
+	if (dev->ctrl.quirks & NVME_QUIRK_DMA_ADDRESS_BITS_48)
+		dma_address_bits = 48;
+	if (dma_set_mask_and_coherent(dev->dev, DMA_BIT_MASK(dma_address_bits)))
 		goto disable;
 
 	if (readl(dev->bar + NVME_REG_CSTS) == -1) {
@@ -3259,6 +3262,13 @@ static const struct pci_device_id nvme_id_table[] = {
 		.driver_data = NVME_QUIRK_DISABLE_WRITE_ZEROES, },
 	{ PCI_DEVICE(0x1d97, 0x2263),   /* SPCC */
 		.driver_data = NVME_QUIRK_DISABLE_WRITE_ZEROES, },
+	{ .vendor = PCI_VENDOR_ID_AMAZON,
+	  .device = PCI_ANY_ID,
+	  .subvendor = PCI_ANY_ID,
+	  .subdevice = PCI_ANY_ID,
+	  .class = PCI_CLASS_STORAGE_EXPRESS,
+	  .class_mask = 0xffffff,
+	  .driver_data = NVME_QUIRK_DMA_ADDRESS_BITS_48 },
 	{ PCI_DEVICE(PCI_VENDOR_ID_APPLE, 0x2001),
 		.driver_data = NVME_QUIRK_SINGLE_VECTOR },
 	{ PCI_DEVICE(PCI_VENDOR_ID_APPLE, 0x2003) },
-- 
2.17.1




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879




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

Re: [PATCH] nvme: Add 48-bit DMA address quirk

[Christoph Hellwig]

On Wed, Feb 03, 2021 at 10:43:38AM +0100, Filippo Sironi wrote:
> Certain NVMe controllers don't support 64-bit DMA addresses.  Instead,
> they are limited to 48-bit DMA addresses.  Let's add a quirk to use them
> properly.

WTF?  This is such a grave NVMe spec compiance bug that I do not think
we should support this buggy mess in Linux.

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

Re: [PATCH] nvme: Add 48-bit DMA address quirk

[Filippo Sironi]

On 2/3/21 10:51 AM, Christoph Hellwig wrote:
> 
> On Wed, Feb 03, 2021 at 10:43:38AM +0100, Filippo Sironi wrote:
>> Certain NVMe controllers don't support 64-bit DMA addresses.  Instead,
>> they are limited to 48-bit DMA addresses.  Let's add a quirk to use them
>> properly.
> 
> WTF?  This is such a grave NVMe spec compiance bug that I do not think
> we should support this buggy mess in Linux.
> 

I don't disagree on the first part of your sentence, this is a big 
oversight.

On the other hand, those controllers are out there and are in use by a 
lot of customers.  We can keep relying on luck, hoping that customers 
don't run into troubles or we can merge a few lines of code :)



Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879


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

Re: [PATCH] nvme: Add 48-bit DMA address quirk

[Christoph Hellwig]

On Wed, Feb 03, 2021 at 12:12:31PM +0100, Filippo Sironi wrote:
> I don't disagree on the first part of your sentence, this is a big 
> oversight.

But it is not what your commit log suggests.

> On the other hand, those controllers are out there and are in use by a lot 
> of customers.  We can keep relying on luck, hoping that customers don't run 
> into troubles or we can merge a few lines of code :)

Your patch does not just quirk a few controllers out there, but all
current and future controllers with an Amazon vendor ID.  We could
probably talk about quirking an existing vendor ID or two as long as
this doesn't happen for future hardware.

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

Re: [PATCH] nvme: Add 48-bit DMA address quirk

[Filippo Sironi]

On 2/3/21 12:15 PM, Christoph Hellwig wrote:
> 
> On Wed, Feb 03, 2021 at 12:12:31PM +0100, Filippo Sironi wrote:
>> I don't disagree on the first part of your sentence, this is a big
>> oversight.
> 
> But it is not what your commit log suggests.

I can definitely rephrase the commit.

>> On the other hand, those controllers are out there and are in use by a lot
>> of customers.  We can keep relying on luck, hoping that customers don't run
>> into troubles or we can merge a few lines of code :)
> 
> Your patch does not just quirk a few controllers out there, but all
> current and future controllers with an Amazon vendor ID.  We could
> probably talk about quirking an existing vendor ID or two as long as
> this doesn't happen for future hardware.

I know that the hardware team is working on this but I don't know the 
timelines and there are a few upcoming controllers - of which I don't 
know the device ids yet - that have the same issue.

To avoid issues, it is easier to apply the quirk to all Amazon NVMe 
controllers for now till the new lines of controllers with the fix comes 
out.  At that point, we'll be able to restrict the application to the 
known bad controllers.



Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879


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

Re: [PATCH] nvme: Add 48-bit DMA address quirk

[Christoph Hellwig]

On Wed, Feb 03, 2021 at 12:22:31PM +0100, Filippo Sironi wrote:
> To avoid issues, it is easier to apply the quirk to all Amazon NVMe 
> controllers for now till the new lines of controllers with the fix comes 
> out.  At that point, we'll be able to restrict the application to the known 
> bad controllers.

No, that is simply not acceptable.  For one we've had enough cases where
knowlege about old devices was lost after a decade or so.  And secondly
shipping more of these broken devices should be as painful as possible
for you so that it preferably does not happen at all.

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

Re: [PATCH] nvme: Add 48-bit DMA address quirk

[Keith Busch]

On Wed, Feb 03, 2021 at 12:22:31PM +0100, Filippo Sironi wrote:
> 
> On 2/3/21 12:15 PM, Christoph Hellwig wrote:
> > 
> > On Wed, Feb 03, 2021 at 12:12:31PM +0100, Filippo Sironi wrote:
> > > I don't disagree on the first part of your sentence, this is a big
> > > oversight.
> > 
> > But it is not what your commit log suggests.
> 
> I can definitely rephrase the commit.
> 
> > > On the other hand, those controllers are out there and are in use by a lot
> > > of customers.  We can keep relying on luck, hoping that customers don't run
> > > into troubles or we can merge a few lines of code :)
> > 
> > Your patch does not just quirk a few controllers out there, but all
> > current and future controllers with an Amazon vendor ID.  We could
> > probably talk about quirking an existing vendor ID or two as long as
> > this doesn't happen for future hardware.
> 
> I know that the hardware team is working on this but I don't know the
> timelines and there are a few upcoming controllers - of which I don't know
> the device ids yet - that have the same issue.
> 
> To avoid issues, it is easier to apply the quirk to all Amazon NVMe
> controllers for now till the new lines of controllers with the fix comes
> out.  At that point, we'll be able to restrict the application to the known
> bad controllers.

Just set the quirk for the known device id's and append more as needed
(which should hopefully never happen). Sure, your future broken devices
may not work with the first kernel that introduced the quirk, but this
is how the quirks should be documented in the code.

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

[PATCH v2] nvme: Add 48-bit DMA address quirk for Amazon NVMe controllers

[Filippo Sironi]

Amazon NVMe controllers do not support 64-bit DMA addresses; they are
limited to 48-bit DMA addresses.  Let's add a quirk to ensure that we
make use of 48-bit DMA addresses to avoid misbehavior.

This affects all Amazon NVMe controllers that expose EBS volumes
(0x0061, 0x0065, 0x8061) and local instance storage (0xcd00, 0xcd01,
0xcd02).

Signed-off-by: Filippo Sironi <sironi@amazon.de>
---
 drivers/nvme/host/nvme.h |  5 +++++
 drivers/nvme/host/pci.c  | 17 ++++++++++++++++-
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 88a6b97247f5..dae747b4ac35 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -144,6 +144,11 @@ enum nvme_quirks {
 	 * NVMe 1.3 compliance.
 	 */
 	NVME_QUIRK_NO_NS_DESC_LIST		= (1 << 15),
+
+	/*
+	 * The controller supports up to 48-bit DMA address.
+	 */
+	NVME_QUIRK_DMA_ADDRESS_BITS_48		= (1 << 16),
 };
 
 /*
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 6bad4d4dcdf0..e7001f5ed6e4 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -2362,13 +2362,16 @@ static int nvme_pci_enable(struct nvme_dev *dev)
 {
 	int result = -ENOMEM;
 	struct pci_dev *pdev = to_pci_dev(dev->dev);
+	int dma_address_bits = 64;
 
 	if (pci_enable_device_mem(pdev))
 		return result;
 
 	pci_set_master(pdev);
 
-	if (dma_set_mask_and_coherent(dev->dev, DMA_BIT_MASK(64)))
+	if (dev->ctrl.quirks & NVME_QUIRK_DMA_ADDRESS_BITS_48)
+		dma_address_bits = 48;
+	if (dma_set_mask_and_coherent(dev->dev, DMA_BIT_MASK(dma_address_bits)))
 		goto disable;
 
 	if (readl(dev->bar + NVME_REG_CSTS) == -1) {
@@ -3263,6 +3266,18 @@ static const struct pci_device_id nvme_id_table[] = {
 		.driver_data = NVME_QUIRK_DISABLE_WRITE_ZEROES, },
 	{ PCI_DEVICE(0x2646, 0x2263),   /* KINGSTON A2000 NVMe SSD  */
 		.driver_data = NVME_QUIRK_NO_DEEPEST_PS, },
+	{ PCI_DEVICE(PCI_VENDOR_ID_AMAZON, 0x0061),
+		.driver_data = NVME_QUIRK_DMA_ADDRESS_BITS_48, },
+	{ PCI_DEVICE(PCI_VENDOR_ID_AMAZON, 0x0065),
+		.driver_data = NVME_QUIRK_DMA_ADDRESS_BITS_48, },
+	{ PCI_DEVICE(PCI_VENDOR_ID_AMAZON, 0x8061),
+		.driver_data = NVME_QUIRK_DMA_ADDRESS_BITS_48, },
+	{ PCI_DEVICE(PCI_VENDOR_ID_AMAZON, 0xcd00),
+		.driver_data = NVME_QUIRK_DMA_ADDRESS_BITS_48, },
+	{ PCI_DEVICE(PCI_VENDOR_ID_AMAZON, 0xcd01),
+		.driver_data = NVME_QUIRK_DMA_ADDRESS_BITS_48, },
+	{ PCI_DEVICE(PCI_VENDOR_ID_AMAZON, 0xcd02),
+		.driver_data = NVME_QUIRK_DMA_ADDRESS_BITS_48, },
 	{ PCI_DEVICE(PCI_VENDOR_ID_APPLE, 0x2001),
 		.driver_data = NVME_QUIRK_SINGLE_VECTOR },
 	{ PCI_DEVICE(PCI_VENDOR_ID_APPLE, 0x2003) },
-- 
2.17.1




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879




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

Re: [PATCH v2] nvme: Add 48-bit DMA address quirk for Amazon NVMe controllers

[Christoph Hellwig]

On Wed, Feb 10, 2021 at 01:39:42AM +0100, Filippo Sironi wrote:
> Amazon NVMe controllers do not support 64-bit DMA addresses; they are
> limited to 48-bit DMA addresses.  Let's add a quirk to ensure that we
> make use of 48-bit DMA addresses to avoid misbehavior.

This should probably say some, and mention that they do not follow
the spec.  But I can fix this up when applying the patch.

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

Re: [PATCH v2] nvme: Add 48-bit DMA address quirk for Amazon NVMe controllers

[Filippo Sironi]

On 2/10/21 8:37 AM, Christoph Hellwig wrote:
> 
> On Wed, Feb 10, 2021 at 01:39:42AM +0100, Filippo Sironi wrote:
>> Amazon NVMe controllers do not support 64-bit DMA addresses; they are
>> limited to 48-bit DMA addresses.  Let's add a quirk to ensure that we
>> make use of 48-bit DMA addresses to avoid misbehavior.
> 
> This should probably say some, and mention that they do not follow
> the spec.  But I can fix this up when applying the patch.
> 

Thanks!

Filippo



Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879


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