[PATCH 0/6] target/arm: -cpu host/max KVM and HVF fixes

[PATCH 0/6] target/arm: -cpu host/max KVM and HVF fixes

[Peter Maydell]

This patchset fixes various minor bugs in KVM and HVF
-cpu host and -cpu max:

(1) KVM -cpu max was incorrectly adding a 'sve-max-vq' property
that wouldn't work and which doesn't exist in KVM -cpu host

(2) HVF -cpu max was using all the TCG ID fields and thus
promising the guest more than the host CPU can actually do

(3) HVF -cpu host wasn't setting up the PAuth properties, so
it defaulted to not telling the guest about PAuth support

This series fixes (1) by moving the '-cpu host' code to
cpu64.c since it's aarch64-specific anyway, which lets us
reuse it from the '-cpu max' init function. It fixes
(2) and (3) mostly by making HVF use the same bits of code
that KVM does for -cpu max and PAuth.

thanks
-- PMM

Peter Maydell (6):
  target/arm: Move '-cpu host' code to cpu64.c
  target/arm: Use aarch64_cpu_register() for 'host' CPU type
  target/arm: Make KVM -cpu max exactly like -cpu host
  target/arm: Unindent unnecessary else-clause
  target/arm: Fix '-cpu max' for HVF
  target/arm: Support PAuth extension for hvf

 target/arm/cpu.c   |  30 -----
 target/arm/cpu64.c | 330 +++++++++++++++++++++++++--------------------
 2 files changed, 181 insertions(+), 179 deletions(-)

-- 
2.25.1

[PATCH 1/6] target/arm: Move '-cpu host' code to cpu64.c

[Peter Maydell]

Now that KVM has dropped AArch32 host support, the 'host' CPU type is
always AArch64, and we can move it to cpu64.c.  This move will allow
us to share code between it and '-cpu max', which should behave
the same as '-cpu host' when using KVM or HVF.

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

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index cdbc4cdd012..d655daa949c 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -39,7 +39,6 @@
 #include "sysemu/tcg.h"
 #include "sysemu/hw_accel.h"
 #include "kvm_arm.h"
-#include "hvf_arm.h"
 #include "disas/capstone.h"
 #include "fpu/softfloat.h"
 
@@ -2075,31 +2074,6 @@ static void arm_cpu_class_init(ObjectClass *oc, void *data)
 #endif /* CONFIG_TCG */
 }
 
-#if defined(CONFIG_KVM) || defined(CONFIG_HVF)
-static void arm_host_initfn(Object *obj)
-{
-    ARMCPU *cpu = ARM_CPU(obj);
-
-#ifdef CONFIG_KVM
-    kvm_arm_set_cpu_features_from_host(cpu);
-    if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) {
-        aarch64_add_sve_properties(obj);
-        aarch64_add_pauth_properties(obj);
-    }
-#else
-    hvf_arm_set_cpu_features_from_host(cpu);
-#endif
-    arm_cpu_post_init(obj);
-}
-
-static const TypeInfo host_arm_cpu_type_info = {
-    .name = TYPE_ARM_HOST_CPU,
-    .parent = TYPE_AARCH64_CPU,
-    .instance_init = arm_host_initfn,
-};
-
-#endif
-
 static void arm_cpu_instance_init(Object *obj)
 {
     ARMCPUClass *acc = ARM_CPU_GET_CLASS(obj);
@@ -2147,10 +2121,6 @@ static const TypeInfo arm_cpu_type_info = {
 static void arm_cpu_register_types(void)
 {
     type_register_static(&arm_cpu_type_info);
-
-#if defined(CONFIG_KVM) || defined(CONFIG_HVF)
-    type_register_static(&host_arm_cpu_type_info);
-#endif
 }
 
 type_init(arm_cpu_register_types)
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 8786be7783e..052666b819e 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -30,6 +30,7 @@
 #endif
 #include "sysemu/kvm.h"
 #include "kvm_arm.h"
+#include "hvf_arm.h"
 #include "qapi/visitor.h"
 #include "hw/qdev-properties.h"
 
@@ -681,6 +682,31 @@ void aarch64_add_pauth_properties(Object *obj)
     }
 }
 
+#if defined(CONFIG_KVM) || defined(CONFIG_HVF)
+static void arm_host_initfn(Object *obj)
+{
+    ARMCPU *cpu = ARM_CPU(obj);
+
+#ifdef CONFIG_KVM
+    kvm_arm_set_cpu_features_from_host(cpu);
+    if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) {
+        aarch64_add_sve_properties(obj);
+        aarch64_add_pauth_properties(obj);
+    }
+#else
+    hvf_arm_set_cpu_features_from_host(cpu);
+#endif
+    arm_cpu_post_init(obj);
+}
+
+static const TypeInfo host_arm_cpu_type_info = {
+    .name = TYPE_ARM_HOST_CPU,
+    .parent = TYPE_AARCH64_CPU,
+    .instance_init = arm_host_initfn,
+};
+
+#endif
+
 /* -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;
@@ -1023,6 +1049,10 @@ static void aarch64_cpu_register_types(void)
     for (i = 0; i < ARRAY_SIZE(aarch64_cpus); ++i) {
         aarch64_cpu_register(&aarch64_cpus[i]);
     }
+
+#if defined(CONFIG_KVM) || defined(CONFIG_HVF)
+    type_register_static(&host_arm_cpu_type_info);
+#endif
 }
 
 type_init(aarch64_cpu_register_types)
-- 
2.25.1

Re: [PATCH 1/6] target/arm: Move '-cpu host' code to cpu64.c

[Richard Henderson]

On 2/5/22 03:55, Peter Maydell wrote:
> Now that KVM has dropped AArch32 host support, the 'host' CPU type is
> always AArch64, and we can move it to cpu64.c.  This move will allow
> us to share code between it and '-cpu max', which should behave
> the same as '-cpu host' when using KVM or HVF.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   target/arm/cpu.c   | 30 ------------------------------
>   target/arm/cpu64.c | 30 ++++++++++++++++++++++++++++++
>   2 files changed, 30 insertions(+), 30 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 2/6] target/arm: Use aarch64_cpu_register() for 'host' CPU type

[Peter Maydell]

Use the aarch64_cpu_register() machinery to register the 'host' CPU
type.  This doesn't gain us anything functionally, but it does mean
that the code for initializing it looks more like that for the other
CPU types, in that its initfn then doesn't need to call
arm_cpu_post_init() (because aarch64_cpu_instance_init() does that
for it).

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/cpu64.c | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 052666b819e..590ac562714 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -683,7 +683,7 @@ void aarch64_add_pauth_properties(Object *obj)
 }
 
 #if defined(CONFIG_KVM) || defined(CONFIG_HVF)
-static void arm_host_initfn(Object *obj)
+static void aarch64_host_initfn(Object *obj)
 {
     ARMCPU *cpu = ARM_CPU(obj);
 
@@ -696,15 +696,7 @@ static void arm_host_initfn(Object *obj)
 #else
     hvf_arm_set_cpu_features_from_host(cpu);
 #endif
-    arm_cpu_post_init(obj);
 }
-
-static const TypeInfo host_arm_cpu_type_info = {
-    .name = TYPE_ARM_HOST_CPU,
-    .parent = TYPE_AARCH64_CPU,
-    .instance_init = arm_host_initfn,
-};
-
 #endif
 
 /* -cpu max: if KVM is enabled, like -cpu host (best possible with this host);
@@ -943,6 +935,9 @@ static const ARMCPUInfo aarch64_cpus[] = {
     { .name = "cortex-a72",         .initfn = aarch64_a72_initfn },
     { .name = "a64fx",              .initfn = aarch64_a64fx_initfn },
     { .name = "max",                .initfn = aarch64_max_initfn },
+#if defined(CONFIG_KVM) || defined(CONFIG_HVF)
+    { .name = "host",               .initfn = aarch64_host_initfn },
+#endif
 };
 
 static bool aarch64_cpu_get_aarch64(Object *obj, Error **errp)
@@ -1049,10 +1044,6 @@ static void aarch64_cpu_register_types(void)
     for (i = 0; i < ARRAY_SIZE(aarch64_cpus); ++i) {
         aarch64_cpu_register(&aarch64_cpus[i]);
     }
-
-#if defined(CONFIG_KVM) || defined(CONFIG_HVF)
-    type_register_static(&host_arm_cpu_type_info);
-#endif
 }
 
 type_init(aarch64_cpu_register_types)
-- 
2.25.1

Re: [PATCH 2/6] target/arm: Use aarch64_cpu_register() for 'host' CPU type

[Richard Henderson]

On 2/5/22 03:55, Peter Maydell wrote:
> Use the aarch64_cpu_register() machinery to register the 'host' CPU
> type.  This doesn't gain us anything functionally, but it does mean
> that the code for initializing it looks more like that for the other
> CPU types, in that its initfn then doesn't need to call
> arm_cpu_post_init() (because aarch64_cpu_instance_init() does that
> for it).
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   target/arm/cpu64.c | 17 ++++-------------
>   1 file changed, 4 insertions(+), 13 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 3/6] target/arm: Make KVM -cpu max exactly like -cpu host

[Peter Maydell]

Currently for KVM the intention is that '-cpu max' and '-cpu host'
are the same thing, but because we did this with two separate
pieces of code they have got a little bit out of sync. Specifically,
'max' has a 'sve-max-vq' property, and 'host' does not.

Bring the two together by having the initfn for 'max' actually
call the initfn for 'host'. This will result in 'max' no longer
exposing the 'sve-max-vq' property when using KVM.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/cpu64.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 590ac562714..ae2e431247f 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -682,22 +682,22 @@ void aarch64_add_pauth_properties(Object *obj)
     }
 }
 
-#if defined(CONFIG_KVM) || defined(CONFIG_HVF)
 static void aarch64_host_initfn(Object *obj)
 {
+#if defined(CONFIG_KVM)
     ARMCPU *cpu = ARM_CPU(obj);
-
-#ifdef CONFIG_KVM
     kvm_arm_set_cpu_features_from_host(cpu);
     if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) {
         aarch64_add_sve_properties(obj);
         aarch64_add_pauth_properties(obj);
     }
-#else
+#elif defined(CONFIG_HVF)
+    ARMCPU *cpu = ARM_CPU(obj);
     hvf_arm_set_cpu_features_from_host(cpu);
+#else
+    g_assert_not_reached();
 #endif
 }
-#endif
 
 /* -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.
@@ -709,7 +709,9 @@ static void aarch64_max_initfn(Object *obj)
     ARMCPU *cpu = ARM_CPU(obj);
 
     if (kvm_enabled()) {
-        kvm_arm_set_cpu_features_from_host(cpu);
+        /* With KVM, '-cpu max' is identical to '-cpu host' */
+        aarch64_host_initfn(obj);
+        return;
     } else {
         uint64_t t;
         uint32_t u;
-- 
2.25.1

Re: [PATCH 3/6] target/arm: Make KVM -cpu max exactly like -cpu host

[Richard Henderson]

On 2/5/22 03:55, Peter Maydell wrote:
> Currently for KVM the intention is that '-cpu max' and '-cpu host'
> are the same thing, but because we did this with two separate
> pieces of code they have got a little bit out of sync. Specifically,
> 'max' has a 'sve-max-vq' property, and 'host' does not.
> 
> Bring the two together by having the initfn for 'max' actually
> call the initfn for 'host'. This will result in 'max' no longer
> exposing the 'sve-max-vq' property when using KVM.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   target/arm/cpu64.c | 14 ++++++++------
>   1 file changed, 8 insertions(+), 6 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

[PATCH 4/6] target/arm: Unindent unnecessary else-clause

[Peter Maydell]

Now that the if() branch of the condition in aarch64_max_initfn()
returns early, we don't need to keep the rest of the code in
the function inside an else block. Remove the else, unindenting
that code.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/cpu64.c | 288 +++++++++++++++++++++++----------------------
 1 file changed, 145 insertions(+), 143 deletions(-)

diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index ae2e431247f..bc25a2567bf 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -707,176 +707,178 @@ static void aarch64_host_initfn(Object *obj)
 static void aarch64_max_initfn(Object *obj)
 {
     ARMCPU *cpu = ARM_CPU(obj);
+    uint64_t t;
+    uint32_t u;
 
     if (kvm_enabled()) {
         /* With KVM, '-cpu max' is identical to '-cpu host' */
         aarch64_host_initfn(obj);
         return;
-    } else {
-        uint64_t t;
-        uint32_t u;
-        aarch64_a57_initfn(obj);
+    }
 
-        /*
-         * Reset MIDR so the guest doesn't mistake our 'max' CPU type for a real
-         * one and try to apply errata workarounds or use impdef features we
-         * don't provide.
-         * An IMPLEMENTER field of 0 means "reserved for software use";
-         * ARCHITECTURE must be 0xf indicating "v7 or later, check ID registers
-         * to see which features are present";
-         * the VARIANT, PARTNUM and REVISION fields are all implementation
-         * defined and we choose to define PARTNUM just in case guest
-         * code needs to distinguish this QEMU CPU from other software
-         * implementations, though this shouldn't be needed.
-         */
-        t = FIELD_DP64(0, MIDR_EL1, IMPLEMENTER, 0);
-        t = FIELD_DP64(t, MIDR_EL1, ARCHITECTURE, 0xf);
-        t = FIELD_DP64(t, MIDR_EL1, PARTNUM, 'Q');
-        t = FIELD_DP64(t, MIDR_EL1, VARIANT, 0);
-        t = FIELD_DP64(t, MIDR_EL1, REVISION, 0);
-        cpu->midr = t;
+    /* '-cpu max' for TCG: we currently do this as "A57 with extra things" */
 
-        t = cpu->isar.id_aa64isar0;
-        t = FIELD_DP64(t, ID_AA64ISAR0, AES, 2); /* AES + PMULL */
-        t = FIELD_DP64(t, ID_AA64ISAR0, SHA1, 1);
-        t = FIELD_DP64(t, ID_AA64ISAR0, SHA2, 2); /* SHA512 */
-        t = FIELD_DP64(t, ID_AA64ISAR0, CRC32, 1);
-        t = FIELD_DP64(t, ID_AA64ISAR0, ATOMIC, 2);
-        t = FIELD_DP64(t, ID_AA64ISAR0, RDM, 1);
-        t = FIELD_DP64(t, ID_AA64ISAR0, SHA3, 1);
-        t = FIELD_DP64(t, ID_AA64ISAR0, SM3, 1);
-        t = FIELD_DP64(t, ID_AA64ISAR0, SM4, 1);
-        t = FIELD_DP64(t, ID_AA64ISAR0, DP, 1);
-        t = FIELD_DP64(t, ID_AA64ISAR0, FHM, 1);
-        t = FIELD_DP64(t, ID_AA64ISAR0, TS, 2); /* v8.5-CondM */
-        t = FIELD_DP64(t, ID_AA64ISAR0, TLB, 2); /* FEAT_TLBIRANGE */
-        t = FIELD_DP64(t, ID_AA64ISAR0, RNDR, 1);
-        cpu->isar.id_aa64isar0 = t;
+    aarch64_a57_initfn(obj);
 
-        t = cpu->isar.id_aa64isar1;
-        t = FIELD_DP64(t, ID_AA64ISAR1, DPB, 2);
-        t = FIELD_DP64(t, ID_AA64ISAR1, JSCVT, 1);
-        t = FIELD_DP64(t, ID_AA64ISAR1, FCMA, 1);
-        t = FIELD_DP64(t, ID_AA64ISAR1, SB, 1);
-        t = FIELD_DP64(t, ID_AA64ISAR1, SPECRES, 1);
-        t = FIELD_DP64(t, ID_AA64ISAR1, BF16, 1);
-        t = FIELD_DP64(t, ID_AA64ISAR1, FRINTTS, 1);
-        t = FIELD_DP64(t, ID_AA64ISAR1, LRCPC, 2); /* ARMv8.4-RCPC */
-        t = FIELD_DP64(t, ID_AA64ISAR1, I8MM, 1);
-        cpu->isar.id_aa64isar1 = t;
+    /*
+     * Reset MIDR so the guest doesn't mistake our 'max' CPU type for a real
+     * one and try to apply errata workarounds or use impdef features we
+     * don't provide.
+     * An IMPLEMENTER field of 0 means "reserved for software use";
+     * ARCHITECTURE must be 0xf indicating "v7 or later, check ID registers
+     * to see which features are present";
+     * the VARIANT, PARTNUM and REVISION fields are all implementation
+     * defined and we choose to define PARTNUM just in case guest
+     * code needs to distinguish this QEMU CPU from other software
+     * implementations, though this shouldn't be needed.
+     */
+    t = FIELD_DP64(0, MIDR_EL1, IMPLEMENTER, 0);
+    t = FIELD_DP64(t, MIDR_EL1, ARCHITECTURE, 0xf);
+    t = FIELD_DP64(t, MIDR_EL1, PARTNUM, 'Q');
+    t = FIELD_DP64(t, MIDR_EL1, VARIANT, 0);
+    t = FIELD_DP64(t, MIDR_EL1, REVISION, 0);
+    cpu->midr = t;
 
-        t = cpu->isar.id_aa64pfr0;
-        t = FIELD_DP64(t, ID_AA64PFR0, SVE, 1);
-        t = FIELD_DP64(t, ID_AA64PFR0, FP, 1);
-        t = FIELD_DP64(t, ID_AA64PFR0, ADVSIMD, 1);
-        t = FIELD_DP64(t, ID_AA64PFR0, SEL2, 1);
-        t = FIELD_DP64(t, ID_AA64PFR0, DIT, 1);
-        cpu->isar.id_aa64pfr0 = t;
+    t = cpu->isar.id_aa64isar0;
+    t = FIELD_DP64(t, ID_AA64ISAR0, AES, 2); /* AES + PMULL */
+    t = FIELD_DP64(t, ID_AA64ISAR0, SHA1, 1);
+    t = FIELD_DP64(t, ID_AA64ISAR0, SHA2, 2); /* SHA512 */
+    t = FIELD_DP64(t, ID_AA64ISAR0, CRC32, 1);
+    t = FIELD_DP64(t, ID_AA64ISAR0, ATOMIC, 2);
+    t = FIELD_DP64(t, ID_AA64ISAR0, RDM, 1);
+    t = FIELD_DP64(t, ID_AA64ISAR0, SHA3, 1);
+    t = FIELD_DP64(t, ID_AA64ISAR0, SM3, 1);
+    t = FIELD_DP64(t, ID_AA64ISAR0, SM4, 1);
+    t = FIELD_DP64(t, ID_AA64ISAR0, DP, 1);
+    t = FIELD_DP64(t, ID_AA64ISAR0, FHM, 1);
+    t = FIELD_DP64(t, ID_AA64ISAR0, TS, 2); /* v8.5-CondM */
+    t = FIELD_DP64(t, ID_AA64ISAR0, TLB, 2); /* FEAT_TLBIRANGE */
+    t = FIELD_DP64(t, ID_AA64ISAR0, RNDR, 1);
+    cpu->isar.id_aa64isar0 = t;
 
-        t = cpu->isar.id_aa64pfr1;
-        t = FIELD_DP64(t, ID_AA64PFR1, BT, 1);
-        t = FIELD_DP64(t, ID_AA64PFR1, SSBS, 2);
-        /*
-         * Begin with full support for MTE. This will be downgraded to MTE=0
-         * during realize if the board provides no tag memory, much like
-         * we do for EL2 with the virtualization=on property.
-         */
-        t = FIELD_DP64(t, ID_AA64PFR1, MTE, 3);
-        cpu->isar.id_aa64pfr1 = t;
+    t = cpu->isar.id_aa64isar1;
+    t = FIELD_DP64(t, ID_AA64ISAR1, DPB, 2);
+    t = FIELD_DP64(t, ID_AA64ISAR1, JSCVT, 1);
+    t = FIELD_DP64(t, ID_AA64ISAR1, FCMA, 1);
+    t = FIELD_DP64(t, ID_AA64ISAR1, SB, 1);
+    t = FIELD_DP64(t, ID_AA64ISAR1, SPECRES, 1);
+    t = FIELD_DP64(t, ID_AA64ISAR1, BF16, 1);
+    t = FIELD_DP64(t, ID_AA64ISAR1, FRINTTS, 1);
+    t = FIELD_DP64(t, ID_AA64ISAR1, LRCPC, 2); /* ARMv8.4-RCPC */
+    t = FIELD_DP64(t, ID_AA64ISAR1, I8MM, 1);
+    cpu->isar.id_aa64isar1 = t;
 
-        t = cpu->isar.id_aa64mmfr0;
-        t = FIELD_DP64(t, ID_AA64MMFR0, PARANGE, 5); /* PARange: 48 bits */
-        cpu->isar.id_aa64mmfr0 = t;
+    t = cpu->isar.id_aa64pfr0;
+    t = FIELD_DP64(t, ID_AA64PFR0, SVE, 1);
+    t = FIELD_DP64(t, ID_AA64PFR0, FP, 1);
+    t = FIELD_DP64(t, ID_AA64PFR0, ADVSIMD, 1);
+    t = FIELD_DP64(t, ID_AA64PFR0, SEL2, 1);
+    t = FIELD_DP64(t, ID_AA64PFR0, DIT, 1);
+    cpu->isar.id_aa64pfr0 = t;
 
-        t = cpu->isar.id_aa64mmfr1;
-        t = FIELD_DP64(t, ID_AA64MMFR1, HPDS, 1); /* HPD */
-        t = FIELD_DP64(t, ID_AA64MMFR1, LO, 1);
-        t = FIELD_DP64(t, ID_AA64MMFR1, VH, 1);
-        t = FIELD_DP64(t, ID_AA64MMFR1, PAN, 2); /* ATS1E1 */
-        t = FIELD_DP64(t, ID_AA64MMFR1, VMIDBITS, 2); /* VMID16 */
-        t = FIELD_DP64(t, ID_AA64MMFR1, XNX, 1); /* TTS2UXN */
-        cpu->isar.id_aa64mmfr1 = t;
+    t = cpu->isar.id_aa64pfr1;
+    t = FIELD_DP64(t, ID_AA64PFR1, BT, 1);
+    t = FIELD_DP64(t, ID_AA64PFR1, SSBS, 2);
+    /*
+     * Begin with full support for MTE. This will be downgraded to MTE=0
+     * during realize if the board provides no tag memory, much like
+     * we do for EL2 with the virtualization=on property.
+     */
+    t = FIELD_DP64(t, ID_AA64PFR1, MTE, 3);
+    cpu->isar.id_aa64pfr1 = t;
 
-        t = cpu->isar.id_aa64mmfr2;
-        t = FIELD_DP64(t, ID_AA64MMFR2, UAO, 1);
-        t = FIELD_DP64(t, ID_AA64MMFR2, CNP, 1); /* TTCNP */
-        t = FIELD_DP64(t, ID_AA64MMFR2, ST, 1); /* TTST */
-        cpu->isar.id_aa64mmfr2 = t;
+    t = cpu->isar.id_aa64mmfr0;
+    t = FIELD_DP64(t, ID_AA64MMFR0, PARANGE, 5); /* PARange: 48 bits */
+    cpu->isar.id_aa64mmfr0 = t;
 
-        t = cpu->isar.id_aa64zfr0;
-        t = FIELD_DP64(t, ID_AA64ZFR0, SVEVER, 1);
-        t = FIELD_DP64(t, ID_AA64ZFR0, AES, 2);  /* PMULL */
-        t = FIELD_DP64(t, ID_AA64ZFR0, BITPERM, 1);
-        t = FIELD_DP64(t, ID_AA64ZFR0, BFLOAT16, 1);
-        t = FIELD_DP64(t, ID_AA64ZFR0, SHA3, 1);
-        t = FIELD_DP64(t, ID_AA64ZFR0, SM4, 1);
-        t = FIELD_DP64(t, ID_AA64ZFR0, I8MM, 1);
-        t = FIELD_DP64(t, ID_AA64ZFR0, F32MM, 1);
-        t = FIELD_DP64(t, ID_AA64ZFR0, F64MM, 1);
-        cpu->isar.id_aa64zfr0 = t;
+    t = cpu->isar.id_aa64mmfr1;
+    t = FIELD_DP64(t, ID_AA64MMFR1, HPDS, 1); /* HPD */
+    t = FIELD_DP64(t, ID_AA64MMFR1, LO, 1);
+    t = FIELD_DP64(t, ID_AA64MMFR1, VH, 1);
+    t = FIELD_DP64(t, ID_AA64MMFR1, PAN, 2); /* ATS1E1 */
+    t = FIELD_DP64(t, ID_AA64MMFR1, VMIDBITS, 2); /* VMID16 */
+    t = FIELD_DP64(t, ID_AA64MMFR1, XNX, 1); /* TTS2UXN */
+    cpu->isar.id_aa64mmfr1 = t;
 
-        /* Replicate the same data to the 32-bit id registers.  */
-        u = cpu->isar.id_isar5;
-        u = FIELD_DP32(u, ID_ISAR5, AES, 2); /* AES + PMULL */
-        u = FIELD_DP32(u, ID_ISAR5, SHA1, 1);
-        u = FIELD_DP32(u, ID_ISAR5, SHA2, 1);
-        u = FIELD_DP32(u, ID_ISAR5, CRC32, 1);
-        u = FIELD_DP32(u, ID_ISAR5, RDM, 1);
-        u = FIELD_DP32(u, ID_ISAR5, VCMA, 1);
-        cpu->isar.id_isar5 = u;
+    t = cpu->isar.id_aa64mmfr2;
+    t = FIELD_DP64(t, ID_AA64MMFR2, UAO, 1);
+    t = FIELD_DP64(t, ID_AA64MMFR2, CNP, 1); /* TTCNP */
+    t = FIELD_DP64(t, ID_AA64MMFR2, ST, 1); /* TTST */
+    cpu->isar.id_aa64mmfr2 = t;
 
-        u = cpu->isar.id_isar6;
-        u = FIELD_DP32(u, ID_ISAR6, JSCVT, 1);
-        u = FIELD_DP32(u, ID_ISAR6, DP, 1);
-        u = FIELD_DP32(u, ID_ISAR6, FHM, 1);
-        u = FIELD_DP32(u, ID_ISAR6, SB, 1);
-        u = FIELD_DP32(u, ID_ISAR6, SPECRES, 1);
-        u = FIELD_DP32(u, ID_ISAR6, BF16, 1);
-        u = FIELD_DP32(u, ID_ISAR6, I8MM, 1);
-        cpu->isar.id_isar6 = u;
+    t = cpu->isar.id_aa64zfr0;
+    t = FIELD_DP64(t, ID_AA64ZFR0, SVEVER, 1);
+    t = FIELD_DP64(t, ID_AA64ZFR0, AES, 2);  /* PMULL */
+    t = FIELD_DP64(t, ID_AA64ZFR0, BITPERM, 1);
+    t = FIELD_DP64(t, ID_AA64ZFR0, BFLOAT16, 1);
+    t = FIELD_DP64(t, ID_AA64ZFR0, SHA3, 1);
+    t = FIELD_DP64(t, ID_AA64ZFR0, SM4, 1);
+    t = FIELD_DP64(t, ID_AA64ZFR0, I8MM, 1);
+    t = FIELD_DP64(t, ID_AA64ZFR0, F32MM, 1);
+    t = FIELD_DP64(t, ID_AA64ZFR0, F64MM, 1);
+    cpu->isar.id_aa64zfr0 = t;
 
-        u = cpu->isar.id_pfr0;
-        u = FIELD_DP32(u, ID_PFR0, DIT, 1);
-        cpu->isar.id_pfr0 = u;
+    /* Replicate the same data to the 32-bit id registers.  */
+    u = cpu->isar.id_isar5;
+    u = FIELD_DP32(u, ID_ISAR5, AES, 2); /* AES + PMULL */
+    u = FIELD_DP32(u, ID_ISAR5, SHA1, 1);
+    u = FIELD_DP32(u, ID_ISAR5, SHA2, 1);
+    u = FIELD_DP32(u, ID_ISAR5, CRC32, 1);
+    u = FIELD_DP32(u, ID_ISAR5, RDM, 1);
+    u = FIELD_DP32(u, ID_ISAR5, VCMA, 1);
+    cpu->isar.id_isar5 = u;
 
-        u = cpu->isar.id_pfr2;
-        u = FIELD_DP32(u, ID_PFR2, SSBS, 1);
-        cpu->isar.id_pfr2 = u;
+    u = cpu->isar.id_isar6;
+    u = FIELD_DP32(u, ID_ISAR6, JSCVT, 1);
+    u = FIELD_DP32(u, ID_ISAR6, DP, 1);
+    u = FIELD_DP32(u, ID_ISAR6, FHM, 1);
+    u = FIELD_DP32(u, ID_ISAR6, SB, 1);
+    u = FIELD_DP32(u, ID_ISAR6, SPECRES, 1);
+    u = FIELD_DP32(u, ID_ISAR6, BF16, 1);
+    u = FIELD_DP32(u, ID_ISAR6, I8MM, 1);
+    cpu->isar.id_isar6 = u;
 
-        u = cpu->isar.id_mmfr3;
-        u = FIELD_DP32(u, ID_MMFR3, PAN, 2); /* ATS1E1 */
-        cpu->isar.id_mmfr3 = u;
+    u = cpu->isar.id_pfr0;
+    u = FIELD_DP32(u, ID_PFR0, DIT, 1);
+    cpu->isar.id_pfr0 = u;
 
-        u = cpu->isar.id_mmfr4;
-        u = FIELD_DP32(u, ID_MMFR4, HPDS, 1); /* AA32HPD */
-        u = FIELD_DP32(u, ID_MMFR4, AC2, 1); /* ACTLR2, HACTLR2 */
-        u = FIELD_DP32(u, ID_MMFR4, CNP, 1); /* TTCNP */
-        u = FIELD_DP32(u, ID_MMFR4, XNX, 1); /* TTS2UXN */
-        cpu->isar.id_mmfr4 = u;
+    u = cpu->isar.id_pfr2;
+    u = FIELD_DP32(u, ID_PFR2, SSBS, 1);
+    cpu->isar.id_pfr2 = u;
 
-        t = cpu->isar.id_aa64dfr0;
-        t = FIELD_DP64(t, ID_AA64DFR0, PMUVER, 5); /* v8.4-PMU */
-        cpu->isar.id_aa64dfr0 = t;
+    u = cpu->isar.id_mmfr3;
+    u = FIELD_DP32(u, ID_MMFR3, PAN, 2); /* ATS1E1 */
+    cpu->isar.id_mmfr3 = u;
 
-        u = cpu->isar.id_dfr0;
-        u = FIELD_DP32(u, ID_DFR0, PERFMON, 5); /* v8.4-PMU */
-        cpu->isar.id_dfr0 = u;
+    u = cpu->isar.id_mmfr4;
+    u = FIELD_DP32(u, ID_MMFR4, HPDS, 1); /* AA32HPD */
+    u = FIELD_DP32(u, ID_MMFR4, AC2, 1); /* ACTLR2, HACTLR2 */
+    u = FIELD_DP32(u, ID_MMFR4, CNP, 1); /* TTCNP */
+    u = FIELD_DP32(u, ID_MMFR4, XNX, 1); /* TTS2UXN */
+    cpu->isar.id_mmfr4 = u;
 
-        u = cpu->isar.mvfr1;
-        u = FIELD_DP32(u, MVFR1, FPHP, 3);      /* v8.2-FP16 */
-        u = FIELD_DP32(u, MVFR1, SIMDHP, 2);    /* v8.2-FP16 */
-        cpu->isar.mvfr1 = u;
+    t = cpu->isar.id_aa64dfr0;
+    t = FIELD_DP64(t, ID_AA64DFR0, PMUVER, 5); /* v8.4-PMU */
+    cpu->isar.id_aa64dfr0 = t;
+
+    u = cpu->isar.id_dfr0;
+    u = FIELD_DP32(u, ID_DFR0, PERFMON, 5); /* v8.4-PMU */
+    cpu->isar.id_dfr0 = u;
+
+    u = cpu->isar.mvfr1;
+    u = FIELD_DP32(u, MVFR1, FPHP, 3);      /* v8.2-FP16 */
+    u = FIELD_DP32(u, MVFR1, SIMDHP, 2);    /* v8.2-FP16 */
+    cpu->isar.mvfr1 = u;
 
 #ifdef CONFIG_USER_ONLY
-        /* 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 */
+    /* 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
 
-        bitmap_fill(cpu->sve_vq_supported, ARM_MAX_VQ);
-    }
+    bitmap_fill(cpu->sve_vq_supported, ARM_MAX_VQ);
 
     aarch64_add_pauth_properties(obj);
     aarch64_add_sve_properties(obj);
-- 
2.25.1

Re: [PATCH 4/6] target/arm: Unindent unnecessary else-clause

[Richard Henderson]

On 2/5/22 03:55, Peter Maydell wrote:
> Now that the if() branch of the condition in aarch64_max_initfn()
> returns early, we don't need to keep the rest of the code in
> the function inside an else block. Remove the else, unindenting
> that code.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   target/arm/cpu64.c | 288 +++++++++++++++++++++++----------------------
>   1 file changed, 145 insertions(+), 143 deletions(-)
> 
> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> index ae2e431247f..bc25a2567bf 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -707,176 +707,178 @@ static void aarch64_host_initfn(Object *obj)
>   static void aarch64_max_initfn(Object *obj)
>   {
>       ARMCPU *cpu = ARM_CPU(obj);
> +    uint64_t t;
> +    uint32_t u;
>   
>       if (kvm_enabled()) {
>           /* With KVM, '-cpu max' is identical to '-cpu host' */
>           aarch64_host_initfn(obj);
>           return;
> -    } else {
> -        uint64_t t;
> -        uint32_t u;
> -        aarch64_a57_initfn(obj);
> +    }


Could move the init of cpu afterward.  It's a runtime call to verify the qom class, and 
we'll wind up doing that again inside aarch64_host_initfn.  But either way,

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

>   
> -        /*
> -         * Reset MIDR so the guest doesn't mistake our 'max' CPU type for a real
> -         * one and try to apply errata workarounds or use impdef features we
> -         * don't provide.
> -         * An IMPLEMENTER field of 0 means "reserved for software use";
> -         * ARCHITECTURE must be 0xf indicating "v7 or later, check ID registers
> -         * to see which features are present";
> -         * the VARIANT, PARTNUM and REVISION fields are all implementation
> -         * defined and we choose to define PARTNUM just in case guest
> -         * code needs to distinguish this QEMU CPU from other software
> -         * implementations, though this shouldn't be needed.
> -         */
> -        t = FIELD_DP64(0, MIDR_EL1, IMPLEMENTER, 0);
> -        t = FIELD_DP64(t, MIDR_EL1, ARCHITECTURE, 0xf);
> -        t = FIELD_DP64(t, MIDR_EL1, PARTNUM, 'Q');
> -        t = FIELD_DP64(t, MIDR_EL1, VARIANT, 0);
> -        t = FIELD_DP64(t, MIDR_EL1, REVISION, 0);
> -        cpu->midr = t;
> +    /* '-cpu max' for TCG: we currently do this as "A57 with extra things" */
>   
> -        t = cpu->isar.id_aa64isar0;
> -        t = FIELD_DP64(t, ID_AA64ISAR0, AES, 2); /* AES + PMULL */
> -        t = FIELD_DP64(t, ID_AA64ISAR0, SHA1, 1);
> -        t = FIELD_DP64(t, ID_AA64ISAR0, SHA2, 2); /* SHA512 */
> -        t = FIELD_DP64(t, ID_AA64ISAR0, CRC32, 1);
> -        t = FIELD_DP64(t, ID_AA64ISAR0, ATOMIC, 2);
> -        t = FIELD_DP64(t, ID_AA64ISAR0, RDM, 1);
> -        t = FIELD_DP64(t, ID_AA64ISAR0, SHA3, 1);
> -        t = FIELD_DP64(t, ID_AA64ISAR0, SM3, 1);
> -        t = FIELD_DP64(t, ID_AA64ISAR0, SM4, 1);
> -        t = FIELD_DP64(t, ID_AA64ISAR0, DP, 1);
> -        t = FIELD_DP64(t, ID_AA64ISAR0, FHM, 1);
> -        t = FIELD_DP64(t, ID_AA64ISAR0, TS, 2); /* v8.5-CondM */
> -        t = FIELD_DP64(t, ID_AA64ISAR0, TLB, 2); /* FEAT_TLBIRANGE */
> -        t = FIELD_DP64(t, ID_AA64ISAR0, RNDR, 1);
> -        cpu->isar.id_aa64isar0 = t;
> +    aarch64_a57_initfn(obj);
>   
> -        t = cpu->isar.id_aa64isar1;
> -        t = FIELD_DP64(t, ID_AA64ISAR1, DPB, 2);
> -        t = FIELD_DP64(t, ID_AA64ISAR1, JSCVT, 1);
> -        t = FIELD_DP64(t, ID_AA64ISAR1, FCMA, 1);
> -        t = FIELD_DP64(t, ID_AA64ISAR1, SB, 1);
> -        t = FIELD_DP64(t, ID_AA64ISAR1, SPECRES, 1);
> -        t = FIELD_DP64(t, ID_AA64ISAR1, BF16, 1);
> -        t = FIELD_DP64(t, ID_AA64ISAR1, FRINTTS, 1);
> -        t = FIELD_DP64(t, ID_AA64ISAR1, LRCPC, 2); /* ARMv8.4-RCPC */
> -        t = FIELD_DP64(t, ID_AA64ISAR1, I8MM, 1);
> -        cpu->isar.id_aa64isar1 = t;
> +    /*
> +     * Reset MIDR so the guest doesn't mistake our 'max' CPU type for a real
> +     * one and try to apply errata workarounds or use impdef features we
> +     * don't provide.
> +     * An IMPLEMENTER field of 0 means "reserved for software use";
> +     * ARCHITECTURE must be 0xf indicating "v7 or later, check ID registers
> +     * to see which features are present";
> +     * the VARIANT, PARTNUM and REVISION fields are all implementation
> +     * defined and we choose to define PARTNUM just in case guest
> +     * code needs to distinguish this QEMU CPU from other software
> +     * implementations, though this shouldn't be needed.
> +     */
> +    t = FIELD_DP64(0, MIDR_EL1, IMPLEMENTER, 0);
> +    t = FIELD_DP64(t, MIDR_EL1, ARCHITECTURE, 0xf);
> +    t = FIELD_DP64(t, MIDR_EL1, PARTNUM, 'Q');
> +    t = FIELD_DP64(t, MIDR_EL1, VARIANT, 0);
> +    t = FIELD_DP64(t, MIDR_EL1, REVISION, 0);
> +    cpu->midr = t;
>   
> -        t = cpu->isar.id_aa64pfr0;
> -        t = FIELD_DP64(t, ID_AA64PFR0, SVE, 1);
> -        t = FIELD_DP64(t, ID_AA64PFR0, FP, 1);
> -        t = FIELD_DP64(t, ID_AA64PFR0, ADVSIMD, 1);
> -        t = FIELD_DP64(t, ID_AA64PFR0, SEL2, 1);
> -        t = FIELD_DP64(t, ID_AA64PFR0, DIT, 1);
> -        cpu->isar.id_aa64pfr0 = t;
> +    t = cpu->isar.id_aa64isar0;
> +    t = FIELD_DP64(t, ID_AA64ISAR0, AES, 2); /* AES + PMULL */
> +    t = FIELD_DP64(t, ID_AA64ISAR0, SHA1, 1);
> +    t = FIELD_DP64(t, ID_AA64ISAR0, SHA2, 2); /* SHA512 */
> +    t = FIELD_DP64(t, ID_AA64ISAR0, CRC32, 1);
> +    t = FIELD_DP64(t, ID_AA64ISAR0, ATOMIC, 2);
> +    t = FIELD_DP64(t, ID_AA64ISAR0, RDM, 1);
> +    t = FIELD_DP64(t, ID_AA64ISAR0, SHA3, 1);
> +    t = FIELD_DP64(t, ID_AA64ISAR0, SM3, 1);
> +    t = FIELD_DP64(t, ID_AA64ISAR0, SM4, 1);
> +    t = FIELD_DP64(t, ID_AA64ISAR0, DP, 1);
> +    t = FIELD_DP64(t, ID_AA64ISAR0, FHM, 1);
> +    t = FIELD_DP64(t, ID_AA64ISAR0, TS, 2); /* v8.5-CondM */
> +    t = FIELD_DP64(t, ID_AA64ISAR0, TLB, 2); /* FEAT_TLBIRANGE */
> +    t = FIELD_DP64(t, ID_AA64ISAR0, RNDR, 1);
> +    cpu->isar.id_aa64isar0 = t;
>   
> -        t = cpu->isar.id_aa64pfr1;
> -        t = FIELD_DP64(t, ID_AA64PFR1, BT, 1);
> -        t = FIELD_DP64(t, ID_AA64PFR1, SSBS, 2);
> -        /*
> -         * Begin with full support for MTE. This will be downgraded to MTE=0
> -         * during realize if the board provides no tag memory, much like
> -         * we do for EL2 with the virtualization=on property.
> -         */
> -        t = FIELD_DP64(t, ID_AA64PFR1, MTE, 3);
> -        cpu->isar.id_aa64pfr1 = t;
> +    t = cpu->isar.id_aa64isar1;
> +    t = FIELD_DP64(t, ID_AA64ISAR1, DPB, 2);
> +    t = FIELD_DP64(t, ID_AA64ISAR1, JSCVT, 1);
> +    t = FIELD_DP64(t, ID_AA64ISAR1, FCMA, 1);
> +    t = FIELD_DP64(t, ID_AA64ISAR1, SB, 1);
> +    t = FIELD_DP64(t, ID_AA64ISAR1, SPECRES, 1);
> +    t = FIELD_DP64(t, ID_AA64ISAR1, BF16, 1);
> +    t = FIELD_DP64(t, ID_AA64ISAR1, FRINTTS, 1);
> +    t = FIELD_DP64(t, ID_AA64ISAR1, LRCPC, 2); /* ARMv8.4-RCPC */
> +    t = FIELD_DP64(t, ID_AA64ISAR1, I8MM, 1);
> +    cpu->isar.id_aa64isar1 = t;
>   
> -        t = cpu->isar.id_aa64mmfr0;
> -        t = FIELD_DP64(t, ID_AA64MMFR0, PARANGE, 5); /* PARange: 48 bits */
> -        cpu->isar.id_aa64mmfr0 = t;
> +    t = cpu->isar.id_aa64pfr0;
> +    t = FIELD_DP64(t, ID_AA64PFR0, SVE, 1);
> +    t = FIELD_DP64(t, ID_AA64PFR0, FP, 1);
> +    t = FIELD_DP64(t, ID_AA64PFR0, ADVSIMD, 1);
> +    t = FIELD_DP64(t, ID_AA64PFR0, SEL2, 1);
> +    t = FIELD_DP64(t, ID_AA64PFR0, DIT, 1);
> +    cpu->isar.id_aa64pfr0 = t;
>   
> -        t = cpu->isar.id_aa64mmfr1;
> -        t = FIELD_DP64(t, ID_AA64MMFR1, HPDS, 1); /* HPD */
> -        t = FIELD_DP64(t, ID_AA64MMFR1, LO, 1);
> -        t = FIELD_DP64(t, ID_AA64MMFR1, VH, 1);
> -        t = FIELD_DP64(t, ID_AA64MMFR1, PAN, 2); /* ATS1E1 */
> -        t = FIELD_DP64(t, ID_AA64MMFR1, VMIDBITS, 2); /* VMID16 */
> -        t = FIELD_DP64(t, ID_AA64MMFR1, XNX, 1); /* TTS2UXN */
> -        cpu->isar.id_aa64mmfr1 = t;
> +    t = cpu->isar.id_aa64pfr1;
> +    t = FIELD_DP64(t, ID_AA64PFR1, BT, 1);
> +    t = FIELD_DP64(t, ID_AA64PFR1, SSBS, 2);
> +    /*
> +     * Begin with full support for MTE. This will be downgraded to MTE=0
> +     * during realize if the board provides no tag memory, much like
> +     * we do for EL2 with the virtualization=on property.
> +     */
> +    t = FIELD_DP64(t, ID_AA64PFR1, MTE, 3);
> +    cpu->isar.id_aa64pfr1 = t;
>   
> -        t = cpu->isar.id_aa64mmfr2;
> -        t = FIELD_DP64(t, ID_AA64MMFR2, UAO, 1);
> -        t = FIELD_DP64(t, ID_AA64MMFR2, CNP, 1); /* TTCNP */
> -        t = FIELD_DP64(t, ID_AA64MMFR2, ST, 1); /* TTST */
> -        cpu->isar.id_aa64mmfr2 = t;
> +    t = cpu->isar.id_aa64mmfr0;
> +    t = FIELD_DP64(t, ID_AA64MMFR0, PARANGE, 5); /* PARange: 48 bits */
> +    cpu->isar.id_aa64mmfr0 = t;
>   
> -        t = cpu->isar.id_aa64zfr0;
> -        t = FIELD_DP64(t, ID_AA64ZFR0, SVEVER, 1);
> -        t = FIELD_DP64(t, ID_AA64ZFR0, AES, 2);  /* PMULL */
> -        t = FIELD_DP64(t, ID_AA64ZFR0, BITPERM, 1);
> -        t = FIELD_DP64(t, ID_AA64ZFR0, BFLOAT16, 1);
> -        t = FIELD_DP64(t, ID_AA64ZFR0, SHA3, 1);
> -        t = FIELD_DP64(t, ID_AA64ZFR0, SM4, 1);
> -        t = FIELD_DP64(t, ID_AA64ZFR0, I8MM, 1);
> -        t = FIELD_DP64(t, ID_AA64ZFR0, F32MM, 1);
> -        t = FIELD_DP64(t, ID_AA64ZFR0, F64MM, 1);
> -        cpu->isar.id_aa64zfr0 = t;
> +    t = cpu->isar.id_aa64mmfr1;
> +    t = FIELD_DP64(t, ID_AA64MMFR1, HPDS, 1); /* HPD */
> +    t = FIELD_DP64(t, ID_AA64MMFR1, LO, 1);
> +    t = FIELD_DP64(t, ID_AA64MMFR1, VH, 1);
> +    t = FIELD_DP64(t, ID_AA64MMFR1, PAN, 2); /* ATS1E1 */
> +    t = FIELD_DP64(t, ID_AA64MMFR1, VMIDBITS, 2); /* VMID16 */
> +    t = FIELD_DP64(t, ID_AA64MMFR1, XNX, 1); /* TTS2UXN */
> +    cpu->isar.id_aa64mmfr1 = t;
>   
> -        /* Replicate the same data to the 32-bit id registers.  */
> -        u = cpu->isar.id_isar5;
> -        u = FIELD_DP32(u, ID_ISAR5, AES, 2); /* AES + PMULL */
> -        u = FIELD_DP32(u, ID_ISAR5, SHA1, 1);
> -        u = FIELD_DP32(u, ID_ISAR5, SHA2, 1);
> -        u = FIELD_DP32(u, ID_ISAR5, CRC32, 1);
> -        u = FIELD_DP32(u, ID_ISAR5, RDM, 1);
> -        u = FIELD_DP32(u, ID_ISAR5, VCMA, 1);
> -        cpu->isar.id_isar5 = u;
> +    t = cpu->isar.id_aa64mmfr2;
> +    t = FIELD_DP64(t, ID_AA64MMFR2, UAO, 1);
> +    t = FIELD_DP64(t, ID_AA64MMFR2, CNP, 1); /* TTCNP */
> +    t = FIELD_DP64(t, ID_AA64MMFR2, ST, 1); /* TTST */
> +    cpu->isar.id_aa64mmfr2 = t;
>   
> -        u = cpu->isar.id_isar6;
> -        u = FIELD_DP32(u, ID_ISAR6, JSCVT, 1);
> -        u = FIELD_DP32(u, ID_ISAR6, DP, 1);
> -        u = FIELD_DP32(u, ID_ISAR6, FHM, 1);
> -        u = FIELD_DP32(u, ID_ISAR6, SB, 1);
> -        u = FIELD_DP32(u, ID_ISAR6, SPECRES, 1);
> -        u = FIELD_DP32(u, ID_ISAR6, BF16, 1);
> -        u = FIELD_DP32(u, ID_ISAR6, I8MM, 1);
> -        cpu->isar.id_isar6 = u;
> +    t = cpu->isar.id_aa64zfr0;
> +    t = FIELD_DP64(t, ID_AA64ZFR0, SVEVER, 1);
> +    t = FIELD_DP64(t, ID_AA64ZFR0, AES, 2);  /* PMULL */
> +    t = FIELD_DP64(t, ID_AA64ZFR0, BITPERM, 1);
> +    t = FIELD_DP64(t, ID_AA64ZFR0, BFLOAT16, 1);
> +    t = FIELD_DP64(t, ID_AA64ZFR0, SHA3, 1);
> +    t = FIELD_DP64(t, ID_AA64ZFR0, SM4, 1);
> +    t = FIELD_DP64(t, ID_AA64ZFR0, I8MM, 1);
> +    t = FIELD_DP64(t, ID_AA64ZFR0, F32MM, 1);
> +    t = FIELD_DP64(t, ID_AA64ZFR0, F64MM, 1);
> +    cpu->isar.id_aa64zfr0 = t;
>   
> -        u = cpu->isar.id_pfr0;
> -        u = FIELD_DP32(u, ID_PFR0, DIT, 1);
> -        cpu->isar.id_pfr0 = u;
> +    /* Replicate the same data to the 32-bit id registers.  */
> +    u = cpu->isar.id_isar5;
> +    u = FIELD_DP32(u, ID_ISAR5, AES, 2); /* AES + PMULL */
> +    u = FIELD_DP32(u, ID_ISAR5, SHA1, 1);
> +    u = FIELD_DP32(u, ID_ISAR5, SHA2, 1);
> +    u = FIELD_DP32(u, ID_ISAR5, CRC32, 1);
> +    u = FIELD_DP32(u, ID_ISAR5, RDM, 1);
> +    u = FIELD_DP32(u, ID_ISAR5, VCMA, 1);
> +    cpu->isar.id_isar5 = u;
>   
> -        u = cpu->isar.id_pfr2;
> -        u = FIELD_DP32(u, ID_PFR2, SSBS, 1);
> -        cpu->isar.id_pfr2 = u;
> +    u = cpu->isar.id_isar6;
> +    u = FIELD_DP32(u, ID_ISAR6, JSCVT, 1);
> +    u = FIELD_DP32(u, ID_ISAR6, DP, 1);
> +    u = FIELD_DP32(u, ID_ISAR6, FHM, 1);
> +    u = FIELD_DP32(u, ID_ISAR6, SB, 1);
> +    u = FIELD_DP32(u, ID_ISAR6, SPECRES, 1);
> +    u = FIELD_DP32(u, ID_ISAR6, BF16, 1);
> +    u = FIELD_DP32(u, ID_ISAR6, I8MM, 1);
> +    cpu->isar.id_isar6 = u;
>   
> -        u = cpu->isar.id_mmfr3;
> -        u = FIELD_DP32(u, ID_MMFR3, PAN, 2); /* ATS1E1 */
> -        cpu->isar.id_mmfr3 = u;
> +    u = cpu->isar.id_pfr0;
> +    u = FIELD_DP32(u, ID_PFR0, DIT, 1);
> +    cpu->isar.id_pfr0 = u;
>   
> -        u = cpu->isar.id_mmfr4;
> -        u = FIELD_DP32(u, ID_MMFR4, HPDS, 1); /* AA32HPD */
> -        u = FIELD_DP32(u, ID_MMFR4, AC2, 1); /* ACTLR2, HACTLR2 */
> -        u = FIELD_DP32(u, ID_MMFR4, CNP, 1); /* TTCNP */
> -        u = FIELD_DP32(u, ID_MMFR4, XNX, 1); /* TTS2UXN */
> -        cpu->isar.id_mmfr4 = u;
> +    u = cpu->isar.id_pfr2;
> +    u = FIELD_DP32(u, ID_PFR2, SSBS, 1);
> +    cpu->isar.id_pfr2 = u;
>   
> -        t = cpu->isar.id_aa64dfr0;
> -        t = FIELD_DP64(t, ID_AA64DFR0, PMUVER, 5); /* v8.4-PMU */
> -        cpu->isar.id_aa64dfr0 = t;
> +    u = cpu->isar.id_mmfr3;
> +    u = FIELD_DP32(u, ID_MMFR3, PAN, 2); /* ATS1E1 */
> +    cpu->isar.id_mmfr3 = u;
>   
> -        u = cpu->isar.id_dfr0;
> -        u = FIELD_DP32(u, ID_DFR0, PERFMON, 5); /* v8.4-PMU */
> -        cpu->isar.id_dfr0 = u;
> +    u = cpu->isar.id_mmfr4;
> +    u = FIELD_DP32(u, ID_MMFR4, HPDS, 1); /* AA32HPD */
> +    u = FIELD_DP32(u, ID_MMFR4, AC2, 1); /* ACTLR2, HACTLR2 */
> +    u = FIELD_DP32(u, ID_MMFR4, CNP, 1); /* TTCNP */
> +    u = FIELD_DP32(u, ID_MMFR4, XNX, 1); /* TTS2UXN */
> +    cpu->isar.id_mmfr4 = u;
>   
> -        u = cpu->isar.mvfr1;
> -        u = FIELD_DP32(u, MVFR1, FPHP, 3);      /* v8.2-FP16 */
> -        u = FIELD_DP32(u, MVFR1, SIMDHP, 2);    /* v8.2-FP16 */
> -        cpu->isar.mvfr1 = u;
> +    t = cpu->isar.id_aa64dfr0;
> +    t = FIELD_DP64(t, ID_AA64DFR0, PMUVER, 5); /* v8.4-PMU */
> +    cpu->isar.id_aa64dfr0 = t;
> +
> +    u = cpu->isar.id_dfr0;
> +    u = FIELD_DP32(u, ID_DFR0, PERFMON, 5); /* v8.4-PMU */
> +    cpu->isar.id_dfr0 = u;
> +
> +    u = cpu->isar.mvfr1;
> +    u = FIELD_DP32(u, MVFR1, FPHP, 3);      /* v8.2-FP16 */
> +    u = FIELD_DP32(u, MVFR1, SIMDHP, 2);    /* v8.2-FP16 */
> +    cpu->isar.mvfr1 = u;
>   
>   #ifdef CONFIG_USER_ONLY
> -        /* 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 */
> +    /* 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
>   
> -        bitmap_fill(cpu->sve_vq_supported, ARM_MAX_VQ);
> -    }
> +    bitmap_fill(cpu->sve_vq_supported, ARM_MAX_VQ);
>   
>       aarch64_add_pauth_properties(obj);
>       aarch64_add_sve_properties(obj);

[PATCH 5/6] target/arm: Fix '-cpu max' for HVF

[Peter Maydell]

Currently when using hvf we mishandle '-cpu max': we fall through to
the TCG version of its initfn, which then sets a lot of feature bits
that the real host CPU doesn't have. The hvf accelerator code then
exposes these bogus ID register values to the guest because it
doesn't check that the host really has the features.

Make '-cpu host' be like '-cpu max' for hvf, as we do with kvm.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/cpu64.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index bc25a2567bf..fd611c97116 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -29,6 +29,7 @@
 #include "hw/loader.h"
 #endif
 #include "sysemu/kvm.h"
+#include "sysemu/hvf.h"
 #include "kvm_arm.h"
 #include "hvf_arm.h"
 #include "qapi/visitor.h"
@@ -710,8 +711,8 @@ static void aarch64_max_initfn(Object *obj)
     uint64_t t;
     uint32_t u;
 
-    if (kvm_enabled()) {
-        /* With KVM, '-cpu max' is identical to '-cpu host' */
+    if (kvm_enabled() || hvf_enabled()) {
+        /* With KVM or HVF, '-cpu max' is identical to '-cpu host' */
         aarch64_host_initfn(obj);
         return;
     }
-- 
2.25.1

Re: [PATCH 5/6] target/arm: Fix '-cpu max' for HVF

[Richard Henderson]

On 2/5/22 03:55, Peter Maydell wrote:
> Currently when using hvf we mishandle '-cpu max': we fall through to
> the TCG version of its initfn, which then sets a lot of feature bits
> that the real host CPU doesn't have. The hvf accelerator code then
> exposes these bogus ID register values to the guest because it
> doesn't check that the host really has the features.
> 
> Make '-cpu host' be like '-cpu max' for hvf, as we do with kvm.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   target/arm/cpu64.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 6/6] target/arm: Support PAuth extension for hvf

[Peter Maydell]

Currently we don't allow guests under hvf to use the PAuth extension,
because we didn't have any special code to handle that, and therefore
in arm_cpu_pauth_finalize() we will sanitize the ID_AA64ISAR1 value
the guest sees to clear the PAuth related fields.

Add support for this in the same way that KVM does it, by defaulting
to "PAuth enabled" if the host CPU has it and allowing the user to
disable it via '-cpu pauth=no' on the command line.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/cpu64.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index fd611c97116..5be5ade6c9d 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -633,9 +633,10 @@ void arm_cpu_pauth_finalize(ARMCPU *cpu, Error **errp)
     uint64_t t;
 
     /* Exit early if PAuth is enabled, and fall through to disable it */
-    if (kvm_enabled() && cpu->prop_pauth) {
+    if ((kvm_enabled() || hvf_enabled()) && cpu->prop_pauth) {
         if (!cpu_isar_feature(aa64_pauth, cpu)) {
-            error_setg(errp, "'pauth' feature not supported by KVM on this host");
+            error_setg(errp, "'pauth' feature not supported by %s on this host",
+                       kvm_enabled() ? "KVM" : "hvf");
         }
 
         return;
@@ -672,10 +673,14 @@ void aarch64_add_pauth_properties(Object *obj)
 
     /* Default to PAUTH on, with the architected algorithm on TCG. */
     qdev_property_add_static(DEVICE(obj), &arm_cpu_pauth_property);
-    if (kvm_enabled()) {
+    if (kvm_enabled() || hvf_enabled()) {
         /*
          * Mirror PAuth support from the probed sysregs back into the
-         * property for KVM. Is it just a bit backward? Yes it is!
+         * property for KVM or hvf. Is it just a bit backward? Yes it is!
+         * Note that prop_pauth is true whether the host CPU supports the
+         * architected QARMA5 algorithm or the IMPDEF one. We don't
+         * provide the separate pauth-impdef property for KVM or hvf,
+         * only for TCG.
          */
         cpu->prop_pauth = cpu_isar_feature(aa64_pauth, cpu);
     } else {
@@ -695,6 +700,7 @@ static void aarch64_host_initfn(Object *obj)
 #elif defined(CONFIG_HVF)
     ARMCPU *cpu = ARM_CPU(obj);
     hvf_arm_set_cpu_features_from_host(cpu);
+    aarch64_add_pauth_properties(obj);
 #else
     g_assert_not_reached();
 #endif
-- 
2.25.1

Re: [PATCH 6/6] target/arm: Support PAuth extension for hvf

[Richard Henderson]

On 2/5/22 03:55, Peter Maydell wrote:
> Currently we don't allow guests under hvf to use the PAuth extension,
> because we didn't have any special code to handle that, and therefore
> in arm_cpu_pauth_finalize() we will sanitize the ID_AA64ISAR1 value
> the guest sees to clear the PAuth related fields.
> 
> Add support for this in the same way that KVM does it, by defaulting
> to "PAuth enabled" if the host CPU has it and allowing the user to
> disable it via '-cpu pauth=no' on the command line.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   target/arm/cpu64.c | 14 ++++++++++----
>   1 file changed, 10 insertions(+), 4 deletions(-)

Currently not a problem in practice because M1 doesn't support PAuth, so the ID fields are 
already clear.  However, it is a confusing difference to kvm, and presumably some future 
Apple chip will enable PAuth.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH 6/6] target/arm: Support PAuth extension for hvf

[Peter Maydell]

On Sun, 6 Feb 2022 at 00:26, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 2/5/22 03:55, Peter Maydell wrote:
> > Currently we don't allow guests under hvf to use the PAuth extension,
> > because we didn't have any special code to handle that, and therefore
> > in arm_cpu_pauth_finalize() we will sanitize the ID_AA64ISAR1 value
> > the guest sees to clear the PAuth related fields.
> >
> > Add support for this in the same way that KVM does it, by defaulting
> > to "PAuth enabled" if the host CPU has it and allowing the user to
> > disable it via '-cpu pauth=no' on the command line.
> >
> > Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> > ---
> >   target/arm/cpu64.c | 14 ++++++++++----
> >   1 file changed, 10 insertions(+), 4 deletions(-)
>
> Currently not a problem in practice because M1 doesn't support PAuth, so the ID fields are
> already clear.  However, it is a confusing difference to kvm, and presumably some future
> Apple chip will enable PAuth.

No, this is an actual bug that was reported to me. The M1
does support PAuth, with an IMPDEF algorithm (and no
support for QARMA5), and we were suppressing this by
clearing the ID register fields.

thanks
-- PMM

Re: [PATCH 0/6] target/arm: -cpu host/max KVM and HVF fixes

[Philippe Mathieu-Daudé via]

On 4/2/22 17:55, Peter Maydell wrote:

> Peter Maydell (6):
>    target/arm: Move '-cpu host' code to cpu64.c
>    target/arm: Use aarch64_cpu_register() for 'host' CPU type
>    target/arm: Make KVM -cpu max exactly like -cpu host
>    target/arm: Unindent unnecessary else-clause
>    target/arm: Fix '-cpu max' for HVF
>    target/arm: Support PAuth extension for hvf

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

Re: [PATCH 0/6] target/arm: -cpu host/max KVM and HVF fixes

[Andrew Jones]

On Fri, Feb 04, 2022 at 04:55:00PM +0000, Peter Maydell wrote:
> This patchset fixes various minor bugs in KVM and HVF
> -cpu host and -cpu max:
> 
> (1) KVM -cpu max was incorrectly adding a 'sve-max-vq' property
> that wouldn't work and which doesn't exist in KVM -cpu host
> 
> (2) HVF -cpu max was using all the TCG ID fields and thus
> promising the guest more than the host CPU can actually do
> 
> (3) HVF -cpu host wasn't setting up the PAuth properties, so
> it defaulted to not telling the guest about PAuth support
> 
> This series fixes (1) by moving the '-cpu host' code to
> cpu64.c since it's aarch64-specific anyway, which lets us
> reuse it from the '-cpu max' init function. It fixes
> (2) and (3) mostly by making HVF use the same bits of code
> that KVM does for -cpu max and PAuth.
> 
> thanks
> -- PMM
> 
> Peter Maydell (6):
>   target/arm: Move '-cpu host' code to cpu64.c
>   target/arm: Use aarch64_cpu_register() for 'host' CPU type
>   target/arm: Make KVM -cpu max exactly like -cpu host
>   target/arm: Unindent unnecessary else-clause
>   target/arm: Fix '-cpu max' for HVF
>   target/arm: Support PAuth extension for hvf
> 
>  target/arm/cpu.c   |  30 -----
>  target/arm/cpu64.c | 330 +++++++++++++++++++++++++--------------------
>  2 files changed, 181 insertions(+), 179 deletions(-)
> 
> -- 
> 2.25.1
>

For the series
 
Reviewed-by: Andrew Jones <drjones@redhat.com>

Thanks,
drew

Re: [PATCH 0/6] target/arm: -cpu host/max KVM and HVF fixes

[Alexander Graf]

On 04.02.22 17:55, Peter Maydell wrote:
> This patchset fixes various minor bugs in KVM and HVF
> -cpu host and -cpu max:
>
> (1) KVM -cpu max was incorrectly adding a 'sve-max-vq' property
> that wouldn't work and which doesn't exist in KVM -cpu host
>
> (2) HVF -cpu max was using all the TCG ID fields and thus
> promising the guest more than the host CPU can actually do
>
> (3) HVF -cpu host wasn't setting up the PAuth properties, so
> it defaulted to not telling the guest about PAuth support
>
> This series fixes (1) by moving the '-cpu host' code to
> cpu64.c since it's aarch64-specific anyway, which lets us
> reuse it from the '-cpu max' init function. It fixes
> (2) and (3) mostly by making HVF use the same bits of code
> that KVM does for -cpu max and PAuth.


Reviewed-by: Alexander Graf <agraf@csgraf.de>


Alex