Patchwork [v1,6/6] arm: a9mpcore: Coreify the SCU

login
register
mail settings
Submitter Peter Crosthwaite
Date Feb. 8, 2013, 4:03 a.m.
Message ID <908d8dfd-22e1-4c9a-ad42-0e44e78832b6@VA3EHSMHS016.ehs.local>
Download mbox | patch
Permalink /patch/219046/
State New
Headers show

Comments

Peter Crosthwaite - Feb. 8, 2013, 4:03 a.m.
Split the SCU in a9mpcore out into its own object definition. mpcore is now
just a container for the mpcore components.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 hw/a9mpcore.c        |  119 +++----------------------------------
 hw/a9scu.c           |  162 ++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/arm/Makefile.objs |    1 +
 3 files changed, 171 insertions(+), 111 deletions(-)
 create mode 100644 hw/a9scu.c
Peter Maydell - Feb. 18, 2013, 6:49 p.m.
On 8 February 2013 04:03, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> Split the SCU in a9mpcore out into its own object definition. mpcore is now
> just a container for the mpcore components.

Good idea.

> --- a/hw/a9mpcore.c
> +++ b/hw/a9mpcore.c
> @@ -14,107 +14,12 @@
>
>  typedef struct A9MPPrivState {
>      SysBusDevice busdev;
> -    uint32_t scu_control;
> -    uint32_t scu_status;
>      uint32_t num_cpu;
> -    MemoryRegion scu_iomem;
>      MemoryRegion container;
>      DeviceState *gic;
>      uint32_t num_irq;
>  } A9MPPrivState;

You need to add a DeviceState* for the scu.

> diff --git a/hw/a9scu.c b/hw/a9scu.c
> new file mode 100644
> index 0000000..0a3d411
> --- /dev/null
> +++ b/hw/a9scu.c
> @@ -0,0 +1,162 @@
> +/*
> + * Cortex-A9MPCore Snoop Control Unit (SCU) emulation.
> + *
> + * Copyright (c) 2009 CodeSourcery.
> + * Copyright (c) 2011 Linaro Limited.
> + * Written by Paul Brook, Peter Maydell.
> + *
> + * This code is licensed under the GPL.
> + */
> +
> +#include "sysbus.h"
> +
> +/* A9MP private memory region.  */

Stale comment (you could just delete it).

> +
> +typedef struct A9SCUState {
> +    SysBusDevice busdev;
> +    MemoryRegion iomem;
> +    uint32_t control;
> +    uint32_t status;
> +    uint32_t num_cpu;
> +} A9SCUState;

> +static const VMStateDescription vmstate_a9_scu = {
> +    .name = "a9_scu",

For new devices, hyphen is preferred, so "a9-scu".

> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT32(control, A9SCUState),
> +        VMSTATE_UINT32(status, A9SCUState),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static Property a9_scu_properties[] = {
> +    DEFINE_PROP_UINT32("num-cpu", A9SCUState, num_cpu, 1),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void a9_scu_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
> +
> +    k->init = a9_scu_init;

This should have an instance_init and/or realize method,
not a SysBusDeviceClass::init (see comments on PL330 patch).

> +    dc->props = a9_scu_properties;
> +    dc->vmsd = &vmstate_a9_scu;
> +    dc->reset = a9_scu_reset;
> +}
> +
> +static TypeInfo a9_scu_info = {
> +    .name          = "arm_a9_scu",

Again, hyphens preferred.

> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(A9SCUState),
> +    .class_init    = a9_scu_class_init,
> +};

thanks
-- PMM
Andreas Färber - Feb. 18, 2013, 8:19 p.m.
Am 18.02.2013 19:49, schrieb Peter Maydell:
> On 8 February 2013 04:03, Peter Crosthwaite
> <peter.crosthwaite@xilinx.com> wrote:
>> Split the SCU in a9mpcore out into its own object definition. mpcore is now
>> just a container for the mpcore components.
> 
> Good idea.
> 
>> --- a/hw/a9mpcore.c
>> +++ b/hw/a9mpcore.c
>> @@ -14,107 +14,12 @@
>>
>>  typedef struct A9MPPrivState {
>>      SysBusDevice busdev;
>> -    uint32_t scu_control;
>> -    uint32_t scu_status;
>>      uint32_t num_cpu;
>> -    MemoryRegion scu_iomem;
>>      MemoryRegion container;
>>      DeviceState *gic;
>>      uint32_t num_irq;
>>  } A9MPPrivState;
> 
> You need to add a DeviceState* for the scu.

No, not a DeviceState*, an A9SCUState. With object_initialize() and
qdev_set_parent_bus(NULL) instead of qdev_create() to be exact and some
child<A9SCUState> property for ownership transfer. 2/7 and commit
message say why.

>> diff --git a/hw/a9scu.c b/hw/a9scu.c
>> new file mode 100644
>> index 0000000..0a3d411
>> --- /dev/null
>> +++ b/hw/a9scu.c
[...]
>> +static void a9_scu_class_init(ObjectClass *klass, void *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
>> +
>> +    k->init = a9_scu_init;
> 
> This should have an instance_init and/or realize method,
> not a SysBusDeviceClass::init (see comments on PL330 patch).
> 
>> +    dc->props = a9_scu_properties;
>> +    dc->vmsd = &vmstate_a9_scu;
>> +    dc->reset = a9_scu_reset;
>> +}
>> +
>> +static TypeInfo a9_scu_info = {

static const

Regards,
Andreas
Peter Crosthwaite - Feb. 19, 2013, 11:54 p.m.
Hi Andreas,

On Tue, Feb 19, 2013 at 6:19 AM, Andreas Färber <afaerber@suse.de> wrote:
> Am 18.02.2013 19:49, schrieb Peter Maydell:
>> On 8 February 2013 04:03, Peter Crosthwaite
>> <peter.crosthwaite@xilinx.com> wrote:
>>> Split the SCU in a9mpcore out into its own object definition. mpcore is now
>>> just a container for the mpcore components.
>>
>> Good idea.
>>
>>> --- a/hw/a9mpcore.c
>>> +++ b/hw/a9mpcore.c
>>> @@ -14,107 +14,12 @@
>>>
>>>  typedef struct A9MPPrivState {
>>>      SysBusDevice busdev;
>>> -    uint32_t scu_control;
>>> -    uint32_t scu_status;
>>>      uint32_t num_cpu;
>>> -    MemoryRegion scu_iomem;
>>>      MemoryRegion container;
>>>      DeviceState *gic;
>>>      uint32_t num_irq;
>>>  } A9MPPrivState;
>>
>> You need to add a DeviceState* for the scu.
>
> No, not a DeviceState*, an A9SCUState. With object_initialize() and
> qdev_set_parent_bus(NULL) instead of qdev_create() to be exact and some
> child<A9SCUState> property for ownership transfer. 2/7 and commit
> message say why.
>

Hi Andreas, what you are proposing is a major change pattern to pretty
much the entire tree - every device that is part of a container object
needs to inlined (thus creating public headers). Despite the fact that
I disagree with that approach, this change is way out of scope of this
series and changing just the SCU to be like this will make it
inconsistent with its peer devices GIC and MPTimer. This is cut and
paste re-organisation of existing code that is groundwork for what you
are talking about and the patch still stands in its own right in that
this scheme is better than what we have today.

So I'd like to take a crawl before we walk approach to this patch. For
the next revision i'm going to do it Peters way and ask that we sort
out the big questions about QOM containers and inline-structs for
MPCore in another patch series. Then we can fix GIC and MPTimer at the
same time and everything is consistent. Too often when contributors
submit patches some minor issue get tangled in large out of scope
discussions about QOM that relate to the entire tree. The whole series
then ends up bitrotting on list or living out of tree forever.

Regards,
Peter

>>> diff --git a/hw/a9scu.c b/hw/a9scu.c
>>> new file mode 100644
>>> index 0000000..0a3d411
>>> --- /dev/null
>>> +++ b/hw/a9scu.c
> [...]
>>> +static void a9_scu_class_init(ObjectClass *klass, void *data)
>>> +{
>>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>>> +    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
>>> +
>>> +    k->init = a9_scu_init;
>>
>> This should have an instance_init and/or realize method,
>> not a SysBusDeviceClass::init (see comments on PL330 patch).
>>
>>> +    dc->props = a9_scu_properties;
>>> +    dc->vmsd = &vmstate_a9_scu;
>>> +    dc->reset = a9_scu_reset;
>>> +}
>>> +
>>> +static TypeInfo a9_scu_info = {
>
> static const
>

Fixed

Regards,
Peter

> Regards,
> Andreas
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>
Peter Maydell - Feb. 20, 2013, 12:03 a.m.
On 19 February 2013 23:54, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> On Tue, Feb 19, 2013 at 6:19 AM, Andreas Färber <afaerber@suse.de> wrote:
>> Am 18.02.2013 19:49, schrieb Peter Maydell:
>>> You need to add a DeviceState* for the scu.
>>
>> No, not a DeviceState*, an A9SCUState. With object_initialize() and
>> qdev_set_parent_bus(NULL) instead of qdev_create() to be exact and some
>> child<A9SCUState> property for ownership transfer. 2/7 and commit
>> message say why.
>
> Hi Andreas, what you are proposing is a major change pattern to pretty
> much the entire tree - every device that is part of a container object
> needs to inlined (thus creating public headers). Despite the fact that
> I disagree with that approach, this change is way out of scope of this
> series and changing just the SCU to be like this will make it
> inconsistent with its peer devices GIC and MPTimer. This is cut and
> paste re-organisation of existing code that is groundwork for what you
> are talking about and the patch still stands in its own right in that
> this scheme is better than what we have today.
>
> So I'd like to take a crawl before we walk approach to this patch. For
> the next revision i'm going to do it Peters way and ask that we sort
> out the big questions about QOM containers and inline-structs for
> MPCore in another patch series. Then we can fix GIC and MPTimer at the
> same time and everything is consistent. Too often when contributors
> submit patches some minor issue get tangled in large out of scope
> discussions about QOM that relate to the entire tree. The whole series
> then ends up bitrotting on list or living out of tree forever.

I agree with all of this. I would like us to actually thrash out
the "how do we do child devices in the QOM way" and write up
the answers (rather than repeating the same skirmishes in every
patch review thread). But we shouldn't derail every patch series
until we have figured that out...

For the moment this patch series is a clear improvement and
(once the minor review issues are fixed) I'm happy to commit
it to arm-devs.next.

-- PMM
Peter Crosthwaite - Feb. 20, 2013, 12:53 a.m.
On Tue, Feb 19, 2013 at 4:49 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 8 February 2013 04:03, Peter Crosthwaite
> <peter.crosthwaite@xilinx.com> wrote:
>> Split the SCU in a9mpcore out into its own object definition. mpcore is now
>> just a container for the mpcore components.
>
> Good idea.
>
>> --- a/hw/a9mpcore.c
>> +++ b/hw/a9mpcore.c
>> @@ -14,107 +14,12 @@
>>
>>  typedef struct A9MPPrivState {
>>      SysBusDevice busdev;
>> -    uint32_t scu_control;
>> -    uint32_t scu_status;
>>      uint32_t num_cpu;
>> -    MemoryRegion scu_iomem;
>>      MemoryRegion container;
>>      DeviceState *gic;
>>      uint32_t num_irq;
>>  } A9MPPrivState;
>
> You need to add a DeviceState* for the scu.
>

Done

>> diff --git a/hw/a9scu.c b/hw/a9scu.c
>> new file mode 100644
>> index 0000000..0a3d411
>> --- /dev/null
>> +++ b/hw/a9scu.c
>> @@ -0,0 +1,162 @@
>> +/*
>> + * Cortex-A9MPCore Snoop Control Unit (SCU) emulation.
>> + *
>> + * Copyright (c) 2009 CodeSourcery.
>> + * Copyright (c) 2011 Linaro Limited.
>> + * Written by Paul Brook, Peter Maydell.
>> + *
>> + * This code is licensed under the GPL.
>> + */
>> +
>> +#include "sysbus.h"
>> +
>> +/* A9MP private memory region.  */
>
> Stale comment (you could just delete it).
>

Done

>> +
>> +typedef struct A9SCUState {
>> +    SysBusDevice busdev;
>> +    MemoryRegion iomem;
>> +    uint32_t control;
>> +    uint32_t status;
>> +    uint32_t num_cpu;
>> +} A9SCUState;
>
>> +static const VMStateDescription vmstate_a9_scu = {
>> +    .name = "a9_scu",
>
> For new devices, hyphen is preferred, so "a9-scu".
>

Fixed globally

>> +    .version_id = 1,
>> +    .minimum_version_id = 1,
>> +    .fields = (VMStateField[]) {
>> +        VMSTATE_UINT32(control, A9SCUState),
>> +        VMSTATE_UINT32(status, A9SCUState),
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
>> +
>> +static Property a9_scu_properties[] = {
>> +    DEFINE_PROP_UINT32("num-cpu", A9SCUState, num_cpu, 1),
>> +    DEFINE_PROP_END_OF_LIST(),
>> +};
>> +
>> +static void a9_scu_class_init(ObjectClass *klass, void *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
>> +
>> +    k->init = a9_scu_init;
>
> This should have an instance_init and/or realize method,
> not a SysBusDeviceClass::init (see comments on PL330 patch).
>

Fixed as per PL330 review.

Regards,
Peter

>> +    dc->props = a9_scu_properties;
>> +    dc->vmsd = &vmstate_a9_scu;
>> +    dc->reset = a9_scu_reset;
>> +}
>> +
>> +static TypeInfo a9_scu_info = {
>> +    .name          = "arm_a9_scu",
>
> Again, hyphens preferred.
>
>> +    .parent        = TYPE_SYS_BUS_DEVICE,
>> +    .instance_size = sizeof(A9SCUState),
>> +    .class_init    = a9_scu_class_init,
>> +};
>
> thanks
> -- PMM
>

Patch

diff --git a/hw/a9mpcore.c b/hw/a9mpcore.c
index 4523074..9aaad0d 100644
--- a/hw/a9mpcore.c
+++ b/hw/a9mpcore.c
@@ -14,107 +14,12 @@ 
 
 typedef struct A9MPPrivState {
     SysBusDevice busdev;
-    uint32_t scu_control;
-    uint32_t scu_status;
     uint32_t num_cpu;
-    MemoryRegion scu_iomem;
     MemoryRegion container;
     DeviceState *gic;
     uint32_t num_irq;
 } A9MPPrivState;
 
-static uint64_t a9_scu_read(void *opaque, hwaddr offset,
-                            unsigned size)
-{
-    A9MPPrivState *s = (A9MPPrivState *)opaque;
-    switch (offset) {
-    case 0x00: /* Control */
-        return s->scu_control;
-    case 0x04: /* Configuration */
-        return (((1 << s->num_cpu) - 1) << 4) | (s->num_cpu - 1);
-    case 0x08: /* CPU Power Status */
-        return s->scu_status;
-    case 0x09: /* CPU status.  */
-        return s->scu_status >> 8;
-    case 0x0a: /* CPU status.  */
-        return s->scu_status >> 16;
-    case 0x0b: /* CPU status.  */
-        return s->scu_status >> 24;
-    case 0x0c: /* Invalidate All Registers In Secure State */
-        return 0;
-    case 0x40: /* Filtering Start Address Register */
-    case 0x44: /* Filtering End Address Register */
-        /* RAZ/WI, like an implementation with only one AXI master */
-        return 0;
-    case 0x50: /* SCU Access Control Register */
-    case 0x54: /* SCU Non-secure Access Control Register */
-        /* unimplemented, fall through */
-    default:
-        return 0;
-    }
-}
-
-static void a9_scu_write(void *opaque, hwaddr offset,
-                         uint64_t value, unsigned size)
-{
-    A9MPPrivState *s = (A9MPPrivState *)opaque;
-    uint32_t mask;
-    uint32_t shift;
-    switch (size) {
-    case 1:
-        mask = 0xff;
-        break;
-    case 2:
-        mask = 0xffff;
-        break;
-    case 4:
-        mask = 0xffffffff;
-        break;
-    default:
-        fprintf(stderr, "Invalid size %u in write to a9 scu register %x\n",
-                size, (unsigned)offset);
-        return;
-    }
-
-    switch (offset) {
-    case 0x00: /* Control */
-        s->scu_control = value & 1;
-        break;
-    case 0x4: /* Configuration: RO */
-        break;
-    case 0x08: case 0x09: case 0x0A: case 0x0B: /* Power Control */
-        shift = (offset - 0x8) * 8;
-        s->scu_status &= ~(mask << shift);
-        s->scu_status |= ((value & mask) << shift);
-        break;
-    case 0x0c: /* Invalidate All Registers In Secure State */
-        /* no-op as we do not implement caches */
-        break;
-    case 0x40: /* Filtering Start Address Register */
-    case 0x44: /* Filtering End Address Register */
-        /* RAZ/WI, like an implementation with only one AXI master */
-        break;
-    case 0x50: /* SCU Access Control Register */
-    case 0x54: /* SCU Non-secure Access Control Register */
-        /* unimplemented, fall through */
-    default:
-        break;
-    }
-}
-
-static const MemoryRegionOps a9_scu_ops = {
-    .read = a9_scu_read,
-    .write = a9_scu_write,
-    .endianness = DEVICE_NATIVE_ENDIAN,
-};
-
-static void a9mp_priv_reset(DeviceState *dev)
-{
-    A9MPPrivState *s = FROM_SYSBUS(A9MPPrivState, SYS_BUS_DEVICE(dev));
-
-    s->scu_control = 0;
-}
-
 static void a9mp_priv_set_irq(void *opaque, int irq, int level)
 {
     A9MPPrivState *s = (A9MPPrivState *)opaque;
@@ -124,7 +29,7 @@  static void a9mp_priv_set_irq(void *opaque, int irq, int level)
 static int a9mp_priv_init(SysBusDevice *dev)
 {
     A9MPPrivState *s = FROM_SYSBUS(A9MPPrivState, dev);
-    SysBusDevice *timerbusdev, *wdtbusdev, *gicbusdev;
+    SysBusDevice *timerbusdev, *wdtbusdev, *gicbusdev, *scubusdev;
     DeviceState *qdev;
     int i;
 
@@ -140,6 +45,11 @@  static int a9mp_priv_init(SysBusDevice *dev)
     /* Pass through inbound GPIO lines to the GIC */
     qdev_init_gpio_in(&s->busdev.qdev, a9mp_priv_set_irq, s->num_irq - 32);
 
+    qdev = qdev_create(NULL, "arm_a9_scu");
+    qdev_prop_set_uint32(qdev, "num-cpu", s->num_cpu);
+    qdev_init_nofail(qdev);
+    scubusdev = SYS_BUS_DEVICE(qdev);
+
     qdev = qdev_create(NULL, "arm_mptimer");
     qdev_prop_set_uint32(qdev, "num-cpu", s->num_cpu);
     qdev_init_nofail(qdev);
@@ -162,8 +72,8 @@  static int a9mp_priv_init(SysBusDevice *dev)
      * We should implement the global timer but don't currently do so.
      */
     memory_region_init(&s->container, "a9mp-priv-container", 0x2000);
-    memory_region_init_io(&s->scu_iomem, &a9_scu_ops, s, "a9mp-scu", 0x100);
-    memory_region_add_subregion(&s->container, 0, &s->scu_iomem);
+    memory_region_add_subregion(&s->container, 0,
+                                sysbus_mmio_get_region(scubusdev, 0));
     /* GIC CPU interface */
     memory_region_add_subregion(&s->container, 0x100,
                                 sysbus_mmio_get_region(gicbusdev, 1));
@@ -192,17 +102,6 @@  static int a9mp_priv_init(SysBusDevice *dev)
     return 0;
 }
 
-static const VMStateDescription vmstate_a9mp_priv = {
-    .name = "a9mpcore_priv",
-    .version_id = 3,
-    .minimum_version_id = 2,
-    .fields = (VMStateField[]) {
-        VMSTATE_UINT32(scu_control, A9MPPrivState),
-        VMSTATE_UINT32_V(scu_status, A9MPPrivState, 2),
-        VMSTATE_END_OF_LIST()
-    }
-};
-
 static Property a9mp_priv_properties[] = {
     DEFINE_PROP_UINT32("num-cpu", A9MPPrivState, num_cpu, 1),
     /* The Cortex-A9MP may have anything from 0 to 224 external interrupt
@@ -222,8 +121,6 @@  static void a9mp_priv_class_init(ObjectClass *klass, void *data)
 
     k->init = a9mp_priv_init;
     dc->props = a9mp_priv_properties;
-    dc->vmsd = &vmstate_a9mp_priv;
-    dc->reset = a9mp_priv_reset;
 }
 
 static const TypeInfo a9mp_priv_info = {
diff --git a/hw/a9scu.c b/hw/a9scu.c
new file mode 100644
index 0000000..0a3d411
--- /dev/null
+++ b/hw/a9scu.c
@@ -0,0 +1,162 @@ 
+/*
+ * Cortex-A9MPCore Snoop Control Unit (SCU) emulation.
+ *
+ * Copyright (c) 2009 CodeSourcery.
+ * Copyright (c) 2011 Linaro Limited.
+ * Written by Paul Brook, Peter Maydell.
+ *
+ * This code is licensed under the GPL.
+ */
+
+#include "sysbus.h"
+
+/* A9MP private memory region.  */
+
+typedef struct A9SCUState {
+    SysBusDevice busdev;
+    MemoryRegion iomem;
+    uint32_t control;
+    uint32_t status;
+    uint32_t num_cpu;
+} A9SCUState;
+
+static uint64_t a9_scu_read(void *opaque, hwaddr offset,
+                            unsigned size)
+{
+    A9SCUState *s = (A9SCUState *)opaque;
+    switch (offset) {
+    case 0x00: /* Control */
+        return s->control;
+    case 0x04: /* Configuration */
+        return (((1 << s->num_cpu) - 1) << 4) | (s->num_cpu - 1);
+    case 0x08: /* CPU Power Status */
+        return s->status;
+    case 0x09: /* CPU status.  */
+        return s->status >> 8;
+    case 0x0a: /* CPU status.  */
+        return s->status >> 16;
+    case 0x0b: /* CPU status.  */
+        return s->status >> 24;
+    case 0x0c: /* Invalidate All Registers In Secure State */
+        return 0;
+    case 0x40: /* Filtering Start Address Register */
+    case 0x44: /* Filtering End Address Register */
+        /* RAZ/WI, like an implementation with only one AXI master */
+        return 0;
+    case 0x50: /* SCU Access Control Register */
+    case 0x54: /* SCU Non-secure Access Control Register */
+        /* unimplemented, fall through */
+    default:
+        return 0;
+    }
+}
+
+static void a9_scu_write(void *opaque, hwaddr offset,
+                         uint64_t value, unsigned size)
+{
+    A9SCUState *s = (A9SCUState *)opaque;
+    uint32_t mask;
+    uint32_t shift;
+    switch (size) {
+    case 1:
+        mask = 0xff;
+        break;
+    case 2:
+        mask = 0xffff;
+        break;
+    case 4:
+        mask = 0xffffffff;
+        break;
+    default:
+        fprintf(stderr, "Invalid size %u in write to a9 scu register %x\n",
+                size, (unsigned)offset);
+        return;
+    }
+
+    switch (offset) {
+    case 0x00: /* Control */
+        s->control = value & 1;
+        break;
+    case 0x4: /* Configuration: RO */
+        break;
+    case 0x08: case 0x09: case 0x0A: case 0x0B: /* Power Control */
+        shift = (offset - 0x8) * 8;
+        s->status &= ~(mask << shift);
+        s->status |= ((value & mask) << shift);
+        break;
+    case 0x0c: /* Invalidate All Registers In Secure State */
+        /* no-op as we do not implement caches */
+        break;
+    case 0x40: /* Filtering Start Address Register */
+    case 0x44: /* Filtering End Address Register */
+        /* RAZ/WI, like an implementation with only one AXI master */
+        break;
+    case 0x50: /* SCU Access Control Register */
+    case 0x54: /* SCU Non-secure Access Control Register */
+        /* unimplemented, fall through */
+    default:
+        break;
+    }
+}
+
+static const MemoryRegionOps a9_scu_ops = {
+    .read = a9_scu_read,
+    .write = a9_scu_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static void a9_scu_reset(DeviceState *dev)
+{
+    A9SCUState *s = FROM_SYSBUS(A9SCUState, SYS_BUS_DEVICE(dev));
+    s->control = 0;
+}
+
+static int a9_scu_init(SysBusDevice *dev)
+{
+    A9SCUState *s = FROM_SYSBUS(A9SCUState, dev);
+
+    memory_region_init_io(&s->iomem, &a9_scu_ops, s, "a9-scu", 0x100);
+    sysbus_init_mmio(dev, &s->iomem);
+    return 0;
+}
+
+static const VMStateDescription vmstate_a9_scu = {
+    .name = "a9_scu",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(control, A9SCUState),
+        VMSTATE_UINT32(status, A9SCUState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static Property a9_scu_properties[] = {
+    DEFINE_PROP_UINT32("num-cpu", A9SCUState, num_cpu, 1),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void a9_scu_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
+
+    k->init = a9_scu_init;
+    dc->props = a9_scu_properties;
+    dc->vmsd = &vmstate_a9_scu;
+    dc->reset = a9_scu_reset;
+}
+
+static TypeInfo a9_scu_info = {
+    .name          = "arm_a9_scu",
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(A9SCUState),
+    .class_init    = a9_scu_class_init,
+};
+
+static void a9mp_register_types(void)
+{
+    type_register_static(&a9_scu_info);
+}
+
+type_init(a9mp_register_types)
diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index 6d049e7..4c10985 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -3,6 +3,7 @@  obj-y += arm_boot.o
 obj-y += xilinx_zynq.o zynq_slcr.o
 obj-y += xilinx_spips.o
 obj-y += arm_gic.o arm_gic_common.o
+obj-y += a9scu.o
 obj-y += realview_gic.o realview.o arm_sysctl.o arm11mpcore.o a9mpcore.o
 obj-y += exynos4210_gic.o exynos4210_combiner.o exynos4210.o
 obj-y += exynos4_boards.o exynos4210_uart.o exynos4210_pwm.o