Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson, Fred Reitberger, Felix Held. Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/64866 )
Change subject: cpu/amd: Add common helpers for TSEG and SMM ......................................................................
cpu/amd: Add common helpers for TSEG and SMM
Change-Id: I73174766980e0405e7b8efd4f059bb400c0c0a25 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/cpu/amd/Kconfig M src/cpu/amd/Makefile.inc M src/cpu/amd/agesa/Kconfig M src/cpu/amd/agesa/family14/model_14_init.c M src/cpu/amd/agesa/family15tn/model_15_init.c M src/cpu/amd/agesa/family16kb/model_16_init.c M src/cpu/amd/pi/00730F01/model_16_init.c M src/cpu/amd/pi/Kconfig M src/cpu/amd/smm/Makefile.inc A src/cpu/amd/smm/smm_helper.c M src/soc/amd/common/block/cpu/Kconfig M src/soc/amd/common/block/cpu/noncar/memmap.c M src/soc/amd/common/block/cpu/smm/Makefile.inc M src/soc/amd/common/block/cpu/smm/finalize.c D src/soc/amd/common/block/cpu/smm/smm_helper.c M src/soc/amd/common/block/cpu/smm/smm_relocate.c M src/soc/amd/common/block/include/amdblocks/smm.h 17 files changed, 122 insertions(+), 90 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/64866/1
diff --git a/src/cpu/amd/Kconfig b/src/cpu/amd/Kconfig index db8989d..0c2bd25 100644 --- a/src/cpu/amd/Kconfig +++ b/src/cpu/amd/Kconfig @@ -1,2 +1,7 @@ source "src/cpu/amd/agesa/Kconfig" source "src/cpu/amd/pi/Kconfig" + +config CPU_AMD_SMM + bool + help + Select this for SMM related functionality on AMD hardware. diff --git a/src/cpu/amd/Makefile.inc b/src/cpu/amd/Makefile.inc index 5c07a66..0f44343 100644 --- a/src/cpu/amd/Makefile.inc +++ b/src/cpu/amd/Makefile.inc @@ -1,2 +1,3 @@ subdirs-$(CONFIG_CPU_AMD_AGESA) += agesa subdirs-$(CONFIG_CPU_AMD_PI) += pi +subdirs-$(CONFIG_CPU_AMD_SMM) += smm diff --git a/src/cpu/amd/agesa/Kconfig b/src/cpu/amd/agesa/Kconfig index 266fc89..ec97da6 100644 --- a/src/cpu/amd/agesa/Kconfig +++ b/src/cpu/amd/agesa/Kconfig @@ -13,6 +13,7 @@ select LAPIC_MONOTONIC_TIMER select SPI_FLASH if HAVE_ACPI_RESUME select SSE2 + select CPU_AMD_SMM
if CPU_AMD_AGESA
diff --git a/src/cpu/amd/agesa/family14/model_14_init.c b/src/cpu/amd/agesa/family14/model_14_init.c index 41fa1ef..cd7f076 100644 --- a/src/cpu/amd/agesa/family14/model_14_init.c +++ b/src/cpu/amd/agesa/family14/model_14_init.c @@ -1,14 +1,15 @@ /* SPDX-License-Identifier: GPL-2.0-only */
+#include <acpi/acpi.h> #include <console/console.h> -#include <cpu/x86/msr.h> #include <cpu/amd/msr.h> -#include <cpu/x86/mtrr.h> #include <cpu/amd/mtrr.h> -#include <device/device.h> +#include <cpu/amd/smm.h> #include <cpu/cpu.h> #include <cpu/x86/cache.h> -#include <acpi/acpi.h> +#include <cpu/x86/msr.h> +#include <cpu/x86/mtrr.h> +#include <device/device.h> #include <northbridge/amd/agesa/agesa_helper.h>
static void model_14_init(struct device *dev) @@ -76,9 +77,7 @@ wrmsr(NB_CFG_MSR, msr);
/* Write protect SMM space with SMMLOCK. */ - msr = rdmsr(HWCR_MSR); - msr.lo |= (1 << 0); - wrmsr(HWCR_MSR, msr); + lock_smm(); }
static struct device_operations cpu_dev_ops = { diff --git a/src/cpu/amd/agesa/family15tn/model_15_init.c b/src/cpu/amd/agesa/family15tn/model_15_init.c index e246383..a557407 100644 --- a/src/cpu/amd/agesa/family15tn/model_15_init.c +++ b/src/cpu/amd/agesa/family15tn/model_15_init.c @@ -1,15 +1,16 @@ /* SPDX-License-Identifier: GPL-2.0-only */
+#include <acpi/acpi.h> #include <console/console.h> -#include <cpu/x86/msr.h> #include <cpu/amd/msr.h> -#include <cpu/x86/mtrr.h> #include <cpu/amd/mtrr.h> -#include <cpu/x86/smm.h> -#include <device/device.h> +#include <cpu/amd/smm.h> #include <cpu/cpu.h> #include <cpu/x86/cache.h> -#include <acpi/acpi.h> +#include <cpu/x86/msr.h> +#include <cpu/x86/mtrr.h> +#include <cpu/x86/smm.h> +#include <device/device.h> #include <northbridge/amd/agesa/agesa_helper.h>
static void model_15_init(struct device *dev) @@ -93,9 +94,7 @@ }
/* Write protect SMM space with SMMLOCK. */ - msr = rdmsr(HWCR_MSR); - msr.lo |= (1 << 0); - wrmsr(HWCR_MSR, msr); + lock_smm(); }
static struct device_operations cpu_dev_ops = { diff --git a/src/cpu/amd/agesa/family16kb/model_16_init.c b/src/cpu/amd/agesa/family16kb/model_16_init.c index 3c2202a..48c44f5 100644 --- a/src/cpu/amd/agesa/family16kb/model_16_init.c +++ b/src/cpu/amd/agesa/family16kb/model_16_init.c @@ -1,14 +1,15 @@ /* SPDX-License-Identifier: GPL-2.0-only */
+#include <acpi/acpi.h> #include <console/console.h> -#include <cpu/x86/msr.h> #include <cpu/amd/msr.h> -#include <cpu/x86/mtrr.h> #include <cpu/amd/mtrr.h> -#include <device/device.h> +#include <cpu/amd/smm.h> #include <cpu/cpu.h> #include <cpu/x86/cache.h> -#include <acpi/acpi.h> +#include <cpu/x86/msr.h> +#include <cpu/x86/mtrr.h> +#include <device/device.h> #include <northbridge/amd/agesa/agesa_helper.h>
static void model_16_init(struct device *dev) @@ -76,9 +77,7 @@ wrmsr(NB_CFG_MSR, msr);
/* Write protect SMM space with SMMLOCK. */ - msr = rdmsr(HWCR_MSR); - msr.lo |= (1 << 0); - wrmsr(HWCR_MSR, msr); + lock_smm(); }
static struct device_operations cpu_dev_ops = { diff --git a/src/cpu/amd/pi/00730F01/model_16_init.c b/src/cpu/amd/pi/00730F01/model_16_init.c index 1a9a2e3..9df5b73 100644 --- a/src/cpu/amd/pi/00730F01/model_16_init.c +++ b/src/cpu/amd/pi/00730F01/model_16_init.c @@ -3,14 +3,15 @@ #include <commonlib/helpers.h> #include <console/console.h> #include <cpu/amd/microcode.h> -#include <cpu/x86/msr.h> #include <cpu/amd/msr.h> -#include <cpu/x86/mtrr.h> #include <cpu/amd/mtrr.h> -#include <device/device.h> -#include <device/pci.h> +#include <cpu/amd/smm.h> #include <cpu/cpu.h> #include <cpu/x86/cache.h> +#include <cpu/x86/msr.h> +#include <cpu/x86/mtrr.h> +#include <device/device.h> +#include <device/pci.h> #include <smp/node.h>
static void model_16_init(struct device *dev) @@ -44,9 +45,7 @@ wrmsr(NB_CFG_MSR, msr);
/* Write protect SMM space with SMMLOCK. */ - msr = rdmsr(HWCR_MSR); - msr.lo |= (1 << 0); - wrmsr(HWCR_MSR, msr); + lock_smm();
amd_update_microcode_from_cbfs();
diff --git a/src/cpu/amd/pi/Kconfig b/src/cpu/amd/pi/Kconfig index 2336d68..99815d3 100644 --- a/src/cpu/amd/pi/Kconfig +++ b/src/cpu/amd/pi/Kconfig @@ -12,6 +12,7 @@ select SPI_FLASH if HAVE_ACPI_RESUME select NO_SMM select SSE2 + select CPU_AMD_SMM
if CPU_AMD_PI
diff --git a/src/cpu/amd/smm/Makefile.inc b/src/cpu/amd/smm/Makefile.inc index 97a6694..87cbb38 100644 --- a/src/cpu/amd/smm/Makefile.inc +++ b/src/cpu/amd/smm/Makefile.inc @@ -1,2 +1,3 @@ - +romstage-y += smm_helper.c +ramstage-y += smm_helper.c ramstage-y += smm_init.c diff --git a/src/cpu/amd/smm/smm_helper.c b/src/cpu/amd/smm/smm_helper.c new file mode 100644 index 0000000..06ad735 --- /dev/null +++ b/src/cpu/amd/smm/smm_helper.c @@ -0,0 +1,71 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <cpu/cpu.h> +#include <assert.h> +#include <cbmem.h> +#include <console/console.h> +#include <cpu/amd/msr.h> +#include <cpu/amd/smm.h> +#include <cpu/amd/mtrr.h> +#include <cpu/x86/msr.h> +#include <cpu/x86/smm.h> +#include <cpu/x86/mp.h> +#include <cpu/x86/mtrr.h> +#include <stdint.h> +#include <cpu/amd/amd64_save_state.h> + +/* + * For data stored in TSEG, ensure TValid is clear so R/W access can reach + * the DRAM when not in SMM. + */ +void clear_tvalid(void) +{ + msr_t hwcr = rdmsr(HWCR_MSR); + msr_t mask = rdmsr(SMM_MASK_MSR); + int tvalid = !!(mask.lo & SMM_TSEG_VALID); + + if (hwcr.lo & SMM_LOCK) { + if (!tvalid) /* not valid but locked means still accessible */ + return; + + printk(BIOS_ERR, "can't clear TValid, already locked\n"); + return; + } + + mask.lo &= ~SMM_TSEG_VALID; + wrmsr(SMM_MASK_MSR, mask); +} + +void setup_tseg(void) +{ + uintptr_t tseg_base; + size_t tseg_size; + + smm_region(&tseg_base, &tseg_size); + + msr_t msr; + msr.lo = tseg_base; + /* Let's assume TSEG < 4G for now. */ + msr.hi = 0; + wrmsr(SMM_BASE_MSR, msr); + + msr.lo = ~(tseg_size - 1); + msr.lo |= SMM_TSEG_WB; + msr.hi = (1 << (cpu_phys_address_size() - 32)) - 1; + wrmsr(SMM_MASK_MSR, msr); +} + +void tseg_valid(void) +{ + msr_t mask = rdmsr(SMM_MASK_MSR); + mask.lo |= SMM_TSEG_VALID; + + wrmsr(SMM_MASK_MSR, mask); +} + +void lock_smm(void) +{ + msr_t hwcr = rdmsr(HWCR_MSR); + hwcr.lo |= SMM_LOCK; + wrmsr(HWCR_MSR, hwcr); +} diff --git a/src/soc/amd/common/block/cpu/Kconfig b/src/soc/amd/common/block/cpu/Kconfig index 56e922d..b7468bb 100644 --- a/src/soc/amd/common/block/cpu/Kconfig +++ b/src/soc/amd/common/block/cpu/Kconfig @@ -82,3 +82,6 @@ depends on SOC_AMD_COMMON_BLOCK_UCODE help Defines the size of the uCode binary in bytes. + +config CPU_AMD_SMM + default y diff --git a/src/soc/amd/common/block/cpu/noncar/memmap.c b/src/soc/amd/common/block/cpu/noncar/memmap.c index dd057c0..6899b11 100644 --- a/src/soc/amd/common/block/cpu/noncar/memmap.c +++ b/src/soc/amd/common/block/cpu/noncar/memmap.c @@ -4,6 +4,7 @@ #include <amdblocks/smm.h> #include <console/console.h> #include <cbmem.h> +#include <cpu/amd/smm.h> #include <cpu/amd/msr.h> #include <cpu/x86/smm.h> #include <fsp/util.h> diff --git a/src/soc/amd/common/block/cpu/smm/Makefile.inc b/src/soc/amd/common/block/cpu/smm/Makefile.inc index 5694d1d..cdb0efb 100644 --- a/src/soc/amd/common/block/cpu/smm/Makefile.inc +++ b/src/soc/amd/common/block/cpu/smm/Makefile.inc @@ -1,10 +1,7 @@ ifeq ($(CONFIG_SOC_AMD_COMMON_BLOCK_SMM),y)
-romstage-y += smm_helper.c -postcar-y += smm_helper.c ramstage-y += finalize.c ramstage-y += smm_relocate.c -ramstage-y += smm_helper.c smm-y += smi_apmc_helper.c smm-y += smi_handler.c
diff --git a/src/soc/amd/common/block/cpu/smm/finalize.c b/src/soc/amd/common/block/cpu/smm/finalize.c index 73b2121..08d93ea 100644 --- a/src/soc/amd/common/block/cpu/smm/finalize.c +++ b/src/soc/amd/common/block/cpu/smm/finalize.c @@ -1,31 +1,28 @@ /* SPDX-License-Identifier: GPL-2.0-only */
#include <acpi/acpi.h> -#include <cpu/x86/mp.h> -#include <cpu/x86/msr.h> -#include <cpu/amd/msr.h> +#include <amdblocks/acpi.h> #include <bootstate.h> #include <console/console.h> -#include <amdblocks/acpi.h> +#include <cpu/amd/msr.h> +#include <cpu/amd/smm.h> +#include <cpu/x86/mp.h> +#include <cpu/x86/msr.h> #include <types.h>
static void per_core_finalize(void *unused) { - msr_t hwcr, mask; + msr_t hwcr;
/* Finalize SMM settings */ hwcr = rdmsr(HWCR_MSR); if (hwcr.lo & SMM_LOCK) /* Skip if already locked, avoid GPF */ return;
- if (CONFIG(HAVE_SMI_HANDLER)) { - mask = rdmsr(SMM_MASK_MSR); - mask.lo |= SMM_TSEG_VALID; - wrmsr(SMM_MASK_MSR, mask); - } + if (CONFIG(HAVE_SMI_HANDLER)) + tseg_valid();
- hwcr.lo |= SMM_LOCK; - wrmsr(HWCR_MSR, hwcr); + lock_smm(); }
static void finalize_cores(void) diff --git a/src/soc/amd/common/block/cpu/smm/smm_helper.c b/src/soc/amd/common/block/cpu/smm/smm_helper.c deleted file mode 100644 index ce80f16..0000000 --- a/src/soc/amd/common/block/cpu/smm/smm_helper.c +++ /dev/null @@ -1,28 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -#include <amdblocks/smm.h> -#include <console/console.h> -#include <cpu/x86/msr.h> -#include <cpu/amd/msr.h> - -/* - * For data stored in TSEG, ensure TValid is clear so R/W access can reach - * the DRAM when not in SMM. - */ -void clear_tvalid(void) -{ - msr_t hwcr = rdmsr(HWCR_MSR); - msr_t mask = rdmsr(SMM_MASK_MSR); - int tvalid = !!(mask.lo & SMM_TSEG_VALID); - - if (hwcr.lo & SMM_LOCK) { - if (!tvalid) /* not valid but locked means still accessible */ - return; - - printk(BIOS_ERR, "can't clear TValid, already locked\n"); - return; - } - - mask.lo &= ~SMM_TSEG_VALID; - wrmsr(SMM_MASK_MSR, mask); -} diff --git a/src/soc/amd/common/block/cpu/smm/smm_relocate.c b/src/soc/amd/common/block/cpu/smm/smm_relocate.c index 11195d8..113b4c4 100644 --- a/src/soc/amd/common/block/cpu/smm/smm_relocate.c +++ b/src/soc/amd/common/block/cpu/smm/smm_relocate.c @@ -4,6 +4,7 @@ #include <console/console.h> #include <cpu/amd/amd64_save_state.h> #include <cpu/amd/msr.h> +#include <cpu/amd/smm.h> #include <cpu/cpu.h> #include <cpu/x86/msr.h> #include <cpu/x86/smm.h> @@ -34,23 +35,9 @@
void smm_relocation_handler(int cpu, uintptr_t curr_smbase, uintptr_t staggered_smbase) { + setup_tseg(); + amd64_smm_state_save_area_t *smm_state; - - uintptr_t tseg_base; - size_t tseg_size; - - smm_region(&tseg_base, &tseg_size); - - msr_t msr; - msr.lo = tseg_base; - msr.hi = 0; - wrmsr(SMM_ADDR_MSR, msr); - - msr.lo = ~(tseg_size - 1); - msr.lo |= SMM_TSEG_WB; - msr.hi = (1 << (cpu_phys_address_size() - 32)) - 1; - wrmsr(SMM_MASK_MSR, relo_params->tseg_mask); - smm_state = (void *)(SMM_AMD64_SAVE_STATE_OFFSET + curr_smbase); smm_state->smbase = staggered_smbase; } diff --git a/src/soc/amd/common/block/include/amdblocks/smm.h b/src/soc/amd/common/block/include/amdblocks/smm.h index 9124607..732dc5c 100644 --- a/src/soc/amd/common/block/include/amdblocks/smm.h +++ b/src/soc/amd/common/block/include/amdblocks/smm.h @@ -11,7 +11,6 @@ void *get_smi_source_handler(int source); void handle_smi_gsmi(void); void handle_smi_store(void); -void clear_tvalid(void); /* See SMITYPE_* for list possible of events. GEVENTS are handled with mainboard_smi_gpi. */ void mainboard_handle_smi(int event);
-
Arthur Heymans (Code Review)