Re: [PATCH 1/2] tpm: add send_recv() op in tpm_class_ops

From: Stefano Garzarella
Date: Thu Mar 27 2025 - 05:48:44 EST


On Wed, Mar 26, 2025 at 06:53:51PM +0200, Jarkko Sakkinen wrote:
On Thu, Mar 20, 2025 at 04:24:32PM +0100, Stefano Garzarella wrote:
From: Stefano Garzarella <sgarzare@xxxxxxxxxx>

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

Simplification can be addressed with no callback changes:

https://lore.kernel.org/linux-integrity/20250326161838.123606-1-jarkko@xxxxxxxxxx/T/#u

I also noticed that tpm_ftpm_tee initalized req_complete_mask and
req_complete_val explictly while they would be already implicitly
zero.

So it reduces this just a matter of getting rid off the extra
buffer.

Yep, as mentioned I think your patch should go either way. So here I can rephrase and put the emphasis on the temporary buffer and the driver simplification.


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.

I don't find anything in the commit message addressing buf_len an
cmd_len (vs "just len"). Why two lengths are required?

Not completely rejecting but this explanation is incomplete.

Right.

The same buffer is used as input and output.
For input, the buffer contains the command (cmd_len) but the driver can use the entire buffer for output (buf_len).
It's basically the same as in tpm_try_transmit(), but we avoid having to parse the header in each driver since we already do that in tpm_try_transmit().

In summary cmd_len = count = be32_to_cpu(header->length).

I admit I'm not good with names, would you prefer a different name or is it okay to explain it better in the commit?

My idea is to add this:

`buf` is used as input and output. It contains the command
(`cmd_len` bytes) as input. The driver will be able to use the
entire buffer (`buf_len` bytes) for the response as output.
Passing `cmd_len` is an optimization to avoid having to access the
command header again in each driver and check it.

WDYT?

Thanks,
Stefano