Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46068 )
Change subject: arch/x86/smbios: Populate SMBIOS type 7 with cache information ......................................................................
Patch Set 10: Code-Review+1
(5 comments)
https://review.coreboot.org/c/coreboot/+/46068/8//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46068/8//COMMIT_MSG@10 PS8, Line 10: Combine L1 Data cache size and : L1 Instruction cache size
Revert it for seperating L1 D-CACHE and L1 I-CACHE.
Done
https://review.coreboot.org/c/coreboot/+/46068/8//COMMIT_MSG@11 PS8, Line 11: L1 Instruction cache size, and multiply the cache size of L1 and L2 : by the number of cores
Please refer to Doc#553625.
Thanks. As I feared, Intel keeps it under wraps. This may take some time.
https://review.coreboot.org/c/coreboot/+/46068/8/src/arch/x86/smbios.c File src/arch/x86/smbios.c:
https://review.coreboot.org/c/coreboot/+/46068/8/src/arch/x86/smbios.c@888 PS8, Line 888: continue;
This will stop us calling smbios_write_type7() for level 1 d-cache. So that […]
Done
https://review.coreboot.org/c/coreboot/+/46068/9/src/arch/x86/smbios.c File src/arch/x86/smbios.c:
https://review.coreboot.org/c/coreboot/+/46068/9/src/arch/x86/smbios.c@513 PS9, Line 513: 1
Done
I see you changed the variable name. The description of leaf 0xb is very odd. I was mislead by the level (sub-leaf 1 is level core but returns numbers for the whole package it seems).
So the name was better before. Sorry for that.
https://review.coreboot.org/c/coreboot/+/46068/10/src/arch/x86/smbios.c File src/arch/x86/smbios.c:
https://review.coreboot.org/c/coreboot/+/46068/10/src/arch/x86/smbios.c@517 PS10, Line 517: if (max_logical_cores_sharing_cache == max_logical_cores_per_package) { This looks very odd, but I think I understand why it's necessary. Both CPUID 0x1 and 0x4 return the number of addressable logical cores, not the actual number of them. I guess, but don't know for sure, that one could use CPUID 0xb/0x1f eax[4..0] to match this and have a single implementation that works for all cache levels.
AIUI, `max_logical_cores_sharing_cache` should match one `1 << (cpuid_ext(0x1f, level).eax & 0x1f)`. Then one would know the topology level and could use its numbers.
For instance, for my CPU, L1D, L1I and L2 all report 2 logical cores sharing the cache, and `cpuid_ext(0xb, 0).eax == 1`. L3 reports 16 logical cores sharing it, and `cpuid_ext(0xb, 1).eax == 4`.