Re: [PATCH 12/20] target/riscv: move KVM only files to kvm subdir

[Daniel Henrique Barboza]

On 8/28/23 13:47, Philippe Mathieu-Daudé wrote:
> On 25/8/23 15:08, Daniel Henrique Barboza wrote:
> > Move the files to a 'kvm' dir to promote more code separation between
> > accelerators and making our lives easier supporting build options such
> > as --disable-tcg.
> >
> > Rename kvm.c to kvm-cpu.c to keep it in line with its TCG counterpart.
> >
> > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> > ---
> >   hw/riscv/virt.c                       | 2 +-
> >   target/riscv/cpu.c                    | 2 +-
> >   target/riscv/{kvm.c => kvm/kvm-cpu.c} | 0
> >   target/riscv/{ => kvm}/kvm-stub.c     | 0
> >   target/riscv/{ => kvm}/kvm_riscv.h    | 0
> >   target/riscv/kvm/meson.build          | 2 ++
> >   target/riscv/meson.build              | 2 +-
> >   7 files changed, 5 insertions(+), 3 deletions(-)
> >   rename target/riscv/{kvm.c => kvm/kvm-cpu.c} (100%)
> >   rename target/riscv/{ => kvm}/kvm-stub.c (100%)
> >   rename target/riscv/{ => kvm}/kvm_riscv.h (100%)
> >   create mode 100644 target/riscv/kvm/meson.build
>
>
> > +++ b/target/riscv/kvm/meson.build
> > @@ -0,0 +1,2 @@
> > +riscv_ss.add(when: 'CONFIG_KVM', if_true: files('kvm-cpu.c'),
> > +                                 if_false: files('kvm-stub.c'))
>
> Hmm maybe we need to add:
>
> -- >8 --
> diff --git a/include/hw/core/accel-cpu.h b/include/hw/core/accel-cpu.h
> index 5dbfd79955..65973b6f2e 100644
> --- a/include/hw/core/accel-cpu.h
> +++ b/include/hw/core/accel-cpu.h
> @@ -33,6 +33,7 @@ typedef struct AccelCPUClass {
>       void (*cpu_class_init)(CPUClass *cc);
>       void (*cpu_instance_init)(CPUState *cpu);
>       bool (*cpu_realizefn)(CPUState *cpu, Error **errp);
> +    ResettablePhases cpu_reset;
>   } AccelCPUClass;
> ---
>
> and here:
>
>   static void kvm_cpu_accel_class_init(ObjectClass *oc, void *data)
>   {
>       AccelCPUClass *acc = ACCEL_CPU_CLASS(oc);
>
>       acc->cpu_instance_init = kvm_cpu_instance_init;
> +    acc->cpu_reset.hold = kvm_riscv_reset_vcpu;
>   }
>
> so then calling some accel_cpu_reset_hold() in riscv_cpu_reset_hold(),
> would call kvm_riscv_reset_vcpu() and we can remove kvm-stub.c.


Or we can just remove kvm-stub.c without doing any other changes :)

Just removing it works fine it seems, even with --enable-debug builds. All 
functions
implemented in the stub are already wrapped in 'if kvm_enabled()' blocks, so the
stub isn't really being used.

I'll take the opportunity to not just remove kvm-stub.c but also add the non-KVM
stubs of all kvm_riscv.h APIs in the header, like you ARM folks are already 
doing
in kvm_arm.h.  I'll do that in a preliminary patch.


Oh, and a side note: I took a quick look at target/arm/kvm-stub.c and how
write_kvmstate_to_list() and write_list_to_kvmstate() are being called.

Turns out that this works for me:

[danielhb@grind qemu]$ git diff
diff --git a/target/arm/meson.build b/target/arm/meson.build
index e645e456da..e524e1114b 100644
--- a/target/arm/meson.build
+++ b/target/arm/meson.build
@@ -8,7 +8,7 @@ arm_ss.add(files(
 ))
 arm_ss.add(zlib)
 
-arm_ss.add(when: 'CONFIG_KVM', if_true: files('hyp_gdbstub.c', 'kvm.c', 'kvm64.c'), if_false: files('kvm-stub.c'))
+arm_ss.add(when: 'CONFIG_KVM', if_true: files('hyp_gdbstub.c', 'kvm.c', 
'kvm64.c'))
 arm_ss.add(when: 'CONFIG_HVF', if_true: files('hyp_gdbstub.c'))
 
 arm_ss.add(when: 'TARGET_AARCH64', if_true: files(

[danielhb@grind qemu]$ git rm target/arm/kvm-stub.c
rm 'target/arm/kvm-stub.c'

[danielhb@grind qemu]$ cd build
[danielhb@grind build]$ ../configure --target-list=aarch64-softmmu --enable-debug 
&& make -j
(...)
[2724/2725] Generating docs/QEMU manual with a custom command
[2725/2725] Generating docs/QEMU man pages with a custom command
[danielhb@grind build]$


I suggest you take a look into target/arm/kvm-stub.c. Seems like you can just 
rip it out.


Thanks,


Daniel


>