Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34809 )
Change subject: arch/x86: Add postcar_frame_common_mtrrs() ......................................................................
arch/x86: Add postcar_frame_common_mtrrs()
As most platforms will share the subset of enabling both low RAM WB and high ROM WP MTRRs, provide them with a single function.
Add possibility for the platform to skip these if required.
Change-Id: Id1f8b7682035e654231f6133a42909a36e3e15a1 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/arch/x86/include/arch/cpu.h M src/arch/x86/postcar_loader.c M src/cpu/intel/car/romstage.c M src/drivers/intel/fsp1_1/car.c M src/northbridge/intel/e7505/memmap.c M src/northbridge/intel/gm45/memmap.c M src/northbridge/intel/haswell/memmap.c M src/northbridge/intel/i440bx/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 M src/soc/intel/baytrail/romstage/romstage.c M src/soc/intel/broadwell/romstage/romstage.c 15 files changed, 22 insertions(+), 74 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/34809/1
diff --git a/src/arch/x86/include/arch/cpu.h b/src/arch/x86/include/arch/cpu.h index 293ca02..09e9146 100644 --- a/src/arch/x86/include/arch/cpu.h +++ b/src/arch/x86/include/arch/cpu.h @@ -308,6 +308,7 @@ uint32_t upper_mask; int max_var_mtrrs; int num_var_mtrrs; + int skip_common_mtrr; };
/* @@ -329,6 +330,11 @@ void postcar_frame_add_romcache(struct postcar_frame *pcf, int type);
/* + * Add a common MTRR setup most platforms will have as a subset. + */ +void postcar_frame_common_mtrrs(struct postcar_frame *pcf); + +/* * Push used MTRR and Max MTRRs on to the stack * and return pointer to stack top. */ diff --git a/src/arch/x86/postcar_loader.c b/src/arch/x86/postcar_loader.c index 35e139f..0778198 100644 --- a/src/arch/x86/postcar_loader.c +++ b/src/arch/x86/postcar_loader.c @@ -120,6 +120,15 @@ postcar_frame_add_mtrr(pcf, CACHE_ROM_BASE, CACHE_ROM_SIZE, type); }
+void postcar_frame_common_mtrrs(struct postcar_frame *pcf) +{ + /* Cache RAM as WB from 0 -> CACHE_TMP_RAMTOP. */ + postcar_frame_add_mtrr(pcf, 0, CACHE_TMP_RAMTOP, MTRR_TYPE_WRBACK); + + /* Cache the ROM as WP just below 4GiB. */ + postcar_frame_add_romcache(pcf, MTRR_TYPE_WRPROT); +} + void *postcar_commit_mtrrs(struct postcar_frame *pcf) { /* diff --git a/src/cpu/intel/car/romstage.c b/src/cpu/intel/car/romstage.c index f07c329..30dac22 100644 --- a/src/cpu/intel/car/romstage.c +++ b/src/cpu/intel/car/romstage.c @@ -32,6 +32,9 @@ die("Unable to initialize postcar frame.\n");
fill_postcar_frame(pcf); + + if (!pcf->skip_common_mtrr) + postcar_frame_common_mtrrs(pcf); }
static void romstage_main(unsigned long bist) diff --git a/src/drivers/intel/fsp1_1/car.c b/src/drivers/intel/fsp1_1/car.c index f6e42d7..b206e6d 100644 --- a/src/drivers/intel/fsp1_1/car.c +++ b/src/drivers/intel/fsp1_1/car.c @@ -28,13 +28,6 @@ { uintptr_t top_of_ram;
- /* Cache the ROM as WP just below 4GiB. */ - postcar_frame_add_mtrr(pcf, CACHE_ROM_BASE, CACHE_ROM_SIZE, - MTRR_TYPE_WRPROT); - - /* Cache RAM as WB from 0 -> CACHE_TMP_RAMTOP. */ - postcar_frame_add_mtrr(pcf, 0, CACHE_TMP_RAMTOP, MTRR_TYPE_WRBACK); - /* Cache at least 8 MiB below the top of ram, and at most 8 MiB * above top of the ram. This satisfies MTRR alignment requirement * with different TSEG size configurations. */ diff --git a/src/northbridge/intel/e7505/memmap.c b/src/northbridge/intel/e7505/memmap.c index c3b59e9..7033f89 100644 --- a/src/northbridge/intel/e7505/memmap.c +++ b/src/northbridge/intel/e7505/memmap.c @@ -54,6 +54,8 @@ * operations when source is left as UC. */
+ pcf->skip_common_mtrr = 1; + /* Cache RAM as WB from 0 -> CACHE_TMP_RAMTOP. */ postcar_frame_add_mtrr(pcf, 0, CACHE_TMP_RAMTOP, MTRR_TYPE_WRBACK);
diff --git a/src/northbridge/intel/gm45/memmap.c b/src/northbridge/intel/gm45/memmap.c index 9337470..71037ae 100644 --- a/src/northbridge/intel/gm45/memmap.c +++ b/src/northbridge/intel/gm45/memmap.c @@ -138,13 +138,6 @@ { uintptr_t top_of_ram;
- - /* Cache the ROM as WP just below 4GiB. */ - postcar_frame_add_romcache(pcf, MTRR_TYPE_WRPROT); - - /* Cache RAM as WB from 0 -> CACHE_TMP_RAMTOP. */ - postcar_frame_add_mtrr(pcf, 0, CACHE_TMP_RAMTOP, MTRR_TYPE_WRBACK); - /* Cache 8 MiB region below the top of ram and 2 MiB above top of * ram to cover both cbmem as the TSEG region. */ diff --git a/src/northbridge/intel/haswell/memmap.c b/src/northbridge/intel/haswell/memmap.c index 13881e9..f43bd2f 100644 --- a/src/northbridge/intel/haswell/memmap.c +++ b/src/northbridge/intel/haswell/memmap.c @@ -57,12 +57,6 @@ { uintptr_t top_of_ram;
- /* Cache the ROM as WP just below 4GiB. */ - postcar_frame_add_romcache(pcf, MTRR_TYPE_WRPROT); - - /* Cache RAM as WB from 0 -> CACHE_TMP_RAMTOP. */ - postcar_frame_add_mtrr(pcf, 0, CACHE_TMP_RAMTOP, MTRR_TYPE_WRBACK); - /* Cache at least 8 MiB below the top of ram, and at most 8 MiB * above top of the ram. This satisfies MTRR alignment requirement * with different TSEG size configurations. diff --git a/src/northbridge/intel/i440bx/memmap.c b/src/northbridge/intel/i440bx/memmap.c index 084679f..e4e5de7 100644 --- a/src/northbridge/intel/i440bx/memmap.c +++ b/src/northbridge/intel/i440bx/memmap.c @@ -72,13 +72,6 @@ { uintptr_t top_of_ram;
- - /* Cache the ROM as WP just below 4GiB. */ - postcar_frame_add_romcache(pcf, MTRR_TYPE_WRPROT); - - /* Cache RAM as WB from 0 -> CACHE_TMP_RAMTOP. */ - postcar_frame_add_mtrr(pcf, 0, CACHE_TMP_RAMTOP, MTRR_TYPE_WRBACK); - /* Cache CBMEM region as WB. */ top_of_ram = (uintptr_t)cbmem_top(); postcar_frame_add_mtrr(pcf, top_of_ram - 8*MiB, 8*MiB, diff --git a/src/northbridge/intel/i945/memmap.c b/src/northbridge/intel/i945/memmap.c index 5f3e256..6092c25 100644 --- a/src/northbridge/intel/i945/memmap.c +++ b/src/northbridge/intel/i945/memmap.c @@ -104,13 +104,6 @@ { uintptr_t top_of_ram;
- - /* Cache the ROM as WP just below 4GiB. */ - postcar_frame_add_romcache(pcf, MTRR_TYPE_WRPROT); - - /* Cache RAM as WB from 0 -> CACHE_TMP_RAMTOP. */ - postcar_frame_add_mtrr(pcf, 0, CACHE_TMP_RAMTOP, MTRR_TYPE_WRBACK); - /* Cache 8 MiB region below the top of ram and 2 MiB above top of * ram to cover both cbmem as the TSEG region. */ diff --git a/src/northbridge/intel/nehalem/memmap.c b/src/northbridge/intel/nehalem/memmap.c index 1a3a6d7..031240c 100644 --- a/src/northbridge/intel/nehalem/memmap.c +++ b/src/northbridge/intel/nehalem/memmap.c @@ -62,13 +62,6 @@ { uintptr_t top_of_ram;
- - /* Cache the ROM as WP just below 4GiB. */ - postcar_frame_add_romcache(pcf, MTRR_TYPE_WRPROT); - - /* Cache RAM as WB from 0 -> CACHE_TMP_RAMTOP. */ - postcar_frame_add_mtrr(pcf, 0, CACHE_TMP_RAMTOP, MTRR_TYPE_WRBACK); - /* Cache at least 8 MiB below the top of ram, and at most 8 MiB * above top of the ram. This satisfies MTRR alignment requirement * with different TSEG size configurations. diff --git a/src/northbridge/intel/pineview/memmap.c b/src/northbridge/intel/pineview/memmap.c index 66900af..2e02889 100644 --- a/src/northbridge/intel/pineview/memmap.c +++ b/src/northbridge/intel/pineview/memmap.c @@ -154,13 +154,6 @@ { uintptr_t top_of_ram;
- - /* Cache the ROM as WP just below 4GiB. */ - postcar_frame_add_romcache(pcf, MTRR_TYPE_WRPROT); - - /* Cache RAM as WB from 0 -> CACHE_TMP_RAMTOP. */ - postcar_frame_add_mtrr(pcf, 0, CACHE_TMP_RAMTOP, MTRR_TYPE_WRBACK); - /* Cache 8 MiB region below the top of ram and 2 MiB above top of * ram to cover both cbmem as the TSEG region. */ diff --git a/src/northbridge/intel/sandybridge/memmap.c b/src/northbridge/intel/sandybridge/memmap.c index 99f11a0..83a67ab 100644 --- a/src/northbridge/intel/sandybridge/memmap.c +++ b/src/northbridge/intel/sandybridge/memmap.c @@ -61,13 +61,6 @@ { uintptr_t top_of_ram;
- - /* Cache the ROM as WP just below 4GiB. */ - postcar_frame_add_romcache(pcf, MTRR_TYPE_WRPROT); - - /* Cache RAM as WB from 0 -> CACHE_TMP_RAMTOP. */ - postcar_frame_add_mtrr(pcf, 0, CACHE_TMP_RAMTOP, MTRR_TYPE_WRBACK); - top_of_ram = (uintptr_t)cbmem_top(); /* Cache 8MiB below the top of ram. On sandybridge systems the top of * ram under 4GiB is the start of the TSEG region. It is required to diff --git a/src/northbridge/intel/x4x/memmap.c b/src/northbridge/intel/x4x/memmap.c index 254ca88..a61d64e 100644 --- a/src/northbridge/intel/x4x/memmap.c +++ b/src/northbridge/intel/x4x/memmap.c @@ -149,13 +149,6 @@ { uintptr_t top_of_ram;
- - /* Cache the ROM as WP just below 4GiB. */ - postcar_frame_add_romcache(pcf, MTRR_TYPE_WRPROT); - - /* Cache RAM as WB from 0 -> CACHE_TMP_RAMTOP. */ - postcar_frame_add_mtrr(pcf, 0, CACHE_TMP_RAMTOP, MTRR_TYPE_WRBACK); - /* Cache 8 MiB region below the top of ram and 2 MiB above top of * ram to cover both cbmem as the TSEG region. */ diff --git a/src/soc/intel/baytrail/romstage/romstage.c b/src/soc/intel/baytrail/romstage/romstage.c index 4778860..f188430 100644 --- a/src/soc/intel/baytrail/romstage/romstage.c +++ b/src/soc/intel/baytrail/romstage/romstage.c @@ -59,6 +59,8 @@ die("Unable to initialize postcar frame.\n");
fill_postcar_frame(pcf); + + postcar_frame_common_mtrrs(pcf); }
static void program_base_addresses(void) @@ -253,12 +255,6 @@ { uintptr_t top_of_ram;
- /* Cache the ROM as WP just below 4GiB. */ - postcar_frame_add_romcache(pcf, MTRR_TYPE_WRPROT); - - /* Cache RAM as WB from 0 -> CACHE_TMP_RAMTOP. */ - postcar_frame_add_mtrr(pcf, 0, CACHE_TMP_RAMTOP, MTRR_TYPE_WRBACK); - /* Cache at least 8 MiB below the top of ram, and at most 8 MiB * above top of the ram. This satisfies MTRR alignment requirement * with different TSEG size configurations. diff --git a/src/soc/intel/broadwell/romstage/romstage.c b/src/soc/intel/broadwell/romstage/romstage.c index ca478f7..5ae39dd 100644 --- a/src/soc/intel/broadwell/romstage/romstage.c +++ b/src/soc/intel/broadwell/romstage/romstage.c @@ -39,12 +39,6 @@ { uintptr_t top_of_ram;
- /* Cache the ROM as WP just below 4GiB. */ - postcar_frame_add_romcache(pcf, MTRR_TYPE_WRPROT); - - /* Cache RAM as WB from 0 -> CACHE_TMP_RAMTOP. */ - postcar_frame_add_mtrr(pcf, 0, CACHE_TMP_RAMTOP, MTRR_TYPE_WRBACK); - /* Cache at least 8 MiB below the top of ram, and at most 8 MiB * above top of the ram. This satisfies MTRR alignment requirement * with different TSEG size configurations.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34809 )
Change subject: arch/x86: Add postcar_frame_common_mtrrs() ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34809/2/src/cpu/intel/car/romstage.... File src/cpu/intel/car/romstage.c:
https://review.coreboot.org/c/coreboot/+/34809/2/src/cpu/intel/car/romstage.... PS2, Line 36: if (!pcf->skip_common_mtrr) I don't think we should be making these decisions here. This field should be checked in postcar_frame_common_mtrrs(). Or why not do this inside run_postcar_phase() and make the field inverted (fill common mtrrs).
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34809 )
Change subject: arch/x86: Add postcar_frame_common_mtrrs() ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34809/2/src/arch/x86/postcar_loader... File src/arch/x86/postcar_loader.c:
https://review.coreboot.org/c/coreboot/+/34809/2/src/arch/x86/postcar_loader... PS2, Line 125: /* Cache RAM as WB from 0 -> CACHE_TMP_RAMTOP. */ CACHE_TMP_RAMTOP is fairly arbitrary. Why would we do this all the time?
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34809 )
Change subject: arch/x86: Add postcar_frame_common_mtrrs() ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34809/2/src/arch/x86/postcar_loader... File src/arch/x86/postcar_loader.c:
https://review.coreboot.org/c/coreboot/+/34809/2/src/arch/x86/postcar_loader... PS2, Line 125: /* Cache RAM as WB from 0 -> CACHE_TMP_RAMTOP. */
CACHE_TMP_RAMTOP is fairly arbitrary. […]
Well currently we do. The only case I remember is some SIPI for MP_INIT that has to be within first megabyte? I would not mind changing this to that with followup.
https://review.coreboot.org/c/coreboot/+/34809/2/src/cpu/intel/car/romstage.... File src/cpu/intel/car/romstage.c:
https://review.coreboot.org/c/coreboot/+/34809/2/src/cpu/intel/car/romstage.... PS2, Line 36: if (!pcf->skip_common_mtrr)
I don't think we should be making these decisions here. […]
Ack
Hello Patrick Rudolph, Huang Jin, Lee Leahy, build bot (Jenkins), Damien Zammit,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34809
to look at the new patch set (#3).
Change subject: arch/x86: Add postcar_frame_common_mtrrs() ......................................................................
arch/x86: Add postcar_frame_common_mtrrs()
As most platforms will share the subset of enabling both low RAM WB and high ROM WP MTRRs, provide them with a single function.
Add possibility for the platform to skip these if required.
Change-Id: Id1f8b7682035e654231f6133a42909a36e3e15a1 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/arch/x86/include/arch/cpu.h M src/arch/x86/postcar_loader.c M src/cpu/intel/car/romstage.c M src/drivers/intel/fsp1_1/car.c M src/northbridge/intel/e7505/memmap.c M src/northbridge/intel/gm45/memmap.c M src/northbridge/intel/haswell/memmap.c M src/northbridge/intel/i440bx/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 M src/soc/intel/baytrail/romstage/romstage.c M src/soc/intel/broadwell/romstage/romstage.c 15 files changed, 24 insertions(+), 74 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/34809/3
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34809 )
Change subject: arch/x86: Add postcar_frame_common_mtrrs() ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34809/2/src/arch/x86/postcar_loader... File src/arch/x86/postcar_loader.c:
https://review.coreboot.org/c/coreboot/+/34809/2/src/arch/x86/postcar_loader... PS2, Line 125: /* Cache RAM as WB from 0 -> CACHE_TMP_RAMTOP. */
Well currently we do. […]
I believe the final BSP MTRR's are already set up before the SIPI call. Besides does it even make much sense to want to cache the SIPI vector? The AP's MTRR's are not synced at that point or does it speed up setting up that stub?
Maybe this a vague remainder of non-relocatable ramstage?
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34809 )
Change subject: arch/x86: Add postcar_frame_common_mtrrs() ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34809/2/src/arch/x86/postcar_loader... File src/arch/x86/postcar_loader.c:
https://review.coreboot.org/c/coreboot/+/34809/2/src/arch/x86/postcar_loader... PS2, Line 125: /* Cache RAM as WB from 0 -> CACHE_TMP_RAMTOP. */
I believe the final BSP MTRR's are already set up before the SIPI call. […]
Functional change should be followup. I thought MTRRs are supposed to be changed in parallel but did not check details. Indeed, mostly remainder of non-relocatable stuff.
Hello Patrick Rudolph, Huang Jin, Lee Leahy, build bot (Jenkins), Damien Zammit,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34809
to look at the new patch set (#5).
Change subject: arch/x86: Add postcar_frame_common_mtrrs() ......................................................................
arch/x86: Add postcar_frame_common_mtrrs()
As most platforms will share the subset of enabling both low RAM WB and high ROM WP MTRRs, provide them with a single function.
Add possibility for the platform to skip these if required.
Change-Id: Id1f8b7682035e654231f6133a42909a36e3e15a1 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/arch/x86/include/arch/cpu.h M src/arch/x86/postcar_loader.c M src/cpu/intel/car/romstage.c M src/drivers/intel/fsp1_1/car.c M src/northbridge/intel/e7505/memmap.c M src/northbridge/intel/gm45/memmap.c M src/northbridge/intel/haswell/memmap.c M src/northbridge/intel/i440bx/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 M src/soc/intel/baytrail/romstage/romstage.c M src/soc/intel/broadwell/romstage/romstage.c 15 files changed, 24 insertions(+), 74 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/34809/5
Hello Patrick Rudolph, Huang Jin, Lee Leahy, build bot (Jenkins), Damien Zammit,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34809
to look at the new patch set (#6).
Change subject: arch/x86: Add postcar_frame_common_mtrrs() ......................................................................
arch/x86: Add postcar_frame_common_mtrrs()
As most platforms will share the subset of enabling both low RAM WB and high ROM WP MTRRs, provide them with a single function.
Add possibility for the platform to skip these if required.
Change-Id: Id1f8b7682035e654231f6133a42909a36e3e15a1 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/arch/x86/include/arch/cpu.h M src/arch/x86/postcar_loader.c M src/cpu/intel/car/romstage.c M src/drivers/intel/fsp1_1/car.c M src/northbridge/intel/e7505/memmap.c M src/northbridge/intel/gm45/memmap.c M src/northbridge/intel/haswell/memmap.c M src/northbridge/intel/i440bx/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 M src/soc/intel/baytrail/romstage/romstage.c M src/soc/intel/broadwell/romstage/romstage.c 15 files changed, 24 insertions(+), 74 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/34809/6
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34809 )
Change subject: arch/x86: Add postcar_frame_common_mtrrs() ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34809/2/src/arch/x86/postcar_loader... File src/arch/x86/postcar_loader.c:
https://review.coreboot.org/c/coreboot/+/34809/2/src/arch/x86/postcar_loader... PS2, Line 125: /* Cache RAM as WB from 0 -> CACHE_TMP_RAMTOP. */
Functional change should be followup. […]
Ack
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34809 )
Change subject: arch/x86: Add postcar_frame_common_mtrrs() ......................................................................
Patch Set 7: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/34809/7/src/arch/x86/include/arch/c... File src/arch/x86/include/arch/cpu.h:
https://review.coreboot.org/c/coreboot/+/34809/7/src/arch/x86/include/arch/c... PS7, Line 305: int skip_common_mtrr; I do think we could have just done with opposite logic -- add_common_mtrr and then hidden postcar_frame_common_mtrrs() within the compilation unit and called it from run_postcar_phase(). However, that reduces the API surface area a bit and no callers care but the ones you are touching here. Let's see how this plays out, though.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34809 )
Change subject: arch/x86: Add postcar_frame_common_mtrrs() ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34809/7/src/arch/x86/include/arch/c... File src/arch/x86/include/arch/cpu.h:
https://review.coreboot.org/c/coreboot/+/34809/7/src/arch/x86/include/arch/c... PS7, Line 305: int skip_common_mtrr;
I do think we could have just done with opposite logic -- add_common_mtrr and then hidden postcar_fr […]
Ack
Kyösti Mälkki has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34809 )
Change subject: arch/x86: Add postcar_frame_common_mtrrs() ......................................................................
arch/x86: Add postcar_frame_common_mtrrs()
As most platforms will share the subset of enabling both low RAM WB and high ROM WP MTRRs, provide them with a single function.
Add possibility for the platform to skip these if required.
Change-Id: Id1f8b7682035e654231f6133a42909a36e3e15a1 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/34809 Reviewed-by: Aaron Durbin adurbin@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/arch/x86/include/arch/cpu.h M src/arch/x86/postcar_loader.c M src/cpu/intel/car/romstage.c M src/drivers/intel/fsp1_1/car.c M src/northbridge/intel/e7505/memmap.c M src/northbridge/intel/gm45/memmap.c M src/northbridge/intel/haswell/memmap.c M src/northbridge/intel/i440bx/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 M src/soc/intel/baytrail/romstage/romstage.c M src/soc/intel/broadwell/romstage/romstage.c 15 files changed, 24 insertions(+), 74 deletions(-)
Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved
diff --git a/src/arch/x86/include/arch/cpu.h b/src/arch/x86/include/arch/cpu.h index 6062021..9aa446e 100644 --- a/src/arch/x86/include/arch/cpu.h +++ b/src/arch/x86/include/arch/cpu.h @@ -302,6 +302,7 @@ uint32_t upper_mask; int max_var_mtrrs; int num_var_mtrrs; + int skip_common_mtrr; };
/* @@ -323,6 +324,11 @@ void postcar_frame_add_romcache(struct postcar_frame *pcf, int type);
/* + * Add a common MTRR setup most platforms will have as a subset. + */ +void postcar_frame_common_mtrrs(struct postcar_frame *pcf); + +/* * Push used MTRR and Max MTRRs on to the stack * and return pointer to stack top. */ diff --git a/src/arch/x86/postcar_loader.c b/src/arch/x86/postcar_loader.c index 35e139f..b1b2da0 100644 --- a/src/arch/x86/postcar_loader.c +++ b/src/arch/x86/postcar_loader.c @@ -120,6 +120,18 @@ postcar_frame_add_mtrr(pcf, CACHE_ROM_BASE, CACHE_ROM_SIZE, type); }
+void postcar_frame_common_mtrrs(struct postcar_frame *pcf) +{ + if (pcf->skip_common_mtrr) + return; + + /* Cache RAM as WB from 0 -> CACHE_TMP_RAMTOP. */ + postcar_frame_add_mtrr(pcf, 0, CACHE_TMP_RAMTOP, MTRR_TYPE_WRBACK); + + /* Cache the ROM as WP just below 4GiB. */ + postcar_frame_add_romcache(pcf, MTRR_TYPE_WRPROT); +} + void *postcar_commit_mtrrs(struct postcar_frame *pcf) { /* diff --git a/src/cpu/intel/car/romstage.c b/src/cpu/intel/car/romstage.c index f6b6219..624f3ff 100644 --- a/src/cpu/intel/car/romstage.c +++ b/src/cpu/intel/car/romstage.c @@ -33,6 +33,8 @@
fill_postcar_frame(pcf);
+ postcar_frame_common_mtrrs(pcf); + run_postcar_phase(pcf); /* We do not return here. */ } diff --git a/src/drivers/intel/fsp1_1/car.c b/src/drivers/intel/fsp1_1/car.c index f6e42d7..b206e6d 100644 --- a/src/drivers/intel/fsp1_1/car.c +++ b/src/drivers/intel/fsp1_1/car.c @@ -28,13 +28,6 @@ { uintptr_t top_of_ram;
- /* Cache the ROM as WP just below 4GiB. */ - postcar_frame_add_mtrr(pcf, CACHE_ROM_BASE, CACHE_ROM_SIZE, - MTRR_TYPE_WRPROT); - - /* Cache RAM as WB from 0 -> CACHE_TMP_RAMTOP. */ - postcar_frame_add_mtrr(pcf, 0, CACHE_TMP_RAMTOP, MTRR_TYPE_WRBACK); - /* Cache at least 8 MiB below the top of ram, and at most 8 MiB * above top of the ram. This satisfies MTRR alignment requirement * with different TSEG size configurations. */ diff --git a/src/northbridge/intel/e7505/memmap.c b/src/northbridge/intel/e7505/memmap.c index c3b59e9..7033f89 100644 --- a/src/northbridge/intel/e7505/memmap.c +++ b/src/northbridge/intel/e7505/memmap.c @@ -54,6 +54,8 @@ * operations when source is left as UC. */
+ pcf->skip_common_mtrr = 1; + /* Cache RAM as WB from 0 -> CACHE_TMP_RAMTOP. */ postcar_frame_add_mtrr(pcf, 0, CACHE_TMP_RAMTOP, MTRR_TYPE_WRBACK);
diff --git a/src/northbridge/intel/gm45/memmap.c b/src/northbridge/intel/gm45/memmap.c index 9337470..71037ae 100644 --- a/src/northbridge/intel/gm45/memmap.c +++ b/src/northbridge/intel/gm45/memmap.c @@ -138,13 +138,6 @@ { uintptr_t top_of_ram;
- - /* Cache the ROM as WP just below 4GiB. */ - postcar_frame_add_romcache(pcf, MTRR_TYPE_WRPROT); - - /* Cache RAM as WB from 0 -> CACHE_TMP_RAMTOP. */ - postcar_frame_add_mtrr(pcf, 0, CACHE_TMP_RAMTOP, MTRR_TYPE_WRBACK); - /* Cache 8 MiB region below the top of ram and 2 MiB above top of * ram to cover both cbmem as the TSEG region. */ diff --git a/src/northbridge/intel/haswell/memmap.c b/src/northbridge/intel/haswell/memmap.c index 13881e9..f43bd2f 100644 --- a/src/northbridge/intel/haswell/memmap.c +++ b/src/northbridge/intel/haswell/memmap.c @@ -57,12 +57,6 @@ { uintptr_t top_of_ram;
- /* Cache the ROM as WP just below 4GiB. */ - postcar_frame_add_romcache(pcf, MTRR_TYPE_WRPROT); - - /* Cache RAM as WB from 0 -> CACHE_TMP_RAMTOP. */ - postcar_frame_add_mtrr(pcf, 0, CACHE_TMP_RAMTOP, MTRR_TYPE_WRBACK); - /* Cache at least 8 MiB below the top of ram, and at most 8 MiB * above top of the ram. This satisfies MTRR alignment requirement * with different TSEG size configurations. diff --git a/src/northbridge/intel/i440bx/memmap.c b/src/northbridge/intel/i440bx/memmap.c index 084679f..e4e5de7 100644 --- a/src/northbridge/intel/i440bx/memmap.c +++ b/src/northbridge/intel/i440bx/memmap.c @@ -72,13 +72,6 @@ { uintptr_t top_of_ram;
- - /* Cache the ROM as WP just below 4GiB. */ - postcar_frame_add_romcache(pcf, MTRR_TYPE_WRPROT); - - /* Cache RAM as WB from 0 -> CACHE_TMP_RAMTOP. */ - postcar_frame_add_mtrr(pcf, 0, CACHE_TMP_RAMTOP, MTRR_TYPE_WRBACK); - /* Cache CBMEM region as WB. */ top_of_ram = (uintptr_t)cbmem_top(); postcar_frame_add_mtrr(pcf, top_of_ram - 8*MiB, 8*MiB, diff --git a/src/northbridge/intel/i945/memmap.c b/src/northbridge/intel/i945/memmap.c index 5f3e256..6092c25 100644 --- a/src/northbridge/intel/i945/memmap.c +++ b/src/northbridge/intel/i945/memmap.c @@ -104,13 +104,6 @@ { uintptr_t top_of_ram;
- - /* Cache the ROM as WP just below 4GiB. */ - postcar_frame_add_romcache(pcf, MTRR_TYPE_WRPROT); - - /* Cache RAM as WB from 0 -> CACHE_TMP_RAMTOP. */ - postcar_frame_add_mtrr(pcf, 0, CACHE_TMP_RAMTOP, MTRR_TYPE_WRBACK); - /* Cache 8 MiB region below the top of ram and 2 MiB above top of * ram to cover both cbmem as the TSEG region. */ diff --git a/src/northbridge/intel/nehalem/memmap.c b/src/northbridge/intel/nehalem/memmap.c index 1a3a6d7..031240c 100644 --- a/src/northbridge/intel/nehalem/memmap.c +++ b/src/northbridge/intel/nehalem/memmap.c @@ -62,13 +62,6 @@ { uintptr_t top_of_ram;
- - /* Cache the ROM as WP just below 4GiB. */ - postcar_frame_add_romcache(pcf, MTRR_TYPE_WRPROT); - - /* Cache RAM as WB from 0 -> CACHE_TMP_RAMTOP. */ - postcar_frame_add_mtrr(pcf, 0, CACHE_TMP_RAMTOP, MTRR_TYPE_WRBACK); - /* Cache at least 8 MiB below the top of ram, and at most 8 MiB * above top of the ram. This satisfies MTRR alignment requirement * with different TSEG size configurations. diff --git a/src/northbridge/intel/pineview/memmap.c b/src/northbridge/intel/pineview/memmap.c index 66900af..2e02889 100644 --- a/src/northbridge/intel/pineview/memmap.c +++ b/src/northbridge/intel/pineview/memmap.c @@ -154,13 +154,6 @@ { uintptr_t top_of_ram;
- - /* Cache the ROM as WP just below 4GiB. */ - postcar_frame_add_romcache(pcf, MTRR_TYPE_WRPROT); - - /* Cache RAM as WB from 0 -> CACHE_TMP_RAMTOP. */ - postcar_frame_add_mtrr(pcf, 0, CACHE_TMP_RAMTOP, MTRR_TYPE_WRBACK); - /* Cache 8 MiB region below the top of ram and 2 MiB above top of * ram to cover both cbmem as the TSEG region. */ diff --git a/src/northbridge/intel/sandybridge/memmap.c b/src/northbridge/intel/sandybridge/memmap.c index 99f11a0..83a67ab 100644 --- a/src/northbridge/intel/sandybridge/memmap.c +++ b/src/northbridge/intel/sandybridge/memmap.c @@ -61,13 +61,6 @@ { uintptr_t top_of_ram;
- - /* Cache the ROM as WP just below 4GiB. */ - postcar_frame_add_romcache(pcf, MTRR_TYPE_WRPROT); - - /* Cache RAM as WB from 0 -> CACHE_TMP_RAMTOP. */ - postcar_frame_add_mtrr(pcf, 0, CACHE_TMP_RAMTOP, MTRR_TYPE_WRBACK); - top_of_ram = (uintptr_t)cbmem_top(); /* Cache 8MiB below the top of ram. On sandybridge systems the top of * ram under 4GiB is the start of the TSEG region. It is required to diff --git a/src/northbridge/intel/x4x/memmap.c b/src/northbridge/intel/x4x/memmap.c index 254ca88..a61d64e 100644 --- a/src/northbridge/intel/x4x/memmap.c +++ b/src/northbridge/intel/x4x/memmap.c @@ -149,13 +149,6 @@ { uintptr_t top_of_ram;
- - /* Cache the ROM as WP just below 4GiB. */ - postcar_frame_add_romcache(pcf, MTRR_TYPE_WRPROT); - - /* Cache RAM as WB from 0 -> CACHE_TMP_RAMTOP. */ - postcar_frame_add_mtrr(pcf, 0, CACHE_TMP_RAMTOP, MTRR_TYPE_WRBACK); - /* Cache 8 MiB region below the top of ram and 2 MiB above top of * ram to cover both cbmem as the TSEG region. */ diff --git a/src/soc/intel/baytrail/romstage/romstage.c b/src/soc/intel/baytrail/romstage/romstage.c index acdf261..8361bb1 100644 --- a/src/soc/intel/baytrail/romstage/romstage.c +++ b/src/soc/intel/baytrail/romstage/romstage.c @@ -62,6 +62,8 @@
fill_postcar_frame(pcf);
+ postcar_frame_common_mtrrs(pcf); + run_postcar_phase(pcf); /* We do not return here. */ } @@ -256,12 +258,6 @@ { uintptr_t top_of_ram;
- /* Cache the ROM as WP just below 4GiB. */ - postcar_frame_add_romcache(pcf, MTRR_TYPE_WRPROT); - - /* Cache RAM as WB from 0 -> CACHE_TMP_RAMTOP. */ - postcar_frame_add_mtrr(pcf, 0, CACHE_TMP_RAMTOP, MTRR_TYPE_WRBACK); - /* Cache at least 8 MiB below the top of ram, and at most 8 MiB * above top of the ram. This satisfies MTRR alignment requirement * with different TSEG size configurations. diff --git a/src/soc/intel/broadwell/romstage/romstage.c b/src/soc/intel/broadwell/romstage/romstage.c index ca478f7..5ae39dd 100644 --- a/src/soc/intel/broadwell/romstage/romstage.c +++ b/src/soc/intel/broadwell/romstage/romstage.c @@ -39,12 +39,6 @@ { uintptr_t top_of_ram;
- /* Cache the ROM as WP just below 4GiB. */ - postcar_frame_add_romcache(pcf, MTRR_TYPE_WRPROT); - - /* Cache RAM as WB from 0 -> CACHE_TMP_RAMTOP. */ - postcar_frame_add_mtrr(pcf, 0, CACHE_TMP_RAMTOP, MTRR_TYPE_WRBACK); - /* Cache at least 8 MiB below the top of ram, and at most 8 MiB * above top of the ram. This satisfies MTRR alignment requirement * with different TSEG size configurations.
-
Aaron Durbin (Code Review) -
Arthur Heymans (Code Review)
-
Kyösti Mälkki (Code Review)