Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35026 )
Change subject: soc/intel/{cnl, icl}: Cache the TSEG region ......................................................................
soc/intel/{cnl, icl}: Cache the TSEG region
Change-Id: Ie92d2c9e50fa299db1cd8c57a6047ea3adaf1452 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/soc/intel/cannonlake/romstage/romstage.c M src/soc/intel/icelake/romstage/romstage.c 2 files changed, 6 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/35026/1
diff --git a/src/soc/intel/cannonlake/romstage/romstage.c b/src/soc/intel/cannonlake/romstage/romstage.c index 9f02c8b..9e2f2f8 100644 --- a/src/soc/intel/cannonlake/romstage/romstage.c +++ b/src/soc/intel/cannonlake/romstage/romstage.c @@ -165,5 +165,8 @@ /* Cache the ROM as WP just below 4GiB. */ postcar_frame_add_romcache(&pcf, MTRR_TYPE_WRPROT);
+ /* Cache the TSEG region. */ + enable_tseg_cache(&pcf); + run_postcar_phase(&pcf); } diff --git a/src/soc/intel/icelake/romstage/romstage.c b/src/soc/intel/icelake/romstage/romstage.c index 8312f17..4d0cc17 100644 --- a/src/soc/intel/icelake/romstage/romstage.c +++ b/src/soc/intel/icelake/romstage/romstage.c @@ -149,5 +149,8 @@ /* Cache the ROM as WP just below 4GiB. */ postcar_frame_add_romcache(&pcf, MTRR_TYPE_WRPROT);
+ /* Cache the TSEG region. */ + enable_tseg_cache(&pcf); + run_postcar_phase(&pcf); }
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35026 )
Change subject: soc/intel/{cnl, icl}: Cache the TSEG region ......................................................................
Patch Set 2:
'cbmem -t' from this commit please.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35026 )
Change subject: soc/intel/{cnl, icl}: Cache the TSEG region ......................................................................
Patch Set 2:
Patch Set 2:
'cbmem -t' from this commit please.
sorry, just to understand your question.
1. do you want to capture the cbmem -t time with this commit as i thought i have captured CB:34995 commit msg already while exposing the new API.
or
2. You wish to get cbmem -t numbers with this CL. Its already shared here right ?
CB:34791
With POSTCAR_STAGE=y and (CB:34805 + 34995) [romstage -> postcar -> ramstage]
Total Time till picking kernel: 813,549 Total Time till picking payload: 635,250
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35026 )
Change subject: soc/intel/{cnl, icl}: Cache the TSEG region ......................................................................
Patch Set 2:
Patch Set 2:
Patch Set 2:
'cbmem -t' from this commit please.
sorry, just to understand your question.
- do you want to capture the cbmem -t time with this commit as i thought i have captured CB:34995 commit msg already while exposing the new API.
This commit 36b7091 exactly. And put the commit hash together with the data in the comments this time. The patchtrain evolved so heavily that CB: references are obscure.
or
- You wish to get cbmem -t numbers with this CL. Its already shared here right ?
CB:34791
With POSTCAR_STAGE=y and (CB:34805 + 34995) [romstage -> postcar -> ramstage]
Total Time till picking kernel: 813,549 Total Time till picking payload: 635,250
You posted the following under CB:34791.
Aug 20 09:55 Total Time: 813,549
AFAICS this result was from a boot with CB:34995 patchset #1 and CB:34791 patchset #11. While it did set TSEG as WB for the postcar_frame, it also immediately called set_var_mtrr() to program CBMEM region as WRPROT. At the time of that run, it is questionable if cbmem_top() was properly aligned to make that set_var_mtrr() call.
In short, I want result from a boot that definetly does not call set_var_mtrr() at all, has POSTCAR_STAGE=y, and definetly adds TSEG as WB in the postcar frame. Commit 36b7091 does that.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35026 )
Change subject: soc/intel/{cnl, icl}: Cache the TSEG region ......................................................................
Patch Set 2:
Patch Set 2:
Patch Set 2:
Patch Set 2:
'cbmem -t' from this commit please.
sorry, just to understand your question.
- do you want to capture the cbmem -t time with this commit as i thought i have captured CB:34995 commit msg already while exposing the new API.
This commit 36b7091 exactly. And put the commit hash together with the data in the comments this time. The patchtrain evolved so heavily that CB: references are obscure.
or
- You wish to get cbmem -t numbers with this CL. Its already shared here right ?
CB:34791
With POSTCAR_STAGE=y and (CB:34805 + 34995) [romstage -> postcar -> ramstage]
Total Time till picking kernel: 813,549 Total Time till picking payload: 635,250
You posted the following under CB:34791.
Aug 20 09:55 Total Time: 813,549
AFAICS this result was from a boot with CB:34995 patchset #1 and CB:34791 patchset #11. While it did set TSEG as WB for the postcar_frame, it also immediately called set_var_mtrr() to program CBMEM region as WRPROT. At the time of that run, it is questionable if cbmem_top() was properly aligned to make that set_var_mtrr() call.
In short, I want result from a boot that definetly does not call set_var_mtrr() at all, has POSTCAR_STAGE=y, and definetly adds TSEG as WB in the postcar frame. Commit 36b7091 does that.
Let me compare the delta between data posted above with POSTCAR_STAGE=y and (CB:34805 + 34995) [romstage -> postcar -> ramstage] vs POSTCAR_STAGE=y + CB:34995 + CB: 35026
#1 With POSTCAR_STAGE=y and (CB:34805 + 34995) [romstage -> postcar -> ramstage]
Aug 20 09:55
Total Time: 813,549
Total Time till picking kernel: 813,549 Total Time till picking payload: 635,250
#2 With POSTCAR_STAGE=y and (CB:34995 + CB: 35026) [romstage -> postcar -> ramstage]
Aug 23
Total Time: 818,078
Total Time till picking kernel: 818,078 Total Time till picking payload: 639,809
so #1 vs #2 delta is ~4ms
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35026 )
Change subject: soc/intel/{cnl, icl}: Cache the TSEG region ......................................................................
Patch Set 2:
#2 With POSTCAR_STAGE=y and (CB:34995 + CB: 35026) [romstage -> postcar -> ramstage]
Aug 23
Total Time: 818,078
Total Time till picking kernel: 818,078 Total Time till picking payload: 639,809
For this commit 36b7091 exactly, I want to see full 'cbmem -t' data as I am trying to figure out the mystery of timestamp 1100, where we seem to consistently loose 10ms if we gained 7ms earlier.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35026 )
Change subject: soc/intel/{cnl, icl}: Cache the TSEG region ......................................................................
Patch Set 2:
Patch Set 2:
#2 With POSTCAR_STAGE=y and (CB:34995 + CB: 35026) [romstage -> postcar -> ramstage]
Aug 23
Total Time: 818,078
Total Time till picking kernel: 818,078 Total Time till picking payload: 639,809
For this commit 36b7091 exactly, I want to see full 'cbmem -t' data as I am trying to figure out the mystery of timestamp 1100, where we seem to consistently loose 10ms if we gained 7ms earlier.
0:1st timestamp 12,859 5:start of verified boot 39,611 (26,752) 503:starting to initialize TPM 40,214 (602) 504:finished TPM initialization 78,129 (37,914) 505:starting to verify keyblock/preamble (RSA) 79,477 (1,348) 506:finished verifying keyblock/preamble (RSA) 94,209 (14,732) 507:starting to verify body (load+SHA2+RSA) 94,211 (2) 508:finished loading body (ignore for x86) 218,497 (124,285) 509:finished calculating body hash (SHA2) 237,400 (18,902) 510:finished verifying body signature (RSA) 240,012 (2,612) 511:starting TPM PCR extend 240,637 (624) 512:finished TPM PCR extend 256,116 (15,479) 513:starting locking TPM 256,117 (0) 514:finished locking TPM 264,444 (8,327) 6:end of verified boot 272,726 (8,282) 13:starting to load romstage 272,744 (18) 14:finished loading romstage 272,745 (0) 1:start of romstage 272,750 (5) 2:before ram initialization 272,793 (42) 950:calling FspMemoryInit 275,628 (2,835) 951:returning from FspMemoryInit 300,986 (25,358) 3:after ram initialization 304,318 (3,332) 4:end of romstage 306,148 (1,829) 100:start of postcar 307,039 (891) 101:end of postcar 307,039 (0) 8:starting to load ramstage 307,181 (141) 15:starting LZMA decompress (ignore for x86) 307,188 (7) 16:finished LZMA decompress (ignore for x86) 332,709 (25,520) 9:finished loading ramstage 332,813 (104) 550:starting to load Chrome OS VPD 332,886 (73) 10:start of ramstage 333,225 (338) 30:device enumeration 378,839 (45,614) 954:calling FspSiliconInit 387,354 (8,515) 955:returning from FspSiliconInit 475,359 (88,004) 40:device configuration 491,026 (15,667) 956:calling FspNotify(AfterPciEnumeration) 525,419 (34,393) 957:returning from FspNotify(AfterPciEnumeration) 525,727 (307) 50:device enable 525,915 (188) 60:device initialization 545,716 (19,800) 70:device setup done 578,491 (32,774) 75:cbmem post 579,000 (509) 80:write tables 579,141 (141) 15:starting LZMA decompress (ignore for x86) 582,037 (2,896) 16:finished LZMA decompress (ignore for x86) 582,298 (260) 85:finalize chips 582,920 (621) 90:load payload 596,851 (13,931) 15:starting LZMA decompress (ignore for x86) 597,138 (287) 16:finished LZMA decompress (ignore for x86) 645,026 (47,887) 958:calling FspNotify(ReadyToBoot) 645,536 (510) 959:returning from FspNotify(ReadyToBoot) 651,697 (6,161) 960:calling FspNotify(EndOfFirmware) 651,792 (95) 961:returning from FspNotify(EndOfFirmware) 652,199 (406) 99:selfboot jump 652,668 (469) 1000:depthcharge start 652,687 (18) 1002:RO vboot init 652,799 (112) 1020:vboot select&load kernel 652,803 (3) 1030:finished EC verification 672,980 (20,177) 1040:finished storage device initialization 674,068 (1,087) 1050:finished reading kernel from disk 681,173 (7,104) 1100:finished vboot kernel verification 828,455 (147,282) 1101:jumping to kernel 830,962 (2,506)
Total Time: 818,078
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35026 )
Change subject: soc/intel/{cnl, icl}: Cache the TSEG region ......................................................................
Patch Set 2:
Patch Set 2:
Patch Set 2:
#2 With POSTCAR_STAGE=y and (CB:34995 + CB: 35026) [romstage -> postcar -> ramstage]
Aug 23
Total Time: 818,078
Total Time till picking kernel: 818,078 Total Time till picking payload: 639,809
For this commit 36b7091 exactly, I want to see full 'cbmem -t' data as I am trying to figure out the mystery of timestamp 1100, where we seem to consistently loose 10ms if we gained 7ms earlier.
0:1st timestamp 12,859 4:end of romstage 306,148 (1,829) 10:start of ramstage 333,225 (338) 30:device enumeration 378,839 (45,614) 99:selfboot jump 652,668 (469) 1100:finished vboot kernel verification 828,455 (147,282) 1101:jumping to kernel 830,962 (2,506)
Total Time: 818,078
As expected, and this is the mystery: Howcome entry 1100 in payload is increased by 10ms with an MTRR change we made in postcar? MTRRs are reprogrammed around timestamp 30 and there is cacheline invalidation (wbinvd call) at about the same time. Maybe some cachelines are stuck in non-evict mode and not available while executing 1100.
This would become the fastest approach we have seen so far if you solve 1100. Top of dram as WC/WP instead of WB? If that worked this would also be the simplest approach in comparison to any intermediate caching or POSTCAR_STAGE=n commits. And sorry, no, I have not had time to dig deeper into why you want to choose WP over WB, but definetly worth testing those. Just don't get any set_var_mtrr() calls involved yet if you experiment with it.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35026 )
Change subject: soc/intel/{cnl, icl}: Cache the TSEG region ......................................................................
Patch Set 2:
Can we get the cbmem -t numbers of commit 3b7091, with the change of using either WC of WP for both CBMEM and TSEG.
I still think there is something wrong wrt. disabling non-evict mode on the platform, behaviour of timestamp 1100 is just weird.
Hello Kyösti Mälkki, Aaron Durbin, Patrick Rudolph, Aamir Bohra, V Sowmya, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35026
to look at the new patch set (#3).
Change subject: soc/intel/{cnl, icl}: Cache the TSEG region ......................................................................
soc/intel/{cnl, icl}: Cache the TSEG region
Change-Id: Ie92d2c9e50fa299db1cd8c57a6047ea3adaf1452 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/soc/intel/cannonlake/memmap.c M src/soc/intel/icelake/memmap.c 2 files changed, 6 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/35026/3
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35026 )
Change subject: soc/intel/{cnl, icl}: Cache the TSEG region ......................................................................
Patch Set 4:
Patch Set 2:
Can we get the cbmem -t numbers of commit 3b7091, with the change of using either WC of WP for both CBMEM and TSEG.
I still think there is something wrong wrt. disabling non-evict mode on the platform, behaviour of timestamp 1100 is just weird.
please refer to https://review.coreboot.org/c/coreboot/+/34995/7 commit msg, i have captured boot time data across different MTRR type
Subrata Banik has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/35026 )
Change subject: soc/intel/{cnl, icl}: Cache the TSEG region ......................................................................
Abandoned
Hello Kyösti Mälkki, Aaron Durbin, Patrick Rudolph, Aamir Bohra, V Sowmya, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35026
to look at the new patch set (#5).
Change subject: soc/intel/{cnl, icl}: Cache the TSEG region ......................................................................
soc/intel/{cnl, icl}: Cache the TSEG region
This patch helps to save additional ~15ms of booting time in normal boot and s3 resume on CML-hatch.
BUG=b:140008206
Change-Id: Ie92d2c9e50fa299db1cd8c57a6047ea3adaf1452 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/soc/intel/cannonlake/memmap.c M src/soc/intel/icelake/memmap.c 2 files changed, 8 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/35026/5
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35026 )
Change subject: soc/intel/{cnl, icl}: Cache the TSEG region ......................................................................
Patch Set 5: Code-Review+2
Hello Kyösti Mälkki, Aaron Durbin, Patrick Rudolph, Aamir Bohra, V Sowmya, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35026
to look at the new patch set (#6).
Change subject: soc/intel/{cnl, icl}: Cache the TSEG region ......................................................................
soc/intel/{cnl, icl}: Cache the TSEG region
This patch helps to save additional ~15ms of booting time in normal boot and s3 resume on CML-hatch.
BUG=b:140008206 TEST=Verified normal boot time on CML-Hatch with latest coreboot
Without this CL: Total Time: 929ms
With this CL: (TSEG marked as WB) Total Time: 910ms
For test marked TSEG as WP/WC: Total Time: ~920ms
Change-Id: Ie92d2c9e50fa299db1cd8c57a6047ea3adaf1452 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/soc/intel/cannonlake/memmap.c M src/soc/intel/icelake/memmap.c 2 files changed, 8 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/35026/6
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35026 )
Change subject: soc/intel/{cnl, icl}: Cache the TSEG region ......................................................................
Patch Set 6: Code-Review+2
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35026 )
Change subject: soc/intel/{cnl, icl}: Cache the TSEG region ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35026/6/src/soc/intel/cannonlake/me... File src/soc/intel/cannonlake/memmap.c:
https://review.coreboot.org/c/coreboot/+/35026/6/src/soc/intel/cannonlake/me... PS6, Line 285: if (CONFIG(SMM_TSEG)) Conditional seems unnecssary.
Hello Kyösti Mälkki, Aaron Durbin, Patrick Rudolph, Aamir Bohra, V Sowmya, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35026
to look at the new patch set (#7).
Change subject: soc/intel/{cnl, icl}: Cache the TSEG region ......................................................................
soc/intel/{cnl, icl}: Cache the TSEG region
This patch helps to save additional ~15ms of booting time in normal boot and s3 resume on CML-hatch.
BUG=b:140008206 TEST=Verified normal boot time on CML-Hatch with latest coreboot
Without this CL: Total Time: 929ms
With this CL: (TSEG marked as WB) Total Time: 910ms
For test marked TSEG as WP/WC: Total Time: ~920ms
Change-Id: Ie92d2c9e50fa299db1cd8c57a6047ea3adaf1452 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/soc/intel/cannonlake/memmap.c M src/soc/intel/icelake/memmap.c 2 files changed, 6 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/35026/7
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35026 )
Change subject: soc/intel/{cnl, icl}: Cache the TSEG region ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35026/6/src/soc/intel/cannonlake/me... File src/soc/intel/cannonlake/memmap.c:
https://review.coreboot.org/c/coreboot/+/35026/6/src/soc/intel/cannonlake/me... PS6, Line 285: if (CONFIG(SMM_TSEG))
Conditional seems unnecssary.
Done
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35026 )
Change subject: soc/intel/{cnl, icl}: Cache the TSEG region ......................................................................
Patch Set 7: Code-Review+1
(2 comments)
https://review.coreboot.org/c/coreboot/+/35026/7//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35026/7//COMMIT_MSG@10 PS7, Line 10: normal boot and s3 resume on CML-hatch. 929 - 910 would be 19ms. I expect some variance here though.
https://review.coreboot.org/c/coreboot/+/35026/7//COMMIT_MSG@19 PS7, Line 19: Total Time: 910ms Aug 23 15:05 Total Time till picking kernel: 818,078
I cannot really determine from the comments if there was some major rebase of vboot or if the numbers even come from the same reference board. I'd like confirmation it was not a regression from some of the work done on common intel romstage, stack guards, or FSP heap/stack changes.
Once merged, performance data (cbmem -t) of this (and the parent) commit should be available for download somewhere. Those will become the reference when we evaluate if POSTCAR_STAGE=n makes any sense. If timestamp 1100 is now (mysteriously) fixed, POSTCAR_STAGE=n approaches are a regression for boot speed.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35026 )
Change subject: soc/intel/{cnl, icl}: Cache the TSEG region ......................................................................
Patch Set 7:
(2 comments)
https://review.coreboot.org/c/coreboot/+/35026/7//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35026/7//COMMIT_MSG@10 PS7, Line 10: normal boot and s3 resume on CML-hatch.
929 - 910 would be 19ms. I expect some variance here though.
i will update commit msg saying 19ms exactly.
https://review.coreboot.org/c/coreboot/+/35026/7//COMMIT_MSG@19 PS7, Line 19: Total Time: 910ms
Aug 23 15:05 Total Time till picking kernel: 818,078
I cannot really determine from the comments if there was some major rebase of vboot or if the numbers even come from the same reference board. I'd like confirmation it was not a regression from some of the work done on common intel romstage, stack guards, or FSP heap/stack changes.
yes, there is a boot time regression with latest FSP code base hence it moved from 800ms range to 900ms range. we are working on fixing the same. Right now boot time is 100ms extra from previous builds.
Once merged, performance data (cbmem -t) of this (and the parent) commit should be available for download somewhere.
i'm not sure if you have access to this devices, if not then may be access to the bug might be good to know the cbmem -t data that i planned to post there to close the bug.
Those will become the reference when we evaluate if POSTCAR_STAGE=n makes any sense. If timestamp 1100 is now (mysteriously) fixed, POSTCAR_STAGE=n approaches are a regression for boot speed.
i need to rebase old POSTCAR_STAGE=n Cl's with latest code base to see how much is boot time and i agree it might regress by 5-7ms for sure
Hello Kyösti Mälkki, Aaron Durbin, Patrick Rudolph, Aamir Bohra, V Sowmya, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35026
to look at the new patch set (#8).
Change subject: soc/intel/{cnl, icl}: Cache the TSEG region ......................................................................
soc/intel/{cnl, icl}: Cache the TSEG region
This patch helps to save additional ~19ms of booting time in normal boot and s3 resume on CML-hatch.
BUG=b:140008206 TEST=Verified normal boot time on CML-Hatch with latest coreboot
Without this CL: Total Time: 929ms
With this CL: (TSEG marked as WB) Total Time: 910ms
For test marked TSEG as WP/WC: Total Time: ~920ms
Change-Id: Ie92d2c9e50fa299db1cd8c57a6047ea3adaf1452 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/soc/intel/cannonlake/memmap.c M src/soc/intel/icelake/memmap.c 2 files changed, 6 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/35026/8
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35026 )
Change subject: soc/intel/{cnl, icl}: Cache the TSEG region ......................................................................
Patch Set 8: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35026 )
Change subject: soc/intel/{cnl, icl}: Cache the TSEG region ......................................................................
Patch Set 8: Code-Review+2
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35026 )
Change subject: soc/intel/{cnl, icl}: Cache the TSEG region ......................................................................
Patch Set 8: Code-Review+2
Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/35026 )
Change subject: soc/intel/{cnl, icl}: Cache the TSEG region ......................................................................
soc/intel/{cnl, icl}: Cache the TSEG region
This patch helps to save additional ~19ms of booting time in normal boot and s3 resume on CML-hatch.
BUG=b:140008206 TEST=Verified normal boot time on CML-Hatch with latest coreboot
Without this CL: Total Time: 929ms
With this CL: (TSEG marked as WB) Total Time: 910ms
For test marked TSEG as WP/WC: Total Time: ~920ms
Change-Id: Ie92d2c9e50fa299db1cd8c57a6047ea3adaf1452 Signed-off-by: Subrata Banik subrata.banik@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/35026 Reviewed-by: Aaron Durbin adurbin@chromium.org Reviewed-by: Furquan Shaikh furquan@google.com Reviewed-by: Kyösti Mälkki kyosti.malkki@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/cannonlake/memmap.c M src/soc/intel/icelake/memmap.c 2 files changed, 6 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Kyösti Mälkki: Looks good to me, approved Aaron Durbin: Looks good to me, approved Furquan Shaikh: Looks good to me, approved
diff --git a/src/soc/intel/cannonlake/memmap.c b/src/soc/intel/cannonlake/memmap.c index f0c21d9..f3286cc 100644 --- a/src/soc/intel/cannonlake/memmap.c +++ b/src/soc/intel/cannonlake/memmap.c @@ -280,4 +280,7 @@ printk(BIOS_DEBUG, "top_of_ram = 0x%lx\n", top_of_ram); top_of_ram -= 16*MiB; postcar_frame_add_mtrr(pcf, top_of_ram, 16*MiB, MTRR_TYPE_WRBACK); + + /* Cache the TSEG region */ + postcar_enable_tseg_cache(pcf); } diff --git a/src/soc/intel/icelake/memmap.c b/src/soc/intel/icelake/memmap.c index 71368c6..20c4e6f 100644 --- a/src/soc/intel/icelake/memmap.c +++ b/src/soc/intel/icelake/memmap.c @@ -278,4 +278,7 @@ printk(BIOS_DEBUG, "top_of_ram = 0x%lx\n", top_of_ram); top_of_ram -= 16*MiB; postcar_frame_add_mtrr(pcf, top_of_ram, 16*MiB, MTRR_TYPE_WRBACK); + + /* Cache the TSEG region */ + postcar_enable_tseg_cache(pcf); }
-
Aaron Durbin (Code Review) -
Furquan Shaikh (Code Review)
-
Kyösti Mälkki (Code Review)
-
Subrata Banik (Code Review)