coreboot-gerrit November 2023

coreboot-gerrit@coreboot.org

1 participants
3037 discussions

[S] Change in coreboot[main]: WIP:soc/amd/mendocino: Add DBG2
by Bao Zheng (Code Review) 23 Nov '23

23 Nov '23

Attention is currently required from: Cliff Huang, Felix Held, Fred Reitberger, Jason Glenesk, Lance Zhao, Matt DeVillier, Tim Wawrzynczak, Zheng Bao. Bao Zheng has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79174?usp=email ) Change subject: WIP:soc/amd/mendocino: Add DBG2 ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/79174/comment/3ddc1ab8_d521b79f : PS2, Line 9: 240426142 > bug id doesn't seem to be related to the patch 303689867 -- To view, visit https://review.coreboot.org/c/coreboot/+/79174?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: I3c97a78d1889549421baf0bc1a2e8f959a0f47e2 Gerrit-Change-Number: 79174 Gerrit-PatchSet: 2 Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: Zheng Bao Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Zheng Bao Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Thu, 23 Nov 2023 01:53:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: mb/emulation/qemu-{i440fx,q35}: Don't report ROM page tables as resource
by Benjamin Doron (Code Review) 23 Nov '23

23 Nov '23

Benjamin Doron has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/79228?usp=email ) Change subject: mb/emulation/qemu-{i440fx,q35}: Don't report ROM page tables as resource ...................................................................... mb/emulation/qemu-{i440fx,q35}: Don't report ROM page tables as resource This is no longer required now that page tables are dynamically generated. Change-Id: I106c1730787a88c03719acfb8a80e7b840b29997 Signed-off-by: Benjamin Doron <benjamin.doron(a)9elements.com> --- M src/mainboard/emulation/qemu-i440fx/mainboard.c M src/mainboard/emulation/qemu-q35/mainboard.c 2 files changed, 1 insertion(+), 16 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/28/79228/1 diff --git a/src/mainboard/emulation/qemu-i440fx/mainboard.c b/src/mainboard/emulation/qemu-i440fx/mainboard.c index 7debf8c..46f9019 100644 --- a/src/mainboard/emulation/qemu-i440fx/mainboard.c +++ b/src/mainboard/emulation/qemu-i440fx/mainboard.c @@ -31,18 +31,8 @@ } } -static void qemu_nb_read_resources(struct device *dev) -{ - pci_dev_read_resources(dev); - - if (CONFIG(ARCH_RAMSTAGE_X86_64)) { - /* Reserve page tables in DRAM. FIXME: Remove once x86_64 page tables reside in CBMEM */ - reserved_ram_range(dev, 0, CONFIG_ARCH_X86_64_PGTBL_LOC, 6 * 0x1000); - } -} - struct device_operations nb_operations = { - .read_resources = qemu_nb_read_resources, + .read_resources = pci_dev_read_resources, .set_resources = pci_dev_set_resources, .enable_resources = pci_dev_enable_resources, .init = qemu_nb_init, diff --git a/src/mainboard/emulation/qemu-q35/mainboard.c b/src/mainboard/emulation/qemu-q35/mainboard.c index 1ff4c04..1152ccb 100644 --- a/src/mainboard/emulation/qemu-q35/mainboard.c +++ b/src/mainboard/emulation/qemu-q35/mainboard.c @@ -53,11 +53,6 @@ mmconf_resource(dev, 2); - if (CONFIG(ARCH_RAMSTAGE_X86_64)) { - /* Reserve page tables in DRAM. FIXME: Remove once x86_64 page tables reside in CBMEM */ - reserved_ram_range(dev, 0, CONFIG_ARCH_X86_64_PGTBL_LOC, 6 * 0x1000); - } - smm_region(&tseg_base, &tseg_size); reserved_ram_range(dev, ESMRAMC, tseg_base, tseg_size); } -- To view, visit https://review.coreboot.org/c/coreboot/+/79228?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: I106c1730787a88c03719acfb8a80e7b840b29997 Gerrit-Change-Number: 79228 Gerrit-PatchSet: 1 Gerrit-Owner: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-MessageType: newchange

1 0

[S] Change in coreboot[main]: [DNM] cpu/x86: Unconditionally compile 64-bit entry code
by Benjamin Doron (Code Review) 23 Nov '23

23 Nov '23

Attention is currently required from: Jérémy Compostella. Benjamin Doron has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/79227?usp=email ) Change subject: [DNM] cpu/x86: Unconditionally compile 64-bit entry code ...................................................................... [DNM] cpu/x86: Unconditionally compile 64-bit entry code We may want to call long-mode code and return to 32-bit coreboot. This requires callers to only include entry64.inc if they mean to switch, so, fix qemu-x86 CPU code that did not. While we're here, only build bootblock static page tables if it's compiled as 64-bit. DNM: Is it necessary to handle non-transitions? (32-bit -> 32-bit) - If so, perhaps it's best to handle it in mode_switch.h Change-Id: Idde8609239426de664907df24a016cc35d320553 Signed-off-by: Benjamin Doron <benjamin.doron(a)9elements.com> --- M src/cpu/qemu-x86/cache_as_ram_bootblock.S M src/cpu/x86/64bit/Makefile.inc M src/cpu/x86/64bit/entry64.inc M src/cpu/x86/Makefile.inc 4 files changed, 7 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/79227/1 diff --git a/src/cpu/qemu-x86/cache_as_ram_bootblock.S b/src/cpu/qemu-x86/cache_as_ram_bootblock.S index 96ed468..052461b 100644 --- a/src/cpu/qemu-x86/cache_as_ram_bootblock.S +++ b/src/cpu/qemu-x86/cache_as_ram_bootblock.S @@ -77,6 +77,7 @@ /* Align the stack and keep aligned for call to bootblock_c_entry() */ and $0xfffffff0, %esp +#if ENV_X86_64 /* Get page table address */ movl $(CONFIG_ARCH_X86_64_PGTBL_LOC), %eax @@ -84,7 +85,6 @@ #include <cpu/x86/64bit/entry64.inc> /* Restore the BIST result and timestamps. */ -#if ENV_X86_64 movd %mm2, %rdi shlq $32, %rdi movd %mm1, %rsi diff --git a/src/cpu/x86/64bit/Makefile.inc b/src/cpu/x86/64bit/Makefile.inc index e1cf743..611538c 100644 --- a/src/cpu/x86/64bit/Makefile.inc +++ b/src/cpu/x86/64bit/Makefile.inc @@ -2,6 +2,8 @@ all_x86-y += mode_switch.S +ifeq ($(CONFIG_ARCH_BOOTBLOCK_X86_64), y) + # Add --defsym=_start=0 to suppress a linker warning. $(objcbfs)/pt: $(dir)/pt.S $(obj)/config.h $(CC_bootblock) $(CFLAGS_bootblock) $(CPPFLAGS_bootblock) -o $@.tmp $< -Wl,--section-start=.rodata=$(CONFIG_ARCH_X86_64_PGTBL_LOC),--defsym=_start=0 @@ -13,3 +15,5 @@ pagetables-type := raw pagetables-compression := none pagetables-COREBOOT-position := $(CONFIG_ARCH_X86_64_PGTBL_LOC) + +endif diff --git a/src/cpu/x86/64bit/entry64.inc b/src/cpu/x86/64bit/entry64.inc index 7a01a4f..50eb8a2 100644 --- a/src/cpu/x86/64bit/entry64.inc +++ b/src/cpu/x86/64bit/entry64.inc @@ -9,9 +9,6 @@ * Clobbers: eax, ecx, edx */ -#if ENV_X86_64 - .code32 - #include <cpu/x86/msr.h> #if defined(__RAMSTAGE__) #include <arch/ram_segs.h> @@ -20,6 +17,7 @@ #endif /* Caller to provide address of page tables in eax */ +.code32 setup_longmode: /* load identity mapped page tables */ movl %eax, %cr3 @@ -50,4 +48,3 @@ .code64 __longmode_start: -#endif diff --git a/src/cpu/x86/Makefile.inc b/src/cpu/x86/Makefile.inc index 9e9b837..2e084a6 100644 --- a/src/cpu/x86/Makefile.inc +++ b/src/cpu/x86/Makefile.inc @@ -5,8 +5,7 @@ subdirs-y += pae subdirs-$(CONFIG_HAVE_SMI_HANDLER) += smm subdirs-$(CONFIG_UDELAY_TSC) += tsc -# Use ARCH_BOOTBLOCK_X86_64 as a proxy for knowing if 64bit is going to be used -subdirs-$(CONFIG_ARCH_BOOTBLOCK_X86_64) += 64bit +subdirs-y += 64bit subdirs-y += cache subdirs-$(CONFIG_PARALLEL_MP) += name -- To view, visit https://review.coreboot.org/c/coreboot/+/79227?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: Idde8609239426de664907df24a016cc35d320553 Gerrit-Change-Number: 79227 Gerrit-PatchSet: 1 Gerrit-Owner: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-MessageType: newchange

1 0

[M] Change in coreboot[main]: treewide: Prepare mode switch code for dynamically generated paging
by Benjamin Doron (Code Review) 23 Nov '23

23 Nov '23

Benjamin Doron has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/79226?usp=email ) Change subject: treewide: Prepare mode switch code for dynamically generated paging ...................................................................... treewide: Prepare mode switch code for dynamically generated paging Make the long mode entry code generic to prepare for dynamically generated page tables. entry64.inc now expects the caller to load the page tables' address into the EAX register. The mode switch assembly now stashes the previous page table address to restore after the protected mode call is incomplete. Follow-up changes will program/use dynamically generated tables for the SIPI vector and SMM stub. This is not a functional change. Change-Id: I1ef7c8bee0c38395d5436f6479f11b3a4208f53e Signed-off-by: Benjamin Doron <benjamin.doron(a)9elements.com> --- M src/cpu/intel/car/core2/cache_as_ram.S M src/cpu/intel/car/non-evict/cache_as_ram.S M src/cpu/intel/car/p4-netburst/cache_as_ram.S M src/cpu/qemu-x86/cache_as_ram_bootblock.S M src/cpu/x86/64bit/entry64.inc M src/cpu/x86/64bit/mode_switch.S M src/cpu/x86/64bit/pt.S M src/cpu/x86/sipi_vector.S M src/cpu/x86/smm/smm_stub.S M src/soc/amd/common/block/cpu/noncar/pre_c.S M src/soc/intel/common/block/cpu/car/cache_as_ram.S 11 files changed, 39 insertions(+), 11 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/79226/1 diff --git a/src/cpu/intel/car/core2/cache_as_ram.S b/src/cpu/intel/car/core2/cache_as_ram.S index e134717..cfdc2a7 100644 --- a/src/cpu/intel/car/core2/cache_as_ram.S +++ b/src/cpu/intel/car/core2/cache_as_ram.S @@ -162,6 +162,8 @@ subl $4, %esp #if ENV_X86_64 + /* Get page table address */ + movl $(CONFIG_ARCH_X86_64_PGTBL_LOC), %eax #include <cpu/x86/64bit/entry64.inc> diff --git a/src/cpu/intel/car/non-evict/cache_as_ram.S b/src/cpu/intel/car/non-evict/cache_as_ram.S index 76986ff..e8f256e 100644 --- a/src/cpu/intel/car/non-evict/cache_as_ram.S +++ b/src/cpu/intel/car/non-evict/cache_as_ram.S @@ -213,6 +213,8 @@ andl $0xfffffff0, %esp #if ENV_X86_64 + /* Get page table address */ + movl $(CONFIG_ARCH_X86_64_PGTBL_LOC), %eax #include <cpu/x86/64bit/entry64.inc> diff --git a/src/cpu/intel/car/p4-netburst/cache_as_ram.S b/src/cpu/intel/car/p4-netburst/cache_as_ram.S index f7c023b..c541b1a 100644 --- a/src/cpu/intel/car/p4-netburst/cache_as_ram.S +++ b/src/cpu/intel/car/p4-netburst/cache_as_ram.S @@ -362,6 +362,9 @@ subl $4, %esp #if ENV_X86_64 + /* Get page table address */ + movl $(CONFIG_ARCH_X86_64_PGTBL_LOC), %eax + #include <cpu/x86/64bit/entry64.inc> movd %mm2, %rdi diff --git a/src/cpu/qemu-x86/cache_as_ram_bootblock.S b/src/cpu/qemu-x86/cache_as_ram_bootblock.S index 0943e35..96ed468 100644 --- a/src/cpu/qemu-x86/cache_as_ram_bootblock.S +++ b/src/cpu/qemu-x86/cache_as_ram_bootblock.S @@ -77,6 +77,9 @@ /* Align the stack and keep aligned for call to bootblock_c_entry() */ and $0xfffffff0, %esp + /* Get page table address */ + movl $(CONFIG_ARCH_X86_64_PGTBL_LOC), %eax + /* entry64.inc preserves ebx. */ #include <cpu/x86/64bit/entry64.inc> diff --git a/src/cpu/x86/64bit/entry64.inc b/src/cpu/x86/64bit/entry64.inc index 7da68b4..7a01a4f 100644 --- a/src/cpu/x86/64bit/entry64.inc +++ b/src/cpu/x86/64bit/entry64.inc @@ -11,9 +11,6 @@ #if ENV_X86_64 .code32 -#if (CONFIG_ARCH_X86_64_PGTBL_LOC & 0xfff) > 0 -#error pagetables must be 4KiB aligned! -#endif #include <cpu/x86/msr.h> #if defined(__RAMSTAGE__) @@ -22,11 +19,8 @@ #include <arch/rom_segs.h> #endif - +/* Caller to provide address of page tables in eax */ setup_longmode: - /* Get page table address */ - movl $(CONFIG_ARCH_X86_64_PGTBL_LOC), %eax - /* load identity mapped page tables */ movl %eax, %cr3 diff --git a/src/cpu/x86/64bit/mode_switch.S b/src/cpu/x86/64bit/mode_switch.S index c27f540..7a4ab90 100644 --- a/src/cpu/x86/64bit/mode_switch.S +++ b/src/cpu/x86/64bit/mode_switch.S @@ -19,6 +19,10 @@ movl %gs, %eax push %rax + /* Backup cr3 to stack */ + movq %cr3, %rax + push %rax + /* Arguments to stack */ push %rdi push %rsi @@ -27,9 +31,9 @@ #include <cpu/x86/64bit/exit32.inc> - movl -56(%ebp), %eax /* Argument count */ - movl -72(%ebp), %edx /* Argument 0 */ - movl -80(%ebp), %ecx /* Argument 1 */ + movl -64(%ebp), %eax /* Argument count */ + movl -80(%ebp), %edx /* Argument 0 */ + movl -88(%ebp), %ecx /* Argument 1 */ /* Align the stack */ andl $0xFFFFFFF0, %esp @@ -50,10 +54,13 @@ pushl %edx /* Argument 0 */ 1: - movl -64(%ebp), %ebx /* Function to call */ + movl -72(%ebp), %ebx /* Function to call */ call *%ebx movl %eax, %ebx + /* Get page table address */ + movl -56(%ebp), %eax + /* Preserves ebx */ #include <cpu/x86/64bit/entry64.inc> diff --git a/src/cpu/x86/64bit/pt.S b/src/cpu/x86/64bit/pt.S index b105528..e990c71 100644 --- a/src/cpu/x86/64bit/pt.S +++ b/src/cpu/x86/64bit/pt.S @@ -7,6 +7,10 @@ * Page table attributes: WB, User+Supervisor, Present, Writeable, Accessed, Dirty */ +#if (CONFIG_ARCH_X86_64_PGTBL_LOC & 0xfff) > 0 +#error pagetables must be 4KiB aligned! +#endif + .section .rodata #define _PRES (1ULL << 0) #define _RW (1ULL << 1) diff --git a/src/cpu/x86/sipi_vector.S b/src/cpu/x86/sipi_vector.S index 39973db..401e8e2 100644 --- a/src/cpu/x86/sipi_vector.S +++ b/src/cpu/x86/sipi_vector.S @@ -222,6 +222,9 @@ andl $0xfffffff0, %esp /* ensure stack alignment */ #if ENV_X86_64 + /* Get page table address */ + movl $(CONFIG_ARCH_X86_64_PGTBL_LOC), %eax + /* entry64.inc preserves ebx, esi, edi, ebp */ #include <cpu/x86/64bit/entry64.inc> movabs c_handler, %eax diff --git a/src/cpu/x86/smm/smm_stub.S b/src/cpu/x86/smm/smm_stub.S index f97ab59..802dd92 100644 --- a/src/cpu/x86/smm/smm_stub.S +++ b/src/cpu/x86/smm/smm_stub.S @@ -194,6 +194,10 @@ */ #if ENV_X86_64 mov %ecx, %edi + + /* Get page table address */ + movl $(CONFIG_ARCH_X86_64_PGTBL_LOC), %eax + /* entry64.inc preserves ebx, esi, edi, ebp */ #include <cpu/x86/64bit/entry64.inc> mov %edi, %ecx diff --git a/src/soc/amd/common/block/cpu/noncar/pre_c.S b/src/soc/amd/common/block/cpu/noncar/pre_c.S index eb556fa..b06f0f5 100644 --- a/src/soc/amd/common/block/cpu/noncar/pre_c.S +++ b/src/soc/amd/common/block/cpu/noncar/pre_c.S @@ -27,6 +27,9 @@ post_code(POSTCODE_BOOTBLOCK_PRE_C_ENTRY) #if ENV_X86_64 + /* Get page table address */ + movl $(CONFIG_ARCH_X86_64_PGTBL_LOC), %eax + #include <cpu/x86/64bit/entry64.inc> #endif diff --git a/src/soc/intel/common/block/cpu/car/cache_as_ram.S b/src/soc/intel/common/block/cpu/car/cache_as_ram.S index 61cbe307..da84e7b 100644 --- a/src/soc/intel/common/block/cpu/car/cache_as_ram.S +++ b/src/soc/intel/common/block/cpu/car/cache_as_ram.S @@ -279,6 +279,9 @@ andl $0xfffffff0, %esp #if ENV_X86_64 + /* Get page table address */ + movl $(CONFIG_ARCH_X86_64_PGTBL_LOC), %eax + #include <cpu/x86/64bit/entry64.inc> movd %mm2, %rdi shlq $32, %rdi -- To view, visit https://review.coreboot.org/c/coreboot/+/79226?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: I1ef7c8bee0c38395d5436f6479f11b3a4208f53e Gerrit-Change-Number: 79226 Gerrit-PatchSet: 1 Gerrit-Owner: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-MessageType: newchange

1 0

[XS] Change in coreboot[main]: mb/google/nissa: make GPP_F17 edge triggered to avoid spamming EC
by Scott Chao (Code Review) 23 Nov '23

23 Nov '23

Attention is currently required from: Eric Lai, Kapil Porwal, Nick Vaccaro, Paul Menzel. Scott Chao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79244?usp=email ) Change subject: mb/google/nissa: make GPP_F17 edge triggered to avoid spamming EC ...................................................................... Patch Set 3: (6 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/79244/comment/0d4c9b5c_69e5f775 : PS1, Line 7: update GPP_F17 to edge trigger > Maybe: […] Done https://review.coreboot.org/c/coreboot/+/79244/comment/8c66c64c_cf480787 : PS1, Line 9: AP > Application Processor? Done https://review.coreboot.org/c/coreboot/+/79244/comment/571cb7aa_0af84ab5 : PS1, Line 9: it will cause > and it causes AP to keep sending Done https://review.coreboot.org/c/coreboot/+/79244/comment/c1f467f8_afc00c0e : PS1, Line 9: GPP_F17 > Unfortunately, I do not understand the commit message without looking at the comment in the diff. […] Done https://review.coreboot.org/c/coreboot/+/79244/comment/4ace17ff_75f6e597 : PS1, Line 10: connect > connecting Done https://review.coreboot.org/c/coreboot/+/79244/comment/9d4c9df6_82286bd5 : PS1, Line 14: TEST=Test on joxer with 6 units, 0 out of 60 times. > 1. What does “unit” refer to? […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/79244?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: I3ceb1dfce46376a6a9a8c6cb6d691d818a0a42ff Gerrit-Change-Number: 79244 Gerrit-PatchSet: 3 Gerrit-Owner: Scott Chao <scott_chao(a)wistron.corp-partner.google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Reka Norman <rekanorman(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anfernee Chen <anfernee_chen(a)wistron.corp-partner.google.com> Gerrit-CC: Mark Hsieh <mark_hsieh(a)wistron.corp-partner.google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Terry Chen <terry_chen(a)wistron.corp-partner.google.com> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Comment-Date: Thu, 23 Nov 2023 00:48:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: mb/google/nissa: make GPP_F17 edge triggered to avoid spamming EC
by Scott Chao (Code Review) 23 Nov '23

23 Nov '23

Attention is currently required from: Eric Lai, Kapil Porwal, Nick Vaccaro, Scott Chao. Hello Eric Lai, Kapil Porwal, Nick Vaccaro, Reka Norman, Subrata Banik, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/79244?usp=email to look at the new patch set (#3). Change subject: mb/google/nissa: make GPP_F17 edge triggered to avoid spamming EC ...................................................................... mb/google/nissa: make GPP_F17 edge triggered to avoid spamming EC In nissa platform, we configured GPP_F17 as SCI+APIC to wake the system and also generate IRQ to the IOAPIC. Currently, we set GPP_F17 to level triggered and it cause AP(Application Processor) to keep sending GET_NEXT_EVENT to EC during resume from suspend by connecting AC. So we change GPP_F17 to edge triggered to avoid this condition. BUG=b:308716748 TEST=Original failure rate was 7 out of 10 times and it reduced to 0 out of 60 times on six joxer systems. Signed-off-by: Scott Chao <scott_chao(a)wistron.corp-partner.google.com> Change-Id: I3ceb1dfce46376a6a9a8c6cb6d691d818a0a42ff --- M src/mainboard/google/brya/variants/baseboard/nissa/gpio.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/44/79244/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/79244?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: I3ceb1dfce46376a6a9a8c6cb6d691d818a0a42ff Gerrit-Change-Number: 79244 Gerrit-PatchSet: 3 Gerrit-Owner: Scott Chao <scott_chao(a)wistron.corp-partner.google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Reka Norman <rekanorman(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anfernee Chen <anfernee_chen(a)wistron.corp-partner.google.com> Gerrit-CC: Mark Hsieh <mark_hsieh(a)wistron.corp-partner.google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Terry Chen <terry_chen(a)wistron.corp-partner.google.com> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Scott Chao <scott_chao(a)wistron.corp-partner.google.com> Gerrit-MessageType: newpatchset

1 0

[XS] Change in coreboot[main]: mb/google/nissa: make GPP_F17 edge triggered to avoid spamming EC
by Scott Chao (Code Review) 23 Nov '23

23 Nov '23

Attention is currently required from: Eric Lai, Kapil Porwal, Nick Vaccaro, Scott Chao. Hello Eric Lai, Kapil Porwal, Nick Vaccaro, Reka Norman, Subrata Banik, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/79244?usp=email to look at the new patch set (#2). Change subject: mb/google/nissa: make GPP_F17 edge triggered to avoid spamming EC ...................................................................... mb/google/nissa: make GPP_F17 edge triggered to avoid spamming EC In nissa platform, we configured GPP_F17 as SCI+APIC to wake the system and also generate IRQ to the IOAPIC. Currently, we set GPP_F17 to level triggered and it cause AP(Application Processor) to keep sending GET_NEXT_EVENT to EC during resume from suspend by connecting AC. So we change GPP_F17 to edge triggered to avoid this condition. BUG=b:308716748 TEST=Original failure rate was 7 out of 10 times and it reduced to 0 out of 60 times on six joxer systems. Signed-off-by: Scott Chao <scott_chao(a)wistron.corp-partner.google.com> Change-Id: I3ceb1dfce46376a6a9a8c6cb6d691d818a0a42ff --- M src/mainboard/google/brya/variants/baseboard/nissa/gpio.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/44/79244/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/79244?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: I3ceb1dfce46376a6a9a8c6cb6d691d818a0a42ff Gerrit-Change-Number: 79244 Gerrit-PatchSet: 2 Gerrit-Owner: Scott Chao <scott_chao(a)wistron.corp-partner.google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Reka Norman <rekanorman(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anfernee Chen <anfernee_chen(a)wistron.corp-partner.google.com> Gerrit-CC: Mark Hsieh <mark_hsieh(a)wistron.corp-partner.google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Terry Chen <terry_chen(a)wistron.corp-partner.google.com> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Scott Chao <scott_chao(a)wistron.corp-partner.google.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in coreboot[main]: treewide: Move stdlib.h to commonlib
by Maximilian Brune (Code Review) 23 Nov '23

23 Nov '23

Attention is currently required from: Andrey Petrov, Arthur Heymans, Christian Walter, Hung-Te Lin, Jakub Czapiga, Johnny Lin, Julius Werner, Martin L Roth, Ronak Kanabar, Tim Chu, Yidi Lin, Yu-Ping Wu. Maximilian Brune has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77969?usp=email ) Change subject: treewide: Move stdlib.h to commonlib ...................................................................... Patch Set 9: (1 comment) File src/commonlib/include/commonlib/stdlib.h: https://review.coreboot.org/c/coreboot/+/77969/comment/4f19e234_df372524 : PS2, Line 44: int dma_coherent(const void *ptr); > Acknowledged Follow up question: If I remove the definition of `dma_malloc` from commonlib and replace the instances with a simple `malloc` and someone would select the sdhci stuff on an ARM platform and would actually use it (with actual data not just CMD1) he/she would run into coherency problems. How do I make sure that no one using an ARM platform (coreboot build not libpayload) selects and uses this (for the future) ? I could move the `dma_coherent` and `dma_malloc` function definition/implementation to something in arch/x86 and if someone would try to compile the code (coreboot build) on an ARM platform it would raise a compiler error. Let me know what you think. -- To view, visit https://review.coreboot.org/c/coreboot/+/77969?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: I3a7ab0d1ddcc7ce9af121a61b4d4eafc9e563a8a Gerrit-Change-Number: 77969 Gerrit-PatchSet: 9 Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Thu, 23 Nov 2023 00:08:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Comment-In-Reply-To: Maximilian Brune <maximilian.brune(a)9elements.com> Comment-In-Reply-To: Yidi Lin <yidilin(a)google.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: Don't build a separate romstage by default on most x86 targets
by Julius Werner (Code Review) 23 Nov '23

23 Nov '23

Attention is currently required from: Andrey Petrov, Arthur Heymans, Frans Hendriks, Paul Menzel, Yu-Ping Wu. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63419?usp=email ) Change subject: Don't build a separate romstage by default on most x86 targets ...................................................................... Patch Set 51: (2 comments) File src/Kconfig: https://review.coreboot.org/c/coreboot/+/63419/comment/194dd7c7_070f428c : PS11, Line 1389: default n if ARCH_X86 > > Should this really be a test for NO_XIP_EARLY_STAGES instead? […] No, I think my point was rather about what to base the heuristic upon for where it makes sense to enable this config by default. The main trade-off is between saving load time because things that are common between stages (e.g. printf code) don't need to be loaded twice, and saving load time on later stages due to compression. I was assuming you made your "default on x86 targets" decision here because XIP stages cannot be compressed anyway, so there the savings from merging stages should always win out? But NO_XIP_EARLY_STAGES boards (including AMD) _can_ compress their romstage, so that logic shouldn't count for them? File src/security/vboot/Kconfig: https://review.coreboot.org/c/coreboot/+/63419/comment/2e7f9f78_04c32920 : PS51, Line 93: select SEPARATE_ROMSTAGE > > Why should this change here? […] Okay, thinking about it some more I see that this is probably the better way. -- To view, visit https://review.coreboot.org/c/coreboot/+/63419?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: I4c53b6fa7a3e66415c5b1c539918a6c1cd8defa1 Gerrit-Change-Number: 63419 Gerrit-PatchSet: 51 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Erik van den Bogaert <ebogaert(a)eltan.com> Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Comment-Date: Wed, 22 Nov 2023 23:59:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

[L] Change in coreboot[main]: crossgcc: Upgrade GCC from 11.4.0 to 13.2.0
by Felix Singer (Code Review) 23 Nov '23

23 Nov '23

Attention is currently required from: Elyes Haouas, Nico Huber, Patrick Georgi. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/70771?usp=email ) Change subject: crossgcc: Upgrade GCC from 11.4.0 to 13.2.0 ...................................................................... Patch Set 30: (1 comment) File util/crossgcc/patches/gcc-11.4.0_ada-musl_workaround.patch: PS29: > Awesome! Thank you. […] I build-tested on Alpine Edge. -- To view, visit https://review.coreboot.org/c/coreboot/+/70771?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: I4f2ed4de4811abaa13528906de71eee29a8f2910 Gerrit-Change-Number: 70771 Gerrit-PatchSet: 30 Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-CC: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-CC: Patrick Georgi <patrick(a)coreboot.org> Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Comment-Date: Wed, 22 Nov 2023 23:47:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2023