diff options
author | Jeremy Compostella <jeremy.compostella@intel.com> | 2023-05-25 16:13:44 -0700 |
---|---|---|
committer | Subrata Banik <subratabanik@google.com> | 2023-06-01 07:52:03 +0000 |
commit | dffb1c8933c09c041b10e1f6a666449e7a7531da (patch) | |
tree | 1c95d92aac21b41a0ff52c6dbc2ca437d8f16af5 /src/include/cpu | |
parent | 90c3df7a211cb30445f1d6fa8f480a99c8f1e56e (diff) |
cpu/x86/cache: Call wbinvd only once CR0.CD is set
This patch removes the wbinvd call preceding CR0.CD setting in
disable_cache() to improve the boot time performances. According to
some experimental measurements, the wbinvd execution takes between 1.6
up and 6 milliseconds to complete so it is preferable to call it only
when necessary.
According to Intel Software Developer Manual Vol 3.A - 12.5.3
Preventing Caching section there is no need to flush and invalidate
the cache before settings CR0.CD. The documented sequence consists in
setting CR0.CD and then call wbinvd.
We also could not find any extra requirements in the AMD64
Architecture Programmer’s Manual - Volume 2 - Memory System chapter.
This extra wbinvd in coreboot disable_cache() function does not seem
documented and looking into the history of the project got us all the
way back to original commit 8ca8d7665d67 ("- Initial checkin of the
freebios2 tree") from April 2003.
Even the original disable_cache() implementation (see below) is a bit
curious as the comment list two actions:
1. Disable cache cover by line 74, 75 and 77
2. Write back the cache and flush TLB - Line 78
But it does not provide any explanation for the wbinvd call line 76.
68 static inline void disable_cache(void)
69 {
70 unsigned int tmp;
71 /* Disable cache */
72 /* Write back the cache and flush TLB */
73 asm volatile (
74 "movl %%cr0, %0\n\t"
75 "orl $0x40000000, %0\n\t"
76 "wbinvd\n\t"
77 "movl %0, %%cr0\n\t"
78 "wbinvd\n\t"
79 :"=r" (tmp)
80 ::"memory");
81 }
BUG=b/260455826
TEST=Successful boot on Skolas and Rex board
Change-Id: I08c6486dc93c4d70cadc22a760d1b7e536e85bfa
Signed-off-by: Jeremy Compostella <jeremy.compostella@intel.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/75474
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Subrata Banik <subratabanik@google.com>
Reviewed-by: Himanshu Sahdev <himanshu.sahdev@intel.com>
Diffstat (limited to 'src/include/cpu')
-rw-r--r-- | src/include/cpu/x86/cache.h | 1 |
1 files changed, 0 insertions, 1 deletions
diff --git a/src/include/cpu/x86/cache.h b/src/include/cpu/x86/cache.h index d35c4e7fd0..4143d972f5 100644 --- a/src/include/cpu/x86/cache.h +++ b/src/include/cpu/x86/cache.h @@ -57,7 +57,6 @@ static __always_inline void disable_cache(void) CRx_TYPE cr0; cr0 = read_cr0(); cr0 |= CR0_CD; - wbinvd(); write_cr0(cr0); wbinvd(); } |