Re: [PATCH 01/20] target/riscv: introduce TCG AccelCPUClass

[Andrew Jones <ajones@ventanamicro.com>]

On Fri, Aug 25, 2023 at 10:08:34AM -0300, 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>
> ---
>  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   | 57 ++++++++++++++++++++++++++++++++++++
>  5 files changed, 66 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 21ebdbf084..38dcbc4dd2 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -2275,9 +2275,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,
> @@ -2436,7 +2434,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..1ad27a26aa
> --- /dev/null
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -0,0 +1,57 @@
> +/*
> + * 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,
> +};

need blank line here

> +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
> 
>

Appears to be consistent with target/i386/tcg/tcg-cpu.c, so

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

Thanks,
drew