Michael Niewöhner has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM and reserved memory returned by FSP ......................................................................
soc/intel: skl,cnl,icl: rely on TOLUM and reserved memory returned by FSP
Instead of doing our own calculations, rely on the TOLUM and reserved memory size returned by FSP. This (hopefully) saves us from making mistakes in calculations of offsets and aligments.
Further this makes it easier to implement e.g. SGX PRMRR size selection via Kconfig as we do not have to know anything and do not have to make any assumptions about aligning but simply pass (valid) values to FSP.
Change-Id: If66a00d1320917bc68afb32c19db0e24c6732812 Signed-off-by: Michael Niewöhner foss@mniewoehner.de --- M src/soc/intel/cannonlake/memmap.c M src/soc/intel/icelake/memmap.c M src/soc/intel/skylake/include/soc/ebda.h M src/soc/intel/skylake/memmap.c 4 files changed, 43 insertions(+), 533 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/36136/1
diff --git a/src/soc/intel/cannonlake/memmap.c b/src/soc/intel/cannonlake/memmap.c index f3286cc..b2fcad0 100644 --- a/src/soc/intel/cannonlake/memmap.c +++ b/src/soc/intel/cannonlake/memmap.c @@ -28,8 +28,7 @@ #include <soc/pci_devs.h> #include <soc/systemagent.h> #include <stdlib.h> - -#include "chip.h" +#include <fsp/util.h>
void smm_region(uintptr_t *start, size_t *size) { @@ -37,161 +36,6 @@ *size = sa_get_tseg_size(); }
-/* Calculate ME Stolen size */ -static size_t get_imr_size(void) -{ - size_t imr_size; - - /* ME stolen memory */ - imr_size = MCHBAR32(IMRLIMIT) - MCHBAR32(IMRBASE); - - return imr_size; -} - -/* Calculate PRMRR size based on user input PRMRR size and alignment */ -static size_t get_prmrr_size(uintptr_t dram_base, - const struct soc_intel_cannonlake_config *config) -{ - uintptr_t prmrr_base = dram_base; - size_t prmrr_size; - - prmrr_size = config->PrmrrSize; - - /* Allocate PRMRR memory for C6DRAM */ - if (!prmrr_size) { - if (config->enable_c6dram) - prmrr_size = 1*MiB; - else - return 0; - } - - /* - * PRMRR Sizes that are > 1MB and < 32MB are - * not supported and will fail out. - */ - if ((prmrr_size > 1*MiB) && (prmrr_size < 32*MiB)) - die("PRMRR Sizes that are > 1MB and < 32MB are not" - "supported!\n"); - - prmrr_base -= prmrr_size; - if (prmrr_size >= 32*MiB) - prmrr_base = ALIGN_DOWN(prmrr_base, 128*MiB); - else - prmrr_base = ALIGN_DOWN(prmrr_base, 16*MiB); - /* PRMRR Area Size */ - prmrr_size = dram_base - prmrr_base; - - return prmrr_size; -} - -/* Calculate Intel Traditional Memory size based on GSM, DSM, TSEG and DPR. */ -static size_t calculate_traditional_mem_size(uintptr_t dram_base, - const struct device *dev) -{ - uintptr_t traditional_mem_base = dram_base; - size_t traditional_mem_size; - - if (dev->enabled) { - /* Read BDSM from Host Bridge */ - traditional_mem_base -= sa_get_dsm_size(); - - /* Read BGSM from Host Bridge */ - traditional_mem_base -= sa_get_gsm_size(); - } - /* Get TSEG size */ - traditional_mem_base -= sa_get_tseg_size(); - - /* Get DPR size */ - if (CONFIG(SA_ENABLE_DPR)) - traditional_mem_base -= sa_get_dpr_size(); - - /* Traditional Area Size */ - traditional_mem_size = dram_base - traditional_mem_base; - - return traditional_mem_size; -} - -/* - * Calculate Intel Reserved Memory size based on - * PRMRR size, Me stolen memory and PTT selection. - */ -static size_t calculate_reserved_mem_size(uintptr_t dram_base, - const struct device *dev) -{ - uintptr_t reserve_mem_base = dram_base; - size_t reserve_mem_size; - const struct soc_intel_cannonlake_config *config; - - config = config_of(dev); - - /* Get PRMRR size */ - reserve_mem_base -= get_prmrr_size(reserve_mem_base, config); - - /* Get Tracehub size */ - reserve_mem_base -= get_imr_size(); - - /* Traditional Area Size */ - reserve_mem_size = dram_base - reserve_mem_base; - - return reserve_mem_size; -} - -/* - * Host Memory Map: - * - * +--------------------------+ TOUUD - * | | - * +--------------------------+ 4GiB - * | PCI Address Space | - * +--------------------------+ TOLUD (also maps into MC address space) - * | iGD | - * +--------------------------+ BDSM - * | GTT | - * +--------------------------+ BGSM - * | TSEG | - * +--------------------------+ TSEGMB - * | DMA Protected Region | - * +--------------------------+ DPR - * | PRM (C6DRAM/SGX) | - * +--------------------------+ PRMRR - * | ME Stolen Memory | - * +--------------------------+ ME Stolen - * | PTT | - * +--------------------------+ top_of_ram - * | Reserved - FSP/CBMEM | - * +--------------------------+ TOLUM - * | Usage DRAM | - * +--------------------------+ 0 - * - * Some of the base registers above can be equal making the size of those - * regions 0. The reason is because the memory controller internally subtracts - * the base registers from each other to determine sizes of the regions. In - * other words, the memory map is in a fixed order no matter what. - */ -static uintptr_t calculate_dram_base(size_t *reserved_mem_size) -{ - uintptr_t dram_base; - const struct device *dev; - - dev = pcidev_on_root(SA_DEV_SLOT_IGD, 0); - if (!dev) - die_with_post_code(POST_HW_INIT_FAILURE, - "ERROR - IGD device not found!"); - - /* Read TOLUD from Host Bridge offset */ - dram_base = sa_get_tolud_base(); - - /* Get Intel Traditional Memory Range Size */ - dram_base -= calculate_traditional_mem_size(dram_base, dev); - - /* Get Intel Reserved Memory Range Size */ - *reserved_mem_size = calculate_reserved_mem_size(dram_base, dev); - - dram_base -= *reserved_mem_size; - - return dram_base; -} - /* * SoC implementation * @@ -210,10 +54,19 @@ /* Fill up memory layout information */ void fill_soc_memmap_ebda(struct ebda_config *cfg) { - size_t chipset_mem_size; + struct range_entry fsp_mem; + struct range_entry tolum;
- cfg->tolum_base = calculate_dram_base(&chipset_mem_size); - cfg->reserved_mem_size = chipset_mem_size; + /* Lookup the FSP_BOOTLOADER_TOLUM_HOB */ + if (fsp_find_bootloader_tolum(&tolum)) + die("9.3: FSP_BOOTLOADER_TOLUM_HOB missing!\n"); + + /* Locate the FSP reserved memory area */ + if (fsp_find_reserved_memory(&fsp_mem)) + die("9.1: FSP_RESERVED_MEMORY_RESOURCE_HOB missing!\n"); + + cfg->tolum_end = range_entry_end(&tolum); + cfg->reserved_mem_size = range_entry_size(&fsp_mem); }
void cbmem_top_init(void) @@ -253,15 +106,6 @@ { struct ebda_config ebda_cfg;
- /* - * Check if Tseg has been initialized, we will use this as a flag - * to check if the MRC is done, and only then continue to read the - * PRMMR_BASE MSR. The system hangs if PRMRR_BASE MSR is read before - * PRMRR_MASK MSR lock bit is set. - */ - if (sa_get_tseg_base() == 0) - return NULL; - retrieve_ebda_object(&ebda_cfg);
return (void *)(uintptr_t)ebda_cfg.tolum_base; diff --git a/src/soc/intel/icelake/memmap.c b/src/soc/intel/icelake/memmap.c index 20c4e6f..9df1065 100644 --- a/src/soc/intel/icelake/memmap.c +++ b/src/soc/intel/icelake/memmap.c @@ -28,6 +28,7 @@ #include <soc/soc_chip.h> #include <soc/systemagent.h> #include <stdlib.h> +#include <fsp/util.h>
void smm_region(uintptr_t *start, size_t *size) { @@ -35,161 +36,6 @@ *size = sa_get_tseg_size(); }
-/* Calculate ME Stolen size */ -static size_t get_imr_size(void) -{ - size_t imr_size; - - /* ME stolen memory */ - imr_size = MCHBAR32(IMRLIMIT) - MCHBAR32(IMRBASE); - - return imr_size; -} - -/* Calculate PRMRR size based on user input PRMRR size and alignment */ -static size_t get_prmrr_size(uintptr_t dram_base, - const struct soc_intel_icelake_config *config) -{ - uintptr_t prmrr_base = dram_base; - size_t prmrr_size; - - prmrr_size = config->PrmrrSize; - - /* Allocate PRMRR memory for C6DRAM */ - if (!prmrr_size) { - if (config->enable_c6dram) - prmrr_size = 1*MiB; - else - return 0; - } - - /* - * PRMRR Sizes that are > 1MB and < 32MB are - * not supported and will fail out. - */ - if ((prmrr_size > 1*MiB) && (prmrr_size < 32*MiB)) - die("PRMRR Sizes that are > 1MB and < 32MB are not" - "supported!\n"); - - prmrr_base -= prmrr_size; - if (prmrr_size >= 32*MiB) - prmrr_base = ALIGN_DOWN(prmrr_base, 128*MiB); - else - prmrr_base = ALIGN_DOWN(prmrr_base, 16*MiB); - /* PRMRR Area Size */ - prmrr_size = dram_base - prmrr_base; - - return prmrr_size; -} - -/* Calculate Intel Traditional Memory size based on GSM, DSM, TSEG and DPR. */ -static size_t calculate_traditional_mem_size(uintptr_t dram_base, - const struct device *dev) -{ - uintptr_t traditional_mem_base = dram_base; - size_t traditional_mem_size; - - if (dev->enabled) { - /* Read BDSM from Host Bridge */ - traditional_mem_base -= sa_get_dsm_size(); - - /* Read BGSM from Host Bridge */ - traditional_mem_base -= sa_get_gsm_size(); - } - /* Get TSEG size */ - traditional_mem_base -= sa_get_tseg_size(); - - /* Get DPR size */ - if (CONFIG(SA_ENABLE_DPR)) - traditional_mem_base -= sa_get_dpr_size(); - - /* Traditional Area Size */ - traditional_mem_size = dram_base - traditional_mem_base; - - return traditional_mem_size; -} - -/* - * Calculate Intel Reserved Memory size based on - * PRMRR size, Me stolen memory and PTT selection. - */ -static size_t calculate_reserved_mem_size(uintptr_t dram_base, - const struct device *dev) -{ - uintptr_t reserve_mem_base = dram_base; - size_t reserve_mem_size; - const struct soc_intel_icelake_config *config; - - config = config_of(dev); - - /* Get PRMRR size */ - reserve_mem_base -= get_prmrr_size(reserve_mem_base, config); - - /* Get Tracehub size */ - reserve_mem_base -= get_imr_size(); - - /* Traditional Area Size */ - reserve_mem_size = dram_base - reserve_mem_base; - - return reserve_mem_size; -} - -/* - * Host Memory Map: - * - * +--------------------------+ TOUUD - * | | - * +--------------------------+ 4GiB - * | PCI Address Space | - * +--------------------------+ TOLUD (also maps into MC address space) - * | iGD | - * +--------------------------+ BDSM - * | GTT | - * +--------------------------+ BGSM - * | TSEG | - * +--------------------------+ TSEGMB - * | DMA Protected Region | - * +--------------------------+ DPR - * | PRM (C6DRAM/SGX) | - * +--------------------------+ PRMRR - * | ME Stolen Memory | - * +--------------------------+ ME Stolen - * | PTT | - * +--------------------------+ top_of_ram - * | Reserved - FSP/CBMEM | - * +--------------------------+ TOLUM - * | Usage DRAM | - * +--------------------------+ 0 - * - * Some of the base registers above can be equal making the size of those - * regions 0. The reason is because the memory controller internally subtracts - * the base registers from each other to determine sizes of the regions. In - * other words, the memory map is in a fixed order no matter what. - */ -static uintptr_t calculate_dram_base(size_t *reserved_mem_size) -{ - uintptr_t dram_base; - const struct device *dev; - - dev = pcidev_on_root(SA_DEV_SLOT_IGD, 0); - if (!dev) - die_with_post_code(POST_HW_INIT_FAILURE, - "ERROR - IGD device not found!"); - - /* Read TOLUD from Host Bridge offset */ - dram_base = sa_get_tolud_base(); - - /* Get Intel Traditional Memory Range Size */ - dram_base -= calculate_traditional_mem_size(dram_base, dev); - - /* Get Intel Reserved Memory Range Size */ - *reserved_mem_size = calculate_reserved_mem_size(dram_base, dev); - - dram_base -= *reserved_mem_size; - - return dram_base; -} - /* * SoC implementation * @@ -208,10 +54,19 @@ /* Fill up memory layout information */ void fill_soc_memmap_ebda(struct ebda_config *cfg) { - size_t chipset_mem_size; + struct range_entry fsp_mem; + struct range_entry tolum;
- cfg->tolum_base = calculate_dram_base(&chipset_mem_size); - cfg->reserved_mem_size = chipset_mem_size; + /* Lookup the FSP_BOOTLOADER_TOLUM_HOB */ + if (fsp_find_bootloader_tolum(&tolum)) + die("9.3: FSP_BOOTLOADER_TOLUM_HOB missing!\n"); + + /* Locate the FSP reserved memory area */ + if (fsp_find_reserved_memory(&fsp_mem)) + die("9.1: FSP_RESERVED_MEMORY_RESOURCE_HOB missing!\n"); + + cfg->tolum_end = range_entry_end(&tolum); + cfg->reserved_mem_size = range_entry_size(&fsp_mem); }
void cbmem_top_init(void) @@ -251,18 +106,9 @@ { struct ebda_config ebda_cfg;
- /* - * Check if Tseg has been initialized, we will use this as a flag - * to check if the MRC is done, and only then continue to read the - * PRMMR_BASE MSR. The system hangs if PRMRR_BASE MSR is read before - * PRMRR_MASK MSR lock bit is set. - */ - if (sa_get_tseg_base() == 0) - return NULL; - retrieve_ebda_object(&ebda_cfg);
- return (void *)(uintptr_t)ebda_cfg.tolum_base; + return (void *)(uintptr_t)ebda_cfg.tolum_end; }
void fill_postcar_frame(struct postcar_frame *pcf) diff --git a/src/soc/intel/skylake/include/soc/ebda.h b/src/soc/intel/skylake/include/soc/ebda.h index ad62394..d588c18 100644 --- a/src/soc/intel/skylake/include/soc/ebda.h +++ b/src/soc/intel/skylake/include/soc/ebda.h @@ -20,8 +20,8 @@
struct ebda_config { uint32_t signature; /* 0x00 - EBDA signature */ - uint32_t tolum_base; /* 0x04 - coreboot memory start */ - uint32_t reserved_mem_size; /* 0x08 - chipset reserved memory size */ + uint32_t tolum_end; /* 0x04 - coreboot memory start */ + uint32_t reserved_mem_size; /* 0x0C - chipset reserved memory size */ };
#endif diff --git a/src/soc/intel/skylake/memmap.c b/src/soc/intel/skylake/memmap.c index 29f2517..5c4cc3a 100644 --- a/src/soc/intel/skylake/memmap.c +++ b/src/soc/intel/skylake/memmap.c @@ -29,8 +29,7 @@ #include <soc/pci_devs.h> #include <soc/systemagent.h> #include <stdlib.h> - -#include "chip.h" +#include <fsp/util.h>
void smm_region(uintptr_t *start, size_t *size) { @@ -38,185 +37,6 @@ *size = sa_get_tseg_size(); }
-static bool is_ptt_enable(void) -{ - if ((read32((void *)PTT_TXT_BASE_ADDRESS) & PTT_PRESENT) == - PTT_PRESENT) - return true; - - return false; -} - -/* Calculate PTT size */ -static size_t get_ptt_size(void) -{ - /* Allocate 4KB for PTT if enabled */ - return is_ptt_enable() ? 4*KiB : 0; -} - -/* Calculate Trace Hub size */ -static size_t get_tracehub_size(uintptr_t dram_base, - const struct soc_intel_skylake_config *config) -{ - uintptr_t tracehub_base = dram_base; - size_t tracehub_size = 0; - - if (!config->ProbelessTrace) - return 0; - - /* GDXC MOT */ - tracehub_base -= GDXC_MOT_MEMORY_SIZE; - /* Round down to natural boundary according to PSMI size */ - tracehub_base = ALIGN_DOWN(tracehub_base, PSMI_BUFFER_AREA_SIZE); - /* GDXC IOT */ - tracehub_base -= GDXC_IOT_MEMORY_SIZE; - /* PSMI buffer area */ - tracehub_base -= PSMI_BUFFER_AREA_SIZE; - - /* Tracehub Area Size */ - tracehub_size = dram_base - tracehub_base; - - return tracehub_size; -} - -/* Calculate PRMRR size based on user input PRMRR size and alignment */ -static size_t get_prmrr_size(uintptr_t dram_base, - const struct soc_intel_skylake_config *config) -{ - uintptr_t prmrr_base = dram_base; - size_t prmrr_size; - - if (CONFIG(PLATFORM_USES_FSP1_1)) - prmrr_size = 1*MiB; - else - prmrr_size = config->PrmrrSize; - - if (!prmrr_size) - return 0; - - /* - * PRMRR Sizes that are > 1MB and < 32MB are - * not supported and will fail out. - */ - if ((prmrr_size > 1*MiB) && (prmrr_size < 32*MiB)) - die("PRMRR Sizes that are > 1MB and < 32MB are not" - "supported!\n"); - - prmrr_base -= prmrr_size; - if (prmrr_size >= 32*MiB) - prmrr_base = ALIGN_DOWN(prmrr_base, 128*MiB); - - /* PRMRR Area Size */ - prmrr_size = dram_base - prmrr_base; - - return prmrr_size; -} - -/* Calculate Intel Traditional Memory size based on GSM, DSM, TSEG and DPR. */ -static size_t calculate_traditional_mem_size(uintptr_t dram_base) -{ - const struct device *igd_dev = pcidev_path_on_root(SA_DEVFN_IGD); - uintptr_t traditional_mem_base = dram_base; - size_t traditional_mem_size; - - if (igd_dev && igd_dev->enabled) { - /* Read BDSM from Host Bridge */ - traditional_mem_base -= sa_get_dsm_size(); - - /* Read BGSM from Host Bridge */ - traditional_mem_base -= sa_get_gsm_size(); - } - /* Get TSEG size */ - traditional_mem_base -= sa_get_tseg_size(); - - /* Get DPR size */ - if (CONFIG(SA_ENABLE_DPR)) - traditional_mem_base -= sa_get_dpr_size(); - - /* Traditional Area Size */ - traditional_mem_size = dram_base - traditional_mem_base; - - return traditional_mem_size; -} - -/* - * Calculate Intel Reserved Memory size based on - * PRMRR size, Trace Hub config and PTT selection. - */ -static size_t calculate_reserved_mem_size(uintptr_t dram_base) -{ - const struct device *dev = pcidev_path_on_root(SA_DEVFN_ROOT); - uintptr_t reserve_mem_base = dram_base; - size_t reserve_mem_size; - const struct soc_intel_skylake_config *config; - - config = config_of(dev); - - /* Get PRMRR size */ - reserve_mem_base -= get_prmrr_size(reserve_mem_base, config); - - /* Get Tracehub size */ - reserve_mem_base -= get_tracehub_size(reserve_mem_base, config); - - /* Get PTT size */ - reserve_mem_base -= get_ptt_size(); - - /* Traditional Area Size */ - reserve_mem_size = dram_base - reserve_mem_base; - - return reserve_mem_size; -} - -/* - * Host Memory Map: - * - * +--------------------------+ TOUUD - * | | - * +--------------------------+ 4GiB - * | PCI Address Space | - * +--------------------------+ TOLUD (also maps into MC address space) - * | iGD | - * +--------------------------+ BDSM - * | GTT | - * +--------------------------+ BGSM - * | TSEG | - * +--------------------------+ TSEGMB - * | DMA Protected Region | - * +--------------------------+ DPR - * | PRM (C6DRAM/SGX) | - * +--------------------------+ PRMRR - * | Trace Memory | - * +--------------------------+ Probless Trace - * | PTT | - * +--------------------------+ top_of_ram - * | Reserved - FSP/CBMEM | - * +--------------------------+ TOLUM - * | Usage DRAM | - * +--------------------------+ 0 - * - * Some of the base registers above can be equal making the size of those - * regions 0. The reason is because the memory controller internally subtracts - * the base registers from each other to determine sizes of the regions. In - * other words, the memory map is in a fixed order no matter what. - */ -static uintptr_t calculate_dram_base(size_t *reserved_mem_size) -{ - uintptr_t dram_base; - - /* Read TOLUD from Host Bridge offset */ - dram_base = sa_get_tolud_base(); - - /* Get Intel Traditional Memory Range Size */ - dram_base -= calculate_traditional_mem_size(dram_base); - - /* Get Intel Reserved Memory Range Size */ - *reserved_mem_size = calculate_reserved_mem_size(dram_base); - - dram_base -= *reserved_mem_size; - - return dram_base; -} - /* * SoC implementation * @@ -235,10 +55,19 @@ /* Fill up memory layout information */ void fill_soc_memmap_ebda(struct ebda_config *cfg) { - size_t chipset_mem_size; + struct range_entry fsp_mem; + struct range_entry tolum;
- cfg->tolum_base = calculate_dram_base(&chipset_mem_size); - cfg->reserved_mem_size = chipset_mem_size; + /* Lookup the FSP_BOOTLOADER_TOLUM_HOB */ + if (fsp_find_bootloader_tolum(&tolum)) + die("9.3: FSP_BOOTLOADER_TOLUM_HOB missing!\n"); + + /* Locate the FSP reserved memory area */ + if (fsp_find_reserved_memory(&fsp_mem)) + die("9.1: FSP_RESERVED_MEMORY_RESOURCE_HOB missing!\n"); + + cfg->tolum_end = range_entry_end(&tolum); + cfg->reserved_mem_size = range_entry_size(&fsp_mem); }
void cbmem_top_init(void) @@ -278,18 +107,9 @@ { struct ebda_config ebda_cfg;
- /* - * Check if Tseg has been initialized, we will use this as a flag - * to check if the MRC is done, and only then continue to read the - * PRMMR_BASE MSR. The system hangs if PRMRR_BASE MSR is read before - * PRMRR_MASK MSR lock bit is set. - */ - if (sa_get_tseg_base() == 0) - return NULL; - retrieve_ebda_object(&ebda_cfg);
- return (void *)(uintptr_t)ebda_cfg.tolum_base; + return (void *)(uintptr_t)ebda_cfg.tolum_end; }
#if CONFIG(PLATFORM_USES_FSP2_0)
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM and reserved memory returned by FSP ......................................................................
Patch Set 2:
This change is ready for review.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM and reserved memory returned by FSP ......................................................................
Uploaded patch set 3.
Hello Aaron Durbin, Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36136
to look at the new patch set (#3).
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM and reserved memory returned by FSP ......................................................................
soc/intel: skl,cnl,icl: rely on TOLUM and reserved memory returned by FSP
Instead of doing our own calculations, rely on the TOLUM and reserved memory size returned by FSP. This (hopefully) saves us from making mistakes in calculations of offsets and aligments.
Further this makes it easier to implement e.g. SGX PRMRR size selection via Kconfig as we do not have to know anything and do not have to make any assumptions about aligning but simply pass (valid) values to FSP.
Tested successfully on X11SSM-F
Change-Id: If66a00d1320917bc68afb32c19db0e24c6732812 Signed-off-by: Michael Niewöhner foss@mniewoehner.de --- M src/soc/intel/cannonlake/include/soc/ebda.h M src/soc/intel/cannonlake/memmap.c M src/soc/intel/icelake/include/soc/ebda.h M src/soc/intel/icelake/memmap.c M src/soc/intel/skylake/include/soc/ebda.h M src/soc/intel/skylake/memmap.c 6 files changed, 49 insertions(+), 559 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/36136/3
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM and reserved memory returned by FSP ......................................................................
Patch Set 3:
(4 comments)
https://review.coreboot.org/c/coreboot/+/36136/1/src/soc/intel/cannonlake/me... File src/soc/intel/cannonlake/memmap.c:
https://review.coreboot.org/c/coreboot/+/36136/1/src/soc/intel/cannonlake/me... PS1, Line 17: #include <arch/romstage.h> : #include <arch/ebda.h> : #include <cbmem.h> : #include <console/console.h> : #include <cpu/x86/mtrr.h> : #include <cpu/x86/smm.h> : #include <device/device.h> : #include <device/pci.h> : #include <fsp/util.h> : #include <intelblocks/ebda.h> : #include <intelblocks/systemagent.h> : #include <soc/pci_devs.h> : #include <soc/systemagent.h> : #include <stdlib.h>
I assume half of this is obsoleted.
Done
https://review.coreboot.org/c/coreboot/+/36136/1/src/soc/intel/cannonlake/me... PS1, Line 31: #include <fsp/util.h>
it's already there :)
Done
https://review.coreboot.org/c/coreboot/+/36136/1/src/soc/intel/skylake/inclu... File src/soc/intel/skylake/include/soc/ebda.h:
https://review.coreboot.org/c/coreboot/+/36136/1/src/soc/intel/skylake/inclu... PS1, Line 23: uint32_t tolum_end; /* 0x04 - coreboot memory start */
This needs to be changed for the other SoCs as well. Or maybe […]
Done
https://review.coreboot.org/c/coreboot/+/36136/1/src/soc/intel/skylake/inclu... PS1, Line 24: uint32_t reserved_mem_size; /* 0x0C - chipset reserved memory size */
huh? […]
Done
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM and reserved memory returned by FSP ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36136/3/src/soc/intel/icelake/memma... File src/soc/intel/icelake/memmap.c:
https://review.coreboot.org/c/coreboot/+/36136/3/src/soc/intel/icelake/memma... PS3, Line 54: fsp_find_bootloader_tolum Fix cbmem_top on all FSP2.0 targets to use this, move it out of hob_verify.c and fix fsp_verify_memory_init_hobs to not compare it with cbmem?
Hello Aaron Durbin, Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36136
to look at the new patch set (#4).
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM and reserved memory returned by FSP ......................................................................
soc/intel: skl,cnl,icl: rely on TOLUM and reserved memory returned by FSP
Instead of doing our own calculations, rely on the TOLUM and reserved memory size returned by FSP. This (hopefully) saves us from making mistakes in calculations of offsets and aligments.
Further this makes it easier to implement e.g. SGX PRMRR size selection via Kconfig as we do not have to know anything and do not have to make any assumptions about aligning but simply pass (valid) values to FSP.
Tested successfully on X11SSM-F
Change-Id: If66a00d1320917bc68afb32c19db0e24c6732812 Signed-off-by: Michael Niewöhner foss@mniewoehner.de --- M src/soc/intel/cannonlake/include/soc/ebda.h M src/soc/intel/cannonlake/memmap.c M src/soc/intel/icelake/include/soc/ebda.h M src/soc/intel/icelake/memmap.c M src/soc/intel/skylake/include/soc/ebda.h M src/soc/intel/skylake/memmap.c 6 files changed, 103 insertions(+), 514 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/36136/4
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM and reserved memory returned by FSP ......................................................................
Uploaded patch set 4.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Patch Set 5:
This change is ready for review.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Uploaded patch set 6.
Hello Aaron Durbin, Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36136
to look at the new patch set (#6).
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP
Instead of doing our own calculations, rely on TOLUM returned by FSP for cbmem_top. This (hopefully) saves us from making mistakes in weird calculations of offsets and aligments.
Further this makes it easier to implement e.g. SGX PRMRR size selection via Kconfig as we do not have to make any assumptions about alignments but can simply pass (valid) values to FSP.
Tested successfully on X11SSM-F
Change-Id: If66a00d1320917bc68afb32c19db0e24c6732812 Signed-off-by: Michael Niewöhner foss@mniewoehner.de --- M src/drivers/intel/fsp2_0/hand_off_block.c M src/drivers/intel/fsp2_0/hob_verify.c M src/drivers/intel/fsp2_0/include/fsp/debug.h M src/drivers/intel/fsp2_0/include/fsp/util.h M src/soc/intel/cannonlake/include/soc/ebda.h M src/soc/intel/cannonlake/memmap.c M src/soc/intel/icelake/include/soc/ebda.h M src/soc/intel/icelake/memmap.c M src/soc/intel/skylake/include/soc/ebda.h M src/soc/intel/skylake/memmap.c 10 files changed, 44 insertions(+), 474 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/36136/6
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Uploaded patch set 7.
Hello Aaron Durbin, Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36136
to look at the new patch set (#7).
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP
Instead of doing our own calculations, rely on TOLUM returned by FSP for cbmem_top. This (hopefully) saves us from making mistakes in weird calculations of offsets and aligments.
Further this makes it easier to implement e.g. SGX PRMRR size selection via Kconfig as we do not have to make any assumptions about alignments but can simply pass (valid) values to FSP.
Tested successfully on X11SSM-F
Change-Id: If66a00d1320917bc68afb32c19db0e24c6732812 Signed-off-by: Michael Niewöhner foss@mniewoehner.de --- M src/drivers/intel/fsp2_0/hand_off_block.c M src/drivers/intel/fsp2_0/hob_verify.c M src/drivers/intel/fsp2_0/include/fsp/debug.h M src/drivers/intel/fsp2_0/include/fsp/util.h M src/soc/intel/cannonlake/include/soc/ebda.h M src/soc/intel/cannonlake/memmap.c M src/soc/intel/icelake/include/soc/ebda.h M src/soc/intel/icelake/memmap.c M src/soc/intel/skylake/include/soc/ebda.h M src/soc/intel/skylake/memmap.c 10 files changed, 44 insertions(+), 493 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/36136/7
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Uploaded patch set 8.
Hello Aaron Durbin, Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36136
to look at the new patch set (#8).
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP
Instead of doing our own calculations, rely on TOLUM returned by FSP for cbmem_top. This (hopefully) saves us from making mistakes in weird calculations of offsets and aligments.
Further this makes it easier to implement e.g. SGX PRMRR size selection via Kconfig as we do not have to make any assumptions about alignments but can simply pass (valid) values to FSP.
Tested successfully on X11SSM-F
Change-Id: If66a00d1320917bc68afb32c19db0e24c6732812 Signed-off-by: Michael Niewöhner foss@mniewoehner.de --- M src/drivers/intel/fsp2_0/hand_off_block.c M src/drivers/intel/fsp2_0/hob_verify.c M src/drivers/intel/fsp2_0/include/fsp/debug.h M src/drivers/intel/fsp2_0/include/fsp/util.h M src/soc/intel/cannonlake/include/soc/ebda.h M src/soc/intel/cannonlake/memmap.c M src/soc/intel/icelake/include/soc/ebda.h M src/soc/intel/icelake/memmap.c M src/soc/intel/skylake/include/soc/ebda.h M src/soc/intel/skylake/memmap.c 10 files changed, 43 insertions(+), 493 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/36136/8
Hello Aaron Durbin, Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36136
to look at the new patch set (#9).
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP
Instead of doing our own calculations, rely on TOLUM returned by FSP for cbmem_top. This (hopefully) saves us from making mistakes in weird calculations of offsets and aligments.
Further this makes it easier to implement e.g. SGX PRMRR size selection via Kconfig as we do not have to make any assumptions about alignments but can simply pass (valid) values to FSP.
Tested successfully on X11SSM-F
Change-Id: If66a00d1320917bc68afb32c19db0e24c6732812 Signed-off-by: Michael Niewöhner foss@mniewoehner.de --- M src/drivers/intel/fsp2_0/hand_off_block.c M src/drivers/intel/fsp2_0/hob_verify.c M src/drivers/intel/fsp2_0/include/fsp/debug.h M src/drivers/intel/fsp2_0/include/fsp/util.h M src/soc/intel/cannonlake/include/soc/ebda.h M src/soc/intel/cannonlake/memmap.c M src/soc/intel/icelake/include/soc/ebda.h M src/soc/intel/icelake/memmap.c M src/soc/intel/skylake/include/soc/ebda.h M src/soc/intel/skylake/memmap.c 10 files changed, 43 insertions(+), 488 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/36136/9
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Uploaded patch set 10: Patch Set 9 was rebased.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Patch Set 10: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/36136/10/src/soc/intel/skylake/memm... File src/soc/intel/skylake/memmap.c:
https://review.coreboot.org/c/coreboot/+/36136/10/src/soc/intel/skylake/memm... PS10, Line 73: die("9.3: FSP_BOOTLOADER_TOLUM_HOB missing!\n"); Should we maybe move this into fsp_find_bootloader_tolum()? Is there any call where we wouldn't want to die()?
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Uploaded patch set 11: Patch Set 10 was rebased.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36136/11/src/soc/intel/skylake/memm... File src/soc/intel/skylake/memmap.c:
https://review.coreboot.org/c/coreboot/+/36136/11/src/soc/intel/skylake/memm... PS11, Line 54: * +--------------------------+ ME Stolen Why did this change? There's trace and ME stolen memory. Though, admittedly, the picture doesn't discuss it. It's very much carved out.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36136/11/src/soc/intel/skylake/memm... File src/soc/intel/skylake/memmap.c:
https://review.coreboot.org/c/coreboot/+/36136/11/src/soc/intel/skylake/memm... PS11, Line 54: * +--------------------------+ ME Stolen
Why did this change? There's trace and ME stolen memory. […]
Actually, if that didn't change over the years, ME UMA is above TOLUD. Either in the >4GiB space, if there is any, or between TOLUD and the MMIO hole.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Patch Set 12:
please don't make this change, this is exactly opposite the direction what we decided in past that we shouldn't relying much on FSP in memory snap shot creation and make design where FSP and bootloader have transparent memory design. please look at original CL when it got merged.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Patch Set 12: Code-Review-2
just to make sure it doesn't get merge so soon.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Patch Set 12:
Patch Set 12:
please don't make this change, this is exactly opposite the direction what we decided in past that we shouldn't relying much on FSP in memory snap shot creation and make design where FSP and bootloader have transparent memory design. please look at original CL when it got merged.
What is trying to be achieved here is reducing the coupling between coreboot and FSP for the memory layout. The original patches were attempting to closely track FSP's behavior.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Patch Set 12:
Patch Set 12:
Patch Set 12:
please don't make this change, this is exactly opposite the direction what we decided in past that we shouldn't relying much on FSP in memory snap shot creation and make design where FSP and bootloader have transparent memory design. please look at original CL when it got merged.
What is trying to be achieved here is reducing the coupling between coreboot and FSP for the memory layout. The original patches were attempting to closely track FSP's behavior.
I think below assumption doesn't hold good for CNL onwards platform, thats the reason we had decided to drop the FSP dependency in memory layout design
/* Lookup the FSP_BOOTLOADER_TOLUM_HOB */ if (fsp_find_bootloader_tolum(&tolum)) die("9.3: FSP_BOOTLOADER_TOLUM_HOB missing!\n");
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Patch Set 12:
Patch Set 12:
Patch Set 12:
Patch Set 12:
please don't make this change, this is exactly opposite the direction what we decided in past that we shouldn't relying much on FSP in memory snap shot creation and make design where FSP and bootloader have transparent memory design. please look at original CL when it got merged.
What is trying to be achieved here is reducing the coupling between coreboot and FSP for the memory layout. The original patches were attempting to closely track FSP's behavior.
I think below assumption doesn't hold good for CNL onwards platform, thats the reason we had decided to drop the FSP dependency in memory layout design
/* Lookup the FSP_BOOTLOADER_TOLUM_HOB */ if (fsp_find_bootloader_tolum(&tolum)) die("9.3: FSP_BOOTLOADER_TOLUM_HOB missing!\n");
Could you please elaborate on what doesn't hold? Why does this not work?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Patch Set 12:
Patch Set 12:
Patch Set 12:
Patch Set 12:
Patch Set 12:
please don't make this change, this is exactly opposite the direction what we decided in past that we shouldn't relying much on FSP in memory snap shot creation and make design where FSP and bootloader have transparent memory design. please look at original CL when it got merged.
What is trying to be achieved here is reducing the coupling between coreboot and FSP for the memory layout. The original patches were attempting to closely track FSP's behavior.
I think below assumption doesn't hold good for CNL onwards platform, thats the reason we had decided to drop the FSP dependency in memory layout design
/* Lookup the FSP_BOOTLOADER_TOLUM_HOB */ if (fsp_find_bootloader_tolum(&tolum)) die("9.3: FSP_BOOTLOADER_TOLUM_HOB missing!\n");
Could you please elaborate on what doesn't hold? Why does this not work?
its 3 year old discussion when we had seen issue with consuming FSP hob to cbmem memory top. Because FSP TOLUM base calculation was different in SKL and CNL (BOOTLOADER TOLUM base and FSP Reserve memory base order) if i'm not wrong.
Top of that Aaron, its you who took stand that why do i relying on FSP to tell me what is my memory start and CB is master not FSP. Then we started looking at transparent memory layout design and come up with this CLs. Now if you are okay with original idea then from SOC side we don't have much objection except a request to don't break other platform with just SKL assumptions.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Uploaded patch set 13.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Patch Set 12:
Patch Set 12:
Patch Set 12:
Patch Set 12:
Patch Set 12:
Patch Set 12:
please don't make this change, this is exactly opposite the direction what we decided in past that we shouldn't relying much on FSP in memory snap shot creation and make design where FSP and bootloader have transparent memory design. please look at original CL when it got merged.
What is trying to be achieved here is reducing the coupling between coreboot and FSP for the memory layout. The original patches were attempting to closely track FSP's behavior.
I think below assumption doesn't hold good for CNL onwards platform, thats the reason we had decided to drop the FSP dependency in memory layout design
/* Lookup the FSP_BOOTLOADER_TOLUM_HOB */ if (fsp_find_bootloader_tolum(&tolum)) die("9.3: FSP_BOOTLOADER_TOLUM_HOB missing!\n");
Could you please elaborate on what doesn't hold? Why does this not work?
its 3 year old discussion when we had seen issue with consuming FSP hob to cbmem memory top. Because FSP TOLUM base calculation was different in SKL and CNL (BOOTLOADER TOLUM base and FSP Reserve memory base order) if i'm not wrong.
Top of that Aaron, its you who took stand that why do i relying on FSP to tell me what is my memory start and CB is master not FSP. Then we started looking at transparent memory layout design and come up with this CLs. Now if you are okay with original idea then from SOC side we don't have much objection except a request to don't break other platform with just SKL assumptions.
I recall, Subrata. I still hold the opinion (and wrote in a comment elsewhere) that I prefer to understand and know the memory map. However, I think I'm in the minority on this, and people are wanting to just rely upon what FSP spits out. If it allows people to utilize coreboot more in many different environments and the memory map tracking is preventing that then I'm ok with this approach. I do think we lose visibility and understanding by going this route, though.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Patch Set 13:
please don't make this change, this is exactly opposite the direction what we decided in past that we shouldn't relying much on FSP in memory snap shot creation and make design where FSP and bootloader have transparent memory design. please look at original CL when it got merged.
What is trying to be achieved here is reducing the coupling between coreboot and FSP for the memory layout. The original patches were attempting to closely track FSP's behavior.
I think below assumption doesn't hold good for CNL onwards platform, thats the reason we had decided to drop the FSP dependency in memory layout design
/* Lookup the FSP_BOOTLOADER_TOLUM_HOB */ if (fsp_find_bootloader_tolum(&tolum)) die("9.3: FSP_BOOTLOADER_TOLUM_HOB missing!\n");
Could you please elaborate on what doesn't hold? Why does this not work?
its 3 year old discussion when we had seen issue with consuming FSP hob to cbmem memory top. Because FSP TOLUM base calculation was different in SKL and CNL (BOOTLOADER TOLUM base and FSP Reserve memory base order) if i'm not wrong.
Top of that Aaron, its you who took stand that why do i relying on FSP to tell me what is my memory start and CB is master not FSP. Then we started looking at transparent memory layout design and come up with this CLs. Now if you are okay with original idea then from SOC side we don't have much objection except a request to don't break other platform with just SKL assumptions.
I recall, Subrata. I still hold the opinion (and wrote in a comment elsewhere) that I prefer to understand and know the memory map. However, I think I'm in the minority on this, and people are wanting to just rely upon what FSP spits out. If it allows people to utilize coreboot more in many different environments and the memory map tracking is preventing that then I'm ok with this approach. I do think we lose visibility and understanding by going this route, though.
Just want to say some words why I support Michael's changes. I was confused originally, when I learned that our cbmem_top() calculation just mimics FSP calculations. Back then, we didn't know any spare scratchpad register nor did we want to use low memory, so I assumed it was the only solution. And it worked for a while. But now and then we discovered that our calculations didn't match FSP's in some corner cases, that was fixed... now it's 2019 and it seems we still don't even got Skylake right. I assume mostly because nobody can review the changes due to lack of documentation and unreadable reference code. Well, that's what we get when we try to maintain an inter- face that doesn't exist.
I agree that there is benefit in having the calculations in coreboot. And honestly, I like it when some coding pattern forces people to leak infor- mation :) But looking at all the working hours wasted to try to keep core- boot's memmaps in sync with FSP, all the reviewing and bikeshedding, the bug hunting when something wasn't perfectly aligned with FSP... I think we just failed. We have to embrace easier, less expensive solutions if we want coreboot to flourish.
So why not try Michaels approach. At least to have a reliable code base for once. If we discover that we really miss some information later (something not easily read from a register like TSEG base or from a HOB like PRMRR base), we'll have all we need in the Git history.
About FSP_BOOTLOADER_TOLUM_HOB, if that fails with newer FSP binaries, well, then that's a violation of the FSP spec. I don't say we shouldn't test these patches. But if we see that it fails, the binaries will have to be fixed. And I wonder, if anybody really discovered a related bug already, why wasn't that reported/fixed?
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Patch Set 13:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36136/3/src/soc/intel/icelake/memma... File src/soc/intel/icelake/memmap.c:
https://review.coreboot.org/c/coreboot/+/36136/3/src/soc/intel/icelake/memma... PS3, Line 54: fsp_find_bootloader_tolum
Fix cbmem_top on all FSP2.0 targets to use this, move it out of hob_verify. […]
quark,apollolake,denverton_ns are left; they all don't have ebda or some equivalent mechanism
https://review.coreboot.org/c/coreboot/+/36136/10/src/soc/intel/skylake/memm... File src/soc/intel/skylake/memmap.c:
https://review.coreboot.org/c/coreboot/+/36136/10/src/soc/intel/skylake/memm... PS10, Line 73: die("9.3: FSP_BOOTLOADER_TOLUM_HOB missing!\n");
Ack
Done
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Patch Set 13:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36136/13//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36136/13//COMMIT_MSG@17 PS13, Line 17: Tested successfully on X11SSM-F needs retest
Hello Aaron Durbin, Patrick Rudolph, Subrata Banik, build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36136
to look at the new patch set (#14).
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP
Instead of doing our own calculations, rely on TOLUM returned by FSP for cbmem_top. This (hopefully) saves us from making mistakes in weird calculations of offsets and aligments.
Further this makes it easier to implement e.g. SGX PRMRR size selection via Kconfig as we do not have to make any assumptions about alignments but can simply pass (valid) values to FSP.
Tested successfully on X11SSM-F
Change-Id: If66a00d1320917bc68afb32c19db0e24c6732812 Signed-off-by: Michael Niewöhner foss@mniewoehner.de --- M src/drivers/intel/fsp2_0/hand_off_block.c M src/drivers/intel/fsp2_0/hob_verify.c M src/drivers/intel/fsp2_0/include/fsp/debug.h M src/drivers/intel/fsp2_0/include/fsp/util.h M src/drivers/intel/fsp2_0/memory_init.c M src/soc/intel/cannonlake/include/soc/ebda.h M src/soc/intel/cannonlake/memmap.c M src/soc/intel/common/block/include/intelblocks/systemagent.h M src/soc/intel/common/block/systemagent/systemagent_early.c M src/soc/intel/icelake/include/soc/ebda.h M src/soc/intel/icelake/memmap.c M src/soc/intel/skylake/include/soc/ebda.h M src/soc/intel/skylake/memmap.c 13 files changed, 43 insertions(+), 572 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/36136/14
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Uploaded patch set 14.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Patch Set 14:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36136/14/src/drivers/intel/fsp2_0/h... File src/drivers/intel/fsp2_0/hand_off_block.c:
https://review.coreboot.org/c/coreboot/+/36136/14/src/drivers/intel/fsp2_0/h... PS14, Line 194: if (fsp_find_range_hob(re, fsp_reserved_memory_guid)); trailing semicolon indicates no statements, indent implies otherwise
https://review.coreboot.org/c/coreboot/+/36136/14/src/drivers/intel/fsp2_0/h... PS14, Line 194: if (fsp_find_range_hob(re, fsp_reserved_memory_guid)); trailing statements should be on next line
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Patch Set 14:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36136/11/src/soc/intel/skylake/memm... File src/soc/intel/skylake/memmap.c:
https://review.coreboot.org/c/coreboot/+/36136/11/src/soc/intel/skylake/memm... PS11, Line 54: * +--------------------------+ ME Stolen
Actually, if that didn't change over the years, ME UMA is above TOLUD. […]
this is caused by moving the skylake memmap there, we just need to find the correct one out of that three :D (cnl, skl, icl)
Hello Aaron Durbin, Patrick Rudolph, Subrata Banik, build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36136
to look at the new patch set (#15).
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP
Instead of doing our own calculations, rely on TOLUM returned by FSP for cbmem_top. This (hopefully) saves us from making mistakes in weird calculations of offsets and aligments.
Further this makes it easier to implement e.g. SGX PRMRR size selection via Kconfig as we do not have to make any assumptions about alignments but can simply pass (valid) values to FSP.
Tested successfully on X11SSM-F
Change-Id: If66a00d1320917bc68afb32c19db0e24c6732812 Signed-off-by: Michael Niewöhner foss@mniewoehner.de --- M src/drivers/intel/fsp2_0/hand_off_block.c M src/drivers/intel/fsp2_0/hob_verify.c M src/drivers/intel/fsp2_0/include/fsp/debug.h M src/drivers/intel/fsp2_0/include/fsp/util.h M src/drivers/intel/fsp2_0/memory_init.c M src/soc/intel/cannonlake/include/soc/ebda.h M src/soc/intel/cannonlake/memmap.c M src/soc/intel/common/block/include/intelblocks/systemagent.h M src/soc/intel/common/block/systemagent/systemagent_early.c M src/soc/intel/icelake/include/soc/ebda.h M src/soc/intel/icelake/memmap.c M src/soc/intel/skylake/include/soc/ebda.h M src/soc/intel/skylake/memmap.c 13 files changed, 43 insertions(+), 572 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/36136/15
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Patch Set 15:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36136/15/src/drivers/intel/fsp2_0/h... File src/drivers/intel/fsp2_0/hand_off_block.c:
https://review.coreboot.org/c/coreboot/+/36136/15/src/drivers/intel/fsp2_0/h... PS15, Line 194: if (fsp_find_range_hob(re, fsp_reserved_memory_guid)); trailing semicolon indicates no statements, indent implies otherwise
https://review.coreboot.org/c/coreboot/+/36136/15/src/drivers/intel/fsp2_0/h... PS15, Line 194: if (fsp_find_range_hob(re, fsp_reserved_memory_guid)); trailing statements should be on next line
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Patch Set 15:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36136/15/src/drivers/intel/fsp2_0/h... File src/drivers/intel/fsp2_0/hand_off_block.c:
https://review.coreboot.org/c/coreboot/+/36136/15/src/drivers/intel/fsp2_0/h... PS15, Line 308: } Separate these changes into their own commit?
https://review.coreboot.org/c/coreboot/+/36136/15/src/soc/intel/common/block... File src/soc/intel/common/block/systemagent/systemagent_early.c:
https://review.coreboot.org/c/coreboot/+/36136/15/src/soc/intel/common/block... PS15, Line 145: } This was removed in a different commit after this one, but it should be in this one.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Patch Set 15:
Some thoughts: Some things do not need to be guessed an can be directly read from hardware registers (SA_BASE, TSEG, BDSM, GSM, ...) other things rely on guessing what the FSP did. Why not keep the calculation for things that are most certainly valid (hardware registers) and use the FSP HOB information to fill in gaps for things that cannot be read back (PRMRR?)? That allows to properly know the memory map, while still provide a correct result wrt the top of usable lower memory.
I'm not so familiar with the FSP interfaces, so IDK if that's just the worst of both worlds...
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Uploaded patch set 16.
(2 comments)
https://review.coreboot.org/c/coreboot/+/36136/15/src/drivers/intel/fsp2_0/h... File src/drivers/intel/fsp2_0/hand_off_block.c:
https://review.coreboot.org/c/coreboot/+/36136/15/src/drivers/intel/fsp2_0/h... PS15, Line 308: }
Separate these changes into their own commit?
Done
https://review.coreboot.org/c/coreboot/+/36136/15/src/soc/intel/common/block... File src/soc/intel/common/block/systemagent/systemagent_early.c:
https://review.coreboot.org/c/coreboot/+/36136/15/src/soc/intel/common/block... PS15, Line 145: }
This was removed in a different commit after this one, but it should be in this one.
Done
Hello Aaron Durbin, Patrick Rudolph, Subrata Banik, build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36136
to look at the new patch set (#16).
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP
Instead of doing our own calculations, rely on TOLUM returned by FSP for cbmem_top. This (hopefully) saves us from making mistakes in weird calculations of offsets and aligments.
Further this makes it easier to implement e.g. SGX PRMRR size selection via Kconfig as we do not have to make any assumptions about alignments but can simply pass (valid) values to FSP.
Tested successfully on X11SSM-F
Change-Id: If66a00d1320917bc68afb32c19db0e24c6732812 Signed-off-by: Michael Niewöhner foss@mniewoehner.de --- M src/drivers/intel/fsp2_0/hand_off_block.c M src/drivers/intel/fsp2_0/hob_verify.c M src/soc/intel/cannonlake/include/soc/ebda.h M src/soc/intel/cannonlake/memmap.c M src/soc/intel/common/block/include/intelblocks/systemagent.h M src/soc/intel/common/block/systemagent/systemagent_early.c M src/soc/intel/icelake/include/soc/ebda.h M src/soc/intel/icelake/memmap.c M src/soc/intel/skylake/include/soc/ebda.h M src/soc/intel/skylake/memmap.c 10 files changed, 33 insertions(+), 565 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/36136/16
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Uploaded patch set 17: Patch Set 16 was rebased.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Patch Set 17:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36136/13//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36136/13//COMMIT_MSG@17 PS13, Line 17: Tested successfully on X11SSM-F
needs retest
Done
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Patch Set 17: Code-Review+1
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Patch Set 17:
(1 comment)
Some thoughts: Some things do not need to be guessed an can be directly read from hardware registers (SA_BASE, TSEG, BDSM, GSM, ...) other things rely on guessing what the FSP did. Why not keep the calculation for things that are most certainly valid (hardware registers) and use the FSP HOB information to fill in gaps for things that cannot be read back (PRMRR?)? That allows to properly know the memory map, while still provide a correct result wrt the top of usable lower memory.
The thing is, the only way to get cbmem_top() without guessing is using the TOLUM HOB. And that already gives us cbmem_top() exactly, without having to query any of the easy registers.
We still use some of the specific locations btw. TSEG, BGSM, for instance, they show up here and there.
https://review.coreboot.org/c/coreboot/+/36136/11/src/soc/intel/skylake/memm... File src/soc/intel/skylake/memmap.c:
https://review.coreboot.org/c/coreboot/+/36136/11/src/soc/intel/skylake/memm... PS11, Line 54: * +--------------------------+ ME Stolen
this is caused by moving the skylake memmap there, we just need to find the correct one out of that […]
Found it somewhere else, both are the same thing, probably an implementation detail? I have no idea, actually, but maybe TraceHub is implemented in the ME or something?
I would prefer to keep the "Trace" name. ME stolen memory is too easily confused with the ME UMA. "Probeless Trace" btw. is also what it says in the integration guides.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Patch Set 17:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36136/11/src/soc/intel/skylake/memm... File src/soc/intel/skylake/memmap.c:
https://review.coreboot.org/c/coreboot/+/36136/11/src/soc/intel/skylake/memm... PS11, Line 54: * +--------------------------+ ME Stolen
Found it somewhere else, both are the same thing, probably an implementation detail? […]
It's not done by ME. It is just where the code is allocating address space for that functionality. It's just a good example of how the allocation is done open coded in a code base not every can see nor is the algorithm documented. Anyway, I think we're good here.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Patch Set 17:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36136/11/src/soc/intel/skylake/memm... File src/soc/intel/skylake/memmap.c:
https://review.coreboot.org/c/coreboot/+/36136/11/src/soc/intel/skylake/memm... PS11, Line 54: * +--------------------------+ ME Stolen
It's not done by ME. It is just where the code is allocating address space for that functionality. […]
Well. I'd be ok with adding both..
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Patch Set 17:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36136/3/src/soc/intel/icelake/memma... File src/soc/intel/icelake/memmap.c:
https://review.coreboot.org/c/coreboot/+/36136/3/src/soc/intel/icelake/memma... PS3, Line 54: fsp_find_bootloader_tolum
quark,apollolake,denverton_ns are left; they all don't have ebda or some equivalent mechanism
Done
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Patch Set 17: Code-Review+1
Subrata, Aaron, can someone give this a test on CML and ICL?
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Patch Set 17: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/36136/17/src/soc/intel/icelake/memm... File src/soc/intel/icelake/memmap.c:
https://review.coreboot.org/c/coreboot/+/36136/17/src/soc/intel/icelake/memm... PS17, Line 239: /* : * Check if Tseg has been initialized, we will use this as a flag : * to check if the MRC is done, and only then continue to read the : * PRMMR_BASE MSR. The system hangs if PRMRR_BASE MSR is read before : * PRMRR_MASK MSR lock bit is set. : */ : if (sa_get_tseg_base() == 0) : return NULL; Do this in a separate commit or add to the commit message? See CB:36334
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Patch Set 17:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36136/17/src/soc/intel/icelake/memm... File src/soc/intel/icelake/memmap.c:
https://review.coreboot.org/c/coreboot/+/36136/17/src/soc/intel/icelake/memm... PS17, Line 239: /* : * Check if Tseg has been initialized, we will use this as a flag : * to check if the MRC is done, and only then continue to read the : * PRMMR_BASE MSR. The system hangs if PRMRR_BASE MSR is read before : * PRMRR_MASK MSR lock bit is set. : */ : if (sa_get_tseg_base() == 0) : return NULL;
Do this in a separate commit or add to the commit message? See CB:36334
why? this is not needed anymore since we don't depend on prmrr anymore.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Patch Set 17:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36136/17/src/soc/intel/icelake/memm... File src/soc/intel/icelake/memmap.c:
https://review.coreboot.org/c/coreboot/+/36136/17/src/soc/intel/icelake/memm... PS17, Line 239: /* : * Check if Tseg has been initialized, we will use this as a flag : * to check if the MRC is done, and only then continue to read the : * PRMMR_BASE MSR. The system hangs if PRMRR_BASE MSR is read before : * PRMRR_MASK MSR lock bit is set. : */ : if (sa_get_tseg_base() == 0) : return NULL;
why? this is not needed anymore since we don't depend on prmrr anymore.
will split this off
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Patch Set 17:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36136/17//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36136/17//COMMIT_MSG@11 PS17, Line 11: aligments alig*n*ments
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Uploaded patch set 18.
Hello Aaron Durbin, Patrick Rudolph, Subrata Banik, Arthur Heymans, build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36136
to look at the new patch set (#18).
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP
Instead of doing our own calculations, rely on TOLUM returned by FSP for cbmem_top. This (hopefully) saves us from making mistakes in weird calculations of offsets and aligments.
Further this makes it easier to implement e.g. SGX PRMRR size selection via Kconfig as we do not have to make any assumptions about alignments but can simply pass (valid) values to FSP.
Tested successfully on X11SSM-F
Change-Id: If66a00d1320917bc68afb32c19db0e24c6732812 Signed-off-by: Michael Niewöhner foss@mniewoehner.de --- M src/drivers/intel/fsp2_0/hand_off_block.c M src/drivers/intel/fsp2_0/hob_verify.c M src/soc/intel/cannonlake/include/soc/ebda.h M src/soc/intel/cannonlake/memmap.c M src/soc/intel/common/block/include/intelblocks/systemagent.h M src/soc/intel/common/block/systemagent/systemagent_early.c M src/soc/intel/icelake/include/soc/ebda.h M src/soc/intel/icelake/memmap.c M src/soc/intel/skylake/include/soc/ebda.h M src/soc/intel/skylake/memmap.c 10 files changed, 33 insertions(+), 538 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/36136/18
Hello Aaron Durbin, Patrick Rudolph, Subrata Banik, Arthur Heymans, build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36136
to look at the new patch set (#19).
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP
Instead of doing our own calculations, rely on TOLUM returned by FSP for cbmem_top. This (hopefully) saves us from making mistakes in weird calculations of offsets and alignments.
Further this makes it easier to implement e.g. SGX PRMRR size selection via Kconfig as we do not have to make any assumptions about alignments but can simply pass (valid) values to FSP.
Tested successfully on X11SSM-F
Change-Id: If66a00d1320917bc68afb32c19db0e24c6732812 Signed-off-by: Michael Niewöhner foss@mniewoehner.de --- M src/drivers/intel/fsp2_0/hand_off_block.c M src/drivers/intel/fsp2_0/hob_verify.c M src/soc/intel/cannonlake/include/soc/ebda.h M src/soc/intel/cannonlake/memmap.c M src/soc/intel/common/block/include/intelblocks/systemagent.h M src/soc/intel/common/block/systemagent/systemagent_early.c M src/soc/intel/icelake/include/soc/ebda.h M src/soc/intel/icelake/memmap.c M src/soc/intel/skylake/include/soc/ebda.h M src/soc/intel/skylake/memmap.c 10 files changed, 33 insertions(+), 538 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/36136/19
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Patch Set 17:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36136/17//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36136/17//COMMIT_MSG@11 PS17, Line 11: aligments
alig*n*ments
Done
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Patch Set 20:
This change is ready for review.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Uploaded patch set 21: Patch Set 20 was rebased.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Patch Set 21: Code-Review+1
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Patch Set 21: Code-Review+2
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Patch Set 21: Code-Review+2
Tested on CFL.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Patch Set 21:
Subrata, is there anything to fix here?
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Uploaded patch set 22: Patch Set 21 was rebased.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Patch Set 22: -Code-Review
Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP
Instead of doing our own calculations, rely on TOLUM returned by FSP for cbmem_top. This (hopefully) saves us from making mistakes in weird calculations of offsets and alignments.
Further this makes it easier to implement e.g. SGX PRMRR size selection via Kconfig as we do not have to make any assumptions about alignments but can simply pass (valid) values to FSP.
Tested successfully on X11SSM-F
Change-Id: If66a00d1320917bc68afb32c19db0e24c6732812 Signed-off-by: Michael Niewöhner foss@mniewoehner.de Reviewed-on: https://review.coreboot.org/c/coreboot/+/36136 Reviewed-by: Arthur Heymans arthur@aheymans.xyz Reviewed-by: Aaron Durbin adurbin@chromium.org Reviewed-by: Nico Huber nico.h@gmx.de Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/drivers/intel/fsp2_0/hand_off_block.c M src/drivers/intel/fsp2_0/hob_verify.c M src/soc/intel/cannonlake/include/soc/ebda.h M src/soc/intel/cannonlake/memmap.c M src/soc/intel/common/block/include/intelblocks/systemagent.h M src/soc/intel/common/block/systemagent/systemagent_early.c M src/soc/intel/icelake/include/soc/ebda.h M src/soc/intel/icelake/memmap.c M src/soc/intel/skylake/include/soc/ebda.h M src/soc/intel/skylake/memmap.c 10 files changed, 33 insertions(+), 547 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved Aaron Durbin: Looks good to me, approved Arthur Heymans: Looks good to me, but someone else must approve
diff --git a/src/drivers/intel/fsp2_0/hand_off_block.c b/src/drivers/intel/fsp2_0/hand_off_block.c index 65ceb20..d2c2b78 100644 --- a/src/drivers/intel/fsp2_0/hand_off_block.c +++ b/src/drivers/intel/fsp2_0/hand_off_block.c @@ -300,3 +300,9 @@ { return fsp_find_extension_hob_by_guid(fsp_nv_storage_guid, size); } + +void fsp_find_bootloader_tolum(struct range_entry *re) +{ + if (fsp_find_range_hob(re, fsp_bootloader_tolum_guid)) + die("9.3: FSP_BOOTLOADER_TOLUM_HOB missing!\n"); +} diff --git a/src/drivers/intel/fsp2_0/hob_verify.c b/src/drivers/intel/fsp2_0/hob_verify.c index e2937d7..0c28a9a 100644 --- a/src/drivers/intel/fsp2_0/hob_verify.c +++ b/src/drivers/intel/fsp2_0/hob_verify.c @@ -16,12 +16,6 @@ #include <console/console.h> #include <fsp/util.h>
-void fsp_find_bootloader_tolum(struct range_entry *re) -{ - if (fsp_find_range_hob(re, fsp_bootloader_tolum_guid)) - die("9.3: FSP_BOOTLOADER_TOLUM_HOB missing!\n"); -} - void fsp_verify_memory_init_hobs(void) { struct range_entry fsp_mem; diff --git a/src/soc/intel/cannonlake/include/soc/ebda.h b/src/soc/intel/cannonlake/include/soc/ebda.h index ad62394..4ed6566 100644 --- a/src/soc/intel/cannonlake/include/soc/ebda.h +++ b/src/soc/intel/cannonlake/include/soc/ebda.h @@ -19,9 +19,8 @@ #include <stdint.h>
struct ebda_config { - uint32_t signature; /* 0x00 - EBDA signature */ - uint32_t tolum_base; /* 0x04 - coreboot memory start */ - uint32_t reserved_mem_size; /* 0x08 - chipset reserved memory size */ + uint32_t signature; /* EBDA signature */ + uint32_t cbmem_top; /* coreboot memory start */ };
#endif diff --git a/src/soc/intel/cannonlake/memmap.c b/src/soc/intel/cannonlake/memmap.c index 7a0d897..63e7acd 100644 --- a/src/soc/intel/cannonlake/memmap.c +++ b/src/soc/intel/cannonlake/memmap.c @@ -15,141 +15,15 @@ */
#include <arch/romstage.h> -#include <arch/ebda.h> #include <cbmem.h> -#include <console/console.h> -#include <cpu/x86/mtrr.h> -#include <cpu/x86/smm.h> -#include <device/device.h> -#include <device/pci.h> #include <fsp/util.h> #include <intelblocks/ebda.h> #include <intelblocks/systemagent.h> -#include <soc/pci_devs.h> -#include <soc/systemagent.h> #include <stdlib.h>
-#include "chip.h" - -static bool is_ptt_enable(void) -{ - if ((read32((void *)PTT_TXT_BASE_ADDRESS) & PTT_PRESENT) == - PTT_PRESENT) - return true; - - return false; -} - -/* Calculate PTT size */ -static size_t get_ptt_size(void) -{ - /* Allocate 4KB for PTT if enabled */ - return is_ptt_enable() ? 4*KiB : 0; -} - -/* Calculate ME Stolen size */ -static size_t get_imr_size(void) -{ - size_t imr_size; - - /* ME stolen memory */ - imr_size = MCHBAR32(IMRLIMIT) - MCHBAR32(IMRBASE); - - return imr_size; -} - -/* Calculate PRMRR size based on user input PRMRR size and alignment */ -static size_t get_prmrr_size(uintptr_t dram_base, - const struct soc_intel_cannonlake_config *config) -{ - uintptr_t prmrr_base = dram_base; - size_t prmrr_size; - - prmrr_size = config->PrmrrSize; - - /* Allocate PRMRR memory for C6DRAM */ - if (!prmrr_size) { - if (config->enable_c6dram) - prmrr_size = 1*MiB; - else - return 0; - } - - /* - * PRMRR Sizes that are > 1MB and < 32MB are - * not supported and will fail out. - */ - if ((prmrr_size > 1*MiB) && (prmrr_size < 32*MiB)) - die("PRMRR Sizes that are > 1MB and < 32MB are not" - "supported!\n"); - - prmrr_base -= prmrr_size; - if (prmrr_size >= 32*MiB) - prmrr_base = ALIGN_DOWN(prmrr_base, 128*MiB); - else - prmrr_base = ALIGN_DOWN(prmrr_base, 16*MiB); - /* PRMRR Area Size */ - prmrr_size = dram_base - prmrr_base; - - return prmrr_size; -} - -/* Calculate Intel Traditional Memory size based on GSM, DSM, TSEG and DPR. */ -static size_t calculate_traditional_mem_size(uintptr_t dram_base, - const struct device *dev) -{ - uintptr_t traditional_mem_base = dram_base; - size_t traditional_mem_size; - - if (dev->enabled) { - /* Read BDSM from Host Bridge */ - traditional_mem_base -= sa_get_dsm_size(); - - /* Read BGSM from Host Bridge */ - traditional_mem_base -= sa_get_gsm_size(); - } - /* Get TSEG size */ - traditional_mem_base -= sa_get_tseg_size(); - - /* Get DPR size */ - if (CONFIG(SA_ENABLE_DPR)) - traditional_mem_base -= sa_get_dpr_size(); - - /* Traditional Area Size */ - traditional_mem_size = dram_base - traditional_mem_base; - - return traditional_mem_size; -} - /* - * Calculate Intel Reserved Memory size based on - * PRMRR size, Me stolen memory and PTT selection. - */ -static size_t calculate_reserved_mem_size(uintptr_t dram_base, - const struct device *dev) -{ - uintptr_t reserve_mem_base = dram_base; - size_t reserve_mem_size; - const struct soc_intel_cannonlake_config *config; - - config = config_of(dev); - - /* Get PRMRR size */ - reserve_mem_base -= get_prmrr_size(reserve_mem_base, config); - - /* Get Tracehub size */ - reserve_mem_base -= get_imr_size(); - - /* Get PTT size */ - reserve_mem_base -= get_ptt_size(); - - /* Traditional Area Size */ - reserve_mem_size = dram_base - reserve_mem_base; - - return reserve_mem_size; -} - -/* + * Fill up memory layout information + * * Host Memory Map: * * +--------------------------+ TOUUD @@ -181,37 +55,12 @@ * the base registers from each other to determine sizes of the regions. In * other words, the memory map is in a fixed order no matter what. */ -static uintptr_t calculate_dram_base(size_t *reserved_mem_size) -{ - uintptr_t dram_base; - const struct device *dev; - - dev = pcidev_on_root(SA_DEV_SLOT_IGD, 0); - if (!dev) - die_with_post_code(POST_HW_INIT_FAILURE, - "ERROR - IGD device not found!"); - - /* Read TOLUD from Host Bridge offset */ - dram_base = sa_get_tolud_base(); - - /* Get Intel Traditional Memory Range Size */ - dram_base -= calculate_traditional_mem_size(dram_base, dev); - - /* Get Intel Reserved Memory Range Size */ - *reserved_mem_size = calculate_reserved_mem_size(dram_base, dev); - - dram_base -= *reserved_mem_size; - - return dram_base; -} - -/* Fill up memory layout information */ void fill_soc_memmap_ebda(struct ebda_config *cfg) { - size_t chipset_mem_size; + struct range_entry tolum;
- cfg->tolum_base = calculate_dram_base(&chipset_mem_size); - cfg->reserved_mem_size = chipset_mem_size; + fsp_find_bootloader_tolum(&tolum); + cfg->cbmem_top = range_entry_end(&tolum); }
void cbmem_top_init(void) @@ -253,5 +102,5 @@
retrieve_ebda_object(&ebda_cfg);
- return (void *)(uintptr_t)ebda_cfg.tolum_base; + return (void *)(uintptr_t)ebda_cfg.cbmem_top; } diff --git a/src/soc/intel/common/block/include/intelblocks/systemagent.h b/src/soc/intel/common/block/include/intelblocks/systemagent.h index ae9213c..163d97e 100644 --- a/src/soc/intel/common/block/include/intelblocks/systemagent.h +++ b/src/soc/intel/common/block/include/intelblocks/systemagent.h @@ -76,18 +76,12 @@ void enable_power_aware_intr(void); /* API to get TOLUD base address */ uintptr_t sa_get_tolud_base(void); -/* API to get DSM size */ -size_t sa_get_dsm_size(void); /* API to get GSM base address */ uintptr_t sa_get_gsm_base(void); -/* API to get GSM size */ -size_t sa_get_gsm_size(void); /* API to get TSEG base address */ uintptr_t sa_get_tseg_base(void); /* API to get TSEG size */ size_t sa_get_tseg_size(void); -/* API to get DPR size */ -size_t sa_get_dpr_size(void); /* * SoC overrides * diff --git a/src/soc/intel/common/block/systemagent/systemagent_early.c b/src/soc/intel/common/block/systemagent/systemagent_early.c index 8c89c07..d6f129d 100644 --- a/src/soc/intel/common/block/systemagent/systemagent_early.c +++ b/src/soc/intel/common/block/systemagent/systemagent_early.c @@ -139,63 +139,12 @@ return ALIGN_DOWN(pci_read_config32(SA_DEV_ROOT, TOLUD), 1*MiB); }
-static uint16_t sa_get_ggc_reg(void) -{ - return pci_read_config16(SA_DEV_ROOT, GGC); -} - -/* - * Internal Graphics Pre-allocated Memory - As per Intel FSP UPD Header - * definition, size of memory preallocatred for internal graphics can be - * configured based on below lists: - * - * 0x00:0MB, 0x01:32MB, 0x02:64MB, 0x03:96MB, 0x04:128MB, 0x05:160MB, - * 0xF0:4MB, 0xF1:8MB, 0xF2:12MB, 0xF3:16MB, 0xF4:20MB, 0xF5:24MB, 0xF6:28MB, - * 0xF7:32MB, 0xF8:36MB, 0xF9:40MB, 0xFA:44MB, 0xFB:48MB, 0xFC:52MB, 0xFD:56MB, - * 0xFE:60MB - * - * Today all existing SoCs(except Cannonlake) are supported under intel - * common code block design may not need to use any other values than 0x0-0x05 - * for GFX DSM range. DSM memory ranges between 0xF0-0xF6 are majorly for - * early SoC samples and validation requirement. This code block to justify - * all differnet possible ranges that FSP may support for a platform. - */ -size_t sa_get_dsm_size(void) -{ - uint32_t prealloc_memory; - uint16_t ggc; - - ggc = sa_get_ggc_reg(); - prealloc_memory = (ggc & G_GMS_MASK) >> G_GMS_OFFSET; - - if (prealloc_memory < 0xF0) - return prealloc_memory * 32*MiB; - else - return (prealloc_memory - 0xEF) * 4*MiB; -} - uintptr_t sa_get_gsm_base(void) { /* All regions concerned for have 1 MiB alignment. */ return ALIGN_DOWN(pci_read_config32(SA_DEV_ROOT, BGSM), 1*MiB); }
-size_t sa_get_gsm_size(void) -{ - uint8_t ggms; - - ggms = (sa_get_ggc_reg() & G_GGMS_MASK) >> G_GGMS_OFFSET; - - /* - * Size of GSM: 0x0: No Preallocated Memory 0x1: 2MB Memory - * 0x2: 4MB Memory 0x3: 8MB Memory - */ - if (ggms) - return 1*MiB << ggms; - else - return 0; -} - uintptr_t sa_get_tseg_base(void) { /* All regions concerned for have 1 MiB alignment. */ @@ -206,23 +155,3 @@ { return sa_get_gsm_base() - sa_get_tseg_base(); } - -/* - * Get DPR size in case CONFIG_SA_ENABLE_DPR is selected by SoC. - */ -size_t sa_get_dpr_size(void) -{ - uintptr_t dpr_reg; - size_t size = 0; - /* - * DMA Protected Range can be reserved below TSEG for PCODE patch - * or TXT/BootGuard related data. Rather than report a base address - * the DPR register reports the TOP of the region, which is the same - * as TSEG base. The region size is reported in MiB in bits 11:4. - */ - dpr_reg = pci_read_config32(SA_DEV_ROOT, DPR); - if (dpr_reg & DPR_EPM) - size = (dpr_reg & DPR_SIZE_MASK) << 16; - - return size; -} diff --git a/src/soc/intel/icelake/include/soc/ebda.h b/src/soc/intel/icelake/include/soc/ebda.h index f4d89e9..5cb5cdc 100644 --- a/src/soc/intel/icelake/include/soc/ebda.h +++ b/src/soc/intel/icelake/include/soc/ebda.h @@ -19,9 +19,8 @@ #include <stdint.h>
struct ebda_config { - uint32_t signature; /* 0x00 - EBDA signature */ - uint32_t tolum_base; /* 0x04 - coreboot memory start */ - uint32_t reserved_mem_size; /* 0x08 - chipset reserved memory size */ + uint32_t signature; /* EBDA signature */ + uint32_t cbmem_top; /* coreboot memory start */ };
#endif diff --git a/src/soc/intel/icelake/memmap.c b/src/soc/intel/icelake/memmap.c index 76a8128..a4fd2e8 100644 --- a/src/soc/intel/icelake/memmap.c +++ b/src/soc/intel/icelake/memmap.c @@ -14,121 +14,15 @@ */
#include <arch/romstage.h> -#include <arch/ebda.h> #include <cbmem.h> -#include <console/console.h> -#include <cpu/x86/mtrr.h> -#include <cpu/x86/smm.h> -#include <device/device.h> -#include <device/pci.h> #include <fsp/util.h> #include <intelblocks/ebda.h> #include <intelblocks/systemagent.h> -#include <soc/pci_devs.h> -#include <soc/soc_chip.h> -#include <soc/systemagent.h> #include <stdlib.h>
-/* Calculate ME Stolen size */ -static size_t get_imr_size(void) -{ - size_t imr_size; - - /* ME stolen memory */ - imr_size = MCHBAR32(IMRLIMIT) - MCHBAR32(IMRBASE); - - return imr_size; -} - -/* Calculate PRMRR size based on user input PRMRR size and alignment */ -static size_t get_prmrr_size(uintptr_t dram_base, - const struct soc_intel_icelake_config *config) -{ - uintptr_t prmrr_base = dram_base; - size_t prmrr_size; - - prmrr_size = config->PrmrrSize; - - /* Allocate PRMRR memory for C6DRAM */ - if (!prmrr_size) { - if (config->enable_c6dram) - prmrr_size = 1*MiB; - else - return 0; - } - - /* - * PRMRR Sizes that are > 1MB and < 32MB are - * not supported and will fail out. - */ - if ((prmrr_size > 1*MiB) && (prmrr_size < 32*MiB)) - die("PRMRR Sizes that are > 1MB and < 32MB are not" - "supported!\n"); - - prmrr_base -= prmrr_size; - if (prmrr_size >= 32*MiB) - prmrr_base = ALIGN_DOWN(prmrr_base, 128*MiB); - else - prmrr_base = ALIGN_DOWN(prmrr_base, 16*MiB); - /* PRMRR Area Size */ - prmrr_size = dram_base - prmrr_base; - - return prmrr_size; -} - -/* Calculate Intel Traditional Memory size based on GSM, DSM, TSEG and DPR. */ -static size_t calculate_traditional_mem_size(uintptr_t dram_base, - const struct device *dev) -{ - uintptr_t traditional_mem_base = dram_base; - size_t traditional_mem_size; - - if (dev->enabled) { - /* Read BDSM from Host Bridge */ - traditional_mem_base -= sa_get_dsm_size(); - - /* Read BGSM from Host Bridge */ - traditional_mem_base -= sa_get_gsm_size(); - } - /* Get TSEG size */ - traditional_mem_base -= sa_get_tseg_size(); - - /* Get DPR size */ - if (CONFIG(SA_ENABLE_DPR)) - traditional_mem_base -= sa_get_dpr_size(); - - /* Traditional Area Size */ - traditional_mem_size = dram_base - traditional_mem_base; - - return traditional_mem_size; -} - /* - * Calculate Intel Reserved Memory size based on - * PRMRR size, Me stolen memory and PTT selection. - */ -static size_t calculate_reserved_mem_size(uintptr_t dram_base, - const struct device *dev) -{ - uintptr_t reserve_mem_base = dram_base; - size_t reserve_mem_size; - const struct soc_intel_icelake_config *config; - - config = config_of(dev); - - /* Get PRMRR size */ - reserve_mem_base -= get_prmrr_size(reserve_mem_base, config); - - /* Get Tracehub size */ - reserve_mem_base -= get_imr_size(); - - /* Traditional Area Size */ - reserve_mem_size = dram_base - reserve_mem_base; - - return reserve_mem_size; -} - -/* + * Fill up memory layout information + * * Host Memory Map: * * +--------------------------+ TOUUD @@ -160,37 +54,12 @@ * the base registers from each other to determine sizes of the regions. In * other words, the memory map is in a fixed order no matter what. */ -static uintptr_t calculate_dram_base(size_t *reserved_mem_size) -{ - uintptr_t dram_base; - const struct device *dev; - - dev = pcidev_on_root(SA_DEV_SLOT_IGD, 0); - if (!dev) - die_with_post_code(POST_HW_INIT_FAILURE, - "ERROR - IGD device not found!"); - - /* Read TOLUD from Host Bridge offset */ - dram_base = sa_get_tolud_base(); - - /* Get Intel Traditional Memory Range Size */ - dram_base -= calculate_traditional_mem_size(dram_base, dev); - - /* Get Intel Reserved Memory Range Size */ - *reserved_mem_size = calculate_reserved_mem_size(dram_base, dev); - - dram_base -= *reserved_mem_size; - - return dram_base; -} - -/* Fill up memory layout information */ void fill_soc_memmap_ebda(struct ebda_config *cfg) { - size_t chipset_mem_size; + struct range_entry tolum;
- cfg->tolum_base = calculate_dram_base(&chipset_mem_size); - cfg->reserved_mem_size = chipset_mem_size; + fsp_find_bootloader_tolum(&tolum); + cfg->cbmem_top = range_entry_end(&tolum); }
void cbmem_top_init(void) @@ -232,5 +101,5 @@
retrieve_ebda_object(&ebda_cfg);
- return (void *)(uintptr_t)ebda_cfg.tolum_base; + return (void *)(uintptr_t)ebda_cfg.cbmem_top; } diff --git a/src/soc/intel/skylake/include/soc/ebda.h b/src/soc/intel/skylake/include/soc/ebda.h index ad62394..4ed6566 100644 --- a/src/soc/intel/skylake/include/soc/ebda.h +++ b/src/soc/intel/skylake/include/soc/ebda.h @@ -19,9 +19,8 @@ #include <stdint.h>
struct ebda_config { - uint32_t signature; /* 0x00 - EBDA signature */ - uint32_t tolum_base; /* 0x04 - coreboot memory start */ - uint32_t reserved_mem_size; /* 0x08 - chipset reserved memory size */ + uint32_t signature; /* EBDA signature */ + uint32_t cbmem_top; /* coreboot memory start */ };
#endif diff --git a/src/soc/intel/skylake/memmap.c b/src/soc/intel/skylake/memmap.c index 09dc6e9..63e7acd 100644 --- a/src/soc/intel/skylake/memmap.c +++ b/src/soc/intel/skylake/memmap.c @@ -15,148 +15,15 @@ */
#include <arch/romstage.h> -#include <arch/ebda.h> -#include <device/mmio.h> #include <cbmem.h> -#include <console/console.h> -#include <cpu/x86/mtrr.h> -#include <cpu/x86/smm.h> -#include <device/device.h> -#include <device/pci.h> +#include <fsp/util.h> #include <intelblocks/ebda.h> #include <intelblocks/systemagent.h> -#include <soc/msr.h> -#include <soc/pci_devs.h> -#include <soc/systemagent.h> #include <stdlib.h>
-#include "chip.h" - -static bool is_ptt_enable(void) -{ - if ((read32((void *)PTT_TXT_BASE_ADDRESS) & PTT_PRESENT) == - PTT_PRESENT) - return true; - - return false; -} - -/* Calculate PTT size */ -static size_t get_ptt_size(void) -{ - /* Allocate 4KB for PTT if enabled */ - return is_ptt_enable() ? 4*KiB : 0; -} - -/* Calculate Trace Hub size */ -static size_t get_tracehub_size(uintptr_t dram_base, - const struct soc_intel_skylake_config *config) -{ - uintptr_t tracehub_base = dram_base; - size_t tracehub_size = 0; - - if (!config->ProbelessTrace) - return 0; - - /* GDXC MOT */ - tracehub_base -= GDXC_MOT_MEMORY_SIZE; - /* Round down to natural boundary according to PSMI size */ - tracehub_base = ALIGN_DOWN(tracehub_base, PSMI_BUFFER_AREA_SIZE); - /* GDXC IOT */ - tracehub_base -= GDXC_IOT_MEMORY_SIZE; - /* PSMI buffer area */ - tracehub_base -= PSMI_BUFFER_AREA_SIZE; - - /* Tracehub Area Size */ - tracehub_size = dram_base - tracehub_base; - - return tracehub_size; -} - -/* Calculate PRMRR size based on user input PRMRR size and alignment */ -static size_t get_prmrr_size(uintptr_t dram_base, - const struct soc_intel_skylake_config *config) -{ - uintptr_t prmrr_base = dram_base; - size_t prmrr_size = config->PrmrrSize; - - if (!prmrr_size) - return 0; - - /* - * PRMRR Sizes that are > 1MB and < 32MB are - * not supported and will fail out. - */ - if ((prmrr_size > 1*MiB) && (prmrr_size < 32*MiB)) - die("PRMRR Sizes that are > 1MB and < 32MB are not" - "supported!\n"); - - prmrr_base -= prmrr_size; - if (prmrr_size >= 32*MiB) - prmrr_base = ALIGN_DOWN(prmrr_base, 128*MiB); - - /* PRMRR Area Size */ - prmrr_size = dram_base - prmrr_base; - - return prmrr_size; -} - -/* Calculate Intel Traditional Memory size based on GSM, DSM, TSEG and DPR. */ -static size_t calculate_traditional_mem_size(uintptr_t dram_base) -{ - const struct device *igd_dev = pcidev_path_on_root(SA_DEVFN_IGD); - uintptr_t traditional_mem_base = dram_base; - size_t traditional_mem_size; - - if (igd_dev && igd_dev->enabled) { - /* Read BDSM from Host Bridge */ - traditional_mem_base -= sa_get_dsm_size(); - - /* Read BGSM from Host Bridge */ - traditional_mem_base -= sa_get_gsm_size(); - } - /* Get TSEG size */ - traditional_mem_base -= sa_get_tseg_size(); - - /* Get DPR size */ - if (CONFIG(SA_ENABLE_DPR)) - traditional_mem_base -= sa_get_dpr_size(); - - /* Traditional Area Size */ - traditional_mem_size = dram_base - traditional_mem_base; - - return traditional_mem_size; -} - /* - * Calculate Intel Reserved Memory size based on - * PRMRR size, Trace Hub config and PTT selection. - */ -static size_t calculate_reserved_mem_size(uintptr_t dram_base) -{ - const struct device *dev = pcidev_path_on_root(SA_DEVFN_ROOT); - uintptr_t reserve_mem_base = dram_base; - size_t reserve_mem_size; - const struct soc_intel_skylake_config *config; - - config = config_of(dev); - - /* Get PRMRR size */ - reserve_mem_base -= get_prmrr_size(reserve_mem_base, config); - - /* Get Tracehub size */ - reserve_mem_base -= get_tracehub_size(reserve_mem_base, config); - - /* Get PTT size */ - reserve_mem_base -= get_ptt_size(); - - /* Traditional Area Size */ - reserve_mem_size = dram_base - reserve_mem_base; - - return reserve_mem_size; -} - -/* + * Fill up memory layout information + * * Host Memory Map: * * +--------------------------+ TOUUD @@ -174,8 +41,8 @@ * +--------------------------+ DPR * | PRM (C6DRAM/SGX) | * +--------------------------+ PRMRR - * | Trace Memory | - * +--------------------------+ Probless Trace + * | ME Stolen Memory | + * +--------------------------+ ME Stolen * | PTT | * +--------------------------+ top_of_ram * | Reserved - FSP/CBMEM | @@ -188,31 +55,12 @@ * the base registers from each other to determine sizes of the regions. In * other words, the memory map is in a fixed order no matter what. */ -static uintptr_t calculate_dram_base(size_t *reserved_mem_size) -{ - uintptr_t dram_base; - - /* Read TOLUD from Host Bridge offset */ - dram_base = sa_get_tolud_base(); - - /* Get Intel Traditional Memory Range Size */ - dram_base -= calculate_traditional_mem_size(dram_base); - - /* Get Intel Reserved Memory Range Size */ - *reserved_mem_size = calculate_reserved_mem_size(dram_base); - - dram_base -= *reserved_mem_size; - - return dram_base; -} - -/* Fill up memory layout information */ void fill_soc_memmap_ebda(struct ebda_config *cfg) { - size_t chipset_mem_size; + struct range_entry tolum;
- cfg->tolum_base = calculate_dram_base(&chipset_mem_size); - cfg->reserved_mem_size = chipset_mem_size; + fsp_find_bootloader_tolum(&tolum); + cfg->cbmem_top = range_entry_end(&tolum); }
void cbmem_top_init(void) @@ -254,5 +102,5 @@
retrieve_ebda_object(&ebda_cfg);
- return (void *)(uintptr_t)ebda_cfg.tolum_base; + return (void *)(uintptr_t)ebda_cfg.cbmem_top; }