[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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).


-boris


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.