Re: [Qemu-devel] [RFC 1/2] ARM: cpu: add "hivecs" property (high vectors on reset)

[Peter Crosthwaite]

On Sat, Dec 7, 2013 at 10:55 AM, Antony Pavlov <address@hidden> wrote:
> Signed-off-by: Antony Pavlov <address@hidden>
> ---
>  target-arm/cpu-qom.h |  1 +
>  target-arm/cpu.c     | 13 +++++++++++++
>  2 files changed, 14 insertions(+)
>
> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
> index b55306a..116320c 100644
> --- a/target-arm/cpu-qom.h
> +++ b/target-arm/cpu-qom.h
> @@ -128,6 +128,7 @@ typedef struct ARMCPU {
>      uint32_t ccsidr[16];
>      uint32_t reset_cbar;
>      uint32_t reset_auxcr;
> +    bool reset_hivecs;
>  } ARMCPU;
>
>  #define TYPE_AARCH64_CPU "aarch64-cpu"
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index d40f2a7..f838499 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -20,6 +20,7 @@
>
>  #include "cpu.h"
>  #include "qemu-common.h"
> +#include "hw/qdev-properties.h"
>  #if !defined(CONFIG_USER_ONLY)
>  #include "hw/loader.h"
>  #endif
> @@ -119,6 +120,12 @@ static void arm_cpu_reset(CPUState *s)
>              env->regs[15] = pc & ~1;
>          }
>      }
> +
> +    if (cpu->reset_hivecs) {
> +            uint32_t c1_sys = env->cp15.c1_sys;
> +            env->cp15.c1_sys = c1_sys | (1 << 13);

Why the read modify write? Cant you do it as a one-liner |= ?

env->cp15.c1_sys |= (1 << 13);

> +    }
> +
>      env->vfp.xregs[ARM_VFP_FPEXC] = 0;
>  #endif
>      set_flush_to_zero(1, &env->vfp.standard_fp_status);
> @@ -854,6 +861,11 @@ typedef struct ARMCPUInfo {
>      void (*class_init)(ObjectClass *oc, void *data);
>  } ARMCPUInfo;
>
> +static Property arm_cpu_properties[] = {
> +    DEFINE_PROP_BOOL("hivecs", ARMCPU, reset_hivecs, false),

As this is indicating a reset value, "reset" should feature in the
property name.

> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
>  static const ARMCPUInfo arm_cpus[] = {
>  #if !defined(CONFIG_USER_ONLY) || !defined(TARGET_AARCH64)
>      { .name = "arm926",      .initfn = arm926_initfn },
> @@ -902,6 +914,7 @@ static void arm_cpu_class_init(ObjectClass *oc, void 
> *data)
>
>      acc->parent_realize = dc->realize;
>      dc->realize = arm_cpu_realizefn;
> +    dc->props = arm_cpu_properties;

This unconditionally adds the property to all ARM CPUs, wheras its
only valid for a subset of ARM CPUs (!M). Check the earlier patches in
the CBAR/highbank/zynq series on list for the change pattern for
adding it as a conditional dynamic property instead. I'll take it into
that series if you want to rebase onto my patch set (will save you
having to conflict by adding post-init fn which is still absent from
mainline).

Regards,
Peter

>
>      acc->parent_reset = cc->reset;
>      cc->reset = arm_cpu_reset;
> --
> 1.8.5
>
>