Attention is currently required from: Dolan Liu, Lawrence Chang, Tongtong Pan.
Karthik Ramasubramanian has posted comments on this change by Tongtong Pan. ( https://review.coreboot.org/c/coreboot/+/83427?usp=email )
Change subject: mb/google/dedede/var/awasuki: Generate 3 RAM IDs
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/83427?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: I9a03c86770101ec70c2ee5d6b914313c1bf23b5f
Gerrit-Change-Number: 83427
Gerrit-PatchSet: 6
Gerrit-Owner: Tongtong Pan <pantongtong(a)huaqin.corp-partner.google.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Lawrence Chang <lawrence.chang(a)intel.com>
Gerrit-Reviewer: Weimin Wu <wuweimin(a)huaqin.corp-partner.google.com>
Gerrit-CC: Dolan Liu <liuyong5(a)huaqin.corp-partner.google.com>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Dolan Liu <liuyong5(a)huaqin.corp-partner.google.com>
Gerrit-Attention: Tongtong Pan <pantongtong(a)huaqin.corp-partner.google.com>
Gerrit-Attention: Lawrence Chang <lawrence.chang(a)intel.com>
Gerrit-Comment-Date: Mon, 15 Jul 2024 19:49:25 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk, Matt DeVillier.
Martin Roth has posted comments on this change by Felix Held. ( https://review.coreboot.org/c/coreboot/+/83446?usp=email )
Change subject: soc/amd/common/block/psp_gen2: add get_psp_mmio_base
......................................................................
Patch Set 2:
(2 comments)
File src/soc/amd/common/block/psp/psp_gen2.c:
https://review.coreboot.org/c/coreboot/+/83446/comment/003e2ea8_4053cf69?us… :
PS2, Line 32: form
from
https://review.coreboot.org/c/coreboot/+/83446/comment/327d9f3c_2d06638c?us… :
PS2, Line 62: /* Don't cache the PSP MMIO base if the register isn't locked */
I don't think this is right. We want to cache the value before the lock happens. If we wait until after it's locked, it could (in theory) be changed before we cache it.
I think we need to cache the value before the SMM register locking, and error out if the value isn't set when we access it after the locking happens.
We can also ignore all caching if we're not in SMM, right? Maybe put a check for being in the SMM component.
I think we should probably also compare the saved base value with what's being read from SMN, and error out if they're not the same.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83446?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: I1d51e30f186508b0fe1ab5eb79c73e6d4b9d1a4a
Gerrit-Change-Number: 83446
Gerrit-PatchSet: 2
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Mon, 15 Jul 2024 19:36:51 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Eric Lai, Martin L Roth, Patrick Georgi.
Jonathon Hall has posted comments on this change by Jonathon Hall. ( https://review.coreboot.org/c/coreboot/+/83475?usp=email )
Change subject: src/lib/malloc.c: If allocation fails, leave the heap unchanged
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
This appeared in the Heads project when I upstreamed a change bumping Librem devices to coreboot 24.02.01. It worked downstream with our bootsplashes, but failed to boot entirely with the Heads bootsplash, which apparently needs a much larger JPEG work area.
Failure to show the bootsplash is not a big deal, but failure to boot entirely is :)
--
To view, visit https://review.coreboot.org/c/coreboot/+/83475?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: Ie72814027d9daa517c0794f3ea7abec2b9a9d596
Gerrit-Change-Number: 83475
Gerrit-PatchSet: 1
Gerrit-Owner: Jonathon Hall <jonathon.hall(a)puri.sm>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Patrick Georgi
Gerrit-Attention: Eric Lai <ericllai(a)google.com>
Gerrit-Attention: Patrick Georgi
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Comment-Date: Mon, 15 Jul 2024 19:25:28 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk, Martin L Roth, Matt DeVillier, Varshit Pandya.
Martin Roth has posted comments on this change by Felix Held. ( https://review.coreboot.org/c/coreboot/+/83445?usp=email )
Change subject: soc/amd: add SoC-specific root_complex.c to SMM
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/83445?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: I4e32084b45f07131c80b642bc73d865fc57688a8
Gerrit-Change-Number: 83445
Gerrit-PatchSet: 2
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Varshit Pandya <pandyavarshit(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Varshit Pandya <pandyavarshit(a)gmail.com>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Mon, 15 Jul 2024 19:23:08 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk, Matt DeVillier.
Martin Roth has posted comments on this change by Felix Held. ( https://review.coreboot.org/c/coreboot/+/83444?usp=email )
Change subject: soc/amd/common/root_complex: move IOHC_MMIO_EN definition to header
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/83444?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: If88950418406d1709ed95b3d05f7e6ad66438f95
Gerrit-Change-Number: 83444
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Mon, 15 Jul 2024 19:22:40 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk, Martin L Roth, Matt DeVillier, Varshit Pandya.
Martin Roth has posted comments on this change by Felix Held. ( https://review.coreboot.org/c/coreboot/+/83443?usp=email )
Change subject: soc/amd/*/root_complex: introduce and use domain_iohc_info struct
......................................................................
Patch Set 1: Code-Review+2
(2 comments)
File src/soc/amd/common/block/root_complex/root_complex.c:
https://review.coreboot.org/c/coreboot/+/83443/comment/e749de75_a76b4d9c?us… :
PS1, Line 15: domain->path.domain.domain
Nit: This domaindomaindomain is kind of ugly. Can any of the domains be anything different to differentiate them? Doesn't need to be updated in this patch.
domain_number, domain_id, domain_struct, pci_domain, kingdom, etc.
https://review.coreboot.org/c/coreboot/+/83443/comment/b80cd6f6_b142adcb?us… :
PS1, Line 34: signed int
The fabric ID is a uint32_t, so why use a signed int for the return type? Looking at the ID values, can we make the ID a signed int or uint16_t so that the ID value fits in the positive values of a signed int?
Probably not a big deal, I know, and I see that it's just moved from the other file, but maybe consider changing it at some point?
--
To view, visit https://review.coreboot.org/c/coreboot/+/83443?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: Ifce3d2b540d14ba3cba36f7cbf248fb7c63483fe
Gerrit-Change-Number: 83443
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Varshit Pandya <pandyavarshit(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Varshit Pandya <pandyavarshit(a)gmail.com>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Mon, 15 Jul 2024 19:22:25 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Jonathon Hall has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/83475?usp=email )
Change subject: src/lib/malloc.c: If allocation fails, leave the heap unchanged
......................................................................
src/lib/malloc.c: If allocation fails, leave the heap unchanged
If an allocation fails because it is too large for the rest of the heap,
don't consume the rest of the heap needlessly.
This started occurring with the Heads bootsplash image in 24.02.01,
following the switch to the Wuffs JPEG decoder. The work area needed
was too large for the heap. The bootsplash failed to show, but worse,
the boot failed entirely because we were then out of heap space, even
though we did not actually use the large allocation that failed.
With this change, that failure no longer prevents boot.
The error message is improved slightly also:
* missing line break is added
* "Tried to round up" now shows the beginning of the allocation before
and after rounding instead of the unrounded beginning and rounded end
(misleading, looked like it was trying to align by 1 MB when it
was actually allocating 1 MB)
Change-Id: Ie72814027d9daa517c0794f3ea7abec2b9a9d596
Signed-off-by: Jonathon Hall <jonathon.hall(a)puri.sm>
---
M src/lib/malloc.c
1 file changed, 15 insertions(+), 16 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/83475/1
diff --git a/src/lib/malloc.c b/src/lib/malloc.c
index 3029806..281792c 100644
--- a/src/lib/malloc.c
+++ b/src/lib/malloc.c
@@ -26,10 +26,22 @@
MALLOCDBG("%s Enter, boundary %zu, size %zu, free_mem_ptr %p\n",
__func__, boundary, size, free_mem_ptr);
- free_mem_ptr = (void *)ALIGN_UP((unsigned long)free_mem_ptr, boundary);
+ p = (void *)ALIGN_UP((unsigned long)free_mem_ptr, boundary);
- p = free_mem_ptr;
- free_mem_ptr += size;
+ if (p + size >= free_mem_end_ptr) {
+ printk(BIOS_ERR, "%s(boundary=%zu, size=%zu): failed: ",
+ __func__, boundary, size);
+ printk(BIOS_ERR, "Tried to round up free_mem_ptr %p to %p\n",
+ free_mem_ptr, p);
+ printk(BIOS_ERR, "but free_mem_end_ptr is %p\n",
+ free_mem_end_ptr);
+ printk(BIOS_ERR, "Error! %s: Out of memory "
+ "(free_mem_ptr >= free_mem_end_ptr)\n",
+ __func__);
+ return NULL;
+ }
+
+ free_mem_ptr = p + size;
/*
* Store last allocation pointer after ALIGN, as malloc() will
* return it. This may cause n bytes of gap between allocations
@@ -37,19 +49,6 @@
*/
free_last_alloc_ptr = p;
- if (free_mem_ptr >= free_mem_end_ptr) {
- printk(BIOS_ERR, "%s(boundary=%zu, size=%zu): failed: ",
- __func__, boundary, size);
- printk(BIOS_ERR, "Tried to round up free_mem_ptr %p to %p\n",
- p, free_mem_ptr);
- printk(BIOS_ERR, "but free_mem_end_ptr is %p\n",
- free_mem_end_ptr);
- printk(BIOS_ERR, "Error! %s: Out of memory "
- "(free_mem_ptr >= free_mem_end_ptr)",
- __func__);
- return NULL;
- }
-
MALLOCDBG("%s %p\n", __func__, p);
return p;
--
To view, visit https://review.coreboot.org/c/coreboot/+/83475?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ie72814027d9daa517c0794f3ea7abec2b9a9d596
Gerrit-Change-Number: 83475
Gerrit-PatchSet: 1
Gerrit-Owner: Jonathon Hall <jonathon.hall(a)puri.sm>
Attention is currently required from: Benjamin Doron, Lean Sheng Tan, Martin L Roth, Matt DeVillier.
Felix Singer has posted comments on this change by Matt DeVillier. ( https://review.coreboot.org/c/coreboot/+/83474?usp=email )
Change subject: payloads/edk2/Makefile: Add $(EDK2_PATH) as dependency for 'gop_driver' target
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/83474?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: Ic510d70041dc099e6bc469528b80d1e271976655
Gerrit-Change-Number: 83474
Gerrit-PatchSet: 1
Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Benjamin Doron <benjamin.doron00(a)gmail.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Benjamin Doron <benjamin.doron00(a)gmail.com>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Comment-Date: Mon, 15 Jul 2024 19:12:38 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Benjamin Doron, Lean Sheng Tan, Martin L Roth, Matt DeVillier.
Sean Rhodes has posted comments on this change by Matt DeVillier. ( https://review.coreboot.org/c/coreboot/+/83474?usp=email )
Change subject: payloads/edk2/Makefile: Add $(EDK2_PATH) as dependency for 'gop_driver' target
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/83474?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: Ic510d70041dc099e6bc469528b80d1e271976655
Gerrit-Change-Number: 83474
Gerrit-PatchSet: 1
Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Benjamin Doron <benjamin.doron00(a)gmail.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Benjamin Doron <benjamin.doron00(a)gmail.com>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Comment-Date: Mon, 15 Jul 2024 19:07:53 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Felix Held, Felix Singer.
Nicholas Chin has posted comments on this change by Nicholas Chin. ( https://review.coreboot.org/c/coreboot/+/83458?usp=email )
Change subject: util/find_usbdebug: Add 8/9 Series PCH rate matching hub IDs
......................................................................
Patch Set 3:
(1 comment)
File util/find_usbdebug/find_usbdebug.sh:
https://review.coreboot.org/c/coreboot/+/83458/comment/d4f82921_cfebfe73?us… :
PS2, Line 38: hubs_to_ignore="8087:0020 8087:0024"
> is it intentional that this patch drops the existing IDs and completely replaces them? from the comm […]
Hmm, not sure what happened there. 0021 and 0026 aren't valid IDs for any Intel chipset with an EHCI rate matching hub. I must have been testing something before and forgot to change it back. Fixed.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83458?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: I52858e2c75e8a3e1424a13bcddc2f5ec1216164b
Gerrit-Change-Number: 83458
Gerrit-PatchSet: 3
Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Mon, 15 Jul 2024 18:33:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>