All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/2] scsi: ufs: Add support for the aggregated read query opcode
       [not found] <CGME20260728092208epcms2p881b17276fb41c006a4229c1d073b4ad0@epcms2p8>
@ 2026-07-28  9:22 ` Hyeoncheol Jeong
  2026-07-28  9:24   ` [PATCH v4 1/2] scsi: ufs: Use unsigned types for the BSG query Hyeoncheol Jeong
                     ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Hyeoncheol Jeong @ 2026-07-28  9:22 UTC (permalink / raw)
  To: James.Bottomley@HansenPartnership.com, martin.petersen@oracle.com,
	bvanassche@acm.org, linux-scsi@vger.kernel.org
  Cc: ALIM AKHTAR, linux-kernel@vger.kernel.org, Hyeoncheol Jeong,
	Jinyoung Choi, Dukhyun Kwon, Jeuk Kim, Keoseong Park,
	Jaemyung Lee, Jieon Seol, Gyusun Lee, Yunjae Jo

UFS 5.0 / JEDEC 220H introduces the AGGREGATED READ query opcode (0x9),
which retrieves an aggregated data packet bundling multiple Descriptors,
Attributes and Flags in a single QUERY RESPONSE UPIU. Such a packet can be
far larger than a single descriptor, so the reserved (device management)
tag gets a dedicated UCD with an enlarged response area.

Patch 1 is a preparatory cleanup that switches the BSG query descriptor
length to unsigned types. Patch 2 adds the aggregated read support.

Changes since v3:
- Patch 1: drop the (u16) cast in min() (Bart Van Assche).
- Patch 2: reject Advanced RPMB BSG requests with more than
  UFSHCD_DEVMAN_SG_ENTRIES scatter-gather segments before dma_map_sg()
  (Sashiko AI / Bart Van Assche).

v3: https://lore.kernel.org/linux-scsi/20260724030812epcms2p4eb2c77cb4dcd5fd51ca9c3eaa5ea4bfa@epcms2p4/
v2: https://lore.kernel.org/linux-scsi/20260722084819epcms2p49c27fce999e821385f7b5d7ea5a02868@epcms2p4/

Hyeoncheol Jeong (2):
  scsi: ufs: Use unsigned types for the BSG query descriptor length
  scsi: ufs: Add support for the aggregated read query opcode

 drivers/ufs/core/ufs-mcq.c | 14 ++++--
 drivers/ufs/core/ufs_bsg.c | 49 +++++++++++--------
 drivers/ufs/core/ufshcd.c  | 98 ++++++++++++++++++++++++++++----------
 include/ufs/ufs.h          |  6 +++
 include/ufs/ufshcd.h       | 20 ++++++++
 include/ufs/ufshci.h       | 12 +++++
 6 files changed, 151 insertions(+), 48 deletions(-)

--
2.25.1

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v4 1/2] scsi: ufs: Use unsigned types for the BSG query
  2026-07-28  9:22 ` [PATCH v4 0/2] scsi: ufs: Add support for the aggregated read query opcode Hyeoncheol Jeong
@ 2026-07-28  9:24   ` Hyeoncheol Jeong
  2026-07-28 21:08     ` Bart Van Assche
  2026-07-28  9:27   ` [PATCH v4 2/2] scsi: ufs: Add support for the aggregated read query opcode Hyeoncheol Jeong
  2026-08-07 15:37   ` [PATCH v4 0/2] " Martin K. Petersen (Oracle)
  2 siblings, 1 reply; 7+ messages in thread
From: Hyeoncheol Jeong @ 2026-07-28  9:24 UTC (permalink / raw)
  To: James.Bottomley@HansenPartnership.com, martin.petersen@oracle.com,
	bvanassche@acm.org, linux-scsi@vger.kernel.org
  Cc: ALIM AKHTAR, linux-kernel@vger.kernel.org, Hyeoncheol Jeong,
	Jinyoung Choi, Dukhyun Kwon, Jeuk Kim, Keoseong Park,
	Jaemyung Lee, Jieon Seol, Gyusun Lee, Yunjae Jo

The UPIU query length field is an unsigned 16-bit value per the UFS
standard, but ufs_bsg carried it around in signed int. Switch the
descriptor length and buffer pointer to u16/u8, fold the trivial
ufs_bsg_get_query_desc_size() helper into its only caller, and replace
min_t(int, ...) with min(). No functional change intended.

Signed-off-by: Hyeoncheol Jeong <hyenc.jeong@samsung.com>
---
 drivers/ufs/core/ufs_bsg.c | 29 +++++++++++------------------
 1 file changed, 11 insertions(+), 18 deletions(-)

diff --git a/drivers/ufs/core/ufs_bsg.c b/drivers/ufs/core/ufs_bsg.c
index 58b506eac6dc..089cf81de275 100644
--- a/drivers/ufs/core/ufs_bsg.c
+++ b/drivers/ufs/core/ufs_bsg.c
@@ -13,21 +13,8 @@
 #include <ufs/ufshcd.h>
 #include "ufshcd-priv.h"
 
-static int ufs_bsg_get_query_desc_size(struct ufs_hba *hba, int *desc_len,
-				       struct utp_upiu_query *qr)
-{
-	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);
-
-	return 0;
-}
-
 static int ufs_bsg_alloc_desc_buffer(struct ufs_hba *hba, struct bsg_job *job,
-				     uint8_t **desc_buff, int *desc_len,
+				     u8 **desc_buff, u16 *desc_len,
 				     enum query_opcode desc_op)
 {
 	struct ufs_bsg_request *bsg_request = job->request;
@@ -39,11 +26,14 @@ static int ufs_bsg_alloc_desc_buffer(struct ufs_hba *hba, struct bsg_job *job,
 		goto out;
 
 	qr = &bsg_request->upiu_req.qr;
-	if (ufs_bsg_get_query_desc_size(hba, desc_len, qr)) {
+	*desc_len = be16_to_cpu(qr->length);
+	if (*desc_len == 0) {
 		dev_err(hba->dev, "Illegal desc size\n");
 		return -EINVAL;
 	}
 
+	*desc_len = min(*desc_len, QUERY_DESC_MAX_SIZE);
+
 	if (*desc_len > job->request_payload.payload_len) {
 		dev_err(hba->dev, "Illegal desc size\n");
 		return -EINVAL;
@@ -136,8 +126,9 @@ static int ufs_bsg_request(struct bsg_job *job)
 	struct ufs_hba *hba = shost_priv(dev_to_shost(job->dev->parent));
 	struct uic_command uc = {};
 	int msgcode;
-	uint8_t *buff = NULL;
-	int desc_len = 0;
+	u8 *buff = NULL;
+	u16 desc_len = 0;
+	int buff_len;
 	enum query_opcode desc_op = UPIU_QUERY_OPCODE_NOP;
 	int ret;
 	bool rpmb = false;
@@ -156,9 +147,11 @@ static int ufs_bsg_request(struct bsg_job *job)
 		fallthrough;
 	case UPIU_TRANSACTION_NOP_OUT:
 	case UPIU_TRANSACTION_TASK_REQ:
+		buff_len = desc_len;
 		ret = ufshcd_exec_raw_upiu_cmd(hba, &bsg_request->upiu_req,
 					       &bsg_reply->upiu_rsp, msgcode,
-					       buff, &desc_len, desc_op);
+					       buff, &buff_len, desc_op);
+		desc_len = buff_len;
 		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) {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v4 2/2] scsi: ufs: Add support for the aggregated read query opcode
  2026-07-28  9:22 ` [PATCH v4 0/2] scsi: ufs: Add support for the aggregated read query opcode Hyeoncheol Jeong
  2026-07-28  9:24   ` [PATCH v4 1/2] scsi: ufs: Use unsigned types for the BSG query Hyeoncheol Jeong
@ 2026-07-28  9:27   ` Hyeoncheol Jeong
  2026-07-28  9:46     ` sashiko-bot
  2026-07-28 21:11     ` Bart Van Assche
  2026-08-07 15:37   ` [PATCH v4 0/2] " Martin K. Petersen (Oracle)
  2 siblings, 2 replies; 7+ messages in thread
From: Hyeoncheol Jeong @ 2026-07-28  9:27 UTC (permalink / raw)
  To: James.Bottomley@HansenPartnership.com, martin.petersen@oracle.com,
	bvanassche@acm.org, linux-scsi@vger.kernel.org
  Cc: ALIM AKHTAR, linux-kernel@vger.kernel.org, Hyeoncheol Jeong,
	Jinyoung Choi, Dukhyun Kwon, Jeuk Kim, Keoseong Park,
	Jaemyung Lee, Jieon Seol, Gyusun Lee, Yunjae Jo

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 a few
KiB vs the 255-byte descriptor limit), so its response UPIU buffer must
be enlarged. Enlarging the shared utp_transfer_cmd_desc would waste that
extra space 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 and returns the reserved tag
(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 | 14 ++++--
 drivers/ufs/core/ufshcd.c  | 98 ++++++++++++++++++++++++++++----------
 include/ufs/ufs.h          |  6 +++
 include/ufs/ufshcd.h       | 20 ++++++++
 include/ufs/ufshci.h       | 12 +++++
 6 files changed, 133 insertions(+), 31 deletions(-)

diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
index 13b60a2d06db..8106d55f4041 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 have a size that is a multiple of 128 bytes */
 	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 089cf81de275..7a66917b3fc0 100644
--- a/drivers/ufs/core/ufs_bsg.c
+++ b/drivers/ufs/core/ufs_bsg.c
@@ -19,10 +19,12 @@ static int ufs_bsg_alloc_desc_buffer(struct ufs_hba *hba, struct bsg_job *job,
 {
 	struct ufs_bsg_request *bsg_request = job->request;
 	struct utp_upiu_query *qr;
+	u16 max_desc_len;
 	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;
@@ -32,7 +34,9 @@ static int ufs_bsg_alloc_desc_buffer(struct ufs_hba *hba, struct bsg_job *job,
 		return -EINVAL;
 	}

-	*desc_len = min(*desc_len, QUERY_DESC_MAX_SIZE);
+	max_desc_len = desc_op == UPIU_QUERY_OPCODE_AGGREGATED_READ ?
+		       QUERY_AGGREGATED_MAX_SIZE : QUERY_DESC_MAX_SIZE;
+	*desc_len = min(*desc_len, max_desc_len);

 	if (*desc_len > job->request_payload.payload_len) {
 		dev_err(hba->dev, "Illegal desc size\n");
@@ -98,6 +102,9 @@ static int ufs_bsg_exec_advanced_rpmb_req(struct ufs_hba *hba, struct bsg_job *j
 		if (!payload->payload_len || !payload->sg_cnt)
 			return -EINVAL;

+		if (payload->sg_cnt > UFSHCD_DEVMAN_SG_ENTRIES)
+			return -EINVAL;
+
 		sg_cnt = dma_map_sg(hba->host->dma_dev, payload->sg_list, payload->sg_cnt, dir);
 		if (unlikely(!sg_cnt))
 			return -ENOMEM;
@@ -154,7 +161,8 @@ static int ufs_bsg_request(struct bsg_job *job)
 		desc_len = buff_len;
 		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..8aa76d69eb9f 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -2944,23 +2944,44 @@ 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;
+
+		/* Non-reserved tags start at UFSHCD_NUM_RESERVED, so slot >= 0. */
+		WARN_ON_ONCE(slot < 0);
+		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 +3178,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 +4020,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 +4037,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 +4118,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 +4160,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 +7690,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 +7863,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 +9289,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..afbb32654fab 100644
--- a/include/ufs/ufs.h
+++ b/include/ufs/ufs.h
@@ -25,6 +25,11 @@ static_assert(sizeof(struct utp_upiu_query) == 20);

 #define GENERAL_UPIU_REQUEST_SIZE (sizeof(struct utp_upiu_req))
 #define QUERY_DESC_MAX_SIZE       255
+/*
+ * Max aggregated read data segment: the devman response area
+ * (ALIGNED_DEVMAN_RSP_SIZE) minus the fixed UPIU header it follows.
+ */
+#define QUERY_AGGREGATED_MAX_SIZE (4096 - GENERAL_UPIU_REQUEST_SIZE)
 #define QUERY_DESC_MIN_SIZE       2
 #define QUERY_DESC_HDR_SIZE       2
 #define QUERY_OSF_SIZE            (GENERAL_UPIU_REQUEST_SIZE - \
@@ -464,6 +469,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..6007c9eeb43f 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;
+	u8 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,18 @@ 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);
 }

+/*
+ * Two entries should be enough for the largest devman PRDT transfer (4 KiB),
+ * like advanced RPMB.
+ */
+#define UFSHCD_DEVMAN_SG_ENTRIES	2
+
+static inline size_t ufshcd_get_devman_ucd_size(const struct ufs_hba *hba)
+{
+	return sizeof(struct utp_devman_cmd_desc) +
+		UFSHCD_DEVMAN_SG_ENTRIES * 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..c3537bb544ae 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,16 @@ 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(sizeof(struct utp_upiu_req) + QUERY_AGGREGATED_MAX_SIZE <=
+	      ALIGNED_DEVMAN_RSP_SIZE);
+
 /**
  * struct request_desc_header - Descriptor Header common to both UTRD and UTMRD
  */
--
2.25.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 2/2] scsi: ufs: Add support for the aggregated read query opcode
  2026-07-28  9:27   ` [PATCH v4 2/2] scsi: ufs: Add support for the aggregated read query opcode Hyeoncheol Jeong
@ 2026-07-28  9:46     ` sashiko-bot
  2026-07-28 21:11     ` Bart Van Assche
  1 sibling, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-07-28  9:46 UTC (permalink / raw)
  To: Hyeoncheol Jeong; +Cc: linux-scsi

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Fallback to SDB mode after `ufshcd_alloc_mcq()` failure uses freed or uninitialized command queues.
--

--- Patch [2]: [PATCH v4 2/2] scsi: ufs: Add support for the aggregated read query opcode ---
commit 687c47ee8e9a0aab4800ae50a40defa3e567d065
Author: Hyeoncheol Jeong <hyenc.jeong@samsung.com>

scsi: ufs: Add support for the aggregated read query opcode

This commit introduces support for the UFS 5.0 AGGREGATED READ query opcode.
It allocates a dedicated device management command descriptor (UCD) with a
larger response area to handle packets up to 4 KiB, bypassing the standard
shared pool for the reserved tag.

> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 79331c2bd38de..328320b8119cc 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c

[ ... ]

> @@ -9236,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);

[Severity: High]
This is a pre-existing issue, but does this fallback path lead to a
use-after-free or NULL pointer dereference?

When ufshcd_alloc_mcq() is called, if the new MCQ tag depth differs from the
SDB tag depth, it frees the existing SDB queues:

ufshcd_add_scsi_host() -> ufshcd_alloc_mcq()
    ufshcd_release_sdb_queue(hba, old_nutrs);

It then attempts to allocate new queues via ufshcd_memory_alloc(). If this
fails, it restores the tag depth and returns an error without reallocating
the SDB memory:

    ret = ufshcd_memory_alloc(hba);
    if (ret)
        goto err;
    ...
err:
    hba->nutrs = old_nutrs;
    return ret;

The caller, ufshcd_add_scsi_host(), catches the error and deliberately falls
back to SDB mode:

    err = ufshcd_alloc_mcq(hba);
    if (err) {
        /* Continue with SDB mode */
        ufshcd_mcq_disable(hba);
        use_mcq_mode = false;

Since the SDB queues were freed and not successfully reallocated, won't any
subsequent command in SDB mode use freed or uninitialized command queues?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728092208epcms2p881b17276fb41c006a4229c1d073b4ad0@epcms2p8?part=2

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 1/2] scsi: ufs: Use unsigned types for the BSG query
  2026-07-28  9:24   ` [PATCH v4 1/2] scsi: ufs: Use unsigned types for the BSG query Hyeoncheol Jeong
@ 2026-07-28 21:08     ` Bart Van Assche
  0 siblings, 0 replies; 7+ messages in thread
From: Bart Van Assche @ 2026-07-28 21:08 UTC (permalink / raw)
  To: hyenc.jeong, James.Bottomley@HansenPartnership.com,
	martin.petersen@oracle.com, linux-scsi@vger.kernel.org
  Cc: ALIM AKHTAR, linux-kernel@vger.kernel.org, Jinyoung Choi,
	Dukhyun Kwon, Jeuk Kim, Keoseong Park, Jaemyung Lee, Jieon Seol,
	Gyusun Lee, Yunjae Jo

On 7/28/26 2:24 AM, Hyeoncheol Jeong wrote:
> The UPIU query length field is an unsigned 16-bit value per the UFS
> standard, but ufs_bsg carried it around in signed int. Switch the
> descriptor length and buffer pointer to u16/u8, fold the trivial
> ufs_bsg_get_query_desc_size() helper into its only caller, and replace
> min_t(int, ...) with min(). No functional change intended.
Reviewed-by: Bart Van Assche <bvanassche@acm.org>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 2/2] scsi: ufs: Add support for the aggregated read query opcode
  2026-07-28  9:27   ` [PATCH v4 2/2] scsi: ufs: Add support for the aggregated read query opcode Hyeoncheol Jeong
  2026-07-28  9:46     ` sashiko-bot
@ 2026-07-28 21:11     ` Bart Van Assche
  1 sibling, 0 replies; 7+ messages in thread
From: Bart Van Assche @ 2026-07-28 21:11 UTC (permalink / raw)
  To: hyenc.jeong, James.Bottomley@HansenPartnership.com,
	martin.petersen@oracle.com, linux-scsi@vger.kernel.org
  Cc: ALIM AKHTAR, linux-kernel@vger.kernel.org, Jinyoung Choi,
	Dukhyun Kwon, Jeuk Kim, Keoseong Park, Jaemyung Lee, Jieon Seol,
	Gyusun Lee, Yunjae Jo

On 7/28/26 2:27 AM, Hyeoncheol Jeong wrote:
> 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.

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 0/2] scsi: ufs: Add support for the aggregated read query opcode
  2026-07-28  9:22 ` [PATCH v4 0/2] scsi: ufs: Add support for the aggregated read query opcode Hyeoncheol Jeong
  2026-07-28  9:24   ` [PATCH v4 1/2] scsi: ufs: Use unsigned types for the BSG query Hyeoncheol Jeong
  2026-07-28  9:27   ` [PATCH v4 2/2] scsi: ufs: Add support for the aggregated read query opcode Hyeoncheol Jeong
@ 2026-08-07 15:37   ` Martin K. Petersen (Oracle)
  2 siblings, 0 replies; 7+ messages in thread
From: Martin K. Petersen (Oracle) @ 2026-08-07 15:37 UTC (permalink / raw)
  To: Hyeoncheol Jeong
  Cc: James.Bottomley@HansenPartnership.com, martin.petersen@oracle.com,
	bvanassche@acm.org, linux-scsi@vger.kernel.org, ALIM AKHTAR,
	linux-kernel@vger.kernel.org, Jinyoung Choi, Dukhyun Kwon,
	Jeuk Kim, Keoseong Park, Jaemyung Lee, Jieon Seol, Gyusun Lee,
	Yunjae Jo


Hyeoncheol,

> UFS 5.0 / JEDEC 220H introduces the AGGREGATED READ query opcode
> (0x9), which retrieves an aggregated data packet bundling multiple
> Descriptors, Attributes and Flags in a single QUERY RESPONSE UPIU.
> Such a packet can be far larger than a single descriptor, so the
> reserved (device management) tag gets a dedicated UCD with an enlarged
> response area.

Applied to 7.3/scsi-staging, thanks!

-- 
Martin K. Petersen

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-08-07 15:37 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <CGME20260728092208epcms2p881b17276fb41c006a4229c1d073b4ad0@epcms2p8>
2026-07-28  9:22 ` [PATCH v4 0/2] scsi: ufs: Add support for the aggregated read query opcode Hyeoncheol Jeong
2026-07-28  9:24   ` [PATCH v4 1/2] scsi: ufs: Use unsigned types for the BSG query Hyeoncheol Jeong
2026-07-28 21:08     ` Bart Van Assche
2026-07-28  9:27   ` [PATCH v4 2/2] scsi: ufs: Add support for the aggregated read query opcode Hyeoncheol Jeong
2026-07-28  9:46     ` sashiko-bot
2026-07-28 21:11     ` Bart Van Assche
2026-08-07 15:37   ` [PATCH v4 0/2] " Martin K. Petersen (Oracle)

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.