Re: [Qemu-devel] [PATCH arm-midr v2 1/2] ARM: Convert MIDR to a property

[Peter Crosthwaite]

On Tue, Jan 14, 2014 at 9:48 AM, Alistair Francis
<address@hidden> wrote:
> Convert the MIDR register to a property. This allows boards to later set
> a custom MIDR value. This has been done in such a way to maintain
> compatibility with all existing CPUs and boards
>
> Signed-off-by: Alistair Francis <address@hidden>
> ---
> I originally added the properties to the cpu->midr variable in a similar
> method to how Peter Crosthwaite did in his 'Fix Support for ARM CBAR and
> reset-hivecs' series.
> V2: Use dc->props to avoid using qdev_*
>
>  target-arm/cpu.c |   13 +++++++++++++
>  1 files changed, 13 insertions(+), 0 deletions(-)
>
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index 408d207..52f7f06 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -253,6 +253,7 @@ static void arm_cpu_post_init(Object *obj)
>  {
>      ARMCPU *cpu = ARM_CPU(obj);
>      Error *err = NULL;
> +    uint32_t temp = cpu->midr;
>
>      if (arm_feature(&cpu->env, ARM_FEATURE_CBAR)) {
>          qdev_property_add_static(DEVICE(obj), &arm_cpu_reset_cbar_property,
> @@ -265,6 +266,17 @@ static void arm_cpu_post_init(Object *obj)
>                                   &err);
>          assert_no_error(err);
>      }
> +
> +    /*
> +     * Initialise the midr property and set it to the original CPU MIDR
> +     * This is used to maintain compatibility with boards that don't set
> +     * a custom MIDR
> +     */
> +    object_property_set_int(OBJECT(cpu), temp, "midr", &err);
> +    if (err) {
> +        error_report("%s", error_get_pretty(err));
> +        exit(1);
> +    }

Do you even need this now? The normal arrayified dc->props properties
are added at device::init time. As TYPE_DEVICE is a parent class, its
init function is called before CPUs (normal inits are called in
parent->child order, the post_inits are reverse). This means the
cpu::init fns should correctly set the device-specific default after
device::init, trampling the bogus default defined in the property
array.

All of this however assumes that MIDR is unconditionally existent for
all ARMCPU. Peter, are you able to confirm that thats ok before we
commit to this implementation over the conditional post_init approach?

Regards,
Peter

>  }
>
>  static void arm_cpu_finalizefn(Object *obj)
> @@ -984,6 +996,7 @@ static const ARMCPUInfo arm_cpus[] = {
>
>  static Property arm_cpu_properties[] = {
>      DEFINE_PROP_BOOL("start-powered-off", ARMCPU, start_powered_off, false),
> +    DEFINE_PROP_UINT32("midr", ARMCPU, midr, 0),
>      DEFINE_PROP_END_OF_LIST()
>  };
>
> --
> 1.7.1
>
>