coreboot-gerrit February 2020

coreboot-gerrit@coreboot.org

1 participants
1797 discussions

Change in coreboot[master]: util/inteltool: fix Ie00fef5a5ba2779e0ff45640cff5cc9f1d096dc1 on FreeBSD
by Idwer Vollering (Code Review) 27 Nov '20

27 Nov '20

Idwer Vollering has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37767 ) Change subject: util/inteltool: fix Ie00fef5a5ba2779e0ff45640cff5cc9f1d096dc1 on FreeBSD ...................................................................... util/inteltool: fix Ie00fef5a5ba2779e0ff45640cff5cc9f1d096dc1 on FreeBSD Change-Id: I4aba72f39b528fd70451a4656fd6c835ff766e49 Signed-off-by: Idwer Vollering <vidwer(a)gmail.com> --- M util/inteltool/Makefile M util/inteltool/inteltool.h 2 files changed, 2 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/37767/1 diff --git a/util/inteltool/Makefile b/util/inteltool/Makefile index cd02fa8..918b6ec 100644 --- a/util/inteltool/Makefile +++ b/util/inteltool/Makefile @@ -23,6 +23,7 @@ INSTALL ?= /usr/bin/env install PREFIX ?= /usr/local CFLAGS ?= -O2 -g -Wall -Wextra -Wmissing-prototypes +CFLAGS += -Wno-unused-function LDFLAGS += -lpci -lz CPPFLAGS += -I$(top)/src/commonlib/include diff --git a/util/inteltool/inteltool.h b/util/inteltool/inteltool.h index fc6dc4b..81ebc72 100644 --- a/util/inteltool/inteltool.h +++ b/util/inteltool/inteltool.h @@ -370,7 +370,7 @@ #define rdmsr freebsd_rdmsr #define wrmsr freebsd_wrmsr typedef struct { uint32_t hi, lo; } msr_t; -msr_t freebsd_rdmsr(int addr); +static msr_t freebsd_rdmsr(int addr); int freebsd_wrmsr(int addr, msr_t msr); #endif typedef struct { uint16_t addr; int size; char *name; } io_register_t; -- To view, visit https://review.coreboot.org/c/coreboot/+/37767 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4aba72f39b528fd70451a4656fd6c835ff766e49 Gerrit-Change-Number: 37767 Gerrit-PatchSet: 1 Gerrit-Owner: Idwer Vollering <vidwer(a)gmail.com> Gerrit-MessageType: newchange

5 18

Change in em100[master]: WIP: Parse command table from installer file
by Stefan Reinauer (Code Review) 24 Nov '20

24 Nov '20

Stefan Reinauer has uploaded this change for review. ( https://review.coreboot.org/c/em100/+/37538 ) Change subject: WIP: Parse command table from installer file ...................................................................... WIP: Parse command table from installer file Change-Id: I126a16e2ae5d5b9df5846d9bef45fde307751c05 Signed-off-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> --- M Makefile A ini.c A ini.h M trace.c 4 files changed, 527 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/em100 refs/changes/38/37538/1 diff --git a/Makefile b/Makefile index fd1be91..7f90cef 100644 --- a/Makefile +++ b/Makefile @@ -36,7 +36,7 @@ XZ = xz/xz_crc32.c xz/xz_crc64.c xz/xz_dec_bcj.c xz/xz_dec_lzma2.c xz/xz_dec_stream.c SOURCES = em100.c firmware.c fpga.c hexdump.c sdram.c spi.c system.c trace.c usb.c -SOURCES += image.c curl.c chips.c tar.c $(XZ) +SOURCES += image.c curl.c chips.c tar.c ini.c $(XZ) OBJECTS = $(SOURCES:.c=.o) all: dep em100 diff --git a/ini.c b/ini.c new file mode 100644 index 0000000..b4d5921 --- /dev/null +++ b/ini.c @@ -0,0 +1,284 @@ +/* inih -- simple .INI file parser + +SPDX-License-Identifier: BSD-3-Clause + +Copyright (C) 2009-2019, Ben Hoyt + +inih is released under the New BSD license (see LICENSE.txt). Go to the project +home page for more info: + +https://github.com/benhoyt/inih + +*/ + +#if defined(_MSC_VER) && !defined(_CRT_SECURE_NO_WARNINGS) +#define _CRT_SECURE_NO_WARNINGS +#endif + +#include <stdio.h> +#include <ctype.h> +#include <string.h> + +#include "ini.h" + +#if !INI_USE_STACK +#include <stdlib.h> +#endif + +#define MAX_SECTION 50 +#define MAX_NAME 50 + +/* Used by ini_parse_string() to keep track of string parsing state. */ +typedef struct { + const char* ptr; + size_t num_left; +} ini_parse_string_ctx; + +/* Strip whitespace chars off end of given string, in place. Return s. */ +static char* rstrip(char* s) +{ + char* p = s + strlen(s); + while (p > s && isspace((unsigned char)(*--p))) + *p = '\0'; + return s; +} + +/* Return pointer to first non-whitespace char in given string. */ +static char* lskip(const char* s) +{ + while (*s && isspace((unsigned char)(*s))) + s++; + return (char*)s; +} + +/* Return pointer to first char (of chars) or inline comment in given string, + or pointer to null at end of string if neither found. Inline comment must + be prefixed by a whitespace character to register as a comment. */ +static char* find_chars_or_comment(const char* s, const char* chars) +{ +#if INI_ALLOW_INLINE_COMMENTS + int was_space = 0; + while (*s && (!chars || !strchr(chars, *s)) && + !(was_space && strchr(INI_INLINE_COMMENT_PREFIXES, *s))) { + was_space = isspace((unsigned char)(*s)); + s++; + } +#else + while (*s && (!chars || !strchr(chars, *s))) { + s++; + } +#endif + return (char*)s; +} + +/* Version of strncpy that ensures dest (size bytes) is null-terminated. */ +static char* strncpy0(char* dest, const char* src, size_t size) +{ + strncpy(dest, src, size - 1); + dest[size - 1] = '\0'; + return dest; +} + +/* See documentation in header file. */ +int ini_parse_stream(ini_reader reader, void* stream, ini_handler handler, + void* user) +{ + /* Uses a fair bit of stack (use heap instead if you need to) */ +#if INI_USE_STACK + char line[INI_MAX_LINE]; + int max_line = INI_MAX_LINE; +#else + char* line; + size_t max_line = INI_INITIAL_ALLOC; +#endif +#if INI_ALLOW_REALLOC && !INI_USE_STACK + char* new_line; + size_t offset; +#endif + char section[MAX_SECTION] = ""; + char prev_name[MAX_NAME] = ""; + + char* start; + char* end; + char* name; + char* value; + int lineno = 0; + int error = 0; + +#if !INI_USE_STACK + line = (char*)malloc(INI_INITIAL_ALLOC); + if (!line) { + return -2; + } +#endif + +#if INI_HANDLER_LINENO +#define HANDLER(u, s, n, v) handler(u, s, n, v, lineno) +#else +#define HANDLER(u, s, n, v) handler(u, s, n, v) +#endif + + /* Scan through stream line by line */ + while (reader(line, (int)max_line, stream) != NULL) { +#if INI_ALLOW_REALLOC && !INI_USE_STACK + offset = strlen(line); + while (offset == max_line - 1 && line[offset - 1] != '\n') { + max_line *= 2; + if (max_line > INI_MAX_LINE) + max_line = INI_MAX_LINE; + new_line = realloc(line, max_line); + if (!new_line) { + free(line); + return -2; + } + line = new_line; + if (reader(line + offset, (int)(max_line - offset), stream) == NULL) + break; + if (max_line >= INI_MAX_LINE) + break; + offset += strlen(line + offset); + } +#endif + + lineno++; + + start = line; +#if INI_ALLOW_BOM + if (lineno == 1 && (unsigned char)start[0] == 0xEF && + (unsigned char)start[1] == 0xBB && + (unsigned char)start[2] == 0xBF) { + start += 3; + } +#endif + start = lskip(rstrip(start)); + + if (strchr(INI_START_COMMENT_PREFIXES, *start)) { + /* Start-of-line comment */ + } +#if INI_ALLOW_MULTILINE + else if (*prev_name && *start && start > line) { + /* Non-blank line with leading whitespace, treat as continuation + of previous name's value (as per Python configparser). */ + if (!HANDLER(user, section, prev_name, start) && !error) + error = lineno; + } +#endif + else if (*start == '[') { + /* A "[section]" line */ + end = find_chars_or_comment(start + 1, "]"); + if (*end == ']') { + *end = '\0'; + strncpy0(section, start + 1, sizeof(section)); + *prev_name = '\0'; +#if INI_CALL_HANDLER_ON_NEW_SECTION + if (!HANDLER(user, section, NULL, NULL) && !error) + error = lineno; +#endif + } + else if (!error) { + /* No ']' found on section line */ + error = lineno; + } + } + else if (*start) { + /* Not a comment, must be a name[=:]value pair */ + end = find_chars_or_comment(start, "=:"); + if (*end == '=' || *end == ':') { + *end = '\0'; + name = rstrip(start); + value = end + 1; +#if INI_ALLOW_INLINE_COMMENTS + end = find_chars_or_comment(value, NULL); + if (*end) + *end = '\0'; +#endif + value = lskip(value); + rstrip(value); + + /* Valid name[=:]value pair found, call handler */ + strncpy0(prev_name, name, sizeof(prev_name)); + if (!HANDLER(user, section, name, value) && !error) + error = lineno; + } + else if (!error) { + /* No '=' or ':' found on name[=:]value line */ +#if INI_ALLOW_NO_VALUE + *end = '\0'; + name = rstrip(start); + if (!HANDLER(user, section, name, NULL) && !error) + error = lineno; +#else + error = lineno; +#endif + } + } + +#if INI_STOP_ON_FIRST_ERROR + if (error) + break; +#endif + } + +#if !INI_USE_STACK + free(line); +#endif + + return error; +} + +/* See documentation in header file. */ +int ini_parse_file(FILE* file, ini_handler handler, void* user) +{ + return ini_parse_stream((ini_reader)fgets, file, handler, user); +} + +/* See documentation in header file. */ +int ini_parse(const char* filename, ini_handler handler, void* user) +{ + FILE* file; + int error; + + file = fopen(filename, "r"); + if (!file) + return -1; + error = ini_parse_file(file, handler, user); + fclose(file); + return error; +} + +/* An ini_reader function to read the next line from a string buffer. This + is the fgets() equivalent used by ini_parse_string(). */ +static char* ini_reader_string(char* str, int num, void* stream) { + ini_parse_string_ctx* ctx = (ini_parse_string_ctx*)stream; + const char* ctx_ptr = ctx->ptr; + size_t ctx_num_left = ctx->num_left; + char* strp = str; + char c; + + if (ctx_num_left == 0 || num < 2) + return NULL; + + while (num > 1 && ctx_num_left != 0) { + c = *ctx_ptr++; + ctx_num_left--; + *strp++ = c; + if (c == '\n') + break; + num--; + } + + *strp = '\0'; + ctx->ptr = ctx_ptr; + ctx->num_left = ctx_num_left; + return str; +} + +/* See documentation in header file. */ +int ini_parse_string(const char* string, ini_handler handler, void* user) { + ini_parse_string_ctx ctx; + + ctx.ptr = string; + ctx.num_left = strlen(string); + return ini_parse_stream((ini_reader)ini_reader_string, &ctx, handler, + user); +} diff --git a/ini.h b/ini.h new file mode 100644 index 0000000..d2e07e6 --- /dev/null +++ b/ini.h @@ -0,0 +1,148 @@ +/* inih -- simple .INI file parser + +SPDX-License-Identifier: BSD-3-Clause + +Copyright (C) 2009-2019, Ben Hoyt + +inih is released under the New BSD license (see LICENSE.txt). Go to the project +home page for more info: + +https://github.com/benhoyt/inih + +*/ + +#ifndef __INI_H__ +#define __INI_H__ + +/* Make this header file easier to include in C++ code */ +#ifdef __cplusplus +extern "C" { +#endif + +#include <stdio.h> + +/* Nonzero if ini_handler callback should accept lineno parameter. */ +#ifndef INI_HANDLER_LINENO +#define INI_HANDLER_LINENO 0 +#endif + +/* Typedef for prototype of handler function. */ +#if INI_HANDLER_LINENO +typedef int (*ini_handler)(void* user, const char* section, + const char* name, const char* value, + int lineno); +#else +typedef int (*ini_handler)(void* user, const char* section, + const char* name, const char* value); +#endif + +/* Typedef for prototype of fgets-style reader function. */ +typedef char* (*ini_reader)(char* str, int num, void* stream); + +/* Parse given INI-style file. May have [section]s, name=value pairs + (whitespace stripped), and comments starting with ';' (semicolon). Section + is "" if name=value pair parsed before any section heading. name:value + pairs are also supported as a concession to Python's configparser. + + For each name=value pair parsed, call handler function with given user + pointer as well as section, name, and value (data only valid for duration + of handler call). Handler should return nonzero on success, zero on error. + + Returns 0 on success, line number of first error on parse error (doesn't + stop on first error), -1 on file open error, or -2 on memory allocation + error (only when INI_USE_STACK is zero). +*/ +int ini_parse(const char* filename, ini_handler handler, void* user); + +/* Same as ini_parse(), but takes a FILE* instead of filename. This doesn't + close the file when it's finished -- the caller must do that. */ +int ini_parse_file(FILE* file, ini_handler handler, void* user); + +/* Same as ini_parse(), but takes an ini_reader function pointer instead of + filename. Used for implementing custom or string-based I/O (see also + ini_parse_string). */ +int ini_parse_stream(ini_reader reader, void* stream, ini_handler handler, + void* user); + +/* Same as ini_parse(), but takes a zero-terminated string with the INI data +instead of a file. Useful for parsing INI data from a network socket or +already in memory. */ +int ini_parse_string(const char* string, ini_handler handler, void* user); + +/* Nonzero to allow multi-line value parsing, in the style of Python's + configparser. If allowed, ini_parse() will call the handler with the same + name for each subsequent line parsed. */ +#ifndef INI_ALLOW_MULTILINE +#define INI_ALLOW_MULTILINE 1 +#endif + +/* Nonzero to allow a UTF-8 BOM sequence (0xEF 0xBB 0xBF) at the start of + the file. See https://github.com/benhoyt/inih/issues/21 */ +#ifndef INI_ALLOW_BOM +#define INI_ALLOW_BOM 1 +#endif + +/* Chars that begin a start-of-line comment. Per Python configparser, allow + both ; and # comments at the start of a line by default. */ +#ifndef INI_START_COMMENT_PREFIXES +#define INI_START_COMMENT_PREFIXES ";#" +#endif + +/* Nonzero to allow inline comments (with valid inline comment characters + specified by INI_INLINE_COMMENT_PREFIXES). Set to 0 to turn off and match + Python 3.2+ configparser behaviour. */ +#ifndef INI_ALLOW_INLINE_COMMENTS +#define INI_ALLOW_INLINE_COMMENTS 1 +#endif +#ifndef INI_INLINE_COMMENT_PREFIXES +#define INI_INLINE_COMMENT_PREFIXES ";" +#endif + +/* Nonzero to use stack for line buffer, zero to use heap (malloc/free). */ +#ifndef INI_USE_STACK +#define INI_USE_STACK 1 +#endif + +/* Maximum line length for any line in INI file (stack or heap). Note that + this must be 3 more than the longest line (due to '\r', '\n', and '\0'). */ +#ifndef INI_MAX_LINE +#define INI_MAX_LINE 200 +#endif + +/* Nonzero to allow heap line buffer to grow via realloc(), zero for a + fixed-size buffer of INI_MAX_LINE bytes. Only applies if INI_USE_STACK is + zero. */ +#ifndef INI_ALLOW_REALLOC +#define INI_ALLOW_REALLOC 0 +#endif + +/* Initial size in bytes for heap line buffer. Only applies if INI_USE_STACK + is zero. */ +#ifndef INI_INITIAL_ALLOC +#define INI_INITIAL_ALLOC 200 +#endif + +/* Stop parsing on first error (default is to keep parsing). */ +#ifndef INI_STOP_ON_FIRST_ERROR +#define INI_STOP_ON_FIRST_ERROR 0 +#endif + +/* Nonzero to call the handler at the start of each new section (with + name and value NULL). Default is to only call the handler on + each name=value pair. */ +#ifndef INI_CALL_HANDLER_ON_NEW_SECTION +#define INI_CALL_HANDLER_ON_NEW_SECTION 0 +#endif + +/* Nonzero to allow a name without a value (no '=' or ':' on the line) and + call the handler with value NULL in this case. Default is to treat + no-value lines as an error. */ +#ifndef INI_ALLOW_NO_VALUE +#define INI_ALLOW_NO_VALUE 0 +#endif + +#ifdef __cplusplus +} +#endif + +#endif /* __INI_H__ */ diff --git a/trace.c b/trace.c index 31b6e06..07d93ab 100644 --- a/trace.c +++ b/trace.c @@ -12,8 +12,89 @@ */ #include <stdio.h> +#include <stdlib.h> #include <string.h> #include "em100.h" +#include "ini.h" + +typedef struct +{ + int id; + int iomode; + int address; + int data; + int dummy; + const char *description; + const char *remarks; +} spi_command_t; + +spi_command_t spi_commands[256]; + +static int handler(void* user, const char* section, const char* name, + const char* value) +{ + spi_command_t *cmd = (spi_command_t *)user; + + int id = strtol(section, NULL, 16); + if (id < 0 || id > 255) + return 0; /* unknown command */ + + cmd[id].id = id; + /* + * [00] + * IO_Mode=0 + * Address=0 + * Data=0 + * Dummy=0 + * Description=No operation(0x00) + * Remarks=Macronix + */ + + if (strcmp(name, "IO_Mode") == 0) { + cmd[id].iomode = atoi(value); + } else if (strcmp(name, "Address") == 0) { + cmd[id].address = atoi(value); + } else if (strcmp(name, "Data") == 0) { + cmd[id].data = atoi(value); + } else if (strcmp(name, "Dummy") == 0) { + cmd[id].dummy = atoi(value); + } else if (strcmp(name, "Description") == 0) { + cmd[id].description = strdup(value); + } else if (strcmp(name, "Remarks") == 0) { + cmd[id].remarks = strdup(value); + } else { + return 0; /* unknown name, error */ + } + + return 1; +} + +static int read_command_table(void) +{ + int i; + char *command_table_name = get_em100_file("EM100ProCmdIOTable.ini"); + + if (ini_parse(command_table_name, handler, &spi_commands) < 0) { + printf("Can't load 'EM100ProCmdIOTable.ini'\n"); + free(command_table_name); + return 1; + } + free(command_table_name); + + for (i=0; i<256; i++) { + if (spi_commands[i].id != i) + continue; + printf("SPI Command: %02x\n", spi_commands[i].id); + printf(" iomode: %d\n", spi_commands[i].iomode); + printf(" address: %d\n", spi_commands[i].address); + printf(" data: %d\n", spi_commands[i].data); + printf(" dummy: %d\n", spi_commands[i].dummy); + printf(" description: %s\n", spi_commands[i].description); + printf(" remarks: %s\n", spi_commands[i].remarks); + } + + return 0; +} /* SPI Trace related operations */ @@ -34,15 +115,6 @@ return 1; } -/** - * read_spi_trace: fetch SPI trace data - * @param em100: em100 device structure - * globals: curpos, counter, cmdid - * - * out(16 bytes): bc 00 00 00 08 00 00 00 00 15 00 00 00 00 00 00 - * in(8x8192 bytes): 2 bytes (BE) number of records (0..0x3ff), - * then records of 8 bytes each - */ static unsigned int counter = 0; static unsigned char curpos = 0; static unsigned char cmdid = 0xff; // timestamp, so never a valid command id @@ -145,6 +217,16 @@ } #define MAX_TRACE_BLOCKLENGTH 6 + +/** + * read_spi_trace: fetch SPI trace data + * @param em100: em100 device structure + * globals: curpos, counter, cmdid + * + * out(16 bytes): bc 00 00 00 08 00 00 00 00 15 00 00 00 00 00 00 + * in(8x8192 bytes): 2 bytes (BE) number of records (0..0x3ff), + * then records of 8 bytes each + */ int read_spi_trace(struct em100 *em100, int display_terminal, unsigned long addr_offset) { @@ -159,6 +241,9 @@ static unsigned long long start_timestamp = 0; static struct spi_cmd_values *spi_cmd_vals = &spi_command_list[3]; + if (!read_command_table()) + return 0; + if (!read_report_buffer(em100, reportdata)) return 0; -- To view, visit https://review.coreboot.org/c/em100/+/37538 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: em100 Gerrit-Branch: master Gerrit-Change-Id: I126a16e2ae5d5b9df5846d9bef45fde307751c05 Gerrit-Change-Number: 37538 Gerrit-PatchSet: 1 Gerrit-Owner: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-MessageType: newchange

1 7

Change in coreboot[master]: cbfs: Add metadata cache
by Julius Werner (Code Review) 21 Nov '20

21 Nov '20

Hello Aaron Durbin, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/38423 to review the following change. Change subject: cbfs: Add metadata cache ...................................................................... cbfs: Add metadata cache This patch adds a new CBFS "mcache" (metadata cache) -- a memory buffer that stores the headers of all CBFS files. Similar to the existing FMAP cache, this cache should reduce the amount of SPI accesses we need to do every boot: rather than having to re-read all CBFS headers from SPI flash every time we're looking for a file, we can just walk the same list in this in-memory copy and finally use it to directly access the flash at the right position for the file data. This patch adds the code to support the cache but doesn't enable it on any platform. The next one will turn it on by default. Change-Id: I5b1084bfdad1c6ab0ee1b143ed8dd796827f4c65 Signed-off-by: Julius Werner <jwerner(a)chromium.org> --- M src/commonlib/Makefile.inc A src/commonlib/bsd/cbfs_mcache.c M src/commonlib/bsd/include/commonlib/bsd/cb_err.h M src/commonlib/bsd/include/commonlib/bsd/cbfs_private.h M src/commonlib/include/commonlib/cbmem_id.h M src/include/cbfs.h M src/include/memlayout.h M src/include/symbols.h M src/lib/Kconfig M src/lib/cbfs.c M src/lib/coreboot_table.c M src/security/vboot/vboot_common.h M src/security/vboot/vboot_loader.c 13 files changed, 346 insertions(+), 31 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/38423/1 diff --git a/src/commonlib/Makefile.inc b/src/commonlib/Makefile.inc index b2225cb..1a38e4a 100644 --- a/src/commonlib/Makefile.inc +++ b/src/commonlib/Makefile.inc @@ -37,6 +37,13 @@ ramstage-y += bsd/cbfs_private.c smm-y += bsd/cbfs_private.c +bootblock-y += bsd/cbfs_mcache.c +verstage-y += bsd/cbfs_mcache.c +romstage-y += bsd/cbfs_mcache.c +postcar-y += bsd/cbfs_mcache.c +ramstage-y += bsd/cbfs_mcache.c +smm-y += bsd/cbfs_mcache.c + decompressor-y += bsd/lz4_wrapper.c bootblock-y += bsd/lz4_wrapper.c verstage-y += bsd/lz4_wrapper.c diff --git a/src/commonlib/bsd/cbfs_mcache.c b/src/commonlib/bsd/cbfs_mcache.c new file mode 100644 index 0000000..d97dcce --- /dev/null +++ b/src/commonlib/bsd/cbfs_mcache.c @@ -0,0 +1,147 @@ +/* SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0-or-later */ + +#include <commonlib/bsd/cbfs_private.h> + +#define MCACHE_TOKEN_MAGIC 0xd +#define MCACHE_OFFSET_FULL 0 +#define MCACHE_OFFSET_END 1 + +struct mcache_token { + uint32_t offset : 28; + uint32_t magic : 4; +}; + +struct cbfs_mcache_build_args { + void *mcache; + void *end; + size_t offset; + int count; +}; + +static void make_token(struct cbfs_mcache_build_args *args, size_t new_offset) +{ + struct mcache_token *token = args->mcache; + + /* If there's no space for the token, lookup() will know the cache is full anyway. */ + if (args->end - args->mcache < sizeof(*token)) + return; + + token->magic = MCACHE_TOKEN_MAGIC; + token->offset = new_offset; + + args->mcache += sizeof(*token); +} + +static cb_err_t build_walker(cbfs_dev_t dev, size_t offset, union cbfs_mdata *mdata, + bool do_hash, void *arg) +{ + struct cbfs_mcache_build_args *args = arg; + const uint32_t data_offset = be32toh(mdata->h.offset); + const uint32_t data_length = be32toh(mdata->h.len); + + if (offset != args->offset) + make_token(args, offset); + + if (args->end - args->mcache < data_offset) { + make_token(args, MCACHE_OFFSET_FULL); + return CB_CBFS_CACHE_FULL; + } + + if (cbfs_copy_fill_metadata(args->mcache, mdata, dev, offset, do_hash) != CB_SUCCESS) + return CB_CBFS_IO; + + args->mcache += data_offset; + args->offset = ALIGN_UP(offset + data_offset + data_length, CBFS_ALIGNMENT); + args->count++; + + return CB_CBFS_NOT_FOUND; +} + +cb_err_t cbfs_mcache_build(cbfs_dev_t dev, void *mcache, size_t size, + struct vb2_hash *master_hash) +{ + struct cbfs_mcache_build_args args = { + .mcache = mcache, + .end = mcache + size, + .offset = 0, + .count = 0, + }; + + cb_err_t ret = cbfs_walk(dev, build_walker, &args, master_hash, 0); + if (ret == CB_CBFS_CACHE_FULL) + ERROR("mcache overflow, should increase CBFS_MCACHE size!\n"); + else if (ret != CB_CBFS_NOT_FOUND) + return ret; + + make_token(&args, MCACHE_OFFSET_END); + LOG("mcache @%p built for %d files, used %#zx bytes\n", + mcache, args.count, args.mcache - mcache); + return CB_SUCCESS; +} + +cb_err_t cbfs_mcache_lookup(cbfs_dev_t dev, void *mcache, size_t mcache_size, const char *name, + union cbfs_mdata *mdata_out, size_t *data_offset_out, + struct vb2_hash *master_hash) +{ + size_t namesize = strlen(name) + 1; /* Count trailing \0 so we can memcmp() it. */ + void *end = mcache + mcache_size; + void *current = mcache; + + size_t offset = 0; + while (end - current >= sizeof(struct mcache_token)) { + union cbfs_mdata *mdata = current; + + if (memcmp(mdata->h.magic, CBFS_FILE_MAGIC, sizeof(mdata->h.magic)) != 0) { + struct mcache_token *token = current; + if (token->magic != MCACHE_TOKEN_MAGIC) { + ERROR("CBFS mcache corrupted at %p\n", token); + return CB_ERR; + } + if (token->offset == MCACHE_OFFSET_END) + return CB_CBFS_NOT_FOUND; + if (token->offset == MCACHE_OFFSET_FULL) + break; + offset = token->offset; + current += sizeof(*token); + continue; + } + + const uint32_t data_offset = be32toh(mdata->h.offset); + const uint32_t data_length = be32toh(mdata->h.len); + if (namesize <= cbfs_filename_size(&mdata->h) && + memcmp(name, mdata->filename, namesize) == 0) { + LOG("Found '%s' @%#zx size %#x in mcache @%p\n", + name, offset, data_length, current); + *data_offset_out = offset + data_offset; + memcpy(mdata_out, mdata, data_offset); + return CB_SUCCESS; + } + + offset = ALIGN_UP(offset + data_offset + data_length, CBFS_ALIGNMENT); + current += data_offset; + } + + return cbfs_lookup(dev, name, mdata_out, data_offset_out, master_hash); +} + +size_t cbfs_mcache_real_size(void *mcache, size_t mcache_size) +{ + void *end = mcache + mcache_size; + void *current = mcache; + + while (end - current >= sizeof(struct mcache_token)) { + union cbfs_mdata *mdata = current; + + if (memcmp(mdata->h.magic, CBFS_FILE_MAGIC, sizeof(mdata->h.magic)) != 0) { + struct mcache_token *token = current; + current += sizeof(*token); + if (token->offset == MCACHE_OFFSET_END || + token->offset == MCACHE_OFFSET_FULL) + break; + } else { + current += be32toh(mdata->h.offset); + } + } + + return current - mcache; +} diff --git a/src/commonlib/bsd/include/commonlib/bsd/cb_err.h b/src/commonlib/bsd/include/commonlib/bsd/cb_err.h index ab422e1..18aa4d7 100644 --- a/src/commonlib/bsd/include/commonlib/bsd/cb_err.h +++ b/src/commonlib/bsd/include/commonlib/bsd/cb_err.h @@ -39,6 +39,7 @@ CB_CBFS_IO = -400, /**< Underlying I/O error */ CB_CBFS_NOT_FOUND = -401, /**< File not found in directory */ CB_CBFS_HASH = -402, /**< Master hash validation failed */ + CB_CBFS_CACHE_FULL = -403, /**< Metadata cache overflowed */ }; /* Don't typedef the enum directly, so the size is unambiguous for serialization. */ diff --git a/src/commonlib/bsd/include/commonlib/bsd/cbfs_private.h b/src/commonlib/bsd/include/commonlib/bsd/cbfs_private.h index b4fc1c7..374a460 100644 --- a/src/commonlib/bsd/include/commonlib/bsd/cbfs_private.h +++ b/src/commonlib/bsd/include/commonlib/bsd/cbfs_private.h @@ -92,4 +92,22 @@ cb_err_t cbfs_lookup(cbfs_dev_t dev, const char *name, union cbfs_mdata *mdata_out, size_t *data_offset_out, struct vb2_hash *master_hash); +/* Build an in-memory CBFS metadata cache out of the CBFS on |dev| into a |mcache_size| bytes + * memory area at |mcache|. Also verify |master_hash| unless it is NULL. */ +cb_err_t cbfs_mcache_build(cbfs_dev_t dev, void *mcache, size_t mcache_size, + struct vb2_hash *master_hash); + +/* + * Find a file named |name| in a CBFS metadata cache and copy its metadata into |mdata_out|. + * Pass out offset to the file data (on the storage backend). May fall back to cbfs_lookup() if + * metadata cache was too small for CBFS. |dev| and |master_hash| must have the same values as + * originally passed to cbfs_mcache_build(). + */ +cb_err_t cbfs_mcache_lookup(cbfs_dev_t dev, void *mcache, size_t mcache_size, const char *name, + union cbfs_mdata *mdata_out, size_t *data_offset_out, + struct vb2_hash *master_hash); + +/* Returns the amount of bytes actually used by the CBFS metadata cache in |mcache|. */ +size_t cbfs_mcache_real_size(void *mcache, size_t mcache_size); + #endif /* _COMMONLIB_BSD_CBFS_PRIVATE_H_ */ diff --git a/src/commonlib/include/commonlib/cbmem_id.h b/src/commonlib/include/commonlib/cbmem_id.h index b063cd1..8f4f7f3 100644 --- a/src/commonlib/include/commonlib/cbmem_id.h +++ b/src/commonlib/include/commonlib/cbmem_id.h @@ -79,6 +79,8 @@ #define CBMEM_ID_ROM2 0x524f4d32 #define CBMEM_ID_ROM3 0x524f4d33 #define CBMEM_ID_FMAP 0x464d4150 +#define CBMEM_ID_CBFS_RO_MCACHE 0x524d5346 +#define CBMEM_ID_CBFS_RW_MCACHE 0x574d5346 #define CBMEM_ID_FSP_LOGO 0x4c4f474f #define CBMEM_ID_TO_NAME_TABLE \ @@ -141,5 +143,7 @@ { CBMEM_ID_ROM1, "VGA ROM #1 "}, \ { CBMEM_ID_ROM2, "VGA ROM #2 "}, \ { CBMEM_ID_ROM3, "VGA ROM #3 "}, \ - { CBMEM_ID_FMAP, "FMAP "}, + { CBMEM_ID_FMAP, "FMAP "}, \ + { CBMEM_ID_CBFS_RO_MCACHE, "RO MCACHE "}, \ + { CBMEM_ID_CBFS_RW_MCACHE, "RW MCACHE "} #endif /* _CBMEM_ID_H_ */ diff --git a/src/include/cbfs.h b/src/include/cbfs.h index 2d16aa7..82c92d7 100644 --- a/src/include/cbfs.h +++ b/src/include/cbfs.h @@ -18,6 +18,7 @@ #include <commonlib/cbfs.h> #include <program_loading.h> +#include <types.h> /*********************************************** * Perform CBFS operations on the boot device. * @@ -59,8 +60,18 @@ /* Load stage into memory filling in prog. Return 0 on success. < 0 on error. */ int cbfs_prog_stage_load(struct prog *prog); -/* Returns the region device of the currently active CBFS. - Return < 0 on error, 0 on success. */ -int cbfs_boot_region_device(struct region_device *rdev); +struct cbfs_boot_device { + struct region_device rdev; + void *mcache; + size_t mcache_size; +}; + +/* + * Retrieves the currently active CBFS boot device. If |force_ro| is set, will + * always return the read-only CBFS instead (this only makes a difference when + * CONFIG_VBOOT is enabled). May perform certain CBFS initialization tasks. + * Returns NULL on error (e.g. boot device IO error). + */ +const struct cbfs_boot_device *cbfs_get_boot_device(bool force_ro); #endif diff --git a/src/include/memlayout.h b/src/include/memlayout.h index e3aeec6..0d430dee 100644 --- a/src/include/memlayout.h +++ b/src/include/memlayout.h @@ -81,6 +81,9 @@ _ = ASSERT(sz >= FMAP_SIZE, \ STR(FMAP does not fit in FMAP_CACHE! (sz < FMAP_SIZE))); +#define CBFS_MCACHE(addr, sz) \ + REGION(cbfs_mcache, addr, sz, 8) + #if ENV_ROMSTAGE_OR_BEFORE #define PRERAM_CBFS_CACHE(addr, size) \ REGION(preram_cbfs_cache, addr, size, 4) \ diff --git a/src/include/symbols.h b/src/include/symbols.h index eec4701..a8e1a82 100644 --- a/src/include/symbols.h +++ b/src/include/symbols.h @@ -34,6 +34,7 @@ DECLARE_REGION(preram_cbfs_cache) DECLARE_REGION(postram_cbfs_cache) DECLARE_REGION(cbfs_cache) +DECLARE_REGION(cbfs_mcache) DECLARE_REGION(fmap_cache) DECLARE_REGION(payload) diff --git a/src/lib/Kconfig b/src/lib/Kconfig index dd9974a..26bf0be 100644 --- a/src/lib/Kconfig +++ b/src/lib/Kconfig @@ -75,3 +75,24 @@ If your platform really doesn't want to use an FMAP cache (e.g. due to space constraints), you can select this to disable warnings and save a bit more code. + +config NO_CBFS_MCACHE + bool + default y + help + Disables the CBFS metadata cache. This means that your platform does + not need to provide a CBFS_MCACHE section in memlayout and can save + the associated CAR/SRAM size. In that case every single CBFS file + lookup must re-read the same CBFS directory entries from flash to find + the respective file. + +config CBFS_MCACHE_RW_PERCENTAGE + int + depends on VBOOT + default 25 if CHROMEOS # Chrome OS stores many L10n files in RO only + default 50 + help + The amount of the CBFS_MCACHE area that's used for the RW CBFS, in + percent from 0 to 100. The remaining area will be used for the RO + CBFS. Default is an even 50/50 split. When VBOOT is disabled, this + will automatically be 0 (meaning the whole MCACHE is used for RO). diff --git a/src/lib/cbfs.c b/src/lib/cbfs.c index d6fb99c..40290fd 100644 --- a/src/lib/cbfs.c +++ b/src/lib/cbfs.c @@ -15,6 +15,7 @@ */ #include <assert.h> +#include <cbmem.h> #include <console/console.h> #include <string.h> #include <stdlib.h> @@ -30,30 +31,40 @@ #include <security/vboot/vboot_crtm.h> #include <security/vboot/vboot_common.h> +static cb_err_t cbfs_boot_lookup(const struct cbfs_boot_device *cbd, + const char *name, union cbfs_mdata *mdata, size_t *data_offset) +{ + if (CONFIG(NO_CBFS_MCACHE)) + return cbfs_lookup(&cbd->rdev, name, mdata, data_offset, NULL); + else + return cbfs_mcache_lookup(&cbd->rdev, + cbd->mcache, cbd->mcache_size, + name, mdata, data_offset, NULL); +} + int cbfs_boot_locate(struct cbfsf *fh, const char *name, uint32_t *type) { - struct region_device rdev; - - if (cbfs_boot_region_device(&rdev)) + const struct cbfs_boot_device *cbd = cbfs_get_boot_device(false); + if (!cbd) return -1; union cbfs_mdata mdata; size_t data_offset; - cb_err_t err = cbfs_lookup(&rdev, name, &mdata, &data_offset, NULL); + cb_err_t err = cbfs_boot_lookup(cbd, name, &mdata, &data_offset); if (CONFIG(VBOOT_ENABLE_CBFS_FALLBACK) && err == CB_CBFS_NOT_FOUND) { printk(BIOS_INFO, "CBFS: Fall back to RO region for %s\n", name); - if (fmap_locate_area_as_rdev("COREBOOT", &rdev)) + if (!(cbd = cbfs_get_boot_device(true))) return -1; - err = cbfs_lookup(&rdev, name, &mdata, &data_offset, NULL); + err = cbfs_boot_lookup(cbd, name, &mdata, &data_offset); } if (err) return -1; size_t msize = be32toh(mdata.h.offset); - if (rdev_chain(&fh->metadata, &rdev, data_offset - msize, msize) || - rdev_chain(&fh->data, &rdev, data_offset, be32toh(mdata.h.len))) + if (rdev_chain(&fh->metadata, &cbd->rdev, data_offset - msize, msize) || + rdev_chain(&fh->data, &cbd->rdev, data_offset, be32toh(mdata.h.len))) return -1; if (type) { if (!*type) @@ -280,9 +291,66 @@ return 0; } -int cbfs_boot_region_device(struct region_device *rdev) +const struct cbfs_boot_device *cbfs_get_boot_device(bool force_ro) { + static struct cbfs_boot_device ro; + + if (!force_ro) { + const struct cbfs_boot_device *rw = vboot_get_cbfs_boot_device(); + if (rw) + return rw; + } + + if (region_device_sz(&ro.rdev)) + return &ro; + + if (!CONFIG(NO_CBFS_MCACHE)) { + const struct cbmem_entry *entry; + if (cbmem_possibly_online() && + (entry = cbmem_entry_find(CBMEM_ID_CBFS_RO_MCACHE))) { + ro.mcache = cbmem_entry_start(entry); + ro.mcache_size = cbmem_entry_size(entry); + } else if (ENV_ROMSTAGE_OR_BEFORE) { + ro.mcache = _cbfs_mcache; + ro.mcache_size = REGION_SIZE(cbfs_mcache) * + (100 - CONFIG_CBFS_MCACHE_RW_PERCENTAGE) / 100; + } + } + boot_device_init(); - return vboot_locate_cbfs(rdev) && - fmap_locate_area_as_rdev("COREBOOT", rdev); + if (fmap_locate_area_as_rdev("COREBOOT", &ro.rdev)) + return NULL; + + if (!CONFIG(NO_CBFS_MCACHE) && ENV_BOOTBLOCK) { + static bool mcache_done = false; + if (!mcache_done && ro.mcache_size > 0 && cbfs_mcache_build( + &ro.rdev, ro.mcache, ro.mcache_size, NULL)) + return NULL; + mcache_done = true; + } + return &ro; } + +#if !CONFIG(NO_CBFS_MCACHE) +static void mcache_to_cbmem(const struct cbfs_boot_device *cbd, u32 cbmem_id) +{ + if (!cbd) + return; + + size_t real_size = cbfs_mcache_real_size(cbd->mcache, cbd->mcache_size); + void *cbmem_mcache = cbmem_add(cbmem_id, real_size); + if (!cbmem_mcache) { + printk(BIOS_ERR, "ERROR: Cannot allocate CBMEM mcache %#x (%#zx bytes)!\n", + cbmem_id, real_size); + return; + } + memcpy(cbmem_mcache, cbd->mcache, real_size); +} + +static void cbfs_mcache_migrate(int unused) +{ + mcache_to_cbmem(vboot_get_cbfs_boot_device(), CBMEM_ID_CBFS_RW_MCACHE); + mcache_to_cbmem(cbfs_get_boot_device(true), CBMEM_ID_CBFS_RO_MCACHE); +} +ROMSTAGE_CBMEM_INIT_HOOK(cbfs_mcache_migrate) +#endif diff --git a/src/lib/coreboot_table.c b/src/lib/coreboot_table.c index e42cb3b..b46ca57 100644 --- a/src/lib/coreboot_table.c +++ b/src/lib/coreboot_table.c @@ -243,11 +243,8 @@ { struct lb_boot_media_params *bmp; const struct region_device *boot_dev; - struct region_device cbfs_dev; - - boot_device_init(); - - if (cbfs_boot_region_device(&cbfs_dev)) + const struct cbfs_boot_device *cbd = cbfs_get_boot_device(false); + if (!cbd) return; boot_dev = boot_device_ro(); @@ -258,8 +255,8 @@ bmp->tag = LB_TAG_BOOT_MEDIA_PARAMS; bmp->size = sizeof(*bmp); - bmp->cbfs_offset = region_device_offset(&cbfs_dev); - bmp->cbfs_size = region_device_sz(&cbfs_dev); + bmp->cbfs_offset = region_device_offset(&cbd->rdev); + bmp->cbfs_size = region_device_sz(&cbd->rdev); bmp->boot_media_size = region_device_sz(boot_dev); bmp->fmap_offset = get_fmap_flash_offset(); diff --git a/src/security/vboot/vboot_common.h b/src/security/vboot/vboot_common.h index 976c26a..0ebefb6 100644 --- a/src/security/vboot/vboot_common.h +++ b/src/security/vboot/vboot_common.h @@ -16,6 +16,7 @@ #define __VBOOT_VBOOT_COMMON_H__ #include <commonlib/region.h> +#include <cbfs.h> #include <stdint.h> #include <vboot_api.h> #include <vboot_struct.h> @@ -71,7 +72,7 @@ int vboot_recovery_mode_memory_retrain(void); int vboot_can_enable_udc(void); void vboot_run_logic(void); -int vboot_locate_cbfs(struct region_device *rdev); +const struct cbfs_boot_device *vboot_get_cbfs_boot_device(void); #else /* !CONFIG_VBOOT */ static inline int vboot_developer_mode_enabled(void) { return 0; } static inline int vboot_recovery_mode_enabled(void) { return 0; } @@ -79,7 +80,7 @@ /* If VBOOT is not enabled, we are okay enabling USB device controller (UDC). */ static inline int vboot_can_enable_udc(void) { return 1; } static inline void vboot_run_logic(void) {} -static inline int vboot_locate_cbfs(struct region_device *rdev) { return -1; } +const struct cbfs_boot_device *vboot_get_cbfs_boot_device(void) { return NULL; } #endif void vboot_save_nvdata_only(struct vb2_context *ctx); diff --git a/src/security/vboot/vboot_loader.c b/src/security/vboot/vboot_loader.c index 7e63775..a8db7a3 100644 --- a/src/security/vboot/vboot_loader.c +++ b/src/security/vboot/vboot_loader.c @@ -13,7 +13,10 @@ * GNU General Public License for more details. */ +#include <boot_device.h> #include <cbfs.h> +#include <cbmem.h> +#include <commonlib/bsd/cbfs_private.h> #include <console/console.h> #include <ec/google/chromeec/ec.h> #include <rmodule.h> @@ -34,12 +37,26 @@ int vboot_executed; +static void build_rw_mcache(void) +{ + if (CONFIG(NO_CBFS_MCACHE)) + return; + + if (vboot_get_context()->flags & VB2_CONTEXT_RECOVERY_MODE) + return; /* No RW mcache if we're staying in RO. */ + + const struct cbfs_boot_device *cbd = vboot_get_cbfs_boot_device(); + if (cbfs_mcache_build(&cbd->rdev, cbd->mcache, cbd->mcache_size, NULL)) + die("Failed to build RW mcache."); /* TODO: -> recovery? */ +} + void vboot_run_logic(void) { if (verification_should_run()) { /* Note: this path is not used for VBOOT_RETURN_FROM_VERSTAGE */ verstage_main(); vboot_executed = 1; + build_rw_mcache(); } else if (verstage_should_load()) { struct cbfsf file; struct prog verstage = @@ -67,21 +84,40 @@ return; vboot_executed = 1; + build_rw_mcache(); } } -int vboot_locate_cbfs(struct region_device *rdev) +const struct cbfs_boot_device *vboot_get_cbfs_boot_device(void) { - struct vb2_context *ctx; - /* Don't honor vboot results until the vboot logic has run. */ if (!vboot_logic_executed()) - return -1; + return NULL; - ctx = vboot_get_context(); + static struct cbfs_boot_device cbd; + if (region_device_sz(&cbd.rdev)) + return &cbd; + struct vb2_context *ctx = vboot_get_context(); if (ctx->flags & VB2_CONTEXT_RECOVERY_MODE) - return -1; + return NULL; - return vboot_locate_firmware(ctx, rdev); + boot_device_init(); + if (vboot_locate_firmware(ctx, &cbd.rdev)) + return NULL; + + if (!CONFIG(NO_CBFS_MCACHE)) { + const struct cbmem_entry *entry; + if (cbmem_possibly_online() && + (entry = cbmem_entry_find(CBMEM_ID_CBFS_RW_MCACHE))) { + cbd.mcache = cbmem_entry_start(entry); + cbd.mcache_size = cbmem_entry_size(entry); + } else if (ENV_ROMSTAGE_OR_BEFORE) { + cbd.mcache_size = REGION_SIZE(cbfs_mcache) * + CONFIG_CBFS_MCACHE_RW_PERCENTAGE / 100; + cbd.mcache = _ecbfs_mcache - cbd.mcache_size; + } + } + + return &cbd; } -- To view, visit https://review.coreboot.org/c/coreboot/+/38423 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5b1084bfdad1c6ab0ee1b143ed8dd796827f4c65 Gerrit-Change-Number: 38423 Gerrit-PatchSet: 1 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-MessageType: newchange

6 60

Change in coreboot[master]: soc/intel/fsp-car: Halt on improperly configured coreboot
by Arthur Heymans (Code Review) 19 Nov '20

19 Nov '20

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37280 ) Change subject: soc/intel/fsp-car: Halt on improperly configured coreboot ...................................................................... soc/intel/fsp-car: Halt on improperly configured coreboot Test if Kconfig symbols actually match what FSP does. If it is not the case it best to halt right away instead of continuing to boot with possibly hard to debug problems. Change-Id: I8520adc868f08938af27ee686a8777c281afd4f9 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/soc/intel/common/block/cpu/car/cache_as_ram_fsp.S 1 file changed, 5 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/37280/1 diff --git a/src/soc/intel/common/block/cpu/car/cache_as_ram_fsp.S b/src/soc/intel/common/block/cpu/car/cache_as_ram_fsp.S index 4174f36..bde3849 100644 --- a/src/soc/intel/common/block/cpu/car/cache_as_ram_fsp.S +++ b/src/soc/intel/common/block/cpu/car/cache_as_ram_fsp.S @@ -86,6 +86,11 @@ cmp $0, %eax jnz .halt_forever + /* Check sanity of coreboot Kconfig symbols that need to match what FSP + does */ + cmpl $(CONFIG_DCACHE_RAM_BASE + CONFIG_DCACHE_RAM_SIZE), %edx + jne .halt_forever + /* Setup bootblock stack */ movl _ecar_stack, %esp -- To view, visit https://review.coreboot.org/c/coreboot/+/37280 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8520adc868f08938af27ee686a8777c281afd4f9 Gerrit-Change-Number: 37280 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newchange

2 2

Change in coreboot[master]: drivers/intel/fsp1.1/car: Halt on improperly configured coreboot
by Arthur Heymans (Code Review) 19 Nov '20

19 Nov '20

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37281 ) Change subject: drivers/intel/fsp1.1/car: Halt on improperly configured coreboot ...................................................................... drivers/intel/fsp1.1/car: Halt on improperly configured coreboot Test if Kconfig symbols actually match what FSP does. If it is not the case it best to halt right away instead of continuing to boot with possibly hard to debug problems. Change-Id: Ieec8259149cc8fd71c40fd1511a6a2cfa67c5337 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/drivers/intel/fsp1_1/cache_as_ram.S 1 file changed, 5 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/81/37281/1 diff --git a/src/drivers/intel/fsp1_1/cache_as_ram.S b/src/drivers/intel/fsp1_1/cache_as_ram.S index fb39884..748c267 100644 --- a/src/drivers/intel/fsp1_1/cache_as_ram.S +++ b/src/drivers/intel/fsp1_1/cache_as_ram.S @@ -143,6 +143,11 @@ cmp $0, %eax jne halt2 + /* Check sanity of coreboot Kconfig symbols that need to match what FSP + does */ + cmpl $(CONFIG_DCACHE_RAM_BASE + CONFIG_DCACHE_RAM_SIZE), %edx + jne .halt_forever + /* Setup BSP stack */ movl _ecar_stack, %esp -- To view, visit https://review.coreboot.org/c/coreboot/+/37281 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ieec8259149cc8fd71c40fd1511a6a2cfa67c5337 Gerrit-Change-Number: 37281 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newchange

1 1

Change in coreboot[master]: sc7180: add qclib_post_fixup to allow SoC specific code
by mturney mturney (Code Review) 11 Nov '20

11 Nov '20

mturney mturney has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37795 ) Change subject: sc7180: add qclib_post_fixup to allow SoC specific code ...................................................................... sc7180: add qclib_post_fixup to allow SoC specific code * Provide hook for SoC specific code to run after QCLib Change-Id: I5507187fddfd7d0af0ee0983b06d67077de0dbc8 Signed-off-by: T Michael Turney <mturney(a)codeaurora.org> --- M src/soc/qualcomm/common/include/soc/qclib_common.h M src/soc/qualcomm/common/qclib.c 2 files changed, 8 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/37795/1 diff --git a/src/soc/qualcomm/common/include/soc/qclib_common.h b/src/soc/qualcomm/common/include/soc/qclib_common.h index 19ec083..5817c08 100644 --- a/src/soc/qualcomm/common/include/soc/qclib_common.h +++ b/src/soc/qualcomm/common/include/soc/qclib_common.h @@ -72,5 +72,6 @@ uint32_t size, uint32_t attrs); void qclib_load_and_run(void); int qclib_soc_blob_load(void); +int qclib_post_fixup(void); #endif // _SOC_QUALCOMM_QCLIB_COMMON_H_ diff --git a/src/soc/qualcomm/common/qclib.c b/src/soc/qualcomm/common/qclib.c index ac80a76..ce07078 100644 --- a/src/soc/qualcomm/common/qclib.c +++ b/src/soc/qualcomm/common/qclib.c @@ -119,6 +119,7 @@ } __weak int qclib_soc_blob_load(void) { return 0; } +__weak int qclib_post_fixup(void) { return 0; } void qclib_load_and_run(void) { @@ -209,6 +210,12 @@ mmu_restore_context(&pre_qclib_mmu_context); mmu_enable(); + /* hook for SoC specific fixup after QCLib runs */ + if (qclib_post_fixup()) { + printk(BIOS_ERR, "qclib_post_fixup failed\n"); + goto fail; + } + /* step through I/F table, handling return values */ for (i = 0; i < qclib_cb_if_table.num_entries; i++) if (qclib_cb_if_table.te[i].blob_attributes & -- To view, visit https://review.coreboot.org/c/coreboot/+/37795 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5507187fddfd7d0af0ee0983b06d67077de0dbc8 Gerrit-Change-Number: 37795 Gerrit-PatchSet: 1 Gerrit-Owner: mturney mturney <mturney(a)codeaurora.org> Gerrit-MessageType: newchange

2 7

Change in coreboot[master]: soc/amd/picasso: Use common SMM save state ops
by Arthur Heymans (Code Review) 11 Nov '20

11 Nov '20

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37019 ) Change subject: soc/amd/picasso: Use common SMM save state ops ...................................................................... soc/amd/picasso: Use common SMM save state ops Change-Id: I9eb42b9f9b14925a682356e21bbb0d2ccaeaafec Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/soc/amd/picasso/smihandler.c 1 file changed, 24 insertions(+), 15 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/37019/1 diff --git a/src/soc/amd/picasso/smihandler.c b/src/soc/amd/picasso/smihandler.c index 39c2dfd..b04eddb 100644 --- a/src/soc/amd/picasso/smihandler.c +++ b/src/soc/amd/picasso/smihandler.c @@ -19,7 +19,6 @@ #include <console/console.h> #include <cpu/x86/smm.h> #include <cpu/x86/cache.h> -#include <cpu/amd/amd64_save_state.h> #include <arch/acpi.h> #include <arch/hlt.h> #include <device/pci_def.h> @@ -41,17 +40,18 @@ SMM_IO_TRAP_PORT_ADDRESS_MASK); } -static void *find_save_state(int cmd) +static int find_save_state_node(int cmd) { int core; - amd64_smm_state_save_area_t *state; + const struct smm_save_state_ops *ops = get_save_state_ops(); u32 smm_io_trap; u8 reg_al; + u64 rax; /* Check all nodes looking for the one that issued the IO */ for (core = 0; core < CONFIG_MAX_CPUS; core++) { - state = smm_get_save_state(core); - smm_io_trap = state->smm_io_trap_offset; + if (ops->get_io_trap(core, &smm_io_trap)) + continue; /* Check for Valid IO Trap Word (bit1==1) */ if (!(smm_io_trap & SMM_IO_TRAP_VALID)) continue; @@ -62,30 +62,39 @@ if (pm_acpi_smi_cmd_port() != get_io_address(smm_io_trap)) continue; /* Check AL against the requested command */ - reg_al = state->rax; + if (ops->get_reg(core, RAX, &rax)) + continue; + reg_al = rax; if (reg_al == cmd) - return state; + return core; } - return NULL; + return -1; } static void southbridge_smi_gsmi(void) { + u32 ret, param; u8 sub_command; - amd64_smm_state_save_area_t *io_smi; - u32 reg_ebx; + int node = find_save_state_node(APM_CNT_ELOG_GSMI); + const struct smm_save_state_ops *ops = get_save_state_ops(); - io_smi = find_save_state(APM_CNT_ELOG_GSMI); - if (!io_smi) + if (node < -1) return; + /* Command and return value in EAX */ - sub_command = (io_smi->rax >> 8) & 0xff; + uint64_t reg; + if (ops->get_reg(node, RAX, &reg)) + return; + sub_command = (u8)(reg >> 8); /* Parameter buffer in EBX */ - reg_ebx = io_smi->rbx; + if (ops->get_reg(node, RBX, &reg)) + return; + param = reg & UINT32_MAX; /* drivers/elog/gsmi.c */ - io_smi->rax = gsmi_exec(sub_command, &reg_ebx); + ret = gsmi_exec(sub_command, (u32 *)param); + ops->set_reg(node, RAX, reg); } static void sb_apmc_smi_handler(void) -- To view, visit https://review.coreboot.org/c/coreboot/+/37019 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9eb42b9f9b14925a682356e21bbb0d2ccaeaafec Gerrit-Change-Number: 37019 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newchange

1 1

Change in coreboot[master]: soc/amd/stoneyridge: Use common SMM save state ops
by Arthur Heymans (Code Review) 11 Nov '20

11 Nov '20

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37018 ) Change subject: soc/amd/stoneyridge: Use common SMM save state ops ...................................................................... soc/amd/stoneyridge: Use common SMM save state ops Change-Id: I2f98062dd254528c0c72e8739035c9e620a8497f Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/soc/amd/stoneyridge/smihandler.c 1 file changed, 24 insertions(+), 15 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/37018/1 diff --git a/src/soc/amd/stoneyridge/smihandler.c b/src/soc/amd/stoneyridge/smihandler.c index 2b88397..d2945c8 100644 --- a/src/soc/amd/stoneyridge/smihandler.c +++ b/src/soc/amd/stoneyridge/smihandler.c @@ -19,7 +19,6 @@ #include <console/console.h> #include <cpu/x86/smm.h> #include <cpu/x86/cache.h> -#include <cpu/amd/amd64_save_state.h> #include <arch/acpi.h> #include <arch/hlt.h> #include <device/pci_def.h> @@ -41,17 +40,18 @@ SMM_IO_TRAP_PORT_ADDRESS_MASK); } -static void *find_save_state(int cmd) +static int find_save_state_node(int cmd) { int core; - amd64_smm_state_save_area_t *state; + const struct smm_save_state_ops *ops = get_save_state_ops(); u32 smm_io_trap; u8 reg_al; + u64 rax; /* Check all nodes looking for the one that issued the IO */ for (core = 0; core < CONFIG_MAX_CPUS; core++) { - state = smm_get_save_state(core); - smm_io_trap = state->smm_io_trap_offset; + if (ops->get_io_trap(core, &smm_io_trap)) + continue; /* Check for Valid IO Trap Word (bit1==1) */ if (!(smm_io_trap & SMM_IO_TRAP_VALID)) continue; @@ -62,30 +62,39 @@ if (pm_acpi_smi_cmd_port() != get_io_address(smm_io_trap)) continue; /* Check AL against the requested command */ - reg_al = state->rax; + if (ops->get_reg(core, RAX, &rax)) + continue; + reg_al = rax; if (reg_al == cmd) - return state; + return core; } - return NULL; + return -1; } static void southbridge_smi_gsmi(void) { + u32 ret, param; u8 sub_command; - amd64_smm_state_save_area_t *io_smi; - u32 reg_ebx; + int node = find_save_state_node(APM_CNT_ELOG_GSMI); + const struct smm_save_state_ops *ops = get_save_state_ops(); - io_smi = find_save_state(APM_CNT_ELOG_GSMI); - if (!io_smi) + if (node < -1) return; + /* Command and return value in EAX */ - sub_command = (io_smi->rax >> 8) & 0xff; + uint64_t reg; + if (ops->get_reg(node, RAX, &reg)) + return; + sub_command = (u8)(reg >> 8); /* Parameter buffer in EBX */ - reg_ebx = io_smi->rbx; + if (ops->get_reg(node, RBX, &reg)) + return; + param = reg & UINT32_MAX; /* drivers/elog/gsmi.c */ - io_smi->rax = gsmi_exec(sub_command, &reg_ebx); + ret = gsmi_exec(sub_command, (u32 *)param); + ops->set_reg(node, RAX, reg); } static void sb_apmc_smi_handler(void) -- To view, visit https://review.coreboot.org/c/coreboot/+/37018 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2f98062dd254528c0c72e8739035c9e620a8497f Gerrit-Change-Number: 37018 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newchange

1 1

Change in coreboot[master]: soc/intel/denverton_ns: Use common SMM save state ops
by Arthur Heymans (Code Review) 11 Nov '20

11 Nov '20

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37017 ) Change subject: soc/intel/denverton_ns: Use common SMM save state ops ...................................................................... soc/intel/denverton_ns: Use common SMM save state ops Change-Id: Ieb5baaa4e36d97414d87addf5c8e318b881e8996 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/soc/intel/denverton_ns/smihandler.c 1 file changed, 36 insertions(+), 28 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/37017/1 diff --git a/src/soc/intel/denverton_ns/smihandler.c b/src/soc/intel/denverton_ns/smihandler.c index c292e4d..ce0022f 100644 --- a/src/soc/intel/denverton_ns/smihandler.c +++ b/src/soc/intel/denverton_ns/smihandler.c @@ -23,7 +23,6 @@ #include <console/console.h> #include <cpu/x86/cache.h> #include <cpu/x86/smm.h> -#include <cpu/intel/em64t100_save_state.h> #include <device/pci_def.h> #include <intelblocks/fast_spi.h> #include <spi-generic.h> @@ -168,35 +167,39 @@ * core in case we are not running on the same core that * initiated the IO transaction. */ -static em64t100_smm_state_save_area_t *smi_apmc_find_state_save(uint8_t cmd) +static int smi_apmc_find_state_save_node(u8 cmd) { - em64t100_smm_state_save_area_t *state; + const struct smm_save_state_ops *ops = get_save_state_ops(); int node; /* Check all nodes looking for the one that issued the IO */ for (node = 0; node < CONFIG_MAX_CPUS; node++) { - state = smm_get_save_state(node); - - /* Check for Synchronous IO (bit0==1) */ - if (!(state->io_misc_info & (1 << 0))) - continue; - - /* Make sure it was a write (bit4==0) */ - if (state->io_misc_info & (1 << 4)) - continue; - - /* Check for APMC IO port */ - if (((state->io_misc_info >> 16) & 0xff) != APM_CNT) - continue; - + uint32_t io_misc_info; + uint64_t rax; + if (ops->get_reg(node, RAX, &rax)) + continue; /* ??? */ /* Check AX against the requested command */ - if ((state->rax & 0xff) != cmd) + if ((rax & 0xff) != cmd) continue; + if (ops->get_io_misc_info(node, &io_misc_info)) { - return state; + /* Check for Synchronous IO (bit0 == 1) */ + if (!(io_misc_info & (1 << 0))) + continue; + + /* Make sure it was a write (bit4 == 0) */ + if (io_misc_info & (1 << 4)) + continue; + + /* Check for APMC IO port */ + if (((io_misc_info >> 16) & 0xff) != APM_CNT) + continue; + } + + return node; } - return NULL; + return -1; } static void finalize(void) @@ -217,7 +220,7 @@ static void southbridge_smi_apmc(void) { uint8_t reg8; - em64t100_smm_state_save_area_t *state; + const struct smm_save_state_ops *ops = get_save_state_ops(); /* Emulate B2 register as the FADT / Linux expects it */ @@ -254,13 +257,18 @@ "SMI#: SMM structures already initialized!\n"); return; } - state = smi_apmc_find_state_save(reg8); - if (state) { - /* EBX in the state save contains the GNVS pointer */ - gnvs = (global_nvs_t *)((uint32_t)state->rbx); - smm_initialized = 1; - printk(BIOS_DEBUG, "SMI#: Setting GNVS to %p\n", gnvs); - } + int node = smi_apmc_find_state_save_node(APM_CNT_GNVS_UPDATE); + if (node < -1) + return; + + uint64_t rbx; + if (ops->get_reg(node, RBX, &rbx)) + return; + + /* EBX in the state save contains the GNVS pointer */ + gnvs = (global_nvs_t *)(u32)rbx; + smm_initialized = 1; + printk(BIOS_DEBUG, "SMI#: Setting GNVS to %p\n", gnvs); break; } -- To view, visit https://review.coreboot.org/c/coreboot/+/37017 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ieb5baaa4e36d97414d87addf5c8e318b881e8996 Gerrit-Change-Number: 37017 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Guckian <david.guckian(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Vanny E <vanessa.f.eusebio(a)intel.com> Gerrit-MessageType: newchange

1 1

Change in coreboot[master]: soc/intel/common: Use common SMM save state ops
by Arthur Heymans (Code Review) 11 Nov '20

11 Nov '20

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37015 ) Change subject: soc/intel/common: Use common SMM save state ops ...................................................................... soc/intel/common: Use common SMM save state ops Change-Id: I656aad823a96eafb507e8b30d2177d21efcf2416 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/soc/intel/apollolake/smihandler.c M src/soc/intel/cannonlake/smihandler.c M src/soc/intel/common/block/include/intelblocks/smihandler.h M src/soc/intel/common/block/smm/smihandler.c M src/soc/intel/common/block/smm/smitraphandler.c M src/soc/intel/icelake/smihandler.c M src/soc/intel/skylake/smihandler.c M src/soc/intel/tigerlake/smihandler.c 8 files changed, 98 insertions(+), 269 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/37015/1 diff --git a/src/soc/intel/apollolake/smihandler.c b/src/soc/intel/apollolake/smihandler.c index 53d2b7e..ede073d 100644 --- a/src/soc/intel/apollolake/smihandler.c +++ b/src/soc/intel/apollolake/smihandler.c @@ -29,11 +29,6 @@ return 1; } -const struct smm_save_state_ops *get_smm_save_state_ops(void) -{ - return &em64t100_smm_ops; -} - /* SMI handlers that should be serviced in SCI mode too. */ uint32_t smihandler_soc_get_sci_mask(void) { diff --git a/src/soc/intel/cannonlake/smihandler.c b/src/soc/intel/cannonlake/smihandler.c index 4d0b241..31ec9e3 100644 --- a/src/soc/intel/cannonlake/smihandler.c +++ b/src/soc/intel/cannonlake/smihandler.c @@ -32,11 +32,6 @@ #define CSME0_BAR 0x0 #define CSME0_FID 0xb0 -const struct smm_save_state_ops *get_smm_save_state_ops(void) -{ - return &em64t101_smm_ops; -} - static void pch_disable_heci(void) { struct pcr_sbi_msg msg = { diff --git a/src/soc/intel/common/block/include/intelblocks/smihandler.h b/src/soc/intel/common/block/include/intelblocks/smihandler.h index d8520f1..91bf88f 100644 --- a/src/soc/intel/common/block/include/intelblocks/smihandler.h +++ b/src/soc/intel/common/block/include/intelblocks/smihandler.h @@ -22,29 +22,7 @@ struct gpi_status; struct global_nvs_t; -/* - * The register value is used with get_reg and set_reg - */ -enum smm_reg { - RAX, - RBX, - RCX, - RDX, -}; - -struct smm_save_state_ops { - /* return io_misc_info from SMM Save State Area */ - uint32_t (*get_io_misc_info)(void *state); - - /* return value of the requested register from - * SMM Save State Area - */ - uint64_t (*get_reg)(void *state, enum smm_reg reg); - - void (*set_reg)(void *state, enum smm_reg reg, uint64_t val); -}; - -typedef void (*smi_handler_t)(const struct smm_save_state_ops *save_state_ops); +typedef void (*smi_handler_t)(void); /* * SOC SMI Handler has to provide this structure which has methods to access @@ -66,79 +44,69 @@ * the SMI event on SLP_EN. The default functionality is provided in * soc/intel/common/block/smm/smihandler.c */ -void smihandler_southbridge_sleep( - const struct smm_save_state_ops *save_state_ops); +void smihandler_southbridge_sleep(void); /* * This function should be implemented in SOC specific code to handle * SMI_APM event. The default functionality is provided in * soc/intel/common/block/smm/smihandler.c */ -void smihandler_southbridge_apmc( - const struct smm_save_state_ops *save_state_ops); +void smihandler_southbridge_apmc(void); /* * This function should be implemented in SOC specific code to handle * SMI_PM1 event. The default functionality is provided in * soc/intel/common/block/smm/smihandler.c */ -void smihandler_southbridge_pm1( - const struct smm_save_state_ops *save_state_ops); +void smihandler_southbridge_pm1(void); /* * This function should be implemented in SOC specific code to handle * SMI_GPE0 event. The default functionality is provided in * soc/intel/common/block/smm/smihandler.c */ -void smihandler_southbridge_gpe0( - const struct smm_save_state_ops *save_state_ops); +void smihandler_southbridge_gpe0(void); /* * This function should be implemented in SOC specific code to handle * MC event. The default functionality is provided in * soc/intel/common/block/smm/smihandler.c */ -void smihandler_southbridge_mc( - const struct smm_save_state_ops *save_state_ops); +void smihandler_southbridge_mc(void); /* * This function should be implemented in SOC specific code to handle * minitor event. The default functionality is provided in * soc/intel/common/block/smm/smihandler.c */ -void smihandler_southbridge_monitor( - const struct smm_save_state_ops *save_state_ops); +void smihandler_southbridge_monitor(void); /* * This function should be implemented in SOC specific code to handle * SMI_TCO event. The default functionality is provided in * soc/intel/common/block/smm/smihandler.c */ -void smihandler_southbridge_tco( - const struct smm_save_state_ops *save_state_ops); +void smihandler_southbridge_tco(void); /* * This function should be implemented in SOC specific code to handle * SMI PERIODIC_STS event. The default functionality is provided in * soc/intel/common/block/smm/smihandler.c */ -void smihandler_southbridge_periodic( - const struct smm_save_state_ops *save_state_ops); +void smihandler_southbridge_periodic(void); /* * This function should be implemented in SOC specific code to handle * SMI GPIO_STS event. The default functionality is provided in * soc/intel/common/block/smm/smihandler.c */ -void smihandler_southbridge_gpi( - const struct smm_save_state_ops *save_state_ops); +void smihandler_southbridge_gpi(void); /* * This function should be implemented in SOC specific code to handle * SMI ESPI_STS event. The default functionality is provided in * soc/intel/common/block/smm/smihandler.c */ -void smihandler_southbridge_espi( - const struct smm_save_state_ops *save_state_ops); +void smihandler_southbridge_espi(void); /* * Returns gnvs pointer within SMM context @@ -172,8 +140,4 @@ /* Mainboard handler for ESPI EMIs */ void mainboard_smi_espi_handler(void); - -extern const struct smm_save_state_ops em64t100_smm_ops; - -extern const struct smm_save_state_ops em64t101_smm_ops; #endif diff --git a/src/soc/intel/common/block/smm/smihandler.c b/src/soc/intel/common/block/smm/smihandler.c index 0581d23..d7330df 100644 --- a/src/soc/intel/common/block/smm/smihandler.c +++ b/src/soc/intel/common/block/smm/smihandler.c @@ -20,8 +20,6 @@ #include <console/console.h> #include <cpu/x86/cache.h> #include <cpu/x86/smm.h> -#include <cpu/intel/em64t100_save_state.h> -#include <cpu/intel/em64t101_save_state.h> #include <delay.h> #include <device/pci_def.h> #include <elog.h> @@ -87,36 +85,44 @@ /* Common Functions */ -static void *find_save_state(const struct smm_save_state_ops *save_state_ops, - int cmd) +/* + * Look for Synchronous IO SMI and use save state from that + * core in case we are not running on the same core that + * initiated the IO transaction. + */ +static int smi_apmc_find_state_save_node(u8 cmd) { + const struct smm_save_state_ops *ops = get_save_state_ops(); int node; - void *state = NULL; - uint32_t io_misc_info; - uint8_t reg_al; /* Check all nodes looking for the one that issued the IO */ for (node = 0; node < CONFIG_MAX_CPUS; node++) { - state = smm_get_save_state(node); + uint32_t io_misc_info; + uint64_t rax; + if (ops->get_reg(node, RAX, &rax)) + continue; /* ??? */ + /* Check AX against the requested command */ + if ((rax & 0xff) != cmd) + continue; + if (ops->get_io_misc_info(node, &io_misc_info)) { - io_misc_info = save_state_ops->get_io_misc_info(state); + /* Check for Synchronous IO (bit0 == 1) */ + if (!(io_misc_info & (1 << 0))) + continue; - /* Check for Synchronous IO (bit0==1) */ - if (!(io_misc_info & (1 << 0))) - continue; - /* Make sure it was a write (bit4==0) */ - if (io_misc_info & (1 << 4)) - continue; - /* Check for APMC IO port */ - if (((io_misc_info >> 16) & 0xff) != APM_CNT) - continue; - /* Check AL against the requested command */ - reg_al = save_state_ops->get_reg(state, RAX); - if (reg_al != cmd) - continue; - break; + /* Make sure it was a write (bit4 == 0) */ + if (io_misc_info & (1 << 4)) + continue; + + /* Check for APMC IO port */ + if (((io_misc_info >> 16) & 0xff) != APM_CNT) + continue; + } + + return node; } - return state; + + return -1; } /* Inherited from cpu/x86/smm.h resulting in a different signature */ @@ -178,8 +184,7 @@ } -void smihandler_southbridge_sleep( - const struct smm_save_state_ops *save_state_ops) +void smihandler_southbridge_sleep(void) { uint32_t reg32; uint8_t slp_typ; @@ -274,47 +279,56 @@ } } -static void southbridge_smi_gsmi( - const struct smm_save_state_ops *save_state_ops) +static void southbridge_smi_gsmi(void) { - u8 sub_command, ret; - void *io_smi = NULL; - uint32_t reg_ebx; + u32 ret, param; + u8 sub_command; + int node = smi_apmc_find_state_save_node(APM_CNT_ELOG_GSMI); + const struct smm_save_state_ops *ops = get_save_state_ops(); - io_smi = find_save_state(save_state_ops, APM_CNT_ELOG_GSMI); - if (!io_smi) + if (node < -1) return; + /* Command and return value in EAX */ - sub_command = (save_state_ops->get_reg(io_smi, RAX) >> 8) - & 0xff; + uint64_t reg; + if (ops->get_reg(node, RAX, &reg)) + return; + sub_command = (u8)(reg >> 8); /* Parameter buffer in EBX */ - reg_ebx = save_state_ops->get_reg(io_smi, RBX); + if (ops->get_reg(node, RBX, &reg)) + return; + param = reg & UINT32_MAX; /* drivers/elog/gsmi.c */ - ret = gsmi_exec(sub_command, &reg_ebx); - save_state_ops->set_reg(io_smi, RAX, ret); + ret = gsmi_exec(sub_command, (u32 *)param); + ops->set_reg(node, RAX, reg); } -static void southbridge_smi_store( - const struct smm_save_state_ops *save_state_ops) +static void southbridge_smi_store(void) { u8 sub_command, ret; - void *io_smi; - uint32_t reg_ebx; + void *param; + int node = smi_apmc_find_state_save_node(APM_CNT_ELOG_GSMI); + const struct smm_save_state_ops *ops = get_save_state_ops(); - io_smi = find_save_state(save_state_ops, APM_CNT_SMMSTORE); - if (!io_smi) + if (node < -1) return; + /* Command and return value in EAX */ - sub_command = (save_state_ops->get_reg(io_smi, RAX) >> 8) & 0xff; + uint64_t reg; + if (ops->get_reg(node, RAX, &reg)) + return; + sub_command = (u8)(reg >> 8); /* Parameter buffer in EBX */ - reg_ebx = save_state_ops->get_reg(io_smi, RBX); + if (ops->get_reg(node, RBX, &reg)) + return; + param = (void *)(uint32_t)reg; /* drivers/smmstore/smi.c */ - ret = smmstore_exec(sub_command, (void *)reg_ebx); - save_state_ops->set_reg(io_smi, RAX, ret); + ret = smmstore_exec(sub_command, param); + ops->set_reg(node, RAX, ret); } static void finalize(void) @@ -335,11 +349,10 @@ smihandler_soc_at_finalize(); } -void smihandler_southbridge_apmc( - const struct smm_save_state_ops *save_state_ops) +void smihandler_southbridge_apmc(void) { uint8_t reg8; - void *state = NULL; + const struct smm_save_state_ops *ops = get_save_state_ops(); static int smm_initialized = 0; /* Emulate B2 register as the FADT / Linux expects it */ @@ -376,22 +389,26 @@ "SMI#: SMM structures already initialized!\n"); return; } - state = find_save_state(save_state_ops, reg8); - if (state) { - /* EBX in the state save contains the GNVS pointer */ - uint32_t reg_ebx = save_state_ops->get_reg(state, RBX); - gnvs = (struct global_nvs_t *)(uintptr_t)reg_ebx; - smm_initialized = 1; - printk(BIOS_DEBUG, "SMI#: Setting GNVS to %p\n", gnvs); - } + int node = smi_apmc_find_state_save_node(APM_CNT_GNVS_UPDATE); + if (node < -1) + return; + + uint64_t rbx; + if (ops->get_reg(node, RBX, &rbx)) + return; + + /* EBX in the state save contains the GNVS pointer */ + gnvs = (global_nvs_t *)(u32)rbx; + smm_initialized = 1; + printk(BIOS_DEBUG, "SMI#: Setting GNVS to %p\n", gnvs); break; case APM_CNT_ELOG_GSMI: if (CONFIG(ELOG_GSMI)) - southbridge_smi_gsmi(save_state_ops); + southbridge_smi_gsmi(); break; case APM_CNT_SMMSTORE: if (CONFIG(SMMSTORE)) - southbridge_smi_store(save_state_ops); + southbridge_smi_store(); break; case APM_CNT_FINALIZE: finalize(); @@ -401,8 +418,7 @@ mainboard_smi_apmc(reg8); } -void smihandler_southbridge_pm1( - const struct smm_save_state_ops *save_state_ops) +void smihandler_southbridge_pm1(void) { uint16_t pm1_sts = pmc_clear_pm1_status(); u16 pm1_en = pmc_read_pm1_enable(); @@ -419,14 +435,12 @@ } } -void smihandler_southbridge_gpe0( - const struct smm_save_state_ops *save_state_ops) +void smihandler_southbridge_gpe0(void) { pmc_clear_all_gpe_status(); } -void smihandler_southbridge_tco( - const struct smm_save_state_ops *save_state_ops) +void smihandler_southbridge_tco(void) { uint32_t tco_sts = pmc_clear_tco_status(); @@ -442,8 +456,7 @@ } } -void smihandler_southbridge_periodic( - const struct smm_save_state_ops *save_state_ops) +void smihandler_southbridge_periodic(void) { uint32_t reg32; @@ -455,8 +468,7 @@ printk(BIOS_DEBUG, "Periodic SMI.\n"); } -void smihandler_southbridge_gpi( - const struct smm_save_state_ops *save_state_ops) +void smihandler_southbridge_gpi(void) { struct gpi_status smi_sts; @@ -467,8 +479,7 @@ gpi_clear_get_smi_status(&smi_sts); } -void smihandler_southbridge_espi( - const struct smm_save_state_ops *save_state_ops) +void smihandler_southbridge_espi(void) { mainboard_smi_espi_handler(); } @@ -477,7 +488,6 @@ { int i; uint32_t smi_sts; - const struct smm_save_state_ops *save_state_ops; /* * We need to clear the SMI status registers, or we won't see what's @@ -496,15 +506,13 @@ if (!smi_sts) return; - save_state_ops = get_smm_save_state_ops(); - /* Call SMI sub handler for each of the status bits */ for (i = 0; i < ARRAY_SIZE(southbridge_smi); i++) { if (!(smi_sts & (1 << i))) continue; if (southbridge_smi[i] != NULL) { - southbridge_smi[i](save_state_ops); + southbridge_smi[i](); } else { printk(BIOS_DEBUG, "SMI_STS[%d] occurred, but no " @@ -512,119 +520,3 @@ } } } - -static uint32_t em64t100_smm_save_state_get_io_misc_info(void *state) -{ - em64t100_smm_state_save_area_t *smm_state = state; - return smm_state->io_misc_info; -} - -static uint64_t em64t100_smm_save_state_get_reg(void *state, enum smm_reg reg) -{ - uintptr_t value = 0; - em64t100_smm_state_save_area_t *smm_state = state; - - switch (reg) { - case RAX: - value = smm_state->rax; - break; - case RBX: - value = smm_state->rbx; - break; - case RCX: - value = smm_state->rcx; - break; - case RDX: - value = smm_state->rdx; - break; - default: - break; - } - return value; -} - -static void em64t100_smm_save_state_set_reg(void *state, enum smm_reg reg, - uint64_t val) -{ - em64t100_smm_state_save_area_t *smm_state = state; - switch (reg) { - case RAX: - smm_state->rax = val; - break; - case RBX: - smm_state->rbx = val; - break; - case RCX: - smm_state->rcx = val; - break; - case RDX: - smm_state->rdx = val; - break; - default: - break; - } -} - -static uint32_t em64t101_smm_save_state_get_io_misc_info(void *state) -{ - em64t101_smm_state_save_area_t *smm_state = state; - return smm_state->io_misc_info; -} - -static uint64_t em64t101_smm_save_state_get_reg(void *state, enum smm_reg reg) -{ - uintptr_t value = 0; - em64t101_smm_state_save_area_t *smm_state = state; - - switch (reg) { - case RAX: - value = smm_state->rax; - break; - case RBX: - value = smm_state->rbx; - break; - case RCX: - value = smm_state->rcx; - break; - case RDX: - value = smm_state->rdx; - break; - default: - break; - } - return value; -} - -static void em64t101_smm_save_state_set_reg(void *state, enum smm_reg reg, - uint64_t val) -{ - em64t101_smm_state_save_area_t *smm_state = state; - switch (reg) { - case RAX: - smm_state->rax = val; - break; - case RBX: - smm_state->rbx = val; - break; - case RCX: - smm_state->rcx = val; - break; - case RDX: - smm_state->rdx = val; - break; - default: - break; - } -} - -const struct smm_save_state_ops em64t100_smm_ops = { - .get_io_misc_info = em64t100_smm_save_state_get_io_misc_info, - .get_reg = em64t100_smm_save_state_get_reg, - .set_reg = em64t100_smm_save_state_set_reg, -}; - -const struct smm_save_state_ops em64t101_smm_ops = { - .get_io_misc_info = em64t101_smm_save_state_get_io_misc_info, - .get_reg = em64t101_smm_save_state_get_reg, - .set_reg = em64t101_smm_save_state_set_reg, -}; diff --git a/src/soc/intel/common/block/smm/smitraphandler.c b/src/soc/intel/common/block/smm/smitraphandler.c index 974c489..3652cde 100644 --- a/src/soc/intel/common/block/smm/smitraphandler.c +++ b/src/soc/intel/common/block/smm/smitraphandler.c @@ -55,8 +55,7 @@ return 0; } -void smihandler_southbridge_mc( - const struct smm_save_state_ops *save_state_ops) +void smihandler_southbridge_mc(void) { u32 reg32 = inl(ACPI_BASE_ADDRESS + SMI_EN); @@ -67,8 +66,7 @@ printk(BIOS_DEBUG, "Microcontroller SMI.\n"); } -void smihandler_southbridge_monitor( - const struct smm_save_state_ops *save_state_ops) +void smihandler_southbridge_monitor(void) { #define IOTRAP(x) (trap_sts & (1 << x)) u32 trap_cycle; diff --git a/src/soc/intel/icelake/smihandler.c b/src/soc/intel/icelake/smihandler.c index b7c37d4..23e0834 100644 --- a/src/soc/intel/icelake/smihandler.c +++ b/src/soc/intel/icelake/smihandler.c @@ -29,11 +29,6 @@ #define CSME0_BAR 0x0 #define CSME0_FID 0xb0 -const struct smm_save_state_ops *get_smm_save_state_ops(void) -{ - return &em64t101_smm_ops; -} - static void pch_disable_heci(void) { struct pcr_sbi_msg msg = { diff --git a/src/soc/intel/skylake/smihandler.c b/src/soc/intel/skylake/smihandler.c index 2e93075..5d01bae 100644 --- a/src/soc/intel/skylake/smihandler.c +++ b/src/soc/intel/skylake/smihandler.c @@ -20,11 +20,6 @@ #include <intelblocks/smihandler.h> #include <soc/pm.h> -const struct smm_save_state_ops *get_smm_save_state_ops(void) -{ - return &em64t101_smm_ops; -} - void smihandler_soc_check_illegal_access(uint32_t tco_sts) { if (!((tco_sts & (1 << 8)) && CONFIG(SPI_FLASH_SMM) diff --git a/src/soc/intel/tigerlake/smihandler.c b/src/soc/intel/tigerlake/smihandler.c index bf07bea..cbc37f8 100644 --- a/src/soc/intel/tigerlake/smihandler.c +++ b/src/soc/intel/tigerlake/smihandler.c @@ -29,11 +29,6 @@ #define CSME0_BAR 0x0 #define CSME0_FID 0xb0 -const struct smm_save_state_ops *get_smm_save_state_ops(void) -{ - return &em64t101_smm_ops; -} - static void pch_disable_heci(void) { struct pcr_sbi_msg msg = { -- To view, visit https://review.coreboot.org/c/coreboot/+/37015 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I656aad823a96eafb507e8b30d2177d21efcf2416 Gerrit-Change-Number: 37015 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

1 8

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit February 2020