Attention is currently required from: Felix Held, Jérémy Compostella, Maximilian Brune, Naresh Solanki, Patrick Rudolph, Paul Menzel.
Angel Pons has posted comments on this change by Naresh Solanki. ( https://review.coreboot.org/c/coreboot/+/85640?usp=email )
Change subject: soc/amd/glinda/cpu: Implement soc_fill_cpu_cache_info helper ......................................................................
Patch Set 5:
(3 comments)
File src/soc/amd/glinda/cpu.c:
https://review.coreboot.org/c/coreboot/+/85640/comment/920a8788_f06336c0?usp... : PS5, Line 24: uint32_t leaf = DETERMINISTIC_CACHE_PARAMETERS_CPUID_AMD; : uint8_t level = 3; : : struct cpuid_result cache_info_res = cpuid_ext(leaf, level); : : info->type = CPUID_CACHE_TYPE(cache_info_res); : info->level = CPUID_CACHE_LEVEL(cache_info_res); : info->num_ways = CPUID_CACHE_WAYS_OF_ASSOC(cache_info_res) + 1; : info->num_sets = CPUID_CACHE_NO_OF_SETS(cache_info_res) + 1; : info->line_size = CPUID_CACHE_COHER_LINE(cache_info_res) + 1; : info->physical_partitions = CPUID_CACHE_PHYS_LINE(cache_info_res) + 1; : info->num_cores_shared = CPUID_CACHE_SHARING_CACHE(cache_info_res) + 1; : info->fully_associative = CPUID_CACHE_FULL_ASSOC(cache_info_res); : info->size = get_cache_size(info); This is pretty much the same as `fill_cpu_cache_info` (excluding the call to `soc_fill_cpu_cache_info`). I'm wondering if there's any way to deduplicate it - perhaps by introducing an "internal" function in `cpu_common.c`.
Also, could any of these values be different between cores of a CPU? If not (this information is the same for all CPUs), it would be easier to count the number of unique IDs and multiply the cache size by that number. Which would simplify things quite a bit - common code can fill in the parameters, and `soc_fill_cpu_cache_info` would be adapted to run after that to modify/extend the cache info accordingly.
https://review.coreboot.org/c/coreboot/+/85640/comment/1693e322_8a05818a?usp... : PS5, Line 52: if (!info) : return false; This is checked by `fill_cpu_cache_info`
https://review.coreboot.org/c/coreboot/+/85640/comment/a4f8ded1_b46f44e5?usp... : PS5, Line 70: :: nit: drop one colon
```suggestion printk(BIOS_SPEW, " Cache ID: %d\n", info_list[i].unique_id); ```