[PATCH] scsi: ufs: Add support for the aggregated read query opcode

[PATCH] scsi: ufs: Add support for the aggregated read query opcode

[Hyeoncheol Jeong]

UFS 5.0 / JEDEC 220H introduces the AGGREGATED READ query opcode (0x9),
which retrieves an aggregated data packet from the device in a single
query request. The packet may bundle multiple Descriptors, Attributes
and Flags, each organized as a group with a group header, and is
returned in the Data Segment of the QUERY RESPONSE UPIU.

Because an aggregated data packet can be considerably larger than a
single descriptor (up to 4 KiB rather than the 255-byte descriptor
limit), the response UPIU buffer must be enlarged to hold it. Introduce
ALIGNED_RSP_UPIU_SIZE (4096) for the response_upiu[] area of the UTP
command descriptor and program the response UPIU length in the UTRD
accordingly.

Teach the BSG raw-UPIU path and the device management command path to
recognize the new opcode so that user space can issue aggregated reads
and receive the returned data segment. Descriptor sizing now honors
QUERY_AGGREGATED_MAX_SIZE for aggregated reads.

Signed-off-by: Hyeoncheol Jeong <hyenc.jeong@samsung.com>
---
drivers/ufs/core/ufs_bsg.c 16 +++++++++++-----
drivers/ufs/core/ufshcd.c    7 ++++---
include/ufs/ufs.h            2 ++
include/ufs/ufshci.h        8 +++++++-
4 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/drivers/ufs/core/ufs_bsg.c b/drivers/ufs/core/ufs_bsg.c
index 58b506eac6dc..176fedd496af 100644
--- a/drivers/ufs/core/ufs_bsg.c
+++ b/drivers/ufs/core/ufs_bsg.c
@@ -14,14 +14,18 @@
#include "ufshcd-priv.h"

static int ufs_bsg_get_query_desc_size(struct ufs_hba *hba, int *desc_len,
-                                      struct utp_upiu_query *qr)
+                                      struct utp_upiu_query *qr,
+                                      enum query_opcode desc_op)
{
         int desc_size = be16_to_cpu(qr->length);

         if (desc_size <= 0)
                 return -EINVAL;

-        *desc_len = min_t(int, QUERY_DESC_MAX_SIZE, desc_size);
+        if (desc_op == UPIU_QUERY_OPCODE_AGGREGATED_READ)
+                *desc_len = min_t(int, QUERY_AGGREGATED_MAX_SIZE, desc_size);
+        else
+                *desc_len = min_t(int, QUERY_DESC_MAX_SIZE, desc_size);

         return 0;
}
@@ -35,11 +39,12 @@ static int ufs_bsg_alloc_desc_buffer(struct ufs_hba *hba, struct bsg_job *job,
         u8 *descp;

         if (desc_op != UPIU_QUERY_OPCODE_WRITE_DESC &&
-            desc_op != UPIU_QUERY_OPCODE_READ_DESC)
+            desc_op != UPIU_QUERY_OPCODE_READ_DESC &&
+            desc_op != UPIU_QUERY_OPCODE_AGGREGATED_READ)
                 goto out;

         qr = &bsg_request->upiu_req.qr;
-        if (ufs_bsg_get_query_desc_size(hba, desc_len, qr)) {
+        if (ufs_bsg_get_query_desc_size(hba, desc_len, qr, desc_op)) {
                 dev_err(hba->dev, "Illegal desc size\n");
                 return -EINVAL;
         }
@@ -161,7 +166,8 @@ static int ufs_bsg_request(struct bsg_job *job)
                                               buff, &desc_len, desc_op);
                 if (ret)
                         dev_err(hba->dev, "exe raw upiu: error code %d\n", ret);
-                else if (desc_op == UPIU_QUERY_OPCODE_READ_DESC && desc_len) {
+                else if ((desc_op == UPIU_QUERY_OPCODE_READ_DESC 
+                          desc_op == UPIU_QUERY_OPCODE_AGGREGATED_READ) && desc_len) {
                         bsg_reply->reply_payload_rcv_len =
                                 sg_copy_from_buffer(job->request_payload.sg_list,
                                                     job->request_payload.sg_cnt,
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index d3044a3089b5..a366224462ca 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -4108,14 +4108,14 @@ static void ufshcd_host_memory_configure(struct ufs_hba *hba)
                         utrdlp[i].prd_table_offset =
                                 cpu_to_le16(prdt_offset);
                         utrdlp[i].response_upiu_length =
-                                cpu_to_le16(ALIGNED_UPIU_SIZE);
+                                cpu_to_le16(ALIGNED_RSP_UPIU_SIZE);
                 } else {
                         utrdlp[i].response_upiu_offset =
                                 cpu_to_le16(response_offset >> 2);
                         utrdlp[i].prd_table_offset =
                                 cpu_to_le16(prdt_offset >> 2);
                         utrdlp[i].response_upiu_length =
-                                cpu_to_le16(ALIGNED_UPIU_SIZE >> 2);
+                                cpu_to_le16(ALIGNED_RSP_UPIU_SIZE >> 2);
                 }
         }
}
@@ -7638,7 +7638,8 @@ static int ufshcd_issue_devman_upiu_cmd(struct ufs_hba *hba,

         /* just copy the upiu response as it is */
         memcpy(rsp_upiu, lrbp->ucd_rsp_ptr, sizeof(*rsp_upiu));
-        if (desc_buff && desc_op == UPIU_QUERY_OPCODE_READ_DESC) {
+        if (desc_buff && (desc_op == UPIU_QUERY_OPCODE_READ_DESC 
+                          desc_op == UPIU_QUERY_OPCODE_AGGREGATED_READ)) {
                 u8 *descp = (u8 *)lrbp->ucd_rsp_ptr + sizeof(*rsp_upiu);
                 u16 resp_len = be16_to_cpu(lrbp->ucd_rsp_ptr->header
                                           .data_segment_length);
diff --git a/include/ufs/ufs.h b/include/ufs/ufs.h
index 0d48e137d66d..01bf9a8c8bb3 100644
--- a/include/ufs/ufs.h
+++ b/include/ufs/ufs.h
@@ -25,6 +25,7 @@ static_assert(sizeof(struct utp_upiu_query) == 20);

#define GENERAL_UPIU_REQUEST_SIZE (sizeof(struct utp_upiu_req))
#define QUERY_DESC_MAX_SIZE      255
+#define QUERY_AGGREGATED_MAX_SIZE 4096
#define QUERY_DESC_MIN_SIZE      2
#define QUERY_DESC_HDR_SIZE      2
#define QUERY_OSF_SIZE            (GENERAL_UPIU_REQUEST_SIZE - \
@@ -464,6 +465,7 @@ enum query_opcode {
         UPIU_QUERY_OPCODE_SET_FLAG        = 0x6,
         UPIU_QUERY_OPCODE_CLEAR_FLAG        = 0x7,
         UPIU_QUERY_OPCODE_TOGGLE_FLAG        = 0x8,
+        UPIU_QUERY_OPCODE_AGGREGATED_READ        = 0x9,
};

/* bRefClkFreq attribute values */
diff --git a/include/ufs/ufshci.h b/include/ufs/ufshci.h
index 9f0fdd850e54..682104fb7390 100644
--- a/include/ufs/ufshci.h
+++ b/include/ufs/ufshci.h
@@ -18,6 +18,12 @@ enum {
         TASK_REQ_UPIU_SIZE_DWORDS        = 8,
         TASK_RSP_UPIU_SIZE_DWORDS        = 8,
         ALIGNED_UPIU_SIZE                = 512,
+        /*
+         * The aggregated read opcode can return an aggregated data packet of
+         * up to QUERY_AGGREGATED_MAX_SIZE bytes, so the response UPIU buffer
+         * needs to be large enough to hold it.
+         */
+        ALIGNED_RSP_UPIU_SIZE                = 4096,
};

/* UFSHCI Registers */
@@ -497,7 +503,7 @@ struct ufshcd_sg_entry {
  */
struct utp_transfer_cmd_desc {
         u8 command_upiu[ALIGNED_UPIU_SIZE];
-        u8 response_upiu[ALIGNED_UPIU_SIZE];
+        u8 response_upiu[ALIGNED_RSP_UPIU_SIZE];
         u8 prd_table[];
};

-- 
2.25.1

Re: [PATCH] scsi: ufs: Add support for the aggregated read query opcode

[Bart Van Assche]

On 7/9/26 10:45 PM, Hyeoncheol Jeong wrote:
> struct utp_transfer_cmd_desc {
>           u8 command_upiu[ALIGNED_UPIU_SIZE];
> -        u8 response_upiu[ALIGNED_UPIU_SIZE];
> +        u8 response_upiu[ALIGNED_RSP_UPIU_SIZE];
>           u8 prd_table[];
> };

This change increases the size of every CQ entry and also of every LRB
entry by about 4 KiB. This is not acceptable.

Bart.

Re: [PATCH] scsi: ufs: Add support for the aggregated read query opcode

[Hyeoncheol Jeong]

Hi Bart,

On 7/10/26 02:46 PM, Bart Van Assche wrote:
> On 7/9/26 10:45 PM, Hyeoncheol Jeong wrote:
> > struct utp_transfer_cmd_desc {
> >           u8 command_upiu[ALIGNED_UPIU_SIZE];
> > -        u8 response_upiu[ALIGNED_UPIU_SIZE];
> > +        u8 response_upiu[ALIGNED_RSP_UPIU_SIZE];
> >           u8 prd_table[];
> > };
>
> This change increases the size of every CQ entry and also of every LRB
> entry by about 4 KiB. This is not acceptable.

Thanks for the review. You're right — growing the shared
utp_transfer_cmd_desc enlarges the response area for every tag,
which is wasteful. The aggregated read only uses the reserved
(device management) tag, so only that tag needs the big buffer.

Would it be okay to keep utp_transfer_cmd_desc as is and give
just the reserved tag a dedicated 4 KiB response descriptor?
Regular tags and normal I/O would stay unchanged.

Let me know if you'd prefer another approach.

Thanks.

Re: [PATCH] scsi: ufs: Add support for the aggregated read query opcode

[Bart Van Assche]

On 7/12/26 7:52 PM, Hyeoncheol Jeong wrote:
> Would it be okay to keep utp_transfer_cmd_desc as is and give
> just the reserved tag a dedicated 4 KiB response descriptor?
> Regular tags and normal I/O would stay unchanged.

That sounds better to me but how to implement the above proposal
without slowing down the hot path?

Thanks,

Bart.

Re: [PATCH] scsi: ufs: Add support for the aggregated read query opcode

[Hyeoncheol Jeong]

On 7/13/26 09:55 PM, Bart Van Assche wrote:
> On 7/12/26 7:52 PM, Hyeoncheol Jeong wrote:
> > Would it be okay to keep utp_transfer_cmd_desc as is and give
> > just the reserved tag a dedicated 4 KiB response descriptor?
> > Regular tags and normal I/O would stay unchanged.
>
> That sounds better to me but how to implement the above proposal
> without slowing down the hot path?

Hi Bart,

Here's what I'm thinking. Keep utp_transfer_cmd_desc as is (512 B) and
add a dedicated 4 KiB one just for the reserved tag:

struct utp_devman_cmd_desc {
	u8 command_upiu[ALIGNED_UPIU_SIZE];
	u8 response_upiu[ALIGNED_DEVMAN_RSP_SIZE];      /* 4 KiB */
	u8 prd_table[];
};
Allocated as a single instance, this keeps the extra cost to +(4 KiB - 512 B)
per host without growing any per-tag structure.

This probably touches two spots on the hot path, though hopefully only
slightly. First, ufshcd_init_lrb() would gain a reserved-tag check to
pick the descriptor:

if (unlikely(blk_mq_is_reserved_rq(scsi_cmd_to_rq(cmd))))
	p = hba->devman_ucd_base_addr;
else
	p = ... + (i - UFSHCD_NUM_RESERVED) * ufshcd_get_ucd_size(hba);
Since the reserved tag is only ever used for device management, this
branch is always false for normal I/O and should be well predicted.

The other is the pre-4.1 MCQ tag recovery, which would gain a check
before the existing division:

if (addr == hba->devman_ucd_dma_addr)
    return 0;       /* reserved tag */
It just gets one extra compare, and on UFSHCI 4.1+ the CQE carries
the tag directly, so that path isn't hit at all.

If the direction sounds reasonable, I'll prepare it as v2.

Thanks.

[PATCH v2] scsi: ufs: Add support for the aggregated read query opcode

[Hyeoncheol Jeong]

UFS 5.0 / JEDEC 220H introduces the AGGREGATED READ query opcode (0x9),
which retrieves an aggregated data packet in a single query request. The
packet may bundle multiple Descriptors, Attributes and Flags as
group-headed groups, returned in the Data Segment of the QUERY RESPONSE
UPIU.

Such a packet can be far larger than a single descriptor (up to 4 KiB
vs the 255-byte descriptor limit), so its response UPIU buffer must be
enlarged. Enlarging the shared utp_transfer_cmd_desc would waste ~4 KiB
per tag, so add a dedicated utp_devman_cmd_desc with a 4 KiB response
area (ALIGNED_DEVMAN_RSP_SIZE), allocated once for the reserved (device
management) tag that aggregated read uses. Regular tags keep the
512-byte descriptor in a pool of (nutrs - UFSHCD_NUM_RESERVED) entries,
leaving normal I/O unchanged.

ufshcd_init_lrb() and ufshcd_host_memory_configure() pick the devman
descriptor for the reserved tag and index the pool at (tag -
UFSHCD_NUM_RESERVED) otherwise. The pre-4.1 MCQ tag recovery adds one
compare against the devman UCD address (UFSHCI 4.1+ carries the tag in
the CQE), and the BSG raw-UPIU and device management paths learn the new
opcode, sizing descriptors by QUERY_AGGREGATED_MAX_SIZE.

Signed-off-by: Hyeoncheol Jeong <hyenc.jeong@samsung.com>
---
 drivers/ufs/core/ufs-mcq.c | 14 ++++--
 drivers/ufs/core/ufs_bsg.c | 16 +++++--
 drivers/ufs/core/ufshcd.c  | 95 ++++++++++++++++++++++++++++----------
 include/ufs/ufs.h          |  2 +
 include/ufs/ufshcd.h       | 13 ++++++
 include/ufs/ufshci.h       | 11 +++++
 6 files changed, 118 insertions(+), 33 deletions(-)

diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
index 13b60a2d06db..f35c6c18578d 100644
--- a/drivers/ufs/core/ufs-mcq.c
+++ b/drivers/ufs/core/ufs-mcq.c
@@ -296,14 +296,20 @@ static int ufshcd_mcq_get_tag(struct ufs_hba *hba, struct cq_entry *cqe)
 	if (hba->ufs_version >= ufshci_version(4, 1))
 		return cqe->task_tag;
 
-	/* sizeof(struct utp_transfer_cmd_desc) must be a multiple of 128 */
+	/* Both UCD types must be a multiple of 128 */
 	BUILD_BUG_ON(sizeof(struct utp_transfer_cmd_desc) & GENMASK(6, 0));
+	BUILD_BUG_ON(sizeof(struct utp_devman_cmd_desc) & GENMASK(6, 0));
 
 	/* Bits 63:7 UCD base address, 6:5 are reserved, 4:0 is SQ ID */
-	addr = (le64_to_cpu(cqe->command_desc_base_addr) & CQE_UCD_BA) -
-		hba->ucdl_dma_addr;
+	addr = le64_to_cpu(cqe->command_desc_base_addr) & CQE_UCD_BA;
 
-	return div_u64(addr, ufshcd_get_ucd_size(hba));
+	/* The devman UCD is outside the pool; return its reserved tag. */
+	if (unlikely(addr == hba->devman_ucd_dma_addr))
+		return hba->dev_cmd.tag;
+
+	/* Pool entries follow the reserved tags. */
+	return div_u64(addr - hba->ucdl_dma_addr, ufshcd_get_ucd_size(hba)) +
+		UFSHCD_NUM_RESERVED;
 }
 
 static void ufshcd_mcq_process_cqe(struct ufs_hba *hba,
diff --git a/drivers/ufs/core/ufs_bsg.c b/drivers/ufs/core/ufs_bsg.c
index 58b506eac6dc..176fedd496af 100644
--- a/drivers/ufs/core/ufs_bsg.c
+++ b/drivers/ufs/core/ufs_bsg.c
@@ -14,14 +14,18 @@
 #include "ufshcd-priv.h"
 
 static int ufs_bsg_get_query_desc_size(struct ufs_hba *hba, int *desc_len,
-				       struct utp_upiu_query *qr)
+				       struct utp_upiu_query *qr,
+				       enum query_opcode desc_op)
 {
 	int desc_size = be16_to_cpu(qr->length);
 
 	if (desc_size <= 0)
 		return -EINVAL;
 
-	*desc_len = min_t(int, QUERY_DESC_MAX_SIZE, desc_size);
+	if (desc_op == UPIU_QUERY_OPCODE_AGGREGATED_READ)
+		*desc_len = min_t(int, QUERY_AGGREGATED_MAX_SIZE, desc_size);
+	else
+		*desc_len = min_t(int, QUERY_DESC_MAX_SIZE, desc_size);
 
 	return 0;
 }
@@ -35,11 +39,12 @@ static int ufs_bsg_alloc_desc_buffer(struct ufs_hba *hba, struct bsg_job *job,
 	u8 *descp;
 
 	if (desc_op != UPIU_QUERY_OPCODE_WRITE_DESC &&
-	    desc_op != UPIU_QUERY_OPCODE_READ_DESC)
+	    desc_op != UPIU_QUERY_OPCODE_READ_DESC &&
+	    desc_op != UPIU_QUERY_OPCODE_AGGREGATED_READ)
 		goto out;
 
 	qr = &bsg_request->upiu_req.qr;
-	if (ufs_bsg_get_query_desc_size(hba, desc_len, qr)) {
+	if (ufs_bsg_get_query_desc_size(hba, desc_len, qr, desc_op)) {
 		dev_err(hba->dev, "Illegal desc size\n");
 		return -EINVAL;
 	}
@@ -161,7 +166,8 @@ static int ufs_bsg_request(struct bsg_job *job)
 					       buff, &desc_len, desc_op);
 		if (ret)
 			dev_err(hba->dev, "exe raw upiu: error code %d\n", ret);
-		else if (desc_op == UPIU_QUERY_OPCODE_READ_DESC && desc_len) {
+		else if ((desc_op == UPIU_QUERY_OPCODE_READ_DESC ||
+			  desc_op == UPIU_QUERY_OPCODE_AGGREGATED_READ) && desc_len) {
 			bsg_reply->reply_payload_rcv_len =
 				sg_copy_from_buffer(job->request_payload.sg_list,
 						    job->request_payload.sg_cnt,
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index d3044a3089b5..a6a9b86597b0 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -2944,23 +2944,41 @@ static void ufshcd_comp_scsi_upiu(struct ufs_hba *hba, struct scsi_cmnd *cmd)
 static void ufshcd_init_lrb(struct ufs_hba *hba, struct scsi_cmnd *cmd)
 {
 	const int i = scsi_cmd_to_rq(cmd)->tag;
-	struct utp_transfer_cmd_desc *cmd_descp =
-		(void *)hba->ucdl_base_addr + i * ufshcd_get_ucd_size(hba);
 	struct utp_transfer_req_desc *utrdlp = hba->utrdl_base_addr;
-	dma_addr_t cmd_desc_element_addr =
-		hba->ucdl_dma_addr + i * ufshcd_get_ucd_size(hba);
 	u16 response_offset = le16_to_cpu(utrdlp[i].response_upiu_offset);
 	u16 prdt_offset = le16_to_cpu(utrdlp[i].prd_table_offset);
 	struct ufshcd_lrb *lrb = scsi_cmd_priv(cmd);
+	u8 *command_upiu, *response_upiu, *prd_table;
+	dma_addr_t cmd_desc_element_addr;
+
+	/* The reserved tag uses a dedicated UCD outside the pool. */
+	if (unlikely(blk_mq_is_reserved_rq(scsi_cmd_to_rq(cmd)))) {
+		struct utp_devman_cmd_desc *cmd_descp = hba->devman_ucd_base_addr;
+
+		cmd_desc_element_addr = hba->devman_ucd_dma_addr;
+		command_upiu = cmd_descp->command_upiu;
+		response_upiu = cmd_descp->response_upiu;
+		prd_table = cmd_descp->prd_table;
+	} else {
+		int slot = i - UFSHCD_NUM_RESERVED;
+		struct utp_transfer_cmd_desc *cmd_descp =
+			(void *)hba->ucdl_base_addr + slot * ufshcd_get_ucd_size(hba);
+
+		cmd_desc_element_addr =
+			hba->ucdl_dma_addr + slot * ufshcd_get_ucd_size(hba);
+		command_upiu = cmd_descp->command_upiu;
+		response_upiu = cmd_descp->response_upiu;
+		prd_table = cmd_descp->prd_table;
+	}
 
 	lrb->utr_descriptor_ptr = utrdlp + i;
 	lrb->utrd_dma_addr =
 		hba->utrdl_dma_addr + i * sizeof(struct utp_transfer_req_desc);
-	lrb->ucd_req_ptr = (struct utp_upiu_req *)cmd_descp->command_upiu;
+	lrb->ucd_req_ptr = (struct utp_upiu_req *)command_upiu;
 	lrb->ucd_req_dma_addr = cmd_desc_element_addr;
-	lrb->ucd_rsp_ptr = (struct utp_upiu_rsp *)cmd_descp->response_upiu;
+	lrb->ucd_rsp_ptr = (struct utp_upiu_rsp *)response_upiu;
 	lrb->ucd_rsp_dma_addr = cmd_desc_element_addr + response_offset;
-	lrb->ucd_prdt_ptr = (struct ufshcd_sg_entry *)cmd_descp->prd_table;
+	lrb->ucd_prdt_ptr = (struct ufshcd_sg_entry *)prd_table;
 	lrb->ucd_prdt_dma_addr = cmd_desc_element_addr + prdt_offset;
 }
 
@@ -3157,6 +3175,7 @@ static void ufshcd_setup_dev_cmd(struct ufs_hba *hba, struct scsi_cmnd *cmd,
 	__ufshcd_setup_cmd(hba, cmd, lun, tag);
 	lrbp->intr_cmd = true; /* No interrupt aggregation */
 	hba->dev_cmd.type = cmd_type;
+	hba->dev_cmd.tag = tag;
 }
 
 /*
@@ -3998,8 +4017,8 @@ static int ufshcd_memory_alloc(struct ufs_hba *hba)
 {
 	size_t utmrdl_size, utrdl_size, ucdl_size;
 
-	/* Allocate memory for UTP command descriptors */
-	ucdl_size = ufshcd_get_ucd_size(hba) * hba->nutrs;
+	/* The reserved tag uses the dedicated UCD below, not this pool. */
+	ucdl_size = ufshcd_get_ucd_size(hba) * (hba->nutrs - UFSHCD_NUM_RESERVED);
 	hba->ucdl_base_addr = dmam_alloc_coherent(hba->dev,
 						  ucdl_size,
 						  &hba->ucdl_dma_addr,
@@ -4015,6 +4034,21 @@ static int ufshcd_memory_alloc(struct ufs_hba *hba)
 		goto out;
 	}
 
+	/* Dedicated UCD for the reserved tag; allocate once (survives MCQ re-init). */
+	if (!hba->devman_ucd_base_addr) {
+		hba->devman_ucd_base_addr =
+			dmam_alloc_coherent(hba->dev,
+					    ufshcd_get_devman_ucd_size(hba),
+					    &hba->devman_ucd_dma_addr,
+					    GFP_KERNEL);
+		if (!hba->devman_ucd_base_addr ||
+		    WARN_ON(hba->devman_ucd_dma_addr & (128 - 1))) {
+			dev_err(hba->dev,
+				"Devman Command Descriptor Memory allocation failed\n");
+			goto out;
+		}
+	}
+
 	/*
 	 * Allocate memory for UTP Transfer descriptors
 	 * UFSHCI requires 1KB alignment of UTRD
@@ -4081,23 +4115,38 @@ static void ufshcd_host_memory_configure(struct ufs_hba *hba)
 	dma_addr_t cmd_desc_element_addr;
 	u16 response_offset;
 	u16 prdt_offset;
+	u16 response_len;
 	int cmd_desc_size;
 	int i;
 
 	utrdlp = hba->utrdl_base_addr;
 
-	response_offset =
-		offsetof(struct utp_transfer_cmd_desc, response_upiu);
-	prdt_offset =
-		offsetof(struct utp_transfer_cmd_desc, prd_table);
-
 	cmd_desc_size = ufshcd_get_ucd_size(hba);
 	cmd_desc_dma_addr = hba->ucdl_dma_addr;
 
 	for (i = 0; i < hba->nutrs; i++) {
+		/*
+		 * Reserved tags (low end) use the dedicated devman UCD with a
+		 * larger response area; other tags index the pool at i - RESERVED.
+		 */
+		if (i < UFSHCD_NUM_RESERVED) {
+			cmd_desc_element_addr = hba->devman_ucd_dma_addr;
+			response_offset = offsetof(struct utp_devman_cmd_desc,
+						   response_upiu);
+			prdt_offset = offsetof(struct utp_devman_cmd_desc,
+					       prd_table);
+			response_len = ALIGNED_DEVMAN_RSP_SIZE;
+		} else {
+			cmd_desc_element_addr = cmd_desc_dma_addr +
+				cmd_desc_size * (i - UFSHCD_NUM_RESERVED);
+			response_offset = offsetof(struct utp_transfer_cmd_desc,
+						   response_upiu);
+			prdt_offset = offsetof(struct utp_transfer_cmd_desc,
+					       prd_table);
+			response_len = ALIGNED_UPIU_SIZE;
+		}
+
 		/* Configure UTRD with command descriptor base address */
-		cmd_desc_element_addr =
-				(cmd_desc_dma_addr + (cmd_desc_size * i));
 		utrdlp[i].command_desc_base_addr =
 				cpu_to_le64(cmd_desc_element_addr);
 
@@ -4108,14 +4157,14 @@ static void ufshcd_host_memory_configure(struct ufs_hba *hba)
 			utrdlp[i].prd_table_offset =
 				cpu_to_le16(prdt_offset);
 			utrdlp[i].response_upiu_length =
-				cpu_to_le16(ALIGNED_UPIU_SIZE);
+				cpu_to_le16(response_len);
 		} else {
 			utrdlp[i].response_upiu_offset =
 				cpu_to_le16(response_offset >> 2);
 			utrdlp[i].prd_table_offset =
 				cpu_to_le16(prdt_offset >> 2);
 			utrdlp[i].response_upiu_length =
-				cpu_to_le16(ALIGNED_UPIU_SIZE >> 2);
+				cpu_to_le16(response_len >> 2);
 		}
 	}
 }
@@ -7638,7 +7687,8 @@ static int ufshcd_issue_devman_upiu_cmd(struct ufs_hba *hba,
 
 	/* just copy the upiu response as it is */
 	memcpy(rsp_upiu, lrbp->ucd_rsp_ptr, sizeof(*rsp_upiu));
-	if (desc_buff && desc_op == UPIU_QUERY_OPCODE_READ_DESC) {
+	if (desc_buff && (desc_op == UPIU_QUERY_OPCODE_READ_DESC ||
+			  desc_op == UPIU_QUERY_OPCODE_AGGREGATED_READ)) {
 		u8 *descp = (u8 *)lrbp->ucd_rsp_ptr + sizeof(*rsp_upiu);
 		u16 resp_len = be16_to_cpu(lrbp->ucd_rsp_ptr->header
 					   .data_segment_length);
@@ -7810,10 +7860,7 @@ int ufshcd_advanced_rpmb_req_handler(struct ufs_hba *hba, struct utp_upiu_req *r
 		 * Message is 02h
 		 */
 		if (ehs_len == 2 && rsp_ehs) {
-			/*
-			 * ucd_rsp_ptr points to a buffer with a length of 512 bytes
-			 * (ALIGNED_UPIU_SIZE = 512), and the EHS data just starts from byte32
-			 */
+			/* EHS data starts from byte32 of the devman UCD response area. */
 			ehs_data = (u8 *)lrbp->ucd_rsp_ptr + EHS_OFFSET_IN_RESPONSE;
 			memcpy(rsp_ehs, ehs_data, ehs_len * 32);
 		}
@@ -9239,7 +9286,7 @@ static void ufshcd_release_sdb_queue(struct ufs_hba *hba, int nutrs)
 {
 	size_t ucdl_size, utrdl_size;
 
-	ucdl_size = ufshcd_get_ucd_size(hba) * nutrs;
+	ucdl_size = ufshcd_get_ucd_size(hba) * (nutrs - UFSHCD_NUM_RESERVED);
 	dmam_free_coherent(hba->dev, ucdl_size, hba->ucdl_base_addr,
 			   hba->ucdl_dma_addr);
 
diff --git a/include/ufs/ufs.h b/include/ufs/ufs.h
index 0d48e137d66d..29f7202173a5 100644
--- a/include/ufs/ufs.h
+++ b/include/ufs/ufs.h
@@ -25,6 +25,7 @@ static_assert(sizeof(struct utp_upiu_query) == 20);
 
 #define GENERAL_UPIU_REQUEST_SIZE (sizeof(struct utp_upiu_req))
 #define QUERY_DESC_MAX_SIZE       255
+#define QUERY_AGGREGATED_MAX_SIZE 4096
 #define QUERY_DESC_MIN_SIZE       2
 #define QUERY_DESC_HDR_SIZE       2
 #define QUERY_OSF_SIZE            (GENERAL_UPIU_REQUEST_SIZE - \
@@ -464,6 +465,7 @@ enum query_opcode {
 	UPIU_QUERY_OPCODE_SET_FLAG	= 0x6,
 	UPIU_QUERY_OPCODE_CLEAR_FLAG	= 0x7,
 	UPIU_QUERY_OPCODE_TOGGLE_FLAG	= 0x8,
+	UPIU_QUERY_OPCODE_AGGREGATED_READ = 0x9,
 };
 
 /* bRefClkFreq attribute values */
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index 248d0a5bef40..f56eafe0f29f 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -237,11 +237,13 @@ struct ufs_query {
  * @type: device management command type - Query, NOP OUT
  * @lock: lock to allow one command at a time
  * @query: Device management query information
+ * @tag: tag of the reserved request in use
  */
 struct ufs_dev_cmd {
 	enum dev_cmd_type type;
 	struct mutex lock;
 	struct ufs_query query;
+	int tag;
 };
 
 /**
@@ -952,9 +954,13 @@ enum ufshcd_mcq_opr {
  * @ucdl_base_addr: UFS Command Descriptor base address
  * @utrdl_base_addr: UTP Transfer Request Descriptor base address
  * @utmrdl_base_addr: UTP Task Management Descriptor base address
+ * @devman_ucd_base_addr: UFS Command Descriptor base address for the reserved
+ *	device management tag (has a larger response area)
  * @ucdl_dma_addr: UFS Command Descriptor DMA address
  * @utrdl_dma_addr: UTRDL DMA address
  * @utmrdl_dma_addr: UTMRDL DMA address
+ * @devman_ucd_dma_addr: UFS Command Descriptor DMA address for the reserved
+ *	device management tag
  * @host: Scsi_Host instance of the driver
  * @dev: device handle
  * @ufs_device_wlun: WLUN that controls the entire UFS device.
@@ -1093,11 +1099,13 @@ struct ufs_hba {
 	struct utp_transfer_cmd_desc *ucdl_base_addr;
 	struct utp_transfer_req_desc *utrdl_base_addr;
 	struct utp_task_req_desc *utmrdl_base_addr;
+	struct utp_devman_cmd_desc *devman_ucd_base_addr;
 
 	/* DMA memory reference */
 	dma_addr_t ucdl_dma_addr;
 	dma_addr_t utrdl_dma_addr;
 	dma_addr_t utmrdl_dma_addr;
+	dma_addr_t devman_ucd_dma_addr;
 
 	struct Scsi_Host *host;
 	struct device *dev;
@@ -1356,6 +1364,11 @@ static inline size_t ufshcd_get_ucd_size(const struct ufs_hba *hba)
 	return sizeof(struct utp_transfer_cmd_desc) + SG_ALL * ufshcd_sg_entry_size(hba);
 }
 
+static inline size_t ufshcd_get_devman_ucd_size(const struct ufs_hba *hba)
+{
+	return sizeof(struct utp_devman_cmd_desc) + SG_ALL * ufshcd_sg_entry_size(hba);
+}
+
 /* Returns true if clocks can be gated. Otherwise false */
 static inline bool ufshcd_is_clkgating_allowed(struct ufs_hba *hba)
 {
diff --git a/include/ufs/ufshci.h b/include/ufs/ufshci.h
index 9f0fdd850e54..c4d563420152 100644
--- a/include/ufs/ufshci.h
+++ b/include/ufs/ufshci.h
@@ -18,6 +18,8 @@ enum {
 	TASK_REQ_UPIU_SIZE_DWORDS	= 8,
 	TASK_RSP_UPIU_SIZE_DWORDS	= 8,
 	ALIGNED_UPIU_SIZE		= 512,
+	/* Larger response area, only for the devman UCD */
+	ALIGNED_DEVMAN_RSP_SIZE		= 4096,
 };
 
 /* UFSHCI Registers */
@@ -501,6 +503,15 @@ struct utp_transfer_cmd_desc {
 	u8 prd_table[];
 };
 
+/* Dedicated UCD for the devman/reserved slot */
+struct utp_devman_cmd_desc {
+	u8 command_upiu[ALIGNED_UPIU_SIZE];
+	u8 response_upiu[ALIGNED_DEVMAN_RSP_SIZE];
+	u8 prd_table[];
+};
+
+static_assert(QUERY_AGGREGATED_MAX_SIZE <= ALIGNED_DEVMAN_RSP_SIZE);
+
 /**
  * struct request_desc_header - Descriptor Header common to both UTRD and UTMRD
  */
-- 
2.25.1

Re: [PATCH v2] scsi: ufs: Add support for the aggregated read query opcode

[Bart Van Assche]

On 7/22/26 1:48 AM, Hyeoncheol Jeong wrote:
> -	/* sizeof(struct utp_transfer_cmd_desc) must be a multiple of 128 */
> +	/* Both UCD types must be a multiple of 128 */

"must be" -> "must have a size that is"

"128" -> "128 bytes"

> -	*desc_len = min_t(int, QUERY_DESC_MAX_SIZE, desc_size);
> +	if (desc_op == UPIU_QUERY_OPCODE_AGGREGATED_READ)
> +		*desc_len = min_t(int, QUERY_AGGREGATED_MAX_SIZE, desc_size);
> +	else
> +		*desc_len = min_t(int, QUERY_DESC_MAX_SIZE, desc_size);

Please convert this patch into a series and add a patch before this 
patch that changes the data types of desc_size and desc_len from 'int'
and 'int *' into unsigned types (u16 or unsigned int). The length field
is an unsigned 16-bits number according to the UFS standard. Hence, the
UFS driver should use an unsigned type for the UPIU query length.

This will allow to change "min_t(int, ...)" into "min(...)".

Since 'desc_op' is only used derive the maximum query size, wouldn't it
be better to move the code that limits 'desc_len' into
ufs_bsg_request()? Then 'desc_op' won't have to be passed to
ufs_bsg_get_query_desc_size().

> +	/* The reserved tag uses a dedicated UCD outside the pool. */
> +	if (unlikely(blk_mq_is_reserved_rq(scsi_cmd_to_rq(cmd)))) {
> +		struct utp_devman_cmd_desc *cmd_descp = hba->devman_ucd_base_addr;
> +
> +		cmd_desc_element_addr = hba->devman_ucd_dma_addr;
> +		command_upiu = cmd_descp->command_upiu;
> +		response_upiu = cmd_descp->response_upiu;
> +		prd_table = cmd_descp->prd_table;
> +	} else {
> +		int slot = i - UFSHCD_NUM_RESERVED;
> +		struct utp_transfer_cmd_desc *cmd_descp =
> +			(void *)hba->ucdl_base_addr + slot * ufshcd_get_ucd_size(hba);
> +
> +		cmd_desc_element_addr =
> +			hba->ucdl_dma_addr + slot * ufshcd_get_ucd_size(hba);
> +		command_upiu = cmd_descp->command_upiu;
> +		response_upiu = cmd_descp->response_upiu;
> +		prd_table = cmd_descp->prd_table;
> +	}

The above code is based on the assumption that
!blk_mq_is_reserved_rq(rq) implies that i >= 1. Please add a
WARN_ON_ONCE() statement that makes this assumption explicit, e.g.
WARN_ON_ONCE(slot >= 0).

> +	int tag;

Shouldn't 'tag' have an unsigned type?

> +static inline size_t ufshcd_get_devman_ucd_size(const struct ufs_hba *hba)
> +{
> +	return sizeof(struct utp_devman_cmd_desc) + SG_ALL * ufshcd_sg_entry_size(hba);
> +}

Why SG_ALL? The data buffer for device management commands is allocated
with kmalloc() and hence is contiguous so a single segment descriptor
should be sufficient.

Otherwise this patch looks good to me.

Thanks,

Bart.

Re: [PATCH v2] scsi: ufs: Add support for the aggregated read query opcode

[Hyeoncheol Jeong]

Hi Bart,

Thanks a lot for the detailed review. I've addressed the comments and
will send a v3. Replies inline below.

On 7/23/26 2:09 AM, Bart Van Assche wrote:
> > -	/* sizeof(struct utp_transfer_cmd_desc) must be a multiple of 128 */
> > +	/* Both UCD types must be a multiple of 128 */
>
> "must be" -> "must have a size that is"
>
> "128" -> "128 bytes"

Done. Now the comment is:
	/* Both UCD types must have a size that is a multiple of 128 bytes */

> > -    *desc_len = min_t(int, QUERY_DESC_MAX_SIZE, desc_size);
> > +    if (desc_op == UPIU_QUERY_OPCODE_AGGREGATED_READ)
> > +        *desc_len = min_t(int, QUERY_AGGREGATED_MAX_SIZE, desc_size);
> > +    else
> > +        *desc_len = min_t(int, QUERY_DESC_MAX_SIZE, desc_size);
>
> Please convert this patch into a series and add a patch before this
> patch that changes the data types of desc_size and desc_len from 'int'
> and 'int *' into unsigned types (u16 or unsigned int). The length field
> is an unsigned 16-bits number according to the UFS standard. Hence, the
> UFS driver should use an unsigned type for the UPIU query length.
>
> This will allow to change "min_t(int, ...)" into "min(...)".

Done. v3 will be two-patch series:
  [1/2] scsi: ufs: Use unsigned types for the BSG query descriptor length
  [2/2] scsi: ufs: Add support for the aggregated read query opcode

Patch 1 switches the descriptor length/buffer to u16/u8 and uses min()
instead of min_t(int, ...).

> Since 'desc_op' is only used derive the maximum query size, wouldn't it
> be better to move the code that limits 'desc_len' into
> ufs_bsg_request()? Then 'desc_op' won't have to be passed to
> ufs_bsg_get_query_desc_size().

Agreed. ufs_bsg_get_query_desc_size() was trivial, so I folded it into
its only caller, ufs_bsg_alloc_desc_buffer(), where desc_op is already
available.

> > +    if (unlikely(blk_mq_is_reserved_rq(scsi_cmd_to_rq(cmd)))) {
> [...]
> > +    } else {
> > +        int slot = i - UFSHCD_NUM_RESERVED;
> [...]
>
> The above code is based on the assumption that
> !blk_mq_is_reserved_rq(rq) implies that i >= 1. Please add a
> WARN_ON_ONCE() statement that makes this assumption explicit, e.g.
> WARN_ON_ONCE(slot >= 0).

Done, added WARN_ON_ONCE(slot < 0) in the non-reserved branch (the
assumption is slot >= 0, so the WARN fires when it does not hold).

> > +    int tag;
>
> Shouldn't 'tag' have an unsigned type?

Done. I made it a u8 in struct ufs_dev_cmd.

> > +static inline size_t ufshcd_get_devman_ucd_size(const struct ufs_hba *hba)
> > +{
> > +    return sizeof(struct utp_devman_cmd_desc) + SG_ALL * ufshcd_sg_entry_size(hba);
> > +}
>
> Why SG_ALL? The data buffer for device management commands is allocated
> with kmalloc() and hence is contiguous so a single segment descriptor
> should be sufficient.

This is the one comment I haven't applied yet, and I'd like to
double-check with you before doing so.

The aggregated read path indeed does not use the PRDT at all. But
the reserved tag is also used by advanced RPMB. I understood that path
builds a PRDT via ufshcd_sgl_to_prdt() from the BSG payload, whose sg_cnt
can exceed one when the user buffer spans multiple segments. Keeping
SG_ALL preserves the existing RPMB behaviour; I thought shrinking to a
single segment could truncate multi-segment transfers.

Did I miss something about how RPMB maps its payload here? If a single
segment is in fact guaranteed, I'll drop the prd_table[] member and size
it as:
/* Dedicated UCD for the devman/reserved slot */
struct utp_devman_cmd_desc {
        u8 command_upiu[ALIGNED_UPIU_SIZE];
        u8 response_upiu[ALIGNED_DEVMAN_RSP_SIZE];
};

static inline size_t ufshcd_get_devman_ucd_size(const struct ufs_hba *hba)
{
        return sizeof(struct utp_devman_cmd_desc);
}

Separately, I fixed the Sashiko-flagged OOB read in v3. I capped
QUERY_AGGREGATED_MAX_SIZE (4096 -> 4064) so that the fixed 32-byte UPIU
header plus a full data segment fits within the 4096-byte
response_upiu[], and added a static_assert to prevent regression. 4064
is still well above the ~3 KiB the spec allows in practice.

Thanks.

Re: [PATCH v2] scsi: ufs: Add support for the aggregated read query opcode

[Bart Van Assche]

On 7/23/26 12:47 AM, Hyeoncheol Jeong wrote:
> Thanks a lot for the detailed review. I've addressed the comments and
> will send a v3. Replies inline below.

Please post v3 as a new email thread instead of a reply to this thread.
Replies are overlooked more easily than a new thread.

> On 7/23/26 2:09 AM, Bart Van Assche wrote:

>>> +static inline size_t ufshcd_get_devman_ucd_size(const struct ufs_hba *hba)
>>> +{
>>> +    return sizeof(struct utp_devman_cmd_desc) + SG_ALL * ufshcd_sg_entry_size(hba);
>>> +}
>>
>> Why SG_ALL? The data buffer for device management commands is allocated
>> with kmalloc() and hence is contiguous so a single segment descriptor
>> should be sufficient.
> 
> This is the one comment I haven't applied yet, and I'd like to
> double-check with you before doing so.
> 
> The aggregated read path indeed does not use the PRDT at all. But
> the reserved tag is also used by advanced RPMB. I understood that path
> builds a PRDT via ufshcd_sgl_to_prdt() from the BSG payload, whose sg_cnt
> can exceed one when the user buffer spans multiple segments. Keeping
> SG_ALL preserves the existing RPMB behaviour; I thought shrinking to a
> single segment could truncate multi-segment transfers.
> 
> Did I miss something about how RPMB maps its payload here?

BSG uses struct sg_io_v4 and that data structure supports I/O vectors.
How about changing SG_ALL into 2, which should be sufficient for 4096
bytes spread over two discontiguous physical pages? I think it's fine
not to support the I/O vectors that can be generated by user space code
like this program:
https://github.com/linux-blktests/blktests/blob/master/src/discontiguous-io.cpp

Thanks,

Bart.