diff mbox

[1/2] hw/misc: Add a model for the ASPEED System Control Unit

Message ID 1466063287-19350-2-git-send-email-andrew@aj.id.au (mailing list archive)
State New, archived
Headers show

Commit Message

Andrew Jeffery June 16, 2016, 7:48 a.m. UTC
The SCU is a collection of chip-level control registers that manage the
various functions supported by the AST2400. Typically the bits control
interactions with clocks, external hardware or reset behaviour, and we
can largly take a hands-off approach to reads and writes.

Firmware makes heavy use of the state to determine how to boot, but the
reset values vary from SoC to SoC. qdev properties are exposed so that
the integrating SoC model can configure the appropriate reset values.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
 hw/misc/Makefile.objs        |   1 +
 hw/misc/aspeed_scu.c         | 147 +++++++++++++++++++++++++++++++++++++++++++
 include/hw/misc/aspeed_scu.h | 108 +++++++++++++++++++++++++++++++
 trace-events                 |   3 +
 4 files changed, 259 insertions(+)
 create mode 100644 hw/misc/aspeed_scu.c
 create mode 100644 include/hw/misc/aspeed_scu.h

Comments

Peter Maydell June 17, 2016, 2:22 p.m. UTC | #1
On 16 June 2016 at 08:48, Andrew Jeffery <andrew@aj.id.au> wrote:
> The SCU is a collection of chip-level control registers that manage the
> various functions supported by the AST2400. Typically the bits control
> interactions with clocks, external hardware or reset behaviour, and we
> can largly take a hands-off approach to reads and writes.
>
> Firmware makes heavy use of the state to determine how to boot, but the
> reset values vary from SoC to SoC. qdev properties are exposed so that
> the integrating SoC model can configure the appropriate reset values.
>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
> Reviewed-by: Joel Stanley <joel@jms.id.au>
> ---
> +static Property aspeed_scu_properties[] = {
> +    DEFINE_PROP_ARRAY("reset", AspeedSCUState, num_resets, reset,
> +                      qdev_prop_uint32, uint32_t),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +#define ASPEED_SCU_NR_REGS (0x1A8 >> 2)

This seems like a very unwieldy way of specifying the reset values
for this device. Are they really all fully configurable in the
hardware? It seems unlikely. I'd much rather see something that
looks more like what you might plausibly be configuring when wiring
up the SoC, which might be some version/revision numbers and/or
some particular tweakable parameters.

thanks
-- PMM
Andrew Jeffery June 20, 2016, 3:44 a.m. UTC | #2
On Fri, 2016-06-17 at 15:22 +0100, Peter Maydell wrote:
On 16 June 2016 at 08:48, Andrew Jeffery <andrew@aj.id.au> wrote:


The SCU is a collection of chip-level control registers that manage the
various functions supported by the AST2400. Typically the bits control
interactions with clocks, external hardware or reset behaviour, and we
can largly take a hands-off approach to reads and writes.


Firmware makes heavy use of the state to determine how to boot, but the
reset values vary from SoC to SoC. qdev properties are exposed so that
the integrating SoC model can configure the appropriate reset values.


Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
+static Property aspeed_scu_properties[] = {
+    DEFINE_PROP_ARRAY("reset", AspeedSCUState, num_resets, reset,
+                      qdev_prop_uint32, uint32_t),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+#define ASPEED_SCU_NR_REGS (0x1A8 >> 2)
This seems like a very unwieldy way of specifying the reset values
for this device. Are they really all fully configurable in the
hardware? It seems unlikely. I'd much rather see something that
looks more like what you might plausibly be configuring when wiring
up the SoC, which might be some version/revision numbers and/or
some particular tweakable parameters.

Right. I left out some context which may clear things up: We are
working with two SoCs at the moment, the AST2400 and AST2500 (hopefully
the AST2500 patches will be sent to the list soon). I wanted to
abstract the configuration to cater for the differences in register
values between the SoCs, less so for wiring the one SoC up in a
different fashion. For what it's worth, out of 86 registers defined in
the IO space between the two SoCs, 37 take the same value and 49
differ.

I can rework the commit message to say as much.

Separately, the qdev array approach was lifted from your commit
9c7d489379c2 hw/vexpress: Set reset values for daughterboard
oscillators. Another approach would be to set the members of the SCU's
reset array directly as we know the type. That seems less convoluted
but my gut instinct was that wasn't how we should be configuring the
devices. Is qdev the right way to go in the SCU's case?

Cheers,

Andrew
Peter Maydell June 20, 2016, 1:57 p.m. UTC | #3
On 20 June 2016 at 04:44, Andrew Jeffery <andrew@aj.id.au> wrote:
> On Fri, 2016-06-17 at 15:22 +0100, Peter Maydell wrote:
>> +static Property aspeed_scu_properties[] = {
>> +    DEFINE_PROP_ARRAY("reset", AspeedSCUState, num_resets, reset,
>> +                      qdev_prop_uint32, uint32_t),
>> +    DEFINE_PROP_END_OF_LIST(),
>> +};
>> +
>> +#define ASPEED_SCU_NR_REGS (0x1A8 >> 2)
>> This seems like a very unwieldy way of specifying the reset values
>> for this device. Are they really all fully configurable in the
>> hardware? It seems unlikely. I'd much rather see something that
>> looks more like what you might plausibly be configuring when wiring
>> up the SoC, which might be some version/revision numbers and/or
>> some particular tweakable parameters.
>
> Right. I left out some context which may clear things up: We are
> working with two SoCs at the moment, the AST2400 and AST2500 (hopefully
> the AST2500 patches will be sent to the list soon). I wanted to
> abstract the configuration to cater for the differences in register
> values between the SoCs, less so for wiring the one SoC up in a
> different fashion. For what it's worth, out of 86 registers defined in
> the IO space between the two SoCs, 37 take the same value and 49
> differ.

I think there are a couple of plausible ways you might model this:

(a) just have a single property for "revision" which corresponds
to the revision of this bit of silicon IP within the SoC; the
model of the device itself then knows what the reset state is
for this revision of the device.
(b) ditto, but also have some configurable flags where relevant
(ie approximately where it's the same IP rev within the SoC
but it's been configured by tying down different config lines;
for instance hw/dma/pl330.c has a collection of properties
which match the configurable knobs for the hardware.)

You might or might not have enough visibility into the thing to
know which of these is closest to what the real hardware is doing;
if not then it's a matter of taste, looking at what is varying
between the two and what isn't, etc. But "board level specifies
all the register reset values" is definitely far too broad
and generalised an API, I think.

> Separately, the qdev array approach was lifted from your commit
> 9c7d489379c2 hw/vexpress: Set reset values for daughterboard
> oscillators.

You'll notice that we only configure the specific things
that need configuring with interfaces specific to those things
(eg "daughterboard clocks" and "daughterboard voltages" are
separate), not a single "have a complete set of register values" API.

> Another approach would be to set the members of the SCU's
> reset array directly as we know the type. That seems less convoluted
> but my gut instinct was that wasn't how we should be configuring the
> devices. Is qdev the right way to go in the SCU's case?

We should definitely use qdev rather than poking directly at
the device's internal state struct.

thanks
-- PMM
Andrew Jeffery June 21, 2016, 3:49 a.m. UTC | #4
On Mon, 2016-06-20 at 14:57 +0100, Peter Maydell wrote:
> On 20 June 2016 at 04:44, Andrew Jeffery <andrew@aj.id.au> wrote:
> > 
> > On Fri, 2016-06-17 at 15:22 +0100, Peter Maydell wrote:
> > > 
> > > +static Property aspeed_scu_properties[] = {
> > > +    DEFINE_PROP_ARRAY("reset", AspeedSCUState, num_resets, reset,
> > > +                      qdev_prop_uint32, uint32_t),
> > > +    DEFINE_PROP_END_OF_LIST(),
> > > +};
> > > +
> > > +#define ASPEED_SCU_NR_REGS (0x1A8 >> 2)
> > > This seems like a very unwieldy way of specifying the reset values
> > > for this device. Are they really all fully configurable in the
> > > hardware? It seems unlikely. I'd much rather see something that
> > > looks more like what you might plausibly be configuring when wiring
> > > up the SoC, which might be some version/revision numbers and/or
> > > some particular tweakable parameters.
> > Right. I left out some context which may clear things up: We are
> > working with two SoCs at the moment, the AST2400 and AST2500 (hopefully
> > the AST2500 patches will be sent to the list soon). I wanted to
> > abstract the configuration to cater for the differences in register
> > values between the SoCs, less so for wiring the one SoC up in a
> > different fashion. For what it's worth, out of 86 registers defined in
> > the IO space between the two SoCs, 37 take the same value and 49
> > differ.
> I think there are a couple of plausible ways you might model this:
> 
> (a) just have a single property for "revision" which corresponds
> to the revision of this bit of silicon IP within the SoC; the
> model of the device itself then knows what the reset state is
> for this revision of the device.
> (b) ditto, but also have some configurable flags where relevant
> (ie approximately where it's the same IP rev within the SoC
> but it's been configured by tying down different config lines;
> for instance hw/dma/pl330.c has a collection of properties
> which match the configurable knobs for the hardware.)

Okay. I think (b) is the most appropriate. The board-controllable bits
are primarily in the hardware strapping register. The register is
composed of fields of mostly unrelated bits, so we could go two ways
here:

(1) expose the register through a single 32bit property
(2) break out a property for each bitfield

Do you have a preference? grepping the tree suggests there isn't a
precedent for "large" numbers of properties* so maybe (2) is overkill,
but (1) feels like it might fit into the overly-general-interface
problem that we're trying to eliminate.

* Seems the microblaze CPU defines the most with 9 properties? Approach
(2) will leave us with 21 properties for the SCU.

    $ git grep -c DEFINE_PROP | sort -t: -k2 -r | head -n1
    target-microblaze/cpu.c:9

> 
> You might or might not have enough visibility into the thing to
> know which of these is closest to what the real hardware is doing;
> if not then it's a matter of taste, looking at what is varying
> between the two and what isn't, etc. But "board level specifies
> all the register reset values" is definitely far too broad
> and generalised an API, I think.
> 
> > 
> > Separately, the qdev array approach was lifted from your commit
> > 9c7d489379c2 hw/vexpress: Set reset values for daughterboard
> > oscillators.
> You'll notice that we only configure the specific things
> that need configuring with interfaces specific to those things
> (eg "daughterboard clocks" and "daughterboard voltages" are
> separate), not a single "have a complete set of register values" API.

Yes, I appreciate that now. Thanks.

Andrew
Peter Maydell June 21, 2016, 6:56 a.m. UTC | #5
On 21 June 2016 at 04:49, Andrew Jeffery <andrew@aj.id.au> wrote:
> On Mon, 2016-06-20 at 14:57 +0100, Peter Maydell wrote:
>> I think there are a couple of plausible ways you might model this:
>>
>> (a) just have a single property for "revision" which corresponds
>> to the revision of this bit of silicon IP within the SoC; the
>> model of the device itself then knows what the reset state is
>> for this revision of the device.
>> (b) ditto, but also have some configurable flags where relevant
>> (ie approximately where it's the same IP rev within the SoC
>> but it's been configured by tying down different config lines;
>> for instance hw/dma/pl330.c has a collection of properties
>> which match the configurable knobs for the hardware.)
>
> Okay. I think (b) is the most appropriate. The board-controllable bits
> are primarily in the hardware strapping register. The register is
> composed of fields of mostly unrelated bits, so we could go two ways
> here:
>
> (1) expose the register through a single 32bit property
> (2) break out a property for each bitfield
>
> Do you have a preference? grepping the tree suggests there isn't a
> precedent for "large" numbers of properties* so maybe (2) is overkill,
> but (1) feels like it might fit into the overly-general-interface
> problem that we're trying to eliminate.
>
> * Seems the microblaze CPU defines the most with 9 properties?

(You forgot '-n' on your sort rune.)
hw/dma/pl330.c has 15.

> Approach (2) will leave us with 21 properties for the SCU.

What would the 21 properties be in this case?

But yes, a single 32 bit property would probably be better.

thanks
-- PMM
Andrew Jeffery June 21, 2016, 7:11 a.m. UTC | #6
On Tue, 2016-06-21 at 07:56 +0100, Peter Maydell wrote:
> On 21 June 2016 at 04:49, Andrew Jeffery <andrew@aj.id.au> wrote:
> > 
> > On Mon, 2016-06-20 at 14:57 +0100, Peter Maydell wrote:
> > > 
> > > I think there are a couple of plausible ways you might model this:
> > > 
> > > (a) just have a single property for "revision" which corresponds
> > > to the revision of this bit of silicon IP within the SoC; the
> > > model of the device itself then knows what the reset state is
> > > for this revision of the device.
> > > (b) ditto, but also have some configurable flags where relevant
> > > (ie approximately where it's the same IP rev within the SoC
> > > but it's been configured by tying down different config lines;
> > > for instance hw/dma/pl330.c has a collection of properties
> > > which match the configurable knobs for the hardware.)
> > Okay. I think (b) is the most appropriate. The board-controllable bits
> > are primarily in the hardware strapping register. The register is
> > composed of fields of mostly unrelated bits, so we could go two ways
> > here:
> > 
> > (1) expose the register through a single 32bit property
> > (2) break out a property for each bitfield
> > 
> > Do you have a preference? grepping the tree suggests there isn't a
> > precedent for "large" numbers of properties* so maybe (2) is overkill,
> > but (1) feels like it might fit into the overly-general-interface
> > problem that we're trying to eliminate.
> > 
> > * Seems the microblaze CPU defines the most with 9 properties?
> (You forgot '-n' on your sort rune.)
> hw/dma/pl330.c has 15.

Ugh.

> 
> > 
> > Approach (2) will leave us with 21 properties for the SCU.
> What would the 21 properties be in this case?
> 
> But yes, a single 32 bit property would probably be better.

Alright, I'll go ahead with that.

Thanks for the feedback.

Andrew
diff mbox

Patch

diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index ffb49c11aca6..54020aa06c00 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -52,3 +52,4 @@  obj-$(CONFIG_PVPANIC) += pvpanic.o
 obj-$(CONFIG_EDU) += edu.o
 obj-$(CONFIG_HYPERV_TESTDEV) += hyperv_testdev.o
 obj-$(CONFIG_AUX) += aux.o
+obj-$(CONFIG_ASPEED_SOC) += aspeed_scu.o
diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
new file mode 100644
index 000000000000..492cf82af425
--- /dev/null
+++ b/hw/misc/aspeed_scu.c
@@ -0,0 +1,147 @@ 
+/*
+ * ASPEED System Control Unit
+ *
+ * Andrew Jeffery <andrew@aj.id.au>
+ *
+ * Copyright 2016 IBM Corp.
+ *
+ * This code is licensed under the GPL version 2 or later.  See
+ * the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include <inttypes.h>
+#include "hw/misc/aspeed_scu.h"
+#include "hw/qdev-properties.h"
+#include "qapi/error.h"
+#include "qapi/visitor.h"
+#include "qemu/bitops.h"
+#include "trace.h"
+
+#define SCU_KEY 0x1688A8A8
+#define SCU_IO_REGION_SIZE 0x20000
+
+static uint64_t aspeed_scu_read(void *opaque, hwaddr offset, unsigned size)
+{
+    AspeedSCUState *s = ASPEED_SCU(opaque);
+
+    if (ASPEED_SCU_TO_REG(offset) >= ARRAY_SIZE(s->regs)) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: Out-of-bounds read at offset 0x%" HWADDR_PRIx "\n",
+                      __func__, offset);
+        return 0;
+    }
+
+    switch (offset) {
+    case ASPEED_SCU_WAKEUP_EN:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: Read of write-only offset 0x%" HWADDR_PRIx "\n",
+                      __func__, offset);
+        break;
+    }
+
+    return s->regs[ASPEED_SCU_TO_REG(offset)];
+}
+
+static void aspeed_scu_write(void *opaque, hwaddr offset, uint64_t data,
+                             unsigned size)
+{
+    AspeedSCUState *s = ASPEED_SCU(opaque);
+
+    if (ASPEED_SCU_TO_REG(offset) >= ARRAY_SIZE(s->regs)) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: Out-of-bounds write at offset 0x%" HWADDR_PRIx "\n",
+                      __func__, offset);
+        return;
+    }
+
+    if (offset > ASPEED_SCU_PROT_KEY && offset < ASPEED_SCU_CPU2_BASE_SEG1 &&
+            s->regs[ASPEED_SCU_TO_REG(ASPEED_SCU_PROT_KEY)] != SCU_KEY) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: SCU is locked!\n", __func__);
+        return;
+    }
+
+    trace_aspeed_scu_write(offset, size, data);
+
+    switch (offset) {
+    case ASPEED_SCU_FREQ_CNTR_EVAL:
+    case ASPEED_SCU_VGA_SCRATCH1 ... ASPEED_SCU_VGA_SCRATCH8:
+    case ASPEED_SCU_RNG_DATA:
+    case ASPEED_SCU_REV_ID:
+    case ASPEED_SCU_FREE_CNTR4:
+    case ASPEED_SCU_FREE_CNTR4_EXT:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: Write to read-only offset 0x%" HWADDR_PRIx "\n",
+                      __func__, offset);
+        return;
+    }
+
+    s->regs[ASPEED_SCU_TO_REG(offset)] = (uint32_t) data;
+}
+
+static const MemoryRegionOps aspeed_scu_ops = {
+    .read = aspeed_scu_read,
+    .write = aspeed_scu_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .valid.min_access_size = 4,
+    .valid.max_access_size = 4,
+    .valid.unaligned = false,
+};
+
+static void aspeed_scu_reset(DeviceState *dev)
+{
+    AspeedSCUState *s = ASPEED_SCU(dev);
+
+    memcpy(s->regs, s->reset, sizeof(s->regs));
+}
+
+static void aspeed_scu_realize(DeviceState *dev, Error **errp)
+{
+    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+    AspeedSCUState *s = ASPEED_SCU(dev);
+
+    memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_scu_ops, s,
+                          TYPE_ASPEED_SCU, SCU_IO_REGION_SIZE);
+
+    sysbus_init_mmio(sbd, &s->iomem);
+}
+
+static const VMStateDescription vmstate_aspeed_scu = {
+    .name = "aspeed.new-vic",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32_ARRAY(regs, AspeedSCUState, ASPEED_SCU_NR_REGS),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static Property aspeed_scu_properties[] = {
+    DEFINE_PROP_ARRAY("reset", AspeedSCUState, num_resets, reset,
+                      qdev_prop_uint32, uint32_t),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void aspeed_scu_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    dc->realize = aspeed_scu_realize;
+    dc->reset = aspeed_scu_reset;
+    dc->desc = "ASPEED System Control Unit";
+    dc->vmsd = &vmstate_aspeed_scu;
+    dc->props = aspeed_scu_properties;
+}
+
+static const TypeInfo aspeed_scu_info = {
+    .name = TYPE_ASPEED_SCU,
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(AspeedSCUState),
+    .class_init = aspeed_scu_class_init,
+};
+
+static void aspeed_scu_register_types(void)
+{
+    type_register_static(&aspeed_scu_info);
+}
+
+type_init(aspeed_scu_register_types);
diff --git a/include/hw/misc/aspeed_scu.h b/include/hw/misc/aspeed_scu.h
new file mode 100644
index 000000000000..f95c5c8a7d74
--- /dev/null
+++ b/include/hw/misc/aspeed_scu.h
@@ -0,0 +1,108 @@ 
+/*
+ * ASPEED System Control Unit
+ *
+ * Andrew Jeffery <andrew@aj.id.au>
+ *
+ * Copyright 2016 IBM Corp.
+ *
+ * This code is licensed under the GPL version 2 or later.  See
+ * the COPYING file in the top-level directory.
+ */
+#ifndef ASPEED_SCU_H
+#define ASPEED_SCU_H
+
+#include "hw/sysbus.h"
+
+#define TYPE_ASPEED_SCU "aspeed.scu"
+#define ASPEED_SCU(obj) OBJECT_CHECK(AspeedSCUState, (obj), TYPE_ASPEED_SCU)
+
+#define ASPEED_SCU_NR_REGS (0x1A8 >> 2)
+
+#define ASPEED_SCU_PROT_KEY             0x00
+#define ASPEED_SCU_SYS_RST_CTRL         0x04
+#define ASPEED_SCU_CLK_SEL              0x08
+#define ASPEED_SCU_CLK_STOP_CTRL        0x0C
+#define ASPEED_SCU_FREQ_CNTR_CTRL       0x10
+#define ASPEED_SCU_FREQ_CNTR_EVAL       0x14
+#define ASPEED_SCU_IRQ_CTRL             0x18
+#define ASPEED_SCU_D2PLL_PARAM          0x1C
+#define ASPEED_SCU_MPLL_PARAM           0x20
+#define ASPEED_SCU_HPLL_PARAM           0x24
+#define ASPEED_SCU_FREQ_CNTR_RANGE      0x28
+#define ASPEED_SCU_MISC_CTRL1           0x2C
+#define ASPEED_SCU_PCI_CTRL1            0x30
+#define ASPEED_SCU_PCI_CTRL2            0x34
+#define ASPEED_SCU_PCI_CTRL3            0x38
+#define ASPEED_SCU_SYS_RST_STATUS       0x3C
+#define ASPEED_SCU_SOC_SCRATCH1         0x40
+#define ASPEED_SCU_SOC_SCRATCH2         0x44
+#define ASPEED_SCU_MAC_CLK_DELAY        0x48
+#define ASPEED_SCU_MISC_CTRL2           0x4C
+#define ASPEED_SCU_VGA_SCRATCH1         0x50
+#define ASPEED_SCU_VGA_SCRATCH2         0x54
+#define ASPEED_SCU_VGA_SCRATCH3         0x58
+#define ASPEED_SCU_VGA_SCRATCH4         0x5C
+#define ASPEED_SCU_VGA_SCRATCH5         0x60
+#define ASPEED_SCU_VGA_SCRATCH6         0x64
+#define ASPEED_SCU_VGA_SCRATCH7         0x68
+#define ASPEED_SCU_VGA_SCRATCH8         0x6C
+#define ASPEED_SCU_HW_STRAP1            0x70
+#define ASPEED_SCU_RNG_CTRL             0x74
+#define ASPEED_SCU_RNG_DATA             0x78
+#define ASPEED_SCU_REV_ID               0x7C
+#define ASPEED_SCU_PINMUX_CTRL1         0x80
+#define ASPEED_SCU_PINMUX_CTRL2         0x84
+#define ASPEED_SCU_PINMUX_CTRL3         0x88
+#define ASPEED_SCU_PINMUX_CTRL4         0x8C
+#define ASPEED_SCU_PINMUX_CTRL5         0x90
+#define ASPEED_SCU_PINMUX_CTRL6         0x94
+#define ASPEED_SCU_WDT_RST_CTRL         0x9C
+#define ASPEED_SCU_PINMUX_CTRL7         0xA0
+#define ASPEED_SCU_PINMUX_CTRL8         0xA4
+#define ASPEED_SCU_PINMUX_CTRL9         0xA8
+#define ASPEED_SCU_WAKEUP_EN            0xC0
+#define ASPEED_SCU_WAKEUP_CTRL          0xC4
+#define ASPEED_SCU_HW_STRAP2            0xD0
+#define ASPEED_SCU_FREE_CNTR4           0xE0
+#define ASPEED_SCU_FREE_CNTR4_EXT       0xE4
+#define ASPEED_SCU_CPU2_CTRL            0x100
+#define ASPEED_SCU_CPU2_BASE_SEG1       0x104
+#define ASPEED_SCU_CPU2_BASE_SEG2       0x108
+#define ASPEED_SCU_CPU2_BASE_SEG3       0x10C
+#define ASPEED_SCU_CPU2_BASE_SEG4       0x110
+#define ASPEED_SCU_CPU2_BASE_SEG5       0x114
+#define ASPEED_SCU_CPU2_CACHE_CTRL      0x118
+#define ASPEED_SCU_UART_HPLL_CLK        0x160
+#define ASPEED_SCU_PCIE_CTRL            0x180
+#define ASPEED_SCU_BMC_MMIO_CTRL        0x184
+#define ASPEED_SCU_RELOC_DECODE_BASE1   0x188
+#define ASPEED_SCU_RELOC_DECODE_BASE2   0x18C
+#define ASPEED_SCU_MAILBOX_DECODE_BASE  0x190
+#define ASPEED_SCU_SRAM_DECODE_BASE1    0x194
+#define ASPEED_SCU_SRAM_DECODE_BASE2    0x198
+#define ASPEED_SCU_BMC_REV_ID           0x19C
+#define ASPEED_SCU_BMC_DEV_ID           0x1A4
+
+#define ASPEED_SCU_TO_REG(offset) ((offset) >> 2)
+
+typedef struct AspeedSCUState {
+    /*< private >*/
+    SysBusDevice parent_obj;
+
+    /*< public >*/
+    MemoryRegion iomem;
+
+    uint32_t regs[ASPEED_SCU_NR_REGS];
+    uint32_t *reset;
+    uint32_t num_resets;
+} AspeedSCUState;
+
+typedef struct AspeedSCUResetCfg {
+    uint32_t offset;
+    uint32_t val;
+} AspeedSCUResetCfg;
+
+void aspeed_scu_configure_reset(AspeedSCUState *scu,
+        const struct AspeedSCUResetCfg vals[], int n, Error **errp);
+
+#endif /* ASPEED_SCU_H */
diff --git a/trace-events b/trace-events
index 2f14205de047..24749c414ee7 100644
--- a/trace-events
+++ b/trace-events
@@ -2164,3 +2164,6 @@  e1000e_cfg_support_virtio(bool support) "Virtio header supported: %d"
 
 e1000e_vm_state_running(void) "VM state is running"
 e1000e_vm_state_stopped(void) "VM state is stopped"
+
+# hw/misc/aspeed_scu.c
+aspeed_scu_write(uint64_t offset, unsigned size, uint32_t data) "To 0x%" PRIx64 " of size %u: 0x%" PRIx32