Attention is currently required from: Arthur Heymans, Marvin Drees, Nico Huber, Patrick Rudolph.
Lean Sheng Tan has posted comments on this change by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/84003?usp=email )
Change subject: Add initial experimental LTO support
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/84003?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: Ieb9204777fd349542744a8946e2207731c37969c
Gerrit-Change-Number: 84003
Gerrit-PatchSet: 6
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Marvin Drees <marvin.drees(a)9elements.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Marvin Drees <marvin.drees(a)9elements.com>
Gerrit-Comment-Date: Fri, 23 Aug 2024 09:57:23 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Arthur Heymans, Jérémy Compostella.
Angel Pons has posted comments on this change by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/84049?usp=email )
Change subject: arch/x86/Kconfig: Fix linking with LLD
......................................................................
Patch Set 8:
(1 comment)
File src/arch/x86/Kconfig:
https://review.coreboot.org/c/coreboot/+/84049/comment/2a9181bf_aaeb9a14?us… :
PS8, Line 416: 4196
Why 4196? 4k is 4096
--
To view, visit https://review.coreboot.org/c/coreboot/+/84049?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: I1a6a5ef9dc6d824fa108681689a69df3faefd3c6
Gerrit-Change-Number: 84049
Gerrit-PatchSet: 8
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Fri, 23 Aug 2024 09:56:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Arthur Heymans, Jérémy Compostella.
Angel Pons has posted comments on this change by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/84048?usp=email )
Change subject: arch/x86/car.ld: Fix overlapping regions
......................................................................
Patch Set 7: Code-Review+1
(1 comment)
Commit Message:
PS7:
Is this tested? How can it be tested?
--
To view, visit https://review.coreboot.org/c/coreboot/+/84048?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: I058bd6790afc313e06f1888e5b783d97b7e93b1e
Gerrit-Change-Number: 84048
Gerrit-PatchSet: 7
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-CC: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Fri, 23 Aug 2024 09:55:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Attention is currently required from: Arthur Heymans.
Angel Pons has posted comments on this change by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/84043?usp=email )
Change subject: util/cbfstool: Deal with how lld organizes loadable segments
......................................................................
Patch Set 4: Code-Review+1
(1 comment)
File util/cbfstool/cbfs-mkstage.c:
https://review.coreboot.org/c/coreboot/+/84043/comment/9fbdcf89_a6e458d3?us… :
PS4, Line 318: /*
: * lld pads the memsz of the .text vaddr till the vaddr of car.data. Since we don't load
: * XIP stages at runtime, we don't care.
: */
nit: reflow comment
```suggestion
/*
* lld pads the memsz of the .text vaddr till the vaddr of car.data.
* Since we don't load XIP stages at runtime, we don't care.
*/
```
--
To view, visit https://review.coreboot.org/c/coreboot/+/84043?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: I1a0541c8ea3dfbebfba83d505d84b6db12000723
Gerrit-Change-Number: 84043
Gerrit-PatchSet: 4
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Fri, 23 Aug 2024 09:54:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Attention is currently required from: Arthur Heymans.
Angel Pons has posted comments on this change by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/84042?usp=email )
Change subject: soc/intel/pmclib.c: Make sure array_size is not 0
......................................................................
Patch Set 4: Code-Review+1
(1 comment)
File src/soc/intel/common/block/pmc/pmclib.c:
https://review.coreboot.org/c/coreboot/+/84042/comment/6b47ba43_8ec8c0e7?us… :
PS4, Line 351: printk(BIOS_DEBUG, "GPE0 STD STS: ");
:
: sts_arr = soc_std_gpe_sts_array(&array_size);
: if (array_size == 0)
: return 0;
: print_num_status_bits(array_size, gpe_sts, sts_arr);
: printk(BIOS_DEBUG, "\n");
I'd still print the newline so as not to mess up the formatting:
```suggestion
printk(BIOS_DEBUG, "GPE0 STD STS: ");
sts_arr = soc_std_gpe_sts_array(&array_size);
if (array_size > 0)
print_num_status_bits(array_size, gpe_sts, sts_arr);
printk(BIOS_DEBUG, "\n");
```
--
To view, visit https://review.coreboot.org/c/coreboot/+/84042?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: Ieee6e9bddc4e738eb560dd0e69dc3087ac9f5da6
Gerrit-Change-Number: 84042
Gerrit-PatchSet: 4
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Fri, 23 Aug 2024 09:52:31 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Attention is currently required from: Arthur Heymans.
Angel Pons has posted comments on this change by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/84041?usp=email )
Change subject: drivers/intel/opregion.c: Also set vbt_size if size is 0
......................................................................
Patch Set 4: Code-Review+2
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/84041/comment/d955d73d_4f2d41e4?us… :
PS4, Line 9: size
sure
--
To view, visit https://review.coreboot.org/c/coreboot/+/84041?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: I4fcc6c02f898640e9b40d769e1165a4a0fb0fdf2
Gerrit-Change-Number: 84041
Gerrit-PatchSet: 4
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Fri, 23 Aug 2024 09:51:17 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Attention is currently required from: Arthur Heymans, Christian Walter, Jakub Czapiga, Johnny Lin, Jonathan Zhang, Jérémy Compostella, Lean Sheng Tan, Patrick Rudolph, Shuo Liu, Tim Chu.
Angel Pons has posted comments on this change by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/84039?usp=email )
Change subject: cbmem.h: Change signature of cbmem_get_region
......................................................................
Patch Set 4:
(2 comments)
File src/lib/fit_payload.c:
https://review.coreboot.org/c/coreboot/+/84039/comment/f99d64c0_1bdcddc0?us… :
PS4, Line 144: die("Could not find cbmem region");
Is it necessary to promote the warning to a fatal error?
File src/lib/imd_cbmem.c:
https://review.coreboot.org/c/coreboot/+/84039/comment/6fbb2f5b_a19ca0c1?us… :
PS4, Line 199: if (cbmem_get_region(&baseptr, &size)) {
I wonder, why only use braces here? It feels inconsistent
--
To view, visit https://review.coreboot.org/c/coreboot/+/84039?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: Ib3e09a75380faf9f533601368993261f042422ef
Gerrit-Change-Number: 84039
Gerrit-PatchSet: 4
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Comment-Date: Fri, 23 Aug 2024 09:50:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Arthur Heymans, Jérémy Compostella.
Angel Pons has posted comments on this change by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/84040?usp=email )
Change subject: ext_stage_cache: Make sure variables are initializated
......................................................................
Patch Set 3: Code-Review+1
Copied votes on follow-up patch sets have been updated:
* Code-Review+1 has been copied to patch set 4 (copy condition: "changekind:NO_CHANGE OR changekind:NO_CODE_CHANGE OR changekind:TRIVIAL_REBASE OR is:MIN").
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/84040/comment/89cdcd07_d07ba20f?us… :
PS3, Line 7: initializated
initialized
File src/lib/ext_stage_cache.c:
https://review.coreboot.org/c/coreboot/+/84040/comment/3de1bf5d_39c9c862?us… :
PS3, Line 20: printk(BIOS_ERR, "Could not find external stage cache");
`stage_cache_external_region` already prints an error, do we need to print an error on the caller side too?
--
To view, visit https://review.coreboot.org/c/coreboot/+/84040?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: Ib1851295646258e97c489dc7402b9df3fcf092c1
Gerrit-Change-Number: 84040
Gerrit-PatchSet: 3
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Fri, 23 Aug 2024 09:48:04 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Attention is currently required from: Christian Walter, Jakub Czapiga, Johnny Lin, Jonathan Zhang, Jérémy Compostella, Lean Sheng Tan, Patrick Rudolph, Shuo Liu, Tim Chu.
Hello Christian Walter, Jakub Czapiga, Johnny Lin, Jonathan Zhang, Jérémy Compostella, Lean Sheng Tan, Patrick Rudolph, Shuo Liu, Tim Chu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/84039?usp=email
to look at the new patch set (#4).
Change subject: cbmem.h: Change signature of cbmem_get_region
......................................................................
cbmem.h: Change signature of cbmem_get_region
The underlying IMD function already returns an integer which indicates
success or failure.
This removes the need to have initialized variables that need to be
checked for NULL later.
Dying is appropriate if cbmem is not found as it is essential to the
bootflow.
Change-Id: Ib3e09a75380faf9f533601368993261f042422ef
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
---
M src/arch/x86/postcar_loader.c
M src/include/cbmem.h
M src/lib/fit_payload.c
M src/lib/imd_cbmem.c
M src/security/memory/memory_clear.c
M src/soc/intel/xeon_sp/memmap.c
M tests/lib/Makefile.mk
7 files changed, 23 insertions(+), 18 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/84039/4
--
To view, visit https://review.coreboot.org/c/coreboot/+/84039?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: Ib3e09a75380faf9f533601368993261f042422ef
Gerrit-Change-Number: 84039
Gerrit-PatchSet: 4
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com>
Attention is currently required from: Arthur Heymans.
Angel Pons has posted comments on this change by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/84044?usp=email )
Change subject: Makefile.mk: Suppress stack-usage link warning
......................................................................
Patch Set 3: Code-Review+1
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/84044/comment/3fbe55ef_648b0c26?us… :
PS3, Line 7: Makefile.mk: Suppress stack-usage link warning
... for LTO builds
--
To view, visit https://review.coreboot.org/c/coreboot/+/84044?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: If08d6d543a4fcff07003af8d1f8dd59ab79f42f8
Gerrit-Change-Number: 84044
Gerrit-PatchSet: 3
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Fri, 23 Aug 2024 09:46:16 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes