Page MenuHomeFreeBSD

Make bhyve SMBIOS table topology aware
ClosedPublic

Authored by rgrimes on Jan 28 2019, 2:35 AM.

Details

Summary

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.

Test Plan

Run various vm's and dump the SMBIOS table with dmidecode, examining type 4 information.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

rgrimes created this revision.Jan 28 2019, 2:35 AM
rgrimes edited the summary of this revision. (Show Details)Jan 28 2019, 2:39 AM
rgrimes edited the test plan for this revision. (Show Details)
rgrimes set the repository for this revision to rS FreeBSD src repository.

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.

jhb added inline comments.Feb 21 2019, 6:08 PM
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.)

jhb added a comment.Feb 21 2019, 6:09 PM

FYI, you can get SMBIOS specs here: https://www.dmtf.org/standards/smbios

In D18998#412749, @jhb wrote:

FYI, you can get SMBIOS specs here: https://www.dmtf.org/standards/smbios

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.

rgrimes updated this revision to Diff 54197.Feb 21 2019, 8:42 PM
rgrimes marked 2 inline comments as done.

Add handling of cores or threads > 254 by truncating to 0, per smbios 2.6 spec

jhb accepted this revision.Feb 21 2019, 8:52 PM

Agree with going with 0 to conform to 2.6.

This revision is now accepted and ready to land.Feb 21 2019, 8:52 PM
This revision was automatically updated to reflect the committed changes.