When the CPU Topology was added to bhyve the SMBIOS table was missed, this table passes topology information to the system and was still using the old concept of each vCPU is a socket with 1 core and 1 thread. This code did not even try to use the old sysctl information to adjust this data.
Details
Details
- Reviewers
jhb tychon pmooney_pfmooney.com - Group Reviewers
bhyve - Commits
- rS346717: Make bhyve SMBIOS table topology aware
Run various vm's and dump the SMBIOS table with dmidecode, examining type 4 information.
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
Soliciting feedback at this time, this is not finished code though
| usr.sbin/bhyve/bhyverun.h | ||
|---|---|---|
| 39 ↗ | (On Diff #53290) | I would like to change these data types to match what SMBIOS uses for them, which is uint8_t. |
| usr.sbin/bhyve/smbiostbl.c | ||
| 660 ↗ | (On Diff #53290) | I am aware that this can overflow, I shall discuss how best to handle that with bde@ before deciding which way to handle it. Also at line 658 since we are stroing a uint16_t into a uint8_t, but that one goes away if I adjust the types of all 3 sockets, cores, threads. |
| usr.sbin/bhyve/bhyverun.h | ||
|---|---|---|
| 39 ↗ | (On Diff #53290) | I think it's fine to just leave these as uint16_t. One question is if we'd rather have a function to query them rather than exporting the actual globals. I don't have a strong opinion either way. |
| usr.sbin/bhyve/smbiostbl.c | ||
| 660 ↗ | (On Diff #53290) | Ok, so the docs I looked at (SMBIOS 3.0.0) say to cap both of these values at 0xFF if these overflow. SMBIOS 3.0.0 adds three new 16-bit fields (core count 2, core enabled 2, and thread count 2). If we aren't yet supporting SMBIOS 3.0.0 then we should just cap these values on overflow. We can't support more than 254 vCPUs for the foreseeable future, so that's probably good enough. I don't think we need to worry about trying to support SMBIOS 3.0.0 for now (and if we did we'd have to go through and check all of the other tables we generate, etc.) |
Comment Actions
I have found that we claim to implement version 2.6 which is here: https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_2.6.0.pdf, this spec does not mention any clamping to 0xff, though it does say for unknown values of cores and threads 0 is used. 2.7, 2.7.1, and 2.8.0 have the same language. Finally in 3.0.0 the language is added that says to clamp at 0xff and use core count 2, but still also says set to 0 for unknown. My proposed solution is to strickly obey 2.6, if core or thread count is to large this value is going to be set to 0.