Re: [PATCH v1 1/1] tpm: tpm_crb_ffa: Only register driver when ACPI TPM2 uses FF-A

[Jarkko Sakkinen <jarkko@kernel.org>]

On Fri, Nov 13, 2026 at 11:31:31AM -0800, Siddharth Chintamaneni wrote:
> On Fri, 13 Nov 2026 at 10:31, Jarkko Sakkinen <jarkko@kernel.org> wrote:
> >
> > On Thu, Nov 12, 2026 at 11:38:07PM +0000, Siddharth Chintamaneni wrote:
> > > Prior to commit 677042afb97a ("tpm: tpm_crb_ffa: revert defered_probed
> > > when tpm_crb_ffa is built-in"), built-in tpm_crb_ffa only registered
> > > tpm_crb_ffa_driver from tpm_crb_ffa_init() after crb_acpi_probe()
> > > confirmed start_method == ACPI_TPM2_CRB_WITH_ARM_FFA. After that
> > > commit, tpm_crb_ffa unconditionally registers on the FF-A bus at
> > > device_initcall time (and when loaded as a module) without checking
> > > the platform's ACPI TPM2 table.
> > >
> > > A platform's firmware may expose a TPM CRB over FF-A Secure Partition
> > > for UEFI boot services while advertising a non-FF-A start method to
> > > the OS in the ACPI TPM2 table. When the ACPI TPM2 table designates a
> > > non-FF-A start method, Linux has no consumer for tpm_crb_ffa and
> > > should not probe the FF-A TPM service.
> > >
> > > On Microsoft Cobalt 200 systems, the ACPI TPM2 table does not use
> > > ACPI_TPM2_CRB_WITH_ARM_FFA, yet ffa_init() discovers the FF-A TPM
> > > partition and tpm_crb_ffa_probe() unconditionally sends
> > > CRB_FFA_GET_INTERFACE_VERSION via FFA_MSG_SEND_DIRECT_REQ2/REQ SMC.
> > > That request wedges the calling CPU in Secure World (with Non-Secure
> > > interrupts and GICv3 pseudo-NMIs masked), hanging boot with an RCU CPU
> > > stall:
> > >
> > >   [    5.507519] ARM FF-A: Driver version 1.2
> > >   [    5.507519] ARM FF-A: Firmware version 1.2 found
> > >   [    5.507551] ARM FF-A: Notification setup failed -61, not enabled
> > >   [    7.150580] Freeing initrd memory: 293640K
> > >   [   65.515248] rcu: INFO: rcu_preempt detected stalls on CPUs/tasks:
> > >   [   65.521346] rcu:     10-...0: (3 GPs behind) idle=dd9c/1/0x4000000000000000 softirq=37/37 fqs=5262
> > >   [   65.530031] rcu:              hardirqs   softirqs   csw/system
> > >   [   65.535589] rcu:      number:        0          0            0
> > >   [   65.541146] rcu:     cputime:        0          0            0   ==> 30028(ms)
> > >   [   65.548102] rcu:     (detected by 66, t=15010 jiffies, g=-951, q=1384 ncpus=132)
> > >   [   65.555228] Sending NMI from CPU 66 to CPUs 10:
> > >   [   75.560686] After 10 seconds, these CPUS still haven't responded to the NMI: 10
> > >
> > > Check the ACPI TPM2 table in tpm_crb_ffa_driver_init() and return
> > > -ENODEV before calling ffa_register() unless the table is present and
> > > specifies ACPI_TPM2_CRB_WITH_ARM_FFA, matching the check_acpi_tpm2()
> > > pattern in tpm_tis.c. This ensures Linux only issues FF-A TPM requests
> > >
> > > when ACPI advertises the FF-A start method (for both built-in and
> > > modular builds), while preserving normal FF-A bus probing on platforms
> > > that use CRB over FF-A.
> > >
> > > Fixes: 677042afb97a ("tpm: tpm_crb_ffa: revert defered_probed when tpm_crb_ffa is built-in")
> > > Fixes: eb93f0734ef1 ("tpm_crb: ffa_tpm: Implement driver compliant to CRB over FF-A")
> >
> > Pick the older.
> >
> > I attached a prepending patch that caches the start method, which can be
> > later used in tpm_tis and tpm_crb.
> >
> > So that totals four patches:
> >
> > 1. My patch.
> > 2. Your fix using the cached value.
> > 3. tpm_tis update to use the cached value.
> > 4. tpm_crb update to use the cached value.
> >
> 
> Sounds good! I can put together that 4 patch v2 series.
> 
> Two quick notes before I spin v2
> 
> - Module loading with CONFIG_TCG_CRB=m and CONFIG_TCG_ARM_CRB_FFA=m:
>   As Sashiko flagged on v1 [1], because tpm_crb.ko directly references
>   tpm_crb_ffa_init() and tpm_crb_ffa_start(), returning -ENODEV from
>   tpm_crb_ffa's module_init would prevent modprobe from loading
>   tpm_crb.ko on non-FF-A systems. In v2, I will move the start_method
>   check into tpm_crb_ffa_probe() (matching check_acpi_tpm2() in tpm_tis)
>   so both modules load cleanly while still skipping the FF-A probe on
>   non-FF-A platforms
> 
> - regarding the location of caching approach -
>   tpm_crb_ffa never allocates a struct tpm_chip, and both tpm_tis
>   (check_acpi_tpm2()) and tpm_crb (crb_acpi_probe()) check start_method
>   before calling tpmm_chip_alloc().  Would it work for you if patch 1
>   caches start_method once in tpm_init() (subsys_initcall in
>   tpm-interface.c) and exports a small helper (e.g.
>   tpm_acpi_start_method()) so tpm_crb_ffa_probe(), tpm_tis, and tpm_crb
>   can all query it before chip allocation? (Also noticed a small '||' ->
>   '&&' typo on the header.length check in the attached patch.)

Move it to tpm_init() and store the result to a global variable, let's
say tpm2_acpi_start_method. There's only one of these so it's all fine.

You can then mark the resulting commit as:

Signed-off-by: Siddharth Chintamaneni <sidchintamaneni@gmail.com>
Co-developed-by: Siddharth Chintamaneni <sidchintamaneni@gmail.com>
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>

I just put those tag because it is easy to mix up with Co-by tag,
when using it.

Br, Jarkko