From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Ostrovsky Subject: Re: [PATCH v5 0/8] Display IO topology when PXM data is available (plus some cleanup) Date: Mon, 23 Mar 2015 09:47:16 -0400 Message-ID: <55101964.7020303@oracle.com> References: <1426802044-19444-1-git-send-email-boris.ostrovsky@oracle.com> <55100A26.7080801@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252"; Format="flowed" Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: <55100A26.7080801@linaro.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Julien Grall , andrew.cooper3@citrix.com, jbeulich@suse.com, keir@xen.org, ian.jackson@eu.citrix.com, stefano.stabellini@eu.citrix.com, ian.campbell@citrix.com, wei.liu2@citrix.com, dario.faggioli@citrix.com, elena.ufimtseva@oracle.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org 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 =91do_sysctl=92: > sysctl.c:349:42: error: =91BAD_APICID=92 undeclared (first use in this fu= nction) > if ( cputopo.core =3D=3D 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 =3D cpu_to_core(i); - if ( cputopo.core =3D=3D BAD_APICID ) + if ( cputopo.core =3D=3D INVALID_COREID ) cputopo.core =3D XEN_INVALID_CORE_ID; cputopo.socket =3D cpu_to_socket(i); - if ( cputopo.socket =3D=3D BAD_APICID ) + if ( cputopo.socket =3D=3D INVALID_SOCKETID ) cputopo.socket =3D XEN_INVALID_SOCKET_ID; cputopo.node =3D cpu_to_node(i); if ( cputopo.node =3D=3D 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 =3D=3D 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 =3D=3D 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 +#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). -boris