[Qemu-devel] [PATCH arm-midr v2 0/2] Make ARM-MIDR a property and use in Zynq

[Qemu-devel] [PATCH arm-midr v2 0/2] Make ARM-MIDR a property and use in Zynq

[Alistair Francis]

This series converts cpu->midr (the MIDR register) to a property.
This allows it to be set after init which is useful for specific
boards (such as Zynq). The change has been done in such a way that
it doesn't break compatibility with boards that don't need a
custom MIDR.

V2: Use dc->props to avoid using qdev_*


Alistair Francis (2):
  ARM: Convert MIDR to a property
  ZYNQ: Implement board MIDR control for Zynq

 hw/arm/xilinx_zynq.c |    7 +++++++
 target-arm/cpu.c     |   13 +++++++++++++
 2 files changed, 20 insertions(+), 0 deletions(-)

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

[Alistair Francis]

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 <alistair.francis@xilinx.com>
---
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);
+    }
 }
 
 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

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
<alistair.francis@xilinx.com> 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 <alistair.francis@xilinx.com>
> ---
> 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
>
>

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

[Peter Maydell]

On 19 January 2014 00:59, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> 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?

IIRC ARMv4 and earlier didn't define the MIDR, but we don't
actually emulate any of those. In general, my intent with all these
constant fields in the ARMCPU struct was that we'd end up making
them just properties available on all ARMCPU objects, and if the
particular subtype of ARMCPU happened not to have FP and so
didn't need the reset_fpsid, for example, it would just ignore whatever
value you set the property to. I don't think we need to tie ourselves
in knots to restrict the properties to particular CPUs if it is too
implementationally awkward (though it would be nice if we can
tie them to ARM_FEATURE_* bits for more or less free).

thanks
-- PMM

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

[Peter Crosthwaite]

On Sun, Jan 19, 2014 at 11:12 AM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> On 19 January 2014 00:59, Peter Crosthwaite
> <peter.crosthwaite@xilinx.com> wrote:
>> 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?
>
> IIRC ARMv4 and earlier didn't define the MIDR, but we don't
> actually emulate any of those. In general, my intent with all these
> constant fields in the ARMCPU struct was that we'd end up making
> them just properties available on all ARMCPU objects, and if the
> particular subtype of ARMCPU happened not to have FP and so
> didn't need the reset_fpsid, for example, it would just ignore whatever
> value you set the property to. I don't think we need to tie ourselves
> in knots to restrict the properties to particular CPUs if it is too
> implementationally awkward (though it would be nice if we can
> tie them to ARM_FEATURE_* bits for more or less free).
>

ARM_FEATURE_V5 exists. Will that do the job? That will exclude just
the V4T AFAICT. Worth bothering?

Regards,
Peter

> thanks
> -- PMM
>

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

[Peter Maydell]

On 19 January 2014 01:46, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> On Sun, Jan 19, 2014 at 11:12 AM, Peter Maydell
> <peter.maydell@linaro.org> wrote:
>> IIRC ARMv4 and earlier didn't define the MIDR, but we don't
>> actually emulate any of those. In general, my intent with all these
>> constant fields in the ARMCPU struct was that we'd end up making
>> them just properties available on all ARMCPU objects, and if the
>> particular subtype of ARMCPU happened not to have FP and so
>> didn't need the reset_fpsid, for example, it would just ignore whatever
>> value you set the property to. I don't think we need to tie ourselves
>> in knots to restrict the properties to particular CPUs if it is too
>> implementationally awkward (though it would be nice if we can
>> tie them to ARM_FEATURE_* bits for more or less free).
>>
>
> ARM_FEATURE_V5 exists. Will that do the job? That will exclude just
> the V4T AFAICT. Worth bothering?

Probably not for MIDR, I think, though maybe for some of the
others we might like to.

thanks
-- PMM

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

[Alistair Francis]

On Sun, Jan 19, 2014 at 9:21 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 19 January 2014 01:46, Peter Crosthwaite
> <peter.crosthwaite@xilinx.com> wrote:
>> On Sun, Jan 19, 2014 at 11:12 AM, Peter Maydell
>> <peter.maydell@linaro.org> wrote:
>>> IIRC ARMv4 and earlier didn't define the MIDR, but we don't
>>> actually emulate any of those. In general, my intent with all these
>>> constant fields in the ARMCPU struct was that we'd end up making
>>> them just properties available on all ARMCPU objects, and if the
>>> particular subtype of ARMCPU happened not to have FP and so
>>> didn't need the reset_fpsid, for example, it would just ignore whatever
>>> value you set the property to. I don't think we need to tie ourselves
>>> in knots to restrict the properties to particular CPUs if it is too
>>> implementationally awkward (though it would be nice if we can
>>> tie them to ARM_FEATURE_* bits for more or less free).
>>>
>>
>> ARM_FEATURE_V5 exists. Will that do the job? That will exclude just
>> the V4T AFAICT. Worth bothering?
>
> Probably not for MIDR, I think, though maybe for some of the
> others we might like to.

I have removed the object_property_set_int() function as it is no longer
required.

I also didn't bother restricting the MIDR to ARMv5

>
> thanks
> -- PMM
>

[Qemu-devel] [PATCH arm-midr v2 2/2] ZYNQ: Implement board MIDR control for Zynq

[Alistair Francis]

This patch uses the fact that the midr variable is now a property
This patch sets the midr variable to the boards custom midr

Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
---

 hw/arm/xilinx_zynq.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 98e0958..9ee21e7 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -37,6 +37,7 @@
 #define IRQ_OFFSET 32 /* pic interrupts start from index 32 */
 
 #define MPCORE_PERIPHBASE 0xF8F00000
+#define ZYNQ_BOARD_MIDR 0x413FC090
 
 static const int dma_irqs[8] = {
     46, 47, 48, 49, 72, 73, 74, 75
@@ -125,6 +126,12 @@ static void zynq_init(QEMUMachineInitArgs *args)
 
     cpu = ARM_CPU(object_new(object_class_get_name(cpu_oc)));
 
+    object_property_set_int(OBJECT(cpu), ZYNQ_BOARD_MIDR, "midr", &err);
+    if (err) {
+        error_report("%s", error_get_pretty(err));
+        exit(1);
+    }
+
     object_property_set_int(OBJECT(cpu), MPCORE_PERIPHBASE, "reset-cbar", &err);
     if (err) {
         error_report("%s", error_get_pretty(err));
-- 
1.7.1