Re: [PATCH 5/6] xen/x86: Derive topologically correct x2APIC IDs from the policy

[Jan Beulich <jbeulich@xxxxxxxx>]

On 01.05.2024 18:35, Alejandro Vallejo wrote:
> Hi,
> 
> On 26/03/2024 16:41, Jan Beulich wrote:
>> On 09.01.2024 16:38, Alejandro Vallejo wrote:
>>> --- a/xen/lib/x86/policy.c
>>> +++ b/xen/lib/x86/policy.c
>>> @@ -2,15 +2,78 @@
>>>  
>>>  #include <xen/lib/x86/cpu-policy.h>
>>>  
>>> -uint32_t x86_x2apic_id_from_vcpu_id(const struct cpu_policy *p, uint32_t 
>>> vcpu_id)
>>> +static uint32_t parts_per_higher_scoped_level(const struct cpu_policy *p, 
>>> size_t lvl)
>>>  {
>>>      /*
>>> -     * TODO: Derive x2APIC ID from the topology information inside `p`
>>> -     *       rather than from vCPU ID. This bodge is a temporary measure
>>> -     *       until all infra is in place to retrieve or derive the initial
>>> -     *       x2APIC ID from migrated domains.
>>> +     * `nr_logical` reported by Intel is the number of THREADS contained in
>>> +     * the next topological scope. For example, assuming a system with 2
>>> +     * threads/core and 3 cores/module in a fully symmetric topology,
>>> +     * `nr_logical` at the core level will report 6. Because it's reporting
>>> +     * the number of threads in a module.
>>> +     *
>>> +     * On AMD/Hygon, nr_logical is already normalized by the higher scoped
>>> +     * level (cores/complex, etc) so we can return it as-is.
>>>       */
>>> -    return vcpu_id * 2;
>>> +    if ( p->x86_vendor != X86_VENDOR_INTEL || !lvl )
>>> +        return p->topo.subleaf[lvl].nr_logical;
>>
>> Is "!= Intel" really appropriate here? I'd rather see this being "AMD || 
>> Hygon".
> 
> Sure, I don't particularly mind, but why? As far as we know only Intel
> has this interpretation for the part counts. I definitely haven't seen
> any non-Intel CPUID dump in which the part count is the total number of
> threads (Centaur/Zhaoxin are not multithreaded, and don't expose leaves
> 1f or e26, as far as I could see).

Because of x86'es origin and perhaps other historical aspects, cloning
Intel behavior is far more likely. The fact that Hygon matches AMD is
simply because they took AMD's design wholesale.

Jan