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 <shentey@gmail.com>
---
 include/hw/char/serial.h   |  4 ++--
 hw/char/diva-gsp.c         | 18 +++++++++---------
 hw/char/serial-isa.c       | 16 ++++++++++------
 hw/char/serial-mm.c        |  8 ++++----
 hw/char/serial-pci-multi.c | 18 +++++++++---------
 hw/char/serial-pci.c       | 10 +++++-----
 hw/char/serial.c           |  5 ++++-
 7 files changed, 43 insertions(+), 36 deletions(-)

diff --git a/include/hw/char/serial.h b/include/hw/char/serial.h
index 0cf641a860..2ee9e5984c 100644
--- a/include/hw/char/serial.h
+++ b/include/hw/char/serial.h
@@ -26,7 +26,7 @@
 #ifndef HW_SERIAL_H
 #define HW_SERIAL_H
 
-#include "hw/core/qdev.h"
+#include "hw/core/sysbus.h"
 #include "chardev/char-fe.h"
 #include "system/memory.h"
 #include "qemu/fifo8.h"
@@ -35,7 +35,7 @@
 #define UART_FIFO_LENGTH    16      /* 16550A Fifo Length */
 
 struct SerialState {
-    DeviceState parent;
+    SysBusDevice parent;
 
     uint16_t divider;
     uint8_t rbr; /* receive register */
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);
 }
@@ -116,7 +116,6 @@ static void diva_pci_realize(PCIDevice *dev, Error **errp)
 {
     PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
     PCIDivaSerialState *pci = DO_UPCAST(PCIDivaSerialState, dev, dev);
-    SerialState *s;
     struct diva_info di = diva_get_diva_info(pc);
     size_t i, offset = 0;
     size_t portmask = di.omask;
@@ -128,19 +127,20 @@ static void diva_pci_realize(PCIDevice *dev, Error **errp)
     pci->irqs = qemu_allocate_irqs(multi_serial_irq_mux, pci, di.nports);
 
     for (i = 0; i < di.nports; i++) {
-        s = pci->state + i;
-        if (!qdev_realize(DEVICE(s), NULL, errp)) {
+        SysBusDevice *sbd = SYS_BUS_DEVICE(pci->state + i);
+        if (!sysbus_realize(sbd, errp)) {
             diva_pci_exit(dev);
             return;
         }
-        s->irq = pci->irqs[i];
+        sysbus_connect_irq(sbd, 0, pci->irqs[i]);
 
         /* calculate offset of given port based on bitmask */
         while ((portmask & BIT(0)) == 0) {
             offset += 8;
             portmask >>= 1;
         }
-        memory_region_add_subregion(&pci->membar, offset, &s->io);
+        memory_region_add_subregion(&pci->membar, offset,
+                                    sysbus_mmio_get_region(sbd, 0));
         offset += 8;
         portmask >>= 1;
         pci->ports++;
diff --git a/hw/char/serial-isa.c b/hw/char/serial-isa.c
index 3a48b2495e..5eb2dc6350 100644
--- a/hw/char/serial-isa.c
+++ b/hw/char/serial-isa.c
@@ -58,7 +58,7 @@ static void serial_isa_realizefn(DeviceState *dev, Error **errp)
     static int index;
     ISADevice *isadev = ISA_DEVICE(dev);
     ISASerialState *isa = ISA_SERIAL(dev);
-    SerialState *s = &isa->state;
+    SysBusDevice *sbd = SYS_BUS_DEVICE(&isa->state);
 
     if (isa->index == -1) {
         isa->index = index;
@@ -76,11 +76,11 @@ 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);
 
-    isa_register_ioport(isadev, &s->io, isa->iobase);
+    isa_register_ioport(isadev, sysbus_mmio_get_region(sbd, 0), isa->iobase);
 }
 
 static void serial_isa_build_aml(AcpiDevAmlIf *adev, Aml *scope)
@@ -187,13 +187,17 @@ void serial_hds_isa_init(ISABus *bus, int from, int to)
 void isa_serial_set_iobase(ISADevice *serial, hwaddr iobase)
 {
     ISASerialState *s = ISA_SERIAL(serial);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(&s->state);
 
     serial->ioport_id = iobase;
     s->iobase = iobase;
-    memory_region_set_address(&s->state.io, s->iobase);
+    memory_region_set_address(sysbus_mmio_get_region(sbd, 0), s->iobase);
 }
 
 void isa_serial_set_enabled(ISADevice *serial, bool enabled)
 {
-    memory_region_set_enabled(&ISA_SERIAL(serial)->state.io, enabled);
+    ISASerialState *s = ISA_SERIAL(serial);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(&s->state);
+
+    memory_region_set_enabled(sysbus_mmio_get_region(sbd, 0), enabled);
 }
diff --git a/hw/char/serial-mm.c b/hw/char/serial-mm.c
index 1dba4fc694..ba4061aa69 100644
--- a/hw/char/serial-mm.c
+++ b/hw/char/serial-mm.c
@@ -33,14 +33,14 @@
 static void serial_mm_realize(DeviceState *dev, Error **errp)
 {
     SerialMM *smm = SERIAL_MM(dev);
-    SerialState *s = &smm->serial;
+    SysBusDevice *sbd = SYS_BUS_DEVICE(&smm->serial);
 
-    if (!qdev_realize(DEVICE(s), NULL, errp)) {
+    if (!sysbus_realize(sbd, errp)) {
         return;
     }
 
-    sysbus_init_mmio(SYS_BUS_DEVICE(smm), &s->io);
-    sysbus_init_irq(SYS_BUS_DEVICE(smm), &smm->serial.irq);
+    sysbus_init_mmio(SYS_BUS_DEVICE(smm), sysbus_mmio_get_region(sbd, 0));
+    sysbus_pass_irq(SYS_BUS_DEVICE(smm), SYS_BUS_DEVICE(sbd));
 }
 
 static const VMStateDescription vmstate_serial_mm = {
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));
     }
 }
 
@@ -91,7 +91,6 @@ static void multi_serial_pci_realize(PCIDevice *dev, Error **errp)
 {
     PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
     PCIMultiSerialState *pci = DO_UPCAST(PCIMultiSerialState, dev, dev);
-    SerialState *s;
     size_t i, nports = multi_serial_get_port_count(pc);
 
     pci->dev.config[PCI_CLASS_PROG] = 2; /* 16550 compatible */
@@ -100,13 +99,14 @@ static void multi_serial_pci_realize(PCIDevice *dev, Error **errp)
     pci_register_bar(&pci->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &pci->iobar);
 
     for (i = 0; i < nports; i++) {
-        s = pci->state + i;
-        if (!qdev_realize(DEVICE(s), NULL, errp)) {
+        SysBusDevice *sbd = SYS_BUS_DEVICE(pci->state + i);
+        if (!sysbus_realize(sbd, errp)) {
             multi_serial_pci_exit(dev);
             return;
         }
-        s->irq = &pci->irqs[i];
-        memory_region_add_subregion(&pci->iobar, 8 * i, &s->io);
+        sysbus_connect_irq(sbd, 0, &pci->irqs[i]);
+        memory_region_add_subregion(&pci->iobar, 8 * i,
+                                    sysbus_mmio_get_region(sbd, 0));
         pci->ports++;
     }
 }
diff --git a/hw/char/serial-pci.c b/hw/char/serial-pci.c
index 9a0bf2d890..b702de8219 100644
--- a/hw/char/serial-pci.c
+++ b/hw/char/serial-pci.c
@@ -46,17 +46,18 @@ OBJECT_DECLARE_SIMPLE_TYPE(PCISerialState, PCI_SERIAL)
 static void serial_pci_realize(PCIDevice *dev, Error **errp)
 {
     PCISerialState *pci = DO_UPCAST(PCISerialState, dev, dev);
-    SerialState *s = &pci->state;
+    SysBusDevice *sbd = SYS_BUS_DEVICE(&pci->state);
 
-    if (!qdev_realize(DEVICE(s), NULL, errp)) {
+    if (!sysbus_realize(sbd, errp)) {
         return;
     }
 
     pci->dev.config[PCI_CLASS_PROG] = 2; /* 16550 compatible */
     pci->dev.config[PCI_INTERRUPT_PIN] = 1;
-    s->irq = pci_allocate_irq(&pci->dev);
+    sysbus_connect_irq(sbd, 0, pci_allocate_irq(&pci->dev));
 
-    pci_register_bar(&pci->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &s->io);
+    pci_register_bar(&pci->dev, 0, PCI_BASE_ADDRESS_SPACE_IO,
+                     sysbus_mmio_get_region(sbd, 0));
 }
 
 static void serial_pci_exit(PCIDevice *dev)
@@ -65,7 +66,6 @@ static void serial_pci_exit(PCIDevice *dev)
     SerialState *s = &pci->state;
 
     qdev_unrealize(DEVICE(s));
-    qemu_free_irq(s->irq);
 }
 
 static const VMStateDescription vmstate_pci_serial = {
diff --git a/hw/char/serial.c b/hw/char/serial.c
index 49227830e1..fc92897376 100644
--- a/hw/char/serial.c
+++ b/hw/char/serial.c
@@ -991,6 +991,9 @@ static void serial_realize(DeviceState *dev, Error **errp)
                              serial_event, serial_be_change, s, NULL, true);
     fifo8_create(&s->recv_fifo, UART_FIFO_LENGTH);
     fifo8_create(&s->xmit_fifo, UART_FIFO_LENGTH);
+
+    sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->io);
+    sysbus_init_irq(SYS_BUS_DEVICE(s), &s->irq);
 }
 
 static void serial_unrealize(DeviceState *dev)
@@ -1034,7 +1037,7 @@ static void serial_class_init(ObjectClass *klass, const void *data)
 
 static const TypeInfo serial_info = {
     .name = TYPE_SERIAL,
-    .parent = TYPE_DEVICE,
+    .parent = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(SerialState),
     .class_init = serial_class_init,
 };
-- 
2.53.0

On Tue, 3 Mar 2026 at 22:22, Bernhard Beschow <shentey@gmail.com> 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 <shentey@gmail.com>



> 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.
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 <peter.maydell@linaro.org>

thanks
-- PMM

Am 5. März 2026 10:07:23 UTC schrieb Peter Maydell <peter.maydell@linaro.org>:
>On Tue, 3 Mar 2026 at 22:22, Bernhard Beschow <shentey@gmail.com> 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 <shentey@gmail.com>
>
>
>
>> 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 <peter.maydell@linaro.org>
>
>thanks
>-- PMM

Bernhard Beschow <shentey@gmail.com> writes:

> 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 <shentey@gmail.com>

We might be at a crossroads here.

Paul Brook designed qdev around "a device plugs into some bus, and may
provide buses".  To make this work, he added a main system bus to serve
as root of a single tree.

The parent of a device in that tree is the bus it plugs into, and the
parent of a bus is the device that provides it, except for the main
system bus, which has no parent.  "info qtree" shows this tree.

Note a machine had just this one system bus back then.

The system bus isn't something hardware people would call a bus.

The qtree is a qdev thing.  It is *not* the QOM composition tree ("info
qom-tree /").

We actually abandoned "a device plugs into some bus" a long time ago.
Moe on that below.

Qdev was later rebased onto QOM, as follows.

A bus QOM type is a subtype of "bus".

A device QOM type is a subtype of "device".

If a device type has a @bus_type, then it plugs into a bus of that QOM
type.  Such a concrete device is commonly[*] not a direct subtype of
"device".  Instead, it's a subtype of the abstract device that plugs
into that bus type.  Example: "isa-serial" is a direct subtype of
"isa-device", and it inherits its bus type "ISA" from there.

If a device has no @bus_type, it doesn't plug into any bus.  It doesn't
show up in "info qtree" (it does in "info qom-tree", of course).
Example: "serial" is a direct subtype of "device".

This lets us build devices from components without having to make up
buses to connect them.  Example: "isa-serial" has a direct QOM child of
type "serial".  Good!

Except infrastructure useful to such bus-less devices lives in sysbus,
where bus-less devices can't access it.  This tempts us to make devices
sysbus devices just to be able to use the infrastructure, and to create
additional sysbuses.

We can elect to go down this path further: make more sysbus devices.
Create more sysbuses.  If this is the right path, then bus-less devices
were a mistake, and we should consider eliminating them.

Or we can pick the other branch: put the infrastructure where bus-less
devices can use it.  If this is the right path, then sysbus is
obsolescent.

Or we can elect to muddle on without giving it much thought.  Always an
option :)


[...]


[*] Are there any exceptions?  I don't know.

On Wed, 4 Mar 2026 at 12:25, Markus Armbruster <armbru@redhat.com> wrote:
> We might be at a crossroads here.
>
> Paul Brook designed qdev around "a device plugs into some bus, and may
> provide buses".  To make this work, he added a main system bus to serve
> as root of a single tree.
>
> The parent of a device in that tree is the bus it plugs into, and the
> parent of a bus is the device that provides it, except for the main
> system bus, which has no parent.  "info qtree" shows this tree.
>
> Note a machine had just this one system bus back then.
>
> The system bus isn't something hardware people would call a bus.
>
> The qtree is a qdev thing.  It is *not* the QOM composition tree ("info
> qom-tree /").
>
> We actually abandoned "a device plugs into some bus" a long time ago.
> Moe on that below.
>
> Qdev was later rebased onto QOM, as follows.
>
> A bus QOM type is a subtype of "bus".
>
> A device QOM type is a subtype of "device".
>
> If a device type has a @bus_type, then it plugs into a bus of that QOM
> type.  Such a concrete device is commonly[*] not a direct subtype of
> "device".  Instead, it's a subtype of the abstract device that plugs
> into that bus type.  Example: "isa-serial" is a direct subtype of
> "isa-device", and it inherits its bus type "ISA" from there.
>
> If a device has no @bus_type, it doesn't plug into any bus.  It doesn't
> show up in "info qtree" (it does in "info qom-tree", of course).
> Example: "serial" is a direct subtype of "device".
>
> This lets us build devices from components without having to make up
> buses to connect them.  Example: "isa-serial" has a direct QOM child of
> type "serial".  Good!
>
> Except infrastructure useful to such bus-less devices lives in sysbus,
> where bus-less devices can't access it.  This tempts us to make devices
> sysbus devices just to be able to use the infrastructure, and to create
> additional sysbuses.

There is only one sysbus -- it is impossible to create more than one,
and there wouldn't be much point in creating a second one anyway.

> We can elect to go down this path further: make more sysbus devices.
> Create more sysbuses.  If this is the right path, then bus-less devices
> were a mistake, and we should consider eliminating them.
>
> Or we can pick the other branch: put the infrastructure where bus-less
> devices can use it.  If this is the right path, then sysbus is
> obsolescent.

I do think that at some point it would be good to move all that
stuff into the base "device" class, and drop sysbus. The real
problem, though, is reset. Currently reset propagates along the
qtree (the tree of buses). So something (like TYPE_SERIAL, and
also like CPU objects) that is a direct child of TYPE_DEVICE does
not get automatically reset. The major overriding reason why
(as it currently stands) I favour making things sysbus devices
and not plain devices is that devices that don't get reset are
either broken or need all their users to take extra steps that
most don't take, or do in a weird way. We can't just push the
reset handling into TYPE_DEVICE because by definition it is tied
to "being a thing on a bus, i.e. the sysbus".

Disentangling reset is extremely painful -- I don't even know
what it ought to be doing. Maybe propagating along the QOM tree?
So for now, we muddle along.

-- PMM

Peter Maydell <peter.maydell@linaro.org> writes:

> On Wed, 4 Mar 2026 at 12:25, Markus Armbruster <armbru@redhat.com> wrote:
>> We might be at a crossroads here.
>>
>> Paul Brook designed qdev around "a device plugs into some bus, and may
>> provide buses".  To make this work, he added a main system bus to serve
>> as root of a single tree.
>>
>> The parent of a device in that tree is the bus it plugs into, and the
>> parent of a bus is the device that provides it, except for the main
>> system bus, which has no parent.  "info qtree" shows this tree.
>>
>> Note a machine had just this one system bus back then.
>>
>> The system bus isn't something hardware people would call a bus.
>>
>> The qtree is a qdev thing.  It is *not* the QOM composition tree ("info
>> qom-tree /").
>>
>> We actually abandoned "a device plugs into some bus" a long time ago.
>> Moe on that below.
>>
>> Qdev was later rebased onto QOM, as follows.
>>
>> A bus QOM type is a subtype of "bus".
>>
>> A device QOM type is a subtype of "device".
>>
>> If a device type has a @bus_type, then it plugs into a bus of that QOM
>> type.  Such a concrete device is commonly[*] not a direct subtype of
>> "device".  Instead, it's a subtype of the abstract device that plugs
>> into that bus type.  Example: "isa-serial" is a direct subtype of
>> "isa-device", and it inherits its bus type "ISA" from there.
>>
>> If a device has no @bus_type, it doesn't plug into any bus.  It doesn't
>> show up in "info qtree" (it does in "info qom-tree", of course).
>> Example: "serial" is a direct subtype of "device".
>>
>> This lets us build devices from components without having to make up
>> buses to connect them.  Example: "isa-serial" has a direct QOM child of
>> type "serial".  Good!
>>
>> Except infrastructure useful to such bus-less devices lives in sysbus,
>> where bus-less devices can't access it.  This tempts us to make devices
>> sysbus devices just to be able to use the infrastructure, and to create
>> additional sysbuses.
>
> There is only one sysbus -- it is impossible to create more than one,
> and there wouldn't be much point in creating a second one anyway.

I was almost entirely mistaken there.  However, you're not quite right,
either :)

We *can* create more than one system bus, and we in fact do: abstract
PCI device "macio" contains a "macio-bus", which is a subtype of the
system bus type.  Its concrete subtypes are "macio-oldworld" and
"macio-newworld".  To see the "macio-bus" in "info qtree", try machine
"g3beige".

But this is the rare exception.  The rule is us doing something
differently undesirable.

Consider ARM machine "ast2700fc".  Its core are three devices:
"ast2700-a2", "aspeed27x0ssp-coprocessor", and
"aspeed27x0tsp-coprocessor".  All three contain component devices.  The
QOM composition tree shows them as "/machine/ca35", "/machine/ssp", and
"/machine/tsp", i.e. they are direct children of the machine object.

Here's a closer look at a small part of the composition tree:

    /machine (ast2700fc-machine)
      [...]
      /ca35 (ast2700-a2)
        [...]
        /ioexp[0] (aspeed.ast1700)
          [...]
          /uart (serial-mm)
            /serial (serial)
            /serial[0] (memory-region)

I'll refer back to this later.

A few components end up in the "/machine/unattached" orphanage instead
due to sloppy modelling.

My point is: the QOM composition tree actually reflects how devices are
built from components, except for the parts where we got sloppy, which
should be fixable.

The qtree (the thing shown by "info qtree") shows a different picture.
The three core devices are bus-less, and therefore not in the qtree.
Their components, however, are mostly sysbus devices, and therefore in
the qtree right under the root.  Here are the parts of the qtree that
correspond to the QOM composition tree snippet above:

bus: main-system-bus
  type System
  [...]
  dev: serial-mm, id ""
    gpio-out "sysbus-irq" 1
    regshift = 2 (0x2)
    endianness = 2 (0x2)
    mmio ffffffffffffffff/0000000000000020
  dev: aspeed.ast1700, id ""
    board-idx = 0 (0x0)
    silicon-rev = 100794627 (0x6020103)
    dram = "/machine/ca35/ca35-dram[0]"
    mmio 0000000030000000/0000000001000000

The "aspeed.ast1700" and its "serial-mm" component are siblings.  The
latter's "serial" component is bus-less, and therefore not in the qtree.
Bernhard's patches make it a sysbus device instead, and therefore add it
to the qtree as another sibling.

My point is: the qtree does not reflect how devices are built from
components / are wired together, at least not where sysbus devices are
involved.

>> We can elect to go down this path further: make more sysbus devices.
>> Create more sysbuses.  If this is the right path, then bus-less devices
>> were a mistake, and we should consider eliminating them.
>>
>> Or we can pick the other branch: put the infrastructure where bus-less
>> devices can use it.  If this is the right path, then sysbus is
>> obsolescent.
>
> I do think that at some point it would be good to move all that
> stuff into the base "device" class, and drop sysbus. The real
> problem, though, is reset. Currently reset propagates along the
> qtree (the tree of buses).

Unfortunately, the qtree is far removed from reality for sysbus devices.

Case in point: "aspeed.ast1700" and its component "serial-mm" are
siblings in the qtree.  In what order will they be reset?

Physical hardware has reset lines.  Devices generally wire up their
components' reset pins to their own.  I believe the sane way to model
this is a reset tree.

>                            So something (like TYPE_SERIAL, and
> also like CPU objects) that is a direct child of TYPE_DEVICE does
> not get automatically reset. The major overriding reason why
> (as it currently stands) I favour making things sysbus devices
> and not plain devices is that devices that don't get reset are
> either broken or need all their users to take extra steps that
> most don't take, or do in a weird way. We can't just push the
> reset handling into TYPE_DEVICE because by definition it is tied
> to "being a thing on a bus, i.e. the sysbus".
>
> Disentangling reset is extremely painful -- I don't even know
> what it ought to be doing. Maybe propagating along the QOM tree?

I think this would make more sense.  Unlike the qtree, the QOM
composition tree contains all devices, and reflects the actual
composition.  It's certainly closer to the real reset tree than the
qtree is.  Is it close enough?  I don't know.  If yes, there's our reset
tree.  If no, I guess we could still use it as a base, with manually
corrected reset lines where the QOM composition tree is off.

> So for now, we muddle along.
>
> -- PMM

On Thu, 5 Mar 2026 at 09:25, Markus Armbruster <armbru@redhat.com> wrote:
> > Disentangling reset is extremely painful -- I don't even know
> > what it ought to be doing. Maybe propagating along the QOM tree?
>
> I think this would make more sense.  Unlike the qtree, the QOM
> composition tree contains all devices, and reflects the actual
> composition.  It's certainly closer to the real reset tree than the
> qtree is.  Is it close enough?  I don't know.  If yes, there's our reset
> tree.  If no, I guess we could still use it as a base, with manually
> corrected reset lines where the QOM composition tree is off.

Yes. We really don't want to add more boilerplate requirements
to how you write "container" type devices like SoC objects.
It's already bad enough that you have to have an instance_init
that manually calls instance_init on all your subcomponents,
and a realize that calls realize on all of them. If we add
a requirement that you need to have reset methods for 3 phases
that call reset on all your subcomponents, people are going to
forget. We need the default to be "do the thing that's right
almost every time", not "do the thing that's wrong".

(And then of course there is the "how do we get there from
here?" question :-))

thanks
-- PMM

Peter Maydell <peter.maydell@linaro.org> writes:

> On Thu, 5 Mar 2026 at 09:25, Markus Armbruster <armbru@redhat.com> wrote:
>> > Disentangling reset is extremely painful -- I don't even know
>> > what it ought to be doing. Maybe propagating along the QOM tree?
>>
>> I think this would make more sense.  Unlike the qtree, the QOM
>> composition tree contains all devices, and reflects the actual
>> composition.  It's certainly closer to the real reset tree than the
>> qtree is.  Is it close enough?  I don't know.  If yes, there's our reset
>> tree.  If no, I guess we could still use it as a base, with manually
>> corrected reset lines where the QOM composition tree is off.

A walk to the farmers market jogged my memory: the QOM composition tree
is in fact off for user-created devices.  These go into
/machine/peripheral and /machine/peripheral-anon.

Most of them plug into a bus, and reset should flow through that bus,
not through their peripheral container.

A few don't, and I can't say how reset is supposed to work then.

Onboard device can also be connected via some bus.  For instance,
machine "pc" contains sysbus device "i440FX-pcihost", which provides a
PCI bus.  It also contains PCI device "PIIX3" (the south bridge)
connected via that PCI bus.

The QOM composition tree has "i440FX-pcihost" and its PCI bus in the
right place, and "PIIX3" in the orphanage:

    /machine (pc-i440fx-11.0-machine)
      /i440fx (i440FX-pcihost)
        /pci.0 (PCI)
      /unattached (container)
        /device[3] (PIIX3)
        /sysbus (System)

Say we fixed that like so

    /machine (pc-i440fx-11.0-machine)
      /i440fx (i440FX-pcihost)
        /pci.0 (PCI)
        /piix3 (PIIX3)

then reset flowing along the composition tree is still problematic: we
probably want to reset the PCI bus before the devices plugged into it.

So maybe reset should flow along the composition tree to buses, from bus
to devices plugged into it, from such a device again along the
composition tree, and so forth.

> Yes. We really don't want to add more boilerplate requirements
> to how you write "container" type devices like SoC objects.
> It's already bad enough that you have to have an instance_init
> that manually calls instance_init on all your subcomponents,
> and a realize that calls realize on all of them. If we add
> a requirement that you need to have reset methods for 3 phases
> that call reset on all your subcomponents, people are going to
> forget. We need the default to be "do the thing that's right
> almost every time", not "do the thing that's wrong".

Good interfaces make doing the right thing easier than doing the wrong
thing.  We clearly failed there.

> (And then of course there is the "how do we get there from
> here?" question :-))

Good question.  Next question?  SCNR!

I'm afraid I don't understand how reset works *now* well enough to come
up with a workable plan.

In other words, I don't have a clear idea of "here", and I fear we
together don't have a sufficiently clear idea of "there".

What can we do to improve on both?

On Wed, 4 Mar 2026, Peter Maydell wrote:
> On Wed, 4 Mar 2026 at 12:25, Markus Armbruster <armbru@redhat.com> wrote:
>> We might be at a crossroads here.
>>
>> Paul Brook designed qdev around "a device plugs into some bus, and may
>> provide buses".  To make this work, he added a main system bus to serve
>> as root of a single tree.
>>
>> The parent of a device in that tree is the bus it plugs into, and the
>> parent of a bus is the device that provides it, except for the main
>> system bus, which has no parent.  "info qtree" shows this tree.
>>
>> Note a machine had just this one system bus back then.
>>
>> The system bus isn't something hardware people would call a bus.
>>
>> The qtree is a qdev thing.  It is *not* the QOM composition tree ("info
>> qom-tree /").
>>
>> We actually abandoned "a device plugs into some bus" a long time ago.
>> Moe on that below.
>>
>> Qdev was later rebased onto QOM, as follows.
>>
>> A bus QOM type is a subtype of "bus".
>>
>> A device QOM type is a subtype of "device".
>>
>> If a device type has a @bus_type, then it plugs into a bus of that QOM
>> type.  Such a concrete device is commonly[*] not a direct subtype of
>> "device".  Instead, it's a subtype of the abstract device that plugs
>> into that bus type.  Example: "isa-serial" is a direct subtype of
>> "isa-device", and it inherits its bus type "ISA" from there.
>>
>> If a device has no @bus_type, it doesn't plug into any bus.  It doesn't
>> show up in "info qtree" (it does in "info qom-tree", of course).
>> Example: "serial" is a direct subtype of "device".
>>
>> This lets us build devices from components without having to make up
>> buses to connect them.  Example: "isa-serial" has a direct QOM child of
>> type "serial".  Good!
>>
>> Except infrastructure useful to such bus-less devices lives in sysbus,
>> where bus-less devices can't access it.  This tempts us to make devices
>> sysbus devices just to be able to use the infrastructure, and to create
>> additional sysbuses.
>
> There is only one sysbus -- it is impossible to create more than one,
> and there wouldn't be much point in creating a second one anyway.
>
>> We can elect to go down this path further: make more sysbus devices.
>> Create more sysbuses.  If this is the right path, then bus-less devices
>> were a mistake, and we should consider eliminating them.
>>
>> Or we can pick the other branch: put the infrastructure where bus-less
>> devices can use it.  If this is the right path, then sysbus is
>> obsolescent.
>
> I do think that at some point it would be good to move all that
> stuff into the base "device" class, and drop sysbus. The real
> problem, though, is reset. Currently reset propagates along the
> qtree (the tree of buses). So something (like TYPE_SERIAL, and
> also like CPU objects) that is a direct child of TYPE_DEVICE does
> not get automatically reset. The major overriding reason why
> (as it currently stands) I favour making things sysbus devices
> and not plain devices is that devices that don't get reset are
> either broken or need all their users to take extra steps that
> most don't take, or do in a weird way. We can't just push the
> reset handling into TYPE_DEVICE because by definition it is tied
> to "being a thing on a bus, i.e. the sysbus".
>
> Disentangling reset is extremely painful -- I don't even know
> what it ought to be doing. Maybe propagating along the QOM tree?
> So for now, we muddle along.

This suggests maybe we need a way to represent qtree in QOM. So while QOM 
devices are organised by parent-child relationship in the qom-tree they 
could have standardised properties that tell which bus they are connected 
to (or buses have properties that list devices connected to them, 
whichever is easier to walk) then the reset or info qtree could select 
devices based on these properties. Could that replace the qtree of sysbus 
devices and thus allow merging SysBusDevice into Device? There are link 
properties in QOM to connect objects so it should be possible to represent 
object relationships with such properties not just by their parent-child 
(which I think are also represented by properties but I did not check it).

Regards,
BALATON Zoltan