To integrate tpm_event_log with IMA subsystem,
tpm_crb and tpm_crb_ffa driver should be built as built-in
(CONFIG_TCG_CRB=y && CONFIG_TCG_CRB_FFA=y).

However, this could make failure for ima_init() gets tpm chip when
each initcall function deployed like:

0000000000000888 l       .initcall6.init	0000000000000000 crb_acpi_driver_init
000000000000088c l       .initcall6.init	0000000000000000 tpm_crb_ffa_driver_init

If crb_api_driver_init() is called first, probing tpm device using CRB over
FFA method is deferred since tpm_crb_ffa_driver_init() isn't called yet.

Deferred probe of the tpm device will be probed by system workqueue
in proper time after deferred_probe_initcall() registers the work.
However, ima_init() could be called first if they're deployed like:

000000000000012c l       .initcall7.init	0000000000000000 init_ima
000000000000016c l       .initcall7.init	0000000000000000 deferred_probe_initcall7

In this situation, ima_init() failed to find tpm device and it failed to
generate boot_aggregate with PCR values.

That's why kernel prints log this situation though tpm device exists:
   [    3.080786] ima: No TPM chip found, activating TPM-bypass!

To resolve this, call ffa_register() in tpm_crb_ffa_init() when it was
built with built-in so that ima can generate boot_aggregate log with
PCR values properly.

Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
---
 drivers/char/tpm/tpm_crb_ffa.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/char/tpm/tpm_crb_ffa.c b/drivers/char/tpm/tpm_crb_ffa.c
index 4ead61f01299..2ef29b66fa5d 100644
--- a/drivers/char/tpm/tpm_crb_ffa.c
+++ b/drivers/char/tpm/tpm_crb_ffa.c
@@ -115,6 +115,7 @@ struct tpm_crb_ffa {
 };
 
 static struct tpm_crb_ffa *tpm_crb_ffa;
+static struct ffa_driver tpm_crb_ffa_driver;
 
 static int tpm_crb_ffa_to_linux_errno(int errno)
 {
@@ -168,13 +169,22 @@ static int tpm_crb_ffa_to_linux_errno(int errno)
  */
 int tpm_crb_ffa_init(void)
 {
-	if (!tpm_crb_ffa)
-		return -ENOENT;
+	int ret = 0;
 
-	if (IS_ERR_VALUE(tpm_crb_ffa))
-		return -ENODEV;
+	if (IS_MODULE(CONFIG_TCG_ARM_CRB_FFA)) {
+		if (!tpm_crb_ffa)
+			ret = -ENOENT;
 
-	return 0;
+		if (IS_ERR_VALUE(tpm_crb_ffa))
+			ret = -ENODEV;
+
+		return ret;
+	}
+
+	ret = ffa_register(&tpm_crb_ffa_driver);
+	BUG_ON(!ret && !tpm_crb_ffa);
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(tpm_crb_ffa_init);
 
@@ -369,7 +379,9 @@ static struct ffa_driver tpm_crb_ffa_driver = {
 	.id_table = tpm_crb_ffa_device_id,
 };
 
+#ifdef MODULE
 module_ffa_driver(tpm_crb_ffa_driver);
+#endif
 
 MODULE_AUTHOR("Arm");
 MODULE_DESCRIPTION("TPM CRB FFA driver");
-- 
LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}

On Fri, Jun 06, 2025 at 11:57:54AM +0100, Yeoreum Yun wrote:
> To integrate tpm_event_log with IMA subsystem,
> tpm_crb and tpm_crb_ffa driver should be built as built-in
> (CONFIG_TCG_CRB=y && CONFIG_TCG_CRB_FFA=y).
> 
> However, this could make failure for ima_init() gets tpm chip when
> each initcall function deployed like:
> 
> 0000000000000888 l       .initcall6.init	0000000000000000 crb_acpi_driver_init
> 000000000000088c l       .initcall6.init	0000000000000000 tpm_crb_ffa_driver_init

The only failure I see is the patch 1/2 which changes init call level,
and leaves kernel Git to a broken state.

It breaks the famous "zero regressions policy".

BR, Jarkko

Hi Jarkko,

> > To integrate tpm_event_log with IMA subsystem,
> > tpm_crb and tpm_crb_ffa driver should be built as built-in
> > (CONFIG_TCG_CRB=y && CONFIG_TCG_CRB_FFA=y).
> >
> > However, this could make failure for ima_init() gets tpm chip when
> > each initcall function deployed like:
> >
> > 0000000000000888 l       .initcall6.init	0000000000000000 crb_acpi_driver_init
> > 000000000000088c l       .initcall6.init	0000000000000000 tpm_crb_ffa_driver_init
>
> The only failure I see is the patch 1/2 which changes init call level,
> and leaves kernel Git to a broken state.
>
> It breaks the famous "zero regressions policy".
>
> BR, Jarkko

Sorry, would you let me know what is broken more detail?
IMHO, by changing the init call level for ffa_init()
it's called early than before device_initcall() and it seems not to
break anything.

What breaks do you mean?

Thanks.

--
Sincerely,
Yeoreum Yun

On Fri, Jun 06, 2025 at 07:12:43PM +0100, Yeoreum Yun wrote:
> Hi Jarkko,
> 
> > > To integrate tpm_event_log with IMA subsystem,
> > > tpm_crb and tpm_crb_ffa driver should be built as built-in
> > > (CONFIG_TCG_CRB=y && CONFIG_TCG_CRB_FFA=y).
> > >
> > > However, this could make failure for ima_init() gets tpm chip when
> > > each initcall function deployed like:
> > >
> > > 0000000000000888 l       .initcall6.init	0000000000000000 crb_acpi_driver_init
> > > 000000000000088c l       .initcall6.init	0000000000000000 tpm_crb_ffa_driver_init
> >
> > The only failure I see is the patch 1/2 which changes init call level,
> > and leaves kernel Git to a broken state.
> >
> > It breaks the famous "zero regressions policy".
> >
> > BR, Jarkko
> 
> Sorry, would you let me know what is broken more detail?
> IMHO, by changing the init call level for ffa_init()
> it's called early than before device_initcall() and it seems not to
> break anything.
> 
> What breaks do you mean?

Let's start from very beginning. Why this change is needed and not just
1/2?

IMA intializes as a late initcall, which after TPM has initialized.

> 
> Thanks.
> 
> --
> Sincerely,
> Yeoreum Yun

BR, Jarkko

On Fri, Jun 06, 2025 at 07:12:43PM +0100, Yeoreum Yun wrote:
> Hi Jarkko,
> 
> > > To integrate tpm_event_log with IMA subsystem,
> > > tpm_crb and tpm_crb_ffa driver should be built as built-in
> > > (CONFIG_TCG_CRB=y && CONFIG_TCG_CRB_FFA=y).
> > >
> > > However, this could make failure for ima_init() gets tpm chip when
> > > each initcall function deployed like:
> > >
> > > 0000000000000888 l       .initcall6.init	0000000000000000 crb_acpi_driver_init
> > > 000000000000088c l       .initcall6.init	0000000000000000 tpm_crb_ffa_driver_init
> >
> > The only failure I see is the patch 1/2 which changes init call level,
> > and leaves kernel Git to a broken state.
> >
> > It breaks the famous "zero regressions policy".
> >
> > BR, Jarkko
> 
> Sorry, would you let me know what is broken more detail?
> IMHO, by changing the init call level for ffa_init()
> it's called early than before device_initcall() and it seems not to
> break anything.
> 
> What breaks do you mean?

Your description in the cover letter and commit messages in unclear
and convoluted. Please describe exact causalities instead of something
not defined could cause "failure" (which is also abstract concept).

I'll check the next round.

> 
> Thanks.
> 
> --
> Sincerely,
> Yeoreum Yun

BR, Jarkko

Hi Jarkko,

[...]
> Your description in the cover letter and commit messages in unclear
> and convoluted. Please describe exact causalities instead of something
> not defined could cause "failure" (which is also abstract concept).
>
> I'll check the next round.

Hmm, I'll do my best to rewrite them
But I’m not sure if that would work well for you…

Thanks.

--
Sincerely,
Yeoreum Yun