Attention is currently required from: Subrata Banik.
Hello Subrata Banik,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/83948?usp=email
to look at the new patch set (#3).
Change subject: soc/intel/common/block/cpu: Fix number of way computation regression
......................................................................
soc/intel/common/block/cpu: Fix number of way computation regression
Commit 16ab9bdcd578612bb3822373547f939eb90afd82 ("soc/intel/common:
Calculate and configure SF Mask 2") breaks the computation of the
number of way and as result, all the derived masks. It results in MSR
such as `IA32_L3_MASK_1' to be improperly programmed yielding
unpredictable NEM issues such as hangs.
Indeed, this commit has introduced a backup of 0x1 into %edx before
comparing the requested cache-as-RAM size against the way size. When
the requested cache-as-RAM is larger, it reaches the second part of
the algorithm which computes the necessary number of way to fit the
requested cache-as-RAM.
This algorithm uses the `div' instruction. Per specification, the div
instruction divides the 64 bits combination of %edx and %eax register.
Since 0x1 got backed up in %edx and assuming a
`CONFIG_DCACHE_RAM_SIZE' of 0x200000, we end up dividing 0x100200000
by the way size instead of 0x200000 which result in a necessary number
of ways of 4098 for a way size of 0x100000.
This commit clears the %edx register before calling the `div'
instruction.
BUG=b:360332771
TEST=Verified on rex
Change-Id: I5cb66da0aa977eecb64a0021268a6827747c521d
Signed-off-by: Jeremy Compostella <jeremy.compostella(a)intel.com>
---
M src/soc/intel/common/block/cpu/car/cache_as_ram.S
1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/83948/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/83948?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I5cb66da0aa977eecb64a0021268a6827747c521d
Gerrit-Change-Number: 83948
Gerrit-PatchSet: 3
Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Attention is currently required from: Subrata Banik.
Hello Subrata Banik,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/83946?usp=email
to look at the new patch set (#3).
Change subject: soc/intel/common/block/cpu: Use the effective way size for NEM+
......................................................................
soc/intel/common/block/cpu: Use the effective way size for NEM+
The Last Level Cache (LLC) way size (or sets) is not necessarily a
power of two. However, on some platforms, the effective way size, the
way size which should be considered for No-Eviction Mode (NEM)
purposes is the biggest power of two of the way size.
Alder Lake External Design Specification #627270 "3.5.2 No-Eviction
Mode (NEM) Sizes" provides some understanding that the maximum NEM
size depends on the number of CBO which used to be accessible via MSR
0x396. Unfortunately, this MSR is not available and as a general
implementation the recommendation is to use the biggest power of two
of the way size instead.
The Kconfig `INTEL_CAR_ENEM_USE_EFFECTIVE_WAY_SIZE' has been
introduced to control this behavior.
BUG=b:360332771
TEST=Verified on rex
Change-Id: I5cb66da0aa977eecb64a0021268a6827747c521c
Signed-off-by: Jeremy Compostella <jeremy.compostella(a)intel.com>
---
M src/soc/intel/common/block/cpu/Kconfig
M src/soc/intel/common/block/cpu/car/cache_as_ram.S
2 files changed, 22 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/83946/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/83946?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I5cb66da0aa977eecb64a0021268a6827747c521c
Gerrit-Change-Number: 83946
Gerrit-PatchSet: 3
Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Jayvik Desai, Kapil Porwal, Nick Vaccaro, Pranava Y N, Rishika Raj, Subrata Banik, Tarun.
Hello Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Jayvik Desai, Kapil Porwal, Nick Vaccaro, Pranava Y N, Rishika Raj, Subrata Banik, Tarun,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/83947?usp=email
to look at the new patch set (#2).
Change subject: soc/intel: Enable NEM+ effective way size use for the right platform
......................................................................
soc/intel: Enable NEM+ effective way size use for the right platform
Alder Lake, Meteor Lake and Panther Lake uses the effective way size
when setting up the Enhanced No-Eviction
Mode (cf. `INTEL_CAR_ENEM_USE_EFFECTIVE_WAY_SIZE').
BUG=b:360332771
TEST=Verified on rex
Change-Id: I5cb66da0aa977eecb64a0021268a6827747c521b
Signed-off-by: Jeremy Compostella <jeremy.compostella(a)intel.com>
---
M src/soc/intel/alderlake/Kconfig
M src/soc/intel/meteorlake/Kconfig
M src/soc/intel/pantherlake/Kconfig
3 files changed, 3 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/47/83947/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/83947?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I5cb66da0aa977eecb64a0021268a6827747c521b
Gerrit-Change-Number: 83947
Gerrit-PatchSet: 2
Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.com>
Gerrit-Reviewer: Rishika Raj <rishikaraj(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tarun <tstuli(a)gmail.com>
Gerrit-Attention: Jayvik Desai <jayvik(a)google.com>
Gerrit-Attention: Rishika Raj <rishikaraj(a)google.com>
Gerrit-Attention: Eran Mitrani <mitrani(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: Tarun <tstuli(a)gmail.com>
Gerrit-Attention: Pranava Y N <pranavayn(a)google.com>
Attention is currently required from: Subrata Banik.
Hello Subrata Banik,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/83948?usp=email
to look at the new patch set (#2).
Change subject: soc/intel/common/block/cpu: Fix number of way computation regression
......................................................................
soc/intel/common/block/cpu: Fix number of way computation regression
Commit 16ab9bdcd578612bb3822373547f939eb90afd82 ("soc/intel/common:
Calculate and configure SF Mask 2") breaks the computation of the
number of way and as result, all the derived masks. It results in MSR
such as `IA32_L3_MASK_1' to be improperly programmed yielding
unpredictable NEM issues such as hangs.
Indeed, this commit has introduced a backup of 0x1 into %edx before
comparing the requested cache-as-RAM size against the way size. When
the requested cache-as-RAM is larger, it reaches the second part of
the algorithm which computes the necessary number of way to fit the
requested cache-as-RAM.
This algorithm uses the `div' instruction. Per specification, the div
instruction divides the 64 bits combination of %edx and %eax register.
Since 0x1 got backed up in %edx and assuming a
`CONFIG_DCACHE_RAM_SIZE' of 0x200000, we end up dividing 0x100200000
by the way size instead of 0x200000 which result in a necessary number
of ways of 4098 for a way size of 0x100000. The mask computation
using only the lower bit of %ecx (%cl), it adds some extra
"randomness".
This commit clears the %edx register before calling the `div'
instruction.
BUG=b:360332771
TEST=Verified on rex
Change-Id: I5cb66da0aa977eecb64a0021268a6827747c521d
Signed-off-by: Jeremy Compostella <jeremy.compostella(a)intel.com>
---
M src/soc/intel/common/block/cpu/car/cache_as_ram.S
1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/83948/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/83948?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I5cb66da0aa977eecb64a0021268a6827747c521d
Gerrit-Change-Number: 83948
Gerrit-PatchSet: 2
Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Jérémy Compostella has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/83946?usp=email )
Change subject: soc/intel/common/block/cpu: Use the effective way size for NEM+
......................................................................
soc/intel/common/block/cpu: Use the effective way size for NEM+
The Last Level Cache (LLC) way size (or sets) is not necessarily a
power of two. However, on some platforms, the effective way size, the
way size which should be considered for No-Eviction Mode (NEM)
purposes is the biggest power of two of the way size.
Alder Lake External Design Specification #627270 "3.5.2 No-Eviction
Mode (NEM) Sizes" provides some understanding that the maximum NEM
size depends on the number of CBO which used to be accessible via MSR
0x396. Unfortunately, this MSR is not available and as a general
implementation the recommendation is to use the biggest power of two
of the way size instead.
The Kconfig `INTEL_CAR_ENEM_USE_EFFECTIVE_WAY_SIZE' is introduce to
control this behavior.
BUG=b:360332771
TEST=Verified on rex
Change-Id: I5cb66da0aa977eecb64a0021268a6827747c521c
Signed-off-by: Jeremy Compostella <jeremy.compostella(a)intel.com>
---
M src/soc/intel/common/block/cpu/Kconfig
M src/soc/intel/common/block/cpu/car/cache_as_ram.S
2 files changed, 22 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/83946/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/83946?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I5cb66da0aa977eecb64a0021268a6827747c521c
Gerrit-Change-Number: 83946
Gerrit-PatchSet: 2
Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com>
Jérémy Compostella has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/83948?usp=email )
Change subject: soc/intel/common/block/cpu: Fix number of way computation regression
......................................................................
soc/intel/common/block/cpu: Fix number of way computation regression
Commit a549b09de4ffa30989d42ad2939630e9f854d6d9 ("soc/intel/common:
Calculate and configure SF Mask 2") breaks the computation of the
number of way and as result, all the derived masks. It results in MSR
such as `IA32_L3_MASK_1' to be improperly programmed yielding
unpredictable NEM issues such as hangs.
Indeed, this commit has introduced a backup of 0x1 into %edx before
comparing the requested cache-as-RAM size against the way size. When
the requested cache-as-RAM is larger, it reaches the second part of
the algorithm which computes the necessary number of way to fit the
requested cache-as-RAM.
This algorithm uses the `div' instruction. Per specification, the div
instruction divides the 64 bits combination of %edx and %eax register.
Since 0x1 got backed up in %edx and assuming a
`CONFIG_DCACHE_RAM_SIZE' of 0x200000, we end up dividing 0x100200000
by the way size instead of 0x200000 which result in a necessary number
of ways of 4098 for a way size of 0x100000. The mask computation
using only the lower bit of %ecx (%cl), it adds some extra
"randomness".
This commit clears the %edx register before calling the `div'
instruction.
BUG=b:TBD
TEST=Verified on rex
Change-Id: I5cb66da0aa977eecb64a0021268a6827747c521d
Signed-off-by: Jeremy Compostella <jeremy.compostella(a)intel.com>
---
M src/soc/intel/common/block/cpu/car/cache_as_ram.S
1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/83948/1
diff --git a/src/soc/intel/common/block/cpu/car/cache_as_ram.S b/src/soc/intel/common/block/cpu/car/cache_as_ram.S
index f68ac4c..05e7a35 100644
--- a/src/soc/intel/common/block/cpu/car/cache_as_ram.S
+++ b/src/soc/intel/common/block/cpu/car/cache_as_ram.S
@@ -523,6 +523,7 @@
* ways to be configured for non-eviction
*/
mov $CONFIG_DCACHE_RAM_SIZE, %eax
+ xor %edx, %edx
div %ecx
mov %eax, %edx /* back up data_ways in edx */
mov %eax, %ecx
--
To view, visit https://review.coreboot.org/c/coreboot/+/83948?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I5cb66da0aa977eecb64a0021268a6827747c521d
Gerrit-Change-Number: 83948
Gerrit-PatchSet: 1
Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com>
Jérémy Compostella has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/83946?usp=email )
Change subject: soc/intel/common/block/cpu: Use the effective way size for NEM+
......................................................................
soc/intel/common/block/cpu: Use the effective way size for NEM+
The Last Level Cache (LLC) way size (or sets) is not necessarily a
power of two. However, on some platforms, the effective way size, the
way size which should be considered for No-Eviction Mode (NEM)
purposes is the biggest power of two of the way size.
Alder Lake External Design Specification #627270 "3.5.2 No-Eviction
Mode (NEM) Sizes" provides some understanding that the maximum NEM
size depends on the number of CBO which used to be accessible via MSR
0x396. Unfortunately, this MSR is not available and as a general
implementation the recommendation is to use the biggest power of two
of the way size instead.
The Kconfig `INTEL_CAR_ENEM_USE_EFFECTIVE_WAY_SIZE' is introduce to
control this behavior.
BUG=b:TBD
TEST=Verified on rex
Change-Id: I5cb66da0aa977eecb64a0021268a6827747c521c
Signed-off-by: Jeremy Compostella <jeremy.compostella(a)intel.com>
---
M src/soc/intel/common/block/cpu/Kconfig
M src/soc/intel/common/block/cpu/car/cache_as_ram.S
2 files changed, 22 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/83946/1
diff --git a/src/soc/intel/common/block/cpu/Kconfig b/src/soc/intel/common/block/cpu/Kconfig
index 814de73..a015db2 100644
--- a/src/soc/intel/common/block/cpu/Kconfig
+++ b/src/soc/intel/common/block/cpu/Kconfig
@@ -81,6 +81,15 @@
ENHANCED NEM guarantees that modified data is always
kept in cache while clean data is replaced.
+config INTEL_CAR_ENEM_USE_EFFECTIVE_WAY_SIZE
+ bool
+ help
+ The LLC way size (or sets) is not necessarily a power of
+ two. However, on some platforms, the effective way size, the
+ way size which should be considered for NEM purposes is the
+ biggest power of two of the way size instead of the entire
+ way size.
+
config CAR_HAS_SF_MASKS
bool
depends on INTEL_CAR_NEM_ENHANCED
diff --git a/src/soc/intel/common/block/cpu/car/cache_as_ram.S b/src/soc/intel/common/block/cpu/car/cache_as_ram.S
index ba98f1b..f68ac4c 100644
--- a/src/soc/intel/common/block/cpu/car/cache_as_ram.S
+++ b/src/soc/intel/common/block/cpu/car/cache_as_ram.S
@@ -495,6 +495,19 @@
div %ebx /* way size */
mov %eax, %ecx
+#if CONFIG(INTEL_CAR_ENEM_USE_EFFECTIVE_WAY_SIZE)
+ /*
+ * Limit the way size to the effective way size defined
+ * as the biggest power of two of the way size.
+ */
+ lzcnt %ecx, %eax
+ movl $31, %ecx
+ subl %eax, %ecx
+ movl $0x01, %eax
+ shl %cl, %eax
+ movl %eax, %ecx
+#endif
+
/*
* Check if way size if bigger than the cache ram size.
* Then we need to allocate just one way for non-eviction
--
To view, visit https://review.coreboot.org/c/coreboot/+/83946?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I5cb66da0aa977eecb64a0021268a6827747c521c
Gerrit-Change-Number: 83946
Gerrit-PatchSet: 1
Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com>
Attention is currently required from: Maximilian Brune.
Elyes Haouas has posted comments on this change by Maximilian Brune. ( https://review.coreboot.org/c/coreboot/+/83617?usp=email )
The change is no longer submittable: All-Comments-Resolved is unsatisfied now.
Change subject: util/cbfstool/cbfs-payload-linux: Add error handling
......................................................................
Patch Set 1:
(1 comment)
File util/cbfstool/cbfs-payload-linux.c:
https://review.coreboot.org/c/coreboot/+/83617/comment/266e8b25_9f867297?us… :
PS1, Line 142: uint32_t type, uint64_t load_addr,
> `code indent should use tabs where possible`
Please fix.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83617?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ie36ad469c73cb3ff9360acc9bbe66c245e8b4a1e
Gerrit-Change-Number: 83617
Gerrit-PatchSet: 1
Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Comment-Date: Fri, 16 Aug 2024 19:30:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Elyes Haouas has posted comments on this change by Nico Huber. ( https://review.coreboot.org/c/coreboot/+/83945?usp=email )
Change subject: mb/qemu-aarch64: Fix include path for device_tree.h
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
Merging early and out of order as this is a trivial fix.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83945?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Id669eeaabbc1710bb7e408659f2d79f682427919
Gerrit-Change-Number: 83945
Gerrit-PatchSet: 2
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Alper Nebi Yasak <alpernebiyasak(a)gmail.com>
Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Fri, 16 Aug 2024 19:17:16 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No