include/hw/core/tcg-cpu-ops.h | 7 +++++++ cpu.c | 23 +++++++++++------------ target/alpha/cpu.c | 1 + target/arm/cpu.c | 1 + target/arm/cpu_tcg.c | 1 + target/avr/cpu.c | 1 + target/cris/cpu.c | 1 + target/hexagon/cpu.c | 1 + target/hppa/cpu.c | 1 + target/i386/tcg/tcg-cpu.c | 1 + target/m68k/cpu.c | 1 + target/microblaze/cpu.c | 1 + target/mips/cpu.c | 1 + target/nios2/cpu.c | 1 + target/openrisc/cpu.c | 1 + target/ppc/cpu_init.c | 1 + target/riscv/cpu.c | 1 + target/rx/cpu.c | 1 + target/s390x/cpu.c | 1 + target/sh4/cpu.c | 1 + target/sparc/cpu.c | 1 + target/tricore/cpu.c | 1 + target/xtensa/cpu.c | 1 + 23 files changed, 39 insertions(+), 12 deletions(-)
Hi,
> > So we need more work to make this actually work right.
>
> Yes. I want have all of tcg in the tcg accel module, not only parts of
> it, but that needs some more refactoring. I'll go start looking at this
> once I managed to wade through my vacation backlog.
So, changed the meson.build files to build tcg modular
(https://git.kraxel.org/cgit/qemu/log/?h=sirius/modules-tcg-meson),
then checked what unresolved symbols we have:
# -*- symbols -*-
18 tlb_flush
8 tb_flush
6 update_fp_status
6 cpu_restore_state
4 x86_register_ferr_irq
4 update_mxcsr_status
4 update_mxcsr_from_sse_status
4 tlb_flush_page
4 curr_cflags
3 cpu_cc_compute_all
2 tlb_reset_dirty
2 tlb_plugin_lookup
2 tcg_flush_softmmu_tlb
2 tcg_exec_unrealizefn
2 tcg_exec_realizefn
2 tb_invalidate_phys_range
2 tb_invalidate_phys_page_range
2 tb_check_watchpoint
2 cpu_x86_update_dr7
2 cpu_set_ignne
# -*- files -*-
22 softmmu/physmem.c tlb_flush tlb_flush_page curr_cflags tlb_reset_dirty tb_invalidate_phys_range tb_check_watchpoint cpu_restore_state
10 target/i386/helper.c tlb_flush cpu_restore_state
15 cpu.c cpu_cc_compute_all tlb_flush tcg_flush_softmmu_tlb tcg_exec_unrealizefn tcg_exec_realizefn tb_invalidate_phys_page_range tb_flush
9 target/i386/cpu.h update_fp_status cpu_cc_compute_all update_mxcsr_status
8 target/i386/machine.c update_mxcsr_status update_fp_status tlb_flush cpu_x86_update_dr7
2 target/i386/gdbstub.c update_mxcsr_from_sse_status
2 target/i386/cpu-dump.c update_mxcsr_from_sse_status
2 plugins/loader.c tb_flush
2 plugins/core.c tb_flush
2 plugins/api.c tlb_plugin_lookup
2 i386/pc_q35.c x86_register_ferr_irq
2 i386/pc_piix.c x86_register_ferr_irq
2 i386/pc.c cpu_set_ignne
4 gdbstub.c update_mxcsr_from_sse_status tb_flush
2 core/cpu-common.c tcg_flush_softmmu_tlb
2 accel/tcg/cpu-exec-common.c cpu_restore_state
It's basically two groups:
* Arch-specific (functions taking CPUX86State as argument), most of the
unresolved symbols in target/i386/ and i386/ directories go into this
category.
* Common (functions taking CPUState as argument). Everything else.
The common functions could be added TCGCPUOps to allow them being called via
CPUClass->tcg_ops->$name instead of a direct symbol reference. Not sure this
is a good idea though. Experimental patch covering tcg_exec_realizefn +
tcg_exec_unrealizefn below.
No idea yet how to handle arch-specific bits best. Seems there is no existing
infrastructure to untangle target-specific code and tcg, so this probably needs
something new.
Noticed softmmu/physmem.c has lots of CONFIG_TCG #ifdefs, splitting this into
softmmu/physmem-{common,tcg}.c is probably a good idea.
Comments?
take care,
Gerd
From c0e3f4cbe6aa6889d344beb4cac300b160253f00 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Thu, 29 Jul 2021 09:14:56 +0200
Subject: [PATCH] tcg: add tcg_exec_{realizefn,unrealizefn} to TCGCPUOps
---
include/hw/core/tcg-cpu-ops.h | 7 +++++++
cpu.c | 23 +++++++++++------------
target/alpha/cpu.c | 1 +
target/arm/cpu.c | 1 +
target/arm/cpu_tcg.c | 1 +
target/avr/cpu.c | 1 +
target/cris/cpu.c | 1 +
target/hexagon/cpu.c | 1 +
target/hppa/cpu.c | 1 +
target/i386/tcg/tcg-cpu.c | 1 +
target/m68k/cpu.c | 1 +
target/microblaze/cpu.c | 1 +
target/mips/cpu.c | 1 +
target/nios2/cpu.c | 1 +
target/openrisc/cpu.c | 1 +
target/ppc/cpu_init.c | 1 +
target/riscv/cpu.c | 1 +
target/rx/cpu.c | 1 +
target/s390x/cpu.c | 1 +
target/sh4/cpu.c | 1 +
target/sparc/cpu.c | 1 +
target/tricore/cpu.c | 1 +
target/xtensa/cpu.c | 1 +
23 files changed, 39 insertions(+), 12 deletions(-)
diff --git a/include/hw/core/tcg-cpu-ops.h b/include/hw/core/tcg-cpu-ops.h
index eab27d0c0309..1f33953a4efc 100644
--- a/include/hw/core/tcg-cpu-ops.h
+++ b/include/hw/core/tcg-cpu-ops.h
@@ -107,6 +107,13 @@ struct TCGCPUOps {
#endif /* CONFIG_SOFTMMU */
#endif /* NEED_CPU_H */
+ void (*exec_realizefn)(CPUState *cpu, Error **errp);
+ void (*exec_unrealizefn)(CPUState *cpu);
};
+#define TCG_CPU_OPS_COMMON \
+ .exec_realizefn = tcg_exec_realizefn, \
+ .exec_unrealizefn = tcg_exec_unrealizefn
+
+
#endif /* TCG_CPU_OPS_H */
diff --git a/cpu.c b/cpu.c
index e1799a15bcf5..982463ce7418 100644
--- a/cpu.c
+++ b/cpu.c
@@ -40,6 +40,10 @@
#include "hw/core/accel-cpu.h"
#include "trace/trace-root.h"
+#ifdef CONFIG_TCG
+#include "hw/core/tcg-cpu-ops.h"
+#endif /* CONFIG_TCG */
+
uintptr_t qemu_host_page_size;
intptr_t qemu_host_page_mask;
@@ -129,20 +133,17 @@ const VMStateDescription vmstate_cpu_common = {
void cpu_exec_realizefn(CPUState *cpu, Error **errp)
{
-#ifndef CONFIG_USER_ONLY
CPUClass *cc = CPU_GET_CLASS(cpu);
-#endif
cpu_list_add(cpu);
if (!accel_cpu_realizefn(cpu, errp)) {
return;
}
-#ifdef CONFIG_TCG
+
/* NB: errp parameter is unused currently */
- if (tcg_enabled()) {
- tcg_exec_realizefn(cpu, errp);
+ if (cc->tcg_ops) {
+ cc->tcg_ops->exec_realizefn(cpu, errp);
}
-#endif /* CONFIG_TCG */
#ifdef CONFIG_USER_ONLY
assert(qdev_get_vmsd(DEVICE(cpu)) == NULL ||
@@ -159,9 +160,9 @@ void cpu_exec_realizefn(CPUState *cpu, Error **errp)
void cpu_exec_unrealizefn(CPUState *cpu)
{
-#ifndef CONFIG_USER_ONLY
CPUClass *cc = CPU_GET_CLASS(cpu);
+#ifndef CONFIG_USER_ONLY
if (cc->sysemu_ops->legacy_vmsd != NULL) {
vmstate_unregister(NULL, cc->sysemu_ops->legacy_vmsd, cpu);
}
@@ -169,12 +170,10 @@ void cpu_exec_unrealizefn(CPUState *cpu)
vmstate_unregister(NULL, &vmstate_cpu_common, cpu);
}
#endif
-#ifdef CONFIG_TCG
- /* NB: errp parameter is unused currently */
- if (tcg_enabled()) {
- tcg_exec_unrealizefn(cpu);
+
+ if (cc->tcg_ops) {
+ cc->tcg_ops->exec_unrealizefn(cpu);
}
-#endif /* CONFIG_TCG */
cpu_list_remove(cpu);
}
diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
index 4871ad0c0a6a..715fb97d6707 100644
--- a/target/alpha/cpu.c
+++ b/target/alpha/cpu.c
@@ -226,6 +226,7 @@ static const struct TCGCPUOps alpha_tcg_ops = {
.do_transaction_failed = alpha_cpu_do_transaction_failed,
.do_unaligned_access = alpha_cpu_do_unaligned_access,
#endif /* !CONFIG_USER_ONLY */
+ TCG_CPU_OPS_COMMON,
};
static void alpha_cpu_class_init(ObjectClass *oc, void *data)
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 2866dd765882..7f051bd24e0b 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1996,6 +1996,7 @@ static const struct TCGCPUOps arm_tcg_ops = {
.debug_check_watchpoint = arm_debug_check_watchpoint,
.debug_check_breakpoint = arm_debug_check_breakpoint,
#endif /* !CONFIG_USER_ONLY */
+ TCG_CPU_OPS_COMMON,
};
#endif /* CONFIG_TCG */
diff --git a/target/arm/cpu_tcg.c b/target/arm/cpu_tcg.c
index ed444bf436a4..0f8657ede860 100644
--- a/target/arm/cpu_tcg.c
+++ b/target/arm/cpu_tcg.c
@@ -913,6 +913,7 @@ static const struct TCGCPUOps arm_v7m_tcg_ops = {
.debug_check_watchpoint = arm_debug_check_watchpoint,
.debug_check_breakpoint = arm_debug_check_breakpoint,
#endif /* !CONFIG_USER_ONLY */
+ TCG_CPU_OPS_COMMON,
};
#endif /* CONFIG_TCG */
diff --git a/target/avr/cpu.c b/target/avr/cpu.c
index ea14175ca557..8b3261ef832c 100644
--- a/target/avr/cpu.c
+++ b/target/avr/cpu.c
@@ -201,6 +201,7 @@ static const struct TCGCPUOps avr_tcg_ops = {
#ifndef CONFIG_USER_ONLY
.do_interrupt = avr_cpu_do_interrupt,
#endif /* !CONFIG_USER_ONLY */
+ TCG_CPU_OPS_COMMON,
};
static void avr_cpu_class_init(ObjectClass *oc, void *data)
diff --git a/target/cris/cpu.c b/target/cris/cpu.c
index 70932b1f8c7d..32ca1051f050 100644
--- a/target/cris/cpu.c
+++ b/target/cris/cpu.c
@@ -221,6 +221,7 @@ static const struct TCGCPUOps crisv32_tcg_ops = {
#ifndef CONFIG_USER_ONLY
.do_interrupt = cris_cpu_do_interrupt,
#endif /* !CONFIG_USER_ONLY */
+ TCG_CPU_OPS_COMMON,
};
static void crisv8_cpu_class_init(ObjectClass *oc, void *data)
diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c
index 3338365c16ec..86c001b67484 100644
--- a/target/hexagon/cpu.c
+++ b/target/hexagon/cpu.c
@@ -273,6 +273,7 @@ static const struct TCGCPUOps hexagon_tcg_ops = {
.initialize = hexagon_translate_init,
.synchronize_from_tb = hexagon_cpu_synchronize_from_tb,
.tlb_fill = hexagon_tlb_fill,
+ TCG_CPU_OPS_COMMON,
};
static void hexagon_cpu_class_init(ObjectClass *c, void *data)
diff --git a/target/hppa/cpu.c b/target/hppa/cpu.c
index 2eace4ee1240..5163118e986d 100644
--- a/target/hppa/cpu.c
+++ b/target/hppa/cpu.c
@@ -151,6 +151,7 @@ static const struct TCGCPUOps hppa_tcg_ops = {
.do_interrupt = hppa_cpu_do_interrupt,
.do_unaligned_access = hppa_cpu_do_unaligned_access,
#endif /* !CONFIG_USER_ONLY */
+ TCG_CPU_OPS_COMMON,
};
static void hppa_cpu_class_init(ObjectClass *oc, void *data)
diff --git a/target/i386/tcg/tcg-cpu.c b/target/i386/tcg/tcg-cpu.c
index 93a79a574154..bf7d91fed6da 100644
--- a/target/i386/tcg/tcg-cpu.c
+++ b/target/i386/tcg/tcg-cpu.c
@@ -79,6 +79,7 @@ static const struct TCGCPUOps x86_tcg_ops = {
.debug_excp_handler = breakpoint_handler,
.debug_check_breakpoint = x86_debug_check_breakpoint,
#endif /* !CONFIG_USER_ONLY */
+ TCG_CPU_OPS_COMMON,
};
static void tcg_cpu_init_ops(AccelCPUClass *accel_cpu, CPUClass *cc)
diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c
index 72de6e972627..df06c07907d6 100644
--- a/target/m68k/cpu.c
+++ b/target/m68k/cpu.c
@@ -522,6 +522,7 @@ static const struct TCGCPUOps m68k_tcg_ops = {
.do_interrupt = m68k_cpu_do_interrupt,
.do_transaction_failed = m68k_cpu_transaction_failed,
#endif /* !CONFIG_USER_ONLY */
+ TCG_CPU_OPS_COMMON,
};
static void m68k_cpu_class_init(ObjectClass *c, void *data)
diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
index 72d8f2a0daa6..0e781264ef7b 100644
--- a/target/microblaze/cpu.c
+++ b/target/microblaze/cpu.c
@@ -373,6 +373,7 @@ static const struct TCGCPUOps mb_tcg_ops = {
.do_transaction_failed = mb_cpu_transaction_failed,
.do_unaligned_access = mb_cpu_do_unaligned_access,
#endif /* !CONFIG_USER_ONLY */
+ TCG_CPU_OPS_COMMON,
};
static void mb_cpu_class_init(ObjectClass *oc, void *data)
diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index d426918291a9..3216a33d0047 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -548,6 +548,7 @@ static const struct TCGCPUOps mips_tcg_ops = {
.do_unaligned_access = mips_cpu_do_unaligned_access,
.io_recompile_replay_branch = mips_io_recompile_replay_branch,
#endif /* !CONFIG_USER_ONLY */
+ TCG_CPU_OPS_COMMON,
};
#endif /* CONFIG_TCG */
diff --git a/target/nios2/cpu.c b/target/nios2/cpu.c
index 5e37defef805..55e21e8cd414 100644
--- a/target/nios2/cpu.c
+++ b/target/nios2/cpu.c
@@ -226,6 +226,7 @@ static const struct TCGCPUOps nios2_tcg_ops = {
.do_interrupt = nios2_cpu_do_interrupt,
.do_unaligned_access = nios2_cpu_do_unaligned_access,
#endif /* !CONFIG_USER_ONLY */
+ TCG_CPU_OPS_COMMON,
};
static void nios2_cpu_class_init(ObjectClass *oc, void *data)
diff --git a/target/openrisc/cpu.c b/target/openrisc/cpu.c
index bd34e429ecbf..0d9e1aa58a8f 100644
--- a/target/openrisc/cpu.c
+++ b/target/openrisc/cpu.c
@@ -192,6 +192,7 @@ static const struct TCGCPUOps openrisc_tcg_ops = {
#ifndef CONFIG_USER_ONLY
.do_interrupt = openrisc_cpu_do_interrupt,
#endif /* !CONFIG_USER_ONLY */
+ TCG_CPU_OPS_COMMON,
};
static void openrisc_cpu_class_init(ObjectClass *oc, void *data)
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 505a0ed6ac09..3f8bf2e3fcbf 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -9021,6 +9021,7 @@ static const struct TCGCPUOps ppc_tcg_ops = {
.cpu_exec_exit = ppc_cpu_exec_exit,
.do_unaligned_access = ppc_cpu_do_unaligned_access,
#endif /* !CONFIG_USER_ONLY */
+ TCG_CPU_OPS_COMMON,
};
#endif /* CONFIG_TCG */
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 991a6bb7604f..6a129b4fe211 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -650,6 +650,7 @@ static const struct TCGCPUOps riscv_tcg_ops = {
.do_transaction_failed = riscv_cpu_do_transaction_failed,
.do_unaligned_access = riscv_cpu_do_unaligned_access,
#endif /* !CONFIG_USER_ONLY */
+ TCG_CPU_OPS_COMMON,
};
static void riscv_cpu_class_init(ObjectClass *c, void *data)
diff --git a/target/rx/cpu.c b/target/rx/cpu.c
index 96cc96e514fe..7a1ebc694f22 100644
--- a/target/rx/cpu.c
+++ b/target/rx/cpu.c
@@ -192,6 +192,7 @@ static const struct TCGCPUOps rx_tcg_ops = {
#ifndef CONFIG_USER_ONLY
.do_interrupt = rx_cpu_do_interrupt,
#endif /* !CONFIG_USER_ONLY */
+ TCG_CPU_OPS_COMMON,
};
static void rx_cpu_class_init(ObjectClass *klass, void *data)
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index 7b7b05f1d3a0..8dfcde1590c0 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -274,6 +274,7 @@ static const struct TCGCPUOps s390_tcg_ops = {
.debug_excp_handler = s390x_cpu_debug_excp_handler,
.do_unaligned_access = s390x_cpu_do_unaligned_access,
#endif /* !CONFIG_USER_ONLY */
+ TCG_CPU_OPS_COMMON,
};
#endif /* CONFIG_TCG */
diff --git a/target/sh4/cpu.c b/target/sh4/cpu.c
index 832692294211..0b4e03913656 100644
--- a/target/sh4/cpu.c
+++ b/target/sh4/cpu.c
@@ -244,6 +244,7 @@ static const struct TCGCPUOps superh_tcg_ops = {
.do_unaligned_access = superh_cpu_do_unaligned_access,
.io_recompile_replay_branch = superh_io_recompile_replay_branch,
#endif /* !CONFIG_USER_ONLY */
+ TCG_CPU_OPS_COMMON,
};
static void superh_cpu_class_init(ObjectClass *oc, void *data)
diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
index da6b30ec747a..2b2d2946223b 100644
--- a/target/sparc/cpu.c
+++ b/target/sparc/cpu.c
@@ -871,6 +871,7 @@ static const struct TCGCPUOps sparc_tcg_ops = {
.do_transaction_failed = sparc_cpu_do_transaction_failed,
.do_unaligned_access = sparc_cpu_do_unaligned_access,
#endif /* !CONFIG_USER_ONLY */
+ TCG_CPU_OPS_COMMON,
};
#endif /* CONFIG_TCG */
diff --git a/target/tricore/cpu.c b/target/tricore/cpu.c
index b95682b7f04d..daeaecca26e0 100644
--- a/target/tricore/cpu.c
+++ b/target/tricore/cpu.c
@@ -154,6 +154,7 @@ static const struct TCGCPUOps tricore_tcg_ops = {
.initialize = tricore_tcg_init,
.synchronize_from_tb = tricore_cpu_synchronize_from_tb,
.tlb_fill = tricore_cpu_tlb_fill,
+ TCG_CPU_OPS_COMMON,
};
static void tricore_cpu_class_init(ObjectClass *c, void *data)
diff --git a/target/xtensa/cpu.c b/target/xtensa/cpu.c
index 58ec3a086224..d7d1b967040b 100644
--- a/target/xtensa/cpu.c
+++ b/target/xtensa/cpu.c
@@ -201,6 +201,7 @@ static const struct TCGCPUOps xtensa_tcg_ops = {
.do_transaction_failed = xtensa_cpu_do_transaction_failed,
.do_unaligned_access = xtensa_cpu_do_unaligned_access,
#endif /* !CONFIG_USER_ONLY */
+ TCG_CPU_OPS_COMMON,
};
static void xtensa_cpu_class_init(ObjectClass *oc, void *data)
--
2.31.1
Hi all,
modularization of TCG is still incomplete in my view: we need to handle the error case better.
If we configure --enable-modules --enable-tcg --enable-kvm , but for some reason the .so is not present or is not loaded successfully,
and kvm fails to be enabled (for example, /dev/kvm not available etc),
we try to revert to tcg as fallback, and then fail with an unhelpful message:
ERROR:../accel/accel-softmmu.c:82:accel_init_ops_interfaces: assertion failed: (ops != NULL)
What we should I think say:
ERROR: tcg has been configured and requested, but plugin could not be loaded.
I'll dig into it a bit, but another pair of eyes would be helful as well..
Thanks,
Claudio
On 7/29/21 11:14, Gerd Hoffmann wrote:
> Hi,
>
>>> So we need more work to make this actually work right.
>>
>> Yes. I want have all of tcg in the tcg accel module, not only parts of
>> it, but that needs some more refactoring. I'll go start looking at this
>> once I managed to wade through my vacation backlog.
>
> So, changed the meson.build files to build tcg modular
> (https://git.kraxel.org/cgit/qemu/log/?h=sirius/modules-tcg-meson),
> then checked what unresolved symbols we have:
>
> # -*- symbols -*-
> 18 tlb_flush
> 8 tb_flush
> 6 update_fp_status
> 6 cpu_restore_state
> 4 x86_register_ferr_irq
> 4 update_mxcsr_status
> 4 update_mxcsr_from_sse_status
> 4 tlb_flush_page
> 4 curr_cflags
> 3 cpu_cc_compute_all
> 2 tlb_reset_dirty
> 2 tlb_plugin_lookup
> 2 tcg_flush_softmmu_tlb
> 2 tcg_exec_unrealizefn
> 2 tcg_exec_realizefn
> 2 tb_invalidate_phys_range
> 2 tb_invalidate_phys_page_range
> 2 tb_check_watchpoint
> 2 cpu_x86_update_dr7
> 2 cpu_set_ignne
> # -*- files -*-
> 22 softmmu/physmem.c tlb_flush tlb_flush_page curr_cflags tlb_reset_dirty tb_invalidate_phys_range tb_check_watchpoint cpu_restore_state
> 10 target/i386/helper.c tlb_flush cpu_restore_state
> 15 cpu.c cpu_cc_compute_all tlb_flush tcg_flush_softmmu_tlb tcg_exec_unrealizefn tcg_exec_realizefn tb_invalidate_phys_page_range tb_flush
> 9 target/i386/cpu.h update_fp_status cpu_cc_compute_all update_mxcsr_status
> 8 target/i386/machine.c update_mxcsr_status update_fp_status tlb_flush cpu_x86_update_dr7
> 2 target/i386/gdbstub.c update_mxcsr_from_sse_status
> 2 target/i386/cpu-dump.c update_mxcsr_from_sse_status
> 2 plugins/loader.c tb_flush
> 2 plugins/core.c tb_flush
> 2 plugins/api.c tlb_plugin_lookup
> 2 i386/pc_q35.c x86_register_ferr_irq
> 2 i386/pc_piix.c x86_register_ferr_irq
> 2 i386/pc.c cpu_set_ignne
> 4 gdbstub.c update_mxcsr_from_sse_status tb_flush
> 2 core/cpu-common.c tcg_flush_softmmu_tlb
> 2 accel/tcg/cpu-exec-common.c cpu_restore_state
>
> It's basically two groups:
>
> * Arch-specific (functions taking CPUX86State as argument), most of the
> unresolved symbols in target/i386/ and i386/ directories go into this
> category.
> * Common (functions taking CPUState as argument). Everything else.
>
> The common functions could be added TCGCPUOps to allow them being called via
> CPUClass->tcg_ops->$name instead of a direct symbol reference. Not sure this
> is a good idea though. Experimental patch covering tcg_exec_realizefn +
> tcg_exec_unrealizefn below.
>
> No idea yet how to handle arch-specific bits best. Seems there is no existing
> infrastructure to untangle target-specific code and tcg, so this probably needs
> something new.
>
> Noticed softmmu/physmem.c has lots of CONFIG_TCG #ifdefs, splitting this into
> softmmu/physmem-{common,tcg}.c is probably a good idea.
>
> Comments?
>
> take care,
> Gerd
>
> From c0e3f4cbe6aa6889d344beb4cac300b160253f00 Mon Sep 17 00:00:00 2001
> From: Gerd Hoffmann <kraxel@redhat.com>
> Date: Thu, 29 Jul 2021 09:14:56 +0200
> Subject: [PATCH] tcg: add tcg_exec_{realizefn,unrealizefn} to TCGCPUOps
>
> ---
> include/hw/core/tcg-cpu-ops.h | 7 +++++++
> cpu.c | 23 +++++++++++------------
> target/alpha/cpu.c | 1 +
> target/arm/cpu.c | 1 +
> target/arm/cpu_tcg.c | 1 +
> target/avr/cpu.c | 1 +
> target/cris/cpu.c | 1 +
> target/hexagon/cpu.c | 1 +
> target/hppa/cpu.c | 1 +
> target/i386/tcg/tcg-cpu.c | 1 +
> target/m68k/cpu.c | 1 +
> target/microblaze/cpu.c | 1 +
> target/mips/cpu.c | 1 +
> target/nios2/cpu.c | 1 +
> target/openrisc/cpu.c | 1 +
> target/ppc/cpu_init.c | 1 +
> target/riscv/cpu.c | 1 +
> target/rx/cpu.c | 1 +
> target/s390x/cpu.c | 1 +
> target/sh4/cpu.c | 1 +
> target/sparc/cpu.c | 1 +
> target/tricore/cpu.c | 1 +
> target/xtensa/cpu.c | 1 +
> 23 files changed, 39 insertions(+), 12 deletions(-)
>
> diff --git a/include/hw/core/tcg-cpu-ops.h b/include/hw/core/tcg-cpu-ops.h
> index eab27d0c0309..1f33953a4efc 100644
> --- a/include/hw/core/tcg-cpu-ops.h
> +++ b/include/hw/core/tcg-cpu-ops.h
> @@ -107,6 +107,13 @@ struct TCGCPUOps {
> #endif /* CONFIG_SOFTMMU */
> #endif /* NEED_CPU_H */
>
> + void (*exec_realizefn)(CPUState *cpu, Error **errp);
> + void (*exec_unrealizefn)(CPUState *cpu);
> };
>
> +#define TCG_CPU_OPS_COMMON \
> + .exec_realizefn = tcg_exec_realizefn, \
> + .exec_unrealizefn = tcg_exec_unrealizefn
> +
> +
> #endif /* TCG_CPU_OPS_H */
> diff --git a/cpu.c b/cpu.c
> index e1799a15bcf5..982463ce7418 100644
> --- a/cpu.c
> +++ b/cpu.c
> @@ -40,6 +40,10 @@
> #include "hw/core/accel-cpu.h"
> #include "trace/trace-root.h"
>
> +#ifdef CONFIG_TCG
> +#include "hw/core/tcg-cpu-ops.h"
> +#endif /* CONFIG_TCG */
> +
> uintptr_t qemu_host_page_size;
> intptr_t qemu_host_page_mask;
>
> @@ -129,20 +133,17 @@ const VMStateDescription vmstate_cpu_common = {
>
> void cpu_exec_realizefn(CPUState *cpu, Error **errp)
> {
> -#ifndef CONFIG_USER_ONLY
> CPUClass *cc = CPU_GET_CLASS(cpu);
> -#endif
>
> cpu_list_add(cpu);
> if (!accel_cpu_realizefn(cpu, errp)) {
> return;
> }
> -#ifdef CONFIG_TCG
> +
> /* NB: errp parameter is unused currently */
> - if (tcg_enabled()) {
> - tcg_exec_realizefn(cpu, errp);
> + if (cc->tcg_ops) {
> + cc->tcg_ops->exec_realizefn(cpu, errp);
> }
> -#endif /* CONFIG_TCG */
>
> #ifdef CONFIG_USER_ONLY
> assert(qdev_get_vmsd(DEVICE(cpu)) == NULL ||
> @@ -159,9 +160,9 @@ void cpu_exec_realizefn(CPUState *cpu, Error **errp)
>
> void cpu_exec_unrealizefn(CPUState *cpu)
> {
> -#ifndef CONFIG_USER_ONLY
> CPUClass *cc = CPU_GET_CLASS(cpu);
>
> +#ifndef CONFIG_USER_ONLY
> if (cc->sysemu_ops->legacy_vmsd != NULL) {
> vmstate_unregister(NULL, cc->sysemu_ops->legacy_vmsd, cpu);
> }
> @@ -169,12 +170,10 @@ void cpu_exec_unrealizefn(CPUState *cpu)
> vmstate_unregister(NULL, &vmstate_cpu_common, cpu);
> }
> #endif
> -#ifdef CONFIG_TCG
> - /* NB: errp parameter is unused currently */
> - if (tcg_enabled()) {
> - tcg_exec_unrealizefn(cpu);
> +
> + if (cc->tcg_ops) {
> + cc->tcg_ops->exec_unrealizefn(cpu);
> }
> -#endif /* CONFIG_TCG */
>
> cpu_list_remove(cpu);
> }
> diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
> index 4871ad0c0a6a..715fb97d6707 100644
> --- a/target/alpha/cpu.c
> +++ b/target/alpha/cpu.c
> @@ -226,6 +226,7 @@ static const struct TCGCPUOps alpha_tcg_ops = {
> .do_transaction_failed = alpha_cpu_do_transaction_failed,
> .do_unaligned_access = alpha_cpu_do_unaligned_access,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
>
> static void alpha_cpu_class_init(ObjectClass *oc, void *data)
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 2866dd765882..7f051bd24e0b 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -1996,6 +1996,7 @@ static const struct TCGCPUOps arm_tcg_ops = {
> .debug_check_watchpoint = arm_debug_check_watchpoint,
> .debug_check_breakpoint = arm_debug_check_breakpoint,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
> #endif /* CONFIG_TCG */
>
> diff --git a/target/arm/cpu_tcg.c b/target/arm/cpu_tcg.c
> index ed444bf436a4..0f8657ede860 100644
> --- a/target/arm/cpu_tcg.c
> +++ b/target/arm/cpu_tcg.c
> @@ -913,6 +913,7 @@ static const struct TCGCPUOps arm_v7m_tcg_ops = {
> .debug_check_watchpoint = arm_debug_check_watchpoint,
> .debug_check_breakpoint = arm_debug_check_breakpoint,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
> #endif /* CONFIG_TCG */
>
> diff --git a/target/avr/cpu.c b/target/avr/cpu.c
> index ea14175ca557..8b3261ef832c 100644
> --- a/target/avr/cpu.c
> +++ b/target/avr/cpu.c
> @@ -201,6 +201,7 @@ static const struct TCGCPUOps avr_tcg_ops = {
> #ifndef CONFIG_USER_ONLY
> .do_interrupt = avr_cpu_do_interrupt,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
>
> static void avr_cpu_class_init(ObjectClass *oc, void *data)
> diff --git a/target/cris/cpu.c b/target/cris/cpu.c
> index 70932b1f8c7d..32ca1051f050 100644
> --- a/target/cris/cpu.c
> +++ b/target/cris/cpu.c
> @@ -221,6 +221,7 @@ static const struct TCGCPUOps crisv32_tcg_ops = {
> #ifndef CONFIG_USER_ONLY
> .do_interrupt = cris_cpu_do_interrupt,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
>
> static void crisv8_cpu_class_init(ObjectClass *oc, void *data)
> diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c
> index 3338365c16ec..86c001b67484 100644
> --- a/target/hexagon/cpu.c
> +++ b/target/hexagon/cpu.c
> @@ -273,6 +273,7 @@ static const struct TCGCPUOps hexagon_tcg_ops = {
> .initialize = hexagon_translate_init,
> .synchronize_from_tb = hexagon_cpu_synchronize_from_tb,
> .tlb_fill = hexagon_tlb_fill,
> + TCG_CPU_OPS_COMMON,
> };
>
> static void hexagon_cpu_class_init(ObjectClass *c, void *data)
> diff --git a/target/hppa/cpu.c b/target/hppa/cpu.c
> index 2eace4ee1240..5163118e986d 100644
> --- a/target/hppa/cpu.c
> +++ b/target/hppa/cpu.c
> @@ -151,6 +151,7 @@ static const struct TCGCPUOps hppa_tcg_ops = {
> .do_interrupt = hppa_cpu_do_interrupt,
> .do_unaligned_access = hppa_cpu_do_unaligned_access,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
>
> static void hppa_cpu_class_init(ObjectClass *oc, void *data)
> diff --git a/target/i386/tcg/tcg-cpu.c b/target/i386/tcg/tcg-cpu.c
> index 93a79a574154..bf7d91fed6da 100644
> --- a/target/i386/tcg/tcg-cpu.c
> +++ b/target/i386/tcg/tcg-cpu.c
> @@ -79,6 +79,7 @@ static const struct TCGCPUOps x86_tcg_ops = {
> .debug_excp_handler = breakpoint_handler,
> .debug_check_breakpoint = x86_debug_check_breakpoint,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
>
> static void tcg_cpu_init_ops(AccelCPUClass *accel_cpu, CPUClass *cc)
> diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c
> index 72de6e972627..df06c07907d6 100644
> --- a/target/m68k/cpu.c
> +++ b/target/m68k/cpu.c
> @@ -522,6 +522,7 @@ static const struct TCGCPUOps m68k_tcg_ops = {
> .do_interrupt = m68k_cpu_do_interrupt,
> .do_transaction_failed = m68k_cpu_transaction_failed,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
>
> static void m68k_cpu_class_init(ObjectClass *c, void *data)
> diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
> index 72d8f2a0daa6..0e781264ef7b 100644
> --- a/target/microblaze/cpu.c
> +++ b/target/microblaze/cpu.c
> @@ -373,6 +373,7 @@ static const struct TCGCPUOps mb_tcg_ops = {
> .do_transaction_failed = mb_cpu_transaction_failed,
> .do_unaligned_access = mb_cpu_do_unaligned_access,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
>
> static void mb_cpu_class_init(ObjectClass *oc, void *data)
> diff --git a/target/mips/cpu.c b/target/mips/cpu.c
> index d426918291a9..3216a33d0047 100644
> --- a/target/mips/cpu.c
> +++ b/target/mips/cpu.c
> @@ -548,6 +548,7 @@ static const struct TCGCPUOps mips_tcg_ops = {
> .do_unaligned_access = mips_cpu_do_unaligned_access,
> .io_recompile_replay_branch = mips_io_recompile_replay_branch,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
> #endif /* CONFIG_TCG */
>
> diff --git a/target/nios2/cpu.c b/target/nios2/cpu.c
> index 5e37defef805..55e21e8cd414 100644
> --- a/target/nios2/cpu.c
> +++ b/target/nios2/cpu.c
> @@ -226,6 +226,7 @@ static const struct TCGCPUOps nios2_tcg_ops = {
> .do_interrupt = nios2_cpu_do_interrupt,
> .do_unaligned_access = nios2_cpu_do_unaligned_access,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
>
> static void nios2_cpu_class_init(ObjectClass *oc, void *data)
> diff --git a/target/openrisc/cpu.c b/target/openrisc/cpu.c
> index bd34e429ecbf..0d9e1aa58a8f 100644
> --- a/target/openrisc/cpu.c
> +++ b/target/openrisc/cpu.c
> @@ -192,6 +192,7 @@ static const struct TCGCPUOps openrisc_tcg_ops = {
> #ifndef CONFIG_USER_ONLY
> .do_interrupt = openrisc_cpu_do_interrupt,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
>
> static void openrisc_cpu_class_init(ObjectClass *oc, void *data)
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index 505a0ed6ac09..3f8bf2e3fcbf 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -9021,6 +9021,7 @@ static const struct TCGCPUOps ppc_tcg_ops = {
> .cpu_exec_exit = ppc_cpu_exec_exit,
> .do_unaligned_access = ppc_cpu_do_unaligned_access,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
> #endif /* CONFIG_TCG */
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 991a6bb7604f..6a129b4fe211 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -650,6 +650,7 @@ static const struct TCGCPUOps riscv_tcg_ops = {
> .do_transaction_failed = riscv_cpu_do_transaction_failed,
> .do_unaligned_access = riscv_cpu_do_unaligned_access,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
>
> static void riscv_cpu_class_init(ObjectClass *c, void *data)
> diff --git a/target/rx/cpu.c b/target/rx/cpu.c
> index 96cc96e514fe..7a1ebc694f22 100644
> --- a/target/rx/cpu.c
> +++ b/target/rx/cpu.c
> @@ -192,6 +192,7 @@ static const struct TCGCPUOps rx_tcg_ops = {
> #ifndef CONFIG_USER_ONLY
> .do_interrupt = rx_cpu_do_interrupt,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
>
> static void rx_cpu_class_init(ObjectClass *klass, void *data)
> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
> index 7b7b05f1d3a0..8dfcde1590c0 100644
> --- a/target/s390x/cpu.c
> +++ b/target/s390x/cpu.c
> @@ -274,6 +274,7 @@ static const struct TCGCPUOps s390_tcg_ops = {
> .debug_excp_handler = s390x_cpu_debug_excp_handler,
> .do_unaligned_access = s390x_cpu_do_unaligned_access,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
> #endif /* CONFIG_TCG */
>
> diff --git a/target/sh4/cpu.c b/target/sh4/cpu.c
> index 832692294211..0b4e03913656 100644
> --- a/target/sh4/cpu.c
> +++ b/target/sh4/cpu.c
> @@ -244,6 +244,7 @@ static const struct TCGCPUOps superh_tcg_ops = {
> .do_unaligned_access = superh_cpu_do_unaligned_access,
> .io_recompile_replay_branch = superh_io_recompile_replay_branch,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
>
> static void superh_cpu_class_init(ObjectClass *oc, void *data)
> diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
> index da6b30ec747a..2b2d2946223b 100644
> --- a/target/sparc/cpu.c
> +++ b/target/sparc/cpu.c
> @@ -871,6 +871,7 @@ static const struct TCGCPUOps sparc_tcg_ops = {
> .do_transaction_failed = sparc_cpu_do_transaction_failed,
> .do_unaligned_access = sparc_cpu_do_unaligned_access,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
> #endif /* CONFIG_TCG */
>
> diff --git a/target/tricore/cpu.c b/target/tricore/cpu.c
> index b95682b7f04d..daeaecca26e0 100644
> --- a/target/tricore/cpu.c
> +++ b/target/tricore/cpu.c
> @@ -154,6 +154,7 @@ static const struct TCGCPUOps tricore_tcg_ops = {
> .initialize = tricore_tcg_init,
> .synchronize_from_tb = tricore_cpu_synchronize_from_tb,
> .tlb_fill = tricore_cpu_tlb_fill,
> + TCG_CPU_OPS_COMMON,
> };
>
> static void tricore_cpu_class_init(ObjectClass *c, void *data)
> diff --git a/target/xtensa/cpu.c b/target/xtensa/cpu.c
> index 58ec3a086224..d7d1b967040b 100644
> --- a/target/xtensa/cpu.c
> +++ b/target/xtensa/cpu.c
> @@ -201,6 +201,7 @@ static const struct TCGCPUOps xtensa_tcg_ops = {
> .do_transaction_failed = xtensa_cpu_do_transaction_failed,
> .do_unaligned_access = xtensa_cpu_do_unaligned_access,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
>
> static void xtensa_cpu_class_init(ObjectClass *oc, void *data)
On 7/29/21 11:14 AM, Gerd Hoffmann wrote:
> Hi,
>
>>> So we need more work to make this actually work right.
>>
>> Yes. I want have all of tcg in the tcg accel module, not only parts of
>> it, but that needs some more refactoring. I'll go start looking at this
>> once I managed to wade through my vacation backlog.
>
> So, changed the meson.build files to build tcg modular
> (https://git.kraxel.org/cgit/qemu/log/?h=sirius/modules-tcg-meson),
> then checked what unresolved symbols we have:
>
> # -*- symbols -*-
> 18 tlb_flush
> 8 tb_flush
> 6 update_fp_status
> 6 cpu_restore_state
> 4 x86_register_ferr_irq
> 4 update_mxcsr_status
> 4 update_mxcsr_from_sse_status
> 4 tlb_flush_page
> 4 curr_cflags
> 3 cpu_cc_compute_all
> 2 tlb_reset_dirty
> 2 tlb_plugin_lookup
> 2 tcg_flush_softmmu_tlb
> 2 tcg_exec_unrealizefn
> 2 tcg_exec_realizefn
> 2 tb_invalidate_phys_range
> 2 tb_invalidate_phys_page_range
> 2 tb_check_watchpoint
> 2 cpu_x86_update_dr7
> 2 cpu_set_ignne
> # -*- files -*-
> 22 softmmu/physmem.c tlb_flush tlb_flush_page curr_cflags tlb_reset_dirty tb_invalidate_phys_range tb_check_watchpoint cpu_restore_state
> 10 target/i386/helper.c tlb_flush cpu_restore_state
> 15 cpu.c cpu_cc_compute_all tlb_flush tcg_flush_softmmu_tlb tcg_exec_unrealizefn tcg_exec_realizefn tb_invalidate_phys_page_range tb_flush
> 9 target/i386/cpu.h update_fp_status cpu_cc_compute_all update_mxcsr_status
> 8 target/i386/machine.c update_mxcsr_status update_fp_status tlb_flush cpu_x86_update_dr7
> 2 target/i386/gdbstub.c update_mxcsr_from_sse_status
> 2 target/i386/cpu-dump.c update_mxcsr_from_sse_status
> 2 plugins/loader.c tb_flush
> 2 plugins/core.c tb_flush
> 2 plugins/api.c tlb_plugin_lookup
> 2 i386/pc_q35.c x86_register_ferr_irq
> 2 i386/pc_piix.c x86_register_ferr_irq
> 2 i386/pc.c cpu_set_ignne
> 4 gdbstub.c update_mxcsr_from_sse_status tb_flush
> 2 core/cpu-common.c tcg_flush_softmmu_tlb
> 2 accel/tcg/cpu-exec-common.c cpu_restore_state
>
> It's basically two groups:
>
> * Arch-specific (functions taking CPUX86State as argument), most of the
> unresolved symbols in target/i386/ and i386/ directories go into this
> category.
Yes, and we need to think about all targets, not just i386.
> * Common (functions taking CPUState as argument). Everything else.
>
> The common functions could be added TCGCPUOps to allow them being called via
TCGCCPUOps are target-specific in their implementation, so I guess it's the arch specific part that could be TCGCPUOps (maybe, would need deep thinking).
> CPUClass->tcg_ops->$name instead of a direct symbol reference. Not sure this
> is a good idea though. Experimental patch covering tcg_exec_realizefn +
> tcg_exec_unrealizefn below.
>
> No idea yet how to handle arch-specific bits best. Seems there is no existing
> infrastructure to untangle target-specific code and tcg, so this probably needs
> something new.
We need target-specific modules. They could at the beginning absorb also the non-target specific parts in my view.
So you have a big tcg-arm module, a tcg-i386 module etc.
I think I sketched already the idea in the Makefile I shared before?
>
> Noticed softmmu/physmem.c has lots of CONFIG_TCG #ifdefs, splitting this into
> softmmu/physmem-{common,tcg}.c is probably a good idea.
>
> Comments?
>
> take care,
> Gerd
>
> From c0e3f4cbe6aa6889d344beb4cac300b160253f00 Mon Sep 17 00:00:00 2001
> From: Gerd Hoffmann <kraxel@redhat.com>
> Date: Thu, 29 Jul 2021 09:14:56 +0200
> Subject: [PATCH] tcg: add tcg_exec_{realizefn,unrealizefn} to TCGCPUOps
>
> ---
> include/hw/core/tcg-cpu-ops.h | 7 +++++++
> cpu.c | 23 +++++++++++------------
> target/alpha/cpu.c | 1 +
> target/arm/cpu.c | 1 +
> target/arm/cpu_tcg.c | 1 +
> target/avr/cpu.c | 1 +
> target/cris/cpu.c | 1 +
> target/hexagon/cpu.c | 1 +
> target/hppa/cpu.c | 1 +
> target/i386/tcg/tcg-cpu.c | 1 +
> target/m68k/cpu.c | 1 +
> target/microblaze/cpu.c | 1 +
> target/mips/cpu.c | 1 +
> target/nios2/cpu.c | 1 +
> target/openrisc/cpu.c | 1 +
> target/ppc/cpu_init.c | 1 +
> target/riscv/cpu.c | 1 +
> target/rx/cpu.c | 1 +
> target/s390x/cpu.c | 1 +
> target/sh4/cpu.c | 1 +
> target/sparc/cpu.c | 1 +
> target/tricore/cpu.c | 1 +
> target/xtensa/cpu.c | 1 +
> 23 files changed, 39 insertions(+), 12 deletions(-)
>
> diff --git a/include/hw/core/tcg-cpu-ops.h b/include/hw/core/tcg-cpu-ops.h
> index eab27d0c0309..1f33953a4efc 100644
> --- a/include/hw/core/tcg-cpu-ops.h
> +++ b/include/hw/core/tcg-cpu-ops.h
> @@ -107,6 +107,13 @@ struct TCGCPUOps {
> #endif /* CONFIG_SOFTMMU */
> #endif /* NEED_CPU_H */
>
> + void (*exec_realizefn)(CPUState *cpu, Error **errp);
> + void (*exec_unrealizefn)(CPUState *cpu);
> };
>
> +#define TCG_CPU_OPS_COMMON \
> + .exec_realizefn = tcg_exec_realizefn, \
> + .exec_unrealizefn = tcg_exec_unrealizefn
> +
> +
> #endif /* TCG_CPU_OPS_H */
> diff --git a/cpu.c b/cpu.c
> index e1799a15bcf5..982463ce7418 100644
> --- a/cpu.c
> +++ b/cpu.c
> @@ -40,6 +40,10 @@
> #include "hw/core/accel-cpu.h"
> #include "trace/trace-root.h"
>
> +#ifdef CONFIG_TCG
> +#include "hw/core/tcg-cpu-ops.h"
> +#endif /* CONFIG_TCG */
> +
> uintptr_t qemu_host_page_size;
> intptr_t qemu_host_page_mask;
>
> @@ -129,20 +133,17 @@ const VMStateDescription vmstate_cpu_common = {
>
> void cpu_exec_realizefn(CPUState *cpu, Error **errp)
> {
> -#ifndef CONFIG_USER_ONLY
> CPUClass *cc = CPU_GET_CLASS(cpu);
> -#endif
>
> cpu_list_add(cpu);
> if (!accel_cpu_realizefn(cpu, errp)) {
> return;
> }
> -#ifdef CONFIG_TCG
> +
> /* NB: errp parameter is unused currently */
> - if (tcg_enabled()) {
> - tcg_exec_realizefn(cpu, errp);
> + if (cc->tcg_ops) {
> + cc->tcg_ops->exec_realizefn(cpu, errp);
> }
> -#endif /* CONFIG_TCG */
>
> #ifdef CONFIG_USER_ONLY
> assert(qdev_get_vmsd(DEVICE(cpu)) == NULL ||
> @@ -159,9 +160,9 @@ void cpu_exec_realizefn(CPUState *cpu, Error **errp)
>
> void cpu_exec_unrealizefn(CPUState *cpu)
> {
> -#ifndef CONFIG_USER_ONLY
> CPUClass *cc = CPU_GET_CLASS(cpu);
>
> +#ifndef CONFIG_USER_ONLY
> if (cc->sysemu_ops->legacy_vmsd != NULL) {
> vmstate_unregister(NULL, cc->sysemu_ops->legacy_vmsd, cpu);
> }
> @@ -169,12 +170,10 @@ void cpu_exec_unrealizefn(CPUState *cpu)
> vmstate_unregister(NULL, &vmstate_cpu_common, cpu);
> }
> #endif
> -#ifdef CONFIG_TCG
> - /* NB: errp parameter is unused currently */
> - if (tcg_enabled()) {
> - tcg_exec_unrealizefn(cpu);
> +
> + if (cc->tcg_ops) {
> + cc->tcg_ops->exec_unrealizefn(cpu);
> }
> -#endif /* CONFIG_TCG */
>
> cpu_list_remove(cpu);
> }
> diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
> index 4871ad0c0a6a..715fb97d6707 100644
> --- a/target/alpha/cpu.c
> +++ b/target/alpha/cpu.c
> @@ -226,6 +226,7 @@ static const struct TCGCPUOps alpha_tcg_ops = {
> .do_transaction_failed = alpha_cpu_do_transaction_failed,
> .do_unaligned_access = alpha_cpu_do_unaligned_access,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
>
> static void alpha_cpu_class_init(ObjectClass *oc, void *data)
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 2866dd765882..7f051bd24e0b 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -1996,6 +1996,7 @@ static const struct TCGCPUOps arm_tcg_ops = {
> .debug_check_watchpoint = arm_debug_check_watchpoint,
> .debug_check_breakpoint = arm_debug_check_breakpoint,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
> #endif /* CONFIG_TCG */
>
> diff --git a/target/arm/cpu_tcg.c b/target/arm/cpu_tcg.c
> index ed444bf436a4..0f8657ede860 100644
> --- a/target/arm/cpu_tcg.c
> +++ b/target/arm/cpu_tcg.c
> @@ -913,6 +913,7 @@ static const struct TCGCPUOps arm_v7m_tcg_ops = {
> .debug_check_watchpoint = arm_debug_check_watchpoint,
> .debug_check_breakpoint = arm_debug_check_breakpoint,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
> #endif /* CONFIG_TCG */
>
> diff --git a/target/avr/cpu.c b/target/avr/cpu.c
> index ea14175ca557..8b3261ef832c 100644
> --- a/target/avr/cpu.c
> +++ b/target/avr/cpu.c
> @@ -201,6 +201,7 @@ static const struct TCGCPUOps avr_tcg_ops = {
> #ifndef CONFIG_USER_ONLY
> .do_interrupt = avr_cpu_do_interrupt,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
>
> static void avr_cpu_class_init(ObjectClass *oc, void *data)
> diff --git a/target/cris/cpu.c b/target/cris/cpu.c
> index 70932b1f8c7d..32ca1051f050 100644
> --- a/target/cris/cpu.c
> +++ b/target/cris/cpu.c
> @@ -221,6 +221,7 @@ static const struct TCGCPUOps crisv32_tcg_ops = {
> #ifndef CONFIG_USER_ONLY
> .do_interrupt = cris_cpu_do_interrupt,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
>
> static void crisv8_cpu_class_init(ObjectClass *oc, void *data)
> diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c
> index 3338365c16ec..86c001b67484 100644
> --- a/target/hexagon/cpu.c
> +++ b/target/hexagon/cpu.c
> @@ -273,6 +273,7 @@ static const struct TCGCPUOps hexagon_tcg_ops = {
> .initialize = hexagon_translate_init,
> .synchronize_from_tb = hexagon_cpu_synchronize_from_tb,
> .tlb_fill = hexagon_tlb_fill,
> + TCG_CPU_OPS_COMMON,
> };
>
> static void hexagon_cpu_class_init(ObjectClass *c, void *data)
> diff --git a/target/hppa/cpu.c b/target/hppa/cpu.c
> index 2eace4ee1240..5163118e986d 100644
> --- a/target/hppa/cpu.c
> +++ b/target/hppa/cpu.c
> @@ -151,6 +151,7 @@ static const struct TCGCPUOps hppa_tcg_ops = {
> .do_interrupt = hppa_cpu_do_interrupt,
> .do_unaligned_access = hppa_cpu_do_unaligned_access,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
>
> static void hppa_cpu_class_init(ObjectClass *oc, void *data)
> diff --git a/target/i386/tcg/tcg-cpu.c b/target/i386/tcg/tcg-cpu.c
> index 93a79a574154..bf7d91fed6da 100644
> --- a/target/i386/tcg/tcg-cpu.c
> +++ b/target/i386/tcg/tcg-cpu.c
> @@ -79,6 +79,7 @@ static const struct TCGCPUOps x86_tcg_ops = {
> .debug_excp_handler = breakpoint_handler,
> .debug_check_breakpoint = x86_debug_check_breakpoint,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
>
> static void tcg_cpu_init_ops(AccelCPUClass *accel_cpu, CPUClass *cc)
> diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c
> index 72de6e972627..df06c07907d6 100644
> --- a/target/m68k/cpu.c
> +++ b/target/m68k/cpu.c
> @@ -522,6 +522,7 @@ static const struct TCGCPUOps m68k_tcg_ops = {
> .do_interrupt = m68k_cpu_do_interrupt,
> .do_transaction_failed = m68k_cpu_transaction_failed,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
>
> static void m68k_cpu_class_init(ObjectClass *c, void *data)
> diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
> index 72d8f2a0daa6..0e781264ef7b 100644
> --- a/target/microblaze/cpu.c
> +++ b/target/microblaze/cpu.c
> @@ -373,6 +373,7 @@ static const struct TCGCPUOps mb_tcg_ops = {
> .do_transaction_failed = mb_cpu_transaction_failed,
> .do_unaligned_access = mb_cpu_do_unaligned_access,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
>
> static void mb_cpu_class_init(ObjectClass *oc, void *data)
> diff --git a/target/mips/cpu.c b/target/mips/cpu.c
> index d426918291a9..3216a33d0047 100644
> --- a/target/mips/cpu.c
> +++ b/target/mips/cpu.c
> @@ -548,6 +548,7 @@ static const struct TCGCPUOps mips_tcg_ops = {
> .do_unaligned_access = mips_cpu_do_unaligned_access,
> .io_recompile_replay_branch = mips_io_recompile_replay_branch,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
> #endif /* CONFIG_TCG */
>
> diff --git a/target/nios2/cpu.c b/target/nios2/cpu.c
> index 5e37defef805..55e21e8cd414 100644
> --- a/target/nios2/cpu.c
> +++ b/target/nios2/cpu.c
> @@ -226,6 +226,7 @@ static const struct TCGCPUOps nios2_tcg_ops = {
> .do_interrupt = nios2_cpu_do_interrupt,
> .do_unaligned_access = nios2_cpu_do_unaligned_access,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
>
> static void nios2_cpu_class_init(ObjectClass *oc, void *data)
> diff --git a/target/openrisc/cpu.c b/target/openrisc/cpu.c
> index bd34e429ecbf..0d9e1aa58a8f 100644
> --- a/target/openrisc/cpu.c
> +++ b/target/openrisc/cpu.c
> @@ -192,6 +192,7 @@ static const struct TCGCPUOps openrisc_tcg_ops = {
> #ifndef CONFIG_USER_ONLY
> .do_interrupt = openrisc_cpu_do_interrupt,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
>
> static void openrisc_cpu_class_init(ObjectClass *oc, void *data)
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index 505a0ed6ac09..3f8bf2e3fcbf 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -9021,6 +9021,7 @@ static const struct TCGCPUOps ppc_tcg_ops = {
> .cpu_exec_exit = ppc_cpu_exec_exit,
> .do_unaligned_access = ppc_cpu_do_unaligned_access,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
> #endif /* CONFIG_TCG */
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 991a6bb7604f..6a129b4fe211 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -650,6 +650,7 @@ static const struct TCGCPUOps riscv_tcg_ops = {
> .do_transaction_failed = riscv_cpu_do_transaction_failed,
> .do_unaligned_access = riscv_cpu_do_unaligned_access,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
>
> static void riscv_cpu_class_init(ObjectClass *c, void *data)
> diff --git a/target/rx/cpu.c b/target/rx/cpu.c
> index 96cc96e514fe..7a1ebc694f22 100644
> --- a/target/rx/cpu.c
> +++ b/target/rx/cpu.c
> @@ -192,6 +192,7 @@ static const struct TCGCPUOps rx_tcg_ops = {
> #ifndef CONFIG_USER_ONLY
> .do_interrupt = rx_cpu_do_interrupt,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
>
> static void rx_cpu_class_init(ObjectClass *klass, void *data)
> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
> index 7b7b05f1d3a0..8dfcde1590c0 100644
> --- a/target/s390x/cpu.c
> +++ b/target/s390x/cpu.c
> @@ -274,6 +274,7 @@ static const struct TCGCPUOps s390_tcg_ops = {
> .debug_excp_handler = s390x_cpu_debug_excp_handler,
> .do_unaligned_access = s390x_cpu_do_unaligned_access,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
> #endif /* CONFIG_TCG */
>
> diff --git a/target/sh4/cpu.c b/target/sh4/cpu.c
> index 832692294211..0b4e03913656 100644
> --- a/target/sh4/cpu.c
> +++ b/target/sh4/cpu.c
> @@ -244,6 +244,7 @@ static const struct TCGCPUOps superh_tcg_ops = {
> .do_unaligned_access = superh_cpu_do_unaligned_access,
> .io_recompile_replay_branch = superh_io_recompile_replay_branch,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
>
> static void superh_cpu_class_init(ObjectClass *oc, void *data)
> diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
> index da6b30ec747a..2b2d2946223b 100644
> --- a/target/sparc/cpu.c
> +++ b/target/sparc/cpu.c
> @@ -871,6 +871,7 @@ static const struct TCGCPUOps sparc_tcg_ops = {
> .do_transaction_failed = sparc_cpu_do_transaction_failed,
> .do_unaligned_access = sparc_cpu_do_unaligned_access,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
> #endif /* CONFIG_TCG */
>
> diff --git a/target/tricore/cpu.c b/target/tricore/cpu.c
> index b95682b7f04d..daeaecca26e0 100644
> --- a/target/tricore/cpu.c
> +++ b/target/tricore/cpu.c
> @@ -154,6 +154,7 @@ static const struct TCGCPUOps tricore_tcg_ops = {
> .initialize = tricore_tcg_init,
> .synchronize_from_tb = tricore_cpu_synchronize_from_tb,
> .tlb_fill = tricore_cpu_tlb_fill,
> + TCG_CPU_OPS_COMMON,
> };
>
> static void tricore_cpu_class_init(ObjectClass *c, void *data)
> diff --git a/target/xtensa/cpu.c b/target/xtensa/cpu.c
> index 58ec3a086224..d7d1b967040b 100644
> --- a/target/xtensa/cpu.c
> +++ b/target/xtensa/cpu.c
> @@ -201,6 +201,7 @@ static const struct TCGCPUOps xtensa_tcg_ops = {
> .do_transaction_failed = xtensa_cpu_do_transaction_failed,
> .do_unaligned_access = xtensa_cpu_do_unaligned_access,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
>
> static void xtensa_cpu_class_init(ObjectClass *oc, void *data)
>
Hi,
> > It's basically two groups:
> >
> > * Arch-specific (functions taking CPUX86State as argument), most of the
> > unresolved symbols in target/i386/ and i386/ directories go into this
> > category.
>
> Yes, and we need to think about all targets, not just i386.
Sure. I just want go forward in small steps, so my plan is to tackle
them one by one (starting with i386).
> > * Common (functions taking CPUState as argument). Everything else.
> >
> > The common functions could be added TCGCPUOps to allow them being called via
>
> TCGCCPUOps are target-specific in their implementation, so I guess
> it's the arch specific part that could be TCGCPUOps (maybe, would need
> deep thinking).
Ok, lets make it three groups then.
(1) generic interface, arch implementation (this is what we have
TCGCPUOps hooks right now).
(2) generic interface, generic implementation (functions taking a
CPUState as argument, simliar to group (1).
(3) arch-specific interface and implementation (functions taking a
CPUX86State argument).
We could add group (2) to TCGCPUOps for this ...
> > CPUClass->tcg_ops->$name instead of a direct symbol reference. Not sure this
> > is a good idea though. Experimental patch covering tcg_exec_realizefn +
> > tcg_exec_unrealizefn below.
... but as I sayed, not sure this is the best plan.
Adding group (3) to TCGCPUOps is a non-starter IMHO given that the
function prototypes are arch-specific (using CPUX86State) and also
the interfaces actually needed are arch-specific. something like
x86_register_ferr_irq or cpu_x86_update_dr7 simply doesn't exist on
!x86. I guess we'll need TCG${arch}Ops for those.
> > No idea yet how to handle arch-specific bits best. Seems there is no existing
> > infrastructure to untangle target-specific code and tcg, so this probably needs
> > something new.
>
> We need target-specific modules. They could at the beginning absorb
> also the non-target specific parts in my view. So you have a big
> tcg-arm module, a tcg-i386 module etc.
>
> I think I sketched already the idea in the Makefile I shared before?
We have target-specific modules in master branch.
Used for qtest (all archs) and tcg (i386/x86_64 only, accel ops only).
The build system changes to build more tcg bits modular are here:
https://git.kraxel.org/cgit/qemu/log/?h=sirius/modules-tcg-meson
Doesn't build due unresolved symbols, but shows which code
changes/cleanups/reorganizations are needed for (more) modular tcg.
take care,
Gerd
On 7/29/21 12:26 PM, Gerd Hoffmann wrote:
> Hi,
>
>>> It's basically two groups:
>>>
>>> * Arch-specific (functions taking CPUX86State as argument), most of the
>>> unresolved symbols in target/i386/ and i386/ directories go into this
>>> category.
>>
>> Yes, and we need to think about all targets, not just i386.
>
> Sure. I just want go forward in small steps, so my plan is to tackle
> them one by one (starting with i386).
>
>>> * Common (functions taking CPUState as argument). Everything else.
>>>
>>> The common functions could be added TCGCPUOps to allow them being called via
>>
>> TCGCCPUOps are target-specific in their implementation, so I guess
>> it's the arch specific part that could be TCGCPUOps (maybe, would need
>> deep thinking).
>
> Ok, lets make it three groups then.
>
> (1) generic interface, arch implementation (this is what we have
> TCGCPUOps hooks right now).
> (2) generic interface, generic implementation (functions taking a
> CPUState as argument, simliar to group (1).
> (3) arch-specific interface and implementation (functions taking a
> CPUX86State argument).
>
> We could add group (2) to TCGCPUOps for this ...
>
>>> CPUClass->tcg_ops->$name instead of a direct symbol reference. Not sure this
>>> is a good idea though. Experimental patch covering tcg_exec_realizefn +
>>> tcg_exec_unrealizefn below.
>
> ... but as I sayed, not sure this is the best plan.
>
> Adding group (3) to TCGCPUOps is a non-starter IMHO given that the
> function prototypes are arch-specific (using CPUX86State) and also
> the interfaces actually needed are arch-specific. something like
> x86_register_ferr_irq or cpu_x86_update_dr7 simply doesn't exist on
> !x86. I guess we'll need TCG${arch}Ops for those.
>
>>> No idea yet how to handle arch-specific bits best. Seems there is no existing
>>> infrastructure to untangle target-specific code and tcg, so this probably needs
>>> something new.
>>
>> We need target-specific modules. They could at the beginning absorb
>> also the non-target specific parts in my view. So you have a big
>> tcg-arm module, a tcg-i386 module etc.
>>
>> I think I sketched already the idea in the Makefile I shared before?
>
> We have target-specific modules in master branch.
> Used for qtest (all archs) and tcg (i386/x86_64 only, accel ops only).
>
> The build system changes to build more tcg bits modular are here:
> https://git.kraxel.org/cgit/qemu/log/?h=sirius/modules-tcg-meson
> Doesn't build due unresolved symbols, but shows which code
> changes/cleanups/reorganizations are needed for (more) modular tcg.
>
> take care,
> Gerd
>
What I mean is, for starters, lets make all tcg code land in the target-specific module.
Sorry I am multitasking quite a bit so I may be missing something obvious.
Ciao,
Claudio
On 7/29/21 11:14 AM, Gerd Hoffmann wrote:
> Hi,
>
>>> So we need more work to make this actually work right.
>>
>> Yes. I want have all of tcg in the tcg accel module, not only parts of
>> it, but that needs some more refactoring. I'll go start looking at this
>> once I managed to wade through my vacation backlog.
>
> So, changed the meson.build files to build tcg modular
> (https://git.kraxel.org/cgit/qemu/log/?h=sirius/modules-tcg-meson),
> then checked what unresolved symbols we have:
>
> # -*- symbols -*-
> 18 tlb_flush
> 8 tb_flush
> 6 update_fp_status
> 6 cpu_restore_state
> 4 x86_register_ferr_irq
> 4 update_mxcsr_status
> 4 update_mxcsr_from_sse_status
> 4 tlb_flush_page
> 4 curr_cflags
> 3 cpu_cc_compute_all
> 2 tlb_reset_dirty
> 2 tlb_plugin_lookup
> 2 tcg_flush_softmmu_tlb
> 2 tcg_exec_unrealizefn
> 2 tcg_exec_realizefn
> 2 tb_invalidate_phys_range
> 2 tb_invalidate_phys_page_range
> 2 tb_check_watchpoint
> 2 cpu_x86_update_dr7
> 2 cpu_set_ignne
> # -*- files -*-
> 22 softmmu/physmem.c tlb_flush tlb_flush_page curr_cflags tlb_reset_dirty tb_invalidate_phys_range tb_check_watchpoint cpu_restore_state
> 10 target/i386/helper.c tlb_flush cpu_restore_state
> 15 cpu.c cpu_cc_compute_all tlb_flush tcg_flush_softmmu_tlb tcg_exec_unrealizefn tcg_exec_realizefn tb_invalidate_phys_page_range tb_flush
> 9 target/i386/cpu.h update_fp_status cpu_cc_compute_all update_mxcsr_status
> 8 target/i386/machine.c update_mxcsr_status update_fp_status tlb_flush cpu_x86_update_dr7
> 2 target/i386/gdbstub.c update_mxcsr_from_sse_status
> 2 target/i386/cpu-dump.c update_mxcsr_from_sse_status
> 2 plugins/loader.c tb_flush
> 2 plugins/core.c tb_flush
> 2 plugins/api.c tlb_plugin_lookup
> 2 i386/pc_q35.c x86_register_ferr_irq
> 2 i386/pc_piix.c x86_register_ferr_irq
> 2 i386/pc.c cpu_set_ignne
> 4 gdbstub.c update_mxcsr_from_sse_status tb_flush
> 2 core/cpu-common.c tcg_flush_softmmu_tlb
> 2 accel/tcg/cpu-exec-common.c cpu_restore_state
>
> It's basically two groups:
>
> * Arch-specific (functions taking CPUX86State as argument), most of the
> unresolved symbols in target/i386/ and i386/ directories go into this
> category.
> * Common (functions taking CPUState as argument). Everything else.
>
> The common functions could be added TCGCPUOps to allow them being called via
> CPUClass->tcg_ops->$name instead of a direct symbol reference. Not sure this
> is a good idea though. Experimental patch covering tcg_exec_realizefn +
> tcg_exec_unrealizefn below.
>
> No idea yet how to handle arch-specific bits best. Seems there is no existing
> infrastructure to untangle target-specific code and tcg, so this probably needs
> something new.
>
> Noticed softmmu/physmem.c has lots of CONFIG_TCG #ifdefs, splitting this into
> softmmu/physmem-{common,tcg}.c is probably a good idea.
>
> Comments?
>
> take care,
> Gerd
>
> From c0e3f4cbe6aa6889d344beb4cac300b160253f00 Mon Sep 17 00:00:00 2001
> From: Gerd Hoffmann <kraxel@redhat.com>
> Date: Thu, 29 Jul 2021 09:14:56 +0200
> Subject: [PATCH] tcg: add tcg_exec_{realizefn,unrealizefn} to TCGCPUOps
>
> ---
> include/hw/core/tcg-cpu-ops.h | 7 +++++++
> cpu.c | 23 +++++++++++------------
> target/alpha/cpu.c | 1 +
> target/arm/cpu.c | 1 +
> target/arm/cpu_tcg.c | 1 +
> target/avr/cpu.c | 1 +
> target/cris/cpu.c | 1 +
> target/hexagon/cpu.c | 1 +
> target/hppa/cpu.c | 1 +
> target/i386/tcg/tcg-cpu.c | 1 +
> target/m68k/cpu.c | 1 +
> target/microblaze/cpu.c | 1 +
> target/mips/cpu.c | 1 +
> target/nios2/cpu.c | 1 +
> target/openrisc/cpu.c | 1 +
> target/ppc/cpu_init.c | 1 +
> target/riscv/cpu.c | 1 +
> target/rx/cpu.c | 1 +
> target/s390x/cpu.c | 1 +
> target/sh4/cpu.c | 1 +
> target/sparc/cpu.c | 1 +
> target/tricore/cpu.c | 1 +
> target/xtensa/cpu.c | 1 +
> 23 files changed, 39 insertions(+), 12 deletions(-)
>
> diff --git a/include/hw/core/tcg-cpu-ops.h b/include/hw/core/tcg-cpu-ops.h
> index eab27d0c0309..1f33953a4efc 100644
> --- a/include/hw/core/tcg-cpu-ops.h
> +++ b/include/hw/core/tcg-cpu-ops.h
> @@ -107,6 +107,13 @@ struct TCGCPUOps {
> #endif /* CONFIG_SOFTMMU */
> #endif /* NEED_CPU_H */
>
> + void (*exec_realizefn)(CPUState *cpu, Error **errp);
> + void (*exec_unrealizefn)(CPUState *cpu);
> };
>
> +#define TCG_CPU_OPS_COMMON \
> + .exec_realizefn = tcg_exec_realizefn, \
> + .exec_unrealizefn = tcg_exec_unrealizefn
> +
> +
> #endif /* TCG_CPU_OPS_H */
> diff --git a/cpu.c b/cpu.c
> index e1799a15bcf5..982463ce7418 100644
> --- a/cpu.c
> +++ b/cpu.c
> @@ -40,6 +40,10 @@
> #include "hw/core/accel-cpu.h"
> #include "trace/trace-root.h"
>
> +#ifdef CONFIG_TCG
> +#include "hw/core/tcg-cpu-ops.h"
> +#endif /* CONFIG_TCG */
> +
> uintptr_t qemu_host_page_size;
> intptr_t qemu_host_page_mask;
>
> @@ -129,20 +133,17 @@ const VMStateDescription vmstate_cpu_common = {
>
> void cpu_exec_realizefn(CPUState *cpu, Error **errp)
> {
> -#ifndef CONFIG_USER_ONLY
> CPUClass *cc = CPU_GET_CLASS(cpu);
> -#endif
>
> cpu_list_add(cpu);
> if (!accel_cpu_realizefn(cpu, errp)) {
> return;
> }
> -#ifdef CONFIG_TCG
> +
> /* NB: errp parameter is unused currently */
> - if (tcg_enabled()) {
> - tcg_exec_realizefn(cpu, errp);
> + if (cc->tcg_ops) {
> + cc->tcg_ops->exec_realizefn(cpu, errp);
> }
> -#endif /* CONFIG_TCG */
>
> #ifdef CONFIG_USER_ONLY
> assert(qdev_get_vmsd(DEVICE(cpu)) == NULL ||
> @@ -159,9 +160,9 @@ void cpu_exec_realizefn(CPUState *cpu, Error **errp)
>
> void cpu_exec_unrealizefn(CPUState *cpu)
> {
> -#ifndef CONFIG_USER_ONLY
> CPUClass *cc = CPU_GET_CLASS(cpu);
>
> +#ifndef CONFIG_USER_ONLY
> if (cc->sysemu_ops->legacy_vmsd != NULL) {
> vmstate_unregister(NULL, cc->sysemu_ops->legacy_vmsd, cpu);
> }
> @@ -169,12 +170,10 @@ void cpu_exec_unrealizefn(CPUState *cpu)
> vmstate_unregister(NULL, &vmstate_cpu_common, cpu);
> }
> #endif
> -#ifdef CONFIG_TCG
> - /* NB: errp parameter is unused currently */
> - if (tcg_enabled()) {
> - tcg_exec_unrealizefn(cpu);
> +
> + if (cc->tcg_ops) {
> + cc->tcg_ops->exec_unrealizefn(cpu);
> }
> -#endif /* CONFIG_TCG */
>
> cpu_list_remove(cpu);
> }
> diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
> index 4871ad0c0a6a..715fb97d6707 100644
> --- a/target/alpha/cpu.c
> +++ b/target/alpha/cpu.c
> @@ -226,6 +226,7 @@ static const struct TCGCPUOps alpha_tcg_ops = {
> .do_transaction_failed = alpha_cpu_do_transaction_failed,
> .do_unaligned_access = alpha_cpu_do_unaligned_access,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
>
> static void alpha_cpu_class_init(ObjectClass *oc, void *data)
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 2866dd765882..7f051bd24e0b 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -1996,6 +1996,7 @@ static const struct TCGCPUOps arm_tcg_ops = {
> .debug_check_watchpoint = arm_debug_check_watchpoint,
> .debug_check_breakpoint = arm_debug_check_breakpoint,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
> #endif /* CONFIG_TCG */
>
> diff --git a/target/arm/cpu_tcg.c b/target/arm/cpu_tcg.c
> index ed444bf436a4..0f8657ede860 100644
> --- a/target/arm/cpu_tcg.c
> +++ b/target/arm/cpu_tcg.c
> @@ -913,6 +913,7 @@ static const struct TCGCPUOps arm_v7m_tcg_ops = {
> .debug_check_watchpoint = arm_debug_check_watchpoint,
> .debug_check_breakpoint = arm_debug_check_breakpoint,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
> #endif /* CONFIG_TCG */
>
> diff --git a/target/avr/cpu.c b/target/avr/cpu.c
> index ea14175ca557..8b3261ef832c 100644
> --- a/target/avr/cpu.c
> +++ b/target/avr/cpu.c
> @@ -201,6 +201,7 @@ static const struct TCGCPUOps avr_tcg_ops = {
> #ifndef CONFIG_USER_ONLY
> .do_interrupt = avr_cpu_do_interrupt,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
>
> static void avr_cpu_class_init(ObjectClass *oc, void *data)
> diff --git a/target/cris/cpu.c b/target/cris/cpu.c
> index 70932b1f8c7d..32ca1051f050 100644
> --- a/target/cris/cpu.c
> +++ b/target/cris/cpu.c
> @@ -221,6 +221,7 @@ static const struct TCGCPUOps crisv32_tcg_ops = {
> #ifndef CONFIG_USER_ONLY
> .do_interrupt = cris_cpu_do_interrupt,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
>
> static void crisv8_cpu_class_init(ObjectClass *oc, void *data)
> diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c
> index 3338365c16ec..86c001b67484 100644
> --- a/target/hexagon/cpu.c
> +++ b/target/hexagon/cpu.c
> @@ -273,6 +273,7 @@ static const struct TCGCPUOps hexagon_tcg_ops = {
> .initialize = hexagon_translate_init,
> .synchronize_from_tb = hexagon_cpu_synchronize_from_tb,
> .tlb_fill = hexagon_tlb_fill,
> + TCG_CPU_OPS_COMMON,
> };
>
> static void hexagon_cpu_class_init(ObjectClass *c, void *data)
> diff --git a/target/hppa/cpu.c b/target/hppa/cpu.c
> index 2eace4ee1240..5163118e986d 100644
> --- a/target/hppa/cpu.c
> +++ b/target/hppa/cpu.c
> @@ -151,6 +151,7 @@ static const struct TCGCPUOps hppa_tcg_ops = {
> .do_interrupt = hppa_cpu_do_interrupt,
> .do_unaligned_access = hppa_cpu_do_unaligned_access,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
>
> static void hppa_cpu_class_init(ObjectClass *oc, void *data)
> diff --git a/target/i386/tcg/tcg-cpu.c b/target/i386/tcg/tcg-cpu.c
> index 93a79a574154..bf7d91fed6da 100644
> --- a/target/i386/tcg/tcg-cpu.c
> +++ b/target/i386/tcg/tcg-cpu.c
> @@ -79,6 +79,7 @@ static const struct TCGCPUOps x86_tcg_ops = {
> .debug_excp_handler = breakpoint_handler,
> .debug_check_breakpoint = x86_debug_check_breakpoint,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
>
> static void tcg_cpu_init_ops(AccelCPUClass *accel_cpu, CPUClass *cc)
> diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c
> index 72de6e972627..df06c07907d6 100644
> --- a/target/m68k/cpu.c
> +++ b/target/m68k/cpu.c
> @@ -522,6 +522,7 @@ static const struct TCGCPUOps m68k_tcg_ops = {
> .do_interrupt = m68k_cpu_do_interrupt,
> .do_transaction_failed = m68k_cpu_transaction_failed,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
>
> static void m68k_cpu_class_init(ObjectClass *c, void *data)
> diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
> index 72d8f2a0daa6..0e781264ef7b 100644
> --- a/target/microblaze/cpu.c
> +++ b/target/microblaze/cpu.c
> @@ -373,6 +373,7 @@ static const struct TCGCPUOps mb_tcg_ops = {
> .do_transaction_failed = mb_cpu_transaction_failed,
> .do_unaligned_access = mb_cpu_do_unaligned_access,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
>
> static void mb_cpu_class_init(ObjectClass *oc, void *data)
> diff --git a/target/mips/cpu.c b/target/mips/cpu.c
> index d426918291a9..3216a33d0047 100644
> --- a/target/mips/cpu.c
> +++ b/target/mips/cpu.c
> @@ -548,6 +548,7 @@ static const struct TCGCPUOps mips_tcg_ops = {
> .do_unaligned_access = mips_cpu_do_unaligned_access,
> .io_recompile_replay_branch = mips_io_recompile_replay_branch,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
> #endif /* CONFIG_TCG */
>
> diff --git a/target/nios2/cpu.c b/target/nios2/cpu.c
> index 5e37defef805..55e21e8cd414 100644
> --- a/target/nios2/cpu.c
> +++ b/target/nios2/cpu.c
> @@ -226,6 +226,7 @@ static const struct TCGCPUOps nios2_tcg_ops = {
> .do_interrupt = nios2_cpu_do_interrupt,
> .do_unaligned_access = nios2_cpu_do_unaligned_access,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
>
> static void nios2_cpu_class_init(ObjectClass *oc, void *data)
> diff --git a/target/openrisc/cpu.c b/target/openrisc/cpu.c
> index bd34e429ecbf..0d9e1aa58a8f 100644
> --- a/target/openrisc/cpu.c
> +++ b/target/openrisc/cpu.c
> @@ -192,6 +192,7 @@ static const struct TCGCPUOps openrisc_tcg_ops = {
> #ifndef CONFIG_USER_ONLY
> .do_interrupt = openrisc_cpu_do_interrupt,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
>
> static void openrisc_cpu_class_init(ObjectClass *oc, void *data)
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index 505a0ed6ac09..3f8bf2e3fcbf 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -9021,6 +9021,7 @@ static const struct TCGCPUOps ppc_tcg_ops = {
> .cpu_exec_exit = ppc_cpu_exec_exit,
> .do_unaligned_access = ppc_cpu_do_unaligned_access,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
> #endif /* CONFIG_TCG */
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 991a6bb7604f..6a129b4fe211 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -650,6 +650,7 @@ static const struct TCGCPUOps riscv_tcg_ops = {
> .do_transaction_failed = riscv_cpu_do_transaction_failed,
> .do_unaligned_access = riscv_cpu_do_unaligned_access,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
>
> static void riscv_cpu_class_init(ObjectClass *c, void *data)
> diff --git a/target/rx/cpu.c b/target/rx/cpu.c
> index 96cc96e514fe..7a1ebc694f22 100644
> --- a/target/rx/cpu.c
> +++ b/target/rx/cpu.c
> @@ -192,6 +192,7 @@ static const struct TCGCPUOps rx_tcg_ops = {
> #ifndef CONFIG_USER_ONLY
> .do_interrupt = rx_cpu_do_interrupt,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
>
> static void rx_cpu_class_init(ObjectClass *klass, void *data)
> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
> index 7b7b05f1d3a0..8dfcde1590c0 100644
> --- a/target/s390x/cpu.c
> +++ b/target/s390x/cpu.c
> @@ -274,6 +274,7 @@ static const struct TCGCPUOps s390_tcg_ops = {
> .debug_excp_handler = s390x_cpu_debug_excp_handler,
> .do_unaligned_access = s390x_cpu_do_unaligned_access,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
> #endif /* CONFIG_TCG */
>
> diff --git a/target/sh4/cpu.c b/target/sh4/cpu.c
> index 832692294211..0b4e03913656 100644
> --- a/target/sh4/cpu.c
> +++ b/target/sh4/cpu.c
> @@ -244,6 +244,7 @@ static const struct TCGCPUOps superh_tcg_ops = {
> .do_unaligned_access = superh_cpu_do_unaligned_access,
> .io_recompile_replay_branch = superh_io_recompile_replay_branch,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
>
> static void superh_cpu_class_init(ObjectClass *oc, void *data)
> diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
> index da6b30ec747a..2b2d2946223b 100644
> --- a/target/sparc/cpu.c
> +++ b/target/sparc/cpu.c
> @@ -871,6 +871,7 @@ static const struct TCGCPUOps sparc_tcg_ops = {
> .do_transaction_failed = sparc_cpu_do_transaction_failed,
> .do_unaligned_access = sparc_cpu_do_unaligned_access,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
> #endif /* CONFIG_TCG */
>
> diff --git a/target/tricore/cpu.c b/target/tricore/cpu.c
> index b95682b7f04d..daeaecca26e0 100644
> --- a/target/tricore/cpu.c
> +++ b/target/tricore/cpu.c
> @@ -154,6 +154,7 @@ static const struct TCGCPUOps tricore_tcg_ops = {
> .initialize = tricore_tcg_init,
> .synchronize_from_tb = tricore_cpu_synchronize_from_tb,
> .tlb_fill = tricore_cpu_tlb_fill,
> + TCG_CPU_OPS_COMMON,
> };
>
> static void tricore_cpu_class_init(ObjectClass *c, void *data)
> diff --git a/target/xtensa/cpu.c b/target/xtensa/cpu.c
> index 58ec3a086224..d7d1b967040b 100644
> --- a/target/xtensa/cpu.c
> +++ b/target/xtensa/cpu.c
> @@ -201,6 +201,7 @@ static const struct TCGCPUOps xtensa_tcg_ops = {
> .do_transaction_failed = xtensa_cpu_do_transaction_failed,
> .do_unaligned_access = xtensa_cpu_do_unaligned_access,
> #endif /* !CONFIG_USER_ONLY */
> + TCG_CPU_OPS_COMMON,
> };
>
> static void xtensa_cpu_class_init(ObjectClass *oc, void *data)
>
And another comment: I think we should have some progress on ARM with the kvm/tcg split and with the KConfig of boards,
before we continue here.
Ciao,
C
Hi, > And another comment: I think we should have some progress on ARM with > the kvm/tcg split and with the KConfig of boards, before we continue > here. Why? This can easily be tacked in parallel. We can flip the switch for modular tcg per target in meson.build. take care, Gerd
On 7/29/21 12:29 PM, Gerd Hoffmann wrote: > Hi, > >> And another comment: I think we should have some progress on ARM with >> the kvm/tcg split and with the KConfig of boards, before we continue >> here. > > Why? This can easily be tacked in parallel. We can flip the switch > for modular tcg per target in meson.build. > > take care, > Gerd > Because in the end we need to do this for ARM too and for the other archs too (s390 is already ok), and in order to be sure not to end up in a dead-end, I think it would be good to have at least a sketch for the other archs as well.. Just my 2c ofc, I think really here still ARM is behind, and we should help it catch up. If I had more time I would have pushed more on the ARM series, but.. yeah. Ciao, Claudio
On 7/29/21 12:44 PM, Claudio Fontana wrote: > On 7/29/21 12:29 PM, Gerd Hoffmann wrote: >> Hi, >> >>> And another comment: I think we should have some progress on ARM with >>> the kvm/tcg split and with the KConfig of boards, before we continue >>> here. >> >> Why? This can easily be tacked in parallel. We can flip the switch >> for modular tcg per target in meson.build. >> >> take care, >> Gerd >> > > Because in the end we need to do this for ARM too and for the other archs too (s390 is already ok), > > and in order to be sure not to end up in a dead-end, I think it would be good to have at least a sketch for the other archs as well.. > > Just my 2c ofc, I think really here still ARM is behind, and we should help it catch up. > > If I had more time I would have pushed more on the ARM series, but.. yeah. IIUC Alex is waiting 6.2 release to respin.
On 7/29/21 1:34 PM, Philippe Mathieu-Daudé wrote: > On 7/29/21 12:44 PM, Claudio Fontana wrote: >> On 7/29/21 12:29 PM, Gerd Hoffmann wrote: >>> Hi, >>> >>>> And another comment: I think we should have some progress on ARM with >>>> the kvm/tcg split and with the KConfig of boards, before we continue >>>> here. >>> >>> Why? This can easily be tacked in parallel. We can flip the switch >>> for modular tcg per target in meson.build. >>> >>> take care, >>> Gerd >>> >> >> Because in the end we need to do this for ARM too and for the other archs too (s390 is already ok), >> >> and in order to be sure not to end up in a dead-end, I think it would be good to have at least a sketch for the other archs as well.. >> >> Just my 2c ofc, I think really here still ARM is behind, and we should help it catch up. >> >> If I had more time I would have pushed more on the ARM series, but.. yeah. > > IIUC Alex is waiting 6.2 release to respin. > Ah that's great to know (and thanks again for picking it up). CLaudio
On 7/29/21 1:34 PM, Philippe Mathieu-Daudé wrote: > On 7/29/21 12:44 PM, Claudio Fontana wrote: >> On 7/29/21 12:29 PM, Gerd Hoffmann wrote: >>> Hi, >>> >>>> And another comment: I think we should have some progress on ARM with >>>> the kvm/tcg split and with the KConfig of boards, before we continue >>>> here. >>> >>> Why? This can easily be tacked in parallel. We can flip the switch >>> for modular tcg per target in meson.build. >>> >>> take care, >>> Gerd >>> >> >> Because in the end we need to do this for ARM too and for the other archs too (s390 is already ok), >> >> and in order to be sure not to end up in a dead-end, I think it would be good to have at least a sketch for the other archs as well.. >> >> Just my 2c ofc, I think really here still ARM is behind, and we should help it catch up. >> >> If I had more time I would have pushed more on the ARM series, but.. yeah. > > IIUC Alex is waiting 6.2 release to respin. > How does the Kconfig for ARM improvements go? I mean I think those improvements (enabling only compatible boards with the chosen accelerators) are important for both tcg-kvm split and possibly for modularization of ARM accelerators too right? Ciao, Claudio
On 7/29/21 4:22 PM, Claudio Fontana wrote: > On 7/29/21 1:34 PM, Philippe Mathieu-Daudé wrote: >> On 7/29/21 12:44 PM, Claudio Fontana wrote: >>> On 7/29/21 12:29 PM, Gerd Hoffmann wrote: >>>> Hi, >>>> >>>>> And another comment: I think we should have some progress on ARM with >>>>> the kvm/tcg split and with the KConfig of boards, before we continue >>>>> here. >>>> >>>> Why? This can easily be tacked in parallel. We can flip the switch >>>> for modular tcg per target in meson.build. >>>> >>>> take care, >>>> Gerd >>>> >>> >>> Because in the end we need to do this for ARM too and for the other archs too (s390 is already ok), >>> >>> and in order to be sure not to end up in a dead-end, I think it would be good to have at least a sketch for the other archs as well.. >>> >>> Just my 2c ofc, I think really here still ARM is behind, and we should help it catch up. >>> >>> If I had more time I would have pushed more on the ARM series, but.. yeah. >> >> IIUC Alex is waiting 6.2 release to respin. >> > > How does the Kconfig for ARM improvements go? I mean I think those improvements (enabling only compatible boards with the chosen accelerators) are important for both tcg-kvm split and possibly for modularization of ARM accelerators too right? I think we all (Alex/you/me) reached the same point where builds work but current the testing framework isn't ready for non-TCG or modularized-TCG so the CI ends failing. I don't want to push for 'CI build-only' because most of the annoying problems were from runtime (interfaces not resolved, ... which are important when using modules or board with unavailable devices). I tried to address that with a QMP command to query accelerators but there is a disagreement whether we should query for available/built-in/ loaded/modularized-but-not-installed/...). At this point I think I fairly understand the technical problems but misunderstand the big picture here, in particular w.r.t. management apps. I spent too many time on this to appear enthusiastic, sorry.
On 7/29/21 4:59 PM, Philippe Mathieu-Daudé wrote: > On 7/29/21 4:22 PM, Claudio Fontana wrote: >> On 7/29/21 1:34 PM, Philippe Mathieu-Daudé wrote: >>> On 7/29/21 12:44 PM, Claudio Fontana wrote: >>>> On 7/29/21 12:29 PM, Gerd Hoffmann wrote: >>>>> Hi, >>>>> >>>>>> And another comment: I think we should have some progress on ARM with >>>>>> the kvm/tcg split and with the KConfig of boards, before we continue >>>>>> here. >>>>> >>>>> Why? This can easily be tacked in parallel. We can flip the switch >>>>> for modular tcg per target in meson.build. >>>>> >>>>> take care, >>>>> Gerd >>>>> >>>> >>>> Because in the end we need to do this for ARM too and for the other archs too (s390 is already ok), >>>> >>>> and in order to be sure not to end up in a dead-end, I think it would be good to have at least a sketch for the other archs as well.. >>>> >>>> Just my 2c ofc, I think really here still ARM is behind, and we should help it catch up. >>>> >>>> If I had more time I would have pushed more on the ARM series, but.. yeah. >>> >>> IIUC Alex is waiting 6.2 release to respin. >>> >> >> How does the Kconfig for ARM improvements go? I mean I think those improvements (enabling only compatible boards with the chosen accelerators) are important for both tcg-kvm split and possibly for modularization of ARM accelerators too right? > > I think we all (Alex/you/me) reached the same point where builds work > but current the testing framework isn't ready for non-TCG or > modularized-TCG so the CI ends failing. > > I don't want to push for 'CI build-only' because most of the annoying > problems were from runtime (interfaces not resolved, ... which are > important when using modules or board with unavailable devices). > > I tried to address that with a QMP command to query accelerators but > there is a disagreement whether we should query for available/built-in/ > loaded/modularized-but-not-installed/...). At this point I think I > fairly understand the technical problems but misunderstand the big > picture here, in particular w.r.t. management apps. I spent too many > time on this to appear enthusiastic, sorry. > The Kconfig thing also depended on the querying the accelerator part, or not? I mean, was there the idea was "I want to build only the ARM boards that are compatible with the accel options I chose", which is totally sensible, and removes all the need for workarounds in the tcg/kvm split series Alex is now maintaining. I think you actually had a solution for quering the accelerators btw, and the problem was just caching the result to improve the performance? Thanks, Claudio
On 29/07/21 11:14, Gerd Hoffmann wrote:
> The common functions could be added TCGCPUOps to allow them being called via
> CPUClass->tcg_ops->$name instead of a direct symbol reference. Not sure this
> is a good idea though. Experimental patch covering tcg_exec_realizefn +
> tcg_exec_unrealizefn below.
A lot of these (though probably not all) are already stubbed out as
"static inline" in include/exec/exec-all.h. I think they can be changed
to function pointers in the style of ui/spice-module.c
(accel/tcg/tcg-module.c?).
> No idea yet how to handle arch-specific bits best. Seems there is no existing
> infrastructure to untangle target-specific code and tcg, so this probably needs
> something new.
If they are few enough, I would just move them out of target/i386/tcg
and into target/i386/cpu-sysemu.c.
> Noticed softmmu/physmem.c has lots of CONFIG_TCG #ifdefs, splitting this into
> softmmu/physmem-{common,tcg}.c is probably a good idea.
I only count one, and those function should be easily moved to
accel/tcg/cputlb.c (after all both physmem.c and cputlb.c used to be a
single file, exec.c, so this is just an oversight).
Paolo
Hi,
On Thu, Jul 29, 2021 at 06:40:17PM +0200, Paolo Bonzini wrote:
> On 29/07/21 11:14, Gerd Hoffmann wrote:
> > The common functions could be added TCGCPUOps to allow them being called via
> > CPUClass->tcg_ops->$name instead of a direct symbol reference. Not sure this
> > is a good idea though. Experimental patch covering tcg_exec_realizefn +
> > tcg_exec_unrealizefn below.
>
> A lot of these (though probably not all) are already stubbed out as "static
> inline" in include/exec/exec-all.h. I think they can be changed to function
> pointers in the style of ui/spice-module.c (accel/tcg/tcg-module.c?).
Yep, was thinking about that too. But then I noticed we already have
TCGCPUOps and wondered whenever extending that would be the better
option.
> > No idea yet how to handle arch-specific bits best. Seems there is no existing
> > infrastructure to untangle target-specific code and tcg, so this probably needs
> > something new.
>
> If they are few enough, I would just move them out of target/i386/tcg and
> into target/i386/cpu-sysemu.c.
I'll have a look.
> > Noticed softmmu/physmem.c has lots of CONFIG_TCG #ifdefs, splitting this into
> > softmmu/physmem-{common,tcg}.c is probably a good idea.
>
> I only count one, and those function should be easily moved to
> accel/tcg/cputlb.c (after all both physmem.c and cputlb.c used to be a
> single file, exec.c, so this is just an oversight).
Well, I noticed one larger block covering multiple functions, didn't
check the whole file ...
thanks & take care,
Gerd
On 7/30/21 11:05 AM, Gerd Hoffmann wrote:
> Hi,
>
> On Thu, Jul 29, 2021 at 06:40:17PM +0200, Paolo Bonzini wrote:
>> On 29/07/21 11:14, Gerd Hoffmann wrote:
>>> The common functions could be added TCGCPUOps to allow them being called via
>>> CPUClass->tcg_ops->$name instead of a direct symbol reference. Not sure this
>>> is a good idea though. Experimental patch covering tcg_exec_realizefn +
>>> tcg_exec_unrealizefn below.
>>
>> A lot of these (though probably not all) are already stubbed out as "static
>> inline" in include/exec/exec-all.h. I think they can be changed to function
>> pointers in the style of ui/spice-module.c (accel/tcg/tcg-module.c?).
>
> Yep, was thinking about that too. But then I noticed we already have
> TCGCPUOps and wondered whenever extending that would be the better
> option.
I'd read TCGCPUOps as TCG CPU-specific Operations in this case,
IIRC they are for the TCG Operations that have cpu target-specific behavior.
Maybe they should be called TCGCPUTargetOps to avoid confusion?
So, a TCGCPUOps will have an arm implementation, x86 implementation and so on.
In my view tcg_exec_realizefn does not fit, as this is just a set of additional
generic non-target-specific operations for the generic cpu_exec_realizefn() that needs to be called specifically and only for TCG.
>
>>> No idea yet how to handle arch-specific bits best. Seems there is no existing
>>> infrastructure to untangle target-specific code and tcg, so this probably needs
>>> something new.
>>
>> If they are few enough, I would just move them out of target/i386/tcg and
>> into target/i386/cpu-sysemu.c.
>
> I'll have a look.
>
>>> Noticed softmmu/physmem.c has lots of CONFIG_TCG #ifdefs, splitting this into
>>> softmmu/physmem-{common,tcg}.c is probably a good idea.
>>
>> I only count one, and those function should be easily moved to
>> accel/tcg/cputlb.c (after all both physmem.c and cputlb.c used to be a
>> single file, exec.c, so this is just an oversight).
>
> Well, I noticed one larger block covering multiple functions, didn't
> check the whole file ...
>
> thanks & take care,
> Gerd
>
© 2016 - 2024 Red Hat, Inc.