Re: [Qemu-devel] [PATCH 2/2] target-arm: Add "anyvfp" CPU

[Peter Crosthwaite]

On Mon, Jul 6, 2015 at 11:53 AM,  <address@hidden> wrote:
> From: Meador Inge <address@hidden>
>
> This patch adds support for `-cpu anyvfp`, which is just
> like `-cpu any`, but enables the coprocessors by default.

So a better way to do this is via QOM properties. You can propertyify
VFP support on the QOM type ARMCPU then users can use -global to set
in on the command line. You could do this for any number of ARM CPU
features you care about to create the combos you want on the command
line rather than having an in tree CPU def for special cases.

Have a look at the way the has_el3 feature is handled as a CPU property.

> This is useful for working with a wide range of bare-metal
> C/C++ applications that have been compiled in different
> ways and assume the coprocessor has been enabled already.
>
> Original patch by Daniel Jacobowitz.

Do you have originals in a git with the author SOBs that you can just
amend with your own editor notes?

>
> Signed-off-by: Meador Inge <address@hidden>
> ---
>  target-arm/cpu.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index 3665f6b..2a00e73 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -181,7 +181,14 @@ static void arm_cpu_reset(CPUState *s)
>          env->regs[15] = 0xFFFF0000;
>      }
>
> -    env->vfp.xregs[ARM_VFP_FPEXC] = 0;
> +    /* For -cpu anyvfp, enable coprocessors by default.  Useful for
> +       testing code that expects something else to turn on the
> +       coprocessor.  */

Comment style should be this:

/* multi line comment style
 * needs *'s on each line.
 */

> +    if (cpu->midr == 0xfffffffe) {
> +        env->vfp.xregs[ARM_VFP_FPEXC] = 1 << 30;
> +    } else {
> +        env->vfp.xregs[ARM_VFP_FPEXC] = 0;
> +    }
>  #endif
>      set_flush_to_zero(1, &env->vfp.standard_fp_status);
>      set_flush_inputs_to_zero(1, &env->vfp.standard_fp_status);
> @@ -1258,6 +1265,13 @@ static void arm_any_initfn(Object *obj)
>      cpu->midr = 0xffffffff;
>  }
>
> +static void arm_anyvfp_initfn(Object *obj)
> +{
> +    ARMCPU *cpu = ARM_CPU(obj);
> +    arm_any_initfn(obj);
> +    cpu->midr = 0xfffffffe;


Do you need to set the MIDR like this or is it just a means to
communicate feature support?

Regards,
Peter

> +}
> +
>  #endif /* !defined(CONFIG_USER_ONLY) || !defined(TARGET_AARCH64) */
>
>  typedef struct ARMCPUInfo {
> @@ -1304,6 +1318,7 @@ static const ARMCPUInfo arm_cpus[] = {
>      { .name = "pxa270-c0",   .initfn = pxa270c0_initfn },
>      { .name = "pxa270-c5",   .initfn = pxa270c5_initfn },
>      { .name = "any",         .initfn = arm_any_initfn },
> +    { .name = "anyvfp",      .initfn = arm_anyvfp_initfn },
>  #endif
>      { .name = NULL }
>  };
> --
> 1.8.1.1
>
>