Am 5. März 2026 10:07:23 UTC schrieb Peter Maydell <[email protected]>:
>On Tue, 3 Mar 2026 at 22:22, 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, DeviceState 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]>
>
>
>
>> diff --git a/hw/char/diva-gsp.c b/hw/char/diva-gsp.c
>> index 2ea60103ea..55c42effe9 100644
>> --- a/hw/char/diva-gsp.c
>> +++ b/hw/char/diva-gsp.c
>> @@ -56,13 +56,13 @@ typedef struct PCIDivaSerialState {
>> static void diva_pci_exit(PCIDevice *dev)
>> {
>> PCIDivaSerialState *pci = DO_UPCAST(PCIDivaSerialState, dev, dev);
>> - SerialState *s;
>> int i;
>>
>> for (i = 0; i < pci->ports; i++) {
>> - s = pci->state + i;
>> - qdev_unrealize(DEVICE(s));
>> - memory_region_del_subregion(&pci->membar, &s->io);
>> + SysBusDevice *sbd = SYS_BUS_DEVICE(pci->state + i);
>> + qdev_unrealize(DEVICE(sbd));
>> + memory_region_del_subregion(&pci->membar,
>> + sysbus_mmio_get_region(sbd, 0));
>> }
>> qemu_free_irqs(pci->irqs, pci->ports);
>> }
>
>The ordering here looks a little odd -- is it really OK to
>keep using sbd (passing it to sysbus_mmio_get_region()) after
>we have unrealized the device ? Remove the region from the membar
>first and then unrealize would seem a more natural order.
Indeed. Will add a patch before this one.
Thanks,
Bermhard
>Perhaps the MR refcounts here save us, though.
>
>> diff --git a/hw/char/serial-pci-multi.c b/hw/char/serial-pci-multi.c
>> index 38e5a78e6f..4e51d14111 100644
>> --- a/hw/char/serial-pci-multi.c
>> +++ b/hw/char/serial-pci-multi.c
>> @@ -50,13 +50,13 @@ typedef struct PCIMultiSerialState {
>> static void multi_serial_pci_exit(PCIDevice *dev)
>> {
>> PCIMultiSerialState *pci = DO_UPCAST(PCIMultiSerialState, dev, dev);
>> - SerialState *s;
>> int i;
>>
>> for (i = 0; i < pci->ports; i++) {
>> - s = pci->state + i;
>> - qdev_unrealize(DEVICE(s));
>> - memory_region_del_subregion(&pci->iobar, &s->io);
>> + SysBusDevice *sbd = SYS_BUS_DEVICE(pci->state + i);
>> + qdev_unrealize(DEVICE(sbd));
>> + memory_region_del_subregion(&pci->iobar,
>> + sysbus_mmio_get_region(sbd, 0));
>> }
>> }
>
>Similarly here.
>
>Otherwise
>Reviewed-by: Peter Maydell <[email protected]>
>
>thanks
>-- PMM
