coreboot-gerrit June 2024

coreboot-gerrit@coreboot.org

1 participants
3527 discussions

[S] Change in coreboot[main]: cbfstool: Read XIP stage alignment requirements from ELF
by Felix Held (Code Review) 20 Jun '24

20 Jun '24

Felix Held has posted comments on this change by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/82102?usp=email ) Change subject: cbfstool: Read XIP stage alignment requirements from ELF ...................................................................... Patch Set 8: (1 comment) Patchset: PS7: > The linker normally aligns everything correctly already. […] ok. thanks for the explanation! -- To view, visit https://review.coreboot.org/c/coreboot/+/82102?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I94e4a4209b7441ecb2966a1342c3d46625771bb8 Gerrit-Change-Number: 82102 Gerrit-PatchSet: 8 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Appukuttan V K <appukuttan.vk(a)intel.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jincheng Li <jincheng.li(a)intel.com> Gerrit-CC: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Subrata Banik <subratabanik(a)google.com> Gerrit-Comment-Date: Thu, 20 Jun 2024 20:12:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>

1 0

[M] Change in coreboot[main]: libpayload/x86: Extend exception handling to x86_64 architecture
by Nick Vaccaro (Code Review) 20 Jun '24

20 Jun '24

Nick Vaccaro has submitted this change. ( https://review.coreboot.org/c/coreboot/+/83036?usp=email ) Change subject: libpayload/x86: Extend exception handling to x86_64 architecture ...................................................................... libpayload/x86: Extend exception handling to x86_64 architecture Adds 64-bit (x86_64) support to libpayload's exception handler, previously limited to x86_32. Ensures `exception_init_asm` is called when building with LP_ARCH_X86_64 Kconfig. BUG=b:336265399 TEST=Successful build and boot of google/rex and google/rex64 on ChromeOS. Verified correct x86_64 exception handling by triggering "Debug Exception" using firmware-shell as below: firmware-shell: mm.l -0 Debug Exception Error code: n/a REG_IP: 0x0000000030023e9f REG_FLAGS: 0x0000000000000046 REG_AX: 0x0000000000000009 REG_BX: 0x0000000000000000 REG_CX: 0x0000002000000000 REG_DX: 0x0000000000000001 REG_SP: 0x0000000034072ec0 REG_BP: 0x0000000000000009 REG_SI: 0x0000000000000029 REG_DI: 0x0000000034072eef REG_R8: 0x0000000000000009 REG_R9: 0x0000000000000000 REG_R10: 0x0000000000000000 REG_R11: 0x0000000034072d70 REG_R12: 0x0000000000000004 REG_R13: 0x0000000000000001 REG_R14: 0x0000000034072ee6 REG_R15: 0x0000000000000004 CS: 0x0020 DS: 0x0000 ES: 0x0000 SS: 0x0018 FS: 0x0018 GS: 0x0050 Dumping stack: 0x340730c0: 3003c32e 00000000 ... 00000000 00000000 0x340730a0: 30034bc6 00000000 ... 0000002a 00000000 0x34073080: 34073234 00000000 ... 00002e65 00000000 ... ... 0x34072ee0: 340730ed 30300000 ... 34073000 00000000 0x34072ec0: 34072ed8 00000000 ... 00000000 00000008 Ready for GDB connection. Change-Id: I8f0aa1da8d179a760e8d49c3764dfd5a69d06887 Signed-off-by: Subrata Banik <subratabanik(a)google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/83036 Reviewed-by: Kapil Porwal <kapilporwal(a)google.com> Reviewed-by: Arthur Heymans <arthur(a)aheymans.xyz> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M payloads/libpayload/arch/x86/exception.c M payloads/libpayload/arch/x86/exception_asm_64.S 2 files changed, 158 insertions(+), 30 deletions(-) Approvals: Kapil Porwal: Looks good to me, approved build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved diff --git a/payloads/libpayload/arch/x86/exception.c b/payloads/libpayload/arch/x86/exception.c index fdab042..9811be3 100644 --- a/payloads/libpayload/arch/x86/exception.c +++ b/payloads/libpayload/arch/x86/exception.c @@ -113,13 +113,15 @@ { int i, j; const int line = 8; - uint32_t *ptr = (uint32_t *)(addr & ~(line * sizeof(*ptr) - 1)); + uint32_t *ptr = (uint32_t *)((uintptr_t)addr & ~(line * sizeof(*ptr) - 1)); printf("Dumping stack:\n"); for (i = bytes / sizeof(*ptr); i >= 0; i -= line) { printf("%p: ", ptr + i); - for (j = i; j < i + line; j++) - printf("%08x ", *(ptr + j)); + for (j = i; j < i + line; j++) { + if ((uintptr_t)(ptr + j) >= addr && (uintptr_t)(ptr + j) < addr + bytes) + printf("%08x ", *(ptr + j)); + } printf("\n"); } } @@ -213,10 +215,6 @@ void exception_init(void) { - /* TODO: Add exception init code for x64, currently only supporting 32-bit code */ - if (CONFIG(LP_ARCH_X86_64)) - return; - exception_stack_end = exception_stack + ARRAY_SIZE(exception_stack); exception_init_asm(); } diff --git a/payloads/libpayload/arch/x86/exception_asm_64.S b/payloads/libpayload/arch/x86/exception_asm_64.S index ee4e9e5..6d91b93 100644 --- a/payloads/libpayload/arch/x86/exception_asm_64.S +++ b/payloads/libpayload/arch/x86/exception_asm_64.S @@ -39,10 +39,10 @@ .quad 0 error_code: .quad 0 -old_rsp: - .quad 0 old_rax: .quad 0 +old_rcx: + .quad 0 .align 16 @@ -55,23 +55,23 @@ .macro stub num exception_stub_\num: - movq $0, error_code - movq $\num, vector - jmp exception_common + movq $0, error_code + movq $\num, vector + jmp exception_common .endm .macro stub_err num exception_stub_\num: - pop error_code - movq $\num, vector - jmp exception_common + pop error_code + movq $\num, vector + jmp exception_common .endm .altmacro - .macro user_defined_stubs from, to - stub \from - .if \to-\from - user_defined_stubs %(from+1),\to + .macro user_defined_stubs from, to + stub \from + .if \to-\from + user_defined_stubs %(from+1),\to .endif .endm @@ -114,9 +114,117 @@ user_defined_stubs 192, 255 exception_common: + /* + * At this point, on x86-64, on the stack there is: + * 0(%rsp) rip + * 8(%rsp) cs + * 16(%rsp) rflags + * 24(%rsp) rsp + * 32(%rsp) ss + * + * This section sets up the exception stack. + * It saves the old stack pointer (rsp) to preserve RIP, CS, RFLAGS and SS. + * Then sets up the new stack pointer to point to the exception stack area. + */ + movq %rax, old_rax + movq %rcx, old_rcx - /* Return from the exception. */ - iretl + mov %rsp, %rax + movq exception_stack_end, %rsp + /* + * The `exception_state` struct is not 16-byte aligned. + * Push an extra 8 bytes to ensure the stack pointer + * is 16-byte aligned before calling exception_dispatch. + */ + push $0 + + /* + * Push values onto the top of the exception stack to form an + * exception state structure. + */ + push vector + push error_code + + /* push of the gs, fs, es, ds, ss and cs */ + mov %gs, %rcx + movl %ecx, -4(%rsp) /* gs */ + mov %fs, %rcx + movl %ecx, -8(%rsp) /* fs */ + movl $0, -12(%rsp) /* es */ + movl $0, -16(%rsp) /* ds */ + movq 32(%rax), %rcx + movl %ecx, -20(%rsp) /* ss */ + movq 8(%rax), %rcx + movl %ecx, -24(%rsp) /* cs */ + sub $24, %rsp + + push 16(%rax) /* rflags */ + push (%rax) /* rip */ + push %r15 + push %r14 + push %r13 + push %r12 + push %r11 + push %r10 + push %r9 + push %r8 + push 24(%rax) /* rsp */ + push %rbp + push %rdi + push %rsi + push %rdx + push old_rcx /* rcx */ + push %rbx + push old_rax /* rax */ + + /* + * Call the C exception handler. It will find the exception state + * using the exception_state global pointer. Not + * passing parameters means we don't have to worry about what ABI + * is being used. + */ + mov %rsp, exception_state + call exception_dispatch + + /* + * Restore state from the exception state structure, including any + * changes that might have been made. + */ + pop old_rax + pop %rbx + pop old_rcx + pop %rdx + pop %rsi + pop %rdi + pop %rbp + lea exception_stack, %rax + pop 24(%rax) /* rsp */ + pop %r8 + pop %r9 + pop %r10 + pop %r11 + pop %r12 + pop %r13 + pop %r14 + pop %r15 + pop (%rax) /* rip */ + pop 16(%rax) /* rflags */ + + /* pop of the gs, fs, es, ds, ss and cs */ + movl (%rsp), %ecx + movq %rcx, 8(%rax) /* cs */ + movl 4(%rsp), %ecx + movq %rcx, 32(%rax) /* ss */ + movl 16(%rsp), %ecx + mov %rcx, %fs /* fs */ + movl 20(%rsp), %ecx + mov %rcx, %gs /* gs */ + + mov %rax, %rsp + movq old_rax, %rax + movq old_rcx, %rcx + + iretq /* * We need segment selectors for the IDT, so we need to know where things are @@ -139,18 +247,18 @@ /* selgdt 0x18, flat 4GB data segment */ .word 0xffff, 0x0000 - .byte 0x00, 0x92, 0xcf, 0x00 + .byte 0x00, 0x92, 0xcf, 0x00 /* selgdt 0x20, flat x64 code segment */ - .word 0xffff, 0x0000 - .byte 0x00, 0x9b, 0xaf, 0x00 + .word 0xffff, 0x0000 + .byte 0x00, 0x9b, 0xaf, 0x00 gdt_end: /* GDT pointer for use with lgdt */ .global gdt_ptr gdt_ptr: - .word gdt_end - gdt - 1 - .quad gdt + .word gdt_end - gdt - 1 + .quad gdt /* * Record the target and construct the actual entry at init time. This @@ -158,8 +266,11 @@ * for us. */ .macro interrupt_gate target - .quad \target - .quad \target + .word 0 /* patchable */ + .word 0x20 /* Target code segment selector */ + .word 0xee00 /* Present, Type 64-bit Interrupt Gate */ + .word 0 /* patchable */ + .quad \target /* patchable */ .endm .altmacro @@ -171,7 +282,7 @@ .endm .align 16 - .global idt + .global idt idt: interrupt_gate exception_stub_0 interrupt_gate exception_stub_1 @@ -216,6 +327,25 @@ .word idt_end - idt - 1 .quad idt - .global exception_init_asm +.section .text.exception_init_asm +.globl exception_init_asm +.type exception_init_asm, @function + exception_init_asm: + /* Set up IDT entries */ + mov $idt, %rax +1: + movq 8(%rax), %rdi + movw %di, (%rax) /* procedure entry point offset bits 0..15 */ + shr $16, %rdi + movw %di, 6(%rax) /* procedure entry point offset bits 16..31 */ + shr $16, %rdi + movl %edi, 8(%rax) /* procedure entry point offset bits 32..63 */ + movl $0, 12(%rax) /* reserved */ + add $16, %rax + cmp $idt_end, %rax + jne 1b + + /* Load the IDT */ + lidt idt_ptr ret -- To view, visit https://review.coreboot.org/c/coreboot/+/83036?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I8f0aa1da8d179a760e8d49c3764dfd5a69d06887 Gerrit-Change-Number: 83036 Gerrit-PatchSet: 20 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Maximilian Brune <maximilian.brune(a)9elements.com>

1 0

[S] Change in coreboot[main]: cbfstool: Read XIP stage alignment requirements from ELF
by Julius Werner (Code Review) 20 Jun '24

20 Jun '24

Julius Werner has submitted this change. ( https://review.coreboot.org/c/coreboot/+/82102?usp=email ) Change subject: cbfstool: Read XIP stage alignment requirements from ELF ...................................................................... cbfstool: Read XIP stage alignment requirements from ELF On x86_64 romstage can contain page tables and a page table pointer which have an larger alignment requirement of 4096. Instead of hardcoding it, read if from the ELF phdrs. Change-Id: I94e4a4209b7441ecb2966a1342c3d46625771bb8 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> Reviewed-on: https://review.coreboot.org/c/coreboot/+/82102 Reviewed-by: Shuo Liu <shuo.liu(a)intel.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Jérémy Compostella <jeremy.compostella(a)intel.com> Reviewed-by: Julius Werner <jwerner(a)chromium.org> --- M Makefile.mk M util/cbfstool/cbfstool.c M util/cbfstool/elfheaders.c M util/cbfstool/elfparsing.h 4 files changed, 20 insertions(+), 23 deletions(-) Approvals: Shuo Liu: Looks good to me, but someone else must approve Jérémy Compostella: Looks good to me, but someone else must approve Julius Werner: Looks good to me, approved build bot (Jenkins): Verified diff --git a/Makefile.mk b/Makefile.mk index e642ac7..3cfd97c 100644 --- a/Makefile.mk +++ b/Makefile.mk @@ -1276,15 +1276,6 @@ $(CONFIG_CBFS_PREFIX)/romstage-options := -b 0 endif ifeq ($(CONFIG_ARCH_ROMSTAGE_X86_32)$(CONFIG_ARCH_ROMSTAGE_X86_64),y) -# Use a 64 byte alignment to provide a minimum alignment -# requirement for the overall romstage. While the first object within -# romstage could have a 4 byte minimum alignment that doesn't mean the linker -# won't decide the entire section should be aligned to a larger value. In the -# future cbfstool should add XIP files proper and honor the alignment -# requirements of the program segment. -# -# Make sure that segment for .car.data is ignored while adding romstage. -$(CONFIG_CBFS_PREFIX)/romstage-align := 64 ifeq ($(CONFIG_NO_XIP_EARLY_STAGES),y) $(CONFIG_CBFS_PREFIX)/romstage-options := -S ".car.data" else diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c index 88bf22b..f81c133 100644 --- a/util/cbfstool/cbfstool.c +++ b/util/cbfstool/cbfstool.c @@ -1156,23 +1156,26 @@ struct cbfs_file *header) { struct buffer output; - size_t data_size; int ret; - if (elf_program_file_size(buffer, &data_size) < 0) { - ERROR("Could not obtain ELF size\n"); - return 1; - } - /* * We need a final location for XIP parsing, so we need to call do_cbfs_locate() early * here. That is okay because XIP stages may not be compressed, so their size cannot * change anymore at a later point. */ - if (param.stage_xip && - do_cbfs_locate(offset, data_size)) { - ERROR("Could not find location for stage.\n"); - return 1; + if (param.stage_xip) { + size_t data_size, alignment; + if (elf_program_file_size_align(buffer, &data_size, &alignment) < 0) { + ERROR("Could not obtain ELF size & alignment\n"); + return 1; + } + + param.alignment = MAX(alignment, param.alignment); + + if (do_cbfs_locate(offset, data_size)) { + ERROR("Could not find location for stage.\n"); + return 1; + } } struct cbfs_file_attr_stageheader *stageheader = (void *) diff --git a/util/cbfstool/elfheaders.c b/util/cbfstool/elfheaders.c index 39faff2..5bcac15 100644 --- a/util/cbfstool/elfheaders.c +++ b/util/cbfstool/elfheaders.c @@ -1434,12 +1434,13 @@ return add_rel(rel_sec, &rel); } -int elf_program_file_size(const struct buffer *input, size_t *file_size) +int elf_program_file_size_align(const struct buffer *input, size_t *file_size, size_t *align) { Elf64_Ehdr ehdr; Elf64_Phdr *phdr; int i; size_t loadable_file_size = 0; + size_t align_size = 0; if (elf_headers(input, &ehdr, &phdr, NULL)) return -1; @@ -1448,9 +1449,11 @@ if (phdr[i].p_type != PT_LOAD) continue; loadable_file_size += phdr[i].p_filesz; + align_size = MAX(align_size, phdr[i].p_align); } *file_size = loadable_file_size; + *align = align_size; free(phdr); diff --git a/util/cbfstool/elfparsing.h b/util/cbfstool/elfparsing.h index d207f45..b80a3b1 100644 --- a/util/cbfstool/elfparsing.h +++ b/util/cbfstool/elfparsing.h @@ -114,9 +114,9 @@ int elf_writer_serialize(struct elf_writer *ew, struct buffer *out); /* - * Calculate the loadable program's file size footprint. Returns < 0 on error, - * 0 on success. + * Calculate the loadable program's file size footprint and alignment requirements. + * Returns < 0 on error, 0 on success. */ -int elf_program_file_size(const struct buffer *input, size_t *file_size); +int elf_program_file_size_align(const struct buffer *input, size_t *file_size, size_t *align); #endif /* ELFPARSING_H */ -- To view, visit https://review.coreboot.org/c/coreboot/+/82102?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I94e4a4209b7441ecb2966a1342c3d46625771bb8 Gerrit-Change-Number: 82102 Gerrit-PatchSet: 8 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Appukuttan V K <appukuttan.vk(a)intel.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jincheng Li <jincheng.li(a)intel.com> Gerrit-CC: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Subrata Banik <subratabanik(a)google.com>

1 0

[S] Change in coreboot[main]: cbfstool: Read XIP stage alignment requirements from ELF
by Julius Werner (Code Review) 20 Jun '24

20 Jun '24

Attention is currently required from: Appukuttan V K, Arthur Heymans, Dinesh Gehlot, Felix Held, Lean Sheng Tan, Patrick Rudolph, Paul Menzel. Julius Werner has posted comments on this change by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/82102?usp=email ) Change subject: cbfstool: Read XIP stage alignment requirements from ELF ...................................................................... Patch Set 7: (1 comment) Patchset: PS7: > i don't know this code too well, but i wonder if after this patch romstage alignment requirements ar […] The linker normally aligns everything correctly already. The only reason we have a problem here is because XIP stages get relocated by cbfstool. For non-xip stages that doesn't happen (except for rmodules but IIRC they just automatically get force-aligned to 4K and that's enough in practice or something). -- To view, visit https://review.coreboot.org/c/coreboot/+/82102?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I94e4a4209b7441ecb2966a1342c3d46625771bb8 Gerrit-Change-Number: 82102 Gerrit-PatchSet: 7 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Appukuttan V K <appukuttan.vk(a)intel.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jincheng Li <jincheng.li(a)intel.com> Gerrit-CC: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Appukuttan V K <appukuttan.vk(a)intel.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Thu, 20 Jun 2024 19:33:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>

1 0

[S] Change in coreboot[main]: cbfstool: Read XIP stage alignment requirements from ELF
by Felix Held (Code Review) 20 Jun '24

20 Jun '24

Attention is currently required from: Appukuttan V K, Arthur Heymans, Dinesh Gehlot, Lean Sheng Tan, Patrick Rudolph, Paul Menzel. Felix Held has posted comments on this change by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/82102?usp=email ) The change is no longer submittable: All-Comments-Resolved is unsatisfied now. Change subject: cbfstool: Read XIP stage alignment requirements from ELF ...................................................................... Patch Set 7: (1 comment) Patchset: PS7: i don't know this code too well, but i wonder if after this patch romstage alignment requirements are still taken into account in the non-xip case -- To view, visit https://review.coreboot.org/c/coreboot/+/82102?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I94e4a4209b7441ecb2966a1342c3d46625771bb8 Gerrit-Change-Number: 82102 Gerrit-PatchSet: 7 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Appukuttan V K <appukuttan.vk(a)intel.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jincheng Li <jincheng.li(a)intel.com> Gerrit-CC: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Appukuttan V K <appukuttan.vk(a)intel.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Comment-Date: Thu, 20 Jun 2024 18:03:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[XS] Change in coreboot[main]: acpigen_ps2_keybd: Support a Do Not Disturb key
by Caveh Jalali (Code Review) 20 Jun '24

20 Jun '24

Attention is currently required from: Aseda Aboagye, Cliff Huang, Lance Zhao, Tim Wawrzynczak. Caveh Jalali has posted comments on this change by Aseda Aboagye. ( https://review.coreboot.org/c/coreboot/+/82997?usp=email ) Change subject: acpigen_ps2_keybd: Support a Do Not Disturb key ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/82997?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I26e719bbde5106305282fe43dd15833a3e48e41e Gerrit-Change-Number: 82997 Gerrit-PatchSet: 2 Gerrit-Owner: Aseda Aboagye <aaboagye(a)google.com> Gerrit-Reviewer: Caveh Jalali <caveh(a)chromium.org> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Forest Mittelberg <bmbm(a)google.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: YH Lin <yueherngl(a)google.com> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Aseda Aboagye <aaboagye(a)google.com> Gerrit-Comment-Date: Thu, 20 Jun 2024 17:00:13 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[XS] Change in coreboot[main]: acpigen_ps2_keybd: Support an Accessibility key
by Caveh Jalali (Code Review) 20 Jun '24

20 Jun '24

Attention is currently required from: Aseda Aboagye, Cliff Huang, Forest Mittelberg, Lance Zhao, Tim Wawrzynczak. Caveh Jalali has posted comments on this change by Aseda Aboagye. ( https://review.coreboot.org/c/coreboot/+/82996?usp=email ) Change subject: acpigen_ps2_keybd: Support an Accessibility key ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/82996?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ifc639b37e89ec251f55859331ab5c2f4b2b45a7d Gerrit-Change-Number: 82996 Gerrit-PatchSet: 2 Gerrit-Owner: Aseda Aboagye <aaboagye(a)google.com> Gerrit-Reviewer: Caveh Jalali <caveh(a)chromium.org> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Forest Mittelberg <bmbm(a)google.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Aseda Aboagye <aaboagye(a)google.com> Gerrit-Attention: Forest Mittelberg <bmbm(a)google.com> Gerrit-Comment-Date: Thu, 20 Jun 2024 16:59:34 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[M] Change in coreboot[main]: soc/intel: Enable crashlog IP for both 32-bit and 64-bit architectures
by Sowmya Aralguppe (Code Review) 20 Jun '24

20 Jun '24

Attention is currently required from: Angel Pons, Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Nick Vaccaro, Pratikkumar V Prajapati, Subrata Banik, Tarun. Sowmya Aralguppe has posted comments on this change by Sowmya Aralguppe. ( https://review.coreboot.org/c/coreboot/+/83106?usp=email ) Change subject: soc/intel: Enable crashlog IP for both 32-bit and 64-bit architectures ...................................................................... Patch Set 19: (1 comment) File src/soc/intel/common/block/crashlog/crashlog.c: https://review.coreboot.org/c/coreboot/+/83106/comment/7a985c5e_05618238?us… : PS15, Line 321: sizeof(uintptr_t) > Yes I agree - we could either keep dest_addr as a 32 bit pointer since we are reading DW by DW and i […] I have changed dest_addr to be u32 *,IMHO that is right solution -- To view, visit https://review.coreboot.org/c/coreboot/+/83106?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I552257d3770abb409e2dcd8a13392506b5e7feb7 Gerrit-Change-Number: 83106 Gerrit-PatchSet: 19 Gerrit-Owner: Sowmya Aralguppe <sowmya.aralguppe(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> 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: Appukuttan V K <appukuttan.vk(a)intel.com> Gerrit-CC: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-CC: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Comment-Date: Thu, 20 Jun 2024 16:49:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Sowmya Aralguppe <sowmya.aralguppe(a)intel.com> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>

1 0

[M] Change in coreboot[main]: soc/intel: Enable crashlog IP for both 32-bit and 64-bit architectures
by Sowmya Aralguppe (Code Review) 20 Jun '24

20 Jun '24

Attention is currently required from: Angel Pons, Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Nick Vaccaro, Pratikkumar V Prajapati, Sowmya Aralguppe, Subrata Banik, Tarun. Hello Angel Pons, Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Nick Vaccaro, Pratikkumar V Prajapati, Subrata Banik, Tarun, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/83106?usp=email to look at the new patch set (#19). Change subject: soc/intel: Enable crashlog IP for both 32-bit and 64-bit architectures ...................................................................... soc/intel: Enable crashlog IP for both 32-bit and 64-bit architectures This patch extends the crashlog IP support beyond 32-bit mode to support Intel future generation SoCs, which may require crashlog support for 64-bit architectures. BUG=b:346676856 TEST=Successfully built Meteor Lake (rex) and tested for google/rex0 and google/rex64 images. Change-Id: I552257d3770abb409e2dcd8a13392506b5e7feb7 Signed-off-by: Sowmya Aralguppe <sowmya.aralguppe(a)intel.com> --- M src/soc/intel/alderlake/crashlog.c M src/soc/intel/common/block/crashlog/crashlog.c M src/soc/intel/common/block/include/intelblocks/crashlog.h M src/soc/intel/meteorlake/crashlog.c M src/soc/intel/tigerlake/crashlog_lib.c 5 files changed, 68 insertions(+), 77 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/83106/19 -- To view, visit https://review.coreboot.org/c/coreboot/+/83106?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I552257d3770abb409e2dcd8a13392506b5e7feb7 Gerrit-Change-Number: 83106 Gerrit-PatchSet: 19 Gerrit-Owner: Sowmya Aralguppe <sowmya.aralguppe(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> 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: Appukuttan V K <appukuttan.vk(a)intel.com> Gerrit-CC: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-CC: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Sowmya Aralguppe <sowmya.aralguppe(a)intel.com> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com>

1 0

[M] Change in coreboot[main]: soc/intel: Enable crashlog IP for both 32-bit and 64-bit architectures
by Sowmya Aralguppe (Code Review) 20 Jun '24

20 Jun '24

Attention is currently required from: Angel Pons, Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Nick Vaccaro, Pratikkumar V Prajapati, Sowmya Aralguppe, Subrata Banik, Tarun. Hello Angel Pons, Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Nick Vaccaro, Pratikkumar V Prajapati, Subrata Banik, Tarun, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/83106?usp=email to look at the new patch set (#18). The following approvals got outdated and were removed: Verified-1 by build bot (Jenkins) Change subject: soc/intel: Enable crashlog IP for both 32-bit and 64-bit architectures ...................................................................... soc/intel: Enable crashlog IP for both 32-bit and 64-bit architectures This patch extends the crashlog IP support beyond 32-bit mode to support Intel future generation SoCs, which may require crashlog support for 64-bit architectures. BUG=b:346676856 TEST=Successfully built Meteor Lake (rex) and tested for google/rex0 and google/rex64 images. Change-Id: I552257d3770abb409e2dcd8a13392506b5e7feb7 Signed-off-by: Sowmya Aralguppe <sowmya.aralguppe(a)intel.com> --- M src/soc/intel/alderlake/crashlog.c M src/soc/intel/common/block/crashlog/crashlog.c M src/soc/intel/common/block/include/intelblocks/crashlog.h M src/soc/intel/meteorlake/crashlog.c M src/soc/intel/tigerlake/crashlog_lib.c 5 files changed, 68 insertions(+), 77 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/83106/18 -- To view, visit https://review.coreboot.org/c/coreboot/+/83106?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I552257d3770abb409e2dcd8a13392506b5e7feb7 Gerrit-Change-Number: 83106 Gerrit-PatchSet: 18 Gerrit-Owner: Sowmya Aralguppe <sowmya.aralguppe(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> 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: Appukuttan V K <appukuttan.vk(a)intel.com> Gerrit-CC: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-CC: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Sowmya Aralguppe <sowmya.aralguppe(a)intel.com> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com>

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit June 2024