* [Qemu-devel] [PATCH 1/3] target-arm: add feature flag for ARMv8 @ 2013-06-07 12:06 Mans Rullgard 2013-06-07 12:06 ` [Qemu-devel] [PATCH 2/3] target-arm: implement LDA/STL instructions Mans Rullgard ` (2 more replies) 0 siblings, 3 replies; 15+ messages in thread From: Mans Rullgard @ 2013-06-07 12:06 UTC (permalink / raw) To: qemu-devel Signed-off-by: Mans Rullgard <mans@mansr.com> --- target-arm/cpu.c | 5 ++++- target-arm/cpu.h | 1 + target-arm/translate.c | 1 + 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/target-arm/cpu.c b/target-arm/cpu.c index 496a59f..f5a1314 100644 --- a/target-arm/cpu.c +++ b/target-arm/cpu.c @@ -162,6 +162,9 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp) CPUARMState *env = &cpu->env; /* Some features automatically imply others: */ + if (arm_feature(env, ARM_FEATURE_V8)) { + set_feature(env, ARM_FEATURE_V7); + } if (arm_feature(env, ARM_FEATURE_V7)) { set_feature(env, ARM_FEATURE_VAPA); set_feature(env, ARM_FEATURE_THUMB2); @@ -748,7 +751,7 @@ static void pxa270c5_initfn(Object *obj) static void arm_any_initfn(Object *obj) { ARMCPU *cpu = ARM_CPU(obj); - set_feature(&cpu->env, ARM_FEATURE_V7); + set_feature(&cpu->env, ARM_FEATURE_V8); set_feature(&cpu->env, ARM_FEATURE_VFP4); set_feature(&cpu->env, ARM_FEATURE_VFP_FP16); set_feature(&cpu->env, ARM_FEATURE_NEON); diff --git a/target-arm/cpu.h b/target-arm/cpu.h index 5438444..b3be588 100644 --- a/target-arm/cpu.h +++ b/target-arm/cpu.h @@ -392,6 +392,7 @@ enum arm_features { ARM_FEATURE_MPIDR, /* has cp15 MPIDR */ ARM_FEATURE_PXN, /* has Privileged Execute Never bit */ ARM_FEATURE_LPAE, /* has Large Physical Address Extension */ + ARM_FEATURE_V8, }; static inline int arm_feature(CPUARMState *env, int feature) diff --git a/target-arm/translate.c b/target-arm/translate.c index b3f26d6..96ac5bc 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -42,6 +42,7 @@ #define ENABLE_ARCH_6K arm_feature(env, ARM_FEATURE_V6K) #define ENABLE_ARCH_6T2 arm_feature(env, ARM_FEATURE_THUMB2) #define ENABLE_ARCH_7 arm_feature(env, ARM_FEATURE_V7) +#define ENABLE_ARCH_8 arm_feature(env, ARM_FEATURE_V8) #define ARCH(x) do { if (!ENABLE_ARCH_##x) goto illegal_op; } while(0) -- 1.8.2.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 2/3] target-arm: implement LDA/STL instructions 2013-06-07 12:06 [Qemu-devel] [PATCH 1/3] target-arm: add feature flag for ARMv8 Mans Rullgard @ 2013-06-07 12:06 ` Mans Rullgard 2013-06-13 14:01 ` Peter Maydell 2013-06-07 12:06 ` [Qemu-devel] [PATCH 3/3] target-arm: explicitly decode SEVL instruction Mans Rullgard 2013-06-13 12:54 ` [Qemu-devel] [PATCH 1/3] target-arm: add feature flag for ARMv8 Peter Maydell 2 siblings, 1 reply; 15+ messages in thread From: Mans Rullgard @ 2013-06-07 12:06 UTC (permalink / raw) To: qemu-devel This adds support for the ARMv8 load acquire/store release instructions. Since qemu does nothing special for memory barriers, these can be emulated like their non-acquire/release counterparts. Signed-off-by: Mans Rullgard <mans@mansr.com> --- target-arm/translate.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 85 insertions(+), 6 deletions(-) diff --git a/target-arm/translate.c b/target-arm/translate.c index 96ac5bc..f529257 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -7274,14 +7274,54 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s) rd = (insn >> 12) & 0xf; if (insn & (1 << 23)) { /* load/store exclusive */ + int excl = (insn >> 9) & 1; op1 = (insn >> 21) & 0x3; - if (op1) + if (!excl) + ARCH(8); + else if (op1) ARCH(6K); else ARCH(6); addr = tcg_temp_local_new_i32(); load_reg_var(s, addr, rn); - if (insn & (1 << 20)) { + if (!excl) { + if (op1 == 1) + goto illegal_op; + tmp = tcg_temp_new_i32(); + if (insn & (1 << 20)) { + switch (op1) { + case 0: /* lda */ + tcg_gen_qemu_ld32u(tmp, addr, IS_USER(s)); + break; + case 2: /* ldab */ + tcg_gen_qemu_ld8u(tmp, addr, IS_USER(s)); + break; + case 3: /* ldah */ + tcg_gen_qemu_ld16u(tmp, addr, IS_USER(s)); + break; + default: + abort(); + } + store_reg(s, rd, tmp); + } else { + rm = insn & 0xf; + tmp = load_reg(s, rm); + switch (op1) { + case 0: /* stl */ + tcg_gen_qemu_st32(tmp, addr, IS_USER(s)); + break; + case 2: /* stlb */ + tcg_gen_qemu_st8(tmp, addr, IS_USER(s)); + break; + case 3: /* stlh */ + tcg_gen_qemu_st16(tmp, addr, IS_USER(s)); + break; + default: + abort(); + } + tcg_temp_free_i32(tmp); + } + } else if (insn & (1 << 20)) { switch (op1) { case 0: /* ldrex */ gen_load_exclusive(s, rd, 15, addr, 2); @@ -8126,7 +8166,7 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw gen_store_exclusive(s, rd, rs, 15, addr, 2); } tcg_temp_free_i32(addr); - } else if ((insn & (1 << 6)) == 0) { + } else if ((insn & (3 << 6)) == 0) { /* Table Branch. */ if (rn == 15) { addr = tcg_temp_new_i32(); @@ -8153,14 +8193,53 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw store_reg(s, 15, tmp); } else { /* Load/store exclusive byte/halfword/doubleword. */ - ARCH(7); + if (((insn >> 6) & 3) != 1) + ARCH(8); + else + ARCH(7); op = (insn >> 4) & 0x3; - if (op == 2) { + if (((insn >> 7) & 1) == 0 && op == 2) { goto illegal_op; } addr = tcg_temp_local_new_i32(); load_reg_var(s, addr, rn); - if (insn & (1 << 20)) { + if ((insn & (1 << 6)) == 0) { + if (op == 3) + goto illegal_op; + tmp = tcg_temp_new_i32(); + if (insn & (1 << 20)) { + switch (op) { + case 0: /* ldab */ + tcg_gen_qemu_ld8u(tmp, addr, IS_USER(s)); + break; + case 1: /* ldah */ + tcg_gen_qemu_ld16u(tmp, addr, IS_USER(s)); + break; + case 2: /* lda */ + tcg_gen_qemu_ld32u(tmp, addr, IS_USER(s)); + break; + default: + abort(); + } + store_reg(s, rs, tmp); + } else { + tmp = load_reg(s, rs); + switch (op) { + case 0: /* stlb */ + tcg_gen_qemu_st8(tmp, addr, IS_USER(s)); + break; + case 1: /* stlh */ + tcg_gen_qemu_st16(tmp, addr, IS_USER(s)); + break; + case 2: /* stl */ + tcg_gen_qemu_st32(tmp, addr, IS_USER(s)); + break; + default: + abort(); + } + tcg_temp_free_i32(tmp); + } + } else if (insn & (1 << 20)) { gen_load_exclusive(s, rs, rd, addr, op); } else { gen_store_exclusive(s, rm, rs, rd, addr, op); -- 1.8.2.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] target-arm: implement LDA/STL instructions 2013-06-07 12:06 ` [Qemu-devel] [PATCH 2/3] target-arm: implement LDA/STL instructions Mans Rullgard @ 2013-06-13 14:01 ` Peter Maydell 2013-06-17 16:50 ` [Qemu-devel] [PATCH v2] " Mans Rullgard 0 siblings, 1 reply; 15+ messages in thread From: Peter Maydell @ 2013-06-13 14:01 UTC (permalink / raw) To: Mans Rullgard; +Cc: qemu-devel On 7 June 2013 13:06, Mans Rullgard <mans@mansr.com> wrote: > This adds support for the ARMv8 load acquire/store release instructions. > Since qemu does nothing special for memory barriers, these can be > emulated like their non-acquire/release counterparts. A brief comment to this effect in the appropriate parts of the code as well as the commit message would be good. > > Signed-off-by: Mans Rullgard <mans@mansr.com> > --- > target-arm/translate.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 85 insertions(+), 6 deletions(-) > > diff --git a/target-arm/translate.c b/target-arm/translate.c > index 96ac5bc..f529257 100644 > --- a/target-arm/translate.c > +++ b/target-arm/translate.c > @@ -7274,14 +7274,54 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s) > rd = (insn >> 12) & 0xf; > if (insn & (1 << 23)) { > /* load/store exclusive */ > + int excl = (insn >> 9) & 1; > op1 = (insn >> 21) & 0x3; > - if (op1) > + if (!excl) > + ARCH(8); > + else if (op1) > ARCH(6K); > else > ARCH(6); This if statement needs braces on all its arms. (QEMU coding style mandates braces, though some existing code is not in line with this; our general policy is that where patches touch such existing code they update it accordingly.) You can use scripts/checkpatch.pl to check for this. This is also continuing to underdecode this space: some of the new exclusive insns are ARCH(8) only but will be allowed on ARCH(6) or 6K by this check. I know we've historically underdecoded, but if we're touching this bit of the decoder I'd prefer it if we tightened it up in the process. > addr = tcg_temp_local_new_i32(); > load_reg_var(s, addr, rn); > - if (insn & (1 << 20)) { > + if (!excl) { > + if (op1 == 1) > + goto illegal_op; This check needs to happen earlier to avoid leaking a tcg temp. > + tmp = tcg_temp_new_i32(); > + if (insn & (1 << 20)) { > + switch (op1) { > + case 0: /* lda */ > + tcg_gen_qemu_ld32u(tmp, addr, IS_USER(s)); > + break; > + case 2: /* ldab */ > + tcg_gen_qemu_ld8u(tmp, addr, IS_USER(s)); > + break; > + case 3: /* ldah */ > + tcg_gen_qemu_ld16u(tmp, addr, IS_USER(s)); > + break; > + default: > + abort(); > + } > + store_reg(s, rd, tmp); > + } else { > + rm = insn & 0xf; > + tmp = load_reg(s, rm); > + switch (op1) { > + case 0: /* stl */ > + tcg_gen_qemu_st32(tmp, addr, IS_USER(s)); > + break; > + case 2: /* stlb */ > + tcg_gen_qemu_st8(tmp, addr, IS_USER(s)); > + break; > + case 3: /* stlh */ > + tcg_gen_qemu_st16(tmp, addr, IS_USER(s)); > + break; > + default: > + abort(); > + } > + tcg_temp_free_i32(tmp); > + } > + } else if (insn & (1 << 20)) { > switch (op1) { > case 0: /* ldrex */ > gen_load_exclusive(s, rd, 15, addr, 2); > @@ -8126,7 +8166,7 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw > gen_store_exclusive(s, rd, rs, 15, addr, 2); > } > tcg_temp_free_i32(addr); > - } else if ((insn & (1 << 6)) == 0) { > + } else if ((insn & (3 << 6)) == 0) { While we're fiddling with the decode here let's actually narrow it down completely, by making this condition ((insn & (7 << 5)) == 0) ... > /* Table Branch. */ > if (rn == 15) { > addr = tcg_temp_new_i32(); > @@ -8153,14 +8193,53 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw > store_reg(s, 15, tmp); > } else { > /* Load/store exclusive byte/halfword/doubleword. */ > - ARCH(7); > + if (((insn >> 6) & 3) != 1) > + ARCH(8); > + else > + ARCH(7); ...and then here do something like: switch ((insn >> 6) & 3) { case 0: goto illegal_op; case 1: /* Load/store exclusive byte/halfword/doubleword */ ARCH(7); break; case 2: case 3: /* Load-acquire/store-release */ ARCH(8); break; } > op = (insn >> 4) & 0x3; > - if (op == 2) { > + if (((insn >> 7) & 1) == 0 && op == 2) { > goto illegal_op; > } It might be better to just pull out the op3 field and then do tests on it rather than combinations of testing op (op3's lower 2 bits) and bits directly from insn. > addr = tcg_temp_local_new_i32(); > load_reg_var(s, addr, rn); > - if (insn & (1 << 20)) { > + if ((insn & (1 << 6)) == 0) { > + if (op == 3) > + goto illegal_op; This check needs to go earlier, otherwise we leak a tcg temp if we take the illegal_op path. You might be able to roll all the illegal-op checks and ARCH checks into a single switch on the whole op3 field. > + tmp = tcg_temp_new_i32(); > + if (insn & (1 << 20)) { > + switch (op) { > + case 0: /* ldab */ > + tcg_gen_qemu_ld8u(tmp, addr, IS_USER(s)); > + break; > + case 1: /* ldah */ > + tcg_gen_qemu_ld16u(tmp, addr, IS_USER(s)); > + break; > + case 2: /* lda */ > + tcg_gen_qemu_ld32u(tmp, addr, IS_USER(s)); > + break; > + default: > + abort(); > + } > + store_reg(s, rs, tmp); > + } else { > + tmp = load_reg(s, rs); > + switch (op) { > + case 0: /* stlb */ > + tcg_gen_qemu_st8(tmp, addr, IS_USER(s)); > + break; > + case 1: /* stlh */ > + tcg_gen_qemu_st16(tmp, addr, IS_USER(s)); > + break; > + case 2: /* stl */ > + tcg_gen_qemu_st32(tmp, addr, IS_USER(s)); > + break; > + default: > + abort(); > + } > + tcg_temp_free_i32(tmp); > + } > + } else if (insn & (1 << 20)) { > gen_load_exclusive(s, rs, rd, addr, op); > } else { > gen_store_exclusive(s, rm, rs, rd, addr, op); Have you tested these with risu yet? thanks -- PMM ^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH v2] target-arm: implement LDA/STL instructions 2013-06-13 14:01 ` Peter Maydell @ 2013-06-17 16:50 ` Mans Rullgard 2013-06-20 17:51 ` Peter Maydell 0 siblings, 1 reply; 15+ messages in thread From: Mans Rullgard @ 2013-06-17 16:50 UTC (permalink / raw) To: qemu-devel This adds support for the ARMv8 load acquire/store release instructions. Since qemu does nothing special for memory barriers, these can be emulated like their non-acquire/release counterparts. Signed-off-by: Mans Rullgard <mans@mansr.com> --- Not thoroughly tested. --- target-arm/translate.c | 126 +++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 116 insertions(+), 10 deletions(-) diff --git a/target-arm/translate.c b/target-arm/translate.c index 3ffe7b8..1da56a4 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -7274,14 +7274,72 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s) rd = (insn >> 12) & 0xf; if (insn & (1 << 23)) { /* load/store exclusive */ + int op2 = (insn >> 8) & 3; op1 = (insn >> 21) & 0x3; - if (op1) - ARCH(6K); - else - ARCH(6); + + switch (op2) { + case 0: /* lda/stl */ + if (op1 == 1) { + goto illegal_op; + } + ARCH(8); + break; + case 1: /* reserved */ + goto illegal_op; + case 2: /* ldaex/stlex */ + ARCH(8); + break; + case 3: /* ldrex/strex */ + if (op1) { + ARCH(6K); + } else { + ARCH(6); + } + break; + } + addr = tcg_temp_local_new_i32(); load_reg_var(s, addr, rn); - if (insn & (1 << 20)) { + + /* Since the emulation does not have barriers, + the acquire/release semantics need no special + handling */ + if (op2 == 0) { + tmp = tcg_temp_new_i32(); + if (insn & (1 << 20)) { + switch (op1) { + case 0: /* lda */ + tcg_gen_qemu_ld32u(tmp, addr, IS_USER(s)); + break; + case 2: /* ldab */ + tcg_gen_qemu_ld8u(tmp, addr, IS_USER(s)); + break; + case 3: /* ldah */ + tcg_gen_qemu_ld16u(tmp, addr, IS_USER(s)); + break; + default: + abort(); + } + store_reg(s, rd, tmp); + } else { + rm = insn & 0xf; + tmp = load_reg(s, rm); + switch (op1) { + case 0: /* stl */ + tcg_gen_qemu_st32(tmp, addr, IS_USER(s)); + break; + case 2: /* stlb */ + tcg_gen_qemu_st8(tmp, addr, IS_USER(s)); + break; + case 3: /* stlh */ + tcg_gen_qemu_st16(tmp, addr, IS_USER(s)); + break; + default: + abort(); + } + tcg_temp_free_i32(tmp); + } + } else if (insn & (1 << 20)) { switch (op1) { case 0: /* ldrex */ gen_load_exclusive(s, rd, 15, addr, 2); @@ -8126,7 +8184,7 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw gen_store_exclusive(s, rd, rs, 15, addr, 2); } tcg_temp_free_i32(addr); - } else if ((insn & (1 << 6)) == 0) { + } else if ((insn & (7 << 5)) == 0) { /* Table Branch. */ if (rn == 15) { addr = tcg_temp_new_i32(); @@ -8152,15 +8210,63 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw tcg_gen_addi_i32(tmp, tmp, s->pc); store_reg(s, 15, tmp); } else { - /* Load/store exclusive byte/halfword/doubleword. */ - ARCH(7); + int op2 = (insn >> 6) & 0x3; op = (insn >> 4) & 0x3; - if (op == 2) { + switch (op2) { + case 0: goto illegal_op; + case 1: + /* Load/store exclusive byte/halfword/doubleword */ + ARCH(7); + break; + case 2: + /* Load-acquire/store-release */ + if (op == 3) { + goto illegal_op; + } + /* Fall through */ + case 3: + /* Load-acquire/store-release exclusive */ + ARCH(8); + break; } addr = tcg_temp_local_new_i32(); load_reg_var(s, addr, rn); - if (insn & (1 << 20)) { + if (!(op2 & 1)) { + tmp = tcg_temp_new_i32(); + if (insn & (1 << 20)) { + switch (op) { + case 0: /* ldab */ + tcg_gen_qemu_ld8u(tmp, addr, IS_USER(s)); + break; + case 1: /* ldah */ + tcg_gen_qemu_ld16u(tmp, addr, IS_USER(s)); + break; + case 2: /* lda */ + tcg_gen_qemu_ld32u(tmp, addr, IS_USER(s)); + break; + default: + abort(); + } + store_reg(s, rs, tmp); + } else { + tmp = load_reg(s, rs); + switch (op) { + case 0: /* stlb */ + tcg_gen_qemu_st8(tmp, addr, IS_USER(s)); + break; + case 1: /* stlh */ + tcg_gen_qemu_st16(tmp, addr, IS_USER(s)); + break; + case 2: /* stl */ + tcg_gen_qemu_st32(tmp, addr, IS_USER(s)); + break; + default: + abort(); + } + tcg_temp_free_i32(tmp); + } + } else if (insn & (1 << 20)) { gen_load_exclusive(s, rs, rd, addr, op); } else { gen_store_exclusive(s, rm, rs, rd, addr, op); -- 1.8.2.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH v2] target-arm: implement LDA/STL instructions 2013-06-17 16:50 ` [Qemu-devel] [PATCH v2] " Mans Rullgard @ 2013-06-20 17:51 ` Peter Maydell 2013-07-01 13:46 ` [Qemu-devel] [PATCH v3] " Mans Rullgard 0 siblings, 1 reply; 15+ messages in thread From: Peter Maydell @ 2013-06-20 17:51 UTC (permalink / raw) To: Mans Rullgard; +Cc: qemu-devel On 17 June 2013 17:50, Mans Rullgard <mans@mansr.com> wrote: > This adds support for the ARMv8 load acquire/store release instructions. > Since qemu does nothing special for memory barriers, these can be > emulated like their non-acquire/release counterparts. Couple more minor issues, otherwise looks good. > addr = tcg_temp_local_new_i32(); > load_reg_var(s, addr, rn); > - if (insn & (1 << 20)) { > + > + /* Since the emulation does not have barriers, > + the acquire/release semantics need no special > + handling */ > + if (op2 == 0) { > + tmp = tcg_temp_new_i32(); This line needs to go inside the next if(), otherwise we leak a temp in the stl/stlb/stlh case. (load_reg() does a temp_new for you, so you need to pair temp_new/store_reg and load_reg/temp_free.) > + if (insn & (1 << 20)) { > + switch (op1) { > + case 0: /* lda */ > + tcg_gen_qemu_ld32u(tmp, addr, IS_USER(s)); > + break; > + case 2: /* ldab */ > + tcg_gen_qemu_ld8u(tmp, addr, IS_USER(s)); > + break; > + case 3: /* ldah */ > + tcg_gen_qemu_ld16u(tmp, addr, IS_USER(s)); > + break; > + default: > + abort(); > + } > + store_reg(s, rd, tmp); > + } else { > + rm = insn & 0xf; > + tmp = load_reg(s, rm); > + switch (op1) { > + case 0: /* stl */ > + tcg_gen_qemu_st32(tmp, addr, IS_USER(s)); > + break; > + case 2: /* stlb */ > + tcg_gen_qemu_st8(tmp, addr, IS_USER(s)); > + break; > + case 3: /* stlh */ > + tcg_gen_qemu_st16(tmp, addr, IS_USER(s)); > + break; > + default: > + abort(); > + } > + tcg_temp_free_i32(tmp); > + } > + } else if (insn & (1 << 20)) { > switch (op1) { > case 0: /* ldrex */ > gen_load_exclusive(s, rd, 15, addr, 2); > @@ -8152,15 +8210,63 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw > tcg_gen_addi_i32(tmp, tmp, s->pc); > store_reg(s, 15, tmp); > } else { > - /* Load/store exclusive byte/halfword/doubleword. */ > - ARCH(7); > + int op2 = (insn >> 6) & 0x3; > op = (insn >> 4) & 0x3; > - if (op == 2) { > + switch (op2) { > + case 0: > goto illegal_op; > + case 1: > + /* Load/store exclusive byte/halfword/doubleword */ > + ARCH(7); > + break; > + case 2: > + /* Load-acquire/store-release */ > + if (op == 3) { > + goto illegal_op; > + } > + /* Fall through */ > + case 3: > + /* Load-acquire/store-release exclusive */ > + ARCH(8); > + break; > } This change has lost the check for op==2 being illegal in the load/store exclusive case (ie case op2==1). > addr = tcg_temp_local_new_i32(); > load_reg_var(s, addr, rn); > - if (insn & (1 << 20)) { > + if (!(op2 & 1)) { > + tmp = tcg_temp_new_i32(); This needs to be inside the following if(), otherwise we leak a temp in the stlb/stlh/stl case. > + if (insn & (1 << 20)) { > + switch (op) { > + case 0: /* ldab */ > + tcg_gen_qemu_ld8u(tmp, addr, IS_USER(s)); > + break; > + case 1: /* ldah */ > + tcg_gen_qemu_ld16u(tmp, addr, IS_USER(s)); > + break; > + case 2: /* lda */ > + tcg_gen_qemu_ld32u(tmp, addr, IS_USER(s)); > + break; > + default: > + abort(); > + } > + store_reg(s, rs, tmp); > + } else { > + tmp = load_reg(s, rs); > + switch (op) { > + case 0: /* stlb */ > + tcg_gen_qemu_st8(tmp, addr, IS_USER(s)); > + break; > + case 1: /* stlh */ > + tcg_gen_qemu_st16(tmp, addr, IS_USER(s)); > + break; > + case 2: /* stl */ > + tcg_gen_qemu_st32(tmp, addr, IS_USER(s)); > + break; > + default: > + abort(); > + } > + tcg_temp_free_i32(tmp); > + } > + } else if (insn & (1 << 20)) { > gen_load_exclusive(s, rs, rd, addr, op); > } else { > gen_store_exclusive(s, rm, rs, rd, addr, op); thanks -- PMM ^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH v3] target-arm: implement LDA/STL instructions 2013-06-20 17:51 ` Peter Maydell @ 2013-07-01 13:46 ` Mans Rullgard 2013-07-02 12:46 ` Peter Maydell 0 siblings, 1 reply; 15+ messages in thread From: Mans Rullgard @ 2013-07-01 13:46 UTC (permalink / raw) To: qemu-devel This adds support for the ARMv8 load acquire/store release instructions. Since qemu does nothing special for memory barriers, these can be emulated like their non-acquire/release counterparts. Signed-off-by: Mans Rullgard <mans@mansr.com> --- Fixed issues found in v2. Could use more testing. --- target-arm/translate.c | 129 +++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 119 insertions(+), 10 deletions(-) diff --git a/target-arm/translate.c b/target-arm/translate.c index af1ef34..0a8c9e0 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -7274,14 +7274,72 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s) rd = (insn >> 12) & 0xf; if (insn & (1 << 23)) { /* load/store exclusive */ + int op2 = (insn >> 8) & 3; op1 = (insn >> 21) & 0x3; - if (op1) - ARCH(6K); - else - ARCH(6); + + switch (op2) { + case 0: /* lda/stl */ + if (op1 == 1) { + goto illegal_op; + } + ARCH(8); + break; + case 1: /* reserved */ + goto illegal_op; + case 2: /* ldaex/stlex */ + ARCH(8); + break; + case 3: /* ldrex/strex */ + if (op1) { + ARCH(6K); + } else { + ARCH(6); + } + break; + } + addr = tcg_temp_local_new_i32(); load_reg_var(s, addr, rn); - if (insn & (1 << 20)) { + + /* Since the emulation does not have barriers, + the acquire/release semantics need no special + handling */ + if (op2 == 0) { + if (insn & (1 << 20)) { + tmp = tcg_temp_new_i32(); + switch (op1) { + case 0: /* lda */ + tcg_gen_qemu_ld32u(tmp, addr, IS_USER(s)); + break; + case 2: /* ldab */ + tcg_gen_qemu_ld8u(tmp, addr, IS_USER(s)); + break; + case 3: /* ldah */ + tcg_gen_qemu_ld16u(tmp, addr, IS_USER(s)); + break; + default: + abort(); + } + store_reg(s, rd, tmp); + } else { + rm = insn & 0xf; + tmp = load_reg(s, rm); + switch (op1) { + case 0: /* stl */ + tcg_gen_qemu_st32(tmp, addr, IS_USER(s)); + break; + case 2: /* stlb */ + tcg_gen_qemu_st8(tmp, addr, IS_USER(s)); + break; + case 3: /* stlh */ + tcg_gen_qemu_st16(tmp, addr, IS_USER(s)); + break; + default: + abort(); + } + tcg_temp_free_i32(tmp); + } + } else if (insn & (1 << 20)) { switch (op1) { case 0: /* ldrex */ gen_load_exclusive(s, rd, 15, addr, 2); @@ -8126,7 +8184,7 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw gen_store_exclusive(s, rd, rs, 15, addr, 2); } tcg_temp_free_i32(addr); - } else if ((insn & (1 << 6)) == 0) { + } else if ((insn & (7 << 5)) == 0) { /* Table Branch. */ if (rn == 15) { addr = tcg_temp_new_i32(); @@ -8152,15 +8210,66 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw tcg_gen_addi_i32(tmp, tmp, s->pc); store_reg(s, 15, tmp); } else { - /* Load/store exclusive byte/halfword/doubleword. */ - ARCH(7); + int op2 = (insn >> 6) & 0x3; op = (insn >> 4) & 0x3; - if (op == 2) { + switch (op2) { + case 0: goto illegal_op; + case 1: + /* Load/store exclusive byte/halfword/doubleword */ + if (op == 2) { + goto illegal_op; + } + ARCH(7); + break; + case 2: + /* Load-acquire/store-release */ + if (op == 3) { + goto illegal_op; + } + /* Fall through */ + case 3: + /* Load-acquire/store-release exclusive */ + ARCH(8); + break; } addr = tcg_temp_local_new_i32(); load_reg_var(s, addr, rn); - if (insn & (1 << 20)) { + if (!(op2 & 1)) { + if (insn & (1 << 20)) { + tmp = tcg_temp_new_i32(); + switch (op) { + case 0: /* ldab */ + tcg_gen_qemu_ld8u(tmp, addr, IS_USER(s)); + break; + case 1: /* ldah */ + tcg_gen_qemu_ld16u(tmp, addr, IS_USER(s)); + break; + case 2: /* lda */ + tcg_gen_qemu_ld32u(tmp, addr, IS_USER(s)); + break; + default: + abort(); + } + store_reg(s, rs, tmp); + } else { + tmp = load_reg(s, rs); + switch (op) { + case 0: /* stlb */ + tcg_gen_qemu_st8(tmp, addr, IS_USER(s)); + break; + case 1: /* stlh */ + tcg_gen_qemu_st16(tmp, addr, IS_USER(s)); + break; + case 2: /* stl */ + tcg_gen_qemu_st32(tmp, addr, IS_USER(s)); + break; + default: + abort(); + } + tcg_temp_free_i32(tmp); + } + } else if (insn & (1 << 20)) { gen_load_exclusive(s, rs, rd, addr, op); } else { gen_store_exclusive(s, rm, rs, rd, addr, op); -- 1.8.2.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH v3] target-arm: implement LDA/STL instructions 2013-07-01 13:46 ` [Qemu-devel] [PATCH v3] " Mans Rullgard @ 2013-07-02 12:46 ` Peter Maydell 0 siblings, 0 replies; 15+ messages in thread From: Peter Maydell @ 2013-07-02 12:46 UTC (permalink / raw) To: Mans Rullgard; +Cc: qemu-devel On 1 July 2013 14:46, Mans Rullgard <mans@mansr.com> wrote: > This adds support for the ARMv8 load acquire/store release instructions. > Since qemu does nothing special for memory barriers, these can be > emulated like their non-acquire/release counterparts. > > Signed-off-by: Mans Rullgard <mans@mansr.com> Thanks; applied to target-arm.next (together with the other 2 patches in the series.) -- PMM ^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 3/3] target-arm: explicitly decode SEVL instruction 2013-06-07 12:06 [Qemu-devel] [PATCH 1/3] target-arm: add feature flag for ARMv8 Mans Rullgard 2013-06-07 12:06 ` [Qemu-devel] [PATCH 2/3] target-arm: implement LDA/STL instructions Mans Rullgard @ 2013-06-07 12:06 ` Mans Rullgard 2013-06-13 12:59 ` Peter Maydell 2013-06-13 13:15 ` Andreas Färber 2013-06-13 12:54 ` [Qemu-devel] [PATCH 1/3] target-arm: add feature flag for ARMv8 Peter Maydell 2 siblings, 2 replies; 15+ messages in thread From: Mans Rullgard @ 2013-06-07 12:06 UTC (permalink / raw) To: qemu-devel The ARMv8 SEVL instruction is in the architectural hint space already emulated as nop. This makes the decoding of SEVL explicit for clarity. Signed-off-by: Mans Rullgard <mans@mansr.com> --- target-arm/translate.c | 1 + 1 file changed, 1 insertion(+) diff --git a/target-arm/translate.c b/target-arm/translate.c index f529257..cfd148e 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -3501,6 +3501,7 @@ static void gen_nop_hint(DisasContext *s, int val) break; case 2: /* wfe */ case 4: /* sev */ + case 5: /* sevl */ /* TODO: Implement SEV and WFE. May help SMP performance. */ default: /* nop */ break; -- 1.8.2.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] target-arm: explicitly decode SEVL instruction 2013-06-07 12:06 ` [Qemu-devel] [PATCH 3/3] target-arm: explicitly decode SEVL instruction Mans Rullgard @ 2013-06-13 12:59 ` Peter Maydell 2013-06-13 13:15 ` Andreas Färber 1 sibling, 0 replies; 15+ messages in thread From: Peter Maydell @ 2013-06-13 12:59 UTC (permalink / raw) To: Mans Rullgard; +Cc: qemu-devel On 7 June 2013 13:06, Mans Rullgard <mans@mansr.com> wrote: > The ARMv8 SEVL instruction is in the architectural hint space already > emulated as nop. This makes the decoding of SEVL explicit for clarity. > > Signed-off-by: Mans Rullgard <mans@mansr.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> -- PMM ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] target-arm: explicitly decode SEVL instruction 2013-06-07 12:06 ` [Qemu-devel] [PATCH 3/3] target-arm: explicitly decode SEVL instruction Mans Rullgard 2013-06-13 12:59 ` Peter Maydell @ 2013-06-13 13:15 ` Andreas Färber 2013-06-13 13:39 ` Måns Rullgård 2013-07-02 12:53 ` Peter Maydell 1 sibling, 2 replies; 15+ messages in thread From: Andreas Färber @ 2013-06-13 13:15 UTC (permalink / raw) To: Mans Rullgard; +Cc: Peter Maydell, qemu-devel Am 07.06.2013 14:06, schrieb Mans Rullgard: > The ARMv8 SEVL instruction is in the architectural hint space already > emulated as nop. This makes the decoding of SEVL explicit for clarity. > > Signed-off-by: Mans Rullgard <mans@mansr.com> > --- > target-arm/translate.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/target-arm/translate.c b/target-arm/translate.c > index f529257..cfd148e 100644 > --- a/target-arm/translate.c > +++ b/target-arm/translate.c > @@ -3501,6 +3501,7 @@ static void gen_nop_hint(DisasContext *s, int val) > break; > case 2: /* wfe */ > case 4: /* sev */ > + case 5: /* sevl */ > /* TODO: Implement SEV and WFE. May help SMP performance. */ > default: /* nop */ > break; So does SEVL need to be implemented? Then the TODO should be updated. Otherwise moving it a line down may be clearer. Regards, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] target-arm: explicitly decode SEVL instruction 2013-06-13 13:15 ` Andreas Färber @ 2013-06-13 13:39 ` Måns Rullgård 2013-07-02 12:53 ` Peter Maydell 1 sibling, 0 replies; 15+ messages in thread From: Måns Rullgård @ 2013-06-13 13:39 UTC (permalink / raw) To: Andreas Färber; +Cc: Peter Maydell, Mans Rullgard, qemu-devel Andreas Färber <afaerber@suse.de> writes: > Am 07.06.2013 14:06, schrieb Mans Rullgard: >> The ARMv8 SEVL instruction is in the architectural hint space already >> emulated as nop. This makes the decoding of SEVL explicit for clarity. >> >> Signed-off-by: Mans Rullgard <mans@mansr.com> >> --- >> target-arm/translate.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/target-arm/translate.c b/target-arm/translate.c >> index f529257..cfd148e 100644 >> --- a/target-arm/translate.c >> +++ b/target-arm/translate.c >> @@ -3501,6 +3501,7 @@ static void gen_nop_hint(DisasContext *s, int val) >> break; >> case 2: /* wfe */ >> case 4: /* sev */ >> + case 5: /* sevl */ >> /* TODO: Implement SEV and WFE. May help SMP performance. */ >> default: /* nop */ >> break; > > So does SEVL need to be implemented? Then the TODO should be updated. > Otherwise moving it a line down may be clearer. It needs to be implemented when/if SEV and WFE are. -- Måns Rullgård mans@mansr.com ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] target-arm: explicitly decode SEVL instruction 2013-06-13 13:15 ` Andreas Färber 2013-06-13 13:39 ` Måns Rullgård @ 2013-07-02 12:53 ` Peter Maydell 1 sibling, 0 replies; 15+ messages in thread From: Peter Maydell @ 2013-07-02 12:53 UTC (permalink / raw) To: Andreas Färber; +Cc: Mans Rullgard, qemu-devel On 13 June 2013 14:15, Andreas Färber <afaerber@suse.de> wrote: > Am 07.06.2013 14:06, schrieb Mans Rullgard: >> case 2: /* wfe */ >> case 4: /* sev */ >> + case 5: /* sevl */ >> /* TODO: Implement SEV and WFE. May help SMP performance. */ >> default: /* nop */ >> break; > > So does SEVL need to be implemented? Then the TODO should be updated. > Otherwise moving it a line down may be clearer. I updated the comment to mention SEVL as part of taking this patch into target-arm.next. thanks -- PMM ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] target-arm: add feature flag for ARMv8 2013-06-07 12:06 [Qemu-devel] [PATCH 1/3] target-arm: add feature flag for ARMv8 Mans Rullgard 2013-06-07 12:06 ` [Qemu-devel] [PATCH 2/3] target-arm: implement LDA/STL instructions Mans Rullgard 2013-06-07 12:06 ` [Qemu-devel] [PATCH 3/3] target-arm: explicitly decode SEVL instruction Mans Rullgard @ 2013-06-13 12:54 ` Peter Maydell 2013-06-13 13:25 ` [Qemu-devel] [PATCH v2] " Mans Rullgard 2 siblings, 1 reply; 15+ messages in thread From: Peter Maydell @ 2013-06-13 12:54 UTC (permalink / raw) To: Mans Rullgard; +Cc: qemu-devel On 7 June 2013 13:06, Mans Rullgard <mans@mansr.com> wrote: > Signed-off-by: Mans Rullgard <mans@mansr.com> > --- > target-arm/cpu.c | 5 ++++- > target-arm/cpu.h | 1 + > target-arm/translate.c | 1 + > 3 files changed, 6 insertions(+), 1 deletion(-) > > diff --git a/target-arm/cpu.c b/target-arm/cpu.c > index 496a59f..f5a1314 100644 > --- a/target-arm/cpu.c > +++ b/target-arm/cpu.c > @@ -162,6 +162,9 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp) > CPUARMState *env = &cpu->env; > > /* Some features automatically imply others: */ > + if (arm_feature(env, ARM_FEATURE_V8)) { > + set_feature(env, ARM_FEATURE_V7); > + } This should also imply ARM_FEATURE_ARM_DIV and ARM_FEATURE_LPAE. thanks -- PMM ^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH v2] target-arm: add feature flag for ARMv8 2013-06-13 12:54 ` [Qemu-devel] [PATCH 1/3] target-arm: add feature flag for ARMv8 Peter Maydell @ 2013-06-13 13:25 ` Mans Rullgard 2013-06-13 14:01 ` Peter Maydell 0 siblings, 1 reply; 15+ messages in thread From: Mans Rullgard @ 2013-06-13 13:25 UTC (permalink / raw) To: qemu-devel Signed-off-by: Mans Rullgard <mans@mansr.com> --- Make V8 imply ARM_DIV and LPAE. --- target-arm/cpu.c | 7 ++++++- target-arm/cpu.h | 1 + target-arm/translate.c | 1 + 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/target-arm/cpu.c b/target-arm/cpu.c index 496a59f..b75f396 100644 --- a/target-arm/cpu.c +++ b/target-arm/cpu.c @@ -162,6 +162,11 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp) CPUARMState *env = &cpu->env; /* Some features automatically imply others: */ + if (arm_feature(env, ARM_FEATURE_V8)) { + set_feature(env, ARM_FEATURE_V7); + set_feature(env, ARM_FEATURE_ARM_DIV); + set_feature(env, ARM_FEATURE_LPAE); + } if (arm_feature(env, ARM_FEATURE_V7)) { set_feature(env, ARM_FEATURE_VAPA); set_feature(env, ARM_FEATURE_THUMB2); @@ -748,7 +753,7 @@ static void pxa270c5_initfn(Object *obj) static void arm_any_initfn(Object *obj) { ARMCPU *cpu = ARM_CPU(obj); - set_feature(&cpu->env, ARM_FEATURE_V7); + set_feature(&cpu->env, ARM_FEATURE_V8); set_feature(&cpu->env, ARM_FEATURE_VFP4); set_feature(&cpu->env, ARM_FEATURE_VFP_FP16); set_feature(&cpu->env, ARM_FEATURE_NEON); diff --git a/target-arm/cpu.h b/target-arm/cpu.h index 5438444..b3be588 100644 --- a/target-arm/cpu.h +++ b/target-arm/cpu.h @@ -392,6 +392,7 @@ enum arm_features { ARM_FEATURE_MPIDR, /* has cp15 MPIDR */ ARM_FEATURE_PXN, /* has Privileged Execute Never bit */ ARM_FEATURE_LPAE, /* has Large Physical Address Extension */ + ARM_FEATURE_V8, }; static inline int arm_feature(CPUARMState *env, int feature) diff --git a/target-arm/translate.c b/target-arm/translate.c index b3f26d6..96ac5bc 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -42,6 +42,7 @@ #define ENABLE_ARCH_6K arm_feature(env, ARM_FEATURE_V6K) #define ENABLE_ARCH_6T2 arm_feature(env, ARM_FEATURE_THUMB2) #define ENABLE_ARCH_7 arm_feature(env, ARM_FEATURE_V7) +#define ENABLE_ARCH_8 arm_feature(env, ARM_FEATURE_V8) #define ARCH(x) do { if (!ENABLE_ARCH_##x) goto illegal_op; } while(0) -- 1.8.2.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH v2] target-arm: add feature flag for ARMv8 2013-06-13 13:25 ` [Qemu-devel] [PATCH v2] " Mans Rullgard @ 2013-06-13 14:01 ` Peter Maydell 0 siblings, 0 replies; 15+ messages in thread From: Peter Maydell @ 2013-06-13 14:01 UTC (permalink / raw) To: Mans Rullgard; +Cc: qemu-devel On 13 June 2013 14:25, Mans Rullgard <mans@mansr.com> wrote: > Signed-off-by: Mans Rullgard <mans@mansr.com> > --- > Make V8 imply ARM_DIV and LPAE. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> -- PMM ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2013-07-02 13:03 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-06-07 12:06 [Qemu-devel] [PATCH 1/3] target-arm: add feature flag for ARMv8 Mans Rullgard 2013-06-07 12:06 ` [Qemu-devel] [PATCH 2/3] target-arm: implement LDA/STL instructions Mans Rullgard 2013-06-13 14:01 ` Peter Maydell 2013-06-17 16:50 ` [Qemu-devel] [PATCH v2] " Mans Rullgard 2013-06-20 17:51 ` Peter Maydell 2013-07-01 13:46 ` [Qemu-devel] [PATCH v3] " Mans Rullgard 2013-07-02 12:46 ` Peter Maydell 2013-06-07 12:06 ` [Qemu-devel] [PATCH 3/3] target-arm: explicitly decode SEVL instruction Mans Rullgard 2013-06-13 12:59 ` Peter Maydell 2013-06-13 13:15 ` Andreas Färber 2013-06-13 13:39 ` Måns Rullgård 2013-07-02 12:53 ` Peter Maydell 2013-06-13 12:54 ` [Qemu-devel] [PATCH 1/3] target-arm: add feature flag for ARMv8 Peter Maydell 2013-06-13 13:25 ` [Qemu-devel] [PATCH v2] " Mans Rullgard 2013-06-13 14:01 ` Peter Maydell
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).