Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37288 )
Change subject: cpu/x86/smm: Add smm_size to relocatable smmstub ......................................................................
cpu/x86/smm: Add smm_size to relocatable smmstub
To mitigate against sinkhole in software which is required on pre-sandybridge hardware, the smm entry point needs to check if the LAPIC base is between smbase and smbase + smmsize. The size needs to be available early so add them to the relocatable module parameters.
When the smmstub is used to relocate SMM the default SMM size 0x10000 is provided. On the permanent handler the size provided by get_smm_info() is used.
Change-Id: I0df6e51bcba284350f1c849ef3d012860757544b Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/cpu/x86/smm/smm_module_loader.c M src/cpu/x86/smm/smm_stub.S M src/include/cpu/x86/smm.h 3 files changed, 10 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/88/37288/1
diff --git a/src/cpu/x86/smm/smm_module_loader.c b/src/cpu/x86/smm/smm_module_loader.c index c6c6b38..4431a44 100644 --- a/src/cpu/x86/smm/smm_module_loader.c +++ b/src/cpu/x86/smm/smm_module_loader.c @@ -173,8 +173,9 @@ * concurrent areas requested. The save state always lives at the top of SMRAM * space, and the entry point is at offset 0x8000. */ -static int smm_module_setup_stub(void *smbase, struct smm_loader_params *params, - void *fxsave_area) +static int smm_module_setup_stub(void *smbase, size_t smm_size, + struct smm_loader_params *params, + void *fxsave_area) { size_t total_save_state_size; size_t smm_stub_size; @@ -266,6 +267,7 @@ stub_params->fxsave_area = (uintptr_t)fxsave_area; stub_params->fxsave_area_size = FXSAVE_SIZE; stub_params->runtime.smbase = (uintptr_t)smbase; + stub_params->runtime.smm_size = smm_size; stub_params->runtime.save_state_size = params->per_cpu_save_state_size;
/* Initialize the APIC id to CPU number table to be 1:1 */ @@ -305,7 +307,8 @@ if (params->num_concurrent_stacks == 0) params->num_concurrent_stacks = CONFIG_MAX_CPUS;
- return smm_module_setup_stub(smram, params, fxsave_area_relocation); + return smm_module_setup_stub(smram, SMM_DEFAULT_SIZE, + params, fxsave_area_relocation); }
/* The SMM module is placed within the provided region in the following @@ -391,5 +394,5 @@ params->handler = rmodule_entry(&smm_mod); params->handler_arg = rmodule_parameters(&smm_mod);
- return smm_module_setup_stub(smram, params, fxsave_area); + return smm_module_setup_stub(smram, size, params, fxsave_area); } diff --git a/src/cpu/x86/smm/smm_stub.S b/src/cpu/x86/smm/smm_stub.S index f0e55f9..304ea4b 100644 --- a/src/cpu/x86/smm/smm_stub.S +++ b/src/cpu/x86/smm/smm_stub.S @@ -42,6 +42,8 @@ smm_runtime: smbase: .long 0 +smm_size: +.long 0 save_state_size: .long 0 /* apic_to_cpu_num is a table mapping the default APIC id to CPU num. If the diff --git a/src/include/cpu/x86/smm.h b/src/include/cpu/x86/smm.h index cf107b1..2e3c639 100644 --- a/src/include/cpu/x86/smm.h +++ b/src/include/cpu/x86/smm.h @@ -63,6 +63,7 @@
struct smm_runtime { u32 smbase; + u32 smm_size; u32 save_state_size; /* 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
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37288 )
Change subject: cpu/x86/smm: Add smm_size to relocatable smmstub ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37288/1/src/include/cpu/x86/smm.h File src/include/cpu/x86/smm.h:
https://review.coreboot.org/c/coreboot/+/37288/1/src/include/cpu/x86/smm.h@6... PS1, Line 66: u32 size_t ?
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37288 )
Change subject: cpu/x86/smm: Add smm_size to relocatable smmstub ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37288/1/src/include/cpu/x86/smm.h File src/include/cpu/x86/smm.h:
https://review.coreboot.org/c/coreboot/+/37288/1/src/include/cpu/x86/smm.h@6... PS1, Line 66: u32
size_t ?
Has to match the relocatable parameters of the stub, so no.
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37288 )
Change subject: cpu/x86/smm: Add smm_size to relocatable smmstub ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37288/1/src/include/cpu/x86/smm.h File src/include/cpu/x86/smm.h:
https://review.coreboot.org/c/coreboot/+/37288/1/src/include/cpu/x86/smm.h@6... PS1, Line 66: u32
size_t ? […]
Thx
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37288 )
Change subject: cpu/x86/smm: Add smm_size to relocatable smmstub ......................................................................
Patch Set 1:
As far as I understand you only need to check that LAPIC doesn't overlap with the GDT in SMM and that's always near the start in assembly code.
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37288 )
Change subject: cpu/x86/smm: Add smm_size to relocatable smmstub ......................................................................
Patch Set 1: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37288 )
Change subject: cpu/x86/smm: Add smm_size to relocatable smmstub ......................................................................
cpu/x86/smm: Add smm_size to relocatable smmstub
To mitigate against sinkhole in software which is required on pre-sandybridge hardware, the smm entry point needs to check if the LAPIC base is between smbase and smbase + smmsize. The size needs to be available early so add them to the relocatable module parameters.
When the smmstub is used to relocate SMM the default SMM size 0x10000 is provided. On the permanent handler the size provided by get_smm_info() is used.
Change-Id: I0df6e51bcba284350f1c849ef3d012860757544b Signed-off-by: Arthur Heymans arthur@aheymans.xyz Reviewed-on: https://review.coreboot.org/c/coreboot/+/37288 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Patrick Georgi pgeorgi@google.com --- M src/cpu/x86/smm/smm_module_loader.c M src/cpu/x86/smm/smm_stub.S M src/include/cpu/x86/smm.h 3 files changed, 10 insertions(+), 4 deletions(-)
Approvals: build bot (Jenkins): Verified Patrick Georgi: Looks good to me, approved
diff --git a/src/cpu/x86/smm/smm_module_loader.c b/src/cpu/x86/smm/smm_module_loader.c index 856ca78..30f115f 100644 --- a/src/cpu/x86/smm/smm_module_loader.c +++ b/src/cpu/x86/smm/smm_module_loader.c @@ -174,8 +174,9 @@ * concurrent areas requested. The save state always lives at the top of SMRAM * space, and the entry point is at offset 0x8000. */ -static int smm_module_setup_stub(void *smbase, struct smm_loader_params *params, - void *fxsave_area) +static int smm_module_setup_stub(void *smbase, size_t smm_size, + struct smm_loader_params *params, + void *fxsave_area) { size_t total_save_state_size; size_t smm_stub_size; @@ -269,6 +270,7 @@ stub_params->fxsave_area = (uintptr_t)fxsave_area; stub_params->fxsave_area_size = FXSAVE_SIZE; stub_params->runtime.smbase = (uintptr_t)smbase; + stub_params->runtime.smm_size = smm_size; stub_params->runtime.save_state_size = params->per_cpu_save_state_size; stub_params->runtime.num_cpus = params->num_concurrent_stacks;
@@ -309,7 +311,8 @@ if (params->num_concurrent_stacks == 0) params->num_concurrent_stacks = CONFIG_MAX_CPUS;
- return smm_module_setup_stub(smram, params, fxsave_area_relocation); + return smm_module_setup_stub(smram, SMM_DEFAULT_SIZE, + params, fxsave_area_relocation); }
/* The SMM module is placed within the provided region in the following @@ -409,5 +412,5 @@ params->handler = rmodule_entry(&smm_mod); params->handler_arg = rmodule_parameters(&smm_mod);
- return smm_module_setup_stub(smram, params, fxsave_area); + return smm_module_setup_stub(smram, size, params, fxsave_area); } diff --git a/src/cpu/x86/smm/smm_stub.S b/src/cpu/x86/smm/smm_stub.S index 8207d23..aa40223 100644 --- a/src/cpu/x86/smm/smm_stub.S +++ b/src/cpu/x86/smm/smm_stub.S @@ -42,6 +42,8 @@ smm_runtime: smbase: .long 0 +smm_size: +.long 0 save_state_size: .long 0 num_cpus: diff --git a/src/include/cpu/x86/smm.h b/src/include/cpu/x86/smm.h index 9efe2e0..26496ee 100644 --- a/src/include/cpu/x86/smm.h +++ b/src/include/cpu/x86/smm.h @@ -63,6 +63,7 @@
struct smm_runtime { u32 smbase; + u32 smm_size; u32 save_state_size; u32 num_cpus; /* STM's 32bit entry into SMI handler */