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.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
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 b0a2437c85..8e467f047d 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;
- memory_region_del_subregion(&pci->membar, &s->io);
- qdev_unrealize(DEVICE(s));
+ SysBusDevice *sbd = SYS_BUS_DEVICE(pci->state + i);
+ memory_region_del_subregion(&pci->membar,
+ sysbus_mmio_get_region(sbd, 0));
+ qdev_unrealize(DEVICE(sbd));
}
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 eda0d43bcf..f5ea7f4188 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;
- memory_region_del_subregion(&pci->iobar, &s->io);
- qdev_unrealize(DEVICE(s));
+ SysBusDevice *sbd = SYS_BUS_DEVICE(pci->state + i);
+ memory_region_del_subregion(&pci->iobar,
+ sysbus_mmio_get_region(sbd, 0));
+ qdev_unrealize(DEVICE(sbd));
}
}
@@ -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.0On 3/5/26 23:09, Bernhard Beschow 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.
>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
The lack of support for automatically resetting children other than
buses is a pity indeed, but I don't think that this patch is correct.
SysBusDevice is for devices connected directly to the board; a device
should not have any children objects connected to a bus (unless the bus
is part of the device itself, as is the case for usb-storage for example).
Unfortunately I don't have a better solution, other than adding
accessors to SerialState that retrieve the MMIO or connect the IRQ.
Paolo
> 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 b0a2437c85..8e467f047d 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;
> - memory_region_del_subregion(&pci->membar, &s->io);
> - qdev_unrealize(DEVICE(s));
> + SysBusDevice *sbd = SYS_BUS_DEVICE(pci->state + i);
> + memory_region_del_subregion(&pci->membar,
> + sysbus_mmio_get_region(sbd, 0));
> + qdev_unrealize(DEVICE(sbd));
> }
> 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 eda0d43bcf..f5ea7f4188 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;
> - memory_region_del_subregion(&pci->iobar, &s->io);
> - qdev_unrealize(DEVICE(s));
> + SysBusDevice *sbd = SYS_BUS_DEVICE(pci->state + i);
> + memory_region_del_subregion(&pci->iobar,
> + sysbus_mmio_get_region(sbd, 0));
> + qdev_unrealize(DEVICE(sbd));
> }
> }
>
> @@ -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,
> };
On Mon, 9 Mar 2026 at 08:33, Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 3/5/26 23:09, Bernhard Beschow 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. > > > > Reviewed-by: Peter Maydell <peter.maydell@linaro.org> > > The lack of support for automatically resetting children other than > buses is a pity indeed, but I don't think that this patch is correct. > > SysBusDevice is for devices connected directly to the board; a device > should not have any children objects connected to a bus (unless the bus > is part of the device itself, as is the case for usb-storage for example). We also use it for various other things beyond that, though: notably we use it extensively in SoC objects, which have child objects which are almost always sysbus devices. Very few devices really connect directly to a board. I actively advise against people creating new devices that directly inherit from TYPE_DEVICE. If you also advise against creating devices that inherit from TYPE_SYSBUS_DEVICE that aren't directly connected to the board, then we are jointly saying "don't create this device" for quite a lot of devices :-) -- PMM
© 2016 - 2026 Red Hat, Inc.