coreboot-gerrit November 2023

coreboot-gerrit@coreboot.org

1 participants
3036 discussions

[S] Change in coreboot[main]: Kconfig: Restore HEAP_SIZE overrides greater than 512 KiB
by Nico Huber (Code Review) 01 Nov '23

01 Nov '23

Attention is currently required from: Bill XIE, Felix Held, Fred Reitberger, Jason Glenesk, Matt DeVillier. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78815?usp=email ) Change subject: Kconfig: Restore HEAP_SIZE overrides greater than 512 KiB ...................................................................... Patch Set 1: Code-Review+1 (1 comment) Patchset: PS1: I would put this commit first, so there are no further, intermediate regressions. -- To view, visit https://review.coreboot.org/c/coreboot/+/78815?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: I78cd56b0683dd2ef94319a98a720f2da58151626 Gerrit-Change-Number: 78815 Gerrit-PatchSet: 1 Gerrit-Owner: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Wed, 01 Nov 2023 12:59:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: Kconfig: Decrease default HEAP_SIZE from 1 MiB to 512 KiB
by Nico Huber (Code Review) 01 Nov '23

01 Nov '23

Attention is currently required from: Bill XIE, Felix Held, Fred Reitberger, Jason Glenesk, Julius Werner, Kyösti Mälkki, Matt DeVillier, Patrick Georgi, Paul Menzel, Subrata Banik, Werner Zeh. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78623?usp=email ) Change subject: Kconfig: Decrease default HEAP_SIZE from 1 MiB to 512 KiB ...................................................................... Patch Set 10: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/78623?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: Ib1de1eb8487df5bdf004e544d40667833a291515 Gerrit-Change-Number: 78623 Gerrit-PatchSet: 10 Gerrit-Owner: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)9elements.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-CC: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)9elements.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Wed, 01 Nov 2023 12:58:29 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[L] Change in coreboot[main]: acpi: Add PPTT support
by Nico Huber (Code Review) 01 Nov '23

01 Nov '23

Attention is currently required from: Arthur Heymans, Cliff Huang, David Milosevic, Lance Zhao, Maximilian Brune, Patrick Rudolph, Tim Wawrzynczak. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78071?usp=email ) Change subject: acpi: Add PPTT support ...................................................................... Patch Set 8: (7 comments) Patchset: PS8: This is nice code overall. Please don't mind the long comments, these ACPI things are sometimes hard to get right from the start. File src/acpi/acpi_pptt.c: https://review.coreboot.org/c/coreboot/+/78071/comment/892443d4_4e280bd2 : PS6, Line 66: > Is that really necessary? I mean, Jenkins does not complain and it really is only a matter of prefer […] I agree with Arthur here. While the style guide is not explicit about this, it does argue against unnecessary empty lines. It's also uncommon enough to make readers, who are used to the kernel (and coreboot) style, stumble (for maybe a second, I was wondering why Gerrit shows the code like this). File src/acpi/acpi_pptt.c: https://review.coreboot.org/c/coreboot/+/78071/comment/7835e789_1f4fdeee : PS8, Line 13: * Disclaimer: : * : * The generated PPTT table might not be optimized for space, since this : * implementation creates unique resources (caches) for each CPU provided by : * the topology tree. Going by the ACPI 6.4 spec, this is fine. We do this to : * avoid further edge cases and keep the logic as simple as possible. : * : * "though less space efficient, it is also acceptable to declare a node : * for each instance of a resource. In the example above, it would be legal to : * declare an L1 for each processor." : * : * "Compaction of identical resources must be avoided if an implementation requires : * any resource instance to be referenced uniquely. For example, in the above example, : * the L1 resource of each processor must be declared using a dedicated structure to : * permit unique references to it." : * I found this to be more confusing than helpful. The quotes reference examples that were not quoted, hence provide no value without reading the actual spec. My first impression was that this is about listing, for instance, shared caches more than once. But it is not. What the spec describes is the possibility to compact individual resources that have the exact same characteristics. There is a related quirk that is, IMO, much more worth to mention: The current structures don't allow to converge lists (cf. Fig. 5.15: Cache Type Structure - Type 1 Example; here, L1D and L1I should point to the same L2 object as next level). I'm not sure if that is something for a comment or something to fix. https://review.coreboot.org/c/coreboot/+/78071/comment/67a4c1f0_0449eecf : PS8, Line 40: * the difference between the start of the PPTT table Nit, I would call it `offset`. And TBH, I don't think we need to say more than that "References are the offset from the start of the PPTT table.". Drawing the picture here might make things more confusing, IMO, but I don't mind. https://review.coreboot.org/c/coreboot/+/78071/comment/660304db_ae4122d3 : PS8, Line 56: u32 It's uncommon to use a fixed-size integer unless you need one (e.g. for an external interface). Simply `unsigned int` would be preferred. https://review.coreboot.org/c/coreboot/+/78071/comment/f45fc417_e93d239b : PS8, Line 127: setup_topology(node->sibling, parent_ref, current); This is just walking the list without any reference to the current setup_topology()'s context, right? Recursion is generally something that should only be used if necessary. And if we can avoid it here, we should. You can probably see clearly what is going on here, I assume Arthur and Patrick, too. But the next person who gets to edit this code in a year or so might not. And that's when bugs sneak in when the code isn't written as simple as possible. I hope I'm not wrong with this. Could this be written as follows, with only one recursion where necessary? ``` static void setup_topology(const struct pptt_topology *node, const u32 parent_ref, unsigned long *current) { for (; node; node = node->sibling) { const u32 cpu_ref = new_pptt_cpu(current, node, parent_ref); setup_topology(node->child, cpu_ref, current); } } ``` File src/include/acpi/acpi.h: https://review.coreboot.org/c/coreboot/+/78071/comment/369a4823_562fd0df : PS3, Line 1442: structures > > It looks like other platforms do not have custom data structures and instead call a function to le […] I first leant to agree with Arthur, but it seems more delicate than one might think. What we would want here is to design structures that are nicer to handle in C but can still express the same as the flat ACPI ones, right? The current structures and code, however, don't express everything ACPI can do (cf. comment on the "Disclaimer" comment in the source). So it seems possible, that if we don't get it right from the start, the next developer adding a different CPU with different topology might have to overhaul the whole code again... Also, reusing the flags values creates redundancies. For instance, `is_leaf` is something that is also expressed by a NULL-pointer in the structures. To wrap this up, I don't mind merging it in this state. Just want to make it obvious, that it won't be correct from the start unless we invest a lot more, also to avoid to tailor this to a specific CPU. It's usually hard to get without multiple, different, real-world examples (and right now we don't even have one under review, AFAICS). If it's worth to even try, IDK. -- To view, visit https://review.coreboot.org/c/coreboot/+/78071?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: Ia119e1ba15756704668116bdbc655190ec94ff10 Gerrit-Change-Number: 78071 Gerrit-PatchSet: 8 Gerrit-Owner: David Milosevic <David.Milosevic(a)9elements.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: David Milosevic <David.Milosevic(a)9elements.com> Gerrit-Comment-Date: Wed, 01 Nov 2023 12:27:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph(a)9elements.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Comment-In-Reply-To: David Milosevic <David.Milosevic(a)9elements.com> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: mb/google/nissa: Add AUDIO_CONFIG in fw_config
by Eric Lai (Code Review) 01 Nov '23

01 Nov '23

Attention is currently required from: Karthik Ramasubramanian, Nick Vaccaro, Ren Kuo, Subrata Banik. Eric Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78791?usp=email ) Change subject: mb/google/nissa: Add AUDIO_CONFIG in fw_config ...................................................................... Patch Set 6: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/78791?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: I7446fce57557204d91151f1a31755381c1813c6f Gerrit-Change-Number: 78791 Gerrit-PatchSet: 6 Gerrit-Owner: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com> Gerrit-Reviewer: David Wu <david_wu(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Reka Norman <rekanorman(a)chromium.org> Gerrit-Reviewer: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Sam McNally <sammc(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Wed, 01 Nov 2023 11:44:56 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: mb/google/nissa/var/craaskov: Disable storage devices based on fw_config
by Eric Lai (Code Review) 01 Nov '23

01 Nov '23

Attention is currently required from: Dtrain Hsu, Kenny Pan, Nick Vaccaro, Rex Chou, Shou-Chieh Hsu, Subrata Banik. Eric Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78843?usp=email ) Change subject: mb/google/nissa/var/craaskov: Disable storage devices based on fw_config ...................................................................... Patch Set 1: (1 comment) File src/mainboard/google/brya/variants/craaskov/Makefile.inc: https://review.coreboot.org/c/coreboot/+/78843/comment/2b5ce8b8_b3e03830 : PS1, Line 5: CONFIG_FW_CONFIG you have wifi sar which is not depend fw config. -- To view, visit https://review.coreboot.org/c/coreboot/+/78843?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: I5993049ac63520c4dfd057c38b566fc69502d825 Gerrit-Change-Number: 78843 Gerrit-PatchSet: 1 Gerrit-Owner: Rex Chou <rex_chou(a)compal.corp-partner.google.com> Gerrit-Reviewer: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Kenny Pan <kennypan(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Shou-Chieh Hsu <shouchieh(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Van Chen <van_chen(a)compal.corp-partner.google.com> Gerrit-Attention: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Rex Chou <rex_chou(a)compal.corp-partner.google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Kenny Pan <kennypan(a)google.com> Gerrit-Attention: Shou-Chieh Hsu <shouchieh(a)google.com> Gerrit-Comment-Date: Wed, 01 Nov 2023 11:42:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: soc/intel/*: Correct ACPI device name for eMMC
by Eric Lai (Code Review) 01 Nov '23

01 Nov '23

Attention is currently required from: Lean Sheng Tan, Matt DeVillier, Nick Vaccaro, Werner Zeh. Eric Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78825?usp=email ) Change subject: soc/intel/*: Correct ACPI device name for eMMC ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/78825?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: Icbc22b6b2a84bbe73f1b09083f27081612db5eba Gerrit-Change-Number: 78825 Gerrit-PatchSet: 2 Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Comment-Date: Wed, 01 Nov 2023 11:39:35 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: drivers/intel/gma/opregion: Use CBFS cache to load VBT
by Nico Huber (Code Review) 01 Nov '23

01 Nov '23

Attention is currently required from: Arthur Heymans, Eran Mitrani, Jakub Czapiga, Jérémy Compostella, Kapil Porwal, Nick Vaccaro, Tarun. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77886?usp=email ) Change subject: drivers/intel/gma/opregion: Use CBFS cache to load VBT ...................................................................... Patch Set 33: (1 comment) File src/drivers/intel/gma/opregion.c: https://review.coreboot.org/c/coreboot/+/77886/comment/e97816bb_c4c87b44 : PS4, Line 27: cbfs_map > > If it's like the generic heap, why have it separate? > > Historical reasons, and the fact that it needs to be available in pre-RAM stages. If you're asking whether we should combine it with the ramstage heap into a single malloc() API for all stages, then... maybe. There are a few gotchas (e.g. it needs special alignment requirements on a few platforms) but it could certainly be done. > > There might also be some concern that exposing a general malloc() API in pre-RAM stages could lead to it being abused too widely and cause out-of-memory edge cases. As long as it's only for files, there are less edge case combinations to test. There could also be the concern that if more code uses this, the chance that a file allocation can still clean up after itself gets lower (because the allocator is only able to free the last two allocations), so that might be a good reason to keep the heaps separate. Good arguments as usual ;) That it's used pre-ramstage is probably the difference I was missing. > > Anyway, probably not a question to decide on this CL. > > > We eventually copy the VBT to CBMEM anyway. So there shouldn't be a need to map twice. > > I'm not really familiar with this code but it looks like this is an exported function that has a bunch of callers outside this file (e.g. 10 just via `vbt_get()`). It's possible that something could be cleaned up and reuse the CBMEM allocation there, but probably not a small task. Yes, never a small task in that area. I just hope we are not refactoring too much at the edges and miss the bigger picture. -- To view, visit https://review.coreboot.org/c/coreboot/+/77886?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: I1e37e718a71bd85b0d7dee1efc4c0391798f16f7 Gerrit-Change-Number: 77886 Gerrit-PatchSet: 33 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Comment-Date: Wed, 01 Nov 2023 11:22:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Jérémy Compostella <jeremy.compostella(a)intel.com> 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]: soc/intel/cmn/block/cse: Support sending EOP from payload
by Werner Zeh (Code Review) 01 Nov '23

01 Nov '23

Attention is currently required from: Julius Werner, Kapil Porwal, Lean Sheng Tan, Paul Menzel, Subrata Banik. Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74765?usp=email ) Change subject: soc/intel/cmn/block/cse: Support sending EOP from payload ...................................................................... Patch Set 12: (1 comment) Patchset: PS12: > > Is it the overall patch that added the 10 ms or just the recent change? […] I guess this is explainable due to the nature of how this feature works? Sending EOP later will cost some time at CSE side, right? -- To view, visit https://review.coreboot.org/c/coreboot/+/74765?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: I0fbb9fd0f8522eefad39960ca3167c2ba764f523 Gerrit-Change-Number: 74765 Gerrit-PatchSet: 12 Gerrit-Owner: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Himanshu Sahdev <himanshu.sahdev(a)intel.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-CC: sridhar siricilla <siricillasridhar(a)gmail.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Comment-Date: Wed, 01 Nov 2023 11:18:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: Documentation: add Nitrokey to distributions
by Markus Meissner (Code Review) 01 Nov '23

01 Nov '23

Attention is currently required from: Felix Singer, Michał Żygowski, Patrick Georgi, Paul Menzel. Markus Meissner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78299?usp=email ) Change subject: Documentation: add Nitrokey to distributions ...................................................................... Patch Set 5: (3 comments) File Documentation/distributions.md: https://review.coreboot.org/c/coreboot/+/78299/comment/7d322b1b_34444bf3 : PS5, Line 13: German > lowercase done - but as of my understanding this should be capitalized. But I might be wrong as I am no native speaker. https://review.coreboot.org/c/coreboot/+/78299/comment/29d0cf33_309fb98a : PS5, Line 17: edk2 > all uppercase done https://review.coreboot.org/c/coreboot/+/78299/comment/152114ba_3c76d02c : PS5, Line 19: send > typo: sen*t* done -- To view, visit https://review.coreboot.org/c/coreboot/+/78299?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: I3ef25e0e1cb97eda5fd457bdb650f3ee3f00210a Gerrit-Change-Number: 78299 Gerrit-PatchSet: 5 Gerrit-Owner: Markus Meissner Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Wed, 01 Nov 2023 10:37:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: Documentation: add Nitrokey to distributions
by Markus Meissner (Code Review) 01 Nov '23

01 Nov '23

Attention is currently required from: Felix Singer, Markus Meissner, Michał Żygowski, Patrick Georgi, Paul Menzel. Hello Felix Singer, Michał Żygowski, Patrick Georgi, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/78299?usp=email to look at the new patch set (#6). The following approvals got outdated and were removed: Code-Review+1 by Felix Singer, Verified+1 by build bot (Jenkins) Change subject: Documentation: add Nitrokey to distributions ...................................................................... Documentation: add Nitrokey to distributions Nitrokey offers various products, which have coreboot pre-installed. Change-Id: I3ef25e0e1cb97eda5fd457bdb650f3ee3f00210a Signed-off-by: Markus Meissner <coder(a)safemailbox.de> --- M Documentation/distributions.md 1 file changed, 10 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/99/78299/6 -- To view, visit https://review.coreboot.org/c/coreboot/+/78299?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: I3ef25e0e1cb97eda5fd457bdb650f3ee3f00210a Gerrit-Change-Number: 78299 Gerrit-PatchSet: 6 Gerrit-Owner: Markus Meissner Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Markus Meissner Gerrit-MessageType: newpatchset

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2023