diff mbox series

[01/13] target/arm: Add ARM_FEATURE_SWP

Message ID 20180915161738.25257-2-richard.henderson@linaro.org
State New
Headers show
Series target/arm: Derive cpu id regs from features | expand

Commit Message

Richard Henderson Sept. 15, 2018, 4:17 p.m. UTC 
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(-)

-- 
2.17.1

Comments

Peter Maydell Sept. 16, 2018, 1:32 a.m. UTC | #1 
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
Richard Henderson Sept. 16, 2018, 3:53 p.m. UTC | #2 
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~
Peter Maydell Sept. 16, 2018, 4:54 p.m. UTC | #3 
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
Peter Maydell Sept. 25, 2018, 9:49 a.m. UTC | #4 
On 16 September 2018 at 16:53, Richard Henderson
<richard.henderson@linaro.org> wrote:
> 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


I just discovered that that is overridden by
arch/arm/kernel/setup.c:elf_hwcap_fixup(), which looks at the
ID registers and suppresses HWCAP_SWP if LDREX/STREX and LDREXB/STREXB
are supported.

(Compat 32-bit support in a 64-bit kernel never advertises
HWCAP_SWP.)

thanks
-- PMM
diff mbox series

Patch

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)) {