| Message ID | 1466648115-17015-2-git-send-email-andrew@aj.id.au (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 06/23/2016 04:15 AM, Andrew Jeffery wrote: > The SCU is a collection of chip-level control registers that manage the > various functions supported by ASPEED SoCs. 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 (eg AST2400 vs AST2500). A qdev > property is exposed so that the integrating SoC model can configure the > silicon revision, which in-turn selects the appropriate reset values. > Further qdev properties are exposed so the board model can configure the > board-dependent hardware strapping. > > Almost all provided AST2400 reset values are specified by the datasheet. > The notable exception is SOC_SCRATCH1, where we mark the DRAM as > successfully initialised to avoid unnecessary dark corners in the SoC's > u-boot support. > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au> Reviewed-by: Cédric Le Goater <clg@kaod.org> Thanks, C.
On 23 June 2016 at 03:15, Andrew Jeffery <andrew@aj.id.au> wrote: > The SCU is a collection of chip-level control registers that manage the > various functions supported by ASPEED SoCs. 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 (eg AST2400 vs AST2500). A qdev > property is exposed so that the integrating SoC model can configure the > silicon revision, which in-turn selects the appropriate reset values. > Further qdev properties are exposed so the board model can configure the > board-dependent hardware strapping. > > Almost all provided AST2400 reset values are specified by the datasheet. > The notable exception is SOC_SCRATCH1, where we mark the DRAM as > successfully initialised to avoid unnecessary dark corners in the SoC's > u-boot support. > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au> Thanks -- I think the interface to the board is much nicer now. I have a couple of comments below. > hw/misc/Makefile.objs | 1 + > hw/misc/aspeed_scu.c | 258 +++++++++++++++++++++++++++++++++++++++++++ > include/hw/misc/aspeed_scu.h | 34 ++++++ > trace-events | 3 + > 4 files changed, 296 insertions(+) > create mode 100644 hw/misc/aspeed_scu.c > create mode 100644 include/hw/misc/aspeed_scu.h > > 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..a714431c45c5 > --- /dev/null > +++ b/hw/misc/aspeed_scu.c > @@ -0,0 +1,258 @@ > +/* > + * 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 TO_REG(offset) ((offset) >> 2) > + > +#define PROT_KEY TO_REG(0x00) > +#define SYS_RST_CTRL TO_REG(0x04) > +#define CLK_SEL TO_REG(0x08) > +#define CLK_STOP_CTRL TO_REG(0x0C) > +#define FREQ_CNTR_CTRL TO_REG(0x10) > +#define FREQ_CNTR_EVAL TO_REG(0x14) > +#define IRQ_CTRL TO_REG(0x18) > +#define D2PLL_PARAM TO_REG(0x1C) > +#define MPLL_PARAM TO_REG(0x20) > +#define HPLL_PARAM TO_REG(0x24) > +#define FREQ_CNTR_RANGE TO_REG(0x28) > +#define MISC_CTRL1 TO_REG(0x2C) > +#define PCI_CTRL1 TO_REG(0x30) > +#define PCI_CTRL2 TO_REG(0x34) > +#define PCI_CTRL3 TO_REG(0x38) > +#define SYS_RST_STATUS TO_REG(0x3C) > +#define SOC_SCRATCH1 TO_REG(0x40) > +#define SOC_SCRATCH2 TO_REG(0x44) > +#define MAC_CLK_DELAY TO_REG(0x48) > +#define MISC_CTRL2 TO_REG(0x4C) > +#define VGA_SCRATCH1 TO_REG(0x50) > +#define VGA_SCRATCH2 TO_REG(0x54) > +#define VGA_SCRATCH3 TO_REG(0x58) > +#define VGA_SCRATCH4 TO_REG(0x5C) > +#define VGA_SCRATCH5 TO_REG(0x60) > +#define VGA_SCRATCH6 TO_REG(0x64) > +#define VGA_SCRATCH7 TO_REG(0x68) > +#define VGA_SCRATCH8 TO_REG(0x6C) > +#define HW_STRAP1 TO_REG(0x70) > +#define RNG_CTRL TO_REG(0x74) > +#define RNG_DATA TO_REG(0x78) > +#define SILICON_REV TO_REG(0x7C) > +#define PINMUX_CTRL1 TO_REG(0x80) > +#define PINMUX_CTRL2 TO_REG(0x84) > +#define PINMUX_CTRL3 TO_REG(0x88) > +#define PINMUX_CTRL4 TO_REG(0x8C) > +#define PINMUX_CTRL5 TO_REG(0x90) > +#define PINMUX_CTRL6 TO_REG(0x94) > +#define WDT_RST_CTRL TO_REG(0x9C) > +#define PINMUX_CTRL7 TO_REG(0xA0) > +#define PINMUX_CTRL8 TO_REG(0xA4) > +#define PINMUX_CTRL9 TO_REG(0xA8) > +#define WAKEUP_EN TO_REG(0xC0) > +#define WAKEUP_CTRL TO_REG(0xC4) > +#define HW_STRAP2 TO_REG(0xD0) > +#define FREE_CNTR4 TO_REG(0xE0) > +#define FREE_CNTR4_EXT TO_REG(0xE4) > +#define CPU2_CTRL TO_REG(0x100) > +#define CPU2_BASE_SEG1 TO_REG(0x104) > +#define CPU2_BASE_SEG2 TO_REG(0x108) > +#define CPU2_BASE_SEG3 TO_REG(0x10C) > +#define CPU2_BASE_SEG4 TO_REG(0x110) > +#define CPU2_BASE_SEG5 TO_REG(0x114) > +#define CPU2_CACHE_CTRL TO_REG(0x118) > +#define UART_HPLL_CLK TO_REG(0x160) > +#define PCIE_CTRL TO_REG(0x180) > +#define BMC_MMIO_CTRL TO_REG(0x184) > +#define RELOC_DECODE_BASE1 TO_REG(0x188) > +#define RELOC_DECODE_BASE2 TO_REG(0x18C) > +#define MAILBOX_DECODE_BASE TO_REG(0x190) > +#define SRAM_DECODE_BASE1 TO_REG(0x194) > +#define SRAM_DECODE_BASE2 TO_REG(0x198) > +#define BMC_REV TO_REG(0x19C) > +#define BMC_DEV_ID TO_REG(0x1A4) > + > +#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 (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 WAKEUP_EN: WAKEUP_EN is defined as TO_REG(something) so you can't use it in a case statement switching on an offset. > + qemu_log_mask(LOG_GUEST_ERROR, > + "%s: Read of write-only offset 0x%" HWADDR_PRIx "\n", > + __func__, offset); > + break; > + } > + > + return s->regs[TO_REG(offset)]; > +} > + > +static void aspeed_scu_write(void *opaque, hwaddr offset, uint64_t data, > + unsigned size) > +{ > + AspeedSCUState *s = ASPEED_SCU(opaque); > + > + if (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 > PROT_KEY && offset < CPU2_BASE_SEG1 && > + s->regs[TO_REG(PROT_KEY)] != SCU_KEY) { PROT_KEY is defined above as TO_REG(0x00), so it's not an offset and using it in comparisons against offset and applying TO_REG() to it again doesn't seem right. Similarly with CPU2_BASE_SEG1, which is more important since it's not zero... > + qemu_log_mask(LOG_GUEST_ERROR, "%s: SCU is locked!\n", __func__); > + return; > + } > + > + trace_aspeed_scu_write(offset, size, data); > + > + switch (offset) { > + case FREQ_CNTR_EVAL: > + case VGA_SCRATCH1 ... VGA_SCRATCH8: > + case RNG_DATA: > + case SILICON_REV: > + case FREE_CNTR4: > + case FREE_CNTR4_EXT: > + qemu_log_mask(LOG_GUEST_ERROR, > + "%s: Write to read-only offset 0x%" HWADDR_PRIx "\n", > + __func__, offset); > + return; > + } > + > + s->regs[TO_REG(offset)] = (uint32_t) data; This cast is unnecessary. > +} > + > +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); > + const uint32_t *reset; > + > + switch (s->silicon_rev) { > + case 0x02000303U: > + reset = ast2400_resets; > + break; default: g_assert_not_reached(); or the compiler will probably complain that you might be using reset uninitialized. > + } > + > + memcpy(s->regs, reset, sizeof(s->regs)); > + s->regs[SILICON_REV] = s->silicon_rev; > + s->regs[HW_STRAP1] = s->hw_strap1; > + s->regs[HW_STRAP2] = s->hw_strap2; > +} > + > +static void aspeed_scu_realize(DeviceState *dev, Error **errp) > +{ > + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); > + AspeedSCUState *s = ASPEED_SCU(dev); You should sanity check your properties here, and fail the realize if they aren't valid values. > + 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); > +} > diff --git a/trace-events b/trace-events > index 9d76de85749d..1b5fd602903c 100644 > --- a/trace-events > +++ b/trace-events > @@ -156,3 +156,6 @@ memory_region_tb_write(int cpu_index, uint64_t addr, uint64_t value, unsigned si > # > # Targets: TCG(all) > disable vcpu tcg guest_mem_before(TCGv vaddr, uint8_t info) "info=%d", "vaddr=0x%016"PRIx64" info=%d" > + > +# hw/misc/aspeed_scu.c > +aspeed_scu_write(uint64_t offset, unsigned size, uint32_t data) "To 0x%" PRIx64 " of size %u: 0x%" PRIx32 > -- This should be in hw/misc/trace-events now -- we've split the big trace-events file into pieces. thanks -- PMM
On 23 June 2016 at 03:15, Andrew Jeffery <andrew@aj.id.au> wrote: > The SCU is a collection of chip-level control registers that manage the > various functions supported by ASPEED SoCs. 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 (eg AST2400 vs AST2500). A qdev > property is exposed so that the integrating SoC model can configure the > silicon revision, which in-turn selects the appropriate reset values. > Further qdev properties are exposed so the board model can configure the > board-dependent hardware strapping. > > Almost all provided AST2400 reset values are specified by the datasheet. > The notable exception is SOC_SCRATCH1, where we mark the DRAM as > successfully initialised to avoid unnecessary dark corners in the SoC's > u-boot support. > +static Property aspeed_scu_properties[] = { > + DEFINE_PROP_UINT32("silicon-rev", AspeedSCUState, silicon_rev, 0), > + DEFINE_PROP_UINT32("hw-strap1", AspeedSCUState, hw_strap1, 0), > + DEFINE_PROP_UINT32("hw-strap2", AspeedSCUState, hw_strap1, 0), > + DEFINE_PROP_END_OF_LIST(), > +}; You don't seem to specify in the board layer or the SoC layer any of these except hw-strap1, so should the default values for these really all be zero? I suspect silicon-rev at least should either have a default value specified here, or have the SoC layer specify it. (It probably should not be specified at the board level.) thanks -- PMM
On Thu, 2016-06-23 at 18:37 +0100, Peter Maydell wrote: > On 23 June 2016 at 03:15, Andrew Jeffery <andrew@aj.id.au> wrote: > > > > The SCU is a collection of chip-level control registers that manage the > > various functions supported by ASPEED SoCs. 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 (eg AST2400 vs AST2500). A qdev > > property is exposed so that the integrating SoC model can configure the > > silicon revision, which in-turn selects the appropriate reset values. > > Further qdev properties are exposed so the board model can configure the > > board-dependent hardware strapping. > > > > Almost all provided AST2400 reset values are specified by the datasheet. > > The notable exception is SOC_SCRATCH1, where we mark the DRAM as > > successfully initialised to avoid unnecessary dark corners in the SoC's > > u-boot support. > > > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au> > Thanks -- I think the interface to the board is much nicer now. > I have a couple of comments below. > > > > > hw/misc/Makefile.objs | 1 + > > hw/misc/aspeed_scu.c | 258 +++++++++++++++++++++++++++++++++++++++++++ > > include/hw/misc/aspeed_scu.h | 34 ++++++ > > trace-events | 3 + > > 4 files changed, 296 insertions(+) > > create mode 100644 hw/misc/aspeed_scu.c > > create mode 100644 include/hw/misc/aspeed_scu.h > > > > 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..a714431c45c5 > > --- /dev/null > > +++ b/hw/misc/aspeed_scu.c > > @@ -0,0 +1,258 @@ > > +/* > > + * 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 > > +#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 TO_REG(offset) ((offset) >> 2) > > + > > +#define PROT_KEY TO_REG(0x00) > > +#define SYS_RST_CTRL TO_REG(0x04) > > +#define CLK_SEL TO_REG(0x08) > > +#define CLK_STOP_CTRL TO_REG(0x0C) > > +#define FREQ_CNTR_CTRL TO_REG(0x10) > > +#define FREQ_CNTR_EVAL TO_REG(0x14) > > +#define IRQ_CTRL TO_REG(0x18) > > +#define D2PLL_PARAM TO_REG(0x1C) > > +#define MPLL_PARAM TO_REG(0x20) > > +#define HPLL_PARAM TO_REG(0x24) > > +#define FREQ_CNTR_RANGE TO_REG(0x28) > > +#define MISC_CTRL1 TO_REG(0x2C) > > +#define PCI_CTRL1 TO_REG(0x30) > > +#define PCI_CTRL2 TO_REG(0x34) > > +#define PCI_CTRL3 TO_REG(0x38) > > +#define SYS_RST_STATUS TO_REG(0x3C) > > +#define SOC_SCRATCH1 TO_REG(0x40) > > +#define SOC_SCRATCH2 TO_REG(0x44) > > +#define MAC_CLK_DELAY TO_REG(0x48) > > +#define MISC_CTRL2 TO_REG(0x4C) > > +#define VGA_SCRATCH1 TO_REG(0x50) > > +#define VGA_SCRATCH2 TO_REG(0x54) > > +#define VGA_SCRATCH3 TO_REG(0x58) > > +#define VGA_SCRATCH4 TO_REG(0x5C) > > +#define VGA_SCRATCH5 TO_REG(0x60) > > +#define VGA_SCRATCH6 TO_REG(0x64) > > +#define VGA_SCRATCH7 TO_REG(0x68) > > +#define VGA_SCRATCH8 TO_REG(0x6C) > > +#define HW_STRAP1 TO_REG(0x70) > > +#define RNG_CTRL TO_REG(0x74) > > +#define RNG_DATA TO_REG(0x78) > > +#define SILICON_REV TO_REG(0x7C) > > +#define PINMUX_CTRL1 TO_REG(0x80) > > +#define PINMUX_CTRL2 TO_REG(0x84) > > +#define PINMUX_CTRL3 TO_REG(0x88) > > +#define PINMUX_CTRL4 TO_REG(0x8C) > > +#define PINMUX_CTRL5 TO_REG(0x90) > > +#define PINMUX_CTRL6 TO_REG(0x94) > > +#define WDT_RST_CTRL TO_REG(0x9C) > > +#define PINMUX_CTRL7 TO_REG(0xA0) > > +#define PINMUX_CTRL8 TO_REG(0xA4) > > +#define PINMUX_CTRL9 TO_REG(0xA8) > > +#define WAKEUP_EN TO_REG(0xC0) > > +#define WAKEUP_CTRL TO_REG(0xC4) > > +#define HW_STRAP2 TO_REG(0xD0) > > +#define FREE_CNTR4 TO_REG(0xE0) > > +#define FREE_CNTR4_EXT TO_REG(0xE4) > > +#define CPU2_CTRL TO_REG(0x100) > > +#define CPU2_BASE_SEG1 TO_REG(0x104) > > +#define CPU2_BASE_SEG2 TO_REG(0x108) > > +#define CPU2_BASE_SEG3 TO_REG(0x10C) > > +#define CPU2_BASE_SEG4 TO_REG(0x110) > > +#define CPU2_BASE_SEG5 TO_REG(0x114) > > +#define CPU2_CACHE_CTRL TO_REG(0x118) > > +#define UART_HPLL_CLK TO_REG(0x160) > > +#define PCIE_CTRL TO_REG(0x180) > > +#define BMC_MMIO_CTRL TO_REG(0x184) > > +#define RELOC_DECODE_BASE1 TO_REG(0x188) > > +#define RELOC_DECODE_BASE2 TO_REG(0x18C) > > +#define MAILBOX_DECODE_BASE TO_REG(0x190) > > +#define SRAM_DECODE_BASE1 TO_REG(0x194) > > +#define SRAM_DECODE_BASE2 TO_REG(0x198) > > +#define BMC_REV TO_REG(0x19C) > > +#define BMC_DEV_ID TO_REG(0x1A4) > > + > > +#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 (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 WAKEUP_EN: > WAKEUP_EN is defined as TO_REG(something) so you can't use it in > a case statement switching on an offset. Right. That was quite an oversight and lead to quite misleading guest- error messages. Thanks for picking that up. > > > > > + qemu_log_mask(LOG_GUEST_ERROR, > > + "%s: Read of write-only offset 0x%" HWADDR_PRIx "\n", > > + __func__, offset); > > + break; > > + } > > + > > + return s->regs[TO_REG(offset)]; > > +} > > + > > +static void aspeed_scu_write(void *opaque, hwaddr offset, uint64_t data, > > + unsigned size) > > +{ > > + AspeedSCUState *s = ASPEED_SCU(opaque); > > + > > + if (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 > PROT_KEY && offset < CPU2_BASE_SEG1 && > > + s->regs[TO_REG(PROT_KEY)] != SCU_KEY) { > PROT_KEY is defined above as TO_REG(0x00), so it's not > an offset and using it in comparisons against offset and > applying TO_REG() to it again doesn't seem right. > Similarly with CPU2_BASE_SEG1, which is more important since > it's not zero... Yes, this was the same issue as above. > > > > > + qemu_log_mask(LOG_GUEST_ERROR, "%s: SCU is locked!\n", __func__); > > + return; > > + } > > + > > + trace_aspeed_scu_write(offset, size, data); > > + > > + switch (offset) { > > + case FREQ_CNTR_EVAL: > > + case VGA_SCRATCH1 ... VGA_SCRATCH8: > > + case RNG_DATA: > > + case SILICON_REV: > > + case FREE_CNTR4: > > + case FREE_CNTR4_EXT: > > + qemu_log_mask(LOG_GUEST_ERROR, > > + "%s: Write to read-only offset 0x%" HWADDR_PRIx "\n", > > + __func__, offset); > > + return; > > + } > > + > > + s->regs[TO_REG(offset)] = (uint32_t) data; > This cast is unnecessary. True! > > > > > +} > > + > > +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); > > + const uint32_t *reset; > > + > > + switch (s->silicon_rev) { > > + case 0x02000303U: > > + reset = ast2400_resets; > > + break; > default: > g_assert_not_reached(); > > or the compiler will probably complain that you might > be using reset uninitialized. Will do. > > > > > + } > > + > > + memcpy(s->regs, reset, sizeof(s->regs)); > > + s->regs[SILICON_REV] = s->silicon_rev; > > + s->regs[HW_STRAP1] = s->hw_strap1; > > + s->regs[HW_STRAP2] = s->hw_strap2; > > +} > > + > > +static void aspeed_scu_realize(DeviceState *dev, Error **errp) > > +{ > > + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); > > + AspeedSCUState *s = ASPEED_SCU(dev); > You should sanity check your properties here, and > fail the realize if they aren't valid values. Will do. > > > > > + 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); > > +} > > > > diff --git a/trace-events b/trace-events > > index 9d76de85749d..1b5fd602903c 100644 > > --- a/trace-events > > +++ b/trace-events > > @@ -156,3 +156,6 @@ memory_region_tb_write(int cpu_index, uint64_t addr, uint64_t value, unsigned si > > # > > # Targets: TCG(all) > > disable vcpu tcg guest_mem_before(TCGv vaddr, uint8_t info) "info=%d", "vaddr=0x%016"PRIx64" info=%d" > > + > > +# hw/misc/aspeed_scu.c > > +aspeed_scu_write(uint64_t offset, unsigned size, uint32_t data) "To 0x%" PRIx64 " of size %u: 0x%" PRIx32 > > -- > This should be in hw/misc/trace-events now -- we've split the > big trace-events file into pieces. Will do. Thanks again for the review and apologies for the noise with the offset/reg mixup, that was an annoying oversight. Cheers, Andrew
On Thu, 2016-06-23 at 18:42 +0100, Peter Maydell wrote: > On 23 June 2016 at 03:15, Andrew Jeffery <andrew@aj.id.au> wrote: > > > > The SCU is a collection of chip-level control registers that manage the > > various functions supported by ASPEED SoCs. 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 (eg AST2400 vs AST2500). A qdev > > property is exposed so that the integrating SoC model can configure the > > silicon revision, which in-turn selects the appropriate reset values. > > Further qdev properties are exposed so the board model can configure the > > board-dependent hardware strapping. > > > > Almost all provided AST2400 reset values are specified by the datasheet. > > The notable exception is SOC_SCRATCH1, where we mark the DRAM as > > successfully initialised to avoid unnecessary dark corners in the SoC's > > u-boot support. > > > > +static Property aspeed_scu_properties[] = { > > + DEFINE_PROP_UINT32("silicon-rev", AspeedSCUState, silicon_rev, 0), > > + DEFINE_PROP_UINT32("hw-strap1", AspeedSCUState, hw_strap1, 0), > > + DEFINE_PROP_UINT32("hw-strap2", AspeedSCUState, hw_strap1, 0), > > + DEFINE_PROP_END_OF_LIST(), > > +}; > You don't seem to specify in the board layer or the SoC layer > any of these except hw-strap1, so should the default values > for these really all be zero? Both strap register default values are 0 according to the datasheet. > > I suspect silicon-rev at least should either have a default > value specified here, or have the SoC layer specify it. > (It probably should not be specified at the board level.) I intended to set silicon-rev in the SoC layer, so I'll fix patch 2/3. With the addition of sanity checking in the SCU's realise() we'll catch the case where it's an invalid value (eg 0). I don't think it's right to plow ahead with an unexpected configuration if a chosen default value doesn't match the SoC at hand. Maybe I shouldn't send patches with a heavy head cold :/ Cheers, Andrew
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..a714431c45c5 --- /dev/null +++ b/hw/misc/aspeed_scu.c @@ -0,0 +1,258 @@ +/* + * 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 TO_REG(offset) ((offset) >> 2) + +#define PROT_KEY TO_REG(0x00) +#define SYS_RST_CTRL TO_REG(0x04) +#define CLK_SEL TO_REG(0x08) +#define CLK_STOP_CTRL TO_REG(0x0C) +#define FREQ_CNTR_CTRL TO_REG(0x10) +#define FREQ_CNTR_EVAL TO_REG(0x14) +#define IRQ_CTRL TO_REG(0x18) +#define D2PLL_PARAM TO_REG(0x1C) +#define MPLL_PARAM TO_REG(0x20) +#define HPLL_PARAM TO_REG(0x24) +#define FREQ_CNTR_RANGE TO_REG(0x28) +#define MISC_CTRL1 TO_REG(0x2C) +#define PCI_CTRL1 TO_REG(0x30) +#define PCI_CTRL2 TO_REG(0x34) +#define PCI_CTRL3 TO_REG(0x38) +#define SYS_RST_STATUS TO_REG(0x3C) +#define SOC_SCRATCH1 TO_REG(0x40) +#define SOC_SCRATCH2 TO_REG(0x44) +#define MAC_CLK_DELAY TO_REG(0x48) +#define MISC_CTRL2 TO_REG(0x4C) +#define VGA_SCRATCH1 TO_REG(0x50) +#define VGA_SCRATCH2 TO_REG(0x54) +#define VGA_SCRATCH3 TO_REG(0x58) +#define VGA_SCRATCH4 TO_REG(0x5C) +#define VGA_SCRATCH5 TO_REG(0x60) +#define VGA_SCRATCH6 TO_REG(0x64) +#define VGA_SCRATCH7 TO_REG(0x68) +#define VGA_SCRATCH8 TO_REG(0x6C) +#define HW_STRAP1 TO_REG(0x70) +#define RNG_CTRL TO_REG(0x74) +#define RNG_DATA TO_REG(0x78) +#define SILICON_REV TO_REG(0x7C) +#define PINMUX_CTRL1 TO_REG(0x80) +#define PINMUX_CTRL2 TO_REG(0x84) +#define PINMUX_CTRL3 TO_REG(0x88) +#define PINMUX_CTRL4 TO_REG(0x8C) +#define PINMUX_CTRL5 TO_REG(0x90) +#define PINMUX_CTRL6 TO_REG(0x94) +#define WDT_RST_CTRL TO_REG(0x9C) +#define PINMUX_CTRL7 TO_REG(0xA0) +#define PINMUX_CTRL8 TO_REG(0xA4) +#define PINMUX_CTRL9 TO_REG(0xA8) +#define WAKEUP_EN TO_REG(0xC0) +#define WAKEUP_CTRL TO_REG(0xC4) +#define HW_STRAP2 TO_REG(0xD0) +#define FREE_CNTR4 TO_REG(0xE0) +#define FREE_CNTR4_EXT TO_REG(0xE4) +#define CPU2_CTRL TO_REG(0x100) +#define CPU2_BASE_SEG1 TO_REG(0x104) +#define CPU2_BASE_SEG2 TO_REG(0x108) +#define CPU2_BASE_SEG3 TO_REG(0x10C) +#define CPU2_BASE_SEG4 TO_REG(0x110) +#define CPU2_BASE_SEG5 TO_REG(0x114) +#define CPU2_CACHE_CTRL TO_REG(0x118) +#define UART_HPLL_CLK TO_REG(0x160) +#define PCIE_CTRL TO_REG(0x180) +#define BMC_MMIO_CTRL TO_REG(0x184) +#define RELOC_DECODE_BASE1 TO_REG(0x188) +#define RELOC_DECODE_BASE2 TO_REG(0x18C) +#define MAILBOX_DECODE_BASE TO_REG(0x190) +#define SRAM_DECODE_BASE1 TO_REG(0x194) +#define SRAM_DECODE_BASE2 TO_REG(0x198) +#define BMC_REV TO_REG(0x19C) +#define BMC_DEV_ID TO_REG(0x1A4) + +#define SCU_KEY 0x1688A8A8 +#define SCU_IO_REGION_SIZE 0x20000 + +static const uint32_t ast2400_resets[ASPEED_SCU_NR_REGS] = { + [SYS_RST_CTRL] = 0xFFCFFEDCU, + [CLK_SEL] = 0xF3F40000U, + [CLK_STOP_CTRL] = 0x19FC3E8BU, + [D2PLL_PARAM] = 0x00026108U, + [MPLL_PARAM] = 0x00030291U, + [HPLL_PARAM] = 0x00000291U, + [MISC_CTRL1] = 0x00000010U, + [PCI_CTRL1] = 0x20001A03U, + [PCI_CTRL2] = 0x20001A03U, + [PCI_CTRL3] = 0x04000030U, + [SYS_RST_STATUS] = 0x00000001U, + [SOC_SCRATCH1] = 0x000000C0U, /* SoC completed DRAM init */ + [MISC_CTRL2] = 0x00000023U, + [RNG_CTRL] = 0x0000000EU, + [SILICON_REV] = 0x02000303U, + [PINMUX_CTRL2] = 0x0000F000U, + [PINMUX_CTRL3] = 0x01000000U, + [PINMUX_CTRL4] = 0x000000FFU, + [PINMUX_CTRL5] = 0x0000A000U, + [WDT_RST_CTRL] = 0x003FFFF3U, + [PINMUX_CTRL8] = 0xFFFF0000U, + [PINMUX_CTRL9] = 0x000FFFFFU, + [FREE_CNTR4] = 0x000000FFU, + [FREE_CNTR4_EXT] = 0x000000FFU, + [CPU2_BASE_SEG1] = 0x80000000U, + [CPU2_BASE_SEG4] = 0x1E600000U, + [CPU2_BASE_SEG5] = 0xC0000000U, + [UART_HPLL_CLK] = 0x00001903U, + [PCIE_CTRL] = 0x0000007BU, + [BMC_DEV_ID] = 0x00002402U +}; + +static uint64_t aspeed_scu_read(void *opaque, hwaddr offset, unsigned size) +{ + AspeedSCUState *s = ASPEED_SCU(opaque); + + if (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 WAKEUP_EN: + qemu_log_mask(LOG_GUEST_ERROR, + "%s: Read of write-only offset 0x%" HWADDR_PRIx "\n", + __func__, offset); + break; + } + + return s->regs[TO_REG(offset)]; +} + +static void aspeed_scu_write(void *opaque, hwaddr offset, uint64_t data, + unsigned size) +{ + AspeedSCUState *s = ASPEED_SCU(opaque); + + if (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 > PROT_KEY && offset < CPU2_BASE_SEG1 && + s->regs[TO_REG(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 FREQ_CNTR_EVAL: + case VGA_SCRATCH1 ... VGA_SCRATCH8: + case RNG_DATA: + case SILICON_REV: + case FREE_CNTR4: + case FREE_CNTR4_EXT: + qemu_log_mask(LOG_GUEST_ERROR, + "%s: Write to read-only offset 0x%" HWADDR_PRIx "\n", + __func__, offset); + return; + } + + s->regs[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); + const uint32_t *reset; + + switch (s->silicon_rev) { + case 0x02000303U: + reset = ast2400_resets; + break; + } + + memcpy(s->regs, reset, sizeof(s->regs)); + s->regs[SILICON_REV] = s->silicon_rev; + s->regs[HW_STRAP1] = s->hw_strap1; + s->regs[HW_STRAP2] = s->hw_strap2; +} + +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.scu", + .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_UINT32("silicon-rev", AspeedSCUState, silicon_rev, 0), + DEFINE_PROP_UINT32("hw-strap1", AspeedSCUState, hw_strap1, 0), + DEFINE_PROP_UINT32("hw-strap2", AspeedSCUState, hw_strap1, 0), + 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..6b8e46f85fad --- /dev/null +++ b/include/hw/misc/aspeed_scu.h @@ -0,0 +1,34 @@ +/* + * 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) + +typedef struct AspeedSCUState { + /*< private >*/ + SysBusDevice parent_obj; + + /*< public >*/ + MemoryRegion iomem; + + uint32_t regs[ASPEED_SCU_NR_REGS]; + uint32_t silicon_rev; + uint32_t hw_strap1; + uint32_t hw_strap2; +} AspeedSCUState; + +#endif /* ASPEED_SCU_H */ diff --git a/trace-events b/trace-events index 9d76de85749d..1b5fd602903c 100644 --- a/trace-events +++ b/trace-events @@ -156,3 +156,6 @@ memory_region_tb_write(int cpu_index, uint64_t addr, uint64_t value, unsigned si # # Targets: TCG(all) disable vcpu tcg guest_mem_before(TCGv vaddr, uint8_t info) "info=%d", "vaddr=0x%016"PRIx64" info=%d" + +# hw/misc/aspeed_scu.c +aspeed_scu_write(uint64_t offset, unsigned size, uint32_t data) "To 0x%" PRIx64 " of size %u: 0x%" PRIx32
The SCU is a collection of chip-level control registers that manage the various functions supported by ASPEED SoCs. 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 (eg AST2400 vs AST2500). A qdev property is exposed so that the integrating SoC model can configure the silicon revision, which in-turn selects the appropriate reset values. Further qdev properties are exposed so the board model can configure the board-dependent hardware strapping. Almost all provided AST2400 reset values are specified by the datasheet. The notable exception is SOC_SCRATCH1, where we mark the DRAM as successfully initialised to avoid unnecessary dark corners in the SoC's u-boot support. Signed-off-by: Andrew Jeffery <andrew@aj.id.au> --- Since v1: * Move reset values into SCU implementation (also make register defines private) * Expose silicon-rev property which is used to select appropriate reset values * Expose hw-strap1/hw-strap2 properties for board-specific SoC configuration hw/misc/Makefile.objs | 1 + hw/misc/aspeed_scu.c | 258 +++++++++++++++++++++++++++++++++++++++++++ include/hw/misc/aspeed_scu.h | 34 ++++++ trace-events | 3 + 4 files changed, 296 insertions(+) create mode 100644 hw/misc/aspeed_scu.c create mode 100644 include/hw/misc/aspeed_scu.h