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,
> >
>
>
--
Marc-André Lureau