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

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

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,
>