2022-11-20 22:25:19

by Bean Huo

[permalink] [raw]
Subject: [PATCH v2 0/6] UFS Advanced RPMB

Changelog:

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 (6):
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 | 176 +++++++++++++++++++++++++------
include/uapi/scsi/scsi_bsg_ufs.h | 46 +++++++-
include/ufs/ufs.h | 29 +++++
include/ufs/ufshcd.h | 6 +-
include/ufs/ufshci.h | 1 +
6 files changed, 315 insertions(+), 80 deletions(-)

--
2.25.1



2022-11-20 22:26:08

by Bean Huo

[permalink] [raw]
Subject: [PATCH v2 4/6] 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 1b252e6cf93f..311172578fd8 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -4956,6 +4956,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-11-20 22:26:43

by Bean Huo

[permalink] [raw]
Subject: [PATCH v2 2/6] 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]>
---
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-11-20 22:34:25

by Bean Huo

[permalink] [raw]
Subject: [PATCH v2 3/6] 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]>
---
drivers/ufs/core/ufshcd.c | 50 ++++++++++++++++++++++++---------------
1 file changed, 31 insertions(+), 19 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 768cb49d269c..1b252e6cf93f 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,26 @@ 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;
+ int sg_segments;
+
+ cmd = lrbp->cmd;
+ 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-11-20 22:49:45

by Bean Huo

[permalink] [raw]
Subject: [PATCH v2 6/6] ufs: core: Add advanced RPMB support in ufs_bsg

From: Bean Huo <[email protected]>

Add advanced RPMB support in ufs_bsg. For these reasons, we try to add
Advanced RPMB support in ufs_bsg:

1. According to the UFS specification, only one RPMB operation can be
performed at any time. We can ensure this by using reserved slot and
its dev_cmd sync operation protection mechanism.

2. For the Advanced RPMB, RPMB metadata is packaged in an EHS(Extra
Header Segment) of a command UPIU, and the corresponding reply EHS
(from the device) should also be returned to the user space.
bsg_job->request and bsg_job->reply allow us to pass and return EHS
from/back to userspace.

Compared to normal/legacy RPMB, the advantage of advanced RPMB are:

1. The data length in the Advanced RPBM data read/write command could
be > 4KB. For the legacy RPMB, the data length in a single RPMB data
transfer is 256 bytes.
2. All of the advanced RPMB operations will be a single command shot.
But for the legacy RPBM, take the read write-counter value as an example,
you need two commands(first SECURITY PROTOCOL OUT, then the second SECURITY
PROTOCOL IN).

Signed-off-by: Bean Huo <[email protected]>
---
drivers/ufs/core/ufs_bsg.c | 93 ++++++++++++++++++++++++++----
drivers/ufs/core/ufshcd.c | 99 ++++++++++++++++++++++++++++++++
include/uapi/scsi/scsi_bsg_ufs.h | 46 ++++++++++++++-
include/ufs/ufs.h | 5 ++
include/ufs/ufshcd.h | 6 +-
include/ufs/ufshci.h | 1 +
6 files changed, 238 insertions(+), 12 deletions(-)

diff --git a/drivers/ufs/core/ufs_bsg.c b/drivers/ufs/core/ufs_bsg.c
index 850a0d798f63..a8e58faa7da2 100644
--- a/drivers/ufs/core/ufs_bsg.c
+++ b/drivers/ufs/core/ufs_bsg.c
@@ -6,6 +6,7 @@
*/

#include <linux/bsg-lib.h>
+#include <linux/dma-mapping.h>
#include <scsi/scsi.h>
#include <scsi/scsi_host.h>
#include "ufs_bsg.h"
@@ -68,6 +69,72 @@ static int ufs_bsg_alloc_desc_buffer(struct ufs_hba *hba, struct bsg_job *job,
return 0;
}

+static int ufs_bsg_exec_advanced_rpmb_req(struct ufs_hba *hba, struct bsg_job *job)
+{
+ struct ufs_rpmb_request *rpmb_request = job->request;
+ struct ufs_rpmb_reply *rpmb_reply = job->reply;
+ struct bsg_buffer *payload = NULL;
+ enum dma_data_direction dir;
+ struct scatterlist *sg_list;
+ int rpmb_req_type;
+ int sg_cnt;
+ int ret;
+ int data_len;
+
+ if (hba->ufs_version < ufshci_version(4, 0) || !hba->dev_info.b_advanced_rpmb_en ||
+ !(hba->capabilities & MASK_EHSLUTRD_SUPPORTED))
+ return -EINVAL;
+
+ if (rpmb_request->ehs_req.length != 2 || rpmb_request->ehs_req.ehs_type != 1)
+ return -EINVAL;
+
+ rpmb_req_type = be16_to_cpu(rpmb_request->ehs_req.meta.req_resp_type);
+
+ switch (rpmb_req_type) {
+ case UFS_RPMB_WRITE_KEY:
+ case UFS_RPMB_READ_CNT:
+ case UFS_RPMB_PURGE_ENABLE:
+ dir = DMA_NONE;
+ break;
+ case UFS_RPMB_WRITE:
+ case UFS_RPMB_SEC_CONF_WRITE:
+ dir = DMA_TO_DEVICE;
+ break;
+ case UFS_RPMB_READ:
+ case UFS_RPMB_SEC_CONF_READ:
+ case UFS_RPMB_PURGE_STATUS_READ:
+ dir = DMA_FROM_DEVICE;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ if (dir != DMA_NONE) {
+ payload = &job->request_payload;
+ if (!payload || !payload->payload_len || !payload->sg_cnt)
+ return -EINVAL;
+
+ sg_cnt = dma_map_sg(hba->host->dma_dev, payload->sg_list, payload->sg_cnt, dir);
+ if (unlikely(!sg_cnt))
+ return -ENOMEM;
+ sg_list = payload->sg_list;
+ data_len = payload->payload_len;
+ }
+
+ ret = ufshcd_advanced_rpmb_req_handler(hba, &rpmb_request->bsg_request.upiu_req,
+ &rpmb_reply->bsg_reply.upiu_rsp, &rpmb_request->ehs_req,
+ &rpmb_reply->ehs_rsp, sg_cnt, sg_list, dir);
+
+ if (dir != DMA_NONE) {
+ dma_unmap_sg(hba->host->dma_dev, payload->sg_list, payload->sg_cnt, dir);
+
+ if (!ret)
+ rpmb_reply->bsg_reply.reply_payload_rcv_len = data_len;
+ }
+
+ return ret;
+}
+
static int ufs_bsg_request(struct bsg_job *job)
{
struct ufs_bsg_request *bsg_request = job->request;
@@ -75,10 +142,11 @@ 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 *desc_buff = NULL;
+ uint8_t *buff = NULL;
int desc_len = 0;
enum query_opcode desc_op = UPIU_QUERY_OPCODE_NOP;
int ret;
+ bool rpmb = false;

bsg_reply->reply_payload_rcv_len = 0;

@@ -88,8 +156,7 @@ static int ufs_bsg_request(struct bsg_job *job)
switch (msgcode) {
case UPIU_TRANSACTION_QUERY_REQ:
desc_op = bsg_request->upiu_req.qr.opcode;
- ret = ufs_bsg_alloc_desc_buffer(hba, job, &desc_buff,
- &desc_len, desc_op);
+ ret = ufs_bsg_alloc_desc_buffer(hba, job, &buff, &desc_len, desc_op);
if (ret)
goto out;
fallthrough;
@@ -97,25 +164,31 @@ static int ufs_bsg_request(struct bsg_job *job)
case UPIU_TRANSACTION_TASK_REQ:
ret = ufshcd_exec_raw_upiu_cmd(hba, &bsg_request->upiu_req,
&bsg_reply->upiu_rsp, msgcode,
- desc_buff, &desc_len, desc_op);
+ 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_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);
+ buff, desc_len);
+ }
break;
case UPIU_TRANSACTION_UIC_CMD:
memcpy(&uc, &bsg_request->upiu_req.uc, UIC_CMD_SIZE);
ret = ufshcd_send_uic_cmd(hba, &uc);
if (ret)
- dev_err(hba->dev,
- "send uic cmd: error code %d\n", ret);
+ dev_err(hba->dev, "send uic cmd: error code %d\n", ret);

memcpy(&bsg_reply->upiu_rsp.uc, &uc, UIC_CMD_SIZE);

break;
+ case UPIU_TRANSACTION_ARPMB_CMD:
+ rpmb = true;
+ ret = ufs_bsg_exec_advanced_rpmb_req(hba, job);
+ if (ret)
+ dev_err(hba->dev, "ARPMB OP failed: error code %d\n", ret);
+ break;
default:
ret = -ENOTSUPP;
dev_err(hba->dev, "unsupported msgcode 0x%x\n", msgcode);
@@ -125,9 +198,9 @@ static int ufs_bsg_request(struct bsg_job *job)

out:
ufshcd_rpm_put_sync(hba);
- kfree(desc_buff);
+ kfree(buff);
bsg_reply->result = ret;
- job->reply_len = sizeof(struct ufs_bsg_reply);
+ job->reply_len = !rpmb ? sizeof(struct ufs_bsg_reply) : sizeof(struct ufs_rpmb_reply);
/* complete the job here only if no error */
if (ret == 0)
bsg_job_done(job, ret, bsg_reply->reply_payload_rcv_len);
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 2936e1e583c3..863aa9dd28bb 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -56,6 +56,9 @@
/* Query request timeout */
#define QUERY_REQ_TIMEOUT 1500 /* 1.5 seconds */

+/* Advanced RPMB request timeout */
+#define ADVANCED_RPMB_REQ_TIMEOUT 3000 /* 3 seconds */
+
/* Task management command timeout */
#define TM_CMD_TIMEOUT 100 /* msecs */

@@ -2956,6 +2959,12 @@ ufshcd_dev_cmd_completion(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
dev_err(hba->dev, "%s: Reject UPIU not fully implemented\n",
__func__);
break;
+ case UPIU_TRANSACTION_RESPONSE:
+ if (hba->dev_cmd.type != DEV_CMD_TYPE_RPMB) {
+ err = -EINVAL;
+ dev_err(hba->dev, "%s: unexpected response %x\n", __func__, resp);
+ }
+ break;
default:
err = -EINVAL;
dev_err(hba->dev, "%s: Invalid device management cmd response: %x\n",
@@ -6984,6 +6993,96 @@ int ufshcd_exec_raw_upiu_cmd(struct ufs_hba *hba,
return err;
}

+/**
+ * ufshcd_advanced_rpmb_req_handler - handle advanced RPMB request
+ * @hba: per adapter instance
+ * @req_upiu: upiu request
+ * @rsp_upiu: upiu reply
+ * @req_ehs: EHS field which contains Advanced RPMB Request Message
+ * @rsp_ehs: EHS field which returns Advanced RPMB Response Message
+ * @sg_cnt: The number of sg lists actually used
+ * @sg_list: Pointer to SG list when DATA IN/OUT UPIU is required in ARPMB operation
+ * @dir: DMA direction
+ *
+ * Returns zero on success, non-zero on failure
+ */
+int ufshcd_advanced_rpmb_req_handler(struct ufs_hba *hba, struct utp_upiu_req *req_upiu,
+ struct utp_upiu_req *rsp_upiu, struct ufs_ehs *req_ehs,
+ struct ufs_ehs *rsp_ehs, int sg_cnt, struct scatterlist *sg_list,
+ enum dma_data_direction dir)
+{
+ DECLARE_COMPLETION_ONSTACK(wait);
+ const u32 tag = hba->reserved_slot;
+ struct ufshcd_lrb *lrbp;
+ int err = 0;
+ u8 upiu_flags;
+ u8 *ehs_data;
+ u16 ehs_len;
+
+ /* Protects use of hba->reserved_slot. */
+ ufshcd_hold(hba, false);
+ mutex_lock(&hba->dev_cmd.lock);
+ down_read(&hba->clk_scaling_lock);
+
+ lrbp = &hba->lrb[tag];
+ WARN_ON(lrbp->cmd);
+ lrbp->cmd = NULL;
+ lrbp->task_tag = tag;
+ lrbp->lun = UFS_UPIU_RPMB_WLUN;
+
+ lrbp->intr_cmd = true;
+ ufshcd_prepare_lrbp_crypto(NULL, lrbp);
+ hba->dev_cmd.type = DEV_CMD_TYPE_RPMB;
+
+ /* Advanced RPMB starts from UFS 4.0, so its command type is UTP_CMD_TYPE_UFS_STORAGE */
+ lrbp->command_type = UTP_CMD_TYPE_UFS_STORAGE;
+
+ ufshcd_prepare_req_desc_hdr(lrbp, &upiu_flags, dir, 2);
+
+ /* update the task tag and LUN in the request upiu */
+ req_upiu->header.dword_0 |= cpu_to_be32(upiu_flags << 16 | UFS_UPIU_RPMB_WLUN << 8 | tag);
+
+ /* copy the UPIU(contains CDB) request as it is */
+ memcpy(lrbp->ucd_req_ptr, req_upiu, sizeof(*lrbp->ucd_req_ptr));
+ /* Copy EHS, starting with byte32, immediately after the CDB package */
+ memcpy(lrbp->ucd_req_ptr + 1, req_ehs, sizeof(*req_ehs));
+
+ if (dir != DMA_NONE && sg_list)
+ ufshcd_sgl_to_prdt(hba, lrbp, sg_cnt, sg_list);
+
+ memset(lrbp->ucd_rsp_ptr, 0, sizeof(struct utp_upiu_rsp));
+
+ hba->dev_cmd.complete = &wait;
+
+ ufshcd_send_command(hba, tag);
+
+ err = ufshcd_wait_for_dev_cmd(hba, lrbp, ADVANCED_RPMB_REQ_TIMEOUT);
+
+ if (!err) {
+ /* just copy the upiu response as it is */
+ memcpy(rsp_upiu, lrbp->ucd_rsp_ptr, sizeof(*rsp_upiu));
+ ehs_len = be32_to_cpu(lrbp->ucd_rsp_ptr->header.dword_2) >> 24;
+ /*
+ * Since the bLength in EHS indicates the total size of the EHS Header and EHS Data
+ * in 32 Byte units, the value of the bLength Request/Response for Advanced RPMB
+ * 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 = (u8 *)lrbp->ucd_rsp_ptr + EHS_OFFSET_IN_RESPONSE;
+ memcpy(rsp_ehs, ehs_data, ehs_len * 32);
+ }
+ }
+
+ up_read(&hba->clk_scaling_lock);
+ mutex_unlock(&hba->dev_cmd.lock);
+ ufshcd_release(hba);
+ return err;
+}
+
/**
* ufshcd_eh_device_reset_handler() - Reset a single logical unit.
* @cmd: SCSI command pointer
diff --git a/include/uapi/scsi/scsi_bsg_ufs.h b/include/uapi/scsi/scsi_bsg_ufs.h
index d55f2176dfd4..1d605aaf5d6f 100644
--- a/include/uapi/scsi/scsi_bsg_ufs.h
+++ b/include/uapi/scsi/scsi_bsg_ufs.h
@@ -14,10 +14,27 @@
*/

#define UFS_CDB_SIZE 16
-#define UPIU_TRANSACTION_UIC_CMD 0x1F
/* uic commands are 4DW long, per UFSHCI V2.1 paragraph 5.6.1 */
#define UIC_CMD_SIZE (sizeof(__u32) * 4)

+enum ufs_bsg_msg_code {
+ UPIU_TRANSACTION_UIC_CMD = 0x1F,
+ UPIU_TRANSACTION_ARPMB_CMD,
+};
+
+/* UFS RPMB Request Message Types */
+enum ufs_rpmb_op_type {
+ UFS_RPMB_WRITE_KEY = 0x01,
+ UFS_RPMB_READ_CNT = 0x02,
+ UFS_RPMB_WRITE = 0x03,
+ UFS_RPMB_READ = 0x04,
+ UFS_RPMB_READ_RESP = 0x05,
+ UFS_RPMB_SEC_CONF_WRITE = 0x06,
+ UFS_RPMB_SEC_CONF_READ = 0x07,
+ UFS_RPMB_PURGE_ENABLE = 0x08,
+ UFS_RPMB_PURGE_STATUS_READ = 0x09,
+};
+
/**
* struct utp_upiu_header - UPIU header structure
* @dword_0: UPIU header DW-0
@@ -79,6 +96,23 @@ struct utp_upiu_req {
};
};

+struct ufs_arpmb_meta {
+ __u16 req_resp_type;
+ __u8 nonce[16];
+ __u32 write_counter;
+ __u16 addr_lun;
+ __u16 block_count;
+ __u16 result;
+};
+
+struct ufs_ehs {
+ __u8 length;
+ __u8 ehs_type;
+ __u16 ehssub_type;
+ struct ufs_arpmb_meta meta;
+ __u8 mac_key[32];
+};
+
/* request (CDB) structure of the sg_io_v4 */
struct ufs_bsg_request {
__u32 msgcode;
@@ -102,4 +136,14 @@ struct ufs_bsg_reply {

struct utp_upiu_req upiu_rsp;
};
+
+struct ufs_rpmb_request {
+ struct ufs_bsg_request bsg_request;
+ struct ufs_ehs ehs_req;
+};
+
+struct ufs_rpmb_reply {
+ struct ufs_bsg_reply bsg_reply;
+ struct ufs_ehs ehs_rsp;
+};
#endif /* UFS_BSG_H */
diff --git a/include/ufs/ufs.h b/include/ufs/ufs.h
index 17e401df674c..0c112195b288 100644
--- a/include/ufs/ufs.h
+++ b/include/ufs/ufs.h
@@ -49,6 +49,11 @@
*/
#define UFS_WB_EXCEED_LIFETIME 0x0B

+/*
+ * In UFS Spec, the Extra Header Segment (EHS) starts from byte 32 in UPIU request/response packet
+ */
+#define EHS_OFFSET_IN_RESPONSE 32
+
/* Well known logical unit id in LUN field of UPIU */
enum {
UFS_UPIU_REPORT_LUNS_WLUN = 0x81,
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index 5cf81dff60aa..c3dfa8084b5c 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -30,6 +30,7 @@ struct ufs_hba;
enum dev_cmd_type {
DEV_CMD_TYPE_NOP = 0x0,
DEV_CMD_TYPE_QUERY = 0x1,
+ DEV_CMD_TYPE_RPMB = 0x2,
};

enum ufs_event_type {
@@ -1201,7 +1202,10 @@ int ufshcd_exec_raw_upiu_cmd(struct ufs_hba *hba,
int msgcode,
u8 *desc_buff, int *buff_len,
enum query_opcode desc_op);
-
+int ufshcd_advanced_rpmb_req_handler(struct ufs_hba *hba, struct utp_upiu_req *req_upiu,
+ struct utp_upiu_req *rsp_upiu, struct ufs_ehs *ehs_req,
+ struct ufs_ehs *ehs_rsp, int sg_cnt,
+ struct scatterlist *sg_list, enum dma_data_direction dir);
int ufshcd_wb_toggle(struct ufs_hba *hba, bool enable);
int ufshcd_wb_toggle_buf_flush(struct ufs_hba *hba, bool enable);
int ufshcd_suspend_prepare(struct device *dev);
diff --git a/include/ufs/ufshci.h b/include/ufs/ufshci.h
index f525566a0864..af216296b86e 100644
--- a/include/ufs/ufshci.h
+++ b/include/ufs/ufshci.h
@@ -63,6 +63,7 @@ enum {
enum {
MASK_TRANSFER_REQUESTS_SLOTS = 0x0000001F,
MASK_TASK_MANAGEMENT_REQUEST_SLOTS = 0x00070000,
+ MASK_EHSLUTRD_SUPPORTED = 0x00400000,
MASK_AUTO_HIBERN8_SUPPORT = 0x00800000,
MASK_64_ADDRESSING_SUPPORT = 0x01000000,
MASK_OUT_OF_ORDER_DATA_DELIVERY_SUPPORT = 0x02000000,
--
2.25.1


2022-11-22 08:54:15

by Avri Altman

[permalink] [raw]
Subject: RE: [PATCH v2 2/6] 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-11-22 09:16:28

by Avri Altman

[permalink] [raw]
Subject: RE: [PATCH v2 3/6] 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]>
A nit below.

Reviewed-by: Avri Altman <[email protected]>

> ---
> drivers/ufs/core/ufshcd.c | 50 ++++++++++++++++++++++++---------------
> 1 file changed, 31 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index
> 768cb49d269c..1b252e6cf93f 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,26 @@ 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;
> + int sg_segments;
> +
> + cmd = lrbp->cmd;
> + sg_segments = scsi_dma_map(cmd);
Maybe initialize in declaration?

> + if (sg_segments < 0)
> + return sg_segments;
> +
> + ufshcd_sgl_to_prdt(hba, lrbp, sg_segments, scsi_sglist(cmd));
>
> return 0;
> }
> --
> 2.25.1

2022-11-22 09:32:26

by Avri Altman

[permalink] [raw]
Subject: RE: [PATCH v2 4/6] 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
> 1b252e6cf93f..311172578fd8 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -4956,6 +4956,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))
If instead you use bit 10 in dExtendedUFSFeaturesSupport,
You wouldn't need to add the rpmb unit descriptor, just the appropriate bit.

Thanks,
Avri

2022-11-22 11:38:23

by Avri Altman

[permalink] [raw]
Subject: RE: [PATCH v2 6/6] ufs: core: Add advanced RPMB support in ufs_bsg

> + sg_cnt = dma_map_sg(hba->host->dma_dev, payload->sg_list,
> payload->sg_cnt, dir);
> + if (unlikely(!sg_cnt))
> + return -ENOMEM;
> + sg_list = payload->sg_list;
> + data_len = payload->payload_len;
> + }
Isn't bsg_map_buffer does that for you already?
For both request & reply?

Thanks,
Avri

2022-11-22 12:11:55

by Avri Altman

[permalink] [raw]
Subject: RE: [PATCH v2 6/6] ufs: core: Add advanced RPMB support in ufs_bsg

> +static int ufs_bsg_exec_advanced_rpmb_req(struct ufs_hba *hba, struct
> +bsg_job *job) {
> + struct ufs_rpmb_request *rpmb_request = job->request;
> + struct ufs_rpmb_reply *rpmb_reply = job->reply;
> + struct bsg_buffer *payload = NULL;
> + enum dma_data_direction dir;
> + struct scatterlist *sg_list;
> + int rpmb_req_type;
> + int sg_cnt;
> + int ret;
> + int data_len;
> +
> + if (hba->ufs_version < ufshci_version(4, 0) || !hba-
> >dev_info.b_advanced_rpmb_en ||
> + !(hba->capabilities & MASK_EHSLUTRD_SUPPORTED))
> + return -EINVAL;
> +
> + if (rpmb_request->ehs_req.length != 2 || rpmb_request-
> >ehs_req.ehs_type != 1)
> + return -EINVAL;
Maybe you could also check:
In case of rpmb write, the request payload 4096 ? Advanced RPMB Block Count,
And same goes for response payload for rpmb read.

Thanks,
Avri

2022-11-22 13:07:47

by Avri Altman

[permalink] [raw]
Subject: RE: [PATCH v2 6/6] ufs: core: Add advanced RPMB support in ufs_bsg

>
> > + sg_cnt = dma_map_sg(hba->host->dma_dev, payload->sg_list,
> > payload->sg_cnt, dir);
> > + if (unlikely(!sg_cnt))
> > + return -ENOMEM;
> > + sg_list = payload->sg_list;
> > + data_len = payload->payload_len;
> > + }
> Isn't bsg_map_buffer does that for you already?
> For both request & reply?
Answering my own question - no it doesn't...

Thanks,
Avri

>
> Thanks,
> Avri

2022-11-24 15:18:33

by Bean Huo

[permalink] [raw]
Subject: Re: [PATCH v2 3/6] ufs: core: Split ufshcd_map_sg

On Tue, 2022-11-22 at 08:15 +0000, Avri Altman wrote:
> > +
> > + cmd = lrbp->cmd;
> > + sg_segments = scsi_dma_map(cmd);
>
> Maybe initialize in declaration?

yes, agree, will change it in the next version

Kind regards,
Bean

2022-11-28 19:28:57

by Bean Huo

[permalink] [raw]
Subject: Re: [PATCH v2 6/6] ufs: core: Add advanced RPMB support in ufs_bsg

On Tue, 2022-11-22 at 11:55 +0000, Avri Altman wrote:
> > +static int ufs_bsg_exec_advanced_rpmb_req(struct ufs_hba *hba,
> > struct
> > +bsg_job *job) {
> > + struct ufs_rpmb_request *rpmb_request = job->request;
> > + struct ufs_rpmb_reply *rpmb_reply = job->reply;
> > + struct bsg_buffer *payload = NULL;
> > + enum dma_data_direction dir;
> > + struct scatterlist *sg_list;
> > + int rpmb_req_type;
> > + int sg_cnt;
> > + int ret;
> > + int data_len;
> > +
> > + if (hba->ufs_version < ufshci_version(4, 0) || !hba-
> > > dev_info.b_advanced_rpmb_en ||
> > + !(hba->capabilities & MASK_EHSLUTRD_SUPPORTED))
> > + return -EINVAL;
> > +
> > + if (rpmb_request->ehs_req.length != 2 || rpmb_request-
> > > ehs_req.ehs_type != 1)
> > + return -EINVAL;
> Maybe you could also check:
> In case of rpmb write, the request payload 4096 × Advanced RPMB Block
> Count,
> And same goes for response payload for rpmb read.
>
> Thanks,
> Avri
>

Hi Avri,

in Spec:

"If the Block Count indicates a value greater than bRPMB_ReadWriteSize,
then the authenticated data write/read operation fails and the Result
is set to “General failure” (0001h)."


I think this should be checked in the application in userspace because
if the application passes a wrong/incorrect payload length, it will
error out and have no effect on UFS. In order to add this check in a
driver in the kernel, we need to maintain bRPMB_ReadWriteSize in kernel
space. Sometimes this is a waste of resources because we don't know if
the client will access the RPMB.

I have enabled Advanced RPMB feature in the ufs-utils as an example,
will be refered in cover-letter in the next version.

Kind regards,
Bean

2022-11-28 20:20:02

by Avri Altman

[permalink] [raw]
Subject: RE: [PATCH v2 6/6] ufs: core: Add advanced RPMB support in ufs_bsg

> On Tue, 2022-11-22 at 11:55 +0000, Avri Altman wrote:
> > > +static int ufs_bsg_exec_advanced_rpmb_req(struct ufs_hba *hba,
> > > struct
> > > +bsg_job *job) {
> > > + struct ufs_rpmb_request *rpmb_request = job->request;
> > > + struct ufs_rpmb_reply *rpmb_reply = job->reply;
> > > + struct bsg_buffer *payload = NULL;
> > > + enum dma_data_direction dir;
> > > + struct scatterlist *sg_list;
> > > + int rpmb_req_type;
> > > + int sg_cnt;
> > > + int ret;
> > > + int data_len;
> > > +
> > > + if (hba->ufs_version < ufshci_version(4, 0) || !hba-
> > > > dev_info.b_advanced_rpmb_en ||
> > > + !(hba->capabilities & MASK_EHSLUTRD_SUPPORTED))
> > > + return -EINVAL;
> > > +
> > > + if (rpmb_request->ehs_req.length != 2 || rpmb_request-
> > > > ehs_req.ehs_type != 1)
> > > + return -EINVAL;
> > Maybe you could also check:
> > In case of rpmb write, the request payload 4096 × Advanced RPMB Block
> > Count, And same goes for response payload for rpmb read.
> >
> > Thanks,
> > Avri
> >
>
> Hi Avri,
>
> in Spec:
>
> "If the Block Count indicates a value greater than bRPMB_ReadWriteSize,
> then the authenticated data write/read operation fails and the Result is set
> to “General failure” (0001h)."
>
>
> I think this should be checked in the application in userspace because if the
> application passes a wrong/incorrect payload length, it will error out and
> have no effect on UFS. In order to add this check in a driver in the kernel, we
> need to maintain bRPMB_ReadWriteSize in kernel space. Sometimes this is a
> waste of resources because we don't know if the client will access the RPMB.
Fair enough.
Please add my reviewed-by tag to this patch as well.

Thanks,
Avri
>
> I have enabled Advanced RPMB feature in the ufs-utils as an example, will be
> refered in cover-letter in the next version.
>
> Kind regards,
> Bean