Francois Toguo Fotso has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31863 )
Change subject: src/arch: An upgrade of SMBIOS to latest version 3.2 ......................................................................
Patch Set 13:
(10 comments)
Patch Set 6:
(1 comment)
Done. Sure the function can be added in futures patches, since this upgrade will be done in several steps.But in this case I am checking to see if the leaf_b is implemented per your previous suggestion. This sentence from the same document captures the approach I am taking:
"If a value entered for CPUID.EAX is less than or equal to the maximum input value and the leaf is not supported on that processor then 0 is returned in all the registers"
https://review.coreboot.org/#/c/31863/4/src/arch/x86/smbios.c File src/arch/x86/smbios.c:
https://review.coreboot.org/#/c/31863/4/src/arch/x86/smbios.c@551 PS4, Line 551: u16 __weak smbios_processor_core_count(void)
Generally, non-empty weak implementations are discouraged. OTOH, […]
Done
https://review.coreboot.org/#/c/31863/4/src/arch/x86/smbios.c@556 PS4, Line 556: while (1) {
please, use a for loop
Done
https://review.coreboot.org/#/c/31863/4/src/arch/x86/smbios.c@558 PS4, Line 558: leaf_b = cpuid_ext(0xb, ecx);
Should we check if leaf 0xb is implemented and return 1 if not? […]
Done
https://review.coreboot.org/#/c/31863/4/src/arch/x86/smbios.c@569 PS4, Line 569: u16 __weak smbios_processor_thread_count(void)
This is basically the same function as the one above. How about […]
Done
https://review.coreboot.org/#/c/31863/4/src/arch/x86/smbios.c@618 PS4, Line 618: & 0x00ff
Nit, explicit masking isn't necessary.
Done
https://review.coreboot.org/#/c/31863/4/src/arch/x86/smbios.c@619 PS4, Line 619: t->core_count2 = 0x00ff;
AFAIR, the spec says this field always contains the correct […]
Done
https://review.coreboot.org/#/c/31863/6/src/arch/x86/smbios.c File src/arch/x86/smbios.c:
https://review.coreboot.org/#/c/31863/6/src/arch/x86/smbios.c@559 PS6, Line 559: if (leaf_b.eax == 0x0000 && leaf_b.ebx == 0x0000
trailing whitespace
Done
https://review.coreboot.org/#/c/31863/6/src/arch/x86/smbios.c@560 PS6, Line 560: && leaf_b.ecx == 0x0000 && leaf_b.edx == 0x0000) {
I guess you have to check the eax result of cpuid leaf 0. If there […]
Sure the function can be added in futures patches, since this upgrade will be done in several steps.But in this case I am checkind to see if the leaf_b is implemented per your previous suggestion. This sentence from the same document captures the approach I am taking: "If a value entered for CPUID.EAX is less than or equal to the maximum input value and the leaf is not supported on that processor then 0 is returned in all the registers"
https://review.coreboot.org/#/c/31863/11/src/arch/x86/smbios.c File src/arch/x86/smbios.c:
https://review.coreboot.org/#/c/31863/11/src/arch/x86/smbios.c@556 PS11, Line 556: int ecx = 0
just ecx
Done
https://review.coreboot.org/#/c/31863/4/src/include/smbios.h File src/include/smbios.h:
https://review.coreboot.org/#/c/31863/4/src/include/smbios.h@429 PS4, Line 429: log_type_descriptor descriptor[256];
any plans to use it?
Done