[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <eidmcwgppc4uobyupns4hzqz562wguapiocpyyqq67j5h26qbl@muhbnfxzqvqt>
Date: Thu, 27 Mar 2025 10:48:17 +0100
From: Stefano Garzarella <sgarzare@...hat.com>
To: Jarkko Sakkinen <jarkko@...nel.org>
Cc: Jason Gunthorpe <jgg@...pe.ca>, linux-kernel@...r.kernel.org,
Peter Huewe <peterhuewe@....de>, linux-integrity@...r.kernel.org,
James Bottomley <James.Bottomley@...senpartnership.com>, Jens Wiklander <jens.wiklander@...aro.org>,
Sumit Garg <sumit.garg@...nel.org>
Subject: Re: [PATCH 1/2] tpm: add send_recv() op in tpm_class_ops
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@...hat.com>
>>
>> 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@kernel.org/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
Powered by blists - more mailing lists