[PATCH RFC V6 10/24] arm/virt: Init PMU at host for all present vCPUs

salil.mehta@opnsrc.net posted 24 patches 1 month, 2 weeks ago
[PATCH RFC V6 10/24] arm/virt: Init PMU at host for all present vCPUs
Posted by salil.mehta@opnsrc.net 1 month, 2 weeks ago
From: Salil Mehta <salil.mehta@huawei.com>

ARM architecture requires that all CPUs which form part of the VM must
expose identical feature sets and consistent system components at creation
time. This includes the Performance Monitoring Unit (PMU). If only the boot
CPUs had their PMU state initialized, the remaining CPUs defined by
`smp.disabled_cpus` would not match this architectural requirement, leading
to inconsistencies and guest misbehavior.

To comply with this constraint, PMU initialization must cover the entire set
of present vCPUs:

    present = smp.cpus + smp.disabled_cpus

CPUs outside this set (`smp.max_cpus - present`) are not considered part of
the machine at creation and are therefore not initialized.

Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 hw/arm/virt.c         | 13 +++++++---
 include/hw/arm/virt.h |  1 +
 include/hw/core/cpu.h | 57 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+), 4 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index ee09aa19bd..3980f553db 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2087,12 +2087,13 @@ static void finalize_gic_version(VirtMachineState *vms)
 static void virt_post_cpus_gic_realized(VirtMachineState *vms,
                                         MemoryRegion *sysmem)
 {
+    CPUArchIdList *possible_cpus = vms->parent.possible_cpus;
     int max_cpus = MACHINE(vms)->smp.max_cpus;
-    bool aarch64, pmu, steal_time;
+    bool aarch64, steal_time;
     CPUState *cpu;
 
     aarch64 = object_property_get_bool(OBJECT(first_cpu), "aarch64", NULL);
-    pmu = object_property_get_bool(OBJECT(first_cpu), "pmu", NULL);
+    vms->pmu = object_property_get_bool(OBJECT(first_cpu), "pmu", NULL);
     steal_time = object_property_get_bool(OBJECT(first_cpu),
                                           "kvm-steal-time", NULL);
 
@@ -2123,8 +2124,12 @@ static void virt_post_cpus_gic_realized(VirtMachineState *vms,
             exit(1);
         }
 
-        CPU_FOREACH(cpu) {
-            if (pmu) {
+        CPU_FOREACH_POSSIBLE(cpu, possible_cpus) {
+            if (!cpu) {
+                continue;
+            }
+
+            if (vms->pmu) {
                 assert(arm_feature(&ARM_CPU(cpu)->env, ARM_FEATURE_PMU));
                 if (kvm_irqchip_in_kernel()) {
                     kvm_arm_pmu_set_irq(ARM_CPU(cpu), VIRTUAL_PMU_IRQ);
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index ace4154cc6..02cc311452 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -154,6 +154,7 @@ struct VirtMachineState {
     bool mte;
     bool dtb_randomness;
     bool second_ns_uart_present;
+    bool pmu;
     OnOffAuto acpi;
     VirtGICType gic_version;
     VirtIOMMUType iommu;
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 5eaf41a566..2ee202a8a5 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -602,6 +602,63 @@ extern CPUTailQ cpus_queue;
 #define CPU_FOREACH_SAFE(cpu, next_cpu) \
     QTAILQ_FOREACH_SAFE_RCU(cpu, &cpus_queue, node, next_cpu)
 
+
+/**
+ * CPU_FOREACH_POSSIBLE(cpu_, archid_list_)
+ *
+ * Iterate over all entries in a CPUArchIdList, assigning each entry’s
+ * CPUState* to @cpu_. This hides the loop index and reads like a normal
+ * C for-loop.
+ *
+ * A CPUArchIdList represents the set of *possible* CPUs for a machine.
+ * Each entry contains:
+ *   - @cpu:        CPUState pointer, or NULL if not realized yet
+ *   - @arch_id:    architecture-specific identifier (e.g. MPIDR)
+ *   - @vcpus_count: number of vCPUs represented (usually 1)
+ *
+ * The list models *possible* CPUs: it includes (a) currently plugged vCPUs
+ * made available through hotplug, (b) present (and perhaps visible to OSPM)
+ * but kept ACPI-disabled vCPUs, and (c) reserved slots for CPUs that may be
+ * created in the future. This supports co-existence of hotpluggable and
+ * admin-disabled vCPUs if architectures permit.
+ *
+ * Example:
+ *
+ *   CPUArchIdList *alist = machine_possible_cpus(ms);
+ *   CPUState *cpu;
+ *
+ *   CPU_FOREACH_POSSIBLE(cpu, alist) {
+ *       if (!cpu) {
+ *           continue; // reserved slot for hotplug case
+ *       }
+ *
+ *       < Do Something >
+ *   }
+ *
+ * Expanded equivalent:
+ *
+ *   for (int __cpu_idx = 0; alist && __cpu_idx < alist->len; __cpu_idx++) {
+ *       if ((cpu = alist->cpus[__cpu_idx].cpu, 1)) {
+ *           if (!cpu) {
+ *               continue;
+ *           }
+ *
+ *           < Do Something >
+ *       }
+ *   }
+ *
+ * Notes:
+ *   - Callers must check @cpu for NULL when filtering unplugged CPUs.
+ *   - Mirrors the style of CPU_FOREACH(), but iterates all *possible* CPUs
+ *     (plugged, ACPI-disabled, and reserved slots) rather than only present
+ *     and enabled vCPUs.
+ */
+#define CPU_FOREACH_POSSIBLE(cpu_, archid_list_) \
+    for (int __cpu_idx = 0; \
+         (archid_list_) && __cpu_idx < (archid_list_)->len; \
+         __cpu_idx++) \
+        if (((cpu_) = (archid_list_)->cpus[__cpu_idx].cpu, 1))
+
 extern __thread CPUState *current_cpu;
 
 /**
-- 
2.34.1


Re: [PATCH RFC V6 10/24] arm/virt: Init PMU at host for all present vCPUs
Posted by Igor Mammedov 1 month, 1 week ago
On Wed,  1 Oct 2025 01:01:13 +0000
salil.mehta@opnsrc.net wrote:

> From: Salil Mehta <salil.mehta@huawei.com>
> 

...

> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index 5eaf41a566..2ee202a8a5 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -602,6 +602,63 @@ extern CPUTailQ cpus_queue;
>  #define CPU_FOREACH_SAFE(cpu, next_cpu) \
>      QTAILQ_FOREACH_SAFE_RCU(cpu, &cpus_queue, node, next_cpu)
>  
> +
> +/**
> + * CPU_FOREACH_POSSIBLE(cpu_, archid_list_)
> + *
> + * Iterate over all entries in a CPUArchIdList, assigning each entry’s
> + * CPUState* to @cpu_. This hides the loop index and reads like a normal
> + * C for-loop.
> + *
> + * A CPUArchIdList represents the set of *possible* CPUs for a machine.
> + * Each entry contains:
> + *   - @cpu:        CPUState pointer, or NULL if not realized yet
> + *   - @arch_id:    architecture-specific identifier (e.g. MPIDR)
> + *   - @vcpus_count: number of vCPUs represented (usually 1)
> + *
> + * The list models *possible* CPUs: it includes (a) currently plugged vCPUs
> + * made available through hotplug, (b) present (and perhaps visible to OSPM)
> + * but kept ACPI-disabled vCPUs, and (c) reserved slots for CPUs that may be
> + * created in the future. This supports co-existence of hotpluggable and
> + * admin-disabled vCPUs if architectures permit.
> + *
> + * Example:
> + *
> + *   CPUArchIdList *alist = machine_possible_cpus(ms);
> + *   CPUState *cpu;
> + *
> + *   CPU_FOREACH_POSSIBLE(cpu, alist) {
> + *       if (!cpu) {
> + *           continue; // reserved slot for hotplug case
> + *       }
> + *
> + *       < Do Something >
> + *   }
> + *
> + * Expanded equivalent:
> + *
> + *   for (int __cpu_idx = 0; alist && __cpu_idx < alist->len; __cpu_idx++) {
> + *       if ((cpu = alist->cpus[__cpu_idx].cpu, 1)) {
> + *           if (!cpu) {
> + *               continue;
> + *           }
> + *
> + *           < Do Something >
> + *       }
> + *   }
> + *
> + * Notes:
> + *   - Callers must check @cpu for NULL when filtering unplugged CPUs.
> + *   - Mirrors the style of CPU_FOREACH(), but iterates all *possible* CPUs
> + *     (plugged, ACPI-disabled, and reserved slots) rather than only present
> + *     and enabled vCPUs.
> + */
> +#define CPU_FOREACH_POSSIBLE(cpu_, archid_list_) \
> +    for (int __cpu_idx = 0; \
> +         (archid_list_) && __cpu_idx < (archid_list_)->len; \
> +         __cpu_idx++) \
> +        if (((cpu_) = (archid_list_)->cpus[__cpu_idx].cpu, 1))
> +
>  extern __thread CPUState *current_cpu;

make it a separate patch and refactor existing loops to use it.

>  
>  /**