* [Qemu-devel] [PATCH 0/2] v8m: make systick banked @ 2017-12-01 18:51 Peter Maydell 2017-12-01 18:51 ` [Qemu-devel] [PATCH 1/2] nvic: Make nvic_sysreg_ns_ops work with any MemoryRegion Peter Maydell 2017-12-01 18:51 ` [Qemu-devel] [PATCH 2/2] nvic: Make systick banked Peter Maydell 0 siblings, 2 replies; 6+ messages in thread From: Peter Maydell @ 2017-12-01 18:51 UTC (permalink / raw) To: qemu-arm, qemu-devel Cc: patches, Richard Henderson, Alex Bennée, Paolo Bonzini This patchset makes the systick device be banked between security states for ARM v8M. It is perhaps not the absolute cleanest way to implement this banking, but the alternative (which I had an irc discussion with Paolo about some weeks back) would involve adding a feature to our IOMMU abstraction to allow them to change transaction attributes, refactoring the NVIC so that it exposes memory regions for its S and NS views separately (to be mapped into the CPU's S and NS address spaces by the board code), and then using an IOMMU region to implement the "S alias that behaves like NS" memory region. This version is less than 100 lines by diffstat :-) thanks -- PMM Peter Maydell (2): nvic: Make nvic_sysreg_ns_ops work with any MemoryRegion nvic: Make systick banked include/hw/intc/armv7m_nvic.h | 4 +- hw/intc/armv7m_nvic.c | 91 ++++++++++++++++++++++++++++++++++++------- 2 files changed, 79 insertions(+), 16 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH 1/2] nvic: Make nvic_sysreg_ns_ops work with any MemoryRegion 2017-12-01 18:51 [Qemu-devel] [PATCH 0/2] v8m: make systick banked Peter Maydell @ 2017-12-01 18:51 ` Peter Maydell 2017-12-05 3:52 ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé 2017-12-01 18:51 ` [Qemu-devel] [PATCH 2/2] nvic: Make systick banked Peter Maydell 1 sibling, 1 reply; 6+ messages in thread From: Peter Maydell @ 2017-12-01 18:51 UTC (permalink / raw) To: qemu-arm, qemu-devel Cc: patches, Richard Henderson, Alex Bennée, Paolo Bonzini Generalize nvic_sysreg_ns_ops so that we can pass it an arbitrary MemoryRegion which it will use as the underlying register implementation to apply the NS-alias behaviour to. We'll want this so we can do the same with systick. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/intc/armv7m_nvic.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index 5d9c883..63f2743 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -1786,10 +1786,12 @@ static MemTxResult nvic_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 nvic_sysreg_write(opaque, addr, value, size, attrs); + return memory_region_dispatch_write(mr, addr, value, size, attrs); } else { /* NS attrs are RAZ/WI for privileged, and BusFault for user */ if (attrs.user) { @@ -1803,10 +1805,12 @@ static MemTxResult nvic_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 nvic_sysreg_read(opaque, addr, data, size, attrs); + return memory_region_dispatch_read(mr, addr, data, size, attrs); } else { /* NS attrs are RAZ/WI for privileged, and BusFault for user */ if (attrs.user) { @@ -2075,7 +2079,7 @@ static void armv7m_nvic_realize(DeviceState *dev, Error **errp) if (arm_feature(&s->cpu->env, ARM_FEATURE_V8)) { memory_region_init_io(&s->sysreg_ns_mem, OBJECT(s), - &nvic_sysreg_ns_ops, s, + &nvic_sysreg_ns_ops, &s->sysregmem, "nvic_sysregs_ns", 0x1000); memory_region_add_subregion(&s->container, 0x20000, &s->sysreg_ns_mem); } -- 2.7.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [Qemu-arm] [PATCH 1/2] nvic: Make nvic_sysreg_ns_ops work with any MemoryRegion 2017-12-01 18:51 ` [Qemu-devel] [PATCH 1/2] nvic: Make nvic_sysreg_ns_ops work with any MemoryRegion Peter Maydell @ 2017-12-05 3:52 ` Philippe Mathieu-Daudé 0 siblings, 0 replies; 6+ messages in thread From: Philippe Mathieu-Daudé @ 2017-12-05 3:52 UTC (permalink / raw) To: Peter Maydell, qemu-arm, qemu-devel Cc: Paolo Bonzini, Richard Henderson, patches On 12/01/2017 03:51 PM, Peter Maydell wrote: > Generalize nvic_sysreg_ns_ops so that we can pass it an > arbitrary MemoryRegion which it will use as the underlying > register implementation to apply the NS-alias behaviour > to. We'll want this so we can do the same with systick. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > hw/intc/armv7m_nvic.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c > index 5d9c883..63f2743 100644 > --- a/hw/intc/armv7m_nvic.c > +++ b/hw/intc/armv7m_nvic.c > @@ -1786,10 +1786,12 @@ static MemTxResult nvic_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 nvic_sysreg_write(opaque, addr, value, size, attrs); > + return memory_region_dispatch_write(mr, addr, value, size, attrs); > } else { > /* NS attrs are RAZ/WI for privileged, and BusFault for user */ > if (attrs.user) { > @@ -1803,10 +1805,12 @@ static MemTxResult nvic_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 nvic_sysreg_read(opaque, addr, data, size, attrs); > + return memory_region_dispatch_read(mr, addr, data, size, attrs); > } else { > /* NS attrs are RAZ/WI for privileged, and BusFault for user */ > if (attrs.user) { > @@ -2075,7 +2079,7 @@ static void armv7m_nvic_realize(DeviceState *dev, Error **errp) > > if (arm_feature(&s->cpu->env, ARM_FEATURE_V8)) { > memory_region_init_io(&s->sysreg_ns_mem, OBJECT(s), > - &nvic_sysreg_ns_ops, s, > + &nvic_sysreg_ns_ops, &s->sysregmem, > "nvic_sysregs_ns", 0x1000); > memory_region_add_subregion(&s->container, 0x20000, &s->sysreg_ns_mem); > } > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH 2/2] nvic: Make systick banked 2017-12-01 18:51 [Qemu-devel] [PATCH 0/2] v8m: make systick banked Peter Maydell 2017-12-01 18:51 ` [Qemu-devel] [PATCH 1/2] nvic: Make nvic_sysreg_ns_ops work with any MemoryRegion Peter Maydell @ 2017-12-01 18:51 ` Peter Maydell 2017-12-05 4:13 ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé 1 sibling, 1 reply; 6+ messages in thread From: Peter Maydell @ 2017-12-01 18:51 UTC (permalink / raw) To: qemu-arm, qemu-devel Cc: patches, Richard Henderson, Alex Bennée, Paolo Bonzini For the v8M security extension, there should be two systick devices, which use separate banked systick exceptions. The register interface is banked in the same way as for other banked registers, including the existence of an NS alias region for secure code to access the nonsecure timer. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/intc/armv7m_nvic.h | 4 ++- hw/intc/armv7m_nvic.c | 81 ++++++++++++++++++++++++++++++++++++------- 2 files changed, 72 insertions(+), 13 deletions(-) diff --git a/include/hw/intc/armv7m_nvic.h b/include/hw/intc/armv7m_nvic.h index ac7997c..8bc2911 100644 --- a/include/hw/intc/armv7m_nvic.h +++ b/include/hw/intc/armv7m_nvic.h @@ -78,13 +78,15 @@ typedef struct NVICState { MemoryRegion sysregmem; MemoryRegion sysreg_ns_mem; + MemoryRegion systickmem; + MemoryRegion systick_ns_mem; MemoryRegion container; uint32_t num_irq; qemu_irq excpout; qemu_irq sysresetreq; - SysTickState systick; + SysTickState systick[M_REG_NUM_BANKS]; } NVICState; #endif diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index 63f2743..5cb44f4 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -1827,6 +1827,36 @@ 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, 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, attrs); +} + +static const MemoryRegionOps nvic_systick_ops = { + .read_with_attrs = nvic_systick_read, + .write_with_attrs = nvic_systick_write, + .endianness = DEVICE_NATIVE_ENDIAN, +}; + static int nvic_post_load(void *opaque, int version_id) { NVICState *s = opaque; @@ -2005,17 +2035,16 @@ static void nvic_systick_trigger(void *opaque, int n, int level) /* SysTick just asked us to pend its exception. * (This is different from an external interrupt line's * behaviour.) - * TODO: when we implement the banked systicks we must make - * this pend the correct banked exception. + * n == 0 : NonSecure systick + * n == 1 : Secure systick */ - armv7m_nvic_set_pending(s, ARMV7M_EXCP_SYSTICK, false); + armv7m_nvic_set_pending(s, ARMV7M_EXCP_SYSTICK, n); } } static void armv7m_nvic_realize(DeviceState *dev, Error **errp) { NVICState *s = NVIC(dev); - SysBusDevice *systick_sbd; Error *err = NULL; int regionlen; @@ -2032,15 +2061,30 @@ static void armv7m_nvic_realize(DeviceState *dev, Error **errp) /* include space for internal exception vectors */ s->num_irq += NVIC_FIRST_IRQ; - object_property_set_bool(OBJECT(&s->systick), true, "realized", &err); + object_property_set_bool(OBJECT(&s->systick[M_REG_NS]), true, + "realized", &err); if (err != NULL) { error_propagate(errp, err); return; } - systick_sbd = SYS_BUS_DEVICE(&s->systick); - sysbus_connect_irq(systick_sbd, 0, + sysbus_connect_irq(SYS_BUS_DEVICE(&s->systick[M_REG_NS]), 0, qdev_get_gpio_in_named(dev, "systick-trigger", 0)); + if (arm_feature(&s->cpu->env, ARM_FEATURE_M_SECURITY)) { + object_initialize(&s->systick[M_REG_S], sizeof(s->systick[M_REG_NS]), + TYPE_SYSTICK); + qdev_set_parent_bus(DEVICE(&s->systick[M_REG_S]), sysbus_get_default()); + + object_property_set_bool(OBJECT(&s->systick[M_REG_S]), true, + "realized", &err); + if (err != NULL) { + error_propagate(errp, err); + return; + } + sysbus_connect_irq(SYS_BUS_DEVICE(&s->systick[M_REG_S]), 0, + qdev_get_gpio_in_named(dev, "systick-trigger", 1)); + } + /* The NVIC and System Control Space (SCS) starts at 0xe000e000 * and looks like this: * 0x004 - ICTR @@ -2073,15 +2117,24 @@ static void armv7m_nvic_realize(DeviceState *dev, Error **errp) memory_region_init_io(&s->sysregmem, OBJECT(s), &nvic_sysreg_ops, s, "nvic_sysregs", 0x1000); memory_region_add_subregion(&s->container, 0, &s->sysregmem); + + memory_region_init_io(&s->systickmem, OBJECT(s), + &nvic_systick_ops, s, + "nvic_systick", 0xe0); + memory_region_add_subregion_overlap(&s->container, 0x10, - sysbus_mmio_get_region(systick_sbd, 0), - 1); + &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, 0x20000, &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, 0x20010, + &s->systick_ns_mem, 1); } sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->container); @@ -2099,12 +2152,16 @@ static void armv7m_nvic_instance_init(Object *obj) NVICState *nvic = NVIC(obj); SysBusDevice *sbd = SYS_BUS_DEVICE(obj); - object_initialize(&nvic->systick, sizeof(nvic->systick), TYPE_SYSTICK); - qdev_set_parent_bus(DEVICE(&nvic->systick), sysbus_get_default()); + object_initialize(&nvic->systick[M_REG_NS], + sizeof(nvic->systick[M_REG_NS]), TYPE_SYSTICK); + qdev_set_parent_bus(DEVICE(&nvic->systick[M_REG_NS]), sysbus_get_default()); + /* 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", 1); + qdev_init_gpio_in_named(dev, nvic_systick_trigger, "systick-trigger", 2); } static void armv7m_nvic_class_init(ObjectClass *klass, void *data) -- 2.7.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [Qemu-arm] [PATCH 2/2] nvic: Make systick banked 2017-12-01 18:51 ` [Qemu-devel] [PATCH 2/2] nvic: Make systick banked Peter Maydell @ 2017-12-05 4:13 ` Philippe Mathieu-Daudé 2017-12-12 17:22 ` Peter Maydell 0 siblings, 1 reply; 6+ messages in thread From: Philippe Mathieu-Daudé @ 2017-12-05 4:13 UTC (permalink / raw) To: Peter Maydell, qemu-arm, qemu-devel Cc: Paolo Bonzini, Richard Henderson, patches Hi Peter, On 12/01/2017 03:51 PM, Peter Maydell wrote: > For the v8M security extension, there should be two systick > devices, which use separate banked systick exceptions. The > register interface is banked in the same way as for other > banked registers, including the existence of an NS alias > region for secure code to access the nonsecure timer. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > include/hw/intc/armv7m_nvic.h | 4 ++- > hw/intc/armv7m_nvic.c | 81 ++++++++++++++++++++++++++++++++++++------- > 2 files changed, 72 insertions(+), 13 deletions(-) > > diff --git a/include/hw/intc/armv7m_nvic.h b/include/hw/intc/armv7m_nvic.h > index ac7997c..8bc2911 100644 > --- a/include/hw/intc/armv7m_nvic.h > +++ b/include/hw/intc/armv7m_nvic.h > @@ -78,13 +78,15 @@ typedef struct NVICState { > > MemoryRegion sysregmem; > MemoryRegion sysreg_ns_mem; > + MemoryRegion systickmem; > + MemoryRegion systick_ns_mem; > MemoryRegion container; > > uint32_t num_irq; > qemu_irq excpout; > qemu_irq sysresetreq; > > - SysTickState systick; > + SysTickState systick[M_REG_NUM_BANKS]; > } NVICState; > > #endif > diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c > index 63f2743..5cb44f4 100644 > --- a/hw/intc/armv7m_nvic.c > +++ b/hw/intc/armv7m_nvic.c > @@ -1827,6 +1827,36 @@ 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, 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, attrs); > +} > + > +static const MemoryRegionOps nvic_systick_ops = { > + .read_with_attrs = nvic_systick_read, > + .write_with_attrs = nvic_systick_write, > + .endianness = DEVICE_NATIVE_ENDIAN, > +}; > + > static int nvic_post_load(void *opaque, int version_id) > { > NVICState *s = opaque; > @@ -2005,17 +2035,16 @@ static void nvic_systick_trigger(void *opaque, int n, int level) > /* SysTick just asked us to pend its exception. > * (This is different from an external interrupt line's > * behaviour.) > - * TODO: when we implement the banked systicks we must make > - * this pend the correct banked exception. > + * n == 0 : NonSecure systick > + * n == 1 : Secure systick > */ > - armv7m_nvic_set_pending(s, ARMV7M_EXCP_SYSTICK, false); > + armv7m_nvic_set_pending(s, ARMV7M_EXCP_SYSTICK, n); > } > } > > static void armv7m_nvic_realize(DeviceState *dev, Error **errp) > { > NVICState *s = NVIC(dev); > - SysBusDevice *systick_sbd; > Error *err = NULL; > int regionlen; > > @@ -2032,15 +2061,30 @@ static void armv7m_nvic_realize(DeviceState *dev, Error **errp) > /* include space for internal exception vectors */ > s->num_irq += NVIC_FIRST_IRQ; > > - object_property_set_bool(OBJECT(&s->systick), true, "realized", &err); > + object_property_set_bool(OBJECT(&s->systick[M_REG_NS]), true, > + "realized", &err); > if (err != NULL) { > error_propagate(errp, err); > return; > } > - systick_sbd = SYS_BUS_DEVICE(&s->systick); > - sysbus_connect_irq(systick_sbd, 0, > + sysbus_connect_irq(SYS_BUS_DEVICE(&s->systick[M_REG_NS]), 0, > qdev_get_gpio_in_named(dev, "systick-trigger", 0)); > > + if (arm_feature(&s->cpu->env, ARM_FEATURE_M_SECURITY)) { > + object_initialize(&s->systick[M_REG_S], sizeof(s->systick[M_REG_NS]), > + TYPE_SYSTICK); > + qdev_set_parent_bus(DEVICE(&s->systick[M_REG_S]), sysbus_get_default()); I got stuck here some time wondering why you init the S systick in this realize(), then continue and read your comment in armv7m_nvic_instance_init() and it made sens. I'm tempted to add a similar comment here. > + > + object_property_set_bool(OBJECT(&s->systick[M_REG_S]), true, > + "realized", &err); > + if (err != NULL) { > + error_propagate(errp, err); > + return; > + } > + sysbus_connect_irq(SYS_BUS_DEVICE(&s->systick[M_REG_S]), 0, > + qdev_get_gpio_in_named(dev, "systick-trigger", 1)); > + } > + > /* The NVIC and System Control Space (SCS) starts at 0xe000e000 > * and looks like this: > * 0x004 - ICTR > @@ -2073,15 +2117,24 @@ static void armv7m_nvic_realize(DeviceState *dev, Error **errp) > memory_region_init_io(&s->sysregmem, OBJECT(s), &nvic_sysreg_ops, s, > "nvic_sysregs", 0x1000); > memory_region_add_subregion(&s->container, 0, &s->sysregmem); > + > + memory_region_init_io(&s->systickmem, OBJECT(s), > + &nvic_systick_ops, s, > + "nvic_systick", 0xe0); > + > memory_region_add_subregion_overlap(&s->container, 0x10, > - sysbus_mmio_get_region(systick_sbd, 0), > - 1); > + &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, 0x20000, &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, 0x20010, > + &s->systick_ns_mem, 1); > } > > sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->container); > @@ -2099,12 +2152,16 @@ static void armv7m_nvic_instance_init(Object *obj) > NVICState *nvic = NVIC(obj); > SysBusDevice *sbd = SYS_BUS_DEVICE(obj); > > - object_initialize(&nvic->systick, sizeof(nvic->systick), TYPE_SYSTICK); > - qdev_set_parent_bus(DEVICE(&nvic->systick), sysbus_get_default()); > + object_initialize(&nvic->systick[M_REG_NS], > + sizeof(nvic->systick[M_REG_NS]), TYPE_SYSTICK); > + qdev_set_parent_bus(DEVICE(&nvic->systick[M_REG_NS]), sysbus_get_default()); > + /* 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", 1); > + qdev_init_gpio_in_named(dev, nvic_systick_trigger, "systick-trigger", 2); Using this #define helps (me) while reviewing: qdev_init_gpio_in_named(dev, nvic_systick_trigger, "systick-trigger", M_REG_NUM_BANKS); One more line but you are still under a 100 diffstat series ;) Anyway: Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > } > > static void armv7m_nvic_class_init(ObjectClass *klass, void *data) > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [Qemu-arm] [PATCH 2/2] nvic: Make systick banked 2017-12-05 4:13 ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé @ 2017-12-12 17:22 ` Peter Maydell 0 siblings, 0 replies; 6+ messages in thread From: Peter Maydell @ 2017-12-12 17:22 UTC (permalink / raw) To: Philippe Mathieu-Daudé Cc: qemu-arm, QEMU Developers, Paolo Bonzini, Richard Henderson, patches@linaro.org On 5 December 2017 at 04:13, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > Anyway: > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Thanks; I will squash this diff in when I apply to target-arm.next: --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -2068,9 +2068,14 @@ static void armv7m_nvic_realize(DeviceState *dev, Error **errp) return; } sysbus_connect_irq(SYS_BUS_DEVICE(&s->systick[M_REG_NS]), 0, - qdev_get_gpio_in_named(dev, "systick-trigger", 0)); + qdev_get_gpio_in_named(dev, "systick-trigger", + M_REG_NS)); if (arm_feature(&s->cpu->env, ARM_FEATURE_M_SECURITY)) { - object_initialize(&s->systick[M_REG_S], sizeof(s->systick[M_REG_NS]), + /* 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(&s->systick[M_REG_S], sizeof(s->systick[M_REG_S]), TYPE_SYSTICK); qdev_set_parent_bus(DEVICE(&s->systick[M_REG_S]), sysbus_get_default()); @@ -2082,7 +2087,8 @@ static void armv7m_nvic_realize(DeviceState *dev, Error **errp) return; } sysbus_connect_irq(SYS_BUS_DEVICE(&s->systick[M_REG_S]), 0, - qdev_get_gpio_in_named(dev, "systick-trigger", 1)); + qdev_get_gpio_in_named(dev, "systick-trigger", + M_REG_S)); } /* The NVIC and System Control Space (SCS) starts at 0xe000e000 @@ -2161,7 +2167,8 @@ static void armv7m_nvic_instance_init(Object *obj) 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); + qdev_init_gpio_in_named(dev, nvic_systick_trigger, "systick-trigger", + M_REG_NUM_BANKS); } static void armv7m_nvic_class_init(ObjectClass *klass, void *data) thanks -- PMM ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-12-12 17:22 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-12-01 18:51 [Qemu-devel] [PATCH 0/2] v8m: make systick banked Peter Maydell 2017-12-01 18:51 ` [Qemu-devel] [PATCH 1/2] nvic: Make nvic_sysreg_ns_ops work with any MemoryRegion Peter Maydell 2017-12-05 3:52 ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé 2017-12-01 18:51 ` [Qemu-devel] [PATCH 2/2] nvic: Make systick banked Peter Maydell 2017-12-05 4:13 ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé 2017-12-12 17:22 ` Peter Maydell
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).