coreboot-gerrit March 2024

coreboot-gerrit@coreboot.org

1 participants
3668 discussions

[L] Change in coreboot[main]: arch/riscv: remove misaligned load/store/fetch handling
by ron minnich (Code Review) 27 Mar '24

27 Mar '24

ron minnich has submitted this change. ( https://review.coreboot.org/c/coreboot/+/81416?usp=email ) ( 7 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: arch/riscv: remove misaligned load/store/fetch handling ...................................................................... arch/riscv: remove misaligned load/store/fetch handling Testing on the unmatched shows the code no longer works completely correctly; Linux has taken over the handling of misalignment anyway, because handling it in firmware, with the growing complexity of the ISA and the awkward way in which it has to be handled, is more trouble than its worth. Plus, we don't WANT misalignment handled, magically, in firmware: the cost of getting it wrong is high (as I've spent a month learning); the performance is terrible (350x slowdown; and most toolchains now know to avoid unaligned load/store on RISC-V anyway. But, mostly, if alignment problems exist, *we need to know*, and if they're handled invisibly in firmware, we don't. The problem with invisible handling was shown a while back in the Go toolchain: runtime had a small error, such that many misaligned load/store were happening, and it was not discovered for some time. Had a trap been directed to kernel or user on misalignment, the problem would have been known immediately, not after many months. (The error, btw, was masking the address with 3, not 7, to detect misalignment; an easy mistake!). But, the coreboot code does not work any more any way, and it's not worth fixing. Remove it. Tested by booting Linux to runlevel 1; before, it would hang on an alignment fault, as the alignment code was failing (somewhere). This takes the coreboot SBI code much closer to revival. Change-Id: I84a8d433ed2f50745686a8c109d101e8718f2a46 Signed-off-by: Ronald G Minnich <rminnich(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/81416 Reviewed-by: Maximilian Brune <maximilian.brune(a)9elements.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/arch/riscv/Makefile.mk M src/arch/riscv/include/arch/exception.h D src/arch/riscv/misaligned.c M src/arch/riscv/trap_handler.c M src/arch/riscv/virtual_memory.c 5 files changed, 4 insertions(+), 265 deletions(-) Approvals: Maximilian Brune: Looks good to me, approved build bot (Jenkins): Verified diff --git a/src/arch/riscv/Makefile.mk b/src/arch/riscv/Makefile.mk index 1131bd5..d5defea 100644 --- a/src/arch/riscv/Makefile.mk +++ b/src/arch/riscv/Makefile.mk @@ -52,7 +52,6 @@ all-y += trap_util.S all-y += trap_handler.c all-y += fp_asm.S -all-y += misaligned.c all-y += sbi.c all-y += mcall.c all-y += virtual_memory.c diff --git a/src/arch/riscv/include/arch/exception.h b/src/arch/riscv/include/arch/exception.h index e807a3f..2eb575e 100644 --- a/src/arch/riscv/include/arch/exception.h +++ b/src/arch/riscv/include/arch/exception.h @@ -28,6 +28,5 @@ void redirect_trap(void); void default_trap_handler(struct trapframe *tf); void handle_supervisor_call(struct trapframe *tf); -void handle_misaligned(struct trapframe *tf); #endif diff --git a/src/arch/riscv/misaligned.c b/src/arch/riscv/misaligned.c deleted file mode 100644 index 151ed99..0000000 --- a/src/arch/riscv/misaligned.c +++ /dev/null @@ -1,260 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -#include <vm.h> -#include <arch/exception.h> -#include <commonlib/helpers.h> -#include <types.h> - -/* these functions are defined in src/arch/riscv/fp_asm.S */ -#if defined(__riscv_flen) -#if __riscv_flen >= 32 -extern void read_f32(int regnum, uint32_t *v); -extern void write_f32(int regnum, uint32_t *v); -#endif // __riscv_flen >= 32 -#if __riscv_flen >= 64 -extern void read_f64(int regnum, uint64_t *v); -extern void write_f64(int regnum, uint64_t *v); -#endif // __riscv_flen >= 64 -#endif // defined(__riscv_flen) - -/* This union makes it easy to read multibyte types by byte operations. */ -union endian_buf { - uint8_t b[8]; - uint16_t h[4]; - uint32_t w[2]; - uint64_t d[1]; - uintptr_t v; -}; - -/* This struct hold info of load/store instruction */ -struct memory_instruction_info { - /* opcode/mask used to identify instruction, - * (instruction_val) & mask == opcode */ - uint32_t opcode; - uint32_t mask; - /* reg_shift/reg_mask/reg_addition used to get register number - * ((instruction_val >> reg_shift) & reg_mask) + reg_addition */ - unsigned int reg_shift; - unsigned int reg_mask; - unsigned int reg_addition; - unsigned int is_fp : 1; /* mark as a float operation */ - unsigned int is_load : 1; /* mark as a load operation */ - unsigned int width : 8; /* Record the memory width of the operation */ - unsigned int sign_extend : 1; /* mark need to be sign extended */ -}; - -static struct memory_instruction_info insn_info[] = { -#if __riscv_xlen == 128 - { 0x00002000, 0x0000e003, 2, 7, 8, 0, 1, 16, 1}, // C.LQ -#else - { 0x00002000, 0x0000e003, 2, 7, 8, 1, 1, 8, 0}, // C.FLD -#endif - { 0x00004000, 0x0000e003, 2, 7, 8, 0, 1, 4, 1}, // C.LW -#if __riscv_xlen == 32 - { 0x00006000, 0x0000e003, 2, 7, 8, 1, 1, 4, 0}, // C.FLW -#else - { 0x00006000, 0x0000e003, 2, 7, 8, 0, 1, 8, 1}, // C.LD -#endif - -#if __riscv_xlen == 128 - { 0x0000a000, 0x0000e003, 2, 7, 8, 0, 0, 16, 0}, // C.SQ -#else - { 0x0000a000, 0x0000e003, 2, 7, 8, 1, 0, 8, 0}, // C.FSD -#endif - { 0x0000c000, 0x0000e003, 2, 7, 8, 0, 0, 4, 0}, // C.SW -#if __riscv_xlen == 32 - { 0x0000e000, 0x0000e003, 2, 7, 8, 1, 0, 4, 0}, // C.FSW -#else - { 0x0000e000, 0x0000e003, 2, 7, 8, 0, 0, 8, 0}, // C.SD -#endif - -#if __riscv_xlen == 128 - { 0x00002002, 0x0000e003, 7, 15, 0, 0, 1, 16, 1}, // C.LQSP -#else - { 0x00002002, 0x0000e003, 7, 15, 0, 1, 1, 8, 0}, // C.FLDSP -#endif - { 0x00004002, 0x0000e003, 7, 15, 0, 0, 1, 4, 1}, // C.LWSP -#if __riscv_xlen == 32 - { 0x00006002, 0x0000e003, 7, 15, 0, 1, 1, 4, 0}, // C.FLWSP -#else - { 0x00006002, 0x0000e003, 7, 15, 0, 0, 1, 8, 1}, // C.LDSP -#endif - -#if __riscv_xlen == 128 - { 0x0000a002, 0x0000e003, 2, 15, 0, 0, 0, 16, 0}, // C.SQSP -#else - { 0x0000a002, 0x0000e003, 2, 15, 0, 1, 0, 8, 0}, // C.FSDSP -#endif - { 0x0000c002, 0x0000e003, 2, 15, 0, 0, 0, 4, 0}, // C.SWSP -#if __riscv_xlen == 32 - { 0x0000e002, 0x0000e003, 2, 15, 0, 1, 0, 4, 0}, // C.FSWSP -#else - { 0x0000e002, 0x0000e003, 2, 15, 0, 0, 0, 8, 0}, // C.SDSP -#endif - - { 0x00000003, 0x0000707f, 7, 15, 0, 0, 1, 1, 1}, // LB - { 0x00001003, 0x0000707f, 7, 15, 0, 0, 1, 2, 1}, // LH - { 0x00002003, 0x0000707f, 7, 15, 0, 0, 1, 4, 1}, // LW -#if __riscv_xlen > 32 - { 0x00003003, 0x0000707f, 7, 15, 0, 0, 1, 8, 1}, // LD -#endif - { 0x00004003, 0x0000707f, 7, 15, 0, 0, 1, 1, 0}, // LBU - { 0x00005003, 0x0000707f, 7, 15, 0, 0, 1, 2, 0}, // LHU - { 0x00006003, 0x0000707f, 7, 15, 0, 0, 1, 4, 0}, // LWU - - { 0x00000023, 0x0000707f, 20, 15, 0, 0, 0, 1, 0}, // SB - { 0x00001023, 0x0000707f, 20, 15, 0, 0, 0, 2, 0}, // SH - { 0x00002023, 0x0000707f, 20, 15, 0, 0, 0, 4, 0}, // SW -#if __riscv_xlen > 32 - { 0x00003023, 0x0000707f, 20, 15, 0, 0, 0, 8, 0}, // SD -#endif - -#if defined(__riscv_flen) -#if __riscv_flen >= 32 - { 0x00002007, 0x0000707f, 7, 15, 0, 1, 1, 4, 0}, // FLW - { 0x00003007, 0x0000707f, 7, 15, 0, 1, 1, 8, 0}, // FLD -#endif // __riscv_flen >= 32 - -#if __riscv_flen >= 64 - { 0x00002027, 0x0000707f, 20, 15, 0, 1, 0, 4, 0}, // FSW - { 0x00003027, 0x0000707f, 20, 15, 0, 1, 0, 8, 0}, // FSD -#endif // __riscv_flen >= 64 -#endif // defined(__riscv_flen) -}; - -static struct memory_instruction_info *match_instruction(uintptr_t insn) -{ - int i; - for (i = 0; i < ARRAY_SIZE(insn_info); i++) - if ((insn_info[i].mask & insn) == insn_info[i].opcode) - return &(insn_info[i]); - return NULL; -} - -static enum cb_err fetch_16bit_instruction(uintptr_t vaddr, uintptr_t *insn, int *size) -{ - uint16_t ins = mprv_read_mxr_u16((uint16_t *)vaddr); - if (EXTRACT_FIELD(ins, 0x3) != 3) { - *insn = ins; - *size = 2; - return CB_SUCCESS; - } - return CB_ERR; -} - -static enum cb_err fetch_32bit_instruction(uintptr_t vaddr, uintptr_t *insn, int *size) -{ - uint32_t l = (uint32_t)mprv_read_mxr_u16((uint16_t *)vaddr + 0); - uint32_t h = (uint32_t)mprv_read_mxr_u16((uint16_t *)vaddr + 1); - uint32_t ins = (h << 16) | l; - if ((EXTRACT_FIELD(ins, 0x3) == 3) && - (EXTRACT_FIELD(ins, 0x1c) != 0x7)) { - *insn = ins; - *size = 4; - return CB_SUCCESS; - } - return CB_ERR; -} - -void handle_misaligned(struct trapframe *tf) -{ - uintptr_t insn = 0; - union endian_buf buff; - int insn_size = 0; - - /* try to fetch 16/32 bits instruction */ - if (fetch_16bit_instruction(tf->epc, &insn, &insn_size) < 0) { - if (fetch_32bit_instruction(tf->epc, &insn, &insn_size) < 0) { - redirect_trap(); - return; - } - } - - /* matching instruction */ - struct memory_instruction_info *match = match_instruction(insn); - - if (!match) { - redirect_trap(); - return; - } - - int regnum; - regnum = ((insn >> match->reg_shift) & match->reg_mask); - regnum = regnum + match->reg_addition; - buff.v = 0; - if (match->is_load) { - /* load operation */ - - /* reading from memory by bytes prevents misaligned - * memory access */ - for (int i = 0; i < match->width; i++) { - uint8_t *addr = (uint8_t *)(tf->badvaddr + i); - buff.b[i] = mprv_read_u8(addr); - } - - /* sign extend for signed integer loading */ - if (match->sign_extend) - if (buff.v >> (8 * match->width - 1)) - buff.v |= -1 << (8 * match->width); - - /* write to register */ - if (match->is_fp) { - int done = 0; -#if defined(__riscv_flen) -#if __riscv_flen >= 32 - /* single-precision floating-point */ - if (match->width == 4) { - write_f32(regnum, buff.w); - done = 1; - } -#endif // __riscv_flen >= 32 -#if __riscv_flen >= 64 - /* double-precision floating-point */ - if (match->width == 8) { - write_f64(regnum, buff.d); - done = 1; - } -#endif // __riscv_flen >= 64 -#endif // defined(__riscv_flen) - if (!done) - redirect_trap(); - } else { - tf->gpr[regnum] = buff.v; - } - } else { - /* store operation */ - - /* reading from register */ - if (match->is_fp) { - int done = 0; -#if defined(__riscv_flen) -#if __riscv_flen >= 32 - if (match->width == 4) { - read_f32(regnum, buff.w); - done = 1; - } -#endif // __riscv_flen >= 32 -#if __riscv_flen >= 64 - if (match->width == 8) { - read_f64(regnum, buff.d); - done = 1; - } -#endif // __riscv_flen >= 64 -#endif // defined(__riscv_flen) - if (!done) - redirect_trap(); - } else { - buff.v = tf->gpr[regnum]; - } - - /* writing to memory by bytes prevents misaligned - * memory access */ - for (int i = 0; i < match->width; i++) { - uint8_t *addr = (uint8_t *)(tf->badvaddr + i); - mprv_write_u8(addr, buff.b[i]); - } - } - - /* return to where we came from */ - write_csr(mepc, read_csr(mepc) + insn_size); -} diff --git a/src/arch/riscv/trap_handler.c b/src/arch/riscv/trap_handler.c index f3968bb..2f22ce2 100644 --- a/src/arch/riscv/trap_handler.c +++ b/src/arch/riscv/trap_handler.c @@ -119,7 +119,6 @@ } switch (tf->cause) { - case CAUSE_MISALIGNED_FETCH: case CAUSE_FETCH_ACCESS: case CAUSE_ILLEGAL_INSTRUCTION: case CAUSE_BREAKPOINT: @@ -133,10 +132,10 @@ case CAUSE_SUPERVISOR_ECALL: handle_sbi(tf); return; + case CAUSE_MISALIGNED_FETCH: case CAUSE_MISALIGNED_LOAD: case CAUSE_MISALIGNED_STORE: print_trap_information(tf); - handle_misaligned(tf); return; default: printk(BIOS_EMERG, "================================\n"); diff --git a/src/arch/riscv/virtual_memory.c b/src/arch/riscv/virtual_memory.c index 43e3d70..ed2127c 100644 --- a/src/arch/riscv/virtual_memory.c +++ b/src/arch/riscv/virtual_memory.c @@ -13,8 +13,10 @@ * to M-mode). In practice, this variable has been a lifesaver. It is * still not quite determined which delegation might by unallowed by * the spec so for now we enumerate and set them all. */ -static int delegate = 0 +static u32 delegate = 0 | (1 << CAUSE_MISALIGNED_FETCH) + | (1 << CAUSE_MISALIGNED_STORE) + | (1 << CAUSE_MISALIGNED_LOAD) | (1 << CAUSE_FETCH_ACCESS) | (1 << CAUSE_ILLEGAL_INSTRUCTION) | (1 << CAUSE_BREAKPOINT) -- To view, visit https://review.coreboot.org/c/coreboot/+/81416?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I84a8d433ed2f50745686a8c109d101e8718f2a46 Gerrit-Change-Number: 81416 Gerrit-PatchSet: 9 Gerrit-Owner: ron minnich <rminnich(a)gmail.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: ron minnich <rminnich(a)gmail.com> Gerrit-CC: Martin L Roth <gaumless(a)gmail.com> Gerrit-MessageType: merged

1 0

[L] Change in coreboot[main]: arch/riscv: remove misaligned load/store/fetch handling
by ron minnich (Code Review) 27 Mar '24

27 Mar '24

Attention is currently required from: Martin L Roth, Philipp Hug. ron minnich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81416?usp=email ) Change subject: arch/riscv: remove misaligned load/store/fetch handling ...................................................................... Patch Set 8: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/81416/comment/cb64660b_2e13733d : PS7, Line 24: invisble > invisible Done -- To view, visit https://review.coreboot.org/c/coreboot/+/81416?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I84a8d433ed2f50745686a8c109d101e8718f2a46 Gerrit-Change-Number: 81416 Gerrit-PatchSet: 8 Gerrit-Owner: ron minnich <rminnich(a)gmail.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Philipp Hug <philipp(a)hug.cx> Gerrit-Comment-Date: Wed, 27 Mar 2024 14:44:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Martin L Roth <gaumless(a)gmail.com> Gerrit-MessageType: comment

1 0

[L] Change in coreboot[main]: arch/riscv: remove misaligned load/store/fetch handling
by ron minnich (Code Review) 27 Mar '24

27 Mar '24

Attention is currently required from: Philipp Hug, ron minnich. Hello Maximilian Brune, Philipp Hug, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/81416?usp=email to look at the new patch set (#8). Change subject: arch/riscv: remove misaligned load/store/fetch handling ...................................................................... arch/riscv: remove misaligned load/store/fetch handling Testing on the unmatched shows the code no longer works completely correctly; Linux has taken over the handling of misalignment anyway, because handling it in firmware, with the growing complexity of the ISA and the awkward way in which it has to be handled, is more trouble than its worth. Plus, we don't WANT misalignment handled, magically, in firmware: the cost of getting it wrong is high (as I've spent a month learning); the performance is terrible (350x slowdown; and most toolchains now know to avoid unaligned load/store on RISC-V anyway. But, mostly, if alignment problems exist, *we need to know*, and if they're handled invisibly in firmware, we don't. The problem with invisible handling was shown a while back in the Go toolchain: runtime had a small error, such that many misaligned load/store were happening, and it was not discovered for some time. Had a trap been directed to kernel or user on misalignment, the problem would have been known immediately, not after many months. (The error, btw, was masking the address with 3, not 7, to detect misalignment; an easy mistake!). But, the coreboot code does not work any more any way, and it's not worth fixing. Remove it. Tested by booting Linux to runlevel 1; before, it would hang on an alignment fault, as the alignment code was failing (somewhere). This takes the coreboot SBI code much closer to revival. Change-Id: I84a8d433ed2f50745686a8c109d101e8718f2a46 Signed-off-by: Ronald G Minnich <rminnich(a)gmail.com> --- M src/arch/riscv/Makefile.mk M src/arch/riscv/include/arch/exception.h D src/arch/riscv/misaligned.c M src/arch/riscv/trap_handler.c M src/arch/riscv/virtual_memory.c 5 files changed, 4 insertions(+), 265 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/81416/8 -- To view, visit https://review.coreboot.org/c/coreboot/+/81416?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I84a8d433ed2f50745686a8c109d101e8718f2a46 Gerrit-Change-Number: 81416 Gerrit-PatchSet: 8 Gerrit-Owner: ron minnich <rminnich(a)gmail.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Philipp Hug <philipp(a)hug.cx> Gerrit-Attention: ron minnich <rminnich(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[XS] Change in coreboot[main]: mb/google/brox: Enable PMC pins to work with PD
by Varshit Pandya (Code Review) 27 Mar '24

27 Mar '24

Attention is currently required from: Ashish Kumar Mishra, Deepti Deshatty, Shelley Chen. Varshit Pandya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81207?usp=email ) Change subject: mb/google/brox: Enable PMC pins to work with PD ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/81207/comment/f3eecbee_845121fb : PS1, Line 9: For PMC-PD communication we need to enable 3 pins. > Thanks. This comment looks applicable to all pins configured as NF1 in this file. […] Hey Deepti, No, I am just talking about pins B11, C6 and C7 which are configured in this patch. -- To view, visit https://review.coreboot.org/c/coreboot/+/81207?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ia678d291e7a14aefe09026e70478fea3f68c8e10 Gerrit-Change-Number: 81207 Gerrit-PatchSet: 1 Gerrit-Owner: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-Reviewer: Deepti Deshatty <deepti.deshatty(a)intel.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Attention: Shelley Chen <shchen(a)google.com> Gerrit-Attention: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-Attention: Deepti Deshatty <deepti.deshatty(a)intel.com> Gerrit-Comment-Date: Wed, 27 Mar 2024 14:42:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Varshit Pandya <pandyavarshit(a)gmail.com> Comment-In-Reply-To: Deepti Deshatty <deepti.deshatty(a)intel.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: device/device_util: Use const qualifier
by Nico Huber (Code Review) 27 Mar '24

27 Mar '24

Attention is currently required from: Arthur Heymans, Christian Walter, Felix Held, Johnny Lin, Jonathan Zhang, Jérémy Compostella, Martin L Roth, Patrick Rudolph, Shuo Liu, Tim Chu. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81275?usp=email ) Change subject: device/device_util: Use const qualifier ...................................................................... Patch Set 13: (2 comments) File src/device/device_util.c: https://review.coreboot.org/c/coreboot/+/81275/comment/8cba0f0f_260bf2a1 : PS13, Line 256: assert(is_pci(dev)); Could we remove the `pci` from the name and comments instead? coreboot has no concept of a PCI domain, and the function should work for any device below a domain. File src/soc/intel/xeon_sp/include/soc/chip_common.h: https://review.coreboot.org/c/coreboot/+/81275/comment/0eb28631_4ecfcb05 : PS13, Line 77: bool is_cxl_domain(const struct device *dev); Just noticed, half of this is unused and seems an unnecessary maintenance and review burden. -- To view, visit https://review.coreboot.org/c/coreboot/+/81275?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Iac04fe6931a43070f6638b399adbff2ce64829c9 Gerrit-Change-Number: 81275 Gerrit-PatchSet: 13 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Wed, 27 Mar 2024 14:37:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[L] Change in coreboot[main]: arch/riscv: remove misaligned load/store/fetch handling
by Martin L Roth (Code Review) 27 Mar '24

27 Mar '24

Attention is currently required from: Philipp Hug, ron minnich. Martin L Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81416?usp=email ) The change is no longer submittable: All-Comments-Resolved is unsatisfied now. Change subject: arch/riscv: remove misaligned load/store/fetch handling ...................................................................... Patch Set 7: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/81416/comment/e242e529_c4f6655c : PS7, Line 24: invisble invisible -- To view, visit https://review.coreboot.org/c/coreboot/+/81416?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I84a8d433ed2f50745686a8c109d101e8718f2a46 Gerrit-Change-Number: 81416 Gerrit-PatchSet: 7 Gerrit-Owner: ron minnich <rminnich(a)gmail.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Philipp Hug <philipp(a)hug.cx> Gerrit-Attention: ron minnich <rminnich(a)gmail.com> Gerrit-Comment-Date: Wed, 27 Mar 2024 14:35:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: lib/spd_bin: Add LPDDR5X dram_type in use_ddr4_params
by Karthik Ramasubramanian (Code Review) 27 Mar '24

27 Mar '24

Attention is currently required from: Ashish Kumar Mishra, Paul Menzel, Varshit Pandya. Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81437?usp=email ) Change subject: lib/spd_bin: Add LPDDR5X dram_type in use_ddr4_params ...................................................................... Patch Set 4: -Code-Review -- To view, visit https://review.coreboot.org/c/coreboot/+/81437?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Id78ef90c0dc2e869c1f0424674b982ba64ba3939 Gerrit-Change-Number: 81437 Gerrit-PatchSet: 4 Gerrit-Owner: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Attention: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Attention: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Wed, 27 Mar 2024 14:19:50 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: mb/google/nissa/var/anraggar: Modify the GPP_F15 of pen to EDGE_BOTH
by Martin L Roth (Code Review) 27 Mar '24

27 Mar '24

Martin L Roth has submitted this change. ( https://review.coreboot.org/c/coreboot/+/81441?usp=email ) Change subject: mb/google/nissa/var/anraggar: Modify the GPP_F15 of pen to EDGE_BOTH ...................................................................... mb/google/nissa/var/anraggar: Modify the GPP_F15 of pen to EDGE_BOTH Currently, simply changing the wake event configuration to ANY does not completely resolve the issue of inserting a pen not waking the system. The pen actually needs to wake up the system both when plugged in and when pulled out. This is because in the pen's GPP_F15 configuration, the original attribute is EDGE_SINGLE, which should be changed to EDGE_BOTH. BUG=b:328351027 TEST=insert and remove pen can wakes system up. Change-Id: I1823afd0bcb86804227117d2d5def38788bc7387 Signed-off-by: Qinghong Zeng <zengqinghong(a)huaqin.corp-partner.google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/81441 Reviewed-by: Weimin Wu <wuweimin(a)huaqin.corp-partner.google.com> Reviewed-by: Subrata Banik <subratabanik(a)google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Eric Lai <ericllai(a)google.com> --- M src/mainboard/google/brya/variants/anraggar/gpio.c 1 file changed, 2 insertions(+), 0 deletions(-) Approvals: Weimin Wu: Looks good to me, but someone else must approve build bot (Jenkins): Verified Subrata Banik: Looks good to me, approved Eric Lai: Looks good to me, approved diff --git a/src/mainboard/google/brya/variants/anraggar/gpio.c b/src/mainboard/google/brya/variants/anraggar/gpio.c index 8bc3f6b..94e03f6 100644 --- a/src/mainboard/google/brya/variants/anraggar/gpio.c +++ b/src/mainboard/google/brya/variants/anraggar/gpio.c @@ -50,6 +50,8 @@ PAD_CFG_GPO_LOCK(GPP_F11, 1, LOCK_CONFIG), /* F12 : GSXDOUT ==> WWAN_RST_L */ PAD_CFG_GPO_LOCK(GPP_F12, 1, LOCK_CONFIG), + /* F15 : GSXSRESET# ==> SOC_PEN_DETECT_ODL */ + PAD_CFG_GPI_SCI_HIGH(GPP_F15, NONE, PLTRST, EDGE_BOTH), /* F18 : THC1_SPI2_INT# ==> EN_PP2800_AFVDD */ PAD_CFG_GPO(GPP_F18, 0, DEEP), /* F23 : V1P05_CTRL ==> NC*/ -- To view, visit https://review.coreboot.org/c/coreboot/+/81441?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I1823afd0bcb86804227117d2d5def38788bc7387 Gerrit-Change-Number: 81441 Gerrit-PatchSet: 3 Gerrit-Owner: Qinghong Zeng <zengqinghong(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Weimin Wu <wuweimin(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[main]: mb/google/brya: Create yavista variant
by Martin L Roth (Code Review) 27 Mar '24

27 Mar '24

Martin L Roth has submitted this change. ( https://review.coreboot.org/c/coreboot/+/80342?usp=email ) Change subject: mb/google/brya: Create yavista variant ...................................................................... mb/google/brya: Create yavista variant Create the yavista variant of the nissa reference board by copying the template files to a new directory named for the variant. (Auto-Generated by create_coreboot_variant.sh version 4.5.0.) BUG=b:321583226 BRANCH=None TEST=util/abuild/abuild -p none -t google/brya -x -a make sure the build includes GOOGLE_YAVISTA. Change-Id: I6fa464a4dcd9551a42e8746e64c724b3582dbe02 Signed-off-by: Hsueh Rasheed <hsueh.rasheed(a)inventec.corp-partner.google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/80342 Reviewed-by: Eric Lai <ericllai(a)google.com> Reviewed-by: Derek Huang <derekhuang(a)google.com> Reviewed-by: Paul Menzel <paulepanter(a)mailbox.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Subrata Banik <subratabanik(a)google.com> --- M src/mainboard/google/brya/Kconfig M src/mainboard/google/brya/Kconfig.name A src/mainboard/google/brya/variants/yavista/include/variant/ec.h A src/mainboard/google/brya/variants/yavista/include/variant/gpio.h A src/mainboard/google/brya/variants/yavista/memory/Makefile.mk A src/mainboard/google/brya/variants/yavista/memory/dram_id.generated.txt A src/mainboard/google/brya/variants/yavista/memory/mem_parts_used.txt A src/mainboard/google/brya/variants/yavista/overridetree.cb 8 files changed, 47 insertions(+), 0 deletions(-) Approvals: Paul Menzel: Looks good to me, but someone else must approve Subrata Banik: Looks good to me, approved Eric Lai: Looks good to me, approved build bot (Jenkins): Verified Derek Huang: Looks good to me, approved diff --git a/src/mainboard/google/brya/Kconfig b/src/mainboard/google/brya/Kconfig index 8cb8428..992be88 100644 --- a/src/mainboard/google/brya/Kconfig +++ b/src/mainboard/google/brya/Kconfig @@ -530,6 +530,9 @@ select INTEL_GMA_HAVE_VBT select EC_GOOGLE_CHROMEEC_INCLUDE_SSFC_IN_FW_CONFIG +config BOARD_GOOGLE_YAVISTA + select BOARD_GOOGLE_BASEBOARD_NISSA + config BOARD_GOOGLE_ZYDRON select BOARD_GOOGLE_BASEBOARD_BRYA select CHROMEOS_WIFI_SAR if CHROMEOS @@ -709,6 +712,7 @@ default "Nova" if BOARD_GOOGLE_NOVA default "Bujia" if BOARD_GOOGLE_BUJIA default "Tivviks" if BOARD_GOOGLE_TIVVIKS + default "Yavista" if BOARD_GOOGLE_YAVISTA config VARIANT_DIR default "brya0" if BOARD_GOOGLE_BRYA0 @@ -767,6 +771,7 @@ default "nova" if BOARD_GOOGLE_NOVA default "bujia" if BOARD_GOOGLE_BUJIA default "nivviks" if BOARD_GOOGLE_TIVVIKS + default "yavista" if BOARD_GOOGLE_YAVISTA config VBOOT select VBOOT_EARLY_EC_SYNC if !BOARD_GOOGLE_BASEBOARD_NISSA diff --git a/src/mainboard/google/brya/Kconfig.name b/src/mainboard/google/brya/Kconfig.name index 639603d..6cedaac 100644 --- a/src/mainboard/google/brya/Kconfig.name +++ b/src/mainboard/google/brya/Kconfig.name @@ -169,3 +169,6 @@ config BOARD_GOOGLE_BUJIA bool "-> Bujia" + +config BOARD_GOOGLE_YAVISTA + bool "-> Yavista" diff --git a/src/mainboard/google/brya/variants/yavista/include/variant/ec.h b/src/mainboard/google/brya/variants/yavista/include/variant/ec.h new file mode 100644 index 0000000..7a2a6ff --- /dev/null +++ b/src/mainboard/google/brya/variants/yavista/include/variant/ec.h @@ -0,0 +1,8 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#ifndef __VARIANT_EC_H__ +#define __VARIANT_EC_H__ + +#include <baseboard/ec.h> + +#endif diff --git a/src/mainboard/google/brya/variants/yavista/include/variant/gpio.h b/src/mainboard/google/brya/variants/yavista/include/variant/gpio.h new file mode 100644 index 0000000..c4fe342 --- /dev/null +++ b/src/mainboard/google/brya/variants/yavista/include/variant/gpio.h @@ -0,0 +1,8 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#ifndef VARIANT_GPIO_H +#define VARIANT_GPIO_H + +#include <baseboard/gpio.h> + +#endif diff --git a/src/mainboard/google/brya/variants/yavista/memory/Makefile.mk b/src/mainboard/google/brya/variants/yavista/memory/Makefile.mk new file mode 100644 index 0000000..eace2e4 --- /dev/null +++ b/src/mainboard/google/brya/variants/yavista/memory/Makefile.mk @@ -0,0 +1,5 @@ +# SPDX-License-Identifier: GPL-2.0-or-later +# This is an auto-generated file. Do not edit!! +# Add memory parts in mem_parts_used.txt and run spd_tools to regenerate. + +SPD_SOURCES = placeholder diff --git a/src/mainboard/google/brya/variants/yavista/memory/dram_id.generated.txt b/src/mainboard/google/brya/variants/yavista/memory/dram_id.generated.txt new file mode 100644 index 0000000..fa24790 --- /dev/null +++ b/src/mainboard/google/brya/variants/yavista/memory/dram_id.generated.txt @@ -0,0 +1 @@ +DRAM Part Name ID to assign diff --git a/src/mainboard/google/brya/variants/yavista/memory/mem_parts_used.txt b/src/mainboard/google/brya/variants/yavista/memory/mem_parts_used.txt new file mode 100644 index 0000000..2499005 --- /dev/null +++ b/src/mainboard/google/brya/variants/yavista/memory/mem_parts_used.txt @@ -0,0 +1,11 @@ +# This is a CSV file containing a list of memory parts used by this variant. +# One part per line with an optional fixed ID in column 2. +# Only include a fixed ID if it is required for legacy reasons! +# Generated IDs are dependent on the order of parts in this file, +# so new parts must always be added at the end of the file! +# +# Generate an updated Makefile.mk and dram_id.generated.txt by running the +# part_id_gen tool from util/spd_tools. +# See util/spd_tools/README.md for more details and instructions. + +# Part Name diff --git a/src/mainboard/google/brya/variants/yavista/overridetree.cb b/src/mainboard/google/brya/variants/yavista/overridetree.cb new file mode 100644 index 0000000..4f2c04a --- /dev/null +++ b/src/mainboard/google/brya/variants/yavista/overridetree.cb @@ -0,0 +1,6 @@ +chip soc/intel/alderlake + + device domain 0 on + end + +end -- To view, visit https://review.coreboot.org/c/coreboot/+/80342?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I6fa464a4dcd9551a42e8746e64c724b3582dbe02 Gerrit-Change-Number: 80342 Gerrit-PatchSet: 10 Gerrit-Owner: Rasheed Hsueh <hsueh.rasheed(a)inventec.corp-partner.google.com> Gerrit-Reviewer: Derek Huang <derekhuang(a)google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Henry Sun <henrysun(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Garen Wu <wu.garen(a)inventec.corp-partner.google.com> Gerrit-CC: Ryan Lin <ryan.lin(a)intel.corp-partner.google.com> Gerrit-CC: Steven Yeh <yeh.stevenct(a)inventec.corp-partner.google.com> Gerrit-MessageType: merged

1 0

[XS] Change in coreboot[main]: mb/google/brox: Enable PMC pins to work with PD
by Deepti Deshatty (Code Review) 27 Mar '24

27 Mar '24

Attention is currently required from: Ashish Kumar Mishra, Shelley Chen, Varshit Pandya. Deepti Deshatty has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81207?usp=email ) Change subject: mb/google/brox: Enable PMC pins to work with PD ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/81207/comment/58456e69_5ea21540 : PS1, Line 9: For PMC-PD communication we need to enable 3 pins. > since you are configuring pins to native function, it would be good to mention what is that native f […] Thanks. This comment looks applicable to all pins configured as NF1 in this file. Suggest taking it as a separate topic as this is a critical change. -- To view, visit https://review.coreboot.org/c/coreboot/+/81207?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ia678d291e7a14aefe09026e70478fea3f68c8e10 Gerrit-Change-Number: 81207 Gerrit-PatchSet: 1 Gerrit-Owner: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-Reviewer: Deepti Deshatty <deepti.deshatty(a)intel.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Attention: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Attention: Shelley Chen <shchen(a)google.com> Gerrit-Attention: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-Comment-Date: Wed, 27 Mar 2024 14:12:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit March 2024