[Qemu-devel] [RFC PATCH 00/13] target/arm: Derive cpu id regs from features

[Qemu-devel] [RFC PATCH 00/13] target/arm: Derive cpu id regs from features

[Richard Henderson]

This is something we talked about in the context of enabling sve in
system mode.  We don't want to replicate info between these two locations.

I'm not 100% happy with this, thus the RFC.  In particular, there are
several places in id_isar0, id_isar2, and id_isar4 that expose micro-
architectural details of the cpus.  We cannot infer these values.
We'll not be able to replicate the exact id values without additional
changes.

But I'll also note that with ARM_FEATURE_SWP, we're now at 60 feature
bits, which means that we only have 4 remaining before we have to come
up with another solution there.

I do wonder if we should instead introduce some little inline functions
to test each of the current feature bits, and once that's done convert
those to test cpu->id_* bits.

Most, but not all, of the feature bits would go away.  We'd have the
exact id values one would expect for a given cpu without having to
replicate the info.

Thoughts, one way or the other?


r~


Richard Henderson (13):
  target/arm: Add ARM_FEATURE_SWP
  target/arm: Derive id_isar0 from features
  target/arm: Derive id_isar1 from features
  target/arm: Derive id_isar2 from features
  target/arm: Derive id_isar3 from features
  target/arm: Derive id_isar4 from features
  target/arm: Derive id_isar5 and id_isar6 from features
  target/arm: Derive id_pfr0 from features
  target/arm: Derive id_pfr1 from features
  target/arm: Derive id_aa64isar0 from features
  target/arm: Derive id_aa64isar1 from features
  target/arm: Derive id_aa64pfr0 from features
  target/arm: Remove assertions from resolve_id_regs

 target/arm/cpu.h       |   1 +
 linux-user/elfload.c   |   3 +-
 target/arm/cpu.c       | 381 +++++++++++++++++++++++++++++++++++++++++
 target/arm/translate.c |   4 +
 4 files changed, 388 insertions(+), 1 deletion(-)

-- 
2.17.1

[Qemu-devel] [PATCH 01/13] target/arm: Add ARM_FEATURE_SWP

[Richard Henderson]

These insns have been removed from the ISA, but are also not
present on some cpus with V7VE.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/cpu.h       |  1 +
 linux-user/elfload.c   |  3 ++-
 target/arm/cpu.c       | 10 ++++++++++
 target/arm/translate.c |  4 ++++
 4 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 65c0fa0a65..acfb2f9104 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1495,6 +1495,7 @@ enum arm_features {
     ARM_FEATURE_V8_FP16, /* implements v8.2 half-precision float */
     ARM_FEATURE_V8_FCMA, /* has complex number part of v8.3 extensions.  */
     ARM_FEATURE_M_MAIN, /* M profile Main Extension */
+    ARM_FEATURE_SWP,  /* implements swp/swpb */
 };
 
 static inline int arm_feature(CPUARMState *env, int feature)
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 8638612aec..fcac2563f1 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -450,7 +450,6 @@ static uint32_t get_elf_hwcap(void)
     ARMCPU *cpu = ARM_CPU(thread_cpu);
     uint32_t hwcaps = 0;
 
-    hwcaps |= ARM_HWCAP_ARM_SWP;
     hwcaps |= ARM_HWCAP_ARM_HALF;
     hwcaps |= ARM_HWCAP_ARM_THUMB;
     hwcaps |= ARM_HWCAP_ARM_FAST_MULT;
@@ -458,7 +457,9 @@ static uint32_t get_elf_hwcap(void)
     /* probe for the extra features */
 #define GET_FEATURE(feat, hwcap) \
     do { if (arm_feature(&cpu->env, feat)) { hwcaps |= hwcap; } } while (0)
+
     /* EDSP is in v5TE and above, but all our v5 CPUs are v5TE */
+    GET_FEATURE(ARM_FEATURE_SWP, ARM_HWCAP_ARM_SWP);
     GET_FEATURE(ARM_FEATURE_V5, ARM_HWCAP_ARM_EDSP);
     GET_FEATURE(ARM_FEATURE_VFP, ARM_HWCAP_ARM_VFP);
     GET_FEATURE(ARM_FEATURE_IWMMXT, ARM_HWCAP_ARM_IWMMXT);
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 258ba6dcaa..3bc7a16327 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1075,6 +1075,7 @@ static void arm926_initfn(Object *obj)
     set_feature(&cpu->env, ARM_FEATURE_DUMMY_C15_REGS);
     set_feature(&cpu->env, ARM_FEATURE_CACHE_TEST_CLEAN);
     set_feature(&cpu->env, ARM_FEATURE_JAZELLE);
+    set_feature(&cpu->env, ARM_FEATURE_SWP);
     cpu->midr = 0x41069265;
     cpu->reset_fpsid = 0x41011090;
     cpu->ctr = 0x1dd20d2;
@@ -1089,6 +1090,7 @@ static void arm946_initfn(Object *obj)
     set_feature(&cpu->env, ARM_FEATURE_V5);
     set_feature(&cpu->env, ARM_FEATURE_PMSA);
     set_feature(&cpu->env, ARM_FEATURE_DUMMY_C15_REGS);
+    set_feature(&cpu->env, ARM_FEATURE_SWP);
     cpu->midr = 0x41059461;
     cpu->ctr = 0x0f004006;
     cpu->reset_sctlr = 0x00000078;
@@ -1105,6 +1107,7 @@ static void arm1026_initfn(Object *obj)
     set_feature(&cpu->env, ARM_FEATURE_DUMMY_C15_REGS);
     set_feature(&cpu->env, ARM_FEATURE_CACHE_TEST_CLEAN);
     set_feature(&cpu->env, ARM_FEATURE_JAZELLE);
+    set_feature(&cpu->env, ARM_FEATURE_SWP);
     cpu->midr = 0x4106a262;
     cpu->reset_fpsid = 0x410110a0;
     cpu->ctr = 0x1dd20d2;
@@ -1139,6 +1142,7 @@ static void arm1136_r2_initfn(Object *obj)
     set_feature(&cpu->env, ARM_FEATURE_DUMMY_C15_REGS);
     set_feature(&cpu->env, ARM_FEATURE_CACHE_DIRTY_REG);
     set_feature(&cpu->env, ARM_FEATURE_CACHE_BLOCK_OPS);
+    set_feature(&cpu->env, ARM_FEATURE_SWP);
     cpu->midr = 0x4107b362;
     cpu->reset_fpsid = 0x410120b4;
     cpu->mvfr0 = 0x11111111;
@@ -1171,6 +1175,7 @@ static void arm1136_initfn(Object *obj)
     set_feature(&cpu->env, ARM_FEATURE_DUMMY_C15_REGS);
     set_feature(&cpu->env, ARM_FEATURE_CACHE_DIRTY_REG);
     set_feature(&cpu->env, ARM_FEATURE_CACHE_BLOCK_OPS);
+    set_feature(&cpu->env, ARM_FEATURE_SWP);
     cpu->midr = 0x4117b363;
     cpu->reset_fpsid = 0x410120b4;
     cpu->mvfr0 = 0x11111111;
@@ -1204,6 +1209,7 @@ static void arm1176_initfn(Object *obj)
     set_feature(&cpu->env, ARM_FEATURE_CACHE_DIRTY_REG);
     set_feature(&cpu->env, ARM_FEATURE_CACHE_BLOCK_OPS);
     set_feature(&cpu->env, ARM_FEATURE_EL3);
+    set_feature(&cpu->env, ARM_FEATURE_SWP);
     cpu->midr = 0x410fb767;
     cpu->reset_fpsid = 0x410120b5;
     cpu->mvfr0 = 0x11111111;
@@ -1235,6 +1241,7 @@ static void arm11mpcore_initfn(Object *obj)
     set_feature(&cpu->env, ARM_FEATURE_VAPA);
     set_feature(&cpu->env, ARM_FEATURE_MPIDR);
     set_feature(&cpu->env, ARM_FEATURE_DUMMY_C15_REGS);
+    set_feature(&cpu->env, ARM_FEATURE_SWP);
     cpu->midr = 0x410fb022;
     cpu->reset_fpsid = 0x410120b4;
     cpu->mvfr0 = 0x11111111;
@@ -1378,6 +1385,7 @@ static void cortex_r5_initfn(Object *obj)
     set_feature(&cpu->env, ARM_FEATURE_ARM_DIV);
     set_feature(&cpu->env, ARM_FEATURE_V7MP);
     set_feature(&cpu->env, ARM_FEATURE_PMSA);
+    set_feature(&cpu->env, ARM_FEATURE_SWP);
     cpu->midr = 0x411fc153; /* r1p3 */
     cpu->id_pfr0 = 0x0131;
     cpu->id_pfr1 = 0x001;
@@ -1426,6 +1434,7 @@ static void cortex_a8_initfn(Object *obj)
     set_feature(&cpu->env, ARM_FEATURE_THUMB2EE);
     set_feature(&cpu->env, ARM_FEATURE_DUMMY_C15_REGS);
     set_feature(&cpu->env, ARM_FEATURE_EL3);
+    set_feature(&cpu->env, ARM_FEATURE_SWP);
     cpu->midr = 0x410fc080;
     cpu->reset_fpsid = 0x410330c0;
     cpu->mvfr0 = 0x11110222;
@@ -1500,6 +1509,7 @@ static void cortex_a9_initfn(Object *obj)
      */
     set_feature(&cpu->env, ARM_FEATURE_V7MP);
     set_feature(&cpu->env, ARM_FEATURE_CBAR);
+    set_feature(&cpu->env, ARM_FEATURE_SWP);
     cpu->midr = 0x410fc090;
     cpu->reset_fpsid = 0x41033090;
     cpu->mvfr0 = 0x11110222;
diff --git a/target/arm/translate.c b/target/arm/translate.c
index c6a5d2ac44..2688380ae6 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -9390,6 +9390,10 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn)
                         TCGv taddr;
                         TCGMemOp opc = s->be_data;
 
+                        if (!arm_dc_feature(s, ARM_FEATURE_SWP)) {
+                            goto illegal_op;
+                        }
+
                         rm = (insn) & 0xf;
 
                         if (insn & (1 << 22)) {
-- 
2.17.1

[Qemu-devel] [PATCH 02/13] target/arm: Derive id_isar0 from features

[Richard Henderson]

??? Test with -machine none -cpu foo.
??? The assertion does fire for quite a lot of cpus,
??? but quite a few of them appear to be existing bugs.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/cpu.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 3bc7a16327..44483e3dea 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -756,6 +756,50 @@ static void arm_cpu_finalizefn(Object *obj)
     }
 }
 
+static uint32_t resolve_id_isar0(CPUARMState *env, uint32_t orig)
+{
+    uint32_t ret = 0;
+
+    if (arm_feature(env, ARM_FEATURE_SWP)) {
+        ret = deposit32(ret, 0, 4, 1);            /* Swap */
+    }
+    if (arm_feature(env, ARM_FEATURE_V5)) {
+        ret = deposit32(ret , 4, 4, 1);           /* BitCount */
+    }
+    if (arm_feature(env, ARM_FEATURE_THUMB2)) {
+        ret = deposit32(ret, 8, 4, 1);            /* BitField */
+        ret = deposit32(ret, 12, 4, 1);           /* CmpBranch */
+    }
+
+    /*
+     * Coproc -- generically, v5te has mcrr (3), v6 has mcrr2 (4),
+     * and v8 requires none (0).  There does not appear to be a way
+     * to guess the value though, as some v6 and v7 cores also use none.
+     */
+    ret |= orig & MAKE_64BIT_MASK(16, 4);
+
+    if (arm_feature(env, ARM_FEATURE_V5)) {
+        ret = deposit32(ret, 20, 4, 1);           /* Debug */
+    }
+    /* Divide */
+    if (arm_feature(env, ARM_FEATURE_ARM_DIV)) {
+        ret = deposit32(ret, 24, 4, 2);
+    } else if (arm_feature(env, ARM_FEATURE_THUMB_DIV)) {
+        ret = deposit32(ret, 24, 4, 1);
+    }
+
+    return ret;
+}
+
+static void resolve_id_regs(ARMCPU *cpu)
+{
+    CPUARMState *env = &cpu->env;
+    uint64_t orig;
+
+    cpu->id_isar0 = resolve_id_isar0(env, orig = cpu->id_isar0);
+    g_assert_cmphex(cpu->id_isar0, ==, orig);
+}
+
 static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
 {
     CPUState *cs = CPU(dev);
@@ -1003,6 +1047,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
         set_feature(env, ARM_FEATURE_VBAR);
     }
 
+    resolve_id_regs(cpu);
     register_cp_regs_for_features(cpu);
     arm_cpu_register_gdb_regs_for_features(cpu);
 
-- 
2.17.1

[Qemu-devel] [PATCH 03/13] target/arm: Derive id_isar1 from features

[Richard Henderson]

??? The assertion does fire for quite a lot of cpus,
??? but quite a few of them appear to be existing bugs.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/cpu.c | 37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 44483e3dea..a477e722af 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -791,13 +791,48 @@ static uint32_t resolve_id_isar0(CPUARMState *env, uint32_t orig)
     return ret;
 }
 
+static uint32_t resolve_id_isar1(CPUARMState *env)
+{
+    uint32_t ret = 0;
+
+    if (arm_feature(env, ARM_FEATURE_V6)) {
+        ret = deposit32(ret, 0, 4, 1);            /* Endian */
+        if (!arm_feature(env, ARM_FEATURE_M)) {
+            ret = deposit32(ret, 4, 4, 1);        /* Except */
+            ret = deposit32(ret, 8, 4, 1);        /* Except_AR */
+        }
+        /* Extend */
+        ret = deposit32(ret, 12, 4,
+                             arm_feature(env, ARM_FEATURE_THUMB_DSP) ? 2 : 1);
+    }
+    if (arm_feature(env, ARM_FEATURE_THUMB2)) {
+        ret = deposit32(ret, 16, 4, 1);           /* IfThen */
+        ret = deposit32(ret, 20, 4, 1);           /* Immediate */
+    }
+    /* Interwork -- note we don't support pre-armv4t.  */
+    ret = deposit32(ret, 24, 4,
+                         arm_feature(env, ARM_FEATURE_V7)
+                         && !arm_feature(env, ARM_FEATURE_M) ? 3 :
+                         arm_feature(env, ARM_FEATURE_V5) ? 2 : 1);
+    if (arm_feature(env, ARM_FEATURE_JAZELLE)) {
+        ret = deposit32(ret, 28, 4, 1);           /* Jazelle */
+    }
+
+    return ret;
+}
+
 static void resolve_id_regs(ARMCPU *cpu)
 {
     CPUARMState *env = &cpu->env;
     uint64_t orig;
 
-    cpu->id_isar0 = resolve_id_isar0(env, orig = cpu->id_isar0);
+    orig = cpu->id_isar0;
+    cpu->id_isar0 = resolve_id_isar0(env, orig);
     g_assert_cmphex(cpu->id_isar0, ==, orig);
+
+    orig = cpu->id_isar1;
+    cpu->id_isar1 = resolve_id_isar1(env);
+    g_assert_cmphex(cpu->id_isar1, ==, orig);
 }
 
 static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
-- 
2.17.1

[Qemu-devel] [PATCH 04/13] target/arm: Derive id_isar2 from features

[Richard Henderson]

??? The assertion does fire for the old cpus; they may be existing bugs.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/cpu.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index a477e722af..379d6a08a4 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -821,6 +821,40 @@ static uint32_t resolve_id_isar1(CPUARMState *env)
     return ret;
 }
 
+static uint32_t resolve_id_isar2(CPUARMState *env, uint32_t orig)
+{
+    uint32_t ret = 0;
+
+    /* LoadStore */
+    ret = deposit32(ret, 0, 4,
+                    arm_feature(env, ARM_FEATURE_V8) ? 2 :
+                    arm_feature(env, ARM_FEATURE_V5) ? 1 : 0);
+    /*
+     * MemHint -- v7mp has pldw (4), v7 has pli (3), but values 1 & 2
+     * mean the same thing, and there does not seem to be a way to tell
+     * them apart for v5 & v6.
+     */
+    ret |= orig & MAKE_64BIT_MASK(4, 4);
+    /* MultiAccessInt -- micro-architectural detail.  */
+    ret |= orig & MAKE_64BIT_MASK(8, 4);
+    /* Mult -- note we don't support pre-armv4t.  */
+    ret = deposit32(ret, 12, 4, arm_feature(env, ARM_FEATURE_THUMB2) ? 2 : 1);
+    /* MultS -- note we don't support pre-armv4t.  */
+    ret = deposit32(ret, 16, 4,
+                    arm_feature(env, ARM_FEATURE_V6) ? 3 :
+                    arm_feature(env, ARM_FEATURE_V5) ? 2 : 1);
+    /* MultU -- note we don't support pre-armv4t.  */
+    ret = deposit32(ret, 20, 4, arm_feature(env, ARM_FEATURE_V6) ? 2 : 1);
+    /* PSR_AR */
+    ret = deposit32(ret, 24, 4, arm_feature(env, ARM_FEATURE_M) ? 0 : 1);
+    /* Reversal */
+    ret = deposit32(ret, 28, 4,
+                    arm_feature(env, ARM_FEATURE_THUMB2) ? 2 :
+                    arm_feature(env, ARM_FEATURE_V6) ? 1 : 0);
+
+    return ret;
+}
+
 static void resolve_id_regs(ARMCPU *cpu)
 {
     CPUARMState *env = &cpu->env;
@@ -833,6 +867,10 @@ static void resolve_id_regs(ARMCPU *cpu)
     orig = cpu->id_isar1;
     cpu->id_isar1 = resolve_id_isar1(env);
     g_assert_cmphex(cpu->id_isar1, ==, orig);
+
+    orig = cpu->id_isar2;
+    cpu->id_isar2 = resolve_id_isar2(env, orig);
+    g_assert_cmphex(cpu->id_isar2, ==, orig);
 }
 
 static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
-- 
2.17.1

[Qemu-devel] [PATCH 05/13] target/arm: Derive id_isar3 from features

[Richard Henderson]

??? The assertion does fire for the old cpus; they may be existing bugs.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/cpu.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 379d6a08a4..2b199845fc 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -855,6 +855,38 @@ static uint32_t resolve_id_isar2(CPUARMState *env, uint32_t orig)
     return ret;
 }
 
+static uint32_t resolve_id_isar3(CPUARMState *env)
+{
+    uint32_t ret = 0;
+
+    if (arm_feature(env, ARM_FEATURE_V5)) {
+        ret = deposit32(ret, 0, 4, 1);            /* Saturate */
+    }
+    if (arm_feature(env, ARM_FEATURE_V6)) {
+        ret = deposit32(ret, 4, 4, 3);            /* SIMD */
+    }
+    ret = deposit32(ret, 8, 4, 1);                /* SVC -- no pre-armv4t */
+    /* SynchPrim */
+    if (arm_feature(env, ARM_FEATURE_V6K)) {
+        ret = deposit32(ret, 12, 4, 2);           /* ldrex, ldrexb, ldrexd */
+    } else if (arm_feature(env, ARM_FEATURE_V6)) {
+        ret = deposit32(ret, 12, 4, 1);           /* ldrex only */
+    }
+    if (arm_feature(env, ARM_FEATURE_THUMB2)) {
+        ret = deposit32(ret, 16, 4, 1);           /* TabBranch */
+        ret = deposit32(ret, 20, 4, 1);           /* T32Copy */
+    }
+    if (arm_feature(env, ARM_FEATURE_THUMB2) ||
+        arm_feature(env, ARM_FEATURE_V6K)) {
+        ret = deposit32(ret, 24, 4, 1);           /* TrueNOP */
+    }
+    if (arm_feature(env, ARM_FEATURE_THUMB2EE)) {
+        ret = deposit32(ret, 28, 4, 1);           /* T32EE */
+    }
+
+    return ret;
+}
+
 static void resolve_id_regs(ARMCPU *cpu)
 {
     CPUARMState *env = &cpu->env;
@@ -871,6 +903,10 @@ static void resolve_id_regs(ARMCPU *cpu)
     orig = cpu->id_isar2;
     cpu->id_isar2 = resolve_id_isar2(env, orig);
     g_assert_cmphex(cpu->id_isar2, ==, orig);
+
+    orig = cpu->id_isar3;
+    cpu->id_isar3 = resolve_id_isar3(env);
+    g_assert_cmphex(cpu->id_isar3, ==, orig);
 }
 
 static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
-- 
2.17.1

[Qemu-devel] [PATCH 06/13] target/arm: Derive id_isar4 from features

[Richard Henderson]

??? The assertion does fire for the old cpus; they may be existing bugs.
??? Willfully provide a value for SWP_frac that matches our implementation.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/cpu.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 2b199845fc..3c6ddd6532 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -887,6 +887,45 @@ static uint32_t resolve_id_isar3(CPUARMState *env)
     return ret;
 }
 
+static uint32_t resolve_id_isar4(CPUARMState *env)
+{
+    uint32_t ret = 0;
+
+    /* Unpriv -- note we don't support pre-armv4t.  */
+    ret = deposit32(ret, 0, 4, arm_feature(env, ARM_FEATURE_THUMB2) ? 2 : 1);
+    /* WithShifts */
+    if (!arm_feature(env, ARM_FEATURE_M)) {
+        ret = deposit32(ret, 4, 4, 4);
+    } else if (arm_feature(env, ARM_FEATURE_V8)) {
+        ret = deposit32(ret, 4, 4, 3);
+    }
+    ret = deposit32(ret, 8, 4, 1);                /* Writeback */
+    if (arm_feature(env, ARM_FEATURE_EL3)) {
+        /* Note that EL3 indicates Security Extensions.  */
+        /* ??? In translate.c we check V6K instead.  */
+        ret = deposit32(ret, 12, 4, 1);           /* SMC */
+    }
+    if (arm_feature(env, ARM_FEATURE_V7)) {
+        ret = deposit32(ret, 16, 4, 1);           /* Barrier */
+    }
+    if (!arm_feature(env, ARM_FEATURE_V6K) &&
+        arm_feature(env, ARM_FEATURE_V6)) {
+        ret = deposit32(ret, 20, 4, 3);           /* SyncPrim_frac */
+    }
+    if (arm_feature(env, ARM_FEATURE_M)) {
+        ret = deposit32(ret, 24, 4, 1);           /* PSR_M */
+    }
+    /*
+     * SWP_frac -- Value 1 indicates that SWP and SWPB only work in a
+     * uniprocessor context.  Looking at ARM_FEATURE_SWP, we will have
+     * already set ID_ISAR0.Swap to 1, which means that SWP_frac must
+     * be ignored.  While leaving this field 0 may not match certain
+     * real cpus, it is correct with respect to our implementation.
+     */
+
+    return ret;
+}
+
 static void resolve_id_regs(ARMCPU *cpu)
 {
     CPUARMState *env = &cpu->env;
@@ -907,6 +946,11 @@ static void resolve_id_regs(ARMCPU *cpu)
     orig = cpu->id_isar3;
     cpu->id_isar3 = resolve_id_isar3(env);
     g_assert_cmphex(cpu->id_isar3, ==, orig);
+
+    orig = cpu->id_isar4;
+    cpu->id_isar4 = resolve_id_isar4(env);
+    /* Willfully ignore the SWP_frac field.  */
+    g_assert_cmphex(cpu->id_isar4 & 0x0fffffff, ==, orig & 0x0fffffff);
 }
 
 static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
-- 
2.17.1

[Qemu-devel] [PATCH 07/13] target/arm: Derive id_isar5 and id_isar6 from features

[Richard Henderson]

Unlike the other id_sar registers, these contain post-v8.0 features
that are not included with any existing cpu models.  They would be
enabled by -cpu max when we enable them for system mode.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/cpu.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 3c6ddd6532..c227044946 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -926,6 +926,49 @@ static uint32_t resolve_id_isar4(CPUARMState *env)
     return ret;
 }
 
+static uint32_t resolve_id_isar5(CPUARMState *env)
+{
+    uint32_t ret = 0;
+
+    /* SEVL -- we always implement as NOP.  */
+    /* AES */
+    if (arm_feature(env, ARM_FEATURE_V8_PMULL)) {
+        ret = deposit32(ret, 4, 4, 2);
+    } else if (arm_feature(env, ARM_FEATURE_V8_AES)) {
+        ret = deposit32(ret, 4, 4, 1);
+    }
+    if (arm_feature(env, ARM_FEATURE_V8_SHA1)) {
+        ret = deposit32(ret, 8, 4, 1);            /* SHA1 */
+    }
+    if (arm_feature(env, ARM_FEATURE_V8_SHA256)) {
+        ret = deposit32(ret, 12, 4, 1);           /* SHA2 */
+    }
+    if (arm_feature(env, ARM_FEATURE_CRC)) {
+        ret = deposit32(ret, 16, 4, 1);           /* CRC32 */
+    }
+    if (arm_feature(env, ARM_FEATURE_V8_RDM)) {
+        ret = deposit32(ret, 24, 4, 1);           /* RDM */
+    }
+    if (arm_feature(env, ARM_FEATURE_V8_FCMA)) {
+        ret = deposit32(ret, 28, 4, 1);           /* VCMA */
+    }
+
+    return ret;
+}
+
+static uint32_t resolve_id_isar6(CPUARMState *env)
+{
+    uint32_t ret = 0;
+
+    /* JSCVT -- not implemented yet */
+    /* FHM -- not implemented yet */
+    if (arm_feature(env, ARM_FEATURE_V8_DOTPROD)) {
+        ret = deposit32(ret, 4, 4, 1);            /* DP */
+    }
+
+    return ret;
+}
+
 static void resolve_id_regs(ARMCPU *cpu)
 {
     CPUARMState *env = &cpu->env;
@@ -951,6 +994,9 @@ static void resolve_id_regs(ARMCPU *cpu)
     cpu->id_isar4 = resolve_id_isar4(env);
     /* Willfully ignore the SWP_frac field.  */
     g_assert_cmphex(cpu->id_isar4 & 0x0fffffff, ==, orig & 0x0fffffff);
+
+    cpu->id_isar5 = resolve_id_isar5(env);
+    cpu->id_isar6 = resolve_id_isar6(env);
 }
 
 static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
-- 
2.17.1

[Qemu-devel] [PATCH 08/13] target/arm: Derive id_pfr0 from features

[Richard Henderson]

??? The assertion does fire for old cpus; they may be existing bugs.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/cpu.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index c227044946..0151c278e8 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -969,6 +969,29 @@ static uint32_t resolve_id_isar6(CPUARMState *env)
     return ret;
 }
 
+static uint32_t resolve_id_pfr0(CPUARMState *env)
+{
+    uint32_t ret = 0;
+
+    if (!arm_feature(env, ARM_FEATURE_M)) {
+        ret = deposit32(ret, 0, 4, 1);            /* State0 -- A32 */
+    }
+    if (arm_feature(env, ARM_FEATURE_THUMB2)) {
+        ret = deposit32(ret, 4, 4, 3);            /* State1 -- T32 */
+    } else if (arm_feature(env, ARM_FEATURE_V5)) {
+        ret = deposit32(ret, 4, 4, 1);            /* State1 -- bl/blx only */
+    }
+    if (arm_feature(env, ARM_FEATURE_JAZELLE)) {
+        ret = deposit32(ret, 8, 4, 1);            /* State2 -- Jazelle */
+    }
+    if (arm_feature(env, ARM_FEATURE_THUMB2EE)) {
+        ret = deposit32(ret, 12, 4, 1);           /* State3 -- T32EE */
+    }
+    /* RAS -- not implemented yet */
+
+    return ret;
+}
+
 static void resolve_id_regs(ARMCPU *cpu)
 {
     CPUARMState *env = &cpu->env;
@@ -997,6 +1020,10 @@ static void resolve_id_regs(ARMCPU *cpu)
 
     cpu->id_isar5 = resolve_id_isar5(env);
     cpu->id_isar6 = resolve_id_isar6(env);
+
+    orig = cpu->id_pfr0;
+    cpu->id_pfr0 = resolve_id_pfr0(env);
+    g_assert_cmphex(cpu->id_pfr0, ==, orig);
 }
 
 static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
-- 
2.17.1

[Qemu-devel] [PATCH 09/13] target/arm: Derive id_pfr1 from features

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/cpu.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 0151c278e8..4fb3e0a9ea 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -992,6 +992,31 @@ static uint32_t resolve_id_pfr0(CPUARMState *env)
     return ret;
 }
 
+static uint32_t resolve_id_pfr1(CPUARMState *env)
+{
+    uint32_t ret = 0;
+
+    if (arm_feature(env, ARM_FEATURE_M)) {
+        ret = deposit32(ret, 8, 4, 2);            /* MProgMod */
+    } else {
+        ret = deposit32(ret, 0, 4, 1);            /* ProgMod */
+    }
+    if (arm_feature(env, ARM_FEATURE_EL3)) {
+        ret = deposit32(ret, 4, 4, 1);            /* Security */
+    }
+    if (arm_feature(env, ARM_FEATURE_EL2)) {
+        ret = deposit32(ret, 12, 4, 1);           /* Virtualization */
+    }
+    if (arm_feature(env, ARM_FEATURE_GENERIC_TIMER)) {
+        ret = deposit32(ret, 16, 4, 1);           /* GenTimer */
+    }
+    /* Sec_frac -- no partial features implemented without EL3 */
+    /* Virt_frac -- no partial features implemented without EL2 */
+    /* GIC -- info not available yet; filled in by id_pfr1_read */
+
+    return ret;
+}
+
 static void resolve_id_regs(ARMCPU *cpu)
 {
     CPUARMState *env = &cpu->env;
@@ -1024,6 +1049,10 @@ static void resolve_id_regs(ARMCPU *cpu)
     orig = cpu->id_pfr0;
     cpu->id_pfr0 = resolve_id_pfr0(env);
     g_assert_cmphex(cpu->id_pfr0, ==, orig);
+
+    orig = cpu->id_pfr1;
+    cpu->id_pfr1 = resolve_id_pfr1(env);
+    g_assert_cmphex(cpu->id_pfr1, ==, orig);
 }
 
 static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
-- 
2.17.1

[Qemu-devel] [PATCH 10/13] target/arm: Derive id_aa64isar0 from features

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/cpu.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 4fb3e0a9ea..1c51b9f631 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1017,6 +1017,51 @@ static uint32_t resolve_id_pfr1(CPUARMState *env)
     return ret;
 }
 
+static uint64_t resolve_id_aa64isar0(CPUARMState *env)
+{
+    uint64_t ret = 0;
+
+    /* AES */
+    if (arm_feature(env, ARM_FEATURE_V8_PMULL)) {
+        ret = deposit64(ret, 4, 4, 2);
+    } else if (arm_feature(env, ARM_FEATURE_V8_AES)) {
+        ret = deposit64(ret, 4, 4, 1);
+    }
+    if (arm_feature(env, ARM_FEATURE_V8_SHA1)) {
+        ret = deposit64(ret, 8, 4, 1);            /* SHA1 */
+    }
+    /* SHA2 */
+    if (arm_feature(env, ARM_FEATURE_V8_SHA512)) {
+        ret = deposit64(ret, 12, 4, 2);
+    } else if (arm_feature(env, ARM_FEATURE_V8_SHA256)) {
+        ret = deposit64(ret, 12, 4, 1);
+    }
+    if (arm_feature(env, ARM_FEATURE_CRC)) {
+        ret = deposit64(ret, 16, 4, 1);           /* CRC32 */
+    }
+    if (arm_feature(env, ARM_FEATURE_V8_ATOMICS)) {
+        ret = deposit64(ret, 20, 4, 2);           /* Atomic */
+    }
+    if (arm_feature(env, ARM_FEATURE_V8_RDM)) {
+        ret = deposit64(ret, 28, 4, 1);           /* RDM */
+    }
+    if (arm_feature(env, ARM_FEATURE_V8_SHA3)) {
+        ret = deposit64(ret, 32, 4, 1);           /* SHA3 */
+    }
+    if (arm_feature(env, ARM_FEATURE_V8_SM3)) {
+        ret = deposit64(ret, 36, 4, 1);           /* SM3 */
+    }
+    if (arm_feature(env, ARM_FEATURE_V8_SM4)) {
+        ret = deposit64(ret, 40, 4, 1);           /* SM4 */
+    }
+    if (arm_feature(env, ARM_FEATURE_V8_DOTPROD)) {
+        ret = deposit64(ret, 44, 4, 1);           /* DP */
+    }
+    /* FHM -- not implemented yet */
+
+    return ret;
+}
+
 static void resolve_id_regs(ARMCPU *cpu)
 {
     CPUARMState *env = &cpu->env;
@@ -1053,6 +1098,10 @@ static void resolve_id_regs(ARMCPU *cpu)
     orig = cpu->id_pfr1;
     cpu->id_pfr1 = resolve_id_pfr1(env);
     g_assert_cmphex(cpu->id_pfr1, ==, orig);
+
+    orig = cpu->id_aa64isar0;
+    cpu->id_aa64isar0 = resolve_id_aa64isar0(env);
+    g_assert_cmphex(cpu->id_aa64isar0, ==, orig);
 }
 
 static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
-- 
2.17.1

[Qemu-devel] [PATCH 11/13] target/arm: Derive id_aa64isar1 from features

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/cpu.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 1c51b9f631..a9724f3bb1 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1062,6 +1062,24 @@ static uint64_t resolve_id_aa64isar0(CPUARMState *env)
     return ret;
 }
 
+static uint64_t resolve_id_aa64isar1(CPUARMState *env)
+{
+    uint64_t ret = 0;
+
+    /* DPB -- not implemented yet */
+    /* APA -- not implemented yet */
+    /* API -- not implemented yet */
+    /* JSCVT -- not implemented yet */
+    if (arm_feature(env, ARM_FEATURE_V8_FCMA)) {
+        ret = deposit64(ret, 16, 4, 1);
+    }
+    /* LRCPC -- not implemented yet */
+    /* GPA -- not implemented yet */
+    /* GPI -- not implemented yet */
+
+    return ret;
+}
+
 static void resolve_id_regs(ARMCPU *cpu)
 {
     CPUARMState *env = &cpu->env;
@@ -1102,6 +1120,8 @@ static void resolve_id_regs(ARMCPU *cpu)
     orig = cpu->id_aa64isar0;
     cpu->id_aa64isar0 = resolve_id_aa64isar0(env);
     g_assert_cmphex(cpu->id_aa64isar0, ==, orig);
+
+    cpu->id_aa64isar1 = resolve_id_aa64isar1(env);
 }
 
 static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
-- 
2.17.1

[Qemu-devel] [PATCH 12/13] target/arm: Derive id_aa64pfr0 from features

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/cpu.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index a9724f3bb1..2ec71104c9 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1080,6 +1080,31 @@ static uint64_t resolve_id_aa64isar1(CPUARMState *env)
     return ret;
 }
 
+static uint64_t resolve_id_aa64pfr0(CPUARMState *env)
+{
+    uint64_t ret = 0;
+
+    ret = deposit64(ret, 0, 4, 2);                /* EL0 */
+    ret = deposit64(ret, 4, 4, 2);                /* EL1 */
+    if (arm_feature(env, ARM_FEATURE_EL2)) {
+        ret = deposit64(ret, 8, 4, 2);            /* EL2 */
+    }
+    if (arm_feature(env, ARM_FEATURE_EL3)) {
+        ret = deposit64(ret, 12, 4, 2);           /* EL3 */
+    }
+    if (arm_feature(env, ARM_FEATURE_V8_FP16)) {
+        ret = deposit64(ret, 16, 4, 1);           /* FP */
+        ret = deposit64(ret, 20, 4, 1);           /* AdvSIMD */
+    }
+    /* GIC -- info not available yet; filled in by id_aa64pfr0_read */
+    /* RAS -- not implemented yet */
+    if (arm_feature(env, ARM_FEATURE_SVE)) {
+        ret = deposit64(ret, 32, 4, 1);           /* SVE */
+    }
+
+    return ret;
+}
+
 static void resolve_id_regs(ARMCPU *cpu)
 {
     CPUARMState *env = &cpu->env;
@@ -1122,6 +1147,10 @@ static void resolve_id_regs(ARMCPU *cpu)
     g_assert_cmphex(cpu->id_aa64isar0, ==, orig);
 
     cpu->id_aa64isar1 = resolve_id_aa64isar1(env);
+
+    orig = cpu->id_aa64pfr0;
+    cpu->id_aa64pfr0 = resolve_id_aa64pfr0(env);
+    g_assert_cmphex(cpu->id_aa64pfr0, ==, orig);
 }
 
 static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
-- 
2.17.1

[Qemu-devel] [PATCH 13/13] target/arm: Remove assertions from resolve_id_regs

[Richard Henderson]

This is a prerequisite to removing the now-redundant
initializations from within the individual cpus.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/cpu.c | 41 +++++++----------------------------------
 1 file changed, 7 insertions(+), 34 deletions(-)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 2ec71104c9..79103926a4 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1108,49 +1108,22 @@ static uint64_t resolve_id_aa64pfr0(CPUARMState *env)
 static void resolve_id_regs(ARMCPU *cpu)
 {
     CPUARMState *env = &cpu->env;
-    uint64_t orig;
 
-    orig = cpu->id_isar0;
-    cpu->id_isar0 = resolve_id_isar0(env, orig);
-    g_assert_cmphex(cpu->id_isar0, ==, orig);
-
-    orig = cpu->id_isar1;
+    cpu->id_isar0 = resolve_id_isar0(env, cpu->id_isar0);
     cpu->id_isar1 = resolve_id_isar1(env);
-    g_assert_cmphex(cpu->id_isar1, ==, orig);
-
-    orig = cpu->id_isar2;
-    cpu->id_isar2 = resolve_id_isar2(env, orig);
-    g_assert_cmphex(cpu->id_isar2, ==, orig);
-
-    orig = cpu->id_isar3;
+    cpu->id_isar2 = resolve_id_isar2(env, cpu->id_isar2);
     cpu->id_isar3 = resolve_id_isar3(env);
-    g_assert_cmphex(cpu->id_isar3, ==, orig);
-
-    orig = cpu->id_isar4;
     cpu->id_isar4 = resolve_id_isar4(env);
-    /* Willfully ignore the SWP_frac field.  */
-    g_assert_cmphex(cpu->id_isar4 & 0x0fffffff, ==, orig & 0x0fffffff);
-
     cpu->id_isar5 = resolve_id_isar5(env);
     cpu->id_isar6 = resolve_id_isar6(env);
-
-    orig = cpu->id_pfr0;
     cpu->id_pfr0 = resolve_id_pfr0(env);
-    g_assert_cmphex(cpu->id_pfr0, ==, orig);
-
-    orig = cpu->id_pfr1;
     cpu->id_pfr1 = resolve_id_pfr1(env);
-    g_assert_cmphex(cpu->id_pfr1, ==, orig);
 
-    orig = cpu->id_aa64isar0;
-    cpu->id_aa64isar0 = resolve_id_aa64isar0(env);
-    g_assert_cmphex(cpu->id_aa64isar0, ==, orig);
-
-    cpu->id_aa64isar1 = resolve_id_aa64isar1(env);
-
-    orig = cpu->id_aa64pfr0;
-    cpu->id_aa64pfr0 = resolve_id_aa64pfr0(env);
-    g_assert_cmphex(cpu->id_aa64pfr0, ==, orig);
+    if (arm_feature(env, ARM_FEATURE_AARCH64)) {
+        cpu->id_aa64isar0 = resolve_id_aa64isar0(env);
+        cpu->id_aa64isar1 = resolve_id_aa64isar1(env);
+        cpu->id_aa64pfr0 = resolve_id_aa64pfr0(env);
+    }
 }
 
 static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
-- 
2.17.1

Re: [Qemu-devel] [PATCH 01/13] target/arm: Add ARM_FEATURE_SWP

[Peter Maydell]

On 15 September 2018 at 17:17, Richard Henderson
<richard.henderson@linaro.org> wrote:
> These insns have been removed from the ISA, but are also not
> present on some cpus with V7VE.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/cpu.h       |  1 +
>  linux-user/elfload.c   |  3 ++-
>  target/arm/cpu.c       | 10 ++++++++++
>  target/arm/translate.c |  4 ++++
>  4 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 65c0fa0a65..acfb2f9104 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -1495,6 +1495,7 @@ enum arm_features {
>      ARM_FEATURE_V8_FP16, /* implements v8.2 half-precision float */
>      ARM_FEATURE_V8_FCMA, /* has complex number part of v8.3 extensions.  */
>      ARM_FEATURE_M_MAIN, /* M profile Main Extension */
> +    ARM_FEATURE_SWP,  /* implements swp/swpb */
>  };
>
>  static inline int arm_feature(CPUARMState *env, int feature)
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index 8638612aec..fcac2563f1 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -450,7 +450,6 @@ static uint32_t get_elf_hwcap(void)
>      ARMCPU *cpu = ARM_CPU(thread_cpu);
>      uint32_t hwcaps = 0;
>
> -    hwcaps |= ARM_HWCAP_ARM_SWP;
>      hwcaps |= ARM_HWCAP_ARM_HALF;
>      hwcaps |= ARM_HWCAP_ARM_THUMB;
>      hwcaps |= ARM_HWCAP_ARM_FAST_MULT;
> @@ -458,7 +457,9 @@ static uint32_t get_elf_hwcap(void)
>      /* probe for the extra features */
>  #define GET_FEATURE(feat, hwcap) \
>      do { if (arm_feature(&cpu->env, feat)) { hwcaps |= hwcap; } } while (0)
> +
>      /* EDSP is in v5TE and above, but all our v5 CPUs are v5TE */
> +    GET_FEATURE(ARM_FEATURE_SWP, ARM_HWCAP_ARM_SWP);
>      GET_FEATURE(ARM_FEATURE_V5, ARM_HWCAP_ARM_EDSP);

This has separated the comment about EDSP from the code line it
refers to.

>      GET_FEATURE(ARM_FEATURE_VFP, ARM_HWCAP_ARM_VFP);
>      GET_FEATURE(ARM_FEATURE_IWMMXT, ARM_HWCAP_ARM_IWMMXT);
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 258ba6dcaa..3bc7a16327 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -1075,6 +1075,7 @@ static void arm926_initfn(Object *obj)
>      set_feature(&cpu->env, ARM_FEATURE_DUMMY_C15_REGS);
>      set_feature(&cpu->env, ARM_FEATURE_CACHE_TEST_CLEAN);
>      set_feature(&cpu->env, ARM_FEATURE_JAZELLE);
> +    set_feature(&cpu->env, ARM_FEATURE_SWP);
>      cpu->midr = 0x41069265;
>      cpu->reset_fpsid = 0x41011090;
>      cpu->ctr = 0x1dd20d2;

In the current scheme of doing things I'd look for whether
we could say that some more generic thing implied SWP
rather than setting it in a lot of initfns (eg v5-but-not-v7VE?),
but maybe the later patches make that a bad approach
(haven't looked at the meat of this series).

> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index c6a5d2ac44..2688380ae6 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -9390,6 +9390,10 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn)
>                          TCGv taddr;
>                          TCGMemOp opc = s->be_data;
>
> +                        if (!arm_dc_feature(s, ARM_FEATURE_SWP)) {
> +                            goto illegal_op;
> +                        }
> +

We want to arrange to have SWP work anyway on linux-user,
I think, since the kernel will typically trap-and-emulate
it assuming it was built with CONFIG_SWP_EMULATE. (I don't
know if those kernels will advertise swp in the hwcaps,
but I guess not.)

>                          rm = (insn) & 0xf;
>
>                          if (insn & (1 << 22)) {
> --
> 2.17.1

thanks
-- PMM

Re: [Qemu-devel] [PATCH 01/13] target/arm: Add ARM_FEATURE_SWP

[Richard Henderson]

On 9/15/18 6:32 PM, Peter Maydell wrote:
> In the current scheme of doing things I'd look for whether
> we could say that some more generic thing implied SWP
> rather than setting it in a lot of initfns (eg v5-but-not-v7VE?),
> but maybe the later patches make that a bad approach
> (haven't looked at the meat of this series).

Perhaps.  But indeed this goes back to the main question
posed in the cover letter.

> We want to arrange to have SWP work anyway on linux-user,
> I think, since the kernel will typically trap-and-emulate
> it assuming it was built with CONFIG_SWP_EMULATE. (I don't
> know if those kernels will advertise swp in the hwcaps,
> but I guess not.)

Ah, I did not know about SWP_EMULATE.  It appears to be
specific to armv7+ (though we don't support the pre-v4
cpus for which it might otherwise be relevant).

It does appear that HWCAP_SWP is advertised anyway:

mm/proc-v7.S:	.long	HWCAP_SWP | HWCAP_HALF | HWCAP_THUMB | HWCAP_FAST_MULT


r~

Re: [Qemu-devel] [PATCH 01/13] target/arm: Add ARM_FEATURE_SWP

[Peter Maydell]

On 16 September 2018 at 16:53, Richard Henderson
<richard.henderson@linaro.org> wrote:
> On 9/15/18 6:32 PM, Peter Maydell wrote:
>> We want to arrange to have SWP work anyway on linux-user,
>> I think, since the kernel will typically trap-and-emulate
>> it assuming it was built with CONFIG_SWP_EMULATE. (I don't
>> know if those kernels will advertise swp in the hwcaps,
>> but I guess not.)
>
> Ah, I did not know about SWP_EMULATE.  It appears to be
> specific to armv7+ (though we don't support the pre-v4
> cpus for which it might otherwise be relevant).

Yes, it's intended to allow older userspace binaries to continue
to work on newer CPUs without SWP, not to try to run new binaries
on older CPUs. (Anything compiled for a CPU new enough for SWP
probably uses other newer insns that some pre-v4 CPU doesn't have
anyway.)

> It does appear that HWCAP_SWP is advertised anyway:
>
> mm/proc-v7.S:   .long   HWCAP_SWP | HWCAP_HALF | HWCAP_THUMB | HWCAP_FAST_MULT

Interesting. I might ask some random kernel person what the semantics
of hwcap are -- is it "will work even if a terrible plan" or "it makes
sense to use this"?

thanks
-- PMM