1. Patchew
  2. QEMU
  3. [PATCH v6 00/39] accel: Preparatory cleanups for split-accel
  4. View patch

[PATCH v6 13/39] accel: Move cpus_are_resettable() declaration to AccelClass

Philippe Mathieu-Daudé posted 39 patches 5 months, 2 weeks ago
Maintainers: Richard Henderson <richard.henderson@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Cameron Esfahani <dirty@apple.com>, Roman Bolshakov <rbolshakov@ddn.com>, Phil Dennis-Jordan <phil@philjordan.eu>, Mads Ynddal <mads@ynddal.dk>, Fabiano Rosas <farosas@suse.de>, Laurent Vivier <lvivier@redhat.com>, Stefano Stabellini <sstabellini@kernel.org>, Anthony PERARD <anthony@xenproject.org>, Paul Durrant <paul@xen.org>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Warner Losh <imp@bsdimp.com>, Kyle Evans <kevans@freebsd.org>, "Alex Bennée" <alex.bennee@linaro.org>, "Dr. David Alan Gilbert" <dave@treblig.org>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Yanan Wang <wangyanan55@huawei.com>, Zhao Liu <zhao1.liu@intel.com>, Reinoud Zandijk <reinoud@netbsd.org>, Sunil Muthuswamy <sunilmut@microsoft.com>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>, Peter Xu <peterx@redhat.com>, David Hildenbrand <david@redhat.com>, Alexander Graf <agraf@csgraf.de>, Peter Maydell <peter.maydell@linaro.org>
[PATCH v6 13/39] accel: Move cpus_are_resettable() declaration to AccelClass
Posted by Philippe Mathieu-Daudé 5 months, 2 weeks ago 
AccelOpsClass is for methods dealing with vCPUs.
When only dealing with AccelState, AccelClass is sufficient.

Move cpus_are_resettable() declaration to accel/accel-system.c.

In order to have AccelClass methods instrospect their state,
we need to pass AccelState by argument.

Adapt KVM handler.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/qemu/accel.h       |  1 +
 include/system/accel-ops.h |  1 -
 accel/accel-system.c       | 10 ++++++++++
 accel/kvm/kvm-accel-ops.c  |  6 ------
 accel/kvm/kvm-all.c        |  6 ++++++
 system/cpus.c              |  8 --------
 6 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/include/qemu/accel.h b/include/qemu/accel.h
index fb176e89bad..f987d16baaa 100644
--- a/include/qemu/accel.h
+++ b/include/qemu/accel.h
@@ -45,6 +45,7 @@ typedef struct AccelClass {
     void (*setup_post)(MachineState *ms, AccelState *accel);
     bool (*has_memory)(MachineState *ms, AddressSpace *as,
                        hwaddr start_addr, hwaddr size);
+    bool (*cpus_are_resettable)(AccelState *as);
 
     /* gdbstub related hooks */
     bool (*supports_guest_debug)(AccelState *as);
diff --git a/include/system/accel-ops.h b/include/system/accel-ops.h
index 700df92ac6d..f19245d0a0e 100644
--- a/include/system/accel-ops.h
+++ b/include/system/accel-ops.h
@@ -33,7 +33,6 @@ struct AccelOpsClass {
     /* initialization function called when accel is chosen */
     void (*ops_init)(AccelOpsClass *ops);
 
-    bool (*cpus_are_resettable)(void);
     void (*cpu_reset_hold)(CPUState *cpu);
 
     void (*create_vcpu_thread)(CPUState *cpu); /* MANDATORY NON-NULL */
diff --git a/accel/accel-system.c b/accel/accel-system.c
index a0f562ae9ff..07b75dae797 100644
--- a/accel/accel-system.c
+++ b/accel/accel-system.c
@@ -62,6 +62,16 @@ void accel_setup_post(MachineState *ms)
     }
 }
 
+bool cpus_are_resettable(void)
+{
+    AccelState *accel = current_accel();
+    AccelClass *acc = ACCEL_GET_CLASS(accel);
+    if (acc->cpus_are_resettable) {
+        return acc->cpus_are_resettable(accel);
+    }
+    return true;
+}
+
 /* initialize the arch-independent accel operation interfaces */
 void accel_init_ops_interfaces(AccelClass *ac)
 {
diff --git a/accel/kvm/kvm-accel-ops.c b/accel/kvm/kvm-accel-ops.c
index 96606090889..99f61044da5 100644
--- a/accel/kvm/kvm-accel-ops.c
+++ b/accel/kvm/kvm-accel-ops.c
@@ -78,11 +78,6 @@ static bool kvm_vcpu_thread_is_idle(CPUState *cpu)
     return !kvm_halt_in_kernel();
 }
 
-static bool kvm_cpus_are_resettable(void)
-{
-    return !kvm_enabled() || !kvm_state->guest_state_protected;
-}
-
 #ifdef TARGET_KVM_HAVE_GUEST_DEBUG
 static int kvm_update_guest_debug_ops(CPUState *cpu)
 {
@@ -96,7 +91,6 @@ static void kvm_accel_ops_class_init(ObjectClass *oc, const void *data)
 
     ops->create_vcpu_thread = kvm_start_vcpu_thread;
     ops->cpu_thread_is_idle = kvm_vcpu_thread_is_idle;
-    ops->cpus_are_resettable = kvm_cpus_are_resettable;
     ops->synchronize_post_reset = kvm_cpu_synchronize_post_reset;
     ops->synchronize_post_init = kvm_cpu_synchronize_post_init;
     ops->synchronize_state = kvm_cpu_synchronize_state;
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index c8611552d19..88fb6d36941 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -3979,6 +3979,11 @@ static void kvm_accel_instance_init(Object *obj)
     s->msr_energy.enable = false;
 }
 
+static bool kvm_cpus_are_resettable(AccelState *as)
+{
+    return !kvm_enabled() || !kvm_state->guest_state_protected;
+}
+
 /**
  * kvm_gdbstub_sstep_flags():
  *
@@ -3997,6 +4002,7 @@ static void kvm_accel_class_init(ObjectClass *oc, const void *data)
     ac->init_machine = kvm_init;
     ac->has_memory = kvm_accel_has_memory;
     ac->allowed = &kvm_allowed;
+    ac->cpus_are_resettable = kvm_cpus_are_resettable;
     ac->gdbstub_supported_sstep_flags = kvm_gdbstub_sstep_flags;
 #ifdef TARGET_KVM_HAVE_GUEST_DEBUG
     ac->supports_guest_debug = kvm_supports_guest_debug;
diff --git a/system/cpus.c b/system/cpus.c
index a43e0e4e796..4fb764ac880 100644
--- a/system/cpus.c
+++ b/system/cpus.c
@@ -195,14 +195,6 @@ void cpu_synchronize_pre_loadvm(CPUState *cpu)
     }
 }
 
-bool cpus_are_resettable(void)
-{
-    if (cpus_accel->cpus_are_resettable) {
-        return cpus_accel->cpus_are_resettable();
-    }
-    return true;
-}
-
 void cpu_exec_reset_hold(CPUState *cpu)
 {
     if (cpus_accel->cpu_reset_hold) {
-- 
2.49.0
Re: [PATCH v6 13/39] accel: Move cpus_are_resettable() declaration to AccelClass
Posted by Zhao Liu 5 months, 2 weeks ago 
On Thu, Jul 03, 2025 at 07:32:19PM +0200, Philippe Mathieu-Daudé wrote:
> Date: Thu,  3 Jul 2025 19:32:19 +0200
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> Subject: [PATCH v6 13/39] accel: Move cpus_are_resettable() declaration to
>  AccelClass
> X-Mailer: git-send-email 2.49.0
> 
> AccelOpsClass is for methods dealing with vCPUs.
> When only dealing with AccelState, AccelClass is sufficient.
> 
> Move cpus_are_resettable() declaration to accel/accel-system.c.
> 
> In order to have AccelClass methods instrospect their state,
> we need to pass AccelState by argument.
> 
> Adapt KVM handler.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/qemu/accel.h       |  1 +
>  include/system/accel-ops.h |  1 -
>  accel/accel-system.c       | 10 ++++++++++
>  accel/kvm/kvm-accel-ops.c  |  6 ------
>  accel/kvm/kvm-all.c        |  6 ++++++
>  system/cpus.c              |  8 --------
>  6 files changed, 17 insertions(+), 15 deletions(-)

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Re: [PATCH v6 13/39] accel: Move cpus_are_resettable() declaration to AccelClass
Posted by Xiaoyao Li 5 months, 2 weeks ago 
On 7/4/2025 1:32 AM, Philippe Mathieu-Daudé wrote:
> AccelOpsClass is for methods dealing with vCPUs.
> When only dealing with AccelState, AccelClass is sufficient.
> 
> Move cpus_are_resettable() declaration to accel/accel-system.c.

I don't think this is necessary unless a solid justfication provided.

One straightfroward question against it, is why don't move 
gdb_supports_guest_debug() to accel/accel-system.c as well in the patch 12.

> In order to have AccelClass methods instrospect their state,
> we need to pass AccelState by argument.

Is this the essential preparation for split-accel work?

> Adapt KVM handler.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   include/qemu/accel.h       |  1 +
>   include/system/accel-ops.h |  1 -
>   accel/accel-system.c       | 10 ++++++++++
>   accel/kvm/kvm-accel-ops.c  |  6 ------
>   accel/kvm/kvm-all.c        |  6 ++++++
>   system/cpus.c              |  8 --------
>   6 files changed, 17 insertions(+), 15 deletions(-)
> 
> diff --git a/include/qemu/accel.h b/include/qemu/accel.h
> index fb176e89bad..f987d16baaa 100644
> --- a/include/qemu/accel.h
> +++ b/include/qemu/accel.h
> @@ -45,6 +45,7 @@ typedef struct AccelClass {
>       void (*setup_post)(MachineState *ms, AccelState *accel);
>       bool (*has_memory)(MachineState *ms, AddressSpace *as,
>                          hwaddr start_addr, hwaddr size);
> +    bool (*cpus_are_resettable)(AccelState *as);
>   
>       /* gdbstub related hooks */
>       bool (*supports_guest_debug)(AccelState *as);
> diff --git a/include/system/accel-ops.h b/include/system/accel-ops.h
> index 700df92ac6d..f19245d0a0e 100644
> --- a/include/system/accel-ops.h
> +++ b/include/system/accel-ops.h
> @@ -33,7 +33,6 @@ struct AccelOpsClass {
>       /* initialization function called when accel is chosen */
>       void (*ops_init)(AccelOpsClass *ops);
>   
> -    bool (*cpus_are_resettable)(void);
>       void (*cpu_reset_hold)(CPUState *cpu);
>   
>       void (*create_vcpu_thread)(CPUState *cpu); /* MANDATORY NON-NULL */
> diff --git a/accel/accel-system.c b/accel/accel-system.c
> index a0f562ae9ff..07b75dae797 100644
> --- a/accel/accel-system.c
> +++ b/accel/accel-system.c
> @@ -62,6 +62,16 @@ void accel_setup_post(MachineState *ms)
>       }
>   }
>   
> +bool cpus_are_resettable(void)
> +{
> +    AccelState *accel = current_accel();
> +    AccelClass *acc = ACCEL_GET_CLASS(accel);
> +    if (acc->cpus_are_resettable) {
> +        return acc->cpus_are_resettable(accel);
> +    }
> +    return true;
> +}
> +
>   /* initialize the arch-independent accel operation interfaces */
>   void accel_init_ops_interfaces(AccelClass *ac)
>   {
> diff --git a/accel/kvm/kvm-accel-ops.c b/accel/kvm/kvm-accel-ops.c
> index 96606090889..99f61044da5 100644
> --- a/accel/kvm/kvm-accel-ops.c
> +++ b/accel/kvm/kvm-accel-ops.c
> @@ -78,11 +78,6 @@ static bool kvm_vcpu_thread_is_idle(CPUState *cpu)
>       return !kvm_halt_in_kernel();
>   }
>   
> -static bool kvm_cpus_are_resettable(void)
> -{
> -    return !kvm_enabled() || !kvm_state->guest_state_protected;
> -}
> -
>   #ifdef TARGET_KVM_HAVE_GUEST_DEBUG
>   static int kvm_update_guest_debug_ops(CPUState *cpu)
>   {
> @@ -96,7 +91,6 @@ static void kvm_accel_ops_class_init(ObjectClass *oc, const void *data)
>   
>       ops->create_vcpu_thread = kvm_start_vcpu_thread;
>       ops->cpu_thread_is_idle = kvm_vcpu_thread_is_idle;
> -    ops->cpus_are_resettable = kvm_cpus_are_resettable;
>       ops->synchronize_post_reset = kvm_cpu_synchronize_post_reset;
>       ops->synchronize_post_init = kvm_cpu_synchronize_post_init;
>       ops->synchronize_state = kvm_cpu_synchronize_state;
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index c8611552d19..88fb6d36941 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -3979,6 +3979,11 @@ static void kvm_accel_instance_init(Object *obj)
>       s->msr_energy.enable = false;
>   }
>   
> +static bool kvm_cpus_are_resettable(AccelState *as)
> +{
> +    return !kvm_enabled() || !kvm_state->guest_state_protected;
> +}
> +
>   /**
>    * kvm_gdbstub_sstep_flags():
>    *
> @@ -3997,6 +4002,7 @@ static void kvm_accel_class_init(ObjectClass *oc, const void *data)
>       ac->init_machine = kvm_init;
>       ac->has_memory = kvm_accel_has_memory;
>       ac->allowed = &kvm_allowed;
> +    ac->cpus_are_resettable = kvm_cpus_are_resettable;
>       ac->gdbstub_supported_sstep_flags = kvm_gdbstub_sstep_flags;
>   #ifdef TARGET_KVM_HAVE_GUEST_DEBUG
>       ac->supports_guest_debug = kvm_supports_guest_debug;
> diff --git a/system/cpus.c b/system/cpus.c
> index a43e0e4e796..4fb764ac880 100644
> --- a/system/cpus.c
> +++ b/system/cpus.c
> @@ -195,14 +195,6 @@ void cpu_synchronize_pre_loadvm(CPUState *cpu)
>       }
>   }
>   
> -bool cpus_are_resettable(void)
> -{
> -    if (cpus_accel->cpus_are_resettable) {
> -        return cpus_accel->cpus_are_resettable();
> -    }
> -    return true;
> -}
> -
>   void cpu_exec_reset_hold(CPUState *cpu)
>   {
>       if (cpus_accel->cpu_reset_hold) {
Re: [PATCH v6 13/39] accel: Move cpus_are_resettable() declaration to AccelClass
Posted by Philippe Mathieu-Daudé 2 months, 1 week ago 
On 4/7/25 07:36, Xiaoyao Li wrote:
> On 7/4/2025 1:32 AM, Philippe Mathieu-Daudé wrote:
>> AccelOpsClass is for methods dealing with vCPUs.
>> When only dealing with AccelState, AccelClass is sufficient.
>>
>> Move cpus_are_resettable() declaration to accel/accel-system.c.
> 
> I don't think this is necessary unless a solid justfication provided.
> 
> One straightfroward question against it, is why don't move 
> gdb_supports_guest_debug() to accel/accel-system.c as well in the patch 12.

gdb_supports_guest_debug() is used in both user / system emulation.

> 
>> In order to have AccelClass methods instrospect their state,
>> we need to pass AccelState by argument.
> 
> Is this the essential preparation for split-accel work?

Yes, but also the aim is to better organize and clarify the APIs.

We have a set of generic handlers that work on the AccelState context,
regardless of vCPUs; and another set which works on vCPUs within Accel
and must get the vCPU context by argument.

> 
>> Adapt KVM handler.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   include/qemu/accel.h       |  1 +
>>   include/system/accel-ops.h |  1 -
>>   accel/accel-system.c       | 10 ++++++++++
>>   accel/kvm/kvm-accel-ops.c  |  6 ------
>>   accel/kvm/kvm-all.c        |  6 ++++++
>>   system/cpus.c              |  8 --------
>>   6 files changed, 17 insertions(+), 15 deletions(-)

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.