Morgan Jang has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46068 )
Change subject: arch/x86/smbios: Update SMBIOS type 7 ......................................................................
arch/x86/smbios: Update SMBIOS type 7
Combine L1 Data cache size and L1 Instruction cache size, and multiply the cache size of L1 and L2 by the number of cores.
TEST=Execute "dmidecode -t 7" to check if the cache information is correct for Deltalake platform
Change-Id: Ieeb5d3346454ffb2291613dc2aa24b31d10c2e04 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com --- M src/arch/x86/smbios.c M src/include/smbios.h 2 files changed, 82 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/46068/1
diff --git a/src/arch/x86/smbios.c b/src/arch/x86/smbios.c index e33b70f..026849f 100644 --- a/src/arch/x86/smbios.c +++ b/src/arch/x86/smbios.c @@ -476,6 +476,52 @@ return (res.eax > 0) ? 0x0c : 0x6; }
+unsigned int __weak smbios_cache_error_correction_type(void) +{ + return SMBIOS_CACHE_ERROR_CORRECTION_UNKNOWN; /* Unknown */ +} +unsigned int __weak smbios_cache_sram_type(void) +{ + return SMBIOS_CACHE_SRAM_TYPE_UNKNOWN; /* Unknown */ +} + +unsigned int __weak smbios_cache_conf_operation_mode(u8 level) +{ + return 3; /* Unknown */ +} + +static size_t get_number_of_caches(u32 leaf, u8 level) +{ + size_t max_logical_cpus_sharing_cache = 0; + size_t number_of_logical_cpus = 0; + size_t max_logical_cpus_per_package = 0; + struct cpuid_result res; + + /* Provide sane defaults even for CPU without CPUID */ + res.eax = res.edx = 0; + res.ebx = 0x10000; + + if (cpu_have_cpuid()) + res = cpuid(1); + + max_logical_cpus_per_package = (res.ebx >> 16) & 0xff; + + if (cpu_have_cpuid() && cpuid_get_max_func() >= 0xb) { + res = cpuid_ext(0xb, 1); + number_of_logical_cpus = res.ebx; + } else { + number_of_logical_cpus = max_logical_cpus_per_package; + } + + res = cpuid_ext(leaf, level); + max_logical_cpus_sharing_cache = ((res.eax >> 14) & 0xfff) + 1; + + if (max_logical_cpus_sharing_cache != max_logical_cpus_per_package) + return number_of_logical_cpus / max_logical_cpus_sharing_cache; + + return 1; +} + static int smbios_write_type1(unsigned long *current, int handle) { struct smbios_type1 *t = (struct smbios_type1 *)*current; @@ -650,11 +696,11 @@ const enum smbios_cache_associativity associativity, const enum smbios_cache_type type, const size_t max_cache_size, - const size_t cache_size) + const size_t cache_size, + const u32 leaf) { struct smbios_type7 *t = (struct smbios_type7 *)*current; int len = sizeof(struct smbios_type7); - static unsigned int cnt = 0; char buf[8];
memset(t, 0, sizeof(struct smbios_type7)); @@ -662,13 +708,13 @@ t->handle = handle; t->length = len - 2;
- snprintf(buf, sizeof(buf), "CACHE%x", cnt++); + snprintf(buf, sizeof(buf), "CACHE%x", level); t->socket_designation = smbios_add_string(t->eos, buf);
t->cache_configuration = SMBIOS_CACHE_CONF_LEVEL(level) | SMBIOS_CACHE_CONF_LOCATION(0) | /* Internal */ SMBIOS_CACHE_CONF_ENABLED(1) | /* Enabled */ - SMBIOS_CACHE_CONF_OPERATION_MODE(3); /* Unknown */ + SMBIOS_CACHE_CONF_OPERATION_MODE(smbios_cache_conf_operation_mode(level));
if (max_cache_size < (SMBIOS_CACHE_SIZE_MASK * KiB)) { t->max_cache_size = max_cache_size / KiB; @@ -704,11 +750,16 @@ t->installed_size2 |= SMBIOS_CACHE_SIZE2_UNIT_64KB; }
+ t->max_cache_size *= get_number_of_caches(leaf, level); + t->installed_size *= get_number_of_caches(leaf, level); + t->max_cache_size2 *= get_number_of_caches(leaf, level); + t->installed_size2 *= get_number_of_caches(leaf, level); + t->associativity = associativity; t->supported_sram_type = sram_type; t->current_sram_type = sram_type; t->cache_speed = 0; /* Unknown */ - t->error_correction_type = SMBIOS_CACHE_ERROR_CORRECTION_UNKNOWN; + t->error_correction_type = smbios_cache_error_correction_type(); t->system_cache_type = type;
len = t->length + smbios_string_table_len(t->eos); @@ -767,6 +818,7 @@ unsigned int cnt = 0; int len = 0; u32 leaf; + size_t cache0_size = 0;
if (!cpu_have_cpuid()) return len; @@ -803,7 +855,8 @@ 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; + size_t cache_size = assoc * partitions * cache_line_size * + number_of_sets;
if (!cache_type) /* No more caches in the system */ @@ -829,11 +882,21 @@ else associativity = smbios_cache_associativity(assoc);
+ if (level == 1 && type == SMBIOS_CACHE_TYPE_DATA) { + cache0_size = cache_size; + continue; + } + + if (level == 1 && type == SMBIOS_CACHE_TYPE_INSTRUCTION) { + cache_size += cache0_size; + type = SMBIOS_CACHE_TYPE_UNIFIED; + } + const int h = (*handle)++;
update_max(len, *max_struct_size, smbios_write_type7(current, h, - level, SMBIOS_CACHE_SRAM_TYPE_UNKNOWN, associativity, - type, cache_size, cache_size)); + level, smbios_cache_sram_type(), associativity, + type, cache_size, cache_size, leaf));
if (type4) { switch (level) { diff --git a/src/include/smbios.h b/src/include/smbios.h index 521339e..216c8f6 100644 --- a/src/include/smbios.h +++ b/src/include/smbios.h @@ -59,6 +59,10 @@ struct cpuid_result; unsigned int smbios_processor_family(struct cpuid_result res);
+unsigned int smbios_cache_error_correction_type(void); +unsigned int smbios_cache_sram_type(void); +unsigned int smbios_cache_conf_operation_mode(u8 level); + /* Used by mainboard to add port information of type 8 */ struct port_information; int smbios_write_type8(unsigned long *current, int *handle, @@ -499,6 +503,13 @@ #define SMBIOS_CACHE_SIZE2_UNIT_64KB (1UL << 31) #define SMBIOS_CACHE_SIZE2_MASK 0x7fffffff
+/* define for cache operation mode */ + +#define SMBIOS_CACHE_OP_MODE_WRITE_THROUGH 0 +#define SMBIOS_CACHE_OP_MODE_WRITE_BACK 1 +#define SMBIOS_CACHE_OP_MODE_VARIES_WITH_MEMORY_ADDRESS 2 +#define SMBIOS_CACHE_OP_MODE_UNKNOWN 3 + struct smbios_type7 { u8 type; u8 length;
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46068 )
Change subject: arch/x86/smbios: Update SMBIOS type 7 ......................................................................
Patch Set 1: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/46068/1/src/arch/x86/smbios.c File src/arch/x86/smbios.c:
https://review.coreboot.org/c/coreboot/+/46068/1/src/arch/x86/smbios.c@859 PS1, Line 859: number_of_sets nit: fits in the previous line (max length is 96 characters)
Morgan Jang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46068 )
Change subject: arch/x86/smbios: Update SMBIOS type 7 ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46068/1/src/arch/x86/smbios.c File src/arch/x86/smbios.c:
https://review.coreboot.org/c/coreboot/+/46068/1/src/arch/x86/smbios.c@859 PS1, Line 859: number_of_sets
nit: fits in the previous line (max length is 96 characters)
Done
Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46068 )
Change subject: arch/x86/smbios: Update SMBIOS type 7 ......................................................................
Patch Set 1: Code-Review+2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46068 )
Change subject: arch/x86/smbios: Update SMBIOS type 7 ......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/c/coreboot/+/46068/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46068/1//COMMIT_MSG@7 PS1, Line 7: arch/x86/smbios: Update SMBIOS type 7
Populate SMBIOS type 7 with cache information
https://review.coreboot.org/c/coreboot/+/46068/1//COMMIT_MSG@8 PS1, Line 8: Please elaborate:
SMBIOS has a field to display the cache size, which is currently set to UNKNOWN undconditionally. …
https://review.coreboot.org/c/coreboot/+/46068/1/src/arch/x86/smbios.c File src/arch/x86/smbios.c:
https://review.coreboot.org/c/coreboot/+/46068/1/src/arch/x86/smbios.c@490 PS1, Line 490: return 3; /* Unknown */ Please define a macro or enum.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46068 )
Change subject: arch/x86/smbios: Update SMBIOS type 7 ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/46068/1/src/arch/x86/smbios.c File src/arch/x86/smbios.c:
https://review.coreboot.org/c/coreboot/+/46068/1/src/arch/x86/smbios.c@490 PS1, Line 490: return 3; /* Unknown */
Please define a macro or enum.
Just use the SMBIOS_CACHE_OP_MODE_UNKNOWN macro you've added.
https://review.coreboot.org/c/coreboot/+/46068/1/src/arch/x86/smbios.c@859 PS1, Line 859: number_of_sets
Done
I don't see any new patchset. Maybe you forgot to push it?
Hello build bot (Jenkins), Patrick Rudolph, Jonathan Zhang, Johnny Lin, Christian Walter, Jingle Hsu, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46068
to look at the new patch set (#2).
Change subject: arch/x86/smbios: Populate SMBIOS type 7 with cache information ......................................................................
arch/x86/smbios: Populate SMBIOS type 7 with cache information
SMBIOS has a field to display the cache size, which is currently set to UNKNOWN undconditionally. Combine L1 Data cache size and L1 Instruction cache size, and multiply the cache size of L1 and L2 by the number of cores.
TEST=Execute "dmidecode -t 7" to check if the cache information is correct for Deltalake platform
Change-Id: Ieeb5d3346454ffb2291613dc2aa24b31d10c2e04 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com --- M src/arch/x86/smbios.c M src/include/smbios.h 2 files changed, 82 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/46068/2
Hello build bot (Jenkins), Patrick Rudolph, Jonathan Zhang, Johnny Lin, Christian Walter, Jingle Hsu, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46068
to look at the new patch set (#3).
Change subject: arch/x86/smbios: Populate SMBIOS type 7 with cache information ......................................................................
arch/x86/smbios: Populate SMBIOS type 7 with cache information
SMBIOS has a field to display the cache size, which is currently set to UNKNOWN undconditionally. Combine L1 Data cache size and L1 Instruction cache size, and multiply the cache size of L1 and L2 by the number of cores.
TEST=Execute "dmidecode -t 7" to check if the cache information is correct for Deltalake platform
Change-Id: Ieeb5d3346454ffb2291613dc2aa24b31d10c2e04 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com --- M src/arch/x86/smbios.c M src/include/smbios.h 2 files changed, 81 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/46068/3
Hello build bot (Jenkins), Patrick Rudolph, Jonathan Zhang, Johnny Lin, Christian Walter, Jingle Hsu, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46068
to look at the new patch set (#4).
Change subject: arch/x86/smbios: Populate SMBIOS type 7 with cache information ......................................................................
arch/x86/smbios: Populate SMBIOS type 7 with cache information
SMBIOS has a field to display the cache size, which is currently set to UNKNOWN undconditionally. Combine L1 Data cache size and L1 Instruction cache size, and multiply the cache size of L1 and L2 by the number of cores.
TEST=Execute "dmidecode -t 7" to check if the cache information is correct for Deltalake platform
Change-Id: Ieeb5d3346454ffb2291613dc2aa24b31d10c2e04 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com --- M src/arch/x86/smbios.c M src/include/smbios.h D src/soc/intel/xeon_sp/Kconfig 3 files changed, 90 insertions(+), 111 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/46068/4
Morgan Jang 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 4:
(3 comments)
https://review.coreboot.org/c/coreboot/+/46068/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46068/1//COMMIT_MSG@7 PS1, Line 7: arch/x86/smbios: Update SMBIOS type 7
Populate SMBIOS type 7 with cache information
Done
https://review.coreboot.org/c/coreboot/+/46068/1//COMMIT_MSG@8 PS1, Line 8:
Please elaborate: […]
Done
https://review.coreboot.org/c/coreboot/+/46068/1/src/arch/x86/smbios.c File src/arch/x86/smbios.c:
https://review.coreboot.org/c/coreboot/+/46068/1/src/arch/x86/smbios.c@490 PS1, Line 490: return 3; /* Unknown */
Just use the SMBIOS_CACHE_OP_MODE_UNKNOWN macro you've added.
Done
Hello build bot (Jenkins), Patrick Rudolph, Jonathan Zhang, Johnny Lin, Christian Walter, Jingle Hsu, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46068
to look at the new patch set (#5).
Change subject: arch/x86/smbios: Populate SMBIOS type 7 with cache information ......................................................................
arch/x86/smbios: Populate SMBIOS type 7 with cache information
SMBIOS has a field to display the cache size, which is currently set to UNKNOWN undconditionally. Combine L1 Data cache size and L1 Instruction cache size, and multiply the cache size of L1 and L2 by the number of cores.
TEST=Execute "dmidecode -t 7" to check if the cache information is correct for Deltalake platform
Change-Id: Ieeb5d3346454ffb2291613dc2aa24b31d10c2e04 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com --- M src/arch/x86/smbios.c M src/include/smbios.h 2 files changed, 90 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/46068/5
Angel Pons 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 5:
(2 comments)
https://review.coreboot.org/c/coreboot/+/46068/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46068/5//COMMIT_MSG@10 PS5, Line 10: d remove `d`
https://review.coreboot.org/c/coreboot/+/46068/5/src/arch/x86/smbios.c File src/arch/x86/smbios.c:
https://review.coreboot.org/c/coreboot/+/46068/5/src/arch/x86/smbios.c@481 PS5, Line 481: CONFIG_CACHE_ERROR_CORRECTION_TYPE Where are these Kconfig options defined? I'd rather use weak functions that take the cache level as parameter.
Hello build bot (Jenkins), Patrick Rudolph, Jonathan Zhang, Johnny Lin, Christian Walter, Jingle Hsu, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46068
to look at the new patch set (#6).
Change subject: arch/x86/smbios: Populate SMBIOS type 7 with cache information ......................................................................
arch/x86/smbios: Populate SMBIOS type 7 with cache information
SMBIOS has a field to display the cache size, which is currently set to UNKNOWN undconditionally. Combine L1 Data cache size and L1 Instruction cache size, and multiply the cache size of L1 and L2 by the number of cores.
TEST=Execute "dmidecode -t 7" to check if the cache information is correct for Deltalake platform
Change-Id: Ieeb5d3346454ffb2291613dc2aa24b31d10c2e04 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com --- M src/arch/x86/smbios.c M src/include/smbios.h 2 files changed, 82 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/46068/6
Hello build bot (Jenkins), Patrick Rudolph, Jonathan Zhang, Johnny Lin, Christian Walter, Jingle Hsu, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46068
to look at the new patch set (#7).
Change subject: arch/x86/smbios: Populate SMBIOS type 7 with cache information ......................................................................
arch/x86/smbios: Populate SMBIOS type 7 with cache information
SMBIOS has a field to display the cache size, which is currently set to UNKNOWN unconditionally. Combine L1 Data cache size and L1 Instruction cache size, and multiply the cache size of L1 and L2 by the number of cores.
TEST=Execute "dmidecode -t 7" to check if the cache information is correct for Deltalake platform
Change-Id: Ieeb5d3346454ffb2291613dc2aa24b31d10c2e04 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com --- M src/arch/x86/smbios.c M src/include/smbios.h 2 files changed, 82 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/46068/7
Morgan Jang 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 7:
(2 comments)
https://review.coreboot.org/c/coreboot/+/46068/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46068/5//COMMIT_MSG@10 PS5, Line 10: d
remove `d`
Done
https://review.coreboot.org/c/coreboot/+/46068/5/src/arch/x86/smbios.c File src/arch/x86/smbios.c:
https://review.coreboot.org/c/coreboot/+/46068/5/src/arch/x86/smbios.c@481 PS5, Line 481: CONFIG_CACHE_ERROR_CORRECTION_TYPE
Where are these Kconfig options defined? I'd rather use weak functions that take the cache level as […]
Done
Angel Pons 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 7: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/46068/7/src/arch/x86/smbios.c File src/arch/x86/smbios.c:
https://review.coreboot.org/c/coreboot/+/46068/7/src/arch/x86/smbios.c@754 PS7, Line 754: get_number_of_caches(leaf, level); Cache the returned value in a const variable?
const size_t number_of_caches = get_number_of_caches(leaf, level); t->max_cache_size *= number_of_caches; t->installed_size *= number_of_caches; t->max_cache_size2 *= number_of_caches; t->installed_size2 *= number_of_caches;
Hello build bot (Jenkins), Patrick Rudolph, Jonathan Zhang, Johnny Lin, Christian Walter, Jingle Hsu, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46068
to look at the new patch set (#8).
Change subject: arch/x86/smbios: Populate SMBIOS type 7 with cache information ......................................................................
arch/x86/smbios: Populate SMBIOS type 7 with cache information
SMBIOS has a field to display the cache size, which is currently set to UNKNOWN unconditionally. Combine L1 Data cache size and L1 Instruction cache size, and multiply the cache size of L1 and L2 by the number of cores.
TEST=Execute "dmidecode -t 7" to check if the cache information is correct for Deltalake platform
Change-Id: Ieeb5d3346454ffb2291613dc2aa24b31d10c2e04 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com --- M src/arch/x86/smbios.c M src/include/smbios.h 2 files changed, 83 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/46068/8
Morgan Jang 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 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46068/7/src/arch/x86/smbios.c File src/arch/x86/smbios.c:
https://review.coreboot.org/c/coreboot/+/46068/7/src/arch/x86/smbios.c@754 PS7, Line 754: get_number_of_caches(leaf, level);
Cache the returned value in a const variable? […]
Done
Angel Pons 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 8: Code-Review+1
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 8:
(2 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 Why?
It seems to be less accurate than what was reported before.
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 Is there any documentation you followed with these calculations?
Jonathan Zhang 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 8: Code-Review+1
(2 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
Why? […]
In SMBIOS type 4 (processor info), there are handles to cache info in SMBIOS type 7. Right now in SMBIOS type 4, there are 3 handles (eg. L1 D-CACHE and L1 I-CACHE share on handle). In other words, this change aligns with current SMBIOS type 4, and aligns with existing practices in production. I agree that separating L1 D-CACHE and L1 I-CACHE is more accurate. This could be done in next step; otherwise it will cause confusions to users and to apps who make use of SMBIOS type 4 and type 7.
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
Is there any documentation you followed with these calculations?
For Intel design, each core has its own L1 caches.
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 8:
(3 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
I agree that separating L1 D-CACHE and L1 I-CACHE is more accurate. This could be done in next step;
I don't follow. Isn't this what the current code (with this change) does already in the type 7 tables?
What seems missing is the association of the processor (type 4) with the caches (type 7). The specification mentions mentions type 14 tables for such a case (where the simple handles in type 4 don't work out).
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
For Intel design, each core has its own L1 caches.
I know that but get_number_of_caches() seems rather complex with many cases. And I was wondering if there is any documentation about that or if it's all based on testing?
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 information will be missing after this change, afaics.
Morgan Jang 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 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46068/8//COMMIT_MSG Commit Message:
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
I know that but get_number_of_caches() seems rather complex with many cases. […]
I refered to the section of cache information in Intel BWG.
Hello build bot (Jenkins), Patrick Rudolph, Jonathan Zhang, Johnny Lin, Christian Walter, Jingle Hsu, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46068
to look at the new patch set (#9).
Change subject: arch/x86/smbios: Populate SMBIOS type 7 with cache information ......................................................................
arch/x86/smbios: Populate SMBIOS type 7 with cache information
SMBIOS has a field to display the cache size, which is currently set to UNKNOWN unconditionally, multiply the cache size of L1 and L2 by the number of cores.
TEST=Execute "dmidecode -t 7" to check if the cache information is correct for Deltalake platform
Change-Id: Ieeb5d3346454ffb2291613dc2aa24b31d10c2e04 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com --- M src/arch/x86/smbios.c M src/include/smbios.h 2 files changed, 71 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/46068/9
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 9:
(6 comments)
https://review.coreboot.org/c/coreboot/+/46068/8//COMMIT_MSG Commit Message:
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
I refered to the section of cache information in Intel BWG.
Which BWG in particular? Intel seems to hide the Server ones, can you share a document number?
I was asking because the doesn't look like anything what I'd expect. I've reviewed it now, but it does weird things that I'm not sure if all my comments are correct.
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@507 PS9, Line 507: cpu_have_cpuid Do we really have to check this? Does coreboot still support CPUs that don't have it.
Much simpler would be a single check, then you don't have to repeat it later, e.g.
if (!cpu_have_cpuid()) return 1;
https://review.coreboot.org/c/coreboot/+/46068/9/src/arch/x86/smbios.c@513 PS9, Line 513: 1 AIUI, `1` gives the number of cores, `0` would give the number of threads, aka. logical cores.
https://review.coreboot.org/c/coreboot/+/46068/9/src/arch/x86/smbios.c@517 PS9, Line 517: } I don't understand why cpuid(0xb) is queried at all. Is it supposed to be more accurate?
https://review.coreboot.org/c/coreboot/+/46068/9/src/arch/x86/smbios.c@519 PS9, Line 519: level `level` is not the same as the sub-leaf here. For instance L1I and L1D can have separate sub-leafs but are on the same level.
https://review.coreboot.org/c/coreboot/+/46068/9/src/arch/x86/smbios.c@522 PS9, Line 522: if (max_logical_cpus_sharing_cache != max_logical_cpus_per_package) With the distinction of `number_of_logical_cpus` and `max_logical_cpus_per_ package`, this looks very odd. Did you maybe mean to use `number_of_logical_cpus` here?
Hello build bot (Jenkins), Patrick Rudolph, Jonathan Zhang, Johnny Lin, Christian Walter, Jingle Hsu, Angel Pons, Michael Niewöhner, Aaron Durbin, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46068
to look at the new patch set (#10).
Change subject: arch/x86/smbios: Populate SMBIOS type 7 with cache information ......................................................................
arch/x86/smbios: Populate SMBIOS type 7 with cache information
SMBIOS has a field to display the cache size, which is currently set to UNKNOWN unconditionally, multiply the cache size of L1 and L2 by the number of cores.
TEST=Execute "dmidecode -t 7" to check if the cache information is correct for Deltalake platform
Change-Id: Ieeb5d3346454ffb2291613dc2aa24b31d10c2e04 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com --- M src/arch/x86/smbios.c M src/include/smbios.h 2 files changed, 74 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/46068/10
Morgan Jang 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:
(7 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
I agree that separating L1 D-CACHE and L1 I-CACHE is more accurate. […]
Revert it for seperating L1 D-CACHE and L1 I-CACHE.
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
Which BWG in particular? Intel seems to hide the Server ones, can you share […]
Please refer to Doc#553625.
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@507 PS9, Line 507: cpu_have_cpuid
Do we really have to check this? Does coreboot still support CPUs that […]
Done
https://review.coreboot.org/c/coreboot/+/46068/9/src/arch/x86/smbios.c@513 PS9, Line 513: 1
AIUI, `1` gives the number of cores, `0` would give the number of threads, […]
Done
https://review.coreboot.org/c/coreboot/+/46068/9/src/arch/x86/smbios.c@517 PS9, Line 517: }
I don't understand why cpuid(0xb) is queried at all. Is it supposed to be […]
Done
https://review.coreboot.org/c/coreboot/+/46068/9/src/arch/x86/smbios.c@519 PS9, Line 519: level
`level` is not the same as the sub-leaf here. For instance L1I and L1D can […]
Done
https://review.coreboot.org/c/coreboot/+/46068/9/src/arch/x86/smbios.c@522 PS9, Line 522: if (max_logical_cpus_sharing_cache != max_logical_cpus_per_package)
With the distinction of `number_of_logical_cpus` and `max_logical_cpus_per_ […]
Done
build bot (Jenkins) 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:
(1 comment)
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@519 PS10, Line 519: } else { else is not generally useful after a break or return
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`.
Hello build bot (Jenkins), Nico Huber, Patrick Rudolph, Jonathan Zhang, Johnny Lin, Christian Walter, Jingle Hsu, Angel Pons, Michael Niewöhner, Aaron Durbin, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46068
to look at the new patch set (#11).
Change subject: arch/x86/smbios: Populate SMBIOS type 7 with cache information ......................................................................
arch/x86/smbios: Populate SMBIOS type 7 with cache information
SMBIOS has a field to display the cache size, which is currently set to UNKNOWN unconditionally, multiply the cache size of L1 and L2 by the number of cores.
TEST=Execute "dmidecode -t 7" to check if the cache information is correct for Deltalake platform
Change-Id: Ieeb5d3346454ffb2291613dc2aa24b31d10c2e04 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com --- M src/arch/x86/smbios.c M src/include/smbios.h 2 files changed, 74 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/46068/11
Morgan Jang 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 11:
(1 comment)
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
I see you changed the variable name. The description of leaf 0xb is very […]
I changed the variable name back.
build bot (Jenkins) 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 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46068/11/src/arch/x86/smbios.c File src/arch/x86/smbios.c:
https://review.coreboot.org/c/coreboot/+/46068/11/src/arch/x86/smbios.c@519 PS11, Line 519: } else { else is not generally useful after a break or return
Jonathan Zhang 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 11: Code-Review+1
Angel Pons 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 11: Code-Review+1
(5 comments)
https://review.coreboot.org/c/coreboot/+/46068/11/src/arch/x86/smbios.c File src/arch/x86/smbios.c:
https://review.coreboot.org/c/coreboot/+/46068/11/src/arch/x86/smbios.c@503 PS11, Line 503: /* Provide sane defaults even for CPU without CPUID */ : res.eax = res.edx = 0; : res.ebx = 0x10000; This isn't necessary anymore
https://review.coreboot.org/c/coreboot/+/46068/11/src/arch/x86/smbios.c@516 PS11, Line 516: ` nit: this is a backtick, but should be a single quote instead: '
https://review.coreboot.org/c/coreboot/+/46068/11/src/arch/x86/smbios.c@519 PS11, Line 519: } else {
else is not generally useful after a break or return
It would be good to handle this
https://review.coreboot.org/c/coreboot/+/46068/11/src/arch/x86/smbios.c@766 PS11, Line 766: t->max_cache_size *= number_of_caches; : t->installed_size *= number_of_caches; : t->max_cache_size2 *= number_of_caches; : t->installed_size2 *= number_of_caches; This will cause problems with the `SMBIOS_CACHE_SIZE_UNIT_64KB` flag set above. I'd instead multiply once in `smbios_write_type7_cache_parameters()`
https://review.coreboot.org/c/coreboot/+/46068/11/src/arch/x86/smbios.c@870 PS11, Line 870: cache_size How about multiplying by `get_number_of_caches()` here instead?
Hello build bot (Jenkins), Nico Huber, Patrick Rudolph, Jonathan Zhang, Johnny Lin, Christian Walter, Jingle Hsu, Angel Pons, Michael Niewöhner, Aaron Durbin, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46068
to look at the new patch set (#12).
Change subject: arch/x86/smbios: Populate SMBIOS type 7 with cache information ......................................................................
arch/x86/smbios: Populate SMBIOS type 7 with cache information
SMBIOS has a field to display the cache size, which is currently set to UNKNOWN unconditionally, multiply the cache size of L1 and L2 by the number of cores.
TEST=Execute "dmidecode -t 7" to check if the cache information is correct for Deltalake platform
Change-Id: Ieeb5d3346454ffb2291613dc2aa24b31d10c2e04 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com --- M src/arch/x86/smbios.c M src/include/smbios.h 2 files changed, 63 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/46068/12
build bot (Jenkins) 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 12:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46068/12/src/arch/x86/smbios.c File src/arch/x86/smbios.c:
https://review.coreboot.org/c/coreboot/+/46068/12/src/arch/x86/smbios.c@513 PS12, Line 513: if (max_logical_cpus_sharing_cache == max_logical_cpus_per_package) { braces {} are not necessary for single statement blocks
Hello build bot (Jenkins), Nico Huber, Patrick Rudolph, Jonathan Zhang, Johnny Lin, Christian Walter, Jingle Hsu, Angel Pons, Michael Niewöhner, Aaron Durbin, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46068
to look at the new patch set (#13).
Change subject: arch/x86/smbios: Populate SMBIOS type 7 with cache information ......................................................................
arch/x86/smbios: Populate SMBIOS type 7 with cache information
SMBIOS has a field to display the cache size, which is currently set to UNKNOWN unconditionally, multiply the cache size of L1 and L2 by the number of cores.
TEST=Execute "dmidecode -t 7" to check if the cache information is correct for Deltalake platform
Change-Id: Ieeb5d3346454ffb2291613dc2aa24b31d10c2e04 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com --- M src/arch/x86/smbios.c M src/include/smbios.h 2 files changed, 62 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/46068/13
Morgan Jang 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 13:
(5 comments)
https://review.coreboot.org/c/coreboot/+/46068/11/src/arch/x86/smbios.c File src/arch/x86/smbios.c:
https://review.coreboot.org/c/coreboot/+/46068/11/src/arch/x86/smbios.c@503 PS11, Line 503: /* Provide sane defaults even for CPU without CPUID */ : res.eax = res.edx = 0; : res.ebx = 0x10000;
This isn't necessary anymore
Done
https://review.coreboot.org/c/coreboot/+/46068/11/src/arch/x86/smbios.c@516 PS11, Line 516: `
nit: this is a backtick, but should be a single quote instead: '
Done
https://review.coreboot.org/c/coreboot/+/46068/11/src/arch/x86/smbios.c@519 PS11, Line 519: } else {
It would be good to handle this
Done
https://review.coreboot.org/c/coreboot/+/46068/11/src/arch/x86/smbios.c@766 PS11, Line 766: t->max_cache_size *= number_of_caches; : t->installed_size *= number_of_caches; : t->max_cache_size2 *= number_of_caches; : t->installed_size2 *= number_of_caches;
This will cause problems with the `SMBIOS_CACHE_SIZE_UNIT_64KB` flag set above. […]
Done
https://review.coreboot.org/c/coreboot/+/46068/11/src/arch/x86/smbios.c@870 PS11, Line 870: cache_size
How about multiplying by `get_number_of_caches()` here instead?
Done
Angel Pons 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 13: Code-Review+1
Jonathan Zhang 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 13: Code-Review+2
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 13: Code-Review+1
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/46068 )
Change subject: arch/x86/smbios: Populate SMBIOS type 7 with cache information ......................................................................
arch/x86/smbios: Populate SMBIOS type 7 with cache information
SMBIOS has a field to display the cache size, which is currently set to UNKNOWN unconditionally, multiply the cache size of L1 and L2 by the number of cores.
TEST=Execute "dmidecode -t 7" to check if the cache information is correct for Deltalake platform
Change-Id: Ieeb5d3346454ffb2291613dc2aa24b31d10c2e04 Signed-off-by: Morgan Jang Morgan_Jang@wiwynn.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/46068 Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Jonathan Zhang jonzhang@fb.com Reviewed-by: Nico Huber nico.h@gmx.de Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/arch/x86/smbios.c M src/include/smbios.h 2 files changed, 62 insertions(+), 6 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, but someone else must approve Angel Pons: Looks good to me, but someone else must approve Jonathan Zhang: Looks good to me, approved
diff --git a/src/arch/x86/smbios.c b/src/arch/x86/smbios.c index 2995ece..19b6f5d 100644 --- a/src/arch/x86/smbios.c +++ b/src/arch/x86/smbios.c @@ -478,6 +478,51 @@ return (res.eax > 0) ? 0x0c : 0x6; }
+unsigned int __weak smbios_cache_error_correction_type(u8 level) +{ + return SMBIOS_CACHE_ERROR_CORRECTION_UNKNOWN; +} + +unsigned int __weak smbios_cache_sram_type(void) +{ + return SMBIOS_CACHE_SRAM_TYPE_UNKNOWN; +} + +unsigned int __weak smbios_cache_conf_operation_mode(u8 level) +{ + return SMBIOS_CACHE_OP_MODE_UNKNOWN; /* Unknown */ +} + +static size_t get_number_of_caches(struct cpuid_result res_deterministic_cache) +{ + size_t max_logical_cpus_sharing_cache = 0; + size_t number_of_cpus_per_package = 0; + size_t max_logical_cpus_per_package = 0; + struct cpuid_result res; + + if (!cpu_have_cpuid()) + return 1; + + res = cpuid(1); + + max_logical_cpus_per_package = (res.ebx >> 16) & 0xff; + + max_logical_cpus_sharing_cache = ((res_deterministic_cache.eax >> 14) & 0xfff) + 1; + + /* Check if it's last level cache */ + if (max_logical_cpus_sharing_cache == max_logical_cpus_per_package) + return 1; + + if (cpuid_get_max_func() >= 0xb) { + res = cpuid_ext(0xb, 1); + number_of_cpus_per_package = res.ebx & 0xff; + } else { + number_of_cpus_per_package = max_logical_cpus_per_package; + } + + return number_of_cpus_per_package / max_logical_cpus_sharing_cache; +} + static int smbios_write_type1(unsigned long *current, int handle) { struct smbios_type1 *t = (struct smbios_type1 *)*current; @@ -662,7 +707,6 @@ { struct smbios_type7 *t = (struct smbios_type7 *)*current; int len = sizeof(struct smbios_type7); - static unsigned int cnt = 0; char buf[8];
memset(t, 0, sizeof(struct smbios_type7)); @@ -670,13 +714,13 @@ t->handle = handle; t->length = len - 2;
- snprintf(buf, sizeof(buf), "CACHE%x", cnt++); + snprintf(buf, sizeof(buf), "CACHE%x", level); t->socket_designation = smbios_add_string(t->eos, buf);
t->cache_configuration = SMBIOS_CACHE_CONF_LEVEL(level) | SMBIOS_CACHE_CONF_LOCATION(0) | /* Internal */ SMBIOS_CACHE_CONF_ENABLED(1) | /* Enabled */ - SMBIOS_CACHE_CONF_OPERATION_MODE(3); /* Unknown */ + SMBIOS_CACHE_CONF_OPERATION_MODE(smbios_cache_conf_operation_mode(level));
if (max_cache_size < (SMBIOS_CACHE_SIZE_MASK * KiB)) { t->max_cache_size = max_cache_size / KiB; @@ -716,7 +760,7 @@ t->supported_sram_type = sram_type; t->current_sram_type = sram_type; t->cache_speed = 0; /* Unknown */ - t->error_correction_type = SMBIOS_CACHE_ERROR_CORRECTION_UNKNOWN; + t->error_correction_type = smbios_cache_error_correction_type(level); t->system_cache_type = type;
len = t->length + smbios_string_table_len(t->eos); @@ -811,7 +855,8 @@ 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; + const size_t cache_size = assoc * partitions * cache_line_size * number_of_sets + * get_number_of_caches(res);
if (!cache_type) /* No more caches in the system */ @@ -840,7 +885,7 @@ const int h = (*handle)++;
update_max(len, *max_struct_size, smbios_write_type7(current, h, - level, SMBIOS_CACHE_SRAM_TYPE_UNKNOWN, associativity, + level, smbios_cache_sram_type(), associativity, type, cache_size, cache_size));
if (type4) { diff --git a/src/include/smbios.h b/src/include/smbios.h index 8033d6c..6a19655 100644 --- a/src/include/smbios.h +++ b/src/include/smbios.h @@ -61,6 +61,10 @@ struct cpuid_result; unsigned int smbios_processor_family(struct cpuid_result res);
+unsigned int smbios_cache_error_correction_type(u8 level); +unsigned int smbios_cache_sram_type(void); +unsigned int smbios_cache_conf_operation_mode(u8 level); + /* Used by mainboard to add port information of type 8 */ struct port_information; int smbios_write_type8(unsigned long *current, int *handle, @@ -501,6 +505,13 @@ #define SMBIOS_CACHE_SIZE2_UNIT_64KB (1UL << 31) #define SMBIOS_CACHE_SIZE2_MASK 0x7fffffff
+/* define for cache operation mode */ + +#define SMBIOS_CACHE_OP_MODE_WRITE_THROUGH 0 +#define SMBIOS_CACHE_OP_MODE_WRITE_BACK 1 +#define SMBIOS_CACHE_OP_MODE_VARIES_WITH_MEMORY_ADDRESS 2 +#define SMBIOS_CACHE_OP_MODE_UNKNOWN 3 + struct smbios_type7 { u8 type; u8 length;
-
Angel Pons (Code Review) -
build bot (Jenkins) (Code Review)
-
Jonathan Zhang (Code Review)
-
Morgan Jang (Code Review)
-
Nico Huber (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)