Change in coreboot[master]: mb/emulation/qemu-q35: Add support for SMM_TSEG with parallel MP init
Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/48262 )
Change subject: mb/emulation/qemu-q35: Add support for SMM_TSEG with parallel MP init ......................................................................
mb/emulation/qemu-q35: Add support for SMM_TSEG with parallel MP init
Change-Id: I612cebcd2ddef809434eb9bfae9d8681cda112ef Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/cpu/qemu-x86/Kconfig M src/mainboard/emulation/qemu-i440fx/northbridge.c M src/mainboard/emulation/qemu-q35/Makefile.inc A src/mainboard/emulation/qemu-q35/cpu.c M src/mainboard/emulation/qemu-q35/memmap.c 5 files changed, 92 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/48262/1
diff --git a/src/cpu/qemu-x86/Kconfig b/src/cpu/qemu-x86/Kconfig index 641cea8..0f59b9c 100644 --- a/src/cpu/qemu-x86/Kconfig +++ b/src/cpu/qemu-x86/Kconfig @@ -36,9 +36,10 @@ depends on !PARALLEL_MP select SMM_ASEG
-#config CPU_QEMU_X86_TSEG_SMM -# bool "SMM in TSEG" -# select SMM_TSEG +config CPU_QEMU_X86_TSEG_SMM + bool "SMM in TSEG" + select SMM_TSEG + depends on PARALLEL_MP
endchoice
diff --git a/src/mainboard/emulation/qemu-i440fx/northbridge.c b/src/mainboard/emulation/qemu-i440fx/northbridge.c index f49d47da..7ee38d3 100644 --- a/src/mainboard/emulation/qemu-i440fx/northbridge.c +++ b/src/mainboard/emulation/qemu-i440fx/northbridge.c @@ -249,9 +249,15 @@ .get_cpu_count = fw_cfg_max_cpus, };
+extern const struct mp_ops mp_ops_with_smm; + void mp_init_cpus(struct bus *cpu_bus) { - if (mp_init_with_smm(cpu_bus, &mp_ops_no_smm)) + const struct mp_ops *ops = &mp_ops_no_smm; + if (CONFIG(SMM_TSEG)) + ops = &mp_ops_with_smm; + + if (mp_init_with_smm(cpu_bus, ops)) printk(BIOS_ERR, "MP initialization failure.\n"); }
diff --git a/src/mainboard/emulation/qemu-q35/Makefile.inc b/src/mainboard/emulation/qemu-q35/Makefile.inc index 4bd91f0..f717405 100644 --- a/src/mainboard/emulation/qemu-q35/Makefile.inc +++ b/src/mainboard/emulation/qemu-q35/Makefile.inc @@ -13,6 +13,7 @@ ramstage-y += ../qemu-i440fx/memmap.c ramstage-y += ../qemu-i440fx/northbridge.c ramstage-y += memmap.c +ramstage-y += cpu.c
verstage-$(CONFIG_CHROMEOS) += chromeos.c verstage-$(CONFIG_CHROMEOS) += ../qemu-i440fx/fw_cfg.c diff --git a/src/mainboard/emulation/qemu-q35/cpu.c b/src/mainboard/emulation/qemu-q35/cpu.c new file mode 100644 index 0000000..c0f4ee5 --- /dev/null +++ b/src/mainboard/emulation/qemu-q35/cpu.c @@ -0,0 +1,64 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <console/console.h> +#include <cpu/x86/mp.h> +#include <stdint.h> +#include <cpu/intel/smm_reloc.h> +#include <cpu/amd/amd64_save_state.h> +#include <mainboard/emulation/qemu-i440fx/fw_cfg.h> + +static void get_smm_info(uintptr_t *perm_smbase, size_t *perm_smsize, + size_t *smm_save_state_size) +{ + printk(BIOS_DEBUG, "Setting up SMI for CPU\n"); + + smm_subregion(SMM_SUBREGION_HANDLER, perm_smbase, perm_smsize); + +// *smm_save_state_size = sizeof(amd64_smm_state_save_area_t); + *smm_save_state_size = 0x400; + printk(BIOS_DEBUG, "Save state size: 0x%lx bytes\n", *smm_save_state_size); +} + +static void per_cpu_smm_trigger(void) +{ + /* Relocate SMM space. */ + smm_initiate_relocation(); +} + +/* The relocation work is actually performed in SMM context, but the code + * resides in the ramstage module. This occurs by trampolining from the default + * SMRAM entry point to here. */ +static void relocation_handler(int cpu, uintptr_t curr_smbase, + uintptr_t staggered_smbase) +{ + /* The em64t101 save state is sufficiently compatible with older + save states with regards of smbase, smm_revision. */ + amd64_smm_state_save_area_t *save_state; + u32 smbase = staggered_smbase; + + save_state = (void *)(curr_smbase + SMM_DEFAULT_SIZE - sizeof(*save_state)); + save_state->smbase = smbase; + + printk(BIOS_DEBUG, "In relocation handler: cpu %d\n", cpu); + printk(BIOS_SPEW, "SMM revision: 0x%08x\n", save_state->smm_revision); + printk(BIOS_DEBUG, "New SMBASE=0x%08x\n", smbase); +} + +static void post_mp_init(void) +{ + /* Now that all APs have been relocated as well as the BSP let SMIs + * start flowing. */ + global_smi_enable(); + + /* Lock down the SMRAM space. */ + smm_lock(); +} + +const struct mp_ops mp_ops_with_smm = { + .get_cpu_count = fw_cfg_max_cpus, + .get_smm_info = get_smm_info, + .pre_mp_smm_init = smm_southbridge_clear_state, + .per_cpu_smm_trigger = per_cpu_smm_trigger, + .relocation_handler = relocation_handler, + .post_mp_init = post_mp_init, +}; diff --git a/src/mainboard/emulation/qemu-q35/memmap.c b/src/mainboard/emulation/qemu-q35/memmap.c index c288680..04cf285 100644 --- a/src/mainboard/emulation/qemu-q35/memmap.c +++ b/src/mainboard/emulation/qemu-q35/memmap.c @@ -2,14 +2,17 @@
#define __SIMPLE_DEVICE__
+#include <console/console.h> #include <cpu/x86/smm.h> #include <device/pci_ops.h> #include <mainboard/emulation/qemu-i440fx/memory.h> #include <mainboard/emulation/qemu-i440fx/fw_cfg.h> +#include <cpu/intel/smm_reloc.h>
#define EXT_TSEG_MBYTES 0x50
#define SMRAMC 0x9d +#define C_BASE_SEG ((0 << 2) | (1 << 1) | (0 << 0)) #define G_SMRAME (1 << 3) #define D_LCK (1 << 4) #define D_CLS (1 << 5) @@ -40,3 +43,16 @@ *start = qemu_get_memory_size() * KiB - *size; printk(BIOS_SPEW, "SMM_BASE: 0x%08lx, SMM_SIZE: %ld MiB\n", *start, *size / MiB); } + +void smm_lock(void) +{ + /* LOCK the SMM memory window and enable normal SMM. + * After running this function, only a full reset can + * make the SMM registers writable again. + */ + printk(BIOS_DEBUG, "Locking SMM.\n"); + + pci_or_config8(PCI_DEV(0, 0, 0), ESMRAMC, T_EN); + pci_write_config8(PCI_DEV(0, 0, 0), SMRAMC, D_LCK | G_SMRAME | C_BASE_SEG); +} +
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48262
to look at the new patch set (#3).
Change subject: mb/emulation/qemu-q35: Add support for SMM_TSEG with parallel MP init ......................................................................
mb/emulation/qemu-q35: Add support for SMM_TSEG with parallel MP init
Change-Id: I612cebcd2ddef809434eb9bfae9d8681cda112ef Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/cpu/qemu-x86/Kconfig M src/mainboard/emulation/qemu-i440fx/northbridge.c M src/mainboard/emulation/qemu-q35/Makefile.inc A src/mainboard/emulation/qemu-q35/cpu.c M src/mainboard/emulation/qemu-q35/memmap.c 5 files changed, 90 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/48262/3
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48262
to look at the new patch set (#4).
Change subject: mb/emulation/qemu-q35: Add support for SMM_TSEG with parallel MP init ......................................................................
mb/emulation/qemu-q35: Add support for SMM_TSEG with parallel MP init
Change-Id: I612cebcd2ddef809434eb9bfae9d8681cda112ef Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/cpu/qemu-x86/Kconfig M src/mainboard/emulation/qemu-i440fx/northbridge.c M src/mainboard/emulation/qemu-q35/Makefile.inc A src/mainboard/emulation/qemu-q35/cpu.c M src/mainboard/emulation/qemu-q35/memmap.c 5 files changed, 88 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/48262/4
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48262 )
Change subject: mb/emulation/qemu-q35: Add support for SMM_TSEG with parallel MP init ......................................................................
Patch Set 4:
(5 comments)
https://review.coreboot.org/c/coreboot/+/48262/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48262/4//COMMIT_MSG@8 PS4, Line 8: Tested w/ / w/o KVM, MP?
https://review.coreboot.org/c/coreboot/+/48262/4/src/mainboard/emulation/qem... File src/mainboard/emulation/qemu-q35/cpu.c:
https://review.coreboot.org/c/coreboot/+/48262/4/src/mainboard/emulation/qem... PS4, Line 17: // *smm_save_state_size = sizeof(amd64_smm_state_save_area_t); : *smm_save_state_size = 0x400; Do you want to add a comment about this? ;)
https://review.coreboot.org/c/coreboot/+/48262/4/src/mainboard/emulation/qem... PS4, Line 30: * SMRAM entry point to here. */ Nit, proper style would be dangling /* and */ each on a line of its own. (or drop the lone asterisks on the left, but at top level I'd prefer the extensive style)
https://review.coreboot.org/c/coreboot/+/48262/4/src/mainboard/emulation/qem... PS4, Line 43: BIOS_SPEW Nit, I guess it's because other code does it, but I don't see a real reason do make this BIOS_SPEW.
https://review.coreboot.org/c/coreboot/+/48262/4/src/mainboard/emulation/qem... PS4, Line 50: * Nit, drop this * like above.
Or maybe just make it a single line?
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48262 )
Change subject: mb/emulation/qemu-q35: Add support for SMM_TSEG with parallel MP init ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48262/4/src/mainboard/emulation/qem... File src/mainboard/emulation/qemu-q35/cpu.c:
https://review.coreboot.org/c/coreboot/+/48262/4/src/mainboard/emulation/qem... PS4, Line 17: // *smm_save_state_size = sizeof(amd64_smm_state_save_area_t); : *smm_save_state_size = 0x400;
Do you want to add a comment about this? ;)
Hmm that should not have made it here ^^. With 64bit the stub size is larger than the AMD64 save state so a proper fix needs to be done in mp init code.
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48262
to look at the new patch set (#5).
Change subject: mb/emulation/qemu-q35: Add support for SMM_TSEG with parallel MP init ......................................................................
mb/emulation/qemu-q35: Add support for SMM_TSEG with parallel MP init
Change-Id: I612cebcd2ddef809434eb9bfae9d8681cda112ef Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/cpu/qemu-x86/Kconfig M src/mainboard/emulation/qemu-i440fx/northbridge.c M src/mainboard/emulation/qemu-q35/Makefile.inc A src/mainboard/emulation/qemu-q35/cpu.c M src/mainboard/emulation/qemu-q35/memmap.c 5 files changed, 88 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/48262/5
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48262
to look at the new patch set (#6).
Change subject: mb/emulation/qemu-q35: Add support for SMM_TSEG with parallel MP init ......................................................................
mb/emulation/qemu-q35: Add support for SMM_TSEG with parallel MP init
Change-Id: I612cebcd2ddef809434eb9bfae9d8681cda112ef Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/cpu/qemu-x86/Kconfig M src/mainboard/emulation/qemu-i440fx/northbridge.c M src/mainboard/emulation/qemu-q35/Makefile.inc A src/mainboard/emulation/qemu-q35/cpu.c M src/mainboard/emulation/qemu-q35/memmap.c 5 files changed, 89 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/48262/6
Attention is currently required from: Nico Huber. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48262 )
Change subject: mb/emulation/qemu-q35: Add support for SMM_TSEG with parallel MP init ......................................................................
Patch Set 6:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/48262/comment/ece0da89_fc9cef46 PS4, Line 8:
Tested w/ / w/o KVM, MP?
I'll run more tests. This was w/o KVM and multiple CPUs
File src/mainboard/emulation/qemu-q35/cpu.c:
https://review.coreboot.org/c/coreboot/+/48262/comment/02b073cb_d9b74eeb PS4, Line 17: // *smm_save_state_size = sizeof(amd64_smm_state_save_area_t); : *smm_save_state_size = 0x400;
Do you want to add a comment about this? ;) […]
Done
Attention is currently required from: Nico Huber, Arthur Heymans. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48262 )
Change subject: mb/emulation/qemu-q35: Add support for SMM_TSEG with parallel MP init ......................................................................
Patch Set 6: Code-Review+1
(1 comment)
File src/mainboard/emulation/qemu-q35/memmap.c:
https://review.coreboot.org/c/coreboot/+/48262/comment/82ec17a6_f1ce74d8 PS6, Line 49: /* Comment style:
/* * LOCK ...
Attention is currently required from: Nico Huber. Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48262
to look at the new patch set (#8).
Change subject: mb/emulation/qemu-q35: Add support for SMM_TSEG with parallel MP init ......................................................................
mb/emulation/qemu-q35: Add support for SMM_TSEG with parallel MP init
Change-Id: I612cebcd2ddef809434eb9bfae9d8681cda112ef Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/cpu/qemu-x86/Kconfig M src/mainboard/emulation/qemu-i440fx/northbridge.c M src/mainboard/emulation/qemu-q35/Makefile.inc A src/mainboard/emulation/qemu-q35/cpu.c M src/mainboard/emulation/qemu-q35/memmap.c 5 files changed, 90 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/48262/8
Attention is currently required from: Nico Huber, Angel Pons, Arthur Heymans. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48262 )
Change subject: mb/emulation/qemu-q35: Add support for SMM_TSEG with parallel MP init ......................................................................
Patch Set 8:
(1 comment)
File src/mainboard/emulation/qemu-q35/memmap.c:
https://review.coreboot.org/c/coreboot/+/48262/comment/e1ad1845_4e56412f PS6, Line 49: /*
Comment style: […]
Done
Attention is currently required from: Nico Huber, Arthur Heymans. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48262 )
Change subject: mb/emulation/qemu-q35: Add support for SMM_TSEG with parallel MP init ......................................................................
Patch Set 8: Code-Review+1
Attention is currently required from: Arthur Heymans. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48262 )
Change subject: mb/emulation/qemu-q35: Add support for SMM_TSEG with parallel MP init ......................................................................
Patch Set 8:
(1 comment)
File src/mainboard/emulation/qemu-q35/cpu.c:
https://review.coreboot.org/c/coreboot/+/48262/comment/dd3f09c5_19f02378 PS8, Line 17: /* FIXME: on X86_64 the save state size is smaller than the size of the SMM stub */ I assume when the stub is compiled for X86_64? Why is this a problem?
Attention is currently required from: Nico Huber. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48262 )
Change subject: mb/emulation/qemu-q35: Add support for SMM_TSEG with parallel MP init ......................................................................
Patch Set 8:
(1 comment)
File src/mainboard/emulation/qemu-q35/cpu.c:
https://review.coreboot.org/c/coreboot/+/48262/comment/e6aab16e_b82c29c0 PS8, Line 17: /* FIXME: on X86_64 the save state size is smaller than the size of the SMM stub */
I assume when the stub is compiled for X86_64? Why is this a problem?
So taking the MAX(stub size, save state size) during the mp init would work, but the smihandler save state handler is not dealing with that properly ATM. In general the SMM setup needs some work/refactoring. I thought it would be best to leave a FIXME as the compilation would fail on x86_64 and setting a larger size is quick fix (even though some things don't work then)
Attention is currently required from: Arthur Heymans. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48262 )
Change subject: mb/emulation/qemu-q35: Add support for SMM_TSEG with parallel MP init ......................................................................
Patch Set 8:
(2 comments)
File src/cpu/qemu-x86/Kconfig:
https://review.coreboot.org/c/coreboot/+/48262/comment/0f3412ab_2d60cc31 PS8, Line 42: depends on PARALLEL_MP In the light of the other comment thread, shouldn't this depend on X86_32?
File src/mainboard/emulation/qemu-q35/cpu.c:
https://review.coreboot.org/c/coreboot/+/48262/comment/9914d456_ec2213f6 PS8, Line 17: /* FIXME: on X86_64 the save state size is smaller than the size of the SMM stub */ Ack
So taking the MAX(stub size, save state size) during the mp init would work
Iow., the MP init code assumes this value is high enough to fit something else (the stub)? Soundslike something to fix there.
Attention is currently required from: Arthur Heymans. Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48262 )
Change subject: mb/emulation/qemu-q35: Add support for SMM_TSEG with parallel MP init ......................................................................
Patch Set 8:
(1 comment)
Patchset:
PS8: Should this work: qemu-system-i386 -bios build/coreboot.rom -M q35 -serial stdio
Seems like SMM module loader is stuck in relocation phase and never reaches the real SMI handlers.
Attention is currently required from: Kyösti Mälkki. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48262 )
Change subject: mb/emulation/qemu-q35: Add support for SMM_TSEG with parallel MP init ......................................................................
Patch Set 8:
(1 comment)
Patchset:
PS8:
Should this work: qemu-system-i386 -bios build/coreboot.rom -M q35 -serial stdio
Seems like SMM module loader is stuck in relocation phase and never reaches the real SMI handlers.
This does work fine here even with the -smp x argument.
Attention is currently required from: Arthur Heymans. Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48262 )
Change subject: mb/emulation/qemu-q35: Add support for SMM_TSEG with parallel MP init ......................................................................
Patch Set 8:
(1 comment)
Patchset:
PS8:
Should this work: […]
Ack
Attention is currently required from: Arthur Heymans. Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48262
to look at the new patch set (#9).
Change subject: mb/emulation/qemu-q35: Add support for SMM_TSEG with parallel MP init ......................................................................
mb/emulation/qemu-q35: Add support for SMM_TSEG with parallel MP init
Change-Id: I612cebcd2ddef809434eb9bfae9d8681cda112ef Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/cpu/qemu-x86/Kconfig M src/mainboard/emulation/qemu-i440fx/northbridge.c M src/mainboard/emulation/qemu-q35/Makefile.inc A src/mainboard/emulation/qemu-q35/cpu.c M src/mainboard/emulation/qemu-q35/memmap.c 5 files changed, 92 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/48262/9
Attention is currently required from: Arthur Heymans. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48262 )
Change subject: mb/emulation/qemu-q35: Add support for SMM_TSEG with parallel MP init ......................................................................
Patch Set 9: Code-Review+1
Attention is currently required from: Arthur Heymans. Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48262
to look at the new patch set (#10).
Change subject: mb/emulation/qemu-q35: Add support for SMM_TSEG with parallel MP init ......................................................................
mb/emulation/qemu-q35: Add support for SMM_TSEG with parallel MP init
Tested with and without -enable-kvm, with -smp 1 2 and 32.
Change-Id: I612cebcd2ddef809434eb9bfae9d8681cda112ef Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/cpu/qemu-x86/Kconfig M src/mainboard/emulation/qemu-i440fx/northbridge.c M src/mainboard/emulation/qemu-q35/Makefile.inc A src/mainboard/emulation/qemu-q35/cpu.c M src/mainboard/emulation/qemu-q35/memmap.c 5 files changed, 92 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/48262/10
Attention is currently required from: Nico Huber. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48262 )
Change subject: mb/emulation/qemu-q35: Add support for SMM_TSEG with parallel MP init ......................................................................
Patch Set 10:
(1 comment)
File src/cpu/qemu-x86/Kconfig:
https://review.coreboot.org/c/coreboot/+/48262/comment/8d882ae4_4ceb0635 PS8, Line 42: depends on PARALLEL_MP
In the light of the other comment thread, shouldn't this depend on X86_32?
allowing for later sizes is fixed in CB:50770. It causes problems if the stub > save state in the current state of the code. This happens to be the case on X86_32 in this CL, but is not necesarrily the case in alter ones that reduce the stub size. As it's fixed later on I'll just leave it like this for now.
Attention is currently required from: Nico Huber. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48262 )
Change subject: mb/emulation/qemu-q35: Add support for SMM_TSEG with parallel MP init ......................................................................
Patch Set 10:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/48262/comment/1f357ad1_4e4e3609 PS4, Line 8:
I'll run more tests. […]
Done
Attention is currently required from: Nico Huber, Arthur Heymans. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48262 )
Change subject: mb/emulation/qemu-q35: Add support for SMM_TSEG with parallel MP init ......................................................................
Patch Set 10: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/48262 )
Change subject: mb/emulation/qemu-q35: Add support for SMM_TSEG with parallel MP init ......................................................................
mb/emulation/qemu-q35: Add support for SMM_TSEG with parallel MP init
Tested with and without -enable-kvm, with -smp 1 2 and 32.
Change-Id: I612cebcd2ddef809434eb9bfae9d8681cda112ef Signed-off-by: Arthur Heymans arthur@aheymans.xyz Reviewed-on: https://review.coreboot.org/c/coreboot/+/48262 Reviewed-by: Angel Pons th3fanbus@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/cpu/qemu-x86/Kconfig M src/mainboard/emulation/qemu-i440fx/northbridge.c M src/mainboard/emulation/qemu-q35/Makefile.inc A src/mainboard/emulation/qemu-q35/cpu.c M src/mainboard/emulation/qemu-q35/memmap.c 5 files changed, 92 insertions(+), 4 deletions(-)
Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved
diff --git a/src/cpu/qemu-x86/Kconfig b/src/cpu/qemu-x86/Kconfig index a22d7f9..85f99e9 100644 --- a/src/cpu/qemu-x86/Kconfig +++ b/src/cpu/qemu-x86/Kconfig @@ -36,9 +36,10 @@ depends on !PARALLEL_MP select SMM_ASEG
-#config CPU_QEMU_X86_TSEG_SMM -# bool "SMM in TSEG" -# select SMM_TSEG +config CPU_QEMU_X86_TSEG_SMM + bool "SMM in TSEG" + select SMM_TSEG + depends on PARALLEL_MP
endchoice
diff --git a/src/mainboard/emulation/qemu-i440fx/northbridge.c b/src/mainboard/emulation/qemu-i440fx/northbridge.c index 80fba1d..fcff7bc 100644 --- a/src/mainboard/emulation/qemu-i440fx/northbridge.c +++ b/src/mainboard/emulation/qemu-i440fx/northbridge.c @@ -249,9 +249,13 @@ .get_cpu_count = fw_cfg_max_cpus, };
+extern const struct mp_ops mp_ops_with_smm; + void mp_init_cpus(struct bus *cpu_bus) { - if (mp_init_with_smm(cpu_bus, &mp_ops_no_smm)) + const struct mp_ops *ops = CONFIG(SMM_TSEG) ? &mp_ops_with_smm : &mp_ops_no_smm; + + if (mp_init_with_smm(cpu_bus, ops)) printk(BIOS_ERR, "MP initialization failure.\n"); }
diff --git a/src/mainboard/emulation/qemu-q35/Makefile.inc b/src/mainboard/emulation/qemu-q35/Makefile.inc index 99980e3..9179e9e 100644 --- a/src/mainboard/emulation/qemu-q35/Makefile.inc +++ b/src/mainboard/emulation/qemu-q35/Makefile.inc @@ -14,6 +14,7 @@ ramstage-y += ../qemu-i440fx/memmap.c ramstage-y += ../qemu-i440fx/northbridge.c ramstage-y += memmap.c +ramstage-y += cpu.c
verstage-$(CONFIG_CHROMEOS) += chromeos.c verstage-$(CONFIG_CHROMEOS) += ../qemu-i440fx/fw_cfg.c diff --git a/src/mainboard/emulation/qemu-q35/cpu.c b/src/mainboard/emulation/qemu-q35/cpu.c new file mode 100644 index 0000000..b30d729 --- /dev/null +++ b/src/mainboard/emulation/qemu-q35/cpu.c @@ -0,0 +1,58 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <console/console.h> +#include <cpu/x86/mp.h> +#include <stdint.h> +#include <cpu/intel/smm_reloc.h> +#include <cpu/amd/amd64_save_state.h> +#include <mainboard/emulation/qemu-i440fx/fw_cfg.h> + +static void get_smm_info(uintptr_t *perm_smbase, size_t *perm_smsize, + size_t *smm_save_state_size) +{ + printk(BIOS_DEBUG, "Setting up SMI for CPU\n"); + + smm_subregion(SMM_SUBREGION_HANDLER, perm_smbase, perm_smsize); + + /* FIXME: on X86_64 the save state size is smaller than the size of the SMM stub */ + *smm_save_state_size = sizeof(amd64_smm_state_save_area_t); + printk(BIOS_DEBUG, "Save state size: 0x%lx bytes\n", *smm_save_state_size); +} + +/* + * The relocation work is actually performed in SMM context, but the code + * resides in the ramstage module. This occurs by trampolining from the default + * SMRAM entry point to here. + */ +static void relocation_handler(int cpu, uintptr_t curr_smbase, + uintptr_t staggered_smbase) +{ + /* The em64t101 save state is sufficiently compatible with older + save states with regards of smbase, smm_revision. */ + amd64_smm_state_save_area_t *save_state; + u32 smbase = staggered_smbase; + + save_state = (void *)(curr_smbase + SMM_DEFAULT_SIZE - sizeof(*save_state)); + save_state->smbase = smbase; + + printk(BIOS_DEBUG, "In relocation handler: cpu %d\n", cpu); + printk(BIOS_DEBUG, "SMM revision: 0x%08x\n", save_state->smm_revision); + printk(BIOS_DEBUG, "New SMBASE=0x%08x\n", smbase); +} + +static void post_mp_init(void) +{ + /* Now that all APs have been relocated as well as the BSP let SMIs start flowing. */ + global_smi_enable(); + + /* Lock down the SMRAM space. */ + smm_lock(); +} + +const struct mp_ops mp_ops_with_smm = { + .get_cpu_count = fw_cfg_max_cpus, + .get_smm_info = get_smm_info, + .pre_mp_smm_init = smm_southbridge_clear_state, + .relocation_handler = relocation_handler, + .post_mp_init = post_mp_init, +}; diff --git a/src/mainboard/emulation/qemu-q35/memmap.c b/src/mainboard/emulation/qemu-q35/memmap.c index f3ce42c..5c42921 100644 --- a/src/mainboard/emulation/qemu-q35/memmap.c +++ b/src/mainboard/emulation/qemu-q35/memmap.c @@ -8,6 +8,7 @@ #include <device/pci_ops.h> #include <mainboard/emulation/qemu-i440fx/memory.h> #include <mainboard/emulation/qemu-i440fx/fw_cfg.h> +#include <cpu/intel/smm_reloc.h>
#include "q35.h"
@@ -35,6 +36,16 @@
/* QEMU-specific register */ #define EXT_TSEG_MBYTES 0x50 +#define SMRAMC 0x9d +#define C_BASE_SEG ((0 << 2) | (1 << 1) | (0 << 0)) +#define G_SMRAME (1 << 3) +#define D_LCK (1 << 4) +#define D_CLS (1 << 5) +#define D_OPEN (1 << 6) +#define ESMRAMC 0x9e +#define T_EN (1 << 0) +#define TSEG_SZ_MASK (3 << 1) +#define H_SMRAME (1 << 7)
void smm_region(uintptr_t *start, size_t *size) { @@ -57,3 +68,16 @@ *start = qemu_get_memory_size() * KiB - *size; printk(BIOS_SPEW, "SMM_BASE: 0x%08lx, SMM_SIZE: %zu MiB\n", *start, *size / MiB); } + +void smm_lock(void) +{ + /* + * LOCK the SMM memory window and enable normal SMM. + * After running this function, only a full reset can + * make the SMM registers writable again. + */ + printk(BIOS_DEBUG, "Locking SMM.\n"); + + pci_or_config8(PCI_DEV(0, 0, 0), ESMRAMC, T_EN); + pci_write_config8(PCI_DEV(0, 0, 0), SMRAMC, D_LCK | G_SMRAME | C_BASE_SEG); +}
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48262 )
Change subject: mb/emulation/qemu-q35: Add support for SMM_TSEG with parallel MP init ......................................................................
Patch Set 11:
(1 comment)
Patchset:
PS8:
Ack
Turned out -i386 did not work while -x86_64 did. See CB:55138.
-
Angel Pons (Code Review) -
Arthur Heymans (Code Review)
-
Kyösti Mälkki (Code Review)
-
Nico Huber (Code Review)
-
Patrick Georgi (Code Review)