[PATCH v2 01/19] target/riscv: introduce TCG AccelCPUClass

Daniel Henrique Barboza posted 19 patches 1 year, 2 months ago
There is a newer version of this series
[PATCH v2 01/19] target/riscv: introduce TCG AccelCPUClass
Posted by Daniel Henrique Barboza 1 year, 2 months ago
target/riscv/cpu.c needs to handle all possible accelerators (TCG and
KVM at this moment) during both init() and realize() time. This forces
us to resort to a lot of "if tcg" and "if kvm" throughout the code,
which isn't wrong, but can get cluttered over time. Splitting
acceleration specific code from cpu.c to its own file will help to
declutter the existing code and it will also make it easier to support
KVM/TCG only builds in the future.

We'll start by adding a new subdir called 'tcg' and a new file called
'tcg-cpu.c'. This file will be used to introduce a new accelerator class
for TCG acceleration in RISC-V, allowing us to center all TCG exclusive
code in its file instead of using 'cpu.c' for everything. This design is
inpired by the work Claudio Fontana did in x86 a few years ago in commit
f5cc5a5c1 ("i386: split cpu accelerators from cpu.c, using
AccelCPUClass").

To avoid moving too much code at once we'll start by adding the new file
and TCG AccelCPUClass declaration. The 'class_init' from the accel class
will init 'tcg_ops', relieving the common riscv_cpu_class_init() from
doing it.

'riscv_tcg_ops' is being exported from 'cpu.c' for now to avoid having
to deal with moving code and files around right now. We'll focus on
decoupling the realize() logic first.

Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
---
 target/riscv/cpu.c           |  5 +---
 target/riscv/cpu.h           |  4 +++
 target/riscv/meson.build     |  2 ++
 target/riscv/tcg/meson.build |  2 ++
 target/riscv/tcg/tcg-cpu.c   | 58 ++++++++++++++++++++++++++++++++++++
 5 files changed, 67 insertions(+), 4 deletions(-)
 create mode 100644 target/riscv/tcg/meson.build
 create mode 100644 target/riscv/tcg/tcg-cpu.c

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index be1c028095..2c6972fa0d 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -2290,9 +2290,7 @@ static const struct SysemuCPUOps riscv_sysemu_ops = {
 };
 #endif
 
-#include "hw/core/tcg-cpu-ops.h"
-
-static const struct TCGCPUOps riscv_tcg_ops = {
+const struct TCGCPUOps riscv_tcg_ops = {
     .initialize = riscv_translate_init,
     .synchronize_from_tb = riscv_cpu_synchronize_from_tb,
     .restore_state_to_opc = riscv_restore_state_to_opc,
@@ -2451,7 +2449,6 @@ static void riscv_cpu_class_init(ObjectClass *c, void *data)
 #endif
     cc->gdb_arch_name = riscv_gdb_arch_name;
     cc->gdb_get_dynamic_xml = riscv_gdb_get_dynamic_xml;
-    cc->tcg_ops = &riscv_tcg_ops;
 
     object_class_property_add(c, "mvendorid", "uint32", cpu_get_mvendorid,
                               cpu_set_mvendorid, NULL, NULL);
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 577abcd724..b84b62f84e 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -707,6 +707,10 @@ enum riscv_pmu_event_idx {
     RISCV_PMU_EVENT_CACHE_ITLB_PREFETCH_MISS = 0x10021,
 };
 
+/* Export tcg_ops until we move everything to tcg/tcg-cpu.c */
+#include "hw/core/tcg-cpu-ops.h"
+extern const struct TCGCPUOps riscv_tcg_ops;
+
 /* CSR function table */
 extern riscv_csr_operations csr_ops[CSR_TABLE_SIZE];
 
diff --git a/target/riscv/meson.build b/target/riscv/meson.build
index 660078bda1..f0486183fa 100644
--- a/target/riscv/meson.build
+++ b/target/riscv/meson.build
@@ -38,5 +38,7 @@ riscv_system_ss.add(files(
   'riscv-qmp-cmds.c',
 ))
 
+subdir('tcg')
+
 target_arch += {'riscv': riscv_ss}
 target_softmmu_arch += {'riscv': riscv_system_ss}
diff --git a/target/riscv/tcg/meson.build b/target/riscv/tcg/meson.build
new file mode 100644
index 0000000000..061df3d74a
--- /dev/null
+++ b/target/riscv/tcg/meson.build
@@ -0,0 +1,2 @@
+riscv_ss.add(when: 'CONFIG_TCG', if_true: files(
+  'tcg-cpu.c'))
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
new file mode 100644
index 0000000000..0326cead0d
--- /dev/null
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -0,0 +1,58 @@
+/*
+ * riscv TCG cpu class initialization
+ *
+ * Copyright (c) 2023 Ventana Micro Systems Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "cpu.h"
+#include "qemu/accel.h"
+#include "hw/core/accel-cpu.h"
+
+static void tcg_cpu_init_ops(AccelCPUClass *accel_cpu, CPUClass *cc)
+{
+    /*
+     * All cpus use the same set of operations.
+     * riscv_tcg_ops is being imported from cpu.c for now.
+     */
+    cc->tcg_ops = &riscv_tcg_ops;
+}
+
+static void tcg_cpu_class_init(CPUClass *cc)
+{
+    cc->init_accel_cpu = tcg_cpu_init_ops;
+}
+
+static void tcg_cpu_accel_class_init(ObjectClass *oc, void *data)
+{
+    AccelCPUClass *acc = ACCEL_CPU_CLASS(oc);
+
+    acc->cpu_class_init = tcg_cpu_class_init;
+}
+
+static const TypeInfo tcg_cpu_accel_type_info = {
+    .name = ACCEL_CPU_NAME("tcg"),
+
+    .parent = TYPE_ACCEL_CPU,
+    .class_init = tcg_cpu_accel_class_init,
+    .abstract = true,
+};
+
+static void tcg_cpu_accel_register_types(void)
+{
+    type_register_static(&tcg_cpu_accel_type_info);
+}
+type_init(tcg_cpu_accel_register_types);
-- 
2.41.0
Re: [PATCH v2 01/19] target/riscv: introduce TCG AccelCPUClass
Posted by LIU Zhiwei 1 year, 2 months ago
On 2023/9/6 17:16, Daniel Henrique Barboza wrote:
> target/riscv/cpu.c needs to handle all possible accelerators (TCG and
> KVM at this moment) during both init() and realize() time. This forces
> us to resort to a lot of "if tcg" and "if kvm" throughout the code,
> which isn't wrong, but can get cluttered over time. Splitting
> acceleration specific code from cpu.c to its own file will help to
> declutter the existing code and it will also make it easier to support
> KVM/TCG only builds in the future.
>
> We'll start by adding a new subdir called 'tcg' and a new file called
> 'tcg-cpu.c'. This file will be used to introduce a new accelerator class
> for TCG acceleration in RISC-V, allowing us to center all TCG exclusive
> code in its file instead of using 'cpu.c' for everything. This design is
> inpired by the work Claudio Fontana did in x86 a few years ago in commit
> f5cc5a5c1 ("i386: split cpu accelerators from cpu.c, using
> AccelCPUClass").
>
> To avoid moving too much code at once we'll start by adding the new file
> and TCG AccelCPUClass declaration. The 'class_init' from the accel class
> will init 'tcg_ops', relieving the common riscv_cpu_class_init() from
> doing it.
>
> 'riscv_tcg_ops' is being exported from 'cpu.c' for now to avoid having
> to deal with moving code and files around right now. We'll focus on
> decoupling the realize() logic first.
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>

Zhiwei

> ---
>   target/riscv/cpu.c           |  5 +---
>   target/riscv/cpu.h           |  4 +++
>   target/riscv/meson.build     |  2 ++
>   target/riscv/tcg/meson.build |  2 ++
>   target/riscv/tcg/tcg-cpu.c   | 58 ++++++++++++++++++++++++++++++++++++
>   5 files changed, 67 insertions(+), 4 deletions(-)
>   create mode 100644 target/riscv/tcg/meson.build
>   create mode 100644 target/riscv/tcg/tcg-cpu.c
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index be1c028095..2c6972fa0d 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -2290,9 +2290,7 @@ static const struct SysemuCPUOps riscv_sysemu_ops = {
>   };
>   #endif
>   
> -#include "hw/core/tcg-cpu-ops.h"
> -
> -static const struct TCGCPUOps riscv_tcg_ops = {
> +const struct TCGCPUOps riscv_tcg_ops = {
>       .initialize = riscv_translate_init,
>       .synchronize_from_tb = riscv_cpu_synchronize_from_tb,
>       .restore_state_to_opc = riscv_restore_state_to_opc,
> @@ -2451,7 +2449,6 @@ static void riscv_cpu_class_init(ObjectClass *c, void *data)
>   #endif
>       cc->gdb_arch_name = riscv_gdb_arch_name;
>       cc->gdb_get_dynamic_xml = riscv_gdb_get_dynamic_xml;
> -    cc->tcg_ops = &riscv_tcg_ops;
>   
>       object_class_property_add(c, "mvendorid", "uint32", cpu_get_mvendorid,
>                                 cpu_set_mvendorid, NULL, NULL);
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 577abcd724..b84b62f84e 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -707,6 +707,10 @@ enum riscv_pmu_event_idx {
>       RISCV_PMU_EVENT_CACHE_ITLB_PREFETCH_MISS = 0x10021,
>   };
>   
> +/* Export tcg_ops until we move everything to tcg/tcg-cpu.c */
> +#include "hw/core/tcg-cpu-ops.h"
> +extern const struct TCGCPUOps riscv_tcg_ops;
> +
>   /* CSR function table */
>   extern riscv_csr_operations csr_ops[CSR_TABLE_SIZE];
>   
> diff --git a/target/riscv/meson.build b/target/riscv/meson.build
> index 660078bda1..f0486183fa 100644
> --- a/target/riscv/meson.build
> +++ b/target/riscv/meson.build
> @@ -38,5 +38,7 @@ riscv_system_ss.add(files(
>     'riscv-qmp-cmds.c',
>   ))
>   
> +subdir('tcg')
> +
>   target_arch += {'riscv': riscv_ss}
>   target_softmmu_arch += {'riscv': riscv_system_ss}
> diff --git a/target/riscv/tcg/meson.build b/target/riscv/tcg/meson.build
> new file mode 100644
> index 0000000000..061df3d74a
> --- /dev/null
> +++ b/target/riscv/tcg/meson.build
> @@ -0,0 +1,2 @@
> +riscv_ss.add(when: 'CONFIG_TCG', if_true: files(
> +  'tcg-cpu.c'))
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> new file mode 100644
> index 0000000000..0326cead0d
> --- /dev/null
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -0,0 +1,58 @@
> +/*
> + * riscv TCG cpu class initialization
> + *
> + * Copyright (c) 2023 Ventana Micro Systems Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "cpu.h"
> +#include "qemu/accel.h"
> +#include "hw/core/accel-cpu.h"
> +
> +static void tcg_cpu_init_ops(AccelCPUClass *accel_cpu, CPUClass *cc)
> +{
> +    /*
> +     * All cpus use the same set of operations.
> +     * riscv_tcg_ops is being imported from cpu.c for now.
> +     */
> +    cc->tcg_ops = &riscv_tcg_ops;
> +}
> +
> +static void tcg_cpu_class_init(CPUClass *cc)
> +{
> +    cc->init_accel_cpu = tcg_cpu_init_ops;
> +}
> +
> +static void tcg_cpu_accel_class_init(ObjectClass *oc, void *data)
> +{
> +    AccelCPUClass *acc = ACCEL_CPU_CLASS(oc);
> +
> +    acc->cpu_class_init = tcg_cpu_class_init;
> +}
> +
> +static const TypeInfo tcg_cpu_accel_type_info = {
> +    .name = ACCEL_CPU_NAME("tcg"),
> +
> +    .parent = TYPE_ACCEL_CPU,
> +    .class_init = tcg_cpu_accel_class_init,
> +    .abstract = true,
> +};
> +
> +static void tcg_cpu_accel_register_types(void)
> +{
> +    type_register_static(&tcg_cpu_accel_type_info);
> +}
> +type_init(tcg_cpu_accel_register_types);