Change in coreboot[master]: [WIP]cpu/x86/cache: CLFLUSH programs to memory before running
Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37196 ) Change subject: [WIP]cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... [WIP]cpu/x86/cache: CLFLUSH programs to memory before running When cbmem is initialized in romstage and postcar placed in the stage cache + cbmem where it is run, the assumption is made that this are all in UC memory such that calling invd in postcar is ok. For performance reasons (e.g. postcar decompression) it is desirable to cache cbmem and the stage cache during romstage. Another reason is that AGESA sets up MTRR during romstage to cache all dram, which is currently worked around by using additional MTRR to make that UC. TODO be nice to reviewers and spit some parts off. UNTESTED Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Signed-off-by: Arthur Heymans <arthur@aheymans.xyz> --- M src/arch/x86/postcar_loader.c M src/cpu/x86/cache/Makefile.inc M src/cpu/x86/cache/cache.c M src/include/cpu/x86/cache.h 4 files changed, 68 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/37196/1 diff --git a/src/arch/x86/postcar_loader.c b/src/arch/x86/postcar_loader.c index b53cbf8..53ab4f9 100644 --- a/src/arch/x86/postcar_loader.c +++ b/src/arch/x86/postcar_loader.c @@ -208,6 +208,21 @@ MTRR_TYPE_WRBACK); } +/* + * POSTCAR will call invd so don't make assumptions on cbmem + * and external stage cache being UC. + */ +static void postcar_flush_cache(void) +{ + uintptr_t stage_cache_base; + size_t stage_cache_size; + prog_segment_loaded((uintptr_t)cbmem_top(), cbmem_overhead_size(), SEG_FINAL); + if (CONFIG(TSEG_STAGE_CACHE)) { + stage_cache_external_region((void **)&stage_cache_base, &stage_cache_size); + prog_segment_loaded(stage_cache_base, stage_cache_size, SEG_FINAL); + } +} + void run_postcar_phase(struct postcar_frame *pcf) { struct prog prog = @@ -222,8 +237,10 @@ parameters between S3 resume and normal boot. On the platforms where the values are the same it's a nop. */ finalize_load(prog.arg, pcf->stack); - } else + } else { load_postcar_cbfs(&prog, pcf); + postcar_flush_cache(); + } /* As postcar exist, it's end of romstage here */ timestamp_add_now(TS_END_ROMSTAGE); diff --git a/src/cpu/x86/cache/Makefile.inc b/src/cpu/x86/cache/Makefile.inc index b33b9ee..759488e 100644 --- a/src/cpu/x86/cache/Makefile.inc +++ b/src/cpu/x86/cache/Makefile.inc @@ -1 +1,2 @@ +romstage-y += cache.c ramstage-y += cache.c diff --git a/src/cpu/x86/cache/cache.c b/src/cpu/x86/cache/cache.c index 2313c4d..ac2c45b 100644 --- a/src/cpu/x86/cache/cache.c +++ b/src/cpu/x86/cache/cache.c @@ -11,8 +11,11 @@ * GNU General Public License for more details. */ +#include <cbmem.h> +#include <program_loading.h> #include <console/console.h> #include <cpu/x86/cache.h> +#include <arch/cpu.h> void x86_enable_cache(void) { @@ -20,3 +23,47 @@ printk(BIOS_INFO, "Enabling cache\n"); enable_cache(); } + +int clflush_supported(void) +{ + return (cpuid_edx(1) >> 19) & 1; +} + +static void clflush_region(uintptr_t start, size_t size) +{ + uintptr_t addr; + size_t cl_size = (cpuid_ebx(1) >> 8) & 0xff; + if (!clflush_supported()) + return; + for (addr = (start / cl_size) * cl_size; addr < start + size; addr += cl_size) + clflush((void *)addr); +} + +static int dram_ready; + +/* + * For each segment of a program loaded this function is called + * to invalidate caches for the addresses of the loaded segment + */ +void arch_segment_loaded(uintptr_t start, size_t size, int flags) +{ + /* INVD is only called in postcar stage so we only need + to make sure that our things hit dram during romstage. */ + if (!ENV_ROMSTAGE) + return; + if (flags != SEG_FINAL) + return; + if (!dram_ready) + return; + + if (!clflush_supported()) + printk(BIOS_DEBUG, "Not flushing cache to ram, CLFLUSH not supported\n"); + clflush_region(start, size); +} + +static void set_dram_ready(int unused) +{ + dram_ready = 1; +} + +ROMSTAGE_CBMEM_INIT_HOOK(set_dram_ready); diff --git a/src/include/cpu/x86/cache.h b/src/include/cpu/x86/cache.h index 713ca32..2d2727f 100644 --- a/src/include/cpu/x86/cache.h +++ b/src/include/cpu/x86/cache.h @@ -55,6 +55,8 @@ asm volatile ("clflush (%0)"::"r" (addr)); } +int clflush_supported(void); + /* The following functions require the __always_inline due to AMD * function STOP_CAR_AND_CPU that disables cache as * RAM, the cache as RAM stack can no longer be used. Called -- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-MessageType: newchange
Hello Subrata Banik, build bot (Jenkins), Patrick Georgi, Martin Roth, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/37196 to look at the new patch set (#2). Change subject: [WIP]cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... [WIP]cpu/x86/cache: CLFLUSH programs to memory before running When cbmem is initialized in romstage and postcar placed in the stage cache + cbmem where it is run, the assumption is made that this are all in UC memory such that calling invd in postcar is ok. For performance reasons (e.g. postcar decompression) it is desirable to cache cbmem and the stage cache during romstage. Another reason is that AGESA sets up MTRR during romstage to cache all dram, which is currently worked around by using additional MTRR to make that UC. TODO be nice to reviewers and spit some parts off. UNTESTED Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Signed-off-by: Arthur Heymans <arthur@aheymans.xyz> --- M src/arch/x86/postcar_loader.c M src/cpu/x86/cache/Makefile.inc M src/cpu/x86/cache/cache.c M src/include/cpu/x86/cache.h 4 files changed, 75 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/37196/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: newpatchset
Hello Subrata Banik, build bot (Jenkins), Patrick Georgi, Martin Roth, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/37196 to look at the new patch set (#5). Change subject: [WIP]cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... [WIP]cpu/x86/cache: CLFLUSH programs to memory before running When cbmem is initialized in romstage and postcar placed in the stage cache + cbmem where it is run, the assumption is made that this are all in UC memory such that calling invd in postcar is ok. For performance reasons (e.g. postcar decompression) it is desirable to cache cbmem and the stage cache during romstage. Another reason is that AGESA sets up MTRR during romstage to cache all dram, which is currently worked around by using additional MTRR to make that UC. TODO be nice to reviewers and spit some parts off. TESTED on ASUS P5QL-EM on both regular and S3 resume bootpath. Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Signed-off-by: Arthur Heymans <arthur@aheymans.xyz> --- M src/arch/x86/postcar_loader.c M src/cpu/x86/cache/Makefile.inc M src/cpu/x86/cache/cache.c M src/include/cpu/x86/cache.h 4 files changed, 74 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/37196/5 -- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 5 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: newpatchset
Hello Subrata Banik, Marshall Dawson, build bot (Jenkins), Patrick Georgi, Martin Roth, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/37196 to look at the new patch set (#7). Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... cpu/x86/cache: CLFLUSH programs to memory before running When cbmem is initialized in romstage and postcar placed in the stage cache + cbmem where it is run, the assumption is made that these are all in UC memory such that calling INVD in postcar is ok. For performance reasons (e.g. postcar decompression) it is desirable to cache cbmem and the stage cache during romstage. Another reason is that AGESA sets up MTRR during romstage to cache all dram, which is currently worked around by using additional MTRR's to make that UC. TESTED on ASUS P5QL-EM on both regular and S3 resume bootpath. Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Signed-off-by: Arthur Heymans <arthur@aheymans.xyz> --- M src/arch/x86/postcar_loader.c M src/cpu/x86/cache/Makefile.inc M src/cpu/x86/cache/cache.c M src/include/cpu/x86/cache.h 4 files changed, 65 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/37196/7 -- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 7 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: newpatchset
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37196 ) Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... Patch Set 7: (3 comments) https://review.coreboot.org/c/coreboot/+/37196/7/src/cpu/x86/cache/cache.c File src/cpu/x86/cache/cache.c: https://review.coreboot.org/c/coreboot/+/37196/7/src/cpu/x86/cache/cache.c@2... PS7, Line 27: int clflush_supported(void) This is not sufficient. When CAR is active it's microarchitecturally defined how a clflush is handled in CAR mode. You need a way for the chipset to indicate this is valid sequence that can be performed. https://review.coreboot.org/c/coreboot/+/37196/7/src/cpu/x86/cache/cache.c@2... PS7, Line 29: 19 Aren't there defines for these bits instead of open coding them? https://review.coreboot.org/c/coreboot/+/37196/7/src/cpu/x86/cache/cache.c@6... PS7, Line 60: if (!cbmem_ready()) Why is a cbmem check in here? It doesn't immediately make sense why this would be the case. Likewise, this function is also active when not loading postcar, e.g. FSP. What about that? Or are you using cbmem to be an indirect proxy for postcar loading? -- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 7 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 06 Feb 2020 18:16:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37196 ) Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... Patch Set 7: (3 comments) https://review.coreboot.org/c/coreboot/+/37196/7/src/cpu/x86/cache/cache.c File src/cpu/x86/cache/cache.c: https://review.coreboot.org/c/coreboot/+/37196/7/src/cpu/x86/cache/cache.c@2... PS7, Line 27: int clflush_supported(void)
This is not sufficient. When CAR is active it's microarchitecturally defined how a clflush is handled in CAR mode. You need a way for the chipset to indicate this is valid sequence that can be performed.
This seems to work for Intel CPUs with a NEM and also older ones lacking it. Subrata tested a similar concept with WB caching cbmem on much more recent Intel CPU's (probably NEM enhanced). So at first sight it looks like this will be ok for most Intel? Probably worth checking for some AMD. https://review.coreboot.org/c/coreboot/+/37196/7/src/cpu/x86/cache/cache.c@2... PS7, Line 29: 19
Aren't there defines for these bits instead of open coding them?
Not yet afaik. https://review.coreboot.org/c/coreboot/+/37196/7/src/cpu/x86/cache/cache.c@6... PS7, Line 60: if (!cbmem_ready())
Why is a cbmem check in here? It doesn't immediately make sense why this would be the case. Likewise, this function is also active when not loading postcar, e.g. FSP. What about that? Or are you using cbmem to be an indirect proxy for postcar loading?
It wanted to use that as proxy that dram is ready and avoid it when things are loaded into CAR like FSP (if non-XIP). Is checking for [start, start + size] being inside the CAR region sufficient/better? -- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 7 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 06 Feb 2020 19:10:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Aaron Durbin <adurbin@chromium.org> Gerrit-MessageType: comment
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37196 ) Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... Patch Set 7: (3 comments) https://review.coreboot.org/c/coreboot/+/37196/7/src/cpu/x86/cache/cache.c File src/cpu/x86/cache/cache.c: https://review.coreboot.org/c/coreboot/+/37196/7/src/cpu/x86/cache/cache.c@2... PS7, Line 27: int clflush_supported(void)
This is not sufficient. When CAR is active it's microarchitecturally defined how a clflush is handled in CAR mode. You need a way for the chipset to indicate this is valid sequence that can be performed.
This seems to work for Intel CPUs with a NEM and also older ones lacking it. Subrata tested a similar concept with WB caching cbmem on much more recent Intel CPU's (probably NEM enhanced). So at first sight it looks like this will be ok for most Intel? Probably worth checking for some AMD.
Those results are clearly empirical which suggests that the chipsets need to opt-in to affirm this will work. https://review.coreboot.org/c/coreboot/+/37196/7/src/cpu/x86/cache/cache.c@2... PS7, Line 29: 19
Aren't there defines for these bits instead of open coding them? […] Can you start?
https://review.coreboot.org/c/coreboot/+/37196/7/src/cpu/x86/cache/cache.c@6... PS7, Line 60: if (!cbmem_ready())
Why is a cbmem check in here? It doesn't immediately make sense why this would be the case. Likewise, this function is also active when not loading postcar, e.g. FSP. What about that? Or are you using cbmem to be an indirect proxy for postcar loading?
It wanted to use that as proxy that dram is ready and avoid it when things are loaded into CAR like FSP (if non-XIP). Is checking for [start, start + size] being inside the CAR region sufficient/better?
If that's the case then please comment the purpose of what the check is for. It's clearly indirect w.r.t. to what you want accomplish from your explanation. -- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 7 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 06 Feb 2020 19:18:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Aaron Durbin <adurbin@chromium.org> Comment-In-Reply-To: Arthur Heymans <arthur@aheymans.xyz> Gerrit-MessageType: comment
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Marshall Dawson, Subrata Banik, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/37196 to look at the new patch set (#8). Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... cpu/x86/cache: CLFLUSH programs to memory before running When cbmem is initialized in romstage and postcar placed in the stage cache + cbmem where it is run, the assumption is made that these are all in UC memory such that calling INVD in postcar is OK. For performance reasons (e.g. postcar decompression) it is desirable to cache cbmem and the stage cache during romstage. Another reason is that AGESA sets up MTRR during romstage to cache all dram, which is currently worked around by using additional MTRR's to make that UC. TESTED on ASUS P5QL-EM on both regular and S3 resume bootpath. Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Signed-off-by: Arthur Heymans <arthur@aheymans.xyz> --- M src/arch/x86/postcar_loader.c M src/cpu/x86/Kconfig M src/cpu/x86/cache/Makefile.inc M src/cpu/x86/cache/cache.c M src/include/cpu/x86/cache.h 5 files changed, 77 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/37196/8 -- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 8 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: newpatchset
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Marshall Dawson, Subrata Banik, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/37196 to look at the new patch set (#10). Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... cpu/x86/cache: CLFLUSH programs to memory before running When cbmem is initialized in romstage and postcar placed in the stage cache + cbmem where it is run, the assumption is made that these are all in UC memory such that calling INVD in postcar is OK. For performance reasons (e.g. postcar decompression) it is desirable to cache cbmem and the stage cache during romstage. Another reason is that AGESA sets up MTRR during romstage to cache all dram, which is currently worked around by using additional MTRR's to make that UC. TESTED on ASUS P5QL-EM on both regular and S3 resume bootpath. Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Signed-off-by: Arthur Heymans <arthur@aheymans.xyz> --- M src/arch/x86/postcar_loader.c M src/cpu/x86/Kconfig M src/cpu/x86/cache/Makefile.inc M src/cpu/x86/cache/cache.c M src/include/cpu/x86/cache.h 5 files changed, 77 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/37196/10 -- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 10 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: newpatchset
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Marshall Dawson, Subrata Banik, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/37196 to look at the new patch set (#11). Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... cpu/x86/cache: CLFLUSH programs to memory before running When cbmem is initialized in romstage and postcar placed in the stage cache + cbmem where it is run, the assumption is made that these are all in UC memory such that calling INVD in postcar is OK. For performance reasons (e.g. postcar decompression) it is desirable to cache cbmem and the stage cache during romstage. Another reason is that AGESA sets up MTRR during romstage to cache all dram, which is currently worked around by using additional MTRR's to make that UC. TESTED on ASUS P5QL-EM on both regular and S3 resume bootpath. Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Signed-off-by: Arthur Heymans <arthur@aheymans.xyz> --- M src/arch/x86/postcar_loader.c M src/cpu/x86/Kconfig M src/cpu/x86/cache/Makefile.inc M src/cpu/x86/cache/cache.c M src/include/cpu/x86/cache.h 5 files changed, 81 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/37196/11 -- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 11 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: newpatchset
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37196 ) Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... Patch Set 11: (3 comments) https://review.coreboot.org/c/coreboot/+/37196/7/src/cpu/x86/cache/cache.c File src/cpu/x86/cache/cache.c: https://review.coreboot.org/c/coreboot/+/37196/7/src/cpu/x86/cache/cache.c@2... PS7, Line 27: int clflush_supported(void)
This is not sufficient. […] Done. Added a Kconfig flag X86_CLFLUSH_CAR
https://review.coreboot.org/c/coreboot/+/37196/7/src/cpu/x86/cache/cache.c@2... PS7, Line 29: 19
Can you start? Done
https://review.coreboot.org/c/coreboot/+/37196/7/src/cpu/x86/cache/cache.c@6... PS7, Line 60: if (!cbmem_ready())
Why is a cbmem check in here? It doesn't immediately make sense why this would be the case. […] Done
-- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 11 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Fri, 07 Aug 2020 16:28:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Arthur Heymans <arthur@aheymans.xyz> Comment-In-Reply-To: Aaron Durbin <adurbin@chromium.org> Gerrit-MessageType: comment
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37196 ) Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... Patch Set 12: (1 comment) https://review.coreboot.org/c/coreboot/+/37196/12//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/37196/12//COMMIT_MSG@20 PS12, Line 20: TESTED on ASUS P5QL-EM on both regular and S3 resume bootpath. Could you verify speed-ups (above you mention “performance reasons”)? -- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 12 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Sat, 08 Aug 2020 06:26:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37196 ) Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... Patch Set 12: Code-Review+1 (1 comment) https://review.coreboot.org/c/coreboot/+/37196/12//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/37196/12//COMMIT_MSG@20 PS12, Line 20: TESTED on ASUS P5QL-EM on both regular and S3 resume bootpath.
Could you verify speed-ups (above you mention “performance reasons”)? I mean, if you have the timestamps before and after, could you please add them?
-- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 12 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-Comment-Date: Mon, 09 Nov 2020 13:49:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: comment
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37196 ) Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... Patch Set 12: (5 comments) https://review.coreboot.org/c/coreboot/+/37196/12/src/cpu/x86/cache/cache.c File src/cpu/x86/cache/cache.c: https://review.coreboot.org/c/coreboot/+/37196/12/src/cpu/x86/cache/cache.c@... PS12, Line 16: int bool? https://review.coreboot.org/c/coreboot/+/37196/12/src/cpu/x86/cache/cache.c@... PS12, Line 21: ENV_POSTCAR || ENV_RAMSTAGE Isn't there a symbol for pre-RAM stages? https://review.coreboot.org/c/coreboot/+/37196/12/src/cpu/x86/cache/cache.c@... PS12, Line 24: CONFIG(X86_CLFLUSH_CAR) Why not place the constant on the left-hand side of the operation? https://review.coreboot.org/c/coreboot/+/37196/12/src/cpu/x86/cache/cache.c@... PS12, Line 29: uintptr_t types.h https://review.coreboot.org/c/coreboot/+/37196/12/src/cpu/x86/cache/cache.c@... PS12, Line 33: ram RAM -- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 12 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Angel Pons <th3fanbus@gmail.com> Gerrit-Comment-Date: Mon, 09 Nov 2020 18:53:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37196 ) Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... Patch Set 13: (2 comments) https://review.coreboot.org/c/coreboot/+/37196/12/src/cpu/x86/cache/cache.c File src/cpu/x86/cache/cache.c: https://review.coreboot.org/c/coreboot/+/37196/12/src/cpu/x86/cache/cache.c@... PS12, Line 21: ENV_POSTCAR || ENV_RAMSTAGE
Isn't there a symbol for pre-RAM stages? `!ENV_CACHE_AS_RAM` might be a good choice, it also covers the `RESET_VECTOR_IN_RAM` case.
https://review.coreboot.org/c/coreboot/+/37196/12/src/cpu/x86/cache/cache.c@... PS12, Line 24: CONFIG(X86_CLFLUSH_CAR)
Why not place the constant on the left-hand side of the operation? I'm curious, does the compiler optimize it differently?
As the the operand has no side effects, it should be safe to treat it the same. -- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 13 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Angel Pons <th3fanbus@gmail.com> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-Comment-Date: Thu, 03 Dec 2020 23:42:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus@gmail.com> Gerrit-MessageType: comment
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37196 ) Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... Patch Set 13: (1 comment) https://review.coreboot.org/c/coreboot/+/37196/12/src/cpu/x86/cache/cache.c File src/cpu/x86/cache/cache.c: https://review.coreboot.org/c/coreboot/+/37196/12/src/cpu/x86/cache/cache.c@... PS12, Line 24: CONFIG(X86_CLFLUSH_CAR)
I'm curious, does the compiler optimize it differently? […] I'm not sure if the compiler can infer that cpuid_edx has no side-effect, so it may not be able to optimize out the code as-is.
In any case, I'd write this function as follows: bool clflush_supported(void) { /* CLFLUSH while operating in CAR might not be supported by platforms */ if (ENV_CACHE_AS_RAM && !CONFIG(X86_CLFLUSH_CAR)) return false; return !!(cpuid_edx(1) >> CPUID_FEATURE_CLFLUSH_BIT); } -- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 13 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Angel Pons <th3fanbus@gmail.com> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-Comment-Date: Sat, 05 Dec 2020 18:35:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h@gmx.de> Comment-In-Reply-To: Angel Pons <th3fanbus@gmail.com> Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37196 ) Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... Patch Set 14: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 14 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-Attention: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Comment-Date: Tue, 23 Mar 2021 00:07:36 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37196 ) Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... Patch Set 14: Code-Review+1 (1 comment) Patchset: PS14: Can this be rebased? -- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 14 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Paul Menzel <paulepanter@mailbox.org> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Furquan Shaikh <furquan.m.shaikh@gmail.com> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-Attention: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Comment-Date: Thu, 09 Dec 2021 12:45:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans. Hello build bot (Jenkins), Martin Roth, Marshall Dawson, Paul Menzel, Angel Pons, Subrata Banik, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/37196 to look at the new patch set (#15). Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... cpu/x86/cache: CLFLUSH programs to memory before running When cbmem is initialized in romstage and postcar placed in the stage cache + cbmem where it is run, the assumption is made that these are all in UC memory such that calling INVD in postcar is OK. For performance reasons (e.g. postcar decompression) it is desirable to cache cbmem and the stage cache during romstage. Another reason is that AGESA sets up MTRR during romstage to cache all dram, which is currently worked around by using additional MTRR's to make that UC. TESTED on ASUS P5QL-EM on both regular and S3 resume bootpath. Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Signed-off-by: Arthur Heymans <arthur@aheymans.xyz> --- M src/arch/x86/postcar_loader.c M src/cpu/x86/Kconfig M src/cpu/x86/Makefile.inc A src/cpu/x86/cache/Makefile.inc A src/cpu/x86/cache/cache.c M src/include/cpu/x86/cache.h 6 files changed, 91 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/37196/15 -- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 15 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Paul Menzel <paulepanter@mailbox.org> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Furquan Shaikh <furquan.m.shaikh@gmail.com> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-Attention: Arthur Heymans <arthur@aheymans.xyz> Gerrit-MessageType: newpatchset
Attention is currently required from: Nico Huber, Angel Pons. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37196 ) Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... Patch Set 15: (5 comments) File src/cpu/x86/cache/cache.c: https://review.coreboot.org/c/coreboot/+/37196/comment/adfd21fb_4816c5ff PS12, Line 16: int
bool? Done
https://review.coreboot.org/c/coreboot/+/37196/comment/0d31a398_68df3ece PS12, Line 21: ENV_POSTCAR || ENV_RAMSTAGE
`!ENV_CACHE_AS_RAM` might be a good choice, it also covers the […] Done
https://review.coreboot.org/c/coreboot/+/37196/comment/6458fd9a_c9dcd380 PS12, Line 24: CONFIG(X86_CLFLUSH_CAR)
I'm not sure if the compiler can infer that cpuid_edx has no side-effect, so it may not be able to optimize out the code as-is.
In any case, I'd write this function as follows:
bool clflush_supported(void) { /* CLFLUSH while operating in CAR might not be supported by platforms */ if (ENV_CACHE_AS_RAM && !CONFIG(X86_CLFLUSH_CAR)) return false;
return !!(cpuid_edx(1) >> CPUID_FEATURE_CLFLUSH_BIT); }
I sort of rewrote it. https://review.coreboot.org/c/coreboot/+/37196/comment/464c0b97_bb3362bc PS12, Line 29: uintptr_t
types. […] Done
https://review.coreboot.org/c/coreboot/+/37196/comment/650ca424_6dd5006f PS12, Line 33: ram
RAM Done
-- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 15 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Paul Menzel <paulepanter@mailbox.org> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Furquan Shaikh <furquan.m.shaikh@gmail.com> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-Attention: Nico Huber <nico.h@gmx.de> Gerrit-Attention: Angel Pons <th3fanbus@gmail.com> Gerrit-Comment-Date: Tue, 12 Apr 2022 20:35:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h@gmx.de> Comment-In-Reply-To: Angel Pons <th3fanbus@gmail.com> Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Angel Pons. Hello build bot (Jenkins), Martin Roth, Marshall Dawson, Paul Menzel, Angel Pons, Subrata Banik, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/37196 to look at the new patch set (#16). Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... cpu/x86/cache: CLFLUSH programs to memory before running When cbmem is initialized in romstage and postcar placed in the stage cache + cbmem where it is run, the assumption is made that these are all in UC memory such that calling INVD in postcar is OK. For performance reasons (e.g. postcar decompression) it is desirable to cache cbmem and the stage cache during romstage. Another reason is that AGESA sets up MTRR during romstage to cache all dram, which is currently worked around by using additional MTRR's to make that UC. TESTED on asus/p5ql-em, up/squared on both regular and S3 resume bootpath. Sometimes there are minimal performance improvements when cbmem is cached (few ms). Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Signed-off-by: Arthur Heymans <arthur@aheymans.xyz> --- M src/arch/x86/postcar_loader.c M src/cpu/x86/Kconfig M src/cpu/x86/Makefile.inc A src/cpu/x86/cache/Makefile.inc A src/cpu/x86/cache/cache.c M src/include/cpu/x86/cache.h 6 files changed, 91 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/37196/16 -- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 16 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Paul Menzel <paulepanter@mailbox.org> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Furquan Shaikh <furquan.m.shaikh@gmail.com> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-Attention: Nico Huber <nico.h@gmx.de> Gerrit-Attention: Angel Pons <th3fanbus@gmail.com> Gerrit-MessageType: newpatchset
Attention is currently required from: Nico Huber, Paul Menzel, Angel Pons. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37196 ) Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... Patch Set 15: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/37196/comment/50ad088b_dcab11b0 PS12, Line 20: TESTED on ASUS P5QL-EM on both regular and S3 resume bootpath.
I mean, if you have the timestamps before and after, could you please add them?
Timestamp differences are minimal. The smaller footprint due to compression is the major win here. -- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 15 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Paul Menzel <paulepanter@mailbox.org> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Furquan Shaikh <furquan.m.shaikh@gmail.com> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-Attention: Nico Huber <nico.h@gmx.de> Gerrit-Attention: Paul Menzel <paulepanter@mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus@gmail.com> Gerrit-Comment-Date: Tue, 12 Apr 2022 20:35:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter@mailbox.org> Gerrit-MessageType: comment
Attention is currently required from: Raul Rangel, Nico Huber, Michał Żygowski, Michał Kopeć, Angel Pons, Arthur Heymans, Felix Held. Reka Norman has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37196 ) Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... Patch Set 19: (1 comment) File src/arch/x86/postcar_loader.c: https://review.coreboot.org/c/coreboot/+/37196/comment/4fe23d44_cb5eedad PS19, Line 178: timestamp_add_now(TS_ROMSTAGE_END); : : console_time_report(); : This timestamp and log are lost because cbmem is still cached here. Do you need to remove the extra MTRRs before flushing? -- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 19 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless@gmail.com> Gerrit-Reviewer: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Paul Menzel <paulepanter@mailbox.org> Gerrit-Reviewer: Raul Rangel <rrangel@chromium.org> Gerrit-Reviewer: Sean Rhodes <sean@starlabs.systems> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Furquan Shaikh <furquan.m.shaikh@gmail.com> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Raul Rangel <rrangel@chromium.org> Gerrit-Attention: Nico Huber <nico.h@gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Attention: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Attention: Angel Pons <th3fanbus@gmail.com> Gerrit-Attention: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Attention: Felix Held <felix-coreboot@felixheld.de> Gerrit-Comment-Date: Mon, 19 Sep 2022 06:13:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Attention is currently required from: Raul Rangel, Nico Huber, Michał Żygowski, Reka Norman, Michał Kopeć, Angel Pons, Felix Held. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37196 ) Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... Patch Set 19: (1 comment) File src/arch/x86/postcar_loader.c: https://review.coreboot.org/c/coreboot/+/37196/comment/3e4997ff_933bea63 PS19, Line 178: timestamp_add_now(TS_ROMSTAGE_END); : : console_time_report(); :
This timestamp and log are lost because cbmem is still cached here. Do you need to remove the extra MTRRs before flushing?
I think it's best to move it up. Nice catch -- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 19 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless@gmail.com> Gerrit-Reviewer: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Paul Menzel <paulepanter@mailbox.org> Gerrit-Reviewer: Raul Rangel <rrangel@chromium.org> Gerrit-Reviewer: Sean Rhodes <sean@starlabs.systems> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Furquan Shaikh <furquan.m.shaikh@gmail.com> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Raul Rangel <rrangel@chromium.org> Gerrit-Attention: Nico Huber <nico.h@gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Attention: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Attention: Angel Pons <th3fanbus@gmail.com> Gerrit-Attention: Felix Held <felix-coreboot@felixheld.de> Gerrit-Comment-Date: Mon, 10 Oct 2022 16:43:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Reka Norman <rekanorman@chromium.org> Gerrit-MessageType: comment
Attention is currently required from: Raul Rangel, Nico Huber, Michał Żygowski, Reka Norman, Michał Kopeć, Angel Pons, Arthur Heymans, Felix Held. Marc Jones has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37196 ) Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... Patch Set 19: (1 comment) File src/cpu/x86/cache/cache.c: https://review.coreboot.org/c/coreboot/+/37196/comment/477c330a_082d7e04 PS19, Line 49: I don't know if there is style guidance around this, but I find this easier to read. if (!ENV_CACHE_AS_RAM && !ENV_ROMSTAGE && !CONFIG(POSTCAR_STAGE) && !CONFIG(X86_CLFLUSH_CAR) && flags != SEG_FINAL) return; -- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 19 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless@gmail.com> Gerrit-Reviewer: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Paul Menzel <paulepanter@mailbox.org> Gerrit-Reviewer: Raul Rangel <rrangel@chromium.org> Gerrit-Reviewer: Sean Rhodes <sean@starlabs.systems> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Furquan Shaikh <furquan.m.shaikh@gmail.com> Gerrit-CC: Marc Jones <marc@marcjonesconsulting.com> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Raul Rangel <rrangel@chromium.org> Gerrit-Attention: Nico Huber <nico.h@gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Attention: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Attention: Angel Pons <th3fanbus@gmail.com> Gerrit-Attention: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Attention: Felix Held <felix-coreboot@felixheld.de> Gerrit-Comment-Date: Fri, 14 Oct 2022 19:00:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Attention is currently required from: Raul Rangel, Nico Huber, Michał Żygowski, Reka Norman, Michał Kopeć, Angel Pons, Felix Held. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37196 ) Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... Patch Set 19: (1 comment) File src/arch/x86/postcar_loader.c: https://review.coreboot.org/c/coreboot/+/37196/comment/3b2c85cc_5ccc55fe PS19, Line 152: if (CONFIG(TSEG_STAGE_CACHE)) { : stage_cache_external_region((void **)&stage_cache_base, &stage_cache_size); : prog_segment_loaded(stage_cache_base, stage_cache_size, SEG_FINAL); : } Don't do that on s3 resume -- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 19 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless@gmail.com> Gerrit-Reviewer: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Paul Menzel <paulepanter@mailbox.org> Gerrit-Reviewer: Raul Rangel <rrangel@chromium.org> Gerrit-Reviewer: Sean Rhodes <sean@starlabs.systems> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Furquan Shaikh <furquan.m.shaikh@gmail.com> Gerrit-CC: Marc Jones <marc@marcjonesconsulting.com> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Raul Rangel <rrangel@chromium.org> Gerrit-Attention: Nico Huber <nico.h@gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Attention: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Attention: Angel Pons <th3fanbus@gmail.com> Gerrit-Attention: Felix Held <felix-coreboot@felixheld.de> Gerrit-Comment-Date: Fri, 21 Oct 2022 08:21:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Attention is currently required from: Raul Rangel, Nico Huber, Michał Żygowski, Subrata Banik, Reka Norman, Michał Kopeć, Angel Pons, Arthur Heymans, Felix Held. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37196 ) Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... Patch Set 19: Code-Review+2 (1 comment) Patchset: PS19: Tested on MTL (Rex). Looking forward to get this CL in sooner. -- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 19 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless@gmail.com> Gerrit-Reviewer: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Paul Menzel <paulepanter@mailbox.org> Gerrit-Reviewer: Raul Rangel <rrangel@chromium.org> Gerrit-Reviewer: Sean Rhodes <sean@starlabs.systems> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Furquan Shaikh <furquan.m.shaikh@gmail.com> Gerrit-CC: Marc Jones <marc@marcjonesconsulting.com> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Raul Rangel <rrangel@chromium.org> Gerrit-Attention: Nico Huber <nico.h@gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Attention: Subrata Banik <subratabanik@google.com> Gerrit-Attention: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Attention: Angel Pons <th3fanbus@gmail.com> Gerrit-Attention: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Attention: Felix Held <felix-coreboot@felixheld.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 10:19:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Attention is currently required from: Raul Rangel, Nico Huber, Michał Żygowski, Reka Norman, Michał Kopeć, Angel Pons, Arthur Heymans, Felix Held. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37196 ) Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... Patch Set 20: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 20 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless@gmail.com> Gerrit-Reviewer: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Paul Menzel <paulepanter@mailbox.org> Gerrit-Reviewer: Raul Rangel <rrangel@chromium.org> Gerrit-Reviewer: Sean Rhodes <sean@starlabs.systems> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Furquan Shaikh <furquan.m.shaikh@gmail.com> Gerrit-CC: Marc Jones <marc@marcjonesconsulting.com> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Raul Rangel <rrangel@chromium.org> Gerrit-Attention: Nico Huber <nico.h@gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Attention: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Attention: Angel Pons <th3fanbus@gmail.com> Gerrit-Attention: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Attention: Felix Held <felix-coreboot@felixheld.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 10:26:53 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Attention is currently required from: Raul Rangel, Nico Huber, Michał Żygowski, Subrata Banik, Reka Norman, Michał Kopeć, Angel Pons, Arthur Heymans, Felix Held. Lean Sheng Tan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37196 ) Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... Patch Set 20: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 20 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan@9elements.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless@gmail.com> Gerrit-Reviewer: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Paul Menzel <paulepanter@mailbox.org> Gerrit-Reviewer: Raul Rangel <rrangel@chromium.org> Gerrit-Reviewer: Sean Rhodes <sean@starlabs.systems> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Furquan Shaikh <furquan.m.shaikh@gmail.com> Gerrit-CC: Marc Jones <marc@marcjonesconsulting.com> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Raul Rangel <rrangel@chromium.org> Gerrit-Attention: Nico Huber <nico.h@gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Attention: Subrata Banik <subratabanik@google.com> Gerrit-Attention: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Attention: Angel Pons <th3fanbus@gmail.com> Gerrit-Attention: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Attention: Felix Held <felix-coreboot@felixheld.de> Gerrit-Comment-Date: Fri, 03 Mar 2023 00:34:59 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Attention is currently required from: Raul Rangel, Nico Huber, Michał Żygowski, Subrata Banik, Reka Norman, Michał Kopeć, Angel Pons, Arthur Heymans, Felix Held. Subrata Banik has uploaded a new patch set (#21) to the change originally created by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/37196 ) Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... cpu/x86/cache: CLFLUSH programs to memory before running When cbmem is initialized in romstage and postcar placed in the stage cache + cbmem where it is run, the assumption is made that these are all in UC memory such that calling INVD in postcar is OK. For performance reasons (e.g. postcar decompression) it is desirable to cache cbmem and the stage cache during romstage. Another reason is that AGESA sets up MTRR during romstage to cache all dram, which is currently worked around by using additional MTRR's to make that UC. TESTED on asus/p5ql-em, up/squared on both regular and S3 resume bootpath. Sometimes there are minimal performance improvements when cbmem is cached (few ms). Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Signed-off-by: Arthur Heymans <arthur@aheymans.xyz> --- M src/arch/x86/postcar_loader.c M src/cpu/x86/Kconfig M src/cpu/x86/Makefile.inc A src/cpu/x86/cache/Makefile.inc A src/cpu/x86/cache/cache.c M src/include/cpu/x86/cache.h 6 files changed, 117 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/37196/21 -- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 21 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan@9elements.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless@gmail.com> Gerrit-Reviewer: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Paul Menzel <paulepanter@mailbox.org> Gerrit-Reviewer: Raul Rangel <rrangel@chromium.org> Gerrit-Reviewer: Sean Rhodes <sean@starlabs.systems> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Furquan Shaikh <furquan.m.shaikh@gmail.com> Gerrit-CC: Marc Jones <marc@marcjonesconsulting.com> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Raul Rangel <rrangel@chromium.org> Gerrit-Attention: Nico Huber <nico.h@gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Attention: Subrata Banik <subratabanik@google.com> Gerrit-Attention: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Attention: Angel Pons <th3fanbus@gmail.com> Gerrit-Attention: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Attention: Felix Held <felix-coreboot@felixheld.de> Gerrit-MessageType: newpatchset
Attention is currently required from: Raul Rangel, Nico Huber, Michał Żygowski, Reka Norman, Michał Kopeć, Angel Pons, Arthur Heymans, Felix Held. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37196 ) Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... Patch Set 21: (2 comments) File src/arch/x86/postcar_loader.c: https://review.coreboot.org/c/coreboot/+/37196/comment/d2cc3baf_24ec7169 PS19, Line 152: if (CONFIG(TSEG_STAGE_CACHE)) { : stage_cache_external_region((void **)&stage_cache_base, &stage_cache_size); : prog_segment_loaded(stage_cache_base, stage_cache_size, SEG_FINAL); : }
Don't do that on s3 resume Ack
https://review.coreboot.org/c/coreboot/+/37196/comment/d1decff6_b41cbc61 PS19, Line 178: timestamp_add_now(TS_ROMSTAGE_END); : : console_time_report(); :
This timestamp and log are lost because cbmem is still cached here. […] Ack
-- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 21 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan@9elements.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless@gmail.com> Gerrit-Reviewer: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Paul Menzel <paulepanter@mailbox.org> Gerrit-Reviewer: Raul Rangel <rrangel@chromium.org> Gerrit-Reviewer: Sean Rhodes <sean@starlabs.systems> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Furquan Shaikh <furquan.m.shaikh@gmail.com> Gerrit-CC: Marc Jones <marc@marcjonesconsulting.com> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Raul Rangel <rrangel@chromium.org> Gerrit-Attention: Nico Huber <nico.h@gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Attention: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Attention: Angel Pons <th3fanbus@gmail.com> Gerrit-Attention: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Attention: Felix Held <felix-coreboot@felixheld.de> Gerrit-Comment-Date: Tue, 07 Mar 2023 19:54:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Reka Norman <rekanorman@chromium.org> Comment-In-Reply-To: Arthur Heymans <arthur@aheymans.xyz> Gerrit-MessageType: comment
Attention is currently required from: Raul Rangel, Nico Huber, Michał Żygowski, Reka Norman, Michał Kopeć, Angel Pons, Arthur Heymans, Felix Held. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37196 ) Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... Patch Set 21: (1 comment) File src/arch/x86/postcar_loader.c: https://review.coreboot.org/c/coreboot/+/37196/comment/a31d4f2e_245f1eba PS21, Line 154: acpi_is_wakeup_s3 getting compilation issue on older chipsets like BSW. need to fix this. -- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 21 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan@9elements.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless@gmail.com> Gerrit-Reviewer: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Paul Menzel <paulepanter@mailbox.org> Gerrit-Reviewer: Raul Rangel <rrangel@chromium.org> Gerrit-Reviewer: Sean Rhodes <sean@starlabs.systems> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Furquan Shaikh <furquan.m.shaikh@gmail.com> Gerrit-CC: Marc Jones <marc@marcjonesconsulting.com> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Raul Rangel <rrangel@chromium.org> Gerrit-Attention: Nico Huber <nico.h@gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Attention: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Attention: Angel Pons <th3fanbus@gmail.com> Gerrit-Attention: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Attention: Felix Held <felix-coreboot@felixheld.de> Gerrit-Comment-Date: Tue, 07 Mar 2023 20:35:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Attention is currently required from: Raul Rangel, Nico Huber, Michał Żygowski, Reka Norman, Michał Kopeć, Angel Pons, Arthur Heymans, Felix Held. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37196 ) Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... Patch Set 21: (1 comment) File src/arch/x86/postcar_loader.c: https://review.coreboot.org/c/coreboot/+/37196/comment/a87beaa3_fbbe92f5 PS21, Line 154: acpi_is_wakeup_s3
getting compilation issue on older chipsets like BSW. need to fix this. Ack
-- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 21 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan@9elements.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless@gmail.com> Gerrit-Reviewer: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Paul Menzel <paulepanter@mailbox.org> Gerrit-Reviewer: Raul Rangel <rrangel@chromium.org> Gerrit-Reviewer: Sean Rhodes <sean@starlabs.systems> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Furquan Shaikh <furquan.m.shaikh@gmail.com> Gerrit-CC: Marc Jones <marc@marcjonesconsulting.com> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Raul Rangel <rrangel@chromium.org> Gerrit-Attention: Nico Huber <nico.h@gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Attention: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Attention: Angel Pons <th3fanbus@gmail.com> Gerrit-Attention: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Attention: Felix Held <felix-coreboot@felixheld.de> Gerrit-Comment-Date: Wed, 08 Mar 2023 06:57:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik@google.com> Gerrit-MessageType: comment
Attention is currently required from: Raul Rangel, Nico Huber, Michał Żygowski, Reka Norman, Michał Kopeć, Angel Pons, Arthur Heymans, Felix Held. Subrata Banik has uploaded a new patch set (#22) to the change originally created by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/37196 ) Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... cpu/x86/cache: CLFLUSH programs to memory before running When cbmem is initialized in romstage and postcar placed in the stage cache + cbmem where it is run, the assumption is made that these are all in UC memory such that calling INVD in postcar is OK. For performance reasons (e.g. postcar decompression) it is desirable to cache cbmem and the stage cache during romstage. Another reason is that AGESA sets up MTRR during romstage to cache all dram, which is currently worked around by using additional MTRR's to make that UC. TESTED on asus/p5ql-em, up/squared on both regular and S3 resume bootpath. Sometimes there are minimal performance improvements when cbmem is cached (few ms). Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Signed-off-by: Arthur Heymans <arthur@aheymans.xyz> --- M src/arch/x86/postcar_loader.c M src/cpu/x86/Kconfig M src/cpu/x86/Makefile.inc A src/cpu/x86/cache/Makefile.inc A src/cpu/x86/cache/cache.c M src/include/cpu/x86/cache.h 6 files changed, 117 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/37196/22 -- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 22 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan@9elements.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless@gmail.com> Gerrit-Reviewer: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Paul Menzel <paulepanter@mailbox.org> Gerrit-Reviewer: Raul Rangel <rrangel@chromium.org> Gerrit-Reviewer: Sean Rhodes <sean@starlabs.systems> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Furquan Shaikh <furquan.m.shaikh@gmail.com> Gerrit-CC: Marc Jones <marc@marcjonesconsulting.com> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Raul Rangel <rrangel@chromium.org> Gerrit-Attention: Nico Huber <nico.h@gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Attention: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Attention: Angel Pons <th3fanbus@gmail.com> Gerrit-Attention: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Attention: Felix Held <felix-coreboot@felixheld.de> Gerrit-MessageType: newpatchset
Attention is currently required from: Raul Rangel, Nico Huber, Michał Żygowski, Subrata Banik, Reka Norman, Michał Kopeć, Angel Pons, Arthur Heymans, Felix Held. Lean Sheng Tan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37196 ) Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... Patch Set 22: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 22 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan@9elements.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless@gmail.com> Gerrit-Reviewer: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Paul Menzel <paulepanter@mailbox.org> Gerrit-Reviewer: Raul Rangel <rrangel@chromium.org> Gerrit-Reviewer: Sean Rhodes <sean@starlabs.systems> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Furquan Shaikh <furquan.m.shaikh@gmail.com> Gerrit-CC: Marc Jones <marc@marcjonesconsulting.com> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Raul Rangel <rrangel@chromium.org> Gerrit-Attention: Nico Huber <nico.h@gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Attention: Subrata Banik <subratabanik@google.com> Gerrit-Attention: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Attention: Angel Pons <th3fanbus@gmail.com> Gerrit-Attention: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Attention: Felix Held <felix-coreboot@felixheld.de> Gerrit-Comment-Date: Wed, 08 Mar 2023 08:39:46 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Attention is currently required from: Raul Rangel, Nico Huber, Michał Żygowski, Subrata Banik, Reka Norman, Michał Kopeć, Angel Pons, Arthur Heymans, Felix Held. Kapil Porwal has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37196 ) Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... Patch Set 22: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 22 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de> Gerrit-Reviewer: Kapil Porwal <kapilporwal@google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan@9elements.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless@gmail.com> Gerrit-Reviewer: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Paul Menzel <paulepanter@mailbox.org> Gerrit-Reviewer: Raul Rangel <rrangel@chromium.org> Gerrit-Reviewer: Sean Rhodes <sean@starlabs.systems> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Furquan Shaikh <furquan.m.shaikh@gmail.com> Gerrit-CC: Marc Jones <marc@marcjonesconsulting.com> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Raul Rangel <rrangel@chromium.org> Gerrit-Attention: Nico Huber <nico.h@gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Attention: Subrata Banik <subratabanik@google.com> Gerrit-Attention: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Attention: Angel Pons <th3fanbus@gmail.com> Gerrit-Attention: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Attention: Felix Held <felix-coreboot@felixheld.de> Gerrit-Comment-Date: Wed, 08 Mar 2023 14:08:52 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Attention is currently required from: Raul Rangel, Nico Huber, Michał Żygowski, Subrata Banik, Reka Norman, Michał Kopeć, Angel Pons, Arthur Heymans, Felix Held. Subrata Banik has uploaded a new patch set (#23) to the change originally created by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/37196 ) Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... cpu/x86/cache: CLFLUSH programs to memory before running When cbmem is initialized in romstage and postcar placed in the stage cache + cbmem where it is run, the assumption is made that these are all in UC memory such that calling INVD in postcar is OK. For performance reasons (e.g. postcar decompression) it is desirable to cache cbmem and the stage cache during romstage. Another reason is that AGESA sets up MTRR during romstage to cache all dram, which is currently worked around by using additional MTRR's to make that UC. TESTED on asus/p5ql-em, up/squared on both regular and S3 resume bootpath. Sometimes there are minimal performance improvements when cbmem is cached (few ms). Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Signed-off-by: Arthur Heymans <arthur@aheymans.xyz> --- M src/arch/x86/postcar_loader.c M src/cpu/x86/Kconfig M src/cpu/x86/Makefile.inc A src/cpu/x86/cache/Makefile.inc A src/cpu/x86/cache/cache.c M src/include/cpu/x86/cache.h 6 files changed, 117 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/37196/23 -- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 23 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de> Gerrit-Reviewer: Kapil Porwal <kapilporwal@google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan@9elements.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless@gmail.com> Gerrit-Reviewer: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Paul Menzel <paulepanter@mailbox.org> Gerrit-Reviewer: Raul Rangel <rrangel@chromium.org> Gerrit-Reviewer: Sean Rhodes <sean@starlabs.systems> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Furquan Shaikh <furquan.m.shaikh@gmail.com> Gerrit-CC: Marc Jones <marc@marcjonesconsulting.com> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Raul Rangel <rrangel@chromium.org> Gerrit-Attention: Nico Huber <nico.h@gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Attention: Subrata Banik <subratabanik@google.com> Gerrit-Attention: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Attention: Angel Pons <th3fanbus@gmail.com> Gerrit-Attention: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Attention: Felix Held <felix-coreboot@felixheld.de> Gerrit-MessageType: newpatchset
Attention is currently required from: Raul Rangel, Nico Huber, Michał Żygowski, Subrata Banik, Reka Norman, Michał Kopeć, Angel Pons, Arthur Heymans, Felix Held. Lean Sheng Tan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37196 ) Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... Patch Set 23: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 23 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de> Gerrit-Reviewer: Kapil Porwal <kapilporwal@google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan@9elements.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless@gmail.com> Gerrit-Reviewer: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Paul Menzel <paulepanter@mailbox.org> Gerrit-Reviewer: Raul Rangel <rrangel@chromium.org> Gerrit-Reviewer: Sean Rhodes <sean@starlabs.systems> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Furquan Shaikh <furquan.m.shaikh@gmail.com> Gerrit-CC: Marc Jones <marc@marcjonesconsulting.com> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Raul Rangel <rrangel@chromium.org> Gerrit-Attention: Nico Huber <nico.h@gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Attention: Subrata Banik <subratabanik@google.com> Gerrit-Attention: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Attention: Angel Pons <th3fanbus@gmail.com> Gerrit-Attention: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Attention: Felix Held <felix-coreboot@felixheld.de> Gerrit-Comment-Date: Thu, 09 Mar 2023 08:57:00 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Attention is currently required from: Raul Rangel, Nico Huber, Michał Żygowski, Subrata Banik, Reka Norman, Michał Kopeć, Angel Pons, Arthur Heymans, Felix Held. Kapil Porwal has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37196 ) Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... Patch Set 23: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 23 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de> Gerrit-Reviewer: Kapil Porwal <kapilporwal@google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan@9elements.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless@gmail.com> Gerrit-Reviewer: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Paul Menzel <paulepanter@mailbox.org> Gerrit-Reviewer: Raul Rangel <rrangel@chromium.org> Gerrit-Reviewer: Sean Rhodes <sean@starlabs.systems> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Furquan Shaikh <furquan.m.shaikh@gmail.com> Gerrit-CC: Marc Jones <marc@marcjonesconsulting.com> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Raul Rangel <rrangel@chromium.org> Gerrit-Attention: Nico Huber <nico.h@gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Attention: Subrata Banik <subratabanik@google.com> Gerrit-Attention: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Attention: Angel Pons <th3fanbus@gmail.com> Gerrit-Attention: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Attention: Felix Held <felix-coreboot@felixheld.de> Gerrit-Comment-Date: Thu, 09 Mar 2023 12:13:06 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Attention is currently required from: Raul Rangel, Nico Huber, Michał Żygowski, Subrata Banik, Reka Norman, Michał Kopeć, Angel Pons, Arthur Heymans, Felix Held. Lean Sheng Tan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37196 ) Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... Patch Set 23: (1 comment) Patchset: PS23: will this config also helps Xeon platforms? -- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 23 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de> Gerrit-Reviewer: Kapil Porwal <kapilporwal@google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan@9elements.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless@gmail.com> Gerrit-Reviewer: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Paul Menzel <paulepanter@mailbox.org> Gerrit-Reviewer: Raul Rangel <rrangel@chromium.org> Gerrit-Reviewer: Sean Rhodes <sean@starlabs.systems> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Daocheng Bu <daocheng.bu@intel.com> Gerrit-CC: David Hendricks <david.hendricks@gmail.com> Gerrit-CC: Furquan Shaikh <furquan.m.shaikh@gmail.com> Gerrit-CC: Jonathan Zhang <jon.zhixiong.zhang@gmail.com> Gerrit-CC: Marc Jones <marc@marcjonesconsulting.com> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Nill Ge <geshijian@bytedance.com> Gerrit-CC: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Raul Rangel <rrangel@chromium.org> Gerrit-Attention: Nico Huber <nico.h@gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Attention: Subrata Banik <subratabanik@google.com> Gerrit-Attention: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Attention: Angel Pons <th3fanbus@gmail.com> Gerrit-Attention: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Attention: Felix Held <felix-coreboot@felixheld.de> Gerrit-Comment-Date: Fri, 10 Mar 2023 07:20:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Attention is currently required from: Raul Rangel, Nico Huber, Michał Żygowski, Reka Norman, Michał Kopeć, Angel Pons, Arthur Heymans, Felix Held. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37196 ) Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... Patch Set 23: (1 comment) Patchset: PS23:
will this config also helps Xeon platforms?
It should help all Intel platform that supports CLFLUSH IMO. -- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 23 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de> Gerrit-Reviewer: Kapil Porwal <kapilporwal@google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan@9elements.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless@gmail.com> Gerrit-Reviewer: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Paul Menzel <paulepanter@mailbox.org> Gerrit-Reviewer: Raul Rangel <rrangel@chromium.org> Gerrit-Reviewer: Sean Rhodes <sean@starlabs.systems> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Daocheng Bu <daocheng.bu@intel.com> Gerrit-CC: David Hendricks <david.hendricks@gmail.com> Gerrit-CC: Furquan Shaikh <furquan.m.shaikh@gmail.com> Gerrit-CC: Jonathan Zhang <jon.zhixiong.zhang@gmail.com> Gerrit-CC: Marc Jones <marc@marcjonesconsulting.com> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Nill Ge <geshijian@bytedance.com> Gerrit-CC: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Raul Rangel <rrangel@chromium.org> Gerrit-Attention: Nico Huber <nico.h@gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Attention: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Attention: Angel Pons <th3fanbus@gmail.com> Gerrit-Attention: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Attention: Felix Held <felix-coreboot@felixheld.de> Gerrit-Comment-Date: Fri, 10 Mar 2023 07:23:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Lean Sheng Tan <sheng.tan@9elements.com> Gerrit-MessageType: comment
Attention is currently required from: Raul Rangel, Nico Huber, Michał Żygowski, Subrata Banik, Reka Norman, Michał Kopeć, Angel Pons, Felix Held. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37196 ) Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... Patch Set 23: (1 comment) Patchset: PS23:
will this config also helps Xeon platforms?
It should help all Intel platform that supports CLFLUSH IMO.
Well on SPR xeon-sp against recommendations TempRamExit was overloaded with functionality to write things to DRAM to speed things up and therefore made mandatory in the bootflow. CLFLUSHING would have been a way better solution, as invalidating cache in a different environment (FSP) is just though to get right even from a security perspective... -- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 23 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de> Gerrit-Reviewer: Kapil Porwal <kapilporwal@google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan@9elements.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless@gmail.com> Gerrit-Reviewer: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Paul Menzel <paulepanter@mailbox.org> Gerrit-Reviewer: Raul Rangel <rrangel@chromium.org> Gerrit-Reviewer: Sean Rhodes <sean@starlabs.systems> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Daocheng Bu <daocheng.bu@intel.com> Gerrit-CC: David Hendricks <david.hendricks@gmail.com> Gerrit-CC: Furquan Shaikh <furquan.m.shaikh@gmail.com> Gerrit-CC: Jonathan Zhang <jon.zhixiong.zhang@gmail.com> Gerrit-CC: Marc Jones <marc@marcjonesconsulting.com> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Nill Ge <geshijian@bytedance.com> Gerrit-CC: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Raul Rangel <rrangel@chromium.org> Gerrit-Attention: Nico Huber <nico.h@gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Attention: Subrata Banik <subratabanik@google.com> Gerrit-Attention: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Attention: Angel Pons <th3fanbus@gmail.com> Gerrit-Attention: Felix Held <felix-coreboot@felixheld.de> Gerrit-Comment-Date: Fri, 10 Mar 2023 08:01:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik@google.com> Comment-In-Reply-To: Lean Sheng Tan <sheng.tan@9elements.com> Gerrit-MessageType: comment
Attention is currently required from: Raul Rangel, Nico Huber, Michał Żygowski, Reka Norman, Michał Kopeć, Angel Pons, Arthur Heymans, Felix Held. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37196 ) Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... Patch Set 23: (1 comment) Patchset: PS23:
will this config also helps Xeon platforms?
It should help all Intel platform that supports CLFLUSH IMO.
Well on SPR xeon-sp against recommendations TempRamExit was overloaded with functionality to write things to DRAM to speed things up and therefore made mandatory in the bootflow. CLFLUSHING would have been a way better solution, as invalidating cache in a different environment (FSP) is just though to get right even from a security perspective...
ah, i missed that SPR is still using FSP-T -- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 23 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de> Gerrit-Reviewer: Kapil Porwal <kapilporwal@google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan@9elements.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless@gmail.com> Gerrit-Reviewer: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Paul Menzel <paulepanter@mailbox.org> Gerrit-Reviewer: Raul Rangel <rrangel@chromium.org> Gerrit-Reviewer: Sean Rhodes <sean@starlabs.systems> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Daocheng Bu <daocheng.bu@intel.com> Gerrit-CC: David Hendricks <david.hendricks@gmail.com> Gerrit-CC: Furquan Shaikh <furquan.m.shaikh@gmail.com> Gerrit-CC: Jonathan Zhang <jon.zhixiong.zhang@gmail.com> Gerrit-CC: Marc Jones <marc@marcjonesconsulting.com> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Nill Ge <geshijian@bytedance.com> Gerrit-CC: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Raul Rangel <rrangel@chromium.org> Gerrit-Attention: Nico Huber <nico.h@gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Attention: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Attention: Angel Pons <th3fanbus@gmail.com> Gerrit-Attention: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Attention: Felix Held <felix-coreboot@felixheld.de> Gerrit-Comment-Date: Fri, 10 Mar 2023 08:06:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik@google.com> Comment-In-Reply-To: Arthur Heymans <arthur@aheymans.xyz> Comment-In-Reply-To: Lean Sheng Tan <sheng.tan@9elements.com> Gerrit-MessageType: comment
Attention is currently required from: Raul Rangel, Nico Huber, Michał Żygowski, Subrata Banik, Reka Norman, Michał Kopeć, Angel Pons, Felix Held. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37196 ) Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... Patch Set 23: (1 comment) Patchset: PS23:
will this config also helps Xeon platforms?
It should help all Intel platform that supports CLFLUSH IMO.
Well on SPR xeon-sp against recommendations TempRamExit was overloaded with functionality to write things to DRAM to speed things up and therefore made mandatory in the bootflow. CLFLUSHING would have been a way better solution, as invalidating cache in a different environment (FSP) is just though to get right even from a security perspective...
ah, i missed that SPR is still using FSP-T
CAR setup is indeed quite complicated on Xeon-SP and requires FSP-T for now (coherency needs to be set up across CPUs). However CAR teardown is the same as (e)NEM. On CPX and SKX coreboot CAR teardown is used over FSP-M TempRamExit. On SPR-SP that's not possible anymore due to the modified bootflow. -- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 23 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de> Gerrit-Reviewer: Kapil Porwal <kapilporwal@google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan@9elements.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless@gmail.com> Gerrit-Reviewer: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Paul Menzel <paulepanter@mailbox.org> Gerrit-Reviewer: Raul Rangel <rrangel@chromium.org> Gerrit-Reviewer: Sean Rhodes <sean@starlabs.systems> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Daocheng Bu <daocheng.bu@intel.com> Gerrit-CC: David Hendricks <david.hendricks@gmail.com> Gerrit-CC: Furquan Shaikh <furquan.m.shaikh@gmail.com> Gerrit-CC: Jonathan Zhang <jon.zhixiong.zhang@gmail.com> Gerrit-CC: Marc Jones <marc@marcjonesconsulting.com> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Nill Ge <geshijian@bytedance.com> Gerrit-CC: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Raul Rangel <rrangel@chromium.org> Gerrit-Attention: Nico Huber <nico.h@gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Attention: Subrata Banik <subratabanik@google.com> Gerrit-Attention: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Attention: Angel Pons <th3fanbus@gmail.com> Gerrit-Attention: Felix Held <felix-coreboot@felixheld.de> Gerrit-Comment-Date: Fri, 10 Mar 2023 08:13:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik@google.com> Comment-In-Reply-To: Arthur Heymans <arthur@aheymans.xyz> Comment-In-Reply-To: Lean Sheng Tan <sheng.tan@9elements.com> Gerrit-MessageType: comment
Attention is currently required from: Raul Rangel, Nico Huber, Michał Żygowski, Subrata Banik, Reka Norman, Michał Kopeć, Angel Pons, Arthur Heymans, Felix Held. Lean Sheng Tan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37196 ) Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... Patch Set 23: (1 comment) Patchset: PS23:
will this config also helps Xeon platforms? […] Ziang or Amos any comment on this?
-- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 23 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de> Gerrit-Reviewer: Kapil Porwal <kapilporwal@google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan@9elements.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless@gmail.com> Gerrit-Reviewer: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Paul Menzel <paulepanter@mailbox.org> Gerrit-Reviewer: Raul Rangel <rrangel@chromium.org> Gerrit-Reviewer: Sean Rhodes <sean@starlabs.systems> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Daocheng Bu <daocheng.bu@intel.com> Gerrit-CC: David Hendricks <david.hendricks@gmail.com> Gerrit-CC: Furquan Shaikh <furquan.m.shaikh@gmail.com> Gerrit-CC: Jonathan Zhang <jon.zhixiong.zhang@gmail.com> Gerrit-CC: Marc Jones <marc@marcjonesconsulting.com> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Nill Ge <geshijian@bytedance.com> Gerrit-CC: Reka Norman <rekanorman@chromium.org> Gerrit-CC: Ziang Wang <ziang.wang@intel.com> Gerrit-Attention: Raul Rangel <rrangel@chromium.org> Gerrit-Attention: Nico Huber <nico.h@gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Attention: Subrata Banik <subratabanik@google.com> Gerrit-Attention: Reka Norman <rekanorman@chromium.org> Gerrit-Attention: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Attention: Angel Pons <th3fanbus@gmail.com> Gerrit-Attention: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Attention: Felix Held <felix-coreboot@felixheld.de> Gerrit-Comment-Date: Fri, 10 Mar 2023 11:21:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik@google.com> Comment-In-Reply-To: Arthur Heymans <arthur@aheymans.xyz> Comment-In-Reply-To: Lean Sheng Tan <sheng.tan@9elements.com> Gerrit-MessageType: comment
Lean Sheng Tan has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37196 ) ( 23 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ...................................................................... cpu/x86/cache: CLFLUSH programs to memory before running When cbmem is initialized in romstage and postcar placed in the stage cache + cbmem where it is run, the assumption is made that these are all in UC memory such that calling INVD in postcar is OK. For performance reasons (e.g. postcar decompression) it is desirable to cache cbmem and the stage cache during romstage. Another reason is that AGESA sets up MTRR during romstage to cache all dram, which is currently worked around by using additional MTRR's to make that UC. TESTED on asus/p5ql-em, up/squared on both regular and S3 resume bootpath. Sometimes there are minimal performance improvements when cbmem is cached (few ms). Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Signed-off-by: Arthur Heymans <arthur@aheymans.xyz> Reviewed-on: https://review.coreboot.org/c/coreboot/+/37196 Reviewed-by: Lean Sheng Tan <sheng.tan@9elements.com> Reviewed-by: Kapil Porwal <kapilporwal@google.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org> --- M src/arch/x86/postcar_loader.c M src/cpu/x86/Kconfig M src/cpu/x86/Makefile.inc A src/cpu/x86/cache/Makefile.inc A src/cpu/x86/cache/cache.c M src/include/cpu/x86/cache.h 6 files changed, 121 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Lean Sheng Tan: Looks good to me, approved Kapil Porwal: Looks good to me, approved diff --git a/src/arch/x86/postcar_loader.c b/src/arch/x86/postcar_loader.c index a8a77e0..1728680 100644 --- a/src/arch/x86/postcar_loader.c +++ b/src/arch/x86/postcar_loader.c @@ -9,6 +9,7 @@ #include <program_loading.h> #include <reset.h> #include <rmodule.h> +#include <romstage_handoff.h> #include <security/vboot/vboot_common.h> #include <stage_cache.h> #include <timestamp.h> @@ -137,6 +138,25 @@ board_reset(); } +/* + * POSTCAR will call invd so don't make assumptions on cbmem + * and external stage cache being UC. + */ +static void postcar_flush_cache(void) +{ + uintptr_t cbmem_base; + size_t cbmem_size; + uintptr_t stage_cache_base; + size_t stage_cache_size; + + cbmem_get_region((void **)&cbmem_base, &cbmem_size); + prog_segment_loaded(cbmem_base, cbmem_size, SEG_FINAL); + if (CONFIG(TSEG_STAGE_CACHE) && !romstage_handoff_is_resume()) { + stage_cache_external_region((void **)&stage_cache_base, &stage_cache_size); + prog_segment_loaded(stage_cache_base, stage_cache_size, SEG_FINAL); + } +} + static void run_postcar_phase(struct postcar_frame *pcf) { struct prog prog = @@ -159,6 +179,8 @@ console_time_report(); + postcar_flush_cache(); + prog_set_arg(&prog, cbmem_top()); prog_run(&prog); diff --git a/src/cpu/x86/Kconfig b/src/cpu/x86/Kconfig index 50766cd..c85eace 100644 --- a/src/cpu/x86/Kconfig +++ b/src/cpu/x86/Kconfig @@ -126,6 +126,11 @@ non-eviction mode and therefore need to be careful to avoid eviction. +config X86_CLFLUSH_CAR + bool + help + Select this on platforms that allow CLFLUSH while operating in CAR. + config HAVE_SMI_HANDLER bool default n diff --git a/src/cpu/x86/Makefile.inc b/src/cpu/x86/Makefile.inc index dffc2b1..3ef3a90 100644 --- a/src/cpu/x86/Makefile.inc +++ b/src/cpu/x86/Makefile.inc @@ -5,6 +5,7 @@ subdirs-$(CONFIG_UDELAY_TSC) += tsc # Use ARCH_BOOTBLOCK_X86_64 as a proxy for knowing if 64bit is going to be used subdirs-$(CONFIG_ARCH_BOOTBLOCK_X86_64) += 64bit +subdirs-y += cache subdirs-$(CONFIG_PARALLEL_MP) += name ramstage-$(CONFIG_PARALLEL_MP) += mp_init.c diff --git a/src/cpu/x86/cache/Makefile.inc b/src/cpu/x86/cache/Makefile.inc new file mode 100644 index 0000000..759488e --- /dev/null +++ b/src/cpu/x86/cache/Makefile.inc @@ -0,0 +1,2 @@ +romstage-y += cache.c +ramstage-y += cache.c diff --git a/src/cpu/x86/cache/cache.c b/src/cpu/x86/cache/cache.c new file mode 100644 index 0000000..d02d6d4 --- /dev/null +++ b/src/cpu/x86/cache/cache.c @@ -0,0 +1,58 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <arch/cpu.h> +#include <cbmem.h> +#include <console/console.h> +#include <cpu/x86/cache.h> +#include <program_loading.h> +#include <types.h> + +bool clflush_supported(void) +{ + return (cpuid_edx(1) >> CPUID_FEATURE_CLFLUSH_BIT) & 1; +} + +static void clflush_region(const uintptr_t start, const size_t size) +{ + uintptr_t addr; + const size_t cl_size = ((cpuid_ebx(1) >> 8) & 0xff) * 8; + + if (!clflush_supported()) { + printk(BIOS_DEBUG, "Not flushing cache to RAM, CLFLUSH not supported\n"); + return; + } + + printk(BIOS_SPEW, "CLFLUSH [0x%lx, 0x%lx]\n", start, start + size); + + for (addr = ALIGN_DOWN(start, cl_size); addr < start + size; addr += cl_size) + clflush((void *)addr); +} + +/* + * For each segment of a program loaded this function is called + * to invalidate caches for the addresses of the loaded segment + */ +void arch_segment_loaded(uintptr_t start, size_t size, int flags) +{ + /* INVD is only called in postcar stage so we only need + to make sure that our code hits dram during romstage. */ + if (!ENV_CACHE_AS_RAM) + return; + if (!ENV_ROMSTAGE) + return; + if (!CONFIG(POSTCAR_STAGE)) + return; + if (!CONFIG(X86_CLFLUSH_CAR)) + return; + if (flags != SEG_FINAL) + return; + + /* + * The assumption is made here that DRAM is only ready after cbmem + * is initialized, to avoid flushing when loading earlier things (e.g. FSP, ...) + */ + if (!cbmem_online()) + return; + + clflush_region(start, size); +} diff --git a/src/include/cpu/x86/cache.h b/src/include/cpu/x86/cache.h index 27b727b..63703a7 100644 --- a/src/include/cpu/x86/cache.h +++ b/src/include/cpu/x86/cache.h @@ -12,6 +12,8 @@ #if !defined(__ASSEMBLER__) +#include <stdbool.h> + static inline void wbinvd(void) { asm volatile ("wbinvd" ::: "memory"); @@ -27,6 +29,8 @@ asm volatile ("clflush (%0)"::"r" (addr)); } +bool clflush_supported(void); + /* The following functions require the __always_inline due to AMD * function STOP_CAR_AND_CPU that disables cache as * RAM, the cache as RAM stack can no longer be used. Called -- To view, visit https://review.coreboot.org/c/coreboot/+/37196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7ff2a57aee620908b71829457ea0f5a0c410ec5b Gerrit-Change-Number: 37196 Gerrit-PatchSet: 25 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de> Gerrit-Reviewer: Kapil Porwal <kapilporwal@google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan@9elements.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless@gmail.com> Gerrit-Reviewer: Michał Kopeć <michal.kopec@3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Paul Menzel <paulepanter@mailbox.org> Gerrit-Reviewer: Raul Rangel <rrangel@chromium.org> Gerrit-Reviewer: Sean Rhodes <sean@starlabs.systems> Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Daocheng Bu <daocheng.bu@intel.com> Gerrit-CC: David Hendricks <david.hendricks@gmail.com> Gerrit-CC: Furquan Shaikh <furquan.m.shaikh@gmail.com> Gerrit-CC: Jonathan Zhang <jon.zhixiong.zhang@gmail.com> Gerrit-CC: Marc Jones <marc@marcjonesconsulting.com> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Nill Ge <geshijian@bytedance.com> Gerrit-CC: Reka Norman <rekanorman@chromium.org> Gerrit-CC: Ziang Wang <ziang.wang@intel.com> Gerrit-MessageType: merged
participants (10)
-
Aaron Durbin (Code Review) -
Angel Pons (Code Review)
-
Arthur Heymans (Code Review)
-
Kapil Porwal (Code Review)
-
Lean Sheng Tan (Code Review)
-
Marc Jones (Code Review)
-
Nico Huber (Code Review)
-
Paul Menzel (Code Review)
-
Reka Norman (Code Review)
-
Subrata Banik (Code Review)