June 2023 - coreboot-gerrit - mail.coreboot.org

[S] Change in coreboot[master]: include/cpu/x86: Skip `wbinvd` on CPUs with cache self-snooping (SS)
by Subrata Banik (Code Review) June 1, 2023

June 1, 2023

Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/75511?usp=email ) Change subject: include/cpu/x86: Skip `wbinvd` on CPUs with cache self-snooping (SS) ...................................................................... include/cpu/x86: Skip `wbinvd` on CPUs with cache self-snooping (SS) This patch refers and backport some of previous work from Linux Kernel (https://lore.kernel.org/all/1561689337-19390-3-git-send-email-ricardo. neri-calderon(a)linux.intel.com/T/#u) that optimizes the MTRR register programming in multi-processor systems by relying on the CPUID (self-snoop feature supported). Refer to the details below: Programming MTRR registers in multi-processor systems is a rather lengthy process as it involves flushing caches. As a result, the process may take a considerable amount of time. Furthermore, all processors must program these registers serially. `wbinvd` instruction is used to invalidate the cache line to ensure that all modified data is written back to memory. All logical processors are stopped from executing until after the write-back and invalidate operation is completed. The amount of time or cycles for WBINVD to complete will vary due to the size of different cache hierarchies and other factors. As a consequence, the use of the WBINVD instruction can have an impact on response time. As per measurements, around 98% of the time needed by the procedure to program MTRRs in multi-processor systems is spent flushing caches with wbinvd(). As per the Section 11.11.8 of the Intel 64 and IA 32 Architectures Software Developer's Manual, it is not necessary to flush caches if the CPU supports cache self-snooping (ss). "Flush all caches using the WBINVD instructions. Note on a processor that supports self-snooping, CPUID feature flag bit 27, this step is unnecessary." Thus, skipping the cache flushes can reduce by several tens of milliseconds the time needed to complete the programming of the MTRR registers: Platform Before After 12-core (14 Threads) MeteorLake 35ms 1ms BUG=b:260455826 TEST=Able to build and boot google/rex. Change-Id: I83cac2b1e1707bbb1bc1bba82cf3073984e9768f Signed-off-by: Subrata Banik <subratabanik(a)google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/75511 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Jérémy Compostella <jeremy.compostella(a)intel.com> Reviewed-by: Lean Sheng Tan <sheng.tan(a)9elements.com> Reviewed-by: Himanshu Sahdev <himanshu.sahdev(a)intel.com> Reviewed-by: Tarun Tuli <taruntuli(a)google.com> --- M src/include/cpu/x86/cache.h 1 file changed, 14 insertions(+), 1 deletion(-) Approvals: Lean Sheng Tan: Looks good to me, approved build bot (Jenkins): Verified Jérémy Compostella: Looks good to me, but someone else must approve Himanshu Sahdev: Looks good to me, but someone else must approve Tarun Tuli: Looks good to me, approved diff --git a/src/include/cpu/x86/cache.h b/src/include/cpu/x86/cache.h index 4143d97..d4d9160 100644 --- a/src/include/cpu/x86/cache.h +++ b/src/include/cpu/x86/cache.h @@ -9,9 +9,11 @@ #define CR0_NoWriteThrough (CR0_NW) #define CPUID_FEATURE_CLFLUSH_BIT 19 +#define CPUID_FEATURE_SELF_SNOOP_BIT 27 #if !defined(__ASSEMBLER__) +#include <arch/cpuid.h> #include <stdbool.h> #include <stddef.h> @@ -51,6 +53,16 @@ write_cr0(cr0); } +/* + * Cache flushing is the most time-consuming step when programming the MTRRs. + * However, if the processor supports cache self-snooping (ss), we can skip + * this step and save time. + */ +static __always_inline bool self_snooping_supported(void) +{ + return (cpuid_edx(1) >> CPUID_FEATURE_SELF_SNOOP_BIT) & 1; +} + static __always_inline void disable_cache(void) { /* Disable and write back the cache */ @@ -58,7 +70,8 @@ cr0 = read_cr0(); cr0 |= CR0_CD; write_cr0(cr0); - wbinvd(); + if (!self_snooping_supported()) + wbinvd(); } #endif /* !__ASSEMBLER__ */ -- To view, visit https://review.coreboot.org/c/coreboot/+/75511?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I83cac2b1e1707bbb1bc1bba82cf3073984e9768f Gerrit-Change-Number: 75511 Gerrit-PatchSet: 5 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Himanshu Sahdev <himanshu.sahdev(a)intel.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-MessageType: merged

1 0

[XS] Change in coreboot[master]: cpu/x86/cache: Call wbinvd only once CR0.CD is set
by Subrata Banik (Code Review) June 1, 2023

June 1, 2023

Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/75474?usp=email ) Change subject: cpu/x86/cache: Call wbinvd only once CR0.CD is set ...................................................................... cpu/x86/cache: Call wbinvd only once CR0.CD is set This patch removes the wbinvd call preceding CR0.CD setting in disable_cache() to improve the boot time performances. According to some experimental measurements, the wbinvd execution takes between 1.6 up and 6 milliseconds to complete so it is preferable to call it only when necessary. According to Intel Software Developer Manual Vol 3.A - 12.5.3 Preventing Caching section there is no need to flush and invalidate the cache before settings CR0.CD. The documented sequence consists in setting CR0.CD and then call wbinvd. We also could not find any extra requirements in the AMD64 Architecture Programmer’s Manual - Volume 2 - Memory System chapter. This extra wbinvd in coreboot disable_cache() function does not seem documented and looking into the history of the project got us all the way back to original commit 8ca8d7665d67 ("- Initial checkin of the freebios2 tree") from April 2003. Even the original disable_cache() implementation (see below) is a bit curious as the comment list two actions: 1. Disable cache cover by line 74, 75 and 77 2. Write back the cache and flush TLB - Line 78 But it does not provide any explanation for the wbinvd call line 76. 68 static inline void disable_cache(void) 69 { 70 unsigned int tmp; 71 /* Disable cache */ 72 /* Write back the cache and flush TLB */ 73 asm volatile ( 74 "movl %%cr0, %0\n\t" 75 "orl $0x40000000, %0\n\t" 76 "wbinvd\n\t" 77 "movl %0, %%cr0\n\t" 78 "wbinvd\n\t" 79 :"=r" (tmp) 80 ::"memory"); 81 } BUG=b/260455826 TEST=Successful boot on Skolas and Rex board Change-Id: I08c6486dc93c4d70cadc22a760d1b7e536e85bfa Signed-off-by: Jeremy Compostella <jeremy.compostella(a)intel.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/75474 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Subrata Banik <subratabanik(a)google.com> Reviewed-by: Himanshu Sahdev <himanshu.sahdev(a)intel.com> --- M src/include/cpu/x86/cache.h 1 file changed, 0 insertions(+), 1 deletion(-) Approvals: Subrata Banik: Looks good to me, approved Himanshu Sahdev: Looks good to me, but someone else must approve build bot (Jenkins): Verified diff --git a/src/include/cpu/x86/cache.h b/src/include/cpu/x86/cache.h index d35c4e7..4143d97 100644 --- a/src/include/cpu/x86/cache.h +++ b/src/include/cpu/x86/cache.h @@ -57,7 +57,6 @@ CRx_TYPE cr0; cr0 = read_cr0(); cr0 |= CR0_CD; - wbinvd(); write_cr0(cr0); wbinvd(); } -- To view, visit https://review.coreboot.org/c/coreboot/+/75474?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I08c6486dc93c4d70cadc22a760d1b7e536e85bfa Gerrit-Change-Number: 75474 Gerrit-PatchSet: 7 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Himanshu Sahdev <himanshu.sahdev(a)intel.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[XS] Change in coreboot[master]: soc/intel/{alderlake, meteorlake}: Properly assign the FSP ASPM UPD
by Subrata Banik (Code Review) June 1, 2023

June 1, 2023

Attention is currently required from: Felix Singer, Kapil Porwal, Sean Rhodes, Tarun Tuli, Tim Crawford. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75481?usp=email ) Change subject: soc/intel/{alderlake, meteorlake}: Properly assign the FSP ASPM UPD ...................................................................... Patch Set 1: (1 comment) File src/soc/intel/alderlake/fsp_params.c: https://review.coreboot.org/c/coreboot/+/75481/comment/afa74fdd_a35daf40 : PS1, Line 899: if (rp_cfg->pcie_rp_aspm) : s_cfg->PcieRpAspm[i] = get_aspm_control(rp_cfg->pcie_rp_aspm); : else if (CONFIG(PCIEXP_ASPM)) /* use auto (FSP default) if PCIEXP_ASPM is enabled */ : s_cfg->PcieRpAspm[i] = ASPM_AUTO; : else : s_cfg->PcieRpAspm[i] = ASPM_DISABLE; > Those aren't disjoint cases. I want both of them. ASPM should be disabled for a *specific device* BUT be "Auto" for *everything else*. > > > we are not relying anywhere on the FSP default value and trying to make sure that coreboot overrides the FSP default as applicable. can you please elaborate > > "Auto" has been the default value used by every version of the FSP I've used for porting boards. So I mean setting it to "Auto" is the same thing as setting it to the value already used by the FSP. > > It seems like to do what I want, I now have to override `PCIEXP_ASPM` to disable it and specify `pcie_rp_aspm` for *every* device (does that even work if the config is disabled; does FSP do it instead?). Compared to the previous behavior where I only needed to specify the device I want to disable. I don't understand the concern here, are you saying that you don't want to make "PCIEXP_ASPM" kconfig default enabled on your platform ? because you want few devices to have ASPM disable vs others enabled ? wondering how having FSP default set to `Auto` was helpful in your case where you wished to make some device ASPM disabled but others enabled ? -- To view, visit https://review.coreboot.org/c/coreboot/+/75481?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib20c7466c79c3e0757ebda98240a529920c32a16 Gerrit-Change-Number: 75481 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: Tim Crawford <tcrawford(a)system76.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Tim Crawford <tcrawford(a)system76.com> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Comment-Date: Thu, 01 Jun 2023 07:51:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Crawford <tcrawford(a)system76.com> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: soc/intel/apollolake: Make SATA speed limit configurable
by Jan Samek (Code Review) June 1, 2023

June 1, 2023

Attention is currently required from: Arthur Heymans, Felix Singer, Mario Scheithauer. Jan Samek has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75364?usp=email ) Change subject: soc/intel/apollolake: Make SATA speed limit configurable ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/75364?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9c3eda0649546e3a40eb24a015b7c6efd8f90e0f Gerrit-Change-Number: 75364 Gerrit-PatchSet: 3 Gerrit-Owner: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Reviewer: Jan Samek <jan.samek(a)siemens.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Thu, 01 Jun 2023 07:28:20 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: soc/intel/apollolake: Make SATA speed limit configurable
by Mario Scheithauer (Code Review) June 1, 2023

June 1, 2023

Attention is currently required from: Arthur Heymans, Felix Singer, Jan Samek. Mario Scheithauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75364?usp=email ) Change subject: soc/intel/apollolake: Make SATA speed limit configurable ...................................................................... Patch Set 3: (1 comment) File src/soc/intel/apollolake/chip.h: https://review.coreboot.org/c/coreboot/+/75364/comment/90c1a433_65d8a08b : PS2, Line 31: SATA_GEN2 > Hmm I see. If gen 3 and auto are the same, then it doesn't matter I guess. […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/75364?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9c3eda0649546e3a40eb24a015b7c6efd8f90e0f Gerrit-Change-Number: 75364 Gerrit-PatchSet: 3 Gerrit-Owner: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Reviewer: Jan Samek <jan.samek(a)siemens.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Jan Samek <jan.samek(a)siemens.com> Gerrit-Comment-Date: Thu, 01 Jun 2023 07:27:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Comment-In-Reply-To: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[master]: mb/google/rex/variants/screebo: add FW_CONFIG for audio/DB
by Kapil Porwal (Code Review) June 1, 2023

June 1, 2023

Attention is currently required from: Kun Liu, Rui Zhou, Simon Zhou, Subrata Banik, Tarun Tuli, YH Lin. Kapil Porwal has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75573?usp=email ) Change subject: mb/google/rex/variants/screebo: add FW_CONFIG for audio/DB ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/75573?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I928aae61d4936509a7b68f4041c0cd72f298e83d Gerrit-Change-Number: 75573 Gerrit-PatchSet: 4 Gerrit-Owner: Simon Zhou <zhouguohui(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Kun Liu <liukun11(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: Rui Zhou <zhourui(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: YH Lin <yueherngl(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: YH Lin <yueherngl(a)google.com> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Rui Zhou <zhourui(a)huaqin.corp-partner.google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Simon Zhou <zhouguohui(a)huaqin.corp-partner.google.com> Gerrit-Attention: Kun Liu <liukun11(a)huaqin.corp-partner.google.com> Gerrit-Comment-Date: Thu, 01 Jun 2023 07:04:20 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: Documentation: Legal action (or threats thereof) has consequences
by Paul Menzel (Code Review) May 31, 2023

May 31, 2023

Attention is currently required from: Patrick Georgi, Ron Minnich. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75554?usp=email ) The change is no longer submittable: All-Comments-Resolved is unsatisfied now. Change subject: Documentation: Legal action (or threats thereof) has consequences ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/75554/comment/a7c1c441_337755b0 : PS1, Line 7: Documentation: Legal action (or threats thereof) has consequences Imperative mood? > Add legal action section > Note consequences of (threats of) legal action -- To view, visit https://review.coreboot.org/c/coreboot/+/75554?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifa865487dc81ed3797fe60e5cef737c57dd85fea Gerrit-Change-Number: 75554 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: Ron Minnich <rminnich(a)gmail.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Attention: Ron Minnich <rminnich(a)gmail.com> Gerrit-Comment-Date: Thu, 01 Jun 2023 06:36:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: include/cpu/x86: Simplify en/dis cache functions
by Himanshu Sahdev (Code Review) May 31, 2023

May 31, 2023

Attention is currently required from: Felix Singer, Jérémy Compostella, Lean Sheng Tan, Subrata Banik. Himanshu Sahdev has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75552?usp=email ) Change subject: include/cpu/x86: Simplify en/dis cache functions ...................................................................... Patch Set 2: (1 comment) File src/include/cpu/x86/cache.h: https://review.coreboot.org/c/coreboot/+/75552/comment/6c3ed77b_341159b5 : PS2, Line 48: CR0_CacheDisable > can we avoid camel case? want me to change it to screaming snake case -> CR0_CACHE_DISABLE? Also, these macros being in use already, better to do it in different CL to change everywhere. -- To view, visit https://review.coreboot.org/c/coreboot/+/75552?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I81688e8bbb073e1d09ecf63f3f33e1651dbd778e Gerrit-Change-Number: 75552 Gerrit-PatchSet: 2 Gerrit-Owner: Himanshu Sahdev <himanshu.sahdev(a)intel.com> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Comment-Date: Thu, 01 Jun 2023 06:01:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: ifdtool: Fix spacing for 10GbE_*
by Stefan Reinauer (Code Review) May 31, 2023

May 31, 2023

Attention is currently required from: Felix Singer, Patrick Rudolph. Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68699?usp=email ) Change subject: ifdtool: Fix spacing for 10GbE_* ...................................................................... Patch Set 8: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/68699?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie803df6ce9a9772116063ad20bad1c677c3b493c Gerrit-Change-Number: 68699 Gerrit-PatchSet: 8 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Comment-Date: Thu, 01 Jun 2023 05:33:15 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[L] Change in coreboot[master]: ifdtool: Move regions to separate file
by Stefan Reinauer (Code Review) May 31, 2023

May 31, 2023

Attention is currently required from: Felix Singer, Maximilian Brune, Patrick Rudolph. Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68695?usp=email ) Change subject: ifdtool: Move regions to separate file ...................................................................... Patch Set 10: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/68695?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6a1ba1799c69881b1f48d903b8f86e7cd93815c2 Gerrit-Change-Number: 68695 Gerrit-PatchSet: 10 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julian Elischer <jrelis(a)google.com> Gerrit-CC: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Comment-Date: Thu, 01 Jun 2023 05:32:53 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0