Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45924 )
Change subject: nb/intel/sandybridge: Use `postcar_enable_tseg_cache` ......................................................................
nb/intel/sandybridge: Use `postcar_enable_tseg_cache`
This may not be necessary at all if all CPUs using this code support SMRRs. Update the corresponding code to reflect that with a TODO.
Change-Id: Ibf5a851a6684c9aa96d1c30a8f8dc068d00f3b81 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/northbridge/intel/sandybridge/memmap.c 1 file changed, 4 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/45924/1
diff --git a/src/northbridge/intel/sandybridge/memmap.c b/src/northbridge/intel/sandybridge/memmap.c index 7f46d66..38fa6e9 100644 --- a/src/northbridge/intel/sandybridge/memmap.c +++ b/src/northbridge/intel/sandybridge/memmap.c @@ -51,11 +51,9 @@ postcar_frame_add_mtrr(pcf, top_of_ram - 8 * MiB, 8 * MiB, MTRR_TYPE_WRBACK);
/* - * Cache 8MiB at the top of ram. Top of RAM on sandybridge systems - * is where the TSEG region resides. However, it is not restricted - * to SMM mode until SMM has been relocated. By setting the region - * to cacheable it provides faster access when relocating the SMM - * handler as well as using the TSEG region for other purposes. + * Cache the TSEG region using regular MTRRs. This is only useful + * when SMRRs are not supported, however. TODO(hell__): Determine + * if all SNB/IVB CPUs support SMRRs, and if so remove this call. */ - postcar_frame_add_mtrr(pcf, top_of_ram, 8 * MiB, MTRR_TYPE_WRBACK); + postcar_enable_tseg_cache(pcf); }
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45924 )
Change subject: nb/intel/sandybridge: Use `postcar_enable_tseg_cache` ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45924/2/src/northbridge/intel/sandy... File src/northbridge/intel/sandybridge/memmap.c:
https://review.coreboot.org/c/coreboot/+/45924/2/src/northbridge/intel/sandy... PS2, Line 54: Cache the TSEG region using regular MTRRs. This is only useful : * when SMRRs are not supported, I don't think this is true. TSEG is sometimes used as a stage cache which implies that it is used before SMRR are set up. Also the postcar MTRR frame is only used upto the CPU init where more permanent MTRR's are used.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45924 )
Change subject: nb/intel/sandybridge: Use `postcar_enable_tseg_cache` ......................................................................
Patch Set 2: Code-Review-2
(1 comment)
On hold until the purpose of caching what is marked as TSEG is clear.
https://review.coreboot.org/c/coreboot/+/45924/2/src/northbridge/intel/sandy... File src/northbridge/intel/sandybridge/memmap.c:
https://review.coreboot.org/c/coreboot/+/45924/2/src/northbridge/intel/sandy... PS2, Line 54: Cache the TSEG region using regular MTRRs. This is only useful : * when SMRRs are not supported,
I don't think this is true. […]
Hrm, good point.
Angel Pons has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/45924 )
Change subject: nb/intel/sandybridge: Use `postcar_enable_tseg_cache` ......................................................................
Abandoned
-
Angel Pons (Code Review) -
Arthur Heymans (Code Review)