[PATCH 0/2] nvme-apple: Prevent tag collision across queues on Apple A11

[PATCH 0/2] nvme-apple: Prevent tag collision across queues on Apple A11

[Nick Chan]

Apple NVMe controllers require tags of pending commands to not be shared
across admin and IO queues.

If a pending command tag is duplicated across queues, the firmware
crashes with: "duplicate tag error for tag N", with N being the tag.

On Apple M1 or above, this is worked around by partitioning the tag
space between the admin and IO queue.

However, on Apple A11 without linear SQ, it is not possible for either
queue to skip over some tags and must go from 0 to the configured maximum
before wrapping around.

Instead of partitioning the tag space, which is not possible without
linear SQ, prevent tag collisions by keeping track of which tags are
currently in-flight across either queues, and return BLK_STS_RESOURCE to
temporaily block command submission when a collision would have occured.

While fixing the issue, it became apparent the admin queue tag space
is limited even on Apple A11. There is no reason to do this as it hampers
performance and does not help preventing tag collisions, so also allow
the admin queue to use the full tag space.

Tested on iPhone 8, iPhone X and Macbook Pro (14-inch, M1 Pro, 2021).

Signed-off-by: Nick Chan <towinchenmi@gmail.com>
---
Nick Chan (1):
      nvme-apple: Only limit admin queue tag space when with Linear SQ is present

Yuriy Havrylyuk (1):
      nvme-apple: Prevent tag collision across queues even if tag space is shared

 drivers/nvme/host/apple.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 69 insertions(+), 1 deletion(-)
---
base-commit: e43ffb69e0438cddd72aaa30898b4dc446f664f8
change-id: 20260606-prevent-tag-collision-t8015-1c8adb3234de

Best regards,
-- 
Nick Chan <towinchenmi@gmail.com>

[PATCH 1/2] nvme-apple: Only limit admin queue tag space when with Linear SQ is present

[Nick Chan]

Apple NVMe controllers require tags of pending commands to not be shared
across admin and IO queues. However, on Apple A11 without linear SQ, it is
not possible for either queue to skip over some tags and must go from 0 to
the configured maximum before wrapping around.

As a result, in order to prevent tag collision, dynamic tag reservation
while a command is in-flight becomes necessary. In this context, there is
no reason to limit the admin queue's tag space, as it is not helpful in
preventing tag collision.

Cc: stable@vger.kernel.org
Fixes: 04d8ecf37b5e ("nvme: apple: Add Apple A11 support")
Signed-off-by: Nick Chan <towinchenmi@gmail.com>
---
 drivers/nvme/host/apple.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/apple.c b/drivers/nvme/host/apple.c
index c692fc73babf..c1115e27a0d6 100644
--- a/drivers/nvme/host/apple.c
+++ b/drivers/nvme/host/apple.c
@@ -1303,7 +1303,10 @@ static int apple_nvme_alloc_tagsets(struct apple_nvme *anv)
 
 	anv->admin_tagset.ops = &apple_nvme_mq_admin_ops;
 	anv->admin_tagset.nr_hw_queues = 1;
-	anv->admin_tagset.queue_depth = APPLE_NVME_AQ_MQ_TAG_DEPTH;
+	if (anv->hw->has_lsq_nvmmu)
+		anv->admin_tagset.queue_depth = APPLE_NVME_AQ_MQ_TAG_DEPTH;
+	else
+		anv->admin_tagset.queue_depth = anv->hw->max_queue_depth - 1;
 	anv->admin_tagset.timeout = NVME_ADMIN_TIMEOUT;
 	anv->admin_tagset.numa_node = NUMA_NO_NODE;
 	anv->admin_tagset.cmd_size = sizeof(struct apple_nvme_iod);

-- 
2.54.0

[PATCH 2/2] nvme-apple: Prevent tag collision across queues even if tag space is shared

[Nick Chan]

From: Yuriy Havrylyuk <yhavry@gmail.com>

Apple NVMe controllers require tags of pending commands to not be shared
across admin and IO queues. However, on Apple A11 without linear SQ, it is
not possible for either queue to skip over some tags and must go from 0 to
the configured maximum before wrapping around.

If a pending command tag is duplicated across queues, the firmware
crashes with: "duplicate tag error for tag N", with N being the tag.

Instead of partitioning the tag space, which is not possible without
linear SQ, prevent tag collisions by keeping track of which tags are
currently in-flight across either queues, and return BLK_STS_RESOURCE to
temporaily block command submission when a collision would have occurred.

Cc: stable@vger.kernel.org
Fixes: 04d8ecf37b5e ("nvme: apple: Add Apple A11 support")
Signed-off-by: Yuriy Havrylyuk <yhavry@gmail.com>
Co-developed-by: Nick Chan <towinchenmi@gmail.com>
Signed-off-by: Nick Chan <towinchenmi@gmail.com>
---
 drivers/nvme/host/apple.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/drivers/nvme/host/apple.c b/drivers/nvme/host/apple.c
index c1115e27a0d6..6354edf27225 100644
--- a/drivers/nvme/host/apple.c
+++ b/drivers/nvme/host/apple.c
@@ -203,6 +203,20 @@ struct apple_nvme {
 
 	int irq;
 	spinlock_t lock;
+
+	/*
+	 * Tags of pending commands must be unique across both Admin and IO
+	 * queue. However, on T8015, unlike T8103, without linear submission
+	 * queues, it is not possible for the either queue to skip some tags,
+	 * and both queues must go from 0 to their respective configured
+	 * maximum.
+	 *
+	 * Instead of reserving some tags for the admin queue, use a bitfield
+	 * to keep track of pending commands on either queue, and temporaily
+	 * block command submission by returning BLK_STS_RESOURCE until the
+	 * tag is freed on the other queue.
+	 */
+	unsigned long t8015_active_tags;
 };
 
 static_assert(sizeof(struct nvme_command) == 64);
@@ -290,6 +304,28 @@ static void apple_nvmmu_inval(struct apple_nvme_queue *q, unsigned int tag)
 				     "NVMMU TCB invalidation failed\n");
 }
 
+static bool apple_nvme_reserve_tag_t8015(struct apple_nvme *anv,
+					 struct nvme_command *cmd)
+{
+	u16 tag = nvme_tag_from_cid(cmd->common.command_id);
+
+	if (WARN_ON_ONCE(tag >= BITS_PER_LONG))
+		return false;
+
+	return !test_and_set_bit(tag, &anv->t8015_active_tags);
+}
+
+static void apple_nvme_release_tag_t8015(struct apple_nvme *anv,
+					 __u16 command_id)
+{
+	u16 tag = nvme_tag_from_cid(command_id);
+
+	if (WARN_ON_ONCE(tag >= BITS_PER_LONG))
+		return;
+
+	clear_bit(tag, &anv->t8015_active_tags);
+}
+
 static void apple_nvme_submit_cmd_t8015(struct apple_nvme_queue *q,
 				  struct nvme_command *cmd)
 {
@@ -652,6 +688,8 @@ static inline void apple_nvme_update_cq_head(struct apple_nvme_queue *q)
 static bool apple_nvme_poll_cq(struct apple_nvme_queue *q,
 			       struct io_comp_batch *iob)
 {
+	struct apple_nvme *anv = queue_to_apple_nvme(q);
+	unsigned long completed_tags = 0;
 	bool found = false;
 
 	while (apple_nvme_cqe_pending(q)) {
@@ -664,11 +702,26 @@ static bool apple_nvme_poll_cq(struct apple_nvme_queue *q,
 		dma_rmb();
 		apple_nvme_handle_cqe(q, iob, q->cq_head);
 		apple_nvme_update_cq_head(q);
+
+		if (!anv->hw->has_lsq_nvmmu) {
+			struct nvme_completion *cqe = &q->cqes[q->cq_head];
+			u16 tag = nvme_tag_from_cid(READ_ONCE(cqe->command_id));
+
+			if (!WARN_ON_ONCE(tag >= BITS_PER_LONG))
+				__set_bit(tag, &completed_tags);
+		}
 	}
 
 	if (found)
 		writel(q->cq_head, q->cq_db);
 
+	if (!anv->hw->has_lsq_nvmmu && completed_tags) {
+		unsigned long tag_bit;
+
+		for_each_set_bit(tag_bit, &completed_tags, BITS_PER_LONG)
+			clear_bit(tag_bit, &anv->t8015_active_tags);
+	}
+
 	return found;
 }
 
@@ -790,6 +843,12 @@ static blk_status_t apple_nvme_queue_rq(struct blk_mq_hw_ctx *hctx,
 	if (ret)
 		return ret;
 
+	if (!anv->hw->has_lsq_nvmmu &&
+	    !apple_nvme_reserve_tag_t8015(anv, cmnd)) {
+		ret = BLK_STS_RESOURCE;
+		goto out_free_cmd;
+	}
+
 	if (blk_rq_nr_phys_segments(req)) {
 		ret = apple_nvme_map_data(anv, req, cmnd);
 		if (ret)
@@ -806,6 +865,9 @@ static blk_status_t apple_nvme_queue_rq(struct blk_mq_hw_ctx *hctx,
 	return BLK_STS_OK;
 
 out_free_cmd:
+	if (!anv->hw->has_lsq_nvmmu)
+		apple_nvme_release_tag_t8015(anv, cmnd->common.command_id);
+
 	nvme_cleanup_cmd(req);
 	return ret;
 }
@@ -1165,6 +1227,9 @@ static void apple_nvme_reset_work(struct work_struct *work)
 	if (ret)
 		goto out;
 
+	if (!anv->hw->has_lsq_nvmmu)
+		WRITE_ONCE(anv->t8015_active_tags, 0);
+
 	dev_dbg(anv->dev, "Starting admin queue");
 	apple_nvme_init_queue(&anv->adminq);
 	nvme_unquiesce_admin_queue(&anv->ctrl);

-- 
2.54.0

Re: [PATCH 2/2] nvme-apple: Prevent tag collision across queues even if tag space is shared

[David Laight]

On Sat, 06 Jun 2026 21:25:26 +0800
Nick Chan <towinchenmi@gmail.com> wrote:

> From: Yuriy Havrylyuk <yhavry@gmail.com>
> 
> Apple NVMe controllers require tags of pending commands to not be shared
> across admin and IO queues. However, on Apple A11 without linear SQ, it is
> not possible for either queue to skip over some tags and must go from 0 to
> the configured maximum before wrapping around.
> 
> If a pending command tag is duplicated across queues, the firmware
> crashes with: "duplicate tag error for tag N", with N being the tag.
> 
> Instead of partitioning the tag space, which is not possible without
> linear SQ, prevent tag collisions by keeping track of which tags are
> currently in-flight across either queues, and return BLK_STS_RESOURCE to
> temporaily block command submission when a collision would have occurred.

I look at using the atomic64_xxx() functions rather than the bitmask ones.
The for_each_bit_set() loop is then an atmomic64_andnot() call.

-- David


> 
> Cc: stable@vger.kernel.org
> Fixes: 04d8ecf37b5e ("nvme: apple: Add Apple A11 support")
> Signed-off-by: Yuriy Havrylyuk <yhavry@gmail.com>
> Co-developed-by: Nick Chan <towinchenmi@gmail.com>
> Signed-off-by: Nick Chan <towinchenmi@gmail.com>
> ---
>  drivers/nvme/host/apple.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 65 insertions(+)
> 
> diff --git a/drivers/nvme/host/apple.c b/drivers/nvme/host/apple.c
> index c1115e27a0d6..6354edf27225 100644
> --- a/drivers/nvme/host/apple.c
> +++ b/drivers/nvme/host/apple.c
> @@ -203,6 +203,20 @@ struct apple_nvme {
>  
>  	int irq;
>  	spinlock_t lock;
> +
> +	/*
> +	 * Tags of pending commands must be unique across both Admin and IO
> +	 * queue. However, on T8015, unlike T8103, without linear submission
> +	 * queues, it is not possible for the either queue to skip some tags,
> +	 * and both queues must go from 0 to their respective configured
> +	 * maximum.
> +	 *
> +	 * Instead of reserving some tags for the admin queue, use a bitfield
> +	 * to keep track of pending commands on either queue, and temporaily
> +	 * block command submission by returning BLK_STS_RESOURCE until the
> +	 * tag is freed on the other queue.
> +	 */
> +	unsigned long t8015_active_tags;
>  };
>  
>  static_assert(sizeof(struct nvme_command) == 64);
> @@ -290,6 +304,28 @@ static void apple_nvmmu_inval(struct apple_nvme_queue *q, unsigned int tag)
>  				     "NVMMU TCB invalidation failed\n");
>  }
>  
> +static bool apple_nvme_reserve_tag_t8015(struct apple_nvme *anv,
> +					 struct nvme_command *cmd)
> +{
> +	u16 tag = nvme_tag_from_cid(cmd->common.command_id);
> +
> +	if (WARN_ON_ONCE(tag >= BITS_PER_LONG))
> +		return false;
> +
> +	return !test_and_set_bit(tag, &anv->t8015_active_tags);
> +}
> +
> +static void apple_nvme_release_tag_t8015(struct apple_nvme *anv,
> +					 __u16 command_id)
> +{
> +	u16 tag = nvme_tag_from_cid(command_id);
> +
> +	if (WARN_ON_ONCE(tag >= BITS_PER_LONG))
> +		return;
> +
> +	clear_bit(tag, &anv->t8015_active_tags);
> +}
> +
>  static void apple_nvme_submit_cmd_t8015(struct apple_nvme_queue *q,
>  				  struct nvme_command *cmd)
>  {
> @@ -652,6 +688,8 @@ static inline void apple_nvme_update_cq_head(struct apple_nvme_queue *q)
>  static bool apple_nvme_poll_cq(struct apple_nvme_queue *q,
>  			       struct io_comp_batch *iob)
>  {
> +	struct apple_nvme *anv = queue_to_apple_nvme(q);
> +	unsigned long completed_tags = 0;
>  	bool found = false;
>  
>  	while (apple_nvme_cqe_pending(q)) {
> @@ -664,11 +702,26 @@ static bool apple_nvme_poll_cq(struct apple_nvme_queue *q,
>  		dma_rmb();
>  		apple_nvme_handle_cqe(q, iob, q->cq_head);
>  		apple_nvme_update_cq_head(q);
> +
> +		if (!anv->hw->has_lsq_nvmmu) {
> +			struct nvme_completion *cqe = &q->cqes[q->cq_head];
> +			u16 tag = nvme_tag_from_cid(READ_ONCE(cqe->command_id));
> +
> +			if (!WARN_ON_ONCE(tag >= BITS_PER_LONG))
> +				__set_bit(tag, &completed_tags);
> +		}
>  	}
>  
>  	if (found)
>  		writel(q->cq_head, q->cq_db);
>  
> +	if (!anv->hw->has_lsq_nvmmu && completed_tags) {
> +		unsigned long tag_bit;
> +
> +		for_each_set_bit(tag_bit, &completed_tags, BITS_PER_LONG)
> +			clear_bit(tag_bit, &anv->t8015_active_tags);
> +	}
> +
>  	return found;
>  }
>  
> @@ -790,6 +843,12 @@ static blk_status_t apple_nvme_queue_rq(struct blk_mq_hw_ctx *hctx,
>  	if (ret)
>  		return ret;
>  
> +	if (!anv->hw->has_lsq_nvmmu &&
> +	    !apple_nvme_reserve_tag_t8015(anv, cmnd)) {
> +		ret = BLK_STS_RESOURCE;
> +		goto out_free_cmd;
> +	}
> +
>  	if (blk_rq_nr_phys_segments(req)) {
>  		ret = apple_nvme_map_data(anv, req, cmnd);
>  		if (ret)
> @@ -806,6 +865,9 @@ static blk_status_t apple_nvme_queue_rq(struct blk_mq_hw_ctx *hctx,
>  	return BLK_STS_OK;
>  
>  out_free_cmd:
> +	if (!anv->hw->has_lsq_nvmmu)
> +		apple_nvme_release_tag_t8015(anv, cmnd->common.command_id);
> +
>  	nvme_cleanup_cmd(req);
>  	return ret;
>  }
> @@ -1165,6 +1227,9 @@ static void apple_nvme_reset_work(struct work_struct *work)
>  	if (ret)
>  		goto out;
>  
> +	if (!anv->hw->has_lsq_nvmmu)
> +		WRITE_ONCE(anv->t8015_active_tags, 0);
> +
>  	dev_dbg(anv->dev, "Starting admin queue");
>  	apple_nvme_init_queue(&anv->adminq);
>  	nvme_unquiesce_admin_queue(&anv->ctrl);
> 

Re: [PATCH 2/2] nvme-apple: Prevent tag collision across queues even if tag space is shared

[Nick Chan]

Nick Chan 於 2026/6/6 晚上9:25 寫道:
> From: Yuriy Havrylyuk <yhavry@gmail.com>
> 
> Apple NVMe controllers require tags of pending commands to not be shared
> across admin and IO queues. However, on Apple A11 without linear SQ, it is
> not possible for either queue to skip over some tags and must go from 0 to
> the configured maximum before wrapping around.
> 
> If a pending command tag is duplicated across queues, the firmware
> crashes with: "duplicate tag error for tag N", with N being the tag.
> 
> Instead of partitioning the tag space, which is not possible without
> linear SQ, prevent tag collisions by keeping track of which tags are
> currently in-flight across either queues, and return BLK_STS_RESOURCE to
> temporaily block command submission when a collision would have occurred.
> 
> Cc: stable@vger.kernel.org
> Fixes: 04d8ecf37b5e ("nvme: apple: Add Apple A11 support")
> Signed-off-by: Yuriy Havrylyuk <yhavry@gmail.com>
> Co-developed-by: Nick Chan <towinchenmi@gmail.com>
> Signed-off-by: Nick Chan <towinchenmi@gmail.com>
> ---
>  drivers/nvme/host/apple.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 65 insertions(+)

There are some issues with this version that make it not actually work, so a v2
will sent.

> 
> diff --git a/drivers/nvme/host/apple.c b/drivers/nvme/host/apple.c
> index c1115e27a0d6..6354edf27225 100644
> --- a/drivers/nvme/host/apple.c
> +++ b/drivers/nvme/host/apple.c
> @@ -203,6 +203,20 @@ struct apple_nvme {
>  
>  	int irq;
>  	spinlock_t lock;
> +
> +	/*
> +	 * Tags of pending commands must be unique across both Admin and IO
> +	 * queue. However, on T8015, unlike T8103, without linear submission
> +	 * queues, it is not possible for the either queue to skip some tags,
> +	 * and both queues must go from 0 to their respective configured
> +	 * maximum.
> +	 *
> +	 * Instead of reserving some tags for the admin queue, use a bitfield
> +	 * to keep track of pending commands on either queue, and temporaily
> +	 * block command submission by returning BLK_STS_RESOURCE until the
> +	 * tag is freed on the other queue.
> +	 */
> +	unsigned long t8015_active_tags;
>  };
>  
>  static_assert(sizeof(struct nvme_command) == 64);
> @@ -290,6 +304,28 @@ static void apple_nvmmu_inval(struct apple_nvme_queue *q, unsigned int tag)
>  				     "NVMMU TCB invalidation failed\n");
>  }
>  
> +static bool apple_nvme_reserve_tag_t8015(struct apple_nvme *anv,
> +					 struct nvme_command *cmd)
> +{
> +	u16 tag = nvme_tag_from_cid(cmd->common.command_id);
> +
> +	if (WARN_ON_ONCE(tag >= BITS_PER_LONG))
> +		return false;
> +
> +	return !test_and_set_bit(tag, &anv->t8015_active_tags);
> +}
> +
> +static void apple_nvme_release_tag_t8015(struct apple_nvme *anv,
> +					 __u16 command_id)
> +{
> +	u16 tag = nvme_tag_from_cid(command_id);
> +
> +	if (WARN_ON_ONCE(tag >= BITS_PER_LONG))
> +		return;
> +
> +	clear_bit(tag, &anv->t8015_active_tags);
> +}
> +
>  static void apple_nvme_submit_cmd_t8015(struct apple_nvme_queue *q,
>  				  struct nvme_command *cmd)
>  {
> @@ -652,6 +688,8 @@ static inline void apple_nvme_update_cq_head(struct apple_nvme_queue *q)
>  static bool apple_nvme_poll_cq(struct apple_nvme_queue *q,
>  			       struct io_comp_batch *iob)
>  {
> +	struct apple_nvme *anv = queue_to_apple_nvme(q);
> +	unsigned long completed_tags = 0;
>  	bool found = false;
>  
>  	while (apple_nvme_cqe_pending(q)) {
> @@ -664,11 +702,26 @@ static bool apple_nvme_poll_cq(struct apple_nvme_queue *q,
>  		dma_rmb();
>  		apple_nvme_handle_cqe(q, iob, q->cq_head);
>  		apple_nvme_update_cq_head(q);
> +
> +		if (!anv->hw->has_lsq_nvmmu) {
> +			struct nvme_completion *cqe = &q->cqes[q->cq_head];
> +			u16 tag = nvme_tag_from_cid(READ_ONCE(cqe->command_id));

Reading command ID here is too late since cq head has already been updated.

> +
> +			if (!WARN_ON_ONCE(tag >= BITS_PER_LONG))
> +				__set_bit(tag, &completed_tags);
> +		}
>  	}
>  
>  	if (found)
>  		writel(q->cq_head, q->cq_db);
>  
> +	if (!anv->hw->has_lsq_nvmmu && completed_tags) {
> +		unsigned long tag_bit;
> +
> +		for_each_set_bit(tag_bit, &completed_tags, BITS_PER_LONG)
> +			clear_bit(tag_bit, &anv->t8015_active_tags);
> +	}
> +
>  	return found;
>  }
>  
> @@ -790,6 +843,12 @@ static blk_status_t apple_nvme_queue_rq(struct blk_mq_hw_ctx *hctx,
>  	if (ret)
>  		return ret;
>  
> +	if (!anv->hw->has_lsq_nvmmu &&
> +	    !apple_nvme_reserve_tag_t8015(anv, cmnd)) {
> +		ret = BLK_STS_RESOURCE;
> +		goto out_free_cmd;

Note goto out_free_cmd here.

> +	}
> +
>  	if (blk_rq_nr_phys_segments(req)) {
>  		ret = apple_nvme_map_data(anv, req, cmnd);
>  		if (ret)
> @@ -806,6 +865,9 @@ static blk_status_t apple_nvme_queue_rq(struct blk_mq_hw_ctx *hctx,
>  	return BLK_STS_OK;
>  
>  out_free_cmd:
> +	if (!anv->hw->has_lsq_nvmmu)
> +		apple_nvme_release_tag_t8015(anv, cmnd->common.command_id);

Combined with above this makes any attempted use of a in-use tag release that
tag, making the workaround ineffective. (and allows nvme to still "work" if
the tester is (un)lucky).

> +
>  	nvme_cleanup_cmd(req);
>  	return ret;
>  }
> @@ -1165,6 +1227,9 @@ static void apple_nvme_reset_work(struct work_struct *work)
>  	if (ret)
>  		goto out;
>  
> +	if (!anv->hw->has_lsq_nvmmu)
> +		WRITE_ONCE(anv->t8015_active_tags, 0);
> +
>  	dev_dbg(anv->dev, "Starting admin queue");
>  	apple_nvme_init_queue(&anv->adminq);
>  	nvme_unquiesce_admin_queue(&anv->ctrl);
> 

Best regards,
Nick Chan

Re: [PATCH 2/2] nvme-apple: Prevent tag collision across queues even if tag space is shared

[Nick Chan]

David Laight 於 2026/6/6 晚上10:29 寫道:
> On Sat, 06 Jun 2026 21:25:26 +0800
> Nick Chan <towinchenmi@gmail.com> wrote:
> 
>> From: Yuriy Havrylyuk <yhavry@gmail.com>
>>
>> Apple NVMe controllers require tags of pending commands to not be shared
>> across admin and IO queues. However, on Apple A11 without linear SQ, it is
>> not possible for either queue to skip over some tags and must go from 0 to
>> the configured maximum before wrapping around.
>>
>> If a pending command tag is duplicated across queues, the firmware
>> crashes with: "duplicate tag error for tag N", with N being the tag.
>>
>> Instead of partitioning the tag space, which is not possible without
>> linear SQ, prevent tag collisions by keeping track of which tags are
>> currently in-flight across either queues, and return BLK_STS_RESOURCE to
>> temporaily block command submission when a collision would have occurred.
> 
> I look at using the atomic64_xxx() functions rather than the bitmask ones.
> The for_each_bit_set() loop is then an atmomic64_andnot() call.

That does in fact simplify the loop code. However, using the atomic function
complicates the apple_nvme_reserve_tag_t8015() and
apple_nvme_release_tag_t8015() since those functions deal with set/clear a
a bit in terms of an integer (the tag).

Especially in the apple_nvme_reserve_tag_t8015() function the function body
is then

	u64 tag_bit = BIT(nvme_tag_from_cid(cmd->common.command_id));

	return !(atomic64_fetch_or(tag_bit, &anv->t8015_active_tags) & tag_bit);

The function would need to explictly convert the tag to a bit, and then
explictly extract the bit value after performing the atomic operation,
both which of could have been done by test_and_set_bit().

So I do not see any overall benefit for using atomic_xxx() functions.

Best Regards,
Nick Chan

> 
> -- David
> 
> 
>>
>> Cc: stable@vger.kernel.org
>> Fixes: 04d8ecf37b5e ("nvme: apple: Add Apple A11 support")
>> Signed-off-by: Yuriy Havrylyuk <yhavry@gmail.com>
>> Co-developed-by: Nick Chan <towinchenmi@gmail.com>
>> Signed-off-by: Nick Chan <towinchenmi@gmail.com>
>> ---
>>  drivers/nvme/host/apple.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 65 insertions(+)
>>
>> diff --git a/drivers/nvme/host/apple.c b/drivers/nvme/host/apple.c
>> index c1115e27a0d6..6354edf27225 100644
>> --- a/drivers/nvme/host/apple.c
>> +++ b/drivers/nvme/host/apple.c
>> @@ -203,6 +203,20 @@ struct apple_nvme {
>>  
>>  	int irq;
>>  	spinlock_t lock;
>> +
>> +	/*
>> +	 * Tags of pending commands must be unique across both Admin and IO
>> +	 * queue. However, on T8015, unlike T8103, without linear submission
>> +	 * queues, it is not possible for the either queue to skip some tags,
>> +	 * and both queues must go from 0 to their respective configured
>> +	 * maximum.
>> +	 *
>> +	 * Instead of reserving some tags for the admin queue, use a bitfield
>> +	 * to keep track of pending commands on either queue, and temporaily
>> +	 * block command submission by returning BLK_STS_RESOURCE until the
>> +	 * tag is freed on the other queue.
>> +	 */
>> +	unsigned long t8015_active_tags;
>>  };
>>  
>>  static_assert(sizeof(struct nvme_command) == 64);
>> @@ -290,6 +304,28 @@ static void apple_nvmmu_inval(struct apple_nvme_queue *q, unsigned int tag)
>>  				     "NVMMU TCB invalidation failed\n");
>>  }
>>  
>> +static bool apple_nvme_reserve_tag_t8015(struct apple_nvme *anv,
>> +					 struct nvme_command *cmd)
>> +{
>> +	u16 tag = nvme_tag_from_cid(cmd->common.command_id);
>> +
>> +	if (WARN_ON_ONCE(tag >= BITS_PER_LONG))
>> +		return false;
>> +
>> +	return !test_and_set_bit(tag, &anv->t8015_active_tags);
>> +}
>> +
>> +static void apple_nvme_release_tag_t8015(struct apple_nvme *anv,
>> +					 __u16 command_id)
>> +{
>> +	u16 tag = nvme_tag_from_cid(command_id);
>> +
>> +	if (WARN_ON_ONCE(tag >= BITS_PER_LONG))
>> +		return;
>> +
>> +	clear_bit(tag, &anv->t8015_active_tags);
>> +}
>> +
>>  static void apple_nvme_submit_cmd_t8015(struct apple_nvme_queue *q,
>>  				  struct nvme_command *cmd)
>>  {
>> @@ -652,6 +688,8 @@ static inline void apple_nvme_update_cq_head(struct apple_nvme_queue *q)
>>  static bool apple_nvme_poll_cq(struct apple_nvme_queue *q,
>>  			       struct io_comp_batch *iob)
>>  {
>> +	struct apple_nvme *anv = queue_to_apple_nvme(q);
>> +	unsigned long completed_tags = 0;
>>  	bool found = false;
>>  
>>  	while (apple_nvme_cqe_pending(q)) {
>> @@ -664,11 +702,26 @@ static bool apple_nvme_poll_cq(struct apple_nvme_queue *q,
>>  		dma_rmb();
>>  		apple_nvme_handle_cqe(q, iob, q->cq_head);
>>  		apple_nvme_update_cq_head(q);
>> +
>> +		if (!anv->hw->has_lsq_nvmmu) {
>> +			struct nvme_completion *cqe = &q->cqes[q->cq_head];
>> +			u16 tag = nvme_tag_from_cid(READ_ONCE(cqe->command_id));
>> +
>> +			if (!WARN_ON_ONCE(tag >= BITS_PER_LONG))
>> +				__set_bit(tag, &completed_tags);
>> +		}
>>  	}
>>  
>>  	if (found)
>>  		writel(q->cq_head, q->cq_db);
>>  
>> +	if (!anv->hw->has_lsq_nvmmu && completed_tags) {
>> +		unsigned long tag_bit;
>> +
>> +		for_each_set_bit(tag_bit, &completed_tags, BITS_PER_LONG)
>> +			clear_bit(tag_bit, &anv->t8015_active_tags);
>> +	}
>> +
>>  	return found;
>>  }
>>  
>> @@ -790,6 +843,12 @@ static blk_status_t apple_nvme_queue_rq(struct blk_mq_hw_ctx *hctx,
>>  	if (ret)
>>  		return ret;
>>  
>> +	if (!anv->hw->has_lsq_nvmmu &&
>> +	    !apple_nvme_reserve_tag_t8015(anv, cmnd)) {
>> +		ret = BLK_STS_RESOURCE;
>> +		goto out_free_cmd;
>> +	}
>> +
>>  	if (blk_rq_nr_phys_segments(req)) {
>>  		ret = apple_nvme_map_data(anv, req, cmnd);
>>  		if (ret)
>> @@ -806,6 +865,9 @@ static blk_status_t apple_nvme_queue_rq(struct blk_mq_hw_ctx *hctx,
>>  	return BLK_STS_OK;
>>  
>>  out_free_cmd:
>> +	if (!anv->hw->has_lsq_nvmmu)
>> +		apple_nvme_release_tag_t8015(anv, cmnd->common.command_id);
>> +
>>  	nvme_cleanup_cmd(req);
>>  	return ret;
>>  }
>> @@ -1165,6 +1227,9 @@ static void apple_nvme_reset_work(struct work_struct *work)
>>  	if (ret)
>>  		goto out;
>>  
>> +	if (!anv->hw->has_lsq_nvmmu)
>> +		WRITE_ONCE(anv->t8015_active_tags, 0);
>> +
>>  	dev_dbg(anv->dev, "Starting admin queue");
>>  	apple_nvme_init_queue(&anv->adminq);
>>  	nvme_unquiesce_admin_queue(&anv->ctrl);
>>
> 

Re: [PATCH 2/2] nvme-apple: Prevent tag collision across queues even if tag space is shared

[Sven Peter]

On 06.06.26 15:25, Nick Chan wrote:
> From: Yuriy Havrylyuk <yhavry@gmail.com>
> 
> Apple NVMe controllers require tags of pending commands to not be shared
> across admin and IO queues. However, on Apple A11 without linear SQ, it is
> not possible for either queue to skip over some tags and must go from 0 to
> the configured maximum before wrapping around.
> 
> If a pending command tag is duplicated across queues, the firmware
> crashes with: "duplicate tag error for tag N", with N being the tag.
> 
> Instead of partitioning the tag space, which is not possible without
> linear SQ, 

Isn't that just what the pci.c driver does with NVME_QUIRK_SHARED_TAGS 
for the T2 macs or what we do in this driver with
	if (anv->hw->has_lsq_nvmmu)
		anv->tagset.reserved_tags = APPLE_NVME_AQ_DEPTH;
?


Sven

Re: [PATCH 2/2] nvme-apple: Prevent tag collision across queues even if tag space is shared

[Nick Chan]

Sven Peter 於 2026/6/7 凌晨12:12 寫道:
> On 06.06.26 15:25, Nick Chan wrote:
>> From: Yuriy Havrylyuk <yhavry@gmail.com>
>>
>> Apple NVMe controllers require tags of pending commands to not be shared
>> across admin and IO queues. However, on Apple A11 without linear SQ, it is
>> not possible for either queue to skip over some tags and must go from 0 to
>> the configured maximum before wrapping around.
>>
>> If a pending command tag is duplicated across queues, the firmware
>> crashes with: "duplicate tag error for tag N", with N being the tag.
>>
>> Instead of partitioning the tag space, which is not possible without
>> linear SQ, 
> 
> Isn't that just what the pci.c driver does with NVME_QUIRK_SHARED_TAGS 
> for the T2 macs or what we do in this driver with
> 	if (anv->hw->has_lsq_nvmmu)
> 		anv->tagset.reserved_tags = APPLE_NVME_AQ_DEPTH;
> ?

After adjusting the apple_nvme_submit_cmd_t8015() function to account for
the admin queue depth, it seems that the existing workaround for M1 of
reserving two tags for the admin queue works on A11 as well.

Will post a much simplified v2.

Best regards,
Nick Chan

> 
> 
> Sven
>