coreboot-gerrit October 2021

coreboot-gerrit@coreboot.org

1 participants
5514 discussions

Change in coreboot[master]: util/cse_serger: Add a new tool for stitching CSE components
by Furquan Shaikh (Code Review) 06 Oct '21

06 Oct '21

Attention is currently required from: Cliff Huang, Martin Roth, Tim Wawrzynczak, Rizwan Qureshi, Sridhar Siricilla, Bernardo Perez Priego, Andrew McRae. Hello build bot (Jenkins), Cliff Huang, Martin Roth, Patrick Georgi, Rizwan Qureshi, Tim Wawrzynczak, Sridhar Siricilla, Bernardo Perez Priego, Bernardo Perez Priego, Andrew McRae, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/55503 to look at the new patch set (#14). Change subject: util/cse_serger: Add a new tool for stitching CSE components ...................................................................... util/cse_serger: Add a new tool for stitching CSE components This change adds a new tool `cse_serger` which can be used to print, dump and stitch together different components for the CSE region. BUG=b:189177186 Change-Id: I90dd809b47fd16afdc80e66431312721082496aa Signed-off-by: Furquan Shaikh <furquan(a)google.com> --- M Makefile.inc M util/cbfstool/Makefile M util/cbfstool/Makefile.inc A util/cbfstool/bpdt_formats/Makefile.inc A util/cbfstool/bpdt_formats/bpdt_1_6.c A util/cbfstool/bpdt_formats/bpdt_1_7.c A util/cbfstool/bpdt_formats/subpart_entry_1.c A util/cbfstool/bpdt_formats/subpart_hdr_1.c A util/cbfstool/bpdt_formats/subpart_hdr_2.c A util/cbfstool/cse_serger.c A util/cbfstool/cse_serger.h 11 files changed, 1,960 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/55503/14 -- To view, visit https://review.coreboot.org/c/coreboot/+/55503 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I90dd809b47fd16afdc80e66431312721082496aa Gerrit-Change-Number: 55503 Gerrit-PatchSet: 14 Gerrit-Owner: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Andrew McRae <amcrae(a)google.com> Gerrit-Reviewer: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com> Gerrit-Reviewer: Bernardo Perez Priego <bernardo.perez.priego(a)intel.corp-partner.google.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com> Gerrit-Attention: Bernardo Perez Priego <bernardo.perez.priego(a)intel.corp-partner.google.com> Gerrit-Attention: Andrew McRae <amcrae(a)google.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: util/cse_fpt: Add a new tool for managing Intel CSE FPT binaries
by Furquan Shaikh (Code Review) 06 Oct '21

06 Oct '21

Attention is currently required from: Cliff Huang, Furquan Shaikh, Martin Roth, Rizwan Qureshi, Sridhar Siricilla, Bernardo Perez Priego, Andrew McRae. Hello build bot (Jenkins), Cliff Huang, Martin Roth, Patrick Georgi, Tim Wawrzynczak, Rizwan Qureshi, Sridhar Siricilla, Bernardo Perez Priego, Andrew McRae, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/55259 to look at the new patch set (#13). Change subject: util/cse_fpt: Add a new tool for managing Intel CSE FPT binaries ...................................................................... util/cse_fpt: Add a new tool for managing Intel CSE FPT binaries This change adds a new tool `cse_fpt` which can be used to print and dump CSE partitions in Flash Partition Table (FPT) format. BUG=b:189167923 Change-Id: I93c8d33e9baa327cbdab918a14f2f7a039953be6 Signed-off-by: Furquan Shaikh <furquan(a)google.com> --- M Makefile.inc M util/cbfstool/Makefile M util/cbfstool/Makefile.inc A util/cbfstool/cse_fpt.c A util/cbfstool/cse_fpt.h A util/cbfstool/fpt_formats/Makefile.inc A util/cbfstool/fpt_formats/fpt_hdr_20.c A util/cbfstool/fpt_formats/fpt_hdr_21.c 8 files changed, 723 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/55259/13 -- To view, visit https://review.coreboot.org/c/coreboot/+/55259 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I93c8d33e9baa327cbdab918a14f2f7a039953be6 Gerrit-Change-Number: 55259 Gerrit-PatchSet: 13 Gerrit-Owner: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Andrew McRae <amcrae(a)google.com> Gerrit-Reviewer: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com> Gerrit-Attention: Andrew McRae <amcrae(a)google.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: cpu/x86: Remove cpu parameter to ap_init
by build bot (Jenkins) (Code Review) 06 Oct '21

06 Oct '21

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58147 ) Change subject: cpu/x86: Remove cpu parameter to ap_init ...................................................................... Patch Set 1: (1 comment) File src/cpu/x86/mp_init.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129908): https://review.coreboot.org/c/coreboot/+/58147/comment/8cf27b56_f686fc56 PS1, Line 184: static void asmlinkage ap_init(void) storage class 'asmlinkage' should be located before type 'void' -- To view, visit https://review.coreboot.org/c/coreboot/+/58147 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8997683b6613b7031784cabf7039a400f0efdea1 Gerrit-Change-Number: 58147 Gerrit-PatchSet: 1 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 06 Oct 2021 17:56:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: cpu/x86/{mp_init,smm}: Remove cpu member from smm_module_params
by Raul Rangel (Code Review) 06 Oct '21

06 Oct '21

Raul Rangel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/58148 ) Change subject: cpu/x86/{mp_init,smm}: Remove cpu member from smm_module_params ...................................................................... cpu/x86/{mp_init,smm}: Remove cpu member from smm_module_params This value can now be obtained by using cpu_info(). This also allows us to clean up the asm a bit too. BUG=b:194391185, b:179699789 TEST=Boot guybrush to OS and verify all CPUs still function Signed-off-by: Raul E Rangel <rrangel(a)chromium.org> Change-Id: I67b31b6f2aea9e255db01ab29bd9a55416f67a7d --- M src/cpu/x86/mp_init.c M src/cpu/x86/smm/smm_module_handler.c M src/cpu/x86/smm/smm_stub.S M src/include/cpu/x86/smm.h 4 files changed, 5 insertions(+), 13 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/58148/1 diff --git a/src/cpu/x86/mp_init.c b/src/cpu/x86/mp_init.c index 81ce101..8993d2d 100644 --- a/src/cpu/x86/mp_init.c +++ b/src/cpu/x86/mp_init.c @@ -720,14 +720,10 @@ */ static void asmlinkage smm_do_relocation(void *arg) { - const struct smm_module_params *p; - int cpu; + int cpu = cpu_info()->index; const uintptr_t curr_smbase = SMM_DEFAULT_BASE; uintptr_t perm_smbase; - p = arg; - cpu = p->cpu; - if (cpu >= CONFIG_MAX_CPUS) { printk(BIOS_CRIT, "Invalid CPU number assigned in SMM stub: %d\n", cpu); @@ -756,7 +752,7 @@ mseg = mp_state.perm_smbase + (mp_state.perm_smsize - CONFIG_MSEG_SIZE); - stm_setup(mseg, p->cpu, + stm_setup(mseg, cpu, perm_smbase, mp_state.perm_smbase, mp_state.reloc_start32_offset); diff --git a/src/cpu/x86/smm/smm_module_handler.c b/src/cpu/x86/smm/smm_module_handler.c index f9ebba4..d04af40 100644 --- a/src/cpu/x86/smm/smm_module_handler.c +++ b/src/cpu/x86/smm/smm_module_handler.c @@ -119,12 +119,11 @@ asmlinkage void smm_handler_start(void *arg) { const struct smm_module_params *p; - int cpu; + int cpu = cpu_info()->index; uintptr_t actual_canary; uintptr_t expected_canary; p = arg; - cpu = p->cpu; expected_canary = (uintptr_t)p->canary; /* Make sure to set the global runtime. It's OK to race as the value diff --git a/src/cpu/x86/smm/smm_stub.S b/src/cpu/x86/smm/smm_stub.S index 60bde99..3e7a158 100644 --- a/src/cpu/x86/smm/smm_stub.S +++ b/src/cpu/x86/smm/smm_stub.S @@ -211,7 +211,6 @@ andl $0xfffffff0, %esp #if ENV_X86_64 - mov %ecx, %edi /* Backup IA32_EFER. Preserves ebx. */ movl $(IA32_EFER), %ecx rdmsr @@ -220,7 +219,6 @@ /* entry64.inc preserves ebx, esi, edi */ #include <cpu/x86/64bit/entry64.inc> - mov %edi, %ecx #endif @@ -230,8 +228,8 @@ * c_handler(&arg) */ #if ENV_X86_64 + push $0x0 /* Padding */ push %rbx /* uintptr_t *canary */ - push %rcx /* size_t cpu */ mov %rsp, %rdi /* *arg */ @@ -257,8 +255,8 @@ #else push $0x0 /* Padding */ + push $0x0 /* Padding */ push %ebx /* uintptr_t *canary */ - push %ecx /* size_t cpu */ push %esp /* smm_module_params *arg (allocated on stack). */ mov c_handler, %eax call *%eax diff --git a/src/include/cpu/x86/smm.h b/src/include/cpu/x86/smm.h index 08404d0..61c6e52 100644 --- a/src/include/cpu/x86/smm.h +++ b/src/include/cpu/x86/smm.h @@ -65,7 +65,6 @@ } __packed; struct smm_module_params { - size_t cpu; /* A canary value that has been placed at the end of the stack. * If (uintptr_t)canary != *canary then a stack overflow has occurred. */ -- To view, visit https://review.coreboot.org/c/coreboot/+/58148 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I67b31b6f2aea9e255db01ab29bd9a55416f67a7d Gerrit-Change-Number: 58148 Gerrit-PatchSet: 1 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: cpu/x86: Remove cpu parameter to ap_init
by Raul Rangel (Code Review) 06 Oct '21

06 Oct '21

Raul Rangel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/58147 ) Change subject: cpu/x86: Remove cpu parameter to ap_init ...................................................................... cpu/x86: Remove cpu parameter to ap_init We now pre-populate cpu_info before jumping to the C handler. We no longer need this parameter. I moved the stack alignment closer to the actual invocation of the C handler so it's easier to reason about. BUG=b:194391185, b:179699789 TEST=Boot guybrush to OS and verify all CPUs still function Signed-off-by: Raul E Rangel <rrangel(a)chromium.org> Change-Id: I8997683b6613b7031784cabf7039a400f0efdea1 --- M src/cpu/x86/mp_init.c M src/cpu/x86/sipi_vector.S 2 files changed, 3 insertions(+), 12 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/47/58147/1 diff --git a/src/cpu/x86/mp_init.c b/src/cpu/x86/mp_init.c index d395377..81ce101 100644 --- a/src/cpu/x86/mp_init.c +++ b/src/cpu/x86/mp_init.c @@ -181,7 +181,7 @@ /* By the time APs call ap_init() caching has been setup, and microcode has * been loaded. */ -static void asmlinkage ap_init(unsigned int cpu) +static void asmlinkage ap_init(void) { struct cpu_info *info = cpu_info(); diff --git a/src/cpu/x86/sipi_vector.S b/src/cpu/x86/sipi_vector.S index 55256de..b8cac96 100644 --- a/src/cpu/x86/sipi_vector.S +++ b/src/cpu/x86/sipi_vector.S @@ -118,11 +118,6 @@ add per_cpu_segment_selector, %eax mov %eax, %gs - andl $0xfffffff0, %esp /* ensure stack alignment */ - - /* Save CPU number. */ - mov %ecx, %esi - /* * The following code only needs to run on Intel platforms and thus the caller * doesn't provide a microcode_ptr if not on Intel. @@ -235,19 +230,15 @@ mov %eax, %cr4 #endif + andl $0xfffffff0, %esp /* ensure stack alignment */ + #if ENV_X86_64 /* entry64.inc preserves ebx. */ #include <cpu/x86/64bit/entry64.inc> - mov %rsi, %rdi /* cpu_num */ - movabs c_handler, %eax call *%rax #else - /* c_handler(cpu_num), preserve proper stack alignment */ - sub $12, %esp - push %esi /* cpu_num */ - mov c_handler, %eax call *%eax #endif -- To view, visit https://review.coreboot.org/c/coreboot/+/58147 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8997683b6613b7031784cabf7039a400f0efdea1 Gerrit-Change-Number: 58147 Gerrit-PatchSet: 1 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: cpu/x86: Require CPU_INFO_V2 when selecting PARALLEL_MP
by Raul Rangel (Code Review) 06 Oct '21

06 Oct '21

Raul Rangel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/58146 ) Change subject: cpu/x86: Require CPU_INFO_V2 when selecting PARALLEL_MP ...................................................................... cpu/x86: Require CPU_INFO_V2 when selecting PARALLEL_MP This will reduce the number of AP init paths we need to support. BUG=b:194391185, b:179699789 TEST=Boot guybrush to OS and see all CPUs initialized correctly Signed-off-by: Raul E Rangel <rrangel(a)chromium.org> Change-Id: I05beb591bd7b3a26b6c51c10d4ffd6f8621c12eb --- M src/cpu/x86/Kconfig M src/cpu/x86/mp_init.c M src/cpu/x86/sipi_vector.S M src/cpu/x86/smm/smm_stub.S 4 files changed, 7 insertions(+), 27 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/58146/1 diff --git a/src/cpu/x86/Kconfig b/src/cpu/x86/Kconfig index 99e33bb..fb5b541 100644 --- a/src/cpu/x86/Kconfig +++ b/src/cpu/x86/Kconfig @@ -2,6 +2,7 @@ def_bool y depends on !LEGACY_SMP_INIT depends on SMP + select CPU_INFO_V2 help This option uses common MP infrastructure for bringing up APs in parallel. It additionally provides a more flexible mechanism diff --git a/src/cpu/x86/mp_init.c b/src/cpu/x86/mp_init.c index 123b2b5..d395377 100644 --- a/src/cpu/x86/mp_init.c +++ b/src/cpu/x86/mp_init.c @@ -183,26 +183,23 @@ * been loaded. */ static void asmlinkage ap_init(unsigned int cpu) { - struct cpu_info *info; + struct cpu_info *info = cpu_info(); /* Ensure the local APIC is enabled */ enable_lapic(); - info = cpu_info(); - info->index = cpu; - info->cpu = cpus_dev[cpu]; + info->cpu = cpus_dev[info->index]; cpu_add_map_entry(info->index); - thread_init_cpu_info_non_bsp(info); /* Fix up APIC id with reality. */ info->cpu->path.apic.apic_id = lapicid(); if (cpu_is_intel()) - printk(BIOS_INFO, "AP: slot %d apic_id %x, MCU rev: 0x%08x\n", cpu, + printk(BIOS_INFO, "AP: slot %zu apic_id %x, MCU rev: 0x%08x\n", info->index, info->cpu->path.apic.apic_id, get_current_microcode_rev()); else - printk(BIOS_INFO, "AP: slot %d apic_id %x\n", cpu, + printk(BIOS_INFO, "AP: slot %zu apic_id %x\n", info->index, info->cpu->path.apic.apic_id); /* Walk the flight plan */ @@ -217,20 +214,10 @@ sp->gdt = (uintptr_t)&gdt; sp->gdtlimit = (uintptr_t)&gdt_end - (uintptr_t)&gdt - 1; sp->idt_ptr = (uintptr_t)&idtarg; - if (CONFIG(CPU_INFO_V2)) { - sp->per_cpu_segment_descriptors = (uintptr_t)&per_cpu_segment_descriptors; - sp->per_cpu_segment_selector = per_cpu_segment_selector; - } + sp->per_cpu_segment_descriptors = (uintptr_t)&per_cpu_segment_descriptors; + sp->per_cpu_segment_selector = per_cpu_segment_selector; sp->stack_size = CONFIG_STACK_SIZE; sp->stack_top = ALIGN_DOWN((uintptr_t)&_estack, CONFIG_STACK_SIZE); - /* - * In the CPU_INFO_V2 case, we don't need to pre-allocate the space on the stack. - * Instead we push them onto the top of the stack in the sipi vector. - */ - if (!CONFIG(CPU_INFO_V2)) { - /* Adjust the stack top to take into account cpu_info. */ - sp->stack_top -= sizeof(struct cpu_info); - } } #define NUM_FIXED_MTRRS 11 diff --git a/src/cpu/x86/sipi_vector.S b/src/cpu/x86/sipi_vector.S index 491f1de..55256de 100644 --- a/src/cpu/x86/sipi_vector.S +++ b/src/cpu/x86/sipi_vector.S @@ -104,7 +104,6 @@ subl %eax, %edx mov %edx, %esp -#if CONFIG(CPU_INFO_V2) push_cpu_info index=%ecx push_per_cpu_segment_data @@ -118,7 +117,6 @@ shl $3, %eax /* The index is << 3 in the segment selector */ add per_cpu_segment_selector, %eax mov %eax, %gs -#endif andl $0xfffffff0, %esp /* ensure stack alignment */ diff --git a/src/cpu/x86/smm/smm_stub.S b/src/cpu/x86/smm/smm_stub.S index e409983..60bde99 100644 --- a/src/cpu/x86/smm/smm_stub.S +++ b/src/cpu/x86/smm/smm_stub.S @@ -84,19 +84,15 @@ .word 0xffff, 0x0000 .byte 0x00, 0x8b, 0x80, 0x00 -#if CONFIG(CPU_INFO_V2) per_cpu_segment_descriptors: .rept CONFIG_MAX_CPUS /* selgdt 0x28, flat data segment */ .word 0xffff, 0x0000 .byte 0x00, 0x93, 0xcf, 0x00 .endr -#endif /* CPU_INFO_V2 */ smm_relocate_gdt_end: -#if CONFIG(CPU_INFO_V2) .set per_cpu_segment_selector, per_cpu_segment_descriptors - smm_relocate_gdt -#endif /* CPU_INFO_V2 */ .align 4 .code32 @@ -167,7 +163,6 @@ movl $0, 4(%ebx) #endif -#if CONFIG(CPU_INFO_V2) push_cpu_info index=%ecx push_per_cpu_segment_data @@ -181,7 +176,6 @@ shl $3, %eax /* The index is << 3 in the segment selector */ add $per_cpu_segment_selector, %eax mov %eax, %gs -#endif /* Create stack frame by pushing a NULL stack base pointer */ pushl $0x0 -- To view, visit https://review.coreboot.org/c/coreboot/+/58146 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I05beb591bd7b3a26b6c51c10d4ffd6f8621c12eb Gerrit-Change-Number: 58146 Gerrit-PatchSet: 1 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: util/cse_serger: Add a new tool for stitching CSE components
by build bot (Jenkins) (Code Review) 06 Oct '21

06 Oct '21

Attention is currently required from: Cliff Huang, Martin Roth, Tim Wawrzynczak, Rizwan Qureshi, Sridhar Siricilla, Bernardo Perez Priego, Andrew McRae. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55503 ) Change subject: util/cse_serger: Add a new tool for stitching CSE components ...................................................................... Patch Set 13: Verified-1 (36 comments) File util/cbfstool/bpdt_formats/subpart_entry_1.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129904): https://review.coreboot.org/c/coreboot/+/55503/comment/fb40bee4_53784bf9 PS13, Line 13: #define SUBPART_COMPRESSED(x) (((x) >> SUBPART_COMPRESSED_SHIFT) & SUBPART_COMPRESSED_MASK) line over 96 characters Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129904): https://review.coreboot.org/c/coreboot/+/55503/comment/5d71de18_e39dea7e PS13, Line 33: e->name, SUBPART_OFFSET(e->offset_bytes), SUBPART_COMPRESSED(e->offset_bytes) ? 'Y' : 'N', line over 96 characters Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129904): https://review.coreboot.org/c/coreboot/+/55503/comment/9d624aa0_c29fb810 PS13, Line 44: for (size_t i = 0; i < count; i++) { braces {} are not necessary for single statement blocks Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129904): https://review.coreboot.org/c/coreboot/+/55503/comment/628d285e_a9b4145f PS13, Line 48: printf("%-25s%-25s%-25s%-25s%-25s%-25s\n", "Entry #", "Name", "Offset", "Huffman Compressed?", line over 96 characters Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129904): https://review.coreboot.org/c/coreboot/+/55503/comment/89286cdd_732f5058 PS13, Line 54: for (size_t i = 0; i < count; i++) { braces {} are not necessary for single statement blocks File util/cbfstool/cse_serger.h: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129904): https://review.coreboot.org/c/coreboot/+/55503/comment/f201d45c_4dc22cbb PS13, Line 51: bool (*match_version)(const struct buffer *); function definition argument 'const struct buffer *' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129904): https://review.coreboot.org/c/coreboot/+/55503/comment/bc105991_ccefbd61 PS13, Line 54: void (*print_hdr)(const bpdt_hdr_ptr); function definition argument 'const bpdt_hdr_ptr' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129904): https://review.coreboot.org/c/coreboot/+/55503/comment/3dc4539e_f7b68a71 PS13, Line 55: bpdt_hdr_ptr (*read_hdr)(struct buffer *); function definition argument 'struct buffer *' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129904): https://review.coreboot.org/c/coreboot/+/55503/comment/62956685_049fd38c PS13, Line 56: int (*write_hdr)(struct buffer *, const bpdt_hdr_ptr); function definition argument 'struct buffer *' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129904): https://review.coreboot.org/c/coreboot/+/55503/comment/c5f30215_70928645 PS13, Line 56: int (*write_hdr)(struct buffer *, const bpdt_hdr_ptr); function definition argument 'const bpdt_hdr_ptr' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129904): https://review.coreboot.org/c/coreboot/+/55503/comment/b5cb8e99_bb8ab645 PS13, Line 58: size_t (*get_entry_count)(const bpdt_hdr_ptr); function definition argument 'const bpdt_hdr_ptr' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129904): https://review.coreboot.org/c/coreboot/+/55503/comment/fa7eef8d_2de3faa4 PS13, Line 59: void (*inc_entry_count)(bpdt_hdr_ptr); function definition argument 'bpdt_hdr_ptr' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129904): https://review.coreboot.org/c/coreboot/+/55503/comment/88d7160b_615c53d0 PS13, Line 61: cse_layout_ptr (*create_layout)(const struct cse_layout_regions *); function definition argument 'const struct cse_layout_regions *' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129904): https://review.coreboot.org/c/coreboot/+/55503/comment/991ee561_ff6c0695 PS13, Line 62: void (*print_layout)(const cse_layout_ptr); function definition argument 'const cse_layout_ptr' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129904): https://review.coreboot.org/c/coreboot/+/55503/comment/cbe54d2d_72fe5f8c PS13, Line 63: cse_layout_ptr (*read_layout)(struct buffer *); function definition argument 'struct buffer *' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129904): https://review.coreboot.org/c/coreboot/+/55503/comment/adc1fb44_27cdcb6f PS13, Line 64: int (*write_layout)(struct buffer *, const cse_layout_ptr); function definition argument 'struct buffer *' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129904): https://review.coreboot.org/c/coreboot/+/55503/comment/0919ad02_34159f09 PS13, Line 64: int (*write_layout)(struct buffer *, const cse_layout_ptr); function definition argument 'const cse_layout_ptr' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129904): https://review.coreboot.org/c/coreboot/+/55503/comment/0e566baa_ae29e5c9 PS13, Line 66: void (*update_checksum)(bpdt_hdr_ptr, struct bpdt_entry *); function definition argument 'bpdt_hdr_ptr' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129904): https://review.coreboot.org/c/coreboot/+/55503/comment/133329de_66e0d67c PS13, Line 66: void (*update_checksum)(bpdt_hdr_ptr, struct bpdt_entry *); function definition argument 'struct bpdt_entry *' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129904): https://review.coreboot.org/c/coreboot/+/55503/comment/60ddb942_d47f528f PS13, Line 67: bool (*validate_checksum)(bpdt_hdr_ptr, struct bpdt_entry *); function definition argument 'bpdt_hdr_ptr' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129904): https://review.coreboot.org/c/coreboot/+/55503/comment/a25d5392_cd198de4 PS13, Line 67: bool (*validate_checksum)(bpdt_hdr_ptr, struct bpdt_entry *); function definition argument 'struct bpdt_entry *' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129904): https://review.coreboot.org/c/coreboot/+/55503/comment/96596ce8_f0edef69 PS13, Line 74: void *(*read)(struct buffer *); function definition argument 'struct buffer *' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129904): https://review.coreboot.org/c/coreboot/+/55503/comment/3991016c_cd497d02 PS13, Line 75: void (*print)(const void *); function definition argument 'const void *' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129904): https://review.coreboot.org/c/coreboot/+/55503/comment/abfee652_a1bb7b18 PS13, Line 76: size_t (*get_entry_count)(const void *); function definition argument 'const void *' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129904): https://review.coreboot.org/c/coreboot/+/55503/comment/2991dd8c_351940a3 PS13, Line 77: void (*free)(void *); function definition argument 'void *' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129904): https://review.coreboot.org/c/coreboot/+/55503/comment/201f4405_86082ac8 PS13, Line 81: void (*print)(struct buffer *, size_t); function definition argument 'struct buffer *' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129904): https://review.coreboot.org/c/coreboot/+/55503/comment/f51cc703_5deea3e4 PS13, Line 81: void (*print)(struct buffer *, size_t); function definition argument 'size_t' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129904): https://review.coreboot.org/c/coreboot/+/55503/comment/cc7721c0_95665b08 PS13, Line 92: void read_member(struct buffer *, void *, size_t); function definition argument 'struct buffer *' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129904): https://review.coreboot.org/c/coreboot/+/55503/comment/c6cee1a9_0e4a071c PS13, Line 92: void read_member(struct buffer *, void *, size_t); function definition argument 'void *' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129904): https://review.coreboot.org/c/coreboot/+/55503/comment/617ac54e_e123bc08 PS13, Line 92: void read_member(struct buffer *, void *, size_t); function definition argument 'size_t' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129904): https://review.coreboot.org/c/coreboot/+/55503/comment/bd800985_2c762512 PS13, Line 93: void write_member(struct buffer *, void *, size_t); function definition argument 'struct buffer *' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129904): https://review.coreboot.org/c/coreboot/+/55503/comment/47afc3ad_831c74aa PS13, Line 93: void write_member(struct buffer *, void *, size_t); function definition argument 'void *' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129904): https://review.coreboot.org/c/coreboot/+/55503/comment/6bcf67a0_8c57b094 PS13, Line 93: void write_member(struct buffer *, void *, size_t); function definition argument 'size_t' should also have an identifier name File util/cbfstool/cse_serger.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129904): https://review.coreboot.org/c/coreboot/+/55503/comment/aeae5805_1a5d7162 PS13, Line 272: ERROR("Part(%d) exceeds file size. Part offset=0x%x, Part size = 0x%x, File size = 0x%zx\n", line over 96 characters Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129904): https://review.coreboot.org/c/coreboot/+/55503/comment/a42cd298_79c51b78 PS13, Line 631: if(params.output_dir && (stat(params.output_dir, &sb) == -1)) { space required before the open parenthesis '(' Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129904): https://review.coreboot.org/c/coreboot/+/55503/comment/7644a162_2a42b25d PS13, Line 923: if (c < LONGOPT_START) { braces {} are not necessary for any arm of this statement -- To view, visit https://review.coreboot.org/c/coreboot/+/55503 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I90dd809b47fd16afdc80e66431312721082496aa Gerrit-Change-Number: 55503 Gerrit-PatchSet: 13 Gerrit-Owner: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Andrew McRae <amcrae(a)google.com> Gerrit-Reviewer: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com> Gerrit-Reviewer: Bernardo Perez Priego <bernardo.perez.priego(a)intel.corp-partner.google.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com> Gerrit-Attention: Bernardo Perez Priego <bernardo.perez.priego(a)intel.corp-partner.google.com> Gerrit-Attention: Andrew McRae <amcrae(a)google.com> Gerrit-Comment-Date: Wed, 06 Oct 2021 17:32:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: cbfstool: Add helper function `buffer_from_file_aligned_size`
by build bot (Jenkins) (Code Review) 06 Oct '21

06 Oct '21

Attention is currently required from: Cliff Huang, Tim Wawrzynczak, Bernardo Perez Priego. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55989 ) Change subject: cbfstool: Add helper function `buffer_from_file_aligned_size` ...................................................................... Patch Set 5: (1 comment) File util/cbfstool/common.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129902): https://review.coreboot.org/c/coreboot/+/55989/comment/3381b537_62eb65f8 PS5, Line 78: if (buffer->size > (size_t)file_size) { braces {} are not necessary for single statement blocks -- To view, visit https://review.coreboot.org/c/coreboot/+/55989 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iad3430d476abcdad850505ac50e36cd5d5deecb4 Gerrit-Change-Number: 55989 Gerrit-PatchSet: 5 Gerrit-Owner: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com> Gerrit-Comment-Date: Wed, 06 Oct 2021 17:31:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: util/cse_fpt: Add a new tool for managing Intel CSE FPT binaries
by build bot (Jenkins) (Code Review) 06 Oct '21

06 Oct '21

Attention is currently required from: Cliff Huang, Furquan Shaikh, Martin Roth, Rizwan Qureshi, Sridhar Siricilla, Bernardo Perez Priego, Andrew McRae. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55259 ) Change subject: util/cse_fpt: Add a new tool for managing Intel CSE FPT binaries ...................................................................... Patch Set 12: Verified-1 (10 comments) File util/cbfstool/cse_fpt.h: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129903): https://review.coreboot.org/c/coreboot/+/55259/comment/68b6b91d_6bb9425d PS12, Line 24: bool (*match_version)(struct buffer *); function definition argument 'struct buffer *' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129903): https://review.coreboot.org/c/coreboot/+/55259/comment/36e28971_1cc17e2e PS12, Line 26: fpt_hdr_ptr (*read)(struct buffer *); function definition argument 'struct buffer *' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129903): https://review.coreboot.org/c/coreboot/+/55259/comment/bfed0690_f517f217 PS12, Line 27: void (*print)(const fpt_hdr_ptr); function definition argument 'const fpt_hdr_ptr' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129903): https://review.coreboot.org/c/coreboot/+/55259/comment/12ac63ce_2bb5177b PS12, Line 29: size_t (*get_entry_count)(const fpt_hdr_ptr); function definition argument 'const fpt_hdr_ptr' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129903): https://review.coreboot.org/c/coreboot/+/55259/comment/74248d87_0e587c04 PS12, Line 35: void read_member(struct buffer *, void *, size_t); function definition argument 'struct buffer *' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129903): https://review.coreboot.org/c/coreboot/+/55259/comment/ccfc546d_3797673f PS12, Line 35: void read_member(struct buffer *, void *, size_t); function definition argument 'void *' should also have an identifier name Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129903): https://review.coreboot.org/c/coreboot/+/55259/comment/8964ce79_2c6bf9c7 PS12, Line 35: void read_member(struct buffer *, void *, size_t); function definition argument 'size_t' should also have an identifier name File util/cbfstool/cse_fpt.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129903): https://review.coreboot.org/c/coreboot/+/55259/comment/c978c26a_16b7d1a8 PS12, Line 290: if (!filepath) { braces {} are not necessary for single statement blocks File util/cbfstool/fpt_formats/fpt_hdr_20.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129903): https://review.coreboot.org/c/coreboot/+/55259/comment/c1e5bcb3_cdb60ccb PS12, Line 46: for (size_t i = 0; i < sizeof(*h); i++) { braces {} are not necessary for single statement blocks File util/cbfstool/fpt_formats/fpt_hdr_21.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-129903): https://review.coreboot.org/c/coreboot/+/55259/comment/5087dd4e_c7aacc0f PS12, Line 89: for (size_t i = 0; i < ARRAY_SIZE(h->reserved); i++) { braces {} are not necessary for single statement blocks -- To view, visit https://review.coreboot.org/c/coreboot/+/55259 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I93c8d33e9baa327cbdab918a14f2f7a039953be6 Gerrit-Change-Number: 55259 Gerrit-PatchSet: 12 Gerrit-Owner: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Andrew McRae <amcrae(a)google.com> Gerrit-Reviewer: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com> Gerrit-Attention: Andrew McRae <amcrae(a)google.com> Gerrit-Comment-Date: Wed, 06 Oct 2021 17:31:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/intel/adlrvp: Correct GPP_A7 configuration
by Sridhar Siricilla (Code Review) 06 Oct '21

06 Oct '21

Sridhar Siricilla has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/58145 ) Change subject: mb/intel/adlrvp: Correct GPP_A7 configuration ...................................................................... mb/intel/adlrvp: Correct GPP_A7 configuration Signed-off-by: Sridahr Siricilla <sridhar.siricilla(a)intel.com> Change-Id: Ib6d32e5a46ca1d1557cef66f71034545489a5e40 --- M src/mainboard/intel/adlrvp/gpio.c 1 file changed, 0 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/58145/1 diff --git a/src/mainboard/intel/adlrvp/gpio.c b/src/mainboard/intel/adlrvp/gpio.c index 1996683..507beba 100644 --- a/src/mainboard/intel/adlrvp/gpio.c +++ b/src/mainboard/intel/adlrvp/gpio.c @@ -191,7 +191,6 @@ PAD_CFG_NF(GPP_R3, NONE, DEEP, NF2), /* I2S2_SCLK */ - PAD_CFG_NF(GPP_A7, NONE, DEEP, NF1), /* I2S2_SFRM */ PAD_CFG_NF(GPP_A8, NONE, DEEP, NF1), /* I2S2_TXD */ -- To view, visit https://review.coreboot.org/c/coreboot/+/58145 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib6d32e5a46ca1d1557cef66f71034545489a5e40 Gerrit-Change-Number: 58145 Gerrit-PatchSet: 1 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-MessageType: newchange

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit October 2021