diff mbox series

[v4,06/37] serial: initial qom-ification

Message ID 20191120152442.26657-7-marcandre.lureau@redhat.com
State New
Headers show
Series Clean-ups: qom-ify serial and remove QDEV_PROP_PTR | expand

Commit Message

Marc-André Lureau Nov. 20, 2019, 3:24 p.m. UTC
Make SerialState a device (the following patches will introduce IO/MM
sysbus serial devices)

None of the serial_{,mm}_init() callers actually free the returned
value (even if they did, it would be quite harmless), so we can change
the object allocation at will.

However, the devices that embed SerialState must now have their field
QOM-initialized manually (isa, pci, pci-multi).

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 hw/char/serial-isa.c       |  9 +++++++++
 hw/char/serial-pci-multi.c | 15 +++++++++++++++
 hw/char/serial-pci.c       | 13 ++++++++++++-
 hw/char/serial.c           | 33 +++++++++++++++++++++++++++------
 include/hw/char/serial.h   |  7 ++++++-
 5 files changed, 69 insertions(+), 8 deletions(-)

Comments

Philippe Mathieu-Daudé Nov. 20, 2019, 4:40 p.m. UTC | #1
On 11/20/19 4:24 PM, Marc-André Lureau wrote:
> Make SerialState a device (the following patches will introduce IO/MM
> sysbus serial devices)
> 
> None of the serial_{,mm}_init() callers actually free the returned
> value (even if they did, it would be quite harmless), so we can change
> the object allocation at will.
> 
> However, the devices that embed SerialState must now have their field
> QOM-initialized manually (isa, pci, pci-multi).
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   hw/char/serial-isa.c       |  9 +++++++++
>   hw/char/serial-pci-multi.c | 15 +++++++++++++++
>   hw/char/serial-pci.c       | 13 ++++++++++++-
>   hw/char/serial.c           | 33 +++++++++++++++++++++++++++------
>   include/hw/char/serial.h   |  7 ++++++-
>   5 files changed, 69 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/char/serial-isa.c b/hw/char/serial-isa.c
> index 9e31c51bb6..9a5928b3ee 100644
> --- a/hw/char/serial-isa.c
> +++ b/hw/char/serial-isa.c
> @@ -111,10 +111,19 @@ static void serial_isa_class_initfn(ObjectClass *klass, void *data)
>       set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
>   }
>   
> +static void serial_isa_initfn(Object *o)
> +{
> +    ISASerialState *self = ISA_SERIAL(o);
> +
> +    object_initialize_child(o, "serial", &self->state, sizeof(self->state),
> +                            TYPE_SERIAL, &error_abort, NULL);
> +}
> +
>   static const TypeInfo serial_isa_info = {
>       .name          = TYPE_ISA_SERIAL,
>       .parent        = TYPE_ISA_DEVICE,
>       .instance_size = sizeof(ISASerialState),
> +    .instance_init = serial_isa_initfn,
>       .class_init    = serial_isa_class_initfn,
>   };
>   
> diff --git a/hw/char/serial-pci-multi.c b/hw/char/serial-pci-multi.c
> index 5c553c30d0..edfbfdca9e 100644
> --- a/hw/char/serial-pci-multi.c
> +++ b/hw/char/serial-pci-multi.c
> @@ -181,10 +181,24 @@ static void multi_4x_serial_pci_class_initfn(ObjectClass *klass, void *data)
>       set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
>   }
>   
> +static void multi_serial_init(Object *o)
> +{
> +    PCIDevice *dev = PCI_DEVICE(o);
> +    PCIMultiSerialState *pms = DO_UPCAST(PCIMultiSerialState, dev, dev);
> +    size_t i, nports = multi_serial_get_port_count(PCI_DEVICE_GET_CLASS(dev));
> +
> +    for (i = 0; i < nports; i++) {
> +        object_initialize_child(o, "serial[*]", &pms->state[i],
> +                                sizeof(pms->state[i]),
> +                                TYPE_SERIAL, &error_abort, NULL);
> +    }
> +}
> +
>   static const TypeInfo multi_2x_serial_pci_info = {
>       .name          = "pci-serial-2x",
>       .parent        = TYPE_PCI_DEVICE,
>       .instance_size = sizeof(PCIMultiSerialState),
> +    .instance_init = multi_serial_init,
>       .class_init    = multi_2x_serial_pci_class_initfn,
>       .interfaces = (InterfaceInfo[]) {
>           { INTERFACE_CONVENTIONAL_PCI_DEVICE },
> @@ -196,6 +210,7 @@ static const TypeInfo multi_4x_serial_pci_info = {
>       .name          = "pci-serial-4x",
>       .parent        = TYPE_PCI_DEVICE,
>       .instance_size = sizeof(PCIMultiSerialState),
> +    .instance_init = multi_serial_init,
>       .class_init    = multi_4x_serial_pci_class_initfn,
>       .interfaces = (InterfaceInfo[]) {
>           { INTERFACE_CONVENTIONAL_PCI_DEVICE },
> diff --git a/hw/char/serial-pci.c b/hw/char/serial-pci.c
> index cb9b76e22b..f99b6c19e0 100644
> --- a/hw/char/serial-pci.c
> +++ b/hw/char/serial-pci.c
> @@ -40,6 +40,8 @@ typedef struct PCISerialState {
>       uint8_t prog_if;
>   } PCISerialState;
>   
> +#define TYPE_PCI_SERIAL "pci-serial"
> +#define PCI_SERIAL(s) OBJECT_CHECK(PCISerialState, (s), TYPE_PCI_SERIAL)
>   
>   static void serial_pci_realize(PCIDevice *dev, Error **errp)
>   {
> @@ -103,10 +105,19 @@ static void serial_pci_class_initfn(ObjectClass *klass, void *data)
>       set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
>   }
>   
> +static void serial_pci_init(Object *o)
> +{
> +    PCISerialState *ps = PCI_SERIAL(o);
> +
> +    object_initialize_child(o, "serial", &ps->state, sizeof(ps->state),
> +                            TYPE_SERIAL, &error_abort, NULL);
> +}
> +
>   static const TypeInfo serial_pci_info = {
> -    .name          = "pci-serial",
> +    .name          = TYPE_PCI_SERIAL,
>       .parent        = TYPE_PCI_DEVICE,
>       .instance_size = sizeof(PCISerialState),
> +    .instance_init = serial_pci_init,
>       .class_init    = serial_pci_class_initfn,
>       .interfaces = (InterfaceInfo[]) {
>           { INTERFACE_CONVENTIONAL_PCI_DEVICE },
> diff --git a/hw/char/serial.c b/hw/char/serial.c
> index b4aa250950..e0a5bec290 100644
> --- a/hw/char/serial.c
> +++ b/hw/char/serial.c
> @@ -983,9 +983,8 @@ const MemoryRegionOps serial_io_ops = {
>   SerialState *serial_init(int base, qemu_irq irq, int baudbase,
>                            Chardev *chr, MemoryRegion *system_io)
>   {
> -    SerialState *s;
> -
> -    s = g_malloc0(sizeof(SerialState));
> +    DeviceState *dev = DEVICE(object_new(TYPE_SERIAL));
> +    SerialState *s = SERIAL(dev);
>   
>       s->irq = irq;
>       s->baudbase = baudbase;
> @@ -993,6 +992,7 @@ SerialState *serial_init(int base, qemu_irq irq, int baudbase,
>       serial_realize_core(s, &error_fatal);
>   
>       vmstate_register(NULL, base, &vmstate_serial, s);
> +    qdev_init_nofail(dev);
>   
>       memory_region_init_io(&s->io, NULL, &serial_io_ops, s, "serial", 8);
>       memory_region_add_subregion(system_io, base, &s->io);
> @@ -1000,6 +1000,20 @@ SerialState *serial_init(int base, qemu_irq irq, int baudbase,
>       return s;
>   }
>   
> +static void serial_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->user_creatable = false;

Why? Due to the "chardev" property?

> +}
> +
> +static const TypeInfo serial_info = {
> +    .name = TYPE_SERIAL,
> +    .parent = TYPE_DEVICE,
> +    .instance_size = sizeof(SerialState),
> +    .class_init = serial_class_init,
> +};
> +
>   /* Memory mapped interface */
>   static uint64_t serial_mm_read(void *opaque, hwaddr addr,
>                                  unsigned size)
> @@ -1045,9 +1059,8 @@ SerialState *serial_mm_init(MemoryRegion *address_space,
>                               qemu_irq irq, int baudbase,
>                               Chardev *chr, enum device_endian end)
>   {
> -    SerialState *s;
> -
> -    s = g_malloc0(sizeof(SerialState));
> +    DeviceState *dev = DEVICE(object_new(TYPE_SERIAL));
> +    SerialState *s = SERIAL(dev);
>   
>       s->it_shift = it_shift;
>       s->irq = irq;
> @@ -1056,9 +1069,17 @@ SerialState *serial_mm_init(MemoryRegion *address_space,
>   
>       serial_realize_core(s, &error_fatal);
>       vmstate_register(NULL, base, &vmstate_serial, s);
> +    qdev_init_nofail(dev);
>   
>       memory_region_init_io(&s->io, NULL, &serial_mm_ops[end], s,
>                             "serial", 8 << it_shift);
>       memory_region_add_subregion(address_space, base, &s->io);
>       return s;
>   }
> +
> +static void serial_register_types(void)
> +{
> +    type_register_static(&serial_info);
> +}
> +
> +type_init(serial_register_types)
> diff --git a/include/hw/char/serial.h b/include/hw/char/serial.h
> index 8be3d8a4f9..180cc7c24e 100644
> --- a/include/hw/char/serial.h
> +++ b/include/hw/char/serial.h
> @@ -30,10 +30,13 @@
>   #include "exec/memory.h"
>   #include "qemu/fifo8.h"
>   #include "chardev/char.h"
> +#include "hw/sysbus.h"
>   
>   #define UART_FIFO_LENGTH    16      /* 16550A Fifo Length */
>   
>   typedef struct SerialState {
> +    DeviceState parent;
> +
>       uint16_t divider;
>       uint8_t rbr; /* receive register */
>       uint8_t thr; /* transmit holding register */
> @@ -84,7 +87,9 @@ void serial_realize_core(SerialState *s, Error **errp);
>   void serial_exit_core(SerialState *s);
>   void serial_set_frequency(SerialState *s, uint32_t frequency);
>   
> -/* legacy pre qom */
> +#define TYPE_SERIAL "serial"
> +#define SERIAL(s) OBJECT_CHECK(SerialState, (s), TYPE_SERIAL)
> +
>   SerialState *serial_init(int base, qemu_irq irq, int baudbase,
>                            Chardev *chr, MemoryRegion *system_io);
>   SerialState *serial_mm_init(MemoryRegion *address_space,
>
Marc-André Lureau Nov. 20, 2019, 4:45 p.m. UTC | #2
Hi

On Wed, Nov 20, 2019 at 8:42 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> On 11/20/19 4:24 PM, Marc-André Lureau wrote:
> > Make SerialState a device (the following patches will introduce IO/MM
> > sysbus serial devices)
> >
> > None of the serial_{,mm}_init() callers actually free the returned
> > value (even if they did, it would be quite harmless), so we can change
> > the object allocation at will.
> >
> > However, the devices that embed SerialState must now have their field
> > QOM-initialized manually (isa, pci, pci-multi).
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >   hw/char/serial-isa.c       |  9 +++++++++
> >   hw/char/serial-pci-multi.c | 15 +++++++++++++++
> >   hw/char/serial-pci.c       | 13 ++++++++++++-
> >   hw/char/serial.c           | 33 +++++++++++++++++++++++++++------
> >   include/hw/char/serial.h   |  7 ++++++-
> >   5 files changed, 69 insertions(+), 8 deletions(-)
> >
> > diff --git a/hw/char/serial-isa.c b/hw/char/serial-isa.c
> > index 9e31c51bb6..9a5928b3ee 100644
> > --- a/hw/char/serial-isa.c
> > +++ b/hw/char/serial-isa.c
> > @@ -111,10 +111,19 @@ static void serial_isa_class_initfn(ObjectClass *klass, void *data)
> >       set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
> >   }
> >
> > +static void serial_isa_initfn(Object *o)
> > +{
> > +    ISASerialState *self = ISA_SERIAL(o);
> > +
> > +    object_initialize_child(o, "serial", &self->state, sizeof(self->state),
> > +                            TYPE_SERIAL, &error_abort, NULL);
> > +}
> > +
> >   static const TypeInfo serial_isa_info = {
> >       .name          = TYPE_ISA_SERIAL,
> >       .parent        = TYPE_ISA_DEVICE,
> >       .instance_size = sizeof(ISASerialState),
> > +    .instance_init = serial_isa_initfn,
> >       .class_init    = serial_isa_class_initfn,
> >   };
> >
> > diff --git a/hw/char/serial-pci-multi.c b/hw/char/serial-pci-multi.c
> > index 5c553c30d0..edfbfdca9e 100644
> > --- a/hw/char/serial-pci-multi.c
> > +++ b/hw/char/serial-pci-multi.c
> > @@ -181,10 +181,24 @@ static void multi_4x_serial_pci_class_initfn(ObjectClass *klass, void *data)
> >       set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
> >   }
> >
> > +static void multi_serial_init(Object *o)
> > +{
> > +    PCIDevice *dev = PCI_DEVICE(o);
> > +    PCIMultiSerialState *pms = DO_UPCAST(PCIMultiSerialState, dev, dev);
> > +    size_t i, nports = multi_serial_get_port_count(PCI_DEVICE_GET_CLASS(dev));
> > +
> > +    for (i = 0; i < nports; i++) {
> > +        object_initialize_child(o, "serial[*]", &pms->state[i],
> > +                                sizeof(pms->state[i]),
> > +                                TYPE_SERIAL, &error_abort, NULL);
> > +    }
> > +}
> > +
> >   static const TypeInfo multi_2x_serial_pci_info = {
> >       .name          = "pci-serial-2x",
> >       .parent        = TYPE_PCI_DEVICE,
> >       .instance_size = sizeof(PCIMultiSerialState),
> > +    .instance_init = multi_serial_init,
> >       .class_init    = multi_2x_serial_pci_class_initfn,
> >       .interfaces = (InterfaceInfo[]) {
> >           { INTERFACE_CONVENTIONAL_PCI_DEVICE },
> > @@ -196,6 +210,7 @@ static const TypeInfo multi_4x_serial_pci_info = {
> >       .name          = "pci-serial-4x",
> >       .parent        = TYPE_PCI_DEVICE,
> >       .instance_size = sizeof(PCIMultiSerialState),
> > +    .instance_init = multi_serial_init,
> >       .class_init    = multi_4x_serial_pci_class_initfn,
> >       .interfaces = (InterfaceInfo[]) {
> >           { INTERFACE_CONVENTIONAL_PCI_DEVICE },
> > diff --git a/hw/char/serial-pci.c b/hw/char/serial-pci.c
> > index cb9b76e22b..f99b6c19e0 100644
> > --- a/hw/char/serial-pci.c
> > +++ b/hw/char/serial-pci.c
> > @@ -40,6 +40,8 @@ typedef struct PCISerialState {
> >       uint8_t prog_if;
> >   } PCISerialState;
> >
> > +#define TYPE_PCI_SERIAL "pci-serial"
> > +#define PCI_SERIAL(s) OBJECT_CHECK(PCISerialState, (s), TYPE_PCI_SERIAL)
> >
> >   static void serial_pci_realize(PCIDevice *dev, Error **errp)
> >   {
> > @@ -103,10 +105,19 @@ static void serial_pci_class_initfn(ObjectClass *klass, void *data)
> >       set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
> >   }
> >
> > +static void serial_pci_init(Object *o)
> > +{
> > +    PCISerialState *ps = PCI_SERIAL(o);
> > +
> > +    object_initialize_child(o, "serial", &ps->state, sizeof(ps->state),
> > +                            TYPE_SERIAL, &error_abort, NULL);
> > +}
> > +
> >   static const TypeInfo serial_pci_info = {
> > -    .name          = "pci-serial",
> > +    .name          = TYPE_PCI_SERIAL,
> >       .parent        = TYPE_PCI_DEVICE,
> >       .instance_size = sizeof(PCISerialState),
> > +    .instance_init = serial_pci_init,
> >       .class_init    = serial_pci_class_initfn,
> >       .interfaces = (InterfaceInfo[]) {
> >           { INTERFACE_CONVENTIONAL_PCI_DEVICE },
> > diff --git a/hw/char/serial.c b/hw/char/serial.c
> > index b4aa250950..e0a5bec290 100644
> > --- a/hw/char/serial.c
> > +++ b/hw/char/serial.c
> > @@ -983,9 +983,8 @@ const MemoryRegionOps serial_io_ops = {
> >   SerialState *serial_init(int base, qemu_irq irq, int baudbase,
> >                            Chardev *chr, MemoryRegion *system_io)
> >   {
> > -    SerialState *s;
> > -
> > -    s = g_malloc0(sizeof(SerialState));
> > +    DeviceState *dev = DEVICE(object_new(TYPE_SERIAL));
> > +    SerialState *s = SERIAL(dev);
> >
> >       s->irq = irq;
> >       s->baudbase = baudbase;
> > @@ -993,6 +992,7 @@ SerialState *serial_init(int base, qemu_irq irq, int baudbase,
> >       serial_realize_core(s, &error_fatal);
> >
> >       vmstate_register(NULL, base, &vmstate_serial, s);
> > +    qdev_init_nofail(dev);
> >
> >       memory_region_init_io(&s->io, NULL, &serial_io_ops, s, "serial", 8);
> >       memory_region_add_subregion(system_io, base, &s->io);
> > @@ -1000,6 +1000,20 @@ SerialState *serial_init(int base, qemu_irq irq, int baudbase,
> >       return s;
> >   }
> >
> > +static void serial_class_init(ObjectClass *klass, void *data)
> > +{
> > +    DeviceClass *dc = DEVICE_CLASS(klass);
> > +
> > +    dc->user_creatable = false;
>
> Why? Due to the "chardev" property?

No, because it's not user usable as such (it's an internal device for
serialio/serialmm and others). And making abstract wouldn't work
either.

>
> > +}
> > +
> > +static const TypeInfo serial_info = {
> > +    .name = TYPE_SERIAL,
> > +    .parent = TYPE_DEVICE,
> > +    .instance_size = sizeof(SerialState),
> > +    .class_init = serial_class_init,
> > +};
> > +
> >   /* Memory mapped interface */
> >   static uint64_t serial_mm_read(void *opaque, hwaddr addr,
> >                                  unsigned size)
> > @@ -1045,9 +1059,8 @@ SerialState *serial_mm_init(MemoryRegion *address_space,
> >                               qemu_irq irq, int baudbase,
> >                               Chardev *chr, enum device_endian end)
> >   {
> > -    SerialState *s;
> > -
> > -    s = g_malloc0(sizeof(SerialState));
> > +    DeviceState *dev = DEVICE(object_new(TYPE_SERIAL));
> > +    SerialState *s = SERIAL(dev);
> >
> >       s->it_shift = it_shift;
> >       s->irq = irq;
> > @@ -1056,9 +1069,17 @@ SerialState *serial_mm_init(MemoryRegion *address_space,
> >
> >       serial_realize_core(s, &error_fatal);
> >       vmstate_register(NULL, base, &vmstate_serial, s);
> > +    qdev_init_nofail(dev);
> >
> >       memory_region_init_io(&s->io, NULL, &serial_mm_ops[end], s,
> >                             "serial", 8 << it_shift);
> >       memory_region_add_subregion(address_space, base, &s->io);
> >       return s;
> >   }
> > +
> > +static void serial_register_types(void)
> > +{
> > +    type_register_static(&serial_info);
> > +}
> > +
> > +type_init(serial_register_types)
> > diff --git a/include/hw/char/serial.h b/include/hw/char/serial.h
> > index 8be3d8a4f9..180cc7c24e 100644
> > --- a/include/hw/char/serial.h
> > +++ b/include/hw/char/serial.h
> > @@ -30,10 +30,13 @@
> >   #include "exec/memory.h"
> >   #include "qemu/fifo8.h"
> >   #include "chardev/char.h"
> > +#include "hw/sysbus.h"
> >
> >   #define UART_FIFO_LENGTH    16      /* 16550A Fifo Length */
> >
> >   typedef struct SerialState {
> > +    DeviceState parent;
> > +
> >       uint16_t divider;
> >       uint8_t rbr; /* receive register */
> >       uint8_t thr; /* transmit holding register */
> > @@ -84,7 +87,9 @@ void serial_realize_core(SerialState *s, Error **errp);
> >   void serial_exit_core(SerialState *s);
> >   void serial_set_frequency(SerialState *s, uint32_t frequency);
> >
> > -/* legacy pre qom */
> > +#define TYPE_SERIAL "serial"
> > +#define SERIAL(s) OBJECT_CHECK(SerialState, (s), TYPE_SERIAL)
> > +
> >   SerialState *serial_init(int base, qemu_irq irq, int baudbase,
> >                            Chardev *chr, MemoryRegion *system_io);
> >   SerialState *serial_mm_init(MemoryRegion *address_space,
> >
>
>
Peter Maydell Nov. 21, 2019, 1:39 p.m. UTC | #3
On Wed, 20 Nov 2019 at 15:25, Marc-André Lureau
<marcandre.lureau@redhat.com> wrote:
>
> Make SerialState a device (the following patches will introduce IO/MM
> sysbus serial devices)
>
> None of the serial_{,mm}_init() callers actually free the returned
> value (even if they did, it would be quite harmless), so we can change
> the object allocation at will.
>
> However, the devices that embed SerialState must now have their field
> QOM-initialized manually (isa, pci, pci-multi).
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> +static void serial_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->user_creatable = false;

The comment describing user_creatable in qdev-core.h says:
     * It should never be cleared without a comment explaining why it
     * is cleared.

So we should have a comment here.

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM
diff mbox series

Patch

diff --git a/hw/char/serial-isa.c b/hw/char/serial-isa.c
index 9e31c51bb6..9a5928b3ee 100644
--- a/hw/char/serial-isa.c
+++ b/hw/char/serial-isa.c
@@ -111,10 +111,19 @@  static void serial_isa_class_initfn(ObjectClass *klass, void *data)
     set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
 }
 
+static void serial_isa_initfn(Object *o)
+{
+    ISASerialState *self = ISA_SERIAL(o);
+
+    object_initialize_child(o, "serial", &self->state, sizeof(self->state),
+                            TYPE_SERIAL, &error_abort, NULL);
+}
+
 static const TypeInfo serial_isa_info = {
     .name          = TYPE_ISA_SERIAL,
     .parent        = TYPE_ISA_DEVICE,
     .instance_size = sizeof(ISASerialState),
+    .instance_init = serial_isa_initfn,
     .class_init    = serial_isa_class_initfn,
 };
 
diff --git a/hw/char/serial-pci-multi.c b/hw/char/serial-pci-multi.c
index 5c553c30d0..edfbfdca9e 100644
--- a/hw/char/serial-pci-multi.c
+++ b/hw/char/serial-pci-multi.c
@@ -181,10 +181,24 @@  static void multi_4x_serial_pci_class_initfn(ObjectClass *klass, void *data)
     set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
 }
 
+static void multi_serial_init(Object *o)
+{
+    PCIDevice *dev = PCI_DEVICE(o);
+    PCIMultiSerialState *pms = DO_UPCAST(PCIMultiSerialState, dev, dev);
+    size_t i, nports = multi_serial_get_port_count(PCI_DEVICE_GET_CLASS(dev));
+
+    for (i = 0; i < nports; i++) {
+        object_initialize_child(o, "serial[*]", &pms->state[i],
+                                sizeof(pms->state[i]),
+                                TYPE_SERIAL, &error_abort, NULL);
+    }
+}
+
 static const TypeInfo multi_2x_serial_pci_info = {
     .name          = "pci-serial-2x",
     .parent        = TYPE_PCI_DEVICE,
     .instance_size = sizeof(PCIMultiSerialState),
+    .instance_init = multi_serial_init,
     .class_init    = multi_2x_serial_pci_class_initfn,
     .interfaces = (InterfaceInfo[]) {
         { INTERFACE_CONVENTIONAL_PCI_DEVICE },
@@ -196,6 +210,7 @@  static const TypeInfo multi_4x_serial_pci_info = {
     .name          = "pci-serial-4x",
     .parent        = TYPE_PCI_DEVICE,
     .instance_size = sizeof(PCIMultiSerialState),
+    .instance_init = multi_serial_init,
     .class_init    = multi_4x_serial_pci_class_initfn,
     .interfaces = (InterfaceInfo[]) {
         { INTERFACE_CONVENTIONAL_PCI_DEVICE },
diff --git a/hw/char/serial-pci.c b/hw/char/serial-pci.c
index cb9b76e22b..f99b6c19e0 100644
--- a/hw/char/serial-pci.c
+++ b/hw/char/serial-pci.c
@@ -40,6 +40,8 @@  typedef struct PCISerialState {
     uint8_t prog_if;
 } PCISerialState;
 
+#define TYPE_PCI_SERIAL "pci-serial"
+#define PCI_SERIAL(s) OBJECT_CHECK(PCISerialState, (s), TYPE_PCI_SERIAL)
 
 static void serial_pci_realize(PCIDevice *dev, Error **errp)
 {
@@ -103,10 +105,19 @@  static void serial_pci_class_initfn(ObjectClass *klass, void *data)
     set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
 }
 
+static void serial_pci_init(Object *o)
+{
+    PCISerialState *ps = PCI_SERIAL(o);
+
+    object_initialize_child(o, "serial", &ps->state, sizeof(ps->state),
+                            TYPE_SERIAL, &error_abort, NULL);
+}
+
 static const TypeInfo serial_pci_info = {
-    .name          = "pci-serial",
+    .name          = TYPE_PCI_SERIAL,
     .parent        = TYPE_PCI_DEVICE,
     .instance_size = sizeof(PCISerialState),
+    .instance_init = serial_pci_init,
     .class_init    = serial_pci_class_initfn,
     .interfaces = (InterfaceInfo[]) {
         { INTERFACE_CONVENTIONAL_PCI_DEVICE },
diff --git a/hw/char/serial.c b/hw/char/serial.c
index b4aa250950..e0a5bec290 100644
--- a/hw/char/serial.c
+++ b/hw/char/serial.c
@@ -983,9 +983,8 @@  const MemoryRegionOps serial_io_ops = {
 SerialState *serial_init(int base, qemu_irq irq, int baudbase,
                          Chardev *chr, MemoryRegion *system_io)
 {
-    SerialState *s;
-
-    s = g_malloc0(sizeof(SerialState));
+    DeviceState *dev = DEVICE(object_new(TYPE_SERIAL));
+    SerialState *s = SERIAL(dev);
 
     s->irq = irq;
     s->baudbase = baudbase;
@@ -993,6 +992,7 @@  SerialState *serial_init(int base, qemu_irq irq, int baudbase,
     serial_realize_core(s, &error_fatal);
 
     vmstate_register(NULL, base, &vmstate_serial, s);
+    qdev_init_nofail(dev);
 
     memory_region_init_io(&s->io, NULL, &serial_io_ops, s, "serial", 8);
     memory_region_add_subregion(system_io, base, &s->io);
@@ -1000,6 +1000,20 @@  SerialState *serial_init(int base, qemu_irq irq, int baudbase,
     return s;
 }
 
+static void serial_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->user_creatable = false;
+}
+
+static const TypeInfo serial_info = {
+    .name = TYPE_SERIAL,
+    .parent = TYPE_DEVICE,
+    .instance_size = sizeof(SerialState),
+    .class_init = serial_class_init,
+};
+
 /* Memory mapped interface */
 static uint64_t serial_mm_read(void *opaque, hwaddr addr,
                                unsigned size)
@@ -1045,9 +1059,8 @@  SerialState *serial_mm_init(MemoryRegion *address_space,
                             qemu_irq irq, int baudbase,
                             Chardev *chr, enum device_endian end)
 {
-    SerialState *s;
-
-    s = g_malloc0(sizeof(SerialState));
+    DeviceState *dev = DEVICE(object_new(TYPE_SERIAL));
+    SerialState *s = SERIAL(dev);
 
     s->it_shift = it_shift;
     s->irq = irq;
@@ -1056,9 +1069,17 @@  SerialState *serial_mm_init(MemoryRegion *address_space,
 
     serial_realize_core(s, &error_fatal);
     vmstate_register(NULL, base, &vmstate_serial, s);
+    qdev_init_nofail(dev);
 
     memory_region_init_io(&s->io, NULL, &serial_mm_ops[end], s,
                           "serial", 8 << it_shift);
     memory_region_add_subregion(address_space, base, &s->io);
     return s;
 }
+
+static void serial_register_types(void)
+{
+    type_register_static(&serial_info);
+}
+
+type_init(serial_register_types)
diff --git a/include/hw/char/serial.h b/include/hw/char/serial.h
index 8be3d8a4f9..180cc7c24e 100644
--- a/include/hw/char/serial.h
+++ b/include/hw/char/serial.h
@@ -30,10 +30,13 @@ 
 #include "exec/memory.h"
 #include "qemu/fifo8.h"
 #include "chardev/char.h"
+#include "hw/sysbus.h"
 
 #define UART_FIFO_LENGTH    16      /* 16550A Fifo Length */
 
 typedef struct SerialState {
+    DeviceState parent;
+
     uint16_t divider;
     uint8_t rbr; /* receive register */
     uint8_t thr; /* transmit holding register */
@@ -84,7 +87,9 @@  void serial_realize_core(SerialState *s, Error **errp);
 void serial_exit_core(SerialState *s);
 void serial_set_frequency(SerialState *s, uint32_t frequency);
 
-/* legacy pre qom */
+#define TYPE_SERIAL "serial"
+#define SERIAL(s) OBJECT_CHECK(SerialState, (s), TYPE_SERIAL)
+
 SerialState *serial_init(int base, qemu_irq irq, int baudbase,
                          Chardev *chr, MemoryRegion *system_io);
 SerialState *serial_mm_init(MemoryRegion *address_space,