Xen devel - Mar 2008 - [PATCH][retry 3][1/2] new vcpu op call to get physical CPU information

Some AMD machines have APIC IDs that not equal to CPU IDs.  In
the default Xen configuration, ACPI calls on these machines
can get confused.  This shows up most noticeably when running
AMD PowerNow!.  The only solution is for dom0 to get the
hypervisor''s cpuid to apicid table when needed (ie, when dom0
vcpus are pinned).

Add a vcpu op to Xen to allow dom0 to query the
hypervisor for architecture dependent physical cpu information
if dom0 vcpus are pinned.  

The second patch adds the dom0 call to this vcpu hypercall.

I have tested this on my 4p/16 core machine and it works.  I
would appreciate testing on other boxes.

-Mark Langsdorf
Operating System Research Center
AMD

Signed-off-by: Mark Langsdorf <mark.langsdorf@amd.com>
diff -r 7530c4dba8a5 xen/arch/x86/domain.c
--- a/xen/arch/x86/domain.c	Mon Mar 03 15:19:39 2008 +0000
+++ b/xen/arch/x86/domain.c	Tue Mar 04 15:42:51 2008 -0600
@@ -952,6 +952,30 @@ arch_do_vcpu_op(
         break;
     }
 
+    case VCPUOP_get_physid:
+    {
+        struct vcpu_get_physid cpu_id;
+        int i;
+        int cpu_num = v->vcpu_id;
+	short temp = 0xff;
+
+        if ( !opt_dom0_vcpus_pin)
+            rc = -EINVAL;
+        rc = -EFAULT;
+
+        cpu_id.phys_id = x86_cpu_to_apicid[cpu_num];
+        for (i = 0; i++; i < NR_CPUS) 
+             if (x86_acpiid_to_apicid[i] == x86_cpu_to_apicid[cpu_num])
+                 temp = i;
+	cpu_id.phys_id |= temp << 8;
+
+        if ( copy_to_guest(arg, &cpu_id, 1) )
+            break;
+        rc = 0;
+
+        break;
+    }
+
     default:
         rc = -ENOSYS;
         break;
diff -r 7530c4dba8a5 xen/common/schedule.c
--- a/xen/common/schedule.c	Mon Mar 03 15:19:39 2008 +0000
+++ b/xen/common/schedule.c	Tue Mar 04 15:42:51 2008 -0600
@@ -39,7 +39,8 @@ string_param("sched", opt_sched);
 string_param("sched", opt_sched);
 
 /* opt_dom0_vcpus_pin: If true, dom0 VCPUs are pinned. */
-static unsigned int opt_dom0_vcpus_pin;
+unsigned int opt_dom0_vcpus_pin;
+EXPORT_SYMBOL(opt_dom0_vcpus_pin);
 boolean_param("dom0_vcpus_pin", opt_dom0_vcpus_pin);
 
 enum cpufreq_controller cpufreq_controller;
diff -r 7530c4dba8a5 xen/include/public/vcpu.h
--- a/xen/include/public/vcpu.h	Mon Mar 03 15:19:39 2008 +0000
+++ b/xen/include/public/vcpu.h	Tue Mar 04 15:42:51 2008 -0600
@@ -182,6 +182,17 @@ DEFINE_XEN_GUEST_HANDLE(vcpu_register_vc
 /* Send an NMI to the specified VCPU. @extra_arg == NULL. */
 #define VCPUOP_send_nmi             11
 
+/* 
+ * Get the physical ID information for a pinned vcpu''s underlying
+ * physical processor.  The phyiscal ID informmation is architecture
+ * defined, but would ACPI and APIC IDs on x86 processors.
+ */
+#define VCPUOP_get_physid           12 /* arg == vcpu_get_physid_t */
+struct vcpu_get_physid {
+    uint64_t phys_id;
+};
+typedef struct vcpu_get_physid vcpu_get_physid_t;
+DEFINE_XEN_GUEST_HANDLE(vcpu_get_physid_t);
 #endif /* __XEN_PUBLIC_VCPU_H__ */
 
 /*
diff -r 7530c4dba8a5 xen/include/xen/sched.h
--- a/xen/include/xen/sched.h	Mon Mar 03 15:19:39 2008 +0000
+++ b/xen/include/xen/sched.h	Tue Mar 04 15:42:51 2008 -0600
@@ -29,6 +29,9 @@ extern unsigned long volatile jiffies;
 
 /* A global pointer to the initial domain (DOM0). */
 extern struct domain *dom0;
+
+/* opt_dom0_vcpus_pin: If true, dom0 VCPUs are pinned. */
+extern unsigned int opt_dom0_vcpus_pin;
 
 #ifndef CONFIG_COMPAT
 #define MAX_EVTCHNS(d)     NR_EVENT_CHANNELS


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

>+	short temp = 0xff;
Please use 0xffffffff here for forward compatibility.
>+
>+        if ( !opt_dom0_vcpus_pin)
>+            rc = -EINVAL;
>+        rc = -EFAULT;
This makes no sense; it''s probably missing a ''break''.
>+
>+        cpu_id.phys_id = x86_cpu_to_apicid[cpu_num];
>+        for (i = 0; i++; i < NR_CPUS) 
>+             if (x86_acpiid_to_apicid[i] == x86_cpu_to_apicid[cpu_num])
>+                 temp = i;
>+	cpu_id.phys_id |= temp << 8;
Please shift by 32 here for forward compatibility.
>-static unsigned int opt_dom0_vcpus_pin;
>+unsigned int opt_dom0_vcpus_pin;
>+EXPORT_SYMBOL(opt_dom0_vcpus_pin);
EXPORT_SYMBOL()?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 4/3/08 21:52, "Mark Langsdorf" <mark.langsdorf@amd.com>
wrote:
> +        for (i = 0; i++; i < NR_CPUS)
> +             if (x86_acpiid_to_apicid[i] == x86_cpu_to_apicid[cpu_num])
> +                 temp = i;
A novel ordering for a for-loop header!

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Jan Beulich writes ("Re: [Xen-devel] [PATCH][retry 3][1/2] new vcpu op call
to get physicalCPU information"):
> >+	short temp = 0xff;
...
> >+	cpu_id.phys_id |= temp << 8;
> 
> Please shift by 32 here for forward compatibility.
short promotes to int, probably 32-bit, but shifting by the word size
or more is not permitted.  In any case even if it worked like you
wanted, the result would be 0.  Perhaps you meant to suggest some
changes of types as well ?

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

>>> Ian Jackson <Ian.Jackson@eu.citrix.com> 05.03.08 12:40
>>>
>Jan Beulich writes ("Re: [Xen-devel] [PATCH][retry 3][1/2] new vcpu op
call to get physicalCPU information"):
>> >+	short temp = 0xff;
>...
>> >+	cpu_id.phys_id |= temp << 8;
>> 
>> Please shift by 32 here for forward compatibility.
>
>short promotes to int, probably 32-bit, but shifting by the word size
>or more is not permitted.  In any case even if it worked like you
>wanted, the result would be 0.  Perhaps you meant to suggest some
>changes of types as well ?
Of course.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel