- 
                Notifications
    
You must be signed in to change notification settings  - Fork 27
 
Description
In fix_cpu_topo(), when specializing a provided CPUID set for the VM's requested topology, we have:
                    if self.has_smt {
                        set.insert(
                            CpuidIdent::subleaf(leaf, 0),
                            CpuidValues {
                                eax: 0x1,
                                ebx: 0x2,
                                ecx: 0x100,
                                // TODO: populate with x2APIC ID
                                edx: 0x0,
                            },
                        );
                    } else {
                        set.insert(
                            CpuidIdent::subleaf(leaf, 0),
                            CpuidValues {
                                eax: 0x0,
                                ebx: 0x1,
                                ecx: 0x100,
                                // TODO: populate with x2APIC ID
                                edx: 0x0,
                            },
                        );
                    }
                    set.insert(
                        CpuidIdent::subleaf(leaf, 1),
                        CpuidValues {
                            eax: 0x0,
                            ebx: num_vcpu,
                            ecx: 0x201,
                            // TODO: populate with x2APIC ID
                            edx: 0x0,
                        },
                    );but quoth APM vol. 3 section E.3.8 "Function Bh - Extended Topology Enumeration":
If this function is executed with an unimplemented level (passed in ECX), the instruction returns all zeros in the EAX register.
so we fix up EBX to report the number of cores in the VM, but then a zeroed EAX implies that the subleaf is invalid. In a hardware CPUID dump eax is something like 7 (e.g. up to 128 threads in a socket). From my 3950x:
   0x0000000b 0x00: eax=0x00000001 ebx=0x00000002 ecx=0x00000100 edx=0x00000015
   0x0000000b 0x01: eax=0x00000007 ebx=0x00000020 ecx=0x00000201 edx=0x00000015
or from a 9365:
CPUID 0000000b:00 = 00000001 00000002 00000100 00000000 | ................
CPUID 0000000b:01 = 00000007 00000048 00000201 00000000 | ....H...........
(I'd take a wild guess that this might be 8 for Turin Dense parts though...)
If !self.has_smt, we'll also set EAX in B.1 to zero. I think the correct thing to do here is set EAX to 1 regardless, with EBX varying between 1 and 2 to reflect threads-per-core. In a many-core no-SMT case I think this also implies that APIC IDs would have to have to skip one from core to core (e.g. IDs 0, 2, 4, 6, ...) so that the right shift of 1 is correct. That said, we always set has_smt in both propolis-standalone and propolis-server, even if there's only one vCPU, so that's.. funky.
Either way can just fix this because guests don't get anything for leaf B right now anyway.