coreboot-gerrit October 2020

coreboot-gerrit@coreboot.org

1 participants
3474 discussions

Change in coreboot[master]: [RFC]drivers/smmstore: Implement SMMSTORE version 2
by Patrick Rudolph (Code Review) 22 Oct '20

22 Oct '20

Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/40520 ) Change subject: [RFC]drivers/smmstore: Implement SMMSTORE version 2 ...................................................................... [RFC]drivers/smmstore: Implement SMMSTORE version 2 Seperates the SMMSTORE into 64KiB blocks that can individually be read/written/erased. To be used by payloads that implement a FaultTolerant Variable store like Tianocore. The implementation has been tested against EDK2 master. An example EDK2 implementation can be found here: https://github.com/9elements/edk2-1/commit/c7cbcd527af397d5d8473f3e52720ffd… Change-Id: I25e49d184135710f3e6dd1ad3bed95de950fe057 Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> --- M src/drivers/smmstore/smi.c M src/drivers/smmstore/store.c M src/include/smmstore.h 3 files changed, 227 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/20/40520/1 diff --git a/src/drivers/smmstore/smi.c b/src/drivers/smmstore/smi.c index 57f79b8..501f3d3 100644 --- a/src/drivers/smmstore/smi.c +++ b/src/drivers/smmstore/smi.c @@ -60,6 +60,47 @@ break; } + case SMMSTORE_CMD_INFO: { + printk(BIOS_DEBUG, "Returning SMM store info\n"); + struct smmstore_params_info *params = param; + if (smmstore_update_info(params) == 0) + ret = SMMSTORE_RET_SUCCESS; + break; + } + case SMMSTORE_CMD_RAW_READ: { + printk(BIOS_DEBUG, "Raw read from SMM store, param = %p\n", param); + struct smmstore_params_raw_read *params = param; + void *buf = (void *)(uintptr_t)params->buf; + + if (range_check(buf, params->bufsize) != 0) + break; + + if (smmstore_rawread_region(buf, params->bufoffset, + params->block_id, &params->bufsize) == 0) + ret = SMMSTORE_RET_SUCCESS; + break; + } + case SMMSTORE_CMD_RAW_WRITE: { + printk(BIOS_DEBUG, "Raw write to SMM store, param = %p\n", param); + struct smmstore_params_raw_write *params = param; + void *buf = (void *)(uintptr_t)params->buf; + + if (range_check(buf, params->bufsize) != 0) + break; + + if (smmstore_rawwrite_region(buf, params->bufoffset, + params->block_id, &params->bufsize) == 0) + ret = SMMSTORE_RET_SUCCESS; + break; + } + case SMMSTORE_CMD_RAW_CLEAR: { + printk(BIOS_DEBUG, "Raw clear SMM store, param = %p\n", param); + struct smmstore_params_raw_clear *params = param; + + if (smmstore_rawclear_region(params->block_id) == 0) + ret = SMMSTORE_RET_SUCCESS; + break; + } default: printk(BIOS_DEBUG, "Unknown SMM store command: 0x%02x\n", command); diff --git a/src/drivers/smmstore/store.c b/src/drivers/smmstore/store.c index dbad085..e10f133 100644 --- a/src/drivers/smmstore/store.c +++ b/src/drivers/smmstore/store.c @@ -263,3 +263,112 @@ return 0; } + + +/* implementation of Version 2 */ + +static struct smmstore_params_info info_params; + +int smmstore_update_info(struct smmstore_params_info *out) +{ + struct region_device store; + + if (!info_params.block_size) { + if (lookup_store(&store) < 0) { + printk(BIOS_WARNING, "smm store: reading region failed\n"); + return -1; + } + info_params.block_size = 64 * KiB; + info_params.num_blocks = region_device_sz(&store) / info_params.block_size; + printk(BIOS_DEBUG, "smm store: %d # blocks with size 0x%x\n", + info_params.num_blocks, info_params.block_size); + } + + out->block_size = info_params.block_size; + out->num_blocks = info_params.num_blocks; + /* FIXME: Broken EDK2 always assumes memory mapped Firmware Block Volumes */ + out->mmap_addr = (uintptr_t)rdev_mmap_full(&store); + return 0; +} + +int smmstore_rawread_region(void *buf, uint32_t offset, uint32_t block_id, uint32_t *bufsize) +{ + struct region_device store; + ssize_t ret; + + if (lookup_store(&store) < 0) { + printk(BIOS_WARNING, "reading region failed\n"); + return -1; + } + + if (offset >= info_params.block_size || block_id >= info_params.num_blocks) { + printk(BIOS_WARNING, "access out of region requested\n"); + return -1; + } + + ssize_t tx = MIN(*bufsize, (block_id + 1) * info_params.block_size - offset); + printk(BIOS_DEBUG, "smm store: reading %p block %d, offset=0x%x, size=%zx\n", + buf, block_id, offset, tx); + ret = rdev_readat(&store, buf, block_id * info_params.block_size + offset, tx); + if (ret < 0) + return -1; + + *bufsize = ret; + + return 0; +} + +int smmstore_rawwrite_region(void *buf, uint32_t offset, uint32_t block_id, uint32_t *bufsize) +{ + struct region_device store; + ssize_t ret; + + if (lookup_store(&store) < 0) { + printk(BIOS_WARNING, "writing region failed\n"); + return -1; + } + printk(BIOS_DEBUG, "smm store: writing %p block %d, offset=0x%x, size=%x\n", + buf, block_id, offset, *bufsize); + + if (offset >= info_params.block_size || block_id >= info_params.num_blocks) { + printk(BIOS_WARNING, "access out of region requested\n"); + return -1; + } + + if (rdev_chain(&store, &store, block_id * info_params.block_size + offset, *bufsize)) { + printk(BIOS_WARNING, "not enough space for new data\n"); + return -1; + } + + ret = rdev_writeat(&store, buf, 0, *bufsize); + if (ret < 0) + return -1; + + *bufsize = ret; + + return 0; +} + +int smmstore_rawclear_region(uint32_t block_id) +{ + struct region_device store; + + if (block_id >= info_params.num_blocks) { + printk(BIOS_WARNING, "access out of region requested\n"); + return -1; + } + + if (lookup_store(&store) < 0) { + printk(BIOS_WARNING, "smm store: reading region failed\n"); + return -1; + } + + ssize_t res = rdev_eraseat(&store, block_id * info_params.block_size, + info_params.block_size); + if (res != info_params.block_size) { + printk(BIOS_WARNING, "smm store: erasing block failed\n"); + return -1; + } + + return 0; +} diff --git a/src/include/smmstore.h b/src/include/smmstore.h index af07ff0..6a32008 100644 --- a/src/include/smmstore.h +++ b/src/include/smmstore.h @@ -11,10 +11,18 @@ #define SMMSTORE_RET_FAILURE 1 #define SMMSTORE_RET_UNSUPPORTED 2 +/* Version 1 */ #define SMMSTORE_CMD_CLEAR 1 #define SMMSTORE_CMD_READ 2 #define SMMSTORE_CMD_APPEND 3 +/* Version 2 */ +#define SMMSTORE_CMD_INFO 4 +#define SMMSTORE_CMD_RAW_READ 5 +#define SMMSTORE_CMD_RAW_WRITE 6 +#define SMMSTORE_CMD_RAW_CLEAR 7 + +/* Version 1 */ struct smmstore_params_read { void *buf; ssize_t bufsize; @@ -27,12 +35,80 @@ size_t valsize; }; +/* Version 2 */ +/* + * Version 2 of the SMM requires as SMMSTORE of at least 256KiB of size, that + * has to be aligned to 64KiB. + * This allows the payload to store raw data in the SMMSTORE flash region. + * This can be used by a FaultTolerantWrite implementation, that uses at least + * two regions in an A/B update scheme. + * + * The Version 2 protocol seperates the SMMSTORE into 64KiB blocks, where each block + * can be read/written/cleared in an independent manner. + * FIXME: The data format isn't specified. + * + */ +/* + * Returns the number of blocks the SMMSTORE supports and their size. + * For EDK2 this should be at least two blocks with 64KiB each. + */ +struct smmstore_params_info { + uint32_t num_blocks; + uint32_t block_size; + uint32_t mmap_addr; +} __packed; + +/* + * Reads a chunk of raw data with size @bufsize from the block specified by + * @block_id starting at @bufoffset. + * The read data is placed in memory pointed to by @buf. + * + * @block_id must be less than num_blocks + * @bufoffset + @bufsize must be less than block_size + */ +struct smmstore_params_raw_write { + uint32_t buf; + uint32_t bufsize; + uint32_t bufoffset; + uint32_t block_id; +} __packed; + +/* + * Writes a chunk of raw data with size @bufsize to the block specified by + * @block_id starting at @bufoffset. + * + * @block_id must be less than num_blocks + * @bufoffset + @bufsize must be less than block_size + */ +struct smmstore_params_raw_read { + uint32_t buf; + uint32_t bufsize; + uint32_t bufoffset; + uint32_t block_id; +} __packed; + +/* + * Erases the specified block. + * + * @block_id must be less than num_blocks + */ +struct smmstore_params_raw_clear { + uint32_t block_id; +} __packed; + + /* SMM responder */ uint32_t smmstore_exec(uint8_t command, void *param); -/* implementation */ +/* implementation of Version 1 */ int smmstore_read_region(void *buf, ssize_t *bufsize); int smmstore_append_data(void *key, uint32_t key_sz, void *value, uint32_t value_sz); int smmstore_clear_region(void); +/* implementation of Version 2 */ +int smmstore_update_info(struct smmstore_params_info *info); +int smmstore_rawread_region(void *buf, uint32_t offset, uint32_t block_id, uint32_t *bufsize); +int smmstore_rawwrite_region(void *buf, uint32_t offset, uint32_t block_id, uint32_t *bufsize); +int smmstore_rawclear_region(uint32_t block_id); + #endif -- To view, visit https://review.coreboot.org/c/coreboot/+/40520 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I25e49d184135710f3e6dd1ad3bed95de950fe057 Gerrit-Change-Number: 40520 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-MessageType: newchange

12 81

Change in coreboot[master]: util/ifdtool: Enable CPU read for ME region
by Usha P (Code Review) 22 Oct '20

22 Oct '20

Usha P has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46441 ) Change subject: util/ifdtool: Enable CPU read for ME region ...................................................................... Patch Set 7: (1 comment) https://review.coreboot.org/c/coreboot/+/46441/7/util/ifdtool/ifdtool.c File util/ifdtool/ifdtool.c: https://review.coreboot.org/c/coreboot/+/46441/7/util/ifdtool/ifdtool.c@1208 PS7, Line 1208: /* CPU/BIOS can read ME. */ > How about making this a parameter? Yes, will implement it as an option and update the patch. -- To view, visit https://review.coreboot.org/c/coreboot/+/46441 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1d9a8e17fba19b717453476fbcb7bcf95b278abe Gerrit-Change-Number: 46441 Gerrit-PatchSet: 7 Gerrit-Owner: Usha P <usha.p(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Thu, 22 Oct 2020 12:19:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: util/ifdtool: Enable CPU read for ME region
by Rizwan Qureshi (Code Review) 22 Oct '20

22 Oct '20

Rizwan Qureshi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46441 ) Change subject: util/ifdtool: Enable CPU read for ME region ...................................................................... Patch Set 7: (2 comments) https://review.coreboot.org/c/coreboot/+/46441/7//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/46441/7//COMMIT_MSG@8 PS7, Line 8: : We are implementing a mechanism in coreboot to update CSME firmware, : this requires coreboot to be able to read CSME region. > Do you have a pointer to this? The current mechanism relies on querying the CSE to provide its versi […] The current mechanism is not relying on determining the version by reading CSE region, however we are exploring another way to determine the version either by reading a fixed offset (like the OEM Build version field) or parsing the CSE region. For that purpose we need a mechanism to enable the read on ME after it is locked. There is internal test case which uses ifdtool to lock ME region using and test write protection on that. When running this tests with the new CSE update implementation we run into problem since the CSE region is not readable. Providing an option in IFD tool to enable CPU read of CSME, will enable the test case to validate the new approach. https://review.coreboot.org/c/coreboot/+/46441/7//COMMIT_MSG@13 PS7, Line 13: This patch will enable read access to CSME region when locking. > Should ifdtool honor the settings that are already present in the descriptor if it is not all ffs? I […] A check in ifdtool to see if there is already a non-default region access applied can be implemented. However, during the development phase where the ME is not locked and the automated test cases use ifdtool to lock ME, we will need an option to skip disabling read. I agree that when ifdtool is used for locking it has to honor non-default region access values if already present. And we can add a -f (force) option to override the existing settings. May be another patch for that. -- To view, visit https://review.coreboot.org/c/coreboot/+/46441 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1d9a8e17fba19b717453476fbcb7bcf95b278abe Gerrit-Change-Number: 46441 Gerrit-PatchSet: 7 Gerrit-Owner: Usha P <usha.p(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Thu, 22 Oct 2020 12:09:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: drivers/aspeed: Downgrade error to info
by Patrick Rudolph (Code Review) 22 Oct '20

22 Oct '20

Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46561 ) Change subject: drivers/aspeed: Downgrade error to info ...................................................................... drivers/aspeed: Downgrade error to info It's quite common to run headless on servers where decoding the EDID would fail. Reduce error to info level when no EDID was found. Change-Id: I4052710d78be72a9ae8677ee2654d2e8d5158ebf Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> --- M src/drivers/aspeed/common/ast_mode_corebootfb.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/46561/1 diff --git a/src/drivers/aspeed/common/ast_mode_corebootfb.c b/src/drivers/aspeed/common/ast_mode_corebootfb.c index 8418b01..62e8d04 100644 --- a/src/drivers/aspeed/common/ast_mode_corebootfb.c +++ b/src/drivers/aspeed/common/ast_mode_corebootfb.c @@ -96,7 +96,7 @@ ast_software_i2c_read(ast, raw); if (decode_edid(raw, sizeof(raw), edid) != EDID_CONFORMANT) { - dev_err(dev->pdev, "Failed to decode EDID\n"); + dev_info(dev->pdev, "Failed to decode EDID\n"); printk(BIOS_DEBUG, "Assuming VGA for KVM\n"); memset(edid, 0, sizeof(*edid)); -- To view, visit https://review.coreboot.org/c/coreboot/+/46561 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4052710d78be72a9ae8677ee2654d2e8d5158ebf Gerrit-Change-Number: 46561 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-MessageType: newchange

2 2

Change in coreboot[master]: console: Make DEFAULT_CONSOLE_LOGLEVEL available for CONSOLE_OVERRIDE...
by Johnny Lin (Code Review) 22 Oct '20

22 Oct '20

Johnny Lin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45866 ) Change subject: console: Make DEFAULT_CONSOLE_LOGLEVEL available for CONSOLE_OVERRIDE_LOGLEVEL ...................................................................... console: Make DEFAULT_CONSOLE_LOGLEVEL available for CONSOLE_OVERRIDE_LOGLEVEL It's more flexible to be able to keep using DEFAULT_CONSOLE_LOGLEVEL when CONSOLE_OVERRIDE_LOGLEVEL is selected. Change-Id: Ib3ca8482459584e039a0e8881d79358948696ac2 Signed-off-by: Johnny Lin <johnny_lin(a)wiwynn.com> --- M src/console/Kconfig 1 file changed, 0 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/45866/1 diff --git a/src/console/Kconfig b/src/console/Kconfig index bad6c56..28138b3 100644 --- a/src/console/Kconfig +++ b/src/console/Kconfig @@ -315,8 +315,6 @@ Set to "y" when the platform overrides the loglevel by providing a get_console_loglevel routine. -if !CONSOLE_OVERRIDE_LOGLEVEL - choice prompt "Default console log level" default DEFAULT_CONSOLE_LOGLEVEL_8 if CHROMEOS @@ -375,8 +373,6 @@ help Map the log level config names to an integer. -endif - config NO_POST bool "Don't show any POST codes" default n -- To view, visit https://review.coreboot.org/c/coreboot/+/45866 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib3ca8482459584e039a0e8881d79358948696ac2 Gerrit-Change-Number: 45866 Gerrit-PatchSet: 1 Gerrit-Owner: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-MessageType: newchange

3 4

Change in coreboot[master]: soc/intel/common: add Kconfig to enable/disable the ACPI PM timer
by Michael Niewöhner (Code Review) 22 Oct '20

22 Oct '20

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45952 ) Change subject: soc/intel/common: add Kconfig to enable/disable the ACPI PM timer ...................................................................... Patch Set 24: (3 comments) https://review.coreboot.org/c/coreboot/+/45952/21//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/45952/21//COMMIT_MSG@21 PS21, Line 21: Note: On Apollolake USE_PM_ACPI_TIMER gets selected statically to avoid : confusing the user in menuconfig, where the option is shown due to use : of the common code for PM Timer emulation. On APL there is not hardware : PM ACPI Timer, so emulation needs to be always enabled. : > needs rework, when we decide to hide USE_PM_ACPI_TIMER somehow Done https://review.coreboot.org/c/coreboot/+/45952/21/src/soc/intel/apollolake/… File src/soc/intel/apollolake/Kconfig: https://review.coreboot.org/c/coreboot/+/45952/21/src/soc/intel/apollolake/… PS21, Line 108: select USE_PM_ACPI_TIMER > hmm... […] Done https://review.coreboot.org/c/coreboot/+/45952/21/src/soc/intel/common/bloc… File src/soc/intel/common/block/pmc/Kconfig: https://review.coreboot.org/c/coreboot/+/45952/21/src/soc/intel/common/bloc… PS21, Line 46: Disabling it enables the ACPI timer emulation in microcode > this is not true anymore Done -- To view, visit https://review.coreboot.org/c/coreboot/+/45952 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7f607f277eb14f84a7370ffb25a13226e7ccc917 Gerrit-Change-Number: 45952 Gerrit-PatchSet: 24 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 22 Oct 2020 07:50:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: security/vboot: remove tpm 1.2 functions
by Shelley Chen (Code Review) 22 Oct '20

22 Oct '20

Shelley Chen has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46615 ) Change subject: security/vboot: remove tpm 1.2 functions ...................................................................... security/vboot: remove tpm 1.2 functions Since MRC_SAVE_HASH_IN_TPM depends on TPM2, we can now remove all the tpm 1.2 functions in secdata_tpm.c. BUG=b:150502246 BRANCH=None TEST=make sure boards are still compiling on coreboot Jenkins Change-Id: I446dde36ce2233fc40687892da1fb515ce35b82b Signed-off-by: Shelley Chen <shchen(a)google.com> --- M src/security/vboot/secdata_tpm.c 1 file changed, 0 insertions(+), 155 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/46615/1 diff --git a/src/security/vboot/secdata_tpm.c b/src/security/vboot/secdata_tpm.c index 0304b92..686111b 100644 --- a/src/security/vboot/secdata_tpm.c +++ b/src/security/vboot/secdata_tpm.c @@ -7,9 +7,6 @@ #include <security/vboot/antirollback.h> #include <security/vboot/tpm_common.h> -#include <security/tpm/tspi.h> -#include <security/tpm/tss.h> -#include <security/tpm/tss/tcg-1.2/tss_structures.h> #include <vb2_api.h> #include <console/console.h> @@ -37,26 +34,6 @@ uint32_t antirollback_read_space_kernel(struct vb2_context *ctx) { - if (!CONFIG(TPM2)) { - /* - * Before reading the kernel space, verify its permissions. If - * the kernel space has the wrong permission, we give up. This - * will need to be fixed by the recovery kernel. We will have - * to worry about this because at any time (even with PP turned - * off) the TPM owner can remove and redefine a PP-protected - * space (but not write to it). - */ - uint32_t perms; - - RETURN_ON_FAILURE(tlcl_get_permissions(KERNEL_NV_INDEX, - &perms)); - if (perms != TPM_NV_PER_PPWRITE) { - printk(BIOS_ERR, - "TPM: invalid secdata_kernel permissions\n"); - return TPM_E_CORRUPTED_STATE; - } - } - uint8_t size = VB2_SECDATA_KERNEL_MIN_SIZE; RETURN_ON_FAILURE(tlcl_read(KERNEL_NV_INDEX, ctx->secdata_kernel, @@ -85,7 +62,6 @@ */ static const uint8_t mrc_hash_data[HASH_NV_SIZE] = { }; -#if CONFIG(TPM2) /* * Different sets of NVRAM space attributes apply to the "ro" spaces, * i.e. those which should not be possible to delete or modify once @@ -213,137 +189,6 @@ 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; - } -} - -/** - * Similarly to safe_write(), this ensures we don't fail a DefineSpace because - * we hit the TPM write limit. This is even less likely to happen than with - * writes because we only define spaces once at initialization, but we'd - * rather be paranoid about this. - */ -static uint32_t safe_define_space(uint32_t index, uint32_t perm, uint32_t size) -{ - uint32_t result = tlcl_define_space(index, perm, size); - if (result == TPM_E_MAXNVWRITES) { - RETURN_ON_FAILURE(tpm_clear_and_reenable()); - return tlcl_define_space(index, perm, size); - } else { - return result; - } -} - -static uint32_t set_mrc_hash_space(uint32_t index, const uint8_t *data) -{ - RETURN_ON_FAILURE(safe_define_space(index, - TPM_NV_PER_GLOBALLOCK | - TPM_NV_PER_PPWRITE, - HASH_NV_SIZE)); - RETURN_ON_FAILURE(safe_write(index, data, - HASH_NV_SIZE)); - - return TPM_SUCCESS; -} - -static uint32_t _factory_initialize_tpm(struct vb2_context *ctx) -{ - TPM_PERMANENT_FLAGS pflags; - uint32_t result; - - vb2api_secdata_kernel_create_v0(ctx); - - result = tlcl_get_permanent_flags(&pflags); - if (result != TPM_SUCCESS) - return result; - - /* - * TPM may come from the factory without physical presence finalized. - * Fix if necessary. - */ - VBDEBUG("TPM: physicalPresenceLifetimeLock=%d\n", - pflags.physicalPresenceLifetimeLock); - if (!pflags.physicalPresenceLifetimeLock) { - VBDEBUG("TPM: Finalizing physical presence\n"); - RETURN_ON_FAILURE(tlcl_finalize_physical_presence()); - } - - /* - * The TPM will not enforce the NV authorization restrictions until the - * execution of a TPM_NV_DefineSpace with the handle of - * TPM_NV_INDEX_LOCK. Here we create that space if it doesn't already - * exist. */ - VBDEBUG("TPM: nvLocked=%d\n", pflags.nvLocked); - if (!pflags.nvLocked) { - VBDEBUG("TPM: Enabling NV locking\n"); - RETURN_ON_FAILURE(tlcl_set_nv_locked()); - } - - /* Clear TPM owner, in case the TPM is already owned for some reason. */ - VBDEBUG("TPM: Clearing owner\n"); - RETURN_ON_FAILURE(tpm_clear_and_reenable()); - - /* Define and write secdata_kernel space. */ - RETURN_ON_FAILURE(safe_define_space(KERNEL_NV_INDEX, - TPM_NV_PER_PPWRITE, - VB2_SECDATA_KERNEL_SIZE_V02)); - RETURN_ON_FAILURE(safe_write(KERNEL_NV_INDEX, - ctx->secdata_kernel, - VB2_SECDATA_KERNEL_SIZE_V02)); - - /* Define and write secdata_firmware space. */ - RETURN_ON_FAILURE(safe_define_space(FIRMWARE_NV_INDEX, - TPM_NV_PER_GLOBALLOCK | - TPM_NV_PER_PPWRITE, - VB2_SECDATA_FIRMWARE_SIZE)); - RETURN_ON_FAILURE(safe_write(FIRMWARE_NV_INDEX, - ctx->secdata_firmware, - VB2_SECDATA_FIRMWARE_SIZE)); - - /* - * Define and set rec hash space, if available. No need to - * create the RW hash space because we will definitely boot - * once in normal mode before shipping, meaning that the space - * will get created with correct permissions while still in in - * our hands. - */ - if (CONFIG(VBOOT_HAS_REC_HASH_SPACE)) - RETURN_ON_FAILURE(set_mrc_hash_space(MRC_REC_HASH_NV_INDEX, mrc_hash_data)); - - return TPM_SUCCESS; -} - -uint32_t antirollback_lock_space_firmware(void) -{ - return tlcl_set_global_lock(); -} - -uint32_t antirollback_lock_space_mrc_hash(uint32_t index) -{ - /* - * Nothing needs to be done here, since global lock is already set while - * locking firmware space. - */ - return TPM_SUCCESS; -} -#endif - /** * Perform one-time initializations. * -- To view, visit https://review.coreboot.org/c/coreboot/+/46615 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I446dde36ce2233fc40687892da1fb515ce35b82b Gerrit-Change-Number: 46615 Gerrit-PatchSet: 1 Gerrit-Owner: Shelley Chen <shchen(a)google.com> Gerrit-MessageType: newchange

3 6

Change in coreboot[master]: util/ifdtool: Enable CPU read for ME region
by Furquan Shaikh (Code Review) 22 Oct '20

22 Oct '20

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46441 ) Change subject: util/ifdtool: Enable CPU read for ME region ...................................................................... Patch Set 7: (2 comments) https://review.coreboot.org/c/coreboot/+/46441/7//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/46441/7//COMMIT_MSG@8 PS7, Line 8: : We are implementing a mechanism in coreboot to update CSME firmware, : this requires coreboot to be able to read CSME region. Do you have a pointer to this? The current mechanism relies on querying the CSE to provide its version. What other information is being gathered by reading the CSE region directly? https://review.coreboot.org/c/coreboot/+/46441/7//COMMIT_MSG@13 PS7, Line 13: This patch will enable read access to CSME region when locking. Should ifdtool honor the settings that are already present in the descriptor if it is not all ffs? Is this a problem that the region access bits are set to one value using FIT tool and then get overridden here? -- To view, visit https://review.coreboot.org/c/coreboot/+/46441 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1d9a8e17fba19b717453476fbcb7bcf95b278abe Gerrit-Change-Number: 46441 Gerrit-PatchSet: 7 Gerrit-Owner: Usha P <usha.p(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Thu, 22 Oct 2020 06:35:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: [WIP]:gpp b2 for delbin and volteer and disable sdcard
by Kane Chen (Code Review) 22 Oct '20

22 Oct '20

Kane Chen has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46648 ) Change subject: [WIP]:gpp b2 for delbin and volteer and disable sdcard ...................................................................... [WIP]:gpp b2 for delbin and volteer and disable sdcard Signed-off-by: Kane Chen <kane.chen(a)intel.com> Change-Id: I79c7ab45769f91536da06860ff0b06db0af2851d --- M src/mainboard/google/volteer/variants/baseboard/devicetree.cb M src/mainboard/google/volteer/variants/baseboard/gpio.c M src/mainboard/google/volteer/variants/delbin/gpio.c 3 files changed, 10 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/46648/1 diff --git a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb index 7486aef..097104a 100644 --- a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb @@ -89,12 +89,13 @@ register "PcieRpLtrEnable[10]" = "1" register "HybridStorageMode" = "0" - # Enable SD Card PCIE 8 using clk 3 - register "PcieRpEnable[7]" = "1" - register "PcieRpLtrEnable[7]" = "1" - register "PcieRpHotPlug[7]" = "1" - register "PcieClkSrcUsage[3]" = "7" - register "PcieClkSrcClkReq[3]" = "3" + # Disable SD Card PCIE 8 using clk 3 + register "PcieRpEnable[7]" = "0" + register "PcieRpLtrEnable[7]" = "0" + register "PcieRpHotPlug[7]" = "0" + register "PcieClkSrcUsage[3]" = "0xff" + + # Enable WLAN PCIE 7 using clk 1 register "PcieRpEnable[6]" = "1" diff --git a/src/mainboard/google/volteer/variants/baseboard/gpio.c b/src/mainboard/google/volteer/variants/baseboard/gpio.c index 5d367e7..16749b5 100644 --- a/src/mainboard/google/volteer/variants/baseboard/gpio.c +++ b/src/mainboard/google/volteer/variants/baseboard/gpio.c @@ -7,6 +7,9 @@ /* Pad configuration in ramstage */ /* Leave eSPI pins untouched from default settings */ static const struct pad_config gpio_table[] = { + + /* B2 : CPU_GP2 ==> EN_PP3300_SD */ + PAD_CFG_GPO(GPP_B2, 1, DEEP), /* A0 thru A6 come configured out of reset, do not touch */ /* A0 : ESPI_IO0 ==> ESPI_IO_0 */ /* A1 : ESPI_IO1 ==> ESPI_IO_1 */ @@ -54,8 +57,6 @@ PAD_CFG_NF(GPP_B0, NONE, DEEP, NF1), /* B1 : CORE_VID1 */ PAD_CFG_NF(GPP_B1, NONE, DEEP, NF1), - /* B2 : VRALERT# ==> VRALERT_L */ - PAD_CFG_NF(GPP_B2, NONE, DEEP, NF1), /* B3 : CPU_GP2 ==> NC */ PAD_NC(GPP_B3, NONE), /* B4 : CPU_GP3 ==> NC */ diff --git a/src/mainboard/google/volteer/variants/delbin/gpio.c b/src/mainboard/google/volteer/variants/delbin/gpio.c index 5748bb3..f3e80cc 100644 --- a/src/mainboard/google/volteer/variants/delbin/gpio.c +++ b/src/mainboard/google/volteer/variants/delbin/gpio.c @@ -27,8 +27,6 @@ /* A23 : I2S1_SCLK ==> I2S1_SPKR_SCLK */ PAD_CFG_NF(GPP_A23, NONE, DEEP, NF1), - /* B2 : VRALERT# ==> NC */ - PAD_NC(GPP_B2, NONE), /* B9 : I2C5_SDA ==> PCH_I2C5_TRACKPAD_SDA */ PAD_CFG_NF(GPP_B9, NONE, DEEP, NF1), /* B10 : I2C5_SCL ==> PCH_I2C5_TRACKPAD_SCL */ -- To view, visit https://review.coreboot.org/c/coreboot/+/46648 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I79c7ab45769f91536da06860ff0b06db0af2851d Gerrit-Change-Number: 46648 Gerrit-PatchSet: 1 Gerrit-Owner: Kane Chen <kane.chen(a)intel.com> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: HACK: mt8192: Add SPI support
by Yidi Lin (Code Review) 22 Oct '20

22 Oct '20

Hello Nicolas Boichat, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/46385 to review the following change. Change subject: HACK: mt8192: Add SPI support ...................................................................... HACK: mt8192: Add SPI support Missing pinctrl, so we just hardcode SPI1/SPI5 pins for now. TODO: There's actually 8 SPI controllers, I think BUG=none TEST=boot asurada Change-Id: I0f169407d1726899fd0c42e144d907024f036c6a --- M src/mainboard/google/asurada/Kconfig M src/mainboard/google/asurada/bootblock.c 2 files changed, 6 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/46385/1 diff --git a/src/mainboard/google/asurada/Kconfig b/src/mainboard/google/asurada/Kconfig index 894d566..7c93815 100644 --- a/src/mainboard/google/asurada/Kconfig +++ b/src/mainboard/google/asurada/Kconfig @@ -39,7 +39,7 @@ config DRIVER_TPM_SPI_BUS hex - default 0x0 + default 0x5 # On MT8192 the SPI flash is actually using a SPI-NOR controller with its own bus. # The number here should be a virtual value as (SPI_BUS_NUMBER + 1). @@ -49,6 +49,6 @@ config EC_GOOGLE_CHROMEEC_SPI_BUS hex - default 0x2 + default 0x1 endif diff --git a/src/mainboard/google/asurada/bootblock.c b/src/mainboard/google/asurada/bootblock.c index 5dcae8c..3eb05e1 100644 --- a/src/mainboard/google/asurada/bootblock.c +++ b/src/mainboard/google/asurada/bootblock.c @@ -1,7 +1,11 @@ /* SPDX-License-Identifier: GPL-2.0-only */ #include <bootblock_common.h> +#include <soc/spi.h> void bootblock_mainboard_init(void) { + mtk_spi_init(CONFIG_EC_GOOGLE_CHROMEEC_SPI_BUS, SPI_PAD0_MASK, 1 * MHz, 0); + mtk_spi_init(CONFIG_DRIVER_TPM_SPI_BUS, SPI_PAD0_MASK, 1 * MHz, 0); + //gpio_eint_configure(CR50_IRQ, IRQ_TYPE_EDGE_RISING); } -- To view, visit https://review.coreboot.org/c/coreboot/+/46385 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0f169407d1726899fd0c42e144d907024f036c6a Gerrit-Change-Number: 46385 Gerrit-PatchSet: 1 Gerrit-Owner: Yidi Lin <yidi.lin(a)mediatek.com> Gerrit-Reviewer: Nicolas Boichat <drinkcat(a)chromium.org> Gerrit-MessageType: newchange

3 7

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit October 2020