xen,arm: enable cpu_hotplug

xen,arm: enable cpu_hotplug

[Stefano Stabellini]

Hi all,

this small patch series enable cpu_hotplug in ARM and ARM64 guests,
using the PV path to plug and unplug the cpus and psci to enable/disable
them.


Stefano Stabellini (3):
      xen/arm: Enable cpu_hotplug.c
      xen, cpu_hotplug: call device_offline instead of cpu_down
      xen/arm: don't try to re-register vcpu_info on cpu_hotplug.

 arch/arm/include/asm/xen/hypervisor.h |    8 ++++++++
 arch/arm/xen/enlighten.c              |    6 ++++++
 arch/x86/include/asm/xen/hypervisor.h |    5 +++++
 arch/x86/xen/enlighten.c              |   15 +++++++++++++++
 drivers/xen/Makefile                  |    2 --
 drivers/xen/cpu_hotplug.c             |    9 ++++++---
 6 files changed, 40 insertions(+), 5 deletions(-)

Cheers,

Stefano

xen,arm: enable cpu_hotplug

[Stefano Stabellini]

Hi all,

this small patch series enable cpu_hotplug in ARM and ARM64 guests,
using the PV path to plug and unplug the cpus and psci to enable/disable
them.


Stefano Stabellini (3):
      xen/arm: Enable cpu_hotplug.c
      xen, cpu_hotplug: call device_offline instead of cpu_down
      xen/arm: don't try to re-register vcpu_info on cpu_hotplug.

 arch/arm/include/asm/xen/hypervisor.h |    8 ++++++++
 arch/arm/xen/enlighten.c              |    6 ++++++
 arch/x86/include/asm/xen/hypervisor.h |    5 +++++
 arch/x86/xen/enlighten.c              |   15 +++++++++++++++
 drivers/xen/Makefile                  |    2 --
 drivers/xen/cpu_hotplug.c             |    9 ++++++---
 6 files changed, 40 insertions(+), 5 deletions(-)

Cheers,

Stefano

[PATCH 1/3] xen/arm: Enable cpu_hotplug.c

[Stefano Stabellini]

Build cpu_hotplug for ARM and ARM64 guests.

Rename arch_(un)register_cpu to xen_(un)register_cpu and provide an
empty implementation on ARM and ARM64. On x86 just call
arch_(un)register_cpu as we are already doing.

Initialize cpu_hotplug on ARM.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 arch/arm/include/asm/xen/hypervisor.h |    8 ++++++++
 arch/x86/include/asm/xen/hypervisor.h |    5 +++++
 arch/x86/xen/enlighten.c              |   15 +++++++++++++++
 drivers/xen/Makefile                  |    2 --
 drivers/xen/cpu_hotplug.c             |    6 ++++--
 5 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/arch/arm/include/asm/xen/hypervisor.h b/arch/arm/include/asm/xen/hypervisor.h
index 04ff8e7..2bc418a 100644
--- a/arch/arm/include/asm/xen/hypervisor.h
+++ b/arch/arm/include/asm/xen/hypervisor.h
@@ -26,4 +26,12 @@ void __init xen_early_init(void);
 static inline void xen_early_init(void) { return; }
 #endif
 
+static inline void xen_arch_register_cpu(int num)
+{
+}
+
+static inline void xen_arch_unregister_cpu(int num)
+{
+}
+
 #endif /* _ASM_ARM_XEN_HYPERVISOR_H */
diff --git a/arch/x86/include/asm/xen/hypervisor.h b/arch/x86/include/asm/xen/hypervisor.h
index d866959..8b2d4be 100644
--- a/arch/x86/include/asm/xen/hypervisor.h
+++ b/arch/x86/include/asm/xen/hypervisor.h
@@ -57,4 +57,9 @@ static inline bool xen_x2apic_para_available(void)
 }
 #endif
 
+#ifdef CONFIG_HOTPLUG_CPU
+void xen_arch_register_cpu(int num);
+void xen_arch_unregister_cpu(int num);
+#endif
+
 #endif /* _ASM_X86_XEN_HYPERVISOR_H */
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 11d6fb4..ba62d8e 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -71,6 +71,7 @@
 #include <asm/mwait.h>
 #include <asm/pci_x86.h>
 #include <asm/pat.h>
+#include <asm/cpu.h>
 
 #ifdef CONFIG_ACPI
 #include <linux/acpi.h>
@@ -1868,3 +1869,17 @@ const struct hypervisor_x86 x86_hyper_xen = {
 	.set_cpu_features       = xen_set_cpu_features,
 };
 EXPORT_SYMBOL(x86_hyper_xen);
+
+#ifdef CONFIG_HOTPLUG_CPU
+void xen_arch_register_cpu(int num)
+{
+	arch_register_cpu(num);
+}
+EXPORT_SYMBOL(xen_arch_register_cpu);
+
+void xen_arch_unregister_cpu(int num)
+{
+	arch_unregister_cpu(num);
+}
+EXPORT_SYMBOL(xen_arch_unregister_cpu);
+#endif
diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
index e293bc5..aa8a7f7 100644
--- a/drivers/xen/Makefile
+++ b/drivers/xen/Makefile
@@ -1,6 +1,4 @@
-ifeq ($(filter y, $(CONFIG_ARM) $(CONFIG_ARM64)),)
 obj-$(CONFIG_HOTPLUG_CPU)		+= cpu_hotplug.o
-endif
 obj-$(CONFIG_X86)			+= fallback.o
 obj-y	+= grant-table.o features.o balloon.o manage.o preempt.o
 obj-y	+= events/
diff --git a/drivers/xen/cpu_hotplug.c b/drivers/xen/cpu_hotplug.c
index cc6513a..122b351 100644
--- a/drivers/xen/cpu_hotplug.c
+++ b/drivers/xen/cpu_hotplug.c
@@ -11,7 +11,7 @@
 static void enable_hotplug_cpu(int cpu)
 {
 	if (!cpu_present(cpu))
-		arch_register_cpu(cpu);
+		xen_arch_register_cpu(cpu);
 
 	set_cpu_present(cpu, true);
 }
@@ -19,7 +19,7 @@ static void enable_hotplug_cpu(int cpu)
 static void disable_hotplug_cpu(int cpu)
 {
 	if (cpu_present(cpu))
-		arch_unregister_cpu(cpu);
+		xen_arch_unregister_cpu(cpu);
 
 	set_cpu_present(cpu, false);
 }
@@ -102,8 +102,10 @@ static int __init setup_vcpu_hotplug_event(void)
 	static struct notifier_block xsn_cpu = {
 		.notifier_call = setup_cpu_watcher };
 
+#ifdef CONFIG_X86
 	if (!xen_pv_domain())
 		return -ENODEV;
+#endif
 
 	register_xenstore_notifier(&xsn_cpu);
 
-- 
1.7.9.5

[PATCH 1/3] xen/arm: Enable cpu_hotplug.c

[Stefano Stabellini]

Build cpu_hotplug for ARM and ARM64 guests.

Rename arch_(un)register_cpu to xen_(un)register_cpu and provide an
empty implementation on ARM and ARM64. On x86 just call
arch_(un)register_cpu as we are already doing.

Initialize cpu_hotplug on ARM.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 arch/arm/include/asm/xen/hypervisor.h |    8 ++++++++
 arch/x86/include/asm/xen/hypervisor.h |    5 +++++
 arch/x86/xen/enlighten.c              |   15 +++++++++++++++
 drivers/xen/Makefile                  |    2 --
 drivers/xen/cpu_hotplug.c             |    6 ++++--
 5 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/arch/arm/include/asm/xen/hypervisor.h b/arch/arm/include/asm/xen/hypervisor.h
index 04ff8e7..2bc418a 100644
--- a/arch/arm/include/asm/xen/hypervisor.h
+++ b/arch/arm/include/asm/xen/hypervisor.h
@@ -26,4 +26,12 @@ void __init xen_early_init(void);
 static inline void xen_early_init(void) { return; }
 #endif
 
+static inline void xen_arch_register_cpu(int num)
+{
+}
+
+static inline void xen_arch_unregister_cpu(int num)
+{
+}
+
 #endif /* _ASM_ARM_XEN_HYPERVISOR_H */
diff --git a/arch/x86/include/asm/xen/hypervisor.h b/arch/x86/include/asm/xen/hypervisor.h
index d866959..8b2d4be 100644
--- a/arch/x86/include/asm/xen/hypervisor.h
+++ b/arch/x86/include/asm/xen/hypervisor.h
@@ -57,4 +57,9 @@ static inline bool xen_x2apic_para_available(void)
 }
 #endif
 
+#ifdef CONFIG_HOTPLUG_CPU
+void xen_arch_register_cpu(int num);
+void xen_arch_unregister_cpu(int num);
+#endif
+
 #endif /* _ASM_X86_XEN_HYPERVISOR_H */
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 11d6fb4..ba62d8e 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -71,6 +71,7 @@
 #include <asm/mwait.h>
 #include <asm/pci_x86.h>
 #include <asm/pat.h>
+#include <asm/cpu.h>
 
 #ifdef CONFIG_ACPI
 #include <linux/acpi.h>
@@ -1868,3 +1869,17 @@ const struct hypervisor_x86 x86_hyper_xen = {
 	.set_cpu_features       = xen_set_cpu_features,
 };
 EXPORT_SYMBOL(x86_hyper_xen);
+
+#ifdef CONFIG_HOTPLUG_CPU
+void xen_arch_register_cpu(int num)
+{
+	arch_register_cpu(num);
+}
+EXPORT_SYMBOL(xen_arch_register_cpu);
+
+void xen_arch_unregister_cpu(int num)
+{
+	arch_unregister_cpu(num);
+}
+EXPORT_SYMBOL(xen_arch_unregister_cpu);
+#endif
diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
index e293bc5..aa8a7f7 100644
--- a/drivers/xen/Makefile
+++ b/drivers/xen/Makefile
@@ -1,6 +1,4 @@
-ifeq ($(filter y, $(CONFIG_ARM) $(CONFIG_ARM64)),)
 obj-$(CONFIG_HOTPLUG_CPU)		+= cpu_hotplug.o
-endif
 obj-$(CONFIG_X86)			+= fallback.o
 obj-y	+= grant-table.o features.o balloon.o manage.o preempt.o
 obj-y	+= events/
diff --git a/drivers/xen/cpu_hotplug.c b/drivers/xen/cpu_hotplug.c
index cc6513a..122b351 100644
--- a/drivers/xen/cpu_hotplug.c
+++ b/drivers/xen/cpu_hotplug.c
@@ -11,7 +11,7 @@
 static void enable_hotplug_cpu(int cpu)
 {
 	if (!cpu_present(cpu))
-		arch_register_cpu(cpu);
+		xen_arch_register_cpu(cpu);
 
 	set_cpu_present(cpu, true);
 }
@@ -19,7 +19,7 @@ static void enable_hotplug_cpu(int cpu)
 static void disable_hotplug_cpu(int cpu)
 {
 	if (cpu_present(cpu))
-		arch_unregister_cpu(cpu);
+		xen_arch_unregister_cpu(cpu);
 
 	set_cpu_present(cpu, false);
 }
@@ -102,8 +102,10 @@ static int __init setup_vcpu_hotplug_event(void)
 	static struct notifier_block xsn_cpu = {
 		.notifier_call = setup_cpu_watcher };
 
+#ifdef CONFIG_X86
 	if (!xen_pv_domain())
 		return -ENODEV;
+#endif
 
 	register_xenstore_notifier(&xsn_cpu);
 
-- 
1.7.9.5

[PATCH 2/3] xen, cpu_hotplug: call device_offline instead of cpu_down

[Stefano Stabellini]

When offlining a cpu, instead of cpu_down, call device_offline, which
also takes care of updating the cpu.dev.offline field. This keeps the
sysfs file /sys/devices/system/cpu/cpuN/online, up to date.  Also move
the call to disable_hotplug_cpu, because it makes more sense to have it
there.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 drivers/xen/cpu_hotplug.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/xen/cpu_hotplug.c b/drivers/xen/cpu_hotplug.c
index 122b351..785f9ce 100644
--- a/drivers/xen/cpu_hotplug.c
+++ b/drivers/xen/cpu_hotplug.c
@@ -18,6 +18,8 @@ static void enable_hotplug_cpu(int cpu)
 
 static void disable_hotplug_cpu(int cpu)
 {
+	if (cpu_online(cpu))
+		device_offline(get_cpu_device(cpu));
 	if (cpu_present(cpu))
 		xen_arch_unregister_cpu(cpu);
 
@@ -55,7 +57,6 @@ static void vcpu_hotplug(unsigned int cpu)
 		enable_hotplug_cpu(cpu);
 		break;
 	case 0:
-		(void)cpu_down(cpu);
 		disable_hotplug_cpu(cpu);
 		break;
 	default:
-- 
1.7.9.5

[PATCH 2/3] xen, cpu_hotplug: call device_offline instead of cpu_down

[Stefano Stabellini]

When offlining a cpu, instead of cpu_down, call device_offline, which
also takes care of updating the cpu.dev.offline field. This keeps the
sysfs file /sys/devices/system/cpu/cpuN/online, up to date.  Also move
the call to disable_hotplug_cpu, because it makes more sense to have it
there.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 drivers/xen/cpu_hotplug.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/xen/cpu_hotplug.c b/drivers/xen/cpu_hotplug.c
index 122b351..785f9ce 100644
--- a/drivers/xen/cpu_hotplug.c
+++ b/drivers/xen/cpu_hotplug.c
@@ -18,6 +18,8 @@ static void enable_hotplug_cpu(int cpu)
 
 static void disable_hotplug_cpu(int cpu)
 {
+	if (cpu_online(cpu))
+		device_offline(get_cpu_device(cpu));
 	if (cpu_present(cpu))
 		xen_arch_unregister_cpu(cpu);
 
@@ -55,7 +57,6 @@ static void vcpu_hotplug(unsigned int cpu)
 		enable_hotplug_cpu(cpu);
 		break;
 	case 0:
-		(void)cpu_down(cpu);
 		disable_hotplug_cpu(cpu);
 		break;
 	default:
-- 
1.7.9.5

[PATCH 3/3] xen/arm: don't try to re-register vcpu_info on cpu_hotplug.

[Stefano Stabellini]

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 arch/arm/xen/enlighten.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index 6c09cc4..59f5421 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -93,6 +93,12 @@ static void xen_percpu_init(void)
 	int err;
 	int cpu = get_cpu();
 
+	/* vcpu_info already registered, can happen with cpu-hotplug */
+	if (per_cpu(xen_vcpu, cpu) != NULL) {
+		put_cpu();
+		return;
+	}
+
 	pr_info("Xen: initializing cpu%d\n", cpu);
 	vcpup = per_cpu_ptr(xen_vcpu_info, cpu);
 
-- 
1.7.9.5

[PATCH 3/3] xen/arm: don't try to re-register vcpu_info on cpu_hotplug.

[Stefano Stabellini]

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 arch/arm/xen/enlighten.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index 6c09cc4..59f5421 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -93,6 +93,12 @@ static void xen_percpu_init(void)
 	int err;
 	int cpu = get_cpu();
 
+	/* vcpu_info already registered, can happen with cpu-hotplug */
+	if (per_cpu(xen_vcpu, cpu) != NULL) {
+		put_cpu();
+		return;
+	}
+
 	pr_info("Xen: initializing cpu%d\n", cpu);
 	vcpup = per_cpu_ptr(xen_vcpu_info, cpu);
 
-- 
1.7.9.5

[Xen-devel] [PATCH 1/3] xen/arm: Enable cpu_hotplug.c

[Konrad Rzeszutek Wilk]

On October 14, 2015 1:49:55 PM EDT, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote:
>Build cpu_hotplug for ARM and ARM64 guests.
>
>Rename arch_(un)register_cpu to xen_(un)register_cpu and provide an
>empty implementation on ARM and ARM64. On x86 just call
>arch_(un)register_cpu as we are already doing.
>
>Initialize cpu_hotplug on ARM.

Have you tested this on x86? As your changes touch the generic code path.

>
>Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>---
> arch/arm/include/asm/xen/hypervisor.h |    8 ++++++++
> arch/x86/include/asm/xen/hypervisor.h |    5 +++++
> arch/x86/xen/enlighten.c              |   15 +++++++++++++++
> drivers/xen/Makefile                  |    2 --
> drivers/xen/cpu_hotplug.c             |    6 ++++--
> 5 files changed, 32 insertions(+), 4 deletions(-)
>
>diff --git a/arch/arm/include/asm/xen/hypervisor.h
>b/arch/arm/include/asm/xen/hypervisor.h
>index 04ff8e7..2bc418a 100644
>--- a/arch/arm/include/asm/xen/hypervisor.h
>+++ b/arch/arm/include/asm/xen/hypervisor.h
>@@ -26,4 +26,12 @@ void __init xen_early_init(void);
> static inline void xen_early_init(void) { return; }
> #endif
> 
>+static inline void xen_arch_register_cpu(int num)
>+{
>+}
>+
>+static inline void xen_arch_unregister_cpu(int num)
>+{
>+}
>+
> #endif /* _ASM_ARM_XEN_HYPERVISOR_H */
>diff --git a/arch/x86/include/asm/xen/hypervisor.h
>b/arch/x86/include/asm/xen/hypervisor.h
>index d866959..8b2d4be 100644
>--- a/arch/x86/include/asm/xen/hypervisor.h
>+++ b/arch/x86/include/asm/xen/hypervisor.h
>@@ -57,4 +57,9 @@ static inline bool xen_x2apic_para_available(void)
> }
> #endif
> 
>+#ifdef CONFIG_HOTPLUG_CPU
>+void xen_arch_register_cpu(int num);
>+void xen_arch_unregister_cpu(int num);
>+#endif
>+
> #endif /* _ASM_X86_XEN_HYPERVISOR_H */
>diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
>index 11d6fb4..ba62d8e 100644
>--- a/arch/x86/xen/enlighten.c
>+++ b/arch/x86/xen/enlighten.c
>@@ -71,6 +71,7 @@
> #include <asm/mwait.h>
> #include <asm/pci_x86.h>
> #include <asm/pat.h>
>+#include <asm/cpu.h>
> 
> #ifdef CONFIG_ACPI
> #include <linux/acpi.h>
>@@ -1868,3 +1869,17 @@ const struct hypervisor_x86 x86_hyper_xen = {
> 	.set_cpu_features       = xen_set_cpu_features,
> };
> EXPORT_SYMBOL(x86_hyper_xen);
>+
>+#ifdef CONFIG_HOTPLUG_CPU
>+void xen_arch_register_cpu(int num)
>+{
>+	arch_register_cpu(num);
>+}
>+EXPORT_SYMBOL(xen_arch_register_cpu);
>+
>+void xen_arch_unregister_cpu(int num)
>+{
>+	arch_unregister_cpu(num);
>+}
>+EXPORT_SYMBOL(xen_arch_unregister_cpu);
>+#endif
>diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
>index e293bc5..aa8a7f7 100644
>--- a/drivers/xen/Makefile
>+++ b/drivers/xen/Makefile
>@@ -1,6 +1,4 @@
>-ifeq ($(filter y, $(CONFIG_ARM) $(CONFIG_ARM64)),)
> obj-$(CONFIG_HOTPLUG_CPU)		+= cpu_hotplug.o
>-endif
> obj-$(CONFIG_X86)			+= fallback.o
> obj-y	+= grant-table.o features.o balloon.o manage.o preempt.o
> obj-y	+= events/
>diff --git a/drivers/xen/cpu_hotplug.c b/drivers/xen/cpu_hotplug.c
>index cc6513a..122b351 100644
>--- a/drivers/xen/cpu_hotplug.c
>+++ b/drivers/xen/cpu_hotplug.c
>@@ -11,7 +11,7 @@
> static void enable_hotplug_cpu(int cpu)
> {
> 	if (!cpu_present(cpu))
>-		arch_register_cpu(cpu);
>+		xen_arch_register_cpu(cpu);
> 
> 	set_cpu_present(cpu, true);
> }
>@@ -19,7 +19,7 @@ static void enable_hotplug_cpu(int cpu)
> static void disable_hotplug_cpu(int cpu)
> {
> 	if (cpu_present(cpu))
>-		arch_unregister_cpu(cpu);
>+		xen_arch_unregister_cpu(cpu);
> 
> 	set_cpu_present(cpu, false);
> }
>@@ -102,8 +102,10 @@ static int __init setup_vcpu_hotplug_event(void)
> 	static struct notifier_block xsn_cpu = {
> 		.notifier_call = setup_cpu_watcher };
> 
>+#ifdef CONFIG_X86
> 	if (!xen_pv_domain())
> 		return -ENODEV;
>+#endif
> 
> 	register_xenstore_notifier(&xsn_cpu);
> 

Re: [Xen-devel] [PATCH 1/3] xen/arm: Enable cpu_hotplug.c

[Konrad Rzeszutek Wilk]

On October 14, 2015 1:49:55 PM EDT, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote:
>Build cpu_hotplug for ARM and ARM64 guests.
>
>Rename arch_(un)register_cpu to xen_(un)register_cpu and provide an
>empty implementation on ARM and ARM64. On x86 just call
>arch_(un)register_cpu as we are already doing.
>
>Initialize cpu_hotplug on ARM.

Have you tested this on x86? As your changes touch the generic code path.

>
>Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>---
> arch/arm/include/asm/xen/hypervisor.h |    8 ++++++++
> arch/x86/include/asm/xen/hypervisor.h |    5 +++++
> arch/x86/xen/enlighten.c              |   15 +++++++++++++++
> drivers/xen/Makefile                  |    2 --
> drivers/xen/cpu_hotplug.c             |    6 ++++--
> 5 files changed, 32 insertions(+), 4 deletions(-)
>
>diff --git a/arch/arm/include/asm/xen/hypervisor.h
>b/arch/arm/include/asm/xen/hypervisor.h
>index 04ff8e7..2bc418a 100644
>--- a/arch/arm/include/asm/xen/hypervisor.h
>+++ b/arch/arm/include/asm/xen/hypervisor.h
>@@ -26,4 +26,12 @@ void __init xen_early_init(void);
> static inline void xen_early_init(void) { return; }
> #endif
> 
>+static inline void xen_arch_register_cpu(int num)
>+{
>+}
>+
>+static inline void xen_arch_unregister_cpu(int num)
>+{
>+}
>+
> #endif /* _ASM_ARM_XEN_HYPERVISOR_H */
>diff --git a/arch/x86/include/asm/xen/hypervisor.h
>b/arch/x86/include/asm/xen/hypervisor.h
>index d866959..8b2d4be 100644
>--- a/arch/x86/include/asm/xen/hypervisor.h
>+++ b/arch/x86/include/asm/xen/hypervisor.h
>@@ -57,4 +57,9 @@ static inline bool xen_x2apic_para_available(void)
> }
> #endif
> 
>+#ifdef CONFIG_HOTPLUG_CPU
>+void xen_arch_register_cpu(int num);
>+void xen_arch_unregister_cpu(int num);
>+#endif
>+
> #endif /* _ASM_X86_XEN_HYPERVISOR_H */
>diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
>index 11d6fb4..ba62d8e 100644
>--- a/arch/x86/xen/enlighten.c
>+++ b/arch/x86/xen/enlighten.c
>@@ -71,6 +71,7 @@
> #include <asm/mwait.h>
> #include <asm/pci_x86.h>
> #include <asm/pat.h>
>+#include <asm/cpu.h>
> 
> #ifdef CONFIG_ACPI
> #include <linux/acpi.h>
>@@ -1868,3 +1869,17 @@ const struct hypervisor_x86 x86_hyper_xen = {
> 	.set_cpu_features       = xen_set_cpu_features,
> };
> EXPORT_SYMBOL(x86_hyper_xen);
>+
>+#ifdef CONFIG_HOTPLUG_CPU
>+void xen_arch_register_cpu(int num)
>+{
>+	arch_register_cpu(num);
>+}
>+EXPORT_SYMBOL(xen_arch_register_cpu);
>+
>+void xen_arch_unregister_cpu(int num)
>+{
>+	arch_unregister_cpu(num);
>+}
>+EXPORT_SYMBOL(xen_arch_unregister_cpu);
>+#endif
>diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
>index e293bc5..aa8a7f7 100644
>--- a/drivers/xen/Makefile
>+++ b/drivers/xen/Makefile
>@@ -1,6 +1,4 @@
>-ifeq ($(filter y, $(CONFIG_ARM) $(CONFIG_ARM64)),)
> obj-$(CONFIG_HOTPLUG_CPU)		+= cpu_hotplug.o
>-endif
> obj-$(CONFIG_X86)			+= fallback.o
> obj-y	+= grant-table.o features.o balloon.o manage.o preempt.o
> obj-y	+= events/
>diff --git a/drivers/xen/cpu_hotplug.c b/drivers/xen/cpu_hotplug.c
>index cc6513a..122b351 100644
>--- a/drivers/xen/cpu_hotplug.c
>+++ b/drivers/xen/cpu_hotplug.c
>@@ -11,7 +11,7 @@
> static void enable_hotplug_cpu(int cpu)
> {
> 	if (!cpu_present(cpu))
>-		arch_register_cpu(cpu);
>+		xen_arch_register_cpu(cpu);
> 
> 	set_cpu_present(cpu, true);
> }
>@@ -19,7 +19,7 @@ static void enable_hotplug_cpu(int cpu)
> static void disable_hotplug_cpu(int cpu)
> {
> 	if (cpu_present(cpu))
>-		arch_unregister_cpu(cpu);
>+		xen_arch_unregister_cpu(cpu);
> 
> 	set_cpu_present(cpu, false);
> }
>@@ -102,8 +102,10 @@ static int __init setup_vcpu_hotplug_event(void)
> 	static struct notifier_block xsn_cpu = {
> 		.notifier_call = setup_cpu_watcher };
> 
>+#ifdef CONFIG_X86
> 	if (!xen_pv_domain())
> 		return -ENODEV;
>+#endif
> 
> 	register_xenstore_notifier(&xsn_cpu);
> 

[Xen-devel] [PATCH 1/3] xen/arm: Enable cpu_hotplug.c

[Stefano Stabellini]

On Wed, 14 Oct 2015, Konrad Rzeszutek Wilk wrote:
> On October 14, 2015 1:49:55 PM EDT, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote:
> >Build cpu_hotplug for ARM and ARM64 guests.
> >
> >Rename arch_(un)register_cpu to xen_(un)register_cpu and provide an
> >empty implementation on ARM and ARM64. On x86 just call
> >arch_(un)register_cpu as we are already doing.
> >
> >Initialize cpu_hotplug on ARM.
> 
> Have you tested this on x86? As your changes touch the generic code path.

Only build-tested at this stage, because all my x86 test boxes are
unavailable at the moment (they are being relocated). As soon as they
come back online I will test.


> >Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> >---
> > arch/arm/include/asm/xen/hypervisor.h |    8 ++++++++
> > arch/x86/include/asm/xen/hypervisor.h |    5 +++++
> > arch/x86/xen/enlighten.c              |   15 +++++++++++++++
> > drivers/xen/Makefile                  |    2 --
> > drivers/xen/cpu_hotplug.c             |    6 ++++--
> > 5 files changed, 32 insertions(+), 4 deletions(-)
> >
> >diff --git a/arch/arm/include/asm/xen/hypervisor.h
> >b/arch/arm/include/asm/xen/hypervisor.h
> >index 04ff8e7..2bc418a 100644
> >--- a/arch/arm/include/asm/xen/hypervisor.h
> >+++ b/arch/arm/include/asm/xen/hypervisor.h
> >@@ -26,4 +26,12 @@ void __init xen_early_init(void);
> > static inline void xen_early_init(void) { return; }
> > #endif
> > 
> >+static inline void xen_arch_register_cpu(int num)
> >+{
> >+}
> >+
> >+static inline void xen_arch_unregister_cpu(int num)
> >+{
> >+}
> >+
> > #endif /* _ASM_ARM_XEN_HYPERVISOR_H */
> >diff --git a/arch/x86/include/asm/xen/hypervisor.h
> >b/arch/x86/include/asm/xen/hypervisor.h
> >index d866959..8b2d4be 100644
> >--- a/arch/x86/include/asm/xen/hypervisor.h
> >+++ b/arch/x86/include/asm/xen/hypervisor.h
> >@@ -57,4 +57,9 @@ static inline bool xen_x2apic_para_available(void)
> > }
> > #endif
> > 
> >+#ifdef CONFIG_HOTPLUG_CPU
> >+void xen_arch_register_cpu(int num);
> >+void xen_arch_unregister_cpu(int num);
> >+#endif
> >+
> > #endif /* _ASM_X86_XEN_HYPERVISOR_H */
> >diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> >index 11d6fb4..ba62d8e 100644
> >--- a/arch/x86/xen/enlighten.c
> >+++ b/arch/x86/xen/enlighten.c
> >@@ -71,6 +71,7 @@
> > #include <asm/mwait.h>
> > #include <asm/pci_x86.h>
> > #include <asm/pat.h>
> >+#include <asm/cpu.h>
> > 
> > #ifdef CONFIG_ACPI
> > #include <linux/acpi.h>
> >@@ -1868,3 +1869,17 @@ const struct hypervisor_x86 x86_hyper_xen = {
> > 	.set_cpu_features       = xen_set_cpu_features,
> > };
> > EXPORT_SYMBOL(x86_hyper_xen);
> >+
> >+#ifdef CONFIG_HOTPLUG_CPU
> >+void xen_arch_register_cpu(int num)
> >+{
> >+	arch_register_cpu(num);
> >+}
> >+EXPORT_SYMBOL(xen_arch_register_cpu);
> >+
> >+void xen_arch_unregister_cpu(int num)
> >+{
> >+	arch_unregister_cpu(num);
> >+}
> >+EXPORT_SYMBOL(xen_arch_unregister_cpu);
> >+#endif
> >diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
> >index e293bc5..aa8a7f7 100644
> >--- a/drivers/xen/Makefile
> >+++ b/drivers/xen/Makefile
> >@@ -1,6 +1,4 @@
> >-ifeq ($(filter y, $(CONFIG_ARM) $(CONFIG_ARM64)),)
> > obj-$(CONFIG_HOTPLUG_CPU)		+= cpu_hotplug.o
> >-endif
> > obj-$(CONFIG_X86)			+= fallback.o
> > obj-y	+= grant-table.o features.o balloon.o manage.o preempt.o
> > obj-y	+= events/
> >diff --git a/drivers/xen/cpu_hotplug.c b/drivers/xen/cpu_hotplug.c
> >index cc6513a..122b351 100644
> >--- a/drivers/xen/cpu_hotplug.c
> >+++ b/drivers/xen/cpu_hotplug.c
> >@@ -11,7 +11,7 @@
> > static void enable_hotplug_cpu(int cpu)
> > {
> > 	if (!cpu_present(cpu))
> >-		arch_register_cpu(cpu);
> >+		xen_arch_register_cpu(cpu);
> > 
> > 	set_cpu_present(cpu, true);
> > }
> >@@ -19,7 +19,7 @@ static void enable_hotplug_cpu(int cpu)
> > static void disable_hotplug_cpu(int cpu)
> > {
> > 	if (cpu_present(cpu))
> >-		arch_unregister_cpu(cpu);
> >+		xen_arch_unregister_cpu(cpu);
> > 
> > 	set_cpu_present(cpu, false);
> > }
> >@@ -102,8 +102,10 @@ static int __init setup_vcpu_hotplug_event(void)
> > 	static struct notifier_block xsn_cpu = {
> > 		.notifier_call = setup_cpu_watcher };
> > 
> >+#ifdef CONFIG_X86
> > 	if (!xen_pv_domain())
> > 		return -ENODEV;
> >+#endif
> > 
> > 	register_xenstore_notifier(&xsn_cpu);
> > 
> 
> 

Re: [Xen-devel] [PATCH 1/3] xen/arm: Enable cpu_hotplug.c

[Stefano Stabellini]

On Wed, 14 Oct 2015, Konrad Rzeszutek Wilk wrote:
> On October 14, 2015 1:49:55 PM EDT, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote:
> >Build cpu_hotplug for ARM and ARM64 guests.
> >
> >Rename arch_(un)register_cpu to xen_(un)register_cpu and provide an
> >empty implementation on ARM and ARM64. On x86 just call
> >arch_(un)register_cpu as we are already doing.
> >
> >Initialize cpu_hotplug on ARM.
> 
> Have you tested this on x86? As your changes touch the generic code path.

Only build-tested at this stage, because all my x86 test boxes are
unavailable at the moment (they are being relocated). As soon as they
come back online I will test.


> >Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> >---
> > arch/arm/include/asm/xen/hypervisor.h |    8 ++++++++
> > arch/x86/include/asm/xen/hypervisor.h |    5 +++++
> > arch/x86/xen/enlighten.c              |   15 +++++++++++++++
> > drivers/xen/Makefile                  |    2 --
> > drivers/xen/cpu_hotplug.c             |    6 ++++--
> > 5 files changed, 32 insertions(+), 4 deletions(-)
> >
> >diff --git a/arch/arm/include/asm/xen/hypervisor.h
> >b/arch/arm/include/asm/xen/hypervisor.h
> >index 04ff8e7..2bc418a 100644
> >--- a/arch/arm/include/asm/xen/hypervisor.h
> >+++ b/arch/arm/include/asm/xen/hypervisor.h
> >@@ -26,4 +26,12 @@ void __init xen_early_init(void);
> > static inline void xen_early_init(void) { return; }
> > #endif
> > 
> >+static inline void xen_arch_register_cpu(int num)
> >+{
> >+}
> >+
> >+static inline void xen_arch_unregister_cpu(int num)
> >+{
> >+}
> >+
> > #endif /* _ASM_ARM_XEN_HYPERVISOR_H */
> >diff --git a/arch/x86/include/asm/xen/hypervisor.h
> >b/arch/x86/include/asm/xen/hypervisor.h
> >index d866959..8b2d4be 100644
> >--- a/arch/x86/include/asm/xen/hypervisor.h
> >+++ b/arch/x86/include/asm/xen/hypervisor.h
> >@@ -57,4 +57,9 @@ static inline bool xen_x2apic_para_available(void)
> > }
> > #endif
> > 
> >+#ifdef CONFIG_HOTPLUG_CPU
> >+void xen_arch_register_cpu(int num);
> >+void xen_arch_unregister_cpu(int num);
> >+#endif
> >+
> > #endif /* _ASM_X86_XEN_HYPERVISOR_H */
> >diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> >index 11d6fb4..ba62d8e 100644
> >--- a/arch/x86/xen/enlighten.c
> >+++ b/arch/x86/xen/enlighten.c
> >@@ -71,6 +71,7 @@
> > #include <asm/mwait.h>
> > #include <asm/pci_x86.h>
> > #include <asm/pat.h>
> >+#include <asm/cpu.h>
> > 
> > #ifdef CONFIG_ACPI
> > #include <linux/acpi.h>
> >@@ -1868,3 +1869,17 @@ const struct hypervisor_x86 x86_hyper_xen = {
> > 	.set_cpu_features       = xen_set_cpu_features,
> > };
> > EXPORT_SYMBOL(x86_hyper_xen);
> >+
> >+#ifdef CONFIG_HOTPLUG_CPU
> >+void xen_arch_register_cpu(int num)
> >+{
> >+	arch_register_cpu(num);
> >+}
> >+EXPORT_SYMBOL(xen_arch_register_cpu);
> >+
> >+void xen_arch_unregister_cpu(int num)
> >+{
> >+	arch_unregister_cpu(num);
> >+}
> >+EXPORT_SYMBOL(xen_arch_unregister_cpu);
> >+#endif
> >diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
> >index e293bc5..aa8a7f7 100644
> >--- a/drivers/xen/Makefile
> >+++ b/drivers/xen/Makefile
> >@@ -1,6 +1,4 @@
> >-ifeq ($(filter y, $(CONFIG_ARM) $(CONFIG_ARM64)),)
> > obj-$(CONFIG_HOTPLUG_CPU)		+= cpu_hotplug.o
> >-endif
> > obj-$(CONFIG_X86)			+= fallback.o
> > obj-y	+= grant-table.o features.o balloon.o manage.o preempt.o
> > obj-y	+= events/
> >diff --git a/drivers/xen/cpu_hotplug.c b/drivers/xen/cpu_hotplug.c
> >index cc6513a..122b351 100644
> >--- a/drivers/xen/cpu_hotplug.c
> >+++ b/drivers/xen/cpu_hotplug.c
> >@@ -11,7 +11,7 @@
> > static void enable_hotplug_cpu(int cpu)
> > {
> > 	if (!cpu_present(cpu))
> >-		arch_register_cpu(cpu);
> >+		xen_arch_register_cpu(cpu);
> > 
> > 	set_cpu_present(cpu, true);
> > }
> >@@ -19,7 +19,7 @@ static void enable_hotplug_cpu(int cpu)
> > static void disable_hotplug_cpu(int cpu)
> > {
> > 	if (cpu_present(cpu))
> >-		arch_unregister_cpu(cpu);
> >+		xen_arch_unregister_cpu(cpu);
> > 
> > 	set_cpu_present(cpu, false);
> > }
> >@@ -102,8 +102,10 @@ static int __init setup_vcpu_hotplug_event(void)
> > 	static struct notifier_block xsn_cpu = {
> > 		.notifier_call = setup_cpu_watcher };
> > 
> >+#ifdef CONFIG_X86
> > 	if (!xen_pv_domain())
> > 		return -ENODEV;
> >+#endif
> > 
> > 	register_xenstore_notifier(&xsn_cpu);
> > 
> 
> 

[PATCH 3/3] xen/arm: don't try to re-register vcpu_info on cpu_hotplug.

[Julien Grall]

Hi Stefano,

On 14/10/2015 18:49, Stefano Stabellini wrote:
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> ---
>   arch/arm/xen/enlighten.c |    6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> index 6c09cc4..59f5421 100644
> --- a/arch/arm/xen/enlighten.c
> +++ b/arch/arm/xen/enlighten.c
> @@ -93,6 +93,12 @@ static void xen_percpu_init(void)
>   	int err;
>   	int cpu = get_cpu();
>
> +	/* vcpu_info already registered, can happen with cpu-hotplug */

Can you please add more comment and explain in the commit message why 
this is necessary for cpu-hotplug?

I had to look at the x86 code to fully understand that it's not possible 
to call VCPUOP_register_vcpu_info twice because there is no hypercall to 
remove the vcpu shared page.


> +	if (per_cpu(xen_vcpu, cpu) != NULL) {
> +		put_cpu();
> +		return;
> +	}
> +
>   	pr_info("Xen: initializing cpu%d\n", cpu);
>   	vcpup = per_cpu_ptr(xen_vcpu_info, cpu);
>
>

Regards,

-- 
Julien Grall

Re: [PATCH 3/3] xen/arm: don't try to re-register vcpu_info on cpu_hotplug.

[Julien Grall]

Hi Stefano,

On 14/10/2015 18:49, Stefano Stabellini wrote:
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> ---
>   arch/arm/xen/enlighten.c |    6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> index 6c09cc4..59f5421 100644
> --- a/arch/arm/xen/enlighten.c
> +++ b/arch/arm/xen/enlighten.c
> @@ -93,6 +93,12 @@ static void xen_percpu_init(void)
>   	int err;
>   	int cpu = get_cpu();
>
> +	/* vcpu_info already registered, can happen with cpu-hotplug */

Can you please add more comment and explain in the commit message why 
this is necessary for cpu-hotplug?

I had to look at the x86 code to fully understand that it's not possible 
to call VCPUOP_register_vcpu_info twice because there is no hypercall to 
remove the vcpu shared page.


> +	if (per_cpu(xen_vcpu, cpu) != NULL) {
> +		put_cpu();
> +		return;
> +	}
> +
>   	pr_info("Xen: initializing cpu%d\n", cpu);
>   	vcpup = per_cpu_ptr(xen_vcpu_info, cpu);
>
>

Regards,

-- 
Julien Grall

[PATCH 1/3] xen/arm: Enable cpu_hotplug.c

[Julien Grall]

Hi Stefano,

On 14/10/2015 18:49, Stefano Stabellini wrote:
> Build cpu_hotplug for ARM and ARM64 guests.
>
> Rename arch_(un)register_cpu to xen_(un)register_cpu and provide an
> empty implementation on ARM and ARM64. On x86 just call
> arch_(un)register_cpu as we are already doing.
>
> Initialize cpu_hotplug on ARM.
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> ---
>   arch/arm/include/asm/xen/hypervisor.h |    8 ++++++++
>   arch/x86/include/asm/xen/hypervisor.h |    5 +++++
>   arch/x86/xen/enlighten.c              |   15 +++++++++++++++
>   drivers/xen/Makefile                  |    2 --
>   drivers/xen/cpu_hotplug.c             |    6 ++++--
>   5 files changed, 32 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/include/asm/xen/hypervisor.h b/arch/arm/include/asm/xen/hypervisor.h
> index 04ff8e7..2bc418a 100644
> --- a/arch/arm/include/asm/xen/hypervisor.h
> +++ b/arch/arm/include/asm/xen/hypervisor.h
> @@ -26,4 +26,12 @@ void __init xen_early_init(void);
>   static inline void xen_early_init(void) { return; }
>   #endif
>

I know that those helpers are empty for now. But I would prefer to see 
them protected by (FWIW, it's what you did for x86).

#ifdef CONFIG_CPU_HOTPLUG

> +static inline void xen_arch_register_cpu(int num)
> +{
> +}
> +
> +static inline void xen_arch_unregister_cpu(int num)
> +{
> +}
> +

#endif

>   #endif /* _ASM_ARM_XEN_HYPERVISOR_H */

[...]

> diff --git a/drivers/xen/cpu_hotplug.c b/drivers/xen/cpu_hotplug.c
> index cc6513a..122b351 100644
> --- a/drivers/xen/cpu_hotplug.c
> +++ b/drivers/xen/cpu_hotplug.c
> @@ -11,7 +11,7 @@
>   static void enable_hotplug_cpu(int cpu)
>   {
>   	if (!cpu_present(cpu))
> -		arch_register_cpu(cpu);
> +		xen_arch_register_cpu(cpu);
>
>   	set_cpu_present(cpu, true);
>   }
> @@ -19,7 +19,7 @@ static void enable_hotplug_cpu(int cpu)
>   static void disable_hotplug_cpu(int cpu)
>   {
>   	if (cpu_present(cpu))
> -		arch_unregister_cpu(cpu);
> +		xen_arch_unregister_cpu(cpu);
>
>   	set_cpu_present(cpu, false);
>   }
> @@ -102,8 +102,10 @@ static int __init setup_vcpu_hotplug_event(void)
>   	static struct notifier_block xsn_cpu = {
>   		.notifier_call = setup_cpu_watcher };
>
> +#ifdef CONFIG_X86
>   	if (!xen_pv_domain())
>   		return -ENODEV;
> +#endif

For ARM, you need to check if it's a Xen domain. Otherwise a kernel 
aware of Xen won't boot on baremetal.

>
>   	register_xenstore_notifier(&xsn_cpu);
>
>

Regards,

-- 
Julien Grall

Re: [PATCH 1/3] xen/arm: Enable cpu_hotplug.c

[Julien Grall]

Hi Stefano,

On 14/10/2015 18:49, Stefano Stabellini wrote:
> Build cpu_hotplug for ARM and ARM64 guests.
>
> Rename arch_(un)register_cpu to xen_(un)register_cpu and provide an
> empty implementation on ARM and ARM64. On x86 just call
> arch_(un)register_cpu as we are already doing.
>
> Initialize cpu_hotplug on ARM.
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> ---
>   arch/arm/include/asm/xen/hypervisor.h |    8 ++++++++
>   arch/x86/include/asm/xen/hypervisor.h |    5 +++++
>   arch/x86/xen/enlighten.c              |   15 +++++++++++++++
>   drivers/xen/Makefile                  |    2 --
>   drivers/xen/cpu_hotplug.c             |    6 ++++--
>   5 files changed, 32 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/include/asm/xen/hypervisor.h b/arch/arm/include/asm/xen/hypervisor.h
> index 04ff8e7..2bc418a 100644
> --- a/arch/arm/include/asm/xen/hypervisor.h
> +++ b/arch/arm/include/asm/xen/hypervisor.h
> @@ -26,4 +26,12 @@ void __init xen_early_init(void);
>   static inline void xen_early_init(void) { return; }
>   #endif
>

I know that those helpers are empty for now. But I would prefer to see 
them protected by (FWIW, it's what you did for x86).

#ifdef CONFIG_CPU_HOTPLUG

> +static inline void xen_arch_register_cpu(int num)
> +{
> +}
> +
> +static inline void xen_arch_unregister_cpu(int num)
> +{
> +}
> +

#endif

>   #endif /* _ASM_ARM_XEN_HYPERVISOR_H */

[...]

> diff --git a/drivers/xen/cpu_hotplug.c b/drivers/xen/cpu_hotplug.c
> index cc6513a..122b351 100644
> --- a/drivers/xen/cpu_hotplug.c
> +++ b/drivers/xen/cpu_hotplug.c
> @@ -11,7 +11,7 @@
>   static void enable_hotplug_cpu(int cpu)
>   {
>   	if (!cpu_present(cpu))
> -		arch_register_cpu(cpu);
> +		xen_arch_register_cpu(cpu);
>
>   	set_cpu_present(cpu, true);
>   }
> @@ -19,7 +19,7 @@ static void enable_hotplug_cpu(int cpu)
>   static void disable_hotplug_cpu(int cpu)
>   {
>   	if (cpu_present(cpu))
> -		arch_unregister_cpu(cpu);
> +		xen_arch_unregister_cpu(cpu);
>
>   	set_cpu_present(cpu, false);
>   }
> @@ -102,8 +102,10 @@ static int __init setup_vcpu_hotplug_event(void)
>   	static struct notifier_block xsn_cpu = {
>   		.notifier_call = setup_cpu_watcher };
>
> +#ifdef CONFIG_X86
>   	if (!xen_pv_domain())
>   		return -ENODEV;
> +#endif

For ARM, you need to check if it's a Xen domain. Otherwise a kernel 
aware of Xen won't boot on baremetal.

>
>   	register_xenstore_notifier(&xsn_cpu);
>
>

Regards,

-- 
Julien Grall

xen,arm: enable cpu_hotplug

[Julien Grall]

On 14/10/2015 18:49, Stefano Stabellini wrote:
> Hi all,

Hi Stefano,

> this small patch series enable cpu_hotplug in ARM and ARM64 guests,
> using the PV path to plug and unplug the cpus and psci to enable/disable
> them.

That's a cool things to have on ARM!

I've got few questions related to CPU hotplug on Xen side.

Firstly, when we create the device tree we are using max_vcpus to 
populate the "/cpus" node. AFAIU, Linux will always start all the vCPU 
because they are marked present. That means that it would not be 
possible to honor vcpus="N" where N is < max_vcpus. Did I miss something?

My second point is related to how Xen is handling interrupt with vCPU. 
When PSCI off is called, we will set the _VFP_down flag. This flag is 
used in vgic_vcpu_inject_irq and when it's set the interrupt will be 
ignored and stay active on the HW GIC forever. If the vCPU is coming 
back online, this interrupt will never be received. AFAIU the spec, the 
interrupt is expected to stay pending on the distributor side and will 
be receive when the vCPU will come back or migrate to another vCPU. A 
similar problem can happen when the vCPU is powered on again because we 
clear all the interrupt state related to vCPU (see vgic_clear_pending_irqs).

That brings me a third one related to migration (and not to this series 
specifically). If the interrupt is edge type, it means that same 
interrupt can come while it's still active in the LR register. If the 
guest has to migrate the IRQ it won't happen because the interrupt is 
already active and in the LR, so we will set the pending bit on the 
current vCPU.

Regards,

-- 
Julien Grall

Re: xen,arm: enable cpu_hotplug

[Julien Grall]

On 14/10/2015 18:49, Stefano Stabellini wrote:
> Hi all,

Hi Stefano,

> this small patch series enable cpu_hotplug in ARM and ARM64 guests,
> using the PV path to plug and unplug the cpus and psci to enable/disable
> them.

That's a cool things to have on ARM!

I've got few questions related to CPU hotplug on Xen side.

Firstly, when we create the device tree we are using max_vcpus to 
populate the "/cpus" node. AFAIU, Linux will always start all the vCPU 
because they are marked present. That means that it would not be 
possible to honor vcpus="N" where N is < max_vcpus. Did I miss something?

My second point is related to how Xen is handling interrupt with vCPU. 
When PSCI off is called, we will set the _VFP_down flag. This flag is 
used in vgic_vcpu_inject_irq and when it's set the interrupt will be 
ignored and stay active on the HW GIC forever. If the vCPU is coming 
back online, this interrupt will never be received. AFAIU the spec, the 
interrupt is expected to stay pending on the distributor side and will 
be receive when the vCPU will come back or migrate to another vCPU. A 
similar problem can happen when the vCPU is powered on again because we 
clear all the interrupt state related to vCPU (see vgic_clear_pending_irqs).

That brings me a third one related to migration (and not to this series 
specifically). If the interrupt is edge type, it means that same 
interrupt can come while it's still active in the LR register. If the 
guest has to migrate the IRQ it won't happen because the interrupt is 
already active and in the LR, so we will set the pending bit on the 
current vCPU.

Regards,

-- 
Julien Grall

[Xen-devel] xen,arm: enable cpu_hotplug

[Ian Campbell]

On Thu, 2015-10-15 at 00:23 +0100, Julien Grall wrote:
> My second point is related to how Xen is handling interrupt with vCPU. 
> When PSCI off is called, we will set the _VFP_down flag. This flag is 
> used in vgic_vcpu_inject_irq and when it's set the interrupt will be 
> ignored and stay active on the HW GIC forever. If the vCPU is coming 
> back online, this interrupt will never be received. AFAIU the spec, the 
> interrupt is expected to stay pending on the distributor side and will 
> be receive when the vCPU will come back or migrate to another vCPU. A 
> similar problem can happen when the vCPU is powered on again because we 
> clear all the interrupt state related to vCPU (see
> vgic_clear_pending_irqs).

Is there also an interaction with our implementation of ITARGETSR of
picking the lowest set bit? e.g. if an IRQ has target 0x6 (targeting CPU 1
and CPU2) we will choose CPU 1. If CPU 1 is then unplugged, will we end up
targeting CPU 2 or the now-offline CPU 1? In the latter case the lack of
interrupts might be considered surprising?

Or maybe the way CPU hotplug is arranged we never end up with holes in the
online cpu space, i.e it is not possible to take down CPU1 and leave CPU2
up?

Ian.

Re: [Xen-devel] xen,arm: enable cpu_hotplug

[Ian Campbell]

On Thu, 2015-10-15 at 00:23 +0100, Julien Grall wrote:
> My second point is related to how Xen is handling interrupt with vCPU. 
> When PSCI off is called, we will set the _VFP_down flag. This flag is 
> used in vgic_vcpu_inject_irq and when it's set the interrupt will be 
> ignored and stay active on the HW GIC forever. If the vCPU is coming 
> back online, this interrupt will never be received. AFAIU the spec, the 
> interrupt is expected to stay pending on the distributor side and will 
> be receive when the vCPU will come back or migrate to another vCPU. A 
> similar problem can happen when the vCPU is powered on again because we 
> clear all the interrupt state related to vCPU (see
> vgic_clear_pending_irqs).

Is there also an interaction with our implementation of ITARGETSR of
picking the lowest set bit? e.g. if an IRQ has target 0x6 (targeting CPU 1
and CPU2) we will choose CPU 1. If CPU 1 is then unplugged, will we end up
targeting CPU 2 or the now-offline CPU 1? In the latter case the lack of
interrupts might be considered surprising?

Or maybe the way CPU hotplug is arranged we never end up with holes in the
online cpu space, i.e it is not possible to take down CPU1 and leave CPU2
up?

Ian.

[Xen-devel] xen,arm: enable cpu_hotplug

[Julien Grall]

Hi Ian,

On 15/10/15 09:39, Ian Campbell wrote:
> On Thu, 2015-10-15 at 00:23 +0100, Julien Grall wrote:
>> My second point is related to how Xen is handling interrupt with vCPU. 
>> When PSCI off is called, we will set the _VFP_down flag. This flag is 
>> used in vgic_vcpu_inject_irq and when it's set the interrupt will be 
>> ignored and stay active on the HW GIC forever. If the vCPU is coming 
>> back online, this interrupt will never be received. AFAIU the spec, the 
>> interrupt is expected to stay pending on the distributor side and will 
>> be receive when the vCPU will come back or migrate to another vCPU. A 
>> similar problem can happen when the vCPU is powered on again because we 
>> clear all the interrupt state related to vCPU (see
>> vgic_clear_pending_irqs).
> 
> Is there also an interaction with our implementation of ITARGETSR of
> picking the lowest set bit? e.g. if an IRQ has target 0x6 (targeting CPU 1
> and CPU2) we will choose CPU 1. If CPU 1 is then unplugged, will we end up
> targeting CPU 2 or the now-offline CPU 1? In the latter case the lack of
> interrupts might be considered surprising?

I though about it when I wrote the mail yesterday night.

>From the spec section 1.4.3 (ARM IHI 0048B.b):

"The ARM GIC architecture does not guarantee that a 1-N interrupt is
presented to:
? all processors listed in the target processor list
? an enabled interface, where at least one interface is enabled."

AFAIU this paragraph, it means that there is no guarantee to receive an
interrupt if the target mask contain a vCPU offline.

> Or maybe the way CPU hotplug is arranged we never end up with holes in the
> online cpu space, i.e it is not possible to take down CPU1 and leave CPU2
> up?

A kernel is allowed to hotplug any vCPU.

Regards,

-- 
Julien Grall

Re: [Xen-devel] xen,arm: enable cpu_hotplug

[Julien Grall]

Hi Ian,

On 15/10/15 09:39, Ian Campbell wrote:
> On Thu, 2015-10-15 at 00:23 +0100, Julien Grall wrote:
>> My second point is related to how Xen is handling interrupt with vCPU. 
>> When PSCI off is called, we will set the _VFP_down flag. This flag is 
>> used in vgic_vcpu_inject_irq and when it's set the interrupt will be 
>> ignored and stay active on the HW GIC forever. If the vCPU is coming 
>> back online, this interrupt will never be received. AFAIU the spec, the 
>> interrupt is expected to stay pending on the distributor side and will 
>> be receive when the vCPU will come back or migrate to another vCPU. A 
>> similar problem can happen when the vCPU is powered on again because we 
>> clear all the interrupt state related to vCPU (see
>> vgic_clear_pending_irqs).
> 
> Is there also an interaction with our implementation of ITARGETSR of
> picking the lowest set bit? e.g. if an IRQ has target 0x6 (targeting CPU 1
> and CPU2) we will choose CPU 1. If CPU 1 is then unplugged, will we end up
> targeting CPU 2 or the now-offline CPU 1? In the latter case the lack of
> interrupts might be considered surprising?

I though about it when I wrote the mail yesterday night.

From the spec section 1.4.3 (ARM IHI 0048B.b):

"The ARM GIC architecture does not guarantee that a 1-N interrupt is
presented to:
— all processors listed in the target processor list
— an enabled interface, where at least one interface is enabled."

AFAIU this paragraph, it means that there is no guarantee to receive an
interrupt if the target mask contain a vCPU offline.

> Or maybe the way CPU hotplug is arranged we never end up with holes in the
> online cpu space, i.e it is not possible to take down CPU1 and leave CPU2
> up?

A kernel is allowed to hotplug any vCPU.

Regards,

-- 
Julien Grall

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[Xen-devel] xen,arm: enable cpu_hotplug

[Julien Grall]

On 15/10/15 10:57, Julien Grall wrote:
> Hi Ian,
> 
> On 15/10/15 09:39, Ian Campbell wrote:
>> On Thu, 2015-10-15 at 00:23 +0100, Julien Grall wrote:
>>> My second point is related to how Xen is handling interrupt with vCPU. 
>>> When PSCI off is called, we will set the _VFP_down flag. This flag is 
>>> used in vgic_vcpu_inject_irq and when it's set the interrupt will be 
>>> ignored and stay active on the HW GIC forever. If the vCPU is coming 
>>> back online, this interrupt will never be received. AFAIU the spec, the 
>>> interrupt is expected to stay pending on the distributor side and will 
>>> be receive when the vCPU will come back or migrate to another vCPU. A 
>>> similar problem can happen when the vCPU is powered on again because we 
>>> clear all the interrupt state related to vCPU (see
>>> vgic_clear_pending_irqs).
>>
>> Is there also an interaction with our implementation of ITARGETSR of
>> picking the lowest set bit? e.g. if an IRQ has target 0x6 (targeting CPU 1
>> and CPU2) we will choose CPU 1. If CPU 1 is then unplugged, will we end up
>> targeting CPU 2 or the now-offline CPU 1? In the latter case the lack of
>> interrupts might be considered surprising?
> 
> I though about it when I wrote the mail yesterday night.
> 
> From the spec section 1.4.3 (ARM IHI 0048B.b):
> 
> "The ARM GIC architecture does not guarantee that a 1-N interrupt is
> presented to:
> ? all processors listed in the target processor list
> ? an enabled interface, where at least one interface is enabled."
> 
> AFAIU this paragraph, it means that there is no guarantee to receive an
> interrupt if the target mask contain a vCPU offline.

Hmmm I may not have been clear here. I meant that the interrupt will
stay pending in the distributor but not received by the guest.

-- 
Julien Grall

Re: [Xen-devel] xen,arm: enable cpu_hotplug

[Julien Grall]

On 15/10/15 10:57, Julien Grall wrote:
> Hi Ian,
> 
> On 15/10/15 09:39, Ian Campbell wrote:
>> On Thu, 2015-10-15 at 00:23 +0100, Julien Grall wrote:
>>> My second point is related to how Xen is handling interrupt with vCPU. 
>>> When PSCI off is called, we will set the _VFP_down flag. This flag is 
>>> used in vgic_vcpu_inject_irq and when it's set the interrupt will be 
>>> ignored and stay active on the HW GIC forever. If the vCPU is coming 
>>> back online, this interrupt will never be received. AFAIU the spec, the 
>>> interrupt is expected to stay pending on the distributor side and will 
>>> be receive when the vCPU will come back or migrate to another vCPU. A 
>>> similar problem can happen when the vCPU is powered on again because we 
>>> clear all the interrupt state related to vCPU (see
>>> vgic_clear_pending_irqs).
>>
>> Is there also an interaction with our implementation of ITARGETSR of
>> picking the lowest set bit? e.g. if an IRQ has target 0x6 (targeting CPU 1
>> and CPU2) we will choose CPU 1. If CPU 1 is then unplugged, will we end up
>> targeting CPU 2 or the now-offline CPU 1? In the latter case the lack of
>> interrupts might be considered surprising?
> 
> I though about it when I wrote the mail yesterday night.
> 
> From the spec section 1.4.3 (ARM IHI 0048B.b):
> 
> "The ARM GIC architecture does not guarantee that a 1-N interrupt is
> presented to:
> — all processors listed in the target processor list
> — an enabled interface, where at least one interface is enabled."
> 
> AFAIU this paragraph, it means that there is no guarantee to receive an
> interrupt if the target mask contain a vCPU offline.

Hmmm I may not have been clear here. I meant that the interrupt will
stay pending in the distributor but not received by the guest.

-- 
Julien Grall

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[Xen-devel] [PATCH 1/3] xen/arm: Enable cpu_hotplug.c

[Stefano Stabellini]

On Wed, 14 Oct 2015, Stefano Stabellini wrote:
> On Wed, 14 Oct 2015, Konrad Rzeszutek Wilk wrote:
> > On October 14, 2015 1:49:55 PM EDT, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote:
> > >Build cpu_hotplug for ARM and ARM64 guests.
> > >
> > >Rename arch_(un)register_cpu to xen_(un)register_cpu and provide an
> > >empty implementation on ARM and ARM64. On x86 just call
> > >arch_(un)register_cpu as we are already doing.
> > >
> > >Initialize cpu_hotplug on ARM.
> > 
> > Have you tested this on x86? As your changes touch the generic code path.
> 
> Only build-tested at this stage, because all my x86 test boxes are
> unavailable at the moment (they are being relocated). As soon as they
> come back online I will test.

Tested with a PV guest on x86 and it's fine.


> 
> > >Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > >---
> > > arch/arm/include/asm/xen/hypervisor.h |    8 ++++++++
> > > arch/x86/include/asm/xen/hypervisor.h |    5 +++++
> > > arch/x86/xen/enlighten.c              |   15 +++++++++++++++
> > > drivers/xen/Makefile                  |    2 --
> > > drivers/xen/cpu_hotplug.c             |    6 ++++--
> > > 5 files changed, 32 insertions(+), 4 deletions(-)
> > >
> > >diff --git a/arch/arm/include/asm/xen/hypervisor.h
> > >b/arch/arm/include/asm/xen/hypervisor.h
> > >index 04ff8e7..2bc418a 100644
> > >--- a/arch/arm/include/asm/xen/hypervisor.h
> > >+++ b/arch/arm/include/asm/xen/hypervisor.h
> > >@@ -26,4 +26,12 @@ void __init xen_early_init(void);
> > > static inline void xen_early_init(void) { return; }
> > > #endif
> > > 
> > >+static inline void xen_arch_register_cpu(int num)
> > >+{
> > >+}
> > >+
> > >+static inline void xen_arch_unregister_cpu(int num)
> > >+{
> > >+}
> > >+
> > > #endif /* _ASM_ARM_XEN_HYPERVISOR_H */
> > >diff --git a/arch/x86/include/asm/xen/hypervisor.h
> > >b/arch/x86/include/asm/xen/hypervisor.h
> > >index d866959..8b2d4be 100644
> > >--- a/arch/x86/include/asm/xen/hypervisor.h
> > >+++ b/arch/x86/include/asm/xen/hypervisor.h
> > >@@ -57,4 +57,9 @@ static inline bool xen_x2apic_para_available(void)
> > > }
> > > #endif
> > > 
> > >+#ifdef CONFIG_HOTPLUG_CPU
> > >+void xen_arch_register_cpu(int num);
> > >+void xen_arch_unregister_cpu(int num);
> > >+#endif
> > >+
> > > #endif /* _ASM_X86_XEN_HYPERVISOR_H */
> > >diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> > >index 11d6fb4..ba62d8e 100644
> > >--- a/arch/x86/xen/enlighten.c
> > >+++ b/arch/x86/xen/enlighten.c
> > >@@ -71,6 +71,7 @@
> > > #include <asm/mwait.h>
> > > #include <asm/pci_x86.h>
> > > #include <asm/pat.h>
> > >+#include <asm/cpu.h>
> > > 
> > > #ifdef CONFIG_ACPI
> > > #include <linux/acpi.h>
> > >@@ -1868,3 +1869,17 @@ const struct hypervisor_x86 x86_hyper_xen = {
> > > 	.set_cpu_features       = xen_set_cpu_features,
> > > };
> > > EXPORT_SYMBOL(x86_hyper_xen);
> > >+
> > >+#ifdef CONFIG_HOTPLUG_CPU
> > >+void xen_arch_register_cpu(int num)
> > >+{
> > >+	arch_register_cpu(num);
> > >+}
> > >+EXPORT_SYMBOL(xen_arch_register_cpu);
> > >+
> > >+void xen_arch_unregister_cpu(int num)
> > >+{
> > >+	arch_unregister_cpu(num);
> > >+}
> > >+EXPORT_SYMBOL(xen_arch_unregister_cpu);
> > >+#endif
> > >diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
> > >index e293bc5..aa8a7f7 100644
> > >--- a/drivers/xen/Makefile
> > >+++ b/drivers/xen/Makefile
> > >@@ -1,6 +1,4 @@
> > >-ifeq ($(filter y, $(CONFIG_ARM) $(CONFIG_ARM64)),)
> > > obj-$(CONFIG_HOTPLUG_CPU)		+= cpu_hotplug.o
> > >-endif
> > > obj-$(CONFIG_X86)			+= fallback.o
> > > obj-y	+= grant-table.o features.o balloon.o manage.o preempt.o
> > > obj-y	+= events/
> > >diff --git a/drivers/xen/cpu_hotplug.c b/drivers/xen/cpu_hotplug.c
> > >index cc6513a..122b351 100644
> > >--- a/drivers/xen/cpu_hotplug.c
> > >+++ b/drivers/xen/cpu_hotplug.c
> > >@@ -11,7 +11,7 @@
> > > static void enable_hotplug_cpu(int cpu)
> > > {
> > > 	if (!cpu_present(cpu))
> > >-		arch_register_cpu(cpu);
> > >+		xen_arch_register_cpu(cpu);
> > > 
> > > 	set_cpu_present(cpu, true);
> > > }
> > >@@ -19,7 +19,7 @@ static void enable_hotplug_cpu(int cpu)
> > > static void disable_hotplug_cpu(int cpu)
> > > {
> > > 	if (cpu_present(cpu))
> > >-		arch_unregister_cpu(cpu);
> > >+		xen_arch_unregister_cpu(cpu);
> > > 
> > > 	set_cpu_present(cpu, false);
> > > }
> > >@@ -102,8 +102,10 @@ static int __init setup_vcpu_hotplug_event(void)
> > > 	static struct notifier_block xsn_cpu = {
> > > 		.notifier_call = setup_cpu_watcher };
> > > 
> > >+#ifdef CONFIG_X86
> > > 	if (!xen_pv_domain())
> > > 		return -ENODEV;
> > >+#endif
> > > 
> > > 	register_xenstore_notifier(&xsn_cpu);
> > > 
> > 
> > 
> 

Re: [Xen-devel] [PATCH 1/3] xen/arm: Enable cpu_hotplug.c

[Stefano Stabellini]

On Wed, 14 Oct 2015, Stefano Stabellini wrote:
> On Wed, 14 Oct 2015, Konrad Rzeszutek Wilk wrote:
> > On October 14, 2015 1:49:55 PM EDT, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote:
> > >Build cpu_hotplug for ARM and ARM64 guests.
> > >
> > >Rename arch_(un)register_cpu to xen_(un)register_cpu and provide an
> > >empty implementation on ARM and ARM64. On x86 just call
> > >arch_(un)register_cpu as we are already doing.
> > >
> > >Initialize cpu_hotplug on ARM.
> > 
> > Have you tested this on x86? As your changes touch the generic code path.
> 
> Only build-tested at this stage, because all my x86 test boxes are
> unavailable at the moment (they are being relocated). As soon as they
> come back online I will test.

Tested with a PV guest on x86 and it's fine.


> 
> > >Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > >---
> > > arch/arm/include/asm/xen/hypervisor.h |    8 ++++++++
> > > arch/x86/include/asm/xen/hypervisor.h |    5 +++++
> > > arch/x86/xen/enlighten.c              |   15 +++++++++++++++
> > > drivers/xen/Makefile                  |    2 --
> > > drivers/xen/cpu_hotplug.c             |    6 ++++--
> > > 5 files changed, 32 insertions(+), 4 deletions(-)
> > >
> > >diff --git a/arch/arm/include/asm/xen/hypervisor.h
> > >b/arch/arm/include/asm/xen/hypervisor.h
> > >index 04ff8e7..2bc418a 100644
> > >--- a/arch/arm/include/asm/xen/hypervisor.h
> > >+++ b/arch/arm/include/asm/xen/hypervisor.h
> > >@@ -26,4 +26,12 @@ void __init xen_early_init(void);
> > > static inline void xen_early_init(void) { return; }
> > > #endif
> > > 
> > >+static inline void xen_arch_register_cpu(int num)
> > >+{
> > >+}
> > >+
> > >+static inline void xen_arch_unregister_cpu(int num)
> > >+{
> > >+}
> > >+
> > > #endif /* _ASM_ARM_XEN_HYPERVISOR_H */
> > >diff --git a/arch/x86/include/asm/xen/hypervisor.h
> > >b/arch/x86/include/asm/xen/hypervisor.h
> > >index d866959..8b2d4be 100644
> > >--- a/arch/x86/include/asm/xen/hypervisor.h
> > >+++ b/arch/x86/include/asm/xen/hypervisor.h
> > >@@ -57,4 +57,9 @@ static inline bool xen_x2apic_para_available(void)
> > > }
> > > #endif
> > > 
> > >+#ifdef CONFIG_HOTPLUG_CPU
> > >+void xen_arch_register_cpu(int num);
> > >+void xen_arch_unregister_cpu(int num);
> > >+#endif
> > >+
> > > #endif /* _ASM_X86_XEN_HYPERVISOR_H */
> > >diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> > >index 11d6fb4..ba62d8e 100644
> > >--- a/arch/x86/xen/enlighten.c
> > >+++ b/arch/x86/xen/enlighten.c
> > >@@ -71,6 +71,7 @@
> > > #include <asm/mwait.h>
> > > #include <asm/pci_x86.h>
> > > #include <asm/pat.h>
> > >+#include <asm/cpu.h>
> > > 
> > > #ifdef CONFIG_ACPI
> > > #include <linux/acpi.h>
> > >@@ -1868,3 +1869,17 @@ const struct hypervisor_x86 x86_hyper_xen = {
> > > 	.set_cpu_features       = xen_set_cpu_features,
> > > };
> > > EXPORT_SYMBOL(x86_hyper_xen);
> > >+
> > >+#ifdef CONFIG_HOTPLUG_CPU
> > >+void xen_arch_register_cpu(int num)
> > >+{
> > >+	arch_register_cpu(num);
> > >+}
> > >+EXPORT_SYMBOL(xen_arch_register_cpu);
> > >+
> > >+void xen_arch_unregister_cpu(int num)
> > >+{
> > >+	arch_unregister_cpu(num);
> > >+}
> > >+EXPORT_SYMBOL(xen_arch_unregister_cpu);
> > >+#endif
> > >diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
> > >index e293bc5..aa8a7f7 100644
> > >--- a/drivers/xen/Makefile
> > >+++ b/drivers/xen/Makefile
> > >@@ -1,6 +1,4 @@
> > >-ifeq ($(filter y, $(CONFIG_ARM) $(CONFIG_ARM64)),)
> > > obj-$(CONFIG_HOTPLUG_CPU)		+= cpu_hotplug.o
> > >-endif
> > > obj-$(CONFIG_X86)			+= fallback.o
> > > obj-y	+= grant-table.o features.o balloon.o manage.o preempt.o
> > > obj-y	+= events/
> > >diff --git a/drivers/xen/cpu_hotplug.c b/drivers/xen/cpu_hotplug.c
> > >index cc6513a..122b351 100644
> > >--- a/drivers/xen/cpu_hotplug.c
> > >+++ b/drivers/xen/cpu_hotplug.c
> > >@@ -11,7 +11,7 @@
> > > static void enable_hotplug_cpu(int cpu)
> > > {
> > > 	if (!cpu_present(cpu))
> > >-		arch_register_cpu(cpu);
> > >+		xen_arch_register_cpu(cpu);
> > > 
> > > 	set_cpu_present(cpu, true);
> > > }
> > >@@ -19,7 +19,7 @@ static void enable_hotplug_cpu(int cpu)
> > > static void disable_hotplug_cpu(int cpu)
> > > {
> > > 	if (cpu_present(cpu))
> > >-		arch_unregister_cpu(cpu);
> > >+		xen_arch_unregister_cpu(cpu);
> > > 
> > > 	set_cpu_present(cpu, false);
> > > }
> > >@@ -102,8 +102,10 @@ static int __init setup_vcpu_hotplug_event(void)
> > > 	static struct notifier_block xsn_cpu = {
> > > 		.notifier_call = setup_cpu_watcher };
> > > 
> > >+#ifdef CONFIG_X86
> > > 	if (!xen_pv_domain())
> > > 		return -ENODEV;
> > >+#endif
> > > 
> > > 	register_xenstore_notifier(&xsn_cpu);
> > > 
> > 
> > 
> 

[PATCH 1/3] xen/arm: Enable cpu_hotplug.c

[Stefano Stabellini]

On Wed, 14 Oct 2015, Julien Grall wrote:
> Hi Stefano,
> 
> On 14/10/2015 18:49, Stefano Stabellini wrote:
> > Build cpu_hotplug for ARM and ARM64 guests.
> > 
> > Rename arch_(un)register_cpu to xen_(un)register_cpu and provide an
> > empty implementation on ARM and ARM64. On x86 just call
> > arch_(un)register_cpu as we are already doing.
> > 
> > Initialize cpu_hotplug on ARM.
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > ---
> >   arch/arm/include/asm/xen/hypervisor.h |    8 ++++++++
> >   arch/x86/include/asm/xen/hypervisor.h |    5 +++++
> >   arch/x86/xen/enlighten.c              |   15 +++++++++++++++
> >   drivers/xen/Makefile                  |    2 --
> >   drivers/xen/cpu_hotplug.c             |    6 ++++--
> >   5 files changed, 32 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/arm/include/asm/xen/hypervisor.h
> > b/arch/arm/include/asm/xen/hypervisor.h
> > index 04ff8e7..2bc418a 100644
> > --- a/arch/arm/include/asm/xen/hypervisor.h
> > +++ b/arch/arm/include/asm/xen/hypervisor.h
> > @@ -26,4 +26,12 @@ void __init xen_early_init(void);
> >   static inline void xen_early_init(void) { return; }
> >   #endif
> > 
> 
> I know that those helpers are empty for now. But I would prefer to see them
> protected by (FWIW, it's what you did for x86).
> 
> #ifdef CONFIG_CPU_HOTPLUG

Fair enough.


> > +static inline void xen_arch_register_cpu(int num)
> > +{
> > +}
> > +
> > +static inline void xen_arch_unregister_cpu(int num)
> > +{
> > +}
> > +
> 
> #endif
> 
> >   #endif /* _ASM_ARM_XEN_HYPERVISOR_H */
> 
> [...]
> 
> > diff --git a/drivers/xen/cpu_hotplug.c b/drivers/xen/cpu_hotplug.c
> > index cc6513a..122b351 100644
> > --- a/drivers/xen/cpu_hotplug.c
> > +++ b/drivers/xen/cpu_hotplug.c
> > @@ -11,7 +11,7 @@
> >   static void enable_hotplug_cpu(int cpu)
> >   {
> >   	if (!cpu_present(cpu))
> > -		arch_register_cpu(cpu);
> > +		xen_arch_register_cpu(cpu);
> > 
> >   	set_cpu_present(cpu, true);
> >   }
> > @@ -19,7 +19,7 @@ static void enable_hotplug_cpu(int cpu)
> >   static void disable_hotplug_cpu(int cpu)
> >   {
> >   	if (cpu_present(cpu))
> > -		arch_unregister_cpu(cpu);
> > +		xen_arch_unregister_cpu(cpu);
> > 
> >   	set_cpu_present(cpu, false);
> >   }
> > @@ -102,8 +102,10 @@ static int __init setup_vcpu_hotplug_event(void)
> >   	static struct notifier_block xsn_cpu = {
> >   		.notifier_call = setup_cpu_watcher };
> > 
> > +#ifdef CONFIG_X86
> >   	if (!xen_pv_domain())
> >   		return -ENODEV;
> > +#endif
> 
> For ARM, you need to check if it's a Xen domain. Otherwise a kernel aware of
> Xen won't boot on baremetal.

That's a good point!


> > 
> >   	register_xenstore_notifier(&xsn_cpu);
> > 
> > 
> 
> Regards,
> 
> -- 
> Julien Grall
> 

Re: [PATCH 1/3] xen/arm: Enable cpu_hotplug.c

[Stefano Stabellini]

On Wed, 14 Oct 2015, Julien Grall wrote:
> Hi Stefano,
> 
> On 14/10/2015 18:49, Stefano Stabellini wrote:
> > Build cpu_hotplug for ARM and ARM64 guests.
> > 
> > Rename arch_(un)register_cpu to xen_(un)register_cpu and provide an
> > empty implementation on ARM and ARM64. On x86 just call
> > arch_(un)register_cpu as we are already doing.
> > 
> > Initialize cpu_hotplug on ARM.
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > ---
> >   arch/arm/include/asm/xen/hypervisor.h |    8 ++++++++
> >   arch/x86/include/asm/xen/hypervisor.h |    5 +++++
> >   arch/x86/xen/enlighten.c              |   15 +++++++++++++++
> >   drivers/xen/Makefile                  |    2 --
> >   drivers/xen/cpu_hotplug.c             |    6 ++++--
> >   5 files changed, 32 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/arm/include/asm/xen/hypervisor.h
> > b/arch/arm/include/asm/xen/hypervisor.h
> > index 04ff8e7..2bc418a 100644
> > --- a/arch/arm/include/asm/xen/hypervisor.h
> > +++ b/arch/arm/include/asm/xen/hypervisor.h
> > @@ -26,4 +26,12 @@ void __init xen_early_init(void);
> >   static inline void xen_early_init(void) { return; }
> >   #endif
> > 
> 
> I know that those helpers are empty for now. But I would prefer to see them
> protected by (FWIW, it's what you did for x86).
> 
> #ifdef CONFIG_CPU_HOTPLUG

Fair enough.


> > +static inline void xen_arch_register_cpu(int num)
> > +{
> > +}
> > +
> > +static inline void xen_arch_unregister_cpu(int num)
> > +{
> > +}
> > +
> 
> #endif
> 
> >   #endif /* _ASM_ARM_XEN_HYPERVISOR_H */
> 
> [...]
> 
> > diff --git a/drivers/xen/cpu_hotplug.c b/drivers/xen/cpu_hotplug.c
> > index cc6513a..122b351 100644
> > --- a/drivers/xen/cpu_hotplug.c
> > +++ b/drivers/xen/cpu_hotplug.c
> > @@ -11,7 +11,7 @@
> >   static void enable_hotplug_cpu(int cpu)
> >   {
> >   	if (!cpu_present(cpu))
> > -		arch_register_cpu(cpu);
> > +		xen_arch_register_cpu(cpu);
> > 
> >   	set_cpu_present(cpu, true);
> >   }
> > @@ -19,7 +19,7 @@ static void enable_hotplug_cpu(int cpu)
> >   static void disable_hotplug_cpu(int cpu)
> >   {
> >   	if (cpu_present(cpu))
> > -		arch_unregister_cpu(cpu);
> > +		xen_arch_unregister_cpu(cpu);
> > 
> >   	set_cpu_present(cpu, false);
> >   }
> > @@ -102,8 +102,10 @@ static int __init setup_vcpu_hotplug_event(void)
> >   	static struct notifier_block xsn_cpu = {
> >   		.notifier_call = setup_cpu_watcher };
> > 
> > +#ifdef CONFIG_X86
> >   	if (!xen_pv_domain())
> >   		return -ENODEV;
> > +#endif
> 
> For ARM, you need to check if it's a Xen domain. Otherwise a kernel aware of
> Xen won't boot on baremetal.

That's a good point!


> > 
> >   	register_xenstore_notifier(&xsn_cpu);
> > 
> > 
> 
> Regards,
> 
> -- 
> Julien Grall
> 

[PATCH 3/3] xen/arm: don't try to re-register vcpu_info on cpu_hotplug.

[Stefano Stabellini]

On Wed, 14 Oct 2015, Julien Grall wrote:
> Hi Stefano,
> 
> On 14/10/2015 18:49, Stefano Stabellini wrote:
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > ---
> >   arch/arm/xen/enlighten.c |    6 ++++++
> >   1 file changed, 6 insertions(+)
> > 
> > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> > index 6c09cc4..59f5421 100644
> > --- a/arch/arm/xen/enlighten.c
> > +++ b/arch/arm/xen/enlighten.c
> > @@ -93,6 +93,12 @@ static void xen_percpu_init(void)
> >   	int err;
> >   	int cpu = get_cpu();
> > 
> > +	/* vcpu_info already registered, can happen with cpu-hotplug */
> 
> Can you please add more comment and explain in the commit message why this is
> necessary for cpu-hotplug?
> 
> I had to look at the x86 code to fully understand that it's not possible to
> call VCPUOP_register_vcpu_info twice because there is no hypercall to remove
> the vcpu shared page.

Sure


> 
> > +	if (per_cpu(xen_vcpu, cpu) != NULL) {
> > +		put_cpu();
> > +		return;
> > +	}
> > +
> >   	pr_info("Xen: initializing cpu%d\n", cpu);
> >   	vcpup = per_cpu_ptr(xen_vcpu_info, cpu);
> > 
> > 
> 
> Regards,
> 
> -- 
> Julien Grall
> 

Re: [PATCH 3/3] xen/arm: don't try to re-register vcpu_info on cpu_hotplug.

[Stefano Stabellini]

On Wed, 14 Oct 2015, Julien Grall wrote:
> Hi Stefano,
> 
> On 14/10/2015 18:49, Stefano Stabellini wrote:
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > ---
> >   arch/arm/xen/enlighten.c |    6 ++++++
> >   1 file changed, 6 insertions(+)
> > 
> > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> > index 6c09cc4..59f5421 100644
> > --- a/arch/arm/xen/enlighten.c
> > +++ b/arch/arm/xen/enlighten.c
> > @@ -93,6 +93,12 @@ static void xen_percpu_init(void)
> >   	int err;
> >   	int cpu = get_cpu();
> > 
> > +	/* vcpu_info already registered, can happen with cpu-hotplug */
> 
> Can you please add more comment and explain in the commit message why this is
> necessary for cpu-hotplug?
> 
> I had to look at the x86 code to fully understand that it's not possible to
> call VCPUOP_register_vcpu_info twice because there is no hypercall to remove
> the vcpu shared page.

Sure


> 
> > +	if (per_cpu(xen_vcpu, cpu) != NULL) {
> > +		put_cpu();
> > +		return;
> > +	}
> > +
> >   	pr_info("Xen: initializing cpu%d\n", cpu);
> >   	vcpup = per_cpu_ptr(xen_vcpu_info, cpu);
> > 
> > 
> 
> Regards,
> 
> -- 
> Julien Grall
> 

[Xen-devel] xen,arm: enable cpu_hotplug

[Stefano Stabellini]

On Thu, 15 Oct 2015, Ian Campbell wrote:
> On Thu, 2015-10-15 at 00:23 +0100, Julien Grall wrote:
> > My second point is related to how Xen is handling interrupt with vCPU. 
> > When PSCI off is called, we will set the _VFP_down flag. This flag is 
> > used in vgic_vcpu_inject_irq and when it's set the interrupt will be 
> > ignored and stay active on the HW GIC forever. If the vCPU is coming 
> > back online, this interrupt will never be received. AFAIU the spec, the 
> > interrupt is expected to stay pending on the distributor side and will 
> > be receive when the vCPU will come back or migrate to another vCPU. A 
> > similar problem can happen when the vCPU is powered on again because we 
> > clear all the interrupt state related to vCPU (see
> > vgic_clear_pending_irqs).
> 
> Is there also an interaction with our implementation of ITARGETSR of
> picking the lowest set bit? e.g. if an IRQ has target 0x6 (targeting CPU 1
> and CPU2) we will choose CPU 1. If CPU 1 is then unplugged, will we end up
> targeting CPU 2 or the now-offline CPU 1? In the latter case the lack of
> interrupts might be considered surprising?
> 
> Or maybe the way CPU hotplug is arranged we never end up with holes in the
> online cpu space, i.e it is not possible to take down CPU1 and leave CPU2
> up?

Keep in mind that there is a distinction between a cpu being online (on
or off, this is triggered by psci operations) and being present (this is
what cpu-hotplug does). See /sys/devices/system/cpu/online,
/sys/devices/system/cpu/present and /sys/devices/system/cpu/possible.

It is possible to shut down cpu1 (psci->cpu_off, which pauses the vcpu)
but it is not possible to remove cpu1 without removing cpu2 first.

If the user configures the target to be cpu 2-4, then shuts down cpu2, it
won't receive any interrupts any more. This is not a cpu-hotplug bug.
Maybe is a do_psci_cpu_off bug, which should be improved to handle this
case.

Re: [Xen-devel] xen,arm: enable cpu_hotplug

[Stefano Stabellini]

On Thu, 15 Oct 2015, Ian Campbell wrote:
> On Thu, 2015-10-15 at 00:23 +0100, Julien Grall wrote:
> > My second point is related to how Xen is handling interrupt with vCPU. 
> > When PSCI off is called, we will set the _VFP_down flag. This flag is 
> > used in vgic_vcpu_inject_irq and when it's set the interrupt will be 
> > ignored and stay active on the HW GIC forever. If the vCPU is coming 
> > back online, this interrupt will never be received. AFAIU the spec, the 
> > interrupt is expected to stay pending on the distributor side and will 
> > be receive when the vCPU will come back or migrate to another vCPU. A 
> > similar problem can happen when the vCPU is powered on again because we 
> > clear all the interrupt state related to vCPU (see
> > vgic_clear_pending_irqs).
> 
> Is there also an interaction with our implementation of ITARGETSR of
> picking the lowest set bit? e.g. if an IRQ has target 0x6 (targeting CPU 1
> and CPU2) we will choose CPU 1. If CPU 1 is then unplugged, will we end up
> targeting CPU 2 or the now-offline CPU 1? In the latter case the lack of
> interrupts might be considered surprising?
> 
> Or maybe the way CPU hotplug is arranged we never end up with holes in the
> online cpu space, i.e it is not possible to take down CPU1 and leave CPU2
> up?

Keep in mind that there is a distinction between a cpu being online (on
or off, this is triggered by psci operations) and being present (this is
what cpu-hotplug does). See /sys/devices/system/cpu/online,
/sys/devices/system/cpu/present and /sys/devices/system/cpu/possible.

It is possible to shut down cpu1 (psci->cpu_off, which pauses the vcpu)
but it is not possible to remove cpu1 without removing cpu2 first.

If the user configures the target to be cpu 2-4, then shuts down cpu2, it
won't receive any interrupts any more. This is not a cpu-hotplug bug.
Maybe is a do_psci_cpu_off bug, which should be improved to handle this
case.

[Xen-devel] xen,arm: enable cpu_hotplug

[Stefano Stabellini]

On Thu, 15 Oct 2015, Julien Grall wrote:
> Hi Ian,
> 
> On 15/10/15 09:39, Ian Campbell wrote:
> > On Thu, 2015-10-15 at 00:23 +0100, Julien Grall wrote:
> >> My second point is related to how Xen is handling interrupt with vCPU. 
> >> When PSCI off is called, we will set the _VFP_down flag. This flag is 
> >> used in vgic_vcpu_inject_irq and when it's set the interrupt will be 
> >> ignored and stay active on the HW GIC forever. If the vCPU is coming 
> >> back online, this interrupt will never be received. AFAIU the spec, the 
> >> interrupt is expected to stay pending on the distributor side and will 
> >> be receive when the vCPU will come back or migrate to another vCPU. A 
> >> similar problem can happen when the vCPU is powered on again because we 
> >> clear all the interrupt state related to vCPU (see
> >> vgic_clear_pending_irqs).
> > 
> > Is there also an interaction with our implementation of ITARGETSR of
> > picking the lowest set bit? e.g. if an IRQ has target 0x6 (targeting CPU 1
> > and CPU2) we will choose CPU 1. If CPU 1 is then unplugged, will we end up
> > targeting CPU 2 or the now-offline CPU 1? In the latter case the lack of
> > interrupts might be considered surprising?
> 
> I though about it when I wrote the mail yesterday night.
> 
> >From the spec section 1.4.3 (ARM IHI 0048B.b):
> 
> "The ARM GIC architecture does not guarantee that a 1-N interrupt is
> presented to:
> ? all processors listed in the target processor list
> ? an enabled interface, where at least one interface is enabled."
> 
> AFAIU this paragraph, it means that there is no guarantee to receive an
> interrupt if the target mask contain a vCPU offline.

Ah good! Our do_psci_cpu_off implementation is OK after all :-)


> > Or maybe the way CPU hotplug is arranged we never end up with holes in the
> > online cpu space, i.e it is not possible to take down CPU1 and leave CPU2
> > up?
> 
> A kernel is allowed to hotplug any vCPU.

Yes, but xl only allowed to hot(un)plug cpus in order, look up vcpu-set
on man xl.

Re: [Xen-devel] xen,arm: enable cpu_hotplug

[Stefano Stabellini]

[-- Attachment #1: Type: text/plain, Size: 2038 bytes --]

On Thu, 15 Oct 2015, Julien Grall wrote:
> Hi Ian,
> 
> On 15/10/15 09:39, Ian Campbell wrote:
> > On Thu, 2015-10-15 at 00:23 +0100, Julien Grall wrote:
> >> My second point is related to how Xen is handling interrupt with vCPU. 
> >> When PSCI off is called, we will set the _VFP_down flag. This flag is 
> >> used in vgic_vcpu_inject_irq and when it's set the interrupt will be 
> >> ignored and stay active on the HW GIC forever. If the vCPU is coming 
> >> back online, this interrupt will never be received. AFAIU the spec, the 
> >> interrupt is expected to stay pending on the distributor side and will 
> >> be receive when the vCPU will come back or migrate to another vCPU. A 
> >> similar problem can happen when the vCPU is powered on again because we 
> >> clear all the interrupt state related to vCPU (see
> >> vgic_clear_pending_irqs).
> > 
> > Is there also an interaction with our implementation of ITARGETSR of
> > picking the lowest set bit? e.g. if an IRQ has target 0x6 (targeting CPU 1
> > and CPU2) we will choose CPU 1. If CPU 1 is then unplugged, will we end up
> > targeting CPU 2 or the now-offline CPU 1? In the latter case the lack of
> > interrupts might be considered surprising?
> 
> I though about it when I wrote the mail yesterday night.
> 
> >From the spec section 1.4.3 (ARM IHI 0048B.b):
> 
> "The ARM GIC architecture does not guarantee that a 1-N interrupt is
> presented to:
> — all processors listed in the target processor list
> — an enabled interface, where at least one interface is enabled."
> 
> AFAIU this paragraph, it means that there is no guarantee to receive an
> interrupt if the target mask contain a vCPU offline.

Ah good! Our do_psci_cpu_off implementation is OK after all :-)


> > Or maybe the way CPU hotplug is arranged we never end up with holes in the
> > online cpu space, i.e it is not possible to take down CPU1 and leave CPU2
> > up?
> 
> A kernel is allowed to hotplug any vCPU.

Yes, but xl only allowed to hot(un)plug cpus in order, look up vcpu-set
on man xl.

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

xen,arm: enable cpu_hotplug

[Stefano Stabellini]

On Thu, 15 Oct 2015, Julien Grall wrote:
> On 14/10/2015 18:49, Stefano Stabellini wrote:
> > Hi all,
> 
> Hi Stefano,
> 
> > this small patch series enable cpu_hotplug in ARM and ARM64 guests,
> > using the PV path to plug and unplug the cpus and psci to enable/disable
> > them.
> 
> That's a cool things to have on ARM!
> 
> I've got few questions related to CPU hotplug on Xen side.
> 
> Firstly, when we create the device tree we are using max_vcpus to populate the
> "/cpus" node. AFAIU, Linux will always start all the vCPU because they are
> marked present. That means that it would not be possible to honor vcpus="N"
> where N is < max_vcpus. Did I miss something?

I think that's OK, it is the intended behaviour. See
cpu-hotplug.c:setup_cpu_watcher in Linux, in particular the
for_each_possible_cpu loop. The guest boots with all vcpus and turns off
the ones which are marked as not available on xenstore.


> My second point is related to how Xen is handling interrupt with vCPU. When
> PSCI off is called, we will set the _VFP_down flag. This flag is used in
> vgic_vcpu_inject_irq and when it's set the interrupt will be ignored and stay
> active on the HW GIC forever. If the vCPU is coming back online, this
> interrupt will never be received. AFAIU the spec, the interrupt is expected to
> stay pending on the distributor side and will be receive when the vCPU will
> come back or migrate to another vCPU. A similar problem can happen when the
> vCPU is powered on again because we clear all the interrupt state related to
> vCPU (see vgic_clear_pending_irqs).
>
> That brings me a third one related to migration (and not to this series
> specifically). If the interrupt is edge type, it means that same interrupt can
> come while it's still active in the LR register. If the guest has to migrate
> the IRQ it won't happen because the interrupt is already active and in the LR,
> so we will set the pending bit on the current vCPU.

These might be valid concerns but they don't have much to do with
hotplug: PSCI can be used to turn on/off vcpus from within the guest
independently from cpu-hotplug. The only thing cpu-hotplug does is
marking as present/non-present some cpus in the guest, which can be
taken online/offline independently using psci.

Re: xen,arm: enable cpu_hotplug

[Stefano Stabellini]

On Thu, 15 Oct 2015, Julien Grall wrote:
> On 14/10/2015 18:49, Stefano Stabellini wrote:
> > Hi all,
> 
> Hi Stefano,
> 
> > this small patch series enable cpu_hotplug in ARM and ARM64 guests,
> > using the PV path to plug and unplug the cpus and psci to enable/disable
> > them.
> 
> That's a cool things to have on ARM!
> 
> I've got few questions related to CPU hotplug on Xen side.
> 
> Firstly, when we create the device tree we are using max_vcpus to populate the
> "/cpus" node. AFAIU, Linux will always start all the vCPU because they are
> marked present. That means that it would not be possible to honor vcpus="N"
> where N is < max_vcpus. Did I miss something?

I think that's OK, it is the intended behaviour. See
cpu-hotplug.c:setup_cpu_watcher in Linux, in particular the
for_each_possible_cpu loop. The guest boots with all vcpus and turns off
the ones which are marked as not available on xenstore.


> My second point is related to how Xen is handling interrupt with vCPU. When
> PSCI off is called, we will set the _VFP_down flag. This flag is used in
> vgic_vcpu_inject_irq and when it's set the interrupt will be ignored and stay
> active on the HW GIC forever. If the vCPU is coming back online, this
> interrupt will never be received. AFAIU the spec, the interrupt is expected to
> stay pending on the distributor side and will be receive when the vCPU will
> come back or migrate to another vCPU. A similar problem can happen when the
> vCPU is powered on again because we clear all the interrupt state related to
> vCPU (see vgic_clear_pending_irqs).
>
> That brings me a third one related to migration (and not to this series
> specifically). If the interrupt is edge type, it means that same interrupt can
> come while it's still active in the LR register. If the guest has to migrate
> the IRQ it won't happen because the interrupt is already active and in the LR,
> so we will set the pending bit on the current vCPU.

These might be valid concerns but they don't have much to do with
hotplug: PSCI can be used to turn on/off vcpus from within the guest
independently from cpu-hotplug. The only thing cpu-hotplug does is
marking as present/non-present some cpus in the guest, which can be
taken online/offline independently using psci.

[Xen-devel] xen,arm: enable cpu_hotplug

[Julien Grall]

On 16/10/15 16:41, Stefano Stabellini wrote:
> It is possible to shut down cpu1 (psci->cpu_off, which pauses the vcpu)
> but it is not possible to remove cpu1 without removing cpu2 first.
> 
> If the user configures the target to be cpu 2-4, then shuts down cpu2, it
> won't receive any interrupts any more. This is not a cpu-hotplug bug.
> Maybe is a do_psci_cpu_off bug, which should be improved to handle this
> case.

It's neither a PSCI cpu off bug. It's a bug in our vGIC implementation.

Regards,

-- 
Julien Grall

Re: [Xen-devel] xen,arm: enable cpu_hotplug

[Julien Grall]

On 16/10/15 16:41, Stefano Stabellini wrote:
> It is possible to shut down cpu1 (psci->cpu_off, which pauses the vcpu)
> but it is not possible to remove cpu1 without removing cpu2 first.
> 
> If the user configures the target to be cpu 2-4, then shuts down cpu2, it
> won't receive any interrupts any more. This is not a cpu-hotplug bug.
> Maybe is a do_psci_cpu_off bug, which should be improved to handle this
> case.

It's neither a PSCI cpu off bug. It's a bug in our vGIC implementation.

Regards,

-- 
Julien Grall