[coreboot-gerrit] Change in coreboot[master]: cpu/intel/common: Create get_cache_info() function

[Subrata Banik (Code Review)]

Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak, Rizwan Qureshi, Angel Pons, Patrick Rudolph.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55965 )

Change subject: cpu/intel/common: Create get_cache_info() function
......................................................................

Patch Set 1:

(2 comments)

File src/cpu/intel/common/common_init.c:

https://review.coreboot.org/c/coreboot/+/55965/comment/04578145_5ebc4ee3 
PS1, Line 331: 0x03
> I assume 0x03 is supposed to be LLC here? Create a macro for that?

if we are taking cache_level as argument then do we still need macro ?

https://review.coreboot.org/c/coreboot/+/55965/comment/c0b57aaa_da141917 
PS1, Line 332: 
             : 	const size_t assoc = CPUID_CACHE_WAYS_OF_ASSOC(res) + 1;
             : 	const size_t partitions = CPUID_CACHE_PHYS_LINE(res) + 1;
             : 	const size_t cache_line_size = CPUID_CACHE_COHER_LINE(res) + 1;
             : 	const size_t number_of_sets = CPUID_CACHE_NO_OF_SETS(res) + 1;
             : 	const size_t cache_size = assoc * partitions * cache_line_size * number_of_sets;
             : 	printk(BIOS_INFO, "Cache Information : ");
             : 	printk(BIOS_INFO, "assoc=%zd par=%zd line_size=%zd sets=%zd\n",
             : 		assoc, partitions, cache_line_size, number_of_sets);
             : 	printk(BIOS_INFO, "@@ cache_size 0x%lx bytes\n", cache_size);
> also remarkably similar to https://review.coreboot.org/plugins/gitiles/coreboot/+/refs/heads/master/src... ... maybe this function should take a parameter for the ecx value passed to CPUID?

Yes, this is doable but then only problem is smbios_write_type7_cache_parameters() expects to get those individual fields like level, cache_type, assoc etc. etc. so we have two possible ways:

1. Do we want to execute cpuid instruction several time and return those bit-fields as part of dedicated function like below?

static struct cpuid_result get_cpu_cache_parameters(int cache_type)
{
   /* 1. Add a check to detect CPU if it belongs to IA or AMD? */
   return cpuid_ext(DETERMINISTIC_CACHE_PARAMETERS_CPUID, cache_type);
}

size_t get_cpu_cache_type(int cache_type)
{
   struct cpuid_result res = get_cpu_cache_parameters();
   return CPUID_CACHE_TYPE(res);
}
...

2. Return all cache parameters as part of a single function call. here cache_type is only input argument and rests are output as below?

size_t get_cache_info(int cache_type, size_t *assoc, size_t *partitions, ...)
{
    struct cpuid_result res = cpuid_ext(DETERMINISTIC_CACHE_PARAMETERS_CPUID, cache_type);
    *assoc = CPUID_CACHE_WAYS_OF_ASSOC(res) + 1;
    *partitions = CPUID_CACHE_PHYS_LINE(res) + 1;
    ...
}

Then caller of get_cache_info() function like smbios_write_type7_cache_parameters() can use these output variables (assoc, partitions) to perform the regular operations.
> `get_cache_info(unsigned int cache_level)` 
> or similar?
> then the SMBIOS code could reuse this function.

Yes, this is possible, but need your help to define one path.