Attention is currently required from: Angel Pons, Christian Walter, Jakub Czapiga, Johnny Lin, Jonathan Zhang, Jérémy Compostella, Lean Sheng Tan, Patrick Rudolph, Shuo Liu, Tim Chu.
Arthur Heymans 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 5:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/84039/comment/4dd77758_cddd7143?us… :
PS2, Line 7: cbmem.h: Change signature of cbmem_get_region
> return type?
Done
File src/lib/fit_payload.c:
https://review.coreboot.org/c/coreboot/+/84039/comment/0bcbc599_0492cfbc?us… :
PS4, Line 144: die("Could not find cbmem region");
> Is it necessary to promote the warning to a fatal error?
Done
--
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: 5
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: Angel Pons <th3fanbus(a)gmail.com>
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 10:26:28 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Shuo Liu <shuo.liu(a)intel.com>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Attention is currently required from: Angel Pons, 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 (#6).
Change subject: cbmem.h: Change return type of cbmem_get_region
......................................................................
cbmem.h: Change return type 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(+), 15 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/84039/6
--
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: 6
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: Angel Pons <th3fanbus(a)gmail.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, Christian Walter, Jakub Czapiga, Johnny Lin, Jonathan Zhang, Jérémy Compostella, Lean Sheng Tan, Patrick Rudolph, 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 (#5).
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(+), 15 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/84039/5
--
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: 5
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: 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>
Attention is currently required from: Angel Pons, Arthur Heymans, Jérémy Compostella.
Hello Angel Pons, Jérémy Compostella,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/84040?usp=email
to look at the new patch set (#5).
The following approvals got outdated and were removed:
Code-Review+1 by Angel Pons
Change subject: ext_stage_cache: Make sure variables are initialized
......................................................................
ext_stage_cache: Make sure variables are initialized
GCC LTO incorrectly warns about this it seems.
Change-Id: Ib1851295646258e97c489dc7402b9df3fcf092c1
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
---
M src/cpu/x86/smm/tseg_region.c
M src/lib/ext_stage_cache.c
2 files changed, 6 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/84040/5
--
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: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ib1851295646258e97c489dc7402b9df3fcf092c1
Gerrit-Change-Number: 84040
Gerrit-PatchSet: 5
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: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Attention is currently required from: Angel Pons, Arthur Heymans.
Hello Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/84041?usp=email
to look at the new patch set (#5).
The following approvals got outdated and were removed:
Code-Review+2 by Angel Pons
Change subject: drivers/intel/opregion.c: Also set vbt_size if size is 0
......................................................................
drivers/intel/opregion.c: Also set vbt_size if size is 0
Make sure size vbt_size is initialized. GCC LTO warns about this.
Change-Id: I4fcc6c02f898640e9b40d769e1165a4a0fb0fdf2
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
---
M src/drivers/intel/gma/opregion.c
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/84041/5
--
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: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I4fcc6c02f898640e9b40d769e1165a4a0fb0fdf2
Gerrit-Change-Number: 84041
Gerrit-PatchSet: 5
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: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Attention is currently required from: Angel Pons, Arthur Heymans.
Hello Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/84042?usp=email
to look at the new patch set (#5).
The following approvals got outdated and were removed:
Code-Review+1 by Angel Pons
Change subject: soc/intel/pmclib.c: Make sure array_size is not 0
......................................................................
soc/intel/pmclib.c: Make sure array_size is not 0
On xeon-sp this is zero. With GCC LTO this triggers the
stringop-overread warning.
Change-Id: Ieee6e9bddc4e738eb560dd0e69dc3087ac9f5da6
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
---
M src/soc/intel/common/block/pmc/pmclib.c
1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/84042/5
--
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: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ieee6e9bddc4e738eb560dd0e69dc3087ac9f5da6
Gerrit-Change-Number: 84042
Gerrit-PatchSet: 5
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: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Attention is currently required from: Angel Pons, Arthur Heymans.
Hello Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/84043?usp=email
to look at the new patch set (#5).
The following approvals got outdated and were removed:
Code-Review+1 by Angel Pons
Change subject: util/cbfstool: Deal with how lld organizes loadable segments
......................................................................
util/cbfstool: Deal with how lld organizes loadable segments
LLD deals with loadable segments in a different manner than BFD. The
MemSiz of the .text loadable section is padded till the virtaddr of the
.car.data section. Since .text is not loaded in ENV_CAR this does not
matter.
Change-Id: I1a0541c8ea3dfbebfba83d505d84b6db12000723
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
---
M util/cbfstool/cbfs-mkstage.c
1 file changed, 8 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/43/84043/5
--
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: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I1a0541c8ea3dfbebfba83d505d84b6db12000723
Gerrit-Change-Number: 84043
Gerrit-PatchSet: 5
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: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Attention is currently required from: Angel Pons, Arthur Heymans.
Hello Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/84044?usp=email
to look at the new patch set (#5).
The following approvals got outdated and were removed:
Code-Review+1 by Angel Pons
Change subject: Makefile.mk: Suppress stack-usage LTO link warning
......................................................................
Makefile.mk: Suppress stack-usage LTO link warning
Suppress the following warning during linking with gcc:
src/drivers/spi/spi_flash.c: In function 'spi_flash_cmd_write':
src/drivers/spi/spi_flash.c:138:5: error: stack usage might be unbounded [-Werror=stack-usage=]
138 | int spi_flash_cmd_write(const struct spi_slave *spi, const u8 *cmd,
Change-Id: If08d6d543a4fcff07003af8d1f8dd59ab79f42f8
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
---
M Makefile.mk
1 file changed, 3 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/44/84044/5
--
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: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: If08d6d543a4fcff07003af8d1f8dd59ab79f42f8
Gerrit-Change-Number: 84044
Gerrit-PatchSet: 5
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: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Attention is currently required from: Angel Pons, Arthur Heymans, Jérémy Compostella.
Hello Angel Pons, Jérémy Compostella,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/84048?usp=email
to look at the new patch set (#8).
The following approvals got outdated and were removed:
Code-Review+1 by Angel Pons
Change subject: arch/x86/car.ld: Fix overlapping regions
......................................................................
arch/x86/car.ld: Fix overlapping regions
Make sure .car.data and .car.fspm_rc_heap don't overlap.
Clang with LLD does not like this.
Change-Id: I058bd6790afc313e06f1888e5b783d97b7e93b1e
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
---
M src/arch/x86/car.ld
1 file changed, 3 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/84048/8
--
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: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I058bd6790afc313e06f1888e5b783d97b7e93b1e
Gerrit-Change-Number: 84048
Gerrit-PatchSet: 8
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: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Attention is currently required from: Arthur Heymans, Jérémy Compostella.
Hello Jérémy Compostella,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/84049?usp=email
to look at the new patch set (#9).
Change subject: arch/x86/Kconfig: Fix LTO linking
......................................................................
arch/x86/Kconfig: Fix LTO linking
Somehow the trick for older binutils is also needed for LTO linking.
Change-Id: I1a6a5ef9dc6d824fa108681689a69df3faefd3c6
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
---
M src/arch/x86/Kconfig
1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/49/84049/9
--
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: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I1a6a5ef9dc6d824fa108681689a69df3faefd3c6
Gerrit-Change-Number: 84049
Gerrit-PatchSet: 9
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>