[RFC PATCH 0/3] tpm: add send_recv() op and use it in tpm_ftpm_tee and tpm_svsm drivers

[RFC PATCH 0/3] tpm: add send_recv() op and use it in tpm_ftpm_tee and tpm_svsm drivers

[Stefano Garzarella]

This series is a follow-up to the discussion we had about whether or not
to add send_recv() op in tpm_class_ops[1].

Some devices do not support interrupts and provide a single operation
to send the command and receive the response on the same buffer.
In order to simplify these drivers and avoid temporary buffers to be
used between the .send() and .recv() callbacks, introduce a new callback
send_recv(). This was suggested by Jason Gunthorpe while reviewing
the new SVSM vTPM driver, but the same callback can be used also for the
fTPM driver to simplify it a bit (second patch of this series).
I only successfully compiled fTPM, but I don't know how to test, if anyone
can test or suggest how to do it, I would be grateful.

This series is based on "[PATCH v3 0/4] Enlightened vTPM support for SVSM
on SEV-SNP" [2] (actually only the last patch in this series).

[1] https://lore.kernel.org/linux-integrity/Z8sfiDEhsG6RATiQ@kernel.org/
[2] https://lore.kernel.org/linux-integrity/20250311094225.35129-1-sgarzare@redhat.com/

Stefano Garzarella (3):
  tpm: add send_recv() op in tpm_class_ops
  tpm/tpm_ftpm_tee: use send_recv() op
  tpm/tpm_svsm: use send_recv() op

 drivers/char/tpm/tpm_ftpm_tee.h  |  4 --
 include/linux/tpm.h              |  2 +
 drivers/char/tpm/tpm-interface.c |  7 +++
 drivers/char/tpm/tpm_ftpm_tee.c  | 86 ++++++++------------------------
 drivers/char/tpm/tpm_svsm.c      | 46 ++++-------------
 5 files changed, 39 insertions(+), 106 deletions(-)


base-commit: f2cb83b7f8c58952b424a83bf86680f1263ad417
-- 
2.48.1

[RFC PATCH 1/3] tpm: add send_recv() op in tpm_class_ops

[Stefano Garzarella]

Some devices do not support interrupts and provide a single operation
to send the command and receive the response on the same buffer.

To support this scenario, a driver could set TPM_CHIP_FLAG_IRQ in the
chip's flags to get recv() to be called immediately after send() in
tpm_try_transmit(), or it needs to implement .status() to return 0,
and set both .req_complete_mask and .req_complete_val to 0.

In order to simplify these drivers and avoid temporary buffers to be
used between the .send() and .recv() callbacks, introduce a new callback
send_recv(). If that callback is defined, it is called in
tpm_try_transmit() to send the command and receive the response on
the same buffer in a single call.

Suggested-by: Jason Gunthorpe <jgg@ziepe.ca>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 include/linux/tpm.h              | 2 ++
 drivers/char/tpm/tpm-interface.c | 7 +++++++
 2 files changed, 9 insertions(+)

diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 20a40ade8030..9baf10240a3d 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -88,6 +88,8 @@ struct tpm_class_ops {
 	bool (*req_canceled)(struct tpm_chip *chip, u8 status);
 	int (*recv) (struct tpm_chip *chip, u8 *buf, size_t len);
 	int (*send) (struct tpm_chip *chip, u8 *buf, size_t len);
+	int (*send_recv)(struct tpm_chip *chip, u8 *buf, size_t buf_len,
+			 size_t cmd_len);
 	void (*cancel) (struct tpm_chip *chip);
 	u8 (*status) (struct tpm_chip *chip);
 	void (*update_timeouts)(struct tpm_chip *chip,
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index b1daa0d7b341..9a6e4b320a8f 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -82,6 +82,12 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip, void *buf, size_t bufsiz)
 		return -E2BIG;
 	}
 
+	if (chip->ops->send_recv) {
+		rc = 0;
+		len = chip->ops->send_recv(chip, buf, bufsiz, count);
+		goto out_send_recv;
+	}
+
 	rc = chip->ops->send(chip, buf, count);
 	if (rc < 0) {
 		if (rc != -EPIPE)
@@ -124,6 +130,7 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip, void *buf, size_t bufsiz)
 
 out_recv:
 	len = chip->ops->recv(chip, buf, bufsiz);
+out_send_recv:
 	if (len < 0) {
 		rc = len;
 		dev_err(&chip->dev, "tpm_transmit: tpm_recv: error %d\n", rc);
-- 
2.48.1

[RFC PATCH 2/3] tpm/tpm_ftpm_tee: use send_recv() op

[Stefano Garzarella]

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.

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
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!
---
 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

[RFC PATCH 3/3] tpm/tpm_svsm: use send_recv() op

[Stefano Garzarella]

This driver does not support interrupts, and receiving the response is
synchronous with sending the command.

Let's simplify the driver by implementing the new send_recv() op.

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
Note: this is based on "[PATCH v3 0/4] Enlightened vTPM support for SVSM
on SEV-SNP" series [1].
If we will merge this series before it, we can just ignore this patch
and I'll squash these changes in that series.

[1] https://lore.kernel.org/linux-integrity/20250311094225.35129-1-sgarzare@redhat.com/
---
 drivers/char/tpm/tpm_svsm.c | 46 ++++++++-----------------------------
 1 file changed, 9 insertions(+), 37 deletions(-)

diff --git a/drivers/char/tpm/tpm_svsm.c b/drivers/char/tpm/tpm_svsm.c
index 5540d0227eed..63208313f86e 100644
--- a/drivers/char/tpm/tpm_svsm.c
+++ b/drivers/char/tpm/tpm_svsm.c
@@ -25,60 +25,32 @@ struct tpm_svsm_priv {
 	u8 locality;
 };
 
-static int tpm_svsm_send(struct tpm_chip *chip, u8 *buf, size_t len)
+static int tpm_svsm_send_recv(struct tpm_chip *chip, u8 *buf, size_t buf_len,
+			      size_t cmd_len)
 {
 	struct tpm_svsm_priv *priv = dev_get_drvdata(&chip->dev);
 	int ret;
 
 	ret = svsm_vtpm_fill_cmd_req((struct tpm_send_cmd_req *)priv->buffer,
-				     priv->locality, buf, len);
+				     priv->locality, buf, cmd_len);
 	if (ret)
 		return ret;
 
 	/*
 	 * The SVSM call uses the same buffer for the command and for the
-	 * response, so after this call, the buffer will contain the response
-	 * that can be used by .recv() op.
+	 * response, so after this call, the buffer will contain the response.
 	 */
-	return snp_svsm_vtpm_send_command(priv->buffer);
-}
-
-static int tpm_svsm_recv(struct tpm_chip *chip, u8 *buf, size_t len)
-{
-	struct tpm_svsm_priv *priv = dev_get_drvdata(&chip->dev);
+	ret = snp_svsm_vtpm_send_command(priv->buffer);
+	if (ret)
+		return ret;
 
-	/*
-	 * The internal buffer contains the response after we send the command
-	 * to SVSM.
-	 */
 	return svsm_vtpm_parse_cmd_resp((struct tpm_send_cmd_resp *)priv->buffer,
-					buf, len);
-}
-
-static void tpm_svsm_cancel(struct tpm_chip *chip)
-{
-	/* not supported */
-}
-
-static u8 tpm_svsm_status(struct tpm_chip *chip)
-{
-	return 0;
-}
-
-static bool tpm_svsm_req_canceled(struct tpm_chip *chip, u8 status)
-{
-	return false;
+					buf, buf_len);
 }
 
 static struct tpm_class_ops tpm_chip_ops = {
 	.flags = TPM_OPS_AUTO_STARTUP,
-	.recv = tpm_svsm_recv,
-	.send = tpm_svsm_send,
-	.cancel = tpm_svsm_cancel,
-	.status = tpm_svsm_status,
-	.req_complete_mask = 0,
-	.req_complete_val = 0,
-	.req_canceled = tpm_svsm_req_canceled,
+	.send_recv = tpm_svsm_send_recv,
 };
 
 static int __init tpm_svsm_probe(struct platform_device *pdev)
-- 
2.48.1

Re: [RFC PATCH 2/3] tpm/tpm_ftpm_tee: use send_recv() op

[Sumit Garg]

+ 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@redhat.com>
> ---
> 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!

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
> 
> 

Re: [RFC PATCH 2/3] tpm/tpm_ftpm_tee: use send_recv() op

[Jens Wiklander]

On Thu, Mar 13, 2025 at 10:13 AM Sumit Garg <sumit.garg@kernel.org> 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@redhat.com>
> > ---
> > 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@linaro.org>

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
> >
> >

Re: [RFC PATCH 2/3] tpm/tpm_ftpm_tee: use send_recv() op

[Stefano Garzarella]

Hi Sumit, Jens,

On Thu, Mar 13, 2025 at 01:59:19PM +0100, Jens Wiklander wrote:
>On Thu, Mar 13, 2025 at 10:13 AM Sumit Garg <sumit.garg@kernel.org> 
>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@redhat.com>
>> > ---
>> > 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@linaro.org>
>

Thanks for testing this!

>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
>>

Thanks for the links, I'll take a look!

Stefano

Re: [RFC PATCH 3/3] tpm/tpm_svsm: use send_recv() op

[Jason Gunthorpe]

On Tue, Mar 11, 2025 at 11:01:30AM +0100, Stefano Garzarella wrote:
> This driver does not support interrupts, and receiving the response is
> synchronous with sending the command.
> 
> Let's simplify the driver by implementing the new send_recv() op.
> 
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
> Note: this is based on "[PATCH v3 0/4] Enlightened vTPM support for SVSM
> on SEV-SNP" series [1].
> If we will merge this series before it, we can just ignore this patch
> and I'll squash these changes in that series.
> 
> [1] https://lore.kernel.org/linux-integrity/20250311094225.35129-1-sgarzare@redhat.com/
> ---
>  drivers/char/tpm/tpm_svsm.c | 46 ++++++++-----------------------------
>  1 file changed, 9 insertions(+), 37 deletions(-)

I think the diffstat speaks for itself, you should send this as
non-RFC

Jason

Re: [RFC PATCH 3/3] tpm/tpm_svsm: use send_recv() op

[Stefano Garzarella]

On Wed, Mar 19, 2025 at 04:58:18PM -0300, Jason Gunthorpe wrote:
>On Tue, Mar 11, 2025 at 11:01:30AM +0100, Stefano Garzarella wrote:
>> This driver does not support interrupts, and receiving the response is
>> synchronous with sending the command.
>>
>> Let's simplify the driver by implementing the new send_recv() op.
>>
>> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>> ---
>> Note: this is based on "[PATCH v3 0/4] Enlightened vTPM support for SVSM
>> on SEV-SNP" series [1].
>> If we will merge this series before it, we can just ignore this patch
>> and I'll squash these changes in that series.
>>
>> [1] https://lore.kernel.org/linux-integrity/20250311094225.35129-1-sgarzare@redhat.com/
>> ---
>>  drivers/char/tpm/tpm_svsm.c | 46 ++++++++-----------------------------
>>  1 file changed, 9 insertions(+), 37 deletions(-)
>
>I think the diffstat speaks for itself, you should send this as
>non-RFC

Ack, I'll re-send it.

Thanks,
Stefano