coreboot-gerrit December 2021

coreboot-gerrit@coreboot.org

1 participants
5525 discussions

Change in coreboot[master]: libpayload/libc/fmap: Implement new FlashMap API
by Julius Werner (Code Review) 10 Dec '21

10 Dec '21

Attention is currently required from: Jakub Czapiga. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59494 ) Change subject: libpayload/libc/fmap: Implement new FlashMap API ...................................................................... Patch Set 5: (11 comments) File payloads/libpayload/libc/fmap.c: https://review.coreboot.org/c/coreboot/+/59494/comment/3574aaa1_19042eed PS5, Line 38: /* Private fmap cache. Left non-static to make testing easier. */ I thought we didn't do things like this? Can't you just use the #include trick? (Exporting a symbol from the compilation unit can preclude optimizations that the compiler might otherwise be able to do... probably not much in this case, but I'd rather not start making test-only concessions in principle if we can avoid them.) https://review.coreboot.org/c/coreboot/+/59494/comment/e8732399_ad926d1c PS5, Line 43: for (size_t i = 0; i < fmap->nareas; ++i) { Would be good to sprinkle le32toh() everywhere applicable here, like has recently been added to the coreboot version. https://review.coreboot.org/c/coreboot/+/59494/comment/1389ba45_f43b8e1a PS5, Line 56: _fmap_check_signature nit: just in general, try to follow the pattern of "function sounds like an action" -> 0 success, negative value error; "function sounds like a question" -> true means yes, false means no. So I would call this something like is_signature_valid() or fmap_signature_valid(). https://review.coreboot.org/c/coreboot/+/59494/comment/8a46289d_3d3b2b14 PS5, Line 61: _ I'm not sure why you're prefixing everything with an underscore here... generally, we don't prefix things with underscores unless there's a special reason. I've used underscores in coreboot's <cbfs.h> for a few things that need to be in the public header for use by inlines but shouldn't be considered a public API that can be called directly... but here these functions are already static, they are already local to the compilation unit, you don't need anything else to signify that they're internal. https://review.coreboot.org/c/coreboot/+/59494/comment/b683311e_c49de534 PS5, Line 63: if (_fmap_cache) nit: you don't really need to check it on both ends https://review.coreboot.org/c/coreboot/+/59494/comment/16908965_9b6f19fe PS5, Line 69: _fmap_cache = (struct fmap *)lib_sysinfo.fmap_cache; phys_to_virt()? https://review.coreboot.org/c/coreboot/+/59494/comment/a3ca7ba6_4b6b8c3d PS5, Line 73: /* Load FMAP from boot media to internal cache */ outdated comment File payloads/libpayload/tests/include/mocks/boot_device.h: https://review.coreboot.org/c/coreboot/+/59494/comment/c58f0009_ec8839ec PS5, Line 8: libpayload_boot_device_read Looks like you meant to delete this file? File payloads/libpayload/tests/libc/fmap_locate_area-test.c: https://review.coreboot.org/c/coreboot/+/59494/comment/4ff37d7c_cdd253e8 PS5, Line 101: /* Different contents of fmap_cache and fmap_offset */ This test doesn't test what the comments suggest it does. https://review.coreboot.org/c/coreboot/+/59494/comment/ae02a34c_0a0a7160 PS5, Line 112: static void test_fmap_locate_area_repeated_area(void **state) Honestly don't think we should test this, because it's an invalid FMAP and the result is undefined (e.g. scanning the FMAP back to front wouldn't be an invalid implementation, but it would break this test). File payloads/libpayload/tests/mocks/boot_device.c: PS5: This file too? -- To view, visit https://review.coreboot.org/c/coreboot/+/59494 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idbf9016ce73aa58e17f3ee19920ab83dc6c25abb Gerrit-Change-Number: 59494 Gerrit-PatchSet: 5 Gerrit-Owner: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-CC: Patrick Georgi <patrick(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Comment-Date: Fri, 10 Dec 2021 00:40:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/cse: config to enable oem key manifest
by YH Lin (Code Review) 10 Dec '21

10 Dec '21

Attention is currently required from: Ravindra, Arthur Heymans, Patrick Rudolph, Ravindra N. YH Lin has uploaded a new patch set (#7) to the change originally created by Ravindra. ( https://review.coreboot.org/c/coreboot/+/59895 ) Change subject: soc/intel/cse: config to enable oem key manifest ...................................................................... soc/intel/cse: config to enable oem key manifest CB change will enable the CSE region sub-partition OEMP, where the OEMP binary will be stitched. OEM KM has Audio FW's key hash. So, CSE uses this information to authenticate Audio FW. BUG=b:207820413 TEST: Boot to kernel and check for the audio authentication is successful localhost ~ # aplay -l **** List of PLAYBACK Hardware Devices **** card 0: sofrt5682 [sof-rt5682], device 0: max357a-spk (*) [] Subdevices: 1/1 Subdevice #0: subdevice #0 card 0: sofrt5682 [sof-rt5682], device 1: Headset (*) [] Subdevices: 1/1 Subdevice #0: subdevice #0 card 0: sofrt5682 [sof-rt5682], device 2: HDMI1 (*) [] Subdevices: 1/1 Subdevice #0: subdevice #0 card 0: sofrt5682 [sof-rt5682], device 3: HDMI2 (*) [] Subdevices: 1/1 Subdevice #0: subdevice #0 card 0: sofrt5682 [sof-rt5682], device 4: HDMI3 (*) [] Subdevices: 1/1 Subdevice #0: subdevice #0 card 0: sofrt5682 [sof-rt5682], device 5: HDMI4 (*) [] Subdevices: 1/1 Subdevice #0: subdevice #0 Cq-Depend: chrome-internal:4286038 Signed-off-by: Ravindra N <ravindra(a)intel.corp-partner.google.com> Change-Id: I3620adb2898efc002104e0ba8b2afd219c31f230 --- M src/soc/intel/alderlake/Makefile.inc M src/soc/intel/common/block/cse/Kconfig 2 files changed, 5 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/59895/7 -- To view, visit https://review.coreboot.org/c/coreboot/+/59895 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3620adb2898efc002104e0ba8b2afd219c31f230 Gerrit-Change-Number: 59895 Gerrit-PatchSet: 7 Gerrit-Owner: Ravindra <ravindra(a)intel.com> Gerrit-Reviewer: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Ravindra N <ravindra(a)intel.corp-partner.google.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Ravindra <ravindra(a)intel.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Ravindra N <ravindra(a)intel.corp-partner.google.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: mb/google/zork/var/shuboz: Add fw_config probe for ALC5682-VD & VS
by Kangheui Won (Code Review) 10 Dec '21

10 Dec '21

Attention is currently required from: Isaac Lee, Reka Norman, Kane Chen, chris wang, Felix Held. Kangheui Won has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59558 ) Change subject: mb/google/zork/var/shuboz: Add fw_config probe for ALC5682-VD & VS ...................................................................... Patch Set 14: Code-Review+2 (1 comment) File src/mainboard/google/zork/variants/shuboz/overridetree.cb: https://review.coreboot.org/c/coreboot/+/59558/comment/c847a019_6a7d10e1 PS5, Line 2: fw_config > Thanks for your help, I modify the setting of "probe" according to the CL you provided, both devices […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/59558 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0c78aa166010ffa4d0cacc8a11d418d5a6906749 Gerrit-Change-Number: 59558 Gerrit-PatchSet: 14 Gerrit-Owner: Kane Chen <kane_chen(a)pegatron.corp-partner.google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Isaac Lee <isaaclee(a)google.com> Gerrit-Reviewer: Kane Chen <kane_chen(a)pegatron.corp-partner.google.com> Gerrit-Reviewer: Kangheui Won <khwon(a)chromium.org> Gerrit-Reviewer: Reka Norman <rekanorman(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: chris wang <Chris.Wang(a)amd.com> Gerrit-CC: Frank Chu <frank_chu(a)pegatron.corp-partner.google.com> Gerrit-CC: Hank Lin <hank2_lin(a)pegatron.corp-partner.google.com> Gerrit-CC: Ken Lu <ken_lu(a)pegatron.corp-partner.google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Randy Wang <ifanw(a)google.com> Gerrit-Attention: Isaac Lee <isaaclee(a)google.com> Gerrit-Attention: Reka Norman <rekanorman(a)chromium.org> Gerrit-Attention: Kane Chen <kane_chen(a)pegatron.corp-partner.google.com> Gerrit-Attention: chris wang <Chris.Wang(a)amd.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Fri, 10 Dec 2021 00:26:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Kangheui Won <khwon(a)chromium.org> Comment-In-Reply-To: Reka Norman <rekanorman(a)chromium.org> Comment-In-Reply-To: Kane Chen <kane_chen(a)pegatron.corp-partner.google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: libpayload: Add boot_device_read() function
by Julius Werner (Code Review) 10 Dec '21

10 Dec '21

Attention is currently required from: Jakub Czapiga. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59492 ) Change subject: libpayload: Add boot_device_read() function ...................................................................... Patch Set 6: Code-Review+2 (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/59492/comment/f2146b53_6976a1f0 PS6, Line 12: This patch also provides a fallback implementation : of this function using cbfs_media, but it will be removed in the future : together with cbfs_media API. nit: outdated -- To view, visit https://review.coreboot.org/c/coreboot/+/59492 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1babd2a8414ed9de3ca49903fcb4f036997b5ff3 Gerrit-Change-Number: 59492 Gerrit-PatchSet: 6 Gerrit-Owner: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Georgi <patrick(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Comment-Date: Fri, 10 Dec 2021 00:08:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/asus/p5qc: Add ASUS P5Q-E as a variant
by Angel Pons (Code Review) 10 Dec '21

10 Dec '21

Attention is currently required from: Stefan Ott, Name of user not set #1004033. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60011 ) Change subject: mb/asus/p5qc: Add ASUS P5Q-E as a variant ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/60011 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3e8415fe88a421c529308279381aa62e2160e171 Gerrit-Change-Number: 60011 Gerrit-PatchSet: 1 Gerrit-Owner: Name of user not set #1004033 Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Stefan Ott <coreboot(a)desire.ch> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Stefan Ott <coreboot(a)desire.ch> Gerrit-Attention: Name of user not set #1004033 Gerrit-Comment-Date: Fri, 10 Dec 2021 00:01:40 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/asus/p5qc: Add ASUS P5Q-E as a variant
by Angel Pons (Code Review) 10 Dec '21

10 Dec '21

Attention is currently required from: Stefan Ott, Name of user not set #1004033. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60011 ) Change subject: mb/asus/p5qc: Add ASUS P5Q-E as a variant ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > I havent tried this yet on my board. Posting for review before trying it. Thanks to hell on IRC. Looks reasonable to test on hardware. It will most likely boot, but some PCIe devices may be missing. -- To view, visit https://review.coreboot.org/c/coreboot/+/60011 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3e8415fe88a421c529308279381aa62e2160e171 Gerrit-Change-Number: 60011 Gerrit-PatchSet: 1 Gerrit-Owner: Name of user not set #1004033 Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Stefan Ott <coreboot(a)desire.ch> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Stefan Ott <coreboot(a)desire.ch> Gerrit-Attention: Name of user not set #1004033 Gerrit-Comment-Date: Fri, 10 Dec 2021 00:01:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Name of user not set #1004033 Gerrit-MessageType: comment

1 0

Change in coreboot[master]: cbfstool: Do host space address conversion earlier when adding files
by Paul Menzel (Code Review) 09 Dec '21

09 Dec '21

Attention is currently required from: Raul Rangel, Patrick Georgi, Tim Wawrzynczak, Julius Werner. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60018 ) Change subject: cbfstool: Do host space address conversion earlier when adding files ...................................................................... Patch Set 3: Code-Review+1 (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/60018/comment/4735d747_7bcc57fa PS3, Line 11: but by? https://review.coreboot.org/c/coreboot/+/60018/comment/c8c69101_4792ad3e PS3, Line 21: CB:59877 Could you please also use the commit hash and summary? -- To view, visit https://review.coreboot.org/c/coreboot/+/60018 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idf4721c5b0700789ddb81c1618d740b3e7f486cb Gerrit-Change-Number: 60018 Gerrit-PatchSet: 3 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Comment-Date: Thu, 09 Dec 2021 23:59:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: Add console deinit API, use in SMM handler
by Rocky Phagura (Code Review) 09 Dec '21

09 Dec '21

Attention is currently required from: Nico Huber, Johnny Lin, Stefan Reinauer, Paul Menzel, Rocky Phagura, Christian Walter, Angel Pons, Arthur Heymans, Kyösti Mälkki, Peter Tsung Ho Wu, Deomid "rojer" Ryabkov, Ron Minnich. Rocky Phagura has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56386 ) Change subject: Add console deinit API, use in SMM handler ...................................................................... Patch Set 11: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/56386/comment/f9878a4d_bd32cf19 PS11, Line 14: on an OCP Delta Lake server: uart8250_init disables interrupts because it > So uart8250_init() is called via console_init() in smi_handler() / smm_handler_start() (depending on […] Patrick - just to provide some more context...when the OS takes over the console it switches to interrupt mode while coreboot uses polling mode. So when entering SMM, coreboot switches to polling mode but upon exiting SMM it needs to restore interrupts, otherwise the console will no longer work. The rule in SMM is to restore the system state otherwise the OS has no idea what happened. So this patch essentially restores console interrupts. -- To view, visit https://review.coreboot.org/c/coreboot/+/56386 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia5e51f385f83cb998c244ca1d1ffc10339d3a714 Gerrit-Change-Number: 56386 Gerrit-PatchSet: 11 Gerrit-Owner: Deomid "rojer" Ryabkov <rojer9(a)fb.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Peter Tsung Ho Wu <tsung(a)amazon.com> Gerrit-Reviewer: Rocky Phagura Gerrit-Reviewer: Rocky Phagura <rphagura(a)fb.com> Gerrit-Reviewer: Ron Minnich <rminnich(a)gmail.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Rocky Phagura <rphagura(a)fb.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Peter Tsung Ho Wu <tsung(a)amazon.com> Gerrit-Attention: Deomid "rojer" Ryabkov <rojer9(a)fb.com> Gerrit-Attention: Ron Minnich <rminnich(a)gmail.com> Gerrit-Comment-Date: Thu, 09 Dec 2021 23:49:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Georgi <patrick(a)coreboot.org> Comment-In-Reply-To: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: cbfstool: Do host space address conversion earlier when adding files
by Julius Werner (Code Review) 09 Dec '21

09 Dec '21

Attention is currently required from: Raul Rangel, Patrick Georgi, Tim Wawrzynczak. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60018 ) Change subject: cbfstool: Do host space address conversion earlier when adding files ...................................................................... Patch Set 3: (1 comment) File util/cbfstool/cbfstool.c: https://review.coreboot.org/c/coreboot/+/60018/comment/bb90a5db_eec7dd58 PS2, Line 1078: offset > Oh crap. Yeah, sorry, I totally misread the code. Let me rethink if this patch makes sense then... Fixed, thank you! -- To view, visit https://review.coreboot.org/c/coreboot/+/60018 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idf4721c5b0700789ddb81c1618d740b3e7f486cb Gerrit-Change-Number: 60018 Gerrit-PatchSet: 3 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Comment-Date: Thu, 09 Dec 2021 23:43:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: cbfstool: Do host space address conversion earlier when adding files
by Julius Werner (Code Review) 09 Dec '21

09 Dec '21

Attention is currently required from: Raul Rangel, Patrick Georgi, Tim Wawrzynczak. Hello build bot (Jenkins), Raul Rangel, Patrick Georgi, Tim Wawrzynczak, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/60018 to look at the new patch set (#3). Change subject: cbfstool: Do host space address conversion earlier when adding files ...................................................................... cbfstool: Do host space address conversion earlier when adding files In cbfs_add_component(), the |offset| variable confusingly jumps back and forth between host address space and flash address space in some cases. This patch tries to clean that logic up a bit but converting it to flash address space very early in the function, and then keeping it that way afterwards. convert() implementations that need the host address space value should store it in a different variable to reduce the risk of confusion. This should also fix a tiny issue where --gen-attribute might have previously encoded the base address as given in CBFS -- it probably makes more sense to always have it store a consistent format (i.e. always flash address). Also revert the unnecessary check for --base-address in add_topswap_bootblock() that was added in CB:59877. On closer inspection, the function actually doesn't use the passed in *offset at all and uses it purely as an out-parameter. So while our current Makefile does pass --base-address when adding the bootblock, it actually has no effect and is redundant for the topswap case. Signed-off-by: Julius Werner <jwerner(a)chromium.org> Change-Id: Idf4721c5b0700789ddb81c1618d740b3e7f486cb --- M util/cbfstool/cbfstool.c 1 file changed, 16 insertions(+), 24 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/60018/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/60018 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idf4721c5b0700789ddb81c1618d740b3e7f486cb Gerrit-Change-Number: 60018 Gerrit-PatchSet: 3 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit December 2021