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
