Re: [PATCH] hw/misc: Add an iBT device model

[Philippe Mathieu-Daudé]

On 9/24/21 01:48, Titus Rwantare wrote:

Hello all,

I'd like some clarification on how the following code transfers irqs
back and forth:

b/hw/arm/aspeed_soc.c
+    /* iBT */
+    if (!sysbus_realize(SYS_BUS_DEVICE(&s->ibt), errp)) {
+        return;
+    }
+    memory_region_add_subregion(&s->lpc.iomem,
+                   sc->memmap[ASPEED_DEV_IBT] - sc->memmap[ASPEED_DEV_LPC],
+                   &s->ibt.iomem);
+    sysbus_connect_irq(SYS_BUS_DEVICE(&s->lpc), 1 + aspeed_lpc_ibt,

Code smell indeed, likely:

        sysbus_connect_irq(SYS_BUS_DEVICE(&s->ibt), 1 + aspeed_lpc_ibt,

+                       qdev_get_gpio_in(DEVICE(&s->lpc), aspeed_lpc_ibt));
}

and

hw/misc/aspeed_ibt.c
+static void aspeed_ibt_realize(DeviceState *dev, Error **errp)
+{
+    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+    AspeedIBTState *ibt = ASPEED_IBT(dev);

...

+
+    sysbus_init_irq(sbd, &ibt->irq);

I ask because the code in aspeed_soc.c seems to connect to the
lpc->subdevice_irqs[aspeed_lpc_ibt], initialised on
hw/misc/aspeed_lpc.c:408.
I noticed that bmc firmware running in qemu was checking the BT_CTRL
register less frequently than I'd like while editing this patch to use
the IPMIInterface.

Thanks,
Titus