[PATCH linux v3 0/9] xen: pvhvm: support bootup on secondary vCPUs

[PATCH linux v3 0/9] xen: pvhvm: support bootup on secondary vCPUs

[Vitaly Kuznetsov]

It may happen that Xen's and Linux's ideas of vCPU id diverge. In
particular, when we crash on a secondary vCPU we may want to do kdump
and unlike plain kexec where we do migrate_to_reboot_cpu() we try booting
on the vCPU which crashed. This doesn't work very well for PVHVM guests as
we have a number of hypercalls where we pass vCPU id as a parameter. These
hypercalls either fail or do something unexpected. To solve the issue we
need to have a mapping between Linux's and Xen's vCPU ids.

This series solves the issue for x86 PVHVM guests. PV guests don't (and
probably won't) support kdump so I always assume Xen's vCPU id == Linux's
vCPU id. ARM guests will probably need to get proper mapping once we start
supporting kexec/kdump there.

Changes since v2:
- Use 'uint32_t' for xen_vcpu_id mapping [Julien Grall]
- Rebased to linux-4.7

Changes since v1:
- "x86/acpi: store ACPI ids from MADT for future usage" patch added.
- Use ACPI ids instead of vLAPIC ids/2 [Andrew Cooper, Jan Beulich]
- Introduce xen_vcpu_nr() helper [David Vrabel].
- Modify all callers of HYPERVISOR_vcpu_op() instead of modifying
  HYPERVISOR_vcpu_op() [David Vrabel]

Vitaly Kuznetsov (9):
  x86/xen: update cpuid.h from Xen-4.7
  x86/acpi: store ACPI ids from MADT for future usage
  xen: introduce xen_vcpu_id mapping
  x86/xen: use xen_vcpu_id mapping for HYPERVISOR_vcpu_op
  x86/xen: use xen_vcpu_id mapping when pointing vcpu_info to the
    shared_info page
  xen/events: use xen_vcpu_id mapping in events_base
  xen/events: fifo: use xen_vcpu_id mapping
  xen/evtchn: use xen_vcpu_id mapping
  xen/pvhvm: run xen_vcpu_setup() for the boot CPU

 arch/arm/xen/enlighten.c         | 13 +++++++++++-
 arch/x86/include/asm/cpu.h       |  1 +
 arch/x86/include/asm/smp.h       |  2 ++
 arch/x86/include/asm/xen/cpuid.h |  5 ++++-
 arch/x86/kernel/acpi/boot.c      | 16 ++++++++++----
 arch/x86/kernel/apic/apic.c      |  2 ++
 arch/x86/kernel/setup_percpu.c   |  3 +++
 arch/x86/xen/enlighten.c         | 45 +++++++++++++++++++++++++++++++---------
 arch/x86/xen/irq.c               |  3 ++-
 arch/x86/xen/smp.c               | 18 +++++++++++-----
 arch/x86/xen/time.c              | 18 ++++++++++------
 arch/x86/xen/xen-ops.h           |  1 +
 drivers/xen/events/events_base.c | 13 ++++++------
 drivers/xen/events/events_fifo.c |  2 +-
 drivers/xen/evtchn.c             |  3 ++-
 drivers/xen/time.c               |  2 +-
 include/xen/xen-ops.h            |  6 ++++++
 17 files changed, 116 insertions(+), 37 deletions(-)

-- 
2.7.4

[PATCH linux v3 1/9] x86/xen: update cpuid.h from Xen-4.7

[Vitaly Kuznetsov]

Update cpuid.h header from xen hypervisor tree to get
XEN_HVM_CPUID_VCPU_ID_PRESENT definition.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 arch/x86/include/asm/xen/cpuid.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/xen/cpuid.h b/arch/x86/include/asm/xen/cpuid.h
index 0d809e9..3bdd10d 100644
--- a/arch/x86/include/asm/xen/cpuid.h
+++ b/arch/x86/include/asm/xen/cpuid.h
@@ -76,15 +76,18 @@
 /*
  * Leaf 5 (0x40000x04)
  * HVM-specific features
+ * EAX: Features
+ * EBX: vcpu id (iff EAX has XEN_HVM_CPUID_VCPU_ID_PRESENT flag)
  */
 
-/* EAX Features */
 /* Virtualized APIC registers */
 #define XEN_HVM_CPUID_APIC_ACCESS_VIRT (1u << 0)
 /* Virtualized x2APIC accesses */
 #define XEN_HVM_CPUID_X2APIC_VIRT      (1u << 1)
 /* Memory mapped from other domains has valid IOMMU entries */
 #define XEN_HVM_CPUID_IOMMU_MAPPINGS   (1u << 2)
+/* vcpu id is present in EBX */
+#define XEN_HVM_CPUID_VCPU_ID_PRESENT  (1u << 3)
 
 #define XEN_CPUID_MAX_NUM_LEAVES 4
 
-- 
2.7.4

[PATCH linux v3 2/9] x86/acpi: store ACPI ids from MADT for future usage

[Vitaly Kuznetsov]

Currently we don't save ACPI ids (unlike LAPIC ids which go to
x86_cpu_to_apicid) from MADT and we may need this information later.
Particularly, ACPI ids is the only existent way for a PVHVM Xen guest
to figure out Xen's idea of its vCPUs ids before these CPUs boot and
in some cases these ids diverge from Linux's cpu ids.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 arch/x86/include/asm/cpu.h     |  1 +
 arch/x86/include/asm/smp.h     |  2 ++
 arch/x86/kernel/acpi/boot.c    | 16 ++++++++++++----
 arch/x86/kernel/apic/apic.c    |  2 ++
 arch/x86/kernel/setup_percpu.c |  3 +++
 5 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
index 59d34c5..9b7fa63 100644
--- a/arch/x86/include/asm/cpu.h
+++ b/arch/x86/include/asm/cpu.h
@@ -16,6 +16,7 @@ extern void prefill_possible_map(void);
 static inline void prefill_possible_map(void) {}
 
 #define cpu_physical_id(cpu)			boot_cpu_physical_apicid
+#define cpu_acpi_id(cpu)			0
 #define safe_smp_processor_id()			0
 
 #endif /* CONFIG_SMP */
diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
index 0576b61..83463c0 100644
--- a/arch/x86/include/asm/smp.h
+++ b/arch/x86/include/asm/smp.h
@@ -33,6 +33,7 @@ static inline struct cpumask *cpu_llc_shared_mask(int cpu)
 }
 
 DECLARE_EARLY_PER_CPU_READ_MOSTLY(u16, x86_cpu_to_apicid);
+DECLARE_EARLY_PER_CPU_READ_MOSTLY(u32, x86_cpu_to_acpiid);
 DECLARE_EARLY_PER_CPU_READ_MOSTLY(u16, x86_bios_cpu_apicid);
 #if defined(CONFIG_X86_LOCAL_APIC) && defined(CONFIG_X86_32)
 DECLARE_EARLY_PER_CPU_READ_MOSTLY(int, x86_cpu_to_logical_apicid);
@@ -147,6 +148,7 @@ void x86_idle_thread_init(unsigned int cpu, struct task_struct *idle);
 void smp_store_boot_cpu_info(void);
 void smp_store_cpu_info(int id);
 #define cpu_physical_id(cpu)	per_cpu(x86_cpu_to_apicid, cpu)
+#define cpu_acpi_id(cpu)	per_cpu(x86_cpu_to_acpiid, cpu)
 
 #else /* !CONFIG_SMP */
 #define wbinvd_on_cpu(cpu)     wbinvd()
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 9414f84..6738e5c 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -161,13 +161,15 @@ static int __init acpi_parse_madt(struct acpi_table_header *table)
 /**
  * acpi_register_lapic - register a local apic and generates a logic cpu number
  * @id: local apic id to register
+ * @acpiid: ACPI id to register
  * @enabled: this cpu is enabled or not
  *
  * Returns the logic cpu number which maps to the local apic
  */
-static int acpi_register_lapic(int id, u8 enabled)
+static int acpi_register_lapic(int id, u32 acpiid, u8 enabled)
 {
 	unsigned int ver = 0;
+	int cpu;
 
 	if (id >= MAX_LOCAL_APIC) {
 		printk(KERN_INFO PREFIX "skipped apicid that is too big\n");
@@ -182,7 +184,11 @@ static int acpi_register_lapic(int id, u8 enabled)
 	if (boot_cpu_physical_apicid != -1U)
 		ver = apic_version[boot_cpu_physical_apicid];
 
-	return generic_processor_info(id, ver);
+	cpu = generic_processor_info(id, ver);
+	if (cpu >= 0)
+		early_per_cpu(x86_cpu_to_acpiid, cpu) = acpiid;
+
+	return cpu;
 }
 
 static int __init
@@ -212,7 +218,7 @@ acpi_parse_x2apic(struct acpi_subtable_header *header, const unsigned long end)
 	if (!apic->apic_id_valid(apic_id) && enabled)
 		printk(KERN_WARNING PREFIX "x2apic entry ignored\n");
 	else
-		acpi_register_lapic(apic_id, enabled);
+		acpi_register_lapic(apic_id, processor->uid, enabled);
 #else
 	printk(KERN_WARNING PREFIX "x2apic entry ignored\n");
 #endif
@@ -240,6 +246,7 @@ acpi_parse_lapic(struct acpi_subtable_header * header, const unsigned long end)
 	 * when we use CPU hotplug.
 	 */
 	acpi_register_lapic(processor->id,	/* APIC ID */
+			    processor->processor_id, /* ACPI ID */
 			    processor->lapic_flags & ACPI_MADT_ENABLED);
 
 	return 0;
@@ -258,6 +265,7 @@ acpi_parse_sapic(struct acpi_subtable_header *header, const unsigned long end)
 	acpi_table_print_madt_entry(header);
 
 	acpi_register_lapic((processor->id << 8) | processor->eid,/* APIC ID */
+			    processor->processor_id, /* ACPI ID */
 			    processor->lapic_flags & ACPI_MADT_ENABLED);
 
 	return 0;
@@ -714,7 +722,7 @@ int acpi_map_cpu(acpi_handle handle, phys_cpuid_t physid, int *pcpu)
 {
 	int cpu;
 
-	cpu = acpi_register_lapic(physid, ACPI_MADT_ENABLED);
+	cpu = acpi_register_lapic(physid, U32_MAX, ACPI_MADT_ENABLED);
 	if (cpu < 0) {
 		pr_info(PREFIX "Unable to map lapic to logical cpu number\n");
 		return cpu;
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index f943d2f..ac8d8ad 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -92,8 +92,10 @@ static int apic_extnmi = APIC_EXTNMI_BSP;
  */
 DEFINE_EARLY_PER_CPU_READ_MOSTLY(u16, x86_cpu_to_apicid, BAD_APICID);
 DEFINE_EARLY_PER_CPU_READ_MOSTLY(u16, x86_bios_cpu_apicid, BAD_APICID);
+DEFINE_EARLY_PER_CPU_READ_MOSTLY(u32, x86_cpu_to_acpiid, U32_MAX);
 EXPORT_EARLY_PER_CPU_SYMBOL(x86_cpu_to_apicid);
 EXPORT_EARLY_PER_CPU_SYMBOL(x86_bios_cpu_apicid);
+EXPORT_EARLY_PER_CPU_SYMBOL(x86_cpu_to_acpiid);
 
 #ifdef CONFIG_X86_32
 
diff --git a/arch/x86/kernel/setup_percpu.c b/arch/x86/kernel/setup_percpu.c
index e4fcb87..7a40e06 100644
--- a/arch/x86/kernel/setup_percpu.c
+++ b/arch/x86/kernel/setup_percpu.c
@@ -236,6 +236,8 @@ void __init setup_per_cpu_areas(void)
 			early_per_cpu_map(x86_cpu_to_apicid, cpu);
 		per_cpu(x86_bios_cpu_apicid, cpu) =
 			early_per_cpu_map(x86_bios_cpu_apicid, cpu);
+		per_cpu(x86_cpu_to_acpiid, cpu) =
+			early_per_cpu_map(x86_cpu_to_acpiid, cpu);
 #endif
 #ifdef CONFIG_X86_32
 		per_cpu(x86_cpu_to_logical_apicid, cpu) =
@@ -271,6 +273,7 @@ void __init setup_per_cpu_areas(void)
 #ifdef CONFIG_X86_LOCAL_APIC
 	early_per_cpu_ptr(x86_cpu_to_apicid) = NULL;
 	early_per_cpu_ptr(x86_bios_cpu_apicid) = NULL;
+	early_per_cpu_ptr(x86_cpu_to_acpiid) = NULL;
 #endif
 #ifdef CONFIG_X86_32
 	early_per_cpu_ptr(x86_cpu_to_logical_apicid) = NULL;
-- 
2.7.4

[PATCH linux v3 3/9] xen: introduce xen_vcpu_id mapping

[Vitaly Kuznetsov]

It may happen that Xen's and Linux's ideas of vCPU id diverge. In
particular, when we crash on a secondary vCPU we may want to do kdump
and unlike plain kexec where we do migrate_to_reboot_cpu() we try booting
on the vCPU which crashed. This doesn't work very well for PVHVM guests as
we have a number of hypercalls where we pass vCPU id as a parameter. These
hypercalls either fail or do something unexpected. To solve the issue
introduce percpu xen_vcpu_id mapping. ARM and PV guests get direct mapping
for now. Boot CPU for PVHVM guest gets its id from CPUID. With secondary
CPUs it is a bit more trickier. Currently, we initialize IPI vectors
before these CPUs boot so we can't use CPUID. Use ACPI ids from MADT
instead.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
Changes since v2:
- Use uint32_t for xen_vcpu_id mapping [Julien Grall]

Changes since v1:
- Introduce xen_vcpu_nr() helper [David Vrabel]
- Use ACPI ids instead of vLAPIC ids /2 [Andrew Cooper, Jan Beulich]
---
 arch/arm/xen/enlighten.c | 10 ++++++++++
 arch/x86/xen/enlighten.c | 23 ++++++++++++++++++++++-
 include/xen/xen-ops.h    |  6 ++++++
 3 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index 75cd734..fe32267 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -46,6 +46,10 @@ struct shared_info *HYPERVISOR_shared_info = (void *)&xen_dummy_shared_info;
 DEFINE_PER_CPU(struct vcpu_info *, xen_vcpu);
 static struct vcpu_info __percpu *xen_vcpu_info;
 
+/* Linux <-> Xen vCPU id mapping */
+DEFINE_PER_CPU(uint32_t, xen_vcpu_id) = U32_MAX;
+EXPORT_PER_CPU_SYMBOL(xen_vcpu_id);
+
 /* These are unused until we support booting "pre-ballooned" */
 unsigned long xen_released_pages;
 struct xen_memory_region xen_extra_mem[XEN_EXTRA_MEM_MAX_REGIONS] __initdata;
@@ -179,6 +183,9 @@ static void xen_percpu_init(void)
 	pr_info("Xen: initializing cpu%d\n", cpu);
 	vcpup = per_cpu_ptr(xen_vcpu_info, cpu);
 
+	/* Direct vCPU id mapping for ARM guests. */
+	per_cpu(xen_vcpu_id, cpu) = cpu;
+
 	info.mfn = virt_to_gfn(vcpup);
 	info.offset = xen_offset_in_page(vcpup);
 
@@ -328,6 +335,9 @@ static int __init xen_guest_init(void)
 	if (xen_vcpu_info == NULL)
 		return -ENOMEM;
 
+	/* Direct vCPU id mapping for ARM guests. */
+	per_cpu(xen_vcpu_id, 0) = 0;
+
 	if (gnttab_setup_auto_xlat_frames(grant_frames)) {
 		free_percpu(xen_vcpu_info);
 		return -ENOMEM;
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 0f87db2..c833912 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -59,6 +59,7 @@
 #include <asm/xen/pci.h>
 #include <asm/xen/hypercall.h>
 #include <asm/xen/hypervisor.h>
+#include <asm/xen/cpuid.h>
 #include <asm/fixmap.h>
 #include <asm/processor.h>
 #include <asm/proto.h>
@@ -118,6 +119,10 @@ DEFINE_PER_CPU(struct vcpu_info *, xen_vcpu);
  */
 DEFINE_PER_CPU(struct vcpu_info, xen_vcpu_info);
 
+/* Linux <-> Xen vCPU id mapping */
+DEFINE_PER_CPU(uint32_t, xen_vcpu_id) = U32_MAX;
+EXPORT_PER_CPU_SYMBOL(xen_vcpu_id);
+
 enum xen_domain_type xen_domain_type = XEN_NATIVE;
 EXPORT_SYMBOL_GPL(xen_domain_type);
 
@@ -1135,8 +1140,11 @@ void xen_setup_vcpu_info_placement(void)
 {
 	int cpu;
 
-	for_each_possible_cpu(cpu)
+	for_each_possible_cpu(cpu) {
+		/* Set up direct vCPU id mapping for PV guests. */
+		per_cpu(xen_vcpu_id, cpu) = cpu;
 		xen_vcpu_setup(cpu);
+	}
 
 	/* xen_vcpu_setup managed to place the vcpu_info within the
 	 * percpu area for all cpus, so make use of it. Note that for
@@ -1727,6 +1735,9 @@ asmlinkage __visible void __init xen_start_kernel(void)
 #endif
 	xen_raw_console_write("about to get started...\n");
 
+	/* Let's presume PV guests always boot on vCPU with id 0. */
+	per_cpu(xen_vcpu_id, 0) = 0;
+
 	xen_setup_runstate_info(0);
 
 	xen_efi_init();
@@ -1795,6 +1806,12 @@ static void __init init_hvm_pv_info(void)
 
 	xen_setup_features();
 
+	cpuid(base + 4, &eax, &ebx, &ecx, &edx);
+	if (eax & XEN_HVM_CPUID_VCPU_ID_PRESENT)
+		this_cpu_write(xen_vcpu_id, ebx);
+	else
+		this_cpu_write(xen_vcpu_id, smp_processor_id());
+
 	pv_info.name = "Xen HVM";
 
 	xen_domain_type = XEN_HVM_DOMAIN;
@@ -1806,6 +1823,10 @@ static int xen_hvm_cpu_notify(struct notifier_block *self, unsigned long action,
 	int cpu = (long)hcpu;
 	switch (action) {
 	case CPU_UP_PREPARE:
+		if (cpu_acpi_id(cpu) != U32_MAX)
+			per_cpu(xen_vcpu_id, cpu) = cpu_acpi_id(cpu);
+		else
+			per_cpu(xen_vcpu_id, cpu) = cpu;
 		xen_vcpu_setup(cpu);
 		if (xen_have_vector_callback) {
 			if (xen_feature(XENFEAT_hvm_safe_pvclock))
diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h
index 86abe07..648ce814 100644
--- a/include/xen/xen-ops.h
+++ b/include/xen/xen-ops.h
@@ -9,6 +9,12 @@
 
 DECLARE_PER_CPU(struct vcpu_info *, xen_vcpu);
 
+DECLARE_PER_CPU(uint32_t, xen_vcpu_id);
+static inline int xen_vcpu_nr(int cpu)
+{
+	return per_cpu(xen_vcpu_id, cpu);
+}
+
 void xen_arch_pre_suspend(void);
 void xen_arch_post_suspend(int suspend_cancelled);
 
-- 
2.7.4

[PATCH linux v3 4/9] x86/xen: use xen_vcpu_id mapping for HYPERVISOR_vcpu_op

[Vitaly Kuznetsov]

HYPERVISOR_vcpu_op() passes Linux's idea of vCPU id as a parameter while
Xen's idea is expected. In some cases these ideas diverge so we need to
do remapping. Convert all callers of HYPERVISOR_vcpu_op() to use
xen_vcpu_nr(). Leave xen_fill_possible_map() and xen_filter_cpu_maps()
intact as they're only being called by PV guests before perpu areas are
initialized. While the issue could be solved by switching to early_percpu
for xen_vcpu_id I think it's not worth it: PV guests will probably never
get to the point where their idea of vCPU id diverges from Xen's.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
Changes since v1:
- Modify all callers of HYPERVISOR_vcpu_op() instead of modifying
  HYPERVISOR_vcpu_op() [David Vrabel]
---
 arch/arm/xen/enlighten.c         |  3 ++-
 arch/x86/xen/enlighten.c         | 10 ++++++----
 arch/x86/xen/irq.c               |  3 ++-
 arch/x86/xen/smp.c               | 11 ++++++-----
 arch/x86/xen/time.c              | 18 ++++++++++++------
 drivers/xen/events/events_base.c |  3 ++-
 drivers/xen/time.c               |  2 +-
 7 files changed, 31 insertions(+), 19 deletions(-)

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index fe32267..0456daf 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -189,7 +189,8 @@ static void xen_percpu_init(void)
 	info.mfn = virt_to_gfn(vcpup);
 	info.offset = xen_offset_in_page(vcpup);
 
-	err = HYPERVISOR_vcpu_op(VCPUOP_register_vcpu_info, cpu, &info);
+	err = HYPERVISOR_vcpu_op(VCPUOP_register_vcpu_info, xen_vcpu_nr(cpu),
+				 &info);
 	BUG_ON(err);
 	per_cpu(xen_vcpu, cpu) = vcpup;
 
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index c833912..3b0ceed 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -228,7 +228,8 @@ static void xen_vcpu_setup(int cpu)
 	   hypervisor has no unregister variant and this hypercall does not
 	   allow to over-write info.mfn and info.offset.
 	 */
-	err = HYPERVISOR_vcpu_op(VCPUOP_register_vcpu_info, cpu, &info);
+	err = HYPERVISOR_vcpu_op(VCPUOP_register_vcpu_info, xen_vcpu_nr(cpu),
+				 &info);
 
 	if (err) {
 		printk(KERN_DEBUG "register_vcpu_info failed: err=%d\n", err);
@@ -252,10 +253,11 @@ void xen_vcpu_restore(void)
 
 	for_each_possible_cpu(cpu) {
 		bool other_cpu = (cpu != smp_processor_id());
-		bool is_up = HYPERVISOR_vcpu_op(VCPUOP_is_up, cpu, NULL);
+		bool is_up = HYPERVISOR_vcpu_op(VCPUOP_is_up, xen_vcpu_nr(cpu),
+						NULL);
 
 		if (other_cpu && is_up &&
-		    HYPERVISOR_vcpu_op(VCPUOP_down, cpu, NULL))
+		    HYPERVISOR_vcpu_op(VCPUOP_down, xen_vcpu_nr(cpu), NULL))
 			BUG();
 
 		xen_setup_runstate_info(cpu);
@@ -264,7 +266,7 @@ void xen_vcpu_restore(void)
 			xen_vcpu_setup(cpu);
 
 		if (other_cpu && is_up &&
-		    HYPERVISOR_vcpu_op(VCPUOP_up, cpu, NULL))
+		    HYPERVISOR_vcpu_op(VCPUOP_up, xen_vcpu_nr(cpu), NULL))
 			BUG();
 	}
 }
diff --git a/arch/x86/xen/irq.c b/arch/x86/xen/irq.c
index a1207cb..33e9295 100644
--- a/arch/x86/xen/irq.c
+++ b/arch/x86/xen/irq.c
@@ -109,7 +109,8 @@ static void xen_safe_halt(void)
 static void xen_halt(void)
 {
 	if (irqs_disabled())
-		HYPERVISOR_vcpu_op(VCPUOP_down, smp_processor_id(), NULL);
+		HYPERVISOR_vcpu_op(VCPUOP_down,
+				   xen_vcpu_nr(smp_processor_id()), NULL);
 	else
 		xen_safe_halt();
 }
diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
index 719cf29..a3118f2 100644
--- a/arch/x86/xen/smp.c
+++ b/arch/x86/xen/smp.c
@@ -454,7 +454,7 @@ cpu_initialize_context(unsigned int cpu, struct task_struct *idle)
 #endif
 	ctxt->user_regs.esp = idle->thread.sp0 - sizeof(struct pt_regs);
 	ctxt->ctrlreg[3] = xen_pfn_to_cr3(virt_to_gfn(swapper_pg_dir));
-	if (HYPERVISOR_vcpu_op(VCPUOP_initialise, cpu, ctxt))
+	if (HYPERVISOR_vcpu_op(VCPUOP_initialise, xen_vcpu_nr(cpu), ctxt))
 		BUG();
 
 	kfree(ctxt);
@@ -492,7 +492,7 @@ static int xen_cpu_up(unsigned int cpu, struct task_struct *idle)
 	if (rc)
 		return rc;
 
-	rc = HYPERVISOR_vcpu_op(VCPUOP_up, cpu, NULL);
+	rc = HYPERVISOR_vcpu_op(VCPUOP_up, xen_vcpu_nr(cpu), NULL);
 	BUG_ON(rc);
 
 	while (cpu_report_state(cpu) != CPU_ONLINE)
@@ -520,7 +520,8 @@ static int xen_cpu_disable(void)
 
 static void xen_cpu_die(unsigned int cpu)
 {
-	while (xen_pv_domain() && HYPERVISOR_vcpu_op(VCPUOP_is_up, cpu, NULL)) {
+	while (xen_pv_domain() && HYPERVISOR_vcpu_op(VCPUOP_is_up,
+						     xen_vcpu_nr(cpu), NULL)) {
 		__set_current_state(TASK_UNINTERRUPTIBLE);
 		schedule_timeout(HZ/10);
 	}
@@ -536,7 +537,7 @@ static void xen_cpu_die(unsigned int cpu)
 static void xen_play_dead(void) /* used only with HOTPLUG_CPU */
 {
 	play_dead_common();
-	HYPERVISOR_vcpu_op(VCPUOP_down, smp_processor_id(), NULL);
+	HYPERVISOR_vcpu_op(VCPUOP_down, xen_vcpu_nr(smp_processor_id()), NULL);
 	cpu_bringup();
 	/*
 	 * commit 4b0c0f294 (tick: Cleanup NOHZ per cpu data on cpu down)
@@ -576,7 +577,7 @@ static void stop_self(void *v)
 
 	set_cpu_online(cpu, false);
 
-	HYPERVISOR_vcpu_op(VCPUOP_down, cpu, NULL);
+	HYPERVISOR_vcpu_op(VCPUOP_down, xen_vcpu_nr(cpu), NULL);
 	BUG();
 }
 
diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
index 6deba5b..e89568c 100644
--- a/arch/x86/xen/time.c
+++ b/arch/x86/xen/time.c
@@ -263,8 +263,10 @@ static int xen_vcpuop_shutdown(struct clock_event_device *evt)
 {
 	int cpu = smp_processor_id();
 
-	if (HYPERVISOR_vcpu_op(VCPUOP_stop_singleshot_timer, cpu, NULL) ||
-	    HYPERVISOR_vcpu_op(VCPUOP_stop_periodic_timer, cpu, NULL))
+	if (HYPERVISOR_vcpu_op(VCPUOP_stop_singleshot_timer, xen_vcpu_nr(cpu),
+			       NULL) ||
+	    HYPERVISOR_vcpu_op(VCPUOP_stop_periodic_timer, xen_vcpu_nr(cpu),
+			       NULL))
 		BUG();
 
 	return 0;
@@ -274,7 +276,8 @@ static int xen_vcpuop_set_oneshot(struct clock_event_device *evt)
 {
 	int cpu = smp_processor_id();
 
-	if (HYPERVISOR_vcpu_op(VCPUOP_stop_periodic_timer, cpu, NULL))
+	if (HYPERVISOR_vcpu_op(VCPUOP_stop_periodic_timer, xen_vcpu_nr(cpu),
+			       NULL))
 		BUG();
 
 	return 0;
@@ -293,7 +296,8 @@ static int xen_vcpuop_set_next_event(unsigned long delta,
 	/* Get an event anyway, even if the timeout is already expired */
 	single.flags = 0;
 
-	ret = HYPERVISOR_vcpu_op(VCPUOP_set_singleshot_timer, cpu, &single);
+	ret = HYPERVISOR_vcpu_op(VCPUOP_set_singleshot_timer, xen_vcpu_nr(cpu),
+				 &single);
 	BUG_ON(ret != 0);
 
 	return ret;
@@ -394,7 +398,8 @@ void xen_timer_resume(void)
 		return;
 
 	for_each_online_cpu(cpu) {
-		if (HYPERVISOR_vcpu_op(VCPUOP_stop_periodic_timer, cpu, NULL))
+		if (HYPERVISOR_vcpu_op(VCPUOP_stop_periodic_timer,
+				       xen_vcpu_nr(cpu), NULL))
 			BUG();
 	}
 }
@@ -414,7 +419,8 @@ static void __init xen_time_init(void)
 
 	clocksource_register_hz(&xen_clocksource, NSEC_PER_SEC);
 
-	if (HYPERVISOR_vcpu_op(VCPUOP_stop_periodic_timer, cpu, NULL) == 0) {
+	if (HYPERVISOR_vcpu_op(VCPUOP_stop_periodic_timer, xen_vcpu_nr(cpu),
+			       NULL) == 0) {
 		/* Successfully turned off 100Hz tick, so we have the
 		   vcpuop-based timer interface */
 		printk(KERN_DEBUG "Xen: using vcpuop timer interface\n");
diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
index 71d49a9..8fb7cbf 100644
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -1211,7 +1211,8 @@ void xen_send_IPI_one(unsigned int cpu, enum ipi_vector vector)
 
 #ifdef CONFIG_X86
 	if (unlikely(vector == XEN_NMI_VECTOR)) {
-		int rc =  HYPERVISOR_vcpu_op(VCPUOP_send_nmi, cpu, NULL);
+		int rc =  HYPERVISOR_vcpu_op(VCPUOP_send_nmi, xen_vcpu_nr(cpu),
+					     NULL);
 		if (rc < 0)
 			printk(KERN_WARNING "Sending nmi to CPU%d failed (rc:%d)\n", cpu, rc);
 		return;
diff --git a/drivers/xen/time.c b/drivers/xen/time.c
index 7107842..781a630 100644
--- a/drivers/xen/time.c
+++ b/drivers/xen/time.c
@@ -82,7 +82,7 @@ void xen_setup_runstate_info(int cpu)
 	area.addr.v = &per_cpu(xen_runstate, cpu);
 
 	if (HYPERVISOR_vcpu_op(VCPUOP_register_runstate_memory_area,
-			       cpu, &area))
+			       xen_vcpu_nr(cpu), &area))
 		BUG();
 }
 
-- 
2.7.4

[PATCH linux v3 5/9] x86/xen: use xen_vcpu_id mapping when pointing vcpu_info to the shared_info page

[Vitaly Kuznetsov]

shared_info page has space for 32 vcpu info slots for first 32 vCPUs but
these are the first 32 vCPUs from Xen's perspective and we should map them
accordingly with the newly introduced xen_vcpu_id mapping.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
Changes since v1:
- Use xen_vcpu_nr() helper [David Vrabel]
---
 arch/x86/xen/enlighten.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 3b0ceed..c650677 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -207,8 +207,9 @@ static void xen_vcpu_setup(int cpu)
 		if (per_cpu(xen_vcpu, cpu) == &per_cpu(xen_vcpu_info, cpu))
 			return;
 	}
-	if (cpu < MAX_VIRT_CPUS)
-		per_cpu(xen_vcpu,cpu) = &HYPERVISOR_shared_info->vcpu_info[cpu];
+	if (xen_vcpu_nr(cpu) < MAX_VIRT_CPUS)
+		per_cpu(xen_vcpu, cpu) =
+			&HYPERVISOR_shared_info->vcpu_info[xen_vcpu_nr(cpu)];
 
 	if (!have_vcpu_info_placement) {
 		if (cpu >= MAX_VIRT_CPUS)
@@ -1781,9 +1782,10 @@ void __ref xen_hvm_init_shared_info(void)
 	 * in that case multiple vcpus might be online. */
 	for_each_online_cpu(cpu) {
 		/* Leave it to be NULL. */
-		if (cpu >= MAX_VIRT_CPUS)
+		if (xen_vcpu_nr(cpu) >= MAX_VIRT_CPUS)
 			continue;
-		per_cpu(xen_vcpu, cpu) = &HYPERVISOR_shared_info->vcpu_info[cpu];
+		per_cpu(xen_vcpu, cpu) =
+			&HYPERVISOR_shared_info->vcpu_info[xen_vcpu_nr(cpu)];
 	}
 }
 
-- 
2.7.4

[PATCH linux v3 6/9] xen/events: use xen_vcpu_id mapping in events_base

[Vitaly Kuznetsov]

EVTCHNOP_bind_ipi and EVTCHNOP_bind_virq pass vCPU id as a parameter and
Xen's idea of vCPU id should be used. Use the newly introduced xen_vcpu_id
mapping to convert it from Linux's id.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
Changes since v1:
- Use xen_vcpu_nr() helper [David Vrabel]
---
 drivers/xen/events/events_base.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
index 8fb7cbf..d5dbdb9 100644
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -895,7 +895,7 @@ static int bind_ipi_to_irq(unsigned int ipi, unsigned int cpu)
 		irq_set_chip_and_handler_name(irq, &xen_percpu_chip,
 					      handle_percpu_irq, "ipi");
 
-		bind_ipi.vcpu = cpu;
+		bind_ipi.vcpu = xen_vcpu_nr(cpu);
 		if (HYPERVISOR_event_channel_op(EVTCHNOP_bind_ipi,
 						&bind_ipi) != 0)
 			BUG();
@@ -991,7 +991,7 @@ int bind_virq_to_irq(unsigned int virq, unsigned int cpu, bool percpu)
 						      handle_edge_irq, "virq");
 
 		bind_virq.virq = virq;
-		bind_virq.vcpu = cpu;
+		bind_virq.vcpu = xen_vcpu_nr(cpu);
 		ret = HYPERVISOR_event_channel_op(EVTCHNOP_bind_virq,
 						&bind_virq);
 		if (ret == 0)
@@ -1319,7 +1319,7 @@ static int rebind_irq_to_cpu(unsigned irq, unsigned tcpu)
 
 	/* Send future instances of this interrupt to other vcpu. */
 	bind_vcpu.port = evtchn;
-	bind_vcpu.vcpu = tcpu;
+	bind_vcpu.vcpu = xen_vcpu_nr(tcpu);
 
 	/*
 	 * Mask the event while changing the VCPU binding to prevent
@@ -1459,7 +1459,7 @@ static void restore_cpu_virqs(unsigned int cpu)
 
 		/* Get a new binding from Xen. */
 		bind_virq.virq = virq;
-		bind_virq.vcpu = cpu;
+		bind_virq.vcpu = xen_vcpu_nr(cpu);
 		if (HYPERVISOR_event_channel_op(EVTCHNOP_bind_virq,
 						&bind_virq) != 0)
 			BUG();
@@ -1483,7 +1483,7 @@ static void restore_cpu_ipis(unsigned int cpu)
 		BUG_ON(ipi_from_irq(irq) != ipi);
 
 		/* Get a new binding from Xen. */
-		bind_ipi.vcpu = cpu;
+		bind_ipi.vcpu = xen_vcpu_nr(cpu);
 		if (HYPERVISOR_event_channel_op(EVTCHNOP_bind_ipi,
 						&bind_ipi) != 0)
 			BUG();
-- 
2.7.4

[PATCH linux v3 7/9] xen/events: fifo: use xen_vcpu_id mapping

[Vitaly Kuznetsov]

EVTCHNOP_init_control has vCPU id as a parameter and Xen's idea of vCPU id
should be used. Use the newly introduced xen_vcpu_id mapping to convert
it from Linux's id.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
Changes since v1:
- Use xen_vcpu_nr() helper [David Vrabel]
---
 drivers/xen/events/events_fifo.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/xen/events/events_fifo.c b/drivers/xen/events/events_fifo.c
index 9289a17..266c2c7 100644
--- a/drivers/xen/events/events_fifo.c
+++ b/drivers/xen/events/events_fifo.c
@@ -113,7 +113,7 @@ static int init_control_block(int cpu,
 
 	init_control.control_gfn = virt_to_gfn(control_block);
 	init_control.offset      = 0;
-	init_control.vcpu        = cpu;
+	init_control.vcpu        = xen_vcpu_nr(cpu);
 
 	return HYPERVISOR_event_channel_op(EVTCHNOP_init_control, &init_control);
 }
-- 
2.7.4

[PATCH linux v3 8/9] xen/evtchn: use xen_vcpu_id mapping

[Vitaly Kuznetsov]

Use the newly introduced xen_vcpu_id mapping to get Xen's idea of vCPU id
for CPU0.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
Changes since v1:
- Use xen_vcpu_nr() helper [David Vrabel]
---
 drivers/xen/evtchn.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/xen/evtchn.c b/drivers/xen/evtchn.c
index f4edd6df..7c3113e 100644
--- a/drivers/xen/evtchn.c
+++ b/drivers/xen/evtchn.c
@@ -55,6 +55,7 @@
 #include <xen/xen.h>
 #include <xen/events.h>
 #include <xen/evtchn.h>
+#include <xen/xen-ops.h>
 #include <asm/xen/hypervisor.h>
 
 struct per_user_data {
@@ -448,7 +449,7 @@ static long evtchn_ioctl(struct file *file,
 			break;
 
 		bind_virq.virq = bind.virq;
-		bind_virq.vcpu = 0;
+		bind_virq.vcpu = xen_vcpu_nr(0);
 		rc = HYPERVISOR_event_channel_op(EVTCHNOP_bind_virq,
 						 &bind_virq);
 		if (rc != 0)
-- 
2.7.4

[PATCH linux v3 9/9] xen/pvhvm: run xen_vcpu_setup() for the boot CPU

[Vitaly Kuznetsov]

Historically we didn't call VCPUOP_register_vcpu_info for CPU0 for PVHVM
guests (while we had it for PV and ARM guests). This is usually fine as we
can use vcpu info in the sared_info page but when we try booting on a vCPU
with Xen's vCPU id > 31 (e.g. when we try to kdump after crashing on this
CPU) we're not able to boot. Switch to always doing
VCPUOP_register_vcpu_info for the boot CPU.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 arch/x86/xen/enlighten.c | 2 +-
 arch/x86/xen/smp.c       | 7 +++++++
 arch/x86/xen/xen-ops.h   | 1 +
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index c650677..9cc454c 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -184,7 +184,7 @@ static void clamp_max_cpus(void)
 #endif
 }
 
-static void xen_vcpu_setup(int cpu)
+void xen_vcpu_setup(int cpu)
 {
 	struct vcpu_register_vcpu_info info;
 	int err;
diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
index a3118f2..0b4d04c 100644
--- a/arch/x86/xen/smp.c
+++ b/arch/x86/xen/smp.c
@@ -322,6 +322,13 @@ static void __init xen_smp_prepare_boot_cpu(void)
 		xen_filter_cpu_maps();
 		xen_setup_vcpu_info_placement();
 	}
+
+	/*
+	 * Setup vcpu_info for boot CPU.
+	 */
+	if (xen_hvm_domain())
+		xen_vcpu_setup(0);
+
 	/*
 	 * The alternative logic (which patches the unlock/lock) runs before
 	 * the smp bootup up code is activated. Hence we need to set this up
diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h
index 4140b07..3cbce3b 100644
--- a/arch/x86/xen/xen-ops.h
+++ b/arch/x86/xen/xen-ops.h
@@ -76,6 +76,7 @@ irqreturn_t xen_debug_interrupt(int irq, void *dev_id);
 
 bool xen_vcpu_stolen(int vcpu);
 
+void xen_vcpu_setup(int cpu);
 void xen_setup_vcpu_info_placement(void);
 
 #ifdef CONFIG_SMP
-- 
2.7.4

Re: [Xen-devel] [PATCH linux v3 0/9] xen: pvhvm: support bootup on secondary vCPUs

[David Vrabel]

On 26/07/16 13:30, Vitaly Kuznetsov wrote:
> It may happen that Xen's and Linux's ideas of vCPU id diverge. In
> particular, when we crash on a secondary vCPU we may want to do kdump
> and unlike plain kexec where we do migrate_to_reboot_cpu() we try booting
> on the vCPU which crashed. This doesn't work very well for PVHVM guests as
> we have a number of hypercalls where we pass vCPU id as a parameter. These
> hypercalls either fail or do something unexpected. To solve the issue we
> need to have a mapping between Linux's and Xen's vCPU ids.
> 
> This series solves the issue for x86 PVHVM guests. PV guests don't (and
> probably won't) support kdump so I always assume Xen's vCPU id == Linux's
> vCPU id. ARM guests will probably need to get proper mapping once we start
> supporting kexec/kdump there.
> 
> Changes since v2:
> - Use 'uint32_t' for xen_vcpu_id mapping [Julien Grall]
> - Rebased to linux-4.7

I already applied v2.  If you provide an incremental patch I can queue
it for 4.9.

David

Re: [Xen-devel] [PATCH linux v3 0/9] xen: pvhvm: support bootup on secondary vCPUs

[Vitaly Kuznetsov]

David Vrabel <david.vrabel@citrix.com> writes:

> On 26/07/16 13:30, Vitaly Kuznetsov wrote:
>> It may happen that Xen's and Linux's ideas of vCPU id diverge. In
>> particular, when we crash on a secondary vCPU we may want to do kdump
>> and unlike plain kexec where we do migrate_to_reboot_cpu() we try booting
>> on the vCPU which crashed. This doesn't work very well for PVHVM guests as
>> we have a number of hypercalls where we pass vCPU id as a parameter. These
>> hypercalls either fail or do something unexpected. To solve the issue we
>> need to have a mapping between Linux's and Xen's vCPU ids.
>> 
>> This series solves the issue for x86 PVHVM guests. PV guests don't (and
>> probably won't) support kdump so I always assume Xen's vCPU id == Linux's
>> vCPU id. ARM guests will probably need to get proper mapping once we start
>> supporting kexec/kdump there.
>> 
>> Changes since v2:
>> - Use 'uint32_t' for xen_vcpu_id mapping [Julien Grall]
>> - Rebased to linux-4.7
>
> I already applied v2.  If you provide an incremental patch I can queue
> it for 4.9.

Ok,

in that case we can wait till the real mapping is done for ARM to change
the type of xen_vcpu_id I guess.

-- 
  Vitaly

Re: [Xen-devel] [PATCH linux v3 0/9] xen: pvhvm: support bootup on secondary vCPUs

[Stefano Stabellini]

On Tue, 26 Jul 2016, Vitaly Kuznetsov wrote:
> David Vrabel <david.vrabel@citrix.com> writes:
> 
> > On 26/07/16 13:30, Vitaly Kuznetsov wrote:
> >> It may happen that Xen's and Linux's ideas of vCPU id diverge. In
> >> particular, when we crash on a secondary vCPU we may want to do kdump
> >> and unlike plain kexec where we do migrate_to_reboot_cpu() we try booting
> >> on the vCPU which crashed. This doesn't work very well for PVHVM guests as
> >> we have a number of hypercalls where we pass vCPU id as a parameter. These
> >> hypercalls either fail or do something unexpected. To solve the issue we
> >> need to have a mapping between Linux's and Xen's vCPU ids.
> >> 
> >> This series solves the issue for x86 PVHVM guests. PV guests don't (and
> >> probably won't) support kdump so I always assume Xen's vCPU id == Linux's
> >> vCPU id. ARM guests will probably need to get proper mapping once we start
> >> supporting kexec/kdump there.
> >> 
> >> Changes since v2:
> >> - Use 'uint32_t' for xen_vcpu_id mapping [Julien Grall]
> >> - Rebased to linux-4.7
> >
> > I already applied v2.  If you provide an incremental patch I can queue
> > it for 4.9.
> 
> Ok,
> 
> in that case we can wait till the real mapping is done for ARM to change
> the type of xen_vcpu_id I guess.

It would still be nice to make the change. Please send it as an
incremental patch. It is OK if it is queued for 4.9.

Re: [Xen-devel] [PATCH linux v3 0/9] xen: pvhvm: support bootup on secondary vCPUs

[Vitaly Kuznetsov]

Stefano Stabellini <sstabellini@kernel.org> writes:

> On Tue, 26 Jul 2016, Vitaly Kuznetsov wrote:
>> David Vrabel <david.vrabel@citrix.com> writes:
>> 
>> > On 26/07/16 13:30, Vitaly Kuznetsov wrote:
>> >> It may happen that Xen's and Linux's ideas of vCPU id diverge. In
>> >> particular, when we crash on a secondary vCPU we may want to do kdump
>> >> and unlike plain kexec where we do migrate_to_reboot_cpu() we try booting
>> >> on the vCPU which crashed. This doesn't work very well for PVHVM guests as
>> >> we have a number of hypercalls where we pass vCPU id as a parameter. These
>> >> hypercalls either fail or do something unexpected. To solve the issue we
>> >> need to have a mapping between Linux's and Xen's vCPU ids.
>> >> 
>> >> This series solves the issue for x86 PVHVM guests. PV guests don't (and
>> >> probably won't) support kdump so I always assume Xen's vCPU id == Linux's
>> >> vCPU id. ARM guests will probably need to get proper mapping once we start
>> >> supporting kexec/kdump there.
>> >> 
>> >> Changes since v2:
>> >> - Use 'uint32_t' for xen_vcpu_id mapping [Julien Grall]
>> >> - Rebased to linux-4.7
>> >
>> > I already applied v2.  If you provide an incremental patch I can queue
>> > it for 4.9.
>> 
>> Ok,
>> 
>> in that case we can wait till the real mapping is done for ARM to change
>> the type of xen_vcpu_id I guess.
>
> It would still be nice to make the change. Please send it as an
> incremental patch. It is OK if it is queued for 4.9.

Sure, will do.

-- 
  Vitaly

Re: [PATCH linux v3 3/9] xen: introduce xen_vcpu_id mapping

[Julien Grall]

Hi Vitaly,

On 26/07/16 13:30, Vitaly Kuznetsov wrote:
> It may happen that Xen's and Linux's ideas of vCPU id diverge. In
> particular, when we crash on a secondary vCPU we may want to do kdump
> and unlike plain kexec where we do migrate_to_reboot_cpu() we try booting
> on the vCPU which crashed. This doesn't work very well for PVHVM guests as
> we have a number of hypercalls where we pass vCPU id as a parameter. These
> hypercalls either fail or do something unexpected. To solve the issue
> introduce percpu xen_vcpu_id mapping. ARM and PV guests get direct mapping
> for now. Boot CPU for PVHVM guest gets its id from CPUID. With secondary
> CPUs it is a bit more trickier. Currently, we initialize IPI vectors
> before these CPUs boot so we can't use CPUID. Use ACPI ids from MADT
> instead.
>
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
> Changes since v2:
> - Use uint32_t for xen_vcpu_id mapping [Julien Grall]
>
> Changes since v1:
> - Introduce xen_vcpu_nr() helper [David Vrabel]
> - Use ACPI ids instead of vLAPIC ids /2 [Andrew Cooper, Jan Beulich]
> ---
>  arch/arm/xen/enlighten.c | 10 ++++++++++
>  arch/x86/xen/enlighten.c | 23 ++++++++++++++++++++++-
>  include/xen/xen-ops.h    |  6 ++++++
>  3 files changed, 38 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> index 75cd734..fe32267 100644
> --- a/arch/arm/xen/enlighten.c
> +++ b/arch/arm/xen/enlighten.c
> @@ -46,6 +46,10 @@ struct shared_info *HYPERVISOR_shared_info = (void *)&xen_dummy_shared_info;
>  DEFINE_PER_CPU(struct vcpu_info *, xen_vcpu);
>  static struct vcpu_info __percpu *xen_vcpu_info;
>
> +/* Linux <-> Xen vCPU id mapping */
> +DEFINE_PER_CPU(uint32_t, xen_vcpu_id) = U32_MAX;
> +EXPORT_PER_CPU_SYMBOL(xen_vcpu_id);
> +
>  /* These are unused until we support booting "pre-ballooned" */
>  unsigned long xen_released_pages;
>  struct xen_memory_region xen_extra_mem[XEN_EXTRA_MEM_MAX_REGIONS] __initdata;
> @@ -179,6 +183,9 @@ static void xen_percpu_init(void)
>  	pr_info("Xen: initializing cpu%d\n", cpu);
>  	vcpup = per_cpu_ptr(xen_vcpu_info, cpu);
>
> +	/* Direct vCPU id mapping for ARM guests. */
> +	per_cpu(xen_vcpu_id, cpu) = cpu;
> +

We did some internal testing on ARM64 with the latest Linux kernel 
(4.8-rc4) and noticed that this patch is breaking SMP support. Sorry for 
noticing the issue that late.

This function is called on the running CPU whilst some code (e.g 
init_control_block in drivers/xen/events/events_fifo.c) is executed 
whilst preparing the CPU on the boot CPU.

So xen_vcpu_nr(cpu) will always return 0 in this case and 
init_control_block will fail to execute.

I am not sure how to fix. I guess we could setup per_cpu(xen_vcpu_id, *) 
in xen_guest_init. Any opinions?

[...]

> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index 0f87db2..c833912 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -1795,6 +1806,12 @@ static void __init init_hvm_pv_info(void)
>
>  	xen_setup_features();
>
> +	cpuid(base + 4, &eax, &ebx, &ecx, &edx);
> +	if (eax & XEN_HVM_CPUID_VCPU_ID_PRESENT)
> +		this_cpu_write(xen_vcpu_id, ebx);
> +	else
> +		this_cpu_write(xen_vcpu_id, smp_processor_id());
> +
>  	pv_info.name = "Xen HVM";
>
>  	xen_domain_type = XEN_HVM_DOMAIN;
> @@ -1806,6 +1823,10 @@ static int xen_hvm_cpu_notify(struct notifier_block *self, unsigned long action,
>  	int cpu = (long)hcpu;
>  	switch (action) {
>  	case CPU_UP_PREPARE:
> +		if (cpu_acpi_id(cpu) != U32_MAX)
> +			per_cpu(xen_vcpu_id, cpu) = cpu_acpi_id(cpu);
> +		else
> +			per_cpu(xen_vcpu_id, cpu) = cpu;

I have not tried myself. But looking at the code, the notifiers 
xen_hvm_cpu_notifier and evtchn_fifo_cpu_notifier have the same 
priority. So what does prevent the code above to be executed after the 
event channel callback?

>  		xen_vcpu_setup(cpu);
>  		if (xen_have_vector_callback) {
>  			if (xen_feature(XENFEAT_hvm_safe_pvclock))
> diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h
> index 86abe07..648ce814 100644
> --- a/include/xen/xen-ops.h
> +++ b/include/xen/xen-ops.h
> @@ -9,6 +9,12 @@
>
>  DECLARE_PER_CPU(struct vcpu_info *, xen_vcpu);
>
> +DECLARE_PER_CPU(uint32_t, xen_vcpu_id);
> +static inline int xen_vcpu_nr(int cpu)
> +{
> +	return per_cpu(xen_vcpu_id, cpu);
> +}
> +
>  void xen_arch_pre_suspend(void);
>  void xen_arch_post_suspend(int suspend_cancelled);

Regards,

-- 
Julien Grall

Re: [PATCH linux v3 3/9] xen: introduce xen_vcpu_id mapping

[Boris Ostrovsky]

On 09/02/2016 11:29 AM, Julien Grall wrote:
>
>> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
>> index 0f87db2..c833912 100644
>> --- a/arch/x86/xen/enlighten.c
>> +++ b/arch/x86/xen/enlighten.c
>> @@ -1795,6 +1806,12 @@ static void __init init_hvm_pv_info(void)
>>
>>      xen_setup_features();
>>
>> +    cpuid(base + 4, &eax, &ebx, &ecx, &edx);
>> +    if (eax & XEN_HVM_CPUID_VCPU_ID_PRESENT)
>> +        this_cpu_write(xen_vcpu_id, ebx);
>> +    else
>> +        this_cpu_write(xen_vcpu_id, smp_processor_id());
>> +
>>      pv_info.name = "Xen HVM";
>>
>>      xen_domain_type = XEN_HVM_DOMAIN;
>> @@ -1806,6 +1823,10 @@ static int xen_hvm_cpu_notify(struct
>> notifier_block *self, unsigned long action,
>>      int cpu = (long)hcpu;
>>      switch (action) {
>>      case CPU_UP_PREPARE:
>> +        if (cpu_acpi_id(cpu) != U32_MAX)
>> +            per_cpu(xen_vcpu_id, cpu) = cpu_acpi_id(cpu);
>> +        else
>> +            per_cpu(xen_vcpu_id, cpu) = cpu;
>
> I have not tried myself. But looking at the code, the notifiers
> xen_hvm_cpu_notifier and evtchn_fifo_cpu_notifier have the same
> priority. So what does prevent the code above to be executed after the
> event channel callback?


We will be converting to new hotplug state machine where this order will
be guaranteed:

 https://lists.xenproject.org/archives/html/xen-devel/2016-08/msg01914.html


-boris

Re: [PATCH linux v3 3/9] xen: introduce xen_vcpu_id mapping

[Vitaly Kuznetsov]

Julien Grall <julien.grall@arm.com> writes:

> Hi Vitaly,
>
> On 26/07/16 13:30, Vitaly Kuznetsov wrote:
>> It may happen that Xen's and Linux's ideas of vCPU id diverge. In
>> particular, when we crash on a secondary vCPU we may want to do kdump
>> and unlike plain kexec where we do migrate_to_reboot_cpu() we try booting
>> on the vCPU which crashed. This doesn't work very well for PVHVM guests as
>> we have a number of hypercalls where we pass vCPU id as a parameter. These
>> hypercalls either fail or do something unexpected. To solve the issue
>> introduce percpu xen_vcpu_id mapping. ARM and PV guests get direct mapping
>> for now. Boot CPU for PVHVM guest gets its id from CPUID. With secondary
>> CPUs it is a bit more trickier. Currently, we initialize IPI vectors
>> before these CPUs boot so we can't use CPUID. Use ACPI ids from MADT
>> instead.
>>
>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>> ---
>> Changes since v2:
>> - Use uint32_t for xen_vcpu_id mapping [Julien Grall]
>>
>> Changes since v1:
>> - Introduce xen_vcpu_nr() helper [David Vrabel]
>> - Use ACPI ids instead of vLAPIC ids /2 [Andrew Cooper, Jan Beulich]
>> ---
>>  arch/arm/xen/enlighten.c | 10 ++++++++++
>>  arch/x86/xen/enlighten.c | 23 ++++++++++++++++++++++-
>>  include/xen/xen-ops.h    |  6 ++++++
>>  3 files changed, 38 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
>> index 75cd734..fe32267 100644
>> --- a/arch/arm/xen/enlighten.c
>> +++ b/arch/arm/xen/enlighten.c
>> @@ -46,6 +46,10 @@ struct shared_info *HYPERVISOR_shared_info = (void *)&xen_dummy_shared_info;
>>  DEFINE_PER_CPU(struct vcpu_info *, xen_vcpu);
>>  static struct vcpu_info __percpu *xen_vcpu_info;
>>
>> +/* Linux <-> Xen vCPU id mapping */
>> +DEFINE_PER_CPU(uint32_t, xen_vcpu_id) = U32_MAX;
>> +EXPORT_PER_CPU_SYMBOL(xen_vcpu_id);
>> +
>>  /* These are unused until we support booting "pre-ballooned" */
>>  unsigned long xen_released_pages;
>>  struct xen_memory_region xen_extra_mem[XEN_EXTRA_MEM_MAX_REGIONS] __initdata;
>> @@ -179,6 +183,9 @@ static void xen_percpu_init(void)
>>  	pr_info("Xen: initializing cpu%d\n", cpu);
>>  	vcpup = per_cpu_ptr(xen_vcpu_info, cpu);
>>
>> +	/* Direct vCPU id mapping for ARM guests. */
>> +	per_cpu(xen_vcpu_id, cpu) = cpu;
>> +
>
> We did some internal testing on ARM64 with the latest Linux kernel
> (4.8-rc4) and noticed that this patch is breaking SMP support. Sorry
> for noticing the issue that late.

Sorry for the breakage :-(

>
> This function is called on the running CPU whilst some code (e.g
> init_control_block in drivers/xen/events/events_fifo.c) is executed
> whilst preparing the CPU on the boot CPU.
>
> So xen_vcpu_nr(cpu) will always return 0 in this case and
> init_control_block will fail to execute.
>

I see,

CPU_UP_PREPARE event happens before xen_starting_cpu() is called.


> I am not sure how to fix. I guess we could setup per_cpu(xen_vcpu_id,
> *) in xen_guest_init. Any opinions?

As we're not doing kexec on ARM we can fix the immediate issue. I don't
know much about ARM and unfortunatelly I don't have a setup to test but
it seems there is no early_per_cpu* infrastructure for ARM so we may fix
it with the following:

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index 3d2cef6..f193414 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -170,9 +170,6 @@ static int xen_starting_cpu(unsigned int cpu)
        pr_info("Xen: initializing cpu%d\n", cpu);
        vcpup = per_cpu_ptr(xen_vcpu_info, cpu);
 
-       /* Direct vCPU id mapping for ARM guests. */
-       per_cpu(xen_vcpu_id, cpu) = cpu;
-
        info.mfn = virt_to_gfn(vcpup);
        info.offset = xen_offset_in_page(vcpup);
 
@@ -330,6 +327,7 @@ static int __init xen_guest_init(void)
 {
        struct xen_add_to_physmap xatp;
        struct shared_info *shared_info_page = NULL;
+       int cpu;
 
        if (!xen_domain())
                return 0;
@@ -380,7 +378,8 @@ static int __init xen_guest_init(void)
                return -ENOMEM;
 
        /* Direct vCPU id mapping for ARM guests. */
-       per_cpu(xen_vcpu_id, 0) = 0;
+       for_each_possible_cpu(cpu)
+               per_cpu(xen_vcpu_id, cpu) = cpu;
 
        xen_auto_xlat_grant_frames.count = gnttab_max_grant_frames();
        if (xen_xlate_map_ballooned_pages(&xen_auto_xlat_grant_frames.pfn,

(not tested, if we can't use for_each_possible_cpu() that early we'll
have to check against NR_CPUS instead).

But unfortunatelly we'll have to get back to this in future. Turns out
we need to know Xen's idea of vCPU id _before_ this vCPU starts
executing code. On x86 we used ACPI_ID from MADT. Is there anything like
it on ARM?

>
> [...]
>
>> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
>> index 0f87db2..c833912 100644
>> --- a/arch/x86/xen/enlighten.c
>> +++ b/arch/x86/xen/enlighten.c
>> @@ -1795,6 +1806,12 @@ static void __init init_hvm_pv_info(void)
>>
>>  	xen_setup_features();
>>
>> +	cpuid(base + 4, &eax, &ebx, &ecx, &edx);
>> +	if (eax & XEN_HVM_CPUID_VCPU_ID_PRESENT)
>> +		this_cpu_write(xen_vcpu_id, ebx);
>> +	else
>> +		this_cpu_write(xen_vcpu_id, smp_processor_id());
>> +
>>  	pv_info.name = "Xen HVM";
>>
>>  	xen_domain_type = XEN_HVM_DOMAIN;
>> @@ -1806,6 +1823,10 @@ static int xen_hvm_cpu_notify(struct notifier_block *self, unsigned long action,
>>  	int cpu = (long)hcpu;
>>  	switch (action) {
>>  	case CPU_UP_PREPARE:
>> +		if (cpu_acpi_id(cpu) != U32_MAX)
>> +			per_cpu(xen_vcpu_id, cpu) = cpu_acpi_id(cpu);
>> +		else
>> +			per_cpu(xen_vcpu_id, cpu) = cpu;
>
> I have not tried myself. But looking at the code, the notifiers
> xen_hvm_cpu_notifier and evtchn_fifo_cpu_notifier have the same
> priority. So what does prevent the code above to be executed after the
> event channel callback?

Nothing, actually, but xen_hvm_guest_init() happens early and we always
get these notifiers in the right order (and, reading Boris' reply, we're
gonna make it explicit in future).

>
>>  		xen_vcpu_setup(cpu);
>>  		if (xen_have_vector_callback) {
>>  			if (xen_feature(XENFEAT_hvm_safe_pvclock))
>> diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h
>> index 86abe07..648ce814 100644
>> --- a/include/xen/xen-ops.h
>> +++ b/include/xen/xen-ops.h
>> @@ -9,6 +9,12 @@
>>
>>  DECLARE_PER_CPU(struct vcpu_info *, xen_vcpu);
>>
>> +DECLARE_PER_CPU(uint32_t, xen_vcpu_id);
>> +static inline int xen_vcpu_nr(int cpu)
>> +{
>> +	return per_cpu(xen_vcpu_id, cpu);
>> +}
>> +
>>  void xen_arch_pre_suspend(void);
>>  void xen_arch_post_suspend(int suspend_cancelled);
>
> Regards,

-- 
  Vitaly

Re: [PATCH linux v3 3/9] xen: introduce xen_vcpu_id mapping

[Stefano Stabellini]

On Mon, 5 Sep 2016, Vitaly Kuznetsov wrote:
> Julien Grall <julien.grall@arm.com> writes:
> 
> > Hi Vitaly,
> >
> > On 26/07/16 13:30, Vitaly Kuznetsov wrote:
> >> It may happen that Xen's and Linux's ideas of vCPU id diverge. In
> >> particular, when we crash on a secondary vCPU we may want to do kdump
> >> and unlike plain kexec where we do migrate_to_reboot_cpu() we try booting
> >> on the vCPU which crashed. This doesn't work very well for PVHVM guests as
> >> we have a number of hypercalls where we pass vCPU id as a parameter. These
> >> hypercalls either fail or do something unexpected. To solve the issue
> >> introduce percpu xen_vcpu_id mapping. ARM and PV guests get direct mapping
> >> for now. Boot CPU for PVHVM guest gets its id from CPUID. With secondary
> >> CPUs it is a bit more trickier. Currently, we initialize IPI vectors
> >> before these CPUs boot so we can't use CPUID. Use ACPI ids from MADT
> >> instead.
> >>
> >> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> >> ---
> >> Changes since v2:
> >> - Use uint32_t for xen_vcpu_id mapping [Julien Grall]
> >>
> >> Changes since v1:
> >> - Introduce xen_vcpu_nr() helper [David Vrabel]
> >> - Use ACPI ids instead of vLAPIC ids /2 [Andrew Cooper, Jan Beulich]
> >> ---
> >>  arch/arm/xen/enlighten.c | 10 ++++++++++
> >>  arch/x86/xen/enlighten.c | 23 ++++++++++++++++++++++-
> >>  include/xen/xen-ops.h    |  6 ++++++
> >>  3 files changed, 38 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> >> index 75cd734..fe32267 100644
> >> --- a/arch/arm/xen/enlighten.c
> >> +++ b/arch/arm/xen/enlighten.c
> >> @@ -46,6 +46,10 @@ struct shared_info *HYPERVISOR_shared_info = (void *)&xen_dummy_shared_info;
> >>  DEFINE_PER_CPU(struct vcpu_info *, xen_vcpu);
> >>  static struct vcpu_info __percpu *xen_vcpu_info;
> >>
> >> +/* Linux <-> Xen vCPU id mapping */
> >> +DEFINE_PER_CPU(uint32_t, xen_vcpu_id) = U32_MAX;
> >> +EXPORT_PER_CPU_SYMBOL(xen_vcpu_id);
> >> +
> >>  /* These are unused until we support booting "pre-ballooned" */
> >>  unsigned long xen_released_pages;
> >>  struct xen_memory_region xen_extra_mem[XEN_EXTRA_MEM_MAX_REGIONS] __initdata;
> >> @@ -179,6 +183,9 @@ static void xen_percpu_init(void)
> >>  	pr_info("Xen: initializing cpu%d\n", cpu);
> >>  	vcpup = per_cpu_ptr(xen_vcpu_info, cpu);
> >>
> >> +	/* Direct vCPU id mapping for ARM guests. */
> >> +	per_cpu(xen_vcpu_id, cpu) = cpu;
> >> +
> >
> > We did some internal testing on ARM64 with the latest Linux kernel
> > (4.8-rc4) and noticed that this patch is breaking SMP support. Sorry
> > for noticing the issue that late.
> 
> Sorry for the breakage :-(
> 
> >
> > This function is called on the running CPU whilst some code (e.g
> > init_control_block in drivers/xen/events/events_fifo.c) is executed
> > whilst preparing the CPU on the boot CPU.
> >
> > So xen_vcpu_nr(cpu) will always return 0 in this case and
> > init_control_block will fail to execute.
> >
> 
> I see,
> 
> CPU_UP_PREPARE event happens before xen_starting_cpu() is called.
> 
> 
> > I am not sure how to fix. I guess we could setup per_cpu(xen_vcpu_id,
> > *) in xen_guest_init. Any opinions?
> 
> As we're not doing kexec on ARM we can fix the immediate issue. I don't
> know much about ARM and unfortunatelly I don't have a setup to test but
> it seems there is no early_per_cpu* infrastructure for ARM so we may fix
> it with the following:
> 
> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> index 3d2cef6..f193414 100644
> --- a/arch/arm/xen/enlighten.c
> +++ b/arch/arm/xen/enlighten.c
> @@ -170,9 +170,6 @@ static int xen_starting_cpu(unsigned int cpu)
>         pr_info("Xen: initializing cpu%d\n", cpu);
>         vcpup = per_cpu_ptr(xen_vcpu_info, cpu);
>  
> -       /* Direct vCPU id mapping for ARM guests. */
> -       per_cpu(xen_vcpu_id, cpu) = cpu;
> -
>         info.mfn = virt_to_gfn(vcpup);
>         info.offset = xen_offset_in_page(vcpup);
>  
> @@ -330,6 +327,7 @@ static int __init xen_guest_init(void)
>  {
>         struct xen_add_to_physmap xatp;
>         struct shared_info *shared_info_page = NULL;
> +       int cpu;
>  
>         if (!xen_domain())
>                 return 0;
> @@ -380,7 +378,8 @@ static int __init xen_guest_init(void)
>                 return -ENOMEM;
>  
>         /* Direct vCPU id mapping for ARM guests. */
> -       per_cpu(xen_vcpu_id, 0) = 0;
> +       for_each_possible_cpu(cpu)
> +               per_cpu(xen_vcpu_id, cpu) = cpu;
>  
>         xen_auto_xlat_grant_frames.count = gnttab_max_grant_frames();
>         if (xen_xlate_map_ballooned_pages(&xen_auto_xlat_grant_frames.pfn,
> 
> (not tested, if we can't use for_each_possible_cpu() that early we'll
> have to check against NR_CPUS instead).

Kind of defeat the purpose of xen_vcpu_id, but I guess it should work.


> But unfortunatelly we'll have to get back to this in future. Turns out
> we need to know Xen's idea of vCPU id _before_ this vCPU starts
> executing code.

Why?


> On x86 we used ACPI_ID from MADT. Is there anything like
> it on ARM?

MPIDR: 

http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0388e/CIHEBGFG.html

But first we should formally document the relationship between MPIDR and
vcpu id.

Re: [PATCH linux v3 3/9] xen: introduce xen_vcpu_id mapping

[Vitaly Kuznetsov]

Stefano Stabellini <sstabellini@kernel.org> writes:

> On Mon, 5 Sep 2016, Vitaly Kuznetsov wrote:
>> Julien Grall <julien.grall@arm.com> writes:
>> 
>> > Hi Vitaly,
>> >
>> > On 26/07/16 13:30, Vitaly Kuznetsov wrote:
>> >> It may happen that Xen's and Linux's ideas of vCPU id diverge. In
>> >> particular, when we crash on a secondary vCPU we may want to do kdump
>> >> and unlike plain kexec where we do migrate_to_reboot_cpu() we try booting
>> >> on the vCPU which crashed. This doesn't work very well for PVHVM guests as
>> >> we have a number of hypercalls where we pass vCPU id as a parameter. These
>> >> hypercalls either fail or do something unexpected. To solve the issue
>> >> introduce percpu xen_vcpu_id mapping. ARM and PV guests get direct mapping
>> >> for now. Boot CPU for PVHVM guest gets its id from CPUID. With secondary
>> >> CPUs it is a bit more trickier. Currently, we initialize IPI vectors
>> >> before these CPUs boot so we can't use CPUID. Use ACPI ids from MADT
>> >> instead.
>> >>
>> >> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>> >> ---
>> >> Changes since v2:
>> >> - Use uint32_t for xen_vcpu_id mapping [Julien Grall]
>> >>
>> >> Changes since v1:
>> >> - Introduce xen_vcpu_nr() helper [David Vrabel]
>> >> - Use ACPI ids instead of vLAPIC ids /2 [Andrew Cooper, Jan Beulich]
>> >> ---
>> >>  arch/arm/xen/enlighten.c | 10 ++++++++++
>> >>  arch/x86/xen/enlighten.c | 23 ++++++++++++++++++++++-
>> >>  include/xen/xen-ops.h    |  6 ++++++
>> >>  3 files changed, 38 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
>> >> index 75cd734..fe32267 100644
>> >> --- a/arch/arm/xen/enlighten.c
>> >> +++ b/arch/arm/xen/enlighten.c
>> >> @@ -46,6 +46,10 @@ struct shared_info *HYPERVISOR_shared_info = (void *)&xen_dummy_shared_info;
>> >>  DEFINE_PER_CPU(struct vcpu_info *, xen_vcpu);
>> >>  static struct vcpu_info __percpu *xen_vcpu_info;
>> >>
>> >> +/* Linux <-> Xen vCPU id mapping */
>> >> +DEFINE_PER_CPU(uint32_t, xen_vcpu_id) = U32_MAX;
>> >> +EXPORT_PER_CPU_SYMBOL(xen_vcpu_id);
>> >> +
>> >>  /* These are unused until we support booting "pre-ballooned" */
>> >>  unsigned long xen_released_pages;
>> >>  struct xen_memory_region xen_extra_mem[XEN_EXTRA_MEM_MAX_REGIONS] __initdata;
>> >> @@ -179,6 +183,9 @@ static void xen_percpu_init(void)
>> >>  	pr_info("Xen: initializing cpu%d\n", cpu);
>> >>  	vcpup = per_cpu_ptr(xen_vcpu_info, cpu);
>> >>
>> >> +	/* Direct vCPU id mapping for ARM guests. */
>> >> +	per_cpu(xen_vcpu_id, cpu) = cpu;
>> >> +
>> >
>> > We did some internal testing on ARM64 with the latest Linux kernel
>> > (4.8-rc4) and noticed that this patch is breaking SMP support. Sorry
>> > for noticing the issue that late.
>> 
>> Sorry for the breakage :-(
>> 
>> >
>> > This function is called on the running CPU whilst some code (e.g
>> > init_control_block in drivers/xen/events/events_fifo.c) is executed
>> > whilst preparing the CPU on the boot CPU.
>> >
>> > So xen_vcpu_nr(cpu) will always return 0 in this case and
>> > init_control_block will fail to execute.
>> >
>> 
>> I see,
>> 
>> CPU_UP_PREPARE event happens before xen_starting_cpu() is called.
>> 
>> 
>> > I am not sure how to fix. I guess we could setup per_cpu(xen_vcpu_id,
>> > *) in xen_guest_init. Any opinions?
>> 
>> As we're not doing kexec on ARM we can fix the immediate issue. I don't
>> know much about ARM and unfortunatelly I don't have a setup to test but
>> it seems there is no early_per_cpu* infrastructure for ARM so we may fix
>> it with the following:
>> 
>> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
>> index 3d2cef6..f193414 100644
>> --- a/arch/arm/xen/enlighten.c
>> +++ b/arch/arm/xen/enlighten.c
>> @@ -170,9 +170,6 @@ static int xen_starting_cpu(unsigned int cpu)
>>         pr_info("Xen: initializing cpu%d\n", cpu);
>>         vcpup = per_cpu_ptr(xen_vcpu_info, cpu);
>>  
>> -       /* Direct vCPU id mapping for ARM guests. */
>> -       per_cpu(xen_vcpu_id, cpu) = cpu;
>> -
>>         info.mfn = virt_to_gfn(vcpup);
>>         info.offset = xen_offset_in_page(vcpup);
>>  
>> @@ -330,6 +327,7 @@ static int __init xen_guest_init(void)
>>  {
>>         struct xen_add_to_physmap xatp;
>>         struct shared_info *shared_info_page = NULL;
>> +       int cpu;
>>  
>>         if (!xen_domain())
>>                 return 0;
>> @@ -380,7 +378,8 @@ static int __init xen_guest_init(void)
>>                 return -ENOMEM;
>>  
>>         /* Direct vCPU id mapping for ARM guests. */
>> -       per_cpu(xen_vcpu_id, 0) = 0;
>> +       for_each_possible_cpu(cpu)
>> +               per_cpu(xen_vcpu_id, cpu) = cpu;
>>  
>>         xen_auto_xlat_grant_frames.count = gnttab_max_grant_frames();
>>         if (xen_xlate_map_ballooned_pages(&xen_auto_xlat_grant_frames.pfn,
>> 
>> (not tested, if we can't use for_each_possible_cpu() that early we'll
>> have to check against NR_CPUS instead).
>
> Kind of defeat the purpose of xen_vcpu_id, but I guess it should work.
>
>> But unfortunatelly we'll have to get back to this in future. Turns out
>> we need to know Xen's idea of vCPU id _before_ this vCPU starts
>> executing code.
>
> Why?

E.g. for FIFO event channels we currently call
evtchn_fifo_cpu_notification() for CPU_UP_PREPARE event (and do a
hypercall which requires us to know vcpu id) and this happens before the
secodary cpu starts. I'm not sure this can't be changed in future.

>
>> On x86 we used ACPI_ID from MADT. Is there anything like
>> it on ARM?
>
> MPIDR: 
>
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0388e/CIHEBGFG.html
>

Thanks, but doesn't it work the same way CPUID instruction works? In
that case we won't be able to use it to figure out other CPU's id.

> But first we should formally document the relationship between MPIDR and
> vcpu id.

Of course.

-- 
  Vitaly

Re: [PATCH linux v3 3/9] xen: introduce xen_vcpu_id mapping

[Stefano Stabellini]

On Tue, 6 Sep 2016, Vitaly Kuznetsov wrote:
> Stefano Stabellini <sstabellini@kernel.org> writes:
> 
> > On Mon, 5 Sep 2016, Vitaly Kuznetsov wrote:
> >> Julien Grall <julien.grall@arm.com> writes:
> >> 
> >> > Hi Vitaly,
> >> >
> >> > On 26/07/16 13:30, Vitaly Kuznetsov wrote:
> >> >> It may happen that Xen's and Linux's ideas of vCPU id diverge. In
> >> >> particular, when we crash on a secondary vCPU we may want to do kdump
> >> >> and unlike plain kexec where we do migrate_to_reboot_cpu() we try booting
> >> >> on the vCPU which crashed. This doesn't work very well for PVHVM guests as
> >> >> we have a number of hypercalls where we pass vCPU id as a parameter. These
> >> >> hypercalls either fail or do something unexpected. To solve the issue
> >> >> introduce percpu xen_vcpu_id mapping. ARM and PV guests get direct mapping
> >> >> for now. Boot CPU for PVHVM guest gets its id from CPUID. With secondary
> >> >> CPUs it is a bit more trickier. Currently, we initialize IPI vectors
> >> >> before these CPUs boot so we can't use CPUID. Use ACPI ids from MADT
> >> >> instead.
> >> >>
> >> >> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> >> >> ---
> >> >> Changes since v2:
> >> >> - Use uint32_t for xen_vcpu_id mapping [Julien Grall]
> >> >>
> >> >> Changes since v1:
> >> >> - Introduce xen_vcpu_nr() helper [David Vrabel]
> >> >> - Use ACPI ids instead of vLAPIC ids /2 [Andrew Cooper, Jan Beulich]
> >> >> ---
> >> >>  arch/arm/xen/enlighten.c | 10 ++++++++++
> >> >>  arch/x86/xen/enlighten.c | 23 ++++++++++++++++++++++-
> >> >>  include/xen/xen-ops.h    |  6 ++++++
> >> >>  3 files changed, 38 insertions(+), 1 deletion(-)
> >> >>
> >> >> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> >> >> index 75cd734..fe32267 100644
> >> >> --- a/arch/arm/xen/enlighten.c
> >> >> +++ b/arch/arm/xen/enlighten.c
> >> >> @@ -46,6 +46,10 @@ struct shared_info *HYPERVISOR_shared_info = (void *)&xen_dummy_shared_info;
> >> >>  DEFINE_PER_CPU(struct vcpu_info *, xen_vcpu);
> >> >>  static struct vcpu_info __percpu *xen_vcpu_info;
> >> >>
> >> >> +/* Linux <-> Xen vCPU id mapping */
> >> >> +DEFINE_PER_CPU(uint32_t, xen_vcpu_id) = U32_MAX;
> >> >> +EXPORT_PER_CPU_SYMBOL(xen_vcpu_id);
> >> >> +
> >> >>  /* These are unused until we support booting "pre-ballooned" */
> >> >>  unsigned long xen_released_pages;
> >> >>  struct xen_memory_region xen_extra_mem[XEN_EXTRA_MEM_MAX_REGIONS] __initdata;
> >> >> @@ -179,6 +183,9 @@ static void xen_percpu_init(void)
> >> >>  	pr_info("Xen: initializing cpu%d\n", cpu);
> >> >>  	vcpup = per_cpu_ptr(xen_vcpu_info, cpu);
> >> >>
> >> >> +	/* Direct vCPU id mapping for ARM guests. */
> >> >> +	per_cpu(xen_vcpu_id, cpu) = cpu;
> >> >> +
> >> >
> >> > We did some internal testing on ARM64 with the latest Linux kernel
> >> > (4.8-rc4) and noticed that this patch is breaking SMP support. Sorry
> >> > for noticing the issue that late.
> >> 
> >> Sorry for the breakage :-(
> >> 
> >> >
> >> > This function is called on the running CPU whilst some code (e.g
> >> > init_control_block in drivers/xen/events/events_fifo.c) is executed
> >> > whilst preparing the CPU on the boot CPU.
> >> >
> >> > So xen_vcpu_nr(cpu) will always return 0 in this case and
> >> > init_control_block will fail to execute.
> >> >
> >> 
> >> I see,
> >> 
> >> CPU_UP_PREPARE event happens before xen_starting_cpu() is called.
> >> 
> >> 
> >> > I am not sure how to fix. I guess we could setup per_cpu(xen_vcpu_id,
> >> > *) in xen_guest_init. Any opinions?
> >> 
> >> As we're not doing kexec on ARM we can fix the immediate issue. I don't
> >> know much about ARM and unfortunatelly I don't have a setup to test but
> >> it seems there is no early_per_cpu* infrastructure for ARM so we may fix
> >> it with the following:
> >> 
> >> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> >> index 3d2cef6..f193414 100644
> >> --- a/arch/arm/xen/enlighten.c
> >> +++ b/arch/arm/xen/enlighten.c
> >> @@ -170,9 +170,6 @@ static int xen_starting_cpu(unsigned int cpu)
> >>         pr_info("Xen: initializing cpu%d\n", cpu);
> >>         vcpup = per_cpu_ptr(xen_vcpu_info, cpu);
> >>  
> >> -       /* Direct vCPU id mapping for ARM guests. */
> >> -       per_cpu(xen_vcpu_id, cpu) = cpu;
> >> -
> >>         info.mfn = virt_to_gfn(vcpup);
> >>         info.offset = xen_offset_in_page(vcpup);
> >>  
> >> @@ -330,6 +327,7 @@ static int __init xen_guest_init(void)
> >>  {
> >>         struct xen_add_to_physmap xatp;
> >>         struct shared_info *shared_info_page = NULL;
> >> +       int cpu;
> >>  
> >>         if (!xen_domain())
> >>                 return 0;
> >> @@ -380,7 +378,8 @@ static int __init xen_guest_init(void)
> >>                 return -ENOMEM;
> >>  
> >>         /* Direct vCPU id mapping for ARM guests. */
> >> -       per_cpu(xen_vcpu_id, 0) = 0;
> >> +       for_each_possible_cpu(cpu)
> >> +               per_cpu(xen_vcpu_id, cpu) = cpu;
> >>  
> >>         xen_auto_xlat_grant_frames.count = gnttab_max_grant_frames();
> >>         if (xen_xlate_map_ballooned_pages(&xen_auto_xlat_grant_frames.pfn,
> >> 
> >> (not tested, if we can't use for_each_possible_cpu() that early we'll
> >> have to check against NR_CPUS instead).
> >
> > Kind of defeat the purpose of xen_vcpu_id, but I guess it should work.
> >
> >> But unfortunatelly we'll have to get back to this in future. Turns out
> >> we need to know Xen's idea of vCPU id _before_ this vCPU starts
> >> executing code.
> >
> > Why?
> 
> E.g. for FIFO event channels we currently call
> evtchn_fifo_cpu_notification() for CPU_UP_PREPARE event (and do a
> hypercall which requires us to know vcpu id) and this happens before the
> secodary cpu starts. I'm not sure this can't be changed in future.
> 
> >
> >> On x86 we used ACPI_ID from MADT. Is there anything like
> >> it on ARM?
> >
> > MPIDR: 
> >
> > http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0388e/CIHEBGFG.html
> >
> 
> Thanks, but doesn't it work the same way CPUID instruction works? In
> that case we won't be able to use it to figure out other CPU's id.

I don't know that much about cpuid, but the virtual MPIDR is constructed
from the vcpu id right now:

    v->arch.vmpidr = MPIDR_SMP | vcpuid_to_vaffinity(v->vcpu_id);
    
    [...]

    static inline register_t vcpuid_to_vaffinity(unsigned int vcpuid)
    {
        register_t vaff;
    
        vaff = (vcpuid & 0x0f) << MPIDR_LEVEL_SHIFT(0);
        vaff |= ((vcpuid >> 4) & MPIDR_LEVEL_MASK) << MPIDR_LEVEL_SHIFT(1);
    
        return vaff;
    }

Re: [PATCH linux v3 3/9] xen: introduce xen_vcpu_id mapping

[Vitaly Kuznetsov]

Stefano Stabellini <sstabellini@kernel.org> writes:

> On Tue, 6 Sep 2016, Vitaly Kuznetsov wrote:
>> Stefano Stabellini <sstabellini@kernel.org> writes:
>> 
>> > On Mon, 5 Sep 2016, Vitaly Kuznetsov wrote:
>> >> Julien Grall <julien.grall@arm.com> writes:
>> >> 
>> >> > Hi Vitaly,
>> >> >
>> >> > On 26/07/16 13:30, Vitaly Kuznetsov wrote:
>> >> >> It may happen that Xen's and Linux's ideas of vCPU id diverge. In
>> >> >> particular, when we crash on a secondary vCPU we may want to do kdump
>> >> >> and unlike plain kexec where we do migrate_to_reboot_cpu() we try booting
>> >> >> on the vCPU which crashed. This doesn't work very well for PVHVM guests as
>> >> >> we have a number of hypercalls where we pass vCPU id as a parameter. These
>> >> >> hypercalls either fail or do something unexpected. To solve the issue
>> >> >> introduce percpu xen_vcpu_id mapping. ARM and PV guests get direct mapping
>> >> >> for now. Boot CPU for PVHVM guest gets its id from CPUID. With secondary
>> >> >> CPUs it is a bit more trickier. Currently, we initialize IPI vectors
>> >> >> before these CPUs boot so we can't use CPUID. Use ACPI ids from MADT
>> >> >> instead.
>> >> >>
>> >> >> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>> >> >> ---
>> >> >> Changes since v2:
>> >> >> - Use uint32_t for xen_vcpu_id mapping [Julien Grall]
>> >> >>
>> >> >> Changes since v1:
>> >> >> - Introduce xen_vcpu_nr() helper [David Vrabel]
>> >> >> - Use ACPI ids instead of vLAPIC ids /2 [Andrew Cooper, Jan Beulich]
>> >> >> ---
>> >> >>  arch/arm/xen/enlighten.c | 10 ++++++++++
>> >> >>  arch/x86/xen/enlighten.c | 23 ++++++++++++++++++++++-
>> >> >>  include/xen/xen-ops.h    |  6 ++++++
>> >> >>  3 files changed, 38 insertions(+), 1 deletion(-)
>> >> >>
>> >> >> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
>> >> >> index 75cd734..fe32267 100644
>> >> >> --- a/arch/arm/xen/enlighten.c
>> >> >> +++ b/arch/arm/xen/enlighten.c
>> >> >> @@ -46,6 +46,10 @@ struct shared_info *HYPERVISOR_shared_info = (void *)&xen_dummy_shared_info;
>> >> >>  DEFINE_PER_CPU(struct vcpu_info *, xen_vcpu);
>> >> >>  static struct vcpu_info __percpu *xen_vcpu_info;
>> >> >>
>> >> >> +/* Linux <-> Xen vCPU id mapping */
>> >> >> +DEFINE_PER_CPU(uint32_t, xen_vcpu_id) = U32_MAX;
>> >> >> +EXPORT_PER_CPU_SYMBOL(xen_vcpu_id);
>> >> >> +
>> >> >>  /* These are unused until we support booting "pre-ballooned" */
>> >> >>  unsigned long xen_released_pages;
>> >> >>  struct xen_memory_region xen_extra_mem[XEN_EXTRA_MEM_MAX_REGIONS] __initdata;
>> >> >> @@ -179,6 +183,9 @@ static void xen_percpu_init(void)
>> >> >>  	pr_info("Xen: initializing cpu%d\n", cpu);
>> >> >>  	vcpup = per_cpu_ptr(xen_vcpu_info, cpu);
>> >> >>
>> >> >> +	/* Direct vCPU id mapping for ARM guests. */
>> >> >> +	per_cpu(xen_vcpu_id, cpu) = cpu;
>> >> >> +
>> >> >
>> >> > We did some internal testing on ARM64 with the latest Linux kernel
>> >> > (4.8-rc4) and noticed that this patch is breaking SMP support. Sorry
>> >> > for noticing the issue that late.
>> >> 
>> >> Sorry for the breakage :-(
>> >> 
>> >> >
>> >> > This function is called on the running CPU whilst some code (e.g
>> >> > init_control_block in drivers/xen/events/events_fifo.c) is executed
>> >> > whilst preparing the CPU on the boot CPU.
>> >> >
>> >> > So xen_vcpu_nr(cpu) will always return 0 in this case and
>> >> > init_control_block will fail to execute.
>> >> >
>> >> 
>> >> I see,
>> >> 
>> >> CPU_UP_PREPARE event happens before xen_starting_cpu() is called.
>> >> 
>> >> 
>> >> > I am not sure how to fix. I guess we could setup per_cpu(xen_vcpu_id,
>> >> > *) in xen_guest_init. Any opinions?
>> >> 
>> >> As we're not doing kexec on ARM we can fix the immediate issue. I don't
>> >> know much about ARM and unfortunatelly I don't have a setup to test but
>> >> it seems there is no early_per_cpu* infrastructure for ARM so we may fix
>> >> it with the following:
>> >> 
>> >> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
>> >> index 3d2cef6..f193414 100644
>> >> --- a/arch/arm/xen/enlighten.c
>> >> +++ b/arch/arm/xen/enlighten.c
>> >> @@ -170,9 +170,6 @@ static int xen_starting_cpu(unsigned int cpu)
>> >>         pr_info("Xen: initializing cpu%d\n", cpu);
>> >>         vcpup = per_cpu_ptr(xen_vcpu_info, cpu);
>> >>  
>> >> -       /* Direct vCPU id mapping for ARM guests. */
>> >> -       per_cpu(xen_vcpu_id, cpu) = cpu;
>> >> -
>> >>         info.mfn = virt_to_gfn(vcpup);
>> >>         info.offset = xen_offset_in_page(vcpup);
>> >>  
>> >> @@ -330,6 +327,7 @@ static int __init xen_guest_init(void)
>> >>  {
>> >>         struct xen_add_to_physmap xatp;
>> >>         struct shared_info *shared_info_page = NULL;
>> >> +       int cpu;
>> >>  
>> >>         if (!xen_domain())
>> >>                 return 0;
>> >> @@ -380,7 +378,8 @@ static int __init xen_guest_init(void)
>> >>                 return -ENOMEM;
>> >>  
>> >>         /* Direct vCPU id mapping for ARM guests. */
>> >> -       per_cpu(xen_vcpu_id, 0) = 0;
>> >> +       for_each_possible_cpu(cpu)
>> >> +               per_cpu(xen_vcpu_id, cpu) = cpu;
>> >>  
>> >>         xen_auto_xlat_grant_frames.count = gnttab_max_grant_frames();
>> >>         if (xen_xlate_map_ballooned_pages(&xen_auto_xlat_grant_frames.pfn,
>> >> 
>> >> (not tested, if we can't use for_each_possible_cpu() that early we'll
>> >> have to check against NR_CPUS instead).
>> >
>> > Kind of defeat the purpose of xen_vcpu_id, but I guess it should work.
>> >
>> >> But unfortunatelly we'll have to get back to this in future. Turns out
>> >> we need to know Xen's idea of vCPU id _before_ this vCPU starts
>> >> executing code.
>> >
>> > Why?
>> 
>> E.g. for FIFO event channels we currently call
>> evtchn_fifo_cpu_notification() for CPU_UP_PREPARE event (and do a
>> hypercall which requires us to know vcpu id) and this happens before the
>> secodary cpu starts. I'm not sure this can't be changed in future.
>> 
>> >
>> >> On x86 we used ACPI_ID from MADT. Is there anything like
>> >> it on ARM?
>> >
>> > MPIDR: 
>> >
>> > http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0388e/CIHEBGFG.html
>> >
>> 
>> Thanks, but doesn't it work the same way CPUID instruction works? In
>> that case we won't be able to use it to figure out other CPU's id.
>
> I don't know that much about cpuid, but the virtual MPIDR is constructed
> from the vcpu id right now:
>
>     v->arch.vmpidr = MPIDR_SMP | vcpuid_to_vaffinity(v->vcpu_id);
>
>     [...]
>
>     static inline register_t vcpuid_to_vaffinity(unsigned int vcpuid)
>     {
>         register_t vaff;
>
>         vaff = (vcpuid & 0x0f) << MPIDR_LEVEL_SHIFT(0);
>         vaff |= ((vcpuid >> 4) & MPIDR_LEVEL_MASK) << MPIDR_LEVEL_SHIFT(1);
>
>         return vaff;
>     }

This could work but only in case there is a way to get MPIDR for _other_
cpu (e.g. CPU0 needs to get MPIDR of CPU1 when CPU1 is not yet runnning)
or we'll have to change the machinery of how we bring up secondary CPUs
- e.g. CPUn starts, writes its id somewhere and 'hangs' waiting for CPU0
to set up event channels.

-- 
  Vitaly

Re: [PATCH linux v3 3/9] xen: introduce xen_vcpu_id mapping

[David Vrabel]

On 07/09/16 10:07, Vitaly Kuznetsov wrote:
> Stefano Stabellini <sstabellini@kernel.org> writes:
> 
>> On Tue, 6 Sep 2016, Vitaly Kuznetsov wrote:
>>> Stefano Stabellini <sstabellini@kernel.org> writes:
>>>
>>>> On Mon, 5 Sep 2016, Vitaly Kuznetsov wrote:
>>>>> Julien Grall <julien.grall@arm.com> writes:
>>>>>
>>>>>> Hi Vitaly,
>>>>>>
>>>>>> On 26/07/16 13:30, Vitaly Kuznetsov wrote:
>>>>>>> It may happen that Xen's and Linux's ideas of vCPU id diverge. In
>>>>>>> particular, when we crash on a secondary vCPU we may want to do kdump
>>>>>>> and unlike plain kexec where we do migrate_to_reboot_cpu() we try booting
>>>>>>> on the vCPU which crashed. This doesn't work very well for PVHVM guests as
>>>>>>> we have a number of hypercalls where we pass vCPU id as a parameter. These
>>>>>>> hypercalls either fail or do something unexpected. To solve the issue
>>>>>>> introduce percpu xen_vcpu_id mapping. ARM and PV guests get direct mapping
>>>>>>> for now. Boot CPU for PVHVM guest gets its id from CPUID. With secondary
>>>>>>> CPUs it is a bit more trickier. Currently, we initialize IPI vectors
>>>>>>> before these CPUs boot so we can't use CPUID. Use ACPI ids from MADT
>>>>>>> instead.
>>>>>>>
>>>>>>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>>>>>>> ---
>>>>>>> Changes since v2:
>>>>>>> - Use uint32_t for xen_vcpu_id mapping [Julien Grall]
>>>>>>>
>>>>>>> Changes since v1:
>>>>>>> - Introduce xen_vcpu_nr() helper [David Vrabel]
>>>>>>> - Use ACPI ids instead of vLAPIC ids /2 [Andrew Cooper, Jan Beulich]
>>>>>>> ---
>>>>>>>  arch/arm/xen/enlighten.c | 10 ++++++++++
>>>>>>>  arch/x86/xen/enlighten.c | 23 ++++++++++++++++++++++-
>>>>>>>  include/xen/xen-ops.h    |  6 ++++++
>>>>>>>  3 files changed, 38 insertions(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
>>>>>>> index 75cd734..fe32267 100644
>>>>>>> --- a/arch/arm/xen/enlighten.c
>>>>>>> +++ b/arch/arm/xen/enlighten.c
>>>>>>> @@ -46,6 +46,10 @@ struct shared_info *HYPERVISOR_shared_info = (void *)&xen_dummy_shared_info;
>>>>>>>  DEFINE_PER_CPU(struct vcpu_info *, xen_vcpu);
>>>>>>>  static struct vcpu_info __percpu *xen_vcpu_info;
>>>>>>>
>>>>>>> +/* Linux <-> Xen vCPU id mapping */
>>>>>>> +DEFINE_PER_CPU(uint32_t, xen_vcpu_id) = U32_MAX;
>>>>>>> +EXPORT_PER_CPU_SYMBOL(xen_vcpu_id);
>>>>>>> +
>>>>>>>  /* These are unused until we support booting "pre-ballooned" */
>>>>>>>  unsigned long xen_released_pages;
>>>>>>>  struct xen_memory_region xen_extra_mem[XEN_EXTRA_MEM_MAX_REGIONS] __initdata;
>>>>>>> @@ -179,6 +183,9 @@ static void xen_percpu_init(void)
>>>>>>>  	pr_info("Xen: initializing cpu%d\n", cpu);
>>>>>>>  	vcpup = per_cpu_ptr(xen_vcpu_info, cpu);
>>>>>>>
>>>>>>> +	/* Direct vCPU id mapping for ARM guests. */
>>>>>>> +	per_cpu(xen_vcpu_id, cpu) = cpu;
>>>>>>> +
>>>>>>
>>>>>> We did some internal testing on ARM64 with the latest Linux kernel
>>>>>> (4.8-rc4) and noticed that this patch is breaking SMP support. Sorry
>>>>>> for noticing the issue that late.
>>>>>
>>>>> Sorry for the breakage :-(
>>>>>
>>>>>>
>>>>>> This function is called on the running CPU whilst some code (e.g
>>>>>> init_control_block in drivers/xen/events/events_fifo.c) is executed
>>>>>> whilst preparing the CPU on the boot CPU.
>>>>>>
>>>>>> So xen_vcpu_nr(cpu) will always return 0 in this case and
>>>>>> init_control_block will fail to execute.
>>>>>>
>>>>>
>>>>> I see,
>>>>>
>>>>> CPU_UP_PREPARE event happens before xen_starting_cpu() is called.
>>>>>
>>>>>
>>>>>> I am not sure how to fix. I guess we could setup per_cpu(xen_vcpu_id,
>>>>>> *) in xen_guest_init. Any opinions?
>>>>>
>>>>> As we're not doing kexec on ARM we can fix the immediate issue. I don't
>>>>> know much about ARM and unfortunatelly I don't have a setup to test but
>>>>> it seems there is no early_per_cpu* infrastructure for ARM so we may fix
>>>>> it with the following:
>>>>>
>>>>> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
>>>>> index 3d2cef6..f193414 100644
>>>>> --- a/arch/arm/xen/enlighten.c
>>>>> +++ b/arch/arm/xen/enlighten.c
>>>>> @@ -170,9 +170,6 @@ static int xen_starting_cpu(unsigned int cpu)
>>>>>         pr_info("Xen: initializing cpu%d\n", cpu);
>>>>>         vcpup = per_cpu_ptr(xen_vcpu_info, cpu);
>>>>>  
>>>>> -       /* Direct vCPU id mapping for ARM guests. */
>>>>> -       per_cpu(xen_vcpu_id, cpu) = cpu;
>>>>> -
>>>>>         info.mfn = virt_to_gfn(vcpup);
>>>>>         info.offset = xen_offset_in_page(vcpup);
>>>>>  
>>>>> @@ -330,6 +327,7 @@ static int __init xen_guest_init(void)
>>>>>  {
>>>>>         struct xen_add_to_physmap xatp;
>>>>>         struct shared_info *shared_info_page = NULL;
>>>>> +       int cpu;
>>>>>  
>>>>>         if (!xen_domain())
>>>>>                 return 0;
>>>>> @@ -380,7 +378,8 @@ static int __init xen_guest_init(void)
>>>>>                 return -ENOMEM;
>>>>>  
>>>>>         /* Direct vCPU id mapping for ARM guests. */
>>>>> -       per_cpu(xen_vcpu_id, 0) = 0;
>>>>> +       for_each_possible_cpu(cpu)
>>>>> +               per_cpu(xen_vcpu_id, cpu) = cpu;
>>>>>  
>>>>>         xen_auto_xlat_grant_frames.count = gnttab_max_grant_frames();
>>>>>         if (xen_xlate_map_ballooned_pages(&xen_auto_xlat_grant_frames.pfn,
>>>>>
>>>>> (not tested, if we can't use for_each_possible_cpu() that early we'll
>>>>> have to check against NR_CPUS instead).
>>>>
>>>> Kind of defeat the purpose of xen_vcpu_id, but I guess it should work.
>>>>
>>>>> But unfortunatelly we'll have to get back to this in future. Turns out
>>>>> we need to know Xen's idea of vCPU id _before_ this vCPU starts
>>>>> executing code.
>>>>
>>>> Why?
>>>
>>> E.g. for FIFO event channels we currently call
>>> evtchn_fifo_cpu_notification() for CPU_UP_PREPARE event (and do a
>>> hypercall which requires us to know vcpu id) and this happens before the
>>> secodary cpu starts. I'm not sure this can't be changed in future.
>>>
>>>>
>>>>> On x86 we used ACPI_ID from MADT. Is there anything like
>>>>> it on ARM?
>>>>
>>>> MPIDR: 
>>>>
>>>> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0388e/CIHEBGFG.html
>>>>
>>>
>>> Thanks, but doesn't it work the same way CPUID instruction works? In
>>> that case we won't be able to use it to figure out other CPU's id.
>>
>> I don't know that much about cpuid, but the virtual MPIDR is constructed
>> from the vcpu id right now:
>>
>>     v->arch.vmpidr = MPIDR_SMP | vcpuid_to_vaffinity(v->vcpu_id);
>>
>>     [...]
>>
>>     static inline register_t vcpuid_to_vaffinity(unsigned int vcpuid)
>>     {
>>         register_t vaff;
>>
>>         vaff = (vcpuid & 0x0f) << MPIDR_LEVEL_SHIFT(0);
>>         vaff |= ((vcpuid >> 4) & MPIDR_LEVEL_MASK) << MPIDR_LEVEL_SHIFT(1);
>>
>>         return vaff;
>>     }
> 
> This could work but only in case there is a way to get MPIDR for _other_
> cpu (e.g. CPU0 needs to get MPIDR of CPU1 when CPU1 is not yet runnning)
> or we'll have to change the machinery of how we bring up secondary CPUs
> - e.g. CPUn starts, writes its id somewhere and 'hangs' waiting for CPU0
> to set up event channels.

Perhaps we can set up event channels on the CPU being brought up? Local
irqs would need to have been disabled, but I don't see any other
restriction.

David
> 

Re: [PATCH linux v3 3/9] xen: introduce xen_vcpu_id mapping

[Julien Grall]

Hi Vitaly,

On 07/09/2016 10:07, Vitaly Kuznetsov wrote:
> Stefano Stabellini <sstabellini@kernel.org> writes:
>> I don't know that much about cpuid, but the virtual MPIDR is constructed
>> from the vcpu id right now:
>>
>>     v->arch.vmpidr = MPIDR_SMP | vcpuid_to_vaffinity(v->vcpu_id);
>>
>>     [...]
>>
>>     static inline register_t vcpuid_to_vaffinity(unsigned int vcpuid)
>>     {
>>         register_t vaff;
>>
>>         vaff = (vcpuid & 0x0f) << MPIDR_LEVEL_SHIFT(0);
>>         vaff |= ((vcpuid >> 4) & MPIDR_LEVEL_MASK) << MPIDR_LEVEL_SHIFT(1);
>>
>>         return vaff;
>>     }
>
> This could work but only in case there is a way to get MPIDR for _other_
> cpu (e.g. CPU0 needs to get MPIDR of CPU1 when CPU1 is not yet runnning)
> or we'll have to change the machinery of how we bring up secondary CPUs
> - e.g. CPUn starts, writes its id somewhere and 'hangs' waiting for CPU0
> to set up event channels.

You can get the MPIDR from both the device tree and ACPI. The firmware 
table is parsed at boot time and the value is stored in cpu_logical_map().

Regards,

-- 
Julien Grall

Re: [PATCH linux v3 3/9] xen: introduce xen_vcpu_id mapping

[Vitaly Kuznetsov]

Julien Grall <julien.grall@arm.com> writes:

> Hi Vitaly,
>
> On 07/09/2016 10:07, Vitaly Kuznetsov wrote:
>> Stefano Stabellini <sstabellini@kernel.org> writes:
>>> I don't know that much about cpuid, but the virtual MPIDR is constructed
>>> from the vcpu id right now:
>>>
>>>     v->arch.vmpidr = MPIDR_SMP | vcpuid_to_vaffinity(v->vcpu_id);
>>>
>>>     [...]
>>>
>>>     static inline register_t vcpuid_to_vaffinity(unsigned int vcpuid)
>>>     {
>>>         register_t vaff;
>>>
>>>         vaff = (vcpuid & 0x0f) << MPIDR_LEVEL_SHIFT(0);
>>>         vaff |= ((vcpuid >> 4) & MPIDR_LEVEL_MASK) << MPIDR_LEVEL_SHIFT(1);
>>>
>>>         return vaff;
>>>     }
>>
>> This could work but only in case there is a way to get MPIDR for _other_
>> cpu (e.g. CPU0 needs to get MPIDR of CPU1 when CPU1 is not yet runnning)
>> or we'll have to change the machinery of how we bring up secondary CPUs
>> - e.g. CPUn starts, writes its id somewhere and 'hangs' waiting for CPU0
>> to set up event channels.
>
> You can get the MPIDR from both the device tree and ACPI. The firmware
> table is parsed at boot time and the value is stored in
> cpu_logical_map().

Good,

in this case we can easily do the same trick we did for x86 and we
don't need to change the way secondary CPUs are strarted.

BTW, were you able to try the patch I suggested? In my opinion it would
be preferable to fix the immediate SMP issue now and play with MPIDR
info later.

>
> Regards,

-- 
  Vitaly

Re: [PATCH linux v3 3/9] xen: introduce xen_vcpu_id mapping

[Julien Grall]

Hi Vitaly,

On 07/09/2016 12:23, Vitaly Kuznetsov wrote:
> BTW, were you able to try the patch I suggested? In my opinion it would
> be preferable to fix the immediate SMP issue now and play with MPIDR
> info later.

Not yet sorry. I will see if I can try to today or tomorrow.

Cheers,

-- 
Julien Grall

Re: [PATCH linux v3 3/9] xen: introduce xen_vcpu_id mapping

[Wei Chen]

Hi Vitaly,

On 2016/9/5 17:42, Vitaly Kuznetsov wrote:
> Julien Grall <julien.grall@arm.com> writes:
>
>> Hi Vitaly,
>>
>> On 26/07/16 13:30, Vitaly Kuznetsov wrote:
>>> It may happen that Xen's and Linux's ideas of vCPU id diverge. In
>>> particular, when we crash on a secondary vCPU we may want to do kdump
>>> and unlike plain kexec where we do migrate_to_reboot_cpu() we try booting
>>> on the vCPU which crashed. This doesn't work very well for PVHVM guests as
>>> we have a number of hypercalls where we pass vCPU id as a parameter. These
>>> hypercalls either fail or do something unexpected. To solve the issue
>>> introduce percpu xen_vcpu_id mapping. ARM and PV guests get direct mapping
>>> for now. Boot CPU for PVHVM guest gets its id from CPUID. With secondary
>>> CPUs it is a bit more trickier. Currently, we initialize IPI vectors
>>> before these CPUs boot so we can't use CPUID. Use ACPI ids from MADT
>>> instead.
>>>
>>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>>> ---
>>> Changes since v2:
>>> - Use uint32_t for xen_vcpu_id mapping [Julien Grall]
>>>
>>> Changes since v1:
>>> - Introduce xen_vcpu_nr() helper [David Vrabel]
>>> - Use ACPI ids instead of vLAPIC ids /2 [Andrew Cooper, Jan Beulich]
>>> ---
>>>  arch/arm/xen/enlighten.c | 10 ++++++++++
>>>  arch/x86/xen/enlighten.c | 23 ++++++++++++++++++++++-
>>>  include/xen/xen-ops.h    |  6 ++++++
>>>  3 files changed, 38 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
>>> index 75cd734..fe32267 100644
>>> --- a/arch/arm/xen/enlighten.c
>>> +++ b/arch/arm/xen/enlighten.c
>>> @@ -46,6 +46,10 @@ struct shared_info *HYPERVISOR_shared_info = (void *)&xen_dummy_shared_info;
>>>  DEFINE_PER_CPU(struct vcpu_info *, xen_vcpu);
>>>  static struct vcpu_info __percpu *xen_vcpu_info;
>>>
>>> +/* Linux <-> Xen vCPU id mapping */
>>> +DEFINE_PER_CPU(uint32_t, xen_vcpu_id) = U32_MAX;
>>> +EXPORT_PER_CPU_SYMBOL(xen_vcpu_id);
>>> +
>>>  /* These are unused until we support booting "pre-ballooned" */
>>>  unsigned long xen_released_pages;
>>>  struct xen_memory_region xen_extra_mem[XEN_EXTRA_MEM_MAX_REGIONS] __initdata;
>>> @@ -179,6 +183,9 @@ static void xen_percpu_init(void)
>>>  pr_info("Xen: initializing cpu%d\n", cpu);
>>>  vcpup = per_cpu_ptr(xen_vcpu_info, cpu);
>>>
>>> +/* Direct vCPU id mapping for ARM guests. */
>>> +per_cpu(xen_vcpu_id, cpu) = cpu;
>>> +
>>
>> We did some internal testing on ARM64 with the latest Linux kernel
>> (4.8-rc4) and noticed that this patch is breaking SMP support. Sorry
>> for noticing the issue that late.
>
> Sorry for the breakage :-(
>
>>
>> This function is called on the running CPU whilst some code (e.g
>> init_control_block in drivers/xen/events/events_fifo.c) is executed
>> whilst preparing the CPU on the boot CPU.
>>
>> So xen_vcpu_nr(cpu) will always return 0 in this case and
>> init_control_block will fail to execute.
>>
>
> I see,
>
> CPU_UP_PREPARE event happens before xen_starting_cpu() is called.
>
>
>> I am not sure how to fix. I guess we could setup per_cpu(xen_vcpu_id,
>> *) in xen_guest_init. Any opinions?
>
> As we're not doing kexec on ARM we can fix the immediate issue. I don't
> know much about ARM and unfortunatelly I don't have a setup to test but
> it seems there is no early_per_cpu* infrastructure for ARM so we may fix
> it with the following:
>
> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> index 3d2cef6..f193414 100644
> --- a/arch/arm/xen/enlighten.c
> +++ b/arch/arm/xen/enlighten.c
> @@ -170,9 +170,6 @@ static int xen_starting_cpu(unsigned int cpu)
>         pr_info("Xen: initializing cpu%d\n", cpu);
>         vcpup = per_cpu_ptr(xen_vcpu_info, cpu);
>
> -       /* Direct vCPU id mapping for ARM guests. */
> -       per_cpu(xen_vcpu_id, cpu) = cpu;
> -
>         info.mfn = virt_to_gfn(vcpup);
>         info.offset = xen_offset_in_page(vcpup);
>
> @@ -330,6 +327,7 @@ static int __init xen_guest_init(void)
>  {
>         struct xen_add_to_physmap xatp;
>         struct shared_info *shared_info_page = NULL;
> +       int cpu;
>
>         if (!xen_domain())
>                 return 0;
> @@ -380,7 +378,8 @@ static int __init xen_guest_init(void)
>                 return -ENOMEM;
>
>         /* Direct vCPU id mapping for ARM guests. */
> -       per_cpu(xen_vcpu_id, 0) = 0;
> +       for_each_possible_cpu(cpu)
> +               per_cpu(xen_vcpu_id, cpu) = cpu;
>
>         xen_auto_xlat_grant_frames.count = gnttab_max_grant_frames();
>         if (xen_xlate_map_ballooned_pages(&xen_auto_xlat_grant_frames.pfn,
>
> (not tested, if we can't use for_each_possible_cpu() that early we'll
> have to check against NR_CPUS instead).
>

I have tested this patch just now, it can work with the latest Linux
kernel and latest Xen hypervisor on my ARM platform:

[    0.299927] xen:events: Using FIFO-based ABI
[    0.304259] Xen: initializing cpu0
[    0.336402] EFI services will not be available.
[    0.388985] Detected PIPT I-cache on CPU1
[    0.389024] Xen: initializing cpu1
[    0.389036] CPU1: Booted secondary processor [411fd072]
[    0.421064] Detected PIPT I-cache on CPU2
[    0.421105] Xen: initializing cpu2
[    0.421119] CPU2: Booted secondary processor [411fd072]
[    0.453143] Detected PIPT I-cache on CPU3
[    0.453178] Xen: initializing cpu3
[    0.453190] CPU3: Booted secondary processor [411fd072]
[    0.485226] Detected PIPT I-cache on CPU4
[    0.485265] Xen: initializing cpu4
[    0.485278] CPU4: Booted secondary processor [411fd072]
[    0.517303] Detected PIPT I-cache on CPU5
[    0.517339] Xen: initializing cpu5
[    0.517351] CPU5: Booted secondary processor [411fd072]
[    0.549389] Detected PIPT I-cache on CPU6
[    0.549428] Xen: initializing cpu6
[    0.549442] CPU6: Booted secondary processor [411fd072]
[    0.581470] Detected PIPT I-cache on CPU7
[    0.581506] Xen: initializing cpu7
[    0.581518] CPU7: Booted secondary processor [411fd072]
[    0.581570] Brought up 8 CPUs
[    0.674360] SMP: Total of 8 processors activated.
[    0.679138] CPU features: detected feature: 32-bit EL0 Support
[    0.685040] CPU: All CPU(s) started at EL1

Regards,

> But unfortunatelly we'll have to get back to this in future. Turns out
> we need to know Xen's idea of vCPU id _before_ this vCPU starts
> executing code. On x86 we used ACPI_ID from MADT. Is there anything like
> it on ARM?
>
>>
>> [...]
>>
>>> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
>>> index 0f87db2..c833912 100644
>>> --- a/arch/x86/xen/enlighten.c
>>> +++ b/arch/x86/xen/enlighten.c
>>> @@ -1795,6 +1806,12 @@ static void __init init_hvm_pv_info(void)
>>>
>>>  xen_setup_features();
>>>
>>> +cpuid(base + 4, &eax, &ebx, &ecx, &edx);
>>> +if (eax & XEN_HVM_CPUID_VCPU_ID_PRESENT)
>>> +this_cpu_write(xen_vcpu_id, ebx);
>>> +else
>>> +this_cpu_write(xen_vcpu_id, smp_processor_id());
>>> +
>>>  pv_info.name = "Xen HVM";
>>>
>>>  xen_domain_type = XEN_HVM_DOMAIN;
>>> @@ -1806,6 +1823,10 @@ static int xen_hvm_cpu_notify(struct notifier_block *self, unsigned long action,
>>>  int cpu = (long)hcpu;
>>>  switch (action) {
>>>  case CPU_UP_PREPARE:
>>> +if (cpu_acpi_id(cpu) != U32_MAX)
>>> +per_cpu(xen_vcpu_id, cpu) = cpu_acpi_id(cpu);
>>> +else
>>> +per_cpu(xen_vcpu_id, cpu) = cpu;
>>
>> I have not tried myself. But looking at the code, the notifiers
>> xen_hvm_cpu_notifier and evtchn_fifo_cpu_notifier have the same
>> priority. So what does prevent the code above to be executed after the
>> event channel callback?
>
> Nothing, actually, but xen_hvm_guest_init() happens early and we always
> get these notifiers in the right order (and, reading Boris' reply, we're
> gonna make it explicit in future).
>
>>
>>>  xen_vcpu_setup(cpu);
>>>  if (xen_have_vector_callback) {
>>>  if (xen_feature(XENFEAT_hvm_safe_pvclock))
>>> diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h
>>> index 86abe07..648ce814 100644
>>> --- a/include/xen/xen-ops.h
>>> +++ b/include/xen/xen-ops.h
>>> @@ -9,6 +9,12 @@
>>>
>>>  DECLARE_PER_CPU(struct vcpu_info *, xen_vcpu);
>>>
>>> +DECLARE_PER_CPU(uint32_t, xen_vcpu_id);
>>> +static inline int xen_vcpu_nr(int cpu)
>>> +{
>>> +return per_cpu(xen_vcpu_id, cpu);
>>> +}
>>> +
>>>  void xen_arch_pre_suspend(void);
>>>  void xen_arch_post_suspend(int suspend_cancelled);
>>
>> Regards,
>


--
Regards,
Wei Chen
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Re: [PATCH linux v3 2/9] x86/acpi: store ACPI ids from MADT for future usage

[Thomas Gleixner]

On Tue, 26 Jul 2016, Vitaly Kuznetsov wrote:

So this patch made it's way into Linus tree via XEN w/o an ack or reviewed
by from the x86 maintainers.

Yes, we were on CC, but it's not that hard to ping the maintainers when
they do not respond on a particular patch.

The whole series ran under the cover letter subject:

 xen: pvhvm: support bootup on secondary vCPU

which suggests that this is a XEN internal affair. And I really have enough
stuff to look after so I don't dive into XEN internals if it's not
obviously required.

Let's look at this after the fact:

> Currently we don't save ACPI ids (unlike LAPIC ids which go to
> x86_cpu_to_apicid) from MADT and we may need this information later.

may need? Maybe, or maybe not.

> Particularly, ACPI ids is the only existent way for a PVHVM Xen guest
> to figure out Xen's idea of its vCPUs ids before these CPUs boot and
> in some cases these ids diverge from Linux's cpu ids.

I have no idea what this sentence means and what kind of divergence this is
talking about.

Dammit, if stuff gets slammed into the x86 tree w/o a proper notice, then
the minimum requirement is at least an understandable changelog which
allows non XEN experts to figure out WHY this is necessary and WHAT this is
about.

> @@ -714,7 +722,7 @@ int acpi_map_cpu(acpi_handle handle, phys_cpuid_t physid, int *pcpu)
>  {
>  	int cpu;
>  
> -	cpu = acpi_register_lapic(physid, ACPI_MADT_ENABLED);
> +	cpu = acpi_register_lapic(physid, U32_MAX, ACPI_MADT_ENABLED);

What the heck is this? ACPIID is U32_MAX? Sure, that's obvious as it can
get and the well thought out comment above this call explains it nicely.

Yes, I know it has been fixed later, but this crap should not have been
merged in the first place.

Yours grumpy

      tglx

Re: [PATCH linux v3 2/9] x86/acpi: store ACPI ids from MADT for future usage

[Boris Ostrovsky]

On 03/01/2017 02:54 PM, Thomas Gleixner wrote:
>
> Yes, I know it has been fixed later, but this crap should not have been
> merged in the first place.
>
> Yours grumpy
>
>       tglx


Yes, we dropped the ball on this, this shouldn't have gone in without
x86 maintainers' review.

-boris