Attention is currently required from: Felix Singer, Jérémy Compostella, Shuo Liu, yuchi.chen(a)intel.com.
Hello Jérémy Compostella, Shuo Liu, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/83315?usp=email
to look at the new patch set (#8).
The following approvals got outdated and were removed:
Code-Review+1 by Shuo Liu, Verified+1 by build bot (Jenkins)
Change subject: soc/intel/common/intelblocks/gpio.h: Allow specifying the pad ownership
......................................................................
soc/intel/common/intelblocks/gpio.h: Allow specifying the pad ownership
Add pad_own_reg_0 to `struct pad_community`. Pad ownership indicates
whether the GPIO is owned by host or Intel Management Engine. If owned
by host, then host ownership indicates whether the GPIO is owned by ACPI
or driver.
Change-Id: I30a934fd00a7a42cb156341da1954e4e4b1231d8
Signed-off-by: Yuchi Chen <yuchi.chen(a)intel.com>
---
M src/soc/intel/common/block/include/intelblocks/gpio.h
1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/83315/8
--
To view, visit https://review.coreboot.org/c/coreboot/+/83315?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I30a934fd00a7a42cb156341da1954e4e4b1231d8
Gerrit-Change-Number: 83315
Gerrit-PatchSet: 8
Gerrit-Owner: yuchi.chen(a)intel.com
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: yuchi.chen(a)intel.com
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com>
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/83563?usp=email )
Change subject: arch/x86/Makefile.mk: Remove obsolete romcc reference
......................................................................
arch/x86/Makefile.mk: Remove obsolete romcc reference
No assembly.inc file is being generated by romcc anymore.
The -I. was only used in a single place that can use the common -Isrc
instead.
Change-Id: I57a3a6e1c2cf7cf30fb0cd94cc8455f715050490
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/83563
Reviewed-by: Elyes Haouas <ehaouas(a)noos.fr>
Reviewed-by: Angel Pons <th3fanbus(a)gmail.com>
Reviewed-by: Nico Huber <nico.h(a)gmx.de>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
---
M src/arch/x86/Makefile.mk
M src/northbridge/intel/sandybridge/mrc_wrapper.S
2 files changed, 1 insertion(+), 4 deletions(-)
Approvals:
Elyes Haouas: Looks good to me, approved
Nico Huber: Looks good to me, approved
Angel Pons: Looks good to me, but someone else must approve
build bot (Jenkins): Verified
diff --git a/src/arch/x86/Makefile.mk b/src/arch/x86/Makefile.mk
index cf5b2bb..2832ac3 100644
--- a/src/arch/x86/Makefile.mk
+++ b/src/arch/x86/Makefile.mk
@@ -58,9 +58,6 @@
# $1 stage name
# $2 oformat
-# The '.' include path is needed for the generated assembly.inc file.
-$(1)-S-ccopts += -I.
-
$$(objcbfs)/$(1).debug: $$$$($(1)-libs) $$$$($(1)-objs)
@printf " LINK $$(subst $$(obj)/,,$$(@))\n"
$$(LD_$(1)) $$(LDFLAGS_$(1)) -o $$@ -L$$(obj) $$(COMPILER_RT_FLAGS_$(1)) --whole-archive --start-group $$(filter-out %.ld,$$($(1)-objs)) $$($(1)-libs) --no-whole-archive $$(COMPILER_RT_$(1)) --end-group -T $(call src-to-obj,$(1),$(CONFIG_MEMLAYOUT_LD_FILE)) --oformat $(2)
diff --git a/src/northbridge/intel/sandybridge/mrc_wrapper.S b/src/northbridge/intel/sandybridge/mrc_wrapper.S
index d68ce09..493f808 100644
--- a/src/northbridge/intel/sandybridge/mrc_wrapper.S
+++ b/src/northbridge/intel/sandybridge/mrc_wrapper.S
@@ -32,6 +32,6 @@
* function do_putchar to print to console.
*/
-#include <src/cpu/x86/64bit/prot2long.inc>
+#include <cpu/x86/64bit/prot2long.inc>
prot2lm_wrapper do_putchar
--
To view, visit https://review.coreboot.org/c/coreboot/+/83563?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I57a3a6e1c2cf7cf30fb0cd94cc8455f715050490
Gerrit-Change-Number: 83563
Gerrit-PatchSet: 4
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Attention is currently required from: Eran Mitrani, Jakub Czapiga, Kapil Porwal, Nico Huber, Paul Menzel, Rishika Raj, Subrata Banik, Tarun.
Julius Werner has posted comments on this change by Rishika Raj. ( https://review.coreboot.org/c/coreboot/+/83540?usp=email )
Change subject: soc/intel/mtl: Increase CAR_STACK_SIZE by 31KB for coreboot compatibility
......................................................................
Patch Set 8:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/83540/comment/637703b9_ed577ab1?us… :
PS5, Line 7: soc/intel/meteorlake: Increase CAR STACK_SIZE by 31KB to meet coreboot requirements
> Thanks, that's much clearer. I'm much more used to know how much stack we need. […]
I'm not sure how much static analysis can do there because coreboot does have a bunch of code paths that are affected by runtime values (and a few recursive functions), so I'm not sure if it's really possible to reliably determine stack use at build time. If you know some solution that can do that it would be great if you could integrate it into the CI, of course. We do use `-Wstack-usage=1536` on non-x86 boards and maybe we should do something like that on x86 as well, but I believe that only checks each function on its own.
There is code in `romstage_main()` to place and check stack canaries and I assume that it should fire if we have an overflow like we're suspecting, but it just writes a BIOS_DEBUG to the console and nothing else. My suspicion is that the FSP may only get into the path that really makes it use its whole 512K in very rare and unreproducible circumstances (e.g. related to memory training), and we've never managed to capture the boot that actually fails in our case on a UART. All we are seeing is corrupted vboot data structures persisted in the TPM that look like they would have been created by coreboot trying to write back vboot state from memory that has been overwritten with a bunch of 0xff.
I am not against moving the UPD and other big things off the stack either if someone wants to do that. (I've also been thinking about doing some other things to harden against stack overflows, e.g. move the stack to the end of the CAR space and let it grow downwards into unallocated space, but I likely won't have time to finish that work any time soon unfortunately.) For now, we're just trying to do the most simple thing that should reasonably ensure we're not going to have a problem on the Meteorlake platform.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83540?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Iba3620b3b7c470176330f5e07989cd3f6238713e
Gerrit-Change-Number: 83540
Gerrit-PatchSet: 8
Gerrit-Owner: Rishika Raj <rishikaraj(a)google.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tarun <tstuli(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Rishika Raj <rishikaraj(a)google.com>
Gerrit-Attention: Eran Mitrani <mitrani(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Tarun <tstuli(a)gmail.com>
Gerrit-Comment-Date: Tue, 23 Jul 2024 23:34:46 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Attention is currently required from: Sean Rhodes.
Matt DeVillier has posted comments on this change by Sean Rhodes. ( https://review.coreboot.org/c/coreboot/+/83620?usp=email )
Change subject: mb/starlabs/starbook/rpl: Don't set tcss_aux_ori
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/83620?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ia0e90179dd05b23f1f36935be51327250c5a8684
Gerrit-Change-Number: 83620
Gerrit-PatchSet: 1
Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Comment-Date: Tue, 23 Jul 2024 20:30:42 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Sean Rhodes has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/83623?usp=email )
Change subject: mb/starlabs/starbook/rpl: Add USB ACPi to devicetree
......................................................................
mb/starlabs/starbook/rpl: Add USB ACPi to devicetree
Use the USB ACPI to add entries for the USB and TCSS ports.
Change-Id: Iab8b6e03c8c05e459fb354bc008109c873a4846f
Signed-off-by: Sean Rhodes <sean(a)starlabs.systems>
---
M src/mainboard/starlabs/starbook/variants/rpl/devicetree.cb
1 file changed, 61 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/83623/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/83623?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Iab8b6e03c8c05e459fb354bc008109c873a4846f
Gerrit-Change-Number: 83623
Gerrit-PatchSet: 2
Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems>
Sean Rhodes has posted comments on this change by Sean Rhodes. ( https://review.coreboot.org/c/coreboot/+/83627?usp=email )
Change subject: mb/starlabs/starbook/rpl: Set I2C0 speed to fast
......................................................................
Set Ready For Review
--
To view, visit https://review.coreboot.org/c/coreboot/+/83627?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I6fdbd3124fca27fcd4dced81d8e2aa2f91fe4651
Gerrit-Change-Number: 83627
Gerrit-PatchSet: 1
Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Comment-Date: Tue, 23 Jul 2024 20:15:26 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Sean Rhodes has posted comments on this change by Sean Rhodes. ( https://review.coreboot.org/c/coreboot/+/83626?usp=email )
Change subject: mb/starlabs/starbook/rpl: Nit GPIO changes
......................................................................
Set Ready For Review
--
To view, visit https://review.coreboot.org/c/coreboot/+/83626?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I5b4691a0b5e8b1348304d11c1d59aa60517041ec
Gerrit-Change-Number: 83626
Gerrit-PatchSet: 1
Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Comment-Date: Tue, 23 Jul 2024 20:06:40 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Sean Rhodes has posted comments on this change by Sean Rhodes. ( https://review.coreboot.org/c/coreboot/+/83625?usp=email )
Change subject: mb/starlabs/starbook/rpl: Disconnect wireless GPIOs
......................................................................
Set Ready For Review
--
To view, visit https://review.coreboot.org/c/coreboot/+/83625?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I7aef1b821420daf5ea9f6ae107021e5d406a5ec3
Gerrit-Change-Number: 83625
Gerrit-PatchSet: 1
Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Comment-Date: Tue, 23 Jul 2024 20:06:38 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No