[Qemu-devel] [PATCH v3 0/6] arm: support -cpu max (and gic-version=max)

[Qemu-devel] [PATCH v3 0/6] arm: support -cpu max (and gic-version=max)

[Peter Maydell]

This patchset adds support for '-cpu max' to Arm, along the lines
of the existing support we have for x86 targets:

 * under KVM, -cpu max is the same as -cpu host
 * under TCG, -cpu max means "emulate with as many features as
   possible"

-cpu max is supported for both usermode and system emulation,
again following the x86 line.

NB that cross-QEMU-version migration is not supported for -cpu max:
in future the definition of "maximum set of features" will change
as we add more emulation features.

The patchset also adds support to the virt board for the "max"
option to -machine gic-version, requesting "best available
interrupt controller", with the same semantics as -cpu max.

Together these should assist users like libguestfs that just want
to be able to run code without having to figure out what the
right command line arguments for this particular host system are.

The patches make 'any' a synonym for 'max', using the approach
of putting a compat hack into arm_cpu_class_by_name() that
Igor suggested. For the moment, we don't enable any of the
CPU feature bits in 'max' that aren't in the a15/a57 if we're
doing system emulation mode. This is a combination of:
 * some of those bits are currently only implemented sufficiently
   for user-only mode, not for system emulation
 * we would also need to update ID register bits to advertise the
   extra features
We should certainly do that, but I'd like to get basic -cpu max
support into 2.12, and the softfreeze deadline is rapidly approaching,
so I've opted to postpone that to post 2.12. If people have the time
to review this series this week I'd really appreciate it.

Changes since v1:
 * the "check CPU realize succeeded in virt board" patch is in master
 * the "target/arm: Make 'any' CPU just an alias for 'max'" is new,
   and unifies 'any' with 'max' as suggested in review of v1
 * the other patches are essentially unchanged

Changes since v2:
 * fixed the silly error in patch 4 that made user-only mode always
   use "max" rather than only if the user asked for "any"...

thanks
-- PMM


Peter Maydell (6):
  target/arm: Query host CPU features on-demand at instance init
  target/arm: Move definition of 'host' cpu type into cpu.c
  target/arm: Add "-cpu max" support
  target/arm: Make 'any' CPU just an alias for 'max'
  hw/arm/virt: Add "max" to the list of CPU types "virt" supports
  hw/arm/virt: Support -machine gic-version=max

 target/arm/cpu-qom.h |  2 ++
 target/arm/cpu.h     |  5 +++
 target/arm/kvm_arm.h | 35 +++++++++++++------
 hw/arm/virt.c        | 30 ++++++++++------
 target/arm/cpu.c     | 97 +++++++++++++++++++++++++++++++++++++++++++---------
 target/arm/cpu64.c   | 66 ++++++++++++++++++++++-------------
 target/arm/kvm.c     | 53 ++++++++++------------------
 target/arm/kvm32.c   |  8 ++---
 target/arm/kvm64.c   |  8 ++---
 9 files changed, 200 insertions(+), 104 deletions(-)

-- 
2.16.2

[Qemu-devel] [PATCH v3 1/6] target/arm: Query host CPU features on-demand at instance init

[Peter Maydell]

Currently we query the host CPU features in the class init function
for the TYPE_ARM_HOST_CPU class, so that we can later copy them
from the class object into the instance object in the object
instance init function. This is awkward for implementing "-cpu max",
which should work like "-cpu host" for KVM but like "cpu with all
implemented features" for TCG.

Move the place where we store the information about the host CPU from
a class object to static variables in kvm.c, and then in the instance
init function call a new kvm_arm_set_cpu_features_from_host()
function which will query the host kernel if necessary and then
fill in the CPU instance fields.

This allows us to drop the special class struct and class init
function for TYPE_ARM_HOST_CPU entirely.

We can't delay the probe until realize, because the ARM
instance_post_init hook needs to look at the feature bits we
set, so we need to do it in the initfn. This is safe because
the probing doesn't affect the actual VM state (it creates a
separate scratch VM to do its testing), but the probe might fail.
Because we can't report errors in retrieving the host features
in the initfn, we check this belatedly in the realize function
(the intervening code will be able to cope with the relevant
fields in the CPU structure being zero).

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/arm/cpu.h     |  5 +++++
 target/arm/kvm_arm.h | 35 ++++++++++++++++++++++++-----------
 target/arm/cpu.c     | 13 +++++++++++++
 target/arm/kvm.c     | 36 +++++++++++++++++++-----------------
 target/arm/kvm32.c   |  8 ++++----
 target/arm/kvm64.c   |  8 ++++----
 6 files changed, 69 insertions(+), 36 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 8dd6b788df..1df46ad7a0 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -745,6 +745,11 @@ struct ARMCPU {
     /* Uniprocessor system with MP extensions */
     bool mp_is_up;
 
+    /* True if we tried kvm_arm_host_cpu_features() during CPU instance_init
+     * and the probe failed (so we need to report the error in realize)
+     */
+    bool host_cpu_probe_failed;
+
     /* The instance init functions for implementation-specific subclasses
      * set these fields to specify the implementation-dependent values of
      * various constant registers and reset values of non-constant
diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index cfb7e5af72..1e2364007d 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -152,20 +152,16 @@ bool kvm_arm_create_scratch_host_vcpu(const uint32_t *cpus_to_try,
 void kvm_arm_destroy_scratch_host_vcpu(int *fdarray);
 
 #define TYPE_ARM_HOST_CPU "host-" TYPE_ARM_CPU
-#define ARM_HOST_CPU_CLASS(klass) \
-    OBJECT_CLASS_CHECK(ARMHostCPUClass, (klass), TYPE_ARM_HOST_CPU)
-#define ARM_HOST_CPU_GET_CLASS(obj) \
-    OBJECT_GET_CLASS(ARMHostCPUClass, (obj), TYPE_ARM_HOST_CPU)
-
-typedef struct ARMHostCPUClass {
-    /*< private >*/
-    ARMCPUClass parent_class;
-    /*< public >*/
 
+/**
+ * ARMHostCPUFeatures: information about the host CPU (identified
+ * by asking the host kernel)
+ */
+typedef struct ARMHostCPUFeatures {
     uint64_t features;
     uint32_t target;
     const char *dtb_compatible;
-} ARMHostCPUClass;
+} ARMHostCPUFeatures;
 
 /**
  * kvm_arm_get_host_cpu_features:
@@ -174,8 +170,16 @@ typedef struct ARMHostCPUClass {
  * Probe the capabilities of the host kernel's preferred CPU and fill
  * in the ARMHostCPUClass struct accordingly.
  */
-bool kvm_arm_get_host_cpu_features(ARMHostCPUClass *ahcc);
+bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf);
 
+/**
+ * kvm_arm_set_cpu_features_from_host:
+ * @cpu: ARMCPU to set the features for
+ *
+ * Set up the ARMCPU struct fields up to match the information probed
+ * from the host CPU.
+ */
+void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu);
 
 /**
  * kvm_arm_sync_mpstate_to_kvm
@@ -200,6 +204,15 @@ void kvm_arm_pmu_init(CPUState *cs);
 
 #else
 
+static inline void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu)
+{
+    /* This should never actually be called in the "not KVM" case,
+     * but set up the fields to indicate an error anyway.
+     */
+    cpu->kvm_target = QEMU_KVM_ARM_TARGET_NONE;
+    cpu->host_cpu_probe_failed = true;
+}
+
 static inline int kvm_arm_vgic_probe(void)
 {
     return 0;
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 6b77aaa445..abf9fb2160 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -725,6 +725,19 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
     int pagebits;
     Error *local_err = NULL;
 
+    /* If we needed to query the host kernel for the CPU features
+     * then it's possible that might have failed in the initfn, but
+     * this is the first point where we can report it.
+     */
+    if (cpu->host_cpu_probe_failed) {
+        if (!kvm_enabled()) {
+            error_setg(errp, "The 'host' CPU type can only be used with KVM");
+        } else {
+            error_setg(errp, "Failed to retrieve host CPU features");
+        }
+        return;
+    }
+
     cpu_exec_realizefn(cs, &local_err);
     if (local_err != NULL) {
         error_propagate(errp, local_err);
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 1219d0062b..1c0e57690a 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -33,6 +33,8 @@ const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
 
 static bool cap_has_mp_state;
 
+static ARMHostCPUFeatures arm_host_cpu_features;
+
 int kvm_arm_vcpu_init(CPUState *cs)
 {
     ARMCPU *cpu = ARM_CPU(cs);
@@ -129,30 +131,32 @@ void kvm_arm_destroy_scratch_host_vcpu(int *fdarray)
     }
 }
 
-static void kvm_arm_host_cpu_class_init(ObjectClass *oc, void *data)
+void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu)
 {
-    ARMHostCPUClass *ahcc = ARM_HOST_CPU_CLASS(oc);
+    CPUARMState *env = &cpu->env;
 
-    /* All we really need to set up for the 'host' CPU
-     * is the feature bits -- we rely on the fact that the
-     * various ID register values in ARMCPU are only used for
-     * TCG CPUs.
-     */
-    if (!kvm_arm_get_host_cpu_features(ahcc)) {
-        fprintf(stderr, "Failed to retrieve host CPU features!\n");
-        abort();
+    if (!arm_host_cpu_features.dtb_compatible) {
+        if (!kvm_enabled() ||
+            !kvm_arm_get_host_cpu_features(&arm_host_cpu_features)) {
+            /* We can't report this error yet, so flag that we need to
+             * in arm_cpu_realizefn().
+             */
+            cpu->kvm_target = QEMU_KVM_ARM_TARGET_NONE;
+            cpu->host_cpu_probe_failed = true;
+            return;
+        }
     }
+
+    cpu->kvm_target = arm_host_cpu_features.target;
+    cpu->dtb_compatible = arm_host_cpu_features.dtb_compatible;
+    env->features = arm_host_cpu_features.features;
 }
 
 static void kvm_arm_host_cpu_initfn(Object *obj)
 {
-    ARMHostCPUClass *ahcc = ARM_HOST_CPU_GET_CLASS(obj);
     ARMCPU *cpu = ARM_CPU(obj);
-    CPUARMState *env = &cpu->env;
 
-    cpu->kvm_target = ahcc->target;
-    cpu->dtb_compatible = ahcc->dtb_compatible;
-    env->features = ahcc->features;
+    kvm_arm_set_cpu_features_from_host(cpu);
 }
 
 static const TypeInfo host_arm_cpu_type_info = {
@@ -163,8 +167,6 @@ static const TypeInfo host_arm_cpu_type_info = {
     .parent = TYPE_ARM_CPU,
 #endif
     .instance_init = kvm_arm_host_cpu_initfn,
-    .class_init = kvm_arm_host_cpu_class_init,
-    .class_size = sizeof(ARMHostCPUClass),
 };
 
 int kvm_arch_init(MachineState *ms, KVMState *s)
diff --git a/target/arm/kvm32.c b/target/arm/kvm32.c
index f77c9c494b..1740cda47d 100644
--- a/target/arm/kvm32.c
+++ b/target/arm/kvm32.c
@@ -28,7 +28,7 @@ static inline void set_feature(uint64_t *features, int feature)
     *features |= 1ULL << feature;
 }
 
-bool kvm_arm_get_host_cpu_features(ARMHostCPUClass *ahcc)
+bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
 {
     /* Identify the feature bits corresponding to the host CPU, and
      * fill out the ARMHostCPUClass fields accordingly. To do this
@@ -74,13 +74,13 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUClass *ahcc)
         return false;
     }
 
-    ahcc->target = init.target;
+    ahcf->target = init.target;
 
     /* This is not strictly blessed by the device tree binding docs yet,
      * but in practice the kernel does not care about this string so
      * there is no point maintaining an KVM_ARM_TARGET_* -> string table.
      */
-    ahcc->dtb_compatible = "arm,arm-v7";
+    ahcf->dtb_compatible = "arm,arm-v7";
 
     for (i = 0; i < ARRAY_SIZE(idregs); i++) {
         ret = ioctl(fdarray[2], KVM_GET_ONE_REG, &idregs[i]);
@@ -132,7 +132,7 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUClass *ahcc)
         set_feature(&features, ARM_FEATURE_VFP4);
     }
 
-    ahcc->features = features;
+    ahcf->features = features;
 
     return true;
 }
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index ac728494a4..e0b8246283 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -443,7 +443,7 @@ static inline void unset_feature(uint64_t *features, int feature)
     *features &= ~(1ULL << feature);
 }
 
-bool kvm_arm_get_host_cpu_features(ARMHostCPUClass *ahcc)
+bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
 {
     /* Identify the feature bits corresponding to the host CPU, and
      * fill out the ARMHostCPUClass fields accordingly. To do this
@@ -471,8 +471,8 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUClass *ahcc)
         return false;
     }
 
-    ahcc->target = init.target;
-    ahcc->dtb_compatible = "arm,arm-v8";
+    ahcf->target = init.target;
+    ahcf->dtb_compatible = "arm,arm-v8";
 
     kvm_arm_destroy_scratch_host_vcpu(fdarray);
 
@@ -486,7 +486,7 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUClass *ahcc)
     set_feature(&features, ARM_FEATURE_AARCH64);
     set_feature(&features, ARM_FEATURE_PMU);
 
-    ahcc->features = features;
+    ahcf->features = features;
 
     return true;
 }
-- 
2.16.2

[Qemu-devel] [PATCH v3 2/6] target/arm: Move definition of 'host' cpu type into cpu.c

[Peter Maydell]

Move the definition of the 'host' cpu type into cpu.c, where all the
other CPU types are defined.  We can do this now we've decoupled it
from the KVM-specific host feature probing.  This means we now create
the type unconditionally (assuming we were built with KVM support at
all), but if you try to use it without -enable-kvm this will end
up in the "host cpu probe failed and KVM not enabled" path in
arm_cpu_realizefn(), for an appropriate error message.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/arm/cpu.c | 24 ++++++++++++++++++++++++
 target/arm/kvm.c | 19 -------------------
 2 files changed, 24 insertions(+), 19 deletions(-)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index abf9fb2160..1a58a2c094 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1858,6 +1858,26 @@ static void arm_cpu_class_init(ObjectClass *oc, void *data)
 #endif
 }
 
+#ifdef CONFIG_KVM
+static void arm_host_initfn(Object *obj)
+{
+    ARMCPU *cpu = ARM_CPU(obj);
+
+    kvm_arm_set_cpu_features_from_host(cpu);
+}
+
+static const TypeInfo host_arm_cpu_type_info = {
+    .name = TYPE_ARM_HOST_CPU,
+#ifdef TARGET_AARCH64
+    .parent = TYPE_AARCH64_CPU,
+#else
+    .parent = TYPE_ARM_CPU,
+#endif
+    .instance_init = arm_host_initfn,
+};
+
+#endif
+
 static void cpu_register(const ARMCPUInfo *info)
 {
     TypeInfo type_info = {
@@ -1902,6 +1922,10 @@ static void arm_cpu_register_types(void)
         cpu_register(info);
         info++;
     }
+
+#ifdef CONFIG_KVM
+    type_register_static(&host_arm_cpu_type_info);
+#endif
 }
 
 type_init(arm_cpu_register_types)
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 1c0e57690a..ecc39ac295 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -152,23 +152,6 @@ void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu)
     env->features = arm_host_cpu_features.features;
 }
 
-static void kvm_arm_host_cpu_initfn(Object *obj)
-{
-    ARMCPU *cpu = ARM_CPU(obj);
-
-    kvm_arm_set_cpu_features_from_host(cpu);
-}
-
-static const TypeInfo host_arm_cpu_type_info = {
-    .name = TYPE_ARM_HOST_CPU,
-#ifdef TARGET_AARCH64
-    .parent = TYPE_AARCH64_CPU,
-#else
-    .parent = TYPE_ARM_CPU,
-#endif
-    .instance_init = kvm_arm_host_cpu_initfn,
-};
-
 int kvm_arch_init(MachineState *ms, KVMState *s)
 {
     /* For ARM interrupt delivery is always asynchronous,
@@ -184,8 +167,6 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
 
     cap_has_mp_state = kvm_check_extension(s, KVM_CAP_MP_STATE);
 
-    type_register_static(&host_arm_cpu_type_info);
-
     return 0;
 }
 
-- 
2.16.2

[Qemu-devel] [PATCH v3 3/6] target/arm: Add "-cpu max" support

[Peter Maydell]

Add support for "-cpu max" for ARM guests. This CPU type behaves
like "-cpu host" when KVM is enabled, and like a system CPU with
the maximum possible feature set otherwise. (Note that this means
it won't be migratable across versions, as we will likely add
features to it in future.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/cpu-qom.h |  2 ++
 target/arm/cpu.c     | 24 ++++++++++++++++++++++++
 target/arm/cpu64.c   | 21 +++++++++++++++++++++
 3 files changed, 47 insertions(+)

diff --git a/target/arm/cpu-qom.h b/target/arm/cpu-qom.h
index a42495bac9..d135ff8e06 100644
--- a/target/arm/cpu-qom.h
+++ b/target/arm/cpu-qom.h
@@ -33,6 +33,8 @@ struct arm_boot_info;
 #define ARM_CPU_GET_CLASS(obj) \
     OBJECT_GET_CLASS(ARMCPUClass, (obj), TYPE_ARM_CPU)
 
+#define TYPE_ARM_MAX_CPU "max-" TYPE_ARM_CPU
+
 /**
  * ARMCPUClass:
  * @parent_realize: The parent class' realize handler.
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 1a58a2c094..e46ddcc613 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1697,6 +1697,27 @@ static void pxa270c5_initfn(Object *obj)
     cpu->reset_sctlr = 0x00000078;
 }
 
+#ifndef TARGET_AARCH64
+/* -cpu max: if KVM is enabled, like -cpu host (best possible with this host);
+ * otherwise, a CPU with as many features enabled as our emulation supports.
+ * The version of '-cpu max' for qemu-system-aarch64 is defined in cpu64.c;
+ * this only needs to handle 32 bits.
+ */
+static void arm_max_initfn(Object *obj)
+{
+    ARMCPU *cpu = ARM_CPU(obj);
+
+    if (kvm_enabled()) {
+        kvm_arm_set_cpu_features_from_host(cpu);
+    } else {
+        cortex_a15_initfn(obj);
+        /* In future we might add feature bits here even if the
+         * real-world A15 doesn't implement them.
+         */
+    }
+}
+#endif
+
 #ifdef CONFIG_USER_ONLY
 static void arm_any_initfn(Object *obj)
 {
@@ -1764,6 +1785,9 @@ static const ARMCPUInfo arm_cpus[] = {
     { .name = "pxa270-b1",   .initfn = pxa270b1_initfn },
     { .name = "pxa270-c0",   .initfn = pxa270c0_initfn },
     { .name = "pxa270-c5",   .initfn = pxa270c5_initfn },
+#ifndef TARGET_AARCH64
+    { .name = "max",         .initfn = arm_max_initfn },
+#endif
 #ifdef CONFIG_USER_ONLY
     { .name = "any",         .initfn = arm_any_initfn },
 #endif
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 4228713b19..9042d3dfd1 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -28,6 +28,7 @@
 #include "hw/arm/arm.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/kvm.h"
+#include "kvm_arm.h"
 
 static inline void set_feature(CPUARMState *env, int feature)
 {
@@ -212,6 +213,25 @@ static void aarch64_a53_initfn(Object *obj)
     define_arm_cp_regs(cpu, cortex_a57_a53_cp_reginfo);
 }
 
+/* -cpu max: if KVM is enabled, like -cpu host (best possible with this host);
+ * otherwise, a CPU with as many features enabled as our emulation supports.
+ * The version of '-cpu max' for qemu-system-arm is defined in cpu.c;
+ * this only needs to handle 64 bits.
+ */
+static void aarch64_max_initfn(Object *obj)
+{
+    ARMCPU *cpu = ARM_CPU(obj);
+
+    if (kvm_enabled()) {
+        kvm_arm_set_cpu_features_from_host(cpu);
+    } else {
+        aarch64_a57_initfn(obj);
+        /* In future we might add feature bits here even if the
+         * real-world A57 doesn't implement them.
+         */
+    }
+}
+
 #ifdef CONFIG_USER_ONLY
 static void aarch64_any_initfn(Object *obj)
 {
@@ -247,6 +267,7 @@ typedef struct ARMCPUInfo {
 static const ARMCPUInfo aarch64_cpus[] = {
     { .name = "cortex-a57",         .initfn = aarch64_a57_initfn },
     { .name = "cortex-a53",         .initfn = aarch64_a53_initfn },
+    { .name = "max",                .initfn = aarch64_max_initfn },
 #ifdef CONFIG_USER_ONLY
     { .name = "any",         .initfn = aarch64_any_initfn },
 #endif
-- 
2.16.2

[Qemu-devel] [PATCH v3 4/6] target/arm: Make 'any' CPU just an alias for 'max'

[Peter Maydell]

Now we have a working '-cpu max', the linux-user-only
'any' CPU is pretty much the same thing, so implement it
that way.

For the moment we don't add any of the extra feature bits
to the system-emulation "max", because we don't set the
ID register bits we would need to to advertise those
features as present.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/cpu.c   | 52 +++++++++++++++++++++++++----------------------
 target/arm/cpu64.c | 59 ++++++++++++++++++++++++++----------------------------
 2 files changed, 56 insertions(+), 55 deletions(-)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index e46ddcc613..6068efb8ff 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -965,9 +965,19 @@ static ObjectClass *arm_cpu_class_by_name(const char *cpu_model)
     ObjectClass *oc;
     char *typename;
     char **cpuname;
+    const char *cpunamestr;
 
     cpuname = g_strsplit(cpu_model, ",", 1);
-    typename = g_strdup_printf(ARM_CPU_TYPE_NAME("%s"), cpuname[0]);
+    cpunamestr = cpuname[0];
+#ifdef CONFIG_USER_ONLY
+    /* For backwards compatibility usermode emulation allows "-cpu any",
+     * which has the same semantics as "-cpu max".
+     */
+    if (!strcmp(cpunamestr, "any")) {
+        cpunamestr = "max";
+    }
+#endif
+    typename = g_strdup_printf(ARM_CPU_TYPE_NAME("%s"), cpunamestr);
     oc = object_class_by_name(typename);
     g_strfreev(cpuname);
     g_free(typename);
@@ -1711,29 +1721,23 @@ static void arm_max_initfn(Object *obj)
         kvm_arm_set_cpu_features_from_host(cpu);
     } else {
         cortex_a15_initfn(obj);
-        /* In future we might add feature bits here even if the
-         * real-world A15 doesn't implement them.
-         */
-    }
-}
-#endif
-
 #ifdef CONFIG_USER_ONLY
-static void arm_any_initfn(Object *obj)
-{
-    ARMCPU *cpu = ARM_CPU(obj);
-    set_feature(&cpu->env, ARM_FEATURE_V8);
-    set_feature(&cpu->env, ARM_FEATURE_VFP4);
-    set_feature(&cpu->env, ARM_FEATURE_NEON);
-    set_feature(&cpu->env, ARM_FEATURE_THUMB2EE);
-    set_feature(&cpu->env, ARM_FEATURE_V8_AES);
-    set_feature(&cpu->env, ARM_FEATURE_V8_SHA1);
-    set_feature(&cpu->env, ARM_FEATURE_V8_SHA256);
-    set_feature(&cpu->env, ARM_FEATURE_V8_PMULL);
-    set_feature(&cpu->env, ARM_FEATURE_CRC);
-    set_feature(&cpu->env, ARM_FEATURE_V8_RDM);
-    set_feature(&cpu->env, ARM_FEATURE_V8_FCMA);
-    cpu->midr = 0xffffffff;
+        /* We don't set these in system emulation mode for the moment,
+         * since we don't correctly set the ID registers to advertise them,
+         */
+        set_feature(&cpu->env, ARM_FEATURE_V8);
+        set_feature(&cpu->env, ARM_FEATURE_VFP4);
+        set_feature(&cpu->env, ARM_FEATURE_NEON);
+        set_feature(&cpu->env, ARM_FEATURE_THUMB2EE);
+        set_feature(&cpu->env, ARM_FEATURE_V8_AES);
+        set_feature(&cpu->env, ARM_FEATURE_V8_SHA1);
+        set_feature(&cpu->env, ARM_FEATURE_V8_SHA256);
+        set_feature(&cpu->env, ARM_FEATURE_V8_PMULL);
+        set_feature(&cpu->env, ARM_FEATURE_CRC);
+        set_feature(&cpu->env, ARM_FEATURE_V8_RDM);
+        set_feature(&cpu->env, ARM_FEATURE_V8_FCMA);
+#endif
+    }
 }
 #endif
 
@@ -1789,7 +1793,7 @@ static const ARMCPUInfo arm_cpus[] = {
     { .name = "max",         .initfn = arm_max_initfn },
 #endif
 #ifdef CONFIG_USER_ONLY
-    { .name = "any",         .initfn = arm_any_initfn },
+    { .name = "any",         .initfn = arm_max_initfn },
 #endif
 #endif
     { .name = NULL }
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 9042d3dfd1..3429875787 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -226,38 +226,38 @@ static void aarch64_max_initfn(Object *obj)
         kvm_arm_set_cpu_features_from_host(cpu);
     } else {
         aarch64_a57_initfn(obj);
-        /* In future we might add feature bits here even if the
-         * real-world A57 doesn't implement them.
+#ifdef CONFIG_USER_ONLY
+        /* We don't set these in system emulation mode for the moment,
+         * since we don't correctly set the ID registers to advertise them,
+         * and in some cases they're only available in AArch64 and not AArch32,
+         * whereas the architecture requires them to be present in both if
+         * present in either.
          */
+        set_feature(&cpu->env, ARM_FEATURE_V8);
+        set_feature(&cpu->env, ARM_FEATURE_VFP4);
+        set_feature(&cpu->env, ARM_FEATURE_NEON);
+        set_feature(&cpu->env, ARM_FEATURE_AARCH64);
+        set_feature(&cpu->env, ARM_FEATURE_V8_AES);
+        set_feature(&cpu->env, ARM_FEATURE_V8_SHA1);
+        set_feature(&cpu->env, ARM_FEATURE_V8_SHA256);
+        set_feature(&cpu->env, ARM_FEATURE_V8_SHA512);
+        set_feature(&cpu->env, ARM_FEATURE_V8_SHA3);
+        set_feature(&cpu->env, ARM_FEATURE_V8_SM3);
+        set_feature(&cpu->env, ARM_FEATURE_V8_SM4);
+        set_feature(&cpu->env, ARM_FEATURE_V8_PMULL);
+        set_feature(&cpu->env, ARM_FEATURE_CRC);
+        set_feature(&cpu->env, ARM_FEATURE_V8_RDM);
+        set_feature(&cpu->env, ARM_FEATURE_V8_FP16);
+        set_feature(&cpu->env, ARM_FEATURE_V8_FCMA);
+        /* For usermode -cpu max we can use a larger and more efficient DCZ
+         * blocksize since we don't have to follow what the hardware does.
+         */
+        cpu->ctr = 0x80038003; /* 32 byte I and D cacheline size, VIPT icache */
+        cpu->dcz_blocksize = 7; /*  512 bytes */
+#endif
     }
 }
 
-#ifdef CONFIG_USER_ONLY
-static void aarch64_any_initfn(Object *obj)
-{
-    ARMCPU *cpu = ARM_CPU(obj);
-
-    set_feature(&cpu->env, ARM_FEATURE_V8);
-    set_feature(&cpu->env, ARM_FEATURE_VFP4);
-    set_feature(&cpu->env, ARM_FEATURE_NEON);
-    set_feature(&cpu->env, ARM_FEATURE_AARCH64);
-    set_feature(&cpu->env, ARM_FEATURE_V8_AES);
-    set_feature(&cpu->env, ARM_FEATURE_V8_SHA1);
-    set_feature(&cpu->env, ARM_FEATURE_V8_SHA256);
-    set_feature(&cpu->env, ARM_FEATURE_V8_SHA512);
-    set_feature(&cpu->env, ARM_FEATURE_V8_SHA3);
-    set_feature(&cpu->env, ARM_FEATURE_V8_SM3);
-    set_feature(&cpu->env, ARM_FEATURE_V8_SM4);
-    set_feature(&cpu->env, ARM_FEATURE_V8_PMULL);
-    set_feature(&cpu->env, ARM_FEATURE_CRC);
-    set_feature(&cpu->env, ARM_FEATURE_V8_RDM);
-    set_feature(&cpu->env, ARM_FEATURE_V8_FP16);
-    set_feature(&cpu->env, ARM_FEATURE_V8_FCMA);
-    cpu->ctr = 0x80038003; /* 32 byte I and D cacheline size, VIPT icache */
-    cpu->dcz_blocksize = 7; /*  512 bytes */
-}
-#endif
-
 typedef struct ARMCPUInfo {
     const char *name;
     void (*initfn)(Object *obj);
@@ -268,9 +268,6 @@ static const ARMCPUInfo aarch64_cpus[] = {
     { .name = "cortex-a57",         .initfn = aarch64_a57_initfn },
     { .name = "cortex-a53",         .initfn = aarch64_a53_initfn },
     { .name = "max",                .initfn = aarch64_max_initfn },
-#ifdef CONFIG_USER_ONLY
-    { .name = "any",         .initfn = aarch64_any_initfn },
-#endif
     { .name = NULL }
 };
 
-- 
2.16.2

[Qemu-devel] [PATCH v3 5/6] hw/arm/virt: Add "max" to the list of CPU types "virt" supports

[Peter Maydell]

Allow the virt board to support '-cpu max' in the same way
it already handles '-cpu host'.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/virt.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index dbb3c8036a..cda4b83586 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -169,6 +169,7 @@ static const char *valid_cpus[] = {
     ARM_CPU_TYPE_NAME("cortex-a53"),
     ARM_CPU_TYPE_NAME("cortex-a57"),
     ARM_CPU_TYPE_NAME("host"),
+    ARM_CPU_TYPE_NAME("max"),
 };
 
 static bool cpu_type_valid(const char *cpu)
-- 
2.16.2

[Qemu-devel] [PATCH v3 6/6] hw/arm/virt: Support -machine gic-version=max

[Peter Maydell]

Add support for passing 'max' to -machine gic-version. By analogy
with the -cpu max option, this picks the "best available" GIC version
whether you're using KVM or TCG, so it behaves like 'host' when
using KVM, and gives you GICv3 when using TCG.

Also like '-cpu host', using -machine gic-version=max' means there
is no guarantee of migration compatibility between QEMU versions;
in future 'max' might mean '4'.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/virt.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index cda4b83586..2c07245047 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1207,16 +1207,23 @@ static void machvirt_init(MachineState *machine)
     /* We can probe only here because during property set
      * KVM is not available yet
      */
-    if (!vms->gic_version) {
+    if (vms->gic_version <= 0) {
+        /* "host" or "max" */
         if (!kvm_enabled()) {
-            error_report("gic-version=host requires KVM");
-            exit(1);
-        }
-
-        vms->gic_version = kvm_arm_vgic_probe();
-        if (!vms->gic_version) {
-            error_report("Unable to determine GIC version supported by host");
-            exit(1);
+            if (vms->gic_version == 0) {
+                error_report("gic-version=host requires KVM");
+                exit(1);
+            } else {
+                /* "max": currently means 3 for TCG */
+                vms->gic_version = 3;
+            }
+        } else {
+            vms->gic_version = kvm_arm_vgic_probe();
+            if (!vms->gic_version) {
+                error_report(
+                    "Unable to determine GIC version supported by host");
+                exit(1);
+            }
         }
     }
 
@@ -1480,9 +1487,11 @@ static void virt_set_gic_version(Object *obj, const char *value, Error **errp)
         vms->gic_version = 2;
     } else if (!strcmp(value, "host")) {
         vms->gic_version = 0; /* Will probe later */
+    } else if (!strcmp(value, "max")) {
+        vms->gic_version = -1; /* Will probe later */
     } else {
         error_setg(errp, "Invalid gic-version value");
-        error_append_hint(errp, "Valid values are 3, 2, host.\n");
+        error_append_hint(errp, "Valid values are 3, 2, host, max.\n");
     }
 }
 
-- 
2.16.2

Re: [Qemu-devel] [PATCH v3 1/6] target/arm: Query host CPU features on-demand at instance init

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> Currently we query the host CPU features in the class init function
> for the TYPE_ARM_HOST_CPU class, so that we can later copy them
> from the class object into the instance object in the object
> instance init function. This is awkward for implementing "-cpu max",
> which should work like "-cpu host" for KVM but like "cpu with all
> implemented features" for TCG.
>
> Move the place where we store the information about the host CPU from
> a class object to static variables in kvm.c, and then in the instance
> init function call a new kvm_arm_set_cpu_features_from_host()
> function which will query the host kernel if necessary and then
> fill in the CPU instance fields.
>
> This allows us to drop the special class struct and class init
> function for TYPE_ARM_HOST_CPU entirely.
>
> We can't delay the probe until realize, because the ARM
> instance_post_init hook needs to look at the feature bits we
> set, so we need to do it in the initfn. This is safe because
> the probing doesn't affect the actual VM state (it creates a
> separate scratch VM to do its testing), but the probe might fail.
> Because we can't report errors in retrieving the host features
> in the initfn, we check this belatedly in the realize function
> (the intervening code will be able to cope with the relevant
> fields in the CPU structure being zero).
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  target/arm/cpu.h     |  5 +++++
>  target/arm/kvm_arm.h | 35 ++++++++++++++++++++++++-----------
>  target/arm/cpu.c     | 13 +++++++++++++
>  target/arm/kvm.c     | 36 +++++++++++++++++++-----------------
>  target/arm/kvm32.c   |  8 ++++----
>  target/arm/kvm64.c   |  8 ++++----
>  6 files changed, 69 insertions(+), 36 deletions(-)
>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 8dd6b788df..1df46ad7a0 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -745,6 +745,11 @@ struct ARMCPU {
>      /* Uniprocessor system with MP extensions */
>      bool mp_is_up;
>
> +    /* True if we tried kvm_arm_host_cpu_features() during CPU instance_init
> +     * and the probe failed (so we need to report the error in realize)
> +     */
> +    bool host_cpu_probe_failed;
> +
>      /* The instance init functions for implementation-specific subclasses
>       * set these fields to specify the implementation-dependent values of
>       * various constant registers and reset values of non-constant
> diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
> index cfb7e5af72..1e2364007d 100644
> --- a/target/arm/kvm_arm.h
> +++ b/target/arm/kvm_arm.h
> @@ -152,20 +152,16 @@ bool kvm_arm_create_scratch_host_vcpu(const uint32_t *cpus_to_try,
>  void kvm_arm_destroy_scratch_host_vcpu(int *fdarray);
>
>  #define TYPE_ARM_HOST_CPU "host-" TYPE_ARM_CPU
> -#define ARM_HOST_CPU_CLASS(klass) \
> -    OBJECT_CLASS_CHECK(ARMHostCPUClass, (klass), TYPE_ARM_HOST_CPU)
> -#define ARM_HOST_CPU_GET_CLASS(obj) \
> -    OBJECT_GET_CLASS(ARMHostCPUClass, (obj), TYPE_ARM_HOST_CPU)
> -
> -typedef struct ARMHostCPUClass {
> -    /*< private >*/
> -    ARMCPUClass parent_class;
> -    /*< public >*/
>
> +/**
> + * ARMHostCPUFeatures: information about the host CPU (identified
> + * by asking the host kernel)
> + */
> +typedef struct ARMHostCPUFeatures {
>      uint64_t features;
>      uint32_t target;
>      const char *dtb_compatible;
> -} ARMHostCPUClass;
> +} ARMHostCPUFeatures;
>
>  /**
>   * kvm_arm_get_host_cpu_features:
> @@ -174,8 +170,16 @@ typedef struct ARMHostCPUClass {
>   * Probe the capabilities of the host kernel's preferred CPU and fill
>   * in the ARMHostCPUClass struct accordingly.
>   */
> -bool kvm_arm_get_host_cpu_features(ARMHostCPUClass *ahcc);
> +bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf);
>
> +/**
> + * kvm_arm_set_cpu_features_from_host:
> + * @cpu: ARMCPU to set the features for
> + *
> + * Set up the ARMCPU struct fields up to match the information probed
> + * from the host CPU.
> + */
> +void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu);
>
>  /**
>   * kvm_arm_sync_mpstate_to_kvm
> @@ -200,6 +204,15 @@ void kvm_arm_pmu_init(CPUState *cs);
>
>  #else
>
> +static inline void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu)
> +{
> +    /* This should never actually be called in the "not KVM" case,
> +     * but set up the fields to indicate an error anyway.
> +     */
> +    cpu->kvm_target = QEMU_KVM_ARM_TARGET_NONE;
> +    cpu->host_cpu_probe_failed = true;
> +}
> +
>  static inline int kvm_arm_vgic_probe(void)
>  {
>      return 0;
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 6b77aaa445..abf9fb2160 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -725,6 +725,19 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
>      int pagebits;
>      Error *local_err = NULL;
>
> +    /* If we needed to query the host kernel for the CPU features
> +     * then it's possible that might have failed in the initfn, but
> +     * this is the first point where we can report it.
> +     */
> +    if (cpu->host_cpu_probe_failed) {
> +        if (!kvm_enabled()) {
> +            error_setg(errp, "The 'host' CPU type can only be used with KVM");
> +        } else {
> +            error_setg(errp, "Failed to retrieve host CPU features");
> +        }
> +        return;
> +    }
> +
>      cpu_exec_realizefn(cs, &local_err);
>      if (local_err != NULL) {
>          error_propagate(errp, local_err);
> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> index 1219d0062b..1c0e57690a 100644
> --- a/target/arm/kvm.c
> +++ b/target/arm/kvm.c
> @@ -33,6 +33,8 @@ const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
>
>  static bool cap_has_mp_state;
>
> +static ARMHostCPUFeatures arm_host_cpu_features;
> +
>  int kvm_arm_vcpu_init(CPUState *cs)
>  {
>      ARMCPU *cpu = ARM_CPU(cs);
> @@ -129,30 +131,32 @@ void kvm_arm_destroy_scratch_host_vcpu(int *fdarray)
>      }
>  }
>
> -static void kvm_arm_host_cpu_class_init(ObjectClass *oc, void *data)
> +void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu)
>  {
> -    ARMHostCPUClass *ahcc = ARM_HOST_CPU_CLASS(oc);
> +    CPUARMState *env = &cpu->env;
>
> -    /* All we really need to set up for the 'host' CPU
> -     * is the feature bits -- we rely on the fact that the
> -     * various ID register values in ARMCPU are only used for
> -     * TCG CPUs.
> -     */
> -    if (!kvm_arm_get_host_cpu_features(ahcc)) {
> -        fprintf(stderr, "Failed to retrieve host CPU features!\n");
> -        abort();
> +    if (!arm_host_cpu_features.dtb_compatible) {
> +        if (!kvm_enabled() ||
> +            !kvm_arm_get_host_cpu_features(&arm_host_cpu_features)) {
> +            /* We can't report this error yet, so flag that we need to
> +             * in arm_cpu_realizefn().
> +             */
> +            cpu->kvm_target = QEMU_KVM_ARM_TARGET_NONE;
> +            cpu->host_cpu_probe_failed = true;
> +            return;
> +        }
>      }
> +
> +    cpu->kvm_target = arm_host_cpu_features.target;
> +    cpu->dtb_compatible = arm_host_cpu_features.dtb_compatible;
> +    env->features = arm_host_cpu_features.features;
>  }
>
>  static void kvm_arm_host_cpu_initfn(Object *obj)
>  {
> -    ARMHostCPUClass *ahcc = ARM_HOST_CPU_GET_CLASS(obj);
>      ARMCPU *cpu = ARM_CPU(obj);
> -    CPUARMState *env = &cpu->env;
>
> -    cpu->kvm_target = ahcc->target;
> -    cpu->dtb_compatible = ahcc->dtb_compatible;
> -    env->features = ahcc->features;
> +    kvm_arm_set_cpu_features_from_host(cpu);
>  }
>
>  static const TypeInfo host_arm_cpu_type_info = {
> @@ -163,8 +167,6 @@ static const TypeInfo host_arm_cpu_type_info = {
>      .parent = TYPE_ARM_CPU,
>  #endif
>      .instance_init = kvm_arm_host_cpu_initfn,
> -    .class_init = kvm_arm_host_cpu_class_init,
> -    .class_size = sizeof(ARMHostCPUClass),
>  };
>
>  int kvm_arch_init(MachineState *ms, KVMState *s)
> diff --git a/target/arm/kvm32.c b/target/arm/kvm32.c
> index f77c9c494b..1740cda47d 100644
> --- a/target/arm/kvm32.c
> +++ b/target/arm/kvm32.c
> @@ -28,7 +28,7 @@ static inline void set_feature(uint64_t *features, int feature)
>      *features |= 1ULL << feature;
>  }
>
> -bool kvm_arm_get_host_cpu_features(ARMHostCPUClass *ahcc)
> +bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>  {
>      /* Identify the feature bits corresponding to the host CPU, and
>       * fill out the ARMHostCPUClass fields accordingly. To do this
> @@ -74,13 +74,13 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUClass *ahcc)
>          return false;
>      }
>
> -    ahcc->target = init.target;
> +    ahcf->target = init.target;
>
>      /* This is not strictly blessed by the device tree binding docs yet,
>       * but in practice the kernel does not care about this string so
>       * there is no point maintaining an KVM_ARM_TARGET_* -> string table.
>       */
> -    ahcc->dtb_compatible = "arm,arm-v7";
> +    ahcf->dtb_compatible = "arm,arm-v7";
>
>      for (i = 0; i < ARRAY_SIZE(idregs); i++) {
>          ret = ioctl(fdarray[2], KVM_GET_ONE_REG, &idregs[i]);
> @@ -132,7 +132,7 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUClass *ahcc)
>          set_feature(&features, ARM_FEATURE_VFP4);
>      }
>
> -    ahcc->features = features;
> +    ahcf->features = features;
>
>      return true;
>  }
> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index ac728494a4..e0b8246283 100644
> --- a/target/arm/kvm64.c
> +++ b/target/arm/kvm64.c
> @@ -443,7 +443,7 @@ static inline void unset_feature(uint64_t *features, int feature)
>      *features &= ~(1ULL << feature);
>  }
>
> -bool kvm_arm_get_host_cpu_features(ARMHostCPUClass *ahcc)
> +bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>  {
>      /* Identify the feature bits corresponding to the host CPU, and
>       * fill out the ARMHostCPUClass fields accordingly. To do this
> @@ -471,8 +471,8 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUClass *ahcc)
>          return false;
>      }
>
> -    ahcc->target = init.target;
> -    ahcc->dtb_compatible = "arm,arm-v8";
> +    ahcf->target = init.target;
> +    ahcf->dtb_compatible = "arm,arm-v8";
>
>      kvm_arm_destroy_scratch_host_vcpu(fdarray);
>
> @@ -486,7 +486,7 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUClass *ahcc)
>      set_feature(&features, ARM_FEATURE_AARCH64);
>      set_feature(&features, ARM_FEATURE_PMU);
>
> -    ahcc->features = features;
> +    ahcf->features = features;
>
>      return true;
>  }


--
Alex Bennée

Re: [Qemu-devel] [PATCH v3 2/6] target/arm: Move definition of 'host' cpu type into cpu.c

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> Move the definition of the 'host' cpu type into cpu.c, where all the
> other CPU types are defined.  We can do this now we've decoupled it
> from the KVM-specific host feature probing.  This means we now create
> the type unconditionally (assuming we were built with KVM support at
> all), but if you try to use it without -enable-kvm this will end
> up in the "host cpu probe failed and KVM not enabled" path in
> arm_cpu_realizefn(), for an appropriate error message.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  target/arm/cpu.c | 24 ++++++++++++++++++++++++
>  target/arm/kvm.c | 19 -------------------
>  2 files changed, 24 insertions(+), 19 deletions(-)
>
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index abf9fb2160..1a58a2c094 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -1858,6 +1858,26 @@ static void arm_cpu_class_init(ObjectClass *oc, void *data)
>  #endif
>  }
>
> +#ifdef CONFIG_KVM
> +static void arm_host_initfn(Object *obj)
> +{
> +    ARMCPU *cpu = ARM_CPU(obj);
> +
> +    kvm_arm_set_cpu_features_from_host(cpu);
> +}
> +
> +static const TypeInfo host_arm_cpu_type_info = {
> +    .name = TYPE_ARM_HOST_CPU,
> +#ifdef TARGET_AARCH64
> +    .parent = TYPE_AARCH64_CPU,
> +#else
> +    .parent = TYPE_ARM_CPU,
> +#endif
> +    .instance_init = arm_host_initfn,
> +};
> +
> +#endif
> +
>  static void cpu_register(const ARMCPUInfo *info)
>  {
>      TypeInfo type_info = {
> @@ -1902,6 +1922,10 @@ static void arm_cpu_register_types(void)
>          cpu_register(info);
>          info++;
>      }
> +
> +#ifdef CONFIG_KVM
> +    type_register_static(&host_arm_cpu_type_info);
> +#endif
>  }
>
>  type_init(arm_cpu_register_types)
> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> index 1c0e57690a..ecc39ac295 100644
> --- a/target/arm/kvm.c
> +++ b/target/arm/kvm.c
> @@ -152,23 +152,6 @@ void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu)
>      env->features = arm_host_cpu_features.features;
>  }
>
> -static void kvm_arm_host_cpu_initfn(Object *obj)
> -{
> -    ARMCPU *cpu = ARM_CPU(obj);
> -
> -    kvm_arm_set_cpu_features_from_host(cpu);
> -}
> -
> -static const TypeInfo host_arm_cpu_type_info = {
> -    .name = TYPE_ARM_HOST_CPU,
> -#ifdef TARGET_AARCH64
> -    .parent = TYPE_AARCH64_CPU,
> -#else
> -    .parent = TYPE_ARM_CPU,
> -#endif
> -    .instance_init = kvm_arm_host_cpu_initfn,
> -};
> -
>  int kvm_arch_init(MachineState *ms, KVMState *s)
>  {
>      /* For ARM interrupt delivery is always asynchronous,
> @@ -184,8 +167,6 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>
>      cap_has_mp_state = kvm_check_extension(s, KVM_CAP_MP_STATE);
>
> -    type_register_static(&host_arm_cpu_type_info);
> -
>      return 0;
>  }


--
Alex Bennée

Re: [Qemu-devel] [PATCH v3 3/6] target/arm: Add "-cpu max" support

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> Add support for "-cpu max" for ARM guests. This CPU type behaves
> like "-cpu host" when KVM is enabled, and like a system CPU with
> the maximum possible feature set otherwise. (Note that this means
> it won't be migratable across versions, as we will likely add
> features to it in future.)
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  target/arm/cpu-qom.h |  2 ++
>  target/arm/cpu.c     | 24 ++++++++++++++++++++++++
>  target/arm/cpu64.c   | 21 +++++++++++++++++++++
>  3 files changed, 47 insertions(+)
>
> diff --git a/target/arm/cpu-qom.h b/target/arm/cpu-qom.h
> index a42495bac9..d135ff8e06 100644
> --- a/target/arm/cpu-qom.h
> +++ b/target/arm/cpu-qom.h
> @@ -33,6 +33,8 @@ struct arm_boot_info;
>  #define ARM_CPU_GET_CLASS(obj) \
>      OBJECT_GET_CLASS(ARMCPUClass, (obj), TYPE_ARM_CPU)
>
> +#define TYPE_ARM_MAX_CPU "max-" TYPE_ARM_CPU
> +
>  /**
>   * ARMCPUClass:
>   * @parent_realize: The parent class' realize handler.
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 1a58a2c094..e46ddcc613 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -1697,6 +1697,27 @@ static void pxa270c5_initfn(Object *obj)
>      cpu->reset_sctlr = 0x00000078;
>  }
>
> +#ifndef TARGET_AARCH64
> +/* -cpu max: if KVM is enabled, like -cpu host (best possible with this host);
> + * otherwise, a CPU with as many features enabled as our emulation supports.
> + * The version of '-cpu max' for qemu-system-aarch64 is defined in cpu64.c;
> + * this only needs to handle 32 bits.
> + */
> +static void arm_max_initfn(Object *obj)
> +{
> +    ARMCPU *cpu = ARM_CPU(obj);
> +
> +    if (kvm_enabled()) {
> +        kvm_arm_set_cpu_features_from_host(cpu);
> +    } else {
> +        cortex_a15_initfn(obj);
> +        /* In future we might add feature bits here even if the
> +         * real-world A15 doesn't implement them.
> +         */
> +    }
> +}
> +#endif
> +
>  #ifdef CONFIG_USER_ONLY
>  static void arm_any_initfn(Object *obj)
>  {
> @@ -1764,6 +1785,9 @@ static const ARMCPUInfo arm_cpus[] = {
>      { .name = "pxa270-b1",   .initfn = pxa270b1_initfn },
>      { .name = "pxa270-c0",   .initfn = pxa270c0_initfn },
>      { .name = "pxa270-c5",   .initfn = pxa270c5_initfn },
> +#ifndef TARGET_AARCH64
> +    { .name = "max",         .initfn = arm_max_initfn },
> +#endif
>  #ifdef CONFIG_USER_ONLY
>      { .name = "any",         .initfn = arm_any_initfn },
>  #endif
> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> index 4228713b19..9042d3dfd1 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -28,6 +28,7 @@
>  #include "hw/arm/arm.h"
>  #include "sysemu/sysemu.h"
>  #include "sysemu/kvm.h"
> +#include "kvm_arm.h"
>
>  static inline void set_feature(CPUARMState *env, int feature)
>  {
> @@ -212,6 +213,25 @@ static void aarch64_a53_initfn(Object *obj)
>      define_arm_cp_regs(cpu, cortex_a57_a53_cp_reginfo);
>  }
>
> +/* -cpu max: if KVM is enabled, like -cpu host (best possible with this host);
> + * otherwise, a CPU with as many features enabled as our emulation supports.
> + * The version of '-cpu max' for qemu-system-arm is defined in cpu.c;
> + * this only needs to handle 64 bits.
> + */
> +static void aarch64_max_initfn(Object *obj)
> +{
> +    ARMCPU *cpu = ARM_CPU(obj);
> +
> +    if (kvm_enabled()) {
> +        kvm_arm_set_cpu_features_from_host(cpu);
> +    } else {
> +        aarch64_a57_initfn(obj);
> +        /* In future we might add feature bits here even if the
> +         * real-world A57 doesn't implement them.
> +         */
> +    }
> +}
> +
>  #ifdef CONFIG_USER_ONLY
>  static void aarch64_any_initfn(Object *obj)
>  {
> @@ -247,6 +267,7 @@ typedef struct ARMCPUInfo {
>  static const ARMCPUInfo aarch64_cpus[] = {
>      { .name = "cortex-a57",         .initfn = aarch64_a57_initfn },
>      { .name = "cortex-a53",         .initfn = aarch64_a53_initfn },
> +    { .name = "max",                .initfn = aarch64_max_initfn },
>  #ifdef CONFIG_USER_ONLY
>      { .name = "any",         .initfn = aarch64_any_initfn },
>  #endif


--
Alex Bennée

Re: [Qemu-devel] [PATCH v3 4/6] target/arm: Make 'any' CPU just an alias for 'max'

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> Now we have a working '-cpu max', the linux-user-only
> 'any' CPU is pretty much the same thing, so implement it
> that way.
>
> For the moment we don't add any of the extra feature bits
> to the system-emulation "max", because we don't set the
> ID register bits we would need to to advertise those
> features as present.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  target/arm/cpu.c   | 52 +++++++++++++++++++++++++----------------------
>  target/arm/cpu64.c | 59 ++++++++++++++++++++++++++----------------------------
>  2 files changed, 56 insertions(+), 55 deletions(-)
>
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index e46ddcc613..6068efb8ff 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -965,9 +965,19 @@ static ObjectClass *arm_cpu_class_by_name(const char *cpu_model)
>      ObjectClass *oc;
>      char *typename;
>      char **cpuname;
> +    const char *cpunamestr;
>
>      cpuname = g_strsplit(cpu_model, ",", 1);
> -    typename = g_strdup_printf(ARM_CPU_TYPE_NAME("%s"), cpuname[0]);
> +    cpunamestr = cpuname[0];
> +#ifdef CONFIG_USER_ONLY
> +    /* For backwards compatibility usermode emulation allows "-cpu any",
> +     * which has the same semantics as "-cpu max".
> +     */
> +    if (!strcmp(cpunamestr, "any")) {
> +        cpunamestr = "max";
> +    }
> +#endif
> +    typename = g_strdup_printf(ARM_CPU_TYPE_NAME("%s"), cpunamestr);
>      oc = object_class_by_name(typename);
>      g_strfreev(cpuname);
>      g_free(typename);
> @@ -1711,29 +1721,23 @@ static void arm_max_initfn(Object *obj)
>          kvm_arm_set_cpu_features_from_host(cpu);
>      } else {
>          cortex_a15_initfn(obj);
> -        /* In future we might add feature bits here even if the
> -         * real-world A15 doesn't implement them.
> -         */
> -    }
> -}
> -#endif
> -
>  #ifdef CONFIG_USER_ONLY
> -static void arm_any_initfn(Object *obj)
> -{
> -    ARMCPU *cpu = ARM_CPU(obj);
> -    set_feature(&cpu->env, ARM_FEATURE_V8);
> -    set_feature(&cpu->env, ARM_FEATURE_VFP4);
> -    set_feature(&cpu->env, ARM_FEATURE_NEON);
> -    set_feature(&cpu->env, ARM_FEATURE_THUMB2EE);
> -    set_feature(&cpu->env, ARM_FEATURE_V8_AES);
> -    set_feature(&cpu->env, ARM_FEATURE_V8_SHA1);
> -    set_feature(&cpu->env, ARM_FEATURE_V8_SHA256);
> -    set_feature(&cpu->env, ARM_FEATURE_V8_PMULL);
> -    set_feature(&cpu->env, ARM_FEATURE_CRC);
> -    set_feature(&cpu->env, ARM_FEATURE_V8_RDM);
> -    set_feature(&cpu->env, ARM_FEATURE_V8_FCMA);
> -    cpu->midr = 0xffffffff;
> +        /* We don't set these in system emulation mode for the moment,
> +         * since we don't correctly set the ID registers to advertise them,
> +         */
> +        set_feature(&cpu->env, ARM_FEATURE_V8);
> +        set_feature(&cpu->env, ARM_FEATURE_VFP4);
> +        set_feature(&cpu->env, ARM_FEATURE_NEON);
> +        set_feature(&cpu->env, ARM_FEATURE_THUMB2EE);
> +        set_feature(&cpu->env, ARM_FEATURE_V8_AES);
> +        set_feature(&cpu->env, ARM_FEATURE_V8_SHA1);
> +        set_feature(&cpu->env, ARM_FEATURE_V8_SHA256);
> +        set_feature(&cpu->env, ARM_FEATURE_V8_PMULL);
> +        set_feature(&cpu->env, ARM_FEATURE_CRC);
> +        set_feature(&cpu->env, ARM_FEATURE_V8_RDM);
> +        set_feature(&cpu->env, ARM_FEATURE_V8_FCMA);
> +#endif
> +    }
>  }
>  #endif
>
> @@ -1789,7 +1793,7 @@ static const ARMCPUInfo arm_cpus[] = {
>      { .name = "max",         .initfn = arm_max_initfn },
>  #endif
>  #ifdef CONFIG_USER_ONLY
> -    { .name = "any",         .initfn = arm_any_initfn },
> +    { .name = "any",         .initfn = arm_max_initfn },
>  #endif
>  #endif
>      { .name = NULL }
> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> index 9042d3dfd1..3429875787 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -226,38 +226,38 @@ static void aarch64_max_initfn(Object *obj)
>          kvm_arm_set_cpu_features_from_host(cpu);
>      } else {
>          aarch64_a57_initfn(obj);
> -        /* In future we might add feature bits here even if the
> -         * real-world A57 doesn't implement them.
> +#ifdef CONFIG_USER_ONLY
> +        /* We don't set these in system emulation mode for the moment,
> +         * since we don't correctly set the ID registers to advertise them,
> +         * and in some cases they're only available in AArch64 and not AArch32,
> +         * whereas the architecture requires them to be present in both if
> +         * present in either.
>           */
> +        set_feature(&cpu->env, ARM_FEATURE_V8);
> +        set_feature(&cpu->env, ARM_FEATURE_VFP4);
> +        set_feature(&cpu->env, ARM_FEATURE_NEON);
> +        set_feature(&cpu->env, ARM_FEATURE_AARCH64);
> +        set_feature(&cpu->env, ARM_FEATURE_V8_AES);
> +        set_feature(&cpu->env, ARM_FEATURE_V8_SHA1);
> +        set_feature(&cpu->env, ARM_FEATURE_V8_SHA256);
> +        set_feature(&cpu->env, ARM_FEATURE_V8_SHA512);
> +        set_feature(&cpu->env, ARM_FEATURE_V8_SHA3);
> +        set_feature(&cpu->env, ARM_FEATURE_V8_SM3);
> +        set_feature(&cpu->env, ARM_FEATURE_V8_SM4);
> +        set_feature(&cpu->env, ARM_FEATURE_V8_PMULL);
> +        set_feature(&cpu->env, ARM_FEATURE_CRC);
> +        set_feature(&cpu->env, ARM_FEATURE_V8_RDM);
> +        set_feature(&cpu->env, ARM_FEATURE_V8_FP16);
> +        set_feature(&cpu->env, ARM_FEATURE_V8_FCMA);
> +        /* For usermode -cpu max we can use a larger and more efficient DCZ
> +         * blocksize since we don't have to follow what the hardware does.
> +         */
> +        cpu->ctr = 0x80038003; /* 32 byte I and D cacheline size, VIPT icache */
> +        cpu->dcz_blocksize = 7; /*  512 bytes */
> +#endif
>      }
>  }
>
> -#ifdef CONFIG_USER_ONLY
> -static void aarch64_any_initfn(Object *obj)
> -{
> -    ARMCPU *cpu = ARM_CPU(obj);
> -
> -    set_feature(&cpu->env, ARM_FEATURE_V8);
> -    set_feature(&cpu->env, ARM_FEATURE_VFP4);
> -    set_feature(&cpu->env, ARM_FEATURE_NEON);
> -    set_feature(&cpu->env, ARM_FEATURE_AARCH64);
> -    set_feature(&cpu->env, ARM_FEATURE_V8_AES);
> -    set_feature(&cpu->env, ARM_FEATURE_V8_SHA1);
> -    set_feature(&cpu->env, ARM_FEATURE_V8_SHA256);
> -    set_feature(&cpu->env, ARM_FEATURE_V8_SHA512);
> -    set_feature(&cpu->env, ARM_FEATURE_V8_SHA3);
> -    set_feature(&cpu->env, ARM_FEATURE_V8_SM3);
> -    set_feature(&cpu->env, ARM_FEATURE_V8_SM4);
> -    set_feature(&cpu->env, ARM_FEATURE_V8_PMULL);
> -    set_feature(&cpu->env, ARM_FEATURE_CRC);
> -    set_feature(&cpu->env, ARM_FEATURE_V8_RDM);
> -    set_feature(&cpu->env, ARM_FEATURE_V8_FP16);
> -    set_feature(&cpu->env, ARM_FEATURE_V8_FCMA);
> -    cpu->ctr = 0x80038003; /* 32 byte I and D cacheline size, VIPT icache */
> -    cpu->dcz_blocksize = 7; /*  512 bytes */
> -}
> -#endif
> -
>  typedef struct ARMCPUInfo {
>      const char *name;
>      void (*initfn)(Object *obj);
> @@ -268,9 +268,6 @@ static const ARMCPUInfo aarch64_cpus[] = {
>      { .name = "cortex-a57",         .initfn = aarch64_a57_initfn },
>      { .name = "cortex-a53",         .initfn = aarch64_a53_initfn },
>      { .name = "max",                .initfn = aarch64_max_initfn },
> -#ifdef CONFIG_USER_ONLY
> -    { .name = "any",         .initfn = aarch64_any_initfn },
> -#endif
>      { .name = NULL }
>  };


--
Alex Bennée

Re: [Qemu-devel] [PATCH v3 5/6] hw/arm/virt: Add "max" to the list of CPU types "virt" supports

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> Allow the virt board to support '-cpu max' in the same way
> it already handles '-cpu host'.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  hw/arm/virt.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index dbb3c8036a..cda4b83586 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -169,6 +169,7 @@ static const char *valid_cpus[] = {
>      ARM_CPU_TYPE_NAME("cortex-a53"),
>      ARM_CPU_TYPE_NAME("cortex-a57"),
>      ARM_CPU_TYPE_NAME("host"),
> +    ARM_CPU_TYPE_NAME("max"),
>  };
>
>  static bool cpu_type_valid(const char *cpu)


--
Alex Bennée

Re: [Qemu-devel] [PATCH v3 6/6] hw/arm/virt: Support -machine gic-version=max

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> Add support for passing 'max' to -machine gic-version. By analogy
> with the -cpu max option, this picks the "best available" GIC version
> whether you're using KVM or TCG, so it behaves like 'host' when
> using KVM, and gives you GICv3 when using TCG.
>
> Also like '-cpu host', using -machine gic-version=max' means there
> is no guarantee of migration compatibility between QEMU versions;
> in future 'max' might mean '4'.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/arm/virt.c | 29 +++++++++++++++++++----------
>  1 file changed, 19 insertions(+), 10 deletions(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index cda4b83586..2c07245047 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1207,16 +1207,23 @@ static void machvirt_init(MachineState *machine)
>      /* We can probe only here because during property set
>       * KVM is not available yet
>       */
> -    if (!vms->gic_version) {
> +    if (vms->gic_version <= 0) {
> +        /* "host" or "max" */
>          if (!kvm_enabled()) {
> -            error_report("gic-version=host requires KVM");
> -            exit(1);
> -        }
> -
> -        vms->gic_version = kvm_arm_vgic_probe();
> -        if (!vms->gic_version) {
> -            error_report("Unable to determine GIC version supported by host");
> -            exit(1);
> +            if (vms->gic_version == 0) {
> +                error_report("gic-version=host requires KVM");
> +                exit(1);
> +            } else {
> +                /* "max": currently means 3 for TCG */
> +                vms->gic_version = 3;
> +            }
> +        } else {
> +            vms->gic_version = kvm_arm_vgic_probe();
> +            if (!vms->gic_version) {
> +                error_report(
> +                    "Unable to determine GIC version supported by host");
> +                exit(1);
> +            }
>          }
>      }
>
> @@ -1480,9 +1487,11 @@ static void virt_set_gic_version(Object *obj, const char *value, Error **errp)
>          vms->gic_version = 2;
>      } else if (!strcmp(value, "host")) {
>          vms->gic_version = 0; /* Will probe later */
> +    } else if (!strcmp(value, "max")) {
> +        vms->gic_version = -1; /* Will probe later */
>      } else {
>          error_setg(errp, "Invalid gic-version value");
> -        error_append_hint(errp, "Valid values are 3, 2, host.\n");
> +        error_append_hint(errp, "Valid values are 3, 2, host,
> max.\n");

nit: host is only valid on aarch64 host. However it will fail later on
with:

  qemu-system-aarch64: gic-version=host requires KVM

Anyway:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

--
Alex Bennée

Re: [Qemu-devel] [PATCH v3 0/6] arm: support -cpu max (and gic-version=max)

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> This patchset adds support for '-cpu max' to Arm, along the lines
> of the existing support we have for x86 targets:
>
>  * under KVM, -cpu max is the same as -cpu host
>  * under TCG, -cpu max means "emulate with as many features as
>    possible"
<snip>

It all looks good to me. The only minor problem I ran into was setting
-M virt,gic-version=max silently hangs with the stock QEMU_EFI.fd on my
version of Ubuntu. It would be nice if there was better feedback we
could give to the user in this case.

It works with the latest snapshot from:

  http://snapshots.linaro.org/reference-platform/enterprise/firmware/15/release/qemu-aarch64/

Anyway this series can have:

Tested-by: Alex Bennée <alex.bennee@linaro.org>

--
Alex Bennée

Re: [Qemu-devel] [PATCH v3 6/6] hw/arm/virt: Support -machine gic-version=max

[Peter Maydell]

On 9 March 2018 at 13:50, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>> @@ -1480,9 +1487,11 @@ static void virt_set_gic_version(Object *obj, const char *value, Error **errp)
>>          vms->gic_version = 2;
>>      } else if (!strcmp(value, "host")) {
>>          vms->gic_version = 0; /* Will probe later */
>> +    } else if (!strcmp(value, "max")) {
>> +        vms->gic_version = -1; /* Will probe later */
>>      } else {
>>          error_setg(errp, "Invalid gic-version value");
>> -        error_append_hint(errp, "Valid values are 3, 2, host.\n");
>> +        error_append_hint(errp, "Valid values are 3, 2, host,
>> max.\n");
>
> nit: host is only valid on aarch64 host. However it will fail later on
> with:
>
>   qemu-system-aarch64: gic-version=host requires KVM

'host' was already in this string. It's valid in the sense that
it's not a syntax error, though it won't work in all situations.
(Similarly, if you try '3' and you have KVM enabled and a gicv2
host system you'll get an error.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 0/6] arm: support -cpu max (and gic-version=max)

[Peter Maydell]

On 9 March 2018 at 13:52, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>
>> This patchset adds support for '-cpu max' to Arm, along the lines
>> of the existing support we have for x86 targets:
>>
>>  * under KVM, -cpu max is the same as -cpu host
>>  * under TCG, -cpu max means "emulate with as many features as
>>    possible"
> <snip>
>
> It all looks good to me. The only minor problem I ran into was setting
> -M virt,gic-version=max silently hangs with the stock QEMU_EFI.fd on my
> version of Ubuntu. It would be nice if there was better feedback we
> could give to the user in this case.

Presumably that also fails if you ask for gic-version=3 ?

> It works with the latest snapshot from:
>
>   http://snapshots.linaro.org/reference-platform/enterprise/firmware/15/release/qemu-aarch64/

Sounds like "earlier firmware versions didn't have gicv3 support,
later ones do" ?

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 3/6] target/arm: Add "-cpu max" support

[Philippe Mathieu-Daudé]

On 03/08/2018 02:06 PM, Peter Maydell wrote:
> Add support for "-cpu max" for ARM guests. This CPU type behaves
> like "-cpu host" when KVM is enabled, and like a system CPU with
> the maximum possible feature set otherwise. (Note that this means
> it won't be migratable across versions, as we will likely add
> features to it in future.)
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  target/arm/cpu-qom.h |  2 ++
>  target/arm/cpu.c     | 24 ++++++++++++++++++++++++
>  target/arm/cpu64.c   | 21 +++++++++++++++++++++
>  3 files changed, 47 insertions(+)
> 
> diff --git a/target/arm/cpu-qom.h b/target/arm/cpu-qom.h
> index a42495bac9..d135ff8e06 100644
> --- a/target/arm/cpu-qom.h
> +++ b/target/arm/cpu-qom.h
> @@ -33,6 +33,8 @@ struct arm_boot_info;
>  #define ARM_CPU_GET_CLASS(obj) \
>      OBJECT_GET_CLASS(ARMCPUClass, (obj), TYPE_ARM_CPU)
>  
> +#define TYPE_ARM_MAX_CPU "max-" TYPE_ARM_CPU
> +
>  /**
>   * ARMCPUClass:
>   * @parent_realize: The parent class' realize handler.
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 1a58a2c094..e46ddcc613 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -1697,6 +1697,27 @@ static void pxa270c5_initfn(Object *obj)
>      cpu->reset_sctlr = 0x00000078;
>  }
>  
> +#ifndef TARGET_AARCH64
> +/* -cpu max: if KVM is enabled, like -cpu host (best possible with this host);
> + * otherwise, a CPU with as many features enabled as our emulation supports.
> + * The version of '-cpu max' for qemu-system-aarch64 is defined in cpu64.c;
> + * this only needs to handle 32 bits.
> + */
> +static void arm_max_initfn(Object *obj)
> +{
> +    ARMCPU *cpu = ARM_CPU(obj);
> +
> +    if (kvm_enabled()) {
> +        kvm_arm_set_cpu_features_from_host(cpu);
> +    } else {
> +        cortex_a15_initfn(obj);
> +        /* In future we might add feature bits here even if the
> +         * real-world A15 doesn't implement them.
> +         */
> +    }
> +}
> +#endif
> +
>  #ifdef CONFIG_USER_ONLY
>  static void arm_any_initfn(Object *obj)
>  {
> @@ -1764,6 +1785,9 @@ static const ARMCPUInfo arm_cpus[] = {
>      { .name = "pxa270-b1",   .initfn = pxa270b1_initfn },
>      { .name = "pxa270-c0",   .initfn = pxa270c0_initfn },
>      { .name = "pxa270-c5",   .initfn = pxa270c5_initfn },
> +#ifndef TARGET_AARCH64
> +    { .name = "max",         .initfn = arm_max_initfn },
> +#endif
>  #ifdef CONFIG_USER_ONLY
>      { .name = "any",         .initfn = arm_any_initfn },
>  #endif
> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> index 4228713b19..9042d3dfd1 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -28,6 +28,7 @@
>  #include "hw/arm/arm.h"
>  #include "sysemu/sysemu.h"
>  #include "sysemu/kvm.h"
> +#include "kvm_arm.h"
>  
>  static inline void set_feature(CPUARMState *env, int feature)
>  {
> @@ -212,6 +213,25 @@ static void aarch64_a53_initfn(Object *obj)
>      define_arm_cp_regs(cpu, cortex_a57_a53_cp_reginfo);
>  }
>  
> +/* -cpu max: if KVM is enabled, like -cpu host (best possible with this host);
> + * otherwise, a CPU with as many features enabled as our emulation supports.
> + * The version of '-cpu max' for qemu-system-arm is defined in cpu.c;
> + * this only needs to handle 64 bits.
> + */
> +static void aarch64_max_initfn(Object *obj)
> +{
> +    ARMCPU *cpu = ARM_CPU(obj);
> +
> +    if (kvm_enabled()) {
> +        kvm_arm_set_cpu_features_from_host(cpu);
> +    } else {
> +        aarch64_a57_initfn(obj);
> +        /* In future we might add feature bits here even if the
> +         * real-world A57 doesn't implement them.
> +         */
> +    }
> +}
> +
>  #ifdef CONFIG_USER_ONLY
>  static void aarch64_any_initfn(Object *obj)
>  {
> @@ -247,6 +267,7 @@ typedef struct ARMCPUInfo {
>  static const ARMCPUInfo aarch64_cpus[] = {
>      { .name = "cortex-a57",         .initfn = aarch64_a57_initfn },
>      { .name = "cortex-a53",         .initfn = aarch64_a53_initfn },
> +    { .name = "max",                .initfn = aarch64_max_initfn },
>  #ifdef CONFIG_USER_ONLY
>      { .name = "any",         .initfn = aarch64_any_initfn },
>  #endif
> 

Re: [Qemu-devel] [PATCH v3 4/6] target/arm: Make 'any' CPU just an alias for 'max'

[Philippe Mathieu-Daudé]

On 03/08/2018 02:06 PM, Peter Maydell wrote:
> Now we have a working '-cpu max', the linux-user-only
> 'any' CPU is pretty much the same thing, so implement it
> that way.
> 
> For the moment we don't add any of the extra feature bits
> to the system-emulation "max", because we don't set the
> ID register bits we would need to to advertise those
> features as present.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  target/arm/cpu.c   | 52 +++++++++++++++++++++++++----------------------
>  target/arm/cpu64.c | 59 ++++++++++++++++++++++++++----------------------------
>  2 files changed, 56 insertions(+), 55 deletions(-)
> 
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index e46ddcc613..6068efb8ff 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -965,9 +965,19 @@ static ObjectClass *arm_cpu_class_by_name(const char *cpu_model)
>      ObjectClass *oc;
>      char *typename;
>      char **cpuname;
> +    const char *cpunamestr;
>  
>      cpuname = g_strsplit(cpu_model, ",", 1);
> -    typename = g_strdup_printf(ARM_CPU_TYPE_NAME("%s"), cpuname[0]);
> +    cpunamestr = cpuname[0];
> +#ifdef CONFIG_USER_ONLY
> +    /* For backwards compatibility usermode emulation allows "-cpu any",
> +     * which has the same semantics as "-cpu max".
> +     */
> +    if (!strcmp(cpunamestr, "any")) {
> +        cpunamestr = "max";
> +    }
> +#endif
> +    typename = g_strdup_printf(ARM_CPU_TYPE_NAME("%s"), cpunamestr);
>      oc = object_class_by_name(typename);
>      g_strfreev(cpuname);
>      g_free(typename);
> @@ -1711,29 +1721,23 @@ static void arm_max_initfn(Object *obj)
>          kvm_arm_set_cpu_features_from_host(cpu);
>      } else {
>          cortex_a15_initfn(obj);
> -        /* In future we might add feature bits here even if the
> -         * real-world A15 doesn't implement them.
> -         */
> -    }
> -}
> -#endif
> -
>  #ifdef CONFIG_USER_ONLY
> -static void arm_any_initfn(Object *obj)
> -{
> -    ARMCPU *cpu = ARM_CPU(obj);
> -    set_feature(&cpu->env, ARM_FEATURE_V8);
> -    set_feature(&cpu->env, ARM_FEATURE_VFP4);
> -    set_feature(&cpu->env, ARM_FEATURE_NEON);
> -    set_feature(&cpu->env, ARM_FEATURE_THUMB2EE);
> -    set_feature(&cpu->env, ARM_FEATURE_V8_AES);
> -    set_feature(&cpu->env, ARM_FEATURE_V8_SHA1);
> -    set_feature(&cpu->env, ARM_FEATURE_V8_SHA256);
> -    set_feature(&cpu->env, ARM_FEATURE_V8_PMULL);
> -    set_feature(&cpu->env, ARM_FEATURE_CRC);
> -    set_feature(&cpu->env, ARM_FEATURE_V8_RDM);
> -    set_feature(&cpu->env, ARM_FEATURE_V8_FCMA);
> -    cpu->midr = 0xffffffff;
> +        /* We don't set these in system emulation mode for the moment,
> +         * since we don't correctly set the ID registers to advertise them,
> +         */
> +        set_feature(&cpu->env, ARM_FEATURE_V8);
> +        set_feature(&cpu->env, ARM_FEATURE_VFP4);
> +        set_feature(&cpu->env, ARM_FEATURE_NEON);
> +        set_feature(&cpu->env, ARM_FEATURE_THUMB2EE);
> +        set_feature(&cpu->env, ARM_FEATURE_V8_AES);
> +        set_feature(&cpu->env, ARM_FEATURE_V8_SHA1);
> +        set_feature(&cpu->env, ARM_FEATURE_V8_SHA256);
> +        set_feature(&cpu->env, ARM_FEATURE_V8_PMULL);
> +        set_feature(&cpu->env, ARM_FEATURE_CRC);
> +        set_feature(&cpu->env, ARM_FEATURE_V8_RDM);
> +        set_feature(&cpu->env, ARM_FEATURE_V8_FCMA);
> +#endif
> +    }
>  }
>  #endif
>  
> @@ -1789,7 +1793,7 @@ static const ARMCPUInfo arm_cpus[] = {
>      { .name = "max",         .initfn = arm_max_initfn },
>  #endif
>  #ifdef CONFIG_USER_ONLY
> -    { .name = "any",         .initfn = arm_any_initfn },
> +    { .name = "any",         .initfn = arm_max_initfn },
>  #endif
>  #endif
>      { .name = NULL }
> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> index 9042d3dfd1..3429875787 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -226,38 +226,38 @@ static void aarch64_max_initfn(Object *obj)
>          kvm_arm_set_cpu_features_from_host(cpu);
>      } else {
>          aarch64_a57_initfn(obj);
> -        /* In future we might add feature bits here even if the
> -         * real-world A57 doesn't implement them.
> +#ifdef CONFIG_USER_ONLY
> +        /* We don't set these in system emulation mode for the moment,
> +         * since we don't correctly set the ID registers to advertise them,
> +         * and in some cases they're only available in AArch64 and not AArch32,
> +         * whereas the architecture requires them to be present in both if
> +         * present in either.
>           */
> +        set_feature(&cpu->env, ARM_FEATURE_V8);
> +        set_feature(&cpu->env, ARM_FEATURE_VFP4);
> +        set_feature(&cpu->env, ARM_FEATURE_NEON);
> +        set_feature(&cpu->env, ARM_FEATURE_AARCH64);
> +        set_feature(&cpu->env, ARM_FEATURE_V8_AES);
> +        set_feature(&cpu->env, ARM_FEATURE_V8_SHA1);
> +        set_feature(&cpu->env, ARM_FEATURE_V8_SHA256);
> +        set_feature(&cpu->env, ARM_FEATURE_V8_SHA512);
> +        set_feature(&cpu->env, ARM_FEATURE_V8_SHA3);
> +        set_feature(&cpu->env, ARM_FEATURE_V8_SM3);
> +        set_feature(&cpu->env, ARM_FEATURE_V8_SM4);
> +        set_feature(&cpu->env, ARM_FEATURE_V8_PMULL);
> +        set_feature(&cpu->env, ARM_FEATURE_CRC);
> +        set_feature(&cpu->env, ARM_FEATURE_V8_RDM);
> +        set_feature(&cpu->env, ARM_FEATURE_V8_FP16);
> +        set_feature(&cpu->env, ARM_FEATURE_V8_FCMA);
> +        /* For usermode -cpu max we can use a larger and more efficient DCZ
> +         * blocksize since we don't have to follow what the hardware does.
> +         */
> +        cpu->ctr = 0x80038003; /* 32 byte I and D cacheline size, VIPT icache */
> +        cpu->dcz_blocksize = 7; /*  512 bytes */
> +#endif
>      }
>  }
>  
> -#ifdef CONFIG_USER_ONLY
> -static void aarch64_any_initfn(Object *obj)
> -{
> -    ARMCPU *cpu = ARM_CPU(obj);
> -
> -    set_feature(&cpu->env, ARM_FEATURE_V8);
> -    set_feature(&cpu->env, ARM_FEATURE_VFP4);
> -    set_feature(&cpu->env, ARM_FEATURE_NEON);
> -    set_feature(&cpu->env, ARM_FEATURE_AARCH64);
> -    set_feature(&cpu->env, ARM_FEATURE_V8_AES);
> -    set_feature(&cpu->env, ARM_FEATURE_V8_SHA1);
> -    set_feature(&cpu->env, ARM_FEATURE_V8_SHA256);
> -    set_feature(&cpu->env, ARM_FEATURE_V8_SHA512);
> -    set_feature(&cpu->env, ARM_FEATURE_V8_SHA3);
> -    set_feature(&cpu->env, ARM_FEATURE_V8_SM3);
> -    set_feature(&cpu->env, ARM_FEATURE_V8_SM4);
> -    set_feature(&cpu->env, ARM_FEATURE_V8_PMULL);
> -    set_feature(&cpu->env, ARM_FEATURE_CRC);
> -    set_feature(&cpu->env, ARM_FEATURE_V8_RDM);
> -    set_feature(&cpu->env, ARM_FEATURE_V8_FP16);
> -    set_feature(&cpu->env, ARM_FEATURE_V8_FCMA);
> -    cpu->ctr = 0x80038003; /* 32 byte I and D cacheline size, VIPT icache */
> -    cpu->dcz_blocksize = 7; /*  512 bytes */
> -}
> -#endif
> -
>  typedef struct ARMCPUInfo {
>      const char *name;
>      void (*initfn)(Object *obj);
> @@ -268,9 +268,6 @@ static const ARMCPUInfo aarch64_cpus[] = {
>      { .name = "cortex-a57",         .initfn = aarch64_a57_initfn },
>      { .name = "cortex-a53",         .initfn = aarch64_a53_initfn },
>      { .name = "max",                .initfn = aarch64_max_initfn },
> -#ifdef CONFIG_USER_ONLY
> -    { .name = "any",         .initfn = aarch64_any_initfn },
> -#endif
>      { .name = NULL }
>  };
>  
> 

Re: [Qemu-devel] [PATCH v3 5/6] hw/arm/virt: Add "max" to the list of CPU types "virt" supports

[Philippe Mathieu-Daudé]

On 03/08/2018 02:06 PM, Peter Maydell wrote:
> Allow the virt board to support '-cpu max' in the same way
> it already handles '-cpu host'.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  hw/arm/virt.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index dbb3c8036a..cda4b83586 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -169,6 +169,7 @@ static const char *valid_cpus[] = {
>      ARM_CPU_TYPE_NAME("cortex-a53"),
>      ARM_CPU_TYPE_NAME("cortex-a57"),
>      ARM_CPU_TYPE_NAME("host"),
> +    ARM_CPU_TYPE_NAME("max"),
>  };
>  
>  static bool cpu_type_valid(const char *cpu)
> 

Re: [Qemu-devel] [PATCH v3 0/6] arm: support -cpu max (and gic-version=max)

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> On 9 March 2018 at 13:52, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> Peter Maydell <peter.maydell@linaro.org> writes:
>>
>>> This patchset adds support for '-cpu max' to Arm, along the lines
>>> of the existing support we have for x86 targets:
>>>
>>>  * under KVM, -cpu max is the same as -cpu host
>>>  * under TCG, -cpu max means "emulate with as many features as
>>>    possible"
>> <snip>
>>
>> It all looks good to me. The only minor problem I ran into was setting
>> -M virt,gic-version=max silently hangs with the stock QEMU_EFI.fd on my
>> version of Ubuntu. It would be nice if there was better feedback we
>> could give to the user in this case.
>
> Presumably that also fails if you ask for gic-version=3 ?

Yes.

>
>> It works with the latest snapshot from:
>>
>>   http://snapshots.linaro.org/reference-platform/enterprise/firmware/15/release/qemu-aarch64/
>
> Sounds like "earlier firmware versions didn't have gicv3 support,
> later ones do" ?

According to the dpkg changelog it should have:

  Adds GICv3 support.  Closes: #810495

However it seems like it wasn't complete support.

--
Alex Bennée