On Mon, 2 Mar 2026 at 22:03, Bernhard Beschow <[email protected]> wrote: > > SerialState currently inherits just from DeviceState and serial devices > use SerialState as an "IP block". Since DeviceState doesn't have an API > to provide MMIO regions or IRQs, all serial devices access attributes > internal to SerialState directly. Fix this by having SerialState inherit > from SysBusDevice. > > In addition, SerialState doesn't participate in the reset framework > while SysBusDevice does. This allows for implementing reset > functionality more idiomatically. > > Signed-off-by: Bernhard Beschow <[email protected]>
> @@ -76,12 +76,13 @@ static void serial_isa_realizefn(DeviceState *dev, Error > **errp) > } > index++; > > - s->irq = isa_get_irq(isadev, isa->isairq); > - qdev_realize(DEVICE(s), NULL, errp); > + sysbus_realize(sbd, errp); > + sysbus_connect_irq(sbd, 0, isa_get_irq(isadev, isa->isairq)); > qdev_set_legacy_instance_id(dev, isa->iobase, 3); > > - memory_region_init_io(&s->io, OBJECT(isa), &serial_io_ops, s, "serial", > 8); > - isa_register_ioport(isadev, &s->io, isa->iobase); > + memory_region_init_io(sysbus_mmio_get_region(sbd, 0), OBJECT(isa), > + &serial_io_ops, sbd, "serial", 8); This looks very odd, because sysbus_mmio_get_region() is supposed to be "the sysbus device provides an MR it has initialized, and this is how you get hold of it". But here we're taking an uninitialized thing and initializing it by calling memory_region_init_io(). It works because it's just returning a pointer into the internal memory, but it's still making assumptions about the internals of the device just as much as directly accessing s->io. Maybe this is worthwhile for the benefit of getting reset handling automatically into the reset tree. thanks -- PMM
