Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34738 )
Change subject: intel/broadwell: Use smm_subregion() ......................................................................
intel/broadwell: Use smm_subregion()
Change-Id: I95f1685f9b74f68fd6cb681a614e52b8e0748216 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/soc/intel/broadwell/Kconfig M src/soc/intel/broadwell/include/soc/smm.h M src/soc/intel/broadwell/memmap.c M src/soc/intel/broadwell/smmrelocate.c 4 files changed, 57 insertions(+), 43 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/34738/1
diff --git a/src/soc/intel/broadwell/Kconfig b/src/soc/intel/broadwell/Kconfig index 696cf98..5b29728 100644 --- a/src/soc/intel/broadwell/Kconfig +++ b/src/soc/intel/broadwell/Kconfig @@ -91,10 +91,6 @@ hex default 0x800000
-config IED_REGION_SIZE - hex - default 0x400000 - config SMM_RESERVED_SIZE hex default 0x100000 diff --git a/src/soc/intel/broadwell/include/soc/smm.h b/src/soc/intel/broadwell/include/soc/smm.h index d3e1cdd..0c6f587 100644 --- a/src/soc/intel/broadwell/include/soc/smm.h +++ b/src/soc/intel/broadwell/include/soc/smm.h @@ -26,10 +26,10 @@ } __packed;
struct smm_relocation_params { - u32 smram_base; - u32 smram_size; - u32 ied_base; - u32 ied_size; + uintptr_t smram_base; + size_t smram_size; + uintptr_t ied_base; + size_t ied_size; msr_t smrr_base; msr_t smrr_mask; msr_t emrr_base; diff --git a/src/soc/intel/broadwell/memmap.c b/src/soc/intel/broadwell/memmap.c index 7c53fa6..08d7b36 100644 --- a/src/soc/intel/broadwell/memmap.c +++ b/src/soc/intel/broadwell/memmap.c @@ -16,6 +16,7 @@ #define __SIMPLE_DEVICE__
#include <cbmem.h> +#include <cpu/x86/smm.h> #include <device/pci.h> #include <device/pci_ops.h> #include <soc/pci_devs.h> @@ -46,14 +47,50 @@ return (void *) dpr_region_start(); }
-void stage_cache_external_region(void **base, size_t *size) +void smm_region(uintptr_t *start, size_t *size) { - /* The ramstage cache lives in the TSEG region. - * The top of RAM is defined to be the TSEG base address. */ - u32 offset = smm_region_size(); - offset -= CONFIG_IED_REGION_SIZE; - offset -= CONFIG_SMM_RESERVED_SIZE; + uintptr_t tseg = pci_read_config32(PCI_DEV(0, 0, 0), TSEG); + uintptr_t bgsm = pci_read_config32(PCI_DEV(0, 0, 0), BGSM);
- *base = (void *)(cbmem_top() + offset); - *size = CONFIG_SMM_RESERVED_SIZE; + *start = tseg; + *size = bgsm - tseg;; +} + +/* Intel Enhanced Debug region must be 4MB */ +#define FIXED_IED_REGION_SIZE (4 << 20) + +int smm_subregion(int sub, uintptr_t *start, size_t *size) +{ + uintptr_t sub_base; + size_t sub_size; + const size_t ied_size = FIXED_IED_REGION_SIZE; + const size_t cache_size = CONFIG_SMM_RESERVED_SIZE; + + smm_region(&sub_base, &sub_size); + + switch (sub) { + case SMM_SUBREGION_HANDLER: + /* Handler starts at the base of TSEG. */ + sub_size -= ied_size; + sub_size -= cache_size; + break; + case SMM_SUBREGION_CACHE: + /* External cache is in the middle of TSEG. */ + sub_base += sub_size - (ied_size + cache_size); + sub_size = cache_size; + break; + case SMM_SUBREGION_IED: + /* IED is at the top. */ + sub_base += sub_size - ied_size; + sub_size = ied_size; + break; + default: + *start = 0; + *size = 0; + return -1; + } + + *start = sub_base; + *size = sub_size; + return 0; } diff --git a/src/soc/intel/broadwell/smmrelocate.c b/src/soc/intel/broadwell/smmrelocate.c index 9ea73b2..7e893c1 100644 --- a/src/soc/intel/broadwell/smmrelocate.c +++ b/src/soc/intel/broadwell/smmrelocate.c @@ -179,22 +179,11 @@ } }
-static u32 northbridge_get_base_reg(struct device *dev, int reg) -{ - u32 value; - - value = pci_read_config32(dev, reg); - /* Base registers are at 1MiB granularity. */ - value &= ~((1 << 20) - 1); - return value; -} - static void fill_in_relocation_params(struct device *dev, struct smm_relocation_params *params) { - u32 tseg_size; - u32 tsegmb; - u32 bgsm; + uintptr_t tseg_base; + size_t tseg_size; u32 emrr_base; u32 emrr_size; int phys_bits; @@ -209,25 +198,17 @@ * SMRAM range as well as the IED range. However, the SMRAM available * to the handler is 4MiB since the IEDRAM lives TSEGMB + 4MiB. */ - tsegmb = northbridge_get_base_reg(dev, TSEG); - bgsm = northbridge_get_base_reg(dev, BGSM); - tseg_size = bgsm - tsegmb; - - params->smram_base = tsegmb; - params->smram_size = 4 << 20; - params->ied_base = tsegmb + params->smram_size; - params->ied_size = tseg_size - params->smram_size; - - /* Adjust available SMM handler memory size. */ - params->smram_size -= CONFIG_SMM_RESERVED_SIZE; + smm_region(&tseg_base, &tseg_size);
/* SMRR has 32-bits of valid address aligned to 4KiB. */ - params->smrr_base.lo = (params->smram_base & rmask) | MTRR_TYPE_WRBACK; + params->smrr_base.lo = (tseg_base & rmask) | MTRR_TYPE_WRBACK; params->smrr_base.hi = 0; - params->smrr_mask.lo = (~(tseg_size - 1) & rmask) - | MTRR_PHYS_MASK_VALID; + params->smrr_mask.lo = (~(tseg_size - 1) & rmask) | MTRR_PHYS_MASK_VALID; params->smrr_mask.hi = 0;
+ smm_subregion(SMM_SUBREGION_HANDLER, ¶ms->smram_base, ¶ms->smram_size); + smm_subregion(SMM_SUBREGION_IED, ¶ms->ied_base, ¶ms->ied_size); + /* The EMRR and UNCORE_EMRR are at IEDBASE + 2MiB */ emrr_base = (params->ied_base + (2 << 20)) & rmask; emrr_size = params->ied_size - (2 << 20);
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34738 )
Change subject: intel/broadwell: Use smm_subregion() ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34738/1/src/soc/intel/broadwell/mem... File src/soc/intel/broadwell/memmap.c:
https://review.coreboot.org/c/coreboot/+/34738/1/src/soc/intel/broadwell/mem... PS1, Line 56: *size = bgsm - tseg;; Statements terminations use 1 semicolon
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34738 )
Change subject: intel/broadwell: Use smm_subregion() ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34738/1/src/soc/intel/broadwell/mem... File src/soc/intel/broadwell/memmap.c:
https://review.coreboot.org/c/coreboot/+/34738/1/src/soc/intel/broadwell/mem... PS1, Line 55: tseg Not aligned to 1MiB anymore.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34738 )
Change subject: intel/broadwell: Use smm_subregion() ......................................................................
Patch Set 1: Code-Review-2
Flagging -2 until tested.
Hello Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34738
to look at the new patch set (#2).
Change subject: intel/broadwell: Use smm_subregion() ......................................................................
intel/broadwell: Use smm_subregion()
Change-Id: I95f1685f9b74f68fd6cb681a614e52b8e0748216 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/soc/intel/broadwell/include/soc/smm.h M src/soc/intel/broadwell/memmap.c M src/soc/intel/broadwell/smmrelocate.c 3 files changed, 22 insertions(+), 45 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/34738/2
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34738 )
Change subject: intel/broadwell: Use smm_subregion() ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34738/2/src/soc/intel/broadwell/mem... File src/soc/intel/broadwell/memmap.c:
https://review.coreboot.org/c/coreboot/+/34738/2/src/soc/intel/broadwell/mem... PS2, Line 57: *size = bgsm - tseg;; Statements terminations use 1 semicolon
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34738 )
Change subject: intel/broadwell: Use smm_subregion() ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34738/1/src/soc/intel/broadwell/mem... File src/soc/intel/broadwell/memmap.c:
https://review.coreboot.org/c/coreboot/+/34738/1/src/soc/intel/broadwell/mem... PS1, Line 55: tseg
Not aligned to 1MiB anymore.
Done
Hello Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34738
to look at the new patch set (#3).
Change subject: intel/broadwell: Use smm_subregion() ......................................................................
intel/broadwell: Use smm_subregion()
Change-Id: I95f1685f9b74f68fd6cb681a614e52b8e0748216 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/soc/intel/broadwell/include/soc/smm.h M src/soc/intel/broadwell/memmap.c M src/soc/intel/broadwell/pei_data.c M src/soc/intel/broadwell/smmrelocate.c 4 files changed, 22 insertions(+), 59 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/34738/3
Hello Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34738
to look at the new patch set (#4).
Change subject: intel/broadwell: Use smm_subregion() ......................................................................
intel/broadwell: Use smm_subregion()
Change-Id: I95f1685f9b74f68fd6cb681a614e52b8e0748216 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/soc/intel/broadwell/include/soc/smm.h M src/soc/intel/broadwell/memmap.c M src/soc/intel/broadwell/pei_data.c M src/soc/intel/broadwell/smmrelocate.c 4 files changed, 22 insertions(+), 59 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/34738/4
Hello Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34738
to look at the new patch set (#8).
Change subject: intel/broadwell: Use smm_subregion() ......................................................................
intel/broadwell: Use smm_subregion()
Change-Id: I95f1685f9b74f68fd6cb681a614e52b8e0748216 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/soc/intel/broadwell/include/soc/smm.h M src/soc/intel/broadwell/memmap.c M src/soc/intel/broadwell/pei_data.c M src/soc/intel/broadwell/smmrelocate.c 4 files changed, 26 insertions(+), 63 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/34738/8
Hello Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34738
to look at the new patch set (#9).
Change subject: intel/broadwell: Use smm_subregion() ......................................................................
intel/broadwell: Use smm_subregion()
Change-Id: I95f1685f9b74f68fd6cb681a614e52b8e0748216 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/soc/intel/broadwell/include/soc/msr.h M src/soc/intel/broadwell/include/soc/smm.h M src/soc/intel/broadwell/memmap.c M src/soc/intel/broadwell/pei_data.c M src/soc/intel/broadwell/smmrelocate.c 5 files changed, 58 insertions(+), 95 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/34738/9
Hello Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34738
to look at the new patch set (#10).
Change subject: intel/broadwell: Use smm_subregion() ......................................................................
intel/broadwell: Use smm_subregion()
Change-Id: I95f1685f9b74f68fd6cb681a614e52b8e0748216 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/soc/intel/broadwell/include/soc/smm.h M src/soc/intel/broadwell/memmap.c M src/soc/intel/broadwell/pei_data.c M src/soc/intel/broadwell/smmrelocate.c 4 files changed, 22 insertions(+), 59 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/34738/10
Hello Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34738
to look at the new patch set (#11).
Change subject: intel/broadwell: Use smm_subregion() ......................................................................
intel/broadwell: Use smm_subregion()
Change-Id: I95f1685f9b74f68fd6cb681a614e52b8e0748216 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/soc/intel/broadwell/include/soc/smm.h M src/soc/intel/broadwell/memmap.c M src/soc/intel/broadwell/pei_data.c M src/soc/intel/broadwell/romstage/romstage.c M src/soc/intel/broadwell/smmrelocate.c 5 files changed, 22 insertions(+), 60 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/34738/11
Kyösti Mälkki has removed a vote from this change. ( https://review.coreboot.org/c/coreboot/+/34738 )
Change subject: intel/broadwell: Use smm_subregion() ......................................................................
Removed Code-Review-2 by Kyösti Mälkki kyosti.malkki@gmail.com
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34738 )
Change subject: intel/broadwell: Use smm_subregion() ......................................................................
Patch Set 13: Code-Review+2
Kyösti Mälkki has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34738 )
Change subject: intel/broadwell: Use smm_subregion() ......................................................................
intel/broadwell: Use smm_subregion()
Change-Id: I95f1685f9b74f68fd6cb681a614e52b8e0748216 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/34738 Reviewed-by: Angel Pons th3fanbus@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/broadwell/include/soc/smm.h M src/soc/intel/broadwell/memmap.c M src/soc/intel/broadwell/pei_data.c M src/soc/intel/broadwell/romstage/romstage.c M src/soc/intel/broadwell/smmrelocate.c 5 files changed, 22 insertions(+), 60 deletions(-)
Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved
diff --git a/src/soc/intel/broadwell/include/soc/smm.h b/src/soc/intel/broadwell/include/soc/smm.h index 1cc65a5..909294c 100644 --- a/src/soc/intel/broadwell/include/soc/smm.h +++ b/src/soc/intel/broadwell/include/soc/smm.h @@ -21,10 +21,8 @@
struct smm_relocation_params { - u32 smram_base; - u32 smram_size; - u32 ied_base; - u32 ied_size; + uintptr_t ied_base; + size_t ied_size; msr_t smrr_base; msr_t smrr_mask; msr_t prmrr_base; @@ -37,15 +35,4 @@ int smm_save_state_in_msrs; };
-/* There is a bug in the order of Kconfig includes in that arch/x86/Kconfig - * is included after chipset code. This causes the chipset's Kconfig to be - * clobbered by the arch/x86/Kconfig if they have the same name. */ -static inline int smm_region_size(void) -{ - /* Make it 8MiB by default. */ - if (CONFIG_SMM_TSEG_SIZE == 0) - return (8 << 20); - return CONFIG_SMM_TSEG_SIZE; -} - #endif diff --git a/src/soc/intel/broadwell/memmap.c b/src/soc/intel/broadwell/memmap.c index 7c53fa6..f4a9d0e 100644 --- a/src/soc/intel/broadwell/memmap.c +++ b/src/soc/intel/broadwell/memmap.c @@ -16,12 +16,12 @@ #define __SIMPLE_DEVICE__
#include <cbmem.h> +#include <cpu/x86/smm.h> #include <device/pci.h> #include <device/pci_ops.h> #include <soc/pci_devs.h> #include <soc/systemagent.h> #include <soc/smm.h> -#include <stage_cache.h> #include <stdint.h>
static uintptr_t dpr_region_start(void) @@ -46,14 +46,13 @@ return (void *) dpr_region_start(); }
-void stage_cache_external_region(void **base, size_t *size) +void smm_region(uintptr_t *start, size_t *size) { - /* The ramstage cache lives in the TSEG region. - * The top of RAM is defined to be the TSEG base address. */ - u32 offset = smm_region_size(); - offset -= CONFIG_IED_REGION_SIZE; - offset -= CONFIG_SMM_RESERVED_SIZE; + uintptr_t tseg = pci_read_config32(PCI_DEV(0, 0, 0), TSEG); + uintptr_t bgsm = pci_read_config32(PCI_DEV(0, 0, 0), BGSM);
- *base = (void *)(cbmem_top() + offset); - *size = CONFIG_SMM_RESERVED_SIZE; + tseg = ALIGN_DOWN(tseg, 1 * MiB); + bgsm = ALIGN_DOWN(bgsm, 1 * MiB); + *start = tseg; + *size = bgsm - tseg; } diff --git a/src/soc/intel/broadwell/pei_data.c b/src/soc/intel/broadwell/pei_data.c index d8059ec..f745348 100644 --- a/src/soc/intel/broadwell/pei_data.c +++ b/src/soc/intel/broadwell/pei_data.c @@ -38,7 +38,7 @@ pei_data->gttbar = EARLY_GTT_BAR; pei_data->pmbase = ACPI_BASE_ADDRESS; pei_data->gpiobase = GPIO_BASE_ADDRESS; - pei_data->tseg_size = smm_region_size(); + pei_data->tseg_size = CONFIG_SMM_TSEG_SIZE; pei_data->temp_mmio_base = EARLY_TEMP_MMIO; pei_data->tx_byte = &send_to_console; pei_data->ddr_refresh_2x = 1; diff --git a/src/soc/intel/broadwell/romstage/romstage.c b/src/soc/intel/broadwell/romstage/romstage.c index b3662e1..ebb72cb 100644 --- a/src/soc/intel/broadwell/romstage/romstage.c +++ b/src/soc/intel/broadwell/romstage/romstage.c @@ -25,7 +25,6 @@ #include <elog.h> #include <program_loading.h> #include <romstage_handoff.h> -#include <stage_cache.h> #include <timestamp.h> #include <soc/gpio.h> #include <soc/me.h> diff --git a/src/soc/intel/broadwell/smmrelocate.c b/src/soc/intel/broadwell/smmrelocate.c index e16b971..21c534a 100644 --- a/src/soc/intel/broadwell/smmrelocate.c +++ b/src/soc/intel/broadwell/smmrelocate.c @@ -181,22 +181,10 @@ } }
-static u32 northbridge_get_base_reg(struct device *dev, int reg) +static void fill_in_relocation_params(struct smm_relocation_params *params) { - u32 value; - - value = pci_read_config32(dev, reg); - /* Base registers are at 1MiB granularity. */ - value &= ~((1 << 20) - 1); - return value; -} - -static void fill_in_relocation_params(struct device *dev, - struct smm_relocation_params *params) -{ - u32 tseg_size; - u32 tsegmb; - u32 bgsm; + uintptr_t tseg_base; + size_t tseg_size; u32 prmrr_base; u32 prmrr_size; int phys_bits; @@ -211,25 +199,16 @@ * SMRAM range as well as the IED range. However, the SMRAM available * to the handler is 4MiB since the IEDRAM lives TSEGMB + 4MiB. */ - tsegmb = northbridge_get_base_reg(dev, TSEG); - bgsm = northbridge_get_base_reg(dev, BGSM); - tseg_size = bgsm - tsegmb; - - params->smram_base = tsegmb; - params->smram_size = 4 << 20; - params->ied_base = tsegmb + params->smram_size; - params->ied_size = tseg_size - params->smram_size; - - /* Adjust available SMM handler memory size. */ - params->smram_size -= CONFIG_SMM_RESERVED_SIZE; + smm_region(&tseg_base, &tseg_size);
/* SMRR has 32-bits of valid address aligned to 4KiB. */ - params->smrr_base.lo = (params->smram_base & rmask) | MTRR_TYPE_WRBACK; + params->smrr_base.lo = (tseg_base & rmask) | MTRR_TYPE_WRBACK; params->smrr_base.hi = 0; - params->smrr_mask.lo = (~(tseg_size - 1) & rmask) - | MTRR_PHYS_MASK_VALID; + params->smrr_mask.lo = (~(tseg_size - 1) & rmask) | MTRR_PHYS_MASK_VALID; params->smrr_mask.hi = 0;
+ smm_subregion(SMM_SUBREGION_CHIPSET, ¶ms->ied_base, ¶ms->ied_size); + /* The PRMRR and UNCORE_PRMRR are at IEDBASE + 2MiB */ prmrr_base = (params->ied_base + (2 << 20)) & rmask; prmrr_size = params->ied_size - (2 << 20); @@ -272,16 +251,14 @@ void smm_info(uintptr_t *perm_smbase, size_t *perm_smsize, size_t *smm_save_state_size) { - struct device *dev = pcidev_path_on_root(SA_DEVFN_ROOT); - printk(BIOS_DEBUG, "Setting up SMI for CPU\n");
- fill_in_relocation_params(dev, &smm_reloc_params); + fill_in_relocation_params(&smm_reloc_params); + + smm_subregion(SMM_SUBREGION_HANDLER, perm_smbase, perm_smsize);
setup_ied_area(&smm_reloc_params);
- *perm_smbase = smm_reloc_params.smram_base; - *perm_smsize = smm_reloc_params.smram_size; *smm_save_state_size = sizeof(em64t101_smm_state_save_area_t); }