diff mbox series

[RFC,v9,17/32] accel/tcg: split TCG-only code from cpu_exec_realizefn

Message ID 20201208194839.31305-18-cfontana@suse.de (mailing list archive)
State New, archived
Headers show
Series i386 cleanup | expand

Commit Message

Claudio Fontana Dec. 8, 2020, 7:48 p.m. UTC 
move away TCG-only code, make it compile only on TCG.

Signed-off-by: Claudio Fontana <cfontana@suse.de>
---
 accel/tcg/cpu-exec.c  | 28 +++++++++++++++++
 cpu.c                 | 70 ++++++++++++++++++++-----------------------
 hw/core/cpu.c         |  6 +++-
 include/hw/core/cpu.h |  8 +++++
 4 files changed, 74 insertions(+), 38 deletions(-)

Comments

Alex Bennée Dec. 9, 2020, 10:42 a.m. UTC | #1 
Claudio Fontana <cfontana@suse.de> writes:

> move away TCG-only code, make it compile only on TCG.
>
> Signed-off-by: Claudio Fontana <cfontana@suse.de>
> ---
>  accel/tcg/cpu-exec.c  | 28 +++++++++++++++++
>  cpu.c                 | 70 ++++++++++++++++++++-----------------------
>  hw/core/cpu.c         |  6 +++-
>  include/hw/core/cpu.h |  8 +++++
>  4 files changed, 74 insertions(+), 38 deletions(-)
>
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index 64cba89356..436dfbf155 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -801,6 +801,34 @@ int cpu_exec(CPUState *cpu)
>      return ret;
>  }
>  
> +void tcg_exec_realizefn(CPUState *cpu, Error **errp)
> +{
> +    static bool tcg_target_initialized;
> +    CPUClass *cc = CPU_GET_CLASS(cpu);
> +
> +    if (!tcg_target_initialized) {
> +        tcg_target_initialized = true;
> +        cc->tcg_ops.initialize();

nit: it makes no difference but stylistically it makes sense to set
tcg_target_initialized after we have in fact initialised.

Also we've dropped the tcg_enabled() check, if indeed it will always be
true should we not assert it to ensure the statement for tcg_exec_init
remains the case: "Must be called before using the QEMU cpus."

Otherwise LGTM:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Claudio Fontana Dec. 9, 2020, 11:22 a.m. UTC | #2 
On 12/9/20 11:42 AM, Alex Bennée wrote:
> 
> Claudio Fontana <cfontana@suse.de> writes:
> 
>> move away TCG-only code, make it compile only on TCG.
>>
>> Signed-off-by: Claudio Fontana <cfontana@suse.de>
>> ---
>>  accel/tcg/cpu-exec.c  | 28 +++++++++++++++++
>>  cpu.c                 | 70 ++++++++++++++++++++-----------------------
>>  hw/core/cpu.c         |  6 +++-
>>  include/hw/core/cpu.h |  8 +++++
>>  4 files changed, 74 insertions(+), 38 deletions(-)
>>
>> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
>> index 64cba89356..436dfbf155 100644
>> --- a/accel/tcg/cpu-exec.c
>> +++ b/accel/tcg/cpu-exec.c
>> @@ -801,6 +801,34 @@ int cpu_exec(CPUState *cpu)
>>      return ret;
>>  }
>>  
>> +void tcg_exec_realizefn(CPUState *cpu, Error **errp)
>> +{
>> +    static bool tcg_target_initialized;
>> +    CPUClass *cc = CPU_GET_CLASS(cpu);
>> +
>> +    if (!tcg_target_initialized) {
>> +        tcg_target_initialized = true;
>> +        cc->tcg_ops.initialize();
> 
> nit: it makes no difference but stylistically it makes sense to set
> tcg_target_initialized after we have in fact initialised.
> 
> Also we've dropped the tcg_enabled() check,


The tcg_enabled() check should be there, not here but in cpu_exec_realizefn,

if (tcg_enabled()) {
  tcg_exec_realizefn(...)
}

will check, thanks!


> if indeed it will always be
> true should we not assert it to ensure the statement for tcg_exec_init
> remains the case: "Must be called before using the QEMU cpus."
> 
> Otherwise LGTM:
> 
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>
diff mbox series

Patch

diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 64cba89356..436dfbf155 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -801,6 +801,34 @@  int cpu_exec(CPUState *cpu)
     return ret;
 }
 
+void tcg_exec_realizefn(CPUState *cpu, Error **errp)
+{
+    static bool tcg_target_initialized;
+    CPUClass *cc = CPU_GET_CLASS(cpu);
+
+    if (!tcg_target_initialized) {
+        tcg_target_initialized = true;
+        cc->tcg_ops.initialize();
+    }
+    tlb_init(cpu);
+    qemu_plugin_vcpu_init_hook(cpu);
+
+#ifndef CONFIG_USER_ONLY
+    tcg_iommu_init_notifier_list(cpu);
+#endif /* !CONFIG_USER_ONLY */
+}
+
+/* undo the initializations in reverse order */
+void tcg_exec_unrealizefn(CPUState *cpu)
+{
+#ifndef CONFIG_USER_ONLY
+    tcg_iommu_free_notifier_list(cpu);
+#endif /* !CONFIG_USER_ONLY */
+
+    qemu_plugin_vcpu_exit_hook(cpu);
+    tlb_destroy(cpu);
+}
+
 #ifndef CONFIG_USER_ONLY
 
 void dump_drift_info(void)
diff --git a/cpu.c b/cpu.c
index 27ad096cc4..5cc8f181be 100644
--- a/cpu.c
+++ b/cpu.c
@@ -124,13 +124,35 @@  const VMStateDescription vmstate_cpu_common = {
 };
 #endif
 
+void cpu_exec_realizefn(CPUState *cpu, Error **errp)
+{
+    CPUClass *cc = CPU_GET_CLASS(cpu);
+
+    cpu_list_add(cpu);
+
+#ifdef CONFIG_TCG
+    /* NB: errp parameter is unused currently */
+    if (tcg_enabled()) {
+        tcg_exec_realizefn(cpu, errp);
+    }
+#endif /* CONFIG_TCG */
+
+#ifdef CONFIG_USER_ONLY
+    assert(cc->vmsd == NULL);
+#else
+    if (qdev_get_vmsd(DEVICE(cpu)) == NULL) {
+        vmstate_register(NULL, cpu->cpu_index, &vmstate_cpu_common, cpu);
+    }
+    if (cc->vmsd != NULL) {
+        vmstate_register(NULL, cpu->cpu_index, cc->vmsd, cpu);
+    }
+#endif /* CONFIG_USER_ONLY */
+}
+
 void cpu_exec_unrealizefn(CPUState *cpu)
 {
     CPUClass *cc = CPU_GET_CLASS(cpu);
 
-    tlb_destroy(cpu);
-    cpu_list_remove(cpu);
-
 #ifdef CONFIG_USER_ONLY
     assert(cc->vmsd == NULL);
 #else
@@ -140,8 +162,15 @@  void cpu_exec_unrealizefn(CPUState *cpu)
     if (qdev_get_vmsd(DEVICE(cpu)) == NULL) {
         vmstate_unregister(NULL, &vmstate_cpu_common, cpu);
     }
-    tcg_iommu_free_notifier_list(cpu);
 #endif
+#ifdef CONFIG_TCG
+    /* NB: errp parameter is unused currently */
+    if (tcg_enabled()) {
+        tcg_exec_unrealizefn(cpu);
+    }
+#endif /* CONFIG_TCG */
+
+    cpu_list_remove(cpu);
 }
 
 Property cpu_common_props[] = {
@@ -171,39 +200,6 @@  void cpu_exec_initfn(CPUState *cpu)
 #endif
 }
 
-void cpu_exec_realizefn(CPUState *cpu, Error **errp)
-{
-    CPUClass *cc = CPU_GET_CLASS(cpu);
-#ifdef CONFIG_TCG
-    static bool tcg_target_initialized;
-#endif /* CONFIG_TCG */
-
-    cpu_list_add(cpu);
-
-#ifdef CONFIG_TCG
-    if (tcg_enabled() && !tcg_target_initialized) {
-        tcg_target_initialized = true;
-        cc->tcg_ops.initialize();
-    }
-#endif /* CONFIG_TCG */
-    tlb_init(cpu);
-
-    qemu_plugin_vcpu_init_hook(cpu);
-
-#ifdef CONFIG_USER_ONLY
-    assert(cc->vmsd == NULL);
-#else /* !CONFIG_USER_ONLY */
-    if (qdev_get_vmsd(DEVICE(cpu)) == NULL) {
-        vmstate_register(NULL, cpu->cpu_index, &vmstate_cpu_common, cpu);
-    }
-    if (cc->vmsd != NULL) {
-        vmstate_register(NULL, cpu->cpu_index, cc->vmsd, cpu);
-    }
-
-    tcg_iommu_init_notifier_list(cpu);
-#endif
-}
-
 const char *parse_cpu_option(const char *cpu_option)
 {
     ObjectClass *oc;
diff --git a/hw/core/cpu.c b/hw/core/cpu.c
index 994a12cb35..1f04aab16b 100644
--- a/hw/core/cpu.c
+++ b/hw/core/cpu.c
@@ -199,6 +199,10 @@  static bool cpu_common_virtio_is_big_endian(CPUState *cpu)
     return target_words_bigendian();
 }
 
+/*
+ * XXX the following #if is always true because this is a common_ss
+ * module, so target CONFIG_* is never defined.
+ */
 #if !defined(CONFIG_USER_ONLY)
 GuestPanicInformation *cpu_get_crash_info(CPUState *cpu)
 {
@@ -340,9 +344,9 @@  static void cpu_common_realizefn(DeviceState *dev, Error **errp)
 static void cpu_common_unrealizefn(DeviceState *dev)
 {
     CPUState *cpu = CPU(dev);
+
     /* NOTE: latest generic point before the cpu is fully unrealized */
     trace_fini_vcpu(cpu);
-    qemu_plugin_vcpu_exit_hook(cpu);
     cpu_exec_unrealizefn(cpu);
 }
 
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index c93b08a0fb..ea648d52ad 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -1119,10 +1119,18 @@  AddressSpace *cpu_get_address_space(CPUState *cpu, int asidx);
 void QEMU_NORETURN cpu_abort(CPUState *cpu, const char *fmt, ...)
     GCC_FMT_ATTR(2, 3);
 extern Property cpu_common_props[];
+
+/* $(top_srcdir)/cpu.c */
 void cpu_exec_initfn(CPUState *cpu);
 void cpu_exec_realizefn(CPUState *cpu, Error **errp);
 void cpu_exec_unrealizefn(CPUState *cpu);
 
+#ifdef CONFIG_TCG
+/* accel/tcg/cpu-exec.c */
+void tcg_exec_realizefn(CPUState *cpu, Error **errp);
+void tcg_exec_unrealizefn(CPUState *cpu);
+#endif /* CONFIG_TCG */
+
 /**
  * target_words_bigendian:
  * Returns true if the (default) endianness of the target is big endian,