coreboot-gerrit February 2022

coreboot-gerrit@coreboot.org

1 participants
7066 discussions

Change in coreboot[master]: soc/intel/skylake: Add function to clear PMCON status bits
by Sridhar Siricilla (Code Review) 07 Feb '22

07 Feb '22

Attention is currently required from: Subrata Banik, Matt DeVillier, Tim Wawrzynczak, Angel Pons, Nick Vaccaro, Patrick Rudolph, EricR Lai. Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61650 ) Change subject: soc/intel/skylake: Add function to clear PMCON status bits ...................................................................... Patch Set 1: (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/61650/comment/7aaefdce_34f0c9d3 PS1, Line 9: This patch adds function to `clear PMCON status bits` into SoC directory nit: This patch adds SoC function to clear GEN_PMCON_A status bits File src/soc/intel/skylake/pmutil.c: https://review.coreboot.org/c/coreboot/+/61650/comment/46fcbccd_2ea95cbe PS1, Line 276: /* Clear SUS_PWR_FLR, GBL_RST_STS, HOST_RST_STS, PWR_FLR bits : * while retaining MS4V write-1-to-clear bit */ nit: Please format the comment. -- To view, visit https://review.coreboot.org/c/coreboot/+/61650 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie786e6ba2daf88accb5d70be33de0abe593f8c53 Gerrit-Change-Number: 61650 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Comment-Date: Mon, 07 Feb 2022 13:56:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: configs/i440fx: Build-test PARALLEL_MP
by Kyösti Mälkki (Code Review) 07 Feb '22

07 Feb '22

Kyösti Mälkki has submitted this change. ( https://review.coreboot.org/c/coreboot/+/59695 ) Change subject: configs/i440fx: Build-test PARALLEL_MP ...................................................................... configs/i440fx: Build-test PARALLEL_MP Change-Id: If30d715c5a3b44be2832c96316003dc9d139b53f Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> Reviewed-on: https://review.coreboot.org/c/coreboot/+/59695 Reviewed-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M configs/config.emulation_qemu_x86_i440fx_debug 1 file changed, 1 insertion(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Kyösti Mälkki: Looks good to me, approved Angel Pons: Looks good to me, approved diff --git a/configs/config.emulation_qemu_x86_i440fx_debug b/configs/config.emulation_qemu_x86_i440fx_debug index 85f5499..3cff033 100644 --- a/configs/config.emulation_qemu_x86_i440fx_debug +++ b/configs/config.emulation_qemu_x86_i440fx_debug @@ -8,3 +8,4 @@ CONFIG_DEBUG_MALLOC=y CONFIG_DEBUG_BOOT_STATE=y CONFIG_DEBUG_ADA_CODE=y +CONFIG_CPU_QEMU_X86_PARALLEL_MP=y -- To view, visit https://review.coreboot.org/c/coreboot/+/59695 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If30d715c5a3b44be2832c96316003dc9d139b53f Gerrit-Change-Number: 59695 Gerrit-PatchSet: 7 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: cpu/x86/Makefile.inc: Build smi_trigger on !HAVE_SMI_HANDLER
by Kyösti Mälkki (Code Review) 07 Feb '22

07 Feb '22

Kyösti Mälkki has submitted this change. ( https://review.coreboot.org/c/coreboot/+/59913 ) Change subject: cpu/x86/Makefile.inc: Build smi_trigger on !HAVE_SMI_HANDLER ...................................................................... cpu/x86/Makefile.inc: Build smi_trigger on !HAVE_SMI_HANDLER A lot of soc code requires a definition of apm_control, which smm/smi_trigger.c provided for !HAVE_SMI_HANDLER, but is not added as a build target. Fixes building Q35 without smihandler. Change-Id: Ie57819b3d169311371a1caca83c9b0c796b46048 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> Reviewed-on: https://review.coreboot.org/c/coreboot/+/59913 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Reviewed-by: HAOUAS Elyes <ehaouas(a)noos.fr> --- M src/cpu/x86/Makefile.inc R src/cpu/x86/smi_trigger.c M src/cpu/x86/smm/Makefile.inc 3 files changed, 3 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Kyösti Mälkki: Looks good to me, but someone else must approve HAOUAS Elyes: Looks good to me, but someone else must approve Angel Pons: Looks good to me, approved diff --git a/src/cpu/x86/Makefile.inc b/src/cpu/x86/Makefile.inc index b9f6417..b970528 100644 --- a/src/cpu/x86/Makefile.inc +++ b/src/cpu/x86/Makefile.inc @@ -8,7 +8,10 @@ subdirs-$(CONFIG_PARALLEL_MP) += name ramstage-$(CONFIG_PARALLEL_MP) += mp_init.c + ramstage-y += backup_default_smm.c +ramstage-y += smi_trigger.c +smm-y += smi_trigger.c subdirs-$(CONFIG_CPU_INTEL_COMMON_SMM) += ../intel/smm diff --git a/src/cpu/x86/smm/smi_trigger.c b/src/cpu/x86/smi_trigger.c similarity index 100% rename from src/cpu/x86/smm/smi_trigger.c rename to src/cpu/x86/smi_trigger.c diff --git a/src/cpu/x86/smm/Makefile.inc b/src/cpu/x86/smm/Makefile.inc index 4d1cdf8..36aa113 100644 --- a/src/cpu/x86/smm/Makefile.inc +++ b/src/cpu/x86/smm/Makefile.inc @@ -1,7 +1,6 @@ ## SPDX-License-Identifier: GPL-2.0-only ramstage-y += smm_module_loader.c -ramstage-y += smi_trigger.c ifeq ($(CONFIG_ARCH_RAMSTAGE_X86_32),y) $(eval $(call create_class_compiler,smm,x86_32)) @@ -29,7 +28,6 @@ endif smm-y += save_state.c -smm-y += smi_trigger.c ifeq ($(CONFIG_SMM_TSEG),y) -- To view, visit https://review.coreboot.org/c/coreboot/+/59913 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie57819b3d169311371a1caca83c9b0c796b46048 Gerrit-Change-Number: 59913 Gerrit-PatchSet: 6 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: cpu/x86/mp_init.c: Rename num_concurrent_stacks
by Kyösti Mälkki (Code Review) 07 Feb '22

07 Feb '22

Kyösti Mälkki has submitted this change. ( https://review.coreboot.org/c/coreboot/+/58699 ) Change subject: cpu/x86/mp_init.c: Rename num_concurrent_stacks ...................................................................... cpu/x86/mp_init.c: Rename num_concurrent_stacks This is just the amount of cpus so rename it for simplicity. Change-Id: Ib2156136894eeda4a29e8e694480abe06da62959 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> Reviewed-on: https://review.coreboot.org/c/coreboot/+/58699 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: HAOUAS Elyes <ehaouas(a)noos.fr> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- 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, 11 insertions(+), 11 deletions(-) Approvals: build bot (Jenkins): Verified HAOUAS Elyes: Looks good to me, but someone else must approve Angel Pons: Looks good to me, approved diff --git a/src/cpu/x86/mp_init.c b/src/cpu/x86/mp_init.c index 7d0a20f..1f1f968 100644 --- a/src/cpu/x86/mp_init.c +++ b/src/cpu/x86/mp_init.c @@ -763,7 +763,7 @@ size_t save_state_size) { struct smm_loader_params smm_params = { - .num_concurrent_stacks = num_cpus, + .num_cpus = num_cpus, .real_cpu_save_state_size = real_save_state_size, .per_cpu_save_state_size = save_state_size, .num_concurrent_save_states = 1, @@ -793,7 +793,7 @@ * size and save state size for each CPU. */ struct smm_loader_params smm_params = { - .num_concurrent_stacks = num_cpus, + .num_cpus = num_cpus, .real_cpu_save_state_size = real_save_state_size, .per_cpu_save_state_size = save_state_size, .num_concurrent_save_states = num_cpus, diff --git a/src/cpu/x86/smm/smm_module_loader.c b/src/cpu/x86/smm/smm_module_loader.c index ce7cf20..beddb3c 100644 --- a/src/cpu/x86/smm/smm_module_loader.c +++ b/src/cpu/x86/smm/smm_module_loader.c @@ -322,7 +322,7 @@ size = smm_size; /* The number of concurrent stacks cannot exceed CONFIG_MAX_CPUS. */ - if (params->num_concurrent_stacks > CONFIG_MAX_CPUS) { + if (params->num_cpus > CONFIG_MAX_CPUS) { printk(BIOS_ERR, "%s: not enough stacks\n", __func__); return -1; } @@ -397,7 +397,7 @@ __func__, smm_size); /* Initialize the APIC id to CPU number table to be 1:1 */ - for (i = 0; i < params->num_concurrent_stacks; i++) + for (i = 0; i < params->num_cpus; i++) stub_params->apic_id_to_cpu[i] = i; /* Allow the initiator to manipulate SMM stub parameters. */ @@ -429,8 +429,8 @@ /* Since the relocation handler always uses stack, adjust the number * of concurrent stack users to be CONFIG_MAX_CPUS. */ - if (params->num_concurrent_stacks == 0) - params->num_concurrent_stacks = CONFIG_MAX_CPUS; + if (params->num_cpus == 0) + params->num_cpus = CONFIG_MAX_CPUS; printk(BIOS_SPEW, "%s: exit\n", __func__); return smm_module_setup_stub(smram, SMM_DEFAULT_SIZE, @@ -506,7 +506,7 @@ /* FXSAVE goes below MSEG */ if (CONFIG(SSE)) { - fxsave_size = FXSAVE_SIZE * params->num_concurrent_stacks; + fxsave_size = FXSAVE_SIZE * params->num_cpus; fxsave_area = (char *)base - fxsave_size; base -= fxsave_size; total_size += fxsave_size; @@ -548,7 +548,7 @@ handler_mod_params->smbase = smram_base; handler_mod_params->smm_size = smram_size; handler_mod_params->save_state_size = params->real_cpu_save_state_size; - handler_mod_params->num_cpus = params->num_concurrent_stacks; + handler_mod_params->num_cpus = params->num_cpus; handler_mod_params->gnvs_ptr = (uintptr_t)acpi_get_gnvs(); printk(BIOS_DEBUG, "%s: smram_start: 0x%lx\n", __func__, smram_base); @@ -585,7 +585,7 @@ return -1; } - for (int i = 0; i < params->num_concurrent_stacks; i++) { + for (int i = 0; i < params->num_cpus; i++) { handler_mod_params->save_state_top[i] = cpus[i].ss_start + params->per_cpu_save_state_size; } diff --git a/src/include/cpu/x86/smm.h b/src/include/cpu/x86/smm.h index 37247ec..8ea8336 100644 --- a/src/include/cpu/x86/smm.h +++ b/src/include/cpu/x86/smm.h @@ -120,7 +120,7 @@ /* SMM Module Loading API */ /* The smm_loader_params structure provides direction to the SMM loader: - * - num_concurrent_stacks - number of concurrent cpus in handler needing stack + * - num_cpus - number of concurrent cpus in handler needing stack * optional for setting up relocation handler. * - per_cpu_save_state_size - the SMM save state size per cpu * - num_concurrent_save_states - number of concurrent cpus needing save state @@ -132,7 +132,7 @@ * handle sparse APIC id space. */ struct smm_loader_params { - size_t num_concurrent_stacks; + size_t num_cpus; size_t real_cpu_save_state_size; size_t per_cpu_save_state_size; -- To view, visit https://review.coreboot.org/c/coreboot/+/58699 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib2156136894eeda4a29e8e694480abe06da62959 Gerrit-Change-Number: 58699 Gerrit-PatchSet: 6 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: cpu/x86/smm: Improve smm stack setup
by Kyösti Mälkki (Code Review) 07 Feb '22

07 Feb '22

Kyösti Mälkki has submitted this change. ( https://review.coreboot.org/c/coreboot/+/58698 ) Change subject: cpu/x86/smm: Improve smm stack setup ...................................................................... cpu/x86/smm: Improve smm stack setup Both the relocation handler and the permanent handler use the same stacks, so things can be simplified. Change-Id: I7bdca775550e8280757a6c5a5150a0d638d5fc2d Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> Reviewed-on: https://review.coreboot.org/c/coreboot/+/58698 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: HAOUAS Elyes <ehaouas(a)noos.fr> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M src/cpu/x86/mp_init.c M src/cpu/x86/smm/smm_module_loader.c M src/include/cpu/x86/mp.h M src/include/cpu/x86/smm.h 4 files changed, 47 insertions(+), 97 deletions(-) Approvals: build bot (Jenkins): Verified HAOUAS Elyes: Looks good to me, but someone else must approve Angel Pons: Looks good to me, approved diff --git a/src/cpu/x86/mp_init.c b/src/cpu/x86/mp_init.c index fee5940..7d0a20f 100644 --- a/src/cpu/x86/mp_init.c +++ b/src/cpu/x86/mp_init.c @@ -760,10 +760,9 @@ } static enum cb_err install_relocation_handler(int num_cpus, size_t real_save_state_size, - size_t save_state_size, uintptr_t perm_smbase) + size_t save_state_size) { struct smm_loader_params smm_params = { - .per_cpu_stack_size = CONFIG_SMM_STUB_STACK_SIZE, .num_concurrent_stacks = num_cpus, .real_cpu_save_state_size = real_save_state_size, .per_cpu_save_state_size = save_state_size, @@ -771,11 +770,7 @@ .handler = smm_do_relocation, }; - /* Allow callback to override parameters. */ - if (mp_state.ops.adjust_smm_params != NULL) - mp_state.ops.adjust_smm_params(&smm_params, 0); - - if (smm_setup_relocation_handler(perm_smbase, &smm_params)) { + if (smm_setup_relocation_handler(&smm_params)) { printk(BIOS_ERR, "%s: smm setup failed\n", __func__); return CB_ERR; } @@ -798,17 +793,12 @@ * size and save state size for each CPU. */ struct smm_loader_params smm_params = { - .per_cpu_stack_size = CONFIG_SMM_MODULE_STACK_SIZE, .num_concurrent_stacks = num_cpus, .real_cpu_save_state_size = real_save_state_size, .per_cpu_save_state_size = save_state_size, .num_concurrent_save_states = num_cpus, }; - /* Allow callback to override parameters. */ - if (mp_state.ops.adjust_smm_params != NULL) - mp_state.ops.adjust_smm_params(&smm_params, 1); - printk(BIOS_DEBUG, "Installing permanent SMM handler to 0x%08lx\n", smbase); if (smm_load_module(smbase, smsize, &smm_params)) @@ -829,10 +819,15 @@ if (!is_smm_enabled()) return; + if (smm_setup_stack(mp_state.perm_smbase, mp_state.perm_smsize, mp_state.cpu_count, + CONFIG_SMM_MODULE_STACK_SIZE)) { + printk(BIOS_ERR, "Unable to install SMM relocation handler.\n"); + smm_disable(); + } + /* Install handlers. */ if (install_relocation_handler(mp_state.cpu_count, real_save_state_size, - smm_save_state_size, mp_state.perm_smbase) != - CB_SUCCESS) { + smm_save_state_size) != CB_SUCCESS) { printk(BIOS_ERR, "Unable to install SMM relocation handler.\n"); smm_disable(); } diff --git a/src/cpu/x86/smm/smm_module_loader.c b/src/cpu/x86/smm/smm_module_loader.c index ba179ca..ce7cf20 100644 --- a/src/cpu/x86/smm/smm_module_loader.c +++ b/src/cpu/x86/smm/smm_module_loader.c @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0-only */ #include <acpi/acpi_gnvs.h> +#include <stddef.h> #include <stdint.h> #include <string.h> #include <rmodule.h> @@ -239,32 +240,26 @@ return 1; } -/* - * Place stacks in base -> base + size region, but ensure the stacks don't - * overlap the staggered entry points. - */ -static uintptr_t smm_stub_place_stacks(const uintptr_t base, struct smm_loader_params *params) +static uintptr_t stack_top; +static size_t g_stack_size; + +int smm_setup_stack(const uintptr_t perm_smbase, const size_t perm_smram_size, + const unsigned int total_cpus, const size_t stack_size) { - size_t total_stack_size; - uintptr_t stacks_top; + /* Need a minimum stack size and alignment. */ + if (stack_size <= SMM_MINIMUM_STACK_SIZE || (stack_size & 3) != 0) { + printk(BIOS_ERR, "%s: need minimum stack size\n", __func__); + return -1; + } - /* If stack space is requested assume the space lives in the lower - * half of SMRAM. */ - total_stack_size = params->per_cpu_stack_size * - params->num_concurrent_stacks; - printk(BIOS_DEBUG, "%s: cpus: %zx : stack space: needed -> %zx\n", - __func__, params->num_concurrent_stacks, - total_stack_size); - - /* There has to be at least one stack user. */ - if (params->num_concurrent_stacks < 1) - return 0; - - /* Stacks extend down to SMBASE */ - stacks_top = base + total_stack_size; - printk(BIOS_DEBUG, "%s: exit, stack_top %lx\n", __func__, stacks_top); - - return stacks_top; + const size_t total_stack_size = total_cpus * stack_size; + if (total_stack_size >= perm_smram_size) { + printk(BIOS_ERR, "%s: Stack won't fit smram\n", __func__); + return -1; + } + stack_top = perm_smbase + total_stack_size; + g_stack_size = stack_size; + return 0; } /* @@ -284,7 +279,7 @@ if (params->num_concurrent_save_states > 1 || stub_entry_offset != 0) { rc = smm_place_entry_code((uintptr_t)base, params->num_concurrent_save_states, - (uintptr_t)params->stack_top, params); + stack_top, params); } return rc; } @@ -309,18 +304,15 @@ * permanent SMM handler. * The CPU stack is decided at runtime in the stub and is treaded as a continuous * region. As this might not fit the default SMRAM region, the same region used - * by the permanent handler can be used during relocation. This is done via the - * smram_start argument. + * by the permanent handler can be used during relocation. */ static int smm_module_setup_stub(const uintptr_t smbase, const size_t smm_size, struct smm_loader_params *params, - void *const fxsave_area, - const uintptr_t smram_start) + void *const fxsave_area) { size_t total_save_state_size; size_t smm_stub_size; uintptr_t smm_stub_loc; - uintptr_t stacks_top; size_t size; uintptr_t base; size_t i; @@ -361,13 +353,6 @@ return -1; } - /* Need a minimum stack size and alignment. */ - if (params->per_cpu_stack_size <= SMM_MINIMUM_STACK_SIZE || - (params->per_cpu_stack_size & 3) != 0) { - printk(BIOS_ERR, "%s: need minimum stack size\n", __func__); - return -1; - } - smm_stub_size = rmodule_memory_size(&smm_stub); /* Put the stub at the main entry point */ @@ -379,16 +364,10 @@ return -1; } - /* The stacks, if requested, live in the lower half of SMRAM space - * for default handler, but for relocated handler it lives at the beginning - * of SMRAM which is TSEG base - */ - stacks_top = smm_stub_place_stacks(smram_start, params); - if (stacks_top == 0) { + if (stack_top == 0) { printk(BIOS_ERR, "%s: error assigning stacks\n", __func__); return -1; } - params->stack_top = stacks_top; /* Load the stub. */ if (rmodule_load((void *)smm_stub_loc, &smm_stub)) { printk(BIOS_ERR, "%s: load module failed\n", __func__); @@ -402,19 +381,15 @@ /* Setup the parameters for the stub code. */ stub_params = rmodule_parameters(&smm_stub); - stub_params->stack_top = (uintptr_t)stacks_top; - stub_params->stack_size = params->per_cpu_stack_size; + stub_params->stack_top = stack_top; + stub_params->stack_size = g_stack_size; stub_params->c_handler = (uintptr_t)params->handler; stub_params->fxsave_area = (uintptr_t)fxsave_area; stub_params->fxsave_area_size = FXSAVE_SIZE; - const size_t total_stack_size = - params->num_concurrent_stacks * params->per_cpu_stack_size; - printk(BIOS_DEBUG, "%s: stack_end = 0x%zx\n", - __func__, stub_params->stack_top - total_stack_size); printk(BIOS_DEBUG, "%s: stack_top = 0x%x\n", __func__, stub_params->stack_top); - printk(BIOS_DEBUG, "%s: stack_size = 0x%x\n", + printk(BIOS_DEBUG, "%s: per cpu stack_size = 0x%x\n", __func__, stub_params->stack_size); printk(BIOS_DEBUG, "%s: runtime.start32_offset = 0x%x\n", __func__, stub_params->start32_offset); @@ -439,7 +414,7 @@ * assumption is that the stub will be entered from the default SMRAM * location: 0x30000 -> 0x40000. */ -int smm_setup_relocation_handler(const uintptr_t perm_smram, struct smm_loader_params *params) +int smm_setup_relocation_handler(struct smm_loader_params *params) { uintptr_t smram = SMM_DEFAULT_BASE; printk(BIOS_SPEW, "%s: enter\n", __func__); @@ -459,7 +434,7 @@ printk(BIOS_SPEW, "%s: exit\n", __func__); return smm_module_setup_stub(smram, SMM_DEFAULT_SIZE, - params, fxsave_area_relocation, perm_smram); + params, fxsave_area_relocation); } /* @@ -519,11 +494,9 @@ if (CONFIG(DEBUG_SMI)) memset((void *)smram_base, 0xcd, smram_size); - total_stack_size = params->per_cpu_stack_size * - params->num_concurrent_stacks; + total_stack_size = stack_top - smram_base; total_size += total_stack_size; /* Stacks are the base of SMRAM */ - params->stack_top = smram_base + total_stack_size; /* MSEG starts at the top of SMRAM and works down */ if (CONFIG(STM)) { @@ -580,8 +553,6 @@ printk(BIOS_DEBUG, "%s: smram_start: 0x%lx\n", __func__, smram_base); printk(BIOS_DEBUG, "%s: smram_end: %lx\n", __func__, smram_base + smram_size); - printk(BIOS_DEBUG, "%s: stack_top: %lx\n", - __func__, params->stack_top); printk(BIOS_DEBUG, "%s: handler start %p\n", __func__, params->handler); printk(BIOS_DEBUG, "%s: handler_size %zx\n", @@ -619,5 +590,5 @@ cpus[i].ss_start + params->per_cpu_save_state_size; } - return smm_module_setup_stub(base, smram_size, params, fxsave_area, smram_base); + return smm_module_setup_stub(base, smram_size, params, fxsave_area); } diff --git a/src/include/cpu/x86/mp.h b/src/include/cpu/x86/mp.h index 934d217..1b4c956 100644 --- a/src/include/cpu/x86/mp.h +++ b/src/include/cpu/x86/mp.h @@ -45,17 +45,6 @@ */ void (*get_microcode_info)(const void **microcode, int *parallel); /* - * Optionally adjust SMM handler parameters to override the default - * values. The is_perm variable indicates if the parameters to adjust - * are for the relocation handler or the permanent handler. This - * function is therefore called twice -- once for each handler. - * By default the parameters for each SMM handler are: - * stack_size num_concurrent_stacks num_concurrent_save_states - * relo: save_state_size get_cpu_count() 1 - * perm: save_state_size get_cpu_count() get_cpu_count() - */ - void (*adjust_smm_params)(struct smm_loader_params *slp, int is_perm); - /* * Optionally provide a callback prior to the APs starting SMM * relocation or CPU driver initialization. However, note that * this callback is called after SMM handlers have been loaded. @@ -92,13 +81,11 @@ * 3. get_smm_info() * 4. get_microcode_info() * 5. adjust_cpu_apic_entry() for each number of get_cpu_count() - * 6. adjust_smm_params(is_perm = 0) - * 7. adjust_smm_params(is_perm = 1) - * 8. pre_mp_smm_init() - * 9. per_cpu_smm_trigger() in parallel for all cpus which calls + * 6. pre_mp_smm_init() + * 7. per_cpu_smm_trigger() in parallel for all cpus which calls * relocation_handler() in SMM. - * 10. mp_initialize_cpu() for each cpu - * 11. post_mp_init() + * 8. mp_initialize_cpu() for each cpu + * 9. post_mp_init() */ enum cb_err mp_init_with_smm(struct bus *cpu_bus, const struct mp_ops *mp_ops); diff --git a/src/include/cpu/x86/smm.h b/src/include/cpu/x86/smm.h index 848996f..37247ec 100644 --- a/src/include/cpu/x86/smm.h +++ b/src/include/cpu/x86/smm.h @@ -120,9 +120,6 @@ /* SMM Module Loading API */ /* The smm_loader_params structure provides direction to the SMM loader: - * - stack_top - optional external stack provided to loader. It must be at - * least per_cpu_stack_size * num_concurrent_stacks in size. - * - per_cpu_stack_size - stack size per CPU for smm modules. * - num_concurrent_stacks - number of concurrent cpus in handler needing stack * optional for setting up relocation handler. * - per_cpu_save_state_size - the SMM save state size per cpu @@ -135,8 +132,6 @@ * handle sparse APIC id space. */ struct smm_loader_params { - uintptr_t stack_top; - size_t per_cpu_stack_size; size_t num_concurrent_stacks; size_t real_cpu_save_state_size; @@ -148,8 +143,10 @@ struct smm_stub_params *stub_params; }; -/* Both of these return 0 on success, < 0 on failure. */ -int smm_setup_relocation_handler(const uintptr_t perm_smram, struct smm_loader_params *params); +/* All of these return 0 on success, < 0 on failure. */ +int smm_setup_stack(const uintptr_t perm_smbase, const size_t perm_smram_size, + const unsigned int total_cpus, const size_t stack_size); +int smm_setup_relocation_handler(struct smm_loader_params *params); int smm_load_module(uintptr_t smram_base, size_t smram_size, struct smm_loader_params *params); u32 smm_get_cpu_smbase(unsigned int cpu_num); -- To view, visit https://review.coreboot.org/c/coreboot/+/58698 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7bdca775550e8280757a6c5a5150a0d638d5fc2d Gerrit-Change-Number: 58698 Gerrit-PatchSet: 6 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: nb/intel/i945,gm45: Use incrementing index with fixed resource
by HAOUAS Elyes (Code Review) 07 Feb '22

07 Feb '22

Attention is currently required from: Nico Huber, Arthur Heymans, Kyösti Mälkki, Patrick Rudolph. HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55927 ) Change subject: nb/intel/i945,gm45: Use incrementing index with fixed resource ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/55927/comment/366a82ff_d14dd9a4 PS1, Line 7: sb > nit: nb Done -- To view, visit https://review.coreboot.org/c/coreboot/+/55927 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7b4822c3909801e91627ed2ffe776d65dfab08d5 Gerrit-Change-Number: 55927 Gerrit-PatchSet: 2 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Mon, 07 Feb 2022 13:44:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: nb/intel/i945,gm45: Use incrementing index with fixed resource
by HAOUAS Elyes (Code Review) 07 Feb '22

07 Feb '22

Attention is currently required from: Nico Huber, Arthur Heymans, Kyösti Mälkki, Patrick Rudolph. HAOUAS Elyes has uploaded a new patch set (#2) to the change originally created by Kyösti Mälkki. ( https://review.coreboot.org/c/coreboot/+/55927 ) Change subject: nb/intel/i945,gm45: Use incrementing index with fixed resource ...................................................................... nb/intel/i945,gm45: Use incrementing index with fixed resource Do this for consistency, while followup will remove the index completely. Change-Id: I7b4822c3909801e91627ed2ffe776d65dfab08d5 Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> --- M src/northbridge/intel/gm45/northbridge.c M src/northbridge/intel/i945/northbridge.c 2 files changed, 16 insertions(+), 14 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/55927/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/55927 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7b4822c3909801e91627ed2ffe776d65dfab08d5 Gerrit-Change-Number: 55927 Gerrit-PatchSet: 2 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: cpu/x86/smm: Improve smm stack setup
by Angel Pons (Code Review) 07 Feb '22

07 Feb '22

Attention is currently required from: Arthur Heymans, Kyösti Mälkki. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58698 ) Change subject: cpu/x86/smm: Improve smm stack setup ...................................................................... Patch Set 5: Code-Review+2 (1 comment) Patchset: PS5: Tested on Asrock B85M Pro4, still reaches payload. -- To view, visit https://review.coreboot.org/c/coreboot/+/58698 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7bdca775550e8280757a6c5a5150a0d638d5fc2d Gerrit-Change-Number: 58698 Gerrit-PatchSet: 5 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Comment-Date: Mon, 07 Feb 2022 13:08:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common/cse: Add `cse_send_end_of_post()` as a public function
by Subrata Banik (Code Review) 07 Feb '22

07 Feb '22

Attention is currently required from: Tim Wawrzynczak, Sridhar Siricilla, Angel Pons, Werner Zeh, Patrick Rudolph, EricR Lai. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61519 ) Change subject: soc/intel/common/cse: Add `cse_send_end_of_post()` as a public function ...................................................................... Patch Set 8: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/61519/comment/f5d71dcf_011d037b PS7, Line 7: a > Did you mean "as"here? Ack -- To view, visit https://review.coreboot.org/c/coreboot/+/61519 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I837c5723eca766d21b191b98e39eb52889498bfc Gerrit-Change-Number: 61519 Gerrit-PatchSet: 8 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Comment-Date: Mon, 07 Feb 2022 13:07:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common/cse: Add `cse_send_end_of_post()` as a public function
by Subrata Banik (Code Review) 07 Feb '22

07 Feb '22

Attention is currently required from: Subrata Banik, Tim Wawrzynczak, Sridhar Siricilla, Angel Pons, Patrick Rudolph, EricR Lai. Hello build bot (Jenkins), Tim Wawrzynczak, Angel Pons, Nick Vaccaro, Werner Zeh, Patrick Rudolph, EricR Lai, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/61519 to look at the new patch set (#8). Change subject: soc/intel/common/cse: Add `cse_send_end_of_post()` as a public function ...................................................................... soc/intel/common/cse: Add `cse_send_end_of_post()` as a public function This patch creates a global function `cse_send_end_of_post()` so that IA common code may get access to this function for sending EOP command to the HECI1/CSE device. Additionally, use static variable to track and prevent sending EOP command more than once in boot flow. BUG=b:211954778 TEST=Able to build and boot Brya. Signed-off-by: Subrata Banik <subratabanik(a)google.com> Change-Id: I837c5723eca766d21b191b98e39eb52889498bfc --- M src/soc/intel/common/block/cse/cse_eop.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 16 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/61519/8 -- To view, visit https://review.coreboot.org/c/coreboot/+/61519 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I837c5723eca766d21b191b98e39eb52889498bfc Gerrit-Change-Number: 61519 Gerrit-PatchSet: 8 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-MessageType: newpatchset

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit February 2022