[Qemu-devel] [PATCH V3] Target-arm: Add the Cortex-M4 CPU

[Qemu-devel] [PATCH V3] Target-arm: Add the Cortex-M4 CPU

[Aurelio C. Remonda]

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.
Created an ARM_FEATURE_THUMB_DSP to be added to any non-M thumb2-compatible
CPU that uses DSP instructions, and manually added it to the M4 in its initfn.

Signed-off-by: Aurelio C. Remonda <aurelioremonda@gmail.com>
---
* Changes in V3: Add the ARM_FEATURE_THUMB_DSP in a separate patch and create 
just the CPU init on this one.
 target-arm/cpu.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 4a888ab..4950897 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -783,6 +783,21 @@ static void cortex_m3_initfn(Object *obj)
     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)
 {
     CPUClass *cc = CPU_CLASS(oc);
@@ -1185,6 +1200,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 },
-- 
1.9.1

Re: [Qemu-devel] [PATCH V3] Target-arm: Add the Cortex-M4 CPU

[Peter Maydell]

On 15 June 2015 at 21:31, 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.

OK, I've checked through the v7M ARM ARM, and I'm happy that
we don't need any further changes to the core code to support
a no-FPU Cortex-M4. so this patch is functionally good, and just
needs a few tweaks.

> Created an ARM_FEATURE_THUMB_DSP to be added to any non-M thumb2-compatible
> CPU that uses DSP instructions, and manually added it to the M4 in its initfn.

This commit message could use updating:
 * the THUMB_DSP feature was created in a different patch
 * it's worth mentioning that we're implementing the no-FPU M4
   here, and why (because the core target-arm code doesn't support
   the M-profile FPU model yet)

> Signed-off-by: Aurelio C. Remonda <aurelioremonda@gmail.com>
> ---
> * Changes in V3: Add the ARM_FEATURE_THUMB_DSP in a separate patch and create
> just the CPU init on this one.
>  target-arm/cpu.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index 4a888ab..4950897 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -783,6 +783,21 @@ static void cortex_m3_initfn(Object *obj)
>      cpu->midr = 0x410fc231;
>  }
>
> +static void cortex_m4_initfn(Object *obj)
> +{
> +    ARMCPU *cpu = ARM_CPU(obj);

You want a blank line here. Compare the other CPU initfns.

> +    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.*/

This whole comment is unnecessary -- the MIDR is the standard
format for all ARM CPUs, and no other initfn spells out the
various fields of it. You can say
    cpu->midr = 0x410fc240; /* r0p0 */
if you like.

> +}
>  static void arm_v7m_class_init(ObjectClass *oc, void *data)
>  {
>      CPUClass *cc = CPU_CLASS(oc);
> @@ -1185,6 +1200,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 },

If you can make those tweaks and resend I think we can
get this into QEMU for 2.4.

thanks
-- PMM

Re: [Qemu-devel] [PATCH V3] Target-arm: Add the Cortex-M4 CPU

[aurelio remonda]

[-- Attachment #1: Type: text/plain, Size: 3407 bytes --]

Thank you! Im working on it
El 16/6/2015 8:21, "Peter Maydell" <peter.maydell@linaro.org> escribió:

> On 15 June 2015 at 21:31, 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.
>
> OK, I've checked through the v7M ARM ARM, and I'm happy that
> we don't need any further changes to the core code to support
> a no-FPU Cortex-M4. so this patch is functionally good, and just
> needs a few tweaks.
>
> > Created an ARM_FEATURE_THUMB_DSP to be added to any non-M
> thumb2-compatible
> > CPU that uses DSP instructions, and manually added it to the M4 in its
> initfn.
>
> This commit message could use updating:
>  * the THUMB_DSP feature was created in a different patch
>  * it's worth mentioning that we're implementing the no-FPU M4
>    here, and why (because the core target-arm code doesn't support
>    the M-profile FPU model yet)
>
> > Signed-off-by: Aurelio C. Remonda <aurelioremonda@gmail.com>
> > ---
> > * Changes in V3: Add the ARM_FEATURE_THUMB_DSP in a separate patch and
> create
> > just the CPU init on this one.
> >  target-arm/cpu.c | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> >
> > diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> > index 4a888ab..4950897 100644
> > --- a/target-arm/cpu.c
> > +++ b/target-arm/cpu.c
> > @@ -783,6 +783,21 @@ static void cortex_m3_initfn(Object *obj)
> >      cpu->midr = 0x410fc231;
> >  }
> >
> > +static void cortex_m4_initfn(Object *obj)
> > +{
> > +    ARMCPU *cpu = ARM_CPU(obj);
>
> You want a blank line here. Compare the other CPU initfns.
>
> > +    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.*/
>
> This whole comment is unnecessary -- the MIDR is the standard
> format for all ARM CPUs, and no other initfn spells out the
> various fields of it. You can say
>     cpu->midr = 0x410fc240; /* r0p0 */
> if you like.
>
> > +}
> >  static void arm_v7m_class_init(ObjectClass *oc, void *data)
> >  {
> >      CPUClass *cc = CPU_CLASS(oc);
> > @@ -1185,6 +1200,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 },
>
> If you can make those tweaks and resend I think we can
> get this into QEMU for 2.4.
>
> thanks
> -- PMM
>

[-- Attachment #2: Type: text/html, Size: 4271 bytes --]

Re: [Qemu-devel] [PATCH V3] Target-arm: Add the Cortex-M4 CPU

[Liviu Ionescu]

> On 16 Jun 2015, at 14:21, Peter Maydell <peter.maydell@linaro.org> wrote:
> 
> 
> ... I think we can
> get this into QEMU for 2.4.

that would be really great, I have quite a lot of M4 MCUs in my Cortex-M branch.


Liviu

Re: [Qemu-devel] [PATCH V3] Target-arm: Add the Cortex-M4 CPU

[aurelio remonda]

about the space between ARMCPU *cpu = ARM_CPU(obj); line and the first
set_feature, the cortex-m3 initfn does not have it, do you want me to
change that one too? Thanks

2015-06-16 9:29 GMT-03:00 Liviu Ionescu <ilg@livius.net>:
>
>> On 16 Jun 2015, at 14:21, Peter Maydell <peter.maydell@linaro.org> wrote:
>>
>>
>> ... I think we can
>> get this into QEMU for 2.4.
>
> that would be really great, I have quite a lot of M4 MCUs in my Cortex-M branch.
>
>
> Liviu
>

Re: [Qemu-devel] [PATCH V3] Target-arm: Add the Cortex-M4 CPU

[Peter Maydell]

On 16 June 2015 at 13:52, aurelio remonda <aurelioremonda@gmail.com> wrote:
> about the space between ARMCPU *cpu = ARM_CPU(obj); line and the first
> set_feature, the cortex-m3 initfn does not have it, do you want me to
> change that one too? Thanks

I would leave that one alone. It's bad style, but fixing
it should be a separate patch, and it isn't sufficiently
bad to be worth fixing IMHO.

-- PMM