Change in coreboot[master]: soc/intel/common/sa: Properly guard the FSP-TOLUM cbmem_top implement...
Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36614 )
Change subject: soc/intel/common/sa: Properly guard the FSP-TOLUM cbmem_top implementation ......................................................................
soc/intel/common/sa: Properly guard the FSP-TOLUM cbmem_top implementation
Saving cbmem_top across stages is not needed so EBDA should not be used. The guard to cbmem_top_chipset implementation was inappropriate.
Change-Id: Ibbb3534b88de4f7b6fc39675a77461265605e56e Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/soc/intel/cannonlake/Kconfig M src/soc/intel/common/block/systemagent/Kconfig M src/soc/intel/common/block/systemagent/Makefile.inc A src/soc/intel/common/block/systemagent/cbmem.c M src/soc/intel/common/block/systemagent/memmap.c M src/soc/intel/icelake/Kconfig M src/soc/intel/skylake/Kconfig 7 files changed, 34 insertions(+), 30 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/36614/1
diff --git a/src/soc/intel/cannonlake/Kconfig b/src/soc/intel/cannonlake/Kconfig index 3330a69..70d2d9a 100644 --- a/src/soc/intel/cannonlake/Kconfig +++ b/src/soc/intel/cannonlake/Kconfig @@ -88,7 +88,7 @@ select SOC_INTEL_COMMON_BLOCK_CHIP_CONFIG select SOC_INTEL_COMMON_BLOCK_CPU select SOC_INTEL_COMMON_BLOCK_CPU_MPINIT - select SOC_INTEL_COMMON_BLOCK_EBDA + select SOC_INTEL_COMMON_BLOCK_SA_FSP_TOLUM select SOC_INTEL_COMMON_BLOCK_GPIO_DUAL_ROUTE_SUPPORT select SOC_INTEL_COMMON_BLOCK_GSPI_VERSION_2 select SOC_INTEL_COMMON_BLOCK_HDA diff --git a/src/soc/intel/common/block/systemagent/Kconfig b/src/soc/intel/common/block/systemagent/Kconfig index 1222573..ac2af32 100644 --- a/src/soc/intel/common/block/systemagent/Kconfig +++ b/src/soc/intel/common/block/systemagent/Kconfig @@ -3,6 +3,12 @@ help Intel Processor common System Agent support
+config SOC_INTEL_COMMON_BLOCK_SA_FSP_TOLUM + bool + help + Select this if you want cbmem_top_initialize use + the TOLUM returned by the FSP HOB. + config MMCONF_BASE_ADDRESS hex default 0xe0000000 diff --git a/src/soc/intel/common/block/systemagent/Makefile.inc b/src/soc/intel/common/block/systemagent/Makefile.inc index 7e49ec7..ecf68a7 100644 --- a/src/soc/intel/common/block/systemagent/Makefile.inc +++ b/src/soc/intel/common/block/systemagent/Makefile.inc @@ -6,3 +6,5 @@ romstage-$(CONFIG_SOC_INTEL_COMMON_BLOCK_SA) += memmap.c ramstage-$(CONFIG_SOC_INTEL_COMMON_BLOCK_SA) += memmap.c postcar-$(CONFIG_SOC_INTEL_COMMON_BLOCK_SA) += memmap.c +romstage-$(CONFIG_SOC_INTEL_COMMON_BLOCK_SA_FSP_TOLUM) += cbmem.c + diff --git a/src/soc/intel/common/block/systemagent/cbmem.c b/src/soc/intel/common/block/systemagent/cbmem.c new file mode 100644 index 0000000..2f82ac7 --- /dev/null +++ b/src/soc/intel/common/block/systemagent/cbmem.c @@ -0,0 +1,23 @@ +/* + * This file is part of the coreboot project. + * + * 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 <cbmem.h> +#include <fsp/util.h> + +void *cbmem_top_chipset(void) +{ + struct range_entry tolum; + + fsp_find_bootloader_tolum(&tolum); + return (void *)(uintptr_t)range_entry_end(&tolum); +} diff --git a/src/soc/intel/common/block/systemagent/memmap.c b/src/soc/intel/common/block/systemagent/memmap.c index 809c13a..2b0fdc4 100644 --- a/src/soc/intel/common/block/systemagent/memmap.c +++ b/src/soc/intel/common/block/systemagent/memmap.c @@ -19,8 +19,6 @@ #include <console/console.h> #include <cpu/x86/mtrr.h> #include <cpu/x86/smm.h> -#include <fsp/util.h> -#include <intelblocks/ebda.h> #include <intelblocks/systemagent.h> #include <stdlib.h>
@@ -70,31 +68,6 @@ *size = sa_get_tseg_size(); }
-#if CONFIG(SOC_INTEL_COMMON_BLOCK_EBDA) -void fill_memmap_ebda(struct ebda_config *cfg) -{ - struct range_entry tolum; - - fsp_find_bootloader_tolum(&tolum); - cfg->cbmem_top = range_entry_end(&tolum); -} - -void cbmem_top_init(void) -{ - /* Initialize EBDA area */ - initialize_ebda_area(); -} - -void *cbmem_top_chipset(void) -{ - struct ebda_config ebda_cfg; - - retrieve_ebda_object(&ebda_cfg); - - return (void *)(uintptr_t)ebda_cfg.cbmem_top; -} -#endif - void fill_postcar_frame(struct postcar_frame *pcf) { uintptr_t top_of_ram; diff --git a/src/soc/intel/icelake/Kconfig b/src/soc/intel/icelake/Kconfig index a04993d..23ec006 100644 --- a/src/soc/intel/icelake/Kconfig +++ b/src/soc/intel/icelake/Kconfig @@ -43,7 +43,7 @@ select SOC_INTEL_COMMON_BLOCK_CHIP_CONFIG select SOC_INTEL_COMMON_BLOCK_CPU select SOC_INTEL_COMMON_BLOCK_CPU_MPINIT - select SOC_INTEL_COMMON_BLOCK_EBDA + select SOC_INTEL_COMMON_BLOCK_SA_FSP_TOLUM select SOC_INTEL_COMMON_BLOCK_GSPI_VERSION_2 select SOC_INTEL_COMMON_BLOCK_HDA select SOC_INTEL_COMMON_BLOCK_SA diff --git a/src/soc/intel/skylake/Kconfig b/src/soc/intel/skylake/Kconfig index d4720a2..292963a 100644 --- a/src/soc/intel/skylake/Kconfig +++ b/src/soc/intel/skylake/Kconfig @@ -58,7 +58,7 @@ select SOC_INTEL_COMMON_BLOCK_CHIP_CONFIG select SOC_INTEL_COMMON_BLOCK_CPU select SOC_INTEL_COMMON_BLOCK_CPU_MPINIT - select SOC_INTEL_COMMON_BLOCK_EBDA + select SOC_INTEL_COMMON_BLOCK_SA_FSP_TOLUM select SOC_INTEL_COMMON_BLOCK_GPIO_DUAL_ROUTE_SUPPORT select SOC_INTEL_COMMON_BLOCK_GPIO_LEGACY_MACROS select SOC_INTEL_COMMON_BLOCK_GPIO_PADCFG_PADTOL
Hello Patrick Rudolph, Michael Niewöhner, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36614
to look at the new patch set (#2).
Change subject: soc/intel/common/sa: Properly guard the FSP-TOLUM cbmem_top implementation ......................................................................
soc/intel/common/sa: Properly guard the FSP-TOLUM cbmem_top implementation
Saving cbmem_top across stages is not needed so EBDA should not be used. The guard to cbmem_top_chipset implementation was inappropriate.
Change-Id: Ibbb3534b88de4f7b6fc39675a77461265605e56e Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/soc/intel/cannonlake/Kconfig M src/soc/intel/common/block/systemagent/Kconfig M src/soc/intel/common/block/systemagent/Makefile.inc A src/soc/intel/common/block/systemagent/cbmem.c M src/soc/intel/common/block/systemagent/memmap.c M src/soc/intel/icelake/Kconfig M src/soc/intel/skylake/Kconfig 7 files changed, 33 insertions(+), 30 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/36614/2
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36614 )
Change subject: soc/intel/common/sa: Properly guard the FSP-TOLUM cbmem_top implementation ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36614/2/src/soc/intel/common/block/... File src/soc/intel/common/block/systemagent/cbmem.c:
https://review.coreboot.org/c/coreboot/+/36614/2/src/soc/intel/common/block/... PS2, Line 22: return (void *)(uintptr_t)range_entry_end(&tolum); code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/36614/2/src/soc/intel/common/block/... PS2, Line 22: return (void *)(uintptr_t)range_entry_end(&tolum); please, no spaces at the start of a line
Hello Patrick Rudolph, Michael Niewöhner, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36614
to look at the new patch set (#3).
Change subject: soc/intel/common/sa: Properly guard the FSP-TOLUM cbmem_top implementation ......................................................................
soc/intel/common/sa: Properly guard the FSP-TOLUM cbmem_top implementation
Saving cbmem_top across stages is not needed so EBDA should not be used. The guard to cbmem_top_chipset implementation was inappropriate.
Change-Id: Ibbb3534b88de4f7b6fc39675a77461265605e56e Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/soc/intel/cannonlake/Kconfig M src/soc/intel/common/block/systemagent/Kconfig M src/soc/intel/common/block/systemagent/Makefile.inc A src/soc/intel/common/block/systemagent/cbmem.c M src/soc/intel/common/block/systemagent/memmap.c M src/soc/intel/icelake/Kconfig M src/soc/intel/skylake/Kconfig 7 files changed, 33 insertions(+), 30 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/36614/3
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36614 )
Change subject: soc/intel/common/sa: Properly guard the FSP-TOLUM cbmem_top implementation ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36614/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36614/3//COMMIT_MSG@9 PS3, Line 9: not needed not needed anymore
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36614 )
Change subject: soc/intel/common/sa: Properly guard the FSP-TOLUM cbmem_top implementation ......................................................................
Patch Set 3: Code-Review+1
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36614 )
Change subject: soc/intel/common/sa: Properly guard the FSP-TOLUM cbmem_top implementation ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36614/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36614/3//COMMIT_MSG@7 PS3, Line 7: soc/intel/common/sa: Properly guard the FSP-TOLUM cbmem_top implementation It's too long, drop "the" and "implementation"?
Though, actually, this is more about getting rid of EBDA, so maybe something like "Implement cbmem_top_chipset() without EBDA"?
https://review.coreboot.org/c/coreboot/+/36614/3/src/soc/intel/common/block/... File src/soc/intel/common/block/systemagent/Kconfig:
https://review.coreboot.org/c/coreboot/+/36614/3/src/soc/intel/common/block/... PS3, Line 9: initialize chipset?
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36614 )
Change subject: soc/intel/common/sa: Properly guard the FSP-TOLUM cbmem_top implementation ......................................................................
Patch Set 3: Code-Review+2
+2 with nicos suggestions applied
Hello Patrick Rudolph, Michael Niewöhner, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36614
to look at the new patch set (#4).
Change subject: soc/intel/common/sa: Remove EBDA dependency ......................................................................
soc/intel/common/sa: Remove EBDA dependency
Saving cbmem_top across stages is not needed anymore so EBDA should not be used. The guard to cbmem_top_chipset implementation was inappropriate.
Change-Id: Ibbb3534b88de4f7b6fc39675a77461265605e56e Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/soc/intel/cannonlake/Kconfig M src/soc/intel/common/block/systemagent/Kconfig M src/soc/intel/common/block/systemagent/Makefile.inc A src/soc/intel/common/block/systemagent/cbmem.c M src/soc/intel/common/block/systemagent/memmap.c M src/soc/intel/icelake/Kconfig M src/soc/intel/skylake/Kconfig 7 files changed, 33 insertions(+), 30 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/36614/4
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36614 )
Change subject: soc/intel/common/sa: Remove EBDA dependency ......................................................................
Patch Set 4: Code-Review+2
(3 comments)
https://review.coreboot.org/c/coreboot/+/36614/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36614/3//COMMIT_MSG@7 PS3, Line 7: soc/intel/common/sa: Properly guard the FSP-TOLUM cbmem_top implementation
It's too long, drop "the" and "implementation"? […]
Done
https://review.coreboot.org/c/coreboot/+/36614/3//COMMIT_MSG@9 PS3, Line 9: not needed
not needed anymore
Done
https://review.coreboot.org/c/coreboot/+/36614/3/src/soc/intel/common/block/... File src/soc/intel/common/block/systemagent/Kconfig:
https://review.coreboot.org/c/coreboot/+/36614/3/src/soc/intel/common/block/... PS3, Line 9: initialize
chipset?
Done
Arthur Heymans has submitted this change. ( https://review.coreboot.org/c/coreboot/+/36614 )
Change subject: soc/intel/common/sa: Remove EBDA dependency ......................................................................
soc/intel/common/sa: Remove EBDA dependency
Saving cbmem_top across stages is not needed anymore so EBDA should not be used. The guard to cbmem_top_chipset implementation was inappropriate.
Change-Id: Ibbb3534b88de4f7b6fc39675a77461265605e56e Signed-off-by: Arthur Heymans arthur@aheymans.xyz Reviewed-on: https://review.coreboot.org/c/coreboot/+/36614 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Michael Niewöhner --- M src/soc/intel/cannonlake/Kconfig M src/soc/intel/common/block/systemagent/Kconfig M src/soc/intel/common/block/systemagent/Makefile.inc A src/soc/intel/common/block/systemagent/cbmem.c M src/soc/intel/common/block/systemagent/memmap.c M src/soc/intel/icelake/Kconfig M src/soc/intel/skylake/Kconfig 7 files changed, 33 insertions(+), 30 deletions(-)
Approvals: build bot (Jenkins): Verified Michael Niewöhner: Looks good to me, approved
diff --git a/src/soc/intel/cannonlake/Kconfig b/src/soc/intel/cannonlake/Kconfig index 3330a69..70d2d9a 100644 --- a/src/soc/intel/cannonlake/Kconfig +++ b/src/soc/intel/cannonlake/Kconfig @@ -88,7 +88,7 @@ select SOC_INTEL_COMMON_BLOCK_CHIP_CONFIG select SOC_INTEL_COMMON_BLOCK_CPU select SOC_INTEL_COMMON_BLOCK_CPU_MPINIT - select SOC_INTEL_COMMON_BLOCK_EBDA + select SOC_INTEL_COMMON_BLOCK_SA_FSP_TOLUM select SOC_INTEL_COMMON_BLOCK_GPIO_DUAL_ROUTE_SUPPORT select SOC_INTEL_COMMON_BLOCK_GSPI_VERSION_2 select SOC_INTEL_COMMON_BLOCK_HDA diff --git a/src/soc/intel/common/block/systemagent/Kconfig b/src/soc/intel/common/block/systemagent/Kconfig index 1222573..4c50d50 100644 --- a/src/soc/intel/common/block/systemagent/Kconfig +++ b/src/soc/intel/common/block/systemagent/Kconfig @@ -3,6 +3,12 @@ help Intel Processor common System Agent support
+config SOC_INTEL_COMMON_BLOCK_SA_FSP_TOLUM + bool + help + Select this if you want cbmem_top_chipset use the TOLUM returned + by the FSP HOB. + config MMCONF_BASE_ADDRESS hex default 0xe0000000 diff --git a/src/soc/intel/common/block/systemagent/Makefile.inc b/src/soc/intel/common/block/systemagent/Makefile.inc index 7e49ec7..1cced4a 100644 --- a/src/soc/intel/common/block/systemagent/Makefile.inc +++ b/src/soc/intel/common/block/systemagent/Makefile.inc @@ -6,3 +6,4 @@ romstage-$(CONFIG_SOC_INTEL_COMMON_BLOCK_SA) += memmap.c ramstage-$(CONFIG_SOC_INTEL_COMMON_BLOCK_SA) += memmap.c postcar-$(CONFIG_SOC_INTEL_COMMON_BLOCK_SA) += memmap.c +romstage-$(CONFIG_SOC_INTEL_COMMON_BLOCK_SA_FSP_TOLUM) += cbmem.c diff --git a/src/soc/intel/common/block/systemagent/cbmem.c b/src/soc/intel/common/block/systemagent/cbmem.c new file mode 100644 index 0000000..7e743df --- /dev/null +++ b/src/soc/intel/common/block/systemagent/cbmem.c @@ -0,0 +1,23 @@ +/* + * This file is part of the coreboot project. + * + * 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 <cbmem.h> +#include <fsp/util.h> + +void *cbmem_top_chipset(void) +{ + struct range_entry tolum; + + fsp_find_bootloader_tolum(&tolum); + return (void *)(uintptr_t)range_entry_end(&tolum); +} diff --git a/src/soc/intel/common/block/systemagent/memmap.c b/src/soc/intel/common/block/systemagent/memmap.c index 809c13a..2b0fdc4 100644 --- a/src/soc/intel/common/block/systemagent/memmap.c +++ b/src/soc/intel/common/block/systemagent/memmap.c @@ -19,8 +19,6 @@ #include <console/console.h> #include <cpu/x86/mtrr.h> #include <cpu/x86/smm.h> -#include <fsp/util.h> -#include <intelblocks/ebda.h> #include <intelblocks/systemagent.h> #include <stdlib.h>
@@ -70,31 +68,6 @@ *size = sa_get_tseg_size(); }
-#if CONFIG(SOC_INTEL_COMMON_BLOCK_EBDA) -void fill_memmap_ebda(struct ebda_config *cfg) -{ - struct range_entry tolum; - - fsp_find_bootloader_tolum(&tolum); - cfg->cbmem_top = range_entry_end(&tolum); -} - -void cbmem_top_init(void) -{ - /* Initialize EBDA area */ - initialize_ebda_area(); -} - -void *cbmem_top_chipset(void) -{ - struct ebda_config ebda_cfg; - - retrieve_ebda_object(&ebda_cfg); - - return (void *)(uintptr_t)ebda_cfg.cbmem_top; -} -#endif - void fill_postcar_frame(struct postcar_frame *pcf) { uintptr_t top_of_ram; diff --git a/src/soc/intel/icelake/Kconfig b/src/soc/intel/icelake/Kconfig index 71c7f83..0404af5 100644 --- a/src/soc/intel/icelake/Kconfig +++ b/src/soc/intel/icelake/Kconfig @@ -43,7 +43,7 @@ select SOC_INTEL_COMMON_BLOCK_CHIP_CONFIG select SOC_INTEL_COMMON_BLOCK_CPU select SOC_INTEL_COMMON_BLOCK_CPU_MPINIT - select SOC_INTEL_COMMON_BLOCK_EBDA + select SOC_INTEL_COMMON_BLOCK_SA_FSP_TOLUM select SOC_INTEL_COMMON_BLOCK_GSPI_VERSION_2 select SOC_INTEL_COMMON_BLOCK_HDA select SOC_INTEL_COMMON_BLOCK_SA diff --git a/src/soc/intel/skylake/Kconfig b/src/soc/intel/skylake/Kconfig index d4720a2..292963a 100644 --- a/src/soc/intel/skylake/Kconfig +++ b/src/soc/intel/skylake/Kconfig @@ -58,7 +58,7 @@ select SOC_INTEL_COMMON_BLOCK_CHIP_CONFIG select SOC_INTEL_COMMON_BLOCK_CPU select SOC_INTEL_COMMON_BLOCK_CPU_MPINIT - select SOC_INTEL_COMMON_BLOCK_EBDA + select SOC_INTEL_COMMON_BLOCK_SA_FSP_TOLUM select SOC_INTEL_COMMON_BLOCK_GPIO_DUAL_ROUTE_SUPPORT select SOC_INTEL_COMMON_BLOCK_GPIO_LEGACY_MACROS select SOC_INTEL_COMMON_BLOCK_GPIO_PADCFG_PADTOL
-
Arthur Heymans (Code Review) -
build bot (Jenkins) (Code Review)
-
Michael Niewöhner (Code Review)
-
Nico Huber (Code Review)