On Thu, Mar 13, 2025 at 10:13 AM Sumit Garg <sumit.garg@xxxxxxxxxx> wrote: > > + Jens > > Hi Stefano, > > On Tue, Mar 11, 2025 at 11:01:29AM +0100, Stefano Garzarella wrote: > > This driver does not support interrupts, and receiving the response is > > synchronous with sending the command. > > > > It used an internal buffer to cache the response when .send() is called, > > and then return it when .recv() is called. > > > > Let's simplify the driver by implementing the new send_recv() op, so that > > we can also remove the 4KB internal buffer used to cache the response. > > Looks like a nice cleanup to me but it needs to be tested. Jens, can you > give this patch a try? > > > > > Signed-off-by: Stefano Garzarella <sgarzare@xxxxxxxxxx> > > --- > > Note: I don't know how to test this driver, so I just build it. > > If someone can test it, or tell me how to do, it will be great! Tested-by: Jens Wiklander <jens.wiklander@xxxxxxxxxx> Cheers, Jens > > The fTPM is now maintained as part of OP-TEE project here [1]. The > instructions to test it on Qemu can be found here [2] as part of CI > pipeline. > > [1] https://github.com/OP-TEE/optee_ftpm > [2] https://github.com/OP-TEE/optee_ftpm/blob/master/.github/workflows/ci.yml > > -Sumit > > > --- > > drivers/char/tpm/tpm_ftpm_tee.h | 4 -- > > drivers/char/tpm/tpm_ftpm_tee.c | 86 ++++++++------------------------- > > 2 files changed, 21 insertions(+), 69 deletions(-) > > > > diff --git a/drivers/char/tpm/tpm_ftpm_tee.h b/drivers/char/tpm/tpm_ftpm_tee.h > > index f98daa7bf68c..72b2f5c41274 100644 > > --- a/drivers/char/tpm/tpm_ftpm_tee.h > > +++ b/drivers/char/tpm/tpm_ftpm_tee.h > > @@ -23,16 +23,12 @@ > > * @chip: struct tpm_chip instance registered with tpm framework. > > * @state: internal state > > * @session: fTPM TA session identifier. > > - * @resp_len: cached response buffer length. > > - * @resp_buf: cached response buffer. > > * @ctx: TEE context handler. > > * @shm: Memory pool shared with fTPM TA in TEE. > > */ > > struct ftpm_tee_private { > > struct tpm_chip *chip; > > u32 session; > > - size_t resp_len; > > - u8 resp_buf[MAX_RESPONSE_SIZE]; > > struct tee_context *ctx; > > struct tee_shm *shm; > > }; > > diff --git a/drivers/char/tpm/tpm_ftpm_tee.c b/drivers/char/tpm/tpm_ftpm_tee.c > > index 139556b21cc6..f0393d843780 100644 > > --- a/drivers/char/tpm/tpm_ftpm_tee.c > > +++ b/drivers/char/tpm/tpm_ftpm_tee.c > > @@ -31,45 +31,19 @@ static const uuid_t ftpm_ta_uuid = > > 0x82, 0xCB, 0x34, 0x3F, 0xB7, 0xF3, 0x78, 0x96); > > > > /** > > - * ftpm_tee_tpm_op_recv() - retrieve fTPM response. > > - * @chip: the tpm_chip description as specified in driver/char/tpm/tpm.h. > > - * @buf: the buffer to store data. > > - * @count: the number of bytes to read. > > - * > > - * Return: > > - * In case of success the number of bytes received. > > - * On failure, -errno. > > - */ > > -static int ftpm_tee_tpm_op_recv(struct tpm_chip *chip, u8 *buf, size_t count) > > -{ > > - struct ftpm_tee_private *pvt_data = dev_get_drvdata(chip->dev.parent); > > - size_t len; > > - > > - len = pvt_data->resp_len; > > - if (count < len) { > > - dev_err(&chip->dev, > > - "%s: Invalid size in recv: count=%zd, resp_len=%zd\n", > > - __func__, count, len); > > - return -EIO; > > - } > > - > > - memcpy(buf, pvt_data->resp_buf, len); > > - pvt_data->resp_len = 0; > > - > > - return len; > > -} > > - > > -/** > > - * ftpm_tee_tpm_op_send() - send TPM commands through the TEE shared memory. > > + * ftpm_tee_tpm_op_send_recv() - send TPM commands through the TEE shared memory > > + * and retrieve the response. > > * @chip: the tpm_chip description as specified in driver/char/tpm/tpm.h > > - * @buf: the buffer to send. > > - * @len: the number of bytes to send. > > + * @buf: the buffer to send and to store the response. > > + * @buf_len: the size of the buffer. > > + * @cmd_len: the number of bytes to send. > > * > > * Return: > > - * In case of success, returns 0. > > + * In case of success, returns the number of bytes received. > > * On failure, -errno > > */ > > -static int ftpm_tee_tpm_op_send(struct tpm_chip *chip, u8 *buf, size_t len) > > +static int ftpm_tee_tpm_op_send_recv(struct tpm_chip *chip, u8 *buf, > > + size_t buf_len, size_t cmd_len) > > { > > struct ftpm_tee_private *pvt_data = dev_get_drvdata(chip->dev.parent); > > size_t resp_len; > > @@ -80,16 +54,15 @@ static int ftpm_tee_tpm_op_send(struct tpm_chip *chip, u8 *buf, size_t len) > > struct tee_param command_params[4]; > > struct tee_shm *shm = pvt_data->shm; > > > > - if (len > MAX_COMMAND_SIZE) { > > + if (cmd_len > MAX_COMMAND_SIZE) { > > dev_err(&chip->dev, > > "%s: len=%zd exceeds MAX_COMMAND_SIZE supported by fTPM TA\n", > > - __func__, len); > > + __func__, cmd_len); > > return -EIO; > > } > > > > memset(&transceive_args, 0, sizeof(transceive_args)); > > memset(command_params, 0, sizeof(command_params)); > > - pvt_data->resp_len = 0; > > > > /* Invoke FTPM_OPTEE_TA_SUBMIT_COMMAND function of fTPM TA */ > > transceive_args = (struct tee_ioctl_invoke_arg) { > > @@ -103,7 +76,7 @@ static int ftpm_tee_tpm_op_send(struct tpm_chip *chip, u8 *buf, size_t len) > > .attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT, > > .u.memref = { > > .shm = shm, > > - .size = len, > > + .size = cmd_len, > > .shm_offs = 0, > > }, > > }; > > @@ -115,7 +88,7 @@ static int ftpm_tee_tpm_op_send(struct tpm_chip *chip, u8 *buf, size_t len) > > return PTR_ERR(temp_buf); > > } > > memset(temp_buf, 0, (MAX_COMMAND_SIZE + MAX_RESPONSE_SIZE)); > > - memcpy(temp_buf, buf, len); > > + memcpy(temp_buf, buf, cmd_len); > > > > command_params[1] = (struct tee_param) { > > .attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT, > > @@ -156,38 +129,21 @@ static int ftpm_tee_tpm_op_send(struct tpm_chip *chip, u8 *buf, size_t len) > > __func__, resp_len); > > return -EIO; > > } > > + if (resp_len > buf_len) { > > + dev_err(&chip->dev, > > + "%s: Invalid size in recv: buf_len=%zd, resp_len=%zd\n", > > + __func__, buf_len, resp_len); > > + return -EIO; > > + } > > > > - /* sanity checks look good, cache the response */ > > - memcpy(pvt_data->resp_buf, temp_buf, resp_len); > > - pvt_data->resp_len = resp_len; > > - > > - return 0; > > -} > > - > > -static void ftpm_tee_tpm_op_cancel(struct tpm_chip *chip) > > -{ > > - /* not supported */ > > -} > > - > > -static u8 ftpm_tee_tpm_op_status(struct tpm_chip *chip) > > -{ > > - return 0; > > -} > > + memcpy(buf, temp_buf, resp_len); > > > > -static bool ftpm_tee_tpm_req_canceled(struct tpm_chip *chip, u8 status) > > -{ > > - return false; > > + return resp_len; > > } > > > > static const struct tpm_class_ops ftpm_tee_tpm_ops = { > > .flags = TPM_OPS_AUTO_STARTUP, > > - .recv = ftpm_tee_tpm_op_recv, > > - .send = ftpm_tee_tpm_op_send, > > - .cancel = ftpm_tee_tpm_op_cancel, > > - .status = ftpm_tee_tpm_op_status, > > - .req_complete_mask = 0, > > - .req_complete_val = 0, > > - .req_canceled = ftpm_tee_tpm_req_canceled, > > + .send_recv = ftpm_tee_tpm_op_send_recv, > > }; > > > > /* > > -- > > 2.48.1 > > > >