diff mbox series

[v2,2/4] tpm: support devices with synchronous send()

Message ID 20250408083208.43512-3-sgarzare@redhat.com (mailing list archive)
State New
Headers show
Series tpm: add support for sync send() and use it in ftpm and svsm drivers | expand

Commit Message

Stefano Garzarella April 8, 2025, 8:32 a.m. UTC
From: Stefano Garzarella <sgarzare@redhat.com>

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

Currently, these types of drivers must use an internal buffer where they
temporarily store the response between .send() and recv() calls.

Introduce a new flag (TPM_CHIP_FLAG_SYNC) to support synchronous send().
If that flag is set by the driver, tpm_try_transmit() will use the send()
callback to send the command and receive the response on the same buffer
synchronously. In that case send() return the number of bytes of the
response on success, or -errno on failure.

Suggested-by: Jason Gunthorpe <jgg@ziepe.ca>
Suggested-by: Jarkko Sakkinen <jarkko@kernel.org>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 include/linux/tpm.h              |  1 +
 drivers/char/tpm/tpm-interface.c | 18 +++++++++++++++---
 2 files changed, 16 insertions(+), 3 deletions(-)

Comments

Jarkko Sakkinen April 8, 2025, 3:31 p.m. UTC | #1
On Tue, Apr 08, 2025 at 10:32:06AM +0200, Stefano Garzarella wrote:
> From: Stefano Garzarella <sgarzare@redhat.com>
> 
> Some devices do not support interrupts and provide a single synchronous
> operation to send the command and receive the response on the same buffer.
> 
> Currently, these types of drivers must use an internal buffer where they
> temporarily store the response between .send() and recv() calls.
> 
> Introduce a new flag (TPM_CHIP_FLAG_SYNC) to support synchronous send().
> If that flag is set by the driver, tpm_try_transmit() will use the send()
> callback to send the command and receive the response on the same buffer
> synchronously. In that case send() return the number of bytes of the
> response on success, or -errno on failure.
> 
> Suggested-by: Jason Gunthorpe <jgg@ziepe.ca>
> Suggested-by: Jarkko Sakkinen <jarkko@kernel.org>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>  include/linux/tpm.h              |  1 +
>  drivers/char/tpm/tpm-interface.c | 18 +++++++++++++++---
>  2 files changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> index 2e38edd5838c..0e9746dc9d30 100644
> --- a/include/linux/tpm.h
> +++ b/include/linux/tpm.h
> @@ -350,6 +350,7 @@ enum tpm_chip_flags {
>  	TPM_CHIP_FLAG_SUSPENDED			= BIT(8),
>  	TPM_CHIP_FLAG_HWRNG_DISABLED		= BIT(9),
>  	TPM_CHIP_FLAG_DISABLE			= BIT(10),
> +	TPM_CHIP_FLAG_SYNC			= BIT(11),
>  };
>  
>  #define to_tpm_chip(d) container_of(d, struct tpm_chip, dev)
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index 3b6ddcdb4051..9fbe84b5a131 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -114,8 +114,17 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip, void *buf, size_t bufsiz)
>  		return rc;
>  	}
>  
> -	/* A sanity check. send() should just return zero on success e.g.
> -	 * not the command length.
> +	/* Synchronous devices return the response directly during the send()
> +	 * call in the same buffer.
> +	 */

Nit:

/*
 * ...

It's wrong in the existing comment.

> +	if (chip->flags & TPM_CHIP_FLAG_SYNC) {
> +		len = rc;
> +		rc = 0;
> +		goto out_send_sync;
> +	}
> +
> +	/* A sanity check. send() of asynchronous devices should just return
> +	 * zero on success e.g. not the command length.
>  	 */
>  	if (rc > 0) {
>  		dev_warn(&chip->dev,
> @@ -151,7 +160,10 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip, void *buf, size_t bufsiz)
>  	if (len < 0) {
>  		rc = len;
>  		dev_err(&chip->dev, "tpm_transmit: tpm_recv: error %d\n", rc);
> -	} else if (len < TPM_HEADER_SIZE || len != be32_to_cpu(header->length))
> +		return rc;
> +	}
> +out_send_sync:

out_sync would be sufficient

> +	if (len < TPM_HEADER_SIZE || len != be32_to_cpu(header->length))
>  		rc = -EFAULT;
>  
>  	return rc ? rc : len;
> -- 
> 2.49.0
> 

BR, Jarkko
Stefano Garzarella April 9, 2025, 7:59 a.m. UTC | #2
On Tue, Apr 08, 2025 at 06:31:30PM +0300, Jarkko Sakkinen wrote:
>On Tue, Apr 08, 2025 at 10:32:06AM +0200, Stefano Garzarella wrote:
>> From: Stefano Garzarella <sgarzare@redhat.com>
>>
>> Some devices do not support interrupts and provide a single synchronous
>> operation to send the command and receive the response on the same buffer.
>>
>> Currently, these types of drivers must use an internal buffer where they
>> temporarily store the response between .send() and recv() calls.
>>
>> Introduce a new flag (TPM_CHIP_FLAG_SYNC) to support synchronous send().
>> If that flag is set by the driver, tpm_try_transmit() will use the send()
>> callback to send the command and receive the response on the same buffer
>> synchronously. In that case send() return the number of bytes of the
>> response on success, or -errno on failure.
>>
>> Suggested-by: Jason Gunthorpe <jgg@ziepe.ca>
>> Suggested-by: Jarkko Sakkinen <jarkko@kernel.org>
>> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>> ---
>>  include/linux/tpm.h              |  1 +
>>  drivers/char/tpm/tpm-interface.c | 18 +++++++++++++++---
>>  2 files changed, 16 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/linux/tpm.h b/include/linux/tpm.h
>> index 2e38edd5838c..0e9746dc9d30 100644
>> --- a/include/linux/tpm.h
>> +++ b/include/linux/tpm.h
>> @@ -350,6 +350,7 @@ enum tpm_chip_flags {
>>  	TPM_CHIP_FLAG_SUSPENDED			= BIT(8),
>>  	TPM_CHIP_FLAG_HWRNG_DISABLED		= BIT(9),
>>  	TPM_CHIP_FLAG_DISABLE			= BIT(10),
>> +	TPM_CHIP_FLAG_SYNC			= BIT(11),
>>  };
>>
>>  #define to_tpm_chip(d) container_of(d, struct tpm_chip, dev)
>> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
>> index 3b6ddcdb4051..9fbe84b5a131 100644
>> --- a/drivers/char/tpm/tpm-interface.c
>> +++ b/drivers/char/tpm/tpm-interface.c
>> @@ -114,8 +114,17 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip, void *buf, size_t bufsiz)
>>  		return rc;
>>  	}
>>
>> -	/* A sanity check. send() should just return zero on success e.g.
>> -	 * not the command length.
>> +	/* Synchronous devices return the response directly during the send()
>> +	 * call in the same buffer.
>> +	 */
>
>Nit:
>
>/*
> * ...
>
>It's wrong in the existing comment.

Yep, I'll fix.

>
>> +	if (chip->flags & TPM_CHIP_FLAG_SYNC) {
>> +		len = rc;
>> +		rc = 0;
>> +		goto out_send_sync;
>> +	}
>> +
>> +	/* A sanity check. send() of asynchronous devices should just return

And I'll fix also this of course.

>> +	 * zero on success e.g. not the command length.
>>  	 */
>>  	if (rc > 0) {
>>  		dev_warn(&chip->dev,
>> @@ -151,7 +160,10 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip, void *buf, size_t bufsiz)
>>  	if (len < 0) {
>>  		rc = len;
>>  		dev_err(&chip->dev, "tpm_transmit: tpm_recv: error %d\n", rc);
>> -	} else if (len < TPM_HEADER_SIZE || len != be32_to_cpu(header->length))
>> +		return rc;
>> +	}
>> +out_send_sync:
>
>out_sync would be sufficient

sure, I'll fix in v3.

Thanks,
Stefano
diff mbox series

Patch

diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 2e38edd5838c..0e9746dc9d30 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -350,6 +350,7 @@  enum tpm_chip_flags {
 	TPM_CHIP_FLAG_SUSPENDED			= BIT(8),
 	TPM_CHIP_FLAG_HWRNG_DISABLED		= BIT(9),
 	TPM_CHIP_FLAG_DISABLE			= BIT(10),
+	TPM_CHIP_FLAG_SYNC			= BIT(11),
 };
 
 #define to_tpm_chip(d) container_of(d, struct tpm_chip, dev)
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 3b6ddcdb4051..9fbe84b5a131 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -114,8 +114,17 @@  static ssize_t tpm_try_transmit(struct tpm_chip *chip, void *buf, size_t bufsiz)
 		return rc;
 	}
 
-	/* A sanity check. send() should just return zero on success e.g.
-	 * not the command length.
+	/* Synchronous devices return the response directly during the send()
+	 * call in the same buffer.
+	 */
+	if (chip->flags & TPM_CHIP_FLAG_SYNC) {
+		len = rc;
+		rc = 0;
+		goto out_send_sync;
+	}
+
+	/* A sanity check. send() of asynchronous devices should just return
+	 * zero on success e.g. not the command length.
 	 */
 	if (rc > 0) {
 		dev_warn(&chip->dev,
@@ -151,7 +160,10 @@  static ssize_t tpm_try_transmit(struct tpm_chip *chip, void *buf, size_t bufsiz)
 	if (len < 0) {
 		rc = len;
 		dev_err(&chip->dev, "tpm_transmit: tpm_recv: error %d\n", rc);
-	} else if (len < TPM_HEADER_SIZE || len != be32_to_cpu(header->length))
+		return rc;
+	}
+out_send_sync:
+	if (len < TPM_HEADER_SIZE || len != be32_to_cpu(header->length))
 		rc = -EFAULT;
 
 	return rc ? rc : len;