coreboot-gerrit February 2021

coreboot-gerrit@coreboot.org

1 participants
7192 discussions

Change in coreboot[master]: mb/emulation/qemu-q35: Mark TSEG region as reserved
by Patrick Rudolph (Code Review) 03 Feb '21

03 Feb '21

Attention is currently required from: Patrick Rudolph. Hello build bot (Jenkins), Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/50267 to look at the new patch set (#2). Change subject: mb/emulation/qemu-q35: Mark TSEG region as reserved ...................................................................... mb/emulation/qemu-q35: Mark TSEG region as reserved Mark TSEG as reserved, which is done on other platforms as well. For some reason CorebootPayloadPkg crashes when using the region where TSEG typically resides, which is basically RAM. UefiPayloadPkg doesn't show this issue. Change-Id: I3ae3659349d2a88bc3575fe9675433c054e28832 Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> --- M src/mainboard/emulation/qemu-i440fx/northbridge.c 1 file changed, 8 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/50267/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/50267 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3ae3659349d2a88bc3575fe9675433c054e28832 Gerrit-Change-Number: 50267 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/intel/broadwell: Convert some CONFIG(CHROMEOS) preprocessor
by Kyösti Mälkki (Code Review) 03 Feb '21

03 Feb '21

Attention is currently required from: Matt DeVillier, Duncan Laurie, Patrick Rudolph. Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50252 ) Change subject: soc/intel/broadwell: Convert some CONFIG(CHROMEOS) preprocessor ...................................................................... Patch Set 1: (1 comment) File src/soc/intel/broadwell/gma.c: https://review.coreboot.org/c/coreboot/+/50252/comment/b6746107_2a36c66a PS1, Line 518: !CONFIG(CHROMEOS) > might be able to just drop this and call display_init_required()? I think originally these function […] See CB:48787 unresolved comment. -- To view, visit https://review.coreboot.org/c/coreboot/+/50252 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie3feee0448175db2b6ed4e8e37d92de3af9be371 Gerrit-Change-Number: 50252 Gerrit-PatchSet: 1 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Wed, 03 Feb 2021 17:40:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: vendorcode/amd/fsp/cezanne: add UPD structs from FSP build
by Marshall Dawson (Code Review) 03 Feb '21

03 Feb '21

Attention is currently required from: Justin Frodsham, Raul Rangel, Felix Held. Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50242 ) Change subject: vendorcode/amd/fsp/cezanne: add UPD structs from FSP build ...................................................................... Patch Set 1: Code-Review+2 (1 comment) File src/vendorcode/amd/fsp/cezanne/FspmUpd.h: https://review.coreboot.org/c/coreboot/+/50242/comment/804d4f55_f7c5fb94 PS1, Line 19: /** Offset 0x0050**/ uint32_t serial_port_baudrate; : /** Offset 0x0054**/ uint32_t serial_port_refclk; > This is a WIP, if we don't line up the structures, we can't get very far in post. […] Raul, also FYI that any serial port activity from the FSP will be for debug builds only. But agree that any reconfiguring would be a problem. -- To view, visit https://review.coreboot.org/c/coreboot/+/50242 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Icade1d7bcab7b85cdd25c4114590eb23b914edcd Gerrit-Change-Number: 50242 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Justin Frodsham <justin.frodsham(a)amd.corp-partner.google.com> Gerrit-Reviewer: Justin Frodsham <justin.frodsham(a)protonmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Justin Frodsham <justin.frodsham(a)amd.corp-partner.google.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Justin Frodsham <justin.frodsham(a)protonmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Wed, 03 Feb 2021 17:37:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Justin Frodsham <justin.frodsham(a)amd.corp-partner.google.com> Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/skylake/acpi: Add PEP table
by Benjamin Doron (Code Review) 03 Feb '21

03 Feb '21

Attention is currently required from: Michael Niewöhner. Benjamin Doron has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49140 ) Change subject: soc/intel/skylake/acpi: Add PEP table ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/49140/comment/33d75649_17adf1ac PS3, Line 11: > > > > > Awesome, then at least S0ix states work just fine. > > > > > > > > > > That error message comes from the kernel SlpS0 self-test. So, let's test something. I assume `cat /sys/power/mem_sleep` has `s2idle` selected? (With s0ix_enable=1 it should. If not, we have another problem.) > > > > > > > > Yes. (I think that's based on the FADT) > > > > > > Right, there is some bit in FADT for that. > > > > > > > > > > > > Does your system stay in sleep after executing `echo freeze >/sys/power/state`? > > > > > > > > Is the alternative that it wakes immediately? Then yes, it stays in sleep. However, when it does wake, it has always been unlocked. I don't know if that's relevant. > > > > > > Unlocked vs. locked is user-space and depends on the desktop environment. XFCE has an option for locking before going to sleep but that only works when sleep was initiated from the DE. > > > > That makes sense; I thought it might have been a possibility. > > > > > If it wakes immediately with that error, the self-test failed. I'm surprised to see the self-test fail *without* immediate wake, though.... Maybe we have to exclude the SlpS0 state from the table LPIT, when s0ix is not supported on a board :S Could you test this, please? Insert a "return" right before the comment "System (Slp_S0) residency counter" in `src/soc/intel/common/block/acpi/lpit.c` and see if the error disappears. > > > > Based on https://01.org/blogs/qwang59/2020/linux-s0ix-troubleshooting, PC10 without S0ix residency is a possible case. If I understand correctly, that situation would be what I see with this board: No S0ix/failed self-test, but PC10 residency/no immediate wake. I will attempt to troubleshoot that sometime by disabling devices, I suppose, following that link. > > > > I'm also not certain what you mean. If S0ix is disabled, would LPIT factor? Besides, if S0ix does relate to hardware state, but not board-design, shouldn't LPIT always advertise both PC10 residency and system residency to warn about broken S0ix support for a board? > > Well, yes PC10 without S0ix is possible. s0ix_enable means means "PC10 and/or SlpS0 possible". Note that SlpS0 is a substate of s0ix but s0ix != SlpS0. > > The LPIT table contains two entries, one for PC10 (the first one) and SlpS0 (the second one. Based on these entries, the OS decides what *actually* is possible. When PC10 and SlpS0 is advertised but SlpS0 doesn't work, that error triggers. So, I want to test what happens, when SlpS0 is not advertised, because it doesn't apply to your board. I see the same error. Additionally, I noticed while looking over the vendor firmware's LPIT table that it defines the PC10 subtable twice, if I'm reading it correctly (two copies of address 0x632 as "FunctionalFixedHW" - the PKG_C10_RESIDENCY MSR?). Even so, the kernel warns that the system was unable to enter PC10. So, while I was initially surprised to hear that SlpS0 might not apply to my board, perhaps that is the case. I can always try to troubleshoot some other time. > > But regardless: `return` or `return current`? The PC10 residency counter portion does increment "current" > > Yes, you're right, it should be `return current` ofc :-) Was this only for testing, or do you think that the S0ix subtable should only be advertised if S0ix is known to work? If S0ix can be temperamental, I think that the OS should always be able to warn about errors. Sorry it took me so long to get back to you. -- To view, visit https://review.coreboot.org/c/coreboot/+/49140 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I08d8c1fde4f447e9292a0508649f802fdc2721e1 Gerrit-Change-Number: 49140 Gerrit-PatchSet: 3 Gerrit-Owner: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Comment-Date: Wed, 03 Feb 2021 17:35:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Benjamin Doron <benjamin.doron00(a)gmail.com> Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/broadwell: Convert some CONFIG(CHROMEOS) preprocessor
by Duncan Laurie (Code Review) 03 Feb '21

03 Feb '21

Attention is currently required from: Matt DeVillier, Kyösti Mälkki, Patrick Rudolph. Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50252 ) Change subject: soc/intel/broadwell: Convert some CONFIG(CHROMEOS) preprocessor ...................................................................... Patch Set 1: (1 comment) File src/soc/intel/broadwell/gma.c: https://review.coreboot.org/c/coreboot/+/50252/comment/db7ce447_a584f2f5 PS1, Line 518: !CONFIG(CHROMEOS) might be able to just drop this and call display_init_required()? I think originally these functions were part of chrome os checks but display_init_required() now checks for CONFIG(VBOOT) internally. -- To view, visit https://review.coreboot.org/c/coreboot/+/50252 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie3feee0448175db2b6ed4e8e37d92de3af9be371 Gerrit-Change-Number: 50252 Gerrit-PatchSet: 1 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Wed, 03 Feb 2021 17:34:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/amd/picasso/fch: add missing iomap.h include
by Raul Rangel (Code Review) 03 Feb '21

03 Feb '21

Attention is currently required from: Jason Glenesk, Marshall Dawson, Felix Held. Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50260 ) Change subject: soc/amd/picasso/fch: add missing iomap.h include ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/50260 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iea9666fe4f61fb503fee4060a90ec75e2d70c24f Gerrit-Change-Number: 50260 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Wed, 03 Feb 2021 17:34:04 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: drivers/intel/fsp2_0/memory_init: check if UPD struct has expected size
by Subrata Banik (Code Review) 03 Feb '21

03 Feb '21

Attention is currently required from: Furquan Shaikh, Meera Ravindranath, Andrey Petrov, Patrick Rudolph, Felix Held. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50241 ) Change subject: drivers/intel/fsp2_0/memory_init: check if UPD struct has expected size ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: @Meera, can you please help to verify on ADL? -- To view, visit https://review.coreboot.org/c/coreboot/+/50241 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia7e9f6f20d0091bbb4abfd42abb40b485da2079d Gerrit-Change-Number: 50241 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Justin Frodsham <justin.frodsham(a)amd.corp-partner.google.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Wed, 03 Feb 2021 17:32:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: vc/google/chromeos: Drop <acpi/vpd.asl>
by Duncan Laurie (Code Review) 03 Feb '21

03 Feb '21

Attention is currently required from: Lance Zhao, Kyösti Mälkki. Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49902 ) Change subject: vc/google/chromeos: Drop <acpi/vpd.asl> ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/49902 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7065bea4b288c689b41cc969989ec6fd87c75f1f Gerrit-Change-Number: 49902 Gerrit-PatchSet: 4 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Lance Zhao Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Comment-Date: Wed, 03 Feb 2021 17:31:18 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/amd/picasso: add UPD for USB3 phy setting adjust
by Martin Roth (Code Review) 03 Feb '21

03 Feb '21

Attention is currently required from: chris wang. Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50240 ) Change subject: soc/amd/picasso: add UPD for USB3 phy setting adjust ...................................................................... Patch Set 6: (1 comment) File src/soc/amd/picasso/fsp_params.c: https://review.coreboot.org/c/coreboot/+/50240/comment/5e9a294d_5d599309 PS4, Line 131: memcpy(&scfg->usb_3_port0_phy_tune, &cfg->usb3_phy_tune_params[0], : sizeof(scfg->usb_3_port0_phy_tune[0])); > For the RX_EQ_DELTA_IQ_OVRD_VAL and RX_EQ_DELTA_IQ_OVRD_EN. […] Right, I'm just wondering if the enable should be more. Right now if we want to update any parameter, we need to update all of them, because there's just the single enable. I'm fine with that, but it should be clear that any fields that aren't listed in the devicetree file are being set to 0, not being left they way they were. -- To view, visit https://review.coreboot.org/c/coreboot/+/50240 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1d5f69e840952cc5171af1ce8597628d1bede5cb Gerrit-Change-Number: 50240 Gerrit-PatchSet: 6 Gerrit-Owner: chris wang <Chris.Wang(a)amd.com> Gerrit-Reviewer: Chris Wang <chris.wang(a)amd.corp-partner.google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Martin Roth <martinroth(a)google.com> Gerrit-Attention: chris wang <Chris.Wang(a)amd.com> Gerrit-Comment-Date: Wed, 03 Feb 2021 17:30:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Martin Roth <martinroth(a)google.com> Comment-In-Reply-To: chris wang <Chris.Wang(a)amd.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: drivers/intel/fsp2_0/memory_init: check if UPD struct has expected size
by Marshall Dawson (Code Review) 03 Feb '21

03 Feb '21

Attention is currently required from: Furquan Shaikh, Subrata Banik, Andrey Petrov, Patrick Rudolph, Felix Held. Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50241 ) Change subject: drivers/intel/fsp2_0/memory_init: check if UPD struct has expected size ...................................................................... Patch Set 1: Code-Review+1 (1 comment) Patchset: PS1: LGTM but will stop short of +2 until it can get some testing with Intel FSP platforms. -- To view, visit https://review.coreboot.org/c/coreboot/+/50241 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia7e9f6f20d0091bbb4abfd42abb40b485da2079d Gerrit-Change-Number: 50241 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Justin Frodsham <justin.frodsham(a)amd.corp-partner.google.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Wed, 03 Feb 2021 17:29:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit February 2021