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 <shentey@gmail.com>
---
include/hw/char/serial.h | 4 ++--
hw/char/diva-gsp.c | 22 +++++++++++-----------
hw/char/serial-isa.c | 19 ++++++++++++-------
hw/char/serial-mm.c | 10 +++++-----
hw/char/serial-pci-multi.c | 22 +++++++++++-----------
hw/char/serial-pci.c | 13 +++++++------
hw/char/serial.c | 5 ++++-
7 files changed, 52 insertions(+), 43 deletions(-)
diff --git a/include/hw/char/serial.h b/include/hw/char/serial.h
index ea82ffac47..650fc27693 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 280d0413c6..60ddc78752 100644
--- a/hw/char/diva-gsp.c
+++ b/hw/char/diva-gsp.c
@@ -57,13 +57,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));
g_free(pci->name[i]);
}
qemu_free_irqs(pci->irqs, pci->ports);
@@ -118,7 +118,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;
@@ -130,22 +129,23 @@ 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]);
pci->name[i] = g_strdup_printf("uart #%zu", i + 1);
- memory_region_init_io(&s->io, OBJECT(pci), &serial_io_ops, s,
- pci->name[i], 8);
+ memory_region_init_io(sysbus_mmio_get_region(sbd, 0), OBJECT(pci),
+ &serial_io_ops, sbd, pci->name[i], 8);
/* 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 a4be0492c5..d3a401f56c 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,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);
+ isa_register_ioport(isadev, sysbus_mmio_get_region(sbd, 0), isa->iobase);
}
static void serial_isa_build_aml(AcpiDevAmlIf *adev, Aml *scope)
@@ -188,13 +189,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 0e0be26fa9..de11498278 100644
--- a/hw/char/serial-mm.c
+++ b/hw/char/serial-mm.c
@@ -71,17 +71,17 @@ static const MemoryRegionOps serial_mm_ops[] = {
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;
}
- memory_region_init_io(&s->io, OBJECT(dev),
+ memory_region_init_io(sysbus_mmio_get_region(sbd, 0), OBJECT(dev),
&serial_mm_ops[smm->endianness], smm, "serial",
8 << smm->regshift);
- 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 17796b93dd..7545ab7425 100644
--- a/hw/char/serial-pci-multi.c
+++ b/hw/char/serial-pci-multi.c
@@ -51,13 +51,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));
g_free(pci->name[i]);
}
}
@@ -93,7 +93,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 */
@@ -102,16 +101,17 @@ 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];
+ sysbus_connect_irq(sbd, 0, &pci->irqs[i]);
pci->name[i] = g_strdup_printf("uart #%zu", i + 1);
- memory_region_init_io(&s->io, OBJECT(pci), &serial_io_ops, s,
- pci->name[i], 8);
- memory_region_add_subregion(&pci->iobar, 8 * i, &s->io);
+ memory_region_init_io(sysbus_mmio_get_region(sbd, 0), OBJECT(pci),
+ &serial_io_ops, sbd, pci->name[i], 8);
+ 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 d8cacc9085..10a99592b6 100644
--- a/hw/char/serial-pci.c
+++ b/hw/char/serial-pci.c
@@ -46,18 +46,20 @@ 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));
- memory_region_init_io(&s->io, OBJECT(pci), &serial_io_ops, s, "serial", 8);
- pci_register_bar(&pci->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &s->io);
+ memory_region_init_io(sysbus_mmio_get_region(sbd, 0), OBJECT(pci),
+ &serial_io_ops, sbd, "serial", 8);
+ pci_register_bar(&pci->dev, 0, PCI_BASE_ADDRESS_SPACE_IO,
+ sysbus_mmio_get_region(sbd, 0));
}
static void serial_pci_exit(PCIDevice *dev)
@@ -66,7 +68,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 0f3469a1e8..c58337bd20 100644
--- a/hw/char/serial.c
+++ b/hw/char/serial.c
@@ -934,6 +934,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)
@@ -984,7 +987,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 Mon, 2 Mar 2026 at 22:03, 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, SerialState 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> > @@ -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
Am 3. März 2026 11:41:52 UTC schrieb Peter Maydell <peter.maydell@linaro.org>: >On Mon, 2 Mar 2026 at 22:03, 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, SerialState 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> > > > >> @@ -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
© 2016 - 2026 Red Hat, Inc.