Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak, Rizwan Qureshi, Angel Pons, Patrick Rudolph.
2 comments:
File src/cpu/intel/common/common_init.c:
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 ?
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/arch/x86/smbios.c#839 ... 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.
To view, visit change 55965. To unsubscribe, or for help writing mail filters, visit settings.