Re: [Xen-devel] [PATCH v5 0/8] Display IO topology when PXM data is available (plus some cleanup)

[George Dunlap <dunlapg@xxxxxxxxx>]

On Mon, Mar 23, 2015 at 1:47 PM, Boris Ostrovsky
<boris.ostrovsky@xxxxxxxxxx> wrote:
> On 03/23/2015 08:42 AM, Julien Grall wrote:
>>
>> Hi,
>>
>> On 19/03/15 21:53, Boris Ostrovsky wrote:
>>>
>>> A few patches that add interface for querying hypervisor about device
>>> topology and allow 'xl info -n' display this information if PXM object
>>> is provided by ACPI.
>>>
>>> This series also makes some optimizations and cleanup of current CPU
>>> topology and NUMA sysctl queries.
>>
>> I saw that some of these patches was pushed upstream last week. It
>> actually breaks compilation on ARM.
>>
>> While the first error is trivial to fix (it's a missing include
>> xen/numa.h in xen/pci.h ). The second one is more difficult
>>
>> sysctl.c: In function âdo_sysctlâ:
>> sysctl.c:349:42: error: âBAD_APICIDâ undeclared (first use in this
>> function)
>>                       if ( cputopo.core == BAD_APICID )
>>                                            ^
>> The value BAD_APICID doesn't have any meaning on ARM. Therefore the
>> usage in common code looks wrong to me. I'm not sure what should be the
>> testing value here given that cpu_to_core is not yet correctly
>> implemented on ARM.
>
>
> How about this (only x86 compile-tested). And perhaps, while at it, fix
> types for cpu_core_id and phys_proc_id.
>
> diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
> index c73dfc9..b319be7 100644
> --- a/xen/common/sysctl.c
> +++ b/xen/common/sysctl.c
> @@ -354,10 +354,10 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t)
> u_sysctl)
>                  if ( cpu_present(i) )
>                  {
>                      cputopo.core = cpu_to_core(i);
> -                    if ( cputopo.core == BAD_APICID )
> +                    if ( cputopo.core == INVALID_COREID )
>                          cputopo.core = XEN_INVALID_CORE_ID;
>                      cputopo.socket = cpu_to_socket(i);
> -                    if ( cputopo.socket == BAD_APICID )
> +                    if ( cputopo.socket == INVALID_SOCKETID )
>                          cputopo.socket = XEN_INVALID_SOCKET_ID;
>                      cputopo.node = cpu_to_node(i);
>                      if ( cputopo.node == NUMA_NO_NODE )
> diff --git a/xen/include/asm-x86/processor.h
> b/xen/include/asm-x86/processor.h
> index 87d80ff..07cee2a 100644
> --- a/xen/include/asm-x86/processor.h
> +++ b/xen/include/asm-x86/processor.h
> @@ -214,8 +214,19 @@ extern void detect_extended_topology(struct cpuinfo_x86
> *c);
>
>  extern void detect_ht(struct cpuinfo_x86 *c);
>
> -#define cpu_to_core(_cpu)   (cpu_data[_cpu].cpu_core_id)
> -#define cpu_to_socket(_cpu) (cpu_data[_cpu].phys_proc_id)
> +inline int cpu_to_core(unsigned cpu)
> +{
> +    if ( cpu_data[cpu].cpu_core_id == BAD_APICID )
> +        return INVALID_COREID;
> +    return cpu_data[cpu].cpu_core_id;
> +}
> +
> +inline int cpu_to_socket(unsigned cpu)
> +{
> +    if ( cpu_data[cpu].phys_proc_id == BAD_APICID )
> +        return INVALID_SOCKETID;
> +    return cpu_data[cpu].phys_proc_id;
> +}
>
>  unsigned int apicid_to_socket(unsigned int);
>
> diff --git a/xen/include/xen/smp.h b/xen/include/xen/smp.h
> index 6febb56..0b4c660 100644
> --- a/xen/include/xen/smp.h
> +++ b/xen/include/xen/smp.h
> @@ -3,6 +3,9 @@
>
>  #include <asm/smp.h>
>
> +#define INVALID_COREID   -1
> +#define INVALID_SOCKETID -1
> +
>  /*
>   * stops all CPUs but the current one:
>   */
>
>
>
>> It would be nice to at least build test it ARM/ARM64 before pushing
>> patches which modify common code.
>
>
> I unfortunately have no ability to build on ARM (but I should have realized
> that APICID has no meaning there).

Does gcc have a tolerable cross-compiler for ARM?  Particularly just
for building the hypervisor.  The resulting code doesn't need to be
that great, it just needs to catch this sort of thing.

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel