Re: [XEN PATCH 2/9] x86: don't access x86_cpu_to_apicid[] directly, use cpu_physical_id(cpu)

[Jan Beulich <jbeulich@xxxxxxxx>]

On 14.11.2023 18:49, Krystian Hebel wrote:
> --- a/xen/arch/x86/apic.c
> +++ b/xen/arch/x86/apic.c
> @@ -950,7 +950,7 @@ __next:
>       */
>      if (boot_cpu_physical_apicid == -1U)
>          boot_cpu_physical_apicid = get_apic_id();
> -    x86_cpu_to_apicid[0] = get_apic_id();
> +    cpu_physical_id(0) = get_apic_id();

While personally I don't mind as much, I expect Andrew would not like
this: Something that looks like a function call on the lhs is against
what normal language structure would be.

> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -1615,7 +1615,7 @@ long do_vcpu_op(int cmd, unsigned int vcpuid, 
> XEN_GUEST_HANDLE_PARAM(void) arg)
>              break;
>  
>          cpu_id.phys_id =
> -            (uint64_t)x86_cpu_to_apicid[v->vcpu_id] |
> +            (uint64_t)cpu_physical_id(v->vcpu_id) |
>              ((uint64_t)acpi_get_processor_id(v->vcpu_id) << 32);

While the cast on the 2nd line is necessary, the one on the 2st isn't
and would be nice to be dropped while touching the line anyway.

> --- a/xen/arch/x86/numa.c
> +++ b/xen/arch/x86/numa.c
> @@ -70,7 +70,7 @@ void __init init_cpu_to_node(void)
>  
>      for ( i = 0; i < nr_cpu_ids; i++ )
>      {
> -        u32 apicid = x86_cpu_to_apicid[i];
> +        u32 apicid = cpu_physical_id(i);
>          if ( apicid == BAD_APICID )
>              continue;
>          node = apicid < MAX_LOCAL_APIC ? apicid_to_node[apicid] : 
> NUMA_NO_NODE;

We're in the process of phasing out u32 and friends, favoring uint32_t.
Would be nice if in code being touched anyway (i.e. not just here) the
conversion would be done right away. Then again fixed-width types are
preferably avoided where not really needed (see ./CODING_STYLE), so
quite likely it actually wants to be unsigned int here.

Furthermore our style demands that declaration(s) and statement(s) are
separated by a blank line. Inserting the missing one in cases like the
one here would be very desirable as well.

Jan