On Sat, 3 Oct 2026 at 15:23, Simon Glass <[email protected]> wrote: > > Hi Ilias, > > On Thu, 1 Oct 2026 at 19:07, Ilias Apalodimas > <[email protected]> wrote: > > > > Hi Simon > > > > On Thu, 1 Oct 2026 at 19:03, Simon Glass <[email protected]> wrote: > > > > > > Hi Ilias, > > > > > > On Thu, 1 Oct 2026 at 09:41, Ilias Apalodimas > > > <[email protected]> wrote: > > > > > > > > Hi all, > > > > > > > > Any reason why we can't add this directly to the uclass probe instead? > > > > According to our docs, the probe is supposed to 'make a device ready > > > > for use'. > > > > > > IIRC we talked about that a few years ago. Probing a device is > > > supposed to get ready for using the hardware, so we typically have a > > > separate operation to actually talk to it and get it into a particular > > > state - otherwise just probing the device would take quite a while (it > > > also means that TPM cannot be used for certain things after init() > > > unless the board is reset!). The same approach is used by the > > > keyboard, for example. > > > > I am not sure I follow this one. What do you mean after init? The TPM > > is only supposed to reset with the CPU. > > Yes...TPM2_Startup can only be sent once per reset, and it determines > things which then cannot change until the next reset. For example, on > a resume from S3 we must send TPM2_SU_STATE rather than TPM2_SU_CLEAR > (see tpm_resume()), or the PCRs are reset and the OS can no longer > unseal its data. tpm_auto_start() always sends CLEAR. > > If probe does the startup, then whatever happens to probe the TPM > first makes that choice, and nothing later can make a different one.
What choice are you describing here? tpm_resume() isn't used anywhere in our code and I really don't expect it to ever be used. I am not interested in theoretical use cases. > > > > > > > > > But given that this keeps coming up, we could perhaps provide DM > > > support for this sort of thing - it's not obvious the best way to do > > > it in a generic way, though. We could have a DM flag and perhaps send > > > an event that the TPM uclass picks up on? > > > > What would we we gain from the event vs just initializing it on probe? > > The event runs once, at a known point in U-Boot proper, and only on > boards which ask for it. A probe runs in every phase which touches the > TPM. tpm2_auto_start() starts with a full self-test even if the TPM is > already started, so a TPM which an earlier phase, or coreboot, has set > up would be self-tested again each time. What do you mean by 'every phase'? TPL, SPL etc? The self tests don't *have* to rerun. We just always trigger them becuase tpm2_auto_start sets fulltest to 'yes'. So I think changing the first call of tpm2_self_test() to TPMI_NO will take care of that, but I have to take a closer look at the spec. > Also, if the self-test fails, > e.g. in failure mode or lockout, the probe would fail and we can't > even talk to the chip to find out why, or to clear or update it. Why? What's different in ingoring the tepm self test result during probing vs having it fail later? In both cases the device is unusable and you can access it, no ? > > On disabling the TPM in the DT for speed: U-Boot often passes its DT > on to Linux, so that would hide the TPM from the OS as well. Which sounds correct. The role of the TPM is to measure the pre OS environment + some OS specific measurements. e.g on the kernel we measure the initrd. Do you have a real world use case where an OS wants a TPM with zero preboot measurements? > > So I'd prefer to keep probe as it is and take Michael's option, which > gives boards that want it a started TPM, before the OS boots, without > relying on the environment. > > BTW, by design, driver model aims to do the least work possible in > probing a device - actually communcating with it is supposed to be > done only when needed. That is mostly about speed and simplicity, but > it is particularly important for devices which have state, such as the > TPM. So the TPM doesnt *require* to run all the selftests. Just the functions you will use and they can run in the background. But that's a bigger change and unrelated to the current discussion. Also the documentation says the device is 'usable' after probe. Maybe we should update that with what you described? Thanks /Ilias > > Regards, > Simon > > > > > On Thu, 1 Oct 2026 at 11:20, Simon Glass <[email protected]> wrote: > > > > > > > > > > Hi Michael, > > > > > > > > > > On 2026-09-30T09:05:12, Michael Haener <[email protected]> > > > > > wrote: > > > > > > tpm: Add an option to start the TPM during init > > > > > > > > > > > > Add CONFIG_TPM_AUTOSTART, which runs the TPM startup and self-test > > > > > > sequence from an EVT_LAST_STAGE_INIT handler. This is the same > > > > > > thing the > > > > > > 'tpm autostart' command does, but it does not depend on a boot > > > > > > script or > > > > > > on the environment, so it cannot be skipped by a stale or edited > > > > > > env. > > > > > > > > > > > > Today a TPM is only started automatically when either > > > > > > CONFIG_MEASURED_BOOT > > > > > > is used (and then only from bootm) or when booting via EFI with > > > > > > CONFIG_EFI_TCG2_PROTOCOL. A plain booti/bootm therefore leaves the > > > > > > chip > > > > > > uninitialised and the OS has to start it itself, which Linux > > > > > > reports as > > > > > > "A TPM error (256) occurred attempting the self test". Boards have > > > > > > worked > > > > > > around this by putting 'tpm autostart' into their environment, see > > > > > > board/xilinx/zynqmp/zynqmp_kria.env. > > > > > > > > > > > > Boards that share one binary between variants with and without a TPM > > > > > > fitted can enable this safely: when no TPM device is found the > > > > > > startup is > > > > > > skipped silently. > > > > > > > > > > > > Signed-off-by: Michael Haener <[email protected]> > > > > > > > > > > > > drivers/tpm/Kconfig | 15 +++++++++++++++ > > > > > > drivers/tpm/Makefile | 1 + > > > > > > drivers/tpm/tpm_autostart.c | 33 +++++++++++++++++++++++++++++++++ > > > > > > 3 files changed, 49 insertions(+) > > > > > > > > > > This seems like a good way to enable the TPM for boards that want it. > > > > > It would be good to document this somewhere, e.g. where we mention > > > > > MEASURED_BOOT > > > > > > > > > > > diff --git a/drivers/tpm/tpm_autostart.c > > > > > > b/drivers/tpm/tpm_autostart.c > > > > > > @@ -0,0 +1,33 @@ > > > > > > + /* A board may share one binary between variants without a > > > > > > TPM. */ > > > > > > + ret = uclass_first_device_err(UCLASS_TPM, &dev); > > > > > > + if (ret) { > > > > > > + log_debug("No TPM found (err=%d)\n", ret); > > > > > > + return 0; > > > > > > + } > > > > > > > > > > It seems odd to ignore an error in this case - if the board maintainer > > > > > says that a TPM must be present, then I would expect U-Boot to fail if > > > > > it is not. > > > > > > > > > > In any case, uclass_first_device_err() also fails when the TPM is in > > > > > the devicetree but fails to probe, e.g. because of a bus or GPIO-reset > > > > > problem on a board that has the chip fitted. That is a real fault, but > > > > > log_debug() hides it. Please can you stay silent only for -ENODEV and > > > > > use log_warning() for anything else? > > > > > > > > > > > diff --git a/drivers/tpm/tpm_autostart.c > > > > > > b/drivers/tpm/tpm_autostart.c > > > > > > @@ -0,0 +1,33 @@ > > > > > > + ret = tpm_auto_start(dev); > > > > > > + if (ret) > > > > > > + log_warning("TPM startup failed (err=%d)\n", ret); > > > > > > > > > > tpm_auto_start() returns a u32, which may be a negative errno or a TPM > > > > > response code (e.g. TPM2_RC_INITIALIZE). A response code printed in > > > > > decimal from an int is hard to match against the spec, so please use a > > > > > u32 and %#x here, as report_return_code() does for the tpm command. > > > > > > > > > > > diff --git a/drivers/tpm/tpm_autostart.c > > > > > > b/drivers/tpm/tpm_autostart.c > > > > > > @@ -0,0 +1,33 @@ > > > > > > +EVENT_SPY_SIMPLE(EVT_LAST_STAGE_INIT, tpm_evt_autostart); > > > > > > > > > > This is not really a driver, so lib/ (next to tpm_api.c) may be a > > > > > better home than drivers/tpm. What do you think? > > > > > > > > > > Please can you enable this on sandbox and it would be nice to have a > > > > > test in test/dm/tpm.c, checking that the TPM is started after init and > > > > > that a missing TPM is handled. Your test could send the event. > > > > > > > > > > A full self-test on a TPMv2 can take a noticeable time, so the help > > > > > should mention that this adds to the boot time on every boot, even > > > > > when the TPM is not used. > > > > > > > > > > BTW, since the commit message mentions > > > > > board/xilinx/zynqmp/zynqmp_kria.env, you could convert that board in a > > > > > follow-up patch to show how the option is used. > > > > > > > > > > Regards, > > > > > Simon
