coreboot-gerrit June 2023

coreboot-gerrit@coreboot.org

1 participants
5387 discussions

[S] Change in coreboot[master]: soc/intel/apollolake: Make SATA speed limit configurable
by Felix Singer (Code Review) 01 Jun '23

01 Jun '23

Attention is currently required from: Arthur Heymans, Mario Scheithauer. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75364?usp=email ) Change subject: soc/intel/apollolake: Make SATA speed limit configurable ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/75364?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9c3eda0649546e3a40eb24a015b7c6efd8f90e0f Gerrit-Change-Number: 75364 Gerrit-PatchSet: 3 Gerrit-Owner: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Jan Samek <jan.samek(a)siemens.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Thu, 01 Jun 2023 14:06:59 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: drivers/ocp/vpd: Overwrite Linux payload's kernel command via VPD
by Paul Menzel (Code Review) 01 Jun '23

01 Jun '23

Attention is currently required from: Johnny Lin. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75510?usp=email ) Change subject: drivers/ocp/vpd: Overwrite Linux payload's kernel command via VPD ...................................................................... Patch Set 6: (1 comment) File src/drivers/ocp/vpd/vpd_cmdline.c: https://review.coreboot.org/c/coreboot/+/75510/comment/b228d6fb_6d85b3e1 : PS6, Line 21: printk(BIOS_INFO, "Invalid VPD for Linux kernel log level\n"); Sorry for the confusion. I meant to print the invalid value here. Also the level should be *warning* or at least *notice*. -- To view, visit https://review.coreboot.org/c/coreboot/+/75510?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idf06c7ab9958c940fc3b23d560bb9dade991a6da Gerrit-Change-Number: 75510 Gerrit-PatchSet: 6 Gerrit-Owner: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Bryant Ou <bryant.ou.q(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jingle Hsu <jingle_hsu(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Nill Ge <geshijian(a)bytedance.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Shelly Chang <Shelly_Chang(a)wiwynn.com> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: TangYiwei Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Comment-Date: Thu, 01 Jun 2023 13:44:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: include/cpu/x86: Simplify en/dis cache functions
by Himanshu Sahdev (Code Review) 01 Jun '23

01 Jun '23

Attention is currently required from: Arthur Heymans, Felix Singer, Himanshu Sahdev, Jérémy Compostella, Lean Sheng Tan. Hello Felix Singer, Jérémy Compostella, Lean Sheng Tan, Subrata Banik, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/75552?usp=email to look at the new patch set (#3). The following approvals got outdated and were removed: Code-Review+1 by Felix Singer, Verified+1 by build bot (Jenkins) Change subject: include/cpu/x86: Simplify en/dis cache functions ...................................................................... include/cpu/x86: Simplify en/dis cache functions Implementation of enable/disable cache functions aren't complex, simply drop cr0 variable usage, still maintains good readablity. Signed-off-by: Himanshu Sahdev <himanshu.sahdev(a)intel.com> Change-Id: I81688e8bbb073e1d09ecf63f3f33e1651dbd778e --- M src/include/cpu/x86/cache.h 1 file changed, 2 insertions(+), 8 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/52/75552/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/75552?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I81688e8bbb073e1d09ecf63f3f33e1651dbd778e Gerrit-Change-Number: 75552 Gerrit-PatchSet: 3 Gerrit-Owner: Himanshu Sahdev <himanshu.sahdev(a)intel.com> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Himanshu Sahdev <himanshu.sahdev(a)intel.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in coreboot[master]: include/cpu/x86: Simplify en/dis cache functions
by Subrata Banik (Code Review) 01 Jun '23

01 Jun '23

Attention is currently required from: Arthur Heymans, Felix Singer, Himanshu Sahdev, Jérémy Compostella, Lean Sheng Tan. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75552?usp=email ) Change subject: include/cpu/x86: Simplify en/dis cache functions ...................................................................... Patch Set 2: (1 comment) File src/include/cpu/x86/cache.h: https://review.coreboot.org/c/coreboot/+/75552/comment/1083e60a_15437861 : PS2, Line 48: CR0_CacheDisable > > > want me to change it to screaming snake case -> CR0_CACHE_DISABLE? > > yup, this is what make sense. > > > > > > > > Also, these macros being in use already, better to do it in different CL to change everywhere. > > > > sure > > CR0_CD and CR0_NW is fine too IMO +1 -- To view, visit https://review.coreboot.org/c/coreboot/+/75552?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I81688e8bbb073e1d09ecf63f3f33e1651dbd778e Gerrit-Change-Number: 75552 Gerrit-PatchSet: 2 Gerrit-Owner: Himanshu Sahdev <himanshu.sahdev(a)intel.com> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Himanshu Sahdev <himanshu.sahdev(a)intel.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Comment-Date: Thu, 01 Jun 2023 13:32:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Himanshu Sahdev <himanshu.sahdev(a)intel.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[master]: soc/intel/common/crashlog: Check cbmem pointer before copying records
by Felix Held (Code Review) 01 Jun '23

01 Jun '23

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/75528?usp=email ) Change subject: soc/intel/common/crashlog: Check cbmem pointer before copying records ...................................................................... soc/intel/common/crashlog: Check cbmem pointer before copying records Check existence of crashlog records in CBMEM before copying them to BERT, otherwise it can lead to NULL pointer access. Bug=None TEST=Able to build. With Meteor Lake SOC related patch, able to capture and decode crashlog. Change-Id: I4288011866283a3a5fb8ec9e10cd51b794052b4e Signed-off-by: Pratikkumar Prajapati <pratikkumar.v.prajapati(a)intel.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/75528 Reviewed-by: Subrata Banik <subratabanik(a)google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/soc/intel/common/block/crashlog/crashlog.c 1 file changed, 8 insertions(+), 0 deletions(-) Approvals: Subrata Banik: Looks good to me, approved build bot (Jenkins): Verified diff --git a/src/soc/intel/common/block/crashlog/crashlog.c b/src/soc/intel/common/block/crashlog/crashlog.c index aa2a240..4b72599 100644 --- a/src/soc/intel/common/block/crashlog/crashlog.c +++ b/src/soc/intel/common/block/crashlog/crashlog.c @@ -488,6 +488,10 @@ printk(BIOS_DEBUG, "CPU crash data collection.\n"); cl_src_addr = cbmem_find(CBMEM_ID_CPU_CRASHLOG); + if (!cl_src_addr) { + printk(BIOS_DEBUG, "CPU crash data, CBMEM not found\n"); + return false; + } memcpy(cl_record, cl_src_addr, m_cpu_crashLog_size); return true; @@ -506,6 +510,10 @@ printk(BIOS_DEBUG, "PMC crash data collection.\n"); cl_src_addr = cbmem_find(CBMEM_ID_PMC_CRASHLOG); + if (!cl_src_addr) { + printk(BIOS_DEBUG, "PMC crash data, CBMEM not found\n"); + return false; + } memcpy(cl_record, cl_src_addr, m_pmc_crashLog_size); return true; -- To view, visit https://review.coreboot.org/c/coreboot/+/75528?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4288011866283a3a5fb8ec9e10cd51b794052b4e Gerrit-Change-Number: 75528 Gerrit-PatchSet: 3 Gerrit-Owner: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[master]: soc/intel/common/crashlog: Check for invalid record
by Felix Held (Code Review) 01 Jun '23

01 Jun '23

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/75527?usp=email ) Change subject: soc/intel/common/crashlog: Check for invalid record ...................................................................... soc/intel/common/crashlog: Check for invalid record Do not copy the crashlog record if the record is 0xdeadbeef Bug=None TEST=Able to build. With Meteor Lake SOC related patch, able to capture and decode crashlog. Change-Id: I0edbf6902685a882876d525e63c5b602c1590ea1 Signed-off-by: Pratikkumar Prajapati <pratikkumar.v.prajapati(a)intel.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/75527 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Subrata Banik <subratabanik(a)google.com> --- M src/soc/intel/common/block/crashlog/crashlog.c M src/soc/intel/common/block/include/intelblocks/crashlog.h 2 files changed, 10 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Subrata Banik: Looks good to me, approved diff --git a/src/soc/intel/common/block/crashlog/crashlog.c b/src/soc/intel/common/block/crashlog/crashlog.c index daa5b76..aa2a240 100644 --- a/src/soc/intel/common/block/crashlog/crashlog.c +++ b/src/soc/intel/common/block/crashlog/crashlog.c @@ -271,6 +271,14 @@ u32 src_addr = src_bar + offset; u32 data = read32((u32 *)src_addr); + + /* First 32bits of the record must not be 0xdeadbeef */ + if (data == INVALID_CRASHLOG_RECORD) { + printk(BIOS_DEBUG, "Invalid data 0x%x at offset 0x%x from addr 0x%x\n", + data, offset, src_bar); + return false; + } + /* PMC: copy if 1st DWORD in buffer is not zero and its 31st bit is not set */ if (pmc_sram && !(data && !(data & BIT(31)))) { printk(BIOS_DEBUG, "Invalid data 0x%x at offset 0x%x from addr 0x%x" diff --git a/src/soc/intel/common/block/include/intelblocks/crashlog.h b/src/soc/intel/common/block/include/intelblocks/crashlog.h index 07dd5a2..cf9ddad 100644 --- a/src/soc/intel/common/block/include/intelblocks/crashlog.h +++ b/src/soc/intel/common/block/include/intelblocks/crashlog.h @@ -23,6 +23,8 @@ #define CRASHLOG_SIZE_DEBUG_PURPOSE 0x640 +#define INVALID_CRASHLOG_RECORD 0xdeadbeef + /* PMC crashlog discovery structs */ typedef union { struct { -- To view, visit https://review.coreboot.org/c/coreboot/+/75527?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0edbf6902685a882876d525e63c5b602c1590ea1 Gerrit-Change-Number: 75527 Gerrit-PatchSet: 3 Gerrit-Owner: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[XS] Change in coreboot[master]: soc/intel/common/crashlog: Fix checking PMC record size
by Felix Held (Code Review) 01 Jun '23

01 Jun '23

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/75526?usp=email ) Change subject: soc/intel/common/crashlog: Fix checking PMC record size ...................................................................... soc/intel/common/crashlog: Fix checking PMC record size Check pmc_record_size variable for collecting PMC records, instead of cpu_record_size variable. Bug=None TEST=Able to build. With Meteor Lake SOC related patch, able to capture and decode crashlog. Change-Id: I4c35ba2bcf757231aa2872802eb82d4d50742cd9 Signed-off-by: Pratikkumar Prajapati <pratikkumar.v.prajapati(a)intel.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/75526 Reviewed-by: Subrata Banik <subratabanik(a)google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/soc/intel/common/block/acpi/acpi_bert.c 1 file changed, 2 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Subrata Banik: Looks good to me, approved diff --git a/src/soc/intel/common/block/acpi/acpi_bert.c b/src/soc/intel/common/block/acpi/acpi_bert.c index 7743ccc..16df60a 100644 --- a/src/soc/intel/common/block/acpi/acpi_bert.c +++ b/src/soc/intel/common/block/acpi/acpi_bert.c @@ -68,8 +68,8 @@ pmc_record_size = cl_get_pmc_record_size(); if (pmc_record_size) { - /* Allocate new FW ERR structure in case CPU crashlog is present */ - if (cpu_record_size && !bert_append_fw_err(status)) { + /* Allocate new FW ERR structure in case PMC crashlog is present */ + if (pmc_record_size && !bert_append_fw_err(status)) { printk(BIOS_ERR, "Crashlog PMC entry would " "exceed available region\n"); return CB_ERR; -- To view, visit https://review.coreboot.org/c/coreboot/+/75526?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4c35ba2bcf757231aa2872802eb82d4d50742cd9 Gerrit-Change-Number: 75526 Gerrit-PatchSet: 3 Gerrit-Owner: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[master]: mb/google/guybrush: Move helper AOAC for console to AOAC header
by Felix Held (Code Review) 01 Jun '23

01 Jun '23

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/75491?usp=email ) Change subject: mb/google/guybrush: Move helper AOAC for console to AOAC header ...................................................................... mb/google/guybrush: Move helper AOAC for console to AOAC header BUG=b:217968734 TEST=Build guybrush firmware Change-Id: I93dfa50cd1116e0f6652186acb37fd43d638cf84 Signed-off-by: Konrad Adamczyk <konrada(a)google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/75491 Reviewed-by: Raul Rangel <rrangel(a)chromium.org> Reviewed-by: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/soc/amd/cezanne/aoac.c M src/soc/amd/cezanne/include/soc/aoac_defs.h 2 files changed, 6 insertions(+), 4 deletions(-) Approvals: Matt DeVillier: Looks good to me, approved Raul Rangel: Looks good to me, approved build bot (Jenkins): Verified diff --git a/src/soc/amd/cezanne/aoac.c b/src/soc/amd/cezanne/aoac.c index 14758e0..e9a7496 100644 --- a/src/soc/amd/cezanne/aoac.c +++ b/src/soc/amd/cezanne/aoac.c @@ -7,10 +7,6 @@ #include <soc/southbridge.h> #include <delay.h> -#define FCH_AOAC_UART_FOR_CONSOLE \ - (CONFIG_UART_FOR_CONSOLE == 0 ? FCH_AOAC_DEV_UART0 \ - : CONFIG_UART_FOR_CONSOLE == 1 ? FCH_AOAC_DEV_UART1 \ - : -1) #if CONFIG(AMD_SOC_CONSOLE_UART) && FCH_AOAC_UART_FOR_CONSOLE == -1 # error Unsupported UART_FOR_CONSOLE chosen #endif diff --git a/src/soc/amd/cezanne/include/soc/aoac_defs.h b/src/soc/amd/cezanne/include/soc/aoac_defs.h index 25311dd..58c67d5 100644 --- a/src/soc/amd/cezanne/include/soc/aoac_defs.h +++ b/src/soc/amd/cezanne/include/soc/aoac_defs.h @@ -17,4 +17,10 @@ #define FCH_AOAC_DEV_ESPI 27 #define FCH_AOAC_DEV_EMMC 28 +/* FCH helper define to determine UART_FOR_CONSOLE */ +#define FCH_AOAC_UART_FOR_CONSOLE \ + (CONFIG_UART_FOR_CONSOLE == 0 ? FCH_AOAC_DEV_UART0 \ + : CONFIG_UART_FOR_CONSOLE == 1 ? FCH_AOAC_DEV_UART1 \ + : -1) + #endif /* AMD_CEZANNE_AOAC_DEFS_H */ -- To view, visit https://review.coreboot.org/c/coreboot/+/75491?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I93dfa50cd1116e0f6652186acb37fd43d638cf84 Gerrit-Change-Number: 75491 Gerrit-PatchSet: 2 Gerrit-Owner: Konrad Adamczyk <konrada(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Grzegorz Bernacki Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[M] Change in coreboot[master]: drivers/ocp/vpd: Overwrite Linux payload's kernel command via VPD
by Felix Held (Code Review) 01 Jun '23

01 Jun '23

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/75510?usp=email ) Change subject: drivers/ocp/vpd: Overwrite Linux payload's kernel command via VPD ...................................................................... drivers/ocp/vpd: Overwrite Linux payload's kernel command via VPD Add a new Kconfig LINUXPAYLOAD_CMDLINE_VPD_OVERWRITE that can overwrite Linux payload's kernel command line from VPD. Currently only overwrite Linux kernel command line 'loglevel' via VPD key 'kernel_log_level'. TESTED=On OCP Delta Lake, with kernel_log_level set to 0, warm reboot time can see about 10 seconds improvement comparing to kernel log level 7. Change-Id: Idf06c7ab9958c940fc3b23d560bb9dade991a6da Signed-off-by: Johnny Lin <johnny_lin(a)wiwynn.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/75510 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: David Hendricks <david.hendricks(a)gmail.com> --- M src/drivers/ocp/include/vpd.h M src/drivers/ocp/vpd/Kconfig M src/drivers/ocp/vpd/Makefile.inc A src/drivers/ocp/vpd/vpd_cmdline.c 4 files changed, 69 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified David Hendricks: Looks good to me, approved diff --git a/src/drivers/ocp/include/vpd.h b/src/drivers/ocp/include/vpd.h index 33fece5..60299e3 100644 --- a/src/drivers/ocp/include/vpd.h +++ b/src/drivers/ocp/include/vpd.h @@ -77,6 +77,9 @@ /* Socket 1 core disable bitmask */ #define CORE_DIS_BITMSK1 "core_disable_bitmask1" +/* Linux payload kernel log level */ +#define KERNEL_LOG_LEVEL "kernel_log_level" + /* Get VPD key with provided fallback value and min/max ranges */ int get_int_from_vpd_range(const char *const key, const int fallback, const int min, const int max); diff --git a/src/drivers/ocp/vpd/Kconfig b/src/drivers/ocp/vpd/Kconfig index ebecbe6..12ee185 100644 --- a/src/drivers/ocp/vpd/Kconfig +++ b/src/drivers/ocp/vpd/Kconfig @@ -4,3 +4,12 @@ depends on VPD help It implements functions that get common VPD variables for OCP projects. + +config LINUXPAYLOAD_CMDLINE_VPD_OVERWRITE + bool + default n + depends on VPD + help + Overwrite Linux payload's kernel command line by using VPD. Currently only + overwrite the value of kernel command line 'loglevel'. The Linux kernel command + line data is detected in the last segment loaded in memory and overwritten. diff --git a/src/drivers/ocp/vpd/Makefile.inc b/src/drivers/ocp/vpd/Makefile.inc index 8db5afa..b40534a 100644 --- a/src/drivers/ocp/vpd/Makefile.inc +++ b/src/drivers/ocp/vpd/Makefile.inc @@ -1,5 +1,6 @@ romstage-$(CONFIG_OCP_VPD) += vpd_util.c ramstage-$(CONFIG_OCP_VPD) += vpd_util.c +ramstage-$(CONFIG_LINUXPAYLOAD_CMDLINE_VPD_OVERWRITE) += vpd_cmdline.c ifeq ($(CONFIG_VPD),y) all-$(CONFIG_CONSOLE_OVERRIDE_LOGLEVEL) += loglevel_vpd.c endif diff --git a/src/drivers/ocp/vpd/vpd_cmdline.c b/src/drivers/ocp/vpd/vpd_cmdline.c new file mode 100644 index 0000000..44e166c --- /dev/null +++ b/src/drivers/ocp/vpd/vpd_cmdline.c @@ -0,0 +1,56 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <console/console.h> +#include <drivers/vpd/vpd.h> +#include <drivers/ocp/include/vpd.h> +#include <string.h> +#include <program_loading.h> + +#define CMDLINE_LOGLVL_STR "loglevel=" + +static void overwrite_kernel_loglevel(uintptr_t start) +{ + int val; + if (!vpd_get_int(KERNEL_LOG_LEVEL, VPD_RW_THEN_RO, &val)) { + printk(BIOS_DEBUG, "%s: not able to get VPD %s\n", __func__, KERNEL_LOG_LEVEL); + return; + } + + printk(BIOS_DEBUG, "%s: VPD %s, got %d\n", __func__, KERNEL_LOG_LEVEL, val); + if (val < 0 || val > 7) { + printk(BIOS_INFO, "Invalid VPD for Linux kernel log level\n"); + return; + } + + int loglevel; + char *loc = strstr((const char *)start, CMDLINE_LOGLVL_STR); + if (!loc) { + printk(BIOS_INFO, "%s is not found from LINUX_COMMAND_LINE\n", + CMDLINE_LOGLVL_STR); + return; + } + + char *loc_bkup; + loc += strlen(CMDLINE_LOGLVL_STR); + loc_bkup = loc; + loglevel = skip_atoi(&loc); + printk(BIOS_DEBUG, "Original kernel log level is %d\n", loglevel); + /* Unlikely but don't overwrite with such an unexpected case. */ + if (loglevel < 0 || loglevel > 7) { + printk(BIOS_DEBUG, "Invalid kernel log level, must be from 0 to 7.\n"); + return; + } + + char c = '0' + val; + printk(BIOS_INFO, "Overwrite kernel log level with %c from VPD.\n", c); + memcpy(loc_bkup, &c, 1); +} + +void platform_segment_loaded(uintptr_t start, size_t size, int flags) +{ + /* CONFIG_LINUX_COMMAND_LINE is in the final segment. */ + if (flags != SEG_FINAL) + return; + + overwrite_kernel_loglevel(start); +} -- To view, visit https://review.coreboot.org/c/coreboot/+/75510?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idf06c7ab9958c940fc3b23d560bb9dade991a6da Gerrit-Change-Number: 75510 Gerrit-PatchSet: 6 Gerrit-Owner: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Bryant Ou <bryant.ou.q(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jingle Hsu <jingle_hsu(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Nill Ge <geshijian(a)bytedance.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Shelly Chang <Shelly_Chang(a)wiwynn.com> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: TangYiwei Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[master]: device/pci: Limit default domain memory window
by Felix Held (Code Review) 01 Jun '23

01 Jun '23

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/75102?usp=email ) ( 3 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: device/pci: Limit default domain memory window ...................................................................... device/pci: Limit default domain memory window When the default pci_domain_read_resources() is used, keep 32-bit memory resources below the limit given by CONFIG_DOMAIN_RESOURCE_32BIT_LIMIT. This serves as a workaround for missing/wrong reservations of chipset resources. This will help to get more stable results from our own allocator, but is far from a complete solution. Indvi- dual platform ASL code also needs to be considered, so the OS won't assign conflicting resources. Most platforms have reserved space between 0xfe000000 and the 4G barrier. So use that as a global default. In case of `soc/intel/common/`, use 0xe0000000 because this is what is advertised in ACPI and there are traces of resources below 0xfe000000 that are unknown to core- boot's C code (PCH_PRESERVED_BASE?). Tested on QEMU/Q35 and Siemens/Chili w/ and w/o top- down allocation. Fixes EHCI w/ top-down in QEMU. Change-Id: Iae0d888eebd0ec11a9d6f12975ae24dc32a80d8c Signed-off-by: Nico Huber <nico.huber(a)secunet.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/75102 Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Paul Menzel <paulepanter(a)mailbox.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/device/Kconfig M src/device/pci_device.c M src/soc/intel/common/block/systemagent/Kconfig 3 files changed, 28 insertions(+), 1 deletion(-) Approvals: Angel Pons: Looks good to me, approved build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve diff --git a/src/device/Kconfig b/src/device/Kconfig index 71292e8..9c9ecd1 100644 --- a/src/device/Kconfig +++ b/src/device/Kconfig @@ -531,6 +531,15 @@ if PCI +config DOMAIN_RESOURCE_32BIT_LIMIT + hex + default 0xfe000000 + help + When the default pci_domain_read_resources() is used, + keep 32-bit memory resources below this limit. This is + used as a workaround for missing/wrong reservations of + chipset resources that usually reside above this limit. + config NO_ECAM_MMCONF_SUPPORT bool default n diff --git a/src/device/pci_device.c b/src/device/pci_device.c index e600f34..5c5a5fb 100644 --- a/src/device/pci_device.c +++ b/src/device/pci_device.c @@ -7,6 +7,7 @@ #include <acpi/acpi.h> #include <assert.h> +#include <cbmem.h> #include <device/pci_ops.h> #include <bootmode.h> #include <console/console.h> @@ -561,8 +562,22 @@ res->flags = IORESOURCE_IO | IORESOURCE_SUBTRACTIVE | IORESOURCE_ASSIGNED; - /* Initialize the system-wide memory resources constraints. */ + /* + * Initialize 32-bit memory resource constraints. + * + * There are often undeclared chipset resources in lower memory + * and memory right below the 4G barrier. Hence, only allow + * one big range from cbmem_top to the configured limit. + */ res = new_resource(dev, IOINDEX_SUBTRACTIVE(1, 0)); + res->base = (uintptr_t)cbmem_top(); + res->limit = CONFIG_DOMAIN_RESOURCE_32BIT_LIMIT - 1; + res->flags = IORESOURCE_MEM | IORESOURCE_SUBTRACTIVE | + IORESOURCE_ASSIGNED; + + /* Initialize 64-bit memory resource constraints above 4G. */ + res = new_resource(dev, IOINDEX_SUBTRACTIVE(2, 0)); + res->base = 4ULL * GiB; res->limit = (1ULL << cpu_phys_address_size()) - 1; res->flags = IORESOURCE_MEM | IORESOURCE_SUBTRACTIVE | IORESOURCE_ASSIGNED; diff --git a/src/soc/intel/common/block/systemagent/Kconfig b/src/soc/intel/common/block/systemagent/Kconfig index d8c217f..4d14fc1 100644 --- a/src/soc/intel/common/block/systemagent/Kconfig +++ b/src/soc/intel/common/block/systemagent/Kconfig @@ -6,6 +6,9 @@ if SOC_INTEL_COMMON_BLOCK_SA +config DOMAIN_RESOURCE_32BIT_LIMIT + default 0xe0000000 + config ECAM_MMCONF_BASE_ADDRESS default 0xe0000000 -- To view, visit https://review.coreboot.org/c/coreboot/+/75102?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iae0d888eebd0ec11a9d6f12975ae24dc32a80d8c Gerrit-Change-Number: 75102 Gerrit-PatchSet: 5 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit June 2023