Am 3. März 2026 11:41:52 UTC schrieb Peter Maydell <[email protected]>:
>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.
I think the logic can be folded into TYPE_SERIAL. I'll send a v2.
Best regards,
Bernhard
>
>thanks
>-- PMM
