Attention is currently required from: Ashish Kumar Mishra, Jérémy Compostella, Paul Menzel, Sowmya Aralguppe, Wonkyu Kim.
Subrata Banik has posted comments on this change by Jérémy Compostella. ( https://review.coreboot.org/c/coreboot/+/83946?usp=email )
Change subject: soc/intel/common/block/cpu: Add Kconfig for effective way size for NEM+ ......................................................................
Patch Set 13:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/83946/comment/6f9e9449_bea157b4?usp... : PS13, Line 23: The issue addressed by this commit can be observed with the following : experiment: using a 18 MB LLC SKU, set `DCACHE_RAM_SIZE` to : 0x400000 (4 MB).
This isn't a real issue, it's just a hypothesis that CONFIG_DCACHE_RAM_SIZE is larger than the way size.
The coreboot enhanced NEM code allows `CONFIG_DCACHE_RAM_SIZE` requiring multiple ways but it violate the Alder Lake, Meteor Lake and Panther Lake specification if such a configuration is used ⇒ the code is wrong and this is the issue these CL are addressing. These CL address a coreboot Intel eNEM code issue not a Google board issue.
I have couple of questions
1. As per eNEM white paper shared by Intel with Google, the 0xc91 and 0xc92 are two MSR responsible to provide the LLC ways for code and data. - now we calculate the way size = Total LLC size / total number of LLC ways - assume way size if > CAR size then we always assume the way count is always 1. - assume CAR size is > way size then we find way count = (CAR size / way size). In case the way count is a floating point number (meaning, CAR size is not divisible by way size), we try to round the way count (mostly increment the quotient aka number of ways by 1 to get the ceiling value). - The logic that you have added now inside a dedicated Kconfig can be fit here w/o any CPP if you wish to calculate the `eff_way_size` over just rounding the way count.
2. But my question is where is the doc to refer to understand the formula to calculate the `eff_way_size` (Therefore, we instead compute the effective way size as the biggest of power of two of the way size which works across all three platforms.) I don't see any document that explains the logic mentioned by you in the commit section? that we should only keep the most significant bit position and mask rest to determine the effective way size.
If you believe that `CONFIG_DCACHE_RAM_SIZE` should never be larger than the effective way size, then why does the bootblock eNEM support multiple ways ? And why is violating the specification by not considering the effective way size ?
If you refuse the code to be fixed arguing that no configuration is taking advantage of it, please implement a mechanism in bootblock limiting the configuration to one **effective** way and remove the support for multiple ways. My only objective here is to fix code that does not comply with the specification and I was able to verify the code does not work by tweaking the configuration. *I tweaked the configuration, not the code*.
My experiments demonstrated that 4MB works like a charm only with these two CLs.
I will get back to you by next week early.