nvme: temporary fix for Apple controller reset

Recent patches added basic support for the Apple NVMe controller but still cause resets and data corruption on that particular controller when a specific pattern of read/flush commands occurs. Limiting the queue depth to 2 works around that issue.

This patch enforces that limit only for the Apple controller and is considered a temporary fix until we find the root source of that problem.

Signed-off-by Stephan G?nther <guenther at tum.de>

Signed-off-by Maurice Leclaire <leclaire at in.tum.de>

---
 drivers/nvme/host/pci.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

drivers/nvme/host/pci.c +12 -0

index 8187df2..d3c2b03 100644
@@ -2701,6 +2701,18 @@ static int nvme_dev_map(struct nvme_dev *dev)
 	dev->q_depth = min_t(int, NVME_CAP_MQES(cap) + 1, NVME_Q_DEPTH);
 	dev->db_stride = 1 << NVME_CAP_STRIDE(cap);
 	dev->dbs = ((void __iomem *)dev->bar) + 4096;
+
+	/*
+ 	 * Temporary fix for the Apple controller found in the MacBook8,1 and
+	 * some MacBook7,1 to avoid controller resets and data loss.
+ 	 */
+	if (pdev->vendor == PCI_VENDOR_ID_APPLE && pdev->device == 0x2001) {
+ 		dev->q_depth = 2;
+		dev_warn(dev->dev, "detected Apple NVMe controller, set "
+			"queue depth=%u to work around controller resets\n",
+			dev->q_depth);
+	}
+
 	if (readl(&dev->bar->vs) >= NVME_VS(1, 2))
 		dev->cmb = nvme_map_cmb(dev);
 

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

The patch below has been reviewed by Christoph and reported to work. However, there is still no sign that it will be applied to linux-4.4.

Please either undo commit c74dc7801d515d01847fd5cf2b472489fa5717b1, which added the PCI ID of the Apple controller, or merge the patch below asap.

Currently, the driver will make that controller destroy data!

Quoted 8 lines · Stephan G?nther

The patch below has been reviewed by Christoph and reported to work. However, there is still no sign that it will be applied to linux-4.4.

Please either undo commit c74dc7801d515d01847fd5cf2b472489fa5717b1, which added the PCI ID of the Apple controller, or merge the patch below asap.

Quoted 1 line

Currently, the driver will make that controller destroy data!

Honestly, I'd rather revert the pci id addition, unless there's conclusive evidence that limiting the (per queue) depth to 2 really does fix the issue.

Is this what the OSX driver does? What testing was done to ascertain that 2 is the magic number? Does it just make it harder to hit, or does it really fix it?

Quoted 9 lines · Jens Axboe

Quoted 8 lines · Stephan G?nther

The patch below has been reviewed by Christoph and reported to work. However, there is still no sign that it will be applied to linux-4.4.

Please either undo commit c74dc7801d515d01847fd5cf2b472489fa5717b1, which added the PCI ID of the Apple controller, or merge the patch below asap.

Quoted 4 lines

Quoted 1 line

Currently, the driver will make that controller destroy data!

Honestly, I'd rather revert the pci id addition, unless there's conclusive evidence that limiting the (per queue) depth to 2 really does fix the issue.

Wes Cilldhaire tested the patch (his answer is in that thread) - as I did the past 3 weeks ago as I'm running solely Linux on that broken MacBook.

Quoted 3 lines

Is this what the OSX driver does? What testing was done to ascertain that 2 is the magic number? Does it just make it harder to hit, or does it really fix it?

I do not know what the OSX driver does. Apple is of absolutely no help whatsoever.

But without that patch mkfs btrfs immediately fails. Even partprobe /dev/nvme0n1 will reset the controller (the latter one without data loss).

As I am running a btrfs on a luks container for 3 weeks with that patch, it *does* work. Performance is, obviously, not the best one I have seen. But given the circumstances I cannot complain at all.

So again: please either revert the previous patch and leave it to the more interested individuals to try it (probably slowing down Linux support for that MacBook as not detecting the hard-soldered disk is quite a blocker), or merge that hotfix until we find a better solution.

With 4.5 that may even become a quirk.

Best,
Stephan

Quoted 9 lines · Jens Axboe

Quoted 8 lines · Stephan G?nther

The patch below has been reviewed by Christoph and reported to work. However, there is still no sign that it will be applied to linux-4.4.

Please either undo commit c74dc7801d515d01847fd5cf2b472489fa5717b1, which added the PCI ID of the Apple controller, or merge the patch below asap.

Quoted 4 lines

Quoted 1 line

Currently, the driver will make that controller destroy data!

Honestly, I'd rather revert the pci id addition, unless there's conclusive evidence that limiting the (per queue) depth to 2 really does fix the issue.

Wes Cilldhaire tested the patch (his answer is in that thread) - as I did the past 3 weeks ago as I'm running solely Linux on that broken MacBook.

Quoted 3 lines

Is this what the OSX driver does? What testing was done to ascertain that 2 is the magic number? Does it just make it harder to hit, or does it really fix it?

I do not know what the OSX driver does. Apple is of absolutely no help whatsoever.

But without that patch mkfs btrfs immediately fails. Even partprobe /dev/nvme0n1 will reset the controller (the latter one without data loss).

As I am running a btrfs on a luks container for 3 weeks with that patch, it *does* work. Performance is, obviously, not the best one I have seen. But given the circumstances I cannot complain at all.

So again: please either revert the previous patch and leave it to the more interested individuals to try it (probably slowing down Linux support for that MacBook as not detecting the hard-soldered disk is quite a blocker), or merge that hotfix until we find a better solution.

With 4.5 that may even become a quirk.