nvme: add command id quirk for apple controllers
Keith Busch
maintainerPATCH v2 1/1 nvme: add command id quirk for apple controllers
Some apple controllers use the command id as an index to implementation specific data structures and will fail if the value is out of bounds. The nvme driver's recently introduced command sequence number breaks this controller.
Provide a quirk so these spec incompliant controllers can function as before. The driver will not have the ability to detect bad completions when this quirk is used, but we weren't previously checking this anyway.
The quirk bit was selected so that it can readily apply to stable.
---v1->v2: fixed logical bug checking the quirk setting
drivers/nvme/host/core.c | 4 +++- drivers/nvme/host/nvme.h | 6 ++++++ drivers/nvme/host/pci.c | 3 ++- 3 files changed, 11 insertions(+), 2 deletions(-)
drivers/nvme/host/core.c +3 -1
@@ -978,6 +978,7 @@ EXPORT_SYMBOL_GPL(nvme_cleanup_cmd);blk_status_t nvme_setup_cmd (struct nvme_ns *ns,struct request *req) {struct nvme_command *cmd =nvme_req (req)->cmd; +struct nvme_ctrl *ctrl =nvme_req (req)->ctrl;blk_status_t ret =BLK_STS_OK ;if (!(req->rq_flags &RQF_DONTPREP )) {
@@ -1026,7 +1027,8 @@ blk_status_t nvme_setup_cmd(struct nvme_ns *ns, struct request *req)return BLK_STS_IOERR ; } -nvme_req (req)->genctr++; +if (!(ctrl->quirks &NVME_QUIRK_SKIP_CID_GEN )) +nvme_req (req)->genctr++; cmd->common.command_id =nvme_cid (req);trace_nvme_setup_cmd (req, cmd);return ret;
drivers/nvme/host/nvme.h +6 -0
@@ -138,6 +138,12 @@ enum nvme_quirks {* 48 bits. */ NVME_QUIRK_DMA_ADDRESS_BITS_48 = (1 <<16 ), + +/* +* The controller requires the command_id value be be limited, so skip +* encoding the generation sequence number. +*/ + NVME_QUIRK_SKIP_CID_GEN = (1 << 17), };/*
drivers/nvme/host/pci.c +2 -1
@@ -3369,7 +3369,8 @@ static const struct pci_device_id nvme_id_table[] = { { PCI_DEVICE(PCI_VENDOR_ID_APPLE, 0x2005), .driver_data =NVME_QUIRK_SINGLE_VECTOR |NVME_QUIRK_128_BYTES_SQES | - NVME_QUIRK_SHARED_TAGS }, +NVME_QUIRK_SHARED_TAGS | + NVME_QUIRK_SKIP_CID_GEN }, { PCI_DEVICE_CLASS(PCI_CLASS_STORAGE_EXPRESS, 0xffffff) }, { 0, }
Signature · 1 line
2.25.4
Looks good:
Jens, given that you're on the thread, do you want to pick this up directly to reduce the patch queue latency?
Jens Axboe
maintainerQuoted 6 lines · Christoph Hellwig
Looks good:
Reviewed-by Christoph Hellwig <[email protected]>Jens, given that you're on the thread, do you want to pick this up directly to reduce the patch queue latency?
Sure, applied.
Signature · 1 line
Jens Axboe
Quoted 16 lines · Keith Busch
Some apple controllers use the command id as an index to implementation specific data structures and will fail if the value is out of bounds. The nvme driver's recently introduced command sequence number breaks this controller.
Provide a quirk so these spec incompliant controllers can function as before. The driver will not have the ability to detect bad completions when this quirk is used, but we weren't previously checking this anyway.
The quirk bit was selected so that it can readily apply to stable.
Cc Sven Peter <[email protected]>Reported-by Orlando Chamberlain <[email protected]>Reported-by Aditya Garg <[email protected]>Signed-off-by Keith Busch <[email protected]>
Quoted 7 lines
---
v1->v2: fixed logical bug checking the quirk settingdrivers/nvme/host/core.c | 4 +++- drivers/nvme/host/nvme.h | 6 ++++++ drivers/nvme/host/pci.c | 3 ++- 3 files changed, 11 insertions(+), 2 deletions(-)
Looks good to me, on the M1 with my out-of-tree driver:
Thanks,
Sven
Quoted 16 lines · Keith Busch
Some apple controllers use the command id as an index to implementation specific data structures and will fail if the value is out of bounds. The nvme driver's recently introduced command sequence number breaks this controller.
Provide a quirk so these spec incompliant controllers can function as before. The driver will not have the ability to detect bad completions when this quirk is used, but we weren't previously checking this anyway.
The quirk bit was selected so that it can readily apply to stable.
Cc Sven Peter <[email protected]>Reported-by Orlando Chamberlain <[email protected]>Reported-by Aditya Garg <[email protected]>Signed-off-by Keith Busch <[email protected]>
Quoted 7 lines
---
v1->v2: fixed logical bug checking the quirk settingdrivers/nvme/host/core.c | 4 +++- drivers/nvme/host/nvme.h | 6 ++++++ drivers/nvme/host/pci.c | 3 ++- 3 files changed, 11 insertions(+), 2 deletions(-)
Works on my MacBookPro16,1.
This patch fixes the issue on my MacBook Pro 16,1. I have tested this on 5.14.7 thus it has fixed the problem on stable.
________________________________________
From: Keith Busch <[email protected]>
Sent: Monday, September 27, 2021 3:43 PM
To: [email protected]; [email protected]; [email protected]
Cc: [email protected]; [email protected]; Keith Busch; Sven Peter; Orlando Chamberlain; Aditya Garg
Subject: [PATCHv2] nvme: add command id quirk for apple controllers
Some apple controllers use the command id as an index to implementation specific data structures and will fail if the value is out of bounds. The nvme driver's recently introduced command sequence number breaks this controller.
Provide a quirk so these spec incompliant controllers can function as before. The driver will not have the ability to detect bad completions when this quirk is used, but we weren't previously checking this anyway.
The quirk bit was selected so that it can readily apply to stable.
---v1->v2: fixed logical bug checking the quirk setting
drivers/nvme/host/core.c | 4 +++- drivers/nvme/host/nvme.h | 6 ++++++ drivers/nvme/host/pci.c | 3 ++- 3 files changed, 11 insertions(+), 2 deletions(-)
drivers/nvme/host/core.c +3 -1
@@ -978,6 +978,7 @@ EXPORT_SYMBOL_GPL(nvme_cleanup_cmd);blk_status_t nvme_setup_cmd (struct nvme_ns *ns,struct request *req) {struct nvme_command *cmd =nvme_req (req)->cmd; +struct nvme_ctrl *ctrl =nvme_req (req)->ctrl;blk_status_t ret =BLK_STS_OK ;if (!(req->rq_flags &RQF_DONTPREP )) {
@@ -1026,7 +1027,8 @@ blk_status_t nvme_setup_cmd(struct nvme_ns *ns, struct request *req)return BLK_STS_IOERR ; } -nvme_req (req)->genctr++; +if (!(ctrl->quirks &NVME_QUIRK_SKIP_CID_GEN )) +nvme_req (req)->genctr++; cmd->common.command_id =nvme_cid (req);trace_nvme_setup_cmd (req, cmd);return ret;
drivers/nvme/host/nvme.h +6 -0
@@ -138,6 +138,12 @@ enum nvme_quirks {* 48 bits. */ NVME_QUIRK_DMA_ADDRESS_BITS_48 = (1 <<16 ), + +/* +* The controller requires the command_id value be be limited, so skip +* encoding the generation sequence number. +*/ + NVME_QUIRK_SKIP_CID_GEN = (1 << 17), };/*
drivers/nvme/host/pci.c +2 -1
@@ -3369,7 +3369,8 @@ static const struct pci_device_id nvme_id_table[] = { { PCI_DEVICE(PCI_VENDOR_ID_APPLE, 0x2005), .driver_data =NVME_QUIRK_SINGLE_VECTOR |NVME_QUIRK_128_BYTES_SQES | - NVME_QUIRK_SHARED_TAGS }, +NVME_QUIRK_SHARED_TAGS | + NVME_QUIRK_SKIP_CID_GEN }, { PCI_DEVICE_CLASS(PCI_CLASS_STORAGE_EXPRESS, 0xffffff) }, { 0, }
--
2.25.4