1. Patchew
  2. QEMU
  3. [PATCH v2 00/10] accel: Only include qdev-realized vCPUs in global &cpus_queue
  4. View patch

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

Igor Mammedov posted 10 patches 1 year ago
[PATCH v2 08/10] cpus: expose only realized vCPUs to global &cpus_queue
Posted by Igor Mammedov 1 year ago 
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(),
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);
-- 
2.43.0
Re: [PATCH v2 08/10] cpus: expose only realized vCPUs to global &cpus_queue
Posted by Philippe Mathieu-Daudé 11 months, 2 weeks ago 
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);
Re: [PATCH v2 08/10] cpus: expose only realized vCPUs to global &cpus_queue
Posted by Igor Mammedov 11 months, 1 week ago 
On Wed, 26 Feb 2025 08:16:52 +0100
Philippe Mathieu-Daudé <philmd@linaro.org> wrote:

> 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.


Will do it later, once folks decide how to fix broken TCG reset path.

do you mean:
 #1 - introduce  cpu_auto_assign_cpu_index()
 #2 - move cpu_list_add() to later stage but keep cpu_auto_assign_cpu_index()
      where it's now?


> 
> > 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);  
>
Re: [PATCH v2 08/10] cpus: expose only realized vCPUs to global &cpus_queue
Posted by Philippe Mathieu-Daudé 11 months, 1 week ago 
On 3/3/25 14:09, Igor Mammedov wrote:
> On Wed, 26 Feb 2025 08:16:52 +0100
> Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> 
>> 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.
> 
> 
> Will do it later, once folks decide how to fix broken TCG reset path.
> 
> do you mean:
>   #1 - introduce  cpu_auto_assign_cpu_index()
>   #2 - move cpu_list_add() to later stage but keep cpu_auto_assign_cpu_index()
>        where it's now?

Exactly, thanks!

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.