xen,arm: enable cpu_hotplug

PATCH Superseded Stefano Stabellini 3 participants 8 messages Oct 19, 2015 – Oct 20, 2015

Subsystems (5)

Arm (32-bit) x86 Xen hypervisor interface Xen hypervisor x86 +1 more subsystems

lore

Share & tools

lore

Stefano Stabellini

author

2015-10-19 16:55 UTC #1 lore raw

PATCH v3 1/3 xen/arm: Enable cpu_hotplug.c

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 <[email protected]>

Reviewed-by Julien Grall <[email protected]>

---

Changes in v2:
- don't initialize if not a xen_domain() on arm
- protect xen_arch_(un)register_cpu with ifdef CONFIG_HOTPLUG_CPU on arm

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

arch/arm/include/asm/xen/hypervisor.h +10 -0

index 04ff8e7..9525151 100644
@@ -26,4 +26,14 @@ void __init xen_early_init(void);
 static inline void xen_early_init(void) { return; }
 #endif
 
+#ifdef CONFIG_HOTPLUG_CPU
+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 */

arch/x86/include/asm/xen/hypervisor.h +5 -0

index d866959..8b2d4be 100644
@@ -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 */

arch/x86/xen/enlighten.c +15 -0

index 11d6fb4..ba62d8e 100644
@@ -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

drivers/xen/Makefile +0 -2

index e293bc5..aa8a7f7 100644
@@ -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/

drivers/xen/cpu_hotplug.c +6 -2

index cc6513a..43de1f5 100644
@@ -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,7 +102,11 @@ 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())
+#else
+	if (!xen_domain())
+#endif
 		return -ENODEV;
 
 	register_xenstore_notifier(&xsn_cpu);

Signature · 1 line

1.7.10.4

Stefano Stabellini

author

2015-10-19 16:55 UTC #2 lore raw

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

In reply to 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 <[email protected]>

CC konrad.wilk at oracle.com

CC boris.ostrovsky at oracle.com

CC david.vrabel at citrix.com

---
 drivers/xen/cpu_hotplug.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/xen/cpu_hotplug.c +2 -1

index 43de1f5..78804b5 100644
@@ -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:

Signature · 1 line

1.7.10.4

Boris Ostrovsky

reviewer

2015-10-19 18:24 UTC #3 lore raw

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

Quoted 19 lines · 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 <[email protected]>
CC konrad.wilk at oracle.com
CC boris.ostrovsky at oracle.com
CC david.vrabel at citrix.com
---
  drivers/xen/cpu_hotplug.c |    3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)
drivers/xen/cpu_hotplug.c +0 -0
index 43de1f5..78804b5 100644

drivers/xen/cpu_hotplug.c static void enable_hotplug_cpu(int cpu) · line 24

Quoted 7 lines

drivers/xen/cpu_hotplug.c +2 -0

@@ -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);

Do we want to call device_online() when plugging cpu in?

And take sysfs lock?

-boris

Quoted 9 lines

drivers/xen/cpu_hotplug.c +0 -1

@@ -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:

Stefano Stabellini

author

2015-10-20 10:28 UTC #4 lore raw

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

Quoted 15 lines · Boris Ostrovsky

Quoted 14 lines · 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 <[email protected]>
CC konrad.wilk at oracle.com
CC boris.ostrovsky at oracle.com
CC david.vrabel at citrix.com
---
  drivers/xen/cpu_hotplug.c |    3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

Quoted 13 lines

drivers/xen/cpu_hotplug.c static void enable_hotplug_cpu(int cpu) · line 23
Quoted 11 lines
drivers/xen/cpu_hotplug.c +2 -0
index 43de1f5..78804b5 100644
@@ -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);
Do we want to call device_online() when plugging cpu in?

I don't know: that would automatically bring the cpu online after the user hot-plugs it. Is it what we want? Current one would have to manually:

echo 1 > /sys/devices/system/cpu/cpu1/online

I thought that was the expected behaviour?

Quoted 1 line

And take sysfs lock?

I think you are right: we need to protect the device_offline (and device_online if we add it) with lock_device_hotplug and unlock_device_hotplug. Thanks!

Quoted 9 lines

Quoted 8 lines

drivers/xen/cpu_hotplug.c +0 -0

  @@ -55,7 +57,6 @@ static void vcpu_hotplug(unsigned int cpu)
  		enable_hotplug_cpu(cpu);

break;
case 0:

drivers/xen/cpu_hotplug.c +0 -1

-		(void)cpu_down(cpu);
  		disable_hotplug_cpu(cpu);
  		break;
  	default:

Boris Ostrovsky

reviewer

2015-10-20 12:55 UTC #5 lore raw

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

Quoted 28 lines · Stefano Stabellini

Quoted 15 lines · Boris Ostrovsky
Quoted 14 lines · 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 <[email protected]>
CC konrad.wilk at oracle.com
CC boris.ostrovsky at oracle.com
CC david.vrabel at citrix.com
---
   drivers/xen/cpu_hotplug.c |    3 ++-
   1 file changed, 2 insertions(+), 1 deletion(-)
Quoted 12 lines
drivers/xen/cpu_hotplug.c static void enable_hotplug_cpu(int cpu) · line 23
Quoted 11 lines
drivers/xen/cpu_hotplug.c +2 -0
index 43de1f5..78804b5 100644
@@ -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);
Do we want to call device_online() when plugging cpu in?

Quoted 6 lines

I don't know: that would automatically bring the cpu online after the user hot-plugs it. Is it what we want? Current one would have to manually:
echo 1 > /sys/devices/system/cpu/cpu1/online
I thought that was the expected behaviour?

Right, we should keep onlining as an explicit operation. I think it's worth mentioning in the commit message why we don't call device_online() while using device_offline(), which is its complement.

-boris

Stefano Stabellini

author

2015-10-19 16:55 UTC #6 lore raw

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

In reply to Stefano Stabellini

Call disable_percpu_irq on CPU_DYING and enable_percpu_irq when the cpu is coming up.

Signed-off-by Stefano Stabellini <[email protected]>

---

Changes in v3:
- call disable_percpu_irq on CPU_DYING
- call enable_percpu_irq even when VCPUOP_register_vcpu_info has already been called

Changes in v2:
- better comment

---
 arch/arm/xen/enlighten.c |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

arch/arm/xen/enlighten.c +14 -0

index 6c09cc4..074b1a2 100644
@@ -93,6 +93,17 @@ static void xen_percpu_init(void)
 	int err;
 	int cpu = get_cpu();
 
+	/* 
+	 * VCPUOP_register_vcpu_info cannot be called twice for the same
+	 * vcpu, so if vcpu_info is already registered, just get out. This
+	 * can happen with cpu-hotplug.
+	 */
+	if (per_cpu(xen_vcpu, cpu) != NULL) {
+		enable_percpu_irq(xen_events_irq, 0);
+		put_cpu();
+		return;
+	}
+
 	pr_info("Xen: initializing cpu%d\n", cpu);
 	vcpup = per_cpu_ptr(xen_vcpu_info, cpu);

@@ -131,6 +142,9 @@ static int xen_cpu_notification(struct notifier_block *self,
 	case CPU_STARTING:
 		xen_percpu_init();
 		break;
+	case CPU_DYING:
+		disable_percpu_irq(xen_events_irq);
+		break;
 	default:
 		break;
 	}

Signature · 1 line

1.7.10.4

Julien Grall

2015-10-19 17:01 UTC #7 lore raw

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

Hi Stefano,

Quoted 7 lines · Stefano Stabellini

arch/arm/xen/enlighten.c +0 -0

index 6c09cc4..074b1a2 100644
@@ -93,6 +93,17 @@ static void xen_percpu_init(void)
 	int err;
 	int cpu = get_cpu();

arch/arm/xen/enlighten.c static void xen_percpu_init(void) · line 104

Quoted 10 lines

arch/arm/xen/enlighten.c +9 -0

 
+	/* 
+	 * VCPUOP_register_vcpu_info cannot be called twice for the same
+	 * vcpu, so if vcpu_info is already registered, just get out. This
+	 * can happen with cpu-hotplug.
+	 */
+	if (per_cpu(xen_vcpu, cpu) != NULL) {
+		enable_percpu_irq(xen_events_irq, 0);
+		put_cpu();
+		return;

I might be too picky, but this function can be reworked to avoid duplicating the code. Something like:

if (per_cpu(xen_vcpu, cpu) != NULL)
goto skip_register_vcpu;

Quoted 16 lines

arch/arm/xen/enlighten.c +5 -0

+	}
+
 	pr_info("Xen: initializing cpu%d\n", cpu);
 	vcpup = per_cpu_ptr(xen_vcpu_info, cpu);

@@ -131,6 +142,9 @@ static int xen_cpu_notification(struct notifier_block *self,
 	case CPU_STARTING:
 		xen_percpu_init();
 		break;
+	case CPU_DYING:
+		disable_percpu_irq(xen_events_irq);
+		break;
 	default:
 		break;
 	}

Signature · 1 line

Julien Grall