[PATCH 0/2] scsi: ufs: batch device-init reads with UFS 5.0 aggregated read

[PATCH 0/2] scsi: ufs: batch device-init reads with UFS 5.0 aggregated read

[Hyeoncheol Jeong]

UFS 5.0 adds the AGGREGATED READ query (opcode 0x9): one QUERY RESPONSE
UPIU returns many descriptors, attributes and flags as a chain of typed
groups.

Patch 1 adds the opcode, the request mask, the group header and the reply
plumbing. Patch 2 uses it during device init: a single aggregated read
replaces the per-item queries for the attributes, flags and ID strings,
falling back to individual queries on older devices or a rejected opcode.

No functional change for pre-5.0 devices.

Hyeoncheol Jeong (2):
  scsi: ufs: Add the aggregated read query opcode and its reply format
  scsi: ufs: Serve the device init reads from one aggregated read

 drivers/ufs/core/ufshcd.c | 365 +++++++++++++++++++++++++++++++++++---
 include/ufs/ufs.h         |  46 +++++
 include/ufs/ufshcd.h      |  11 ++
 3 files changed, 402 insertions(+), 20 deletions(-)

-- 
2.25.1

[PATCH 1/2] scsi: ufs: Add the aggregated read query opcode and its reply format

[Hyeoncheol Jeong]

UFS 5.0 adds the AGGREGATED READ query opcode (0x9): a single QUERY
RESPONSE UPIU returns many descriptors, attributes and flags as a chain
of typed groups (JESD220H 10.7.9.14).

Add the AGGREGATION TYPE mask, the group types and the group header, and
let ufshcd_copy_query_response() copy the reply's data segment like a
descriptor read, recording its length in struct ufs_query_res.

Signed-off-by: Hyeoncheol Jeong <hyenc.jeong@samsung.com>
---
 drivers/ufs/core/ufshcd.c |  7 +++++--
 include/ufs/ufs.h         | 43 +++++++++++++++++++++++++++++++++++++++
 include/ufs/ufshcd.h      |  2 ++
 3 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 2ba244cf40ac..3c404d9bc18c 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -2473,10 +2473,13 @@ int ufshcd_copy_query_response(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
 	struct ufs_query_res *query_res = &hba->dev_cmd.query.response;
 
 	memcpy(&query_res->upiu_res, &lrbp->ucd_rsp_ptr->qr, QUERY_OSF_SIZE);
+	query_res->data_segment_length =
+		be16_to_cpu(lrbp->ucd_rsp_ptr->header.data_segment_length);
 
-	/* Get the descriptor */
+	/* Get the descriptor or aggregated data packet */
 	if (hba->dev_cmd.query.descriptor &&
-	    lrbp->ucd_rsp_ptr->qr.opcode == UPIU_QUERY_OPCODE_READ_DESC) {
+	    (lrbp->ucd_rsp_ptr->qr.opcode == UPIU_QUERY_OPCODE_READ_DESC ||
+	     lrbp->ucd_rsp_ptr->qr.opcode == UPIU_QUERY_OPCODE_AGGREGATED_READ)) {
 		u8 *descp = (u8 *)lrbp->ucd_rsp_ptr +
 				GENERAL_UPIU_REQUEST_SIZE;
 		u16 resp_len;
diff --git a/include/ufs/ufs.h b/include/ufs/ufs.h
index afbb32654fab..7ae5c1933dfb 100644
--- a/include/ufs/ufs.h
+++ b/include/ufs/ufs.h
@@ -30,6 +30,7 @@ static_assert(sizeof(struct utp_upiu_query) == 20);
  * (ALIGNED_DEVMAN_RSP_SIZE) minus the fixed UPIU header it follows.
  */
 #define QUERY_AGGREGATED_MAX_SIZE (4096 - GENERAL_UPIU_REQUEST_SIZE)
+#define QUERY_AGG_GROUP_HDR_SIZE  4
 #define QUERY_DESC_MIN_SIZE       2
 #define QUERY_DESC_HDR_SIZE       2
 #define QUERY_OSF_SIZE            (GENERAL_UPIU_REQUEST_SIZE - \
@@ -569,6 +570,48 @@ enum ufs_dev_pwr_mode {
 
 #define UFS_WB_BUF_REMAIN_PERCENT(val) ((val) / 10)
 
+/* AGGREGATION TYPE field of an AGGREGATED READ query request */
+enum ufs_agg_type {
+	UFS_AGG_TYPE_ALL_FLAGS			= BIT(0),
+	UFS_AGG_TYPE_ALL_ATTRS			= BIT(1),
+	UFS_AGG_TYPE_DEVICE_DESC		= BIT(2),
+	/* Unit descriptors and the RPMB unit descriptor */
+	UFS_AGG_TYPE_UNIT_DESC			= BIT(3),
+	UFS_AGG_TYPE_INTERCONNECT_DESC	= BIT(4),
+	UFS_AGG_TYPE_STRING_DESC		= BIT(5),
+	/* Geometry descriptor and power descriptor */
+	UFS_AGG_TYPE_GEOMETRY_POWER_DESC	= BIT(6),
+	UFS_AGG_TYPE_HEALTH_DESC		= BIT(7),
+};
+
+/* Group Type field of an aggregated data packet group header. */
+enum ufs_agg_group_type {
+	UFS_AGG_GROUP_FLAGS		= 0x01,
+	UFS_AGG_GROUP_ATTRS		= 0x02,
+	/* Any descriptor with a unique IDN, i.e. all but the string ones */
+	UFS_AGG_GROUP_DESCS		= 0x03,
+	UFS_AGG_GROUP_MANUFACTURER_STR	= 0x04,
+	UFS_AGG_GROUP_PRODUCT_NAME_STR	= 0x05,
+	UFS_AGG_GROUP_OEM_ID_STR	= 0x06,
+	UFS_AGG_GROUP_SERIAL_NUMBER_STR	= 0x07,
+	UFS_AGG_GROUP_PRODUCT_REV_STR	= 0x08,
+};
+
+/**
+ * struct utp_agg_group_header - aggregated data packet group header
+ * @group_type: type of the data carried by this group, see ufs_agg_group_type
+ * @reserved: reserved
+ * @next_group_offset: byte offset from the start of the packet to the next
+ *	group; zero ends the chain
+ */
+struct utp_agg_group_header {
+	u8 group_type;
+	u8 reserved;
+	__be16 next_group_offset;
+};
+
+static_assert(sizeof(struct utp_agg_group_header) == QUERY_AGG_GROUP_HDR_SIZE);
+
 /**
  * struct utp_cmd_rsp - RESPONSE UPIU structure
  * @residual_transfer_count: Residual transfer count DW-3
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index dfd302f2dc7c..1a5e181ff9e3 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -213,9 +213,11 @@ struct ufs_query_req {
 /**
  * struct ufs_query_res - UPIU QUERY
  * @upiu_res: query response data
+ * @data_segment_length: response data segment length
  */
 struct ufs_query_res {
 	struct utp_upiu_query upiu_res;
+	u16 data_segment_length;
 };
 
 /**
-- 
2.25.1

[PATCH 2/2] scsi: ufs: Serve the device init reads from one aggregated read

[Hyeoncheol Jeong]

Device init reads several attributes and flags one query at a time. On
UFS 5.0, issue one AGGREGATED READ once the device descriptor has
established wSpecVersion, cache the reply, and serve the following
attribute, flag and ID-string reads from it. Anything not present, and
older devices or a rejected opcode, fall back to individual queries.

The following per-item queries are folded into the one aggregated read:

Attributes (ALL_ATTRS group)
	0x0c bMaxNumOfRTT                  - ufshcd_set_rtt()
	0x17 bRefClkGatingWaitTime         - ufshcd_get_ref_clk_gating_wait()
	0x1e bWriteBoosterBufferLifeTimeEst - ufshcd_wb_probe()

Flags (ALL_FLAGS group)
	0x03 fPowerOnWPEn                  - ufshcd_device_params_init()

Strings (STRING_DESC groups)
	Product Name string                - ufs_get_device_desc()
	Serial Number string               - ufshcd_create_device_id()

(bWriteBoosterBufferLifeTimeEst is served from the packet only in the
shared-buffer mode)

Signed-off-by: Hyeoncheol Jeong <hyenc.jeong@samsung.com>
---
 drivers/ufs/core/ufshcd.c | 356 ++++++++++++++++++++++++++++++++++++--
 include/ufs/ufs.h         |   3 +
 include/ufs/ufshcd.h      |   9 +
 3 files changed, 350 insertions(+), 18 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 3c404d9bc18c..859a81d4f3cc 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -3792,6 +3792,313 @@ int ufshcd_query_descriptor_retry(struct ufs_hba *hba,
 	return err;
 }
 
+/* Attribute data width by IDN for aggregated read (JESD220H Table 14.28). */
+static const u8 ufs_agg_attr_width[] = {
+	[QUERY_ATTR_IDN_BOOT_LU_EN] = 1, [QUERY_ATTR_IDN_POWER_MODE] = 1,
+	[QUERY_ATTR_IDN_ACTIVE_ICC_LVL] = 1, [QUERY_ATTR_IDN_OOO_DATA_EN] = 1,
+	[QUERY_ATTR_IDN_BKOPS_STATUS] = 1, [QUERY_ATTR_IDN_PURGE_STATUS] = 1,
+	[QUERY_ATTR_IDN_MAX_DATA_IN] = 1, [QUERY_ATTR_IDN_MAX_DATA_OUT] = 1,
+	[QUERY_ATTR_IDN_DYN_CAP_NEEDED] = 4, [QUERY_ATTR_IDN_REF_CLK_FREQ] = 1,
+	[QUERY_ATTR_IDN_CONF_DESC_LOCK] = 1, [QUERY_ATTR_IDN_MAX_NUM_OF_RTT] = 1,
+	[QUERY_ATTR_IDN_EE_CONTROL] = 2, [QUERY_ATTR_IDN_EE_STATUS] = 2,
+	[QUERY_ATTR_IDN_SECONDS_PASSED] = 4, [QUERY_ATTR_IDN_CNTX_CONF] = 2,
+	[QUERY_ATTR_IDN_FFU_STATUS] = 1, [QUERY_ATTR_IDN_PSA_STATE] = 1,
+	[QUERY_ATTR_IDN_PSA_DATA_SIZE] = 4,
+	[QUERY_ATTR_IDN_REF_CLK_GATING_WAIT_TIME] = 1,
+	[QUERY_ATTR_IDN_CASE_ROUGH_TEMP] = 1, [QUERY_ATTR_IDN_HIGH_TEMP_BOUND] = 1,
+	[QUERY_ATTR_IDN_LOW_TEMP_BOUND] = 1, [0x1b] = 1,
+	[QUERY_ATTR_IDN_WB_FLUSH_STATUS] = 1,
+	[QUERY_ATTR_IDN_AVAIL_WB_BUFF_SIZE] = 1,
+	[QUERY_ATTR_IDN_WB_BUFF_LIFE_TIME_EST] = 1,
+	[QUERY_ATTR_IDN_CURR_WB_BUFF_SIZE] = 4,
+};
+
+/**
+ * ufshcd_agg_group - find a group in the cached aggregated data packet
+ * @hba: per-adapter instance
+ * @type: group type to find
+ * @group_len: set to the group payload length on a hit
+ *
+ * Return: the group payload, or NULL if @type is not present.
+ */
+static const u8 *ufshcd_agg_group(const struct ufs_hba *hba, u8 type,
+				  u16 *group_len)
+{
+	const u8 *packet = hba->agg_packet;
+	u16 hdr_off = 0;
+
+	while (packet && hdr_off + QUERY_AGG_GROUP_HDR_SIZE <= hba->agg_packet_len) {
+		const struct utp_agg_group_header *hdr =
+			(const void *)(packet + hdr_off);
+		u16 next_off = be16_to_cpu(hdr->next_group_offset);
+		u16 payload_off = hdr_off + QUERY_AGG_GROUP_HDR_SIZE;
+
+		if (next_off && (next_off < payload_off ||
+				 next_off + QUERY_AGG_GROUP_HDR_SIZE > hba->agg_packet_len))
+			return NULL;
+
+		if (hdr->group_type == type) {
+			*group_len = (next_off ? next_off : hba->agg_packet_len) -
+				     payload_off;
+			return packet + payload_off;
+		}
+
+		if (!next_off)
+			break;
+
+		hdr_off = next_off;
+	}
+
+	return NULL;
+}
+
+/**
+ * ufshcd_agg_string - find a string descriptor in the cached aggregated packet
+ * @hba: per-adapter instance
+ * @index: string descriptor index
+ * @len: set to the descriptor length on a hit
+ *
+ * Return: the string descriptor, or NULL if not present.
+ */
+static const u8 *ufshcd_agg_string(struct ufs_hba *hba, u8 index, u8 *len)
+{
+	u16 group_len;
+	const u8 *group_buf;
+	int i;
+
+	if (!index)
+		return NULL;
+
+	for (i = 0; i < ARRAY_SIZE(hba->agg_str_idx); i++)
+		if (hba->agg_str_idx[i] == index)
+			break;
+
+	if (i == ARRAY_SIZE(hba->agg_str_idx))
+		return NULL;
+
+	group_buf = ufshcd_agg_group(hba, UFS_AGG_GROUP_MANUFACTURER_STR + i,
+				     &group_len);
+	if (!group_buf || group_len < QUERY_DESC_HDR_SIZE)
+		return NULL;
+
+	*len = group_buf[QUERY_DESC_LENGTH_OFFSET];
+	if (*len < QUERY_DESC_HDR_SIZE || *len > group_len)
+		return NULL;
+
+	return group_buf;
+}
+
+/**
+ * ufshcd_agg_desc - find a descriptor in the cached aggregated data packet
+ * @hba: per-adapter instance
+ * @idn: descriptor IDN
+ * @index: unit index, for unit descriptors; string index for string ones
+ * @len: set to the descriptor length on a hit
+ *
+ * Return: the descriptor, or NULL if not present.
+ */
+static const u8 *ufshcd_agg_desc(struct ufs_hba *hba, enum desc_idn idn,
+				 u8 index, u8 *len)
+{
+	u16 group_len, off = 0;
+	const u8 *group_buf;
+
+	if (idn == QUERY_DESC_IDN_STRING)
+		return ufshcd_agg_string(hba, index, len);
+
+	group_buf = ufshcd_agg_group(hba, UFS_AGG_GROUP_DESCS, &group_len);
+	while (group_buf && off + QUERY_DESC_HDR_SIZE <= group_len) {
+		const u8 *desc_buf = group_buf + off;
+		u8 desc_len = desc_buf[QUERY_DESC_LENGTH_OFFSET];
+
+		if (desc_len < QUERY_DESC_HDR_SIZE || off + desc_len > group_len)
+			break;
+
+		if (desc_buf[QUERY_DESC_DESC_TYPE_OFFSET] == idn &&
+		    (idn != QUERY_DESC_IDN_UNIT ||
+		     (desc_len > UNIT_DESC_PARAM_UNIT_INDEX &&
+		      desc_buf[UNIT_DESC_PARAM_UNIT_INDEX] == index))) {
+			*len = desc_len;
+			return desc_buf;
+		}
+
+		off += desc_len;
+	}
+
+	return NULL;
+}
+
+/**
+ * ufshcd_agg_attr - read an attribute from the cached aggregated data packet
+ * @hba: per-adapter instance
+ * @idn: attribute IDN (index 0 only)
+ * @out: set to the value on a hit
+ *
+ * Return: 0 on a hit, -ENOENT otherwise.
+ */
+static int ufshcd_agg_attr(struct ufs_hba *hba, u8 idn, u32 *out)
+{
+	u16 group_len, off = 0;
+	const u8 *group_buf;
+	int i;
+
+	if (idn >= ARRAY_SIZE(ufs_agg_attr_width) || !ufs_agg_attr_width[idn])
+		return -ENOENT;
+
+	group_buf = ufshcd_agg_group(hba, UFS_AGG_GROUP_ATTRS, &group_len);
+	if (!group_buf)
+		return -ENOENT;
+
+	for (i = 0; i < idn; i++)
+		off += ufs_agg_attr_width[i];
+
+	if (off + ufs_agg_attr_width[idn] > group_len)
+		return -ENOENT;
+
+	*out = 0;
+	for (i = 0; i < ufs_agg_attr_width[idn]; i++)
+		*out = (*out << 8) | group_buf[off + i];
+
+	return 0;
+}
+
+/**
+ * ufshcd_agg_flag - read a flag from the cached aggregated data packet
+ * @hba: per-adapter instance
+ * @idn: flag IDN
+ * @out: set to the value on a hit
+ *
+ * Return: 0 on a hit, -ENOENT otherwise.
+ */
+static int ufshcd_agg_flag(struct ufs_hba *hba, u8 idn, bool *out)
+{
+	u16 group_len;
+	const u8 *group_buf = ufshcd_agg_group(hba, UFS_AGG_GROUP_FLAGS,
+					       &group_len);
+
+	if (!group_buf || idn >= group_len)
+		return -ENOENT;
+
+	*out = group_buf[idn];
+	return 0;
+}
+
+/**
+ * ufshcd_read_attr - read an attribute, from the cached packet if present
+ * @hba: per-adapter instance
+ * @idn: attribute IDN
+ * @index: attribute index
+ * @out: result
+ *
+ * Return: 0 on success, < 0 on error.
+ */
+static int ufshcd_read_attr(struct ufs_hba *hba, u8 idn, u8 index, u32 *out)
+{
+	if (index == 0 && !ufshcd_agg_attr(hba, idn, out))
+		return 0;
+
+	return ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_READ_ATTR, idn,
+				       index, 0, out);
+}
+
+/**
+ * ufshcd_read_flag - read a flag, from the cached packet if present
+ * @hba: per-adapter instance
+ * @idn: flag IDN
+ * @out: result
+ *
+ * Return: 0 on success, < 0 on error.
+ */
+static int ufshcd_read_flag(struct ufs_hba *hba, u8 idn, bool *out)
+{
+	if (!ufshcd_agg_flag(hba, idn, out))
+		return 0;
+
+	return ufshcd_query_flag_retry(hba, UPIU_QUERY_OPCODE_READ_FLAG, idn, 0,
+				       out);
+}
+
+/**
+ * ufshcd_query_aggregated_read - issue an AGGREGATED READ query
+ * @hba: per-adapter instance
+ * @agg_type: AGGREGATION TYPE mask (OSF) selecting the items to fetch
+ * @buf: buffer for the reply data segment
+ * @buf_len: requested length in, received length out
+ *
+ * Return: 0 on success; > 0 on an OCS error; < 0 otherwise.
+ */
+static int ufshcd_query_aggregated_read(struct ufs_hba *hba, u8 agg_type,
+					u8 *buf, int *buf_len)
+{
+	struct ufs_query_req *request = NULL;
+	struct ufs_query_res *response = NULL;
+	int err;
+
+	if (*buf_len <= 0 || *buf_len > QUERY_AGGREGATED_MAX_SIZE)
+		return -EINVAL;
+
+	ufshcd_dev_man_lock(hba);
+	ufshcd_init_query(hba, &request, &response,
+			  UPIU_QUERY_OPCODE_AGGREGATED_READ, agg_type, 0, 0);
+	request->query_func = UPIU_QUERY_FUNC_STANDARD_READ_REQUEST;
+	request->upiu_req.length = cpu_to_be16(*buf_len);
+	hba->dev_cmd.query.descriptor = buf;
+
+	err = ufshcd_exec_dev_cmd(hba, DEV_CMD_TYPE_QUERY, dev_cmd_timeout);
+	if (!err)
+		*buf_len = response->data_segment_length;
+
+	hba->dev_cmd.query.descriptor = NULL;
+	ufshcd_dev_man_unlock(hba);
+	return err;
+}
+
+/**
+ * ufshcd_agg_read_begin - cache one AGGREGATED READ for the reads that follow
+ * @hba: per-adapter instance
+ * @agg_type: AGGREGATION TYPE mask to fetch
+ *
+ * Paired with ufshcd_agg_read_end(). Requires a UFS 5.0 device.
+ */
+static void ufshcd_agg_read_begin(struct ufs_hba *hba, u8 agg_type)
+{
+	int len = QUERY_AGGREGATED_MAX_SIZE;
+	int retries;
+	u8 *packet;
+
+	if (hba->dev_info.wspecversion < 0x500 ||
+	    hba->dev_info.agg_read_unsupported)
+		return;
+
+	/* Zeroed so a device over-reporting LENGTH terminates the walk safely. */
+	packet = kzalloc(QUERY_AGGREGATED_MAX_SIZE, GFP_KERNEL);
+	if (!packet)
+		return;
+
+	for (retries = QUERY_REQ_RETRIES; retries > 0; retries--) {
+		if (!ufshcd_query_aggregated_read(hba, agg_type, packet, &len))
+			break;
+	}
+
+	if (!retries) {
+		dev_dbg(hba->dev, "aggregated read unsupported, using individual queries\n");
+		hba->dev_info.agg_read_unsupported = true;
+		kfree(packet);
+		return;
+	}
+
+	hba->agg_packet = packet;
+	hba->agg_packet_len = len;
+}
+
+/* ufshcd_agg_read_end - drop the packet cached by ufshcd_agg_read_begin() */
+static void ufshcd_agg_read_end(struct ufs_hba *hba)
+{
+	kfree(hba->agg_packet);
+	hba->agg_packet = NULL;
+	hba->agg_packet_len = 0;
+}
+
 /**
  * ufshcd_read_desc_param - read the specified descriptor parameter
  * @hba: Pointer to adapter instance
@@ -3813,6 +4120,8 @@ int ufshcd_read_desc_param(struct ufs_hba *hba,
 {
 	int ret;
 	u8 *desc_buf;
+	const u8 *descp;
+	u8 desc_len;
 	int buff_len = QUERY_DESC_MAX_SIZE;
 	bool is_kmalloc = true;
 
@@ -3830,14 +4139,20 @@ int ufshcd_read_desc_param(struct ufs_hba *hba,
 		is_kmalloc = false;
 	}
 
-	/* Request for full descriptor */
-	ret = ufshcd_query_descriptor_retry(hba, UPIU_QUERY_OPCODE_READ_DESC,
-					    desc_id, desc_index, 0,
-					    desc_buf, &buff_len);
-	if (ret) {
-		dev_err(hba->dev, "%s: Failed reading descriptor. desc_id %d, desc_index %d, param_offset %d, ret %d\n",
-			__func__, desc_id, desc_index, param_offset, ret);
-		goto out;
+	/* Serve from the cached aggregated packet, else request the full one. */
+	descp = ufshcd_agg_desc(hba, desc_id, desc_index, &desc_len);
+	if (descp) {
+		memcpy(desc_buf, descp, desc_len);
+		ret = 0;
+	} else {
+		ret = ufshcd_query_descriptor_retry(hba, UPIU_QUERY_OPCODE_READ_DESC,
+						    desc_id, desc_index, 0,
+						    desc_buf, &buff_len);
+		if (ret) {
+			dev_err(hba->dev, "%s: Failed reading descriptor. desc_id %d, desc_index %d, param_offset %d, ret %d\n",
+				__func__, desc_id, desc_index, param_offset, ret);
+			goto out;
+		}
 	}
 
 	/* Update descriptor length */
@@ -4019,9 +4334,8 @@ static int ufshcd_get_ref_clk_gating_wait(struct ufs_hba *hba)
 	u32 gating_wait = UFSHCD_REF_CLK_GATING_WAIT_US;
 
 	if (hba->dev_info.wspecversion >= 0x300) {
-		err = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_READ_ATTR,
-				QUERY_ATTR_IDN_REF_CLK_GATING_WAIT_TIME, 0, 0,
-				&gating_wait);
+		err = ufshcd_read_attr(hba, QUERY_ATTR_IDN_REF_CLK_GATING_WAIT_TIME,
+				       0, &gating_wait);
 		if (err)
 			dev_err(hba->dev, "Failed reading bRefClkGatingWait. err = %d, use default %uus\n",
 					 err, gating_wait);
@@ -6531,9 +6845,8 @@ static bool ufshcd_is_wb_buf_lifetime_available(struct ufs_hba *hba)
 	u8 index;
 
 	index = ufshcd_wb_get_query_index(hba);
-	ret = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_READ_ATTR,
-				      QUERY_ATTR_IDN_WB_BUFF_LIFE_TIME_EST,
-				      index, 0, &lifetime);
+	ret = ufshcd_read_attr(hba, QUERY_ATTR_IDN_WB_BUFF_LIFE_TIME_EST,
+			       index, &lifetime);
 	if (ret) {
 		dev_err(hba->dev,
 			"%s: bWriteBoosterBufferLifeTimeEst read failed %d\n",
@@ -8671,8 +8984,7 @@ static void ufshcd_set_rtt(struct ufs_hba *hba)
 	if (dev_info->wspecversion < 0x400)
 		return;
 
-	if (ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_READ_ATTR,
-				    QUERY_ATTR_IDN_MAX_NUM_OF_RTT, 0, 0, &dev_rtt)) {
+	if (ufshcd_read_attr(hba, QUERY_ATTR_IDN_MAX_NUM_OF_RTT, 0, &dev_rtt)) {
 		dev_err(hba->dev, "failed reading bMaxNumOfRTT\n");
 		return;
 	}
@@ -8893,6 +9205,14 @@ static int ufs_get_device_desc(struct ufs_hba *hba)
 				      desc_buf[DEVICE_DESC_PARAM_SPEC_VER + 1];
 	dev_info->bqueuedepth = desc_buf[DEVICE_DESC_PARAM_Q_DPTH];
 
+	hba->agg_str_idx[0] = desc_buf[DEVICE_DESC_PARAM_MANF_NAME];
+	hba->agg_str_idx[1] = desc_buf[DEVICE_DESC_PARAM_PRDCT_NAME];
+	hba->agg_str_idx[2] = desc_buf[DEVICE_DESC_PARAM_OEM_ID];
+	hba->agg_str_idx[3] = desc_buf[DEVICE_DESC_PARAM_SN];
+	hba->agg_str_idx[4] = desc_buf[DEVICE_DESC_PARAM_PRDCT_REV];
+	ufshcd_agg_read_begin(hba, UFS_AGG_TYPE_ALL_ATTRS | UFS_AGG_TYPE_ALL_FLAGS |
+			      UFS_AGG_TYPE_STRING_DESC);
+
 	/*
 	 * According to the UFS standard, the UFS device queue depth
 	 * (bQueueDepth) must be in the range 1..255 if the shared queueing
@@ -9208,8 +9528,7 @@ static int ufshcd_device_params_init(struct ufs_hba *hba)
 
 	ufshcd_get_ref_clk_gating_wait(hba);
 
-	if (!ufshcd_query_flag_retry(hba, UPIU_QUERY_OPCODE_READ_FLAG,
-			QUERY_FLAG_IDN_PWR_ON_WPE, 0, &flag))
+	if (!ufshcd_read_flag(hba, QUERY_FLAG_IDN_PWR_ON_WPE, &flag))
 		hba->dev_info.f_power_on_wp_en = flag;
 
 	/* Probe maximum power mode co-supported by both UFS host and device */
@@ -9220,6 +9539,7 @@ static int ufshcd_device_params_init(struct ufs_hba *hba)
 
 	ufshcd_retrieve_tx_eq_settings(hba);
 out:
+	ufshcd_agg_read_end(hba);
 	return ret;
 }
 
diff --git a/include/ufs/ufs.h b/include/ufs/ufs.h
index 7ae5c1933dfb..25e9513158cf 100644
--- a/include/ufs/ufs.h
+++ b/include/ufs/ufs.h
@@ -706,6 +706,9 @@ struct ufs_dev_info {
 
 	bool hid_sup;
 
+	/* Set once the device has rejected an AGGREGATED READ query. */
+	bool agg_read_unsupported;
+
 	/* Unique device ID string (manufacturer+model+serial+version+date) */
 	char *device_id;
 	u8 rpmb_io_size;
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index 1a5e181ff9e3..53a7573890cd 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -1098,6 +1098,10 @@ enum ufshcd_mcq_opr {
  * @device_deemphasis_cap: a bitfield to indicate supported DeEmphasis dBs of device's TX lanes,
  *	cache of device M-PHY TX_HS_DeEmphasis_Setting_Capability Attribute (ID 0x12)
  * @tx_eq_params: TX Equalization settings
+ * @agg_packet: aggregated data packet cached during a read batch, or NULL
+ * @agg_packet_len: length of @agg_packet
+ * @agg_str_idx: string descriptor index per string group, indexed by
+ *	group type - UFS_AGG_GROUP_MANUFACTURER_STR
  */
 struct ufs_hba {
 	void __iomem *mmio_base;
@@ -1282,6 +1286,11 @@ struct ufs_hba {
 	u8 device_preshoot_cap;
 	u8 device_deemphasis_cap;
 	struct ufshcd_tx_eq_params tx_eq_params[UFS_HS_GEAR_MAX];
+
+	u8 *agg_packet;
+	u16 agg_packet_len;
+	u8 agg_str_idx[UFS_AGG_GROUP_PRODUCT_REV_STR -
+		       UFS_AGG_GROUP_MANUFACTURER_STR + 1];
 };
 
 /**
-- 
2.25.1

Re: [PATCH 2/2] scsi: ufs: Serve the device init reads from one aggregated read

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Potential unaligned memory access when parsing malicious or buggy UFS 5.0 aggregated read packets.
--

commit 6958e42c99c17e67c3aba30be65b988c84328c42
Author: Hyeoncheol Jeong <hyenc.jeong@samsung.com>

scsi: ufs: Serve the device init reads from one aggregated read

The commit caches a UFS 5.0 AGGREGATED READ payload into a buffer during
initialization and serves individual attribute, flag, and string descriptor
requests from the cached packet.

> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index e0254ddb31729..05a2f4532be5d 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c

[ ... ]

> @@ -3816,6 +3816,313 @@ int ufshcd_query_descriptor_retry(struct ufs_hba *hba,
>  	return err;
>  }
>  
[ ... ]
> +/**
> + * ufshcd_agg_group - find a group in the cached aggregated data packet
> + * @hba: per-adapter instance
> + * @type: group type to find
> + * @group_len: set to the group payload length on a hit
> + *
> + * Return: the group payload, or NULL if @type is not present.
> + */
> +static const u8 *ufshcd_agg_group(const struct ufs_hba *hba, u8 type,
> +				  u16 *group_len)
> +{
> +	const u8 *packet = hba->agg_packet;
> +	u16 hdr_off = 0;
> +
> +	while (packet && hdr_off + QUERY_AGG_GROUP_HDR_SIZE <= hba->agg_packet_len) {
> +		const struct utp_agg_group_header *hdr =
> +			(const void *)(packet + hdr_off);
> +		u16 next_off = be16_to_cpu(hdr->next_group_offset);

[Severity: High]
Can this cause an unaligned memory access crash on strict-alignment
architectures?

Since struct utp_agg_group_header lacks the __packed attribute, the compiler
expects instances of this struct to be 2-byte aligned.

If a UFS device returns an odd value for next_group_offset in the payload,
hdr_off will be odd on the next iteration. Casting packet + hdr_off to
struct utp_agg_group_header * and dereferencing hdr->next_group_offset
would cause the compiler to emit an aligned 16-bit load instruction.

If the pointer is unaligned, this can lead to a hardware alignment exception
when dereferenced. Should this code validate the alignment, use
get_unaligned_be16(), or define struct utp_agg_group_header as __packed?

> +		u16 payload_off = hdr_off + QUERY_AGG_GROUP_HDR_SIZE;
> +
> +		if (next_off && (next_off < payload_off ||
> +				 next_off + QUERY_AGG_GROUP_HDR_SIZE > hba->agg_packet_len))
> +			return NULL;
> +
> +		if (hdr->group_type == type) {
> +			*group_len = (next_off ? next_off : hba->agg_packet_len) -
> +				     payload_off;
> +			return packet + payload_off;
> +		}
> +
> +		if (!next_off)
> +			break;
> +
> +		hdr_off = next_off;
> +	}
> +
> +	return NULL;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914083838epcms2p1336cbd08ce6ed192eeed9adbad365a93@epcms2p1?part=2