2022-12-01 14:42:27

by Bean Huo

[permalink] [raw]
Subject: [PATCH v3 0/7] UFS Advanced RPMB

Hi,

This series of changes is to add support for UFS advanced RPMB
in ufs_bsg. The advanced RPMB application of user space is ufs_utils, the reference code is
https://github.com/beanhuo/ufs-utils-Micron/blob/ufs_arpmb/ufs_arpmb.c.

Changes to ufs_utils will be pushed to https://github.com/westerndigitalcorporation/ufs-utils
When ARPMB patch is merged into the kernel.


Changelog:

V2 -- V3:
1. Add new patch 1/7
2. Addresses several coding issues raised by Avri Altman.
3. Add __attribute__((__packed__)) for EHS struct in 7/7

V1 -- V2:
1. Added RPMB request completion handling in patch 6/6

RFC -- V1:
1. Split the patch and Remove RFC
2. Add all 8 types of rpmb operations
3. Fix one EHS copy error in ufshcd_advanced_rpmb_req_handler()
4. Fix several issues raised by Avri in the RFC patch:
https://patchwork.kernel.org/project/linux-scsi/patch/[email protected]/#25081912

Bean Huo (7):
ufs: bsg: Let result in struct ufs_bsg_reply be signed int
ufs: ufs_bsg: Remove unnecessary length checkup
ufs: ufs_bsg: Cleanup ufs_bsg_request
ufs: core: Split ufshcd_map_sg
ufs: core: Advanced RPMB detection
ufs: core: Pass EHS length into ufshcd_prepare_req_desc_hdr()
ufs: core: Add advanced RPMB support in ufs_bsg

drivers/ufs/core/ufs_bsg.c | 137 +++++++++++++++---------
drivers/ufs/core/ufshcd.c | 178 +++++++++++++++++++++++++------
include/uapi/scsi/scsi_bsg_ufs.h | 48 ++++++++-
include/ufs/ufs.h | 29 +++++
include/ufs/ufshcd.h | 6 +-
include/ufs/ufshci.h | 1 +
6 files changed, 318 insertions(+), 81 deletions(-)

--
2.25.1


2022-12-01 14:42:39

by Bean Huo

[permalink] [raw]
Subject: [PATCH v3 4/7] ufs: core: Split ufshcd_map_sg

From: Bean Huo <[email protected]>

Take out the "map scatter-gather list to prdt" part of the code in
ufshcd_map_sg and split it into a new function ufshcd_sgl_to_prdt.

Signed-off-by: Bean Huo <[email protected]>
Reviewed-by: Avri Altman <[email protected]>
---
drivers/ufs/core/ufshcd.c | 48 +++++++++++++++++++++++----------------
1 file changed, 29 insertions(+), 19 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 2dbe24977822..d1bcb4c4e4e4 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -2399,38 +2399,30 @@ int ufshcd_send_uic_cmd(struct ufs_hba *hba, struct uic_command *uic_cmd)
}

/**
- * ufshcd_map_sg - Map scatter-gather list to prdt
- * @hba: per adapter instance
- * @lrbp: pointer to local reference block
- *
- * Returns 0 in case of success, non-zero value in case of failure
+ * ufshcd_sgl_to_prdt - SG list to PRTD (Physical Region Description Table, 4DW format)
+ * @hba: per-adapter instance
+ * @lrbp: pointer to local reference block
+ * @sg_entries: The number of sg lists actually used
+ * @sg_list: Pointer to SG list
*/
-static int ufshcd_map_sg(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
+static void ufshcd_sgl_to_prdt(struct ufs_hba *hba, struct ufshcd_lrb *lrbp, int sg_entries,
+ struct scatterlist *sg_list)
{
struct ufshcd_sg_entry *prd_table;
struct scatterlist *sg;
- struct scsi_cmnd *cmd;
- int sg_segments;
int i;

- cmd = lrbp->cmd;
- sg_segments = scsi_dma_map(cmd);
- if (sg_segments < 0)
- return sg_segments;
-
- if (sg_segments) {
+ if (sg_entries) {

if (hba->quirks & UFSHCD_QUIRK_PRDT_BYTE_GRAN)
lrbp->utr_descriptor_ptr->prd_table_length =
- cpu_to_le16((sg_segments *
- sizeof(struct ufshcd_sg_entry)));
+ cpu_to_le16((sg_entries * sizeof(struct ufshcd_sg_entry)));
else
- lrbp->utr_descriptor_ptr->prd_table_length =
- cpu_to_le16(sg_segments);
+ lrbp->utr_descriptor_ptr->prd_table_length = cpu_to_le16(sg_entries);

prd_table = lrbp->ucd_prdt_ptr;

- scsi_for_each_sg(cmd, sg, sg_segments, i) {
+ for_each_sg(sg_list, sg, sg_entries, i) {
const unsigned int len = sg_dma_len(sg);

/*
@@ -2449,6 +2441,24 @@ static int ufshcd_map_sg(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
} else {
lrbp->utr_descriptor_ptr->prd_table_length = 0;
}
+}
+
+/**
+ * ufshcd_map_sg - Map scatter-gather list to prdt
+ * @hba: per adapter instance
+ * @lrbp: pointer to local reference block
+ *
+ * Returns 0 in case of success, non-zero value in case of failure
+ */
+static int ufshcd_map_sg(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
+{
+ struct scsi_cmnd *cmd = lrbp->cmd;
+ int sg_segments = scsi_dma_map(cmd);
+
+ if (sg_segments < 0)
+ return sg_segments;
+
+ ufshcd_sgl_to_prdt(hba, lrbp, sg_segments, scsi_sglist(cmd));

return 0;
}
--
2.25.1

2022-12-01 14:47:29

by Bean Huo

[permalink] [raw]
Subject: [PATCH v3 2/7] ufs: ufs_bsg: Remove unnecessary length checkup

From: Bean Huo <[email protected]>

Remove checks on job->request_len and job->reply_len because
The following msgcode checks will rule out malicious requests.

Signed-off-by: Bean Huo <[email protected]>
Acked-by: Avri Altman <[email protected]>
---
drivers/ufs/core/ufs_bsg.c | 21 ---------------------
1 file changed, 21 deletions(-)

diff --git a/drivers/ufs/core/ufs_bsg.c b/drivers/ufs/core/ufs_bsg.c
index b99e3f3dc4ef..9ac8204f1ee6 100644
--- a/drivers/ufs/core/ufs_bsg.c
+++ b/drivers/ufs/core/ufs_bsg.c
@@ -30,21 +30,6 @@ static int ufs_bsg_get_query_desc_size(struct ufs_hba *hba, int *desc_len,
return 0;
}

-static int ufs_bsg_verify_query_size(struct ufs_hba *hba,
- unsigned int request_len,
- unsigned int reply_len)
-{
- int min_req_len = sizeof(struct ufs_bsg_request);
- int min_rsp_len = sizeof(struct ufs_bsg_reply);
-
- if (min_req_len > request_len || min_rsp_len > reply_len) {
- dev_err(hba->dev, "not enough space assigned\n");
- return -EINVAL;
- }
-
- return 0;
-}
-
static int ufs_bsg_alloc_desc_buffer(struct ufs_hba *hba, struct bsg_job *job,
uint8_t **desc_buff, int *desc_len,
enum query_opcode desc_op)
@@ -88,8 +73,6 @@ static int ufs_bsg_request(struct bsg_job *job)
struct ufs_bsg_request *bsg_request = job->request;
struct ufs_bsg_reply *bsg_reply = job->reply;
struct ufs_hba *hba = shost_priv(dev_to_shost(job->dev->parent));
- unsigned int req_len = job->request_len;
- unsigned int reply_len = job->reply_len;
struct uic_command uc = {};
int msgcode;
uint8_t *desc_buff = NULL;
@@ -97,10 +80,6 @@ static int ufs_bsg_request(struct bsg_job *job)
enum query_opcode desc_op = UPIU_QUERY_OPCODE_NOP;
int ret;

- ret = ufs_bsg_verify_query_size(hba, req_len, reply_len);
- if (ret)
- goto out;
-
bsg_reply->reply_payload_rcv_len = 0;

ufshcd_rpm_get_sync(hba);
--
2.25.1

2022-12-01 14:48:09

by Bean Huo

[permalink] [raw]
Subject: [PATCH v3 6/7] ufs: core: Pass EHS length into ufshcd_prepare_req_desc_hdr()

From: Bean Huo <[email protected]>

We need to fill in the total EHS length in UTP Transfer Request Descriptor,
add this functionality to ufshcd_prepare_req_desc_hdr().

Signed-off-by: Bean Huo <[email protected]>
Reviewed-by: Avri Altman <[email protected]>
---
drivers/ufs/core/ufshcd.c | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 641fed6dae5d..bacc94f87ee2 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -2506,14 +2506,15 @@ static void ufshcd_disable_intr(struct ufs_hba *hba, u32 intrs)
}

/**
- * ufshcd_prepare_req_desc_hdr() - Fills the requests header
+ * ufshcd_prepare_req_desc_hdr - Fill UTP Transfer request descriptor header according to request
* descriptor according to request
* @lrbp: pointer to local reference block
* @upiu_flags: flags required in the header
* @cmd_dir: requests data direction
+ * @ehs_length: Total EHS Length (in 32‐bytes units of all Extra Header Segments)
*/
-static void ufshcd_prepare_req_desc_hdr(struct ufshcd_lrb *lrbp,
- u8 *upiu_flags, enum dma_data_direction cmd_dir)
+static void ufshcd_prepare_req_desc_hdr(struct ufshcd_lrb *lrbp, u8 *upiu_flags,
+ enum dma_data_direction cmd_dir, int ehs_length)
{
struct utp_transfer_req_desc *req_desc = lrbp->utr_descriptor_ptr;
u32 data_direction;
@@ -2532,8 +2533,8 @@ static void ufshcd_prepare_req_desc_hdr(struct ufshcd_lrb *lrbp,
*upiu_flags = UPIU_CMD_FLAGS_NONE;
}

- dword_0 = data_direction | (lrbp->command_type
- << UPIU_COMMAND_TYPE_OFFSET);
+ dword_0 = data_direction | (lrbp->command_type << UPIU_COMMAND_TYPE_OFFSET) |
+ ehs_length << 8;
if (lrbp->intr_cmd)
dword_0 |= UTP_REQ_DESC_INT_CMD;

@@ -2588,8 +2589,7 @@ void ufshcd_prepare_utp_scsi_cmd_upiu(struct ufshcd_lrb *lrbp, u8 upiu_flags)
}

/**
- * ufshcd_prepare_utp_query_req_upiu() - fills the utp_transfer_req_desc,
- * for query requsts
+ * ufshcd_prepare_utp_query_req_upiu() - fill the utp_transfer_req_desc for query request
* @hba: UFS hba
* @lrbp: local reference block pointer
* @upiu_flags: flags
@@ -2660,7 +2660,7 @@ static int ufshcd_compose_devman_upiu(struct ufs_hba *hba,
else
lrbp->command_type = UTP_CMD_TYPE_UFS_STORAGE;

- ufshcd_prepare_req_desc_hdr(lrbp, &upiu_flags, DMA_NONE);
+ ufshcd_prepare_req_desc_hdr(lrbp, &upiu_flags, DMA_NONE, 0);
if (hba->dev_cmd.type == DEV_CMD_TYPE_QUERY)
ufshcd_prepare_utp_query_req_upiu(hba, lrbp, upiu_flags);
else if (hba->dev_cmd.type == DEV_CMD_TYPE_NOP)
@@ -2688,8 +2688,7 @@ static int ufshcd_comp_scsi_upiu(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
lrbp->command_type = UTP_CMD_TYPE_UFS_STORAGE;

if (likely(lrbp->cmd)) {
- ufshcd_prepare_req_desc_hdr(lrbp, &upiu_flags,
- lrbp->cmd->sc_data_direction);
+ ufshcd_prepare_req_desc_hdr(lrbp, &upiu_flags, lrbp->cmd->sc_data_direction, 0);
ufshcd_prepare_utp_scsi_cmd_upiu(lrbp, upiu_flags);
} else {
ret = -EINVAL;
@@ -6884,7 +6883,7 @@ static int ufshcd_issue_devman_upiu_cmd(struct ufs_hba *hba,
/* update the task tag in the request upiu */
req_upiu->header.dword_0 |= cpu_to_be32(tag);

- ufshcd_prepare_req_desc_hdr(lrbp, &upiu_flags, DMA_NONE);
+ ufshcd_prepare_req_desc_hdr(lrbp, &upiu_flags, DMA_NONE, 0);

/* just copy the upiu request as it is */
memcpy(lrbp->ucd_req_ptr, req_upiu, sizeof(*lrbp->ucd_req_ptr));
--
2.25.1

2022-12-01 14:50:23

by Bean Huo

[permalink] [raw]
Subject: [PATCH v3 1/7] ufs: bsg: Let result in struct ufs_bsg_reply be signed int

From: Bean Huo <[email protected]>

According to the comments in struct ufs_bsg_reply and its usage, the
result should be signed int, not __u32.

Signed-off-by: Bean Huo <[email protected]>
---
include/uapi/scsi/scsi_bsg_ufs.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/scsi/scsi_bsg_ufs.h b/include/uapi/scsi/scsi_bsg_ufs.h
index d55f2176dfd4..64b0cb33e549 100644
--- a/include/uapi/scsi/scsi_bsg_ufs.h
+++ b/include/uapi/scsi/scsi_bsg_ufs.h
@@ -95,7 +95,7 @@ struct ufs_bsg_reply {
* msg and status fields. The per-msgcode reply structure
* will contain valid data.
*/
- __u32 result;
+ int result;

/* If there was reply_payload, how much was received? */
__u32 reply_payload_rcv_len;
--
2.25.1

2022-12-01 14:50:48

by Bean Huo

[permalink] [raw]
Subject: [PATCH v3 5/7] ufs: core: Advanced RPMB detection

From: Bean Huo <[email protected]>

Check UFS Advanced RPMB LU enablement during ufshcd_lu_init().

Signed-off-by: Bean Huo <[email protected]>
---
drivers/ufs/core/ufshcd.c | 6 ++++++
include/ufs/ufs.h | 24 ++++++++++++++++++++++++
2 files changed, 30 insertions(+)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index d1bcb4c4e4e4..641fed6dae5d 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -4954,6 +4954,12 @@ static void ufshcd_lu_init(struct ufs_hba *hba, struct scsi_device *sdev)
desc_buf[UNIT_DESC_PARAM_LU_WR_PROTECT] == UFS_LU_POWER_ON_WP)
hba->dev_info.is_lu_power_on_wp = true;

+ /* In case of RPMB LU, check if advanced RPMB mode is enabled */
+ if (desc_buf[UNIT_DESC_PARAM_UNIT_INDEX] == UFS_UPIU_RPMB_WLUN &&
+ desc_buf[RPMB_UNIT_DESC_PARAM_REGION_EN] & BIT(4))
+ hba->dev_info.b_advanced_rpmb_en = true;
+
+
kfree(desc_buf);
set_qdepth:
/*
diff --git a/include/ufs/ufs.h b/include/ufs/ufs.h
index 1bba3fead2ce..17e401df674c 100644
--- a/include/ufs/ufs.h
+++ b/include/ufs/ufs.h
@@ -212,6 +212,28 @@ enum unit_desc_param {
UNIT_DESC_PARAM_WB_BUF_ALLOC_UNITS = 0x29,
};

+/* RPMB Unit descriptor parameters offsets in bytes*/
+enum rpmb_unit_desc_param {
+ RPMB_UNIT_DESC_PARAM_LEN = 0x0,
+ RPMB_UNIT_DESC_PARAM_TYPE = 0x1,
+ RPMB_UNIT_DESC_PARAM_UNIT_INDEX = 0x2,
+ RPMB_UNIT_DESC_PARAM_LU_ENABLE = 0x3,
+ RPMB_UNIT_DESC_PARAM_BOOT_LUN_ID = 0x4,
+ RPMB_UNIT_DESC_PARAM_LU_WR_PROTECT = 0x5,
+ RPMB_UNIT_DESC_PARAM_LU_Q_DEPTH = 0x6,
+ RPMB_UNIT_DESC_PARAM_PSA_SENSITIVE = 0x7,
+ RPMB_UNIT_DESC_PARAM_MEM_TYPE = 0x8,
+ RPMB_UNIT_DESC_PARAM_REGION_EN = 0x9,
+ RPMB_UNIT_DESC_PARAM_LOGICAL_BLK_SIZE = 0xA,
+ RPMB_UNIT_DESC_PARAM_LOGICAL_BLK_COUNT = 0xB,
+ RPMB_UNIT_DESC_PARAM_REGION0_SIZE = 0x13,
+ RPMB_UNIT_DESC_PARAM_REGION1_SIZE = 0x14,
+ RPMB_UNIT_DESC_PARAM_REGION2_SIZE = 0x15,
+ RPMB_UNIT_DESC_PARAM_REGION3_SIZE = 0x16,
+ RPMB_UNIT_DESC_PARAM_PROVISIONING_TYPE = 0x17,
+ RPMB_UNIT_DESC_PARAM_PHY_MEM_RSRC_CNT = 0x18,
+};
+
/* Device descriptor parameters offsets in bytes*/
enum device_desc_param {
DEVICE_DESC_PARAM_LEN = 0x0,
@@ -601,6 +623,8 @@ struct ufs_dev_info {

bool b_rpm_dev_flush_capable;
u8 b_presrv_uspc_en;
+
+ bool b_advanced_rpmb_en;
};

/*
--
2.25.1

2022-12-01 15:56:51

by Bean Huo

[permalink] [raw]
Subject: [PATCH v3 3/7] ufs: ufs_bsg: Cleanup ufs_bsg_request

From: Bean Huo <[email protected]>

Move sg_copy_from_buffer() below its associated case statement.

Signed-off-by: Bean Huo <[email protected]>
Reviewed-by: Avri Altman <[email protected]>
---
drivers/ufs/core/ufs_bsg.c | 27 ++++++++-------------------
1 file changed, 8 insertions(+), 19 deletions(-)

diff --git a/drivers/ufs/core/ufs_bsg.c b/drivers/ufs/core/ufs_bsg.c
index 9ac8204f1ee6..850a0d798f63 100644
--- a/drivers/ufs/core/ufs_bsg.c
+++ b/drivers/ufs/core/ufs_bsg.c
@@ -90,11 +90,8 @@ static int ufs_bsg_request(struct bsg_job *job)
desc_op = bsg_request->upiu_req.qr.opcode;
ret = ufs_bsg_alloc_desc_buffer(hba, job, &desc_buff,
&desc_len, desc_op);
- if (ret) {
- ufshcd_rpm_put_sync(hba);
+ if (ret)
goto out;
- }
-
fallthrough;
case UPIU_TRANSACTION_NOP_OUT:
case UPIU_TRANSACTION_TASK_REQ:
@@ -102,9 +99,12 @@ static int ufs_bsg_request(struct bsg_job *job)
&bsg_reply->upiu_rsp, msgcode,
desc_buff, &desc_len, desc_op);
if (ret)
- dev_err(hba->dev,
- "exe raw upiu: error code %d\n", ret);
-
+ dev_err(hba->dev, "exe raw upiu: error code %d\n", ret);
+ else if (desc_op == UPIU_QUERY_OPCODE_READ_DESC && desc_len)
+ bsg_reply->reply_payload_rcv_len =
+ sg_copy_from_buffer(job->request_payload.sg_list,
+ job->request_payload.sg_cnt,
+ desc_buff, desc_len);
break;
case UPIU_TRANSACTION_UIC_CMD:
memcpy(&uc, &bsg_request->upiu_req.uc, UIC_CMD_SIZE);
@@ -123,20 +123,9 @@ static int ufs_bsg_request(struct bsg_job *job)
break;
}

+out:
ufshcd_rpm_put_sync(hba);
-
- if (!desc_buff)
- goto out;
-
- if (desc_op == UPIU_QUERY_OPCODE_READ_DESC && desc_len)
- bsg_reply->reply_payload_rcv_len =
- sg_copy_from_buffer(job->request_payload.sg_list,
- job->request_payload.sg_cnt,
- desc_buff, desc_len);
-
kfree(desc_buff);
-
-out:
bsg_reply->result = ret;
job->reply_len = sizeof(struct ufs_bsg_reply);
/* complete the job here only if no error */
--
2.25.1

2022-12-30 21:16:54

by Martin K. Petersen

[permalink] [raw]
Subject: Re: [PATCH v3 0/7] UFS Advanced RPMB


Bean,

> This series of changes is to add support for UFS advanced RPMB in
> ufs_bsg. The advanced RPMB application of user space is ufs_utils, the
> reference code is
> https://github.com/beanhuo/ufs-utils-Micron/blob/ufs_arpmb/ufs_arpmb.c.
>
> Changes to ufs_utils will be pushed to
> https://github.com/westerndigitalcorporation/ufs-utils When ARPMB
> patch is merged into the kernel.

Applied to 6.3/scsi-staging, thanks!

--
Martin K. Petersen Oracle Linux Engineering