4 files changed, 24 insertions(+), 9 deletions(-)
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
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.
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.
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.
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.
© 2016 - 2026 Red Hat, Inc.