[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] Target-arm: Add the Cortex-M4 CPU
|
From: |
Peter Crosthwaite |
|
Subject: |
Re: [Qemu-devel] [PATCH] Target-arm: Add the Cortex-M4 CPU |
|
Date: |
Sat, 30 May 2015 03:46:35 -0700 |
On Thu, May 28, 2015 at 2:09 PM, Aurelio C. Remonda
<address@hidden> wrote:
> This patch adds the Cortex-M4 CPU. The M4 is basically the same as the M3,
> the main differences being the DSP instructions and an optional FPU.
> The DSP instructions are already implemented in Qemu, as the A and R profiles
> use them.
>
> I created an ARM_FEATURE_THUMB_DSP to be added to any non-M thumb2-compatible
> CPU that uses DSP instructions, and I manually added it to the M4 in its
> initfn.
>
This should be a separate patch, one for your refactoring adding the
new ARM_FEATURE then a second one for the cortex-m4.
> The optional FPU in the M4 could be added in the future as a "Cortex-M4F" CPU.
> All we'd have to do is add the ARM_FEATURE_VFP4 to the initfn.
>
> There are 85 DSP instructions (all of them thumb2). On disas_thumb2_insn
> the DSP feature is tested before the instruction is generated; if it's not
> enabled then its an illegal op.
>
> Signed-off-by: Aurelio C. Remonda <address@hidden>
> ---
> target-arm/cpu.c | 22 +++++++++
> target-arm/cpu.h | 1 +
> target-arm/translate.c | 130
> +++++++++++++++++++++++++++++++++++++++++++++++--
> 3 files changed, 149 insertions(+), 4 deletions(-)
>
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index 3ca3fa8..9be4a06 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -512,6 +512,10 @@ static void arm_cpu_realizefn(DeviceState *dev, Error
> **errp)
> if (arm_feature(env, ARM_FEATURE_CBAR_RO)) {
> set_feature(env, ARM_FEATURE_CBAR);
> }
> + if (arm_feature(env,ARM_FEATURE_THUMB2) &&
> + !arm_feature(env,ARM_FEATURE_M)) {
> + set_feature(env, ARM_FEATURE_THUMB_DSP);
> + }
>
> if (cpu->reset_hivecs) {
> cpu->reset_sctlr |= (1 << 13);
> @@ -761,6 +765,22 @@ static void cortex_m3_initfn(Object *obj)
> set_feature(&cpu->env, ARM_FEATURE_M);
> cpu->midr = 0x410fc231;
> }
> +static void cortex_m4_initfn(Object *obj)
> +{
> + ARMCPU *cpu = ARM_CPU(obj);
> + set_feature(&cpu->env, ARM_FEATURE_V7);
> + set_feature(&cpu->env, ARM_FEATURE_M);
> + set_feature(&cpu->env, ARM_FEATURE_THUMB_DSP);
> + cpu->midr = 0x410fc240;
> + /* Main id register CPUID bit assignments
> + * Bits NAME Function
> + * [31:24] IMPLEMENTER Indicates implementor: 0x41 = ARM
> + * [23:20] VARIANT Indicates processor revision: 0x0 = Revision 0
> + * [19:16] (Constant) Reads as 0xF
> + * [15:4] PARTNO Indicates part number: 0xC24 = Cortex-M4
> + * [3:0] REVISION Indicates patch release: 0x0 = Patch 0.
> + */
> +}
>
> static void arm_v7m_class_init(ObjectClass *oc, void *data)
> {
> @@ -1164,6 +1184,8 @@ static const ARMCPUInfo arm_cpus[] = {
> { .name = "arm11mpcore", .initfn = arm11mpcore_initfn },
> { .name = "cortex-m3", .initfn = cortex_m3_initfn,
> .class_init = arm_v7m_class_init },
> + { .name = "cortex-m4", .initfn = cortex_m4_initfn,
> + .class_init = arm_v7m_class_init },
> { .name = "cortex-a8", .initfn = cortex_a8_initfn },
> { .name = "cortex-a9", .initfn = cortex_a9_initfn },
> { .name = "cortex-a15", .initfn = cortex_a15_initfn },
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index d4a5899..fa5c3bc 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -886,6 +886,7 @@ enum arm_features {
> ARM_FEATURE_V8_SHA1, /* implements SHA1 part of v8 Crypto Extensions */
> ARM_FEATURE_V8_SHA256, /* implements SHA256 part of v8 Crypto Extensions
> */
> ARM_FEATURE_V8_PMULL, /* implements PMULL part of v8 Crypto Extensions */
> + ARM_FEATURE_THUMB_DSP,/* DSP insns supported in the Thumb encodings */
Space after ","
> };
>
> static inline int arm_feature(CPUARMState *env, int feature)
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 9116529..c9d2e4f 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -9428,6 +9428,10 @@ static int disas_thumb2_insn(CPUARMState *env,
> DisasContext *s, uint16_t insn_hw
>
> op = (insn >> 21) & 0xf;
> if (op == 6) {
> + if (!arm_dc_feature(s, ARM_FEATURE_THUMB_DSP)) {
> + /* pkhtb, pkfbt are DSP instructions */
> + goto illegal_op;
> + }
> /* Halfword pack. */
> tmp = load_reg(s, rn);
> tmp2 = load_reg(s, rm);
> @@ -9502,13 +9506,37 @@ static int disas_thumb2_insn(CPUARMState *env,
> DisasContext *s, uint16_t insn_hw
> switch (op) {
> case 0: gen_sxth(tmp); break;
> case 1: gen_uxth(tmp); break;
> - case 2: gen_sxtb16(tmp); break;
> - case 3: gen_uxtb16(tmp); break;
> + case 2:
> + if (!(arm_dc_feature(s, ARM_FEATURE_THUMB_DSP))){
Space after ))). Does checkpatch catch this?
one set of parenthesis un-needed
> + /* sxtab16, sxtb16 are DSP instructions */
> + tcg_temp_free_i32(tmp);
> + goto illegal_op;
> + }
> + gen_sxtb16(tmp);
> + break;
> + case 3:
> + if (!(arm_dc_feature(s, ARM_FEATURE_THUMB_DSP))){
> + /* uxtb16, uxtab16 are DSP instructions */
> + tcg_temp_free_i32(tmp);
> + goto illegal_op;
> + }
> + gen_uxtb16(tmp);
> + break;
> case 4: gen_sxtb(tmp); break;
> case 5: gen_uxtb(tmp); break;
> default: goto illegal_op;
> }
> + /*-sxtab->-sxtab16->*/
> if (rn != 15) {
> + if (!(arm_dc_feature(s, ARM_FEATURE_THUMB_DSP))){
> + /* sxtab, sxtah, uxtab, uxtah are DSP instructions.
> + * sxtb, sxth, uxtb, uxth are not DSP according to
> + * ARMv7-M Architecture Reference Manual
> + */
> + /* need to free this so there's no TCG temporary leak */
> + tcg_temp_free_i32(tmp);
> + goto illegal_op;
> + }
> tmp2 = load_reg(s, rn);
> if ((op >> 1) == 1) {
> gen_add16(tmp, tmp2);
> @@ -9521,6 +9549,12 @@ static int disas_thumb2_insn(CPUARMState *env,
> DisasContext *s, uint16_t insn_hw
> break;
> case 2: /* SIMD add/subtract. */
> op = (insn >> 20) & 7;
> + if (!(arm_dc_feature(s, ARM_FEATURE_THUMB_DSP)) ){
same. Also extra space here.
> + /* add16, sub16, asx, sax, add8, sub8 (with q, s, sh, u, uh,
> + * and uq variants) and usad8, usada8
> + */
> + goto illegal_op;
> + }
> shift = (insn >> 4) & 7;
> if ((op & 3) == 3 || (shift & 3) == 3)
> goto illegal_op;
> @@ -9532,8 +9566,13 @@ static int disas_thumb2_insn(CPUARMState *env,
> DisasContext *s, uint16_t insn_hw
> break;
> case 3: /* Other data processing. */
> op = ((insn >> 17) & 0x38) | ((insn >> 4) & 7);
> +
Out of scope change.
> if (op < 4) {
> /* Saturating add/subtract. */
> + if (!(arm_dc_feature(s, ARM_FEATURE_THUMB_DSP))){
> + /* qsub, qadd, qdadd, qdsub are DSP instructions. */
> + goto illegal_op;
> + }
> tmp = load_reg(s, rn);
> tmp2 = load_reg(s, rm);
> if (op & 1)
> @@ -9559,6 +9598,12 @@ static int disas_thumb2_insn(CPUARMState *env,
> DisasContext *s, uint16_t insn_hw
> gen_revsh(tmp);
> break;
> case 0x10: /* sel */
> + if (!(arm_dc_feature(s, ARM_FEATURE_THUMB_DSP))){
> + /* sel is a DSP instruction. */
> + /* need to free this so there's no TCG temporary leak */
> + tcg_temp_free_i32(tmp);
> + goto illegal_op;
> + }
> tmp2 = load_reg(s, rm);
> tmp3 = tcg_temp_new_i32();
> tcg_gen_ld_i32(tmp3, cpu_env, offsetof(CPUARMState, GE));
> @@ -9624,6 +9669,15 @@ static int disas_thumb2_insn(CPUARMState *env,
> DisasContext *s, uint16_t insn_hw
> }
> break;
> case 1: /* 16 x 16 -> 32 */
> + if (!(arm_dc_feature(s, ARM_FEATURE_THUMB_DSP))){
> + /* smlabb, smlabt, smlatb, smlatt, smulbb, smulbt, smultt
> + * and smultb are DSP instructions
> + */
> + /* need to free this so there's no TCG temporary leak */
Comment un-needed.
Regards,
Peter
> + tcg_temp_free_i32(tmp);
> + tcg_temp_free_i32(tmp2);
> + goto illegal_op;
> + }
> gen_mulxy(tmp, tmp2, op & 2, op & 1);
> tcg_temp_free_i32(tmp2);
> if (rs != 15) {
> @@ -9634,6 +9688,14 @@ static int disas_thumb2_insn(CPUARMState *env,
> DisasContext *s, uint16_t insn_hw
> break;
> case 2: /* Dual multiply add. */
> case 4: /* Dual multiply subtract. */
> + if (!(arm_dc_feature(s, ARM_FEATURE_THUMB_DSP))){
> + /* smlad, smladx, smlsd, smusd are DSP instructions */
> +
> + /* need to free this so there's no TCG temporary leak */
> + tcg_temp_free_i32(tmp);
> + tcg_temp_free_i32(tmp2);
> + goto illegal_op;
> + }
> if (op)
> gen_swap_half(tmp2);
> gen_smul_dual(tmp, tmp2);
> @@ -9656,6 +9718,13 @@ static int disas_thumb2_insn(CPUARMState *env,
> DisasContext *s, uint16_t insn_hw
> }
> break;
> case 3: /* 32 * 16 -> 32msb */
> + if (!(arm_dc_feature(s, ARM_FEATURE_THUMB_DSP))){
> + /* smlawb, smlawt, smulwt, smulwb are DSP instructions */
> + /* need to free this so there's no TCG temporary leak */
> + tcg_temp_free_i32(tmp);
> + tcg_temp_free_i32(tmp2);
> + goto illegal_op;
> + }
> if (op)
> tcg_gen_sari_i32(tmp2, tmp2, 16);
> else
> @@ -9673,6 +9742,15 @@ static int disas_thumb2_insn(CPUARMState *env,
> DisasContext *s, uint16_t insn_hw
> }
> break;
> case 5: case 6: /* 32 * 32 -> 32msb (SMMUL, SMMLA, SMMLS) */
> + if (!(arm_dc_feature(s, ARM_FEATURE_THUMB_DSP))){
> + /* smmla, smmls, smmul, smuad, smmlar,
> + * smmlsr, smmulr are DSP instructions
> + */
> + /* need to free this so there's no TCG temporary leak */
> + tcg_temp_free_i32(tmp);
> + tcg_temp_free_i32(tmp2);
> + goto illegal_op;
> + }
> tmp64 = gen_muls_i64_i32(tmp, tmp2);
> if (rs != 15) {
> tmp = load_reg(s, rs);
> @@ -9719,6 +9797,13 @@ static int disas_thumb2_insn(CPUARMState *env,
> DisasContext *s, uint16_t insn_hw
> store_reg(s, rd, tmp);
> } else if ((op & 0xe) == 0xc) {
> /* Dual multiply accumulate long. */
> + if (!(arm_dc_feature(s, ARM_FEATURE_THUMB_DSP))){
> + /* smlald, smlsld are DSP instructions */
> + /* need to free this so there's no TCG temporary leak */
> + tcg_temp_free_i32(tmp);
> + tcg_temp_free_i32(tmp2);
> + goto illegal_op;
> + }
> if (op & 1)
> gen_swap_half(tmp2);
> gen_smul_dual(tmp, tmp2);
> @@ -9742,6 +9827,17 @@ static int disas_thumb2_insn(CPUARMState *env,
> DisasContext *s, uint16_t insn_hw
> } else {
> if (op & 8) {
> /* smlalxy */
> + if (!(arm_dc_feature(s, ARM_FEATURE_THUMB_DSP))){
> + /* smlalbb, smlalbt, smlaltb, smlaltt
> + * are DSP instructions
> + */
> + /* need to free this so there's no TCG
> + * temporary leak
> + */
> + tcg_temp_free_i32(tmp2);
> + tcg_temp_free_i32(tmp);
> + goto illegal_op;
> + }
> gen_mulxy(tmp, tmp2, op & 2, op & 1);
> tcg_temp_free_i32(tmp2);
> tmp64 = tcg_temp_new_i64();
> @@ -9754,6 +9850,12 @@ static int disas_thumb2_insn(CPUARMState *env,
> DisasContext *s, uint16_t insn_hw
> }
> if (op & 4) {
> /* umaal */
> + if (!(arm_dc_feature(s, ARM_FEATURE_THUMB_DSP))){
> + /* ummal is a DSP instruction */
> + /* need to free this so there's no TCG temporary
> leak */
> + tcg_temp_free_i64(tmp64);
> + goto illegal_op;
> + }
> gen_addq_lo(s, tmp64, rs);
> gen_addq_lo(s, tmp64, rd);
> } else if (op & 0x40) {
> @@ -10018,14 +10120,34 @@ static int disas_thumb2_insn(CPUARMState *env,
> DisasContext *s, uint16_t insn_hw
> tmp2 = tcg_const_i32(imm);
> if (op & 4) {
> /* Unsigned. */
> - if ((op & 1) && shift == 0)
> + if ((op & 1) && shift == 0){
> + if (!(arm_dc_feature(s,
> ARM_FEATURE_THUMB_DSP))){
> + /* usat16 is a DSP instruction */
> + /* need to free this so there's no TCG
> + * temporary leak
> + */
> + tcg_temp_free_i32(tmp);
> + tcg_temp_free_i32(tmp2);
> + goto illegal_op;
> + }
> gen_helper_usat16(tmp, cpu_env, tmp, tmp2);
> + }
> else
> gen_helper_usat(tmp, cpu_env, tmp, tmp2);
> } else {
> /* Signed. */
> - if ((op & 1) && shift == 0)
> + if ((op & 1) && shift == 0){
> + if (!(arm_dc_feature(s,
> ARM_FEATURE_THUMB_DSP))){
> + /* ssat16 is a DSP instruction */
> + /* need to free this so there's no TCG
> + * temporary leak
> + */
> + tcg_temp_free_i32(tmp);
> + tcg_temp_free_i32(tmp2);
> + goto illegal_op;
> + }
> gen_helper_ssat16(tmp, cpu_env, tmp, tmp2);
> + }
> else
> gen_helper_ssat(tmp, cpu_env, tmp, tmp2);
> }
> --
> 1.9.1
>
>
Re: [Qemu-devel] [PATCH] Target-arm: Add the Cortex-M4 CPU,
Peter Crosthwaite <=