Re: [PATCH v2 08/10] cpus: expose only realized vCPUs to global &cpus_queue

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 7/2/25 17:20, Igor Mammedov wrote:
> cpu_list_add() was doing 2 distinct things:
> - assign some index to vCPU
> - add unrealized (thus in inconsistent state) vCPU to &cpus_queue
> 
> Code using CPU_FOREACH() macro would iterate over possibly
> unrealized vCPUs, often dealt with special casing.
> 
> Instead of working around of vCPU existence in cpus_queue,
> split out cpu_index assignment from cpu_list_add(),

Better split 2 distinct changes in 2 patches for clarity.

> and move the later to the end of realize stage,
> right before vCPU is let run.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> CC: Yanan Wang <wangyanan55@huawei.com>
> CC: Zhao Liu <zhao1.liu@intel.com>
> ---
>   include/hw/core/cpu.h |  6 ++++++
>   cpu-common.c          | 23 ++++++++++++++---------
>   cpu-target.c          |  2 +-
>   hw/core/cpu-common.c  |  2 ++
>   4 files changed, 23 insertions(+), 10 deletions(-)
> 
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index fb397cdfc5..c338fd31bd 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -750,6 +750,12 @@ bool cpu_virtio_is_big_endian(CPUState *cpu);
>   
>   #endif /* CONFIG_USER_ONLY */
>   
> +/**
> + * cpu_auto_assign_cpu_index:
> + * @cpu: The CPU to be assigned a cpu_index
> + */
> +void cpu_auto_assign_cpu_index(CPUState *cpu);
> +
>   /**
>    * cpu_list_add:
>    * @cpu: The CPU to be added to the list of CPUs.
> diff --git a/cpu-common.c b/cpu-common.c
> index 4248b2d727..92f3d00e56 100644
> --- a/cpu-common.c
> +++ b/cpu-common.c
> @@ -71,15 +71,7 @@ int cpu_get_free_index(void)
>       return max_cpu_index;
>   }
>   
> -CPUTailQ cpus_queue = QTAILQ_HEAD_INITIALIZER(cpus_queue);
> -static unsigned int cpu_list_generation_id;
> -
> -unsigned int cpu_list_generation_id_get(void)
> -{
> -    return cpu_list_generation_id;
> -}
> -
> -void cpu_list_add(CPUState *cpu)
> +void cpu_auto_assign_cpu_index(CPUState *cpu)
>   {
>       static bool cpu_index_auto_assigned;
>   
> @@ -91,6 +83,19 @@ void cpu_list_add(CPUState *cpu)
>       } else {
>           assert(!cpu_index_auto_assigned);
>       }
> +}
> +
> +CPUTailQ cpus_queue = QTAILQ_HEAD_INITIALIZER(cpus_queue);
> +static unsigned int cpu_list_generation_id;
> +
> +unsigned int cpu_list_generation_id_get(void)
> +{
> +    return cpu_list_generation_id;
> +}
> +
> +void cpu_list_add(CPUState *cpu)
> +{
> +    QEMU_LOCK_GUARD(&qemu_cpu_list_lock);
>       QTAILQ_INSERT_TAIL_RCU(&cpus_queue, cpu, node);
>       cpu_list_generation_id++;
>   }
> diff --git a/cpu-target.c b/cpu-target.c
> index 667688332c..0c86c18a50 100644
> --- a/cpu-target.c
> +++ b/cpu-target.c
> @@ -142,7 +142,7 @@ bool cpu_exec_realizefn(CPUState *cpu, Error **errp)
>       }
>   
>       /* Wait until cpu initialization complete before exposing cpu. */
> -    cpu_list_add(cpu);
> +    cpu_auto_assign_cpu_index(cpu);
>   
>   #ifdef CONFIG_USER_ONLY
>       assert(qdev_get_vmsd(DEVICE(cpu)) == NULL ||
> diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c
> index cb79566cc5..c29737e5e3 100644
> --- a/hw/core/cpu-common.c
> +++ b/hw/core/cpu-common.c
> @@ -211,6 +211,8 @@ static void cpu_common_realizefn(DeviceState *dev, Error **errp)
>           }
>       }
>   
> +    cpu_list_add(cpu);
> +
>       if (dev->hotplugged) {
>           cpu_synchronize_post_init(cpu);
>           cpu_resume(cpu);