Patchwork [08/20] target-ppc: Add hooks for handling tcg and kvm limitations

login
register
mail settings
Submitter Andreas Färber
Date April 15, 2012, 6:38 p.m.
Message ID <1334515144-26485-9-git-send-email-afaerber@suse.de>
Download mbox | patch
Permalink /patch/152696/
State New
Headers show

Comments

Andreas Färber - April 15, 2012, 6:38 p.m.
From: David Gibson <david@gibson.dropbear.id.au>

On target-ppc, our table of CPU types and features encodes the features as
found on the hardware, regardless of whether these features are actually
usable under TCG or KVM.  We already have cases where the information from
the cpu table must be fixed up to account for limitations in the emulation
method we're using.  e.g. TCG does not support the DFP and VSX instructions
and KVM needs different numbering of the CPUs in order to tell it the
correct thread to core mappings.

This patch cleans up these hacks to handle emulation limitations by
consolidating them into a pair of functions specifically for the purpose.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
[AF: Style and typo fixes, rename new functions and drop ppc_def_t arg]
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 target-ppc/helper.c         |    9 -------
 target-ppc/kvm.c            |   14 +++++++++++
 target-ppc/kvm_ppc.h        |    5 ++++
 target-ppc/translate_init.c |   51 +++++++++++++++++++++++++++++-------------
 4 files changed, 54 insertions(+), 25 deletions(-)
Avi Kivity - April 16, 2012, 10:13 a.m.
On 04/15/2012 09:38 PM, Andreas Färber wrote:
> From: David Gibson <david@gibson.dropbear.id.au>
>
> On target-ppc, our table of CPU types and features encodes the features as
> found on the hardware, regardless of whether these features are actually
> usable under TCG or KVM.  We already have cases where the information from
> the cpu table must be fixed up to account for limitations in the emulation
> method we're using.  e.g. TCG does not support the DFP and VSX instructions
> and KVM needs different numbering of the CPUs in order to tell it the
> correct thread to core mappings.
>
> This patch cleans up these hacks to handle emulation limitations by
> consolidating them into a pair of functions specifically for the purpose.
>
>      env->check_pow = def->check_pow;
> +
> +    if (kvm_enabled()) {
> +        if (kvmppc_fixup_cpu(env) != 0) {
> +            fprintf(stderr, "Unable to virtualize selected CPU with KVM\n");
> +            exit(1);
> +        }
> +    } else {
> +        if (ppc_fixup_cpu(env) != 0) {
> +            fprintf(stderr, "Unable to emulate selected CPU with TCG\n");
> +            exit(1);
> +        }
> +    }
> +
>

I wish we had an error like that for x86.  Right now we silently add and
drop cpu features.
Andreas Färber - April 16, 2012, 10:19 a.m.
Am 16.04.2012 12:13, schrieb Avi Kivity:
> On 04/15/2012 09:38 PM, Andreas Färber wrote:
>> From: David Gibson <david@gibson.dropbear.id.au>
>>
>> On target-ppc, our table of CPU types and features encodes the features as
>> found on the hardware, regardless of whether these features are actually
>> usable under TCG or KVM.  We already have cases where the information from
>> the cpu table must be fixed up to account for limitations in the emulation
>> method we're using.  e.g. TCG does not support the DFP and VSX instructions
>> and KVM needs different numbering of the CPUs in order to tell it the
>> correct thread to core mappings.
>>
>> This patch cleans up these hacks to handle emulation limitations by
>> consolidating them into a pair of functions specifically for the purpose.
>>
>>      env->check_pow = def->check_pow;
>> +
>> +    if (kvm_enabled()) {
>> +        if (kvmppc_fixup_cpu(env) != 0) {
>> +            fprintf(stderr, "Unable to virtualize selected CPU with KVM\n");
>> +            exit(1);
>> +        }
>> +    } else {
>> +        if (ppc_fixup_cpu(env) != 0) {
>> +            fprintf(stderr, "Unable to emulate selected CPU with TCG\n");
>> +            exit(1);
>> +        }
>> +    }
>> +
>>
> 
> I wish we had an error like that for x86.  Right now we silently add and
> drop cpu features.

x86 has an ,+enforce option for that, doesn't it?

Andreas
Avi Kivity - April 16, 2012, 12:31 p.m.
On 04/16/2012 01:19 PM, Andreas Färber wrote:
> Am 16.04.2012 12:13, schrieb Avi Kivity:
> > On 04/15/2012 09:38 PM, Andreas Färber wrote:
> >> From: David Gibson <david@gibson.dropbear.id.au>
> >>
> >> On target-ppc, our table of CPU types and features encodes the features as
> >> found on the hardware, regardless of whether these features are actually
> >> usable under TCG or KVM.  We already have cases where the information from
> >> the cpu table must be fixed up to account for limitations in the emulation
> >> method we're using.  e.g. TCG does not support the DFP and VSX instructions
> >> and KVM needs different numbering of the CPUs in order to tell it the
> >> correct thread to core mappings.
> >>
> >> This patch cleans up these hacks to handle emulation limitations by
> >> consolidating them into a pair of functions specifically for the purpose.
> >>
> >>      env->check_pow = def->check_pow;
> >> +
> >> +    if (kvm_enabled()) {
> >> +        if (kvmppc_fixup_cpu(env) != 0) {
> >> +            fprintf(stderr, "Unable to virtualize selected CPU with KVM\n");
> >> +            exit(1);
> >> +        }
> >> +    } else {
> >> +        if (ppc_fixup_cpu(env) != 0) {
> >> +            fprintf(stderr, "Unable to emulate selected CPU with TCG\n");
> >> +            exit(1);
> >> +        }
> >> +    }
> >> +
> >>
> > 
> > I wish we had an error like that for x86.  Right now we silently add and
> > drop cpu features.
>
> x86 has an ,+enforce option for that, doesn't it?
>

So it does.  And one day it will be the default.

Patch

diff --git a/target-ppc/helper.c b/target-ppc/helper.c
index f61b8b2..b34dcbe 100644
--- a/target-ppc/helper.c
+++ b/target-ppc/helper.c
@@ -3198,15 +3198,6 @@  CPUPPCState *cpu_ppc_init (const char *cpu_model)
     if (tcg_enabled()) {
         ppc_translate_init();
     }
-    /* Adjust cpu index for SMT */
-#if !defined(CONFIG_USER_ONLY)
-    if (kvm_enabled()) {
-        int smt = kvmppc_smt_threads();
-
-        env->cpu_index = (env->cpu_index / smp_threads)*smt
-            + (env->cpu_index % smp_threads);
-    }
-#endif /* !CONFIG_USER_ONLY */
     env->cpu_model_str = cpu_model;
     cpu_ppc_register_internal(env, def);
 
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index d929213..c09cc39 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -27,6 +27,7 @@ 
 #include "kvm.h"
 #include "kvm_ppc.h"
 #include "cpu.h"
+#include "cpus.h"
 #include "device_tree.h"
 #include "hw/sysbus.h"
 #include "hw/spapr.h"
@@ -938,6 +939,19 @@  const ppc_def_t *kvmppc_host_cpu_def(void)
     return spec;
 }
 
+int kvmppc_fixup_cpu(CPUPPCState *env)
+{
+    int smt;
+
+    /* Adjust cpu index for SMT */
+    smt = kvmppc_smt_threads();
+    env->cpu_index = (env->cpu_index / smp_threads) * smt
+        + (env->cpu_index % smp_threads);
+
+    return 0;
+}
+
+
 bool kvm_arch_stop_on_emulation_error(CPUPPCState *env)
 {
     return true;
diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
index 8f1267c..34ecad3 100644
--- a/target-ppc/kvm_ppc.h
+++ b/target-ppc/kvm_ppc.h
@@ -29,6 +29,7 @@  void *kvmppc_create_spapr_tce(uint32_t liobn, uint32_t window_size, int *pfd);
 int kvmppc_remove_spapr_tce(void *table, int pfd, uint32_t window_size);
 #endif /* !CONFIG_USER_ONLY */
 const ppc_def_t *kvmppc_host_cpu_def(void);
+int kvmppc_fixup_cpu(CPUPPCState *env);
 
 #else
 
@@ -95,6 +96,10 @@  static inline const ppc_def_t *kvmppc_host_cpu_def(void)
     return NULL;
 }
 
+static inline int kvmppc_fixup_cpu(CPUPPCState *env)
+{
+    return -1;
+}
 #endif
 
 #ifndef CONFIG_KVM
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index b1f8785..067e07e 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -9889,6 +9889,28 @@  static int gdb_set_spe_reg(CPUPPCState *env, uint8_t *mem_buf, int n)
     return 0;
 }
 
+static int ppc_fixup_cpu(CPUPPCState *env)
+{
+    /* TCG doesn't (yet) emulate some groups of instructions that
+     * are implemented on some otherwise supported CPUs (e.g. VSX
+     * and decimal floating point instructions on POWER7).  We
+     * remove unsupported instruction groups from the cpu state's
+     * instruction masks and hope the guest can cope.  For at
+     * least the pseries machine, the unavailability of these
+     * instructions can be advertised to the guest via the device
+     * tree. */
+    if ((env->insns_flags & ~PPC_TCG_INSNS)
+        || (env->insns_flags2 & ~PPC_TCG_INSNS2)) {
+        fprintf(stderr, "Warning: Disabling some instructions which are not "
+                "emulated by TCG (0x%" PRIx64 ", 0x%" PRIx64 ")\n",
+                env->insns_flags & ~PPC_TCG_INSNS,
+                env->insns_flags2 & ~PPC_TCG_INSNS2);
+    }
+    env->insns_flags &= PPC_TCG_INSNS;
+    env->insns_flags2 &= PPC_TCG_INSNS2;
+    return 0;
+}
+
 int cpu_ppc_register_internal (CPUPPCState *env, const ppc_def_t *def)
 {
     env->msr_mask = def->msr_mask;
@@ -9897,25 +9919,22 @@  int cpu_ppc_register_internal (CPUPPCState *env, const ppc_def_t *def)
     env->bus_model = def->bus_model;
     env->insns_flags = def->insns_flags;
     env->insns_flags2 = def->insns_flags2;
-    if (!kvm_enabled()) {
-        /* TCG doesn't (yet) emulate some groups of instructions that
-         * are implemented on some otherwise supported CPUs (e.g. VSX
-         * and decimal floating point instructions on POWER7).  We
-         * remove unsupported instruction groups from the cpu state's
-         * instruction masks and hope the guest can cope.  For at
-         * least the pseries machine, the unavailability of these
-         * instructions can be advertise to the guest via the device
-         * tree.
-         *
-         * FIXME: we should have a similar masking for CPU features
-         * not accessible under KVM, but so far, there aren't any of
-         * those. */
-        env->insns_flags &= PPC_TCG_INSNS;
-        env->insns_flags2 &= PPC_TCG_INSNS2;
-    }
     env->flags = def->flags;
     env->bfd_mach = def->bfd_mach;
     env->check_pow = def->check_pow;
+
+    if (kvm_enabled()) {
+        if (kvmppc_fixup_cpu(env) != 0) {
+            fprintf(stderr, "Unable to virtualize selected CPU with KVM\n");
+            exit(1);
+        }
+    } else {
+        if (ppc_fixup_cpu(env) != 0) {
+            fprintf(stderr, "Unable to emulate selected CPU with TCG\n");
+            exit(1);
+        }
+    }
+
     if (create_ppc_opcodes(env, def) < 0)
         return -1;
     init_ppc_proc(env, def);