On Sun, 16 Feb 2025 at 03:54, Sourojeet Adhikari <s23ad...@csclub.uwaterloo.ca> wrote: > > Hello everyone, > > This patch adds support for the system timer interrupts > in QEMU's BCM2838 model. It defines a new constant > called GIC400_TIMER_INT, and connects 4 timer interupts > to the GIC-400. > Previously timer interupts were not being routed to the > interupt controller, causing scheduling, and some other > stuff to have issues (me and a few friends bumped into > this, and that's why this was written lol). > > Signed-off-by: Sourojeet Adhikari <s23ad...@csclub.uwaterloo.ca>
Hi; thanks for sending this patch, but I'm afraid this doesn't look like a correct fix for the bug you've run into. > --- > hw/arm/bcm2838.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/hw/arm/bcm2838.c b/hw/arm/bcm2838.c > index ddb7c5f..0a4179f 100644 > --- a/hw/arm/bcm2838.c > +++ b/hw/arm/bcm2838.c > @@ -21,6 +21,8 @@ > #define GIC400_TIMER_S_EL1_IRQ 13 > #define GIC400_TIMER_NS_EL1_IRQ 14 > #define GIC400_LEGACY_IRQ 15 > +#define GIC400_TIMER_INT (96 - 32) > + > > /* Number of external interrupt lines to configure the GIC with */ > #define GIC_NUM_IRQS 192 > @@ -176,6 +178,15 @@ static void bcm2838_realize(DeviceState *dev, Error > **errp) > qdev_get_gpio_in(gicdev, PPI(n, VIRTUAL_PMU_IRQ))); > } > > + sysbus_connect_irq(SYS_BUS_DEVICE(&ps_base->systmr), 0, > + qdev_get_gpio_in(DEVICE(&s->gic), GIC400_TIMER_INT + > INTERRUPT_TIMER0)); The values passed to qdev_get_gpio_in(DEVICE(&s->gic), ...) should be GIC_SPI_INTERRUPT_* values, which we define in include/hw/arm/bcm2838_peripherals.h. You can add new ones for the four timers. > + sysbus_connect_irq(SYS_BUS_DEVICE(&ps_base->systmr), 1, > + qdev_get_gpio_in(DEVICE(&s->gic), GIC400_TIMER_INT + > INTERRUPT_TIMER1)); > + sysbus_connect_irq(SYS_BUS_DEVICE(&ps_base->systmr), 2, > + qdev_get_gpio_in(DEVICE(&s->gic), GIC400_TIMER_INT + > INTERRUPT_TIMER2)); > + sysbus_connect_irq(SYS_BUS_DEVICE(&ps_base->systmr), 3, > + qdev_get_gpio_in(DEVICE(&s->gic), GIC400_TIMER_INT + > INTERRUPT_TIMER3)); The systmr INTERRUPT_TIMER0..3 sysbus IRQ outputs are already being wired up in the function bcm_soc_peripherals_common_realize() in hw/arm/bcm2835_peripherals.c (to the TYPE_BCM2835_IC interrupt controller), and it isn't valid to wire one input directly to multiple outputs. In fact it looks like we are currently getting this wrong for all of the interrupts that need to be wired to both the "legacy interrupt controller" and the GIC. I think at the moment what happens is that the wiring to the GIC will happen last and this overrides the earlier wiring to the legacy interrupt controller, so code using the latter won't work correctly. thanks -- PMM
