coreboot-gerrit February 2024

coreboot-gerrit@coreboot.org

1 participants
3067 discussions

[XS] Change in coreboot[main]: drivers/intel/gma: Allow SPARK function with side effects
by Nico Huber (Code Review) 29 Feb '24

29 Feb '24

Attention is currently required from: Nico Huber, Thomas Heijligen. Hello Thomas Heijligen, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/80616?usp=email to look at the new patch set (#3). The following approvals got outdated and were removed: Code-Review+2 by Thomas Heijligen, Verified+1 by build bot (Jenkins) The change is no longer submittable: Code-Review and Verified are unsatisfied now. Change subject: drivers/intel/gma: Allow SPARK function with side effects ...................................................................... drivers/intel/gma: Allow SPARK function with side effects Explicitly specifying the aspect `Side_Effects' is necessary for GCC toolchains from 14.0 on. As older toolchains don't know the aspect, we have to silence a warning about it, though. Change-Id: I1eb879f57437587dc11d879fcc4042a70d384786 Signed-off-by: Nico Huber <nico.huber(a)secunet.com> --- M gnat.adc M src/drivers/intel/gma/gma.ads 2 files changed, 5 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/80616/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/80616?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I1eb879f57437587dc11d879fcc4042a70d384786 Gerrit-Change-Number: 80616 Gerrit-PatchSet: 3 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newpatchset

1 0

[S] Change in coreboot[main]: lib/program.ld: Make (NOLOAD) and to_load more explicit
by Arthur Heymans (Code Review) 29 Feb '24

29 Feb '24

Arthur Heymans has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/80803?usp=email ) Change subject: lib/program.ld: Make (NOLOAD) and to_load more explicit ...................................................................... lib/program.ld: Make (NOLOAD) and to_load more explicit (NOLOAD) indicates that the section occupies no space in the file, but does take up space in memory during process execution. It's typically used for bss sections which contain uninitialized global/static variables. to_load makes sure the section is part of the program headers. This is needed for instance with relocatable stages to know how much memory the program will use. This fixes commit 99bf23c9e73c ("lib: Explicitly declare heap as NOLOAD"), which broke setups with a relocatable ramstage as the heap size was not accounted for when allocating memory in cbmem. The alloc code could trash higher cbmem entries in that case. Change-Id: Ic14543ba580abe7a34c69bba714eae8cce504977 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/lib/program.ld 1 file changed, 5 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/80803/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/80803?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ic14543ba580abe7a34c69bba714eae8cce504977 Gerrit-Change-Number: 80803 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in coreboot[main]: lib: Declare heap in assembly
by Arthur Heymans (Code Review) 29 Feb '24

29 Feb '24

Attention is currently required from: Julius Werner, Maximilian Brune, Nico Huber. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80776?usp=email ) Change subject: lib: Declare heap in assembly ...................................................................... Patch Set 3: (1 comment) File src/lib/program.ld: https://review.coreboot.org/c/coreboot/+/80776/comment/5072e9a0_07ca8c42 : PS3, Line 141: } > > I think a simpler solution is to just add a `: to_load` here. (We already have that for the `.text` section as well, although I'm not sure why. It is implicit everywhere else too, so maybe we should just make it explicit everywhere to reduce ambiguity.) > > That works indeed. It's a bit confusing to have (NOLOAD) and to_load for the same section? CB:80803 does it. -- To view, visit https://review.coreboot.org/c/coreboot/+/80776?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I67cb5ce886fda313e0720b0bc7c6e66e4aae45fa Gerrit-Change-Number: 80776 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Tim Van Patten <timvp(a)google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Comment-Date: Thu, 29 Feb 2024 14:10:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: lib/program.ld: Make (NOLOAD) and to_load more explicit
by Arthur Heymans (Code Review) 29 Feb '24

29 Feb '24

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/80803?usp=email ) Change subject: lib/program.ld: Make (NOLOAD) and to_load more explicit ...................................................................... lib/program.ld: Make (NOLOAD) and to_load more explicit (NOLOAD) indicates that the section occupies no space in the file, but does take up space in memory during process execution. It's typically used for bss sections which contain uninitialized global/static variables. to_load makes sure the section is part of the program headers. This is needed for instance with relocatable stages to know how much memory the program will use. This fixes commit 99bf23c9e73c, which broke setups with a relocatable ramstage as the heap size was not accounted for when allocating memory in cbmem. The alloc code could trash higher cbmem entries in that case. Change-Id: Ic14543ba580abe7a34c69bba714eae8cce504977 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/lib/program.ld 1 file changed, 5 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/80803/1 diff --git a/src/lib/program.ld b/src/lib/program.ld index 6d72d9e..1784447 100644 --- a/src/lib/program.ld +++ b/src/lib/program.ld @@ -68,7 +68,7 @@ LONG(0); LONG(0); __CTOR_END__ = .; -} +} : to_load #endif /* Include data, bss, and heap in that order. Not defined for all stages. */ @@ -113,11 +113,11 @@ . = ALIGN(ARCH_POINTER_ALIGN_SIZE); _edata = .; RECORD_SIZE(data) -} +} : to_load #endif #if !ENV_SEPARATE_DATA_AND_BSS -.bss . : { +.bss . (NOLOAD) : { . = ALIGN(ARCH_POINTER_ALIGN_SIZE); _bss = .; *(.bss) @@ -127,7 +127,7 @@ . = ALIGN(ARCH_POINTER_ALIGN_SIZE); _ebss = .; RECORD_SIZE(bss) -} +} : to_load #endif #if ENV_HAS_HEAP_SECTION @@ -138,7 +138,7 @@ . = ALIGN(ARCH_POINTER_ALIGN_SIZE); _eheap = .; RECORD_SIZE(heap) -} +} : to_load #endif #if ENV_RAMSTAGE && CONFIG(ASAN_IN_RAMSTAGE) -- To view, visit https://review.coreboot.org/c/coreboot/+/80803?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ic14543ba580abe7a34c69bba714eae8cce504977 Gerrit-Change-Number: 80803 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newchange

1 0

[XS] Change in coreboot[main]: [TEST] drivers/intel/gma: Allow SPARK function with side effects
by Thomas Heijligen (Code Review) 29 Feb '24

29 Feb '24

Attention is currently required from: Nico Huber. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80616?usp=email ) Change subject: [TEST] drivers/intel/gma: Allow SPARK function with side effects ...................................................................... Patch Set 2: (1 comment) File src/drivers/intel/gma/gma.ads: https://review.coreboot.org/c/coreboot/+/80616/comment/557f0327_13d1ae15 : PS2, Line 18: > ``` […] Done, in favor of gnat.adc -- To view, visit https://review.coreboot.org/c/coreboot/+/80616?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I1eb879f57437587dc11d879fcc4042a70d384786 Gerrit-Change-Number: 80616 Gerrit-PatchSet: 2 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Thu, 29 Feb 2024 14:09:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: [TEST] drivers/intel/gma: Allow SPARK function with side effects
by Thomas Heijligen (Code Review) 29 Feb '24

29 Feb '24

Attention is currently required from: Nico Huber. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80616?usp=email ) Change subject: [TEST] drivers/intel/gma: Allow SPARK function with side effects ...................................................................... Patch Set 2: Code-Review+2 (1 comment) File src/drivers/intel/gma/gma.ads: https://review.coreboot.org/c/coreboot/+/80616/comment/3121f53f_708db16c : PS2, Line 18: ``` pragma Warnings (GNAT, Off, """Side_Effects"" is not a valid aspect identifier"); ``` Only the warning in the function above should be turned off. -- To view, visit https://review.coreboot.org/c/coreboot/+/80616?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I1eb879f57437587dc11d879fcc4042a70d384786 Gerrit-Change-Number: 80616 Gerrit-PatchSet: 2 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Thu, 29 Feb 2024 14:04:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: lib: Declare heap in assembly
by Arthur Heymans (Code Review) 29 Feb '24

29 Feb '24

Attention is currently required from: Julius Werner, Maximilian Brune, Nico Huber. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80776?usp=email ) Change subject: lib: Declare heap in assembly ...................................................................... Patch Set 3: (1 comment) File src/lib/program.ld: https://review.coreboot.org/c/coreboot/+/80776/comment/3880b474_42bde7ed : PS3, Line 141: } > I think a simpler solution is to just add a `: to_load` here. (We already have that for the `.text` section as well, although I'm not sure why. It is implicit everywhere else too, so maybe we should just make it explicit everywhere to reduce ambiguity.) That works indeed. It's a bit confusing to have (NOLOAD) and to_load for the same section? -- To view, visit https://review.coreboot.org/c/coreboot/+/80776?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I67cb5ce886fda313e0720b0bc7c6e66e4aae45fa Gerrit-Change-Number: 80776 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Tim Van Patten <timvp(a)google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Comment-Date: Thu, 29 Feb 2024 13:59:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

[XL] Change in coreboot[main]: util: add smmstoretool for editing SMMSTORE
by Sergii Dmytruk (Code Review) 29 Feb '24

29 Feb '24

Attention is currently required from: Filip Lewiński, Michał Kopeć, Michał Żygowski, Sergii Dmytruk. Hello Filip Lewiński, Michał Kopeć, Michał Żygowski, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/79080?usp=email to look at the new patch set (#8). The following approvals got outdated and were removed: Code-Review+2 by Michał Żygowski, Verified+1 by build bot (Jenkins) The change is no longer submittable: Code-Review and Verified are unsatisfied now. Change subject: util: add smmstoretool for editing SMMSTORE ...................................................................... util: add smmstoretool for editing SMMSTORE Offline SMMSTORE variable modification tool. Can be used to pre-configure ROM image or debug EFI state stored in a dump. Change-Id: I6c1c06f1d0c39c13b5be76a3070f09b715aca6e0 Signed-off-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> --- M Documentation/util.md A Documentation/util/smmstoretool/index.md M util/README.md A util/smmstoretool/.gitignore A util/smmstoretool/Makefile A util/smmstoretool/data.c A util/smmstoretool/data.h A util/smmstoretool/description.md A util/smmstoretool/fv.c A util/smmstoretool/fv.h A util/smmstoretool/guids.c A util/smmstoretool/guids.h A util/smmstoretool/main.c A util/smmstoretool/storage.c A util/smmstoretool/storage.h A util/smmstoretool/udk2017.h A util/smmstoretool/utils.c A util/smmstoretool/utils.h A util/smmstoretool/vs.c A util/smmstoretool/vs.h M util/util_readme/post_util.md 21 files changed, 1,730 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/79080/8 -- To view, visit https://review.coreboot.org/c/coreboot/+/79080?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I6c1c06f1d0c39c13b5be76a3070f09b715aca6e0 Gerrit-Change-Number: 79080 Gerrit-PatchSet: 8 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Filip Lewiński Gerrit-Reviewer: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Filip Lewiński Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in coreboot[main]: device/pnp_device: fix log levels for unassigned resource messages
by Felix Held (Code Review) 29 Feb '24

29 Feb '24

Attention is currently required from: Nico Huber, Paul Menzel. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80774?usp=email ) Change subject: device/pnp_device: fix log levels for unassigned resource messages ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/80774/comment/5d6ca826_d062887c : PS1, Line 7: fix log levels > Maybe more specific: […] "device/pnp_device: Use notice log level for all unassigned resource messages" doesn't fit into 72 chars also changing the log level to notice in all cases is what the patch does, but the main intention is to fix a regression -- To view, visit https://review.coreboot.org/c/coreboot/+/80774?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I232e60ef7ae672e18cc1837b8e6a0427d01c142b Gerrit-Change-Number: 80774 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Thu, 29 Feb 2024 13:26:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: soc/intel/xeon_sp: Drop unused MACROs
by Felix Held (Code Review) 29 Feb '24

29 Feb '24

Attention is currently required from: Arthur Heymans, Christian Walter, Johnny Lin, Patrick Rudolph, Tim Chu. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80096?usp=email ) The change is no longer submittable: All-Comments-Resolved is unsatisfied now. Change subject: soc/intel/xeon_sp: Drop unused MACROs ...................................................................... Patch Set 12: (1 comment) File src/soc/intel/xeon_sp/cpx/include/soc/pci_devs.h: https://review.coreboot.org/c/coreboot/+/80096/comment/9e5f0075_01484cb8 : PS12, Line 66: #define PCU_DEV_CR3(bus) _PCU_DEV(bus, PCU_CR3_FUN) this had already been fixed in patchset 11, so patchset 12 is a regression -- To view, visit https://review.coreboot.org/c/coreboot/+/80096?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I4067a1940f6cb3ee6d40c784877d7906495251a4 Gerrit-Change-Number: 80096 Gerrit-PatchSet: 12 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.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: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-CC: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.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: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Thu, 29 Feb 2024 13:17:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit February 2024