coreboot-gerrit February 2024

coreboot-gerrit@coreboot.org

1 participants
3067 discussions

[M] Change in coreboot[main]: cpu/x86/smm: Set up page tables in safe SMRAM
by Arthur Heymans (Code Review) 02 Feb '24

02 Feb '24

Attention is currently required from: Jérémy Compostella. Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/80336?usp=email ) Change subject: cpu/x86/smm: Set up page tables in safe SMRAM ...................................................................... cpu/x86/smm: Set up page tables in safe SMRAM Relying on page tables being in RO flash is not safe in every setup, therefore set up some page tables in SMRAM that the permanent smihandler can use. Tested on QEMU. Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> Change-Id: Icb3086abd577b9abb9966dd910a264a873ace4ed --- M src/cpu/x86/mp_init.c M src/cpu/x86/smm/smm_module_loader.c M src/include/cpu/x86/smm.h 3 files changed, 54 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/80336/1 diff --git a/src/cpu/x86/mp_init.c b/src/cpu/x86/mp_init.c index 5a0d8a7..8f18de1 100644 --- a/src/cpu/x86/mp_init.c +++ b/src/cpu/x86/mp_init.c @@ -791,7 +791,6 @@ .num_cpus = num_cpus, .cpu_save_state_size = save_state_size, .num_concurrent_save_states = num_cpus, - .cr3 = read_cr3(), }; printk(BIOS_DEBUG, "Installing permanent SMM handler to 0x%08lx\n", smbase); diff --git a/src/cpu/x86/smm/smm_module_loader.c b/src/cpu/x86/smm/smm_module_loader.c index d965593..ca8c75d 100644 --- a/src/cpu/x86/smm/smm_module_loader.c +++ b/src/cpu/x86/smm/smm_module_loader.c @@ -8,6 +8,7 @@ #include <cpu/cpu.h> #include <cpu/x86/smm.h> #include <device/device.h> +#include <device/mmio.h> #include <rmodule.h> #include <stdio.h> #include <string.h> @@ -259,6 +260,7 @@ stub_params->stack_top = stack_top; stub_params->stack_size = g_stack_size; stub_params->c_handler = (uintptr_t)params->handler; + stub_params->cr3 = params->cr3; /* This runs on the BSP. All the APs are its siblings */ struct cpu_info *info = cpu_info(); @@ -353,8 +355,8 @@ region_end(&region)); } -/* STM + Handler + (Stub + Save state) * CONFIG_MAX_CPUS + stacks */ -#define SMM_REGIONS_ARRAY_SIZE (1 + 1 + CONFIG_MAX_CPUS * 2 + 1) +/* STM + Handler + (Stub + Save state) * CONFIG_MAX_CPUS + stacks + page tables*/ +#define SMM_REGIONS_ARRAY_SIZE (1 + 1 + CONFIG_MAX_CPUS * 2 + 1 + 1) static int append_and_check_region(const struct region smram, const struct region region, @@ -389,6 +391,39 @@ return 0; } +#define _PRES (1ULL << 0) +#define _RW (1ULL << 1) +#define _US (1ULL << 2) +#define _A (1ULL << 5) +#define _D (1ULL << 6) +#define _PS (1ULL << 7) +#define _GEN_DIR(a) (_PRES + _RW + _US + _A + (a)) +#define _GEN_PAGE(a) (_PRES + _RW + _US + _PS + _A + _D + (a)) +#define PAGE_SIZE 8 + +/* Return the PM4LE */ +static uintptr_t install_page_table(const uintptr_t handler_base) +{ + const bool one_g_pages = !!(cpuid_edx(0x80000001) & (1 << 26)); + /* 4 1G pages or 4 PDPE entries with 512 * 2M pages */ + const size_t pages_needed = one_g_pages ? 4 : 2048 + 4; + const uintptr_t pages_base = ALIGN_DOWN(handler_base - pages_needed * PAGE_SIZE, 4096); + const uintptr_t pm4le = ALIGN_DOWN(pages_base - 8, 4096); + + if (one_g_pages) { + for (int i = 0; i < 4; i++) + write64p(pages_base + i * PAGE_SIZE, _GEN_PAGE(1 * GiB * i)); + write64p(pm4le, _GEN_DIR(pages_base)); + } else { + for (int i = 0; i < 2048; i++) + write64p(pages_base + i * PAGE_SIZE, _GEN_PAGE(2 * MiB * i)); + write64p(pm4le, _GEN_DIR(pages_base + 2048 * PAGE_SIZE)); + for (int i = 0; i < 4; i++) + write64p(pages_base + (2048 + i) * PAGE_SIZE, _GEN_DIR(pages_base + 4096 * i)); + } + return pm4le; +} + /* *The SMM module is placed within the provided region in the following * manner: @@ -398,6 +433,8 @@ * +-----------------+ * | smi handler | * | ... | + * +-----------------+ + * | page tables | * +-----------------+ <- cpu0 * | stub code | <- cpu1 * | stub code | <- cpu2 @@ -453,7 +490,20 @@ if (append_and_check_region(smram, handler, region_list, "HANDLER")) return -1; - uintptr_t stub_segment_base = handler_base - SMM_CODE_SEGMENT_SIZE; + uintptr_t stub_segment_base; + if (ENV_X86_64) { + uintptr_t pt_base = install_page_table(handler_base); + struct region page_tables = { + .offset = pt_base, + .size = handler_base - pt_base, + }; + if (append_and_check_region(smram, page_tables, region_list, "PAGE TABLES")) + return -1; + params->cr3 = pt_base; + stub_segment_base = pt_base - SMM_CODE_SEGMENT_SIZE; + } else { + stub_segment_base = handler_base - SMM_CODE_SEGMENT_SIZE; + } if (!smm_create_map(stub_segment_base, params->num_concurrent_save_states, params)) { printk(BIOS_ERR, "%s: Error creating CPU map\n", __func__); diff --git a/src/include/cpu/x86/smm.h b/src/include/cpu/x86/smm.h index 6a0ec3f..a12065b 100644 --- a/src/include/cpu/x86/smm.h +++ b/src/include/cpu/x86/smm.h @@ -109,6 +109,7 @@ u32 stack_size; u32 stack_top; u32 c_handler; + u32 cr3; /* The apic_id_to_cpu provides a mapping from APIC id to CPU number. * The CPU number is indicated by the index into the array by matching * the default APIC id and value at the index. The stub loader -- To view, visit https://review.coreboot.org/c/coreboot/+/80336?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: Icb3086abd577b9abb9966dd910a264a873ace4ed Gerrit-Change-Number: 80336 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> 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

[S] Change in coreboot[main]: cpu/x86/(sipi|smm): Pass on CR3 from ramstage
by Arthur Heymans (Code Review) 02 Feb '24

02 Feb '24

Attention is currently required from: Jérémy Compostella. Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/80335?usp=email ) Change subject: cpu/x86/(sipi|smm): Pass on CR3 from ramstage ...................................................................... cpu/x86/(sipi|smm): Pass on CR3 from ramstage Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> Change-Id: I1250ea6f63c65228178ee66e06d988dadfcc2a37 --- M src/cpu/x86/mp_init.c M src/cpu/x86/sipi_vector.S M src/cpu/x86/smm/smm_stub.S M src/include/cpu/x86/smm.h 4 files changed, 12 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/80335/1 diff --git a/src/cpu/x86/mp_init.c b/src/cpu/x86/mp_init.c index d3af6ecc..5a0d8a7 100644 --- a/src/cpu/x86/mp_init.c +++ b/src/cpu/x86/mp_init.c @@ -98,6 +98,7 @@ uint32_t msr_table_ptr; uint32_t msr_count; uint32_t c_handler; + uint32_t cr3; atomic_t ap_count; } __packed; @@ -361,6 +362,8 @@ else sp->microcode_lock = 0; sp->c_handler = (uintptr_t)&ap_init; + if (ENV_X86_64) + sp->cr3 = read_cr3(); ap_count = &sp->ap_count; atomic_set(ap_count, 0); @@ -763,6 +766,7 @@ .cpu_save_state_size = save_state_size, .num_concurrent_save_states = 1, .handler = smm_do_relocation, + .cr3 = read_cr3(), }; if (smm_setup_relocation_handler(&smm_params)) { @@ -787,6 +791,7 @@ .num_cpus = num_cpus, .cpu_save_state_size = save_state_size, .num_concurrent_save_states = num_cpus, + .cr3 = read_cr3(), }; printk(BIOS_DEBUG, "Installing permanent SMM handler to 0x%08lx\n", smbase); diff --git a/src/cpu/x86/sipi_vector.S b/src/cpu/x86/sipi_vector.S index 923e398..b7d700f 100644 --- a/src/cpu/x86/sipi_vector.S +++ b/src/cpu/x86/sipi_vector.S @@ -38,6 +38,8 @@ .long 0 c_handler: .long 0 +cr3: +.long 0 ap_count: .long 0 @@ -224,7 +226,7 @@ #if ENV_X86_64 /* entry64.inc preserves ebx, esi, edi, ebp */ - setup_longmode $(CONFIG_ARCH_X86_64_PGTBL_LOC) + setup_longmode cr3 movabs c_handler, %eax call *%rax diff --git a/src/cpu/x86/smm/smm_stub.S b/src/cpu/x86/smm/smm_stub.S index 9f1f21d..9b4b966 100644 --- a/src/cpu/x86/smm/smm_stub.S +++ b/src/cpu/x86/smm/smm_stub.S @@ -23,6 +23,8 @@ .long 0 c_handler: .long 0 +cr3: +.long 0 /* apic_to_cpu_num is a table mapping the default APIC id to CPU num. If the * APIC id is found at the given index, the contiguous CPU number is index * into the table. */ @@ -196,7 +198,7 @@ #if ENV_X86_64 mov %ecx, %edi /* entry64.inc preserves ebx, esi, edi, ebp */ - setup_longmode $(CONFIG_ARCH_X86_64_PGTBL_LOC) + setup_longmode cr3 mov %edi, %ecx diff --git a/src/include/cpu/x86/smm.h b/src/include/cpu/x86/smm.h index beb88fa..6a0ec3f 100644 --- a/src/include/cpu/x86/smm.h +++ b/src/include/cpu/x86/smm.h @@ -165,6 +165,7 @@ size_t num_concurrent_save_states; smm_handler_t handler; + uint32_t cr3; }; /* All of these return 0 on success, < 0 on failure. */ -- To view, visit https://review.coreboot.org/c/coreboot/+/80335?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: I1250ea6f63c65228178ee66e06d988dadfcc2a37 Gerrit-Change-Number: 80335 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> 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

[S] Change in coreboot[main]: cpu/x86/64bit: Turn jumping to long mode into a macro
by Arthur Heymans (Code Review) 02 Feb '24

02 Feb '24

Attention is currently required from: Jérémy Compostella. Hello Jérémy Compostella, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/79261?usp=email to look at the new patch set (#2). Change subject: cpu/x86/64bit: Turn jumping to long mode into a macro ...................................................................... cpu/x86/64bit: Turn jumping to long mode into a macro This makes it easier to reuse, e.g. if you want to do it twice in one assembly file. Change-Id: Ida861338004187e4e714be41e17c8447fa4cf935 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- 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/mode_switch2.S M src/cpu/x86/sipi_vector.S M src/cpu/x86/smm/smm_stub.S M src/soc/intel/common/block/cpu/car/cache_as_ram.S 9 files changed, 27 insertions(+), 18 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/79261/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/79261?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: Ida861338004187e4e714be41e17c8447fa4cf935 Gerrit-Change-Number: 79261 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in coreboot[main]: brox: Handle GPI_INT pin value lower to GPI_WAKE
by Karthik Ramasubramanian (Code Review) 02 Feb '24

02 Feb '24

Attention is currently required from: Ashish Kumar Mishra, Bora Guvendik, Shelley Chen. Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80334?usp=email ) Change subject: brox: Handle GPI_INT pin value lower to GPI_WAKE ...................................................................... Patch Set 1: (2 comments) File src/soc/intel/common/block/gpio/gpio.c: https://review.coreboot.org/c/coreboot/+/80334/comment/c4c0ec0b_2bb1e0fc : PS1, Line 292: !(cfg->pad_config[0] & PAD_CFG0_ROUTE_SWAPPED) Why does polarity config applicable only for non-swapped IRQ pins? https://review.coreboot.org/c/coreboot/+/80334/comment/087ba2c6_093047cb : PS1, Line 354: Nit: Extraneous line -- To view, visit https://review.coreboot.org/c/coreboot/+/80334?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: I8c3d557e888b8d0ceac203f49b702910fba26d6d Gerrit-Change-Number: 80334 Gerrit-PatchSet: 1 Gerrit-Owner: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Deepti Deshatty <deepti.deshatty(a)intel.com> Gerrit-CC: Haribalaraman Ramasubramanian <haribalaraman.r(a)intel.com> Gerrit-CC: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Attention: Shelley Chen <shchen(a)google.com> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-Comment-Date: Fri, 02 Feb 2024 17:29:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: mb/google/nissa/var/yaviks: Enable USE_MTCL for YAVIKS
by Subrata Banik (Code Review) 02 Feb '24

02 Feb '24

Attention is currently required from: David Ruth, Eric Lai, Kapil Porwal, Nick Vaccaro. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80305?usp=email ) Change subject: mb/google/nissa/var/yaviks: Enable USE_MTCL for YAVIKS ...................................................................... Patch Set 3: (1 comment) File src/mainboard/google/brya/Kconfig: https://review.coreboot.org/c/coreboot/+/80305/comment/fcecd931_a04619ad : PS1, Line 489: select USE_MTCL > Done thanks David -- To view, visit https://review.coreboot.org/c/coreboot/+/80305?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: Iec54fc582d68b443665fceda47187c28f1a9216c Gerrit-Change-Number: 80305 Gerrit-PatchSet: 3 Gerrit-Owner: David Ruth <druth(a)chromium.org> 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: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: David Ruth <druth(a)chromium.org> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Comment-Date: Fri, 02 Feb 2024 17:24:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Eric Lai <ericllai(a)google.com> Comment-In-Reply-To: David Ruth <druth(a)chromium.org> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: mb/google/nissa/var/yaviks: Enable USE_MTCL for YAVIKS
by Subrata Banik (Code Review) 02 Feb '24

02 Feb '24

1 0

[XS] Change in coreboot[main]: mb/google/nissa/var/yaviks: Enable USE_MTCL for YAVIKS
by David Ruth (Code Review) 02 Feb '24

02 Feb '24

Attention is currently required from: Eric Lai, Kapil Porwal, Nick Vaccaro, Subrata Banik. David Ruth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80305?usp=email ) Change subject: mb/google/nissa/var/yaviks: Enable USE_MTCL for YAVIKS ...................................................................... Patch Set 3: (1 comment) File src/mainboard/google/brya/Kconfig: https://review.coreboot.org/c/coreboot/+/80305/comment/3a630fe6_626ee604 : PS1, Line 489: select USE_MTCL > > Nothing declares DRIVERS_INTEL_WIFI, but almost all of these boards have variants with Intel WiFi […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/80305?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: Iec54fc582d68b443665fceda47187c28f1a9216c Gerrit-Change-Number: 80305 Gerrit-PatchSet: 3 Gerrit-Owner: David Ruth <druth(a)chromium.org> 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: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Comment-Date: Fri, 02 Feb 2024 17:01:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Eric Lai <ericllai(a)google.com> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: David Ruth <druth(a)chromium.org> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: mb/google/nissa/var/yaviks: Enable USE_MTCL for YAVIKS
by David Ruth (Code Review) 02 Feb '24

02 Feb '24

Attention is currently required from: David Ruth, Eric Lai, Kapil Porwal, Nick Vaccaro. Hello Eric Lai, Kapil Porwal, Nick Vaccaro, Subrata Banik, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/80305?usp=email to look at the new patch set (#3). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: mb/google/nissa/var/yaviks: Enable USE_MTCL for YAVIKS ...................................................................... mb/google/nissa/var/yaviks: Enable USE_MTCL for YAVIKS Yaviks will be the first model to take advantage of this feature. BUG=b:295544553 TEST=Build on similar model (PUJJO) that I have access to and verify the flag and feature work as intended. TEST=Add wifi_mtcls.bin blob to cbfs TEST=Build coreboot for pujjo `emerge-nissa coreboot chromeos-bootimage` TEST=Verify that MTCL defined in the file is present: TEST=`acpidump -b` TEST=`iasl ssdt.dat` TEST=`less ssdt.dsl` TEST=Search for MTCL Change-Id: Iec54fc582d68b443665fceda47187c28f1a9216c Signed-off-by: David Ruth <druth(a)chromium.org> --- M src/mainboard/google/brya/Kconfig 1 file changed, 2 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/80305/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/80305?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: Iec54fc582d68b443665fceda47187c28f1a9216c Gerrit-Change-Number: 80305 Gerrit-PatchSet: 3 Gerrit-Owner: David Ruth <druth(a)chromium.org> 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: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: David Ruth <druth(a)chromium.org> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[XS] Change in coreboot[main]: mb/google/nissa/var/yaviks: Enable USE_MTCL for YAVIKS
by Subrata Banik (Code Review) 02 Feb '24

02 Feb '24

Attention is currently required from: David Ruth, Eric Lai, Kapil Porwal, Nick Vaccaro. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80305?usp=email ) Change subject: mb/google/nissa/var/yaviks: Enable USE_MTCL for YAVIKS ...................................................................... Patch Set 2: (1 comment) File src/mainboard/google/brya/Kconfig: https://review.coreboot.org/c/coreboot/+/80305/comment/ec54e994_1067c9b3 : PS1, Line 489: select USE_MTCL > Nothing declares DRIVERS_INTEL_WIFI, but almost all of these boards have variants with Intel WiFi chips. I tried to figure out why that was, and I found the following: > > PCIEXP_PLUGIN_SUPPORT is enabled by default (see /src/third_party/coreboot/src/device/Kconfig), so DRIVERS_MTK_WIFI and DRIVERS_INTEL_WIFI are both effectively enabled by default. > > This isn't technically necessary (I've tested it multiple times at this point), and it seems to go against convention. My inclination (unless there's a strong preference otherwise) is to not add this. if it's default enabled then we are good bt ideally it's better u select the required Kconfig as per BoM choice directly. -- To view, visit https://review.coreboot.org/c/coreboot/+/80305?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: Iec54fc582d68b443665fceda47187c28f1a9216c Gerrit-Change-Number: 80305 Gerrit-PatchSet: 2 Gerrit-Owner: David Ruth <druth(a)chromium.org> 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: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: David Ruth <druth(a)chromium.org> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Comment-Date: Fri, 02 Feb 2024 16:54:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Eric Lai <ericllai(a)google.com> Comment-In-Reply-To: David Ruth <druth(a)chromium.org> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: drivers/wifi: Add MTCL function to ACPI SSDT
by Subrata Banik (Code Review) 02 Feb '24

02 Feb '24

Attention is currently required from: Arthur Heymans, Cliff Huang, David Ruth, Eric Lai, Kapil Porwal, Lance Zhao, Nick Vaccaro, Nico Huber, Paul Menzel, Subrata Banik, Tim Wawrzynczak. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80170?usp=email ) Change subject: drivers/wifi: Add MTCL function to ACPI SSDT ...................................................................... Patch Set 19: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/80170?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: I9b5e7312a44e114270e664b983626faa6cfee350 Gerrit-Change-Number: 80170 Gerrit-PatchSet: 19 Gerrit-Owner: David Ruth <druth(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Subrata Banik <subi.banik(a)gmail.com> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Subrata Banik <subi.banik(a)gmail.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: David Ruth <druth(a)chromium.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Fri, 02 Feb 2024 16:52:50 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit February 2024