| Message ID | 20210812093356.1946-3-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | arm: Get rid of system_clock_scale global | expand |
On Thu, Aug 12, 2021 at 7:36 PM Peter Maydell <peter.maydell@linaro.org> wrote: > > There's no particular reason why the NVIC should be owning the > SysTick device objects; move them into the ARMv7M container object > instead, as part of consolidating the "create the devices which are > built into an M-profile CPU and map them into their architected > locations in the address space" work into one place. > > This involves temporarily creating a duplicate copy of the > nvic_sysreg_ns_ops struct and its read/write functions (renamed as > v7m_sysreg_ns_*), but we will delete the NVIC's copy of this code in > a subsequent patch. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Acked-by: Alistair Francis <alistair.francis@wdc.com> Alistair > --- > include/hw/arm/armv7m.h | 12 ++++ > include/hw/intc/armv7m_nvic.h | 4 -- > hw/arm/armv7m.c | 125 ++++++++++++++++++++++++++++++++++ > hw/intc/armv7m_nvic.c | 73 -------------------- > 4 files changed, 137 insertions(+), 77 deletions(-) > > diff --git a/include/hw/arm/armv7m.h b/include/hw/arm/armv7m.h > index 4cae0d7eeaa..360c35c5fb2 100644 > --- a/include/hw/arm/armv7m.h > +++ b/include/hw/arm/armv7m.h > @@ -60,11 +60,23 @@ struct ARMv7MState { > BitBandState bitband[ARMV7M_NUM_BITBANDS]; > ARMCPU *cpu; > ARMv7MRAS ras; > + SysTickState systick[M_REG_NUM_BANKS]; > > /* MemoryRegion we pass to the CPU, with our devices layered on > * top of the ones the board provides in board_memory. > */ > MemoryRegion container; > + /* > + * MemoryRegion which passes the transaction to either the S or the > + * NS systick device depending on the transaction attributes > + */ > + MemoryRegion systickmem; > + /* > + * MemoryRegion which enforces the S/NS handling of the systick > + * device NS alias region and passes the transaction to the > + * NS systick device if appropriate. > + */ > + MemoryRegion systick_ns_mem; > > /* Properties */ > char *cpu_type; > diff --git a/include/hw/intc/armv7m_nvic.h b/include/hw/intc/armv7m_nvic.h > index 33b6d8810c7..6a6a99090c7 100644 > --- a/include/hw/intc/armv7m_nvic.h > +++ b/include/hw/intc/armv7m_nvic.h > @@ -81,16 +81,12 @@ struct NVICState { > > MemoryRegion sysregmem; > MemoryRegion sysreg_ns_mem; > - MemoryRegion systickmem; > - MemoryRegion systick_ns_mem; > MemoryRegion container; > MemoryRegion defaultmem; > > uint32_t num_irq; > qemu_irq excpout; > qemu_irq sysresetreq; > - > - SysTickState systick[M_REG_NUM_BANKS]; > }; > > #endif > diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c > index 8964730d153..364ac069702 100644 > --- a/hw/arm/armv7m.c > +++ b/hw/arm/armv7m.c > @@ -124,6 +124,85 @@ static const hwaddr bitband_output_addr[ARMV7M_NUM_BITBANDS] = { > 0x22000000, 0x42000000 > }; > > +static MemTxResult v7m_sysreg_ns_write(void *opaque, hwaddr addr, > + uint64_t value, unsigned size, > + MemTxAttrs attrs) > +{ > + MemoryRegion *mr = opaque; > + > + if (attrs.secure) { > + /* S accesses to the alias act like NS accesses to the real region */ > + attrs.secure = 0; > + return memory_region_dispatch_write(mr, addr, value, > + size_memop(size) | MO_TE, attrs); > + } else { > + /* NS attrs are RAZ/WI for privileged, and BusFault for user */ > + if (attrs.user) { > + return MEMTX_ERROR; > + } > + return MEMTX_OK; > + } > +} > + > +static MemTxResult v7m_sysreg_ns_read(void *opaque, hwaddr addr, > + uint64_t *data, unsigned size, > + MemTxAttrs attrs) > +{ > + MemoryRegion *mr = opaque; > + > + if (attrs.secure) { > + /* S accesses to the alias act like NS accesses to the real region */ > + attrs.secure = 0; > + return memory_region_dispatch_read(mr, addr, data, > + size_memop(size) | MO_TE, attrs); > + } else { > + /* NS attrs are RAZ/WI for privileged, and BusFault for user */ > + if (attrs.user) { > + return MEMTX_ERROR; > + } > + *data = 0; > + return MEMTX_OK; > + } > +} > + > +static const MemoryRegionOps v7m_sysreg_ns_ops = { > + .read_with_attrs = v7m_sysreg_ns_read, > + .write_with_attrs = v7m_sysreg_ns_write, > + .endianness = DEVICE_NATIVE_ENDIAN, > +}; > + > +static MemTxResult v7m_systick_write(void *opaque, hwaddr addr, > + uint64_t value, unsigned size, > + MemTxAttrs attrs) > +{ > + ARMv7MState *s = opaque; > + MemoryRegion *mr; > + > + /* Direct the access to the correct systick */ > + mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->systick[attrs.secure]), 0); > + return memory_region_dispatch_write(mr, addr, value, > + size_memop(size) | MO_TE, attrs); > +} > + > +static MemTxResult v7m_systick_read(void *opaque, hwaddr addr, > + uint64_t *data, unsigned size, > + MemTxAttrs attrs) > +{ > + ARMv7MState *s = opaque; > + MemoryRegion *mr; > + > + /* Direct the access to the correct systick */ > + mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->systick[attrs.secure]), 0); > + return memory_region_dispatch_read(mr, addr, data, size_memop(size) | MO_TE, > + attrs); > +} > + > +static const MemoryRegionOps v7m_systick_ops = { > + .read_with_attrs = v7m_systick_read, > + .write_with_attrs = v7m_systick_write, > + .endianness = DEVICE_NATIVE_ENDIAN, > +}; > + > static void armv7m_instance_init(Object *obj) > { > ARMv7MState *s = ARMV7M(obj); > @@ -137,6 +216,13 @@ static void armv7m_instance_init(Object *obj) > object_property_add_alias(obj, "num-irq", > OBJECT(&s->nvic), "num-irq"); > > + object_initialize_child(obj, "systick-reg-ns", &s->systick[M_REG_NS], > + TYPE_SYSTICK); > + /* > + * We can't initialize the secure systick here, as we don't know > + * yet if we need it. > + */ > + > for (i = 0; i < ARRAY_SIZE(s->bitband); i++) { > object_initialize_child(obj, "bitband[*]", &s->bitband[i], > TYPE_BITBAND); > @@ -231,6 +317,45 @@ static void armv7m_realize(DeviceState *dev, Error **errp) > memory_region_add_subregion(&s->container, 0xe0000000, > sysbus_mmio_get_region(sbd, 0)); > > + /* Create and map the systick devices */ > + if (!sysbus_realize(SYS_BUS_DEVICE(&s->systick[M_REG_NS]), errp)) { > + return; > + } > + sysbus_connect_irq(SYS_BUS_DEVICE(&s->systick[M_REG_NS]), 0, > + qdev_get_gpio_in_named(DEVICE(&s->nvic), > + "systick-trigger", M_REG_NS)); > + > + if (arm_feature(&s->cpu->env, ARM_FEATURE_M_SECURITY)) { > + /* > + * We couldn't init the secure systick device in instance_init > + * as we didn't know then if the CPU had the security extensions; > + * so we have to do it here. > + */ > + object_initialize_child(OBJECT(dev), "systick-reg-s", > + &s->systick[M_REG_S], TYPE_SYSTICK); > + > + if (!sysbus_realize(SYS_BUS_DEVICE(&s->systick[M_REG_S]), errp)) { > + return; > + } > + sysbus_connect_irq(SYS_BUS_DEVICE(&s->systick[M_REG_S]), 0, > + qdev_get_gpio_in_named(DEVICE(&s->nvic), > + "systick-trigger", M_REG_S)); > + } > + > + memory_region_init_io(&s->systickmem, OBJECT(s), > + &v7m_systick_ops, s, > + "v7m_systick", 0xe0); > + > + memory_region_add_subregion_overlap(&s->container, 0xe000e010, > + &s->systickmem, 1); > + if (arm_feature(&s->cpu->env, ARM_FEATURE_V8)) { > + memory_region_init_io(&s->systick_ns_mem, OBJECT(s), > + &v7m_sysreg_ns_ops, &s->systickmem, > + "v7m_systick_ns", 0xe0); > + memory_region_add_subregion_overlap(&s->container, 0xe002e010, > + &s->systick_ns_mem, 1); > + } > + > /* If the CPU has RAS support, create the RAS register block */ > if (cpu_isar_feature(aa32_ras, s->cpu)) { > object_initialize_child(OBJECT(dev), "armv7m-ras", > diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c > index a5975592dfa..2b3e79a3da9 100644 > --- a/hw/intc/armv7m_nvic.c > +++ b/hw/intc/armv7m_nvic.c > @@ -2517,38 +2517,6 @@ static const MemoryRegionOps nvic_sysreg_ns_ops = { > .endianness = DEVICE_NATIVE_ENDIAN, > }; > > -static MemTxResult nvic_systick_write(void *opaque, hwaddr addr, > - uint64_t value, unsigned size, > - MemTxAttrs attrs) > -{ > - NVICState *s = opaque; > - MemoryRegion *mr; > - > - /* Direct the access to the correct systick */ > - mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->systick[attrs.secure]), 0); > - return memory_region_dispatch_write(mr, addr, value, > - size_memop(size) | MO_TE, attrs); > -} > - > -static MemTxResult nvic_systick_read(void *opaque, hwaddr addr, > - uint64_t *data, unsigned size, > - MemTxAttrs attrs) > -{ > - NVICState *s = opaque; > - MemoryRegion *mr; > - > - /* Direct the access to the correct systick */ > - mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->systick[attrs.secure]), 0); > - return memory_region_dispatch_read(mr, addr, data, size_memop(size) | MO_TE, > - attrs); > -} > - > -static const MemoryRegionOps nvic_systick_ops = { > - .read_with_attrs = nvic_systick_read, > - .write_with_attrs = nvic_systick_write, > - .endianness = DEVICE_NATIVE_ENDIAN, > -}; > - > /* > * Unassigned portions of the PPB space are RAZ/WI for privileged > * accesses, and fault for non-privileged accesses. > @@ -2801,29 +2769,6 @@ static void armv7m_nvic_realize(DeviceState *dev, Error **errp) > > s->num_prio_bits = arm_feature(&s->cpu->env, ARM_FEATURE_V7) ? 8 : 2; > > - if (!sysbus_realize(SYS_BUS_DEVICE(&s->systick[M_REG_NS]), errp)) { > - return; > - } > - sysbus_connect_irq(SYS_BUS_DEVICE(&s->systick[M_REG_NS]), 0, > - qdev_get_gpio_in_named(dev, "systick-trigger", > - M_REG_NS)); > - > - if (arm_feature(&s->cpu->env, ARM_FEATURE_M_SECURITY)) { > - /* We couldn't init the secure systick device in instance_init > - * as we didn't know then if the CPU had the security extensions; > - * so we have to do it here. > - */ > - object_initialize_child(OBJECT(dev), "systick-reg-s", > - &s->systick[M_REG_S], TYPE_SYSTICK); > - > - if (!sysbus_realize(SYS_BUS_DEVICE(&s->systick[M_REG_S]), errp)) { > - return; > - } > - sysbus_connect_irq(SYS_BUS_DEVICE(&s->systick[M_REG_S]), 0, > - qdev_get_gpio_in_named(dev, "systick-trigger", > - M_REG_S)); > - } > - > /* > * This device provides a single sysbus memory region which > * represents the whole of the "System PPB" space. This is the > @@ -2877,23 +2822,11 @@ static void armv7m_nvic_realize(DeviceState *dev, Error **errp) > "nvic_sysregs", 0x1000); > memory_region_add_subregion(&s->container, 0xe000, &s->sysregmem); > > - memory_region_init_io(&s->systickmem, OBJECT(s), > - &nvic_systick_ops, s, > - "nvic_systick", 0xe0); > - > - memory_region_add_subregion_overlap(&s->container, 0xe010, > - &s->systickmem, 1); > - > if (arm_feature(&s->cpu->env, ARM_FEATURE_V8)) { > memory_region_init_io(&s->sysreg_ns_mem, OBJECT(s), > &nvic_sysreg_ns_ops, &s->sysregmem, > "nvic_sysregs_ns", 0x1000); > memory_region_add_subregion(&s->container, 0x2e000, &s->sysreg_ns_mem); > - memory_region_init_io(&s->systick_ns_mem, OBJECT(s), > - &nvic_sysreg_ns_ops, &s->systickmem, > - "nvic_systick_ns", 0xe0); > - memory_region_add_subregion_overlap(&s->container, 0x2e010, > - &s->systick_ns_mem, 1); > } > > sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->container); > @@ -2905,12 +2838,6 @@ static void armv7m_nvic_instance_init(Object *obj) > NVICState *nvic = NVIC(obj); > SysBusDevice *sbd = SYS_BUS_DEVICE(obj); > > - object_initialize_child(obj, "systick-reg-ns", &nvic->systick[M_REG_NS], > - TYPE_SYSTICK); > - /* We can't initialize the secure systick here, as we don't know > - * yet if we need it. > - */ > - > sysbus_init_irq(sbd, &nvic->excpout); > qdev_init_gpio_out_named(dev, &nvic->sysresetreq, "SYSRESETREQ", 1); > qdev_init_gpio_in_named(dev, nvic_systick_trigger, "systick-trigger", > -- > 2.20.1 > >
On 10:33 Thu 12 Aug , Peter Maydell wrote: > There's no particular reason why the NVIC should be owning the > SysTick device objects; move them into the ARMv7M container object > instead, as part of consolidating the "create the devices which are > built into an M-profile CPU and map them into their architected > locations in the address space" work into one place. > > This involves temporarily creating a duplicate copy of the > nvic_sysreg_ns_ops struct and its read/write functions (renamed as > v7m_sysreg_ns_*), but we will delete the NVIC's copy of this code in > a subsequent patch. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Luc Michel <luc@lmichel.fr> > --- > include/hw/arm/armv7m.h | 12 ++++ > include/hw/intc/armv7m_nvic.h | 4 -- > hw/arm/armv7m.c | 125 ++++++++++++++++++++++++++++++++++ > hw/intc/armv7m_nvic.c | 73 -------------------- > 4 files changed, 137 insertions(+), 77 deletions(-) > > diff --git a/include/hw/arm/armv7m.h b/include/hw/arm/armv7m.h > index 4cae0d7eeaa..360c35c5fb2 100644 > --- a/include/hw/arm/armv7m.h > +++ b/include/hw/arm/armv7m.h > @@ -60,11 +60,23 @@ struct ARMv7MState { > BitBandState bitband[ARMV7M_NUM_BITBANDS]; > ARMCPU *cpu; > ARMv7MRAS ras; > + SysTickState systick[M_REG_NUM_BANKS]; > > /* MemoryRegion we pass to the CPU, with our devices layered on > * top of the ones the board provides in board_memory. > */ > MemoryRegion container; > + /* > + * MemoryRegion which passes the transaction to either the S or the > + * NS systick device depending on the transaction attributes > + */ > + MemoryRegion systickmem; > + /* > + * MemoryRegion which enforces the S/NS handling of the systick > + * device NS alias region and passes the transaction to the > + * NS systick device if appropriate. > + */ > + MemoryRegion systick_ns_mem; > > /* Properties */ > char *cpu_type; > diff --git a/include/hw/intc/armv7m_nvic.h b/include/hw/intc/armv7m_nvic.h > index 33b6d8810c7..6a6a99090c7 100644 > --- a/include/hw/intc/armv7m_nvic.h > +++ b/include/hw/intc/armv7m_nvic.h > @@ -81,16 +81,12 @@ struct NVICState { > > MemoryRegion sysregmem; > MemoryRegion sysreg_ns_mem; > - MemoryRegion systickmem; > - MemoryRegion systick_ns_mem; > MemoryRegion container; > MemoryRegion defaultmem; > > uint32_t num_irq; > qemu_irq excpout; > qemu_irq sysresetreq; > - > - SysTickState systick[M_REG_NUM_BANKS]; > }; > > #endif > diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c > index 8964730d153..364ac069702 100644 > --- a/hw/arm/armv7m.c > +++ b/hw/arm/armv7m.c > @@ -124,6 +124,85 @@ static const hwaddr bitband_output_addr[ARMV7M_NUM_BITBANDS] = { > 0x22000000, 0x42000000 > }; > > +static MemTxResult v7m_sysreg_ns_write(void *opaque, hwaddr addr, > + uint64_t value, unsigned size, > + MemTxAttrs attrs) > +{ > + MemoryRegion *mr = opaque; > + > + if (attrs.secure) { > + /* S accesses to the alias act like NS accesses to the real region */ > + attrs.secure = 0; > + return memory_region_dispatch_write(mr, addr, value, > + size_memop(size) | MO_TE, attrs); > + } else { > + /* NS attrs are RAZ/WI for privileged, and BusFault for user */ > + if (attrs.user) { > + return MEMTX_ERROR; > + } > + return MEMTX_OK; > + } > +} > + > +static MemTxResult v7m_sysreg_ns_read(void *opaque, hwaddr addr, > + uint64_t *data, unsigned size, > + MemTxAttrs attrs) > +{ > + MemoryRegion *mr = opaque; > + > + if (attrs.secure) { > + /* S accesses to the alias act like NS accesses to the real region */ > + attrs.secure = 0; > + return memory_region_dispatch_read(mr, addr, data, > + size_memop(size) | MO_TE, attrs); > + } else { > + /* NS attrs are RAZ/WI for privileged, and BusFault for user */ > + if (attrs.user) { > + return MEMTX_ERROR; > + } > + *data = 0; > + return MEMTX_OK; > + } > +} > + > +static const MemoryRegionOps v7m_sysreg_ns_ops = { > + .read_with_attrs = v7m_sysreg_ns_read, > + .write_with_attrs = v7m_sysreg_ns_write, > + .endianness = DEVICE_NATIVE_ENDIAN, > +}; > + > +static MemTxResult v7m_systick_write(void *opaque, hwaddr addr, > + uint64_t value, unsigned size, > + MemTxAttrs attrs) > +{ > + ARMv7MState *s = opaque; > + MemoryRegion *mr; > + > + /* Direct the access to the correct systick */ > + mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->systick[attrs.secure]), 0); > + return memory_region_dispatch_write(mr, addr, value, > + size_memop(size) | MO_TE, attrs); > +} > + > +static MemTxResult v7m_systick_read(void *opaque, hwaddr addr, > + uint64_t *data, unsigned size, > + MemTxAttrs attrs) > +{ > + ARMv7MState *s = opaque; > + MemoryRegion *mr; > + > + /* Direct the access to the correct systick */ > + mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->systick[attrs.secure]), 0); > + return memory_region_dispatch_read(mr, addr, data, size_memop(size) | MO_TE, > + attrs); > +} > + > +static const MemoryRegionOps v7m_systick_ops = { > + .read_with_attrs = v7m_systick_read, > + .write_with_attrs = v7m_systick_write, > + .endianness = DEVICE_NATIVE_ENDIAN, > +}; > + > static void armv7m_instance_init(Object *obj) > { > ARMv7MState *s = ARMV7M(obj); > @@ -137,6 +216,13 @@ static void armv7m_instance_init(Object *obj) > object_property_add_alias(obj, "num-irq", > OBJECT(&s->nvic), "num-irq"); > > + object_initialize_child(obj, "systick-reg-ns", &s->systick[M_REG_NS], > + TYPE_SYSTICK); > + /* > + * We can't initialize the secure systick here, as we don't know > + * yet if we need it. > + */ > + > for (i = 0; i < ARRAY_SIZE(s->bitband); i++) { > object_initialize_child(obj, "bitband[*]", &s->bitband[i], > TYPE_BITBAND); > @@ -231,6 +317,45 @@ static void armv7m_realize(DeviceState *dev, Error **errp) > memory_region_add_subregion(&s->container, 0xe0000000, > sysbus_mmio_get_region(sbd, 0)); > > + /* Create and map the systick devices */ > + if (!sysbus_realize(SYS_BUS_DEVICE(&s->systick[M_REG_NS]), errp)) { > + return; > + } > + sysbus_connect_irq(SYS_BUS_DEVICE(&s->systick[M_REG_NS]), 0, > + qdev_get_gpio_in_named(DEVICE(&s->nvic), > + "systick-trigger", M_REG_NS)); > + > + if (arm_feature(&s->cpu->env, ARM_FEATURE_M_SECURITY)) { > + /* > + * We couldn't init the secure systick device in instance_init > + * as we didn't know then if the CPU had the security extensions; > + * so we have to do it here. > + */ > + object_initialize_child(OBJECT(dev), "systick-reg-s", > + &s->systick[M_REG_S], TYPE_SYSTICK); > + > + if (!sysbus_realize(SYS_BUS_DEVICE(&s->systick[M_REG_S]), errp)) { > + return; > + } > + sysbus_connect_irq(SYS_BUS_DEVICE(&s->systick[M_REG_S]), 0, > + qdev_get_gpio_in_named(DEVICE(&s->nvic), > + "systick-trigger", M_REG_S)); > + } > + > + memory_region_init_io(&s->systickmem, OBJECT(s), > + &v7m_systick_ops, s, > + "v7m_systick", 0xe0); > + > + memory_region_add_subregion_overlap(&s->container, 0xe000e010, > + &s->systickmem, 1); > + if (arm_feature(&s->cpu->env, ARM_FEATURE_V8)) { > + memory_region_init_io(&s->systick_ns_mem, OBJECT(s), > + &v7m_sysreg_ns_ops, &s->systickmem, > + "v7m_systick_ns", 0xe0); > + memory_region_add_subregion_overlap(&s->container, 0xe002e010, > + &s->systick_ns_mem, 1); > + } > + > /* If the CPU has RAS support, create the RAS register block */ > if (cpu_isar_feature(aa32_ras, s->cpu)) { > object_initialize_child(OBJECT(dev), "armv7m-ras", > diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c > index a5975592dfa..2b3e79a3da9 100644 > --- a/hw/intc/armv7m_nvic.c > +++ b/hw/intc/armv7m_nvic.c > @@ -2517,38 +2517,6 @@ static const MemoryRegionOps nvic_sysreg_ns_ops = { > .endianness = DEVICE_NATIVE_ENDIAN, > }; > > -static MemTxResult nvic_systick_write(void *opaque, hwaddr addr, > - uint64_t value, unsigned size, > - MemTxAttrs attrs) > -{ > - NVICState *s = opaque; > - MemoryRegion *mr; > - > - /* Direct the access to the correct systick */ > - mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->systick[attrs.secure]), 0); > - return memory_region_dispatch_write(mr, addr, value, > - size_memop(size) | MO_TE, attrs); > -} > - > -static MemTxResult nvic_systick_read(void *opaque, hwaddr addr, > - uint64_t *data, unsigned size, > - MemTxAttrs attrs) > -{ > - NVICState *s = opaque; > - MemoryRegion *mr; > - > - /* Direct the access to the correct systick */ > - mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->systick[attrs.secure]), 0); > - return memory_region_dispatch_read(mr, addr, data, size_memop(size) | MO_TE, > - attrs); > -} > - > -static const MemoryRegionOps nvic_systick_ops = { > - .read_with_attrs = nvic_systick_read, > - .write_with_attrs = nvic_systick_write, > - .endianness = DEVICE_NATIVE_ENDIAN, > -}; > - > /* > * Unassigned portions of the PPB space are RAZ/WI for privileged > * accesses, and fault for non-privileged accesses. > @@ -2801,29 +2769,6 @@ static void armv7m_nvic_realize(DeviceState *dev, Error **errp) > > s->num_prio_bits = arm_feature(&s->cpu->env, ARM_FEATURE_V7) ? 8 : 2; > > - if (!sysbus_realize(SYS_BUS_DEVICE(&s->systick[M_REG_NS]), errp)) { > - return; > - } > - sysbus_connect_irq(SYS_BUS_DEVICE(&s->systick[M_REG_NS]), 0, > - qdev_get_gpio_in_named(dev, "systick-trigger", > - M_REG_NS)); > - > - if (arm_feature(&s->cpu->env, ARM_FEATURE_M_SECURITY)) { > - /* We couldn't init the secure systick device in instance_init > - * as we didn't know then if the CPU had the security extensions; > - * so we have to do it here. > - */ > - object_initialize_child(OBJECT(dev), "systick-reg-s", > - &s->systick[M_REG_S], TYPE_SYSTICK); > - > - if (!sysbus_realize(SYS_BUS_DEVICE(&s->systick[M_REG_S]), errp)) { > - return; > - } > - sysbus_connect_irq(SYS_BUS_DEVICE(&s->systick[M_REG_S]), 0, > - qdev_get_gpio_in_named(dev, "systick-trigger", > - M_REG_S)); > - } > - > /* > * This device provides a single sysbus memory region which > * represents the whole of the "System PPB" space. This is the > @@ -2877,23 +2822,11 @@ static void armv7m_nvic_realize(DeviceState *dev, Error **errp) > "nvic_sysregs", 0x1000); > memory_region_add_subregion(&s->container, 0xe000, &s->sysregmem); > > - memory_region_init_io(&s->systickmem, OBJECT(s), > - &nvic_systick_ops, s, > - "nvic_systick", 0xe0); > - > - memory_region_add_subregion_overlap(&s->container, 0xe010, > - &s->systickmem, 1); > - > if (arm_feature(&s->cpu->env, ARM_FEATURE_V8)) { > memory_region_init_io(&s->sysreg_ns_mem, OBJECT(s), > &nvic_sysreg_ns_ops, &s->sysregmem, > "nvic_sysregs_ns", 0x1000); > memory_region_add_subregion(&s->container, 0x2e000, &s->sysreg_ns_mem); > - memory_region_init_io(&s->systick_ns_mem, OBJECT(s), > - &nvic_sysreg_ns_ops, &s->systickmem, > - "nvic_systick_ns", 0xe0); > - memory_region_add_subregion_overlap(&s->container, 0x2e010, > - &s->systick_ns_mem, 1); > } > > sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->container); > @@ -2905,12 +2838,6 @@ static void armv7m_nvic_instance_init(Object *obj) > NVICState *nvic = NVIC(obj); > SysBusDevice *sbd = SYS_BUS_DEVICE(obj); > > - object_initialize_child(obj, "systick-reg-ns", &nvic->systick[M_REG_NS], > - TYPE_SYSTICK); > - /* We can't initialize the secure systick here, as we don't know > - * yet if we need it. > - */ > - > sysbus_init_irq(sbd, &nvic->excpout); > qdev_init_gpio_out_named(dev, &nvic->sysresetreq, "SYSRESETREQ", 1); > qdev_init_gpio_in_named(dev, nvic_systick_trigger, "systick-trigger", > -- > 2.20.1 > --
diff --git a/include/hw/arm/armv7m.h b/include/hw/arm/armv7m.h index 4cae0d7eeaa..360c35c5fb2 100644 --- a/include/hw/arm/armv7m.h +++ b/include/hw/arm/armv7m.h @@ -60,11 +60,23 @@ struct ARMv7MState { BitBandState bitband[ARMV7M_NUM_BITBANDS]; ARMCPU *cpu; ARMv7MRAS ras; + SysTickState systick[M_REG_NUM_BANKS]; /* MemoryRegion we pass to the CPU, with our devices layered on * top of the ones the board provides in board_memory. */ MemoryRegion container; + /* + * MemoryRegion which passes the transaction to either the S or the + * NS systick device depending on the transaction attributes + */ + MemoryRegion systickmem; + /* + * MemoryRegion which enforces the S/NS handling of the systick + * device NS alias region and passes the transaction to the + * NS systick device if appropriate. + */ + MemoryRegion systick_ns_mem; /* Properties */ char *cpu_type; diff --git a/include/hw/intc/armv7m_nvic.h b/include/hw/intc/armv7m_nvic.h index 33b6d8810c7..6a6a99090c7 100644 --- a/include/hw/intc/armv7m_nvic.h +++ b/include/hw/intc/armv7m_nvic.h @@ -81,16 +81,12 @@ struct NVICState { MemoryRegion sysregmem; MemoryRegion sysreg_ns_mem; - MemoryRegion systickmem; - MemoryRegion systick_ns_mem; MemoryRegion container; MemoryRegion defaultmem; uint32_t num_irq; qemu_irq excpout; qemu_irq sysresetreq; - - SysTickState systick[M_REG_NUM_BANKS]; }; #endif diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c index 8964730d153..364ac069702 100644 --- a/hw/arm/armv7m.c +++ b/hw/arm/armv7m.c @@ -124,6 +124,85 @@ static const hwaddr bitband_output_addr[ARMV7M_NUM_BITBANDS] = { 0x22000000, 0x42000000 }; +static MemTxResult v7m_sysreg_ns_write(void *opaque, hwaddr addr, + uint64_t value, unsigned size, + MemTxAttrs attrs) +{ + MemoryRegion *mr = opaque; + + if (attrs.secure) { + /* S accesses to the alias act like NS accesses to the real region */ + attrs.secure = 0; + return memory_region_dispatch_write(mr, addr, value, + size_memop(size) | MO_TE, attrs); + } else { + /* NS attrs are RAZ/WI for privileged, and BusFault for user */ + if (attrs.user) { + return MEMTX_ERROR; + } + return MEMTX_OK; + } +} + +static MemTxResult v7m_sysreg_ns_read(void *opaque, hwaddr addr, + uint64_t *data, unsigned size, + MemTxAttrs attrs) +{ + MemoryRegion *mr = opaque; + + if (attrs.secure) { + /* S accesses to the alias act like NS accesses to the real region */ + attrs.secure = 0; + return memory_region_dispatch_read(mr, addr, data, + size_memop(size) | MO_TE, attrs); + } else { + /* NS attrs are RAZ/WI for privileged, and BusFault for user */ + if (attrs.user) { + return MEMTX_ERROR; + } + *data = 0; + return MEMTX_OK; + } +} + +static const MemoryRegionOps v7m_sysreg_ns_ops = { + .read_with_attrs = v7m_sysreg_ns_read, + .write_with_attrs = v7m_sysreg_ns_write, + .endianness = DEVICE_NATIVE_ENDIAN, +}; + +static MemTxResult v7m_systick_write(void *opaque, hwaddr addr, + uint64_t value, unsigned size, + MemTxAttrs attrs) +{ + ARMv7MState *s = opaque; + MemoryRegion *mr; + + /* Direct the access to the correct systick */ + mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->systick[attrs.secure]), 0); + return memory_region_dispatch_write(mr, addr, value, + size_memop(size) | MO_TE, attrs); +} + +static MemTxResult v7m_systick_read(void *opaque, hwaddr addr, + uint64_t *data, unsigned size, + MemTxAttrs attrs) +{ + ARMv7MState *s = opaque; + MemoryRegion *mr; + + /* Direct the access to the correct systick */ + mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->systick[attrs.secure]), 0); + return memory_region_dispatch_read(mr, addr, data, size_memop(size) | MO_TE, + attrs); +} + +static const MemoryRegionOps v7m_systick_ops = { + .read_with_attrs = v7m_systick_read, + .write_with_attrs = v7m_systick_write, + .endianness = DEVICE_NATIVE_ENDIAN, +}; + static void armv7m_instance_init(Object *obj) { ARMv7MState *s = ARMV7M(obj); @@ -137,6 +216,13 @@ static void armv7m_instance_init(Object *obj) object_property_add_alias(obj, "num-irq", OBJECT(&s->nvic), "num-irq"); + object_initialize_child(obj, "systick-reg-ns", &s->systick[M_REG_NS], + TYPE_SYSTICK); + /* + * We can't initialize the secure systick here, as we don't know + * yet if we need it. + */ + for (i = 0; i < ARRAY_SIZE(s->bitband); i++) { object_initialize_child(obj, "bitband[*]", &s->bitband[i], TYPE_BITBAND); @@ -231,6 +317,45 @@ static void armv7m_realize(DeviceState *dev, Error **errp) memory_region_add_subregion(&s->container, 0xe0000000, sysbus_mmio_get_region(sbd, 0)); + /* Create and map the systick devices */ + if (!sysbus_realize(SYS_BUS_DEVICE(&s->systick[M_REG_NS]), errp)) { + return; + } + sysbus_connect_irq(SYS_BUS_DEVICE(&s->systick[M_REG_NS]), 0, + qdev_get_gpio_in_named(DEVICE(&s->nvic), + "systick-trigger", M_REG_NS)); + + if (arm_feature(&s->cpu->env, ARM_FEATURE_M_SECURITY)) { + /* + * We couldn't init the secure systick device in instance_init + * as we didn't know then if the CPU had the security extensions; + * so we have to do it here. + */ + object_initialize_child(OBJECT(dev), "systick-reg-s", + &s->systick[M_REG_S], TYPE_SYSTICK); + + if (!sysbus_realize(SYS_BUS_DEVICE(&s->systick[M_REG_S]), errp)) { + return; + } + sysbus_connect_irq(SYS_BUS_DEVICE(&s->systick[M_REG_S]), 0, + qdev_get_gpio_in_named(DEVICE(&s->nvic), + "systick-trigger", M_REG_S)); + } + + memory_region_init_io(&s->systickmem, OBJECT(s), + &v7m_systick_ops, s, + "v7m_systick", 0xe0); + + memory_region_add_subregion_overlap(&s->container, 0xe000e010, + &s->systickmem, 1); + if (arm_feature(&s->cpu->env, ARM_FEATURE_V8)) { + memory_region_init_io(&s->systick_ns_mem, OBJECT(s), + &v7m_sysreg_ns_ops, &s->systickmem, + "v7m_systick_ns", 0xe0); + memory_region_add_subregion_overlap(&s->container, 0xe002e010, + &s->systick_ns_mem, 1); + } + /* If the CPU has RAS support, create the RAS register block */ if (cpu_isar_feature(aa32_ras, s->cpu)) { object_initialize_child(OBJECT(dev), "armv7m-ras", diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index a5975592dfa..2b3e79a3da9 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -2517,38 +2517,6 @@ static const MemoryRegionOps nvic_sysreg_ns_ops = { .endianness = DEVICE_NATIVE_ENDIAN, }; -static MemTxResult nvic_systick_write(void *opaque, hwaddr addr, - uint64_t value, unsigned size, - MemTxAttrs attrs) -{ - NVICState *s = opaque; - MemoryRegion *mr; - - /* Direct the access to the correct systick */ - mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->systick[attrs.secure]), 0); - return memory_region_dispatch_write(mr, addr, value, - size_memop(size) | MO_TE, attrs); -} - -static MemTxResult nvic_systick_read(void *opaque, hwaddr addr, - uint64_t *data, unsigned size, - MemTxAttrs attrs) -{ - NVICState *s = opaque; - MemoryRegion *mr; - - /* Direct the access to the correct systick */ - mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->systick[attrs.secure]), 0); - return memory_region_dispatch_read(mr, addr, data, size_memop(size) | MO_TE, - attrs); -} - -static const MemoryRegionOps nvic_systick_ops = { - .read_with_attrs = nvic_systick_read, - .write_with_attrs = nvic_systick_write, - .endianness = DEVICE_NATIVE_ENDIAN, -}; - /* * Unassigned portions of the PPB space are RAZ/WI for privileged * accesses, and fault for non-privileged accesses. @@ -2801,29 +2769,6 @@ static void armv7m_nvic_realize(DeviceState *dev, Error **errp) s->num_prio_bits = arm_feature(&s->cpu->env, ARM_FEATURE_V7) ? 8 : 2; - if (!sysbus_realize(SYS_BUS_DEVICE(&s->systick[M_REG_NS]), errp)) { - return; - } - sysbus_connect_irq(SYS_BUS_DEVICE(&s->systick[M_REG_NS]), 0, - qdev_get_gpio_in_named(dev, "systick-trigger", - M_REG_NS)); - - if (arm_feature(&s->cpu->env, ARM_FEATURE_M_SECURITY)) { - /* We couldn't init the secure systick device in instance_init - * as we didn't know then if the CPU had the security extensions; - * so we have to do it here. - */ - object_initialize_child(OBJECT(dev), "systick-reg-s", - &s->systick[M_REG_S], TYPE_SYSTICK); - - if (!sysbus_realize(SYS_BUS_DEVICE(&s->systick[M_REG_S]), errp)) { - return; - } - sysbus_connect_irq(SYS_BUS_DEVICE(&s->systick[M_REG_S]), 0, - qdev_get_gpio_in_named(dev, "systick-trigger", - M_REG_S)); - } - /* * This device provides a single sysbus memory region which * represents the whole of the "System PPB" space. This is the @@ -2877,23 +2822,11 @@ static void armv7m_nvic_realize(DeviceState *dev, Error **errp) "nvic_sysregs", 0x1000); memory_region_add_subregion(&s->container, 0xe000, &s->sysregmem); - memory_region_init_io(&s->systickmem, OBJECT(s), - &nvic_systick_ops, s, - "nvic_systick", 0xe0); - - memory_region_add_subregion_overlap(&s->container, 0xe010, - &s->systickmem, 1); - if (arm_feature(&s->cpu->env, ARM_FEATURE_V8)) { memory_region_init_io(&s->sysreg_ns_mem, OBJECT(s), &nvic_sysreg_ns_ops, &s->sysregmem, "nvic_sysregs_ns", 0x1000); memory_region_add_subregion(&s->container, 0x2e000, &s->sysreg_ns_mem); - memory_region_init_io(&s->systick_ns_mem, OBJECT(s), - &nvic_sysreg_ns_ops, &s->systickmem, - "nvic_systick_ns", 0xe0); - memory_region_add_subregion_overlap(&s->container, 0x2e010, - &s->systick_ns_mem, 1); } sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->container); @@ -2905,12 +2838,6 @@ static void armv7m_nvic_instance_init(Object *obj) NVICState *nvic = NVIC(obj); SysBusDevice *sbd = SYS_BUS_DEVICE(obj); - object_initialize_child(obj, "systick-reg-ns", &nvic->systick[M_REG_NS], - TYPE_SYSTICK); - /* We can't initialize the secure systick here, as we don't know - * yet if we need it. - */ - sysbus_init_irq(sbd, &nvic->excpout); qdev_init_gpio_out_named(dev, &nvic->sysresetreq, "SYSRESETREQ", 1); qdev_init_gpio_in_named(dev, nvic_systick_trigger, "systick-trigger",
There's no particular reason why the NVIC should be owning the SysTick device objects; move them into the ARMv7M container object instead, as part of consolidating the "create the devices which are built into an M-profile CPU and map them into their architected locations in the address space" work into one place. This involves temporarily creating a duplicate copy of the nvic_sysreg_ns_ops struct and its read/write functions (renamed as v7m_sysreg_ns_*), but we will delete the NVIC's copy of this code in a subsequent patch. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/arm/armv7m.h | 12 ++++ include/hw/intc/armv7m_nvic.h | 4 -- hw/arm/armv7m.c | 125 ++++++++++++++++++++++++++++++++++ hw/intc/armv7m_nvic.c | 73 -------------------- 4 files changed, 137 insertions(+), 77 deletions(-) -- 2.20.1