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

[Jorge Ramirez <jorge.ramirez@oss.qualcomm.com>]

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.

> 
> > 
> > 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.

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 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.

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