* [Qemu-devel] [PATCH] Target-arm: Add the Cortex-M4 CPU
@ 2015-05-28 21:09 Aurelio C. Remonda
2015-05-28 21:22 ` Liviu Ionescu
2015-05-30 10:46 ` Peter Crosthwaite
0 siblings, 2 replies; 15+ messages in thread
From: Aurelio C. Remonda @ 2015-05-28 21:09 UTC (permalink / raw)
To: qemu-devel, ilg, peter.maydell, martin.galvan, daniel.gutson
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.
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 <aurelioremonda@gmail.com>
---
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 */
};
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))){
+ /* 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)) ){
+ /* 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);
+
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 */
+ 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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH] Target-arm: Add the Cortex-M4 CPU
2015-05-28 21:09 [Qemu-devel] [PATCH] Target-arm: Add the Cortex-M4 CPU Aurelio C. Remonda
@ 2015-05-28 21:22 ` Liviu Ionescu
2015-05-28 22:03 ` Peter Maydell
2015-05-29 12:55 ` aurelio remonda
2015-05-30 10:46 ` Peter Crosthwaite
1 sibling, 2 replies; 15+ messages in thread
From: Liviu Ionescu @ 2015-05-28 21:22 UTC (permalink / raw)
To: Aurelio C. Remonda
Cc: peter.maydell, daniel.gutson, qemu-devel, martin.galvan
> On 29 May 2015, at 00:09, Aurelio C. Remonda <aurelioremonda@gmail.com> 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.
great!
Peter mentioned some differences in exception processing, did you check if they require changes in emulation?
> The optional FPU in the M4 could be added in the future as a "Cortex-M4F" CPU.
in my implementation I had a single name ("cortex-m4") and some flags, but a separate name is probably better. can we reserve
{ .name = "cortex-m4f", ... }
for this purpose?
> All we'd have to do is add the ARM_FEATURE_VFP4 to the initfn.
if it is that simple, why don't we add it in for now?
regards,
Liviu
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH] Target-arm: Add the Cortex-M4 CPU
2015-05-28 21:22 ` Liviu Ionescu
@ 2015-05-28 22:03 ` Peter Maydell
2015-05-29 12:55 ` aurelio remonda
1 sibling, 0 replies; 15+ messages in thread
From: Peter Maydell @ 2015-05-28 22:03 UTC (permalink / raw)
To: Liviu Ionescu
Cc: daniel.gutson, Aurelio C. Remonda, QEMU Developers, Martin Galvan
On 28 May 2015 at 22:22, Liviu Ionescu <ilg@livius.net> wrote:
>
>> On 29 May 2015, at 00:09, Aurelio C. Remonda <aurelioremonda@gmail.com> wrote:
>> All we'd have to do is add the ARM_FEATURE_VFP4 to the initfn.
>
> if it is that simple, why don't we add it in for now?
It's not -- the FPU needs support in the exception handling
(lazy FPU saving is particularly interesting, but there's also
a bunch of minor stuff like "expose the FPU version registers
as memory mapped regs" IIRC).
-- PMM
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH] Target-arm: Add the Cortex-M4 CPU
2015-05-28 21:22 ` Liviu Ionescu
2015-05-28 22:03 ` Peter Maydell
@ 2015-05-29 12:55 ` aurelio remonda
2015-05-29 13:21 ` Peter Maydell
2015-05-29 19:15 ` Liviu Ionescu
1 sibling, 2 replies; 15+ messages in thread
From: aurelio remonda @ 2015-05-29 12:55 UTC (permalink / raw)
To: Liviu Ionescu; +Cc: Peter Maydell, Daniel Gutson, qemu-devel, Martin Galvan
2015-05-28 18:22 GMT-03:00 Liviu Ionescu <ilg@livius.net>:
> > On 29 May 2015, at 00:09, Aurelio C. Remonda <aurelioremonda@gmail.com> wrote:
> > The optional FPU in the M4 could be added in the future as a "Cortex-M4F" CPU.
>
> in my implementation I had a single name ("cortex-m4") and some flags, but a separate name is probably better. can we reserve
>
> { .name = "cortex-m4f", ... }
>
> for this purpose?
Thanks for the feedback! Yes, we could but I think its outside of the
scope of this particular contribution.
> > All we'd have to do is add the ARM_FEATURE_VFP4 to the initfn.
>
> if it is that simple, why don't we add it in for now?
As Peter said, it may not actually be that simple.
Perhaps we could commit this now and add the VFP in the future?
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH] Target-arm: Add the Cortex-M4 CPU
2015-05-29 12:55 ` aurelio remonda
@ 2015-05-29 13:21 ` Peter Maydell
2015-05-29 19:15 ` Liviu Ionescu
1 sibling, 0 replies; 15+ messages in thread
From: Peter Maydell @ 2015-05-29 13:21 UTC (permalink / raw)
To: aurelio remonda
Cc: Liviu Ionescu, Daniel Gutson, QEMU Developers, Martin Galvan
On 29 May 2015 at 13:55, aurelio remonda <aurelioremonda@gmail.com> wrote
> Perhaps we could commit this now and add the VFP in the future?
That's how we've generally worked, yes -- if you just ask
for a "Cortex-A9" or whatever you get the maximum set of
features that CPU can support (so always Neon & VFP even if
some A9 hardware is configured without VFP), and if we add
support for a feature that the hardware has after the
CPU model was first added then we add the feature to that
CPU. (So we didn't have TrustZone support when the A9
was initially put into QEMU, but when we put in the TZ
support we turned on the ARM_FEATURE_EL3 bit for the A9 by
default.)
If we need to also support CPUs of that model that don't have
a particular optional feature we can do that via a CPU property
(as we do with 'has_el3' for disabling TZ support). Then a board
that wants an M4 without FPU can set the property to false, or
a user who wants to control it from the command line can do so
via -cpu cortex-m4,has-fpu=false or similar.
-- PMM
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH] Target-arm: Add the Cortex-M4 CPU
2015-05-29 12:55 ` aurelio remonda
2015-05-29 13:21 ` Peter Maydell
@ 2015-05-29 19:15 ` Liviu Ionescu
2015-05-29 19:16 ` Martin Galvan
1 sibling, 1 reply; 15+ messages in thread
From: Liviu Ionescu @ 2015-05-29 19:15 UTC (permalink / raw)
To: aurelio remonda; +Cc: Peter Maydell, Daniel Gutson, qemu-devel, Martin Galvan
> On 29 May 2015, at 15:55, aurelio remonda <aurelioremonda@gmail.com> wrote:
>
> 2015-05-28 18:22 GMT-03:00 Liviu Ionescu <ilg@livius.net>:
>>> On 29 May 2015, at 00:09, Aurelio C. Remonda <aurelioremonda@gmail.com> wrote:
>>> The optional FPU in the M4 could be added in the future as a "Cortex-M4F" CPU.
>>
>> in my implementation I had a single name ("cortex-m4") and some flags, but a separate name is probably better. can we reserve
>>
>> { .name = "cortex-m4f", ... }
>>
>> for this purpose?
>
> Thanks for the feedback! Yes, we could but I think its outside of the
> scope of this particular contribution.
ok, I already updated my code to use cortex-m4 or cortex-m4f.
>
>>> All we'd have to do is add the ARM_FEATURE_VFP4 to the initfn.
>>
>> if it is that simple, why don't we add it in for now?
>
> As Peter said, it may not actually be that simple.
>
> Perhaps we could commit this now and add the VFP in the future?
this is fine with me.
unfortunately I cannot review your patch, since I have not enough experience with that part of qemu.
regards,
Liviu
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH] Target-arm: Add the Cortex-M4 CPU
2015-05-29 19:15 ` Liviu Ionescu
@ 2015-05-29 19:16 ` Martin Galvan
2015-05-29 19:30 ` Peter Maydell
2015-06-10 9:58 ` Liviu Ionescu
0 siblings, 2 replies; 15+ messages in thread
From: Martin Galvan @ 2015-05-29 19:16 UTC (permalink / raw)
To: Liviu Ionescu
Cc: Daniel Gutson, Peter Maydell, aurelio remonda, QEMU Developers
On Fri, May 29, 2015 at 4:15 PM, Liviu Ionescu <ilg@livius.net> wrote:
> this is fine with me.
>
> unfortunately I cannot review your patch, since I have not enough experience with that part of qemu.
I think Peter M is the maintainer for Target-ARM.
Peter, is this OK to commit?
--
Martin Galvan
Software Engineer
Taller Technologies Argentina
San Lorenzo 47, 3rd Floor, Office 5
Córdoba, Argentina
Phone: 54 351 4217888 / +54 351 4218211
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH] Target-arm: Add the Cortex-M4 CPU
2015-05-29 19:16 ` Martin Galvan
@ 2015-05-29 19:30 ` Peter Maydell
2015-06-10 9:58 ` Liviu Ionescu
1 sibling, 0 replies; 15+ messages in thread
From: Peter Maydell @ 2015-05-29 19:30 UTC (permalink / raw)
To: Martin Galvan
Cc: Daniel Gutson, Liviu Ionescu, aurelio remonda, QEMU Developers
On 29 May 2015 at 20:16, Martin Galvan
<martin.galvan@tallertechnologies.com> wrote:
> On Fri, May 29, 2015 at 4:15 PM, Liviu Ionescu <ilg@livius.net> wrote:
>> this is fine with me.
>>
>> unfortunately I cannot review your patch, since I have not enough experience with that part of qemu.
>
> I think Peter M is the maintainer for Target-ARM.
>
> Peter, is this OK to commit?
The patch is on my to-review queue; unfortunately I'm quite
heavily loaded with patches to review at the moment, so it
may be a week or so before I can get to this one in detail.
Sorry about that.
At a quick glance it looks broadly OK. Personally I prefer
hoisting the "UNDEF if feature missing" checks earlier in the
decode function rather than having them later and having to
do manual tcg_temp_free operations, but this isn't always
feasible with the existing structure of the code.
I might also split it into two patches: one which just
adds and implements the new feature bit, and then one
which creates the new M4 CPU. (That way if it turns out
that we need to make other changes for the M4 before
we can enable it, the first patch can still be reviewed
and possibly committed even if we have to wait before
putting in the new CPU itself.)
thanks
-- PMM
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH] Target-arm: Add the Cortex-M4 CPU
2015-05-28 21:09 [Qemu-devel] [PATCH] Target-arm: Add the Cortex-M4 CPU Aurelio C. Remonda
2015-05-28 21:22 ` Liviu Ionescu
@ 2015-05-30 10:46 ` Peter Crosthwaite
2015-05-30 22:08 ` aurelio remonda
1 sibling, 1 reply; 15+ messages in thread
From: Peter Crosthwaite @ 2015-05-30 10:46 UTC (permalink / raw)
To: Aurelio C. Remonda
Cc: Liviu Ionescu, Martin Galvan, daniel.gutson,
qemu-devel@nongnu.org Developers, Peter Maydell
On Thu, May 28, 2015 at 2:09 PM, Aurelio C. Remonda
<aurelioremonda@gmail.com> 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 <aurelioremonda@gmail.com>
> ---
> 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
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH] Target-arm: Add the Cortex-M4 CPU
2015-05-30 10:46 ` Peter Crosthwaite
@ 2015-05-30 22:08 ` aurelio remonda
2015-05-30 23:16 ` Peter Crosthwaite
0 siblings, 1 reply; 15+ messages in thread
From: aurelio remonda @ 2015-05-30 22:08 UTC (permalink / raw)
To: Peter Crosthwaite
Cc: Liviu Ionescu, Martin Galvan, Daniel Gutson,
qemu-devel@nongnu.org Developers, Peter Maydell
>> 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.
Why do you think it's not needed? Any comments are helpful as long as
they're correct. Thanks
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH] Target-arm: Add the Cortex-M4 CPU
2015-05-30 22:08 ` aurelio remonda
@ 2015-05-30 23:16 ` Peter Crosthwaite
0 siblings, 0 replies; 15+ messages in thread
From: Peter Crosthwaite @ 2015-05-30 23:16 UTC (permalink / raw)
To: aurelio remonda
Cc: Peter Maydell, Liviu Ionescu, Daniel Gutson, Martin Galvan,
qemu-devel@nongnu.org Developers
On Sat, May 30, 2015 at 3:08 PM, aurelio remonda
<aurelioremonda@gmail.com> wrote:
>>> 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.
>
> Why do you think it's not needed? Any comments are helpful as long as
> they're correct. Thanks
>
Not always. Comments on code are a maintainance burden. It's
preferrable to have the code self document as much as possible so that
changes only need to be applied once - to the code itself. The only
reason to ever call tcg_temp_free is to avoid a leak so its self
explanatory IMO.
Regards,
Peter
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH] Target-arm: Add the Cortex-M4 CPU
2015-05-29 19:16 ` Martin Galvan
2015-05-29 19:30 ` Peter Maydell
@ 2015-06-10 9:58 ` Liviu Ionescu
2015-06-10 10:44 ` Peter Maydell
1 sibling, 1 reply; 15+ messages in thread
From: Liviu Ionescu @ 2015-06-10 9:58 UTC (permalink / raw)
To: Martin Galvan
Cc: Daniel Gutson, Peter Maydell, aurelio remonda, QEMU Developers
> On 29 May 2015, at 22:16, Martin Galvan <martin.galvan@tallertechnologies.com> wrote:
>
>
> ... I think Peter M is the maintainer for Target-ARM.
>
> Peter, is this OK to commit?
was this patch accepted? if so, where can I get it from?
regards,
Liviu
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH] Target-arm: Add the Cortex-M4 CPU
2015-06-10 9:58 ` Liviu Ionescu
@ 2015-06-10 10:44 ` Peter Maydell
2015-06-10 10:49 ` Daniel Gutson
0 siblings, 1 reply; 15+ messages in thread
From: Peter Maydell @ 2015-06-10 10:44 UTC (permalink / raw)
To: Liviu Ionescu
Cc: Daniel Gutson, aurelio remonda, Martin Galvan, QEMU Developers
On 10 June 2015 at 10:58, Liviu Ionescu <ilg@livius.net> wrote:
>
>> On 29 May 2015, at 22:16, Martin Galvan <martin.galvan@tallertechnologies.com> wrote:
>>
>>
>> ... I think Peter M is the maintainer for Target-ARM.
>>
>> Peter, is this OK to commit?
>
> was this patch accepted?
No; as Peter C suggested, Martin split it into two patches.
Those patches then got code review, but I don't think Martin
has sent out a revised series yet.
-- PMM
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH] Target-arm: Add the Cortex-M4 CPU
2015-06-10 10:44 ` Peter Maydell
@ 2015-06-10 10:49 ` Daniel Gutson
2015-06-10 10:53 ` Peter Maydell
0 siblings, 1 reply; 15+ messages in thread
From: Daniel Gutson @ 2015-06-10 10:49 UTC (permalink / raw)
To: Peter Maydell
Cc: Liviu Ionescu, Aurelio Remonda, Martin Galvan, QEMU Developers
[-- Attachment #1: Type: text/plain, Size: 715 bytes --]
El 10/6/2015 7:45, "Peter Maydell" <peter.maydell@linaro.org> escribió:
>
> On 10 June 2015 at 10:58, Liviu Ionescu <ilg@livius.net> wrote:
> >
> >> On 29 May 2015, at 22:16, Martin Galvan <
martin.galvan@tallertechnologies.com> wrote:
> >>
> >>
> >> ... I think Peter M is the maintainer for Target-ARM.
> >>
> >> Peter, is this OK to commit?
> >
> > was this patch accepted?
>
> No; as Peter C suggested, Martin split it into two patches.
> Those patches then got code review, but I don't think Martin
> has sent out a revised series yet.
Aurelio is the primary author of the patch.
He's busy these days but he will commit the new version soon, hopefully
during the weekend.
>
> -- PMM
[-- Attachment #2: Type: text/html, Size: 1111 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH] Target-arm: Add the Cortex-M4 CPU
2015-06-10 10:49 ` Daniel Gutson
@ 2015-06-10 10:53 ` Peter Maydell
0 siblings, 0 replies; 15+ messages in thread
From: Peter Maydell @ 2015-06-10 10:53 UTC (permalink / raw)
To: Daniel Gutson
Cc: Liviu Ionescu, Aurelio Remonda, Martin Galvan, QEMU Developers
On 10 June 2015 at 11:49, Daniel Gutson
<daniel.gutson@tallertechnologies.com> wrote:
> El 10/6/2015 7:45, "Peter Maydell" <peter.maydell@linaro.org> escribió:
>> No; as Peter C suggested, Martin split it into two patches.
>> Those patches then got code review, but I don't think Martin
>> has sent out a revised series yet.
>
> Aurelio is the primary author of the patch.
My apologies -- overly hasty misreading of the names in this
thread.
-- PMM
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2015-06-10 10:54 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-05-28 21:09 [Qemu-devel] [PATCH] Target-arm: Add the Cortex-M4 CPU Aurelio C. Remonda
2015-05-28 21:22 ` Liviu Ionescu
2015-05-28 22:03 ` Peter Maydell
2015-05-29 12:55 ` aurelio remonda
2015-05-29 13:21 ` Peter Maydell
2015-05-29 19:15 ` Liviu Ionescu
2015-05-29 19:16 ` Martin Galvan
2015-05-29 19:30 ` Peter Maydell
2015-06-10 9:58 ` Liviu Ionescu
2015-06-10 10:44 ` Peter Maydell
2015-06-10 10:49 ` Daniel Gutson
2015-06-10 10:53 ` Peter Maydell
2015-05-30 10:46 ` Peter Crosthwaite
2015-05-30 22:08 ` aurelio remonda
2015-05-30 23:16 ` Peter Crosthwaite
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).