Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36027 )
Change subject: nb/intel/nehalem: Add a VBOOT TPM init workaround ......................................................................
nb/intel/nehalem: Add a VBOOT TPM init workaround
nb/intel/nehalem needs to issue a CPU reset during the raminit. On this reset the platform is not reset however, which includes the TPM. To avoid initializing the TPM twice add an function to inform VBOOT to skip the TPM initialization.
Change-Id: I238b30866f78608c414de877b05a73cf8fdb9bbd Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/northbridge/intel/nehalem/Kconfig M src/northbridge/intel/nehalem/Makefile.inc M src/northbridge/intel/nehalem/raminit.c A src/northbridge/intel/nehalem/vboot_quirk.c M src/security/vboot/Kconfig M src/security/vboot/tpm_common.c M src/security/vboot/tpm_common.h 7 files changed, 49 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/36027/1
diff --git a/src/northbridge/intel/nehalem/Kconfig b/src/northbridge/intel/nehalem/Kconfig index a88d4c9..6afcbc5 100644 --- a/src/northbridge/intel/nehalem/Kconfig +++ b/src/northbridge/intel/nehalem/Kconfig @@ -29,6 +29,7 @@ select VBOOT_MUST_REQUEST_DISPLAY select VBOOT_STARTS_IN_BOOTBLOCK select VBOOT_SEPARATE_VERSTAGE + select VBOOT_ROMSTAGE_CPU_RESET_QUIRK_WORKAROUND
config MMCONF_BUS_NUMBER int diff --git a/src/northbridge/intel/nehalem/Makefile.inc b/src/northbridge/intel/nehalem/Makefile.inc index 225f0ce..eef49e2 100644 --- a/src/northbridge/intel/nehalem/Makefile.inc +++ b/src/northbridge/intel/nehalem/Makefile.inc @@ -17,6 +17,8 @@
bootblock-y += bootblock.c
+verstage-y += vboot_quirk.c + ramstage-y += memmap.c ramstage-y += northbridge.c ramstage-y += smi.c diff --git a/src/northbridge/intel/nehalem/raminit.c b/src/northbridge/intel/nehalem/raminit.c index b9d407a..8f4eee5 100644 --- a/src/northbridge/intel/nehalem/raminit.c +++ b/src/northbridge/intel/nehalem/raminit.c @@ -4241,6 +4241,7 @@ if (x2ca8 == 0) { MCHBAR8_AND(0x2ca8, ~3); MCHBAR8(0x2ca8) = MCHBAR8(0x2ca8) + 4; // "+" or "|"? + /* This issues a CPU reset without resetting the platform */ MCHBAR32_OR(0x1af0, 0x10); halt(); } diff --git a/src/northbridge/intel/nehalem/vboot_quirk.c b/src/northbridge/intel/nehalem/vboot_quirk.c new file mode 100644 index 0000000..aa04916 --- /dev/null +++ b/src/northbridge/intel/nehalem/vboot_quirk.c @@ -0,0 +1,29 @@ +/* + * This file is part of the coreboot project. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <security/vboot/tpm_common.h> +#include "nehalem.h" + +/* + * The raminit needs to issue a CPU reset. On this reset the platform is not + * reset however, which includes the TPM. To avoid initializing the TPM twice + * this informs VBOOT to skip the TPM initialization. The raminit uses the + * MCHBAR8(0x2ca8) sticky scratchpad register to keep track if it's on the first + * or second boot. + */ +int cpu_was_reset(void) +{ + if (MCHBAR16(0x2ca8) != 0) + return 1; + return 0; +} diff --git a/src/security/vboot/Kconfig b/src/security/vboot/Kconfig index d6d74ca..cf15d86 100644 --- a/src/security/vboot/Kconfig +++ b/src/security/vboot/Kconfig @@ -139,6 +139,13 @@ or the romstage and runs as part of that stage (cf. related options VBOOT_STARTS_IN_BOOTBLOCK/_ROMSTAGE and VBOOT_RETURN_FROM_VERSTAGE).
+config VBOOT_ROMSTAGE_CPU_RESET_QUIRK_WORKAROUND + bool + help + Some platforms need to reset the CPU, but not the platform as part of + their initialization. The result is that the TPM is already initialized + when VBOOT runs the second time. Select this to workaround this. + config VBOOT_RETURN_FROM_VERSTAGE bool default n diff --git a/src/security/vboot/tpm_common.c b/src/security/vboot/tpm_common.c index 0a211c5..232aea3 100644 --- a/src/security/vboot/tpm_common.c +++ b/src/security/vboot/tpm_common.c @@ -23,6 +23,10 @@ { uint32_t result;
+ if (CONFIG(VBOOT_ROMSTAGE_CPU_RESET_QUIRK_WORKAROUND)) + if (cpu_was_reset()) + return TPM_SUCCESS; + result = tpm_setup(ctx->flags & VB2_CONTEXT_S3_RESUME); if (result == TPM_E_MUST_REBOOT) ctx->flags |= VB2_CONTEXT_SECDATA_WANTS_REBOOT; diff --git a/src/security/vboot/tpm_common.h b/src/security/vboot/tpm_common.h index e1faa0c..e913c78 100644 --- a/src/security/vboot/tpm_common.h +++ b/src/security/vboot/tpm_common.h @@ -13,6 +13,9 @@
#if CONFIG(TPM1) || CONFIG(TPM2)
+#include <stdint.h> +#include <vb2_api.h> + /* Start of the root of trust */ uint32_t vboot_setup_tpm(struct vb2_context *ctx);
@@ -20,6 +23,8 @@ vb2_error_t vboot_extend_pcr(struct vb2_context *ctx, int pcr, enum vb2_pcr_digest which_digest);
+int cpu_was_reset(void); + #else
#define vboot_setup_tpm(ctx) 0
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36027 )
Change subject: nb/intel/nehalem: Add a VBOOT TPM init workaround ......................................................................
Patch Set 1:
(3 comments)
Please add the downside of initializing the TPM twice. Just waste of time or other problems?
https://review.coreboot.org/c/coreboot/+/36027/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36027/1//COMMIT_MSG@11 PS1, Line 11: an a
https://review.coreboot.org/c/coreboot/+/36027/1/src/northbridge/intel/nehal... File src/northbridge/intel/nehalem/vboot_quirk.c:
https://review.coreboot.org/c/coreboot/+/36027/1/src/northbridge/intel/nehal... PS1, Line 28: 0 CB_SUCCESS
https://review.coreboot.org/c/coreboot/+/36027/1/src/security/vboot/Kconfig File src/security/vboot/Kconfig:
https://review.coreboot.org/c/coreboot/+/36027/1/src/security/vboot/Kconfig@... PS1, Line 147: workaround work around
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36027 )
Change subject: nb/intel/nehalem: Add a VBOOT TPM init workaround ......................................................................
Patch Set 1:
Patch Set 1:
(3 comments)
Please add the downside of initializing the TPM twice. Just waste of time or other problems?
It seems a TPM can only be initialized once.
Hello Patrick Rudolph, Aaron Durbin, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36027
to look at the new patch set (#2).
Change subject: nb/intel/nehalem: Add a VBOOT TPM init workaround ......................................................................
nb/intel/nehalem: Add a VBOOT TPM init workaround
nb/intel/nehalem needs to issue a CPU reset during the raminit. On this reset the platform is not reset however, which includes the TPM. To avoid initializing the TPM twice, since it will fail and mess up VBOOT the second time, add an function to inform VBOOT to skip the TPM initialization.
Change-Id: I238b30866f78608c414de877b05a73cf8fdb9bbd Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/northbridge/intel/nehalem/Kconfig M src/northbridge/intel/nehalem/Makefile.inc M src/northbridge/intel/nehalem/raminit.c A src/northbridge/intel/nehalem/vboot_quirk.c M src/security/vboot/Kconfig M src/security/vboot/tpm_common.c M src/security/vboot/tpm_common.h 7 files changed, 50 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/36027/2
Hello Patrick Rudolph, Aaron Durbin, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36027
to look at the new patch set (#3).
Change subject: nb/intel/nehalem: Add a VBOOT TPM init workaround ......................................................................
nb/intel/nehalem: Add a VBOOT TPM init workaround
nb/intel/nehalem needs to issue a CPU reset during the raminit. On this reset the platform is not reset however, which includes the TPM. To avoid initializing the TPM twice, since it will fail and mess up VBOOT the second time, add a function to inform VBOOT to skip the TPM initialization.
Change-Id: I238b30866f78608c414de877b05a73cf8fdb9bbd Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/northbridge/intel/nehalem/Kconfig M src/northbridge/intel/nehalem/Makefile.inc M src/northbridge/intel/nehalem/raminit.c A src/northbridge/intel/nehalem/vboot_quirk.c M src/security/vboot/Kconfig M src/security/vboot/tpm_common.c M src/security/vboot/tpm_common.h 7 files changed, 50 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/36027/3
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36027 )
Change subject: nb/intel/nehalem: Add a VBOOT TPM init workaround ......................................................................
Patch Set 3:
(3 comments)
https://review.coreboot.org/c/coreboot/+/36027/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36027/1//COMMIT_MSG@11 PS1, Line 11: an
a
Done
https://review.coreboot.org/c/coreboot/+/36027/1/src/northbridge/intel/nehal... File src/northbridge/intel/nehalem/vboot_quirk.c:
https://review.coreboot.org/c/coreboot/+/36027/1/src/northbridge/intel/nehal... PS1, Line 28: 0
CB_SUCCESS
It's not an error or succes, it just reflects the state of the system. So 0 and 1 are fine I think?
https://review.coreboot.org/c/coreboot/+/36027/1/src/security/vboot/Kconfig File src/security/vboot/Kconfig:
https://review.coreboot.org/c/coreboot/+/36027/1/src/security/vboot/Kconfig@... PS1, Line 147: workaround
work around
Done
Hello Patrick Rudolph, Aaron Durbin, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36027
to look at the new patch set (#4).
Change subject: nb/intel/nehalem: Add a VBOOT TPM init workaround ......................................................................
nb/intel/nehalem: Add a VBOOT TPM init workaround
nb/intel/nehalem needs to issue a CPU reset during the raminit. On this reset the platform is not reset however, which includes the TPM. To avoid initializing the TPM twice, since it will fail and mess up VBOOT the second time, add a function to inform VBOOT to skip the TPM initialization.
Change-Id: I238b30866f78608c414de877b05a73cf8fdb9bbd Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/northbridge/intel/nehalem/Kconfig M src/northbridge/intel/nehalem/Makefile.inc M src/northbridge/intel/nehalem/bootblock.c M src/northbridge/intel/nehalem/raminit.c A src/northbridge/intel/nehalem/vboot_quirk.c M src/security/vboot/Kconfig M src/security/vboot/tpm_common.c M src/security/vboot/tpm_common.h 8 files changed, 56 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/36027/4
Hello Patrick Rudolph, Aaron Durbin, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36027
to look at the new patch set (#5).
Change subject: nb/intel/nehalem: Add a VBOOT TPM init workaround ......................................................................
nb/intel/nehalem: Add a VBOOT TPM init workaround
nb/intel/nehalem needs to issue a CPU reset during the raminit. On this reset the platform is not reset however, which includes the TPM. To avoid initializing the TPM twice, since it will fail and mess up VBOOT the second time, add a function to inform VBOOT to skip the TPM initialization.
Change-Id: I238b30866f78608c414de877b05a73cf8fdb9bbd Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/northbridge/intel/nehalem/Kconfig M src/northbridge/intel/nehalem/Makefile.inc M src/northbridge/intel/nehalem/bootblock.c A src/northbridge/intel/nehalem/vboot_quirk.c M src/security/vboot/Kconfig M src/security/vboot/tpm_common.c M src/security/vboot/tpm_common.h 7 files changed, 55 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/36027/5
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36027 )
Change subject: nb/intel/nehalem: Add a VBOOT TPM init workaround ......................................................................
Patch Set 5:
(1 comment)
Something similar is needed for Intel TXT boot, as the TPM is initialized before CPU reset. Is there a way to make it less platform specific? @Google What do you think about this?
https://review.coreboot.org/c/coreboot/+/36027/5/src/northbridge/intel/nehal... File src/northbridge/intel/nehalem/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36027/5/src/northbridge/intel/nehal... PS5, Line 23: pci_write_config32(PCI_DEV(0, 0x00, 0), MCHBAR, (uintptr_t)DEFAULT_MCHBAR | 1); mention why it's done here
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36027 )
Change subject: nb/intel/nehalem: Add a VBOOT TPM init workaround ......................................................................
Patch Set 5:
(1 comment)
Patch Set 5:
(1 comment)
Something similar is needed for Intel TXT boot, as the TPM is initialized before CPU reset. Is there a way to make it less platform specific? @Google What do you think about this?
If there is a way to probe if the TPM is already initialized I'm all ears. I could rename the Kconfig hook to something like VBOOT_TPM_POSSIBLY_INITIALIZED with a hook 'int is_tpm_initialized(void)' ?
https://review.coreboot.org/c/coreboot/+/36027/5/src/northbridge/intel/nehal... File src/northbridge/intel/nehalem/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36027/5/src/northbridge/intel/nehal... PS5, Line 23: pci_write_config32(PCI_DEV(0, 0x00, 0), MCHBAR, (uintptr_t)DEFAULT_MCHBAR | 1);
mention why it's done here
Ok.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36027 )
Change subject: nb/intel/nehalem: Add a VBOOT TPM init workaround ......................................................................
Patch Set 5: Code-Review-1
This is really undermining basic assumptions in vboot (e.g. that the firmware TPM NVRAM space is always writable during firmware verification, so I would rather not go in this direction, even behind a Kconfig. One of the vboot platform requirements is that the platform can reset itself, including the TPM. If a platform can't do that, then it can't fully run vboot (it should set MOCK_SECDATA instead).
Can you explain more about why/when this reset is needed? Why can't you just reset the whole board (including TPM) in that case instead?
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36027 )
Change subject: nb/intel/nehalem: Add a VBOOT TPM init workaround ......................................................................
Patch Set 5:
Patch Set 5: Code-Review-1
This is really undermining basic assumptions in vboot (e.g. that the firmware TPM NVRAM space is always writable during firmware verification, so I would rather not go in this direction, even behind a Kconfig. One of the vboot platform requirements is that the platform can reset itself, including the TPM. If a platform can't do that, then it can't fully run vboot (it should set MOCK_SECDATA instead).
Setting MOCK_SECDATA is currently not an option with TPM enabled as even with MOCK_SECDATA a different path will be taken on TPM init failure. A new Kconfig option NO_TPM_IN_VBOOT could be added to work around it maybe?
This is just a way to skip the init the second time VBOOT runs after the soft reset as the init command returns failure if already initialized. Another way to work around this issue is to only run verstage after the reset and therefore running the RO romstage (partially) before reset.
The last option is to use VBOOT_STARTS_IN_ROMSTAGE, but that's my least preferred option as being able to update the romstage, even if compatibility is at stake with the option described above, is a big plus.
Can you explain more about why/when this reset is needed? Why can't you just reset the whole board (including TPM) in that case instead?
I don't have any useful documentation for this hardware in this regard and the raminit has been reversed engineered, with close to no useful comments. From what I can tell it needs to program some (PLL or other?) settings that only get updated/work after a special reset command that only resets the CPU. Using other reset types don't seem to do the trick and also reset those needed settings. Not issuing that reset also results in raminit failures. I doubt it is possible to work around it.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36027 )
Change subject: nb/intel/nehalem: Add a VBOOT TPM init workaround ......................................................................
Patch Set 5:
Setting MOCK_SECDATA is currently not an option with TPM enabled as even with MOCK_SECDATA a different path will be taken on TPM init failure. A new Kconfig option NO_TPM_IN_VBOOT could be added to work around it maybe?
Yes, maybe we should do that. In CB:34510 we just assumed that everyone who has TPM1 || TPM2 enabled in Kconfig would want the TPM initialization to happen in vboot. But maybe we should insert a separate VBOOT_USE_TPM option in-between to decouple that and allow platforms to override if necessary. (I assume just running completely without TPM support is not an option for your use case?)
This is just a way to skip the init the second time VBOOT runs after the soft reset as the init command returns failure if already initialized. Another way to work around this issue is to only run verstage after the reset and therefore running the RO romstage (partially) before reset.
This could be reasonable but do you have a clean way to do that? I don't think there's a way for platforms to override the vboot_locator that decides when to run vboot, and I don't think we'd want to add that either.
The last option is to use VBOOT_STARTS_IN_ROMSTAGE, but that's my least preferred option as being able to update the romstage, even if compatibility is at stake with the option described above, is a big plus.
I think this would honestly be one of the better options to solve this, sounds like that platform just doesn't work well with the early vboot model.
Another really hacky option might be to make sure vboot_platform_is_resuming() returns true for the second boot, and pretend it's an S3 resume. That's not any better than this patch, but at least it would contain those hacks within the platform.
Hello Naresh Solanki, Patrick Rudolph, Aaron Durbin, Julius Werner, Philipp Deppenwiese, build bot (Jenkins), Aaron Durbin, Joel Kitching, Furquan Shaikh, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36027
to look at the new patch set (#6).
Change subject: [WIP]nb/intel/nehalem: Add a VBOOT TPM init workaround ......................................................................
[WIP]nb/intel/nehalem: Add a VBOOT TPM init workaround
nb/intel/nehalem needs to issue a CPU reset during the raminit. On this reset the platform is not reset however, which includes the TPM. To avoid initializing the TPM twice, since it will fail and mess up VBOOT the second time, fake S3_resume on the second boot during verstage.
TODO the optional linking of pmbase.c is better done after CB:33200 is merged with a new Kconfig symbol.
Change-Id: I238b30866f78608c414de877b05a73cf8fdb9bbd Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/northbridge/intel/nehalem/Makefile.inc M src/northbridge/intel/nehalem/bootblock.c A src/northbridge/intel/nehalem/vboot_quirk.c M src/southbridge/intel/common/Makefile.inc 4 files changed, 46 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/36027/6
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36027 )
Change subject: [WIP]nb/intel/nehalem: Add a VBOOT TPM init workaround ......................................................................
Patch Set 6:
Patch Set 5:
Setting MOCK_SECDATA is currently not an option with TPM enabled as even with MOCK_SECDATA a different path will be taken on TPM init failure. A new Kconfig option NO_TPM_IN_VBOOT could be added to work around it maybe?
Yes, maybe we should do that. In CB:34510 we just assumed that everyone who has TPM1 || TPM2 enabled in Kconfig would want the TPM initialization to happen in vboot. But maybe we should insert a separate VBOOT_USE_TPM option in-between to decouple that and allow platforms to override if necessary. (I assume just running completely without TPM support is not an option for your use case?)
This might look like the most sane approach for now.
This is just a way to skip the init the second time VBOOT runs after the soft reset as the init command returns failure if already initialized. Another way to work around this issue is to only run verstage after the reset and therefore running the RO romstage (partially) before reset.
This could be reasonable but do you have a clean way to do that? I don't think there's a way for platforms to override the vboot_locator that decides when to run vboot, and I don't think we'd want to add that either.
Patrick also stated a somewhat similar use case where TXT already initializes the TPM before VBOOT. How unreasonable is it to provide a hook in the TPM init to deal tlcl_startup() failure if one knows the TPM has already been initialized? This is quite similar to what is done on the S3 path in tpm_setup_s3_helper(). This could be unified to deal with other situations where the TPM is known to be initialized prior to VBOOT?
Another really hacky option might be to make sure vboot_platform_is_resuming() returns true for the second boot, and pretend it's an S3 resume. That's not any better than this patch, but at least it would contain those hacks within the platform.
Your last suggestion seems to work but if one request recovery version checking should be disabled but this is not the case anymore, so this is actually worse than with the quirk handling.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36027 )
Change subject: [WIP]nb/intel/nehalem: Add a VBOOT TPM init workaround ......................................................................
Patch Set 6:
Patrick also stated a somewhat similar use case where TXT already initializes the TPM before VBOOT. How unreasonable is it to provide a hook in the TPM init to deal tlcl_startup() failure if one knows the TPM has already been initialized? This is quite similar to what is done on the S3 path in tpm_setup_s3_helper(). This could be unified to deal with other situations where the TPM is known to be initialized prior to VBOOT?
I guess if you tried to sell it as a TPM library option rather than a vboot hack, that makes it look more reasonable. But then are we just going to skip the tlcl_startup() entirely, or are we going to send it but ignore errors? The former seems cleaner for the TXT case but wouldn't work for you. If they're okay doing the latter for TXT, I guess we can do that (and add an option like TPM_STARTUP_IGNORE_POSTINIT).
Another really hacky option might be to make sure vboot_platform_is_resuming() returns true for the second boot, and pretend it's an S3 resume. That's not any better than this patch, but at least it would contain those hacks within the platform.
Your last suggestion seems to work but if one request recovery version checking should be disabled but this is not the case anymore, so this is actually worse than with the quirk handling.
I don't really understand what you're saying here, but it sounds like it's not worth trying that direction.
Hello Naresh Solanki, Patrick Rudolph, Aaron Durbin, Julius Werner, Philipp Deppenwiese, build bot (Jenkins), Joel Kitching, Furquan Shaikh, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36027
to look at the new patch set (#7).
Change subject: security/tpm: Add a Kconfig to disregard INVALID_POSTINIT on startup ......................................................................
security/tpm: Add a Kconfig to disregard INVALID_POSTINIT on startup
There are use cases where TPM has already been set up in a previous stage, e.g. TXT or when a CPU reset without a platform reset happens. If this is the case the TPM startup will return a INVALID_POSTINIT (return code 0x26). This adds a Kconfig to allow platforms to disregard that return code.
Change-Id: I238b30866f78608c414de877b05a73cf8fdb9bbd Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/security/tpm/Kconfig M src/security/tpm/tspi/tspi.c 2 files changed, 13 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/36027/7
Hello Naresh Solanki, Patrick Rudolph, Aaron Durbin, Julius Werner, Philipp Deppenwiese, build bot (Jenkins), Joel Kitching, Furquan Shaikh, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36027
to look at the new patch set (#8).
Change subject: security/tpm: Add a Kconfig to disregard INVALID_POSTINIT on startup ......................................................................
security/tpm: Add a Kconfig to disregard INVALID_POSTINIT on startup
There are use cases where TPM has already been set up in a previous stage, e.g. TXT or when a CPU reset without a platform reset happens. If this is the case the TPM startup will return a INVALID_POSTINIT (return code 0x26). This adds a Kconfig to allow platforms to disregard that return code.
Change-Id: I238b30866f78608c414de877b05a73cf8fdb9bbd Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/security/tpm/Kconfig M src/security/tpm/tspi/tspi.c 2 files changed, 14 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/36027/8
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36027 )
Change subject: security/tpm: Add a Kconfig to disregard INVALID_POSTINIT on startup ......................................................................
Patch Set 8: Code-Review+1
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36027 )
Change subject: security/tpm: Add a Kconfig to disregard INVALID_POSTINIT on startup ......................................................................
Patch Set 8: Code-Review+2
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36027 )
Change subject: security/tpm: Add a Kconfig to disregard INVALID_POSTINIT on startup ......................................................................
Patch Set 8:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36027/5/src/northbridge/intel/nehal... File src/northbridge/intel/nehalem/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36027/5/src/northbridge/intel/nehal... PS5, Line 23: pci_write_config32(PCI_DEV(0, 0x00, 0), MCHBAR, (uintptr_t)DEFAULT_MCHBAR | 1);
mention why it's done here […]
latest patch set doesn't do it anymore
https://review.coreboot.org/c/coreboot/+/36027/1/src/northbridge/intel/nehal... File src/northbridge/intel/nehalem/vboot_quirk.c:
https://review.coreboot.org/c/coreboot/+/36027/1/src/northbridge/intel/nehal... PS1, Line 28: 0
CB_SUCCESS […]
not done in this CL at all
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/36027 )
Change subject: security/tpm: Add a Kconfig to disregard INVALID_POSTINIT on startup ......................................................................
security/tpm: Add a Kconfig to disregard INVALID_POSTINIT on startup
There are use cases where TPM has already been set up in a previous stage, e.g. TXT or when a CPU reset without a platform reset happens. If this is the case the TPM startup will return a INVALID_POSTINIT (return code 0x26). This adds a Kconfig to allow platforms to disregard that return code.
Change-Id: I238b30866f78608c414de877b05a73cf8fdb9bbd Signed-off-by: Arthur Heymans arthur@aheymans.xyz Reviewed-on: https://review.coreboot.org/c/coreboot/+/36027 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net Reviewed-by: Julius Werner jwerner@chromium.org --- M src/security/tpm/Kconfig M src/security/tpm/tspi/tspi.c 2 files changed, 14 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Julius Werner: Looks good to me, approved
diff --git a/src/security/tpm/Kconfig b/src/security/tpm/Kconfig index 3af6d69..95c0bb9 100644 --- a/src/security/tpm/Kconfig +++ b/src/security/tpm/Kconfig @@ -93,4 +93,13 @@ to work around a race-condition-related issue, possibly caused by ill-programmed TPM firmware.
+config TPM_STARTUP_IGNORE_POSTINIT + bool + help + Select this to ignore POSTINIT INVALID return codes on TPM + startup. This is useful on platforms where a previous stage + issued a TPM startup. Examples of use cases are Intel TXT + or VBOOT on the Intel Nehalem northbridge which issues a + CPU-only reset during the romstage. + endmenu # Trusted Platform Module (tpm) diff --git a/src/security/tpm/tspi/tspi.c b/src/security/tpm/tspi/tspi.c index 4698a4d..966b8b7 100644 --- a/src/security/tpm/tspi/tspi.c +++ b/src/security/tpm/tspi/tspi.c @@ -141,6 +141,11 @@ }
result = tlcl_startup(); + if (CONFIG(TPM_STARTUP_IGNORE_POSTINIT) + && result == TPM_E_INVALID_POSTINIT) { + printk(BIOS_DEBUG, "TPM: ignoring invalid POSTINIT\n"); + result = TPM_SUCCESS; + } if (result != TPM_SUCCESS) { printk(BIOS_ERR, "TPM: Can't run startup command.\n"); return tpm_setup_epilogue(result);
-
Arthur Heymans (Code Review) -
Julius Werner (Code Review)
-
Patrick Georgi (Code Review)
-
Patrick Rudolph (Code Review)
-
Paul Menzel (Code Review)