Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34740 )
Change subject: intel/smm/gen1: Use smm_subregion() ......................................................................
intel/smm/gen1: Use smm_subregion()
Change-Id: I371ed41f485b3143e47f091681198d6674928897 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/cpu/intel/model_206ax/Kconfig M src/cpu/intel/model_206ax/model_206ax.h M src/cpu/intel/smm/gen1/smi.h M src/cpu/intel/smm/gen1/smmrelocate.c M src/northbridge/intel/gm45/memmap.c M src/northbridge/intel/i945/memmap.c M src/northbridge/intel/nehalem/memmap.c M src/northbridge/intel/pineview/memmap.c M src/northbridge/intel/sandybridge/memmap.c M src/northbridge/intel/x4x/memmap.c 10 files changed, 246 insertions(+), 109 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/34740/1
diff --git a/src/cpu/intel/model_206ax/Kconfig b/src/cpu/intel/model_206ax/Kconfig index ced3340..fd46861 100644 --- a/src/cpu/intel/model_206ax/Kconfig +++ b/src/cpu/intel/model_206ax/Kconfig @@ -34,9 +34,4 @@ hex default 0x100000
-# Intel Enhanced Debug region must be 4MB -config IED_REGION_SIZE - hex - default 0x400000 - endif diff --git a/src/cpu/intel/model_206ax/model_206ax.h b/src/cpu/intel/model_206ax/model_206ax.h index 2dc9293..6fac13a 100644 --- a/src/cpu/intel/model_206ax/model_206ax.h +++ b/src/cpu/intel/model_206ax/model_206ax.h @@ -81,9 +81,12 @@ #define PSS_LATENCY_TRANSITION 10 #define PSS_LATENCY_BUSMASTER 10
+/* Intel Enhanced Debug region must be 4MB */ +#define FIXED_IED_REGION_SIZE (4 << 20) + /* Sanity check config options. */ -#if (CONFIG_SMM_TSEG_SIZE <= (CONFIG_IED_REGION_SIZE + CONFIG_SMM_RESERVED_SIZE)) -# error "CONFIG_SMM_TSEG_SIZE <= (CONFIG_IED_REGION_SIZE + CONFIG_SMM_RESERVED_SIZE)" +#if (CONFIG_SMM_TSEG_SIZE <= (FIXED_IED_REGION_SIZE + CONFIG_SMM_RESERVED_SIZE)) +# error "CONFIG_SMM_TSEG_SIZE <= (FIXED_IED_REGION_SIZE + CONFIG_SMM_RESERVED_SIZE)" #endif #if (CONFIG_SMM_TSEG_SIZE < 0x800000) # error "CONFIG_SMM_TSEG_SIZE must at least be 8MiB" @@ -91,9 +94,6 @@ #if ((CONFIG_SMM_TSEG_SIZE & (CONFIG_SMM_TSEG_SIZE - 1)) != 0) # error "CONFIG_SMM_TSEG_SIZE is not a power of 2" #endif -#if ((CONFIG_IED_REGION_SIZE & (CONFIG_IED_REGION_SIZE - 1)) != 0) -# error "CONFIG_IED_REGION_SIZE is not a power of 2" -#endif
#ifdef __SMM__ /* Lock MSRs */ diff --git a/src/cpu/intel/smm/gen1/smi.h b/src/cpu/intel/smm/gen1/smi.h index 3d5149a..961a6ec 100644 --- a/src/cpu/intel/smm/gen1/smi.h +++ b/src/cpu/intel/smm/gen1/smi.h @@ -17,8 +17,6 @@
/* These helpers are for performing SMM relocation. */ void southbridge_smm_init(void); -u32 northbridge_get_tseg_base(void); -u32 northbridge_get_tseg_size(void); void northbridge_write_smram(u8 smram);
bool cpu_has_alternative_smrr(void); diff --git a/src/cpu/intel/smm/gen1/smmrelocate.c b/src/cpu/intel/smm/gen1/smmrelocate.c index d8021e6..20a1e36 100644 --- a/src/cpu/intel/smm/gen1/smmrelocate.c +++ b/src/cpu/intel/smm/gen1/smmrelocate.c @@ -47,10 +47,10 @@
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; }; @@ -102,50 +102,26 @@
static void fill_in_relocation_params(struct smm_relocation_params *params) { + uintptr_t tseg_base; + size_t tseg_size; + /* All range registers are aligned to 4KiB */ const u32 rmask = ~((1 << 12) - 1);
- const u32 tsegmb = northbridge_get_tseg_base(); - /* TSEG base is usually aligned down (to 8MiB). So we can't - derive the TSEG size from the distance to GTT but use the - configuration value instead. */ - const u32 tseg_size = northbridge_get_tseg_size(); + smm_region(&tseg_base, &tseg_size);
- params->smram_base = tsegmb; - params->smram_size = tseg_size; - if (CONFIG_IED_REGION_SIZE != 0) { - ASSERT(params->smram_size > CONFIG_IED_REGION_SIZE); - params->smram_size -= CONFIG_IED_REGION_SIZE; - params->ied_base = tsegmb + tseg_size - CONFIG_IED_REGION_SIZE; - params->ied_size = CONFIG_IED_REGION_SIZE; - } + /* SMRR has 32-bits of valid address aligned to 4KiB. */ + 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.hi = 0;
- /* Adjust available SMM handler memory size. */ - if (CONFIG(TSEG_STAGE_CACHE)) { - ASSERT(params->smram_size > CONFIG_SMM_RESERVED_SIZE); - params->smram_size -= CONFIG_SMM_RESERVED_SIZE; - } + /* On model_6fx and model_1067x bits [0:11] on smrr_base are reserved */ + if (cpu_has_alternative_smrr()) + params->smrr_base.lo &= ~rmask;
- if (IS_ALIGNED(tsegmb, tseg_size)) { - /* SMRR has 32-bits of valid address aligned to 4KiB. */ - struct cpuinfo_x86 c; - - /* On model_6fx and model_1067x bits [0:11] on smrr_base - are reserved */ - get_fms(&c, cpuid_eax(1)); - if (cpu_has_alternative_smrr()) - params->smrr_base.lo = (params->smram_base & rmask); - else - params->smrr_base.lo = (params->smram_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.hi = 0; - } else { - printk(BIOS_WARNING, - "TSEG base not aligned with TSEG SIZE! Not setting SMRR\n"); - } + smm_subregion(SMM_SUBREGION_HANDLER, ¶ms->smram_base, ¶ms->smram_size); + smm_subregion(SMM_SUBREGION_IED, ¶ms->ied_base, ¶ms->ied_size); }
static void setup_ied_area(struct smm_relocation_params *params) @@ -185,7 +161,7 @@
fill_in_relocation_params(&smm_reloc_params);
- if (CONFIG_IED_REGION_SIZE != 0) + if (smm_reloc_params.ied_size) setup_ied_area(&smm_reloc_params);
*perm_smbase = smm_reloc_params.smram_base; @@ -220,7 +196,7 @@ printk(BIOS_DEBUG, "In relocation handler: cpu %d\n", cpu);
/* Make appropriate changes to the save state map. */ - if (CONFIG_IED_REGION_SIZE != 0) + if (relo_params->ied_size) printk(BIOS_DEBUG, "New SMBASE=0x%08x IEDBASE=0x%08x\n", smbase, iedbase); else diff --git a/src/northbridge/intel/gm45/memmap.c b/src/northbridge/intel/gm45/memmap.c index 6795f7a..17b86f7 100644 --- a/src/northbridge/intel/gm45/memmap.c +++ b/src/northbridge/intel/gm45/memmap.c @@ -24,6 +24,7 @@ #include <console/console.h> #include <cpu/intel/romstage.h> #include <cpu/x86/mtrr.h> +#include <cpu/x86/smm.h> #include <cbmem.h> #include <program_loading.h> #include <stage_cache.h> @@ -85,7 +86,7 @@ } }
-u32 northbridge_get_tseg_base(void) +static uintptr_t northbridge_get_tseg_base(void) { const pci_devfn_t dev = PCI_DEV(0, 0, 0);
@@ -108,7 +109,7 @@ return tor; }
-u32 northbridge_get_tseg_size(void) +static size_t northbridge_get_tseg_size(void) { const u8 esmramc = pci_read_config8(PCI_DEV(0, 0, 0), D0F0_ESMRAMC); return decode_tseg_size(esmramc) << 10; @@ -124,14 +125,39 @@ return (void *) top_of_ram; }
-void stage_cache_external_region(void **base, size_t *size) +void smm_region(uintptr_t *start, size_t *size) { - /* The stage cache lives at the end of the TSEG region. - * The top of RAM is defined to be the TSEG base address. - */ - *size = CONFIG_SMM_RESERVED_SIZE; - *base = (void *)((uintptr_t)northbridge_get_tseg_base() - + northbridge_get_tseg_size() - CONFIG_SMM_RESERVED_SIZE); + *start = northbridge_get_tseg_base(); + *size = northbridge_get_tseg_size(); +} + +int smm_subregion(int sub, uintptr_t *start, size_t *size) +{ + uintptr_t sub_base; + size_t sub_size; + const size_t cache_size = CONFIG_SMM_RESERVED_SIZE; + + smm_region(&sub_base, &sub_size); + + switch (sub) { + case SMM_SUBREGION_HANDLER: + /* Handler starts at the base of TSEG. */ + sub_size -= cache_size; + break; + case SMM_SUBREGION_CACHE: + /* External cache is in the middle of TSEG. */ + sub_base += sub_size - cache_size; + sub_size = cache_size; + break; + default: + *start = 0; + *size = 0; + return -1; + } + + *start = sub_base; + *size = sub_size; + return 0; }
/* platform_enter_postcar() determines the stack to use after diff --git a/src/northbridge/intel/i945/memmap.c b/src/northbridge/intel/i945/memmap.c index ac1499e..31d8e9c 100644 --- a/src/northbridge/intel/i945/memmap.c +++ b/src/northbridge/intel/i945/memmap.c @@ -23,6 +23,7 @@ #include <console/console.h> #include <cpu/intel/romstage.h> #include <cpu/x86/mtrr.h> +#include <cpu/x86/smm.h> #include <program_loading.h> #include <cpu/intel/smm/gen1/smi.h> #include <stdint.h> @@ -46,7 +47,7 @@ } }
-u32 northbridge_get_tseg_base(void) +static uintptr_t northbridge_get_tseg_base(void) { uintptr_t tom;
@@ -61,7 +62,7 @@ return tom; }
-u32 northbridge_get_tseg_size(void) +static size_t northbridge_get_tseg_size(void) { const u8 esmramc = pci_read_config8(PCI_DEV(0, 0, 0), ESMRAMC); return decode_tseg_size(esmramc); @@ -90,14 +91,40 @@ return ggc2uma[gms] << 10; }
-void stage_cache_external_region(void **base, size_t *size) + +void smm_region(uintptr_t *start, size_t *size) { - /* The stage cache lives at the end of the TSEG region. - * The top of RAM is defined to be the TSEG base address. - */ - *size = CONFIG_SMM_RESERVED_SIZE; - *base = (void *)((uintptr_t)northbridge_get_tseg_base() - + northbridge_get_tseg_size() - CONFIG_SMM_RESERVED_SIZE); + *start = northbridge_get_tseg_base(); + *size = northbridge_get_tseg_size(); +} + +int smm_subregion(int sub, uintptr_t *start, size_t *size) +{ + uintptr_t sub_base; + size_t sub_size; + const size_t cache_size = CONFIG_SMM_RESERVED_SIZE; + + smm_region(&sub_base, &sub_size); + + switch (sub) { + case SMM_SUBREGION_HANDLER: + /* Handler starts at the base of TSEG. */ + sub_size -= cache_size; + break; + case SMM_SUBREGION_CACHE: + /* External cache is in the middle of TSEG. */ + sub_base += sub_size - cache_size; + sub_size = cache_size; + break; + default: + *start = 0; + *size = 0; + return -1; + } + + *start = sub_base; + *size = sub_size; + return 0; }
/* platform_enter_postcar() determines the stack to use after diff --git a/src/northbridge/intel/nehalem/memmap.c b/src/northbridge/intel/nehalem/memmap.c index ec036c9..b97e5c1 100644 --- a/src/northbridge/intel/nehalem/memmap.c +++ b/src/northbridge/intel/nehalem/memmap.c @@ -22,6 +22,7 @@ #include <console/console.h> #include <cpu/intel/romstage.h> #include <cpu/x86/mtrr.h> +#include <cpu/x86/smm.h> #include <program_loading.h> #include <stage_cache.h> #include <cpu/intel/smm/gen1/smi.h> @@ -34,12 +35,12 @@ return tom; }
-u32 northbridge_get_tseg_base(void) +static uintptr_t northbridge_get_tseg_base(void) { - return (u32)smm_region_start(); + return smm_region_start(); }
-u32 northbridge_get_tseg_size(void) +static size_t northbridge_get_tseg_size(void) { return CONFIG_SMM_TSEG_SIZE; } @@ -49,13 +50,39 @@ return (void *) smm_region_start(); }
-void stage_cache_external_region(void **base, size_t *size) +void smm_region(uintptr_t *start, size_t *size) { - /* The stage cache lives at the end of TSEG region. - * The top of RAM is defined to be the TSEG base address. */ - *size = CONFIG_SMM_RESERVED_SIZE; - *base = (void *)((uintptr_t)northbridge_get_tseg_base() + - northbridge_get_tseg_size() - CONFIG_SMM_RESERVED_SIZE); + *start = northbridge_get_tseg_base(); + *size = northbridge_get_tseg_size(); +} + +int smm_subregion(int sub, uintptr_t *start, size_t *size) +{ + uintptr_t sub_base; + size_t sub_size; + const size_t cache_size = CONFIG_SMM_RESERVED_SIZE; + + smm_region(&sub_base, &sub_size); + + switch (sub) { + case SMM_SUBREGION_HANDLER: + /* Handler starts at the base of TSEG. */ + sub_size -= cache_size; + break; + case SMM_SUBREGION_CACHE: + /* External cache is in the middle of TSEG. */ + sub_base += sub_size - cache_size; + sub_size = cache_size; + break; + default: + *start = 0; + *size = 0; + return -1; + } + + *start = sub_base; + *size = sub_size; + return 0; }
/* platform_enter_postcar() determines the stack to use after diff --git a/src/northbridge/intel/pineview/memmap.c b/src/northbridge/intel/pineview/memmap.c index 2f3ff6e..1a5d58a 100644 --- a/src/northbridge/intel/pineview/memmap.c +++ b/src/northbridge/intel/pineview/memmap.c @@ -24,6 +24,7 @@ #include <cbmem.h> #include <northbridge/intel/pineview/pineview.h> #include <cpu/x86/mtrr.h> +#include <cpu/x86/smm.h> #include <cpu/intel/romstage.h> #include <cpu/intel/smm/gen1/smi.h> #include <stdint.h> @@ -117,13 +118,13 @@ } }
-u32 northbridge_get_tseg_size(void) +static size_t northbridge_get_tseg_size(void) { const u8 esmramc = pci_read_config8(PCI_DEV(0, 0, 0), ESMRAMC); return decode_tseg_size(esmramc); }
-u32 northbridge_get_tseg_base(void) +static uintptr_t northbridge_get_tseg_base(void) { return pci_read_config32(PCI_DEV(0, 0, 0), TSEG); } @@ -140,14 +141,39 @@
}
-void stage_cache_external_region(void **base, size_t *size) +void smm_region(uintptr_t *start, size_t *size) { - /* The stage cache lives at the end of the TSEG region. - * The top of RAM is defined to be the TSEG base address. - */ - *size = CONFIG_SMM_RESERVED_SIZE; - *base = (void *)((uintptr_t)northbridge_get_tseg_base() - + northbridge_get_tseg_size() - CONFIG_SMM_RESERVED_SIZE); + *start = northbridge_get_tseg_base(); + *size = northbridge_get_tseg_size(); +} + +int smm_subregion(int sub, uintptr_t *start, size_t *size) +{ + uintptr_t sub_base; + size_t sub_size; + const size_t cache_size = CONFIG_SMM_RESERVED_SIZE; + + smm_region(&sub_base, &sub_size); + + switch (sub) { + case SMM_SUBREGION_HANDLER: + /* Handler starts at the base of TSEG. */ + sub_size -= cache_size; + break; + case SMM_SUBREGION_CACHE: + /* External cache is in the middle of TSEG. */ + sub_base += sub_size - cache_size; + sub_size = cache_size; + break; + default: + *start = 0; + *size = 0; + return -1; + } + + *start = sub_base; + *size = sub_size; + return 0; }
/* platform_enter_postcar() determines the stack to use after diff --git a/src/northbridge/intel/sandybridge/memmap.c b/src/northbridge/intel/sandybridge/memmap.c index 7d5c173..815e1d6 100644 --- a/src/northbridge/intel/sandybridge/memmap.c +++ b/src/northbridge/intel/sandybridge/memmap.c @@ -22,6 +22,7 @@ #include <cpu/intel/romstage.h> #include <cpu/intel/smm/gen1/smi.h> #include <cpu/x86/mtrr.h> +#include <cpu/x86/smm.h> #include <program_loading.h> #include <stage_cache.h> #include "sandybridge.h" @@ -38,23 +39,57 @@ return (void *) smm_region_start(); }
-u32 northbridge_get_tseg_base(void) +static uintptr_t northbridge_get_tseg_base(void) { return ALIGN_DOWN(smm_region_start(), 1*MiB); }
-u32 northbridge_get_tseg_size(void) +static size_t northbridge_get_tseg_size(void) { return CONFIG_SMM_TSEG_SIZE; }
-void stage_cache_external_region(void **base, size_t *size) +void smm_region(uintptr_t *start, size_t *size) { - /* The stage cache lives at the end of TSEG region. - * The top of RAM is defined to be the TSEG base address. */ - *size = CONFIG_SMM_RESERVED_SIZE; - *base = (void *)((uintptr_t)northbridge_get_tseg_base() + northbridge_get_tseg_size() - - CONFIG_IED_REGION_SIZE - CONFIG_SMM_RESERVED_SIZE); + *start = northbridge_get_tseg_base(); + *size = northbridge_get_tseg_size(); +} + +int smm_subregion(int sub, uintptr_t *start, size_t *size) +{ + uintptr_t sub_base; + size_t sub_size; + const size_t 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; }
/* platform_enter_postcar() determines the stack to use after diff --git a/src/northbridge/intel/x4x/memmap.c b/src/northbridge/intel/x4x/memmap.c index dda8387..fc4bd51 100644 --- a/src/northbridge/intel/x4x/memmap.c +++ b/src/northbridge/intel/x4x/memmap.c @@ -26,6 +26,7 @@ #include <console/console.h> #include <cpu/intel/romstage.h> #include <cpu/x86/mtrr.h> +#include <cpu/x86/smm.h> #include <northbridge/intel/x4x/x4x.h> #include <program_loading.h> #include <cpu/intel/smm/gen1/smi.h> @@ -113,13 +114,13 @@ return 1; }
-u32 northbridge_get_tseg_size(void) +static size_t northbridge_get_tseg_size(void) { const u8 esmramc = pci_read_config8(PCI_DEV(0, 0, 0), D0F0_ESMRAMC); return decode_tseg_size(esmramc); }
-u32 northbridge_get_tseg_base(void) +static uintptr_t northbridge_get_tseg_base(void) { return pci_read_config32(PCI_DEV(0, 0, 0), D0F0_TSEG); } @@ -135,16 +136,42 @@ return (void *) top_of_ram; }
-void stage_cache_external_region(void **base, size_t *size) +void smm_region(uintptr_t *start, size_t *size) { - /* The stage cache lives at the end of the TSEG region. - * The top of RAM is defined to be the TSEG base address. - */ - *size = CONFIG_SMM_RESERVED_SIZE; - *base = (void *)((uintptr_t)northbridge_get_tseg_base() - + northbridge_get_tseg_size() - CONFIG_SMM_RESERVED_SIZE); + *start = northbridge_get_tseg_base(); + *size = northbridge_get_tseg_size(); }
+int smm_subregion(int sub, uintptr_t *start, size_t *size) +{ + uintptr_t sub_base; + size_t sub_size; + const size_t cache_size = CONFIG_SMM_RESERVED_SIZE; + + smm_region(&sub_base, &sub_size); + + switch (sub) { + case SMM_SUBREGION_HANDLER: + /* Handler starts at the base of TSEG. */ + sub_size -= cache_size; + break; + case SMM_SUBREGION_CACHE: + /* External cache is in the middle of TSEG. */ + sub_base += sub_size - cache_size; + sub_size = cache_size; + break; + default: + *start = 0; + *size = 0; + return -1; + } + + *start = sub_base; + *size = sub_size; + return 0; +} + + /* platform_enter_postcar() determines the stack to use after * cache-as-ram is torn down as well as the MTRR settings to use, * and continues execution in postcar stage. */
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34740 )
Change subject: intel/smm/gen1: Use smm_subregion() ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34740/1/src/cpu/intel/smm/gen1/smmr... File src/cpu/intel/smm/gen1/smmrelocate.c:
https://review.coreboot.org/c/coreboot/+/34740/1/src/cpu/intel/smm/gen1/smmr... PS1, Line 129: I'd not remove this check. If this is not the case, I recall there were some weird issues. On older systems it could happen with some gttsize/gfxsize and tseg size settings that the TSEG base is not aligned. Currently a nice value is typically chosen in the raminit for this to not happen.
https://review.coreboot.org/c/coreboot/+/34740/1/src/northbridge/intel/gm45/... File src/northbridge/intel/gm45/memmap.c:
https://review.coreboot.org/c/coreboot/+/34740/1/src/northbridge/intel/gm45/... PS1, Line 140: sub_size ASSERT(sub_size > cache_size) somewhere?
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34740 )
Change subject: intel/smm/gen1: Use smm_subregion() ......................................................................
Patch Set 1: Code-Review-2
Flagging -2 until tested.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34740 )
Change subject: intel/smm/gen1: Use smm_subregion() ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34740/1/src/cpu/intel/smm/gen1/smmr... File src/cpu/intel/smm/gen1/smmrelocate.c:
https://review.coreboot.org/c/coreboot/+/34740/1/src/cpu/intel/smm/gen1/smmr... PS1, Line 129:
I'd not remove this check. If this is not the case, I recall there were some weird issues. […]
If setup is wrong, is worth coming to ramstage at all? Fail even before postcar, because stage cache would attempt to access TSEG earlier.
https://review.coreboot.org/c/coreboot/+/34740/1/src/northbridge/intel/gm45/... File src/northbridge/intel/gm45/memmap.c:
https://review.coreboot.org/c/coreboot/+/34740/1/src/northbridge/intel/gm45/... PS1, Line 140: sub_size
ASSERT(sub_size > cache_size) somewhere?
Ack
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34740 )
Change subject: intel/smm/gen1: Use smm_subregion() ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34740/1/src/cpu/intel/smm/gen1/smmr... File src/cpu/intel/smm/gen1/smmrelocate.c:
https://review.coreboot.org/c/coreboot/+/34740/1/src/cpu/intel/smm/gen1/smmr... PS1, Line 129:
If setup is wrong, is worth coming to ramstage at all? Fail even before postcar, because stage cache […]
Those setups are not wrong, per se. It just that in some cases (that we now avoid) this MTRR cannot be used to cover TSEG due to the impossibility of alignment. It seems better to check for that before setting up SMRR.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34740 )
Change subject: intel/smm/gen1: Use smm_subregion() ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34740/1/src/cpu/intel/smm/gen1/smmr... File src/cpu/intel/smm/gen1/smmrelocate.c:
https://review.coreboot.org/c/coreboot/+/34740/1/src/cpu/intel/smm/gen1/smmr... PS1, Line 129:
Those setups are not wrong, per se. […]
CB:34776 puts an early assert.
IMHO we cannot just leave SMRR unprogrammed and continue. Weird cases whether SMI handler gets locked or not, or whether even SMI can access TSEG.
Hello Patrick Rudolph, build bot (Jenkins), Damien Zammit,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34740
to look at the new patch set (#7).
Change subject: intel/smm/gen1: Use smm_subregion() ......................................................................
intel/smm/gen1: Use smm_subregion()
Change-Id: I371ed41f485b3143e47f091681198d6674928897 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/cpu/intel/smm/gen1/smi.h M src/cpu/intel/smm/gen1/smmrelocate.c M src/northbridge/intel/gm45/memmap.c M src/northbridge/intel/i945/memmap.c M src/northbridge/intel/nehalem/memmap.c M src/northbridge/intel/pineview/memmap.c M src/northbridge/intel/sandybridge/memmap.c M src/northbridge/intel/x4x/memmap.c 8 files changed, 57 insertions(+), 108 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/34740/7
Hello Patrick Rudolph, build bot (Jenkins), Damien Zammit,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34740
to look at the new patch set (#8).
Change subject: intel/smm/gen1: Use smm_subregion() ......................................................................
intel/smm/gen1: Use smm_subregion()
Change-Id: I371ed41f485b3143e47f091681198d6674928897 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/cpu/intel/smm/gen1/smmrelocate.c M src/include/cpu/intel/smm_reloc.h M src/northbridge/intel/gm45/memmap.c M src/northbridge/intel/i945/memmap.c M src/northbridge/intel/nehalem/memmap.c M src/northbridge/intel/pineview/memmap.c M src/northbridge/intel/sandybridge/memmap.c M src/northbridge/intel/x4x/memmap.c 8 files changed, 57 insertions(+), 108 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/34740/8
Hello Patrick Rudolph, build bot (Jenkins), Damien Zammit,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34740
to look at the new patch set (#9).
Change subject: intel/smm/gen1: Use smm_subregion() ......................................................................
intel/smm/gen1: Use smm_subregion()
Change-Id: I371ed41f485b3143e47f091681198d6674928897 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/cpu/intel/smm/gen1/smmrelocate.c M src/include/cpu/intel/smm_reloc.h M src/northbridge/intel/gm45/memmap.c M src/northbridge/intel/i945/memmap.c M src/northbridge/intel/nehalem/memmap.c M src/northbridge/intel/pineview/memmap.c M src/northbridge/intel/sandybridge/memmap.c M src/northbridge/intel/x4x/memmap.c 8 files changed, 62 insertions(+), 107 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/34740/9
Hello Patrick Rudolph, build bot (Jenkins), Damien Zammit,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34740
to look at the new patch set (#10).
Change subject: intel/smm/gen1: Use smm_subregion() ......................................................................
intel/smm/gen1: Use smm_subregion()
Change-Id: I371ed41f485b3143e47f091681198d6674928897 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/cpu/intel/smm/gen1/smmrelocate.c M src/include/cpu/intel/smm_reloc.h M src/northbridge/intel/gm45/memmap.c M src/northbridge/intel/i945/memmap.c M src/northbridge/intel/nehalem/memmap.c M src/northbridge/intel/pineview/memmap.c M src/northbridge/intel/sandybridge/memmap.c M src/northbridge/intel/x4x/memmap.c 8 files changed, 62 insertions(+), 107 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/34740/10
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34740 )
Change subject: intel/smm/gen1: Use smm_subregion() ......................................................................
Patch Set 10: Code-Review+2
(1 comment)
Tested on Lenovo X220. SMI handler properly installed and external IMD properly initialized and used.
https://review.coreboot.org/c/coreboot/+/34740/10/src/cpu/intel/smm/gen1/smm... File src/cpu/intel/smm/gen1/smmrelocate.c:
https://review.coreboot.org/c/coreboot/+/34740/10/src/cpu/intel/smm/gen1/smm... PS10, Line 238: I guess writing 0 to the SMRR does not have any functional effects?
Hello Patrick Rudolph, Arthur Heymans, build bot (Jenkins), Damien Zammit,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34740
to look at the new patch set (#11).
Change subject: intel/smm/gen1: Use smm_subregion() ......................................................................
intel/smm/gen1: Use smm_subregion()
Change-Id: I371ed41f485b3143e47f091681198d6674928897 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/cpu/intel/smm/gen1/smmrelocate.c M src/include/cpu/intel/smm_reloc.h M src/northbridge/intel/gm45/memmap.c M src/northbridge/intel/i945/memmap.c M src/northbridge/intel/nehalem/memmap.c M src/northbridge/intel/pineview/memmap.c M src/northbridge/intel/sandybridge/memmap.c M src/northbridge/intel/x4x/memmap.c 8 files changed, 62 insertions(+), 107 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/34740/11
Kyösti Mälkki has removed a vote from this change. ( https://review.coreboot.org/c/coreboot/+/34740 )
Change subject: intel/smm/gen1: Use smm_subregion() ......................................................................
Removed Code-Review-2 by Kyösti Mälkki kyosti.malkki@gmail.com
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34740 )
Change subject: intel/smm/gen1: Use smm_subregion() ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34740/10/src/cpu/intel/smm/gen1/smm... File src/cpu/intel/smm/gen1/smmrelocate.c:
https://review.coreboot.org/c/coreboot/+/34740/10/src/cpu/intel/smm/gen1/smm... PS10, Line 238:
I guess writing 0 to the SMRR does not have any functional effects?
There is MTRR_PHYS_MASK_VALID bit that remains unset.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34740 )
Change subject: intel/smm/gen1: Use smm_subregion() ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34740/10/src/cpu/intel/smm/gen1/smm... File src/cpu/intel/smm/gen1/smmrelocate.c:
https://review.coreboot.org/c/coreboot/+/34740/10/src/cpu/intel/smm/gen1/smm... PS10, Line 238:
There is MTRR_PHYS_MASK_VALID bit that remains unset.
Ack
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34740 )
Change subject: intel/smm/gen1: Use smm_subregion() ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34740/1/src/cpu/intel/smm/gen1/smmr... File src/cpu/intel/smm/gen1/smmrelocate.c:
https://review.coreboot.org/c/coreboot/+/34740/1/src/cpu/intel/smm/gen1/smmr... PS1, Line 129:
CB:34776 puts an early assert. […]
SMRR is programmed with MTRR_PHYS_MASK_VALID_BIT unset.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34740 )
Change subject: intel/smm/gen1: 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/+/34740 )
Change subject: intel/smm/gen1: Use smm_subregion() ......................................................................
intel/smm/gen1: Use smm_subregion()
Change-Id: I371ed41f485b3143e47f091681198d6674928897 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/34740 Reviewed-by: Angel Pons th3fanbus@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/cpu/intel/smm/gen1/smmrelocate.c M src/include/cpu/intel/smm_reloc.h M src/northbridge/intel/gm45/memmap.c M src/northbridge/intel/i945/memmap.c M src/northbridge/intel/nehalem/memmap.c M src/northbridge/intel/pineview/memmap.c M src/northbridge/intel/sandybridge/memmap.c M src/northbridge/intel/x4x/memmap.c 8 files changed, 62 insertions(+), 107 deletions(-)
Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved
diff --git a/src/cpu/intel/smm/gen1/smmrelocate.c b/src/cpu/intel/smm/gen1/smmrelocate.c index 9b0175d..0a477bb 100644 --- a/src/cpu/intel/smm/gen1/smmrelocate.c +++ b/src/cpu/intel/smm/gen1/smmrelocate.c @@ -43,10 +43,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; }; @@ -103,50 +101,31 @@
static void fill_in_relocation_params(struct smm_relocation_params *params) { + uintptr_t tseg_base; + size_t tseg_size; + /* All range registers are aligned to 4KiB */ const u32 rmask = ~((1 << 12) - 1);
- const u32 tsegmb = northbridge_get_tseg_base(); - /* TSEG base is usually aligned down (to 8MiB). So we can't - derive the TSEG size from the distance to GTT but use the - configuration value instead. */ - const u32 tseg_size = northbridge_get_tseg_size(); + smm_region(&tseg_base, &tseg_size);
- params->smram_base = tsegmb; - params->smram_size = tseg_size; - if (CONFIG_IED_REGION_SIZE != 0) { - ASSERT(params->smram_size > CONFIG_IED_REGION_SIZE); - params->smram_size -= CONFIG_IED_REGION_SIZE; - params->ied_base = tsegmb + tseg_size - CONFIG_IED_REGION_SIZE; - params->ied_size = CONFIG_IED_REGION_SIZE; - } - - /* Adjust available SMM handler memory size. */ - if (CONFIG(TSEG_STAGE_CACHE)) { - ASSERT(params->smram_size > CONFIG_SMM_RESERVED_SIZE); - params->smram_size -= CONFIG_SMM_RESERVED_SIZE; - } - - if (IS_ALIGNED(tsegmb, tseg_size)) { - /* SMRR has 32-bits of valid address aligned to 4KiB. */ - struct cpuinfo_x86 c; - - /* On model_6fx and model_1067x bits [0:11] on smrr_base - are reserved */ - get_fms(&c, cpuid_eax(1)); - if (cpu_has_alternative_smrr()) - params->smrr_base.lo = (params->smram_base & rmask); - else - params->smrr_base.lo = (params->smram_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.hi = 0; - } else { + if (!IS_ALIGNED(tseg_base, tseg_size)) { printk(BIOS_WARNING, "TSEG base not aligned with TSEG SIZE! Not setting SMRR\n"); + return; } + + /* SMRR has 32-bits of valid address aligned to 4KiB. */ + 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.hi = 0; + + /* On model_6fx and model_1067x bits [0:11] on smrr_base are reserved */ + if (cpu_has_alternative_smrr()) + params->smrr_base.lo &= ~rmask; + + smm_subregion(SMM_SUBREGION_CHIPSET, ¶ms->ied_base, ¶ms->ied_size); }
static void setup_ied_area(struct smm_relocation_params *params) @@ -186,11 +165,11 @@
fill_in_relocation_params(&smm_reloc_params);
- if (CONFIG_IED_REGION_SIZE != 0) + smm_subregion(SMM_SUBREGION_HANDLER, perm_smbase, perm_smsize); + + if (smm_reloc_params.ied_size) 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); }
@@ -221,7 +200,7 @@ printk(BIOS_DEBUG, "In relocation handler: cpu %d\n", cpu);
/* Make appropriate changes to the save state map. */ - if (CONFIG_IED_REGION_SIZE != 0) + if (relo_params->ied_size) printk(BIOS_DEBUG, "New SMBASE=0x%08x IEDBASE=0x%08x\n", smbase, iedbase); else @@ -235,7 +214,7 @@
/* Write EMRR and SMRR MSRs based on indicated support. */ mtrr_cap = rdmsr(MTRR_CAP_MSR); - if (!(mtrr_cap.lo & SMRR_SUPPORTED && relo_params->smrr_mask.lo != 0)) + if (!(mtrr_cap.lo & SMRR_SUPPORTED)) return;
if (cpu_has_alternative_smrr()) diff --git a/src/include/cpu/intel/smm_reloc.h b/src/include/cpu/intel/smm_reloc.h index 5213bc90..cb196fc 100644 --- a/src/include/cpu/intel/smm_reloc.h +++ b/src/include/cpu/intel/smm_reloc.h @@ -23,8 +23,6 @@ } __packed;
/* These helpers are for performing SMM relocation. */ -u32 northbridge_get_tseg_base(void); -u32 northbridge_get_tseg_size(void); void northbridge_write_smram(u8 smram);
void smm_lock(void); diff --git a/src/northbridge/intel/gm45/memmap.c b/src/northbridge/intel/gm45/memmap.c index 6e2f703..7479a78 100644 --- a/src/northbridge/intel/gm45/memmap.c +++ b/src/northbridge/intel/gm45/memmap.c @@ -23,9 +23,9 @@ #include <device/pci_def.h> #include <console/console.h> #include <cpu/x86/mtrr.h> +#include <cpu/x86/smm.h> #include <cbmem.h> #include <program_loading.h> -#include <stage_cache.h> #include <cpu/intel/smm_reloc.h> #include "gm45.h"
@@ -84,7 +84,7 @@ } }
-u32 northbridge_get_tseg_base(void) +static uintptr_t northbridge_get_tseg_base(void) { const pci_devfn_t dev = PCI_DEV(0, 0, 0);
@@ -107,7 +107,7 @@ return tor; }
-u32 northbridge_get_tseg_size(void) +static size_t northbridge_get_tseg_size(void) { const u8 esmramc = pci_read_config8(PCI_DEV(0, 0, 0), D0F0_ESMRAMC); return decode_tseg_size(esmramc) << 10; @@ -123,14 +123,10 @@ return (void *) top_of_ram; }
-void stage_cache_external_region(void **base, size_t *size) +void smm_region(uintptr_t *start, size_t *size) { - /* The stage cache lives at the end of the TSEG region. - * The top of RAM is defined to be the TSEG base address. - */ - *size = CONFIG_SMM_RESERVED_SIZE; - *base = (void *)((uintptr_t)northbridge_get_tseg_base() - + northbridge_get_tseg_size() - CONFIG_SMM_RESERVED_SIZE); + *start = northbridge_get_tseg_base(); + *size = northbridge_get_tseg_size(); }
void fill_postcar_frame(struct postcar_frame *pcf) diff --git a/src/northbridge/intel/i945/memmap.c b/src/northbridge/intel/i945/memmap.c index 8179f17..8207d06 100644 --- a/src/northbridge/intel/i945/memmap.c +++ b/src/northbridge/intel/i945/memmap.c @@ -22,10 +22,10 @@ #include "i945.h" #include <console/console.h> #include <cpu/x86/mtrr.h> +#include <cpu/x86/smm.h> #include <program_loading.h> #include <cpu/intel/smm_reloc.h> #include <stdint.h> -#include <stage_cache.h>
/* Decodes TSEG region size to bytes. */ u32 decode_tseg_size(const u8 esmramc) @@ -45,7 +45,7 @@ } }
-u32 northbridge_get_tseg_base(void) +static uintptr_t northbridge_get_tseg_base(void) { uintptr_t tom;
@@ -60,7 +60,7 @@ return tom; }
-u32 northbridge_get_tseg_size(void) +static size_t northbridge_get_tseg_size(void) { const u8 esmramc = pci_read_config8(PCI_DEV(0, 0, 0), ESMRAMC); return decode_tseg_size(esmramc); @@ -89,14 +89,10 @@ return ggc2uma[gms] << 10; }
-void stage_cache_external_region(void **base, size_t *size) +void smm_region(uintptr_t *start, size_t *size) { - /* The stage cache lives at the end of the TSEG region. - * The top of RAM is defined to be the TSEG base address. - */ - *size = CONFIG_SMM_RESERVED_SIZE; - *base = (void *)((uintptr_t)northbridge_get_tseg_base() - + northbridge_get_tseg_size() - CONFIG_SMM_RESERVED_SIZE); + *start = northbridge_get_tseg_base(); + *size = northbridge_get_tseg_size(); }
void fill_postcar_frame(struct postcar_frame *pcf) diff --git a/src/northbridge/intel/nehalem/memmap.c b/src/northbridge/intel/nehalem/memmap.c index 1687ddf..fd10542 100644 --- a/src/northbridge/intel/nehalem/memmap.c +++ b/src/northbridge/intel/nehalem/memmap.c @@ -21,8 +21,8 @@ #include <cbmem.h> #include <console/console.h> #include <cpu/x86/mtrr.h> +#include <cpu/x86/smm.h> #include <program_loading.h> -#include <stage_cache.h> #include <cpu/intel/smm_reloc.h> #include "nehalem.h"
@@ -33,12 +33,12 @@ return tom; }
-u32 northbridge_get_tseg_base(void) +static uintptr_t northbridge_get_tseg_base(void) { - return (u32)smm_region_start(); + return smm_region_start(); }
-u32 northbridge_get_tseg_size(void) +static size_t northbridge_get_tseg_size(void) { return CONFIG_SMM_TSEG_SIZE; } @@ -48,13 +48,10 @@ return (void *) smm_region_start(); }
-void stage_cache_external_region(void **base, size_t *size) +void smm_region(uintptr_t *start, size_t *size) { - /* The stage cache lives at the end of TSEG region. - * The top of RAM is defined to be the TSEG base address. */ - *size = CONFIG_SMM_RESERVED_SIZE; - *base = (void *)((uintptr_t)northbridge_get_tseg_base() + - northbridge_get_tseg_size() - CONFIG_SMM_RESERVED_SIZE); + *start = northbridge_get_tseg_base(); + *size = northbridge_get_tseg_size(); }
void fill_postcar_frame(struct postcar_frame *pcf) diff --git a/src/northbridge/intel/pineview/memmap.c b/src/northbridge/intel/pineview/memmap.c index 9908f11..b4fef6b 100644 --- a/src/northbridge/intel/pineview/memmap.c +++ b/src/northbridge/intel/pineview/memmap.c @@ -24,9 +24,9 @@ #include <cbmem.h> #include <northbridge/intel/pineview/pineview.h> #include <cpu/x86/mtrr.h> +#include <cpu/x86/smm.h> #include <cpu/intel/smm_reloc.h> #include <stdint.h> -#include <stage_cache.h>
u8 decode_pciebar(u32 *const base, u32 *const len) { @@ -116,13 +116,13 @@ } }
-u32 northbridge_get_tseg_size(void) +static size_t northbridge_get_tseg_size(void) { const u8 esmramc = pci_read_config8(PCI_DEV(0, 0, 0), ESMRAMC); return decode_tseg_size(esmramc); }
-u32 northbridge_get_tseg_base(void) +static uintptr_t northbridge_get_tseg_base(void) { return pci_read_config32(PCI_DEV(0, 0, 0), TSEG); } @@ -139,14 +139,10 @@
}
-void stage_cache_external_region(void **base, size_t *size) +void smm_region(uintptr_t *start, size_t *size) { - /* The stage cache lives at the end of the TSEG region. - * The top of RAM is defined to be the TSEG base address. - */ - *size = CONFIG_SMM_RESERVED_SIZE; - *base = (void *)((uintptr_t)northbridge_get_tseg_base() - + northbridge_get_tseg_size() - CONFIG_SMM_RESERVED_SIZE); + *start = northbridge_get_tseg_base(); + *size = northbridge_get_tseg_size(); }
void fill_postcar_frame(struct postcar_frame *pcf) diff --git a/src/northbridge/intel/sandybridge/memmap.c b/src/northbridge/intel/sandybridge/memmap.c index fa29b37..95bf458 100644 --- a/src/northbridge/intel/sandybridge/memmap.c +++ b/src/northbridge/intel/sandybridge/memmap.c @@ -21,8 +21,8 @@ #include <console/console.h> #include <cpu/intel/smm_reloc.h> #include <cpu/x86/mtrr.h> +#include <cpu/x86/smm.h> #include <program_loading.h> -#include <stage_cache.h> #include "sandybridge.h"
static uintptr_t smm_region_start(void) @@ -37,23 +37,20 @@ return (void *) smm_region_start(); }
-u32 northbridge_get_tseg_base(void) +static uintptr_t northbridge_get_tseg_base(void) { return ALIGN_DOWN(smm_region_start(), 1*MiB); }
-u32 northbridge_get_tseg_size(void) +static size_t northbridge_get_tseg_size(void) { return CONFIG_SMM_TSEG_SIZE; }
-void stage_cache_external_region(void **base, size_t *size) +void smm_region(uintptr_t *start, size_t *size) { - /* The stage cache lives at the end of TSEG region. - * The top of RAM is defined to be the TSEG base address. */ - *size = CONFIG_SMM_RESERVED_SIZE; - *base = (void *)((uintptr_t)northbridge_get_tseg_base() + northbridge_get_tseg_size() - - CONFIG_IED_REGION_SIZE - CONFIG_SMM_RESERVED_SIZE); + *start = northbridge_get_tseg_base(); + *size = northbridge_get_tseg_size(); }
void fill_postcar_frame(struct postcar_frame *pcf) diff --git a/src/northbridge/intel/x4x/memmap.c b/src/northbridge/intel/x4x/memmap.c index 2f50768..41e4912 100644 --- a/src/northbridge/intel/x4x/memmap.c +++ b/src/northbridge/intel/x4x/memmap.c @@ -25,10 +25,10 @@ #include <device/pci_def.h> #include <console/console.h> #include <cpu/x86/mtrr.h> +#include <cpu/x86/smm.h> #include <northbridge/intel/x4x/x4x.h> #include <program_loading.h> #include <cpu/intel/smm_reloc.h> -#include <stage_cache.h>
/** Decodes used Graphics Mode Select (GMS) to kilobytes. */ u32 decode_igd_memory_size(const u32 gms) @@ -112,13 +112,13 @@ return 1; }
-u32 northbridge_get_tseg_size(void) +static size_t northbridge_get_tseg_size(void) { const u8 esmramc = pci_read_config8(PCI_DEV(0, 0, 0), D0F0_ESMRAMC); return decode_tseg_size(esmramc); }
-u32 northbridge_get_tseg_base(void) +static uintptr_t northbridge_get_tseg_base(void) { return pci_read_config32(PCI_DEV(0, 0, 0), D0F0_TSEG); } @@ -134,14 +134,10 @@ return (void *) top_of_ram; }
-void stage_cache_external_region(void **base, size_t *size) +void smm_region(uintptr_t *start, size_t *size) { - /* The stage cache lives at the end of the TSEG region. - * The top of RAM is defined to be the TSEG base address. - */ - *size = CONFIG_SMM_RESERVED_SIZE; - *base = (void *)((uintptr_t)northbridge_get_tseg_base() - + northbridge_get_tseg_size() - CONFIG_SMM_RESERVED_SIZE); + *start = northbridge_get_tseg_base(); + *size = northbridge_get_tseg_size(); }
void fill_postcar_frame(struct postcar_frame *pcf)
Bill XIE has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34740 )
Change subject: intel/smm/gen1: Use smm_subregion() ......................................................................
Patch Set 14:
Sadly, it breaks on platforms using gm45, such as x200s. I found the cause (this commit) by bisect.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34740 )
Change subject: intel/smm/gen1: Use smm_subregion() ......................................................................
Patch Set 14:
Patch Set 14:
Sadly, it breaks on platforms using gm45, such as x200s. I found the cause (this commit) by bisect.
OK... can you get log over usbdebug? And are you confident it is gm45 platforms and not the CPU installed on x200s?
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34740 )
Change subject: intel/smm/gen1: Use smm_subregion() ......................................................................
Patch Set 14:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34740/14/src/cpu/intel/smm/gen1/smm... File src/cpu/intel/smm/gen1/smmrelocate.c:
https://review.coreboot.org/c/coreboot/+/34740/14/src/cpu/intel/smm/gen1/smm... PS14, Line 127: Bill, how about this?
params->smrr_base.lo &= rmask;
Bill XIE has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34740 )
Change subject: intel/smm/gen1: Use smm_subregion() ......................................................................
Patch Set 14: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/34740/14/src/cpu/intel/smm/gen1/smm... File src/cpu/intel/smm/gen1/smmrelocate.c:
https://review.coreboot.org/c/coreboot/+/34740/14/src/cpu/intel/smm/gen1/smm... PS14, Line 127:
Bill, how about this? […]
The regression gets fixed with CB:35780.
-
Angel Pons (Code Review) -
Arthur Heymans (Code Review)
-
Bill XIE (Code Review)
-
Kyösti Mälkki (Code Review)