Re: [RFC PATCH v2 3/6] tpm: add send_recv() ops in tpm_class_ops

Stefano Garzarella
2025-03-07T15:37:21Z

On Thu, Mar 06, 2025 at 11:52:46PM +0200, Jarkko Sakkinen wrote:
>On Wed, Mar 05, 2025 at 03:02:29PM -0400, Jason Gunthorpe wrote:
>> On Wed, Mar 05, 2025 at 10:04:25AM +0100, Stefano Garzarella wrote:
>> > Jason suggested the send_recv() ops [2], which I liked, but if you prefer to
>> > avoid that, I can restore what we did in v1 and replace the
>> > TPM_CHIP_FLAG_IRQ hack with your point 2 (or use TPM_CHIP_FLAG_IRQ if you
>> > think it is fine).
>>
>> I think it is a pretty notable simplification for the driver as it
>> does not need to implement send, status, req_canceled and more ops.
>>
>> Given the small LOC on the core side I'd call that simplification a
>> win..
>
>I'm sorry to disagree with you on this but adding a callback for
>one leaf driver is not what I would call "a win" :-)

IIUC in the ftpm driver (tpm_ftpm_tee.c) we could also use send_recv()
and save a memcpy() to a temporally buffer (pvt_data->resp_buf) and also
that 4k buffer allocated with the private data of the driver.

BTW if you agree, for now I'll do something similar of what we do in the
ftpm driver (which would be what Jarkko recommended - status() returns
0, .req_complete_mask = 0, .req_complete_val = 0) and we can discuss
send_recv() in a new series where I can include changes for the ftpm
driver too, to see whether it makes sense or not.

WDYT?

Thanks,
Stefano

>
>I mean, it's either a minor twist in
>
>1. "the framework code" which affects in a way all other leaf drivers.
> At bare minimum it adds a tiny bit of complexity to the callback
> interface and a tiny bit of accumulated maintenance cost.
>2. in the leaf driver
>
>So I'd really would want to keep that tiny bit of extra complexity
>localized.
>
>>
>> Jason
>
>BR, Jarkko
>