coreboot-gerrit November 2021

coreboot-gerrit@coreboot.org

1 participants
6394 discussions

Change in coreboot[master]: Revert "soc/intel/adl: Drop SGPM, RGPM and EGPM methods"
by Maulik V Vaghela (Code Review) 08 Nov '21

08 Nov '21

Attention is currently required from: Furquan Shaikh, Maulik V Vaghela, Tim Wawrzynczak, Meera Ravindranath, Angel Pons, Subrata Banik, Nick Vaccaro, Patrick Rudolph. Hello build bot (Jenkins), Furquan Shaikh, Tim Wawrzynczak, Meera Ravindranath, Angel Pons, Subrata Banik, Nick Vaccaro, Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/58811 to look at the new patch set (#2). Change subject: Revert "soc/intel/adl: Drop SGPM, RGPM and EGPM methods" ...................................................................... Revert "soc/intel/adl: Drop SGPM, RGPM and EGPM methods" This reverts commit 1399442289607acc5203fb12df64e9081b3c3aa4. Reason for revert: Some Cr50 chips with old firmware version (x.y.22) don't support long pulse interrupt command, requiring dynamic GPIO PM to be disabled to intercept short pulse interrupt. Due to this coreboot needs to expose SGPM, RGPM and EGPM ACPI methods to support power gating of GPIO communities from the kernel when dynamic GPIO PM is disabled. Change-Id: I2b5b00878062f8a499641d7a47db54ed078cd6cf --- M src/soc/intel/alderlake/acpi/gpio.asl 1 file changed, 41 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/58811/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/58811 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2b5b00878062f8a499641d7a47db54ed078cd6cf Gerrit-Change-Number: 58811 Gerrit-PatchSet: 2 Gerrit-Owner: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan.m.shaikh(a)gmail.com> Gerrit-Reviewer: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Furquan Shaikh <furquan.m.shaikh(a)gmail.com> Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/intel/elkhartlake: Introduce Intel PSE
by Werner Zeh (Code Review) 08 Nov '21

08 Nov '21

Attention is currently required from: Arthur Heymans, Nico Huber, Maulik V Vaghela, Paul Menzel, Mario Scheithauer, Angel Pons, Subrata Banik, Michael Niewöhner, Lean Sheng Tan, Patrick Rudolph, Felix Held. Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55367 ) Change subject: soc/intel/elkhartlake: Introduce Intel PSE ...................................................................... Patch Set 49: (4 comments) File src/soc/intel/elkhartlake/Kconfig: https://review.coreboot.org/c/coreboot/+/55367/comment/88e3ebdf_d7004861 PS49, Line 204: added to CBFS and loaded by FSP to run PSE. > Maybe a misunderstanding. We don't explicitly mention that a ramstage binary > is required to be added to CBFS, that an smihandler binary is required to be > added to CBFS etc. Hence, I assumed this "required" is referring to the manual > work necessary to add the binary to coreboot, which is what we should avoid > and replace with a coreboot-native solution right? I got 'required' more like it is needed to have the possible needed IPs working. This can be written more precise here. I can work with Sheng to point it out and you are wellcome to place your suggestions if you want, too. File src/soc/intel/elkhartlake/Makefile.inc: https://review.coreboot.org/c/coreboot/+/55367/comment/29dea86e_75c07d88 PS49, Line 66: > Well, no better solution from the top of my head, and I'm not paid > to find one. The basic requirement is that it will be build tested > by Jenkins. This can either be done by having a board port in the > tree that enables the option by default or by an explicit test- > config placed in the `configs/` dir. This particular code is responsible for putting the image into cbfs, which we do not have right now. So build-testing it without having the image around is tricky without cheating right now (I know, this is currently discussed heavily on the mailing list). And I am kind of opposed to cheating here (e.g. just taking a dummy-file for build-testing). I hope that we can convince Intel to release at least their current code to public, but this will take time to sort out. Would it be an option for you to have that as a TODO? File src/soc/intel/elkhartlake/fsp_params.c: https://review.coreboot.org/c/coreboot/+/55367/comment/46f97740_39e05907 PS49, Line 385: FSP_ARRAY_LOAD > > I do not see exactly how the macro itself hides the types from memcpy(). > > It doesn't, I said it hides the implementation (hides that memcpy() is used). OK, I see. > > > From gcc's point of view > > I'm more concerned about the reviewers and human readers point of view. The former version of it was even worse readable for the reviewer. So I guess the macro at least increases readability with the burden that the reader needs to know its implementation. > > > , after the preprocessor has done its job, the code looks like it was without the macro. And in addition we have '-Werror=sizeof-pointer-div' enabled so that using just two pointers as parameters to the macro will be caught. > > Not concerned about that. OK, then I got it wrong. > > > > > Could you please elaborate your concerns here a bit more so that it is more clear here? > > As a reviewer I couldn't see what is going on without looking into > the macro implementation and practically review that too. The macro > currently does a size check that seems insufficient to remedy the > use of memcpy(). I know FSP makes these things tedious, and I don't > know a sweet solution either. From a C developer's point of view > (who cares about errors made due to the lack of type checking), the > best solution seems to be to write explicit loops, copying the > values of the arrays one at a time. This way, nothing could go > wrong, type-wise. There's still the issue of comparing the length > of the arrays, I would do that in the header file where the arrays > are declared to keep the code readable. If one really wants to use > memcpy(), I suggest to check that the array lengths are exactly the > same and the size and alignment (or effective size of the whole > array) of the elements are the same. > > Note that calling memcpy() implicitly converts all pointers to > `void *`, this way bypassing the little type checking C offers. Understood. File src/soc/intel/elkhartlake/romstage/fsp_params.c: https://review.coreboot.org/c/coreboot/+/55367/comment/6ee532aa_f7537f27 PS49, Line 127: m_cfg->PchPseEnable = CONFIG(PSE_ENABLE); > Could we check for the file in CBFS here, so we could still boot […] Do we have a CBFS API that can check for a file without loading it? -- To view, visit https://review.coreboot.org/c/coreboot/+/55367 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifea08fb82fea18ef66bab04b3ce378e79a0afbf7 Gerrit-Change-Number: 55367 Gerrit-PatchSet: 49 Gerrit-Owner: Lean Sheng Tan <lean.sheng.tan(a)intel.com> Gerrit-Reviewer: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-CC: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Attention: Lean Sheng Tan <lean.sheng.tan(a)intel.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Mon, 08 Nov 2021 12:35:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/dedede/var/galtic: update Wifi SAR for convertibles
by Henry Sun (Code Review) 08 Nov '21

08 Nov '21

Attention is currently required from: Frank Chu, Marco Chen, Zhuohao Lee, Karthik Ramasubramanian. Henry Sun has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58734 ) Change subject: mb/google/dedede/var/galtic: update Wifi SAR for convertibles ...................................................................... Patch Set 6: (1 comment) File src/mainboard/google/dedede/variants/galtic/variant.c: https://review.coreboot.org/c/coreboot/+/58734/comment/ca6fcb80_5a50633a PS6, Line 27: galith should be gallop? Thanks. -- To view, visit https://review.coreboot.org/c/coreboot/+/58734 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If4203d176dd717fa62c88d9b4fab8a53847213fe Gerrit-Change-Number: 58734 Gerrit-PatchSet: 6 Gerrit-Owner: Frank Chu <frank_chu(a)pegatron.corp-partner.google.com> Gerrit-Reviewer: Frank Chu <frank_chu(a)pegatron.corp-partner.google.com> Gerrit-Reviewer: Henry Sun <henrysun(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Shou-Chieh Hsu <shouchieh(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Daniel Peng <daniel_peng(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: Marco Chen <marcochen(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Zhuohao Lee <zhuohao(a)google.com> Gerrit-Attention: Frank Chu <frank_chu(a)pegatron.corp-partner.google.com> Gerrit-Attention: Marco Chen <marcochen(a)google.com> Gerrit-Attention: Zhuohao Lee <zhuohao(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Mon, 08 Nov 2021 12:35:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel: generate SSDT instead of using GNVS for SGX
by Nico Huber (Code Review) 08 Nov '21

08 Nov '21

Attention is currently required from: Felix Singer, Pratik Prajapati, Furquan Shaikh, Paul Menzel, Angel Pons, Subrata Banik, Michael Niewöhner, Andrey Petrov, Kyösti Mälkki, Patrick Rudolph, Pratikkumar V Prajapati. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58394 ) Change subject: soc/intel: generate SSDT instead of using GNVS for SGX ...................................................................... Patch Set 8: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/58394 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Icf9f035e0c2b8617eef82fb043293bcb913e3012 Gerrit-Change-Number: 58394 Gerrit-PatchSet: 8 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan.m.shaikh(a)gmail.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Pratik Prajapati <pratik.prajapati(a)gmail.com> Gerrit-Reviewer: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Pratik Prajapati <pratik.prajapati(a)gmail.com> Gerrit-Attention: Furquan Shaikh <furquan.m.shaikh(a)gmail.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-Comment-Date: Mon, 08 Nov 2021 12:29:22 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel: move SGX ACPI code to block/acpi
by Michael Niewöhner (Code Review) 08 Nov '21

08 Nov '21

Attention is currently required from: Nico Huber, Angel Pons, Patrick Rudolph. Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58925 ) Change subject: soc/intel: move SGX ACPI code to block/acpi ...................................................................... Patch Set 4: (2 comments) File src/soc/intel/common/block/acpi/sgx.c: https://review.coreboot.org/c/coreboot/+/58925/comment/ddc2507d_9d5f4644 PS3, Line 6: #include <cpu/x86/msr.h> > I don't see anything about MSR in the code? Done https://review.coreboot.org/c/coreboot/+/58925/comment/76fef48e_2841cbc3 PS3, Line 8: #include <intelblocks/msr.h> > I don't see anything about MSR in the code? Done -- To view, visit https://review.coreboot.org/c/coreboot/+/58925 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I089d0ee97c37df2be060b5996183201bfa9b49ca Gerrit-Change-Number: 58925 Gerrit-PatchSet: 4 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.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: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Mon, 08 Nov 2021 12:22:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel: move SGX ACPI code to block/acpi
by build bot (Jenkins) (Code Review) 08 Nov '21

08 Nov '21

Attention is currently required from: Angel Pons, Patrick Rudolph. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58925 ) Change subject: soc/intel: move SGX ACPI code to block/acpi ...................................................................... Patch Set 4: (1 comment) File src/soc/intel/common/block/include/intelblocks/msr.h: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-132580): https://review.coreboot.org/c/coreboot/+/58925/comment/8271bafe_df35bf5e PS4, Line 112: #endif /* SOC_INTEL_COMMON_MSR_H */ adding a line without newline at end of file -- To view, visit https://review.coreboot.org/c/coreboot/+/58925 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I089d0ee97c37df2be060b5996183201bfa9b49ca Gerrit-Change-Number: 58925 Gerrit-PatchSet: 4 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Mon, 08 Nov 2021 12:22:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google,intel: Split chromeos.c files
by build bot (Jenkins) (Code Review) 08 Nov '21

08 Nov '21

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59011 ) Change subject: mb/google,intel: Split chromeos.c files ...................................................................... Patch Set 3: (1 comment) File src/mainboard/google/stout/bootmode.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-132579): https://review.coreboot.org/c/coreboot/+/59011/comment/cc834474_90d1dde7 PS3, Line 44: printk(BIOS_SPEW,"%s: EC status:%#x RTC_BAT: %x\n", space required after that ',' (ctx:VxV) -- To view, visit https://review.coreboot.org/c/coreboot/+/59011 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I71a02c5fa1b256316b86b673660bf22dfd284f7f Gerrit-Change-Number: 59011 Gerrit-PatchSet: 3 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Mon, 08 Nov 2021 12:17:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google,intel: Split chromeos.c files
by Kyösti Mälkki (Code Review) 08 Nov '21

08 Nov '21

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/59011 to look at the new patch set (#3). Change subject: mb/google,intel: Split chromeos.c files ...................................................................... mb/google,intel: Split chromeos.c files Move all the low-level GPIO support in bootmode.c files and build them for all stages. Keep ChromeOS related ACPI and lbtable support in chromeos.c files and build them only for ramstage. Change-Id: I71a02c5fa1b256316b86b673660bf22dfd284f7f Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> --- M src/mainboard/google/auron/Makefile.inc A src/mainboard/google/auron/bootmode.c M src/mainboard/google/auron/chromeos.c M src/mainboard/google/beltino/Makefile.inc A src/mainboard/google/beltino/bootmode.c M src/mainboard/google/beltino/chromeos.c M src/mainboard/google/butterfly/Makefile.inc A src/mainboard/google/butterfly/bootmode.c M src/mainboard/google/butterfly/chromeos.c M src/mainboard/google/jecht/Makefile.inc A src/mainboard/google/jecht/bootmode.c M src/mainboard/google/jecht/chromeos.c M src/mainboard/google/link/Makefile.inc A src/mainboard/google/link/bootmode.c M src/mainboard/google/link/chromeos.c M src/mainboard/google/parrot/Makefile.inc A src/mainboard/google/parrot/bootmode.c M src/mainboard/google/parrot/chromeos.c M src/mainboard/google/slippy/Makefile.inc A src/mainboard/google/slippy/bootmode.c M src/mainboard/google/slippy/chromeos.c M src/mainboard/google/stout/Makefile.inc A src/mainboard/google/stout/bootmode.c M src/mainboard/google/stout/chromeos.c M src/mainboard/intel/baskingridge/Makefile.inc A src/mainboard/intel/baskingridge/bootmode.c M src/mainboard/intel/baskingridge/chromeos.c M src/mainboard/intel/emeraldlake2/Makefile.inc A src/mainboard/intel/emeraldlake2/bootmode.c M src/mainboard/intel/emeraldlake2/chromeos.c M src/mainboard/intel/strago/Makefile.inc A src/mainboard/intel/strago/bootmode.c M src/mainboard/intel/strago/chromeos.c M src/mainboard/samsung/lumpy/Makefile.inc A src/mainboard/samsung/lumpy/bootmode.c M src/mainboard/samsung/lumpy/chromeos.c M src/mainboard/samsung/stumpy/Makefile.inc A src/mainboard/samsung/stumpy/bootmode.c M src/mainboard/samsung/stumpy/chromeos.c 39 files changed, 345 insertions(+), 278 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/59011/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/59011 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I71a02c5fa1b256316b86b673660bf22dfd284f7f Gerrit-Change-Number: 59011 Gerrit-PatchSet: 3 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: mb/google,intel: Split chromeos.c files
by build bot (Jenkins) (Code Review) 08 Nov '21

08 Nov '21

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59011 ) Change subject: mb/google,intel: Split chromeos.c files ...................................................................... Patch Set 2: (1 comment) File src/mainboard/google/stout/bootmode.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-132576): https://review.coreboot.org/c/coreboot/+/59011/comment/fedf5123_e1e96386 PS2, Line 44: printk(BIOS_SPEW,"%s: EC status:%#x RTC_BAT: %x\n", space required after that ',' (ctx:VxV) -- To view, visit https://review.coreboot.org/c/coreboot/+/59011 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I71a02c5fa1b256316b86b673660bf22dfd284f7f Gerrit-Change-Number: 59011 Gerrit-PatchSet: 2 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Mon, 08 Nov 2021 11:28:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/elkhartlake: Introduce Intel PSE
by Nico Huber (Code Review) 08 Nov '21

08 Nov '21

Attention is currently required from: Arthur Heymans, Maulik V Vaghela, Paul Menzel, Mario Scheithauer, Angel Pons, Subrata Banik, Michael Niewöhner, Lean Sheng Tan, Werner Zeh, Patrick Rudolph, Felix Held. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55367 ) Change subject: soc/intel/elkhartlake: Introduce Intel PSE ...................................................................... Patch Set 49: (3 comments) File src/soc/intel/elkhartlake/Kconfig: https://review.coreboot.org/c/coreboot/+/55367/comment/953e79b4_f04011d8 PS49, Line 204: added to CBFS and loaded by FSP to run PSE. > "A PSE FW binary is required be added to CBFS and loaded by FSP to run PSE." […] Maybe a misunderstanding. We don't explicitly mention that a ramstage binary is required to be added to CBFS, that an smihandler binary is required to be added to CBFS etc. Hence, I assumed this "required" is referring to the manual work necessary to add the binary to coreboot, which is what we should avoid and replace with a coreboot-native solution right? File src/soc/intel/elkhartlake/Makefile.inc: https://review.coreboot.org/c/coreboot/+/55367/comment/9131c748_62fe2bc4 PS49, Line 66: > The issue with your approach would be the size checking. […] Well, no better solution from the top of my head, and I'm not paid to find one. The basic requirement is that it will be build tested by Jenkins. This can either be done by having a board port in the tree that enables the option by default or by an explicit test- config placed in the `configs/` dir. File src/soc/intel/elkhartlake/fsp_params.c: https://review.coreboot.org/c/coreboot/+/55367/comment/6c08ffa8_8c08f6ab PS49, Line 385: FSP_ARRAY_LOAD > I do not see exactly how the macro itself hides the types from memcpy(). It doesn't, I said it hides the implementation (hides that memcpy() is used). > From gcc's point of view I'm more concerned about the reviewers and human readers point of view. > , after the preprocessor has done its job, the code looks like it was without the macro. And in addition we have '-Werror=sizeof-pointer-div' enabled so that using just two pointers as parameters to the macro will be caught. Not concerned about that. > > Could you please elaborate your concerns here a bit more so that it is more clear here? As a reviewer I couldn't see what is going on without looking into the macro implementation and practically review that too. The macro currently does a size check that seems insufficient to remedy the use of memcpy(). I know FSP makes these things tedious, and I don't know a sweet solution either. From a C developer's point of view (who cares about errors made due to the lack of type checking), the best solution seems to be to write explicit loops, copying the values of the arrays one at a time. This way, nothing could go wrong, type-wise. There's still the issue of comparing the length of the arrays, I would do that in the header file where the arrays are declared to keep the code readable. If one really wants to use memcpy(), I suggest to check that the array lengths are exactly the same and the size and alignment (or effective size of the whole array) of the elements are the same. Note that calling memcpy() implicitly converts all pointers to `void *`, this way bypassing the little type checking C offers. -- To view, visit https://review.coreboot.org/c/coreboot/+/55367 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifea08fb82fea18ef66bab04b3ce378e79a0afbf7 Gerrit-Change-Number: 55367 Gerrit-PatchSet: 49 Gerrit-Owner: Lean Sheng Tan <lean.sheng.tan(a)intel.com> Gerrit-Reviewer: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-CC: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Attention: Lean Sheng Tan <lean.sheng.tan(a)intel.com> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Mon, 08 Nov 2021 11:28:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2021