Re: [PATCH 1/1] tpm: add TEE driver for a discrete TPM behind OP-TEE

[Jarkko Sakkinen <jarkko@kernel.org>]

On Tue, Oct 27, 2026 at 09:24:42AM +0100, Jorge Ramirez wrote:
> On 26/10/26 22:07:20, Jarkko Sakkinen wrote:
> > On Mon, Oct 26, 2026 at 05:32:48PM +0100, Jorge Ramirez-Ortiz wrote:
> > > Add tpm_dtpm_tee for a discrete TPM that OP-TEE owns and the normal world
> > > cannot reach directly (e.g. an SPI TPM on a bus driven by OP-TEE). The
> > > driver binds to OP-TEE's passthrough pseudo TA by UUID and forwards TPM2
> > > commands through it, presenting a standard TPM2 character device.
> > 
> > "OP-TEE hosts" would be more understandable.
> > 
> > I have trouble following the hops you are doing by speaking about normal
> > world and then jumping into SPI bus. Please try to somehow make this
> > more rigid and less terminology decorated.
> 
> I'd rather keep "owns", not "hosts": OP-TEE does not host this TPM (that
> is the fTPM case, where the TPM runs inside the TEE). Here OP-TEE owns
> the bus controller the chip is wired to, so Linux cannot drive the
> chip.
> 
> That distinction is the reason this is not an fTPM.

OK, so then it would help if it was broken down how these different
components and their interaction work in simple terms.

> 
> > 
> > > 
> > > It is a separate driver rather than a reuse of tpm_ftpm_tee: the TEE
> > > transport is the same, but that driver is tied to the Microsoft fTPM TA
> > > and opens a public-login session, whereas this is a distinct TA for a real
> > > chip, gated to REE-kernel access so that user space and other TAs cannot
> > > reach the TPM. Sharing the fTPM identity would conflate two different
> > > devices and collide on a platform that has both.
> > 
> > And they don't share any of the same DNA?
> 
> Sure, they do. The TEE transport is effectively identical: the
> shared-memory, the invoke and the response validation, plus the
> probe/teardown sequence.
> 
> What differs is the device identity: TA UUID, login (REE-kernel vs
> public), and bind method (ftpm also binds via platform/OF and has a
> shutdown path).
> 
> > 
> > Two other options:
> > 
> > 1. tpm_tis_core type of solution.
> > 2. Implement the feature as part of tpm_ftpm_tee.c.
> 
> I'd rather do (1). The shared transport goes into a small core; the
> per-device identity stays in thin front-ends.
> 
> Something along these lines:
> 
>   /* tpm_tee.h */
>   struct tpm_tee_config {
>           uuid_t  ta_uuid;
>           u32     login;          /* TEE_IOCTL_LOGIN_* */
>           u32     cmd_id;         /* SUBMIT_COMMAND function ID */
>   };
> 
>   int  tpm_tee_core_probe(struct device *dev,
>                           const struct tpm_tee_config *cfg);
>   void tpm_tee_core_remove(struct device *dev);
> 
> 
> tpm_tee_core.c would own everything that is common today:
>  - open context
>  - open session
>  - shm allocation,
>  - chip alloc/register
>  - teardown and so forth
> 
> The front-ends collapse to config plus their own bus binding, e.g.:
> 
>   /* tpm_dtpm_tee.c */
>   static const struct tpm_tee_config dtpm_cfg = {
>           .ta_uuid = UUID_INIT(0x50568cd9, 0x370a, 0x46b8,
>                                0x9f, 0x2c, 0x6f, 0xd9,
>                                0x06, 0x64, 0xd2, 0xee),
>           .login   = TEE_IOCTL_LOGIN_REE_KERNEL,
>           .cmd_id  = 0,
>   };
> 
>   static int dtpm_tee_probe(struct tee_client_device *tcdev)
>   {
>           return tpm_tee_core_probe(&tcdev->dev, &dtpm_cfg);
>   }
> 
>   static void dtpm_tee_remove(struct tee_client_device *tcdev)
>   {
>           tpm_tee_core_remove(&tcdev->dev);
>   }
>   /* id_table + tee_client_driver unchanged */
> 
> 
> I'd push back on (2): one file would carry a public-login firmware TPM
> and a REE-kernel discrete chip, plus it breaks on a platform with both.

Let's write this down to the commit message.

> 
> The core split removes the duplication and avoids that coupling.
> 
> Questions:
> - Is the core in tpm_tee.c / tpm_tee.h acceptable, or do you want a
>   different split.
> 

I'll answer below.

> > 
> > I don't think tpm_ftpm_tee was that great name choice [1] but we have
> > to live with it. 
> > 
> > So more logical name as per taxonomy would be probably
> > tpm_ftpm_tee_dtpm.
> 
> tpm_ftpm_tee_dtpm reads as "firmware TPM ... discrete TPM", which is a
> contradiction (this device is _not_ a firmware TPM)
> 
> Per your own note in [1], the ftpm prefix was a mistake, so extending it
> seems worse than leaving it.
> 
> With the core split this seems clean to me:
> 
>   tpm_tee.c         transport core
>   tpm_ftpm_tee.c    firmware TPM front-end
>   tpm_dtpm_tee.c    discrete TPM front-end
> 
> Does that work? I'm open to another name, as long as firmware vs
> discrete stays visible.

OK so tpm_ftpm_tee is/was a bad name but also renaming modules causes
issues.

It should have probably been "tpm_optee_ftpm".

So I would do instead this:

- tpm_optee -core
- tpm_optee_dtpm -your driver
- tpm_ftpm_tee

I.e. let's accept a glitch but don't repeat the faulty naming pattern.

> 
> The rest I'll take: = {} initalizers (drop the memsets), module name as
> the function prefix, and the MODULE_DESCRIPTION reworded once the layout
> is settled.
> 
> TIA
> jorge
> 

Br, Jarkko