* [PATCHv2] nvme: add command id quirk for apple controllers
@ 2021-09-27 15:43 ` Keith Busch
0 siblings, 0 replies; 12+ messages in thread
From: Keith Busch @ 2021-09-27 15:43 UTC (permalink / raw)
To: linux-nvme, sagi, hch
Cc: linux-kernel, axboe, Keith Busch, Sven Peter,
Orlando Chamberlain, Aditya Garg
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.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=214509
Cc: Sven Peter <sven@svenpeter.dev>
Reported-by: Orlando Chamberlain <redecorating@protonmail.com>
Reported-by: Aditya Garg <gargaditya08@live.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
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(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index e486845d2c7e..7712a8f78337 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -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;
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 9871c0c9374c..ed79a6c7e804 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -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),
};
/*
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index b82492cd7503..456a0e8a5718 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -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
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCHv2] nvme: add command id quirk for apple controllers
2021-09-27 15:43 ` Keith Busch
@ 2021-09-27 15:48 ` Christoph Hellwig
-1 siblings, 0 replies; 12+ messages in thread
From: Christoph Hellwig @ 2021-09-27 15:48 UTC (permalink / raw)
To: Keith Busch
Cc: linux-nvme, sagi, hch, linux-kernel, axboe, Sven Peter,
Orlando Chamberlain, Aditya Garg
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
Jens, given that you're on the thread, do you want to pick this up
directly to reduce the patch queue latency?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCHv2] nvme: add command id quirk for apple controllers
@ 2021-09-27 15:48 ` Christoph Hellwig
0 siblings, 0 replies; 12+ messages in thread
From: Christoph Hellwig @ 2021-09-27 15:48 UTC (permalink / raw)
To: Keith Busch
Cc: linux-nvme, sagi, hch, linux-kernel, axboe, Sven Peter,
Orlando Chamberlain, Aditya Garg
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
Jens, given that you're on the thread, do you want to pick this up
directly to reduce the patch queue latency?
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCHv2] nvme: add command id quirk for apple controllers
2021-09-27 15:48 ` Christoph Hellwig
@ 2021-09-27 16:01 ` Jens Axboe
-1 siblings, 0 replies; 12+ messages in thread
From: Jens Axboe @ 2021-09-27 16:01 UTC (permalink / raw)
To: Christoph Hellwig, Keith Busch
Cc: linux-nvme, sagi, linux-kernel, Sven Peter, Orlando Chamberlain,
Aditya Garg
On 9/27/21 9:48 AM, Christoph Hellwig wrote:
> Looks good:
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
>
> Jens, given that you're on the thread, do you want to pick this up
> directly to reduce the patch queue latency?
Sure, applied.
--
Jens Axboe
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCHv2] nvme: add command id quirk for apple controllers
@ 2021-09-27 16:01 ` Jens Axboe
0 siblings, 0 replies; 12+ messages in thread
From: Jens Axboe @ 2021-09-27 16:01 UTC (permalink / raw)
To: Christoph Hellwig, Keith Busch
Cc: linux-nvme, sagi, linux-kernel, Sven Peter, Orlando Chamberlain,
Aditya Garg
On 9/27/21 9:48 AM, Christoph Hellwig wrote:
> Looks good:
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
>
> Jens, given that you're on the thread, do you want to pick this up
> directly to reduce the patch queue latency?
Sure, applied.
--
Jens Axboe
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCHv2] nvme: add command id quirk for apple controllers
2021-09-27 15:43 ` Keith Busch
@ 2021-09-27 15:59 ` Sven Peter
-1 siblings, 0 replies; 12+ messages in thread
From: Sven Peter @ 2021-09-27 15:59 UTC (permalink / raw)
To: Keith Busch, linux-nvme, sagi, hch
Cc: linux-kernel, axboe, Orlando Chamberlain, Aditya Garg
On Mon, Sep 27, 2021, at 17:43, Keith Busch wrote:
> 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.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=214509
> Cc: Sven Peter <sven@svenpeter.dev>
> Reported-by: Orlando Chamberlain <redecorating@protonmail.com>
> Reported-by: Aditya Garg <gargaditya08@live.com>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
> 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(-)
Looks good to me, on the M1 with my out-of-tree driver:
Tested-by: Sven Peter <sven@svenpeter.dev>
Thanks,
Sven
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCHv2] nvme: add command id quirk for apple controllers
@ 2021-09-27 15:59 ` Sven Peter
0 siblings, 0 replies; 12+ messages in thread
From: Sven Peter @ 2021-09-27 15:59 UTC (permalink / raw)
To: Keith Busch, linux-nvme, sagi, hch
Cc: linux-kernel, axboe, Orlando Chamberlain, Aditya Garg
On Mon, Sep 27, 2021, at 17:43, Keith Busch wrote:
> 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.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=214509
> Cc: Sven Peter <sven@svenpeter.dev>
> Reported-by: Orlando Chamberlain <redecorating@protonmail.com>
> Reported-by: Aditya Garg <gargaditya08@live.com>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
> 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(-)
Looks good to me, on the M1 with my out-of-tree driver:
Tested-by: Sven Peter <sven@svenpeter.dev>
Thanks,
Sven
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCHv2] nvme: add command id quirk for apple controllers
2021-09-27 15:43 ` Keith Busch
@ 2021-09-28 3:55 ` Orlando Chamberlain
-1 siblings, 0 replies; 12+ messages in thread
From: Orlando Chamberlain @ 2021-09-28 3:55 UTC (permalink / raw)
To: Keith Busch, linux-nvme, sagi, hch
Cc: linux-kernel, axboe, Sven Peter, Aditya Garg
On 28/9/21 01:43, Keith Busch wrote:
> 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.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=214509
> Cc: Sven Peter <sven@svenpeter.dev>
> Reported-by: Orlando Chamberlain <redecorating@protonmail.com>
> Reported-by: Aditya Garg <gargaditya08@live.com>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
> 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(-)
Works on my MacBookPro16,1.
Tested-by: Orlando Chamberlain <redecorating@protonmail.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCHv2] nvme: add command id quirk for apple controllers
@ 2021-09-28 3:55 ` Orlando Chamberlain
0 siblings, 0 replies; 12+ messages in thread
From: Orlando Chamberlain @ 2021-09-28 3:55 UTC (permalink / raw)
To: Keith Busch, linux-nvme, sagi, hch
Cc: linux-kernel, axboe, Sven Peter, Aditya Garg
On 28/9/21 01:43, Keith Busch wrote:
> 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.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=214509
> Cc: Sven Peter <sven@svenpeter.dev>
> Reported-by: Orlando Chamberlain <redecorating@protonmail.com>
> Reported-by: Aditya Garg <gargaditya08@live.com>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
> 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(-)
Works on my MacBookPro16,1.
Tested-by: Orlando Chamberlain <redecorating@protonmail.com>
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCHv2] nvme: add command id quirk for apple controllers
2021-09-27 15:43 ` Keith Busch
@ 2021-09-28 5:38 ` Aditya Garg
-1 siblings, 0 replies; 12+ messages in thread
From: Aditya Garg @ 2021-09-28 5:38 UTC (permalink / raw)
To: Keith Busch, linux-nvme, sagi, hch
Cc: linux-kernel, axboe, sven, Orlando Chamberlain
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.
Tested-by: Aditya Garg <gargaditya08@live.com>
________________________________________
From: Keith Busch <kbusch@kernel.org>
Sent: Monday, September 27, 2021 3:43 PM
To: linux-nvme@lists.infradead.org; sagi@grimberg.me; hch@lst.de
Cc: linux-kernel@vger.kernel.org; axboe@kernel.dk; 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.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=214509
Cc: Sven Peter <sven@svenpeter.dev>
Reported-by: Orlando Chamberlain <redecorating@protonmail.com>
Reported-by: Aditya Garg <gargaditya08@live.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
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(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index e486845d2c7e..7712a8f78337 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -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;
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 9871c0c9374c..ed79a6c7e804 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -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),
};
/*
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index b82492cd7503..456a0e8a5718 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -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
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCHv2] nvme: add command id quirk for apple controllers
@ 2021-09-28 5:38 ` Aditya Garg
0 siblings, 0 replies; 12+ messages in thread
From: Aditya Garg @ 2021-09-28 5:38 UTC (permalink / raw)
To: Keith Busch, linux-nvme, sagi, hch
Cc: linux-kernel, axboe, sven, Orlando Chamberlain
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.
Tested-by: Aditya Garg <gargaditya08@live.com>
________________________________________
From: Keith Busch <kbusch@kernel.org>
Sent: Monday, September 27, 2021 3:43 PM
To: linux-nvme@lists.infradead.org; sagi@grimberg.me; hch@lst.de
Cc: linux-kernel@vger.kernel.org; axboe@kernel.dk; 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.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=214509
Cc: Sven Peter <sven@svenpeter.dev>
Reported-by: Orlando Chamberlain <redecorating@protonmail.com>
Reported-by: Aditya Garg <gargaditya08@live.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
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(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index e486845d2c7e..7712a8f78337 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -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;
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 9871c0c9374c..ed79a6c7e804 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -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),
};
/*
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index b82492cd7503..456a0e8a5718 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -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
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 12+ messages in thread