coreboot-gerrit November 2022

coreboot-gerrit@coreboot.org

1 participants
7277 discussions

[S] Change in coreboot[master]: soc/amd/common: Only call into enabled memory types
by Angel Pons (Code Review) 02 Nov '22

02 Nov '22

Attention is currently required from: Jason Glenesk, Raul Rangel, Martin L Roth, Matt DeVillier, Fred Reitberger, Felix Held. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68993 ) Change subject: soc/amd/common: Only call into enabled memory types ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/68993 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie239039b3dd2b5d0a6f8e9230fd3466bb8309761 Gerrit-Change-Number: 68993 Gerrit-PatchSet: 1 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Wed, 02 Nov 2022 13:37:28 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: device/dram: Add kconfig options for memory types
by Angel Pons (Code Review) 02 Nov '22

02 Nov '22

Attention is currently required from: Martin L Roth. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68992 ) Change subject: device/dram: Add kconfig options for memory types ...................................................................... Patch Set 1: Code-Review+1 (1 comment) File src/device/dram/Kconfig: https://review.coreboot.org/c/coreboot/+/68992/comment/48a8daec_f9943c16 PS1, Line 3: # Short-term plan: Start adding 'USE_' and "NO_" options to each chip. : # : # Long-term plan: Every SoC or chipset should select the memory types they : # use. When they all select their memory, the 'no_' options can be removed : # and the defaults for all memory types can be set to n. : : config NO_DDR5 : bool : : config NO_LPDDR4 : bool : : config NO_DDR4 : bool : : config NO_DDR3 : bool : : config NO_DDR2 : bool OK, so the negative options are temporary. Got it. -- To view, visit https://review.coreboot.org/c/coreboot/+/68992 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I07c98a702e0d67c5ad7bd9b8a4ff24c9288ab569 Gerrit-Change-Number: 68992 Gerrit-PatchSet: 1 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Comment-Date: Wed, 02 Nov 2022 13:37:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: MAINTAINERS: Make Misc Fixes
by Angel Pons (Code Review) 02 Nov '22

02 Nov '22

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68709 ) Change subject: MAINTAINERS: Make Misc Fixes ...................................................................... Patch Set 5: (1 comment) File MAINTAINERS: https://review.coreboot.org/c/coreboot/+/68709/comment/219b55e5_712b9f4f PS2, Line 697: MAINTAINED > Yeah - it's hard. I'm not adding anyone's name who isn't volunteering for it. […] Makes sense, thanks for the explanation. -- To view, visit https://review.coreboot.org/c/coreboot/+/68709 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib6d47a8c34482c81ff96dbeec760852cba01dabc Gerrit-Change-Number: 68709 Gerrit-PatchSet: 5 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com> Gerrit-CC: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: ron minnich <rminnich(a)gmail.com> Gerrit-Comment-Date: Wed, 02 Nov 2022 13:36:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: cbmem_top_chipset: Change the return value to uintptr_t
by Elyes Haouas (Code Review) 02 Nov '22

02 Nov '22

Attention is currently required from: Hung-Te Lin, Philipp Hug, Jakub Czapiga, Matt DeVillier, Julius Werner, Andrey Petrov, ron minnich, Elyes Haouas, Felix Held. Hello Hung-Te Lin, Philipp Hug, Jakub Czapiga, Matt DeVillier, Angel Pons, Julius Werner, Andrey Petrov, ron minnich, Felix Held, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/69078 to look at the new patch set (#2). Change subject: cbmem_top_chipset: Change the return value to uintptr_t ...................................................................... cbmem_top_chipset: Change the return value to uintptr_t Get rid of a lot of casts. Change-Id: I93645ef5dd270905ce421e68e342aff4c331eae6 Signed-off-by: Elyes Haouas <ehaouas(a)noos.fr> --- M src/drivers/amd/agesa/romstage.c M src/drivers/intel/fsp2_0/cbmem.c M src/include/cbmem.h M src/lib/imd_cbmem.c M src/mainboard/emulation/qemu-aarch64/cbmem.c M src/mainboard/emulation/qemu-armv7/cbmem.c M src/mainboard/emulation/qemu-i440fx/memmap.c M src/mainboard/emulation/qemu-power8/cbmem.c M src/mainboard/emulation/qemu-power9/cbmem.c M src/northbridge/intel/gm45/memmap.c M src/northbridge/intel/haswell/memmap.c M src/northbridge/intel/i440bx/memmap.c M src/northbridge/intel/i945/memmap.c M src/northbridge/intel/ironlake/memmap.c M src/northbridge/intel/pineview/memmap.c M src/northbridge/intel/sandybridge/memmap.c M src/northbridge/intel/x4x/memmap.c M src/soc/amd/stoneyridge/memmap.c M src/soc/cavium/cn81xx/cbmem.c M src/soc/intel/baytrail/memmap.c M src/soc/intel/braswell/memmap.c M src/soc/intel/broadwell/memmap.c M src/soc/mediatek/common/cbmem.c M src/soc/nvidia/tegra124/cbmem.c M src/soc/nvidia/tegra210/cbmem.c M src/soc/qualcomm/ipq40xx/cbmem.c M src/soc/qualcomm/ipq806x/cbmem.c M src/soc/qualcomm/qcs405/cbmem.c M src/soc/qualcomm/sc7180/cbmem.c M src/soc/qualcomm/sc7280/cbmem.c M src/soc/rockchip/common/cbmem.c M src/soc/samsung/exynos5250/cbmem.c M src/soc/samsung/exynos5420/cbmem.c M src/soc/sifive/fu540/cbmem.c M src/soc/ti/am335x/cbmem.c M src/soc/ucb/riscv/cbmem.c M tests/lib/coreboot_table-test.c M tests/lib/imd_cbmem-test.c 38 files changed, 89 insertions(+), 79 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/78/69078/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/69078 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I93645ef5dd270905ce421e68e342aff4c331eae6 Gerrit-Change-Number: 69078 Gerrit-PatchSet: 2 Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx> Gerrit-Reviewer: ron minnich <rminnich(a)gmail.com> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Philipp Hug <philipp(a)hug.cx> Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: ron minnich <rminnich(a)gmail.com> Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newpatchset

1 0

[M] Change in coreboot[master]: security/tpm: support compiling in multiple TPM drivers
by build bot (Jenkins) (Code Review) 02 Nov '22

02 Nov '22

Attention is currently required from: Michał Żygowski, Christian Walter, Julius Werner, Krystian Hebel. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69162 ) Change subject: security/tpm: support compiling in multiple TPM drivers ...................................................................... Patch Set 1: (2 comments) File src/security/tpm/tis.h: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-162139): https://review.coreboot.org/c/coreboot/+/69162/comment/7a4e6fa4_303a91dd PS1, Line 101: #define __tis_driver __attribute__((used, __section__(".rodata.tis_driver"))) Prefer __used over __attribute__((used)) Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-162139): https://review.coreboot.org/c/coreboot/+/69162/comment/7a619fca_b1bd57a0 PS1, Line 101: #define __tis_driver __attribute__((used, __section__(".rodata.tis_driver"))) Prefer __section(".rodata.tis_driver") over __attribute__((__section__(".rodata.tis_driver"))) -- To view, visit https://review.coreboot.org/c/coreboot/+/69162 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I44c5a1d825afe414c2f5c2c90f4cfe41ba9bef5f Gerrit-Change-Number: 69162 Gerrit-PatchSet: 1 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Comment-Date: Wed, 02 Nov 2022 13:29:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: security/tpm: support compiling in multiple TPM drivers
by Sergii Dmytruk (Code Review) 02 Nov '22

02 Nov '22

Attention is currently required from: Michał Żygowski, Christian Walter, Julius Werner, Krystian Hebel. Hello Michał Żygowski, Julius Werner, Krystian Hebel, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/69162 to review the following change. Change subject: security/tpm: support compiling in multiple TPM drivers ...................................................................... security/tpm: support compiling in multiple TPM drivers Starting from here CONFIG_TPM1 and CONFIG_TPM2 are no longer mutually exclusive. Change-Id: I44c5a1d825afe414c2f5c2c90f4cfe41ba9bef5f Ticket: https://ticket.coreboot.org/issues/433 Signed-off-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> --- M src/drivers/crb/tis.c M src/drivers/i2c/tpm/Makefile.inc M src/drivers/i2c/tpm/tis.c M src/drivers/i2c/tpm/tis_atmel.c M src/drivers/pc80/tpm/tis.c M src/drivers/spi/tpm/tis.c M src/lib/program.ld M src/security/tpm/Kconfig M src/security/tpm/tis.h M src/security/tpm/tss/tss.c 10 files changed, 61 insertions(+), 31 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/69162/1 diff --git a/src/drivers/crb/tis.c b/src/drivers/crb/tis.c index 812947d..d84f22d 100644 --- a/src/drivers/crb/tis.c +++ b/src/drivers/crb/tis.c @@ -46,7 +46,7 @@ return 0; } -tis_sendrecv_fn tis_probe(int *tpm_family) +static tis_sendrecv_fn crb_tis_probe(int *tpm_family) { struct tpm2_info info; @@ -72,6 +72,8 @@ return &crb_tpm_sendrecv; } +static const __tis_driver tis_probe_fn crb_tis_driver = crb_tis_probe; + static void crb_tpm_fill_ssdt(const struct device *dev) { const char *path = acpi_device_path(dev); diff --git a/src/drivers/i2c/tpm/Makefile.inc b/src/drivers/i2c/tpm/Makefile.inc index ae50f2b..8a13672 100644 --- a/src/drivers/i2c/tpm/Makefile.inc +++ b/src/drivers/i2c/tpm/Makefile.inc @@ -1,14 +1,9 @@ ifeq ($(CONFIG_TPM)$(CONFIG_I2C_TPM),yy) all-$(CONFIG_DRIVER_TIS_DEFAULT) += tis.c - -ifeq ($(CONFIG_TPM_ATMEL),y) -all-y += tis_atmel.c -else ifeq ($(CONFIG_TPM_GOOGLE),y) -all-y += cr50.c -else +all-$(CONFIG_TPM_ATMEL) += tis_atmel.c +all-$(CONFIG_TPM_GOOGLE) += cr50.c all-y += tpm.c -endif endif diff --git a/src/drivers/i2c/tpm/tis.c b/src/drivers/i2c/tpm/tis.c index 68e2a67..3d394b9 100644 --- a/src/drivers/i2c/tpm/tis.c +++ b/src/drivers/i2c/tpm/tis.c @@ -119,7 +119,7 @@ return 0; } -tis_sendrecv_fn tis_probe(int *tpm_family) +static tis_sendrecv_fn i2c_tis_probe(int *tpm_family) { if (tpm_vendor_probe(CONFIG_DRIVER_TPM_I2C_BUS, CONFIG_DRIVER_TPM_I2C_ADDR, tpm_family)) return NULL; @@ -136,3 +136,5 @@ return &i2c_tpm_sendrecv; } + +static const __tis_driver tis_probe_fn i2c_tis_driver = i2c_tis_probe; diff --git a/src/drivers/i2c/tpm/tis_atmel.c b/src/drivers/i2c/tpm/tis_atmel.c index b1a6ccd..58176a3 100644 --- a/src/drivers/i2c/tpm/tis_atmel.c +++ b/src/drivers/i2c/tpm/tis_atmel.c @@ -103,8 +103,10 @@ return 0; } -tis_sendrecv_fn tis_probe(int *tpm_family) +static tis_sendrecv_fn atmel_i2c_tis_probe(int *tpm_family) { *tpm_family = 1; return &i2c_tis_sendrecv; } + +static const __tis_driver tis_probe_fn atmel_i2c_tis_driver = atmel_i2c_tis_probe; diff --git a/src/drivers/pc80/tpm/tis.c b/src/drivers/pc80/tpm/tis.c index 937de43..b21191c 100644 --- a/src/drivers/pc80/tpm/tis.c +++ b/src/drivers/pc80/tpm/tis.c @@ -387,7 +387,7 @@ * Returns 0 on success (the device is found or was found during an earlier * invocation) or TPM_DRIVER_ERR if the device is not found. */ -static u32 pc80_tis_probe(int *tpm_family) +static u32 pc80_tpm_probe(int *tpm_family) { const char *device_name = "unknown"; const char *vendor_name = device_name; @@ -708,7 +708,7 @@ } /* - * tis_probe() + * pc80_tis_probe() * * Probe for the TPM device and set it up for use within locality 0. * @@ -716,9 +716,9 @@ * * Returns pointer to send-receive function on success or NULL on failure. */ -tis_sendrecv_fn tis_probe(int *tpm_family) +static tis_sendrecv_fn pc80_tis_probe(int *tpm_family) { - if (pc80_tis_probe(tpm_family)) + if (pc80_tpm_probe(tpm_family)) return NULL; if (pc80_tis_open()) @@ -727,6 +727,8 @@ return &pc80_tpm_sendrecv; } +static const __tis_driver tis_probe_fn pc80_tis_driver = pc80_tis_probe; + /* * tis_setup_interrupt() * diff --git a/src/drivers/spi/tpm/tis.c b/src/drivers/spi/tpm/tis.c index 44576a2..95aeb0c 100644 --- a/src/drivers/spi/tpm/tis.c +++ b/src/drivers/spi/tpm/tis.c @@ -40,7 +40,7 @@ return 0; } -tis_sendrecv_fn tis_probe(int *tpm_family) +static tis_sendrecv_fn spi_tis_probe(int *tpm_family) { struct spi_slave spi; struct tpm2_info info; @@ -65,3 +65,5 @@ return &tpm_sendrecv; } + +static const __tis_driver tis_probe_fn spi_tis_driver = spi_tis_probe; diff --git a/src/lib/program.ld b/src/lib/program.ld index 67f685f..e552d63 100644 --- a/src/lib/program.ld +++ b/src/lib/program.ld @@ -39,6 +39,12 @@ _ersbe_init_begin = .; RECORD_SIZE(rsbe_init_begin) + . = ALIGN(ARCH_POINTER_ALIGN_SIZE); + _tis_drivers = .; + KEEP(*(.rodata.tis_driver)); + _etis_drivers = .; + RECORD_SIZE(tis_drivers) + #if ENV_RAMSTAGE . = ALIGN(ARCH_POINTER_ALIGN_SIZE); _pci_drivers = .; diff --git a/src/security/tpm/Kconfig b/src/security/tpm/Kconfig index fc339a2..9c859f9 100644 --- a/src/security/tpm/Kconfig +++ b/src/security/tpm/Kconfig @@ -4,14 +4,9 @@ menu "Trusted Platform Module" -choice - prompt "Trusted Platform Module" - default TPM2 if MAINBOARD_HAS_TPM2 - default TPM1 if MAINBOARD_HAS_TPM1 - default NO_TPM - config NO_TPM - bool "No TPM" + bool + default y if !TPM1 && !TPM2 help No TPM support. Select this option if your system doesn't have a TPM, or if you don't want coreboot to communicate with your TPM in any way. @@ -21,19 +16,17 @@ config TPM1 bool "TPM 1.2" depends on I2C_TPM || MEMORY_MAPPED_TPM || SPI_TPM || CRB_TPM - depends on !MAINBOARD_HAS_TPM2 + default y if MAINBOARD_HAS_TPM1 help Select this option if your TPM uses the older TPM 1.2 protocol. config TPM2 bool "TPM 2.0" depends on I2C_TPM || MEMORY_MAPPED_TPM || SPI_TPM || CRB_TPM - depends on !MAINBOARD_HAS_TPM1 + default y if MAINBOARD_HAS_TPM2 help Select this option if your TPM uses the newer TPM 2.0 protocol. -endchoice - config TPM bool default y @@ -52,7 +45,7 @@ always uses the 2.0 protocol, and that it should be on by default. config TPM_DEACTIVATE - bool "Deactivate TPM" + bool "Deactivate TPM (for TPM1)" default n depends on !VBOOT depends on TPM1 diff --git a/src/security/tpm/tis.h b/src/security/tpm/tis.h index eab62bd..c4a2df7 100644 --- a/src/security/tpm/tis.h +++ b/src/security/tpm/tis.h @@ -48,15 +48,13 @@ size_t *recv_len); /* - * tis_probe() - * * Probe for the TPM device and set it up for use within locality 0. * * @tpm_family - pointer to int which is set to TPM family of the device (1 or 2) * * Returns pointer to send-receive function on success or NULL on failure. */ -tis_sendrecv_fn tis_probe(int *tpm_family); +typedef tis_sendrecv_fn (*tis_probe_fn)(int *tpm_family); /* TODO: This is supposed to be used only for Google TPM. Consider moving this to drivers/tpm/cr50.h. */ @@ -100,4 +98,11 @@ return ENV_SEPARATE_VERSTAGE || ENV_BOOTBLOCK || !CONFIG(VBOOT); } +#define __tis_driver __attribute__((used, __section__(".rodata.tis_driver"))) + +/** start of compile time generated tis driver array */ +extern tis_probe_fn _tis_drivers[]; +/** end of compile time generated tis driver array */ +extern tis_probe_fn _etis_drivers[]; + #endif /* TIS_H_ */ diff --git a/src/security/tpm/tss/tss.c b/src/security/tpm/tss/tss.c index 381c45b1..571de1a 100644 --- a/src/security/tpm/tss/tss.c +++ b/src/security/tpm/tss/tss.c @@ -18,12 +18,19 @@ /* Probe for TPM device and chose implementation based on the returned TPM family. */ uint32_t tlcl_lib_init(void) { + tis_probe_fn *tis_probe; tis_sendrecv_fn tis_sendrecv; if (tpm_family != 0) return VB2_SUCCESS; - tis_sendrecv = tis_probe(&tpm_family); + tis_sendrecv = NULL; + for (tis_probe = _tis_drivers; tis_probe != _etis_drivers; tis_probe++) { + tis_sendrecv = (*tis_probe)(&tpm_family); + if (tis_sendrecv != NULL) + break; + } + if (tis_sendrecv == NULL) { printk(BIOS_ERR, "%s: tis_probe returned error\n", __func__); return VB2_ERROR_UNKNOWN; -- To view, visit https://review.coreboot.org/c/coreboot/+/69162 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I44c5a1d825afe414c2f5c2c90f4cfe41ba9bef5f Gerrit-Change-Number: 69162 Gerrit-PatchSet: 1 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-MessageType: newchange

1 0

[M] Change in coreboot[master]: security/tpm: replace CONFIG(TPMx) checks with runtime check
by Sergii Dmytruk (Code Review) 02 Nov '22

02 Nov '22

Attention is currently required from: Lance Zhao, Michał Żygowski, Christian Walter, Tim Wawrzynczak, Julius Werner, Krystian Hebel, Yu-Ping Wu. Hello Michał Żygowski, Julius Werner, Krystian Hebel, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/69161 to review the following change. Change subject: security/tpm: replace CONFIG(TPMx) checks with runtime check ...................................................................... security/tpm: replace CONFIG(TPMx) checks with runtime check Change-Id: Id9cc25aad8d1d7bfad12b7a92059b1b3641bbfa9 Ticket: https://ticket.coreboot.org/issues/433 Signed-off-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> --- M src/acpi/acpi.c M src/drivers/crb/tis.c M src/drivers/pc80/tpm/tis.c M src/drivers/tpm/ppi.c M src/security/tpm/tspi/crtm.h M src/security/tpm/tspi/tspi.c M src/security/tpm/tss.h M src/security/tpm/tss/tss.c M src/security/vboot/secdata_tpm.c M src/security/vboot/tpm_common.c M src/vendorcode/google/chromeos/cse_board_reset.c M src/vendorcode/google/chromeos/tpm2.c 12 files changed, 108 insertions(+), 65 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/69161/1 diff --git a/src/acpi/acpi.c b/src/acpi/acpi.c index e211558..8f6019c 100644 --- a/src/acpi/acpi.c +++ b/src/acpi/acpi.c @@ -26,6 +26,7 @@ #include <device/mmio.h> #include <device/pci.h> #include <pc80/mc146818rtc.h> +#include <security/tpm/tss.h> #include <string.h> #include <types.h> #include <version.h> @@ -1781,7 +1782,7 @@ acpi_add_table(rsdp, mcfg); } - if (CONFIG(TPM1)) { + if (tlcl_get_family() == 1) { printk(BIOS_DEBUG, "ACPI: * TCPA\n"); tcpa = (acpi_tcpa_t *) current; acpi_create_tcpa(tcpa); @@ -1792,7 +1793,7 @@ } } - if (CONFIG(TPM2)) { + if (tlcl_get_family() == 2) { printk(BIOS_DEBUG, "ACPI: * TPM2\n"); tpm2 = (acpi_tpm2_t *) current; acpi_create_tpm2(tpm2); diff --git a/src/drivers/crb/tis.c b/src/drivers/crb/tis.c index 1955466..812947d 100644 --- a/src/drivers/crb/tis.c +++ b/src/drivers/crb/tis.c @@ -140,6 +140,9 @@ uint32_t fw_ver1, fw_ver2; uint8_t major_spec_ver, minor_spec_ver; + if (tlcl_get_family() == 1) + return get_smbios_data(dev, handle, current); + tpm2_get_info(&info); /* If any of these have invalid values, assume TPM not present or disabled */ diff --git a/src/drivers/pc80/tpm/tis.c b/src/drivers/pc80/tpm/tis.c index e4cd05b..937de43 100644 --- a/src/drivers/pc80/tpm/tis.c +++ b/src/drivers/pc80/tpm/tis.c @@ -20,6 +20,7 @@ #include <device/device.h> #include <console/console.h> #include <security/tpm/tis.h> +#include <security/tpm/tss.h> #include <device/pnp.h> #include <drivers/tpm/tpm_ppi.h> #include <timer.h> @@ -803,7 +804,7 @@ acpigen_write_scope(path); acpigen_write_device(acpi_device_name(dev)); - if (CONFIG(TPM2)) { + if (tlcl_get_family() == 2) { acpigen_write_name_string("_HID", "MSFT0101"); acpigen_write_name_string("_CID", "MSFT0101"); } else { diff --git a/src/drivers/tpm/ppi.c b/src/drivers/tpm/ppi.c index 3e74314..fdbdd8e 100644 --- a/src/drivers/tpm/ppi.c +++ b/src/drivers/tpm/ppi.c @@ -5,6 +5,7 @@ #include <acpi/acpi_device.h> #include <cbmem.h> #include <console/console.h> +#include <security/tpm/tss.h> #include "tpm_ppi.h" @@ -62,7 +63,7 @@ acpigen_write_store(); acpigen_emit_namestring("^FSUP"); acpigen_emit_byte(LOCAL2_OP); - acpigen_emit_byte(CONFIG(TPM1) ? ONE_OP : ZERO_OP); + acpigen_emit_byte(tlcl_get_family() == 1 ? ONE_OP : ZERO_OP); acpigen_emit_byte(LOCAL1_OP); acpigen_write_if_lequal_op_int(LOCAL1_OP, 0); @@ -84,7 +85,7 @@ acpigen_write_store(); acpigen_emit_namestring("^FSUP"); acpigen_emit_byte(LOCAL2_OP); - acpigen_emit_byte(CONFIG(TPM1) ? ZERO_OP : ONE_OP); + acpigen_emit_byte(tlcl_get_family() == 1 ? ZERO_OP : ONE_OP); acpigen_emit_byte(LOCAL1_OP); acpigen_write_if_lequal_op_int(LOCAL1_OP, 1); @@ -114,7 +115,7 @@ */ static void tpm_ppi_func1_cb(void *arg) { - if (CONFIG(TPM2)) + if (tlcl_get_family() == 2) /* Interface version: 1.3 */ acpigen_write_return_string("1.3"); else @@ -410,7 +411,7 @@ acpigen_write_store(); acpigen_emit_namestring("^FSUP"); acpigen_emit_byte(LOCAL2_OP); - acpigen_emit_byte(CONFIG(TPM1) ? ONE_OP : ZERO_OP); + acpigen_emit_byte(tlcl_get_family() == 1 ? ONE_OP : ZERO_OP); acpigen_emit_byte(LOCAL1_OP); acpigen_write_if_lequal_op_int(LOCAL1_OP, 0); acpigen_write_return_byte(0); /* Not implemented */ @@ -418,7 +419,7 @@ // FIXME: Only advertise supported functions - if (CONFIG(TPM1)) { + if (tlcl_get_family() == 1) { /* * Some functions do not require PP depending on configuration. * Those aren't listed here, so the 'required PP' is always set for those. @@ -434,7 +435,7 @@ acpigen_write_return_integer(PPI8_RET_ALLOWED); acpigen_pop_len(); /* Pop : If */ } - } else if (CONFIG(TPM2)) { + } else if (tlcl_get_family() == 2) { /* * Some functions do not require PP depending on configuration. * Those aren't listed here, so the 'required PP' is always set for those. @@ -574,7 +575,7 @@ bool found = false; /* Fill in defaults, the TPM command executor may overwrite this list */ memset(ppib->func, 0, sizeof(ppib->func)); - if (CONFIG(TPM1)) { + if (tlcl_get_family() == 1) { for (size_t i = 0; i < ARRAY_SIZE(tpm1_funcs); i++) { ppib->func[tpm1_funcs[i]] = 1; if (ppib->pprq == tpm1_funcs[i]) @@ -716,7 +717,7 @@ tpm_ppi->tag = LB_TAG_TPM_PPI_HANDOFF; tpm_ppi->size = sizeof(*tpm_ppi); tpm_ppi->ppi_address = (uintptr_t)ppib; - tpm_ppi->tpm_version = CONFIG(TPM1) ? LB_TPM_VERSION_TPM_VERSION_1_2 : + tpm_ppi->tpm_version = tlcl_get_family() == 1 ? LB_TPM_VERSION_TPM_VERSION_1_2 : LB_TPM_VERSION_TPM_VERSION_2; tpm_ppi->ppi_version = BCD(1, 3); diff --git a/src/security/tpm/tspi/crtm.h b/src/security/tpm/tspi/crtm.h index bd5bc57..951c250 100644 --- a/src/security/tpm/tspi/crtm.h +++ b/src/security/tpm/tspi/crtm.h @@ -16,7 +16,7 @@ */ #define TPM_RUNTIME_DATA_PCR 3 -#define TPM_MEASURE_ALGO (CONFIG(TPM1) ? VB2_HASH_SHA1 : VB2_HASH_SHA256) +#define TPM_MEASURE_ALGO (tlcl_get_family() == 1 ? VB2_HASH_SHA1 : VB2_HASH_SHA256) /** * Measure digests cached in TCPA log entries into PCRs diff --git a/src/security/tpm/tspi/tspi.c b/src/security/tpm/tspi/tspi.c index 525522b..d6e0534 100644 --- a/src/security/tpm/tspi/tspi.c +++ b/src/security/tpm/tspi/tspi.c @@ -16,6 +16,9 @@ uint8_t deactivated; uint32_t result = TPM_SUCCESS; + if (tlcl_get_family() != 1) + return result; + /* Check that the TPM is enabled and activated. */ result = tlcl12_get_flags(&disabled, &deactivated, NULL); if (result != TPM_SUCCESS) { @@ -200,19 +203,19 @@ return result; } -#if CONFIG(TPM1) - result = tlcl12_set_enable(); - if (result != TPM_SUCCESS) { - printk(BIOS_ERR, "TPM: Can't set enabled state.\n"); - return result; - } + if (tlcl_get_family() == 1) { + result = tlcl12_set_enable(); + if (result != TPM_SUCCESS) { + printk(BIOS_ERR, "TPM: Can't set enabled state.\n"); + return result; + } - result = tlcl12_set_deactivated(0); - if (result != TPM_SUCCESS) { - printk(BIOS_ERR, "TPM: Can't set deactivated state.\n"); - return result; + result = tlcl12_set_deactivated(0); + if (result != TPM_SUCCESS) { + printk(BIOS_ERR, "TPM: Can't set deactivated state.\n"); + return result; + } } -#endif return TPM_SUCCESS; } diff --git a/src/security/tpm/tss.h b/src/security/tpm/tss.h index 9d6e55a..dc910b9 100644 --- a/src/security/tpm/tss.h +++ b/src/security/tpm/tss.h @@ -137,6 +137,11 @@ */ uint32_t tlcl_lib_init(void); +/** + * Query active TPM family. Returns 0 if uninitialized and 1 or 2 otherwise. + */ +int tlcl_get_family(void); + /* Commands */ /** diff --git a/src/security/tpm/tss/tss.c b/src/security/tpm/tss/tss.c index 36ea575..381c45b1 100644 --- a/src/security/tpm/tss/tss.c +++ b/src/security/tpm/tss/tss.c @@ -48,6 +48,11 @@ return VB2_ERROR_UNKNOWN; } +int tlcl_get_family(void) +{ + return tpm_family; +} + uint32_t tlcl_startup(void) { #if CONFIG(TPM1) diff --git a/src/security/vboot/secdata_tpm.c b/src/security/vboot/secdata_tpm.c index ad2d812..9c84ecf 100644 --- a/src/security/vboot/secdata_tpm.c +++ b/src/security/vboot/secdata_tpm.c @@ -38,7 +38,7 @@ uint32_t antirollback_read_space_kernel(struct vb2_context *ctx) { - if (!CONFIG(TPM2)) { + if (tlcl_get_family() != 2) { /* * Before reading the kernel space, verify its permissions. If * the kernel space has the wrong permission, we give up. This @@ -233,12 +233,6 @@ return rv; } -/* Nothing special in the TPM2 path yet. */ -static uint32_t safe_write(uint32_t index, const void *data, uint32_t length) -{ - return tlcl_write(index, data, length); -} - static uint32_t setup_space(const char *name, uint32_t index, const void *data, uint32_t length, const TPMA_NV nv_attributes, const uint8_t *nv_policy, size_t nv_policy_size) @@ -381,7 +375,7 @@ return TPM_SUCCESS; } -static uint32_t _factory_initialize_tpm(struct vb2_context *ctx) +static uint32_t _factory_initialize_tpm2(struct vb2_context *ctx) { RETURN_ON_FAILURE(tlcl_force_clear()); @@ -431,11 +425,6 @@ return TPM_SUCCESS; } -uint32_t antirollback_lock_space_firmware(void) -{ - return tlcl_lock_nv_write(FIRMWARE_NV_INDEX); -} - uint32_t antirollback_read_space_mrc_hash(uint32_t index, uint8_t *data, uint32_t size) { if (size != HASH_NV_SIZE) { @@ -480,25 +469,7 @@ return tlcl_lock_nv_write(index); } -#else - -/** - * Like tlcl_write(), but checks for write errors due to hitting the 64-write - * limit and clears the TPM when that happens. This can only happen when the - * TPM is unowned, so it is OK to clear it (and we really have no choice). - * This is not expected to happen frequently, but it could happen. - */ - -static uint32_t safe_write(uint32_t index, const void *data, uint32_t length) -{ - uint32_t result = tlcl_write(index, data, length); - if (result == TPM_E_MAXNVWRITES) { - RETURN_ON_FAILURE(tpm_clear_and_reenable()); - return tlcl_write(index, data, length); - } else { - return result; - } -} +#endif /* CONFIG(TPM2) */ /** * Similarly to safe_write(), this ensures we don't fail a DefineSpace because @@ -517,7 +488,7 @@ } } -static uint32_t _factory_initialize_tpm(struct vb2_context *ctx) +static uint32_t _factory_initialize_tpm1(struct vb2_context *ctx) { TPM_PERMANENT_FLAGS pflags; uint32_t result; @@ -575,12 +546,44 @@ return TPM_SUCCESS; } -uint32_t antirollback_lock_space_firmware(void) +#endif /* CONFIG(TPM1) */ + +static uint32_t safe_write(uint32_t index, const void *data, uint32_t length) { - return tlcl_set_global_lock(); + uint32_t result; + + if (tlcl_get_family() == 2) + /* Nothing special in the TPM2 path yet. */ + return tlcl_write(index, data, length); + + /** + * Like tlcl_write(), but checks for write errors due to hitting the 64-write + * limit and clears the TPM when that happens. This can only happen when the + * TPM is unowned, so it is OK to clear it (and we really have no choice). + * This is not expected to happen frequently, but it could happen. + */ + result = tlcl_write(index, data, length); + if (result == TPM_E_MAXNVWRITES) { + RETURN_ON_FAILURE(tpm_clear_and_reenable()); + return tlcl_write(index, data, length); + } else { + return result; + } } -#endif +static uint32_t _factory_initialize_tpm(struct vb2_context *ctx) +{ + if (tlcl_get_family() == 2) + return _factory_initialize_tpm2(ctx); + return _factory_initialize_tpm1(ctx); +} + +uint32_t antirollback_lock_space_firmware(void) +{ + if (tlcl_get_family() == 2) + return tlcl_lock_nv_write(FIRMWARE_NV_INDEX); + return tlcl_set_global_lock(); +} /** * Perform one-time initializations. diff --git a/src/security/vboot/tpm_common.c b/src/security/vboot/tpm_common.c index e67cc01..a757477 100644 --- a/src/security/vboot/tpm_common.c +++ b/src/security/vboot/tpm_common.c @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0-only */ #include <security/tpm/tspi.h> +#include <security/tpm/tss.h> #include <security/vboot/tpm_common.h> #include <vb2_api.h> #include <vb2_sha.h> @@ -45,7 +46,7 @@ */ _Static_assert(sizeof(buffer) >= VB2_SHA256_DIGEST_SIZE, "Buffer needs to be able to fit at least a SHA256"); - enum vb2_hash_algorithm algo = CONFIG(TPM1) ? VB2_HASH_SHA1 : VB2_HASH_SHA256; + enum vb2_hash_algorithm algo = tlcl_get_family() == 1 ? VB2_HASH_SHA1 : VB2_HASH_SHA256; switch (which_digest) { /* SHA1 of (devmode|recmode|keyblock) bits */ diff --git a/src/vendorcode/google/chromeos/cse_board_reset.c b/src/vendorcode/google/chromeos/cse_board_reset.c index 08db7e2..9b6f878 100644 --- a/src/vendorcode/google/chromeos/cse_board_reset.c +++ b/src/vendorcode/google/chromeos/cse_board_reset.c @@ -16,6 +16,11 @@ int ret; struct cr50_firmware_version version; + /* + * Assuming that if particular TPM implementation is enabled at compile + * time, it's the one being used. This isn't generic code, so can + * probably get away with it. + */ if (CONFIG(TPM2) && CONFIG(TPM_GOOGLE_CR50)) { /* Initialize TPM and get the cr50 firmware version. */ ret = tlcl_lib_init(); @@ -27,11 +32,11 @@ cr50_get_firmware_version(&version); /* - * Cr50 firmware versions 0.[3|4].20 or newer support strap - * config 0xe where PLTRST from AP is connected to cr50's - * PLTRST# signal. So return immediately and trigger a global - * reset. - */ + * Cr50 firmware versions 0.[3|4].20 or newer support strap + * config 0xe where PLTRST from AP is connected to cr50's + * PLTRST# signal. So return immediately and trigger a global + * reset. + */ if (version.epoch != 0 || version.major > 4 || (version.major >= 3 && version.minor >= 20)) return; diff --git a/src/vendorcode/google/chromeos/tpm2.c b/src/vendorcode/google/chromeos/tpm2.c index 98fd815..4020155 100644 --- a/src/vendorcode/google/chromeos/tpm2.c +++ b/src/vendorcode/google/chromeos/tpm2.c @@ -22,6 +22,10 @@ return; } + /* In case both families are enabled, but TPM1 is in use. */ + if (tlcl_get_family() != 2) + return; + ret = tlcl_disable_platform_hierarchy(); if (ret != TPM_SUCCESS) printk(BIOS_ERR, "Platform hierarchy disablement failed: %x\n", -- To view, visit https://review.coreboot.org/c/coreboot/+/69161 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id9cc25aad8d1d7bfad12b7a92059b1b3641bbfa9 Gerrit-Change-Number: 69161 Gerrit-PatchSet: 1 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Lance Zhao Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-MessageType: newchange

1 0

[L] Change in coreboot[master]: security/tpm: resolve conflicts in TSS implementations
by Sergii Dmytruk (Code Review) 02 Nov '22

02 Nov '22

Attention is currently required from: Michał Żygowski, Christian Walter, Julius Werner, Krystian Hebel, Yu-Ping Wu. Hello Michał Żygowski, Julius Werner, Krystian Hebel, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/69160 to review the following change. Change subject: security/tpm: resolve conflicts in TSS implementations ...................................................................... security/tpm: resolve conflicts in TSS implementations No functional changes. Just code updates to don't have any compiler or linker errors if TSS 1.2 and TSS 2.0 were both compiled in. Change-Id: Ia0ea5a917c46ada9fc3274f17240e12bca98db6a Ticket: https://ticket.coreboot.org/issues/433 Signed-off-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> --- M src/security/tpm/Makefile.inc M src/security/tpm/tspi/tspi.c M src/security/tpm/tss.h M src/security/tpm/tss/tcg-1.2/tss.c A src/security/tpm/tss/tcg-1.2/tss.h M src/security/tpm/tss/tcg-2.0/tss.c A src/security/tpm/tss/tcg-2.0/tss.h M src/security/tpm/tss/tcg-2.0/tss_marshaling.c A src/security/tpm/tss/tss.c M src/security/tpm/tss/vendor/cr50/cr50.c M src/security/vboot/secdata_tpm.c 11 files changed, 457 insertions(+), 212 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/69160/1 diff --git a/src/security/tpm/Makefile.inc b/src/security/tpm/Makefile.inc index 8f633a8..748896a 100644 --- a/src/security/tpm/Makefile.inc +++ b/src/security/tpm/Makefile.inc @@ -10,6 +10,12 @@ verstage-y += tss/tcg-1.2/tss.c postcar-y += tss/tcg-1.2/tss.c +ramstage-y += tss/tss.c +romstage-y += tss/tss.c +bootblock-y += tss/tss.c +verstage-y += tss/tss.c +postcar-y += tss/tss.c + ## TSPI ramstage-y += tspi/tspi.c @@ -24,18 +30,23 @@ ramstage-y += tss/tcg-2.0/tss_marshaling.c ramstage-y += tss/tcg-2.0/tss.c +ramstage-y += tss/tss.c romstage-y += tss/tcg-2.0/tss_marshaling.c romstage-y += tss/tcg-2.0/tss.c +romstage-y += tss/tss.c verstage-$(CONFIG_VBOOT) += tss/tcg-2.0/tss_marshaling.c verstage-$(CONFIG_VBOOT) += tss/tcg-2.0/tss.c +verstage-$(CONFIG_VBOOT) += tss/tss.c postcar-y += tss/tcg-2.0/tss_marshaling.c postcar-y += tss/tcg-2.0/tss.c +postcar-y += tss/tss.c bootblock-y += tss/tcg-2.0/tss_marshaling.c bootblock-y += tss/tcg-2.0/tss.c +bootblock-y += tss/tss.c ## TSPI diff --git a/src/security/tpm/tspi/tspi.c b/src/security/tpm/tspi/tspi.c index 7bf8d6c..525522b 100644 --- a/src/security/tpm/tspi/tspi.c +++ b/src/security/tpm/tspi/tspi.c @@ -17,7 +17,7 @@ uint32_t result = TPM_SUCCESS; /* Check that the TPM is enabled and activated. */ - result = tlcl_get_flags(&disabled, &deactivated, NULL); + result = tlcl12_get_flags(&disabled, &deactivated, NULL); if (result != TPM_SUCCESS) { printk(BIOS_ERR, "TPM: Can't read capabilities.\n"); return result; @@ -26,7 +26,7 @@ if (disabled) { printk(BIOS_INFO, "TPM: is disabled. Enabling...\n"); - result = tlcl_set_enable(); + result = tlcl12_set_enable(); if (result != TPM_SUCCESS) { printk(BIOS_ERR, "TPM: Can't set enabled state.\n"); return result; @@ -36,7 +36,7 @@ if (!!deactivated != CONFIG(TPM_DEACTIVATE)) { printk(BIOS_INFO, "TPM: Unexpected TPM deactivated state. Toggling...\n"); - result = tlcl_set_deactivated(!deactivated); + result = tlcl12_set_deactivated(!deactivated); if (result != TPM_SUCCESS) { printk(BIOS_ERR, "TPM: Can't toggle deactivated state.\n"); @@ -201,13 +201,13 @@ } #if CONFIG(TPM1) - result = tlcl_set_enable(); + result = tlcl12_set_enable(); if (result != TPM_SUCCESS) { printk(BIOS_ERR, "TPM: Can't set enabled state.\n"); return result; } - result = tlcl_set_deactivated(0); + result = tlcl12_set_deactivated(0); if (result != TPM_SUCCESS) { printk(BIOS_ERR, "TPM: Can't set deactivated state.\n"); return result; diff --git a/src/security/tpm/tss.h b/src/security/tpm/tss.h index f68e1f4..9d6e55a 100644 --- a/src/security/tpm/tss.h +++ b/src/security/tpm/tss.h @@ -14,88 +14,129 @@ #include <security/tpm/tss/common/tss_common.h> #include <security/tpm/tss_errors.h> #include <security/tpm/tss/vendor/cr50/cr50.h> - -#if CONFIG(TPM1) - #include <security/tpm/tss/tcg-1.2/tss_structures.h> +#include <security/tpm/tss/tcg-2.0/tss_structures.h> + +/* + * Some operations don't have counterparts in standard and are directly exposed + * here. + * + * Other operations are applicable to both TPM versions and have wrappers which + * pick the implementation based on version determined during initialization via + * tlcl_lib_init(). + */ + +/* + * TPM1.2-specific + */ /** * Define a space with permission [perm]. [index] is the index for the space, * [size] the usable data size. The TPM error code is returned. */ -uint32_t tlcl_define_space(uint32_t index, uint32_t perm, uint32_t size); +uint32_t tlcl12_define_space(uint32_t index, uint32_t perm, uint32_t size); /** * Issue a PhysicalEnable. The TPM error code is returned. */ -uint32_t tlcl_set_enable(void); +uint32_t tlcl12_set_enable(void); /** * Issue a SetDeactivated. Pass 0 to activate. Returns result code. */ -uint32_t tlcl_set_deactivated(uint8_t flag); +uint32_t tlcl12_set_deactivated(uint8_t flag); /** * Get flags of interest. Pointers for flags you aren't interested in may * be NULL. The TPM error code is returned. */ -uint32_t tlcl_get_flags(uint8_t *disable, uint8_t *deactivated, - uint8_t *nvlocked); +uint32_t tlcl12_get_flags(uint8_t *disable, uint8_t *deactivated, uint8_t *nvlocked); + +/** + * Perform a raw TPM request/response transaction. + */ +uint32_t tlcl12_send_receive(const uint8_t *request, uint8_t *response, int max_length); + +/** + * Run the self test in the background. + */ +uint32_t tlcl12_continue_self_test(void); + +/** + * Set the nvLocked bit. The TPM error code is returned. + */ +uint32_t tlcl12_set_nv_locked(void); /** * Get the entire set of permanent flags. */ -uint32_t tlcl_get_permanent_flags(TPM_PERMANENT_FLAGS *pflags); +uint32_t tlcl12_get_permanent_flags(TPM_PERMANENT_FLAGS *pflags); -#endif +/** + * Set the bGlobalLock flag, which only a reboot can clear. The TPM error + * code is returned. + */ +uint32_t tlcl12_set_global_lock(void); -#if CONFIG(TPM2) +/** + * Get the permission bits for the NVRAM space with |index|. + */ +uint32_t tlcl12_get_permissions(uint32_t index, uint32_t *permissions); -#include <security/tpm/tss/tcg-2.0/tss_structures.h> +/* + * TPM2-specific + */ /* * Define a TPM2 space. The define space command TPM command used by the tlcl * layer offers the ability to use custom nv attributes and policies. */ -uint32_t tlcl_define_space(uint32_t space_index, size_t space_size, - const TPMA_NV nv_attributes, - const uint8_t *nv_policy, size_t nv_policy_size); +uint32_t tlcl2_define_space(uint32_t space_index, size_t space_size, + const TPMA_NV nv_attributes, + const uint8_t *nv_policy, size_t nv_policy_size); /* * Issue TPM2_GetCapability command */ -uint32_t tlcl_get_capability(TPM_CAP capability, uint32_t property, - uint32_t property_count, - TPMS_CAPABILITY_DATA *capability_data); +uint32_t tlcl2_get_capability(TPM_CAP capability, uint32_t property, + uint32_t property_count, + TPMS_CAPABILITY_DATA *capability_data); /* Issue TPM2_NV_SetBits command */ -uint32_t tlcl_set_bits(uint32_t index, uint64_t bits); +uint32_t tlcl2_set_bits(uint32_t index, uint64_t bits); /* - * Makes tpm_process_command available for on top implementations of + * Makes tlcl2_process_command available for on top implementations of * custom tpm standards like cr50 */ -void *tpm_process_command(TPM_CC command, void *command_body); +void *tlcl2_process_command(TPM_CC command, void *command_body); /* Return digest size of hash algorithm */ -uint16_t tlcl_get_hash_size_from_algo(TPMI_ALG_HASH hash_algo); +uint16_t tlcl2_get_hash_size_from_algo(TPMI_ALG_HASH hash_algo); -#endif +/** + * Set Clear Control. The TPM error code is returned. + */ +uint32_t tlcl2_clear_control(bool disable); + +/** + * Make an NV Ram location read_only. The TPM error code is returned. + */ +uint32_t tlcl2_lock_nv_write(uint32_t index); + +/** + * Disable platform hierarchy. Specific to TPM2. The TPM error code is returned. + */ +uint32_t tlcl2_disable_platform_hierarchy(void); /*****************************************************************************/ -/* Generic Functions implemented in tlcl.c */ +/* Generic Functions */ /** * Call this first. Returns 0 if success, nonzero if error. */ uint32_t tlcl_lib_init(void); -/** - * Perform a raw TPM request/response transaction. - */ -uint32_t tlcl_send_receive(const uint8_t *request, uint8_t *response, - int max_length); - /* Commands */ /** @@ -126,11 +167,6 @@ uint32_t tlcl_self_test_full(void); /** - * Run the self test in the background. - */ -uint32_t tlcl_continue_self_test(void); - -/** * Write [length] bytes of [data] to space at [index]. The TPM error code is * returned. */ @@ -159,45 +195,14 @@ uint32_t tlcl_finalize_physical_presence(void); /** - * Set the nvLocked bit. The TPM error code is returned. - */ -uint32_t tlcl_set_nv_locked(void); - -/** * Issue a ForceClear. The TPM error code is returned. */ uint32_t tlcl_force_clear(void); /** - * Set Clear Control. The TPM error code is returned. - */ -uint32_t tlcl_clear_control(bool disable); - -/** - * Set the bGlobalLock flag, which only a reboot can clear. The TPM error - * code is returned. - */ -uint32_t tlcl_set_global_lock(void); - -/** - * Make an NV Ram location read_only. The TPM error code is returned. - */ -uint32_t tlcl_lock_nv_write(uint32_t index); - -/** * Perform a TPM_Extend. */ uint32_t tlcl_extend(int pcr_num, const uint8_t *in_digest, uint8_t *out_digest); -/** - * Disable platform hierarchy. Specific to TPM2. The TPM error code is returned. - */ -uint32_t tlcl_disable_platform_hierarchy(void); - -/** - * Get the permission bits for the NVRAM space with |index|. - */ -uint32_t tlcl_get_permissions(uint32_t index, uint32_t *permissions); - #endif /* TSS_H_ */ diff --git a/src/security/tpm/tss/tcg-1.2/tss.c b/src/security/tpm/tss/tcg-1.2/tss.c index f7c9788..089cc17 100644 --- a/src/security/tpm/tss/tcg-1.2/tss.c +++ b/src/security/tpm/tss/tcg-1.2/tss.c @@ -18,6 +18,7 @@ #include <vb2_api.h> #include <security/tpm/tss.h> +#include "tss.h" #include "tss_internal.h" #include "tss_commands.h" @@ -74,8 +75,8 @@ * Like TlclSendReceive below, but do not retry if NEEDS_SELFTEST or * DOING_SELFTEST errors are returned. */ -static uint32_t tlcl_send_receive_no_retry(const uint8_t *request, - uint8_t *response, int max_length) +static uint32_t tlcl12_send_receive_no_retry(const uint8_t *request, + uint8_t *response, int max_length) { uint32_t response_length = max_length; uint32_t result; @@ -103,21 +104,18 @@ /* Sends a TPM command and gets a response. Returns 0 if success or the TPM * error code if error. Waits for the self test to complete if needed. */ -uint32_t tlcl_send_receive(const uint8_t *request, uint8_t *response, - int max_length) +uint32_t tlcl12_send_receive(const uint8_t *request, uint8_t *response, int max_length) { - uint32_t result = tlcl_send_receive_no_retry(request, response, - max_length); + uint32_t result = tlcl12_send_receive_no_retry(request, response, max_length); /* If the command fails because the self test has not completed, try it * again after attempting to ensure that the self test has completed. */ if (result == TPM_E_NEEDS_SELFTEST || result == TPM_E_DOING_SELFTEST) { - result = tlcl_continue_self_test(); + result = tlcl12_continue_self_test(); if (result != TPM_SUCCESS) return result; #if defined(TPM_BLOCKING_CONTINUESELFTEST) || defined(VB_RECOVERY_MODE) /* Retry only once */ - result = tlcl_send_receive_no_retry(request, response, - max_length); + result = tlcl12_send_receive_no_retry(request, response, max_length); #else /* This needs serious testing. The TPM specification says: "iii. * The caller MUST wait for the actions of TPM_ContinueSelfTest @@ -125,8 +123,7 @@ * ContinueSelfTest is non-blocking, how do we know that the * actions have completed other than trying again? */ do { - result = tlcl_send_receive_no_retry(request, response, - max_length); + result = tlcl12_send_receive_no_retry(request, response, max_length); } while (result == TPM_E_DOING_SELFTEST); #endif } @@ -137,64 +134,50 @@ static uint32_t send(const uint8_t *command) { uint8_t response[TPM_LARGE_ENOUGH_COMMAND_SIZE]; - return tlcl_send_receive(command, response, sizeof(response)); + return tlcl12_send_receive(command, response, sizeof(response)); } /* Exported functions. */ -uint32_t tlcl_lib_init(void) +void tlcl12_lib_init(tis_sendrecv_fn sendrecv) { - int tpm_family; - - if (tis_sendrecv != NULL) - return VB2_SUCCESS; - - tis_sendrecv = tis_probe(&tpm_family); - if (tis_sendrecv == NULL) - return VB2_ERROR_UNKNOWN; - - if (tpm_family != 1) { - tis_sendrecv = NULL; - return VB2_ERROR_UNKNOWN; - } - - return VB2_SUCCESS; + tis_sendrecv = sendrecv; } -uint32_t tlcl_startup(void) +uint32_t tlcl12_startup(void) { VBDEBUG("TPM: Startup\n"); return send(tpm_startup_cmd.buffer); } -uint32_t tlcl_resume(void) +uint32_t tlcl12_resume(void) { VBDEBUG("TPM: Resume\n"); return send(tpm_resume_cmd.buffer); } -uint32_t tlcl_save_state(void) +uint32_t tlcl12_save_state(void) { VBDEBUG("TPM: Save state\n"); return send(tpm_savestate_cmd.buffer); } -uint32_t tlcl_self_test_full(void) +uint32_t tlcl12_self_test_full(void) { VBDEBUG("TPM: Self test full\n"); return send(tpm_selftestfull_cmd.buffer); } -uint32_t tlcl_continue_self_test(void) +uint32_t tlcl12_continue_self_test(void) { uint8_t response[TPM_LARGE_ENOUGH_COMMAND_SIZE]; VBDEBUG("TPM: Continue self test\n"); /* Call the No Retry version of SendReceive to avoid recursion. */ - return tlcl_send_receive_no_retry(tpm_continueselftest_cmd.buffer, - response, sizeof(response)); + return tlcl12_send_receive_no_retry(tpm_continueselftest_cmd.buffer, + response, sizeof(response)); } -uint32_t tlcl_define_space(uint32_t index, uint32_t perm, uint32_t size) +uint32_t tlcl12_define_space(uint32_t index, uint32_t perm, uint32_t size) { struct s_tpm_nv_definespace_cmd cmd; VBDEBUG("TPM: TlclDefineSpace(0x%x, 0x%x, %d)\n", index, perm, size); @@ -205,7 +188,7 @@ return send(cmd.buffer); } -uint32_t tlcl_write(uint32_t index, const void *data, uint32_t length) +uint32_t tlcl12_write(uint32_t index, const void *data, uint32_t length) { struct s_tpm_nv_write_cmd cmd; uint8_t response[TPM_LARGE_ENOUGH_COMMAND_SIZE]; @@ -222,10 +205,10 @@ if (length > 0) memcpy(cmd.buffer + tpm_nv_write_cmd.data, data, length); - return tlcl_send_receive(cmd.buffer, response, sizeof(response)); + return tlcl12_send_receive(cmd.buffer, response, sizeof(response)); } -uint32_t tlcl_read(uint32_t index, void *data, uint32_t length) +uint32_t tlcl12_read(uint32_t index, void *data, uint32_t length) { struct s_tpm_nv_read_cmd cmd; uint8_t response[TPM_LARGE_ENOUGH_COMMAND_SIZE]; @@ -237,7 +220,7 @@ to_tpm_uint32(cmd.buffer + tpm_nv_read_cmd.index, index); to_tpm_uint32(cmd.buffer + tpm_nv_read_cmd.length, length); - result = tlcl_send_receive(cmd.buffer, response, sizeof(response)); + result = tlcl12_send_receive(cmd.buffer, response, sizeof(response)); if (result == TPM_SUCCESS && length > 0) { uint8_t *nv_read_cursor = response + kTpmResponseHeaderLength; from_tpm_uint32(nv_read_cursor, &result_length); @@ -250,43 +233,43 @@ return result; } -uint32_t tlcl_assert_physical_presence(void) +uint32_t tlcl12_assert_physical_presence(void) { VBDEBUG("TPM: Asserting physical presence\n"); return send(tpm_ppassert_cmd.buffer); } -uint32_t tlcl_physical_presence_cmd_enable(void) +uint32_t tlcl12_physical_presence_cmd_enable(void) { VBDEBUG("TPM: Enable the physical presence command\n"); return send(tpm_ppenable_cmd.buffer); } -uint32_t tlcl_finalize_physical_presence(void) +uint32_t tlcl12_finalize_physical_presence(void) { VBDEBUG("TPM: Enable PP cmd, disable HW pp, and set lifetime lock\n"); return send(tpm_finalizepp_cmd.buffer); } -uint32_t tlcl_set_nv_locked(void) +uint32_t tlcl12_set_nv_locked(void) { VBDEBUG("TPM: Set NV locked\n"); - return tlcl_define_space(TPM_NV_INDEX_LOCK, 0, 0); + return tlcl12_define_space(TPM_NV_INDEX_LOCK, 0, 0); } -uint32_t tlcl_force_clear(void) +uint32_t tlcl12_force_clear(void) { VBDEBUG("TPM: Force clear\n"); return send(tpm_forceclear_cmd.buffer); } -uint32_t tlcl_set_enable(void) +uint32_t tlcl12_set_enable(void) { VBDEBUG("TPM: Enabling TPM\n"); return send(tpm_physicalenable_cmd.buffer); } -uint32_t tlcl_set_deactivated(uint8_t flag) +uint32_t tlcl12_set_deactivated(uint8_t flag) { struct s_tpm_physicalsetdeactivated_cmd cmd; VBDEBUG("TPM: SetDeactivated(%d)\n", flag); @@ -295,12 +278,12 @@ return send(cmd.buffer); } -uint32_t tlcl_get_permanent_flags(TPM_PERMANENT_FLAGS *pflags) +uint32_t tlcl12_get_permanent_flags(TPM_PERMANENT_FLAGS *pflags) { uint8_t response[TPM_LARGE_ENOUGH_COMMAND_SIZE]; uint32_t size; - uint32_t result = tlcl_send_receive(tpm_getflags_cmd.buffer, response, - sizeof(response)); + uint32_t result = tlcl12_send_receive(tpm_getflags_cmd.buffer, response, + sizeof(response)); if (result != TPM_SUCCESS) return result; from_tpm_uint32(response + kTpmResponseHeaderLength, &size); @@ -311,11 +294,10 @@ return result; } -uint32_t tlcl_get_flags(uint8_t *disable, uint8_t *deactivated, - uint8_t *nvlocked) +uint32_t tlcl12_get_flags(uint8_t *disable, uint8_t *deactivated, uint8_t *nvlocked) { TPM_PERMANENT_FLAGS pflags; - uint32_t result = tlcl_get_permanent_flags(&pflags); + uint32_t result = tlcl12_get_permanent_flags(&pflags); if (result == TPM_SUCCESS) { if (disable) *disable = pflags.disable; @@ -329,14 +311,13 @@ return result; } -uint32_t tlcl_set_global_lock(void) +uint32_t tlcl12_set_global_lock(void) { VBDEBUG("TPM: Set global lock\n"); - return tlcl_write(TPM_NV_INDEX0, NULL, 0); + return tlcl12_write(TPM_NV_INDEX0, NULL, 0); } -uint32_t tlcl_extend(int pcr_num, const uint8_t *in_digest, - uint8_t *out_digest) +uint32_t tlcl12_extend(int pcr_num, const uint8_t *in_digest, uint8_t *out_digest) { struct s_tpm_extend_cmd cmd; uint8_t response[kTpmResponseHeaderLength + kPcrDigestLength]; @@ -346,7 +327,7 @@ to_tpm_uint32(cmd.buffer + tpm_extend_cmd.pcrNum, pcr_num); memcpy(cmd.buffer + cmd.inDigest, in_digest, kPcrDigestLength); - result = tlcl_send_receive(cmd.buffer, response, sizeof(response)); + result = tlcl12_send_receive(cmd.buffer, response, sizeof(response)); if (result != TPM_SUCCESS) return result; @@ -356,7 +337,7 @@ return result; } -uint32_t tlcl_get_permissions(uint32_t index, uint32_t *permissions) +uint32_t tlcl12_get_permissions(uint32_t index, uint32_t *permissions) { struct s_tpm_getpermissions_cmd cmd; uint8_t response[TPM_LARGE_ENOUGH_COMMAND_SIZE]; @@ -366,7 +347,7 @@ memcpy(&cmd, &tpm_getpermissions_cmd, sizeof(cmd)); to_tpm_uint32(cmd.buffer + tpm_getpermissions_cmd.index, index); - result = tlcl_send_receive(cmd.buffer, response, sizeof(response)); + result = tlcl12_send_receive(cmd.buffer, response, sizeof(response)); if (result != TPM_SUCCESS) return result; diff --git a/src/security/tpm/tss/tcg-1.2/tss.h b/src/security/tpm/tss/tcg-1.2/tss.h new file mode 100644 index 0000000..7eba1dd --- /dev/null +++ b/src/security/tpm/tss/tcg-1.2/tss.h @@ -0,0 +1,26 @@ +/* SPDX-License-Identifier: BSD-3-Clause */ + +#ifndef TSS12_H_ +#define TSS12_H_ + +#include "tss_structures.h" + +/* + * Declarations for "private" functions which are dispatched to by tss.c in + * the parent directory based on TPM family. + */ + +void tlcl12_lib_init(tis_sendrecv_fn sendrecv); +uint32_t tlcl12_save_state(void); +uint32_t tlcl12_resume(void); +uint32_t tlcl12_startup(void); +uint32_t tlcl12_self_test_full(void); +uint32_t tlcl12_read(uint32_t index, void *data, uint32_t length); +uint32_t tlcl12_write(uint32_t index, const void *data, uint32_t length); +uint32_t tlcl12_assert_physical_presence(void); +uint32_t tlcl12_physical_presence_cmd_enable(void); +uint32_t tlcl12_finalize_physical_presence(void); +uint32_t tlcl12_force_clear(void); +uint32_t tlcl12_extend(int pcr_num, const uint8_t *in_digest, uint8_t *out_digest); + +#endif diff --git a/src/security/tpm/tss/tcg-2.0/tss.c b/src/security/tpm/tss/tcg-2.0/tss.c index d270329..bd8f86d 100644 --- a/src/security/tpm/tss/tcg-2.0/tss.c +++ b/src/security/tpm/tss/tcg-2.0/tss.c @@ -7,6 +7,7 @@ #include <security/tpm/tis.h> #include <security/tpm/tss.h> +#include "tss.h" #include "tss_structures.h" #include "tss_marshaling.h" @@ -18,7 +19,7 @@ static tis_sendrecv_fn tis_sendrecv; -void *tpm_process_command(TPM_CC command, void *command_body) +void *tlcl2_process_command(TPM_CC command, void *command_body) { struct obuf ob; struct ibuf ib; @@ -48,13 +49,13 @@ return tpm_unmarshal_response(command, &ib); } -static uint32_t tlcl_send_startup(TPM_SU type) +static uint32_t tlcl2_send_startup(TPM_SU type) { struct tpm2_startup startup; struct tpm2_response *response; startup.startup_type = type; - response = tpm_process_command(TPM2_Startup, &startup); + response = tlcl2_process_command(TPM2_Startup, &startup); /* IO error, tpm2_response pointer is empty. */ if (!response) { @@ -77,18 +78,18 @@ return TPM_E_IOERROR; } -uint32_t tlcl_resume(void) +uint32_t tlcl2_resume(void) { - return tlcl_send_startup(TPM_SU_STATE); + return tlcl2_send_startup(TPM_SU_STATE); } -static uint32_t tlcl_send_shutdown(TPM_SU type) +static uint32_t tlcl2_send_shutdown(TPM_SU type) { struct tpm2_shutdown shutdown; struct tpm2_response *response; shutdown.shutdown_type = type; - response = tpm_process_command(TPM2_Shutdown, &shutdown); + response = tlcl2_process_command(TPM2_Shutdown, &shutdown); /* IO error, tpm2_response pointer is empty. */ if (!response) { @@ -106,12 +107,12 @@ return TPM_E_IOERROR; } -uint32_t tlcl_save_state(void) +uint32_t tlcl2_save_state(void) { - return tlcl_send_shutdown(TPM_SU_STATE); + return tlcl2_send_shutdown(TPM_SU_STATE); } -uint32_t tlcl_assert_physical_presence(void) +uint32_t tlcl2_assert_physical_presence(void) { /* * Nothing to do on TPM2 for this, use platform hierarchy availability @@ -124,8 +125,7 @@ * The caller will provide the digest in a 32 byte buffer, let's consider it a * sha256 digest. */ -uint32_t tlcl_extend(int pcr_num, const uint8_t *in_digest, - uint8_t *out_digest) +uint32_t tlcl2_extend(int pcr_num, const uint8_t *in_digest, uint8_t *out_digest) { struct tpm2_pcr_extend_cmd pcr_ext_cmd; struct tpm2_response *response; @@ -136,7 +136,7 @@ memcpy(pcr_ext_cmd.digests.digests[0].digest.sha256, in_digest, sizeof(pcr_ext_cmd.digests.digests[0].digest.sha256)); - response = tpm_process_command(TPM2_PCR_Extend, &pcr_ext_cmd); + response = tlcl2_process_command(TPM2_PCR_Extend, &pcr_ext_cmd); printk(BIOS_INFO, "%s: response is %x\n", __func__, response ? response->hdr.tpm_code : -1); @@ -146,18 +146,18 @@ return TPM_SUCCESS; } -uint32_t tlcl_finalize_physical_presence(void) +uint32_t tlcl2_finalize_physical_presence(void) { /* Nothing needs to be done with tpm2. */ printk(BIOS_INFO, "%s:%s:%d\n", __FILE__, __func__, __LINE__); return TPM_SUCCESS; } -uint32_t tlcl_force_clear(void) +uint32_t tlcl2_force_clear(void) { struct tpm2_response *response; - response = tpm_process_command(TPM2_Clear, NULL); + response = tlcl2_process_command(TPM2_Clear, NULL); printk(BIOS_INFO, "%s: response is %x\n", __func__, response ? response->hdr.tpm_code : -1); @@ -167,14 +167,14 @@ return TPM_SUCCESS; } -uint32_t tlcl_clear_control(bool disable) +uint32_t tlcl2_clear_control(bool disable) { struct tpm2_response *response; struct tpm2_clear_control_cmd cc = { .disable = 0, }; - response = tpm_process_command(TPM2_ClearControl, &cc); + response = tlcl2_process_command(TPM2_ClearControl, &cc); printk(BIOS_INFO, "%s: response is %x\n", __func__, response ? response->hdr.tpm_code : -1); @@ -184,37 +184,18 @@ return TPM_SUCCESS; } -/* This function is called directly by vboot, uses vboot return types. */ -uint32_t tlcl_lib_init(void) +void tlcl2_lib_init(tis_sendrecv_fn sendrecv) { - int tpm_family; - - if (tis_sendrecv != NULL) - return VB2_SUCCESS; - - tis_sendrecv = tis_probe(&tpm_family); - if (tis_sendrecv == NULL) { - printk(BIOS_ERR, "%s: tis_probe returned error\n", __func__); - return VB2_ERROR_UNKNOWN; - } - - if (tpm_family != 2) { - tis_sendrecv = NULL; - printk(BIOS_ERR, "%s: tis_probe returned unsupported TPM family: %d\n", - __func__, tpm_family); - return VB2_ERROR_UNKNOWN; - } - - return VB2_SUCCESS; + tis_sendrecv = sendrecv; } -uint32_t tlcl_physical_presence_cmd_enable(void) +uint32_t tlcl2_physical_presence_cmd_enable(void) { printk(BIOS_INFO, "%s:%s:%d\n", __FILE__, __func__, __LINE__); return TPM_SUCCESS; } -uint32_t tlcl_read(uint32_t index, void *data, uint32_t length) +uint32_t tlcl2_read(uint32_t index, void *data, uint32_t length) { struct tpm2_nv_read_cmd nv_readc; struct tpm2_response *response; @@ -224,7 +205,7 @@ nv_readc.nvIndex = HR_NV_INDEX + index; nv_readc.size = length; - response = tpm_process_command(TPM2_NV_Read, &nv_readc); + response = tlcl2_process_command(TPM2_NV_Read, &nv_readc); /* Need to map tpm error codes into internal values. */ if (!response) @@ -263,20 +244,20 @@ return TPM_SUCCESS; } -uint32_t tlcl_self_test_full(void) +uint32_t tlcl2_self_test_full(void) { struct tpm2_self_test st; struct tpm2_response *response; st.yes_no = 1; - response = tpm_process_command(TPM2_SelfTest, &st); + response = tlcl2_process_command(TPM2_SelfTest, &st); printk(BIOS_INFO, "%s: response is %x\n", __func__, response ? response->hdr.tpm_code : -1); return TPM_SUCCESS; } -uint32_t tlcl_lock_nv_write(uint32_t index) +uint32_t tlcl2_lock_nv_write(uint32_t index) { struct tpm2_response *response; /* TPM Will reject attempts to write at non-defined index. */ @@ -284,7 +265,7 @@ .nvIndex = HR_NV_INDEX + index, }; - response = tpm_process_command(TPM2_NV_WriteLock, &nv_wl); + response = tlcl2_process_command(TPM2_NV_WriteLock, &nv_wl); printk(BIOS_INFO, "%s: response is %x\n", __func__, response ? response->hdr.tpm_code : -1); @@ -295,12 +276,12 @@ return TPM_SUCCESS; } -uint32_t tlcl_startup(void) +uint32_t tlcl2_startup(void) { - return tlcl_send_startup(TPM_SU_CLEAR); + return tlcl2_send_startup(TPM_SU_CLEAR); } -uint32_t tlcl_write(uint32_t index, const void *data, uint32_t length) +uint32_t tlcl2_write(uint32_t index, const void *data, uint32_t length) { struct tpm2_nv_write_cmd nv_writec; struct tpm2_response *response; @@ -311,7 +292,7 @@ nv_writec.data.t.size = length; nv_writec.data.t.buffer = data; - response = tpm_process_command(TPM2_NV_Write, &nv_writec); + response = tlcl2_process_command(TPM2_NV_Write, &nv_writec); printk(BIOS_INFO, "%s: response is %x\n", __func__, response ? response->hdr.tpm_code : -1); @@ -323,7 +304,7 @@ return TPM_SUCCESS; } -uint32_t tlcl_set_bits(uint32_t index, uint64_t bits) +uint32_t tlcl2_set_bits(uint32_t index, uint64_t bits) { struct tpm2_nv_setbits_cmd nvsb_cmd; struct tpm2_response *response; @@ -334,7 +315,7 @@ nvsb_cmd.nvIndex = HR_NV_INDEX + index; nvsb_cmd.bits = bits; - response = tpm_process_command(TPM2_NV_SetBits, &nvsb_cmd); + response = tlcl2_process_command(TPM2_NV_SetBits, &nvsb_cmd); printk(BIOS_INFO, "%s: response is %x\n", __func__, response ? response->hdr.tpm_code : -1); @@ -346,9 +327,9 @@ return TPM_SUCCESS; } -uint32_t tlcl_define_space(uint32_t space_index, size_t space_size, - const TPMA_NV nv_attributes, - const uint8_t *nv_policy, size_t nv_policy_size) +uint32_t tlcl2_define_space(uint32_t space_index, size_t space_size, + const TPMA_NV nv_attributes, + const uint8_t *nv_policy, size_t nv_policy_size) { struct tpm2_nv_define_space_cmd nvds_cmd; struct tpm2_response *response; @@ -371,7 +352,7 @@ nvds_cmd.publicInfo.authPolicy.t.size = nv_policy_size; } - response = tpm_process_command(TPM2_NV_DefineSpace, &nvds_cmd); + response = tlcl2_process_command(TPM2_NV_DefineSpace, &nvds_cmd); printk(BIOS_INFO, "%s: response is %x\n", __func__, response ? response->hdr.tpm_code : -1); @@ -389,7 +370,7 @@ } } -uint16_t tlcl_get_hash_size_from_algo(TPMI_ALG_HASH hash_algo) +uint16_t tlcl2_get_hash_size_from_algo(TPMI_ALG_HASH hash_algo) { uint16_t value; @@ -421,7 +402,7 @@ return value; } -uint32_t tlcl_disable_platform_hierarchy(void) +uint32_t tlcl2_disable_platform_hierarchy(void) { struct tpm2_response *response; struct tpm2_hierarchy_control_cmd hc = { @@ -429,7 +410,7 @@ .state = 0, }; - response = tpm_process_command(TPM2_Hierarchy_Control, &hc); + response = tlcl2_process_command(TPM2_Hierarchy_Control, &hc); if (!response || response->hdr.tpm_code) return TPM_E_INTERNAL_INCONSISTENCY; @@ -437,9 +418,9 @@ return TPM_SUCCESS; } -uint32_t tlcl_get_capability(TPM_CAP capability, uint32_t property, - uint32_t property_count, - TPMS_CAPABILITY_DATA *capability_data) +uint32_t tlcl2_get_capability(TPM_CAP capability, uint32_t property, + uint32_t property_count, + TPMS_CAPABILITY_DATA *capability_data) { struct tpm2_get_capability cmd; struct tpm2_response *response; @@ -454,7 +435,7 @@ return TPM_E_IOERROR; } - response = tpm_process_command(TPM2_GetCapability, &cmd); + response = tlcl2_process_command(TPM2_GetCapability, &cmd); if (!response) { printk(BIOS_ERR, "%s: Command Failed\n", __func__); diff --git a/src/security/tpm/tss/tcg-2.0/tss.h b/src/security/tpm/tss/tcg-2.0/tss.h new file mode 100644 index 0000000..48e2454 --- /dev/null +++ b/src/security/tpm/tss/tcg-2.0/tss.h @@ -0,0 +1,26 @@ +/* SPDX-License-Identifier: BSD-3-Clause */ + +#ifndef TSS2_H__ +#define TSS2_H__ + +#include "tss_structures.h" + +/* + * Declarations for "private" functions which are dispatched to by tss.c in + * the parent directory based on TPM family. + */ + +void tlcl2_lib_init(tis_sendrecv_fn sendrecv); +uint32_t tlcl2_save_state(void); +uint32_t tlcl2_resume(void); +uint32_t tlcl2_startup(void); +uint32_t tlcl2_self_test_full(void); +uint32_t tlcl2_read(uint32_t index, void *data, uint32_t length); +uint32_t tlcl2_write(uint32_t index, const void *data, uint32_t length); +uint32_t tlcl2_assert_physical_presence(void); +uint32_t tlcl2_physical_presence_cmd_enable(void); +uint32_t tlcl2_finalize_physical_presence(void); +uint32_t tlcl2_force_clear(void); +uint32_t tlcl2_extend(int pcr_num, const uint8_t *in_digest, uint8_t *out_digest); + +#endif diff --git a/src/security/tpm/tss/tcg-2.0/tss_marshaling.c b/src/security/tpm/tss/tcg-2.0/tss_marshaling.c index f1b9522..383c1e6 100644 --- a/src/security/tpm/tss/tcg-2.0/tss_marshaling.c +++ b/src/security/tpm/tss/tcg-2.0/tss_marshaling.c @@ -79,23 +79,23 @@ switch (tpmtha->hashAlg) { case TPM_ALG_SHA1: rc |= obuf_write(ob, tpmtha->digest.sha1, - tlcl_get_hash_size_from_algo(tpmtha->hashAlg)); + tlcl2_get_hash_size_from_algo(tpmtha->hashAlg)); break; case TPM_ALG_SHA256: rc |= obuf_write(ob, tpmtha->digest.sha256, - tlcl_get_hash_size_from_algo(tpmtha->hashAlg)); + tlcl2_get_hash_size_from_algo(tpmtha->hashAlg)); break; case TPM_ALG_SM3_256: rc |= obuf_write(ob, tpmtha->digest.sm3_256, - tlcl_get_hash_size_from_algo(tpmtha->hashAlg)); + tlcl2_get_hash_size_from_algo(tpmtha->hashAlg)); break; case TPM_ALG_SHA384: rc |= obuf_write(ob, tpmtha->digest.sha384, - tlcl_get_hash_size_from_algo(tpmtha->hashAlg)); + tlcl2_get_hash_size_from_algo(tpmtha->hashAlg)); break; case TPM_ALG_SHA512: rc |= obuf_write(ob, tpmtha->digest.sha512, - tlcl_get_hash_size_from_algo(tpmtha->hashAlg)); + tlcl2_get_hash_size_from_algo(tpmtha->hashAlg)); break; default: rc = -1; diff --git a/src/security/tpm/tss/tss.c b/src/security/tpm/tss/tss.c new file mode 100644 index 0000000..36ea575 --- /dev/null +++ b/src/security/tpm/tss/tss.c @@ -0,0 +1,203 @@ +/* SPDX-License-Identifier: BSD-3-Clause */ + +#include <console/console.h> +#include <security/tpm/tis.h> +#include <security/tpm/tss.h> +#include <vb2_api.h> + +#include "tcg-1.2/tss.h" +#include "tcg-2.0/tss.h" + +/* + * This unit dispatches to either TPM1.2 or TPM2.0 implementation based on + * TPM family determined on probing during initialization. + */ + +static int tpm_family; + +/* Probe for TPM device and chose implementation based on the returned TPM family. */ +uint32_t tlcl_lib_init(void) +{ + tis_sendrecv_fn tis_sendrecv; + + if (tpm_family != 0) + return VB2_SUCCESS; + + tis_sendrecv = tis_probe(&tpm_family); + if (tis_sendrecv == NULL) { + printk(BIOS_ERR, "%s: tis_probe returned error\n", __func__); + return VB2_ERROR_UNKNOWN; + } + +#if CONFIG(TPM1) + if (tpm_family == 1) { + tlcl12_lib_init(tis_sendrecv); + return VB2_SUCCESS; + } +#endif +#if CONFIG(TPM2) + if (tpm_family == 2) { + tlcl2_lib_init(tis_sendrecv); + return VB2_SUCCESS; + } +#endif + + printk(BIOS_ERR, "%s: tis_probe returned incorrect tpm_family: %d\n", __func__, + tpm_family); + tpm_family = 0; + return VB2_ERROR_UNKNOWN; +} + +uint32_t tlcl_startup(void) +{ +#if CONFIG(TPM1) + if (tpm_family == 1) + return tlcl12_startup(); +#endif +#if CONFIG(TPM2) + if (tpm_family == 2) + return tlcl2_startup(); +#endif + + die("%s: can't be used with TPM %d\n", __func__, tpm_family); +} + +uint32_t tlcl_resume(void) +{ +#if CONFIG(TPM1) + if (tpm_family == 1) + return tlcl12_resume(); +#endif +#if CONFIG(TPM2) + if (tpm_family == 2) + return tlcl2_resume(); +#endif + + die("%s: can't be used with TPM %d\n", __func__, tpm_family); +} + +uint32_t tlcl_save_state(void) +{ +#if CONFIG(TPM1) + if (tpm_family == 1) + return tlcl12_save_state(); +#endif +#if CONFIG(TPM2) + if (tpm_family == 2) + return tlcl2_save_state(); +#endif + + die("%s: can't be used with TPM %d\n", __func__, tpm_family); +} + +uint32_t tlcl_self_test_full(void) +{ +#if CONFIG(TPM1) + if (tpm_family == 1) + return tlcl12_self_test_full(); +#endif +#if CONFIG(TPM2) + if (tpm_family == 2) + return tlcl2_self_test_full(); +#endif + + die("%s: can't be used with TPM %d\n", __func__, tpm_family); +} + +uint32_t tlcl_write(uint32_t index, const void *data, uint32_t length) +{ +#if CONFIG(TPM1) + if (tpm_family == 1) + return tlcl12_write(index, data, length); +#endif +#if CONFIG(TPM2) + if (tpm_family == 2) + return tlcl2_write(index, data, length); +#endif + + die("%s: can't be used with TPM %d\n", __func__, tpm_family); +} + +uint32_t tlcl_read(uint32_t index, void *data, uint32_t length) +{ +#if CONFIG(TPM1) + if (tpm_family == 1) + return tlcl12_read(index, data, length); +#endif +#if CONFIG(TPM2) + if (tpm_family == 2) + return tlcl2_read(index, data, length); +#endif + + die("%s: can't be used with TPM %d\n", __func__, tpm_family); +} + +uint32_t tlcl_assert_physical_presence(void) +{ +#if CONFIG(TPM1) + if (tpm_family == 1) + return tlcl12_assert_physical_presence(); +#endif +#if CONFIG(TPM2) + if (tpm_family == 2) + return tlcl2_assert_physical_presence(); +#endif + + die("%s: can't be used with TPM %d\n", __func__, tpm_family); +} + +uint32_t tlcl_physical_presence_cmd_enable(void) +{ +#if CONFIG(TPM1) + if (tpm_family == 1) + return tlcl12_physical_presence_cmd_enable(); +#endif +#if CONFIG(TPM2) + if (tpm_family == 2) + return tlcl2_physical_presence_cmd_enable(); +#endif + + die("%s: can't be used with TPM %d\n", __func__, tpm_family); +} + +uint32_t tlcl_finalize_physical_presence(void) +{ +#if CONFIG(TPM1) + if (tpm_family == 1) + return tlcl12_finalize_physical_presence(); +#endif +#if CONFIG(TPM2) + if (tpm_family == 2) + return tlcl2_finalize_physical_presence(); +#endif + + die("%s: can't be used with TPM %d\n", __func__, tpm_family); +} + +uint32_t tlcl_force_clear(void) +{ +#if CONFIG(TPM1) + if (tpm_family == 1) + return tlcl12_force_clear(); +#endif +#if CONFIG(TPM2) + if (tpm_family == 2) + return tlcl2_force_clear(); +#endif + + die("%s: can't be used with TPM %d\n", __func__, tpm_family); +} + +uint32_t tlcl_extend(int pcr_num, const uint8_t *in_digest, uint8_t *out_digest) +{ +#if CONFIG(TPM1) + if (tpm_family == 1) + return tlcl12_extend(pcr_num, in_digest, out_digest); +#endif +#if CONFIG(TPM2) + if (tpm_family == 2) + return tlcl2_extend(pcr_num, in_digest, out_digest); +#endif + + die("%s: can't be used with TPM %d\n", __func__, tpm_family); +} diff --git a/src/security/tpm/tss/vendor/cr50/cr50.c b/src/security/tpm/tss/vendor/cr50/cr50.c index 57d0b61..3237410 100644 --- a/src/security/tpm/tss/vendor/cr50/cr50.c +++ b/src/security/tpm/tss/vendor/cr50/cr50.c @@ -16,7 +16,7 @@ printk(BIOS_INFO, "Enabling cr50 nvmem commits\n"); - response = tpm_process_command(TPM2_CR50_VENDOR_COMMAND, &sub_command); + response = tlcl2_process_command(TPM2_CR50_VENDOR_COMMAND, &sub_command); if (!response || (response && response->hdr.tpm_code)) { if (response) @@ -39,7 +39,7 @@ printk(BIOS_INFO, "Checking cr50 for pending updates\n"); - response = tpm_process_command(TPM2_CR50_VENDOR_COMMAND, command_body); + response = tlcl2_process_command(TPM2_CR50_VENDOR_COMMAND, command_body); if (!response || response->hdr.tpm_code) return TPM_E_IOERROR; @@ -55,7 +55,7 @@ printk(BIOS_INFO, "Checking cr50 for recovery request\n"); - response = tpm_process_command(TPM2_CR50_VENDOR_COMMAND, &sub_command); + response = tlcl2_process_command(TPM2_CR50_VENDOR_COMMAND, &sub_command); if (!response || response->hdr.tpm_code) return TPM_E_IOERROR; @@ -72,7 +72,7 @@ printk(BIOS_INFO, "Reading cr50 TPM mode\n"); - response = tpm_process_command(TPM2_CR50_VENDOR_COMMAND, &mode_command); + response = tlcl2_process_command(TPM2_CR50_VENDOR_COMMAND, &mode_command); if (!response) return TPM_E_IOERROR; @@ -112,7 +112,7 @@ printk(BIOS_DEBUG, "Reading cr50 boot mode\n"); - response = tpm_process_command(TPM2_CR50_VENDOR_COMMAND, &mode_command); + response = tlcl2_process_command(TPM2_CR50_VENDOR_COMMAND, &mode_command); if (!response) return TPM_E_IOERROR; @@ -141,8 +141,7 @@ * Issue an immediate reset to the Cr50. */ printk(BIOS_INFO, "Issuing cr50 reset\n"); - response = tpm_process_command(TPM2_CR50_VENDOR_COMMAND, - &reset_command_body); + response = tlcl2_process_command(TPM2_CR50_VENDOR_COMMAND, &reset_command_body); if (!response) return TPM_E_IOERROR; @@ -157,7 +156,7 @@ printk(BIOS_DEBUG, "Issuing EC reset\n"); - response = tpm_process_command(TPM2_CR50_VENDOR_COMMAND, &reset_cmd); + response = tlcl2_process_command(TPM2_CR50_VENDOR_COMMAND, &reset_cmd); if (!response) return TPM_E_IOERROR; diff --git a/src/security/vboot/secdata_tpm.c b/src/security/vboot/secdata_tpm.c index 7885069..ad2d812 100644 --- a/src/security/vboot/secdata_tpm.c +++ b/src/security/vboot/secdata_tpm.c @@ -216,8 +216,7 @@ { uint32_t rv; - rv = tlcl_define_space(index, length, nv_attributes, nv_policy, - nv_policy_size); + rv = tlcl2_define_space(index, length, nv_attributes, nv_policy, nv_policy_size); if (rv == TPM_E_NV_DEFINED) { /* * Continue with writing: it may be defined, but not written @@ -509,10 +508,10 @@ */ static uint32_t safe_define_space(uint32_t index, uint32_t perm, uint32_t size) { - uint32_t result = tlcl_define_space(index, perm, size); + uint32_t result = tlcl12_define_space(index, perm, size); if (result == TPM_E_MAXNVWRITES) { RETURN_ON_FAILURE(tpm_clear_and_reenable()); - return tlcl_define_space(index, perm, size); + return tlcl12_define_space(index, perm, size); } else { return result; } -- To view, visit https://review.coreboot.org/c/coreboot/+/69160 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia0ea5a917c46ada9fc3274f17240e12bca98db6a Gerrit-Change-Number: 69160 Gerrit-PatchSet: 1 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-MessageType: newchange

1 0

[M] Change in coreboot[master]: security/tpm: make tis_probe() return tpm_family
by Sergii Dmytruk (Code Review) 02 Nov '22

02 Nov '22

Attention is currently required from: Michał Żygowski, Christian Walter, Julius Werner, Krystian Hebel. Hello Michał Żygowski, Julius Werner, Krystian Hebel, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/69159 to review the following change. Change subject: security/tpm: make tis_probe() return tpm_family ...................................................................... security/tpm: make tis_probe() return tpm_family Via an out parameter. This is needed to be able to dynamically pick TSS implementation based on the information discovered on probing. Change-Id: I5006e0cdfef76ff79ce9e1cf280fcd5515ae01b0 Ticket: https://ticket.coreboot.org/issues/433 Signed-off-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> --- M src/drivers/crb/tis.c M src/drivers/i2c/tpm/cr50.c M src/drivers/i2c/tpm/tis.c M src/drivers/i2c/tpm/tis_atmel.c M src/drivers/i2c/tpm/tpm.c M src/drivers/i2c/tpm/tpm.h M src/drivers/pc80/tpm/tis.c M src/drivers/spi/tpm/tis.c M src/security/tpm/tis.h M src/security/tpm/tss/tcg-1.2/tss.c M src/security/tpm/tss/tcg-2.0/tss.c 11 files changed, 66 insertions(+), 22 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/69159/1 diff --git a/src/drivers/crb/tis.c b/src/drivers/crb/tis.c index 0bb53c7..1955466 100644 --- a/src/drivers/crb/tis.c +++ b/src/drivers/crb/tis.c @@ -46,10 +46,12 @@ return 0; } -tis_sendrecv_fn tis_probe(void) +tis_sendrecv_fn tis_probe(int *tpm_family) { struct tpm2_info info; + *tpm_family = 2; + /* Wake TPM up (if necessary) */ if (tpm2_init() != 0) return NULL; diff --git a/src/drivers/i2c/tpm/cr50.c b/src/drivers/i2c/tpm/cr50.c index d908823..fe307bb 100644 --- a/src/drivers/i2c/tpm/cr50.c +++ b/src/drivers/i2c/tpm/cr50.c @@ -411,8 +411,9 @@ chip->cancel = &cr50_i2c_tis_ready; } -int tpm_vendor_probe(unsigned int bus, uint32_t addr) +int tpm_vendor_probe(unsigned int bus, uint32_t addr, int *tpm_family) { + *tpm_family = 2; return 0; } diff --git a/src/drivers/i2c/tpm/tis.c b/src/drivers/i2c/tpm/tis.c index cc89924..68e2a67 100644 --- a/src/drivers/i2c/tpm/tis.c +++ b/src/drivers/i2c/tpm/tis.c @@ -119,9 +119,9 @@ return 0; } -tis_sendrecv_fn tis_probe(void) +tis_sendrecv_fn tis_probe(int *tpm_family) { - if (tpm_vendor_probe(CONFIG_DRIVER_TPM_I2C_BUS, CONFIG_DRIVER_TPM_I2C_ADDR)) + if (tpm_vendor_probe(CONFIG_DRIVER_TPM_I2C_BUS, CONFIG_DRIVER_TPM_I2C_ADDR, tpm_family)) return NULL; if (chip.is_open) { diff --git a/src/drivers/i2c/tpm/tis_atmel.c b/src/drivers/i2c/tpm/tis_atmel.c index 376586b..b1a6ccd 100644 --- a/src/drivers/i2c/tpm/tis_atmel.c +++ b/src/drivers/i2c/tpm/tis_atmel.c @@ -103,7 +103,8 @@ return 0; } -tis_sendrecv_fn tis_probe(void) +tis_sendrecv_fn tis_probe(int *tpm_family) { + *tpm_family = 1; return &i2c_tis_sendrecv; } diff --git a/src/drivers/i2c/tpm/tpm.c b/src/drivers/i2c/tpm/tpm.c index 68b7042..47d4b42 100644 --- a/src/drivers/i2c/tpm/tpm.c +++ b/src/drivers/i2c/tpm/tpm.c @@ -451,13 +451,15 @@ /* Initialization of I2C TPM */ -int tpm_vendor_probe(unsigned int bus, uint32_t addr) +int tpm_vendor_probe(unsigned int bus, uint32_t addr, int *tpm_family) { struct stopwatch sw; uint8_t buf = 0; int ret; long sw_run_duration = SLEEP_DURATION_PROBE_MS; + *tpm_family = 1; + tpm_dev.chip_type = UNKNOWN; tpm_dev.bus = bus; tpm_dev.addr = addr; diff --git a/src/drivers/i2c/tpm/tpm.h b/src/drivers/i2c/tpm/tpm.h index d4176cc..1cd9890 100644 --- a/src/drivers/i2c/tpm/tpm.h +++ b/src/drivers/i2c/tpm/tpm.h @@ -51,7 +51,7 @@ /* ---------- Interface for TPM vendor ------------ */ -int tpm_vendor_probe(unsigned int bus, uint32_t addr); +int tpm_vendor_probe(unsigned int bus, uint32_t addr, int *tpm_family); int tpm_vendor_init(struct tpm_chip *chip, unsigned int bus, uint32_t dev_addr); diff --git a/src/drivers/pc80/tpm/tis.c b/src/drivers/pc80/tpm/tis.c index e2b6a02..e4cd05b 100644 --- a/src/drivers/pc80/tpm/tis.c +++ b/src/drivers/pc80/tpm/tis.c @@ -386,14 +386,14 @@ * Returns 0 on success (the device is found or was found during an earlier * invocation) or TPM_DRIVER_ERR if the device is not found. */ -static u32 pc80_tis_probe(void) +static u32 pc80_tis_probe(int *tpm_family) { const char *device_name = "unknown"; const char *vendor_name = device_name; const struct device_name *dev; u32 didvid, intf_id; u16 vid, did; - int i, tpm_family; + int i; if (vendor_dev_id) return 0; /* Already probed. */ @@ -405,7 +405,7 @@ } intf_id = be32_to_cpu(tpm_read_intf_id(0)); - tpm_family = ((intf_id & 0xf) == 0xf ? 1 : 2); + *tpm_family = ((intf_id & 0xf) == 0xf ? 1 : 2); vendor_dev_id = didvid; @@ -420,7 +420,7 @@ } dev = &vendor_names[i].dev_names[j]; while (dev->dev_id != 0xffff) { - if (dev->dev_id == did && dev->tpm_family == tpm_family) { + if (dev->dev_id == did && dev->tpm_family == *tpm_family) { device_name = dev->dev_name; break; } @@ -440,7 +440,7 @@ return TPM_DRIVER_ERR; } - printk(BIOS_INFO, "Found TPM %d %s (0x%04x) by %s (0x%04x)\n", tpm_family, device_name, + printk(BIOS_INFO, "Found TPM %d %s (0x%04x) by %s (0x%04x)\n", *tpm_family, device_name, did, vendor_name, vid); return 0; } @@ -709,12 +709,15 @@ /* * tis_probe() * - * Probe for the TPM device and set it up for use within locality 0. Returns - * pointer to send-receive function on success or NULL on failure. + * Probe for the TPM device and set it up for use within locality 0. + * + * @tpm_family - pointer to int which is set to TPM family of the device (1 or 2) + * + * Returns pointer to send-receive function on success or NULL on failure. */ -tis_sendrecv_fn tis_probe(void) +tis_sendrecv_fn tis_probe(int *tpm_family) { - if (pc80_tis_probe()) + if (pc80_tis_probe(tpm_family)) return NULL; if (pc80_tis_open()) diff --git a/src/drivers/spi/tpm/tis.c b/src/drivers/spi/tpm/tis.c index 310b1c0..44576a2 100644 --- a/src/drivers/spi/tpm/tis.c +++ b/src/drivers/spi/tpm/tis.c @@ -40,11 +40,13 @@ return 0; } -tis_sendrecv_fn tis_probe(void) +tis_sendrecv_fn tis_probe(int *tpm_family) { struct spi_slave spi; struct tpm2_info info; + *tpm_family = 2; + if (spi_setup_slave(CONFIG_DRIVER_TPM_SPI_BUS, CONFIG_DRIVER_TPM_SPI_CHIP, &spi)) { printk(BIOS_ERR, "Failed to setup TPM SPI slave\n"); diff --git a/src/security/tpm/tis.h b/src/security/tpm/tis.h index 513a28c..eab62bd 100644 --- a/src/security/tpm/tis.h +++ b/src/security/tpm/tis.h @@ -50,10 +50,13 @@ /* * tis_probe() * - * Probe for the TPM device and set it up for use within locality 0. Returns - * pointer to send-receive function on success or NULL on failure. + * Probe for the TPM device and set it up for use within locality 0. + * + * @tpm_family - pointer to int which is set to TPM family of the device (1 or 2) + * + * Returns pointer to send-receive function on success or NULL on failure. */ -tis_sendrecv_fn tis_probe(void); +tis_sendrecv_fn tis_probe(int *tpm_family); /* TODO: This is supposed to be used only for Google TPM. Consider moving this to drivers/tpm/cr50.h. */ diff --git a/src/security/tpm/tss/tcg-1.2/tss.c b/src/security/tpm/tss/tcg-1.2/tss.c index ccdb6d7..f7c9788 100644 --- a/src/security/tpm/tss/tcg-1.2/tss.c +++ b/src/security/tpm/tss/tcg-1.2/tss.c @@ -144,13 +144,20 @@ uint32_t tlcl_lib_init(void) { + int tpm_family; + if (tis_sendrecv != NULL) return VB2_SUCCESS; - tis_sendrecv = tis_probe(); + tis_sendrecv = tis_probe(&tpm_family); if (tis_sendrecv == NULL) return VB2_ERROR_UNKNOWN; + if (tpm_family != 1) { + tis_sendrecv = NULL; + return VB2_ERROR_UNKNOWN; + } + return VB2_SUCCESS; } diff --git a/src/security/tpm/tss/tcg-2.0/tss.c b/src/security/tpm/tss/tcg-2.0/tss.c index f1acf9e..d270329 100644 --- a/src/security/tpm/tss/tcg-2.0/tss.c +++ b/src/security/tpm/tss/tcg-2.0/tss.c @@ -187,15 +187,24 @@ /* This function is called directly by vboot, uses vboot return types. */ uint32_t tlcl_lib_init(void) { + int tpm_family; + if (tis_sendrecv != NULL) return VB2_SUCCESS; - tis_sendrecv = tis_probe(); + tis_sendrecv = tis_probe(&tpm_family); if (tis_sendrecv == NULL) { printk(BIOS_ERR, "%s: tis_probe returned error\n", __func__); return VB2_ERROR_UNKNOWN; } + if (tpm_family != 2) { + tis_sendrecv = NULL; + printk(BIOS_ERR, "%s: tis_probe returned unsupported TPM family: %d\n", + __func__, tpm_family); + return VB2_ERROR_UNKNOWN; + } + return VB2_SUCCESS; } -- To view, visit https://review.coreboot.org/c/coreboot/+/69159 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5006e0cdfef76ff79ce9e1cf280fcd5515ae01b0 Gerrit-Change-Number: 69159 Gerrit-PatchSet: 1 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-MessageType: newchange

1 0

[M] Change in coreboot[master]: drivers/pc80/tpm: probe for TPM family of a device
by Sergii Dmytruk (Code Review) 02 Nov '22

02 Nov '22

Attention is currently required from: Michał Żygowski, Christian Walter, Julius Werner, Krystian Hebel. Hello build bot (Jenkins), Michał Żygowski, Christian Walter, Julius Werner, Krystian Hebel, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/69023 to look at the new patch set (#2). Change subject: drivers/pc80/tpm: probe for TPM family of a device ...................................................................... drivers/pc80/tpm: probe for TPM family of a device This is to handle the situation when device ID is the same for TPM1 and TPM2 versions of a device. Important change of behaviour: the code now rejects unknown TPMs. Change-Id: I5464771836c66bcc441efb7189ded416b8f53827 Ticket: https://ticket.coreboot.org/issues/433 Signed-off-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> --- M src/drivers/pc80/tpm/tis.c 1 file changed, 56 insertions(+), 23 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/69023/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/69023 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5464771836c66bcc441efb7189ded416b8f53827 Gerrit-Change-Number: 69023 Gerrit-PatchSet: 2 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-MessageType: newpatchset

1 0

Jump to page:

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2022