coreboot-gerrit October 2022

coreboot-gerrit@coreboot.org

1 participants
5881 discussions

[S] Change in coreboot[master]: soc/amd/common: Add a config to keep signed AMD/PSP FW separately
by Martin Roth (Code Review) 03 Oct '22

03 Oct '22

Martin Roth has submitted this change. ( https://review.coreboot.org/c/coreboot/+/59867 ) ( 15 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: soc/amd/common: Add a config to keep signed AMD/PSP FW separately ...................................................................... soc/amd/common: Add a config to keep signed AMD/PSP FW separately Enabling this config will put signed amd firmwares into SIGNED_AMDFW_[AB] region which is outside FW_MAIN_[AB]. Vboot only verifies FW_MAIN_[AB] so these regions will not be verified by vboot, instead the PSP will verify them. As a result we have less to load and verify from SPI rom which means faster boot time. BUG=b:206909680 TEST=Build Skyrim with modified fmap and Kconfig. Change-Id: If4fd3cff11a38d82afb8c5ce379f1d1b5b9adfbf Signed-off-by: Kangheui Won <khwon(a)chromium.org> Signed-off-by: Karthikeyan Ramasubramanian <kramasub(a)google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/59867 Reviewed-by: Jon Murphy <jpmurphy(a)google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/soc/amd/common/psp_verstage/Kconfig 1 file changed, 31 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Jon Murphy: Looks good to me, approved diff --git a/src/soc/amd/common/psp_verstage/Kconfig b/src/soc/amd/common/psp_verstage/Kconfig index 13dd716..526a4ae7 100644 --- a/src/soc/amd/common/psp_verstage/Kconfig +++ b/src/soc/amd/common/psp_verstage/Kconfig @@ -29,3 +29,9 @@ On SoCs where PSP uses A/B recovery layout, PSP support relative addressing from the start of the SPI ROM. Enable this config on SoCs where PSP supports relative addressing so that PSP verstage can pass the offset. + +config SEPARATE_SIGNED_PSPFW + def_bool n + help + Put signed AMD/PSP firmwares outside FW_MAIN_[AB] so vboot doesn't verify them, + and rely on PSP's verification. -- To view, visit https://review.coreboot.org/c/coreboot/+/59867 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If4fd3cff11a38d82afb8c5ce379f1d1b5b9adfbf Gerrit-Change-Number: 59867 Gerrit-PatchSet: 17 Gerrit-Owner: Kangheui Won <khwon(a)chromium.org> 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: Jon Murphy <jpmurphy(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.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-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

[L] Change in coreboot[master]: util/amdfwtool: Add options to separate signed firmwares
by Martin Roth (Code Review) 03 Oct '22

03 Oct '22

Martin Roth has submitted this change. ( https://review.coreboot.org/c/coreboot/+/59866 ) Change subject: util/amdfwtool: Add options to separate signed firmwares ...................................................................... util/amdfwtool: Add options to separate signed firmwares Add support for separating signed firmwares into another CBFS. If sig_opt flag in AMD/PSPFW file header is 1, it means that the firmware is signed against AMD chain of trust and will be verified by PSP. If those firmware binaries are put outside FW_MAIN_[AB], vboot can skip redundant verification, improving overall verification time. BUG=b:206909680 TEST=Build amdfwtool. Build Skyrim BIOS image and boot to OS. Change-Id: I9f3610a7002b2a9c70946b083b0b3be6934200b0 Signed-off-by: Kangheui Won <khwon(a)chromium.org> Signed-off-by: Karthikeyan Ramasubramanian <kramasub(a)google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/59866 Reviewed-by: Jon Murphy <jpmurphy(a)google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M util/amdfwtool/amdfwtool.c M util/amdfwtool/amdfwtool.h 2 files changed, 322 insertions(+), 13 deletions(-) Approvals: build bot (Jenkins): Verified Jon Murphy: Looks good to me, approved diff --git a/util/amdfwtool/amdfwtool.c b/util/amdfwtool/amdfwtool.c index c25c390..ff4b623 100644 --- a/util/amdfwtool/amdfwtool.c +++ b/util/amdfwtool/amdfwtool.c @@ -223,7 +223,7 @@ } amd_fw_entry amd_psp_fw_table[] = { - { .type = AMD_FW_PSP_PUBKEY, .level = PSP_BOTH | PSP_LVL2_AB }, + { .type = AMD_FW_PSP_PUBKEY, .level = PSP_BOTH | PSP_LVL2_AB, .skip_hashing = true }, { .type = AMD_FW_PSP_BOOTLOADER, .level = PSP_BOTH | PSP_LVL2_AB }, { .type = AMD_FW_PSP_SMU_FIRMWARE, .subprog = 0, .level = PSP_BOTH | PSP_LVL2_AB }, { .type = AMD_FW_PSP_RECOVERY, .level = PSP_LVL1 }, @@ -231,7 +231,8 @@ { .type = AMD_FW_PSP_SECURED_OS, .level = PSP_LVL2 | PSP_LVL2_AB }, { .type = AMD_FW_PSP_NVRAM, .level = PSP_LVL2 | PSP_LVL2_AB }, { .type = AMD_FW_PSP_SMU_FIRMWARE, .subprog = 2, .level = PSP_BOTH | PSP_LVL2_AB }, - { .type = AMD_FW_PSP_SECURED_DEBUG, .level = PSP_LVL2 | PSP_LVL2_AB }, + { .type = AMD_FW_PSP_SECURED_DEBUG, .level = PSP_LVL2 | PSP_LVL2_AB, + .skip_hashing = true }, { .type = AMD_FW_PSP_TRUSTLETS, .level = PSP_LVL2 | PSP_LVL2_AB }, { .type = AMD_FW_PSP_TRUSTLETKEY, .level = PSP_LVL2 | PSP_LVL2_AB }, { .type = AMD_FW_PSP_SMU_FIRMWARE2, .subprog = 2, .level = PSP_BOTH | PSP_LVL2_AB }, @@ -242,7 +243,7 @@ { .type = AMD_PSP_FUSE_CHAIN, .level = PSP_LVL2 | PSP_LVL2_AB }, { .type = AMD_DEBUG_UNLOCK, .level = PSP_LVL2 | PSP_LVL2_AB }, { .type = AMD_HW_IPCFG, .level = PSP_LVL2 | PSP_LVL2_AB }, - { .type = AMD_WRAPPED_IKEK, .level = PSP_BOTH | PSP_LVL2_AB }, + { .type = AMD_WRAPPED_IKEK, .level = PSP_BOTH | PSP_LVL2_AB, .skip_hashing = true }, { .type = AMD_TOKEN_UNLOCK, .level = PSP_BOTH | PSP_LVL2_AB }, { .type = AMD_SEC_GASKET, .subprog = 0, .level = PSP_BOTH | PSP_LVL2_AB }, { .type = AMD_SEC_GASKET, .subprog = 2, .level = PSP_BOTH | PSP_LVL2_AB }, @@ -267,7 +268,7 @@ { .type = AMD_FW_SPIROM_CFG, .level = PSP_LVL2 | PSP_LVL2_AB }, { .type = AMD_RPMC_NVRAM, .level = PSP_LVL2 | PSP_LVL2_AB }, { .type = AMD_FW_PSP_BOOTLOADER_AB, .level = PSP_LVL2 | PSP_LVL2_AB }, - { .type = AMD_TA_IKEK, .level = PSP_BOTH | PSP_LVL2_AB }, + { .type = AMD_TA_IKEK, .level = PSP_BOTH | PSP_LVL2_AB, .skip_hashing = true }, { .type = AMD_ABL0, .level = PSP_BOTH | PSP_LVL2_AB }, { .type = AMD_ABL1, .level = PSP_BOTH | PSP_LVL2_AB }, { .type = AMD_ABL2, .level = PSP_BOTH | PSP_LVL2_AB }, @@ -543,6 +544,66 @@ return bytes; } +static ssize_t read_from_file_to_buf(int fd, void *buf, size_t buf_size) +{ + ssize_t bytes; + size_t total_bytes = 0; + + do { + bytes = read(fd, buf + total_bytes, buf_size - total_bytes); + if (bytes == 0) { + fprintf(stderr, "Reached EOF probably\n"); + break; + } + + if (bytes < 0 && errno == EAGAIN) + bytes = 0; + + if (bytes < 0) { + fprintf(stderr, "Read failure %s\n", strerror(errno)); + return bytes; + } + + total_bytes += bytes; + } while (total_bytes < buf_size); + + if (total_bytes != buf_size) { + fprintf(stderr, "Read data size(%zu) != buffer size(%zu)\n", + total_bytes, buf_size); + return -1; + } + return buf_size; +} + +static ssize_t write_from_buf_to_file(int fd, const void *buf, size_t buf_size) +{ + ssize_t bytes; + size_t total_bytes = 0; + + do { + bytes = write(fd, buf + total_bytes, buf_size - total_bytes); + if (bytes < 0 && errno == EAGAIN) + bytes = 0; + + if (bytes < 0) { + fprintf(stderr, "Write failure %s\n", strerror(errno)); + lseek(fd, SEEK_CUR, -total_bytes); + return bytes; + } + + total_bytes += bytes; + } while (total_bytes < buf_size); + + if (total_bytes != buf_size) { + fprintf(stderr, "Wrote more data(%zu) than buffer size(%zu)\n", + total_bytes, buf_size); + lseek(fd, SEEK_CUR, -total_bytes); + return -1; + } + + return buf_size; +} + enum platform { PLATFORM_UNKNOWN, PLATFORM_STONEYRIDGE, @@ -582,6 +643,18 @@ return psp_id; } +static uint16_t get_psp_fw_type(enum platform soc_id, struct amd_fw_header *header) +{ + switch (soc_id) { + case PLATFORM_MENDOCINO: + /* Fallback to fw_type if fw_id is not populated, which serves the same + purpose on older SoCs. */ + return header->fw_id ? header->fw_id : header->fw_type; + default: + return header->fw_type; + } +} + static void integrate_firmwares(context *ctx, embedded_firmware *romsig, amd_fw_entry *fw_table) @@ -674,6 +747,149 @@ } } +/** + * process_signed_psp_firmwares() - Process the signed PSP binaries to keep them separate + * @signed_rom: Output file path grouping all the signed PSP binaries. + * @fw_table: Table of all the PSP firmware entries/binaries to be processed. + * @signed_start_addr: Offset of the FMAP section, within the flash device, to hold + * the signed PSP binaries. + * @soc_id: SoC ID of the PSP binaries. + */ +static void process_signed_psp_firmwares(const char *signed_rom, + amd_fw_entry *fw_table, + uint64_t signed_start_addr, + enum platform soc_id) +{ + unsigned int i; + int fd; + int signed_rom_fd; + ssize_t bytes, align_bytes; + uint8_t *buf; + struct amd_fw_header header; + struct stat fd_stat; + /* Every blob in amdfw*.rom has to start at address aligned to 0x100. Prepare an + alignment data with 0xff to pad the blobs and meet the alignment requirement. */ + uint8_t align_data[BLOB_ALIGNMENT - 1]; + + memset(align_data, 0xff, sizeof(align_data)); + signed_rom_fd = open(signed_rom, O_RDWR | O_CREAT | O_TRUNC, + S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH); + if (signed_rom_fd < 0) { + fprintf(stderr, "Error opening file: %s: %s\n", + signed_rom, strerror(errno)); + return; + } + + for (i = 0; fw_table[i].type != AMD_FW_INVALID; i++) { + if (!(fw_table[i].filename) || fw_table[i].skip_hashing) + continue; + + memset(&header, 0, sizeof(header)); + + fd = open(fw_table[i].filename, O_RDONLY); + if (fd < 0) { + /* Keep the file along with set of unsigned PSP binaries & continue. */ + fprintf(stderr, "Error opening file: %s: %s\n", + fw_table[i].filename, strerror(errno)); + continue; + } + + if (fstat(fd, &fd_stat)) { + /* Keep the file along with set of unsigned PSP binaries & continue. */ + fprintf(stderr, "fstat error: %s\n", strerror(errno)); + close(fd); + continue; + } + + bytes = read_from_file_to_buf(fd, &header, sizeof(struct amd_fw_header)); + if (bytes != (ssize_t)sizeof(struct amd_fw_header)) { + /* Keep the file along with set of unsigned PSP binaries & continue. */ + fprintf(stderr, "%s: Error reading header from %s\n", + __func__, fw_table[i].filename); + close(fd); + continue; + } + + /* If firmware header looks like invalid, assume it's not signed */ + if (!header.fw_type && !header.fw_id) { + fprintf(stderr, "%s: Invalid FWID for %s\n", + __func__, fw_table[i].filename); + close(fd); + continue; + } + + if (header.size_total != fd_stat.st_size) { + close(fd); + continue; + } + + /* PSP binary is not signed and should not be part of signed PSP binaries + set. */ + if (header.sig_opt != 1) { + close(fd); + continue; + } + + buf = malloc(fd_stat.st_size); + if (!buf) { + /* Keep the file along with set of unsigned PSP binaries & continue. */ + fprintf(stderr, "%s: failed to allocate memory with size %lld\n", + __func__, (long long)fd_stat.st_size); + close(fd); + continue; + } + + lseek(fd, SEEK_SET, 0); + bytes = read_from_file_to_buf(fd, buf, fd_stat.st_size); + if (bytes != fd_stat.st_size) { + /* Keep the file along with set of unsigned PSP binaries & continue. */ + fprintf(stderr, "%s: failed to read %s\n", + __func__, fw_table[i].filename); + free(buf); + close(fd); + continue; + } + + bytes = write_from_buf_to_file(signed_rom_fd, buf, fd_stat.st_size); + if (bytes != fd_stat.st_size) { + /* Keep the file along with set of unsigned PSP binaries & continue. */ + fprintf(stderr, "%s: failed to write %s\n", + __func__, fw_table[i].filename); + free(buf); + close(fd); + continue; + } + + /* Write Blob alignment bytes */ + align_bytes = 0; + if (fd_stat.st_size & (BLOB_ALIGNMENT - 1)) { + align_bytes = BLOB_ALIGNMENT - + (fd_stat.st_size & (BLOB_ALIGNMENT - 1)); + bytes = write_from_buf_to_file(signed_rom_fd, align_data, align_bytes); + if (bytes != align_bytes) { + fprintf(stderr, "%s: failed to write alignment data for %s\n", + __func__, fw_table[i].filename); + lseek(signed_rom_fd, SEEK_CUR, -fd_stat.st_size); + free(buf); + close(fd); + continue; + } + } + + /* File is successfully processed and is part of signed PSP binaries set. */ + fw_table[i].fw_id = get_psp_fw_type(soc_id, &header); + fw_table[i].addr_signed = signed_start_addr; + fw_table[i].file_size = (uint32_t)fd_stat.st_size; + + signed_start_addr += fd_stat.st_size + align_bytes; + + free(buf); + close(fd); + } + + close(signed_rom_fd); +} + static void integrate_psp_ab(context *ctx, psp_directory_table *pspdir, psp_directory_table *pspdir2, ish_directory_table *ish, amd_fw_type ab, enum platform soc_id) @@ -825,22 +1041,31 @@ count++; } else if (fw_table[i].filename != NULL) { - bytes = copy_blob(BUFF_CURRENT(*ctx), - fw_table[i].filename, BUFF_ROOM(*ctx)); - if (bytes < 0) { - free(ctx->rom); - exit(1); + if (fw_table[i].addr_signed) { + pspdir->entries[count].addr = + RUN_OFFSET(*ctx, fw_table[i].addr_signed); + pspdir->entries[count].address_mode = + SET_ADDR_MODE_BY_TABLE(pspdir); + bytes = fw_table[i].file_size; + } else { + bytes = copy_blob(BUFF_CURRENT(*ctx), + fw_table[i].filename, BUFF_ROOM(*ctx)); + if (bytes < 0) { + free(ctx->rom); + exit(1); + } + pspdir->entries[count].addr = RUN_CURRENT(*ctx); + pspdir->entries[count].address_mode = + SET_ADDR_MODE_BY_TABLE(pspdir); + ctx->current = ALIGN(ctx->current + bytes, + BLOB_ALIGNMENT); } pspdir->entries[count].type = fw_table[i].type; pspdir->entries[count].subprog = fw_table[i].subprog; pspdir->entries[count].rsvd = 0; pspdir->entries[count].size = (uint32_t)bytes; - pspdir->entries[count].addr = RUN_CURRENT(*ctx); - pspdir->entries[count].address_mode = SET_ADDR_MODE_BY_TABLE(pspdir); - ctx->current = ALIGN(ctx->current + bytes, - BLOB_ALIGNMENT); count++; } else { /* This APU doesn't have this firmware. */ @@ -1257,6 +1482,8 @@ AMDFW_OPT_SHAREDMEM, AMDFW_OPT_SHAREDMEM_SIZE, AMDFW_OPT_SOC_NAME, + AMDFW_OPT_SIGNED_OUTPUT, + AMDFW_OPT_SIGNED_ADDR, /* begin after ASCII characters */ LONGOPT_SPI_READ_MODE = 256, LONGOPT_SPI_SPEED = 257, @@ -1317,6 +1544,9 @@ {"sharedmem-size", required_argument, 0, AMDFW_OPT_SHAREDMEM_SIZE }, {"soc-name", required_argument, 0, AMDFW_OPT_SOC_NAME }, + {"signed-output", required_argument, 0, AMDFW_OPT_SIGNED_OUTPUT }, + {"signed-addr", required_argument, 0, AMDFW_OPT_SIGNED_ADDR }, + {"config", required_argument, 0, AMDFW_OPT_CONFIG }, {"debug", no_argument, 0, AMDFW_OPT_DEBUG }, {"help", no_argument, 0, AMDFW_OPT_HELP }, @@ -1567,6 +1797,8 @@ uint8_t efs_spi_readmode = 0xff; uint8_t efs_spi_speed = 0xff; uint8_t efs_spi_micron_flag = 0xff; + const char *signed_output_file = NULL; + uint64_t signed_start_addr = 0x0; amd_cb_config cb_config = { 0 }; int debug = 0; @@ -1713,6 +1945,14 @@ } sub = instance = 0; break; + case AMDFW_OPT_SIGNED_OUTPUT: + signed_output_file = optarg; + sub = instance = 0; + break; + case AMDFW_OPT_SIGNED_ADDR: + signed_start_addr = strtoull(optarg, NULL, 10); + sub = instance = 0; + break; case LONGOPT_SPI_READ_MODE: efs_spi_readmode = strtoull(optarg, NULL, 16); sub = instance = 0; @@ -1919,6 +2159,14 @@ ctx.current = ALIGN(ctx.current, 0x10000U); /* TODO: is it necessary? */ ctx.current_table = 0; + /* If the tool is invoked with command-line options to keep the signed PSP + binaries separate, process the signed binaries first. */ + if (signed_output_file && signed_start_addr) + process_signed_psp_firmwares(signed_output_file, + amd_psp_fw_table, + signed_start_addr, + soc_id); + if (cb_config.multi_level) { /* Do 2nd PSP directory followed by 1st */ pspdir2 = new_psp_dir(&ctx, cb_config.multi_level); diff --git a/util/amdfwtool/amdfwtool.h b/util/amdfwtool/amdfwtool.h index 6b13edc..fd04064 100644 --- a/util/amdfwtool/amdfwtool.h +++ b/util/amdfwtool/amdfwtool.h @@ -275,14 +275,52 @@ #define PSP_BOTH_AB (PSP_LVL1_AB | PSP_LVL2_AB) typedef struct _amd_fw_entry { amd_fw_type type; + /* Mendocino and later SoCs use fw_id instead of fw_type. fw_type is still around + for backwards compatibility. fw_id can be populated from the PSP binary file. */ + uint16_t fw_id; char *filename; uint8_t subprog; uint64_t dest; size_t size; int level; uint64_t other; + /* If the binary is signed and the tool is invoked to keep the signed binaries separate, + then this field is populated with the offset of the concerned PSP binary (relative to + BIOS or PSP Directory table). */ + uint64_t addr_signed; + uint32_t file_size; + /* Some files that don't have amd_fw_header have to be skipped from hashing. These files + include but not limited to: *iKek*, *.tkn, *.stkn */ + bool skip_hashing; } amd_fw_entry; +/* Most PSP binaries, if not all, have the following header format. */ +struct amd_fw_header { + uint8_t reserved_0[20]; + uint32_t fw_size_signed; + uint8_t reserved_18[24]; + /* 1 if the image is signed, 0 otherwise */ + uint32_t sig_opt; + uint32_t sig_id; + uint8_t sig_param[16]; + uint32_t comp_opt; + uint8_t reserved_4c[4]; + uint32_t uncomp_size; + uint32_t comp_size; + /* Starting MDN fw_id is populated instead of fw_type. */ + uint16_t fw_id; + uint8_t reserved_5a[18]; + uint32_t size_total; + uint8_t reserved_70[12]; + /* Starting MDN fw_id is populated instead of fw_type. fw_type will still be around + for backwards compatibility. */ + uint8_t fw_type; + uint8_t fw_subtype; + uint8_t fw_subprog; + uint8_t reserved_7f; + uint8_t reserved_80[128]; +} __packed; + typedef struct _amd_cb_config { bool have_whitelist; bool unlock_secure; -- To view, visit https://review.coreboot.org/c/coreboot/+/59866 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9f3610a7002b2a9c70946b083b0b3be6934200b0 Gerrit-Change-Number: 59866 Gerrit-PatchSet: 14 Gerrit-Owner: Kangheui Won <khwon(a)chromium.org> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Jon Murphy <jpmurphy(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Robert Zieba <robertzieba(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[master]: util/amdfwtool: Include the header with __packed definition
by Martin Roth (Code Review) 03 Oct '22

03 Oct '22

Martin Roth has submitted this change. ( https://review.coreboot.org/c/coreboot/+/67384 ) ( 5 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: util/amdfwtool: Include the header with __packed definition ...................................................................... util/amdfwtool: Include the header with __packed definition Checkpatch script recommends to use __packed instead of __attribute__((packed)). Currently the build rule for amdfwtool does not include the required header file with __packed definition. Update the compiler flag to include the required header file. BUG=None TEST=Build amdfwtool. Signed-off-by: Karthikeyan Ramasubramanian <kramasub(a)google.com> Change-Id: I448cbad533608dd5c2bd4f2d827fcc5db5dee5cb Reviewed-on: https://review.coreboot.org/c/coreboot/+/67384 Reviewed-by: Jon Murphy <jpmurphy(a)google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M util/amdfwtool/Makefile M util/amdfwtool/Makefile.inc M util/amdfwtool/amdfwtool.h 3 files changed, 25 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Jon Murphy: Looks good to me, approved diff --git a/util/amdfwtool/Makefile b/util/amdfwtool/Makefile index bdc3dc3..e0ce3b7 100644 --- a/util/amdfwtool/Makefile +++ b/util/amdfwtool/Makefile @@ -1,5 +1,6 @@ # SPDX-License-Identifier: BSD-3-Clause +top ?= $(abspath ../..) HOSTCC ?= cc READ_SRC = amdfwread.c @@ -10,6 +11,7 @@ TARGETS = amdfwread amdfwtool WERROR=-Werror CFLAGS=-O2 -Wall -Wextra -Wshadow ${WERROR} +CFLAGS += -I $(top)/src/commonlib/bsd/include all: $(TARGETS) diff --git a/util/amdfwtool/Makefile.inc b/util/amdfwtool/Makefile.inc index 4b7db76..d45d273 100644 --- a/util/amdfwtool/Makefile.inc +++ b/util/amdfwtool/Makefile.inc @@ -3,6 +3,7 @@ amdfwtoolobj = amdfwtool.o data_parse.o AMDFWTOOLCFLAGS=-O2 -Wall -Wextra -Wshadow -Werror +AMDFWTOOLCFLAGS += -I $(top)/src/commonlib/bsd/include $(objutil)/amdfwtool/%.o: $(top)/util/amdfwtool/%.c # $(HEADER) $(HOSTCC) $(AMDFWTOOLCFLAGS) $(HOSTCFLAGS) -c -o $@ $< diff --git a/util/amdfwtool/amdfwtool.h b/util/amdfwtool/amdfwtool.h index 027a8f3..6b13edc 100644 --- a/util/amdfwtool/amdfwtool.h +++ b/util/amdfwtool/amdfwtool.h @@ -3,6 +3,7 @@ #ifndef _AMD_FW_TOOL_H_ #define _AMD_FW_TOOL_H_ +#include <commonlib/bsd/compiler.h> #include <stdint.h> #include <stdbool.h> -- To view, visit https://review.coreboot.org/c/coreboot/+/67384 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I448cbad533608dd5c2bd4f2d827fcc5db5dee5cb Gerrit-Change-Number: 67384 Gerrit-PatchSet: 7 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jon Murphy <jpmurphy(a)google.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Robert Zieba <robertzieba(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[master]: util/docker/coreboot-sdk: add graphicsmagick-imagemagick-compat
by Martin Roth (Code Review) 03 Oct '22

03 Oct '22

Martin Roth has submitted this change. ( https://review.coreboot.org/c/coreboot/+/67159 ) Change subject: util/docker/coreboot-sdk: add graphicsmagick-imagemagick-compat ...................................................................... util/docker/coreboot-sdk: add graphicsmagick-imagemagick-compat edkII requires ImageMagick's `convert` to compile. The `graphicsmagick-imagemagick-compat` package provides `convert` without the full ImageMagick library. Change-Id: I8fc01526842eb408b0015c0652043c20f826a015 Signed-off-by: Tom Hiller <thrilleratplay(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/67159 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Michał Żygowski <michal.zygowski(a)3mdeb.com> --- M util/docker/coreboot-sdk/Dockerfile 1 file changed, 18 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Michał Żygowski: Looks good to me, approved diff --git a/util/docker/coreboot-sdk/Dockerfile b/util/docker/coreboot-sdk/Dockerfile index 1b4a271..d135fd7 100644 --- a/util/docker/coreboot-sdk/Dockerfile +++ b/util/docker/coreboot-sdk/Dockerfile @@ -37,6 +37,7 @@ git \ gnat-12 \ golang \ + graphicsmagick-imagemagick-compat \ graphviz \ lcov \ libcrypto++-dev \ -- To view, visit https://review.coreboot.org/c/coreboot/+/67159 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8fc01526842eb408b0015c0652043c20f826a015 Gerrit-Change-Number: 67159 Gerrit-PatchSet: 2 Gerrit-Owner: Tom Hiller <thrilleratplay(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[master]: mb/google/skyrim: Update Kconfig to point to SPL
by Martin Roth (Code Review) 03 Oct '22

03 Oct '22

Attention is currently required from: Jason Nien, Jon Murphy, Karthik Ramasubramanian. Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/67928 ) Change subject: mb/google/skyrim: Update Kconfig to point to SPL ...................................................................... Patch Set 10: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/67928/comment/12c43f87_5a5844d0 PS10, Line 8: > Possible unwrapped commit description (prefer a maximum 72 chars per line) Please fix. -- To view, visit https://review.coreboot.org/c/coreboot/+/67928 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I70dcb19983c970283ee887b78a18c0668e83d4b0 Gerrit-Change-Number: 67928 Gerrit-PatchSet: 10 Gerrit-Owner: Jon Murphy <jpmurphy(a)google.com> Gerrit-Reviewer: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Martin Roth <martin.roth(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 Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Attention: Jon Murphy <jpmurphy(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Sun, 02 Oct 2022 22:07:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: Update blobs submodule to upstream master
by Martin Roth (Code Review) 03 Oct '22

03 Oct '22

Martin Roth has submitted this change. ( https://review.coreboot.org/c/coreboot/+/67986 ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: Update blobs submodule to upstream master ...................................................................... Update blobs submodule to upstream master Updating from commit id d55c315: 2022-07-05 14:51:39 +0000 - (mb/starlabs: Remove padding from logo) to commit id 5a19332: 2022-09-28 20:00:40 +0000 - (mb/google/skyrim: Add SPL Table for ChromeOS) This brings in 10 new commits: 5a19332 mb/google/skyrim: Add SPL Table for ChromeOS a543a27 soc/mediatek/mt8188: Update MCUPM firmware from v1.01.01 to v1.01.02 9a76f55 soc/mediatek/mt8188: Update MCUPM firmware to v1.01.01 835f951 mb/google/skyrim: Add initial APCB release for skyrim board 4635ce0 soc/mediatek/mt8188: Add dram.elf version 0.1.0 for DRAM calibration 05afca2 soc/mediatek/mt8188: Add SPM firmware 3324df4 soc/mediatek/mt8188: Add dpm.pm and dpm.dm version 0.1 10a740e soc/mediatek/mt8188: Add SSPM firmware v1.88.00 db990c6 soc/mediatek/mt8188: Add MCUPM firmware v1.01.00 c5a4fda soc/mediatek/mt8188: Add MT8188 basic files Signed-off-by: Jon Murphy <jpmurphy(a)google.com> Change-Id: Idac3c5eb7ad1eb586ca5a33c7f46e16c762948d3 Reviewed-on: https://review.coreboot.org/c/coreboot/+/67986 Reviewed-by: Raul Rangel <rrangel(a)chromium.org> Reviewed-by: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M 3rdparty/blobs 1 file changed, 36 insertions(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved Martin Roth: Looks good to me, approved diff --git a/3rdparty/blobs b/3rdparty/blobs index d55c315..5a19332 160000 --- a/3rdparty/blobs +++ b/3rdparty/blobs @@ -1 +1 @@ -Subproject commit d55c315b6126fbcca6a3881b5be8dff1509fe7ef +Subproject commit 5a19332deb332479c98e13923b1023f1a5cb271f -- To view, visit https://review.coreboot.org/c/coreboot/+/67986 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idac3c5eb7ad1eb586ca5a33c7f46e16c762948d3 Gerrit-Change-Number: 67986 Gerrit-PatchSet: 4 Gerrit-Owner: Jon Murphy <jpmurphy(a)google.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)amd.corp-partner.google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Martin Roth <martin.roth(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-MessageType: merged

1 0

[S] Change in coreboot[master]: mb/google/skyrim: Adjust Makefile to look for SPD
by Martin Roth (Code Review) 03 Oct '22

03 Oct '22

Attention is currently required from: Raul Rangel, Jason Nien, Paul Menzel, Jon Murphy, Karthik Ramasubramanian. Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68018 ) Change subject: mb/google/skyrim: Adjust Makefile to look for SPD ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/68018 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9cf13acb1188309ea6a1e6bdacc37d80b01f70a8 Gerrit-Change-Number: 68018 Gerrit-PatchSet: 4 Gerrit-Owner: Jon Murphy <jpmurphy(a)google.com> Gerrit-Reviewer: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Mark Hasemeyer <markhas(a)google.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Robert Zieba <robertzieba(a)google.com> Gerrit-Reviewer: Tim Van Patten <timvp(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Jon Murphy <jpmurphy(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Sun, 02 Oct 2022 22:06:09 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: mb/google/skyrim: Adjust Makefile to look for SPD
by Martin Roth (Code Review) 03 Oct '22

03 Oct '22

Attention is currently required from: Raul Rangel, Jason Nien, Paul Menzel, Jon Murphy, Karthik Ramasubramanian. Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68018 ) Change subject: mb/google/skyrim: Adjust Makefile to look for SPD ...................................................................... Patch Set 4: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/68018/comment/ce16573d_e99c7039 PS3, Line 7: src/ > Please remove. Done -- To view, visit https://review.coreboot.org/c/coreboot/+/68018 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9cf13acb1188309ea6a1e6bdacc37d80b01f70a8 Gerrit-Change-Number: 68018 Gerrit-PatchSet: 4 Gerrit-Owner: Jon Murphy <jpmurphy(a)google.com> Gerrit-Reviewer: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Mark Hasemeyer <markhas(a)google.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Robert Zieba <robertzieba(a)google.com> Gerrit-Reviewer: Tim Van Patten <timvp(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Jon Murphy <jpmurphy(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Sun, 02 Oct 2022 22:05:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: mb/google/skyrim: Adjust Makefile to look for SPD
by Martin Roth (Code Review) 03 Oct '22

03 Oct '22

Attention is currently required from: Raul Rangel, Jason Nien, Jon Murphy, Martin Roth, Karthik Ramasubramanian. Martin Roth has uploaded a new patch set (#4) to the change originally created by Jon Murphy. ( https://review.coreboot.org/c/coreboot/+/68018 ) Change subject: mb/google/skyrim: Adjust Makefile to look for SPD ...................................................................... mb/google/skyrim: Adjust Makefile to look for SPD Adjust the Makefile to look for SPD source Makefile. The current SPD guard isn't set up correctly and is attempting to build the APCB with SPD when SPD isn't present. BUG=b:249988439 TEST=util/abuild/abuild -x -t GOOGLE_MORTHAL --verbose util/abuild/abuild -x -t GOOGLE_SKYRIM --verbose util/abuild/abuild -x -t GOOGLE_WINTERHOLD --verbose Change-Id: I9cf13acb1188309ea6a1e6bdacc37d80b01f70a8 Signed-off-by: Jon Murphy <jpmurphy(a)google.com> --- M src/mainboard/google/skyrim/Makefile.inc 1 file changed, 20 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/68018/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/68018 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9cf13acb1188309ea6a1e6bdacc37d80b01f70a8 Gerrit-Change-Number: 68018 Gerrit-PatchSet: 4 Gerrit-Owner: Jon Murphy <jpmurphy(a)google.com> Gerrit-Reviewer: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Mark Hasemeyer <markhas(a)google.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Robert Zieba <robertzieba(a)google.com> Gerrit-Reviewer: Tim Van Patten <timvp(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Attention: Jon Murphy <jpmurphy(a)google.com> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in coreboot[master]: mb/google/brya/var/agah: Update NVVDD VR PGOOD GPP_E3
by Martin Roth (Code Review) 03 Oct '22

03 Oct '22

Martin Roth has submitted this change. ( https://review.coreboot.org/c/coreboot/+/67288 ) Change subject: mb/google/brya/var/agah: Update NVVDD VR PGOOD GPP_E3 ...................................................................... mb/google/brya/var/agah: Update NVVDD VR PGOOD GPP_E3 This pin was originally set as output in error. This should be a input to behave like GPP_E16 on the older variants. BUG=b:239721380 TEST=build Signed-off-by: Tarun Tuli <taruntuli(a)google.com> Change-Id: Ic0f793ff52adb425ae5378b88d2837bb9e58edd2 Reviewed-on: https://review.coreboot.org/c/coreboot/+/67288 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Tim Wawrzynczak <inforichland(a)gmail.com> Reviewed-by: Paul Menzel <paulepanter(a)mailbox.org> --- M src/mainboard/google/brya/variants/agah/variant.c 1 file changed, 21 insertions(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Tim Wawrzynczak: Looks good to me, approved diff --git a/src/mainboard/google/brya/variants/agah/variant.c b/src/mainboard/google/brya/variants/agah/variant.c index 9512522..247913e 100644 --- a/src/mainboard/google/brya/variants/agah/variant.c +++ b/src/mainboard/google/brya/variants/agah/variant.c @@ -155,7 +155,7 @@ gpu_off_seq[3].pwr_en_gpio = GPP_E10; } else { const struct pad_config board_rev_3_gpios[] = { - PAD_CFG_GPO(GPP_E3, 0, PLTRST), + PAD_CFG_GPI(GPP_E3, NONE, PLTRST), PAD_NC(GPP_E10, NONE), PAD_NC(GPP_E16, NONE), PAD_CFG_GPO(GPP_F12, 0, PLTRST), -- To view, visit https://review.coreboot.org/c/coreboot/+/67288 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic0f793ff52adb425ae5378b88d2837bb9e58edd2 Gerrit-Change-Number: 67288 Gerrit-PatchSet: 4 Gerrit-Owner: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit October 2022