coreboot-gerrit March 2022

coreboot-gerrit@coreboot.org

1 participants
6700 discussions

Change in coreboot[master]: drivers/smmstore: Enable 4KiB blocks in SMMSTORE
by Sean Rhodes (Code Review) 22 Mar '22

22 Mar '22

Attention is currently required from: Patrick Rudolph, Arthur Heymans. Sean Rhodes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62990 ) Change subject: drivers/smmstore: Enable 4KiB blocks in SMMSTORE ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: Sorry dude, the logic is lost on me. If we don't change any defaults, someone selects an option that breaks something because they haven't read up on it, it's trial and error anyway - they'd just unselect it... > Have you tried the following using native coreboot tools to edit the layout? No, as it would break the manifest. -- To view, visit https://review.coreboot.org/c/coreboot/+/62990 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ieeed9f5895b8ddde753ebf596b803e8f440d5b74 Gerrit-Change-Number: 62990 Gerrit-PatchSet: 1 Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Tue, 22 Mar 2022 14:29:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Sean Rhodes <sean(a)starlabs.systems> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: drivers/smmstore: Enable 4KiB blocks in SMMSTORE
by Arthur Heymans (Code Review) 22 Mar '22

22 Mar '22

Attention is currently required from: Sean Rhodes, Patrick Rudolph. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62990 ) Change subject: drivers/smmstore: Enable 4KiB blocks in SMMSTORE ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > > fix the payload > Pandora's box, first thing they'll ask is why 64KiB - we dont want people to configure it wrong is a weak argument. > Having options to configure things is ok. Having options to configure things potentially incorrectly should be avoided. A Kconfig to work around the failings of a specific payload makes using coreboot even more complicated than it already is. You'd need to understand a whole lot of things to be able to make a decision whether 4K or 64K block erase in SMMSTORE is a good idea for your payload. Your then essentially pushing responsibility from developer to users and we try to avoid that. > Plus you've got to get past Patrick first :p > > > Can you not just top align the COREBOOT region in fmap and BIOS region in IFD to work around this? > I can not, I have tried and failed - to the point of thinking it's impossible. I'd happily be proved wrong. The Intel tooling does not seem to allow for that indeed. (it looks pretty bad in general ^^) Have you tried the following using native coreboot tools to edit the layout? $ util/ifdtool/ifdtool build/coreboot.bin -f /tmp/layout -p aplk -> modify /tmp/layout so that BIOS region end is 64K aligned $ util/ifdtool/ifdtool build/coreboot.bin -n /tmp/layout -p aplk -> Now you have a worthless image but with a good ifd. extract it $ util/ifdtool/ifdtool build/coreboot.bin -x -> use that new IFD & adapt fmap and rebuild. Totally untested btw, just a suggestion. -- To view, visit https://review.coreboot.org/c/coreboot/+/62990 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ieeed9f5895b8ddde753ebf596b803e8f440d5b74 Gerrit-Change-Number: 62990 Gerrit-PatchSet: 1 Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Tue, 22 Mar 2022 14:12:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Sean Rhodes <sean(a)starlabs.systems> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: drivers/smmstore: Enable 4KiB blocks in SMMSTORE
by Sean Rhodes (Code Review) 22 Mar '22

22 Mar '22

Attention is currently required from: Patrick Rudolph, Arthur Heymans. Sean Rhodes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62990 ) Change subject: drivers/smmstore: Enable 4KiB blocks in SMMSTORE ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > fix the payload Pandora's box, first thing they'll ask is why 64KiB - we dont want people to configure it wrong is a weak argument. Plus you've got to get past Patrick first :p > Can you not just top align the COREBOOT region in fmap and BIOS region in IFD to work around this? I can not, I have tried and failed - to the point of thinking it's impossible. I'd happily be proved wrong. -- To view, visit https://review.coreboot.org/c/coreboot/+/62990 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ieeed9f5895b8ddde753ebf596b803e8f440d5b74 Gerrit-Change-Number: 62990 Gerrit-PatchSet: 1 Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Tue, 22 Mar 2022 13:29:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Sean Rhodes <sean(a)starlabs.systems> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: drivers/smmstore: Enable 4KiB blocks in SMMSTORE
by Arthur Heymans (Code Review) 22 Mar '22

22 Mar '22

1 0

Change in coreboot[master]: drivers/smmstore: Enable 4KiB blocks in SMMSTORE
by Sean Rhodes (Code Review) 22 Mar '22

22 Mar '22

Attention is currently required from: Patrick Rudolph, Arthur Heymans. Sean Rhodes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62990 ) Change subject: drivers/smmstore: Enable 4KiB blocks in SMMSTORE ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > > It's this^ change, or this -https://github. […] Okay, so easy solution, make it a Kconfig option? -- To view, visit https://review.coreboot.org/c/coreboot/+/62990 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ieeed9f5895b8ddde753ebf596b803e8f440d5b74 Gerrit-Change-Number: 62990 Gerrit-PatchSet: 1 Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Tue, 22 Mar 2022 12:42:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Sean Rhodes <sean(a)starlabs.systems> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: drivers/smmstore: Enable 4KiB blocks in SMMSTORE
by Arthur Heymans (Code Review) 22 Mar '22

22 Mar '22

Attention is currently required from: Sean Rhodes, Patrick Rudolph. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62990 ) Change subject: drivers/smmstore: Enable 4KiB blocks in SMMSTORE ...................................................................... Patch Set 1: (2 comments) Patchset: PS1: > See https://github.com/coreboot/coreboot/blob/1f54599b9875e0de80c8636626bdfbe01… > > Not sure what the minimum flash sector size is on AMD or older platforms. Maybe this can be gathered from Kconfig/bootmedia API? It depends on the coreboot driver for the flash chip see 'sector_size_kib_shift'. 64K is the smallest common denomination. PS1: > It's this^ change, or this -https://github.com/StarLabsLtd/edk2/commit/e5033f0e9a01c2d4288370ef25cb12f2ee0a339e > I'm not sure I understand that patch. It just looks like the block alignment check is broken in EDK2 and that 4K writes is just a workaround. > The suggestion came from Patrick, to avoid the edk2 patches. It does not break erasing. That depends on the flash chip you have installed. spi_flash_cmd_erase() will check the block erase size against the erase size arguments. So this would break some flash chips. -- To view, visit https://review.coreboot.org/c/coreboot/+/62990 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ieeed9f5895b8ddde753ebf596b803e8f440d5b74 Gerrit-Change-Number: 62990 Gerrit-PatchSet: 1 Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Tue, 22 Mar 2022 12:34:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Sean Rhodes <sean(a)starlabs.systems> Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph(a)9elements.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/amd/non_car/memlayout_x86.ld: Top aling the bootblock
by Arthur Heymans (Code Review) 22 Mar '22

22 Mar '22

Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson, Felix Held. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55796 ) Change subject: soc/amd/non_car/memlayout_x86.ld: Top aling the bootblock ...................................................................... Patch Set 1: (2 comments) File src/soc/amd/common/block/cpu/noncar/memlayout_x86.ld: https://review.coreboot.org/c/coreboot/+/55796/comment/e1d349b5_24864173 PS1, Line 115: _bootblock > Hrmm, what would it take to use arch/x86/bootblock.ld? It looks like we just need a KCONFIG to remove the .id/.fit pointer If there is nothing in those sections that should work too, so only conditionally include id.S. Fit has already been moved out. https://review.coreboot.org/c/coreboot/+/55796/comment/e537ee8f_d98a9597 PS1, Line 124: 64 > Why the extra 64? ALIGN(x) aligns downwards but we are aligning the bootblock to the top, so some extra bytes appended help. -- To view, visit https://review.coreboot.org/c/coreboot/+/55796 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I22116bd4b5be86e21cd5a16d6663166a10add17e Gerrit-Change-Number: 55796 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> 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-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Tue, 22 Mar 2022 12:22:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common: Add support to control CSE firmware update
by Sridhar Siricilla (Code Review) 22 Mar '22

22 Mar '22

Attention is currently required from: Subrata Banik, Patrick Rudolph. Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62715 ) Change subject: soc/intel/common: Add support to control CSE firmware update ...................................................................... Patch Set 3: (1 comment) File src/soc/intel/common/block/cse/cse_lite.c: https://review.coreboot.org/c/coreboot/+/62715/comment/78414652_678da8f2 PS1, Line 12: #include <intelbasecode/debug_feature.h> > Will fix it. […] Ack -- To view, visit https://review.coreboot.org/c/coreboot/+/62715 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9f234b142191eb83137d5d83f21e890e1cb828ba Gerrit-Change-Number: 62715 Gerrit-PatchSet: 3 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Tue, 22 Mar 2022 12:20:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common: Add support to control coreboot and Intel SoC features
by Sridhar Siricilla (Code Review) 22 Mar '22

22 Mar '22

Attention is currently required from: Nico Huber, Subrata Banik, Patrick Rudolph. Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61380 ) Change subject: soc/intel/common: Add support to control coreboot and Intel SoC features ...................................................................... Patch Set 16: (2 comments) File src/soc/intel/common/basecode/debug/Kconfig: https://review.coreboot.org/c/coreboot/+/61380/comment/dd4dcc8d_958f141c PS14, Line 5: Driver to control run time features of Intel SoC & coreboot > Sure, I will improve description. Ack File src/soc/intel/common/basecode/include/intelbasecode/debug_feature.h: https://review.coreboot.org/c/coreboot/+/61380/comment/ca9b594a_54596231 PS14, Line 3: RT > Not required. Ack -- To view, visit https://review.coreboot.org/c/coreboot/+/61380 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5ba40926bd9ad909654f152e48cdd648b28afd62 Gerrit-Change-Number: 61380 Gerrit-PatchSet: 16 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.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-CC: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Tue, 22 Mar 2022 12:20:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common: Add support to control CSE firmware update
by Sridhar Siricilla (Code Review) 22 Mar '22

22 Mar '22

Attention is currently required from: Subrata Banik, Patrick Rudolph. Hello Subrata Banik, Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/62715 to look at the new patch set (#3). Change subject: soc/intel/common: Add support to control CSE firmware update ...................................................................... soc/intel/common: Add support to control CSE firmware update The patch adds support to control CSE Lite firmware update dynamically. BUG=b:153410586 TEST=Verified controlling CSE Lite firmware update dynamically on Brya Signed-off-by: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Change-Id: I9f234b142191eb83137d5d83f21e890e1cb828ba --- M src/soc/intel/common/basecode/debug/debug_feature.c M src/soc/intel/common/basecode/include/intelbasecode/debug_feature.h M src/soc/intel/common/block/cse/cse_lite.c 3 files changed, 30 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/62715/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/62715 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9f234b142191eb83137d5d83f21e890e1cb828ba Gerrit-Change-Number: 62715 Gerrit-PatchSet: 3 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newpatchset

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit March 2022