new vcpu op call to get physical CPU information

PATCH In review Mark Langsdorf 3 participants 3 messages Mar 4, 2008 – Mar 5, 2008 Xen hypervisor interface

Share & tools

Mark Langsdorf

author

2008-03-04 21:55 UTC #0 lore raw

PATCH 2/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).

Make dom0 call a new vcpu op hypercall that returns architecture dependent information about the physical cpu if and only iff the vcpus are pinned. In this particular case, the information is the underlying ACPIID and APICIDs for the physical processor. Only the APICIDs are used in this case as the ACPIIDs are set up correctly by the ACPI subsystem already.

The decision logic (dom0_vcpus_pinned) is currently hard-coded but should be passed from the hypervisor. Keir wrote that he would take care of that when he suggested this solution.

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

drivers/xen/core/smpboot.c +30 -7

diff -r 1cf7ba68d855 drivers/xen/core/smpboot.c
@@ -254,21 +254,35 @@ static void __cpuinit cpu_initialize_con
 	spin_unlock(&ctxt_lock);
 }
 
+int dom0_vcpus_pinned = 1;
 void __init smp_prepare_cpus(unsigned int max_cpus)
 {
 	unsigned int cpu;
 	struct task_struct *idle;
+	int apicid;
 #ifdef __x86_64__
 	struct desc_ptr *gdt_descr;
 #else
 	struct Xgt_desc_struct *gdt_descr;
 #endif
 
-	boot_cpu_data.apicid = 0;
+	if (dom0_vcpus_pinned) {
+		vcpu_get_physid_t cpu_data;
+		if (HYPERVISOR_vcpu_op(VCPUOP_get_physid, 0, &cpu_data)) {
+			/* failed to get valid data from hypervisor */
+			printk(KERN_ERR "failed to get valid APIC ID information\n");
+			dom0_vcpus_pinned = 0;
+			apicid = 0;
+		}
+		else
+			apicid = GET_APICID_FROM_PHYSID(cpu_data.phys_id);
+	} else
+		apicid = 0;
+	boot_cpu_data.apicid = apicid;
 	cpu_data[0] = boot_cpu_data;
 
-	cpu_2_logical_apicid[0] = 0;
-	x86_cpu_to_apicid[0] = 0;
+	cpu_2_logical_apicid[0] = apicid;
+	x86_cpu_to_apicid[0] = apicid;
 
 	current_thread_info()->cpu = 0;

@@ -312,11 +326,20 @@ void __init smp_prepare_cpus(unsigned in
 			(void *)gdt_descr->address,
 			XENFEAT_writable_descriptor_tables);
 
+		if (dom0_vcpus_pinned) {
+			vcpu_get_physid_t cpu_data;
+			if (HYPERVISOR_vcpu_op(VCPUOP_get_physid, cpu, &cpu_data)) {
+				dom0_vcpus_pinned = 0;
+				apicid = cpu;
+			} else
+				apicid = GET_APICID_FROM_PHYSID(cpu_data.phys_id); 
+		} else
+			apicid = cpu;
 		cpu_data[cpu] = boot_cpu_data;
-		cpu_data[cpu].apicid = cpu;
-
-		cpu_2_logical_apicid[cpu] = cpu;
-		x86_cpu_to_apicid[cpu] = cpu;
+		cpu_data[cpu].apicid = apicid;
+
+		cpu_2_logical_apicid[cpu] = apicid;
+		x86_cpu_to_apicid[cpu] = apicid;
 
 		idle = fork_idle(cpu);
 		if (IS_ERR(idle))

include/asm-x86_64/mach-xen/asm/smp.h +6 -0

diff -r 1cf7ba68d855 include/asm-x86_64/mach-xen/asm/smp.h
@@ -146,5 +146,11 @@ static __inline int logical_smp_processo
 #define cpu_physical_id(cpu)		boot_cpu_id
 #endif
 
+#define APICMASK			0xff
+#define APICSHIFT			0
+#define ACPIMASK			0xff00
+#define ACPISHIFT			8
+#define GET_APICID_FROM_PHYSID(physid)	(physid & APICMASK) >> APICSHIFT;
+#define GET_ACPIID_FROM_PHYSID(physid)	(physid & ACPIMASK) >> ACPISHIFT;
 #endif

include/xen/interface/vcpu.h +11 -0

diff -r 1cf7ba68d855 include/xen/interface/vcpu.h
@@ -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__ */
 
 /*

Jan Beulich

2008-03-05 08:18 UTC #1 lore raw

Re: [PATCH][retry 3][2/2] new vcpu op call to get physicalCPU information

Given that you modify a common (32- and 64-bit) file, you also need to touch asm-i386/mach-xen/asm/smp.h. But perhaps that logic would be better placed in a Xen interface file anyway?

Jan

Quoted 1 line

Quoted 1 line
Quoted 1 line
Mark Langsdorf <[email protected]> 04.03.08 22:55 >>>

The decision logic (dom0_vcpus_pinned) is currently hard-coded but should be passed from the hypervisor. Keir wrote that he would take care of that when he suggested this solution.

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

drivers/xen/core/smpboot.c +30 -7

diff -r 1cf7ba68d855 drivers/xen/core/smpboot.c
@@ -254,21 +254,35 @@ static void __cpuinit cpu_initialize_con
 	spin_unlock(&ctxt_lock);
 }
 
+int dom0_vcpus_pinned = 1;
 void __init smp_prepare_cpus(unsigned int max_cpus)
 {
 	unsigned int cpu;
 	struct task_struct *idle;
+	int apicid;
 #ifdef __x86_64__
 	struct desc_ptr *gdt_descr;
 #else
 	struct Xgt_desc_struct *gdt_descr;
 #endif
 
-	boot_cpu_data.apicid = 0;
+	if (dom0_vcpus_pinned) {
+		vcpu_get_physid_t cpu_data;
+		if (HYPERVISOR_vcpu_op(VCPUOP_get_physid, 0, &cpu_data)) {
+			/* failed to get valid data from hypervisor */
+			printk(KERN_ERR "failed to get valid APIC ID information\n");
+			dom0_vcpus_pinned = 0;
+			apicid = 0;
+		}
+		else
+			apicid = GET_APICID_FROM_PHYSID(cpu_data.phys_id);
+	} else
+		apicid = 0;
+	boot_cpu_data.apicid = apicid;
 	cpu_data[0] = boot_cpu_data;
 
-	cpu_2_logical_apicid[0] = 0;
-	x86_cpu_to_apicid[0] = 0;
+	cpu_2_logical_apicid[0] = apicid;
+	x86_cpu_to_apicid[0] = apicid;
 
 	current_thread_info()->cpu = 0;

@@ -312,11 +326,20 @@ void __init smp_prepare_cpus(unsigned in
 			(void *)gdt_descr->address,
 			XENFEAT_writable_descriptor_tables);
 
+		if (dom0_vcpus_pinned) {
+			vcpu_get_physid_t cpu_data;
+			if (HYPERVISOR_vcpu_op(VCPUOP_get_physid, cpu, &cpu_data)) {
+				dom0_vcpus_pinned = 0;
+				apicid = cpu;
+			} else
+				apicid = GET_APICID_FROM_PHYSID(cpu_data.phys_id); 
+		} else
+			apicid = cpu;
 		cpu_data[cpu] = boot_cpu_data;
-		cpu_data[cpu].apicid = cpu;
-
-		cpu_2_logical_apicid[cpu] = cpu;
-		x86_cpu_to_apicid[cpu] = cpu;
+		cpu_data[cpu].apicid = apicid;
+
+		cpu_2_logical_apicid[cpu] = apicid;
+		x86_cpu_to_apicid[cpu] = apicid;
 
 		idle = fork_idle(cpu);
 		if (IS_ERR(idle))

include/asm-x86_64/mach-xen/asm/smp.h +6 -0

diff -r 1cf7ba68d855 include/asm-x86_64/mach-xen/asm/smp.h
@@ -146,5 +146,11 @@ static __inline int logical_smp_processo
 #define cpu_physical_id(cpu)		boot_cpu_id
 #endif
 
+#define APICMASK			0xff
+#define APICSHIFT			0
+#define ACPIMASK			0xff00
+#define ACPISHIFT			8
+#define GET_APICID_FROM_PHYSID(physid)	(physid & APICMASK) >> APICSHIFT;
+#define GET_ACPIID_FROM_PHYSID(physid)	(physid & ACPIMASK) >> ACPISHIFT;
 #endif

include/xen/interface/vcpu.h +11 -0

diff -r 1cf7ba68d855 include/xen/interface/vcpu.h
@@ -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__ */
 
 /*

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Keir Fraser

2008-03-05 08:22 UTC #2 lore raw

Re: [PATCH][retry 3][2/2] new vcpu op call to get physicalCPU information

I'll be making a number of tasteful changes when I commit these patches. What's been submitted is close enough for me to run with, I think.

Cheers,
Keir

Quoted 144 lines · Jan Beulich

Given that you modify a common (32- and 64-bit) file, you also need to touch asm-i386/mach-xen/asm/smp.h. But perhaps that logic would be better placed in a Xen interface file anyway?
Jan
Quoted 1 line
Quoted 1 line
Quoted 1 line
Mark Langsdorf <[email protected]> 04.03.08 22:55 >>>
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).
Make dom0 call a new vcpu op hypercall that returns architecture dependent information about the physical cpu if and only iff the vcpus are pinned. In this particular case, the information is the underlying ACPIID and APICIDs for the physical processor. Only the APICIDs are used in this case as the ACPIIDs are set up correctly by the ACPI subsystem already.
The decision logic (dom0_vcpus_pinned) is currently hard-coded but should be passed from the hypervisor. Keir wrote that he would take care of that when he suggested this solution.
I have tested this on my 4p/16 core machine and it works. I would appreciate testing on other boxes.
diff -r 1cf7ba68d855 drivers/xen/core/smpboot.c
drivers/xen/core/smpboot.c Tue Mar 04 15:45:19 2008 -0600 renamed from drivers/xen/core/smpboot.c Mon Mar 03 13:36:57 2008 +0000 +30 -7
@@ -254,21 +254,35 @@ static void __cpuinit cpu_initialize_con
 spin_unlock(&ctxt_lock);
 }
 
+int dom0_vcpus_pinned = 1;
 void __init smp_prepare_cpus(unsigned int max_cpus)
 {
 unsigned int cpu;
 struct task_struct *idle;
+ int apicid;
 #ifdef __x86_64__
 struct desc_ptr *gdt_descr;
 #else
 struct Xgt_desc_struct *gdt_descr;
 #endif
 
- boot_cpu_data.apicid = 0;
+ if (dom0_vcpus_pinned) {
+  vcpu_get_physid_t cpu_data;
+  if (HYPERVISOR_vcpu_op(VCPUOP_get_physid, 0, &cpu_data)) {
+   /* failed to get valid data from hypervisor */
+   printk(KERN_ERR "failed to get valid APIC ID information\n");
+   dom0_vcpus_pinned = 0;
+   apicid = 0;
+  }
+  else
+   apicid = GET_APICID_FROM_PHYSID(cpu_data.phys_id);
+ } else
+  apicid = 0;
+ boot_cpu_data.apicid = apicid;
 cpu_data[0] = boot_cpu_data;
 
- cpu_2_logical_apicid[0] = 0;
- x86_cpu_to_apicid[0] = 0;
+ cpu_2_logical_apicid[0] = apicid;
+ x86_cpu_to_apicid[0] = apicid;
 
 current_thread_info()->cpu = 0;
 
@@ -312,11 +326,20 @@ void __init smp_prepare_cpus(unsigned in
 (void *)gdt_descr->address,
 XENFEAT_writable_descriptor_tables);
 
+  if (dom0_vcpus_pinned) {
+   vcpu_get_physid_t cpu_data;
+   if (HYPERVISOR_vcpu_op(VCPUOP_get_physid, cpu, &cpu_data)) {
+    dom0_vcpus_pinned = 0;
+    apicid = cpu;
+   } else
+    apicid = GET_APICID_FROM_PHYSID(cpu_data.phys_id);
+  } else
+   apicid = cpu;
 cpu_data[cpu] = boot_cpu_data;
-  cpu_data[cpu].apicid = cpu;
-
-  cpu_2_logical_apicid[cpu] = cpu;
-  x86_cpu_to_apicid[cpu] = cpu;
+  cpu_data[cpu].apicid = apicid;
+
+  cpu_2_logical_apicid[cpu] = apicid;
+  x86_cpu_to_apicid[cpu] = apicid;
 
 idle = fork_idle(cpu);
 if (IS_ERR(idle))
diff -r 1cf7ba68d855 include/asm-x86_64/mach-xen/asm/smp.h
include/asm-x86_64/mach-xen/asm/smp.h Tue Mar 04 15:45:19 2008 -0600 renamed from include/asm-x86_64/mach-xen/asm/smp.h Mon Mar 03 13:36:57 2008 +0000 +6 -0
@@ -146,5 +146,11 @@ static __inline int logical_smp_processo
 #define cpu_physical_id(cpu)  boot_cpu_id
 #endif
 
+#define APICMASK   0xff
+#define APICSHIFT   0
+#define ACPIMASK   0xff00
+#define ACPISHIFT   8
+#define GET_APICID_FROM_PHYSID(physid) (physid & APICMASK) >> APICSHIFT;
+#define GET_ACPIID_FROM_PHYSID(physid) (physid & ACPIMASK) >> ACPISHIFT;
 #endif
 
diff -r 1cf7ba68d855 include/xen/interface/vcpu.h
include/xen/interface/vcpu.h Tue Mar 04 15:45:19 2008 -0600 renamed from include/xen/interface/vcpu.h Mon Mar 03 13:36:57 2008 +0000 +11 -0
@@ -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__ */
 
 /*
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel