Before sending a command to attempt the self test, the TPM
may need to be started, otherwise the self test returns
TPM2_RC_INITIALIZE value causing a log as follows:
"tpm tpm0: A TPM error (256) occurred attempting the self test".
Signed-off-by: Hermin Anggawijaya <[email protected]>
---
drivers/char/tpm/tpm2-cmd.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 93545be190a5..0530f3b5f86a 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -737,15 +737,15 @@ int tpm2_auto_startup(struct tpm_chip *chip)
if (rc)
goto out;
+ rc = tpm2_startup(chip);
+ if (rc && rc != TPM2_RC_INITIALIZE)
+ goto out;
+
rc = tpm2_do_selftest(chip);
if (rc && rc != TPM2_RC_INITIALIZE)
goto out;
if (rc == TPM2_RC_INITIALIZE) {
- rc = tpm2_startup(chip);
- if (rc)
- goto out;
-
rc = tpm2_do_selftest(chip);
if (rc)
goto out;
--
2.43.0
Dear Hermin,
Thank you for your patch.
It’d be great if you removed the dot/period from the end of the commit
message summary/title.
Am 22.11.23 um 07:55 schrieb Hermin Anggawijaya:
> Before sending a command to attempt the self test, the TPM
> may need to be started, otherwise the self test returns
> TPM2_RC_INITIALIZE value causing a log as follows:
> "tpm tpm0: A TPM error (256) occurred attempting the self test".
Please document on what platform this happens.
> Signed-off-by: Hermin Anggawijaya <[email protected]>
> ---
> drivers/char/tpm/tpm2-cmd.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index 93545be190a5..0530f3b5f86a 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -737,15 +737,15 @@ int tpm2_auto_startup(struct tpm_chip *chip)
> if (rc)
> goto out;
>
> + rc = tpm2_startup(chip);
> + if (rc && rc != TPM2_RC_INITIALIZE)
> + goto out;
> +
> rc = tpm2_do_selftest(chip);
> if (rc && rc != TPM2_RC_INITIALIZE)
> goto out;
>
> if (rc == TPM2_RC_INITIALIZE) {
> - rc = tpm2_startup(chip);
> - if (rc)
> - goto out;
> -
> rc = tpm2_do_selftest(chip);
> if (rc)
> goto out;
Kind regards,
Paul
On 11/22/23 01:55, Hermin Anggawijaya wrote:
> Before sending a command to attempt the self test, the TPM
> may need to be started, otherwise the self test returns
> TPM2_RC_INITIALIZE value causing a log as follows:
> "tpm tpm0: A TPM error (256) occurred attempting the self test".
>
> Signed-off-by: Hermin Anggawijaya <[email protected]>
> ---
> drivers/char/tpm/tpm2-cmd.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index 93545be190a5..0530f3b5f86a 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -737,15 +737,15 @@ int tpm2_auto_startup(struct tpm_chip *chip)
> if (rc)
> goto out;
>
> + rc = tpm2_startup(chip);
> + if (rc && rc != TPM2_RC_INITIALIZE)
> + goto out;
> +
Most platforms should have firmware initialize the TPM 2 these days.
Therefore, a selftest should work and in case it doesn't work you fall
back to the tpm2_startup below and if you get an error message in the
log you at least know that you firmware is not up-to-date.
> rc = tpm2_do_selftest(chip);
> if (rc && rc != TPM2_RC_INITIALIZE)
> goto out;
>
> if (rc == TPM2_RC_INITIALIZE) {
> - rc = tpm2_startup(chip);
> - if (rc)
> - goto out;
> -
> rc = tpm2_do_selftest(chip);
> if (rc)
> goto out;
On Wed Nov 22, 2023 at 8:55 AM EET, Hermin Anggawijaya wrote:
> Before sending a command to attempt the self test, the TPM
> may need to be started, otherwise the self test returns
> TPM2_RC_INITIALIZE value causing a log as follows:
> "tpm tpm0: A TPM error (256) occurred attempting the self test".
>
> Signed-off-by: Hermin Anggawijaya <[email protected]>
Firmware does TPM power on.
BR, Jarkko
On 22/11/2023 8:10 pm, Paul Menzel wrote:
>
> Dear Hermin,
>
>
> Thank you for your patch.
>
> It’d be great if you removed the dot/period from the end of the commit
> message summary/title.
>
> Am 22.11.23 um 07:55 schrieb Hermin Anggawijaya:
>> Before sending a command to attempt the self test, the TPM
>> may need to be started, otherwise the self test returns
>> TPM2_RC_INITIALIZE value causing a log as follows:
>> "tpm tpm0: A TPM error (256) occurred attempting the self test".
>
> Please document on what platform this happens.
>
>> Signed-off-by: Hermin Anggawijaya
>> <[email protected]>
>> ---
>> drivers/char/tpm/tpm2-cmd.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
>> index 93545be190a5..0530f3b5f86a 100644
>> --- a/drivers/char/tpm/tpm2-cmd.c
>> +++ b/drivers/char/tpm/tpm2-cmd.c
>> @@ -737,15 +737,15 @@ int tpm2_auto_startup(struct tpm_chip *chip)
>> if (rc)
>> goto out;
>> + rc = tpm2_startup(chip);
>> + if (rc && rc != TPM2_RC_INITIALIZE)
>> + goto out;
>> +
>> rc = tpm2_do_selftest(chip);
>> if (rc && rc != TPM2_RC_INITIALIZE)
>> goto out;
>> if (rc == TPM2_RC_INITIALIZE) {
>> - rc = tpm2_startup(chip);
>> - if (rc)
>> - goto out;
>> -
>> rc = tpm2_do_selftest(chip);
>> if (rc)
>> goto out;
>
>
> Kind regards,
>
> Paul
Hello Paul
Thank you for your comments.
>Please document on what platform this happens.
The error log message is observed on a custom hardware design (a router)
with an Infineon SLM 9670 TPM2.0.
The patch is useful for us, as the firmware (boot loader) of the router
does not support TPM yet, thus the kernel
needs to start the TPM before starting the self test.
I will reply to Jarkko's and Stefan's comments separately, and if in
principle, the patch is accepted, I will send the
second version of the patch with your comments addressed.
Kind regards
Hermin
On 23/11/2023 1:34 am, Stefan Berger wrote:
>
>
> On 11/22/23 01:55, Hermin Anggawijaya wrote:
>> Before sending a command to attempt the self test, the TPM
>> may need to be started, otherwise the self test returns
>> TPM2_RC_INITIALIZE value causing a log as follows:
>> "tpm tpm0: A TPM error (256) occurred attempting the self test".
>>
>> Signed-off-by: Hermin Anggawijaya
>> <[email protected]>
>> ---
>> drivers/char/tpm/tpm2-cmd.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
>> index 93545be190a5..0530f3b5f86a 100644
>> --- a/drivers/char/tpm/tpm2-cmd.c
>> +++ b/drivers/char/tpm/tpm2-cmd.c
>> @@ -737,15 +737,15 @@ int tpm2_auto_startup(struct tpm_chip *chip)
>> if (rc)
>> goto out;
>> + rc = tpm2_startup(chip);
>> + if (rc && rc != TPM2_RC_INITIALIZE)
>> + goto out;
>> +
>
> Most platforms should have firmware initialize the TPM 2 these days.
> Therefore, a selftest should work and in case it doesn't work you fall
> back to the tpm2_startup below and if you get an error message in the
> log you at least know that you firmware is not up-to-date.
>
>> rc = tpm2_do_selftest(chip);
>> if (rc && rc != TPM2_RC_INITIALIZE)
>> goto out;
>> if (rc == TPM2_RC_INITIALIZE) {
>> - rc = tpm2_startup(chip);
>> - if (rc)
>> - goto out;
>> -
>> rc = tpm2_do_selftest(chip);
>> if (rc)
>> goto out;
Hello Stefan
Thank you for your comments.
Unfortunately our platforms (custom hardware design) are the ones which
do not initialize/start the TPM2 from boot loader yet, and because of
that the
self test in tpm2_auto_startup always produce a log error message on the
platform start up.
While I understand your point about the log being useful for "pointing
out not up-to-date firmware", but it might also generate unnecessary support
queries from some users on such platforms ? And maybe the kernel being
able to deal with TPM being started more than once is better ?
If wanted, I have the second version of the patch which consist of code
changes as in v1, plus ability for tpm2_transmit_cmd to handle multiple
attempts to start up the TPM silently, for example, once by the firmware
and another by the kernel during tpm2 auto-startup.
Kind regards
Hermin Anggawijaya
On 24/11/2023 2:42 pm, Jarkko Sakkinen wrote:
> On Wed Nov 22, 2023 at 8:55 AM EET, Hermin Anggawijaya wrote:
>> Before sending a command to attempt the self test, the TPM
>> may need to be started, otherwise the self test returns
>> TPM2_RC_INITIALIZE value causing a log as follows:
>> "tpm tpm0: A TPM error (256) occurred attempting the self test".
>>
>> Signed-off-by: Hermin Anggawijaya <[email protected]>
> Firmware does TPM power on.
>
> BR, Jarkko
Hello Jarkko
Thank you for your comment on the patch. As indicated in my previous
reply to Stefan's comment,
I have v2 version of the patch which also deals with multiple attempts
to start up the TPM gracefully,
for example, once by the firmware and another by the kernel during tpm2
auto-startup.
If you think the idea is OK, I can send the v2 of the patch.
Kind regards
Hermin
On Mon Nov 27, 2023 at 4:02 AM EET, Angga wrote:
> On 23/11/2023 1:34 am, Stefan Berger wrote:
> >
> >
> > On 11/22/23 01:55, Hermin Anggawijaya wrote:
> >> Before sending a command to attempt the self test, the TPM
> >> may need to be started, otherwise the self test returns
> >> TPM2_RC_INITIALIZE value causing a log as follows:
> >> "tpm tpm0: A TPM error (256) occurred attempting the self test".
> >>
> >> Signed-off-by: Hermin Anggawijaya
> >> <[email protected]>
> >> ---
> >> drivers/char/tpm/tpm2-cmd.c | 8 ++++----
> >> 1 file changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> >> index 93545be190a5..0530f3b5f86a 100644
> >> --- a/drivers/char/tpm/tpm2-cmd.c
> >> +++ b/drivers/char/tpm/tpm2-cmd.c
> >> @@ -737,15 +737,15 @@ int tpm2_auto_startup(struct tpm_chip *chip)
> >> if (rc)
> >> goto out;
> >> + rc = tpm2_startup(chip);
> >> + if (rc && rc != TPM2_RC_INITIALIZE)
> >> + goto out;
> >> +
> >
> > Most platforms should have firmware initialize the TPM 2 these days.
> > Therefore, a selftest should work and in case it doesn't work you fall
> > back to the tpm2_startup below and if you get an error message in the
> > log you at least know that you firmware is not up-to-date.
> >
> >> rc = tpm2_do_selftest(chip);
> >> if (rc && rc != TPM2_RC_INITIALIZE)
> >> goto out;
> >> if (rc == TPM2_RC_INITIALIZE) {
> >> - rc = tpm2_startup(chip);
> >> - if (rc)
> >> - goto out;
> >> -
> >> rc = tpm2_do_selftest(chip);
> >> if (rc)
> >> goto out;
>
> Hello Stefan
>
> Thank you for your comments.
>
> Unfortunately our platforms (custom hardware design) are the ones which
> do not initialize/start the TPM2 from boot loader yet, and because of
> that the
> self test in tpm2_auto_startup always produce a log error message on the
> platform start up.
>
> While I understand your point about the log being useful for "pointing
> out not up-to-date firmware", but it might also generate unnecessary support
> queries from some users on such platforms ? And maybe the kernel being
> able to deal with TPM being started more than once is better ?
>
> If wanted, I have the second version of the patch which consist of code
> changes as in v1, plus ability for tpm2_transmit_cmd to handle multiple
> attempts to start up the TPM silently, for example, once by the firmware
> and another by the kernel during tpm2 auto-startup.
To save your time: no.
Mainline kernel is not modified based hardware prototypes.
You have freedom to maintain your own kernel tree for whatever changes
you need but this is totally wrong place for these type of patches.
>
> Kind regards
>
> Hermin Anggawijaya
BR, Jarkko