coreboot-gerrit November 2023

coreboot-gerrit@coreboot.org

1 participants
3037 discussions

[S] Change in coreboot[main]: WIP:soc/amd/mendocino: Add DBG2
by Felix Held (Code Review) 22 Nov '23

22 Nov '23

Attention is currently required from: Bao Zheng, Cliff Huang, Fred Reitberger, Jason Glenesk, Lance Zhao, Matt DeVillier, Tim Wawrzynczak, Zheng Bao. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79174?usp=email ) Change subject: WIP:soc/amd/mendocino: Add DBG2 ...................................................................... Patch Set 2: (4 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/79174/comment/8d5f5cd8_1d18e93e : PS2, Line 9: 240426142 bug id doesn't seem to be related to the patch File src/acpi/acpi.c: https://review.coreboot.org/c/coreboot/+/79174/comment/53e1f451_ec7a00a8 : PS2, Line 899: 16550 to me this name is a bit misleading; i'd expect io-space and byte size access from the name. i'd use acpi_16550_mmio32_write_dbg2_uart as function name instead which makes it clearer what type of UART this is for File src/soc/amd/mendocino/agesa_acpi.c: https://review.coreboot.org/c/coreboot/+/79174/comment/85312243_1ab43581 : PS2, Line 30: Add table DBG2? https://review.coreboot.org/c/coreboot/+/79174/comment/3cae31b4_15b6f89d : PS2, Line 31: current = acpi_16550_write_dbg2_uart(rsdp, current, 0xfedc9000, NULL); shouldn't this call be put into an if CONFIG(AMD_SOC_CONSOLE_UART) block? i agree with Raul that the base address shouldn't be fixed here and the one from the debug uart selected in coreboot should be used -- To view, visit https://review.coreboot.org/c/coreboot/+/79174?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I3c97a78d1889549421baf0bc1a2e8f959a0f47e2 Gerrit-Change-Number: 79174 Gerrit-PatchSet: 2 Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: Zheng Bao Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Zheng Bao Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Comment-Date: Tue, 21 Nov 2023 23:37:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: [UNTESTED] acpi/acpi: add & use ACPI_DBG2_PORT_SERIAL_16550_GENERIC
by Felix Held (Code Review) 22 Nov '23

22 Nov '23

Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/79219?usp=email ) Change subject: [UNTESTED] acpi/acpi: add & use ACPI_DBG2_PORT_SERIAL_16550_GENERIC ...................................................................... [UNTESTED] acpi/acpi: add & use ACPI_DBG2_PORT_SERIAL_16550_GENERIC The Microsoft Debug Port Table 2 (DBG2) specification says that the serial port subtype 0x00 (in coreboot ACPI_DBG2_PORT_SERIAL_16550) should only be used for I/O-mapped 16550 compatible UARTs. The subtype 0x12 is a superset of that, and supports specifying MMIO vs IO and the register access size via the generic address structure. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: I816bb22e6f76e661c8b8e39a2a4cb83b0085acb5 --- M src/acpi/acpi.c M src/include/acpi/acpi.h 2 files changed, 2 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/79219/1 diff --git a/src/acpi/acpi.c b/src/acpi/acpi.c index 17cd24c..6070685 100644 --- a/src/acpi/acpi.c +++ b/src/acpi/acpi.c @@ -834,7 +834,7 @@ int subtype; /* 16550-compatible with parameters defined in Generic Address Structure */ if (CONFIG(DRIVERS_UART_8250IO) || CONFIG(DRIVERS_UART_8250MEM)) - subtype = ACPI_DBG2_PORT_SERIAL_16550; + subtype = ACPI_DBG2_PORT_SERIAL_16550_GENERIC; else if (CONFIG(DRIVERS_UART_PL011)) subtype = ACPI_DBG2_PORT_SERIAL_ARM_PL011; else diff --git a/src/include/acpi/acpi.h b/src/include/acpi/acpi.h index 9a9a66d..55804fc 100644 --- a/src/include/acpi/acpi.h +++ b/src/include/acpi/acpi.h @@ -872,6 +872,7 @@ #define ACPI_DBG2_PORT_SERIAL_ARM_SBSA 0x000e #define ACPI_DBG2_PORT_SERIAL_ARM_DDC 0x000f #define ACPI_DBG2_PORT_SERIAL_BCM2835 0x0010 +#define ACPI_DBG2_PORT_SERIAL_16550_GENERIC 0x0012 #define ACPI_DBG2_PORT_IEEE1394 0x8001 #define ACPI_DBG2_PORT_IEEE1394_STANDARD 0x0000 #define ACPI_DBG2_PORT_USB 0x8002 -- To view, visit https://review.coreboot.org/c/coreboot/+/79219?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I816bb22e6f76e661c8b8e39a2a4cb83b0085acb5 Gerrit-Change-Number: 79219 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newchange

1 0

[M] Change in coreboot[main]: treewide: Move string.h to commonlib
by Maximilian Brune (Code Review) 22 Nov '23

22 Nov '23

Maximilian Brune has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/79224?usp=email ) Change subject: treewide: Move string.h to commonlib ...................................................................... treewide: Move string.h to commonlib This moves libpayloads string.h to commonlib/bsd/string.h and removes all function declarations that are duplicate of coreboots GPL2 licensed string.h and libpayloads string.h Signed-off-by: Maximilian Brune <maximilian.brune(a)9elements.com> Change-Id: Ifb05d70e474ad2dc9e5d4726363ce01c70699f7f --- M payloads/libpayload/include/string.h A src/commonlib/bsd/include/commonlib/bsd/string.h M src/include/string.h 3 files changed, 57 insertions(+), 52 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/79224/1 diff --git a/payloads/libpayload/include/string.h b/payloads/libpayload/include/string.h index e87dea5..f5d7384 100644 --- a/payloads/libpayload/include/string.h +++ b/payloads/libpayload/include/string.h @@ -29,43 +29,19 @@ #ifndef _STRING_H #define _STRING_H +#include <commonlib/bsd/string.h> #include <stddef.h> /** - * @defgroup memory Memory manipulation functions - * @{ - */ -void *memset(void *s, int c, size_t n); -void *memcpy(void *dst, const void *src, size_t n); -void *memmove(void *dst, const void *src, size_t n); -void *memchr(const void *s, int c, size_t n); -int memcmp(const void *s1, const void *s2, size_t len); -/** @} */ - -/** * @defgroup string String functions * @{ */ -size_t strnlen(const char *str, size_t maxlen); -size_t strlen(const char *str); -int strcmp(const char *s1, const char *s2); -int strncmp(const char *s1, const char *s2, size_t maxlen); int strcasecmp(const char *s1, const char *s2); int strncasecmp(const char *s1, const char *s2, size_t maxlen); -char *strncpy(char *d, const char *s, size_t n); -char *strcpy(char *d, const char *s); char *strncat(char *d, const char *s, size_t n); char *strcat(char *d, const char *s); -char *strchr(const char *s, int c); -char *strrchr(const char *s, int c); -char *strdup(const char *s); char *strndup(const char *s, size_t size); -char *strstr(const char *h, const char *n); char *strsep(char **stringp, const char *delim); -size_t strspn(const char *s, const char *a); -size_t strcspn(const char *s, const char *a); -char *strtok(char *str, const char *delim); -char *strtok_r(char *str, const char *delim, char **ptr); char *strerror(int errnum); /** @} */ diff --git a/src/commonlib/bsd/include/commonlib/bsd/string.h b/src/commonlib/bsd/include/commonlib/bsd/string.h new file mode 100644 index 0000000..eafa77f --- /dev/null +++ b/src/commonlib/bsd/include/commonlib/bsd/string.h @@ -0,0 +1,55 @@ +/* + * + * Copyright (C) 2008 Advanced Micro Devices, Inc. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * 3. The name of the author may not be used to endorse or promote products + * derived from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ + +#ifndef _STRING_H +#define _STRING_H + +#include <stddef.h> + +void *memset(void *s, int c, size_t n); +void *memcpy(void *dst, const void *src, size_t n); +void *memmove(void *dst, const void *src, size_t n); +void *memchr(const void *s, int c, size_t n); +int memcmp(const void *s1, const void *s2, size_t len); + +size_t strnlen(const char *str, size_t maxlen); +size_t strlen(const char *str); +int strcmp(const char *s1, const char *s2); +int strncmp(const char *s1, const char *s2, size_t maxlen); +char *strncpy(char *d, const char *s, size_t n); +char *strcpy(char *d, const char *s); +char *strchr(const char *s, int c); +char *strrchr(const char *s, int c); +char *strdup(const char *s); +char *strstr(const char *h, const char *n); +size_t strspn(const char *s, const char *a); +size_t strcspn(const char *s, const char *a); +char *strtok(char *str, const char *delim); +char *strtok_r(char *str, const char *delim, char **ptr); + +#endif diff --git a/src/include/string.h b/src/include/string.h index 92ea5e5..74ccf91 100644 --- a/src/include/string.h +++ b/src/include/string.h @@ -3,41 +3,15 @@ #ifndef STRING_H #define STRING_H +#include <commonlib/bsd/string.h> #include <stdarg.h> /* IWYU pragma: export */ #include <stddef.h> #include <stdio.h> /* IWYU pragma: export */ -void *memcpy(void *dest, const void *src, size_t n); -void *memmove(void *dest, const void *src, size_t n); -void *memset(void *s, int c, size_t n); -int memcmp(const void *s1, const void *s2, size_t n); -void *memchr(const void *s, int c, size_t n); -char *strdup(const char *s); char *strconcat(const char *s1, const char *s2); -size_t strnlen(const char *src, size_t max); -size_t strlen(const char *src); char *strchr(const char *s, int c); -char *strncpy(char *to, const char *from, int count); -char *strcpy(char *dst, const char *src); -int strcmp(const char *s1, const char *s2); -int strncmp(const char *s1, const char *s2, int maxlen); -int strspn(const char *str, const char *spn); -int strcspn(const char *str, const char *spn); -char *strstr(const char *haystack, const char *needle); -char *strtok_r(char *str, const char *delim, char **ptr); -char *strtok(char *str, const char *delim); long atol(const char *str); -/** - * Find a character in a string. - * - * @param s The string. - * @param c The character. - * @return A pointer to the last occurrence of the character in the - * string, or NULL if the character was not encountered within the string. - */ -char *strrchr(const char *s, int c); - /* * Parses an unsigned integer and moves the input pointer forward to the first * character that's not a valid digit. s and *s must not be NULL. Result -- To view, visit https://review.coreboot.org/c/coreboot/+/79224?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ifb05d70e474ad2dc9e5d4726363ce01c70699f7f Gerrit-Change-Number: 79224 Gerrit-PatchSet: 1 Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-MessageType: newchange

1 0

[M] Change in coreboot[main]: treewide: Add commonlib/bsd/stdlib.h
by Maximilian Brune (Code Review) 22 Nov '23

22 Nov '23

Attention is currently required from: Andrey Petrov, Arthur Heymans, Christian Walter, Hung-Te Lin, Jakub Czapiga, Johnny Lin, Maximilian Brune, Ronak Kanabar, Tim Chu, Yidi Lin, Yu-Ping Wu. Hello Andrey Petrov, Arthur Heymans, Christian Walter, Hung-Te Lin, Jakub Czapiga, Johnny Lin, Ronak Kanabar, Tim Chu, Yidi Lin, Yu-Ping Wu, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/77969?usp=email to look at the new patch set (#8). Change subject: treewide: Add commonlib/bsd/stdlib.h ...................................................................... treewide: Add commonlib/bsd/stdlib.h This patch moves commonlib/stdlib.h -> commonlib/bsd/stdlib.h, since all code is BSD licensed anyway. It also moves some code from libpayloads stdlib.h to commonlib/bsd/stdlib.h so that it can be shared with coreboot. This is useful for a subsequent commit that adds devicetree.c into commonlib. Signed-off-by: Maximilian Brune <maximilian.brune(a)9elements.com> Change-Id: I3a7ab0d1ddcc7ce9af121a61b4d4eafc9e563a8a --- M Makefile.inc M payloads/libpayload/include/stdlib.h R src/commonlib/bsd/include/commonlib/bsd/stdlib.h M src/commonlib/storage/bouncebuf.c M src/commonlib/storage/sdhci.c M src/drivers/intel/fsp2_0/silicon_init.c M src/include/stdlib.h M src/lib/device_tree.c M src/lib/fit.c M src/soc/intel/xeon_sp/spr/numa.c M src/soc/mediatek/common/pcie.c M src/soc/mediatek/mt8195/pcie.c 12 files changed, 15 insertions(+), 50 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/77969/8 -- To view, visit https://review.coreboot.org/c/coreboot/+/77969?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I3a7ab0d1ddcc7ce9af121a61b4d4eafc9e563a8a Gerrit-Change-Number: 77969 Gerrit-PatchSet: 8 Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-MessageType: newpatchset

1 0

[L] Change in coreboot[main]: acpi: Add PPTT support
by Nico Huber (Code Review) 21 Nov '23

21 Nov '23

Attention is currently required from: Angel Pons, Arthur Heymans, Cliff Huang, David Milosevic, Lance Zhao, Martin L Roth, Maximilian Brune, Patrick Rudolph, Tim Wawrzynczak. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78071?usp=email ) Change subject: acpi: Add PPTT support ...................................................................... Patch Set 22: (2 comments) File src/acpi/acpi_pptt.c: https://review.coreboot.org/c/coreboot/+/78071/comment/979d23c9_454403f3 : PS17, Line 131: cache_reference_t cache_refs[CONFIG_ACPI_PPTT_MAX_CACHES]; > > Looks like https://uefi. […] Well, QEMU-SBSA is a rather simple example, with a homogenuous hierarchy and caches known at compile time. I just wonder how it will look like for a more complex setup when one has to allocate `struct pptt_cache` nodes at runtime. Then it would be harder to reuse the nodes and their pointers. But I guess we can handle that if/when the time comes. File src/include/acpi/acpi.h: https://review.coreboot.org/c/coreboot/+/78071/comment/1e014ee7_d84a9707 : PS21, Line 1448: u32 size_valid : 1; : u32 n_sets_valid : 1; : u32 associativity_valid : 1; : u32 alloc_type_valid : 1; : u32 cache_type_valid : 1; : u32 write_policy_valid : 1; : u32 line_size_valid : 1; : u32 cache_id_valid : 1; : u32 reserved : 24; > Are bitfields portable? Or can they break depending on endianness? Bitfields or not, unless the table is defined to be in host endianness, all serialization of multi-byte words would break with a different endianness. Not sure about ACPI. I think we decided some time ago not to care about big-endian anymore (I disagree, but much of our code is broken in that regard anyway. If we'd ever start from scratch, I wouldn't use structs for serialization). -- To view, visit https://review.coreboot.org/c/coreboot/+/78071?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ia119e1ba15756704668116bdbc655190ec94ff10 Gerrit-Change-Number: 78071 Gerrit-PatchSet: 22 Gerrit-Owner: David Milosevic <David.Milosevic(a)9elements.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-CC: Martin L Roth <gaumless(a)gmail.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: David Milosevic <David.Milosevic(a)9elements.com> Gerrit-Comment-Date: Tue, 21 Nov 2023 22:45:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Comment-In-Reply-To: David Milosevic <David.Milosevic(a)9elements.com> Gerrit-MessageType: comment

1 0

[L] Change in coreboot[main]: [RFC] drivers/option: Add forms in cbtables
by Nico Huber (Code Review) 21 Nov '23

21 Nov '23

Attention is currently required from: Angel Pons, Benjamin Doron, Lean Sheng Tan, Martin L Roth, Maximilian Brune, Patrick Rudolph, Sean Rhodes. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74121?usp=email ) Change subject: [RFC] drivers/option: Add forms in cbtables ...................................................................... Patch Set 21: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/74121/comment/6c5cb197_578c8a18 : PS18, Line 15: The system currently lacks a way : to describe where to find option values. > Agreed. […] Technically, backends that store the name of an option as a string would not need to be mentioned per option. That is, unless we want to restrict options to certain backends. How about a new table tag per backend. When an entry with that tag is found directly below the CFR table, it means coreboot knows and queries this backend (and will continue with further entries until a value is found). When an entry is found below a specific option, it means for this option, the backend list is overwritten. For instance: Flash + CMOS (1) ---------------- ``` <cfr <flash> <option "somelongstring"> <option "somebool" <flash> <cmos start_bit=123 end_bit=123> > > ``` "somelongstring" would only be looked up in flash. "somebool" would only be looked up in CMOS if there is no entry in flash. IOW, flash could override the CMOS value for "somebool". Flash + CMOS (2) ---------------- ``` <cfr <flash> <option "somelongstring"> <option "somebool" <cmos start_bit=123 end_bit=123> > > ``` Similar, but without flash override for "somebool". Flash only ---------- ``` <cfr <flash> <option "someoption"> <option "anotheroption"> <option "athirdoption"> ... > ``` Compact form for all options in flash. -- To view, visit https://review.coreboot.org/c/coreboot/+/74121?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I304de7d26d79245a2e31a6d01f6c5643b31cb772 Gerrit-Change-Number: 74121 Gerrit-PatchSet: 21 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Daniel Maslowski <info(a)orangecms.org> Gerrit-CC: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-CC: Martin L Roth <gaumless(a)gmail.com> Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Comment-Date: Tue, 21 Nov 2023 22:23:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Martin L Roth <gaumless(a)gmail.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: src/arch/x86/Makefile.inc: Do not pass CPPFLAGS to linker
by Srinivas Hegde (Code Review) 21 Nov '23

21 Nov '23

Srinivas Hegde has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/79218?usp=email ) Change subject: src/arch/x86/Makefile.inc: Do not pass CPPFLAGS to linker ...................................................................... src/arch/x86/Makefile.inc: Do not pass CPPFLAGS to linker We seem to be erroneously passing CPPFLAGS to linker in x86 arch ramstage. These are only meant to be compiler flags and should not be passed to the linker. Change-Id: Ia3cd51be6be252aa796191cf0d2cd91d393c8878 Signed-off-by: Srinivas Hegde <srinivashegde(a)google.com> --- M src/arch/x86/Makefile.inc 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/79218/1 diff --git a/src/arch/x86/Makefile.inc b/src/arch/x86/Makefile.inc index 75b9d3d..62294a6 100644 --- a/src/arch/x86/Makefile.inc +++ b/src/arch/x86/Makefile.inc @@ -317,7 +317,7 @@ $(objcbfs)/ramstage.debug: $(objgenerated)/ramstage.o $(call src-to-obj,ramstage,$(CONFIG_MEMLAYOUT_LD_FILE)) @printf " CC $(subst $(obj)/,,$(@))\n" - $(LD_ramstage) $(CPPFLAGS) $(LDFLAGS_ramstage) -o $@ -L$(obj) $< -T $(call src-to-obj,ramstage,$(CONFIG_MEMLAYOUT_LD_FILE)) + $(LD_ramstage) $(LDFLAGS_ramstage) -o $@ -L$(obj) $< -T $(call src-to-obj,ramstage,$(CONFIG_MEMLAYOUT_LD_FILE)) $(objgenerated)/ramstage.o: $$(ramstage-objs) $(COMPILER_RT_ramstage) $$(ramstage-libs) @printf " CC $(subst $(obj)/,,$(@))\n" -- To view, visit https://review.coreboot.org/c/coreboot/+/79218?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ia3cd51be6be252aa796191cf0d2cd91d393c8878 Gerrit-Change-Number: 79218 Gerrit-PatchSet: 1 Gerrit-Owner: Srinivas Hegde <srinivashegde(a)google.com> Gerrit-MessageType: newchange

1 0

[L] Change in coreboot[main]: crossgcc: Upgrade GCC from 11.4.0 to 13.2.0
by Felix Singer (Code Review) 21 Nov '23

21 Nov '23

Attention is currently required from: Elyes Haouas, Lennart Eichhorn, Nico Huber, Patrick Georgi. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/70771?usp=email ) Change subject: crossgcc: Upgrade GCC from 11.4.0 to 13.2.0 ...................................................................... Patch Set 29: (1 comment) File util/crossgcc/buildgcc: https://review.coreboot.org/c/coreboot/+/70771/comment/72597707_d62c5ac7 : PS17, Line 1155: have_gnat > Looking at Jenkins' output, I guess CB:76489 is currently the only working solution. […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/70771?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I4f2ed4de4811abaa13528906de71eee29a8f2910 Gerrit-Change-Number: 70771 Gerrit-PatchSet: 29 Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-CC: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-CC: Patrick Georgi <patrick(a)coreboot.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Attention: Lennart Eichhorn <lennarteichhorn(a)googlemail.com> Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Comment-Date: Tue, 21 Nov 2023 21:59:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Patrick Georgi <patrick(a)coreboot.org> Comment-In-Reply-To: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: acpi.c: Fix generating pointer to cb_tables located >4G
by Nico Huber (Code Review) 21 Nov '23

21 Nov '23

Attention is currently required from: Angel Pons, Arthur Heymans, Benjamin Doron, Cliff Huang, Eric Lai, Felix Held, Lance Zhao, Maximilian Brune, Tim Wawrzynczak. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/76181?usp=email ) The change is no longer submittable: All-Comments-Resolved is unsatisfied now. Change subject: acpi.c: Fix generating pointer to cb_tables located >4G ...................................................................... Patch Set 10: (1 comment) File src/acpi/acpi.c: https://review.coreboot.org/c/coreboot/+/76181/comment/53603fd0_27d8f459 : PS10, Line 301: + `- 1` (not plus). Basically, we want to cover the case that we hit the maximum. For instance, a 256MiB range at 0xf0000000 should still fit: ``` 0xf0000000 + 0x10000000 - 1 <= 0xffffffff ``` -- To view, visit https://review.coreboot.org/c/coreboot/+/76181?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I1bc553b18d08cee502b765166227810f8e619631 Gerrit-Change-Number: 76181 Gerrit-PatchSet: 10 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Tue, 21 Nov 2023 21:49:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: acpigen.c: Add resource producer functions for mmio
by Nico Huber (Code Review) 21 Nov '23

21 Nov '23

Attention is currently required from: Arthur Heymans, Cliff Huang, Felix Held, Lance Zhao, Tim Wawrzynczak. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79217?usp=email ) Change subject: acpigen.c: Add resource producer functions for mmio ...................................................................... Patch Set 4: (4 comments) File src/acpi/acpigen.c: https://review.coreboot.org/c/coreboot/+/79217/comment/78b93e61_5eecc138 : PS3, Line 2260: _x Not sure if we need the `_x`. It only has meaning to me because I know the history of the code. https://review.coreboot.org/c/coreboot/+/79217/comment/8121157d_dd7c3c01 : PS3, Line 2261: u16 type_flags) Suggestion: Having a `gen_flags` (as second last parameter?) instead, would make it more generic. And at the same time, the caller would be easier to read. Currently one would have to look up what the true/false means, while ``` acpigen_resource_x_mmio32(mmio_base, mmio_limit, ADDR_SPACE_GENERAL_FLAG_PRODUCER, type_flags) ``` would be more explicit. https://review.coreboot.org/c/coreboot/+/79217/comment/4d2aba15_048e3874 : PS3, Line 2310: acpigen_resource_producer_mmio64(mmio_base, mmio_limit, type_flags); Could this if/else be generic too? That would save us half of the wrappers. i.e. a generic acpigen_resource_x_mmio() called by acpigen_resource_producer_mmio() and acpigen_resource_consumer_mmio(). https://review.coreboot.org/c/coreboot/+/79217/comment/e2b601b8_e34e62ba : PS3, Line 2331: Single empty line is more common and preferred, I think. -- To view, visit https://review.coreboot.org/c/coreboot/+/79217?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Id9e4adcd976e1f56ef7f502d9df16dbefce95c3e Gerrit-Change-Number: 79217 Gerrit-PatchSet: 4 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Tue, 21 Nov 2023 21:47:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2023