Re: [Xen-devel] [PATCH v2] libxc/x86: avoid overflow in CPUID APIC ID adjustments

[Jan Beulich <jbeulich@xxxxxxxx>]

On 20.09.2019 15:54, Jan Beulich wrote:
> Recent AMD processors may report up to 128 logical processors in CPUID
> leaf 1. Doubling this value produces 0 (which OSes sincerely dislike),
> as the respective field is only 8 bits wide. Suppress doubling the value
> (and its leaf 0x80000008 counterpart) in such a case.
> 
> Note that while there's a similar overflow in intel_xc_cpuid_policy(),
> that one is being left alone for now.
> 
> Note further that while it was considered to suppress the multiplication
> by 2 altogether if the host topology already provides at least one bit
> of thread ID within APIC IDs, it was decided to avoid more change here
> than really needed at this point.
> 
> Also zap leaf 4 (and at the same time leaf 2) EDX output for AMD, as it
> should have been from the beginning.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> v2: Drop use of physinfo. Drop Intel-only leaf 4 change. Increment
>     ApicIdCoreSize only when doubling NumberOfCores.

Thinking about it some more, dropping the leaf 4 change seems at least
somewhat risky to me. This being just a 6-bit field (and effectively
already saturating in a way, at least when power-of-two maximum core
counts are involved), and hence there being 2 bits of "playing room"
between this and the involved leaf 1 field, the calculation there not
getting adjusted is still a latent risk imo with guest side calculations
like this one

        smp_num_siblings = smp_num_siblings / c->x86_max_cores;

found in basically all versions of Linux (where certain functions,
e.g. apic_id_is_primary_thread(), won't cope with smp_num_siblings
ending up as zero, while others, e.g. topology_smt_supported(), do).

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel