Re: [PATCH 12/20] target/riscv: move KVM only files to kvm subdir
On 8/31/23 08:30, Andrew Jones wrote: On Fri, Aug 25, 2023 at 10:08:45AM -0300, 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. Now that we have 'kvm' and 'tcg' subdirectories, it seems like we should be removing prefixes from files, i.e. 'kvm/cpu.c' and 'tcg/cpu.c' would be less verbose and just as easy to identify, but whatever people like... I don't mind shortening the names. I chose these because x86 names it that way. It's also easier to identify in the editor which file I'm editing at that moment via the filename alone, e.g. tcg-cpu.c is easier to identify than seeing 'cpu.c' and having to check the rest of the path. Yes, VSCode user here, too old for vim/emacs :D Thanks, Daniel Signed-off-by: Daniel Henrique Barboza --- 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 diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c index 388e52a294..77c384ddc3 100644 --- a/hw/riscv/virt.c +++ b/hw/riscv/virt.c @@ -35,7 +35,7 @@ #include "hw/riscv/virt.h" #include "hw/riscv/boot.h" #include "hw/riscv/numa.h" -#include "kvm_riscv.h" +#include "kvm/kvm_riscv.h" #include "hw/intc/riscv_aclint.h" #include "hw/intc/riscv_aplic.h" #include "hw/intc/riscv_imsic.h" diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index 04c6bfaeef..bf6c8519b1 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -33,7 +33,7 @@ #include "fpu/softfloat-helpers.h" #include "sysemu/kvm.h" #include "sysemu/tcg.h" -#include "kvm_riscv.h" +#include "kvm/kvm_riscv.h" #include "tcg/tcg.h" /* RISC-V CPU definitions */ diff --git a/target/riscv/kvm.c b/target/riscv/kvm/kvm-cpu.c similarity index 100% rename from target/riscv/kvm.c rename to target/riscv/kvm/kvm-cpu.c diff --git a/target/riscv/kvm-stub.c b/target/riscv/kvm/kvm-stub.c similarity index 100% rename from target/riscv/kvm-stub.c rename to target/riscv/kvm/kvm-stub.c diff --git a/target/riscv/kvm_riscv.h b/target/riscv/kvm/kvm_riscv.h similarity index 100% rename from target/riscv/kvm_riscv.h rename to target/riscv/kvm/kvm_riscv.h diff --git a/target/riscv/kvm/meson.build b/target/riscv/kvm/meson.build new file mode 100644 index 00..1cd6783894 --- /dev/null +++ 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')) diff --git a/target/riscv/meson.build b/target/riscv/meson.build index f0486183fa..c53962215f 100644 --- a/target/riscv/meson.build +++ b/target/riscv/meson.build @@ -24,7 +24,6 @@ riscv_ss.add(files( 'zce_helper.c', 'vcrypto_helper.c' )) -riscv_ss.add(when: 'CONFIG_KVM', if_true: files('kvm.c'), if_false: files('kvm-stub.c')) riscv_system_ss = ss.source_set() riscv_system_ss.add(files( @@ -39,6 +38,7 @@ riscv_system_ss.add(files( )) subdir('tcg') +subdir('kvm') target_arch += {'riscv': riscv_ss} target_softmmu_arch += {'riscv': riscv_system_ss} -- 2.41.0 Reviewed-by: Andrew Jones
Re: [PATCH 12/20] target/riscv: move KVM only files to kvm subdir
On Fri, Aug 25, 2023 at 10:08:45AM -0300, 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. Now that we have 'kvm' and 'tcg' subdirectories, it seems like we should be removing prefixes from files, i.e. 'kvm/cpu.c' and 'tcg/cpu.c' would be less verbose and just as easy to identify, but whatever people like... > > Signed-off-by: Daniel Henrique Barboza > --- > 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 > > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c > index 388e52a294..77c384ddc3 100644 > --- a/hw/riscv/virt.c > +++ b/hw/riscv/virt.c > @@ -35,7 +35,7 @@ > #include "hw/riscv/virt.h" > #include "hw/riscv/boot.h" > #include "hw/riscv/numa.h" > -#include "kvm_riscv.h" > +#include "kvm/kvm_riscv.h" > #include "hw/intc/riscv_aclint.h" > #include "hw/intc/riscv_aplic.h" > #include "hw/intc/riscv_imsic.h" > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index 04c6bfaeef..bf6c8519b1 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -33,7 +33,7 @@ > #include "fpu/softfloat-helpers.h" > #include "sysemu/kvm.h" > #include "sysemu/tcg.h" > -#include "kvm_riscv.h" > +#include "kvm/kvm_riscv.h" > #include "tcg/tcg.h" > > /* RISC-V CPU definitions */ > diff --git a/target/riscv/kvm.c b/target/riscv/kvm/kvm-cpu.c > similarity index 100% > rename from target/riscv/kvm.c > rename to target/riscv/kvm/kvm-cpu.c > diff --git a/target/riscv/kvm-stub.c b/target/riscv/kvm/kvm-stub.c > similarity index 100% > rename from target/riscv/kvm-stub.c > rename to target/riscv/kvm/kvm-stub.c > diff --git a/target/riscv/kvm_riscv.h b/target/riscv/kvm/kvm_riscv.h > similarity index 100% > rename from target/riscv/kvm_riscv.h > rename to target/riscv/kvm/kvm_riscv.h > diff --git a/target/riscv/kvm/meson.build b/target/riscv/kvm/meson.build > new file mode 100644 > index 00..1cd6783894 > --- /dev/null > +++ 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')) > diff --git a/target/riscv/meson.build b/target/riscv/meson.build > index f0486183fa..c53962215f 100644 > --- a/target/riscv/meson.build > +++ b/target/riscv/meson.build > @@ -24,7 +24,6 @@ riscv_ss.add(files( >'zce_helper.c', >'vcrypto_helper.c' > )) > -riscv_ss.add(when: 'CONFIG_KVM', if_true: files('kvm.c'), if_false: > files('kvm-stub.c')) > > riscv_system_ss = ss.source_set() > riscv_system_ss.add(files( > @@ -39,6 +38,7 @@ riscv_system_ss.add(files( > )) > > subdir('tcg') > +subdir('kvm') > > target_arch += {'riscv': riscv_ss} > target_softmmu_arch += {'riscv': riscv_system_ss} > -- > 2.41.0 > > Reviewed-by: Andrew Jones
Re: [PATCH 12/20] target/riscv: move KVM only files to kvm subdir
On 30/8/23 20:21, Daniel Henrique Barboza wrote: 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 --- 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. Great news, thanks Daniel!
Re: [PATCH 12/20] target/riscv: move KVM only files to kvm subdir
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 --- 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
Re: [PATCH 12/20] target/riscv: move KVM only files to kvm subdir
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 --- 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.
[PATCH 12/20] target/riscv: move KVM only files to kvm subdir
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 --- 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 diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c index 388e52a294..77c384ddc3 100644 --- a/hw/riscv/virt.c +++ b/hw/riscv/virt.c @@ -35,7 +35,7 @@ #include "hw/riscv/virt.h" #include "hw/riscv/boot.h" #include "hw/riscv/numa.h" -#include "kvm_riscv.h" +#include "kvm/kvm_riscv.h" #include "hw/intc/riscv_aclint.h" #include "hw/intc/riscv_aplic.h" #include "hw/intc/riscv_imsic.h" diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index 04c6bfaeef..bf6c8519b1 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -33,7 +33,7 @@ #include "fpu/softfloat-helpers.h" #include "sysemu/kvm.h" #include "sysemu/tcg.h" -#include "kvm_riscv.h" +#include "kvm/kvm_riscv.h" #include "tcg/tcg.h" /* RISC-V CPU definitions */ diff --git a/target/riscv/kvm.c b/target/riscv/kvm/kvm-cpu.c similarity index 100% rename from target/riscv/kvm.c rename to target/riscv/kvm/kvm-cpu.c diff --git a/target/riscv/kvm-stub.c b/target/riscv/kvm/kvm-stub.c similarity index 100% rename from target/riscv/kvm-stub.c rename to target/riscv/kvm/kvm-stub.c diff --git a/target/riscv/kvm_riscv.h b/target/riscv/kvm/kvm_riscv.h similarity index 100% rename from target/riscv/kvm_riscv.h rename to target/riscv/kvm/kvm_riscv.h diff --git a/target/riscv/kvm/meson.build b/target/riscv/kvm/meson.build new file mode 100644 index 00..1cd6783894 --- /dev/null +++ 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')) diff --git a/target/riscv/meson.build b/target/riscv/meson.build index f0486183fa..c53962215f 100644 --- a/target/riscv/meson.build +++ b/target/riscv/meson.build @@ -24,7 +24,6 @@ riscv_ss.add(files( 'zce_helper.c', 'vcrypto_helper.c' )) -riscv_ss.add(when: 'CONFIG_KVM', if_true: files('kvm.c'), if_false: files('kvm-stub.c')) riscv_system_ss = ss.source_set() riscv_system_ss.add(files( @@ -39,6 +38,7 @@ riscv_system_ss.add(files( )) subdir('tcg') +subdir('kvm') target_arch += {'riscv': riscv_ss} target_softmmu_arch += {'riscv': riscv_system_ss} -- 2.41.0