[RFC,6/7] tpm: add __always_inline for tpm_is_hwrng_enabled

Commit Message

Chen Linxuan April 11, 2025, 10:54 a.m. UTC

From: Winston Wen <[e-mail]>

On x86_64 with gcc version 13.3.0, I compile kernel with:

  make defconfig
  ./scripts/kconfig/merge_config.sh .config <(
    echo CONFIG_TCG_TPM=y
    echo CONFIG_HW_RANDOM=m
  )
  make KCFLAGS="-fno-inline-small-functions -fno-inline-functions-called-once"

Then I get a link error:

  ld: vmlinux.o: in function `tpm_add_hwrng':
  tpm-chip.c:(.text+0x6c5924): undefined reference to `hwrng_register'
  ld: vmlinux.o: in function `tpm_chip_unregister':
  (.text+0x6c5bc9): undefined reference to `hwrng_unregister'
  ld: vmlinux.o: in function `tpm_chip_register':
  (.text+0x6c5c9b): undefined reference to `hwrng_unregister'

Signed-off-by: Winston Wen <[e-mail]>
Co-Developed-by: Chen Linxuan <[e-mail]>
Signed-off-by: Chen Linxuan <[e-mail]>
---
 drivers/char/tpm/tpm-chip.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jarkko Sakkinen April 12, 2025, 12:47 a.m. UTC | #1

On Fri, Apr 11, 2025 at 06:54:54PM +0800, Chen Linxuan wrote:
> From: Winston Wen <[e-mail]>
> 
> On x86_64 with gcc version 13.3.0, I compile kernel with:

Use passive:

"Presume that kernel is compiled for x86_64 with gcc version 13.3.0:"

> 
>   make defconfig
>   ./scripts/kconfig/merge_config.sh .config <(
>     echo CONFIG_TCG_TPM=y
>     echo CONFIG_HW_RANDOM=m
>   )
>   make KCFLAGS="-fno-inline-small-functions -fno-inline-functions-called-once"
> 
> Then I get a link error:

"This results a link error:"

> 
>   ld: vmlinux.o: in function `tpm_add_hwrng':
>   tpm-chip.c:(.text+0x6c5924): undefined reference to `hwrng_register'
>   ld: vmlinux.o: in function `tpm_chip_unregister':
>   (.text+0x6c5bc9): undefined reference to `hwrng_unregister'
>   ld: vmlinux.o: in function `tpm_chip_register':
>   (.text+0x6c5c9b): undefined reference to `hwrng_unregister'

The resolution is lacking i.e., why adding __always_inline addresses
the linking problem.

> 
> Signed-off-by: Winston Wen <[e-mail]>
> Co-Developed-by: Chen Linxuan <[e-mail]>
> Signed-off-by: Chen Linxuan <[e-mail]>
> ---
>  drivers/char/tpm/tpm-chip.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index e25daf2396d3..48cc74d84247 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -534,7 +534,7 @@ static int tpm_hwrng_read(struct hwrng *rng, void *data, size_t max, bool wait)
>  	return tpm_get_random(chip, data, max);
>  }
>  
> -static bool tpm_is_hwrng_enabled(struct tpm_chip *chip)
> +static __always_inline bool tpm_is_hwrng_enabled(struct tpm_chip *chip)
>  {
>  	if (!IS_ENABLED(CONFIG_HW_RANDOM_TPM))
>  		return false;
> -- 
> 2.48.1
> 

BR, Jarkko

Chen Linxuan April 15, 2025, 2:28 a.m. UTC | #2

Jarkko Sakkinen <[e-mail]> 于2025年4月12日周六 08:47写道：
>
> On Fri, Apr 11, 2025 at 06:54:54PM +0800, Chen Linxuan wrote:
> > From: Winston Wen <[e-mail]>
> >
> > On x86_64 with gcc version 13.3.0, I compile kernel with:
>
> Use passive:
>
> "Presume that kernel is compiled for x86_64 with gcc version 13.3.0:"
>
> >
> >   make defconfig
> >   ./scripts/kconfig/merge_config.sh .config <(
> >     echo CONFIG_TCG_TPM=y
> >     echo CONFIG_HW_RANDOM=m
> >   )
> >   make KCFLAGS="-fno-inline-small-functions -fno-inline-functions-called-once"
> >
> > Then I get a link error:
>
> "This results a link error:"
>

I will update commit message when I send v2.

> >
> >   ld: vmlinux.o: in function `tpm_add_hwrng':
> >   tpm-chip.c:(.text+0x6c5924): undefined reference to `hwrng_register'
> >   ld: vmlinux.o: in function `tpm_chip_unregister':
> >   (.text+0x6c5bc9): undefined reference to `hwrng_unregister'
> >   ld: vmlinux.o: in function `tpm_chip_register':
> >   (.text+0x6c5c9b): undefined reference to `hwrng_unregister'
>
> The resolution is lacking i.e., why adding __always_inline addresses
> the linking problem.

Regarding your comment about the resolution,
here’s a detailed explanation of
 why adding the `__always_inline` attribute addresses the linking issue:

With `CONFIG_TCG_TPM=y` and `CONFIG_HW_RANDOM=m`,
the functions `tpm_add_hwrng`, `tpm_chip_unregister`, and
`tpm_chip_register` are compiled into `vmlinux.o`
and reference the symbols `hwrng_register` and `hwrng_unregister`.
These symbols, however, are compiled into `rng-core.ko`, which results
in the linking error.

I am not sure but I think this weird linking error only arises when
auto inlining is disabled because of some dead code elimination.

`CONFIG_TCG_TPM=y` and `CONFIG_HW_RANDOM=m` set `CONFIG_HW_RANDOM_TPM=n`.
This causes the function `tpm_is_hwrng_enabled` to always return
`false`, as shown below:

```c
static bool tpm_is_hwrng_enabled(struct tpm_chip *chip)
{
    if (!IS_ENABLED(CONFIG_HW_RANDOM_TPM))
        return false;
    if (tpm_is_firmware_upgrade(chip))
        return false;
    if (chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED)
        return false;
    return true;
}
```

When `tpm_is_hwrng_enabled` is inlined, dead code elimination
optimizations are applied and the reference to the `hwrng_*` functions
will been removed.
For instance, in the `tpm_chip_unregister` function:

```c
void tpm_chip_unregister(struct tpm_chip *chip)
{
#ifdef CONFIG_TCG_TPM2_HMAC
    int rc;

    rc = tpm_try_get_ops(chip);
    if (!rc) {
        tpm2_end_auth_session(chip);
        tpm_put_ops(chip);
    }
#endif

    tpm_del_legacy_sysfs(chip);
    if (tpm_is_hwrng_enabled(chip))
        hwrng_unregister(&chip->hwrng);
    tpm_bios_log_teardown(chip);
    if (chip->flags & TPM_CHIP_FLAG_TPM2 && !tpm_is_firmware_upgrade(chip))
        tpm_devs_remove(chip);
    tpm_del_char_device(chip);
}
```

When `tpm_is_hwrng_enabled` is inlined and always returns `false`,
the call to `hwrng_unregister` is effectively part of a `if (false)` block,
which I guess that will be then optimized out.

However, when the `-fno-inline-small-functions` and
`-fno-inline-functions-called-once` flags are used,
tpm_is_hwrng_enabled is not inline.

And this optimization some how cannot occur,
leading to the undefined reference errors during linking.

Adding the `__always_inline` attribute ensures that
`tpm_is_hwrng_enabled` is inlined regardless of the compiler flags.
This allows the dead code elimination to proceed as expected,
resolving the linking issue.

There might be better ways to fix this.
But it is directly caused by adding `-fno-inline-small-functions` and
`-fno-inline-functions-called-once` flags,
I think add `__always_inline` is good enough.

>
> >
> > Signed-off-by: Winston Wen <[e-mail]>
> > Co-Developed-by: Chen Linxuan <[e-mail]>
> > Signed-off-by: Chen Linxuan <[e-mail]>
> > ---
> >  drivers/char/tpm/tpm-chip.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> > index e25daf2396d3..48cc74d84247 100644
> > --- a/drivers/char/tpm/tpm-chip.c
> > +++ b/drivers/char/tpm/tpm-chip.c
> > @@ -534,7 +534,7 @@ static int tpm_hwrng_read(struct hwrng *rng, void *data, size_t max, bool wait)
> >       return tpm_get_random(chip, data, max);
> >  }
> >
> > -static bool tpm_is_hwrng_enabled(struct tpm_chip *chip)
> > +static __always_inline bool tpm_is_hwrng_enabled(struct tpm_chip *chip)
> >  {
> >       if (!IS_ENABLED(CONFIG_HW_RANDOM_TPM))
> >               return false;
> > --
> > 2.48.1
> >
>
> BR, Jarkko
>
>

Patch

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index e25daf2396d3..48cc74d84247 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -534,7 +534,7 @@  static int tpm_hwrng_read(struct hwrng *rng, void *data, size_t max, bool wait)
 	return tpm_get_random(chip, data, max);
 }
 
-static bool tpm_is_hwrng_enabled(struct tpm_chip *chip)
+static __always_inline bool tpm_is_hwrng_enabled(struct tpm_chip *chip)
 {
 	if (!IS_ENABLED(CONFIG_HW_RANDOM_TPM))
 		return false;