Implement CPU hotplug on Arm

Move IRQs from dying CPU to the online ones when a CPU is getting offlined. When onlining, rebalance all IRQs in a round-robin fashion. Guest-bound IRQs are already handled by scheduler in the process of moving vCPUs to active pCPUs, so we only need to handle IRQs used by Xen itself.

Signed-off-by Mykyta Poturai <[email protected]>

---
v6->v7:
* replace ifdef with IS_ENABLED

v5->v6:
* don't do any balancing on boot
* only do balancing when cpu hotplug is enabled

v4->v5:
* handle CPU onlining as well
* more comments
* fix crash when ESPI is disabled
* don't assume CPU 0 is a boot CPU
* use insigned int for irq number
* remove assumption that all irqs a bound to CPU 0 by default from the commit message

v3->v4:
* patch introduced

---
  xen/arch/arm/include/asm/irq.h |  6 ++++
  xen/arch/arm/irq.c             | 59 ++++++++++++++++++++++++++++++++++
  xen/arch/arm/smpboot.c         |  7 ++++
  3 files changed, 72 insertions(+)

xen/arch/arm/include/asm/irq.h +6 -0

index 09788dbfeb..3ed55e02c3 100644
@@ -126,6 +126,12 @@ bool irq_type_set_by_domain(const struct domain *d);
  void irq_end_none(struct irq_desc *irq);
  #define irq_end_none irq_end_none
 
+#ifdef CONFIG_CPU_HOTPLUG
+void rebalance_irqs(unsigned int from, bool up);
+#else
+static inline void rebalance_irqs(unsigned int from, bool up) {}
+#endif
+
  #endif /* _ASM_HW_IRQ_H */
  /*
   * Local variables:

xen/arch/arm/irq.c +40 -0

index 7204bc2b68..447bee428e 100644
@@ -158,6 +158,60 @@ static int init_local_irq_data(unsigned int cpu)
      return 0;
  }
 
+#ifdef CONFIG_CPU_HOTPLUG
+static int cpu_next;
+
+static void balance_irq(int irq, unsigned int from, bool up)
+{
+    struct irq_desc *desc = irq_to_desc(irq);
+    unsigned long flags;
+
+    ASSERT(!cpumask_empty(&cpu_online_map));
+
+    spin_lock_irqsave(&desc->lock, flags);
+    if ( likely(!desc->action) )
+        goto out;
+
+    if ( likely(test_bit(_IRQ_GUEST, &desc->status) ||
+                test_bit(_IRQ_MOVE_PENDING, &desc->status)) )
+        goto out;
+
+    /*
+     * Setting affinity to a mask of multiple CPUs causes the GIC drivers to
+     * select one CPU from that mask. If the dying CPU was included in the IRQ's
+     * affinity mask, we cannot determine exactly which CPU the interrupt is
+     * currently routed to, as GIC drivers lack a concrete get_affinity API. So
+     * to be safe we must reroute it to a new, definitely online, CPU. In the
+     * case of CPU going down, we move only the interrupt that could reside on
+     * it. Otherwise, we rearrange all interrupts in a round-robin fashion.
+     */
+    if ( !up && !cpumask_test_cpu(from, desc->affinity) )
+        goto out;
+
+    cpu_next = cpumask_cycle(cpu_next, &cpu_online_map);
+    irq_set_affinity(desc, cpumask_of(cpu_next));
+
+out:
+    spin_unlock_irqrestore(&desc->lock, flags);
+}
+
+void rebalance_irqs(unsigned int from, bool up)
+{
+    int irq;

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

+#endif
+}
+#endif /* CONFIG_CPU_HOTPLUG */
+
  static int cpu_callback(struct notifier_block *nfb, unsigned long action,
                          void *hcpu)
  {

@@ -172,6 +226,11 @@ static int cpu_callback(struct notifier_block *nfb, unsigned long action,
              printk(XENLOG_ERR "Unable to allocate local IRQ for CPU%u\n",
                     cpu);
          break;
+    case CPU_ONLINE:
+        if ( IS_ENABLED(CONFIG_CPU_HOTPLUG) &&
+             system_state >= SYS_STATE_active )
+            rebalance_irqs(cpu, true);
+        break;
      }
 
      return notifier_from_errno(rc);

xen/arch/arm/smpboot.c +7 -0

index 7f3cfa812e..7d877179c0 100644
@@ -425,6 +425,13 @@ void __cpu_disable(void)
 
      smp_mb();
 
+    /*
+     * Now that the interrupts are cleared and the CPU marked as offline,
+     * move interrupts out of it
+     */
+    if ( IS_ENABLED(CONFIG_CPU_HOTPLUG) )
+        rebalance_irqs(cpu, false);
+
      /* Return to caller; eventually the IPI mechanism will unwind and the
       * scheduler will drop to the idle loop, which will call stop_cpu(). */
  }

For the purposes of certification, we want as little code as possible to be unconditionally compiled in. Make CPU hotplug and SMT operations configurable to ease the process. This will also help with introducing CPU hotplug on Arm, where it needs to be configurable.

Signed-off-by Mykyta Poturai <[email protected]>

Quoted 6 lines · Mykyta Poturai

For the purposes of certification, we want as little code as possible to be unconditionally compiled in. Make CPU hotplug and SMT operations configurable to ease the process. This will also help with introducing CPU hotplug on Arm, where it needs to be configurable.

Signed-off-by Mykyta Poturai <[email protected]>

Looks largely okay from a technical pov; one nit and one (repeated) remark below.

xen/arch/x86/sysctl.c static long cf_check smt_up_down_helper(void *data) · line 61

Quoted 12 lines

xen/arch/x86/sysctl.c +5 -0

@@ -53,6 +53,11 @@ static long cf_check smt_up_down_helper(void *data)
      unsigned int cpu, sibling_mask = boot_cpu_data.x86_num_siblings - 1;
      int ret = 0;
 
+    if ( !IS_ENABLED(CONFIG_CPU_HOTPLUG) )
+    {
+        ASSERT_UNREACHABLE();
+        return -EOPNOTSUPP;
+    }
      opt_smt = up;

Another blank line above this one perhaps?

xen/common/Kconfig config SYSTEM_SUSPEND · line 646

Quoted 13 lines

xen/common/Kconfig +7 -0

@@ -637,6 +637,14 @@ config SYSTEM_SUSPEND
 
  	  If unsure, say N.
 
+config CPU_HOTPLUG
+	bool "CPU online/offline support"
+	depends on X86
+	default y
+	help
+	  Enable support for bringing CPUs online and offline at runtime. On
+	  X86 this is required for disabling SMT.

The name of this option may need input from others; I'm not quite convinced that this is a good name, as there's no true "hot-plugging" involved here. IOW I fear the present name is misleading.

Jan

Quoted 32 lines · Jan Beulich

Quoted 6 lines · Mykyta Poturai

For the purposes of certification, we want as little code as possible to be unconditionally compiled in. Make CPU hotplug and SMT operations configurable to ease the process. This will also help with introducing CPU hotplug on Arm, where it needs to be configurable.

Signed-off-by Mykyta Poturai <[email protected]>

Looks largely okay from a technical pov; one nit and one (repeated) remark below.

xen/arch/x86/sysctl.c static long cf_check smt_up_down_helper(void *data) · line 61

Quoted 12 lines

xen/arch/x86/sysctl.c +5 -0

@@ -53,6 +53,11 @@ static long cf_check smt_up_down_helper(void *data)
      unsigned int cpu, sibling_mask = boot_cpu_data.x86_num_siblings - 1;
      int ret = 0;
 
+    if ( !IS_ENABLED(CONFIG_CPU_HOTPLUG) )
+    {
+        ASSERT_UNREACHABLE();
+        return -EOPNOTSUPP;
+    }
      opt_smt = up;

Another blank line above this one perhaps?

Quoted 6 lines

xen/common/Kconfig +0 -0

@@ -637,6 +637,14 @@ config SYSTEM_SUSPEND
 
  	  If unsure, say N.
 

Quoted 11 lines

xen/common/Kconfig config SYSTEM_SUSPEND · line 646

Quoted 7 lines

xen/common/Kconfig +7 -0

+config CPU_HOTPLUG
+	bool "CPU online/offline support"
+	depends on X86
+	default y
+	help
+	  Enable support for bringing CPUs online and offline at runtime. On
+	  X86 this is required for disabling SMT.

The name of this option may need input from others; I'm not quite convinced that this is a good name, as there's no true "hot-plugging" involved here. IOW I fear the present name is misleading.

My first idea was "CONFIG_RUNTIME_CPU_CONTROL" I can switch back to it.

On 08.04.2026 14:21, Mykyta Poturai wrote:

Quoted 45 lines · Jan Beulich

Quoted 43 lines · Mykyta Poturai

Quoted 6 lines

For the purposes of certification, we want as little code as possible to be unconditionally compiled in. Make CPU hotplug and SMT operations configurable to ease the process. This will also help with introducing CPU hotplug on Arm, where it needs to be configurable.

Signed-off-by Mykyta Poturai <[email protected]>

Looks largely okay from a technical pov; one nit and one (repeated) remark below.

xen/arch/x86/sysctl.c static long cf_check smt_up_down_helper(void *data) · line 61

Quoted 12 lines

xen/arch/x86/sysctl.c +5 -0

@@ -53,6 +53,11 @@ static long cf_check smt_up_down_helper(void *data)
       unsigned int cpu, sibling_mask = boot_cpu_data.x86_num_siblings - 1;
       int ret = 0;
 
+    if ( !IS_ENABLED(CONFIG_CPU_HOTPLUG) )
+    {
+        ASSERT_UNREACHABLE();
+        return -EOPNOTSUPP;
+    }
       opt_smt = up;

Another blank line above this one perhaps?

xen/common/Kconfig config SYSTEM_SUSPEND · line 646

Quoted 13 lines

xen/common/Kconfig +7 -0

@@ -637,6 +637,14 @@ config SYSTEM_SUSPEND
 
   	  If unsure, say N.
 
+config CPU_HOTPLUG
+	bool "CPU online/offline support"
+	depends on X86
+	default y
+	help
+	  Enable support for bringing CPUs online and offline at runtime. On
+	  X86 this is required for disabling SMT.

The name of this option may need input from others; I'm not quite convinced that this is a good name, as there's no true "hot-plugging" involved here. IOW I fear the present name is misleading.

Quoted 1 line

My first idea was "CONFIG_RUNTIME_CPU_CONTROL" I can switch back to it.

I could live with that, for at least not being misleading. CPU_ONLINE or CPU_ONLINE_OFFLINE might be another option, possibly better suited to later become a dependency (select) of a true CPU_HOTPLUG. As said, input from others may be helpful.

Jan

Quoted 5 lines · Mykyta Poturai

xen/common/Kconfig +0 -0

@@ -638,9 +638,9 @@ config SYSTEM_SUSPEND
  	  If unsure, say N.
 

xen/common/Kconfig config SYSTEM_SUSPEND · line 641

Quoted 5 lines

xen/common/Kconfig +1 -3

  config CPU_HOTPLUG
-	bool "CPU online/offline support"
-	depends on X86
-	default y
+	bool "CPU online/offline support" if EXPERT || X86

Why not just EXPERT?

xen/common/Kconfig config SYSTEM_SUSPEND · line 642

Quoted 1 line

xen/common/Kconfig +1 -0

+	depends on X86 || (ARM_64 && !HAS_ITS)

The !HAS_ITS is puzzling, and it doesn't help that that option looks misnamed (HAS_* shouldn't have prompts imo). The description says something there, yes, but then also mentions FFA and TEE. Yet for those the option remains available.

xen/common/Kconfig config SYSTEM_SUSPEND · line 643

Quoted 1 line

xen/common/Kconfig +1 -0

+	default y if X86

Shorter as "default X86".

Jan

Quoted 6 lines · Jan Beulich

Quoted 5 lines · Mykyta Poturai

xen/common/Kconfig +0 -0

@@ -638,9 +638,9 @@ config SYSTEM_SUSPEND
  	  If unsure, say N.
 

Quoted 7 lines

xen/common/Kconfig config SYSTEM_SUSPEND · line 641

Quoted 5 lines

xen/common/Kconfig +1 -3

  config CPU_HOTPLUG
-	bool "CPU online/offline support"
-	depends on X86
-	default y
+	bool "CPU online/offline support" if EXPERT || X86

Why not just EXPERT?

Should it be marked as EXPERT on x86? I considered that if the option was non configurable (always enabled), it should stay enabled by default and always visible.

With CPU hotplug sysctls implemented on Arm it becomes useful to have a tool for calling them.

According to the commit history it seems that putting hptool under config MIGRATE was a measure to fix IA64 build. As IA64 is no longer supported it can now be brought back. So build it unconditionally.

Signed-off-by Mykyta Poturai <[email protected]>

---
v6->v7:
* no changes

v5->v6:
* don't change order in Makefile

v4->v5:
* make hptool always build

v3->v4:
* no changes

v2->v3:
* no changes

v1->v2:
* switch to configure from legacy config

---
 tools/libs/guest/Makefile.common | 2 +-
 tools/misc/Makefile              | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

tools/libs/guest/Makefile.common +1 -0

index b928a4a246..03dfcee7fa 100644
@@ -7,6 +7,7 @@ OBJS-y += xg_private.o
 OBJS-y += xg_domain.o
 OBJS-y += xg_suspend.o
 OBJS-y += xg_resume.o
+OBJS-y += xg_offline_page.o
 ifeq ($(CONFIG_MIGRATE),y)
 OBJS-y += xg_sr_common.o
 OBJS-$(CONFIG_X86) += xg_sr_common_x86.o

Quoted 44 lines · Mykyta Poturai

With CPU hotplug sysctls implemented on Arm it becomes useful to have a tool for calling them.

According to the commit history it seems that putting hptool under config MIGRATE was a measure to fix IA64 build. As IA64 is no longer supported it can now be brought back. So build it unconditionally.

Signed-off-by Mykyta Poturai <[email protected]>

---
v6->v7:
* no changes

v5->v6:
* don't change order in Makefile

v4->v5:
* make hptool always build

v3->v4:
* no changes

v2->v3:
* no changes

v1->v2:
* switch to configure from legacy config

---
  tools/libs/guest/Makefile.common | 2 +-
  tools/misc/Makefile              | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

tools/libs/guest/Makefile.common +1 -0

index b928a4a246..03dfcee7fa 100644
@@ -7,6 +7,7 @@ OBJS-y += xg_private.o
  OBJS-y += xg_domain.o
  OBJS-y += xg_suspend.o
  OBJS-y += xg_resume.o
+OBJS-y += xg_offline_page.o
  ifeq ($(CONFIG_MIGRATE),y)
  OBJS-y += xg_sr_common.o
  OBJS-$(CONFIG_X86) += xg_sr_common_x86.o

tools/libs/guest/Makefile.common OBJS-$(CONFIG_X86) += xg_sr_save_x86_pv.o · line 23

Quoted 7 lines

tools/libs/guest/Makefile.common +0 -1

@@ -17,7 +18,6 @@ OBJS-$(CONFIG_X86) += xg_sr_save_x86_pv.o
  OBJS-$(CONFIG_X86) += xg_sr_save_x86_hvm.o
  OBJS-y += xg_sr_restore.o
  OBJS-y += xg_sr_save.o
-OBJS-y += xg_offline_page.o
  else
  OBJS-y += xg_nomigrate.o
  endif

This looks wrong to me. There are x86-specifics in that file, which shouldn't be built on Arm. And the name of the file also doesn't indicate any relation to CPU management.

Jan

Quoted 48 lines · Jan Beulich

Quoted 47 lines · Mykyta Poturai

With CPU hotplug sysctls implemented on Arm it becomes useful to have a tool for calling them.

According to the commit history it seems that putting hptool under config MIGRATE was a measure to fix IA64 build. As IA64 is no longer supported it can now be brought back. So build it unconditionally.

Signed-off-by Mykyta Poturai <[email protected]>

---
v6->v7:
* no changes

v5->v6:
* don't change order in Makefile

v4->v5:
* make hptool always build

v3->v4:
* no changes

v2->v3:
* no changes

v1->v2:
* switch to configure from legacy config

---
  tools/libs/guest/Makefile.common | 2 +-
  tools/misc/Makefile              | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

tools/libs/guest/Makefile.common +1 -0

index b928a4a246..03dfcee7fa 100644
@@ -7,6 +7,7 @@ OBJS-y += xg_private.o
  OBJS-y += xg_domain.o
  OBJS-y += xg_suspend.o
  OBJS-y += xg_resume.o
+OBJS-y += xg_offline_page.o
  ifeq ($(CONFIG_MIGRATE),y)
  OBJS-y += xg_sr_common.o
  OBJS-$(CONFIG_X86) += xg_sr_common_x86.o

@@ -17,7 +18,6 @@ OBJS-$(CONFIG_X86) += xg_sr_save_x86_pv.o
  OBJS-$(CONFIG_X86) += xg_sr_save_x86_hvm.o
  OBJS-y += xg_sr_restore.o
  OBJS-y += xg_sr_save.o

Quoted 8 lines

tools/libs/guest/Makefile.common OBJS-$(CONFIG_X86) += xg_sr_save_x86_pv.o · line 23

Quoted 4 lines

tools/libs/guest/Makefile.common +0 -1

-OBJS-y += xg_offline_page.o
  else
  OBJS-y += xg_nomigrate.o
  endif

This looks wrong to me. There are x86-specifics in that file, which shouldn't be built on Arm. And the name of the file also doesn't indicate any relation to CPU management.

xen-hptool requires xg_offline_page as it has both CPU and memory hotplug commands. Without building xg_offline_page it fails with

xen-hptool: symbol lookup error: xen-hptool: undefined symbol:
xc_mark_page_offline, version libxenguest_4.22.0

when trying to do memory ops.

Is it an acceptable behavior?

If so I guess we can build xg_offline page only on x86.

Quoted 61 lines · Mykyta Poturai

Quoted 48 lines · Jan Beulich

Quoted 47 lines · Mykyta Poturai

With CPU hotplug sysctls implemented on Arm it becomes useful to have a tool for calling them.

According to the commit history it seems that putting hptool under config MIGRATE was a measure to fix IA64 build. As IA64 is no longer supported it can now be brought back. So build it unconditionally.

Signed-off-by Mykyta Poturai <[email protected]>

---
v6->v7:
* no changes

v5->v6:
* don't change order in Makefile

v4->v5:
* make hptool always build

v3->v4:
* no changes

v2->v3:
* no changes

v1->v2:
* switch to configure from legacy config

---
   tools/libs/guest/Makefile.common | 2 +-
   tools/misc/Makefile              | 2 +-
   2 files changed, 2 insertions(+), 2 deletions(-)

tools/libs/guest/Makefile.common +1 -0

index b928a4a246..03dfcee7fa 100644
@@ -7,6 +7,7 @@ OBJS-y += xg_private.o
   OBJS-y += xg_domain.o
   OBJS-y += xg_suspend.o
   OBJS-y += xg_resume.o
+OBJS-y += xg_offline_page.o
   ifeq ($(CONFIG_MIGRATE),y)
   OBJS-y += xg_sr_common.o
   OBJS-$(CONFIG_X86) += xg_sr_common_x86.o

@@ -17,7 +18,6 @@ OBJS-$(CONFIG_X86) += xg_sr_save_x86_pv.o
   OBJS-$(CONFIG_X86) += xg_sr_save_x86_hvm.o
   OBJS-y += xg_sr_restore.o
   OBJS-y += xg_sr_save.o

Quoted 8 lines

tools/libs/guest/Makefile.common OBJS-$(CONFIG_X86) += xg_sr_save_x86_pv.o · line 23

Quoted 4 lines

tools/libs/guest/Makefile.common +0 -1

-OBJS-y += xg_offline_page.o
   else
   OBJS-y += xg_nomigrate.o
   endif

This looks wrong to me. There are x86-specifics in that file, which shouldn't be built on Arm. And the name of the file also doesn't indicate any relation to CPU management.

xen-hptool requires xg_offline_page as it has both CPU and memory hotplug commands. Without building xg_offline_page it fails with

Quoted 6 lines

xen-hptool: symbol lookup error: xen-hptool: undefined symbol:
xc_mark_page_offline, version libxenguest_4.22.0

when trying to do memory ops.

Is it an acceptable behavior?

I don't think so, no. The tool wouldn't, aiui, load at all then if built with "bindnow" enabled.

Quoted 1 line

If so I guess we can build xg_offline page only on x86.

We still need to, imo. But the tool still needs to be usable no matter how specifically it is built. It should avoid referencing xg_offline_page.c functions when built for non-x86.

Jan

Quoted 74 lines · Jan Beulich

Quoted 61 lines · Mykyta Poturai

Quoted 48 lines · Jan Beulich

Quoted 47 lines · Mykyta Poturai

With CPU hotplug sysctls implemented on Arm it becomes useful to have a tool for calling them.

According to the commit history it seems that putting hptool under config MIGRATE was a measure to fix IA64 build. As IA64 is no longer supported it can now be brought back. So build it unconditionally.

Signed-off-by Mykyta Poturai <[email protected]>

---
v6->v7:
* no changes

v5->v6:
* don't change order in Makefile

v4->v5:
* make hptool always build

v3->v4:
* no changes

v2->v3:
* no changes

v1->v2:
* switch to configure from legacy config

---
   tools/libs/guest/Makefile.common | 2 +-
   tools/misc/Makefile              | 2 +-
   2 files changed, 2 insertions(+), 2 deletions(-)

tools/libs/guest/Makefile.common +1 -0

index b928a4a246..03dfcee7fa 100644
@@ -7,6 +7,7 @@ OBJS-y += xg_private.o
   OBJS-y += xg_domain.o
   OBJS-y += xg_suspend.o
   OBJS-y += xg_resume.o
+OBJS-y += xg_offline_page.o
   ifeq ($(CONFIG_MIGRATE),y)
   OBJS-y += xg_sr_common.o
   OBJS-$(CONFIG_X86) += xg_sr_common_x86.o

@@ -17,7 +18,6 @@ OBJS-$(CONFIG_X86) += xg_sr_save_x86_pv.o
   OBJS-$(CONFIG_X86) += xg_sr_save_x86_hvm.o
   OBJS-y += xg_sr_restore.o
   OBJS-y += xg_sr_save.o

Quoted 8 lines

tools/libs/guest/Makefile.common OBJS-$(CONFIG_X86) += xg_sr_save_x86_pv.o · line 23

Quoted 4 lines

tools/libs/guest/Makefile.common +0 -1

-OBJS-y += xg_offline_page.o
   else
   OBJS-y += xg_nomigrate.o
   endif

This looks wrong to me. There are x86-specifics in that file, which shouldn't be built on Arm. And the name of the file also doesn't indicate any relation to CPU management.

xen-hptool requires xg_offline_page as it has both CPU and memory hotplug commands. Without building xg_offline_page it fails with

Quoted 6 lines

xen-hptool: symbol lookup error: xen-hptool: undefined symbol:
xc_mark_page_offline, version libxenguest_4.22.0

when trying to do memory ops.

Is it an acceptable behavior?

I don't think so, no. The tool wouldn't, aiui, load at all then if built with "bindnow" enabled.

Quoted 1 line

If so I guess we can build xg_offline page only on x86.

Quoted 3 lines

We still need to, imo. But the tool still needs to be usable no matter how specifically it is built. It should avoid referencing xg_offline_page.c functions when built for non-x86.

As I understand, the usage of arch-specific compile time checks is heavily discouraged in tools. So I don’t think it would be approved by tools maintainers. Do we really need to omit this file if memory ops are already getting blocked by Xen on Arm anyway?

Quoted 21 lines · Jan Beulich

Quoted 8 lines · Mykyta Poturai

Quoted 3 lines · Jan Beulich

This looks wrong to me. There are x86-specifics in that file, which shouldn't be built on Arm. And the name of the file also doesn't indicate any relation to CPU management.

xen-hptool requires xg_offline_page as it has both CPU and memory hotplug commands. Without building xg_offline_page it fails with

Quoted 6 lines

xen-hptool: symbol lookup error: xen-hptool: undefined symbol:
xc_mark_page_offline, version libxenguest_4.22.0

when trying to do memory ops.

Is it an acceptable behavior?

I don't think so, no. The tool wouldn't, aiui, load at all then if built with "bindnow" enabled.

Quoted 1 line

If so I guess we can build xg_offline page only on x86.

Quoted 3 lines

We still need to, imo. But the tool still needs to be usable no matter how specifically it is built. It should avoid referencing xg_offline_page.c functions when built for non-x86.

As I understand, the usage of arch-specific compile time checks is heavily discouraged in tools. So I don’t think it would be approved by tools maintainers. Do we really need to omit this file if memory ops are already getting blocked by Xen on Arm anyway?

Quoted 22 lines · Mykyta Poturai

Quoted 21 lines · Jan Beulich

Quoted 8 lines · Mykyta Poturai

Quoted 3 lines · Jan Beulich

This looks wrong to me. There are x86-specifics in that file, which shouldn't be built on Arm. And the name of the file also doesn't indicate any relation to CPU management.

xen-hptool requires xg_offline_page as it has both CPU and memory hotplug commands. Without building xg_offline_page it fails with

Quoted 6 lines

xen-hptool: symbol lookup error: xen-hptool: undefined symbol:
xc_mark_page_offline, version libxenguest_4.22.0

when trying to do memory ops.

Is it an acceptable behavior?

I don't think so, no. The tool wouldn't, aiui, load at all then if built with "bindnow" enabled.

Quoted 1 line

If so I guess we can build xg_offline page only on x86.

Quoted 8 lines

Quoted 3 lines

We still need to, imo. But the tool still needs to be usable no matter how specifically it is built. It should avoid referencing xg_offline_page.c functions when built for non-x86.

As I understand, the usage of arch-specific compile time checks is heavily discouraged in tools. So I don’t think it would be approved by tools maintainers. Do we really need to omit this file if memory ops are already getting blocked by Xen on Arm anyway?

So you are trying to modify a library and introduced untested functionality just to be able to build a different tool? I don't think that a good idea especially in this case where it's more than just glue code between a binary and xen.

We could change the library to provide the missing symbols, but it is probably best to keep it that way for now.

So, how about changing xen-hptool to have reduced functionality on other platform, and keep the 'mem-*' command on x86 only? You could move the function that implement the 'mem-*' command into a separate file, that compile only on x86 (or more specifically when CONFIG_MIGRATE is set) and just have a "#if defined(__i386__) || defined(__x86_64__)" in the main_options array.

They are compile-time arch-specific check everywhere in tools. Arch specific are often implemented in separated source file, this mean we can limit the #ifdefs to a minimum and keep the code readable.

Thanks,

Quoted 15 lines · Anthony PERARD

So you are trying to modify a library and introduced untested functionality just to be able to build a different tool? I don't think that a good idea especially in this case where it's more than just glue code between a binary and xen.

We could change the library to provide the missing symbols, but it is probably best to keep it that way for now.

So, how about changing xen-hptool to have reduced functionality on other platform, and keep the 'mem-*' command on x86 only? You could move the function that implement the 'mem-*' command into a separate file, that compile only on x86 (or more specifically when CONFIG_MIGRATE is set) and just have a "#if defined(__i386__) || defined(__x86_64__)" in the main_options array.

Quoted 3 lines

They are compile-time arch-specific check everywhere in tools. Arch specific are often implemented in separated source file, this mean we can limit the #ifdefs to a minimum and keep the code readable.

Should I also do the same thing for SMT operations?