Re: HVM guest only bring up a single vCPU

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 27/08/2021 10:26, Jan Beulich wrote:
> On 26.08.2021 23:00, Julien Grall wrote:
>> Digging down, Linux will set smp_num_siblings to 0 (via 
>> detect_ht_early()) and as a result will skip all the CPUs. The value is 
>> retrieve from a CPUID leaf. So it sounds like we don't set the leaft 
>> correctly.
> Xen leaves leaf 1 EBX[23:16] untouched from what the tool stack
> passes. The tool stack doubles the value coming from hardware
> (or qemu in your case), unless the result would overflow. Hence
> it would look to me as if the value coming from qemu has got to
> be zero. Which is perfectly fine if HTT is off, just that
> libxenguest isn't prepared for this. Could you see whether the
> patch below helps (making our hack even hackier)?
>
> Jan
>
> libxenguest/x86: ensure CPUID[1].EBX[32:16] is non-zero for HVM
>
> We unconditionally set HTT, so merely doubling the value read from
> hardware isn't going to be correct if that value is zero.
>
> Reported-by: Julien Grall <julien@xxxxxxx>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

I don't particularly like this, but I don't like any of the junk we
currently have here.

This codepath ought to be limited to virtual environments which have
given us garbage in p->basic.lppp in the first place.

Therefore, Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

> ---
> I question the doubling in the first place, as that leads to absurd
> values when the underlying hardware has a value larger than 1 here.

It's broken for several reasons, perhaps most obviously because it is a
gross assumption that all systems look like a Intel Core/Xeon with
Hyperthreading.

The right way to fix this is to pack the APIC IDs tightly (which
actually lets us break the 128 vcpu barrier without vIOMMU), and adjust
the SMT mask in leaf 0xd to compensate.

We need a slide of 8 on the APIC IDs to do AMD legacy topology by the
book, but as we've not had that before, I'm quite tempted to leave
implementing that algorithm to whomever first actually needs it.

> I'd be inclined to suggest to double the value only if the incoming value
> has bit 0 set. And then we'd want to also deal with the case of both
> bit 0 and bit 7 being set (perhaps by clearing bit 0 in this case).

Honestly, until someone starts the "lets do topology correctly,
following Intel and AMD's topology algorithms", I recommend not
tinkering.  It is incredibly fragile logic.

~Andrew