coreboot-gerrit August 2024

coreboot-gerrit@coreboot.org

1 participants
3912 discussions

[L] Change in coreboot[main]: drivers/efi/uefi_capsules.c: coalesce and store UEFI capsules
by Martin L Roth (Code Review) 30 Aug '24

30 Aug '24

Martin L Roth has submitted this change. ( https://review.coreboot.org/c/coreboot/+/83422?usp=email ) Change subject: drivers/efi/uefi_capsules.c: coalesce and store UEFI capsules ...................................................................... drivers/efi/uefi_capsules.c: coalesce and store UEFI capsules How it approximately works: (During a normal system run): 1. OS puts a capsule into RAM and calls UpdateCapsule() function of EFI runtime 2. If applying the update requires a reboot, EFI implementation creates a new CapsuleUpdateData* EFI variable pointing at the beginning of capsules description (not data, but description of the data) and does a warm reboot leaving capsule data and its description in RAM to be picked by firmware on the next boot process (After DEV_INIT:) 3. Capsules are discovered by checking for CapsuleUpdateData* variables 4. Capsule description in memory and capsule data is validated for sanity 5. Capsule data is coalesced into a continuous piece of memory (On BS_WRITE_TABLES via dasharo_add_capsules_to_bootmem() hook:) 6. Buffer with coalesced capsules is marked as reserved (On BS_WRITE_TABLES via lb_uefi_capsules() hook:) 7. coreboot table entry is added for each of the discovered capsules (In UEFI payload:) 8. CapsuleUpdateData* get removed 9. coreboot table is checked for any update capsules which are then applied Change-Id: I162d678ae5c504906084b59c1a8d8c26dadb9433 Signed-off-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/83422 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Krystian Hebel <krystian.hebel(a)3mdeb.com> --- M src/commonlib/include/commonlib/coreboot_tables.h M src/drivers/efi/Kconfig M src/drivers/efi/Makefile.mk A src/drivers/efi/capsules.c A src/drivers/efi/capsules.h M src/include/boot/coreboot_tables.h M src/lib/bootmem.c M src/lib/coreboot_table.c M src/security/memory/memory_clear.c 9 files changed, 815 insertions(+), 0 deletions(-) Approvals: Krystian Hebel: Looks good to me, approved build bot (Jenkins): Verified diff --git a/src/commonlib/include/commonlib/coreboot_tables.h b/src/commonlib/include/commonlib/coreboot_tables.h index a9e14c7..7cbc7da 100644 --- a/src/commonlib/include/commonlib/coreboot_tables.h +++ b/src/commonlib/include/commonlib/coreboot_tables.h @@ -88,6 +88,7 @@ LB_TAG_ACPI_RSDP = 0x0043, LB_TAG_PCIE = 0x0044, LB_TAG_EFI_FW_INFO = 0x0045, + LB_TAG_CAPSULE = 0x0046, /* The following options are CMOS-related */ LB_TAG_CMOS_OPTION_TABLE = 0x00c8, LB_TAG_OPTION = 0x00c9, diff --git a/src/drivers/efi/Kconfig b/src/drivers/efi/Kconfig index 399443b..07e5f14 100644 --- a/src/drivers/efi/Kconfig +++ b/src/drivers/efi/Kconfig @@ -38,3 +38,11 @@ 32-bit unsigned integer representing lowest firmware version number that is allowed to replace the current one. Can be used to forbid bugged versions. + +config DRIVERS_EFI_UPDATE_CAPSULES + bool "Include EFI update capsules driver" + depends on DRIVERS_EFI_VARIABLE_STORE && SMMSTORE_V2 && DRIVERS_EFI_FW_INFO + help + Adds a driver that is able to parse CapsuleUpdateData* EFI variables + to discover firmware updates and expose them for tianocore thorough + CBMEM for execution. tianocore is responsible for erasing the variables. diff --git a/src/drivers/efi/Makefile.mk b/src/drivers/efi/Makefile.mk index 47f0e82..2c3cf9c 100644 --- a/src/drivers/efi/Makefile.mk +++ b/src/drivers/efi/Makefile.mk @@ -3,6 +3,8 @@ all-$(CONFIG_DRIVERS_EFI_VARIABLE_STORE) += efivars.c smm-$(CONFIG_DRIVERS_EFI_VARIABLE_STORE) += efivars.c +ramstage-$(CONFIG_DRIVERS_EFI_UPDATE_CAPSULES) += capsules.c + all-$(CONFIG_USE_UEFI_VARIABLE_STORE) += option.c smm-$(CONFIG_USE_UEFI_VARIABLE_STORE) += option.c diff --git a/src/drivers/efi/capsules.c b/src/drivers/efi/capsules.c new file mode 100644 index 0000000..e674e33 --- /dev/null +++ b/src/drivers/efi/capsules.c @@ -0,0 +1,770 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <acpi/acpi.h> +#include <boot/coreboot_tables.h> +#include <bootmem.h> +#include <bootstate.h> +#include <cbmem.h> +#include <console/console.h> +#include <cpu/x86/pae.h> +#include <drivers/efi/efivars.h> +#include <drivers/efi/capsules.h> +#include <memrange.h> +#include <string.h> +#include <stdio.h> +#include <smmstore.h> +#include <types.h> + +#include <Uefi/UefiSpec.h> +#include <Guid/GlobalVariable.h> +#include <Guid/FmpCapsule.h> +#include <IndustryStandard/WindowsUxCapsule.h> + +/* + * Overview + * + * SG stands for scatter-gather. SG list consists of SG blocks that describe a + * potentially discontinuous sequence of memory blocks while not necessarily + * lying in continuous memory themselves. + * + * SG list is basically a linked list of arrays of block descriptors (SG + * blocks). Each of SG blocks can be: + * - a data block, which points to capsule's data + * - a continuation block, which says where other SG blocks are to be found + * - end-of-list block, which indicates there are no more blocks + * + * Each of the CapsuleUpdateData* EFI variables point to some SG list which + * might contain one or more update capsules. SG blocks never contain data of + * more than one of the capsules. Boundary between capsules in an SG list is + * determined by parsing capsule headers and counting amount of data seen so + * far. + * + * There can be multiple CapsuleUpdateData* variables (CapsuleUpdateData, + * CapsuleUpdateData1, etc.) in which case their SG lists are chained together + * after sanity checks. + */ + +/* This should be more than enough. */ +#define MAX_CAPSULES 32 + +/* 4 should be enough, but 8 won't hurt. */ +#define CAPSULE_ALIGNMENT 8 + +/* + * A helper structure which bundles physical block address with its data. It's + * necessary because 32-bit code can't easily access anything beyond 4 GiB + * boundary and this structure allows reading the data, passing it around and, + * if necessary, updating it. + * + * Usage: + * 1) Set .self to physical address + * 2) Check block's address with is_good_block() + * 3) Use load_block() to fetch or store_block() to update data + */ +struct block_descr { + /* Where the data comes from. */ + uint64_t self; + + /* + * Data read from the self address above. Three cases: + * - len != 0 && addr != 0 => len bytes of capsule data at addr + * next block_descr follows this one (self + 16) + * - len == 0 && addr != 0 => no data + * next block_descr is at addr + * - len == 0 && addr == 0 => no data + * no next block_descr + */ + uint64_t len; + uint64_t addr; +}; + +/* For passing data from efi_parse_capsules() to bootmem and CBMEM callbacks. */ +struct memory_range { + uint32_t base; + uint32_t len; +}; + +static const EFI_GUID capsule_vendor_guid = { + 0x711C703F, 0xC285, 0x4B10, { 0xA3, 0xB0, 0x36, 0xEC, 0xBD, 0x3C, 0x8B, 0xE2 } +}; +static const EFI_GUID windows_ux_capsule_guid = WINDOWS_UX_CAPSULE_GUID; +static const EFI_GUID edk2_capsule_on_disk_name_guid = { + 0x98C80A4F, 0xE16B, 0x4D11, { 0x93, 0x9A, 0xAB, 0xE5, 0x61, 0x26, 0x3, 0x30 } +}; +static const EFI_GUID efi_fmp_capsule_guid = EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID; + +/* Memory map to keep track of unused or reserved ranges. */ +struct memranges memory_map; + +/* Page tables required for pae_map_2M_page(). */ +static char pae_page_tables[20 * KiB] __aligned(4 * KiB); + +/* Where all coalesced capsules are located. */ +struct memory_range coalesce_buffer; + +/* Where individual coalesced capsules are located and their count. */ +static struct memory_range uefi_capsules[MAX_CAPSULES]; +static int uefi_capsule_count; + +static bool is_data_block(const struct block_descr *block) +{ + return (block->len != 0); +} + +static bool is_final_block(const struct block_descr *block) +{ + return (block->len == 0 && block->addr == 0); +} + +static void *map_range(uint64_t base, uint32_t len) +{ + static uint64_t last_mapping_base = UINT64_MAX; + + /* Using MMCONF should be safe as long as we don't do any device + initialization during parsing of capsules and don't forget to call + paging_disable_pae() at the end. */ + _Static_assert(IS_ALIGNED(CONFIG_ECAM_MMCONF_BASE_ADDRESS, 2 * MiB)); + uintptr_t window_base = CONFIG_ECAM_MMCONF_BASE_ADDRESS; + size_t window_size = 2 * MiB; + + printk(BIOS_SPEW, "capsules: mapping %#010x bytes at %#010llx.\n", len, base); + + if (base + len <= 4ULL * GiB && + (base + len <= window_base || base >= window_base + window_size)) { + /* Don't bother with the mapping, the whole range must be + already accessible without it. */ + printk(BIOS_SPEW, "capsules: no need to map anything.\n"); + return (void *)(uintptr_t)base; + } + + uint64_t aligned_base = ALIGN_DOWN(base, 2 * MiB); + if (base - aligned_base + len > 2 * MiB) + die("capsules: memory range map request can't be satisfied.\n"); + + /* No need to map the same data. */ + if (aligned_base != last_mapping_base) { + printk(BIOS_SPEW, "capsules: mapping from %#010llx.\n", aligned_base); + pae_map_2M_page(&pae_page_tables, aligned_base, (void *)window_base); + last_mapping_base = aligned_base; + } + + return (uint8_t *)window_base + (base - aligned_base); +} + +/* + * Alignment requirement on EFI_CAPSULE_BLOCK_DESCRIPTOR seems to be 8 bytes, + * which means that it can be cut in half by a mapping. Could map two 2 MiB + * pages instead, but should be easier to simply read those 16 bytes and pass + * them around. + * + * `volatile` is to guard against a hypothetical statement reordering. + */ + +static void load_block(struct block_descr *block) +{ + volatile uint64_t *len = map_range(block->self, sizeof(uint64_t)); + block->len = *len; + + volatile uint64_t *addr = map_range(block->self + sizeof(uint64_t), sizeof(uint64_t)); + block->addr = *addr; +} + +static void store_block(const struct block_descr *block) +{ + volatile uint64_t *len = map_range(block->self, sizeof(uint64_t)); + *len = block->len; + + volatile uint64_t *addr = map_range(block->self + sizeof(uint64_t), sizeof(uint64_t)); + *addr = block->addr; +} + +static void advance_block(struct block_descr *block) +{ + if (is_final_block(block)) + die("capsules: attempt to advance beyond final SG block of UEFI capsules.\n"); + + if (is_data_block(block)) { + /* That was at least part of a capsule. */ + block->self = block->self + sizeof(EFI_CAPSULE_BLOCK_DESCRIPTOR); + } else { + /* End of continuous sequence of descriptors, but there are more. */ + block->self = block->addr; + } +} + +static bool is_good_capsule(const EFI_CAPSULE_HEADER *capsule) +{ + if (capsule->HeaderSize < sizeof(*capsule)) { + printk(BIOS_ERR, "capsules: capsule header size is too small: %#010x.\n", + capsule->HeaderSize); + return false; + } + if (capsule->CapsuleImageSize <= capsule->HeaderSize) { + printk(BIOS_ERR, "capsules: capsule image size is too small: %#010x.\n", + capsule->CapsuleImageSize); + return false; + } + if (!(capsule->Flags & CAPSULE_FLAGS_PERSIST_ACROSS_RESET)) { + printk(BIOS_ERR, + "capsules: this capsule should not have persisted, flags: %#010x.\n", + capsule->Flags); + return false; + } + + const EFI_GUID *guid = &capsule->CapsuleGuid; + if (memcmp(guid, &windows_ux_capsule_guid, sizeof(*guid)) == 0) + return true; + if (memcmp(guid, &edk2_capsule_on_disk_name_guid, sizeof(*guid)) == 0) + return true; + if (memcmp(guid, &efi_fmp_capsule_guid, sizeof(*guid)) == 0) + return true; + + printk(BIOS_ERR, "capsules: unrecognized capsule GUID.\n"); + return false; +} + +static bool is_in_unused_ram(uint64_t base, uint64_t len) +{ + if (len == 0) { + die("capsules: %s() was passed an empty range: %#010llx:%#010llx.\n", + __func__, base, len); + } + if (base + len < base) { + die("capsules: %s() was passed an invalid range: %#010llx:%#010llx.\n", + __func__, base, len); + } + + const struct range_entry *r; + memranges_each_entry(r, &memory_map) { + if (range_entry_tag(r) != BM_MEM_RAM) + continue; + + if (base >= range_entry_base(r) && base + len <= range_entry_end(r)) + return true; + } + + return false; +} + +static bool is_good_block(struct block_descr *block) +{ + if (!IS_ALIGNED(block->self, sizeof(uint64_t))) { + printk(BIOS_ERR, "capsules: misaligned SG block at %#010llx.\n", block->self); + return false; + } + + if (!is_in_unused_ram(block->self, sizeof(*block))) { + printk(BIOS_ERR, "capsules: SG block is not in unused memory.\n"); + return false; + } + + return true; +} + +static bool is_good_capsule_head(struct block_descr *block) +{ + if (!is_data_block(block)) { + printk(BIOS_ERR, "capsules: first capsule SG block is not a data block.\n"); + return false; + } + + if (block->len < sizeof(EFI_CAPSULE_HEADER)) { + printk(BIOS_ERR, "capsules: first SG block of a capsule is too small.\n"); + return false; + } + + if (!is_in_unused_ram(block->addr, block->len)) { + printk(BIOS_ERR, "capsules: capsule header is not in unused memory.\n"); + return false; + } + + return true; +} + +static bool is_good_capsule_block(struct block_descr *block, uint32_t size_left) +{ + if (is_final_block(block)) { + printk(BIOS_ERR, "capsules: not enough SG blocks to cover a capsule.\n"); + return false; + } + + if (!is_data_block(block)) { + printk(BIOS_ERR, "capsules: capsule SG block is not a data block.\n"); + return false; + } + + if (block->len > size_left) { + printk(BIOS_ERR, "capsules: SG blocks reach beyond a capsule.\n"); + return false; + } + + if (!is_in_unused_ram(block->addr, block->len)) { + printk(BIOS_ERR, "capsules: capsule data is not in unused memory.\n"); + return false; + } + + return true; +} + +/* Checks a single SG list for sanity. Returns its end-of-list descriptor or + an empty one on error. */ +static struct block_descr check_capsule_block(struct block_descr first_block, + uint64_t *total_data_size) +{ + struct block_descr block = first_block; + if (!is_good_block(&block)) { + printk(BIOS_ERR, "capsules: bad capsule block start.\n"); + goto error; + } + + load_block(&block); + + uint64_t data_size = 0; + while (!is_final_block(&block)) { + /* + * This results in dropping of this capsule block if any of + * contained capsule headers looks weird. An alternative is to + * cut the capsule block upon finding a bad header. Maybe + * could even jump over a broken capsule, temporarily trusting + * size field in its header because invalid value should not + * break parsing anyway, and then cut it out of the sequence of + * blocks. EDK doesn't bother, so only noting the possibility. + */ + if (!is_good_capsule_head(&block)) { + printk(BIOS_ERR, "capsules: bad capsule header @ %#010llx.\n", + block.addr); + goto error; + } + + const EFI_CAPSULE_HEADER *capsule_hdr = + map_range(block.addr, sizeof(*capsule_hdr)); + if (!is_good_capsule(capsule_hdr)) { + printk(BIOS_ERR, "capsules: bad capsule header @ %#010llx.\n", + block.addr); + goto error; + } + + data_size += ALIGN_UP(capsule_hdr->CapsuleImageSize, CAPSULE_ALIGNMENT); + + uint32_t size_left = capsule_hdr->CapsuleImageSize; + while (size_left != 0) { + /* is_good_block() holds here whether it's the first iteration or + not. */ + + if (!is_good_capsule_block(&block, size_left)) + goto error; + + size_left -= block.len; + + advance_block(&block); + if (!is_good_block(&block)) { + printk(BIOS_ERR, "capsules: capsule body has a bad block.\n"); + goto error; + } + + load_block(&block); + if (!is_final_block(&block) && !is_data_block(&block)) { + /* Advance to the next page of block descriptors. */ + advance_block(&block); + if (!is_good_block(&block)) { + printk(BIOS_ERR, "capsules: bad SG continuation.\n"); + goto error; + } + + load_block(&block); + /* Not expecting a continuation to be followed by another + continuation or an end-of-list. */ + if (!is_data_block(&block)) { + printk(BIOS_ERR, + "capsules: chained SG continuations.\n"); + goto error; + } + } + } + } + + /* Increase the size only on successful parsing of the capsule block. */ + *total_data_size += data_size; + + return block; + +error: + return (struct block_descr){ .self = 0 }; +} + +/* Fills an array with pointers to capsule blocks. Returns number of + discovered capsule blocks or -1 on error. */ +static int discover_capsule_blocks(struct region_device *rdev, + struct block_descr *blocks, + int max_blocks) +{ + int block_count = 0; + for (int i = 0; block_count < max_blocks; ++i) { + char var_name[32]; + if (i == 0) + strcpy(var_name, "CapsuleUpdateData"); + else + snprintf(var_name, sizeof(var_name), "CapsuleUpdateData%d", i); + + struct block_descr block; + uint32_t size = sizeof(block.self); + enum cb_err ret = efi_fv_get_option(rdev, &capsule_vendor_guid, var_name, + &block.self, &size); + if (ret != CB_SUCCESS) { + /* No more variables. */ + break; + } + if (size != sizeof(block.self)) { + printk(BIOS_ERR, "capsules: unexpected capsule data size (%d).\n", + size); + return -1; + } + + /* + * EDK2 checks for duplicates probably because we'll get into + * trouble with chaining if there are any, so do the check. + * + * This, however, won't handle all possible situations which + * lead to loops or processing the same capsule more than once. + */ + int j; + for (j = 0; j < block_count; ++j) { + if (blocks[j].self == block.self) + break; + } + if (j < block_count) { + printk(BIOS_INFO, "capsules: skipping duplicated %s.\n", var_name); + continue; + } + + printk(BIOS_INFO, "capsules: capsule block #%d at %#010llx.\n", + block_count, block.self); + blocks[block_count++] = block; + } + + return block_count; +} + +/* + * This function connects tail of one block of descriptors with the head of the + * next one and returns pointer to the head of the whole chain. While at it: + * - validate structures and pointers for sanity + * - compute total amount of memory needed for coalesced capsules + * + * Returns block that starts at 0 on error. + */ +static struct block_descr verify_and_chain_blocks(struct block_descr *blocks, + int block_count, + uint64_t *total_data_size) +{ + /* This won't be blocks[0] if there is something wrong with the first capsule block. */ + struct block_descr head = {0}; + + /* End-of-list descriptor of the last chained block. */ + struct block_descr tail = {0}; + + *total_data_size = 0; + + for (int i = 0; i < block_count; ++i) { + struct block_descr last_block = check_capsule_block(blocks[i], total_data_size); + if (last_block.self == 0) { + /* Fail hard instead? EDK just keeps going, as if capsule + blocks are always independent. */ + printk(BIOS_WARNING, + "capsules: skipping damaged capsule block #%d @ %#010llx.\n", + i, blocks[i].self); + continue; + } + + if (head.self == 0) { + head = blocks[i]; + } else { + tail.addr = blocks[i].self; + store_block(&tail); + } + + tail = last_block; + } + + return head; +} + +/* Marks structures and data of SG lists as BM_MEM_RESERVED so we don't step on + them when looking for usable memory. */ +static void reserve_capsules(struct block_descr block_chain) +{ + struct block_descr block = block_chain; + + /* This is the first block of a continuous sequence of blocks. */ + struct block_descr seq_start = {0}; + + /* The code reserves sequences of blocks to avoid invoking + memranges_insert() on each of a bunch of adjacent 16-byte blocks. */ + + load_block(&block); + for (; !is_final_block(&block); advance_block(&block), load_block(&block)) { + if (seq_start.self == 0) + seq_start = block; + + if (is_data_block(&block)) { + /* Reserve capsule data. */ + memranges_insert(&memory_map, block.addr, block.len, BM_MEM_RESERVED); + } else { + /* This isn't the final or a data block, so it must be the + last block of a continuous sequence. Reserve the whole + sequence. */ + memranges_insert(&memory_map, + seq_start.self, + block.self - seq_start.self + + sizeof(EFI_CAPSULE_BLOCK_DESCRIPTOR), + BM_MEM_RESERVED); + + /* Will be set on the next iteration if there is one. */ + seq_start.self = 0; + } + } + + /* If continuations never show up in a row as checked by + check_capsule_block(), seq_start must be non-NULL here. */ + memranges_insert(&memory_map, + seq_start.self, + block.self - seq_start.self + sizeof(EFI_CAPSULE_BLOCK_DESCRIPTOR), + BM_MEM_RESERVED); +} + +/* + * Find a buffer below 4 GiB for coalesced capsules. + * + * Keeping it simple and allocating a single buffer. However, there is + * no requirement to put all the capsules together, only that each of + * them is continuous in memory. So if this is bad for some reason, + * can allocate a separate block for each. + * + * Returns buffer that starts at 0 on error. + */ +static struct memory_range pick_buffer(uint64_t total_data_size) +{ + struct memory_range buffer = {0}; + + /* 4 * KiB is the alignment set by memranges_init(). */ + total_data_size = ALIGN_UP(total_data_size, 4 * KiB); + + const struct range_entry *r; + memranges_each_entry(r, &memory_map) { + if (range_entry_tag(r) != BM_MEM_RAM) + continue; + + resource_t base = range_entry_base(r); + if (base >= 4ULL * GiB) + break; + + /* Possibly reduce size to not deal with ranges that cross 4 GiB boundary. */ + resource_t size = range_entry_size(r); + if (base + size > 4ULL * GiB) + size -= base + size - 4ULL * GiB; + + if (size >= total_data_size) { + /* + * To not create troubles for payloads prefer higher addresses: + * - use the top part of a suitable range + * - exit the loop only after hitting 4 GiB boundary or end of the list + */ + buffer.base = base + size - total_data_size; + buffer.len = total_data_size; + } + } + + return buffer; +} + +/* Puts capsules into continuous physical memory. */ +static void coalesce_capsules(struct block_descr block_chain, uint8_t *target) +{ + struct block_descr block = block_chain; + uint8_t *capsule_start = NULL; + uint32_t size_left = 0; + + /* No safety checks in this function, as all of them were done earlier. */ + + load_block(&block); + for (; !is_final_block(&block); advance_block(&block), load_block(&block)) { + /* Advance over a continuation. */ + if (!is_data_block(&block)) + continue; + + /* This must be the first block of a capsule. */ + if (size_left == 0) { + const EFI_CAPSULE_HEADER *capsule_hdr = + map_range(block.addr, sizeof(*capsule_hdr)); + size_left = capsule_hdr->CapsuleImageSize; + capsule_start = target; + } + + uint64_t addr = block.addr; + uint64_t data_left = block.len; + while (data_left != 0) { + uint64_t piece_len = MIN(data_left, 2 * MiB - (addr % 2 * MiB)); + void *data = map_range(addr, piece_len); + + memcpy(target, data, piece_len); + + target += piece_len; + addr += piece_len; + data_left -= piece_len; + } + + size_left -= block.len; + + /* This must be the last block of a capsule, record it. */ + if (size_left == 0) { + /* If we can just ignore corrupted capsules, then we can simply + drop those which don't fit. */ + if (uefi_capsule_count == MAX_CAPSULES) { + printk(BIOS_WARNING, + "capsules: ignoring all capsules after #%d.\n", + MAX_CAPSULES); + break; + } + + uefi_capsules[uefi_capsule_count].base = (uintptr_t)capsule_start; + uefi_capsules[uefi_capsule_count].len = block.len; + uefi_capsule_count++; + + /* This is to align start of the next capsule (assumes that + initial value of target was suitably aligned). */ + if (!IS_ALIGNED(block.len, CAPSULE_ALIGNMENT)) + target += ALIGN_UP(block.len, CAPSULE_ALIGNMENT) - block.len; + } + } + + printk(BIOS_INFO, "capsules: found %d capsule(s).\n", uefi_capsule_count); +} + +void efi_parse_capsules(void) +{ + /* EDK2 starts with 20 items and then grows the list, but it's unlikely + to be necessary in practice. */ + enum { MAX_CAPSULE_BLOCKS = MAX_CAPSULES }; + + struct region_device rdev; + if (smmstore_lookup_region(&rdev)) { + printk(BIOS_INFO, "capsules: no SMMSTORE region, no update capsules.\n"); + return; + } + + memranges_init(&memory_map, IORESOURCE_MEM | IORESOURCE_FIXED | IORESOURCE_STORED | + IORESOURCE_ASSIGNED | IORESOURCE_CACHEABLE, IORESOURCE_MEM | + IORESOURCE_FIXED | IORESOURCE_STORED | IORESOURCE_ASSIGNED | + IORESOURCE_CACHEABLE, BM_MEM_RAM); + + init_pae_pagetables(&pae_page_tables); + + /* Blocks are collected here when traversing CapsuleUpdateData* + variables, duplicates are skipped. */ + struct block_descr blocks[MAX_CAPSULE_BLOCKS]; + int block_count = discover_capsule_blocks(&rdev, blocks, ARRAY_SIZE(blocks)); + if (block_count <= 0) { + if (block_count == 0) + printk(BIOS_INFO, "capsules: no UEFI capsules were discovered.\n"); + goto exit; + } + + printk(BIOS_INFO, "capsules: processing %d capsule block(s).\n", block_count); + + /* Broken capsules are ignored, ignore those which didn't fit as well. */ + if (block_count == ARRAY_SIZE(blocks)) { + printk(BIOS_WARNING, + "capsules: hit limit on capsule blocks, some might be ignored.\n"); + } + + /* Chaining is done to not pass around and update an array of pointers. */ + uint64_t total_data_size; + struct block_descr block_chain = + verify_and_chain_blocks(blocks, block_count, &total_data_size); + if (block_chain.self == 0) { + printk(BIOS_ERR, "capsules: no valid capsules to process.\n"); + goto exit; + } + + printk(BIOS_DEBUG, "capsules: chained capsule blocks.\n"); + + /* Reserve all blocks and the data they point to to avoid checking for + overlaps when looking for a buffer. */ + reserve_capsules(block_chain); + + printk(BIOS_DEBUG, "capsules: reserved capsule blocks.\n"); + + /* Also reserve memory range for cbmem. Since it will still grow in + size by an unknown amount, try to account for that by reserving at + least 4 MiB more. */ + void *cbmem_current; + size_t cbmem_size; + cbmem_get_region(&cbmem_current, &cbmem_size); + uintptr_t cbmem_future_base = ALIGN_DOWN((uintptr_t)cbmem_current - 4 * MiB, MiB); + memranges_insert(&memory_map, + cbmem_future_base, + (uintptr_t)cbmem_current + cbmem_size - cbmem_future_base, + BM_MEM_RESERVED); + + coalesce_buffer = pick_buffer(total_data_size); + if (coalesce_buffer.base == 0) { + printk(BIOS_ERR, + "capsules: failed to find a buffer (%#llx bytes) for coalesced UEFI capsules.\n", + total_data_size); + } else { + printk(BIOS_DEBUG, "capsules: coalescing capsules data @ %#010x.\n", + coalesce_buffer.base); + coalesce_capsules(block_chain, (void *)(uintptr_t)coalesce_buffer.base); + } + +exit: + paging_disable_pae(); + memranges_teardown(&memory_map); +} + +void lb_efi_capsules(struct lb_header *header) +{ + int i; + for (i = 0; i < uefi_capsule_count; ++i) { + struct lb_range *capsule = (void *)lb_new_record(header); + + printk(BIOS_INFO, "capsules: publishing a capsule @ %#010x.\n", + uefi_capsules[i].base); + + capsule->tag = LB_TAG_CAPSULE; + capsule->size = sizeof(*capsule); + capsule->range_start = uefi_capsules[i].base; + capsule->range_size = uefi_capsules[i].len; + } +} + +void efi_add_capsules_to_bootmem(void) +{ + if (coalesce_buffer.len != 0) { + printk(BIOS_INFO, "capsules: reserving capsules data @ %#010x.\n", + coalesce_buffer.base); + bootmem_add_range(coalesce_buffer.base, coalesce_buffer.len, BM_MEM_RESERVED); + } +} + +/* + * The code from this unit is typically executed by clear_memory() which is run + * after DEV_INIT. However, clear_memory() might not be compiled in in which + * case we still want to process capsules. + * + * State machine doesn't enforce any particular ordering for callbacks and + * running before DEV_INIT is too early due to MTTRs not being initialized. + * Hence invoking code is in two different places that should be mutually + * exclusive (can't set a "done" flag due to unknown ordering). + */ +#if !CONFIG(PLATFORM_HAS_DRAM_CLEAR) + +static void parse_capsules(void *unused) +{ + if (!acpi_is_wakeup_s3()) + efi_parse_capsules(); +} + +BOOT_STATE_INIT_ENTRY(BS_DEV_INIT, BS_ON_EXIT, parse_capsules, NULL); + +#endif diff --git a/src/drivers/efi/capsules.h b/src/drivers/efi/capsules.h new file mode 100644 index 0000000..608ce34 --- /dev/null +++ b/src/drivers/efi/capsules.h @@ -0,0 +1,20 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef _EDK2_CAPSULES_H_ +#define _EDK2_CAPSULES_H_ + +#if CONFIG(DRIVERS_EFI_UPDATE_CAPSULES) + +void efi_parse_capsules(void); + +void efi_add_capsules_to_bootmem(void); + +#else + +static inline void efi_parse_capsules(void) { } + +static inline void efi_add_capsules_to_bootmem(void) { } + +#endif + +#endif /* _EDK2_CAPSULES_H_ */ diff --git a/src/include/boot/coreboot_tables.h b/src/include/boot/coreboot_tables.h index a87933d..090889c 100644 --- a/src/include/boot/coreboot_tables.h +++ b/src/include/boot/coreboot_tables.h @@ -30,6 +30,9 @@ /* Adds LB_TAG_EFI_FW_INFO table entry. */ void lb_efi_fw_info(struct lb_header *header); +/* Adds LB_TAG_CAPSULE table entries. */ +void lb_efi_capsules(struct lb_header *header); + /* Define this function to fill in the frame buffer returning 0 on success and < 0 on error. */ int fill_lb_framebuffer(struct lb_framebuffer *framebuffer); diff --git a/src/lib/bootmem.c b/src/lib/bootmem.c index 578ddac..ab15e85 100644 --- a/src/lib/bootmem.c +++ b/src/lib/bootmem.c @@ -4,6 +4,7 @@ #include <bootmem.h> #include <cbmem.h> #include <device/resource.h> +#include <drivers/efi/capsules.h> #include <symbols.h> #include <assert.h> #include <types.h> @@ -80,6 +81,8 @@ /* Add memory used by CBMEM. */ cbmem_add_bootmem(); + efi_add_capsules_to_bootmem(); + bootmem_add_range((uintptr_t)_stack, REGION_SIZE(stack), BM_MEM_RAMSTAGE); bootmem_add_range((uintptr_t)_program, REGION_SIZE(program), diff --git a/src/lib/coreboot_table.c b/src/lib/coreboot_table.c index 913588f..e954edb 100644 --- a/src/lib/coreboot_table.c +++ b/src/lib/coreboot_table.c @@ -543,6 +543,10 @@ /* Add board-specific table entries, if any. */ lb_board(head); + /* Possibly add UEFI capsules. */ + if (CONFIG(DRIVERS_EFI_UPDATE_CAPSULES)) + lb_efi_capsules(head); + if (CONFIG(CHROMEOS_RAMOOPS)) lb_ramoops(head); diff --git a/src/security/memory/memory_clear.c b/src/security/memory/memory_clear.c index 9a8a23a..e747f4f 100644 --- a/src/security/memory/memory_clear.c +++ b/src/security/memory/memory_clear.c @@ -20,6 +20,7 @@ #include <security/memory/memory.h> #include <cbmem.h> #include <acpi/acpi.h> +#include <drivers/efi/capsules.h> /* Helper to find free space for memset_pae. */ static uintptr_t get_free_memory_range(struct memranges *mem, @@ -60,6 +61,9 @@ if (acpi_is_wakeup_s3()) return; + /* Process capsules before clearing memory and only if not waking up from S3. */ + efi_parse_capsules(); + if (!security_clear_dram_request()) return; -- To view, visit https://review.coreboot.org/c/coreboot/+/83422?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I162d678ae5c504906084b59c1a8d8c26dadb9433 Gerrit-Change-Number: 83422 Gerrit-PatchSet: 6 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[S] Change in coreboot[main]: mb/qemu-{i440fx,q35}: reduce default ROM size to 8 MiB
by Martin L Roth (Code Review) 30 Aug '24

30 Aug '24

Martin L Roth has submitted this change. ( https://review.coreboot.org/c/coreboot/+/82639?usp=email ) Change subject: mb/qemu-{i440fx,q35}: reduce default ROM size to 8 MiB ...................................................................... mb/qemu-{i440fx,q35}: reduce default ROM size to 8 MiB By default, QEMU bails when trying to use bigger images mounted with '-drive if=pflash', which is required to make use of writable flash introduced in CB:82555. This changes both default size in Kconfig as well as FMAP layouts. Since QEMU 5.0.0 it is possible to change the limit of firmware size with `max-fw-size` machine configuration option, up to 16 MiB, as bigger sizes would overlap with IO APIC memory range. Default is still 8 MiB, so it makes sense to have identical default in coreboot. Error thrown by QEMU when trying to use too big ROM: qemu-system-x86_64: combined size of system firmware exceeds 8388608 bytes Change-Id: If36cb754a8e75e23bce49ff568dd88e5db279bb8 Signed-off-by: Krystian Hebel <krystian.hebel(a)3mdeb.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/82639 Reviewed-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Reviewed-by: Maximilian Brune <maximilian.brune(a)9elements.com> --- M src/mainboard/emulation/qemu-i440fx/Kconfig R src/mainboard/emulation/qemu-i440fx/vboot-rwa-8M.fmd M src/mainboard/emulation/qemu-q35/Kconfig D src/mainboard/emulation/qemu-q35/vboot-rwa-16M.fmd C src/mainboard/emulation/qemu-q35/vboot-rwa-8M.fmd R src/mainboard/emulation/qemu-q35/vboot-rwab-8M.fmd 6 files changed, 15 insertions(+), 34 deletions(-) Approvals: Maciej Pijanowski: Looks good to me, but someone else must approve Patrick Rudolph: Looks good to me, approved Maximilian Brune: Looks good to me, approved build bot (Jenkins): Verified diff --git a/src/mainboard/emulation/qemu-i440fx/Kconfig b/src/mainboard/emulation/qemu-i440fx/Kconfig index 842c053..6d6181c 100644 --- a/src/mainboard/emulation/qemu-i440fx/Kconfig +++ b/src/mainboard/emulation/qemu-i440fx/Kconfig @@ -12,7 +12,7 @@ select HAVE_PIRQ_TABLE select HAVE_ACPI_TABLES select BOARD_ROMSIZE_KB_4096 if !VBOOT - select BOARD_ROMSIZE_KB_16384 if VBOOT + select BOARD_ROMSIZE_KB_8192 if VBOOT select MAINBOARD_HAS_NATIVE_VGA_INIT select MAINBOARD_FORCE_NATIVE_VGA_INIT select HAVE_ASAN_IN_ROMSTAGE @@ -52,7 +52,7 @@ endif config FMDFILE - default "src/mainboard/\$(CONFIG_MAINBOARD_DIR)/vboot-rwa-16M.fmd" if VBOOT_SLOTS_RW_A + default "src/mainboard/\$(CONFIG_MAINBOARD_DIR)/vboot-rwa-8M.fmd" if VBOOT_SLOTS_RW_A config VBOOT_VBNV_OFFSET hex diff --git a/src/mainboard/emulation/qemu-i440fx/vboot-rwa-16M.fmd b/src/mainboard/emulation/qemu-i440fx/vboot-rwa-8M.fmd similarity index 65% rename from src/mainboard/emulation/qemu-i440fx/vboot-rwa-16M.fmd rename to src/mainboard/emulation/qemu-i440fx/vboot-rwa-8M.fmd index 0d2c9da..55a38b7 100644 --- a/src/mainboard/emulation/qemu-i440fx/vboot-rwa-16M.fmd +++ b/src/mainboard/emulation/qemu-i440fx/vboot-rwa-8M.fmd @@ -1,8 +1,8 @@ -FLASH@0xff000000 0x1000000 { - SI_BIOS 0x1000000 { - RW_SECTION_A 0x7c0000 { +FLASH@0xff800000 0x800000 { + SI_BIOS 0x800000 { + RW_SECTION_A 0x3c0000 { VBLOCK_A 0x10000 - FW_MAIN_A(CBFS) 0x74ffc0 + FW_MAIN_A(CBFS) RW_FWID_A 0x40 } RW_VPD(PRESERVE) 0x1000 diff --git a/src/mainboard/emulation/qemu-q35/Kconfig b/src/mainboard/emulation/qemu-q35/Kconfig index 2fb180b..70a7437 100644 --- a/src/mainboard/emulation/qemu-q35/Kconfig +++ b/src/mainboard/emulation/qemu-q35/Kconfig @@ -10,7 +10,7 @@ select HAVE_OPTION_TABLE # select HAVE_PIRQ_TABLE select HAVE_ACPI_TABLES - select BOARD_ROMSIZE_KB_16384 + select BOARD_ROMSIZE_KB_8192 select MAINBOARD_HAS_NATIVE_VGA_INIT select MAINBOARD_FORCE_NATIVE_VGA_INIT if !CHROMEOS select MEMORY_MAPPED_TPM @@ -29,8 +29,8 @@ select GBB_FLAG_DISABLE_FWMP config FMDFILE - default "src/mainboard/\$(CONFIG_MAINBOARD_DIR)/vboot-rwa-16M.fmd" if VBOOT_SLOTS_RW_A && !VBOOT_SLOTS_RW_AB - default "src/mainboard/\$(CONFIG_MAINBOARD_DIR)/vboot-rwab-16M.fmd" if VBOOT_SLOTS_RW_AB + default "src/mainboard/\$(CONFIG_MAINBOARD_DIR)/vboot-rwa-8M.fmd" if VBOOT_SLOTS_RW_A && !VBOOT_SLOTS_RW_AB + default "src/mainboard/\$(CONFIG_MAINBOARD_DIR)/vboot-rwab-8M.fmd" if VBOOT_SLOTS_RW_AB if ARCH_BOOTBLOCK_X86_64 diff --git a/src/mainboard/emulation/qemu-q35/vboot-rwa-16M.fmd b/src/mainboard/emulation/qemu-q35/vboot-rwa-16M.fmd deleted file mode 100644 index 0d2c9da..0000000 --- a/src/mainboard/emulation/qemu-q35/vboot-rwa-16M.fmd +++ /dev/null @@ -1,19 +0,0 @@ -FLASH@0xff000000 0x1000000 { - SI_BIOS 0x1000000 { - RW_SECTION_A 0x7c0000 { - VBLOCK_A 0x10000 - FW_MAIN_A(CBFS) 0x74ffc0 - RW_FWID_A 0x40 - } - RW_VPD(PRESERVE) 0x1000 - - WP_RO { - FMAP 0x800 - RO_FRID 0x40 - RO_PADDING 0x7c0 - RO_VPD(PRESERVE) 0x1000 - GBB 0x1e000 - COREBOOT(CBFS) - } - } -} diff --git a/src/mainboard/emulation/qemu-i440fx/vboot-rwa-16M.fmd b/src/mainboard/emulation/qemu-q35/vboot-rwa-8M.fmd similarity index 65% copy from src/mainboard/emulation/qemu-i440fx/vboot-rwa-16M.fmd copy to src/mainboard/emulation/qemu-q35/vboot-rwa-8M.fmd index 0d2c9da..55a38b7 100644 --- a/src/mainboard/emulation/qemu-i440fx/vboot-rwa-16M.fmd +++ b/src/mainboard/emulation/qemu-q35/vboot-rwa-8M.fmd @@ -1,8 +1,8 @@ -FLASH@0xff000000 0x1000000 { - SI_BIOS 0x1000000 { - RW_SECTION_A 0x7c0000 { +FLASH@0xff800000 0x800000 { + SI_BIOS 0x800000 { + RW_SECTION_A 0x3c0000 { VBLOCK_A 0x10000 - FW_MAIN_A(CBFS) 0x74ffc0 + FW_MAIN_A(CBFS) RW_FWID_A 0x40 } RW_VPD(PRESERVE) 0x1000 diff --git a/src/mainboard/emulation/qemu-q35/vboot-rwab-16M.fmd b/src/mainboard/emulation/qemu-q35/vboot-rwab-8M.fmd similarity index 89% rename from src/mainboard/emulation/qemu-q35/vboot-rwab-16M.fmd rename to src/mainboard/emulation/qemu-q35/vboot-rwab-8M.fmd index fcbfa95..8064fdf 100644 --- a/src/mainboard/emulation/qemu-q35/vboot-rwab-16M.fmd +++ b/src/mainboard/emulation/qemu-q35/vboot-rwab-8M.fmd @@ -1,5 +1,5 @@ -FLASH@0xff000000 0x1000000 { - SI_BIOS 0x1000000 { +FLASH@0xff800000 0x800000 { + SI_BIOS 0x800000 { RW_SECTION_A 0x1c0000 { VBLOCK_A 0x10000 FW_MAIN_A(CBFS) -- To view, visit https://review.coreboot.org/c/coreboot/+/82639?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: If36cb754a8e75e23bce49ff568dd88e5db279bb8 Gerrit-Change-Number: 82639 Gerrit-PatchSet: 7 Gerrit-Owner: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[L] Change in coreboot[main]: mb/qemu-q35/smihandler.c: add support for SMIs on QEMU
by Martin L Roth (Code Review) 30 Aug '24

30 Aug '24

Martin L Roth has submitted this change. ( https://review.coreboot.org/c/coreboot/+/82558?usp=email ) ( 9 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: mb/qemu-q35/smihandler.c: add support for SMIs on QEMU ...................................................................... mb/qemu-q35/smihandler.c: add support for SMIs on QEMU qemu-system-x86_64 uses AMD64 SMM save state format, despite emulating Intel chipset. In addition, even though it implements SMI_STS register, QEMU never sets any bits in it. As there is little emulated hardware that can be generating SMI, assume that all SMIs come from APM. This source is used e.g. to disable ACPI (which wasn't working until now on QEMU) and SMMSTORE. Tested by invoking SMMSTORE commands from the payload with SMM logging. Change-Id: I2fc7b74bdc13be8d76bc536283ab5a14fffec45f Signed-off-by: Krystian Hebel <krystian.hebel(a)3mdeb.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/82558 Reviewed-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/mainboard/emulation/qemu-q35/Makefile.mk A src/mainboard/emulation/qemu-q35/smihandler.c 2 files changed, 318 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Sergii Dmytruk: Looks good to me, approved diff --git a/src/mainboard/emulation/qemu-q35/Makefile.mk b/src/mainboard/emulation/qemu-q35/Makefile.mk index a60fbd6..c1619f5 100644 --- a/src/mainboard/emulation/qemu-q35/Makefile.mk +++ b/src/mainboard/emulation/qemu-q35/Makefile.mk @@ -23,3 +23,4 @@ smm-y += ../qemu-i440fx/rom_media.c smm-y += memmap.c +smm-y += smihandler.c diff --git a/src/mainboard/emulation/qemu-q35/smihandler.c b/src/mainboard/emulation/qemu-q35/smihandler.c new file mode 100644 index 0000000..a0417b5 --- /dev/null +++ b/src/mainboard/emulation/qemu-q35/smihandler.c @@ -0,0 +1,317 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <cpu/amd/amd64_save_state.h> +#include <cpu/x86/legacy_save_state.h> +#include <cpu/x86/save_state.h> +#include <cpu/x86/smm.h> +#include <elog.h> +#include <smmstore.h> +#include <southbridge/intel/common/pmbase.h> +#include <southbridge/intel/common/pmutil.h> +#include <string.h> + +/* + * SMM in QEMU is unlike that on real hardware. Most notable differences: + * + * - Revision ID is either 0x20000 or 0x20064, depending on whether + * qemu-system-i386 or qemu-system-x86_64 is being used. + * - SMI_STS is always 0. + * - Since I/O Instruction Restart bit in revision ID field is not set, none of + * the fields related to I/O instructions is set in saved state. It is + * impossible to check if SMI was generated by write to APMC port that way. + * - On older versions of QEMU, SMI isn't immediately emulated when Tiny Code + * Generator (TCG) accelerator is used, due to how Translation Blocks (TBs) + * are built. This means that contents of registers may change before SMI + * handler gets invoked. This can be worked around on the caller side by + * either writing to APMC port from a non-inlined function that doesn't + * return a result (so RAX isn't overwritten), or by following write to port + * with an instruction that forces generation of a new TB, e.g. 'pause'. In + * both cases, RIP in the save state will not point to the instruction + * directly following 'out'. When KVM is used, or in newer QEMU (8.2.2 is + * known to work) SMIs are injected immediately and RIP represents next + * instruction after `out`. + */ + +static const uint32_t amd64_revisions[] = { + 0x00020064, + SMM_REV_INVALID, +}; + +static int amd64_get_reg(const enum cpu_reg reg, const int node, void *out, + const uint8_t length) +{ + amd64_smm_state_save_area_t *save_state = smm_get_save_state(node); + + if (length != 1 && length != 2 && length != 4 && length != 8) + return -1; + + switch (reg) { + case RAX: + memcpy(out, &save_state->rax, length); + return 0; + case RCX: + memcpy(out, &save_state->rcx, length); + return 0; + case RDX: + memcpy(out, &save_state->rdx, length); + return 0; + case RBX: + memcpy(out, &save_state->rbx, length); + return 0; + } + + return -1; +} + +static int amd64_set_reg(const enum cpu_reg reg, const int node, void *in, + const uint8_t length) +{ + amd64_smm_state_save_area_t *save_state = smm_get_save_state(node); + + if (length != 1 && length != 2 && length != 4 && length != 8) + return -1; + + switch (reg) { + case RAX: + save_state->rax = 0; + memcpy(&save_state->rax, in, length); + return 0; + case RCX: + save_state->rcx = 0; + memcpy(&save_state->rcx, in, length); + return 0; + case RDX: + save_state->rdx = 0; + memcpy(&save_state->rdx, in, length); + return 0; + case RBX: + save_state->rbx = 0; + memcpy(&save_state->rbx, in, length); + return 0; + } + + return -1; +} + +static int amd64_apmc_node(u8 cmd) +{ + amd64_smm_state_save_area_t *save_state; + int node; + + for (node = 0; (unsigned int)node < CONFIG_MAX_CPUS; node++) { + save_state = smm_get_save_state(node); + + if (!save_state) + continue; + + /* + * Since fields related to I/O instructions are not filled in, check + * RAX against command number only. There is 1/256 probability of false + * positive. + * + * The alternative would be to: + * - parse saved CR0 and EFER to discover host's execution mode + * - parse saved CR3 and host page tables to obtain physical address + * corresponding to RIP + * - map that page (or multiple, potentially nonconsecutive pages) that + * cover the code + * - analyze the code and saved state against one of the `out` + * instructions to APM_CNT port + * - ideally do so in constant-time manner to not leak information + */ + if ((save_state->rax & 0xFF) == cmd) + return node; + } + + return -1; +} + +static const struct smm_save_state_ops _amd64_ops = { + .revision_table = amd64_revisions, + .get_reg = amd64_get_reg, + .set_reg = amd64_set_reg, + .apmc_node = amd64_apmc_node, +}; + +const struct smm_save_state_ops *amd64_ops = &_amd64_ops; + +static const uint32_t legacy_revisions[] = { + 0x00020000, + SMM_REV_INVALID, +}; + +static int legacy_get_reg(const enum cpu_reg reg, const int node, void *out, + const uint8_t length) +{ + legacy_smm_state_save_area_t *save_state = smm_get_save_state(node); + + if (length != 1 && length != 2 && length != 4) + return -1; + + switch (reg) { + case RAX: + memcpy(out, &save_state->eax, length); + return 0; + case RCX: + memcpy(out, &save_state->ecx, length); + return 0; + case RDX: + memcpy(out, &save_state->edx, length); + return 0; + case RBX: + memcpy(out, &save_state->ebx, length); + return 0; + } + + return -1; +} + +static int legacy_set_reg(const enum cpu_reg reg, const int node, void *in, + const uint8_t length) +{ + legacy_smm_state_save_area_t *save_state = smm_get_save_state(node); + + if (length != 1 && length != 2 && length != 4) + return -1; + + switch (reg) { + case RAX: + save_state->eax = 0; + memcpy(&save_state->eax, in, length); + return 0; + case RCX: + save_state->ecx = 0; + memcpy(&save_state->ecx, in, length); + return 0; + case RDX: + save_state->edx = 0; + memcpy(&save_state->edx, in, length); + return 0; + case RBX: + save_state->ebx = 0; + memcpy(&save_state->ebx, in, length); + return 0; + } + + return -1; +} + +static int legacy_apmc_node(u8 cmd) +{ + legacy_smm_state_save_area_t *save_state; + int node; + + for (node = 0; (unsigned int)node < CONFIG_MAX_CPUS; node++) { + save_state = smm_get_save_state(node); + + if (!save_state) + continue; + + /* + * Since fields related to I/O instructions are not filled in, check + * EAX against command number only. There is 1/256 probability of false + * positive. + * + * See comment in amd64_apmc_node(). + */ + if ((save_state->eax & 0xFF) == cmd) + return node; + } + + return -1; +} + +static const struct smm_save_state_ops _legacy_ops = { + .revision_table = legacy_revisions, + .get_reg = legacy_get_reg, + .set_reg = legacy_set_reg, + .apmc_node = legacy_apmc_node, +}; + +const struct smm_save_state_ops *legacy_ops = &_legacy_ops; + +static void mainboard_smi_gsmi(void) +{ + u32 ret; + u8 sub_command; + uintptr_t param; + int node = get_apmc_node(APM_CNT_ELOG_GSMI); + + if (node < 0) + return; + + /* Command and return value in EAX */ + if (get_save_state_reg(RAX, node, &ret, sizeof(ret))) + return; + + sub_command = (u8)(ret >> 8); + + /* Parameter buffer in EBX */ + if (get_save_state_reg(RBX, node, &param, sizeof(param))) + return; + + /* drivers/elog/gsmi.c */ + ret = gsmi_exec(sub_command, (u32 *)param); + + set_save_state_reg(RAX, node, &ret, sizeof(ret)); +} + +static void mainboard_smi_store(void) +{ + u32 ret; + u8 sub_command; + uintptr_t reg_rbx; + int node = get_apmc_node(APM_CNT_SMMSTORE); + + if (node < 0) + return; + + /* Command and return value in EAX */ + if (get_save_state_reg(RAX, node, &ret, sizeof(ret))) + return; + + sub_command = (u8)(ret >> 8); + + /* Parameter buffer in EBX */ + if (get_save_state_reg(RBX, node, &reg_rbx, sizeof(reg_rbx))) + return; + + /* drivers/smmstore/smi.c */ + ret = smmstore_exec(sub_command, (void *)reg_rbx); + + set_save_state_reg(RAX, node, &ret, sizeof(ret)); +} + +static int mainboard_finalized = 0; + +void cpu_smi_handler(void) +{ + u8 reg8; + + reg8 = apm_get_apmc(); + switch (reg8) { + case APM_CNT_ACPI_DISABLE: + write_pmbase32(PM1_CNT, read_pmbase32(PM1_CNT) & ~SCI_EN); + break; + case APM_CNT_ACPI_ENABLE: + write_pmbase32(PM1_CNT, read_pmbase32(PM1_CNT) | SCI_EN); + break; + case APM_CNT_FINALIZE: + if (mainboard_finalized) { + printk(BIOS_DEBUG, "SMI#: Already finalized\n"); + return; + } + + southbridge_finalize_all(); + mainboard_finalized = 1; + break; + case APM_CNT_ELOG_GSMI: + if (CONFIG(ELOG_GSMI)) + mainboard_smi_gsmi(); + break; + case APM_CNT_SMMSTORE: + if (CONFIG(SMMSTORE)) + mainboard_smi_store(); + break; + } +} -- To view, visit https://review.coreboot.org/c/coreboot/+/82558?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I2fc7b74bdc13be8d76bc536283ab5a14fffec45f Gerrit-Change-Number: 82558 Gerrit-PatchSet: 11 Gerrit-Owner: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>

1 0

[L] Change in coreboot[main]: mb/qemu-q35/smihandler.c: add support for SMIs on QEMU
by Martin L Roth (Code Review) 30 Aug '24

30 Aug '24

Attention is currently required from: Arthur Heymans, Krystian Hebel, Michał Kopeć, Michał Żygowski, Patrick Rudolph, Paul Menzel. Martin L Roth has posted comments on this change by Krystian Hebel. ( https://review.coreboot.org/c/coreboot/+/82558?usp=email ) Change subject: mb/qemu-q35/smihandler.c: add support for SMIs on QEMU ...................................................................... Patch Set 10: (2 comments) Patchset: PS2: > Moved the implementation to board directory. This also allowed me to fix SMM for qemu-system-i386. Done File src/mainboard/emulation/qemu-q35/smihandler.c: https://review.coreboot.org/c/coreboot/+/82558/comment/2ff1248f_2eeddb5b?us… : PS9, Line 20: It is : * impossible to check if SMI was generated by write to APMC port that way. > Technically possible, but not that simple. […] Acknowledged -- To view, visit https://review.coreboot.org/c/coreboot/+/82558?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I2fc7b74bdc13be8d76bc536283ab5a14fffec45f Gerrit-Change-Number: 82558 Gerrit-PatchSet: 10 Gerrit-Owner: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Martin L Roth <gaumless(a)gmail.com> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Comment-Date: Fri, 30 Aug 2024 15:44:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph(a)9elements.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Comment-In-Reply-To: Krystian Hebel <krystian.hebel(a)3mdeb.com>

1 0

[S] Change in coreboot[main]: mb/google/zork: Ensure eSPI GPIOs programmed w/o vboot
by Martin L Roth (Code Review) 30 Aug '24

30 Aug '24

Attention is currently required from: Jason Nien, Martin Roth, Matt DeVillier. Martin L Roth has posted comments on this change by Matt DeVillier. ( https://review.coreboot.org/c/coreboot/+/84148?usp=email ) Change subject: mb/google/zork: Ensure eSPI GPIOs programmed w/o vboot ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/84148?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I03efe6def37a018c3de410523be21bf008174e94 Gerrit-Change-Number: 84148 Gerrit-PatchSet: 1 Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Comment-Date: Fri, 30 Aug 2024 15:41:34 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[S] Change in coreboot[main]: mb/google/zork: Ensure early GPIOs programmed w/o vboot
by Martin L Roth (Code Review) 30 Aug '24

30 Aug '24

Attention is currently required from: Jason Nien, Martin Roth, Matt DeVillier. Martin L Roth has posted comments on this change by Matt DeVillier. ( https://review.coreboot.org/c/coreboot/+/84147?usp=email ) Change subject: mb/google/zork: Ensure early GPIOs programmed w/o vboot ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/84147?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I0bb49678b2d913c447d5bc761a6f0e00fca6334f Gerrit-Change-Number: 84147 Gerrit-PatchSet: 2 Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Comment-Date: Fri, 30 Aug 2024 15:41:19 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[S] Change in coreboot[main]: mb/google/brox/var/jubilant: Remove STORAGE_UNKNOWN fw_config option
by Karthik Ramasubramanian (Code Review) 30 Aug '24

30 Aug '24

Attention is currently required from: Bob Moragues, Bob Moragues, Eran Mitrani, Karthik Ramasubramanian, Ren Kuo, Ren Kuo. Karthik Ramasubramanian has posted comments on this change by Karthik Ramasubramanian. ( https://review.coreboot.org/c/coreboot/+/84095?usp=email ) Change subject: mb/google/brox/var/jubilant: Remove STORAGE_UNKNOWN fw_config option ...................................................................... Set Ready For Review -- To view, visit https://review.coreboot.org/c/coreboot/+/84095?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I4f6ace4b39a1ee0b63486d3872b20c8da719ae4a Gerrit-Change-Number: 84095 Gerrit-PatchSet: 8 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Bob Moragues <moragues(a)chromium.org> Gerrit-Reviewer: Bob Moragues <moragues(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Ren Kuo <ren.kuo(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Ren Kuo <ren.kuo(a)quantatw.com> Gerrit-Attention: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com> Gerrit-Attention: Bob Moragues <moragues(a)google.com> Gerrit-Attention: Bob Moragues <moragues(a)chromium.org> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Fri, 30 Aug 2024 15:40:31 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No

1 0

[S] Change in coreboot[main]: mb/google/brox/jubilant: I2C signal tuning
by Karthik Ramasubramanian (Code Review) 30 Aug '24

30 Aug '24

Attention is currently required from: Bob Moragues, Kenneth Chan, Nick Vaccaro, Ren Kuo. Karthik Ramasubramanian has posted comments on this change by Ren Kuo. ( https://review.coreboot.org/c/coreboot/+/84143?usp=email ) Change subject: mb/google/brox/jubilant: I2C signal tuning ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/84143?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I42a60edc0b361bfabacf5376ef89f436efedb356 Gerrit-Change-Number: 84143 Gerrit-PatchSet: 1 Gerrit-Owner: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Bob Moragues <moragues(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Kenneth Chan <kenneth.chan(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com> Gerrit-Attention: Bob Moragues <moragues(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Kenneth Chan <kenneth.chan(a)quanta.corp-partner.google.com> Gerrit-Comment-Date: Fri, 30 Aug 2024 15:35:05 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[XS] Change in coreboot[main]: mb/google/brox/var/lotso: Update verb table
by Karthik Ramasubramanian (Code Review) 30 Aug '24

30 Aug '24

Attention is currently required from: Jian Tong, Shelley Chen. Karthik Ramasubramanian has posted comments on this change by Jian Tong. ( https://review.coreboot.org/c/coreboot/+/83920?usp=email ) Change subject: mb/google/brox/var/lotso: Update verb table ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/83920?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ic4f03d09010efa7e32713b2697d5832255f64317 Gerrit-Change-Number: 83920 Gerrit-PatchSet: 3 Gerrit-Owner: Jian Tong <tongjian(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Kun Liu <liukun11(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jinfang Mao <maojinfang(a)huaqin.corp-partner.google.com> Gerrit-Attention: Shelley Chen <shchen(a)google.com> Gerrit-Attention: Jian Tong <tongjian(a)huaqin.corp-partner.google.com> Gerrit-Comment-Date: Fri, 30 Aug 2024 15:31:54 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[XS] Change in coreboot[main]: mb/google/brox/jubilant: Modify FP IRQ pin
by Karthik Ramasubramanian (Code Review) 30 Aug '24

30 Aug '24

Attention is currently required from: Marx Wang, Marx Wang, Nick Vaccaro, Ren Kuo, Subrata Banik. Karthik Ramasubramanian has posted comments on this change by Ren Kuo. ( https://review.coreboot.org/c/coreboot/+/84124?usp=email ) Change subject: mb/google/brox/jubilant: Modify FP IRQ pin ...................................................................... Patch Set 8: -Code-Review (1 comment) File src/mainboard/google/brox/variants/jubilant/gpio.c: https://review.coreboot.org/c/coreboot/+/84124/comment/e71356e1_dc4b30af?us… : PS8, Line 44: PPAD Typo: PAD -- To view, visit https://review.coreboot.org/c/coreboot/+/84124?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ic4a7ca07eab0dab234ab025cf77bbb8093b6b9d1 Gerrit-Change-Number: 84124 Gerrit-PatchSet: 8 Gerrit-Owner: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Bob Moragues <moragues(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Kenneth Chan <kenneth.chan(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Marx Wang <marx.wang(a)intel.com> Gerrit-Reviewer: Marx Wang <marx.wang(a)intel.corp-partner.google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Marx Wang <marx.wang(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Marx Wang <marx.wang(a)intel.corp-partner.google.com> Gerrit-Attention: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Comment-Date: Fri, 30 Aug 2024 15:27:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit August 2024