[PATCH 0/5] accel: Restrict tcg_exec_[un]realizefn() to TCG

[PATCH 0/5] accel: Restrict tcg_exec_[un]realizefn() to TCG

[Philippe Mathieu-Daudé]

- Add missing accel_cpu_unrealize()
- Add AccelClass::[un]realize_cpu handlers
- Use tcg_exec_[un]realizefn as AccelClass handlers

Philippe Mathieu-Daudé (5):
  accel: Rename accel_cpu_realizefn() ->  accel_cpu_realize()
  accel: Introduce accel_cpu_unrealize() stub
  accel: Declare AccelClass::[un]realize_cpu() handlers
  accel/tcg: Have tcg_exec_realizefn() return a boolean
  accel/tcg: Restrict tcg_exec_[un]realizefn() to TCG

 accel/tcg/internal.h      |  3 +++
 include/exec/cpu-all.h    |  2 --
 include/qemu/accel.h      | 12 ++++++++++--
 accel/accel-common.c      | 27 ++++++++++++++++++++++++---
 accel/tcg/cpu-exec.c      |  4 +++-
 accel/tcg/tcg-all.c       |  2 ++
 cpu.c                     | 13 +++----------
 target/i386/kvm/kvm-cpu.c |  2 +-
 8 files changed, 46 insertions(+), 19 deletions(-)

-- 
2.41.0

[PATCH 1/5] accel: Rename accel_cpu_realizefn() -> accel_cpu_realize()

[Philippe Mathieu-Daudé]

We use the '*fn' suffix for handlers, this is a public method.
Drop the suffix.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/qemu/accel.h      | 4 ++--
 accel/accel-common.c      | 2 +-
 cpu.c                     | 2 +-
 target/i386/kvm/kvm-cpu.c | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/qemu/accel.h b/include/qemu/accel.h
index e84db2e3e5..cb64a07b84 100644
--- a/include/qemu/accel.h
+++ b/include/qemu/accel.h
@@ -90,11 +90,11 @@ void accel_setup_post(MachineState *ms);
 void accel_cpu_instance_init(CPUState *cpu);
 
 /**
- * accel_cpu_realizefn:
+ * accel_cpu_realize:
  * @cpu: The CPU that needs to call accel-specific cpu realization.
  * @errp: currently unused.
  */
-bool accel_cpu_realizefn(CPUState *cpu, Error **errp);
+bool accel_cpu_realize(CPUState *cpu, Error **errp);
 
 /**
  * accel_supported_gdbstub_sstep_flags:
diff --git a/accel/accel-common.c b/accel/accel-common.c
index df72cc989a..b953855e8b 100644
--- a/accel/accel-common.c
+++ b/accel/accel-common.c
@@ -119,7 +119,7 @@ void accel_cpu_instance_init(CPUState *cpu)
     }
 }
 
-bool accel_cpu_realizefn(CPUState *cpu, Error **errp)
+bool accel_cpu_realize(CPUState *cpu, Error **errp)
 {
     CPUClass *cc = CPU_GET_CLASS(cpu);
 
diff --git a/cpu.c b/cpu.c
index 0769b0b153..61c9760e62 100644
--- a/cpu.c
+++ b/cpu.c
@@ -136,7 +136,7 @@ void cpu_exec_realizefn(CPUState *cpu, Error **errp)
     /* cache the cpu class for the hotpath */
     cpu->cc = CPU_GET_CLASS(cpu);
 
-    if (!accel_cpu_realizefn(cpu, errp)) {
+    if (!accel_cpu_realize(cpu, errp)) {
         return;
     }
 
diff --git a/target/i386/kvm/kvm-cpu.c b/target/i386/kvm/kvm-cpu.c
index 7237378a7d..4474689f81 100644
--- a/target/i386/kvm/kvm-cpu.c
+++ b/target/i386/kvm/kvm-cpu.c
@@ -35,7 +35,7 @@ static bool kvm_cpu_realizefn(CPUState *cs, Error **errp)
      * x86_cpu_realize():
      *  -> x86_cpu_expand_features()
      *  -> cpu_exec_realizefn():
-     *            -> accel_cpu_realizefn()
+     *            -> accel_cpu_realize()
      *               kvm_cpu_realizefn() -> host_cpu_realizefn()
      *  -> check/update ucode_rev, phys_bits, mwait
      */
-- 
2.41.0

Re: [PATCH 1/5] accel: Rename accel_cpu_realizefn() -> accel_cpu_realize()

[Claudio Fontana]

Reviewed-by: Claudio Fontana <cfontana@suse.de>

On 9/15/23 21:00, Philippe Mathieu-Daudé wrote:
> We use the '*fn' suffix for handlers, this is a public method.
> Drop the suffix.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  include/qemu/accel.h      | 4 ++--
>  accel/accel-common.c      | 2 +-
>  cpu.c                     | 2 +-
>  target/i386/kvm/kvm-cpu.c | 2 +-
>  4 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/include/qemu/accel.h b/include/qemu/accel.h
> index e84db2e3e5..cb64a07b84 100644
> --- a/include/qemu/accel.h
> +++ b/include/qemu/accel.h
> @@ -90,11 +90,11 @@ void accel_setup_post(MachineState *ms);
>  void accel_cpu_instance_init(CPUState *cpu);
>  
>  /**
> - * accel_cpu_realizefn:
> + * accel_cpu_realize:
>   * @cpu: The CPU that needs to call accel-specific cpu realization.
>   * @errp: currently unused.
>   */
> -bool accel_cpu_realizefn(CPUState *cpu, Error **errp);
> +bool accel_cpu_realize(CPUState *cpu, Error **errp);
>  
>  /**
>   * accel_supported_gdbstub_sstep_flags:
> diff --git a/accel/accel-common.c b/accel/accel-common.c
> index df72cc989a..b953855e8b 100644
> --- a/accel/accel-common.c
> +++ b/accel/accel-common.c
> @@ -119,7 +119,7 @@ void accel_cpu_instance_init(CPUState *cpu)
>      }
>  }
>  
> -bool accel_cpu_realizefn(CPUState *cpu, Error **errp)
> +bool accel_cpu_realize(CPUState *cpu, Error **errp)
>  {
>      CPUClass *cc = CPU_GET_CLASS(cpu);
>  
> diff --git a/cpu.c b/cpu.c
> index 0769b0b153..61c9760e62 100644
> --- a/cpu.c
> +++ b/cpu.c
> @@ -136,7 +136,7 @@ void cpu_exec_realizefn(CPUState *cpu, Error **errp)
>      /* cache the cpu class for the hotpath */
>      cpu->cc = CPU_GET_CLASS(cpu);
>  
> -    if (!accel_cpu_realizefn(cpu, errp)) {
> +    if (!accel_cpu_realize(cpu, errp)) {
>          return;
>      }
>  
> diff --git a/target/i386/kvm/kvm-cpu.c b/target/i386/kvm/kvm-cpu.c
> index 7237378a7d..4474689f81 100644
> --- a/target/i386/kvm/kvm-cpu.c
> +++ b/target/i386/kvm/kvm-cpu.c
> @@ -35,7 +35,7 @@ static bool kvm_cpu_realizefn(CPUState *cs, Error **errp)
>       * x86_cpu_realize():
>       *  -> x86_cpu_expand_features()
>       *  -> cpu_exec_realizefn():
> -     *            -> accel_cpu_realizefn()
> +     *            -> accel_cpu_realize()
>       *               kvm_cpu_realizefn() -> host_cpu_realizefn()
>       *  -> check/update ucode_rev, phys_bits, mwait
>       */

[PATCH 2/5] accel: Introduce accel_cpu_unrealize() stub

[Philippe Mathieu-Daudé]

Prepare the stub for parity with accel_cpu_realize().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/qemu/accel.h | 6 ++++++
 accel/accel-common.c | 4 ++++
 cpu.c                | 3 ++-
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/include/qemu/accel.h b/include/qemu/accel.h
index cb64a07b84..23254c6c9c 100644
--- a/include/qemu/accel.h
+++ b/include/qemu/accel.h
@@ -96,6 +96,12 @@ void accel_cpu_instance_init(CPUState *cpu);
  */
 bool accel_cpu_realize(CPUState *cpu, Error **errp);
 
+/**
+ * accel_cpu_unrealizefn:
+ * @cpu: The CPU that needs to call accel-specific cpu unrealization.
+ */
+void accel_cpu_unrealize(CPUState *cpu);
+
 /**
  * accel_supported_gdbstub_sstep_flags:
  *
diff --git a/accel/accel-common.c b/accel/accel-common.c
index b953855e8b..cc3a45e663 100644
--- a/accel/accel-common.c
+++ b/accel/accel-common.c
@@ -129,6 +129,10 @@ bool accel_cpu_realize(CPUState *cpu, Error **errp)
     return true;
 }
 
+void accel_cpu_unrealize(CPUState *cpu)
+{
+}
+
 int accel_supported_gdbstub_sstep_flags(void)
 {
     AccelState *accel = current_accel();
diff --git a/cpu.c b/cpu.c
index 61c9760e62..b928bbed50 100644
--- a/cpu.c
+++ b/cpu.c
@@ -187,8 +187,9 @@ void cpu_exec_unrealizefn(CPUState *cpu)
     cpu_list_remove(cpu);
     /*
      * Now that the vCPU has been removed from the RCU list, we can call
-     * tcg_exec_unrealizefn, which may free fields using call_rcu.
+     * accel_cpu_unrealize, which may free fields using call_rcu.
      */
+    accel_cpu_unrealize(cpu);
     if (tcg_enabled()) {
         tcg_exec_unrealizefn(cpu);
     }
-- 
2.41.0

Re: [PATCH 2/5] accel: Introduce accel_cpu_unrealize() stub

[Philippe Mathieu-Daudé]

On 15/9/23 21:00, Philippe Mathieu-Daudé wrote:
> Prepare the stub for parity with accel_cpu_realize().
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/qemu/accel.h | 6 ++++++
>   accel/accel-common.c | 4 ++++
>   cpu.c                | 3 ++-
>   3 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/include/qemu/accel.h b/include/qemu/accel.h
> index cb64a07b84..23254c6c9c 100644
> --- a/include/qemu/accel.h
> +++ b/include/qemu/accel.h
> @@ -96,6 +96,12 @@ void accel_cpu_instance_init(CPUState *cpu);
>    */
>   bool accel_cpu_realize(CPUState *cpu, Error **errp);
>   
> +/**
> + * accel_cpu_unrealizefn:

"accel_cpu_unrealize"

> + * @cpu: The CPU that needs to call accel-specific cpu unrealization.
> + */
> +void accel_cpu_unrealize(CPUState *cpu);

[PATCH 3/5] accel: Declare AccelClass::[un]realize_cpu() handlers

[Philippe Mathieu-Daudé]

Currently accel_cpu_realize() only performs target-specific
realization. Introduce the [un]realize_cpu fields in the
base AccelClass to be able to perform target-agnostic
[un]realization of vCPUs.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/qemu/accel.h |  2 ++
 accel/accel-common.c | 21 +++++++++++++++++++--
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/include/qemu/accel.h b/include/qemu/accel.h
index 23254c6c9c..7bd9907d2a 100644
--- a/include/qemu/accel.h
+++ b/include/qemu/accel.h
@@ -43,6 +43,8 @@ typedef struct AccelClass {
     bool (*has_memory)(MachineState *ms, AddressSpace *as,
                        hwaddr start_addr, hwaddr size);
 #endif
+    bool (*realize_cpu)(CPUState *cpu, Error **errp);
+    void (*unrealize_cpu)(CPUState *cpu);
 
     /* gdbstub related hooks */
     int (*gdbstub_supported_sstep_flags)(void);
diff --git a/accel/accel-common.c b/accel/accel-common.c
index cc3a45e663..6d427f2b9d 100644
--- a/accel/accel-common.c
+++ b/accel/accel-common.c
@@ -122,15 +122,32 @@ void accel_cpu_instance_init(CPUState *cpu)
 bool accel_cpu_realize(CPUState *cpu, Error **errp)
 {
     CPUClass *cc = CPU_GET_CLASS(cpu);
+    AccelState *accel = current_accel();
+    AccelClass *acc = ACCEL_GET_CLASS(accel);
 
-    if (cc->accel_cpu && cc->accel_cpu->cpu_realizefn) {
-        return cc->accel_cpu->cpu_realizefn(cpu, errp);
+    /* target specific realization */
+    if (cc->accel_cpu && cc->accel_cpu->cpu_realizefn
+        && !cc->accel_cpu->cpu_realizefn(cpu, errp)) {
+        return false;
     }
+
+    /* generic realization */
+    if (acc->realize_cpu && !acc->realize_cpu(cpu, errp)) {
+        return false;
+    }
+
     return true;
 }
 
 void accel_cpu_unrealize(CPUState *cpu)
 {
+    AccelState *accel = current_accel();
+    AccelClass *acc = ACCEL_GET_CLASS(accel);
+
+    /* generic unrealization */
+    if (acc->unrealize_cpu) {
+        acc->unrealize_cpu(cpu);
+    }
 }
 
 int accel_supported_gdbstub_sstep_flags(void)
-- 
2.41.0

Re: [PATCH 3/5] accel: Declare AccelClass::[un]realize_cpu() handlers

[Claudio Fontana]

On 9/15/23 21:00, Philippe Mathieu-Daudé wrote:
> Currently accel_cpu_realize() only performs target-specific
> realization. Introduce the [un]realize_cpu fields in the
> base AccelClass to be able to perform target-agnostic
> [un]realization of vCPUs.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Just thinking, for the benefit of the reader trying to understand the code later on,
maybe putting in a "target_" in there somewhere in the function name?
like "realize_cpu_target", vs "realize_cpu_generic" ?

Ciao,

C

> ---
>  include/qemu/accel.h |  2 ++
>  accel/accel-common.c | 21 +++++++++++++++++++--
>  2 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/include/qemu/accel.h b/include/qemu/accel.h
> index 23254c6c9c..7bd9907d2a 100644
> --- a/include/qemu/accel.h
> +++ b/include/qemu/accel.h
> @@ -43,6 +43,8 @@ typedef struct AccelClass {
>      bool (*has_memory)(MachineState *ms, AddressSpace *as,
>                         hwaddr start_addr, hwaddr size);
>  #endif
> +    bool (*realize_cpu)(CPUState *cpu, Error **errp);
> +    void (*unrealize_cpu)(CPUState *cpu);
>  
>      /* gdbstub related hooks */
>      int (*gdbstub_supported_sstep_flags)(void);
> diff --git a/accel/accel-common.c b/accel/accel-common.c
> index cc3a45e663..6d427f2b9d 100644
> --- a/accel/accel-common.c
> +++ b/accel/accel-common.c
> @@ -122,15 +122,32 @@ void accel_cpu_instance_init(CPUState *cpu)
>  bool accel_cpu_realize(CPUState *cpu, Error **errp)
>  {
>      CPUClass *cc = CPU_GET_CLASS(cpu);
> +    AccelState *accel = current_accel();
> +    AccelClass *acc = ACCEL_GET_CLASS(accel);
>  
> -    if (cc->accel_cpu && cc->accel_cpu->cpu_realizefn) {
> -        return cc->accel_cpu->cpu_realizefn(cpu, errp);
> +    /* target specific realization */
> +    if (cc->accel_cpu && cc->accel_cpu->cpu_realizefn
> +        && !cc->accel_cpu->cpu_realizefn(cpu, errp)) {
> +        return false;
>      }
> +
> +    /* generic realization */
> +    if (acc->realize_cpu && !acc->realize_cpu(cpu, errp)) {
> +        return false;
> +    }
> +
>      return true;
>  }
>  
>  void accel_cpu_unrealize(CPUState *cpu)
>  {
> +    AccelState *accel = current_accel();
> +    AccelClass *acc = ACCEL_GET_CLASS(accel);
> +
> +    /* generic unrealization */
> +    if (acc->unrealize_cpu) {
> +        acc->unrealize_cpu(cpu);
> +    }
>  }
>  
>  int accel_supported_gdbstub_sstep_flags(void)

Re: [PATCH 3/5] accel: Declare AccelClass::[un]realize_cpu() handlers

[Philippe Mathieu-Daudé]

On 3/10/23 10:55, Claudio Fontana wrote:
> On 9/15/23 21:00, Philippe Mathieu-Daudé wrote:
>> Currently accel_cpu_realize() only performs target-specific
>> realization. Introduce the [un]realize_cpu fields in the
>> base AccelClass to be able to perform target-agnostic
>> [un]realization of vCPUs.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
> Just thinking, for the benefit of the reader trying to understand the code later on,
> maybe putting in a "target_" in there somewhere in the function name?
> like "realize_cpu_target", vs "realize_cpu_generic" ?

Good idea, I like it, thanks!

[PATCH 4/5] accel/tcg: Have tcg_exec_realizefn() return a boolean

[Philippe Mathieu-Daudé]

Following the example documented since commit e3fe3988d7 ("error:
Document Error API usage rules"), have tcg_exec_realizefn() return
a boolean indicating whether an error is set or not.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/exec/cpu-all.h | 2 +-
 accel/tcg/cpu-exec.c   | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index c2c62160c6..1e5c530ee1 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -422,7 +422,7 @@ void dump_exec_info(GString *buf);
 
 /* accel/tcg/cpu-exec.c */
 int cpu_exec(CPUState *cpu);
-void tcg_exec_realizefn(CPUState *cpu, Error **errp);
+bool tcg_exec_realizefn(CPUState *cpu, Error **errp);
 void tcg_exec_unrealizefn(CPUState *cpu);
 
 /**
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index e2c494e75e..fa97e9f191 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -1088,7 +1088,7 @@ int cpu_exec(CPUState *cpu)
     return ret;
 }
 
-void tcg_exec_realizefn(CPUState *cpu, Error **errp)
+bool tcg_exec_realizefn(CPUState *cpu, Error **errp)
 {
     static bool tcg_target_initialized;
     CPUClass *cc = CPU_GET_CLASS(cpu);
@@ -1104,6 +1104,8 @@ void tcg_exec_realizefn(CPUState *cpu, Error **errp)
     tcg_iommu_init_notifier_list(cpu);
 #endif /* !CONFIG_USER_ONLY */
     /* qemu_plugin_vcpu_init_hook delayed until cpu_index assigned. */
+
+    return true;
 }
 
 /* undo the initializations in reverse order */
-- 
2.41.0

Re: [PATCH 4/5] accel/tcg: Have tcg_exec_realizefn() return a boolean

[Claudio Fontana]

On 9/15/23 21:00, Philippe Mathieu-Daudé wrote:
> Following the example documented since commit e3fe3988d7 ("error:
> Document Error API usage rules"), have tcg_exec_realizefn() return
> a boolean indicating whether an error is set or not.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Claudio Fontana <cfontana@suse.de>

> ---
>  include/exec/cpu-all.h | 2 +-
>  accel/tcg/cpu-exec.c   | 4 +++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> index c2c62160c6..1e5c530ee1 100644
> --- a/include/exec/cpu-all.h
> +++ b/include/exec/cpu-all.h
> @@ -422,7 +422,7 @@ void dump_exec_info(GString *buf);
>  
>  /* accel/tcg/cpu-exec.c */
>  int cpu_exec(CPUState *cpu);
> -void tcg_exec_realizefn(CPUState *cpu, Error **errp);
> +bool tcg_exec_realizefn(CPUState *cpu, Error **errp);
>  void tcg_exec_unrealizefn(CPUState *cpu);
>  
>  /**
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index e2c494e75e..fa97e9f191 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -1088,7 +1088,7 @@ int cpu_exec(CPUState *cpu)
>      return ret;
>  }
>  
> -void tcg_exec_realizefn(CPUState *cpu, Error **errp)
> +bool tcg_exec_realizefn(CPUState *cpu, Error **errp)
>  {
>      static bool tcg_target_initialized;
>      CPUClass *cc = CPU_GET_CLASS(cpu);
> @@ -1104,6 +1104,8 @@ void tcg_exec_realizefn(CPUState *cpu, Error **errp)
>      tcg_iommu_init_notifier_list(cpu);
>  #endif /* !CONFIG_USER_ONLY */
>      /* qemu_plugin_vcpu_init_hook delayed until cpu_index assigned. */
> +
> +    return true;
>  }
>  
>  /* undo the initializations in reverse order */

[PATCH 5/5] accel/tcg: Restrict tcg_exec_[un]realizefn() to TCG

[Philippe Mathieu-Daudé]

We don't need to expose these TCG-specific methods to the
whole code base. Register them as AccelClass handlers, they
will be called by the generic accel_cpu_[un]realize() methods.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 accel/tcg/internal.h   | 3 +++
 include/exec/cpu-all.h | 2 --
 accel/tcg/tcg-all.c    | 2 ++
 cpu.c                  | 8 --------
 4 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/accel/tcg/internal.h b/accel/tcg/internal.h
index e8cbbde581..57ab397df1 100644
--- a/accel/tcg/internal.h
+++ b/accel/tcg/internal.h
@@ -80,6 +80,9 @@ bool tb_invalidate_phys_page_unwind(tb_page_addr_t addr, uintptr_t pc);
 void cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb,
                                uintptr_t host_pc);
 
+bool tcg_exec_realizefn(CPUState *cpu, Error **errp);
+void tcg_exec_unrealizefn(CPUState *cpu);
+
 /* Return the current PC from CPU, which may be cached in TB. */
 static inline vaddr log_pc(CPUState *cpu, const TranslationBlock *tb)
 {
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 1e5c530ee1..230525ebf7 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -422,8 +422,6 @@ void dump_exec_info(GString *buf);
 
 /* accel/tcg/cpu-exec.c */
 int cpu_exec(CPUState *cpu);
-bool tcg_exec_realizefn(CPUState *cpu, Error **errp);
-void tcg_exec_unrealizefn(CPUState *cpu);
 
 /**
  * cpu_set_cpustate_pointers(cpu)
diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
index 03dfd67e9e..6942a9766a 100644
--- a/accel/tcg/tcg-all.c
+++ b/accel/tcg/tcg-all.c
@@ -227,6 +227,8 @@ static void tcg_accel_class_init(ObjectClass *oc, void *data)
     AccelClass *ac = ACCEL_CLASS(oc);
     ac->name = "tcg";
     ac->init_machine = tcg_init_machine;
+    ac->realize_cpu = tcg_exec_realizefn;
+    ac->unrealize_cpu = tcg_exec_unrealizefn;
     ac->allowed = &tcg_allowed;
     ac->gdbstub_supported_sstep_flags = tcg_gdbstub_supported_sstep_flags;
 
diff --git a/cpu.c b/cpu.c
index b928bbed50..1a8e730bed 100644
--- a/cpu.c
+++ b/cpu.c
@@ -140,11 +140,6 @@ void cpu_exec_realizefn(CPUState *cpu, Error **errp)
         return;
     }
 
-    /* NB: errp parameter is unused currently */
-    if (tcg_enabled()) {
-        tcg_exec_realizefn(cpu, errp);
-    }
-
     /* Wait until cpu initialization complete before exposing cpu. */
     cpu_list_add(cpu);
 
@@ -190,9 +185,6 @@ void cpu_exec_unrealizefn(CPUState *cpu)
      * accel_cpu_unrealize, which may free fields using call_rcu.
      */
     accel_cpu_unrealize(cpu);
-    if (tcg_enabled()) {
-        tcg_exec_unrealizefn(cpu);
-    }
 }
 
 /*
-- 
2.41.0

Re: [PATCH 5/5] accel/tcg: Restrict tcg_exec_[un]realizefn() to TCG

[Claudio Fontana]

On 9/15/23 21:00, Philippe Mathieu-Daudé wrote:
> We don't need to expose these TCG-specific methods to the
> whole code base. Register them as AccelClass handlers, they
> will be called by the generic accel_cpu_[un]realize() methods.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Claudio Fontana <cfontana@suse.de>

> ---
>  accel/tcg/internal.h   | 3 +++
>  include/exec/cpu-all.h | 2 --
>  accel/tcg/tcg-all.c    | 2 ++
>  cpu.c                  | 8 --------
>  4 files changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/accel/tcg/internal.h b/accel/tcg/internal.h
> index e8cbbde581..57ab397df1 100644
> --- a/accel/tcg/internal.h
> +++ b/accel/tcg/internal.h
> @@ -80,6 +80,9 @@ bool tb_invalidate_phys_page_unwind(tb_page_addr_t addr, uintptr_t pc);
>  void cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb,
>                                 uintptr_t host_pc);
>  
> +bool tcg_exec_realizefn(CPUState *cpu, Error **errp);
> +void tcg_exec_unrealizefn(CPUState *cpu);
> +
>  /* Return the current PC from CPU, which may be cached in TB. */
>  static inline vaddr log_pc(CPUState *cpu, const TranslationBlock *tb)
>  {
> diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> index 1e5c530ee1..230525ebf7 100644
> --- a/include/exec/cpu-all.h
> +++ b/include/exec/cpu-all.h
> @@ -422,8 +422,6 @@ void dump_exec_info(GString *buf);
>  
>  /* accel/tcg/cpu-exec.c */
>  int cpu_exec(CPUState *cpu);
> -bool tcg_exec_realizefn(CPUState *cpu, Error **errp);
> -void tcg_exec_unrealizefn(CPUState *cpu);
>  
>  /**
>   * cpu_set_cpustate_pointers(cpu)
> diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
> index 03dfd67e9e..6942a9766a 100644
> --- a/accel/tcg/tcg-all.c
> +++ b/accel/tcg/tcg-all.c
> @@ -227,6 +227,8 @@ static void tcg_accel_class_init(ObjectClass *oc, void *data)
>      AccelClass *ac = ACCEL_CLASS(oc);
>      ac->name = "tcg";
>      ac->init_machine = tcg_init_machine;
> +    ac->realize_cpu = tcg_exec_realizefn;
> +    ac->unrealize_cpu = tcg_exec_unrealizefn;
>      ac->allowed = &tcg_allowed;
>      ac->gdbstub_supported_sstep_flags = tcg_gdbstub_supported_sstep_flags;
>  
> diff --git a/cpu.c b/cpu.c
> index b928bbed50..1a8e730bed 100644
> --- a/cpu.c
> +++ b/cpu.c
> @@ -140,11 +140,6 @@ void cpu_exec_realizefn(CPUState *cpu, Error **errp)
>          return;
>      }
>  
> -    /* NB: errp parameter is unused currently */
> -    if (tcg_enabled()) {
> -        tcg_exec_realizefn(cpu, errp);
> -    }
> -
>      /* Wait until cpu initialization complete before exposing cpu. */
>      cpu_list_add(cpu);
>  
> @@ -190,9 +185,6 @@ void cpu_exec_unrealizefn(CPUState *cpu)
>       * accel_cpu_unrealize, which may free fields using call_rcu.
>       */
>      accel_cpu_unrealize(cpu);
> -    if (tcg_enabled()) {
> -        tcg_exec_unrealizefn(cpu);
> -    }
>  }
>  
>  /*

Re: [PATCH 0/5] accel: Restrict tcg_exec_[un]realizefn() to TCG

[Philippe Mathieu-Daudé]

On 15/9/23 21:00, Philippe Mathieu-Daudé wrote:
> - Add missing accel_cpu_unrealize()
> - Add AccelClass::[un]realize_cpu handlers
> - Use tcg_exec_[un]realizefn as AccelClass handlers
> 
> Philippe Mathieu-Daudé (5):
>    accel: Rename accel_cpu_realizefn() ->  accel_cpu_realize()
>    accel: Introduce accel_cpu_unrealize() stub
>    accel: Declare AccelClass::[un]realize_cpu() handlers
>    accel/tcg: Have tcg_exec_realizefn() return a boolean
>    accel/tcg: Restrict tcg_exec_[un]realizefn() to TCG

Ping?

Re: [PATCH 0/5] accel: Restrict tcg_exec_[un]realizefn() to TCG

[Richard Henderson]

On 10/2/23 23:44, Philippe Mathieu-Daudé wrote:
> On 15/9/23 21:00, Philippe Mathieu-Daudé wrote:
>> - Add missing accel_cpu_unrealize()
>> - Add AccelClass::[un]realize_cpu handlers
>> - Use tcg_exec_[un]realizefn as AccelClass handlers
>>
>> Philippe Mathieu-Daudé (5):
>>    accel: Rename accel_cpu_realizefn() ->  accel_cpu_realize()
>>    accel: Introduce accel_cpu_unrealize() stub
>>    accel: Declare AccelClass::[un]realize_cpu() handlers
>>    accel/tcg: Have tcg_exec_realizefn() return a boolean
>>    accel/tcg: Restrict tcg_exec_[un]realizefn() to TCG
> 
> Ping?
> 

I have this series queued for the next tcg pull.

r~

Re: [PATCH 0/5] accel: Restrict tcg_exec_[un]realizefn() to TCG

[Claudio Fontana]

On 10/3/23 16:04, Richard Henderson wrote:
> On 10/2/23 23:44, Philippe Mathieu-Daudé wrote:
>> On 15/9/23 21:00, Philippe Mathieu-Daudé wrote:
>>> - Add missing accel_cpu_unrealize()
>>> - Add AccelClass::[un]realize_cpu handlers
>>> - Use tcg_exec_[un]realizefn as AccelClass handlers
>>>
>>> Philippe Mathieu-Daudé (5):
>>>    accel: Rename accel_cpu_realizefn() ->  accel_cpu_realize()
>>>    accel: Introduce accel_cpu_unrealize() stub
>>>    accel: Declare AccelClass::[un]realize_cpu() handlers
>>>    accel/tcg: Have tcg_exec_realizefn() return a boolean
>>>    accel/tcg: Restrict tcg_exec_[un]realizefn() to TCG
>>
>> Ping?
>>
> 
> I have this series queued for the next tcg pull.
> 
> r~

I reviewed and tested the V2 if you want to queue that instead.

C

Re: [PATCH 0/5] accel: Restrict tcg_exec_[un]realizefn() to TCG

[Philippe Mathieu-Daudé]

On 3/10/23 16:04, Richard Henderson wrote:
> On 10/2/23 23:44, Philippe Mathieu-Daudé wrote:
>> On 15/9/23 21:00, Philippe Mathieu-Daudé wrote:
>>> - Add missing accel_cpu_unrealize()
>>> - Add AccelClass::[un]realize_cpu handlers
>>> - Use tcg_exec_[un]realizefn as AccelClass handlers
>>>
>>> Philippe Mathieu-Daudé (5):
>>>    accel: Rename accel_cpu_realizefn() ->  accel_cpu_realize()
>>>    accel: Introduce accel_cpu_unrealize() stub
>>>    accel: Declare AccelClass::[un]realize_cpu() handlers
>>>    accel/tcg: Have tcg_exec_realizefn() return a boolean
>>>    accel/tcg: Restrict tcg_exec_[un]realizefn() to TCG
>>
>> Ping?
>>
> 
> I have this series queued for the next tcg pull.

Oh I didn't noticed, thanks!

My preference would be v2, which Claudio already
reviewed and tested:
https://lore.kernel.org/qemu-devel/20231003123026.99229-1-philmd@linaro.org/