coreboot-gerrit March 2024

coreboot-gerrit@coreboot.org

1 participants
3668 discussions

[S] Change in coreboot[main]: ec/hp/kbc1126/acpi: Drop unnecessary _STA methods
by Nico Huber (Code Review) 16 Mar '24

16 Mar '24

Attention is currently required from: Nicholas Sudsgaard. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81206?usp=email ) Change subject: ec/hp/kbc1126/acpi: Drop unnecessary _STA methods ...................................................................... Patch Set 5: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/81206?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: I0390767aa866e322c762038c12116a15b280af1a Gerrit-Change-Number: 81206 Gerrit-PatchSet: 5 Gerrit-Owner: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com> Gerrit-Comment-Date: Sat, 16 Mar 2024 13:56:37 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: ec/hp/kbc1126/acpi: Drop unnecessary method arguments
by Nico Huber (Code Review) 16 Mar '24

16 Mar '24

Attention is currently required from: Nicholas Sudsgaard, Paul Menzel, Tim Wawrzynczak. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81205?usp=email ) Change subject: ec/hp/kbc1126/acpi: Drop unnecessary method arguments ...................................................................... Patch Set 6: Code-Review+2 (1 comment) Patchset: PS3: > I think it would be fair to assume anyone writing ASL would know about the defaults up to `NotSerial […] I wouldn't say "know" but I expect most people would guess right (if there is an serialization option, why would it be on by default?). :) -- To view, visit https://review.coreboot.org/c/coreboot/+/81205?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: Ic24e004500a7fa2a5a5b38a3f6f0e13e4ce7dfac Gerrit-Change-Number: 81205 Gerrit-PatchSet: 6 Gerrit-Owner: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com> Gerrit-Comment-Date: Sat, 16 Mar 2024 13:56:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Tim Wawrzynczak <inforichland(a)gmail.com> Comment-In-Reply-To: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: drivers/intel/gma/acpi: Limit OpRegion size
by Nico Huber (Code Review) 16 Mar '24

16 Mar '24

Attention is currently required from: Patrick Rudolph. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81295?usp=email ) Change subject: drivers/intel/gma/acpi: Limit OpRegion size ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/81295?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: Ia2affa799e5cd84c0a03330e0f78919755f0e8ac Gerrit-Change-Number: 81295 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Comment-Date: Sat, 16 Mar 2024 13:53:18 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: soc/intel/mtl: Enable RAMTOP caching at SoC level for MTL devices
by Stefan Reinauer (Code Review) 16 Mar '24

16 Mar '24

Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Subrata Banik, Tarun. Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81271?usp=email ) Change subject: soc/intel/mtl: Enable RAMTOP caching at SoC level for MTL devices ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/81271?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: I3aa3a83c190d0a0e93c267222a9dca0ac7651f9c Gerrit-Change-Number: 81271 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Comment-Date: Sat, 16 Mar 2024 13:50:28 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: soc/intel/cmn/ramtop: Refactor MTRR handling for RAMTOP range
by Stefan Reinauer (Code Review) 16 Mar '24

16 Mar '24

Attention is currently required from: Subrata Banik. Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81269?usp=email ) Change subject: soc/intel/cmn/ramtop: Refactor MTRR handling for RAMTOP range ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/81269?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: Ia9a8f0d37d581b05c19ea7f9b1a07933caa956d4 Gerrit-Change-Number: 81269 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Comment-Date: Sat, 16 Mar 2024 13:48:23 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: arch/x86: Add API to check if cache sets are power-of-two
by Stefan Reinauer (Code Review) 16 Mar '24

16 Mar '24

Attention is currently required from: Jérémy Compostella, Subrata Banik. Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81268?usp=email ) Change subject: arch/x86: Add API to check if cache sets are power-of-two ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/81268?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: I819e0d1aeb4c1dbe1cdf3115b2e172588a6e8da5 Gerrit-Change-Number: 81268 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Comment-Date: Sat, 16 Mar 2024 13:46:09 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: drivers/intel/gma/acpi: Limit OpRegion size
by Patrick Rudolph (Code Review) 16 Mar '24

16 Mar '24

Attention is currently required from: Nico Huber. Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81295?usp=email ) Change subject: drivers/intel/gma/acpi: Limit OpRegion size ...................................................................... Patch Set 1: (1 comment) File src/drivers/intel/gma/acpi/configure_brightness_levels.asl: https://review.coreboot.org/c/coreboot/+/81295/comment/550e25da_377d281d : PS1, Line 14: OperationRegion (OPRG, SystemMemory, ASLS, 0x400) > Please leave a comment, documenting why we do this (and which generations […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/81295?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: Ia2affa799e5cd84c0a03330e0f78919755f0e8ac Gerrit-Change-Number: 81295 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Sat, 16 Mar 2024 13:41:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: drivers/intel/gma/acpi: Limit OpRegion size
by Patrick Rudolph (Code Review) 16 Mar '24

16 Mar '24

Attention is currently required from: Nico Huber, Patrick Rudolph. Hello Nico Huber, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/81295?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Code-Review+1 by Nico Huber, Verified+1 by build bot (Jenkins) Change subject: drivers/intel/gma/acpi: Limit OpRegion size ...................................................................... drivers/intel/gma/acpi: Limit OpRegion size Limit the ACPI OpRegion to cover only MBOX3. This seems to fix BSOD errors seen on Windows 10/11 as reported at [1]. 1: https://ticket.coreboot.org/issues/327 Change-Id: Ia2affa799e5cd84c0a03330e0f78919755f0e8ac Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> --- M src/drivers/intel/gma/acpi/configure_brightness_levels.asl 1 file changed, 5 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/81295/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/81295?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: Ia2affa799e5cd84c0a03330e0f78919755f0e8ac Gerrit-Change-Number: 81295 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in coreboot[main]: [RFC] region: Introduce region_create() functions
by Nico Huber (Code Review) 16 Mar '24

16 Mar '24

Attention is currently required from: Andrey Petrov, Julius Werner, Jérémy Compostella, Martin L Roth, Maximilian Brune, Ronak Kanabar. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79905?usp=email ) Change subject: [RFC] region: Introduce region_create() functions ...................................................................... Patch Set 6: (3 comments) Patchset: PS6: Max, I'll try to push an update that keeps the original two-create-functions idea in the next few days. I don't want to ignore you. But it feels like we are getting nowhere when I try to imagine what you have in mind and probably imagine it wrong. When the whole patch train is cleaned up, it should be easy to write an alternative version of this patch that uses a separate validity function. Then we could decide which version we like better. Does that sound ok? File src/commonlib/include/commonlib/region.h: https://review.coreboot.org/c/coreboot/+/79905/comment/2be928b8_76e46bf8 : PS1, Line 103: static inline int region_create_untrusted( > Well you have to repeat that for all sources that we deem "untrusted". I don't think there are many of them in coreboot. I personally can only think of SMM and maybe exception handlers in general (I may be missing a few right now). Plus potential future cases. > But I would like to see a `input_is_valid` function of some kind at the beginning of all our untrusted sources anyway. Because these input values may be used for something else than `region_create` and are then unsafe again. We cannot that assume that the input values are only used for a `region_create`. I would feel better to first check all input values and after that point assume that they are "safe" and then use the usual functions. Of course that requires that the input values are always correctly checked, which may be not 100% realistic. I'm sorry, I really fail to imagine any comprehensive solution in this direction that would make sense. IIUC, you mean to implement an input_is_valid() that is always right, independent from the region_create() use case? But that seems impossible. Validity always requires context. Currently, the region API is limited by `size_t`, but other APIs may have stronger requirements like things always fitting into 32-bit. I also seems like we'd lose cohesion this way. If we check for an overflow that can happen because of an implementation detail of the region API, that should be obvious and not happen somewhere where the reader doesn't see the region API being used. File util/cbfstool/cbfstool.c: https://review.coreboot.org/c/coreboot/+/79905/comment/f243ca76_77dd3825 : PS1, Line 334: if (region_create_untrusted( > IMO command-line input doesn't count as untrusted. […] We don't have to call it unstrusted. How about `create_region_checked()`? Just anything that says this is the version that checks and can fail. You could also see it like this: Do you trust the command line to be free of typos? -- To view, visit https://review.coreboot.org/c/coreboot/+/79905?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: I4ae3e6274c981c9ab4fb1263c2a72fa68ef1c32b Gerrit-Change-Number: 79905 Gerrit-PatchSet: 6 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-CC: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Comment-Date: Sat, 16 Mar 2024 13:37:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Maximilian Brune <maximilian.brune(a)9elements.com> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: drivers/intel/gma/acpi: Limit OpRegion size
by Nico Huber (Code Review) 16 Mar '24

16 Mar '24

Attention is currently required from: Patrick Rudolph. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81295?usp=email ) Change subject: drivers/intel/gma/acpi: Limit OpRegion size ...................................................................... Patch Set 1: Code-Review+1 (1 comment) File src/drivers/intel/gma/acpi/configure_brightness_levels.asl: https://review.coreboot.org/c/coreboot/+/81295/comment/3a634281_25fd7191 : PS1, Line 14: OperationRegion (OPRG, SystemMemory, ASLS, 0x400) Please leave a comment, documenting why we do this (and which generations are affected). If the spec says it's 8KiB, there's a high chance somebody would want to "fix" this again later and we'd be in a loop. -- To view, visit https://review.coreboot.org/c/coreboot/+/81295?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: Ia2affa799e5cd84c0a03330e0f78919755f0e8ac Gerrit-Change-Number: 81295 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Comment-Date: Sat, 16 Mar 2024 12:45:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit March 2024