"Bruno Larsen (billionai)" <bruno.lar...@eldorado.org.br> writes:
> this commit presents 2 possible solutions for substituting TCG emulation
> with KVM calls. One - used in machine.c and arch_dump.c - explicitly
As I mentioned in my reply to your introductory email I don't think we
should be replacing TCG routines with calls into KVM. Because that would
mean we have been up until now running KVM-supporting code but relying
on TCG routines which would (aside the most simple functions) be
wrong. The whole KVM runs natively, TCG is translated deal.
I don't see what in the vscr helpers makes them TCG-only. They seem like
they would work just fine with KVM code. The kvm_arch_get/put_registers
functions should already take care of synchronizing the CPUPPCState with
KVM.
Does that make sense? Tell me if I'm missing something. =)
> adds the KVM function and has the possibility of adding the TCG one
> for more generic compilation, prioritizing te KVM option. The second
> option, implemented in kvm_ppc.h, transparently changes the helper
> into the KVM call, if TCG is not enabled. I believe the first solution
> is better, but it is less readable, so I wanted to have some feedback
>
> Signed-off-by: Bruno Larsen (billionai) <bruno.lar...@eldorado.org.br>
> ---
> target/ppc/arch_dump.c | 17 +++++++++++++++++
> target/ppc/kvm.c | 30 ++++++++++++++++++++++++++++++
> target/ppc/kvm_ppc.h | 11 +++++++++++
> target/ppc/machine.c | 33 ++++++++++++++++++++++++++++++++-
> 4 files changed, 90 insertions(+), 1 deletion(-)
>
> diff --git a/target/ppc/arch_dump.c b/target/ppc/arch_dump.c
> index 9ab04b2c38..c53e01011a 100644
> --- a/target/ppc/arch_dump.c
> +++ b/target/ppc/arch_dump.c
> @@ -17,7 +17,10 @@
> #include "elf.h"
> #include "sysemu/dump.h"
> #include "sysemu/kvm.h"
> +#include "kvm_ppc.h"
> +#if defined(CONFIG_TCG)
> #include "exec/helper-proto.h"
> +#endif /* CONFIG_TCG */
>
> #ifdef TARGET_PPC64
> #define ELFCLASS ELFCLASS64
> @@ -176,7 +179,21 @@ static void ppc_write_elf_vmxregset(NoteFuncArg *arg,
> PowerPCCPU *cpu)
> vmxregset->avr[i].u64[1] = avr->u64[1];
> }
> }
> + /* This is the first solution implemented. My personal favorite as it
> + * allows for explicit error handling, however it is much less readable
> */
> +#if defined(CONFIG_KVM)
> + if(kvm_enabled()){
> + vmxregset->vscr.u32[3] = cpu_to_dump32(s, kvmppc_mfvscr(cpu));
> + }else
> +#endif
> +
> +#if defined(CONFIG_TCG)
> vmxregset->vscr.u32[3] = cpu_to_dump32(s, helper_mfvscr(&cpu->env));
> +#else
> + {
> + /* TODO: add proper error handling, even tough this should never be
> reached */
> + }
> +#endif
> }
>
> static void ppc_write_elf_vsxregset(NoteFuncArg *arg, PowerPCCPU *cpu)
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index 104a308abb..8ed54d12d8 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -51,6 +51,7 @@
> #include "elf.h"
> #include "sysemu/kvm_int.h"
>
> +
> #define PROC_DEVTREE_CPU "/proc/device-tree/cpus/"
>
> #define DEBUG_RETURN_GUEST 0
> @@ -2947,3 +2948,32 @@ bool kvm_arch_cpu_check_are_resettable(void)
> {
> return true;
> }
> +
> +/* Functions added to replace helper_m(t|f)vscr from int_helper.c */
> +int kvmppc_mtvscr(PowerPCCPU *cpu, uint32_t val){
> + CPUState *cs = CPU(cpu);
> + CPUPPCState *env = &cpu->env;
> + struct kvm_one_reg reg;
> + int ret;
> + reg.id = KVM_REG_PPC_VSCR;
> + reg.addr = (uintptr_t) &env->vscr;
> + ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
> + if(ret < 0){
> + fprintf(stderr, "Unable to set VSCR to KVM: %s", strerror(errno));
> + }
> + return ret;
> +}
> +
> +int kvmppc_mfvscr(PowerPCCPU *cpu){
> + CPUState *cs = CPU(cpu);
> + CPUPPCState *env = &cpu->env;
> + struct kvm_one_reg reg;
> + int ret;
> + reg.id = KVM_REG_PPC_VSCR;
> + reg.addr = (uintptr_t) &env->vscr;
> + ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
> + if(ret < 0){
> + fprintf(stderr, "Unable to get VSCR to KVM: %s", strerror(errno));
> + }
> + return ret;
> +}
> diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
> index 989f61ace0..f618cb28b1 100644
> --- a/target/ppc/kvm_ppc.h
> +++ b/target/ppc/kvm_ppc.h
> @@ -86,6 +86,17 @@ void kvmppc_set_reg_tb_offset(PowerPCCPU *cpu, int64_t
> tb_offset);
>
> int kvm_handle_nmi(PowerPCCPU *cpu, struct kvm_run *run);
>
> +int kvmppc_mtvscr(PowerPCCPU*, uint32_t);
> +int kvmppc_mfvscr(PowerPCCPU*);
> +
> +/* This is the second (quick and dirty) solution. Not my personal favorite
> + * as it hides what is actually happening from the user and doesn't allow
> + * for error checking. but it requires less change in other files */
> +#ifndef CONFIG_TCG
> +#define helper_mtvscr(env, val) kvmppc_mtvscr(env_archcpu(env),val)
> +#define helper_mfvscr(env) kvmppc_mfvscr(env_archcpu(env))
> +#endif
> +
> #else
>
> static inline uint32_t kvmppc_get_tbfreq(void)
> diff --git a/target/ppc/machine.c b/target/ppc/machine.c
> index 283db1d28a..d92bc18859 100644
> --- a/target/ppc/machine.c
> +++ b/target/ppc/machine.c
> @@ -8,7 +8,9 @@
> #include "qapi/error.h"
> #include "qemu/main-loop.h"
> #include "kvm_ppc.h"
> +#ifdef CONFIG_TCG
> #include "exec/helper-proto.h"
> +#endif /*CONFIG_TCG*/
>
> static int cpu_load_old(QEMUFile *f, void *opaque, int version_id)
> {
> @@ -95,7 +97,18 @@ static int cpu_load_old(QEMUFile *f, void *opaque, int
> version_id)
> ppc_store_sdr1(env, sdr1);
> }
> qemu_get_be32s(f, &vscr);
> - helper_mtvscr(env, vscr);
> +#if defined(CONFIG_KVM)
> + if(kvm_enabled()){
> + kvmppc_mtvscr(cpu, vscr);
> + }else
> +#endif
> +#if defined(CONFIG_TCG)
> + helper_mtvscr(env, vscr);
> +#else
> + {
> + /* TODO: Add correct error handling, even though this should never
> be reached */
> + }
> +#endif
> qemu_get_be64s(f, &env->spe_acc);
> qemu_get_be32s(f, &env->spe_fscr);
> qemu_get_betls(f, &env->msr_mask);
> @@ -450,7 +463,16 @@ static int get_vscr(QEMUFile *f, void *opaque, size_t
> size,
> const VMStateField *field)
> {
> PowerPCCPU *cpu = opaque;
> +#if defined(CONFIG_KVM)
> + if(kvm_enabled()){
> + kvmppc_mtvscr(cpu, qemu_get_be32(f));
> + return 0;
> + }
> +#endif /*CONFIG_KVM*/
> +
> +#if defined(CONFIG_TCG)
> helper_mtvscr(&cpu->env, qemu_get_be32(f));
> +#endif /*CONFIG_TCG*/
> return 0;
> }
>
> @@ -458,7 +480,16 @@ static int put_vscr(QEMUFile *f, void *opaque, size_t
> size,
> const VMStateField *field, JSONWriter *vmdesc)
> {
> PowerPCCPU *cpu = opaque;
> +#if defined(CONFIG_KVM)
> + if(kvm_enabled()){
> + qemu_put_be32(f, kvmppc_mfvscr(cpu));
> + return 0;
> + }
> +#endif /*CONFIG_KVM*/
> +
> +#if defined(CONFIG_TCG)
> qemu_put_be32(f, helper_mfvscr(&cpu->env));
> +#endif /*CONFIG_TCG*/
> return 0;
> }
