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

[Stefano Garzarella]

On Wed, Mar 26, 2025 at 10:37:09PM +0200, Jarkko Sakkinen wrote:
> On Wed, Mar 26, 2025 at 05:58:33PM +0200, Jarkko Sakkinen wrote:
> > On Wed, Mar 26, 2025 at 04:57:47PM +0200, Jarkko Sakkinen wrote:
> > > On Wed, Mar 26, 2025 at 11:34:01AM -0300, Jason Gunthorpe wrote:
> > > > On Wed, Mar 26, 2025 at 02:11:12PM +0200, Jarkko Sakkinen wrote:
> > > >
> > > > > Generally speaking I don't see enough value in complicating
> > > > > callback interface. It's better to handle complications in
> > > > > the leaves (i.e. dictatorship of majority ;-) ).
> > > >
> > > > That is very much not the way most driver subsystems view the
> > > > world. We want to pull logical things into the core code and remove
> > > > them from drivers to make the drivers simpler and more robust.
> > > >
> > > > The amount of really dumb driver boiler plate that this series
> > > > obviously removes is exactly the sort of stuff we should be fixing by
> > > > improving the core code.
> > > >
> > > > The callback interface was never really sanely designed, it was just
> > > > built around the idea of pulling the timout processing into the core
> > > > code for TIS hardware. It should be revised to properly match these
> > > > new HW types that don't have this kind of timeout mechanism.
> > >
> > > Both TIS and CRB, which are TCG standards and they span to many
> > > different types of drivers and busses. I don't have the figures but
> > > probably they cover vast majority of the hardware.
> > >
> > > We are talking about 39 lines of reduced complexity at the cost
> > > of complicating branching at the top level. I doubt that there
> > > is either any throughput or latency issues.
> > >
> > > What is measurable benefit? The rationale is way way too abstract
> > > for me to cope, sorry.
> >
> > E.g., here's how you can get rid of extra cruft in tpm_ftpm_tee w/o
> > any new callbacks.

Yeah, I agree that your patch should go in any case, with send_recv() or 
not. It's a good cleanup.

> Measurable benefit: no need to allocate memory buffer.

That's right, I read the whole thread before responding, but that's 
exactly what I wanted to highlight. Implementing send_recv() we could 
completely remove the buffer for the cache here in tpm_ftpm_tee, 
simplifying it quite a bit.

In tpm_svsm instead we allocate it while probing anyway to avoid having 
to allocate it every time, but we could potentially do the same (I don't 
know if it makes sense honestly). We do this because for SVSM any buffer 
is fine, as it can access all guest kernel memory, whereas IIUC for ftpm 
it has to be taken from shared memory.

> Let's take that as a starting point ;-)

Yeah!

> On that basis I can consider this (i.e. something to measure).

Okay, I explain this better in the commit description for the next 
version!

Thanks,
Stefano