Kyösti Mälkki has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34776 )
Change subject: cpu/x86/smm: Define single smm_subregion() ......................................................................
cpu/x86/smm: Define single smm_subregion()
At the moment we only have two splitting of TSEG, one with and one without IED. They can all use same implementation.
Make configuration problems of TSEG region assertion failures.
Rename file from stage_cache.c to tseg_region.c to reflect it's purpose.
Change-Id: I9daf0dec8fbaaa1f4e6004ea034869f43412d7d5 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/34776 Reviewed-by: Furquan Shaikh furquan@google.com Reviewed-by: David Guckian Reviewed-by: Aaron Durbin adurbin@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/cpu/x86/smm/Makefile.inc D src/cpu/x86/smm/stage_cache.c A src/cpu/x86/smm/tseg_region.c M src/soc/amd/picasso/ramtop.c M src/soc/amd/stoneyridge/ramtop.c M src/soc/intel/apollolake/memmap.c M src/soc/intel/braswell/memmap.c M src/soc/intel/cannonlake/memmap.c M src/soc/intel/denverton_ns/memmap.c M src/soc/intel/icelake/memmap.c M src/soc/intel/skylake/memmap.c 11 files changed, 95 insertions(+), 339 deletions(-)
Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved Furquan Shaikh: Looks good to me, approved David Guckian: Looks good to me, but someone else must approve
diff --git a/src/cpu/x86/smm/Makefile.inc b/src/cpu/x86/smm/Makefile.inc index fe149f1..2bb4fd7 100644 --- a/src/cpu/x86/smm/Makefile.inc +++ b/src/cpu/x86/smm/Makefile.inc @@ -42,9 +42,9 @@
ifeq ($(CONFIG_SMM_TSEG),y)
-ramstage-y += stage_cache.c -romstage-y += stage_cache.c -postcar-y += stage_cache.c +ramstage-y += tseg_region.c +romstage-y += tseg_region.c +postcar-y += tseg_region.c
smmstub-y += smm_stub.S
diff --git a/src/cpu/x86/smm/stage_cache.c b/src/cpu/x86/smm/stage_cache.c deleted file mode 100644 index 0a816ba..0000000 --- a/src/cpu/x86/smm/stage_cache.c +++ /dev/null @@ -1,33 +0,0 @@ -/* - * This file is part of the coreboot project. - * - * Copyright 2013 Google Inc. - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; version 2 of the License. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - */ - -#include <console/console.h> -#include <cpu/x86/smm.h> -#include <stage_cache.h> -#include <types.h> - -int __weak smm_subregion(int sub, uintptr_t *base, size_t *size) -{ - return -1; -} - -void __weak stage_cache_external_region(void **base, size_t *size) -{ - if (smm_subregion(SMM_SUBREGION_CACHE, (uintptr_t *)base, size)) { - printk(BIOS_ERR, "ERROR: No cache SMM subregion.\n"); - *base = NULL; - *size = 0; - } -} diff --git a/src/cpu/x86/smm/tseg_region.c b/src/cpu/x86/smm/tseg_region.c new file mode 100644 index 0000000..df9dea5 --- /dev/null +++ b/src/cpu/x86/smm/tseg_region.c @@ -0,0 +1,86 @@ +/* + * This file is part of the coreboot project. + * + * Copyright 2013 Google Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <assert.h> +#include <commonlib/helpers.h> +#include <console/console.h> +#include <cpu/x86/smm.h> +#include <stage_cache.h> +#include <types.h> + +void __weak smm_region(uintptr_t *start, size_t *size) +{ + *start = 0; + *size = 0; +} + +/* + * Subregions within SMM + * +-------------------------+ + * | IED | IED_REGION_SIZE + * +-------------------------+ + * | External Stage Cache | SMM_RESERVED_SIZE + * +-------------------------+ + * | code and data | + * | (TSEG) | + * +-------------------------+ TSEG + */ +int smm_subregion(int sub, uintptr_t *start, size_t *size) +{ + uintptr_t sub_base; + size_t sub_size; + const size_t ied_size = CONFIG_IED_REGION_SIZE; + const size_t cache_size = CONFIG_SMM_RESERVED_SIZE; + + smm_region(&sub_base, &sub_size); + + ASSERT(IS_ALIGNED(sub_base, sub_size)); + ASSERT(sub_size > (cache_size + ied_size)); + + switch (sub) { + case SMM_SUBREGION_HANDLER: + /* Handler starts at the base of TSEG. */ + sub_size -= ied_size; + sub_size -= cache_size; + break; + case SMM_SUBREGION_CACHE: + /* External cache is in the middle of TSEG. */ + sub_base += sub_size - (ied_size + cache_size); + sub_size = cache_size; + break; + case SMM_SUBREGION_CHIPSET: + /* IED is at the top. */ + sub_base += sub_size - ied_size; + sub_size = ied_size; + break; + default: + *start = 0; + *size = 0; + return -1; + } + + *start = sub_base; + *size = sub_size; + return 0; +} + +void __weak stage_cache_external_region(void **base, size_t *size) +{ + if (smm_subregion(SMM_SUBREGION_CACHE, (uintptr_t *)base, size)) { + printk(BIOS_ERR, "ERROR: No cache SMM subregion.\n"); + *base = NULL; + *size = 0; + } +} diff --git a/src/soc/amd/picasso/ramtop.c b/src/soc/amd/picasso/ramtop.c index 672fdd8..09af7e4 100644 --- a/src/soc/amd/picasso/ramtop.c +++ b/src/soc/amd/picasso/ramtop.c @@ -81,12 +81,6 @@ return CONFIG_SMM_TSEG_SIZE; }
-void smm_region(uintptr_t *start, size_t *size) -{ - *start = smm_region_start(); - *size = smm_region_size(); -} - /* * For data stored in TSEG, ensure TValid is clear so R/W access can reach * the DRAM when not in SMM. @@ -109,39 +103,15 @@ wrmsr(SMM_MASK_MSR, mask); }
-int smm_subregion(int sub, uintptr_t *start, size_t *size) +void smm_region(uintptr_t *start, size_t *size) { static int once; - uintptr_t sub_base; - size_t sub_size; - const size_t cache_size = CONFIG_SMM_RESERVED_SIZE;
- smm_region(&sub_base, &sub_size); - assert(sub_size > CONFIG_SMM_RESERVED_SIZE); + *start = smm_region_start(); + *size = smm_region_size();
if (!once) { clear_tvalid(); once = 1; } - - switch (sub) { - case SMM_SUBREGION_HANDLER: - /* Handler starts at the base of TSEG. */ - sub_size -= cache_size; - break; - case SMM_SUBREGION_CACHE: - /* External cache is in the middle of TSEG. */ - sub_base += sub_size - cache_size; - sub_size = cache_size; - break; - default: - *start = 0; - *size = 0; - return -1; - } - - *start = (void *)sub_base; - *size = sub_size; - - return 0; } diff --git a/src/soc/amd/stoneyridge/ramtop.c b/src/soc/amd/stoneyridge/ramtop.c index e2b0f7d..09af7e4 100644 --- a/src/soc/amd/stoneyridge/ramtop.c +++ b/src/soc/amd/stoneyridge/ramtop.c @@ -81,12 +81,6 @@ return CONFIG_SMM_TSEG_SIZE; }
-void smm_region(uintptr_t *start, size_t *size) -{ - *start = smm_region_start(); - *size = smm_region_size(); -} - /* * For data stored in TSEG, ensure TValid is clear so R/W access can reach * the DRAM when not in SMM. @@ -109,38 +103,15 @@ wrmsr(SMM_MASK_MSR, mask); }
-int smm_subregion(int sub, uintptr_t *start, size_t *size) +void smm_region(uintptr_t *start, size_t *size) { static int once; - uintptr_t sub_base; - size_t sub_size; - const size_t cache_size = CONFIG_SMM_RESERVED_SIZE;
- smm_region(&sub_base, &sub_size); - assert(sub_size > CONFIG_SMM_RESERVED_SIZE); + *start = smm_region_start(); + *size = smm_region_size();
if (!once) { clear_tvalid(); once = 1; } - - switch (sub) { - case SMM_SUBREGION_HANDLER: - /* Handler starts at the base of TSEG. */ - sub_size -= cache_size; - break; - case SMM_SUBREGION_CACHE: - /* External cache is in the middle of TSEG. */ - sub_base += sub_size - cache_size; - sub_size = cache_size; - break; - default: - *start = 0; - *size = 0; - return -1; - } - - *start = sub_base; - *size = sub_size; - return 0; } diff --git a/src/soc/intel/apollolake/memmap.c b/src/soc/intel/apollolake/memmap.c index 17dfb3f..0eb66dc 100644 --- a/src/soc/intel/apollolake/memmap.c +++ b/src/soc/intel/apollolake/memmap.c @@ -48,32 +48,3 @@ *start = sa_get_tseg_base(); *size = sa_get_tseg_size(); } - -int smm_subregion(int sub, uintptr_t *start, size_t *size) -{ - uintptr_t sub_base; - size_t sub_size; - const size_t cache_size = CONFIG_SMM_RESERVED_SIZE; - - smm_region(&sub_base, &sub_size); - - switch (sub) { - case SMM_SUBREGION_HANDLER: - /* Handler starts at the base of TSEG. */ - sub_size -= cache_size; - break; - case SMM_SUBREGION_CACHE: - /* External cache is in the middle of TSEG. */ - sub_base += sub_size - cache_size; - sub_size = cache_size; - break; - default: - *start = 0; - *size = 0; - return -1; - } - - *start = sub_base; - *size = sub_size; - return 0; -} diff --git a/src/soc/intel/braswell/memmap.c b/src/soc/intel/braswell/memmap.c index 51b7b36..b4c69a4 100644 --- a/src/soc/intel/braswell/memmap.c +++ b/src/soc/intel/braswell/memmap.c @@ -34,44 +34,6 @@ *size = smm_region_size(); }
-/* - * Subregions within SMM - * +-------------------------+ BUNIT_SMRRH - * | External Stage Cache | SMM_RESERVED_SIZE - * +-------------------------+ - * | code and data | - * | (TSEG) | - * +-------------------------+ BUNIT_SMRRL - */ -int smm_subregion(int sub, uintptr_t *start, size_t *size) -{ - uintptr_t sub_base; - size_t sub_size; - const size_t cache_size = CONFIG_SMM_RESERVED_SIZE; - - smm_region(&sub_base, &sub_size); - - switch (sub) { - case SMM_SUBREGION_HANDLER: - /* Handler starts at the base of TSEG. */ - sub_size -= cache_size; - break; - case SMM_SUBREGION_CACHE: - /* External cache is in the middle of TSEG. */ - sub_base += sub_size - cache_size; - sub_size = cache_size; - break; - default: - *start = 0; - *size = 0; - return -1; - } - - *start = sub_base; - *size = sub_size; - return 0; -} - void *cbmem_top(void) { uintptr_t smm_base; diff --git a/src/soc/intel/cannonlake/memmap.c b/src/soc/intel/cannonlake/memmap.c index 004e35c..a842ff6 100644 --- a/src/soc/intel/cannonlake/memmap.c +++ b/src/soc/intel/cannonlake/memmap.c @@ -35,53 +35,6 @@ *size = sa_get_tseg_size(); }
-/* - * Subregions within SMM - * +-------------------------+ BGSM - * | IED | IED_REGION_SIZE - * +-------------------------+ - * | External Stage Cache | SMM_RESERVED_SIZE - * +-------------------------+ - * | code and data | - * | (TSEG) | - * +-------------------------+ TSEG - */ -int smm_subregion(int sub, uintptr_t *start, size_t *size) -{ - uintptr_t sub_base; - size_t sub_size; - const size_t ied_size = CONFIG_IED_REGION_SIZE; - const size_t cache_size = CONFIG_SMM_RESERVED_SIZE; - - smm_region(&sub_base, &sub_size); - - switch (sub) { - case SMM_SUBREGION_HANDLER: - /* Handler starts at the base of TSEG. */ - sub_size -= ied_size; - sub_size -= cache_size; - break; - case SMM_SUBREGION_CACHE: - /* External cache is in the middle of TSEG. */ - sub_base += sub_size - (ied_size + cache_size); - sub_size = cache_size; - break; - case SMM_SUBREGION_CHIPSET: - /* IED is at the top. */ - sub_base += sub_size - ied_size; - sub_size = ied_size; - break; - default: - *start = 0; - *size = 0; - return -1; - } - - *start = sub_base; - *size = sub_size; - return 0; -} - /* Calculate ME Stolen size */ static size_t get_imr_size(void) { diff --git a/src/soc/intel/denverton_ns/memmap.c b/src/soc/intel/denverton_ns/memmap.c index 9507d7f..0cca4b9 100644 --- a/src/soc/intel/denverton_ns/memmap.c +++ b/src/soc/intel/denverton_ns/memmap.c @@ -75,33 +75,3 @@ *start = smm_region_start(); *size = smm_region_size(); } - -int smm_subregion(int sub, uintptr_t *start, size_t *size) -{ - uintptr_t sub_base; - size_t sub_size; - const size_t cache_size = CONFIG_SMM_RESERVED_SIZE; - - smm_region(&sub_base, &sub_size); - assert(sub_size > CONFIG_SMM_RESERVED_SIZE); - - switch (sub) { - case SMM_SUBREGION_HANDLER: - /* Handler starts at the base of TSEG. */ - sub_size -= cache_size; - break; - case SMM_SUBREGION_CACHE: - /* External cache is in the middle of TSEG. */ - sub_base += sub_size - cache_size; - sub_size = cache_size; - break; - default: - *start = 0; - *size = 0; - return -1; - } - - *start = sub_base; - *size = sub_size; - return 0; -} diff --git a/src/soc/intel/icelake/memmap.c b/src/soc/intel/icelake/memmap.c index 13eb947..e0d7d68 100644 --- a/src/soc/intel/icelake/memmap.c +++ b/src/soc/intel/icelake/memmap.c @@ -33,53 +33,6 @@ *size = sa_get_tseg_size(); }
-/* - * Subregions within SMM - * +-------------------------+ BGSM - * | IED | IED_REGION_SIZE - * +-------------------------+ - * | External Stage Cache | SMM_RESERVED_SIZE - * +-------------------------+ - * | code and data | - * | (TSEG) | - * +-------------------------+ TSEG - */ -int smm_subregion(int sub, uintptr_t *start, size_t *size) -{ - uintptr_t sub_base; - size_t sub_size; - const size_t ied_size = CONFIG_IED_REGION_SIZE; - const size_t cache_size = CONFIG_SMM_RESERVED_SIZE; - - smm_region(&sub_base, &sub_size); - - switch (sub) { - case SMM_SUBREGION_HANDLER: - /* Handler starts at the base of TSEG. */ - sub_size -= ied_size; - sub_size -= cache_size; - break; - case SMM_SUBREGION_CACHE: - /* External cache is in the middle of TSEG. */ - sub_base += sub_size - (ied_size + cache_size); - sub_size = cache_size; - break; - case SMM_SUBREGION_CHIPSET: - /* IED is at the top. */ - sub_base += sub_size - ied_size; - sub_size = ied_size; - break; - default: - *start = 0; - *size = 0; - return -1; - } - - *start = sub_base; - *size = sub_size; - return 0; -} - /* Calculate ME Stolen size */ static size_t get_imr_size(void) { diff --git a/src/soc/intel/skylake/memmap.c b/src/soc/intel/skylake/memmap.c index 963a500..e4e2516 100644 --- a/src/soc/intel/skylake/memmap.c +++ b/src/soc/intel/skylake/memmap.c @@ -37,53 +37,6 @@ *size = sa_get_tseg_size(); }
-/* - * Subregions within SMM - * +-------------------------+ BGSM - * | IED | IED_REGION_SIZE - * +-------------------------+ - * | External Stage Cache | SMM_RESERVED_SIZE - * +-------------------------+ - * | code and data | - * | (TSEG) | - * +-------------------------+ TSEG - */ -int smm_subregion(int sub, uintptr_t *start, size_t *size) -{ - uintptr_t sub_base; - size_t sub_size; - const size_t ied_size = CONFIG_IED_REGION_SIZE; - const size_t cache_size = CONFIG_SMM_RESERVED_SIZE; - - smm_region(&sub_base, &sub_size); - - switch (sub) { - case SMM_SUBREGION_HANDLER: - /* Handler starts at the base of TSEG. */ - sub_size -= ied_size; - sub_size -= cache_size; - break; - case SMM_SUBREGION_CACHE: - /* External cache is in the middle of TSEG. */ - sub_base += sub_size - (ied_size + cache_size); - sub_size = cache_size; - break; - case SMM_SUBREGION_CHIPSET: - /* IED is at the top. */ - sub_base += sub_size - ied_size; - sub_size = ied_size; - break; - default: - *start = 0; - *size = 0; - return -1; - } - - *start = sub_base; - *size = sub_size; - return 0; -} - static bool is_ptt_enable(void) { if ((read32((void *)PTT_TXT_BASE_ADDRESS) & PTT_PRESENT) ==
-
Kyösti Mälkki (Code Review)