[PATCH v2 00/19] hw: Use QOM macros and remove DO_UPCAST() uses

[PATCH v2 00/19] hw: Use QOM macros and remove DO_UPCAST() uses

[Philippe Mathieu-Daudé]

v2: Rebased

QOM housekeeping series which replace the DO_UPCAST() macro
uses by equivalent QOM ones. Also:
- Use DEVICE() macro
- Define some TYPE_xxx
- Define some type arrays using DEFINE_TYPES() macro
- Introduce abstract QOM (QDev) parent when relevant.

Based-on: <20230213070423.76428-1-philmd@linaro.org>
          hw/qdev: Housekeeping around qdev_get_parent_bus()

Philippe Mathieu-Daudé (19):
  hw/char/serial-pci: Replace DO_UPCAST(PCISerialState) by PCI_SERIAL()
  hw/char/serial-pci-multi: Batch register types using DEFINE_TYPES
    macro
  hw/char/serial-pci-multi: Introduce PCI_MULTISERIAL QOM abstract
    parent
  hw/char/serial-pci-multi: Factor multi_serial_class_initfn() out
  hw/char/serial-pci-multi: Replace DO_UPCAST() by PCI_MULTISERIAL()
  hw/ide/qdev: Replace DO_UPCAST(IDEDevice) by IDE_DEVICE()
  hw/ide/qdev: Replace DO_UPCAST(IDEBus) by IDE_BUS()
  hw/net/eepro100: Introduce TYPE_EEPRO100 QOM abstract parent
  hw/net/eepro100: Replace DO_UPCAST(EEPRO100State) by EEPRO100()
  hw/net/ne2000-pci: Replace DO_UPCAST(PCINE2000State) by PCI_NE2000()
  hw/net/tulip: Finish QOM conversion
  hw/pci/pci: Replace DO_UPCAST(PCIBus) by PCI_BUS()
  hw/scsi/scsi-bus: Replace DO_UPCAST(SCSIBus) by SCSI_BUS()
  hw/scsi/scsi-bus: Inline two uses of scsi_bus_from_device()
  hw/s390x/event-facility: Replace DO_UPCAST(SCLPEvent) by SCLP_EVENT()
  hw/vfio/ccw: Replace DO_UPCAST(VFIOCCWDevice) by VFIO_CCW()
  hw/usb/dev-hub: Use QOM USB_HUB() macro instead of casting
  hw/usb: Replace DO_UPCAST(USBBus) by USB_BUS()
  hw/usb: Inline usb_bus_from_device()

 hw/char/serial-pci-multi.c | 93 +++++++++++++++++++-------------------
 hw/char/serial-pci.c       |  7 ++-
 hw/ide/qdev.c              | 10 ++--
 hw/net/eepro100.c          | 46 ++++++++++++-------
 hw/net/ne2000-pci.c        | 18 +++++---
 hw/net/tulip.c             | 20 ++++----
 hw/pci/pci.c               |  2 +-
 hw/s390x/event-facility.c  |  3 +-
 hw/s390x/ipl.c             |  7 +--
 hw/scsi/scsi-bus.c         | 14 +++---
 hw/usb/bus.c               | 10 ++--
 hw/usb/core.c              |  6 +--
 hw/usb/dev-hub.c           | 10 ++--
 hw/usb/dev-serial.c        | 10 ++--
 hw/usb/hcd-xhci.c          |  2 +-
 hw/vfio/ccw.c              | 35 +++++++-------
 include/hw/scsi/scsi.h     |  5 --
 include/hw/usb.h           |  5 --
 18 files changed, 155 insertions(+), 148 deletions(-)

-- 
2.38.1

[PATCH v2 01/19] hw/char/serial-pci: Replace DO_UPCAST(PCISerialState) by PCI_SERIAL()

[Philippe Mathieu-Daudé]

Use the PCI_SERIAL() QOM type-checking macro to avoid DO_UPCAST().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/char/serial-pci.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/hw/char/serial-pci.c b/hw/char/serial-pci.c
index 801b769aba..9689645cac 100644
--- a/hw/char/serial-pci.c
+++ b/hw/char/serial-pci.c
@@ -36,7 +36,10 @@
 #include "qom/object.h"
 
 struct PCISerialState {
+    /*< private >*/
     PCIDevice dev;
+    /*< public >*/
+
     SerialState state;
     uint8_t prog_if;
 };
@@ -46,7 +49,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(PCISerialState, PCI_SERIAL)
 
 static void serial_pci_realize(PCIDevice *dev, Error **errp)
 {
-    PCISerialState *pci = DO_UPCAST(PCISerialState, dev, dev);
+    PCISerialState *pci = PCI_SERIAL(dev);
     SerialState *s = &pci->state;
 
     if (!qdev_realize(DEVICE(s), NULL, errp)) {
@@ -63,7 +66,7 @@ static void serial_pci_realize(PCIDevice *dev, Error **errp)
 
 static void serial_pci_exit(PCIDevice *dev)
 {
-    PCISerialState *pci = DO_UPCAST(PCISerialState, dev, dev);
+    PCISerialState *pci = PCI_SERIAL(dev);
     SerialState *s = &pci->state;
 
     qdev_unrealize(DEVICE(s));
-- 
2.38.1

[PATCH v2 02/19] hw/char/serial-pci-multi: Batch register types using DEFINE_TYPES macro

[Philippe Mathieu-Daudé]

See rationale in commit 38b5d79b2e ("qom: add helper
macro DEFINE_TYPES()").

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/char/serial-pci-multi.c | 52 +++++++++++++++++---------------------
 1 file changed, 23 insertions(+), 29 deletions(-)

diff --git a/hw/char/serial-pci-multi.c b/hw/char/serial-pci-multi.c
index f18b8dcce5..54768d3d53 100644
--- a/hw/char/serial-pci-multi.c
+++ b/hw/char/serial-pci-multi.c
@@ -189,34 +189,28 @@ static void multi_serial_init(Object *o)
     }
 }
 
-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 },
-        { },
-    },
+static const TypeInfo multi_serial_pci_types[] = {
+    {
+        .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 },
+            { },
+        },
+    }, {
+        .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 },
+            { },
+        },
+    }
 };
 
-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 },
-        { },
-    },
-};
-
-static void multi_serial_pci_register_types(void)
-{
-    type_register_static(&multi_2x_serial_pci_info);
-    type_register_static(&multi_4x_serial_pci_info);
-}
-
-type_init(multi_serial_pci_register_types)
+DEFINE_TYPES(multi_serial_pci_types)
-- 
2.38.1

[PATCH v2 03/19] hw/char/serial-pci-multi: Introduce PCI_MULTISERIAL QOM abstract parent

[Philippe Mathieu-Daudé]

Introduce PCI_MULTISERIAL ("pci-serial"), QOM abstract parent of
"pci-serial-2x" and "pci-serial-4x".

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/char/serial-pci-multi.c | 35 ++++++++++++++++++++---------------
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/hw/char/serial-pci-multi.c b/hw/char/serial-pci-multi.c
index 54768d3d53..faeb0a9476 100644
--- a/hw/char/serial-pci-multi.c
+++ b/hw/char/serial-pci-multi.c
@@ -38,8 +38,15 @@
 
 #define PCI_SERIAL_MAX_PORTS 4
 
-typedef struct PCIMultiSerialState {
+#define TYPE_PCI_MULTISERIAL  "pci-serial"
+
+OBJECT_DECLARE_SIMPLE_TYPE(PCIMultiSerialState, PCI_MULTISERIAL)
+
+struct PCIMultiSerialState {
+    /*< private >*/
     PCIDevice    dev;
+    /*< public >*/
+
     MemoryRegion iobar;
     uint32_t     ports;
     char         *name[PCI_SERIAL_MAX_PORTS];
@@ -47,7 +54,7 @@ typedef struct PCIMultiSerialState {
     uint32_t     level[PCI_SERIAL_MAX_PORTS];
     qemu_irq     *irqs;
     uint8_t      prog_if;
-} PCIMultiSerialState;
+};
 
 static void multi_serial_pci_exit(PCIDevice *dev)
 {
@@ -191,25 +198,23 @@ static void multi_serial_init(Object *o)
 
 static const TypeInfo multi_serial_pci_types[] = {
     {
-        .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[]) {
+        .name           = TYPE_PCI_MULTISERIAL,
+        .parent         = TYPE_PCI_DEVICE,
+        .instance_size  = sizeof(PCIMultiSerialState),
+        .instance_init  = multi_serial_init,
+        .abstract       = true,
+        .interfaces     = (InterfaceInfo[]) {
             { INTERFACE_CONVENTIONAL_PCI_DEVICE },
             { },
         },
+    }, {
+        .name          = "pci-serial-2x",
+        .parent        = TYPE_PCI_MULTISERIAL,
+        .class_init    = multi_2x_serial_pci_class_initfn,
     }, {
         .name          = "pci-serial-4x",
-        .parent        = TYPE_PCI_DEVICE,
-        .instance_size = sizeof(PCIMultiSerialState),
-        .instance_init = multi_serial_init,
+        .parent        = TYPE_PCI_MULTISERIAL,
         .class_init    = multi_4x_serial_pci_class_initfn,
-        .interfaces = (InterfaceInfo[]) {
-            { INTERFACE_CONVENTIONAL_PCI_DEVICE },
-            { },
-        },
     }
 };
 
-- 
2.38.1

Re: [PATCH v2 03/19] hw/char/serial-pci-multi: Introduce PCI_MULTISERIAL QOM abstract parent

[Philippe Mathieu-Daudé]

On 13/2/23 08:08, Philippe Mathieu-Daudé wrote:
> Introduce PCI_MULTISERIAL ("pci-serial"), QOM abstract parent of
> "pci-serial-2x" and "pci-serial-4x".
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/char/serial-pci-multi.c | 35 ++++++++++++++++++++---------------
>   1 file changed, 20 insertions(+), 15 deletions(-)
> 
> diff --git a/hw/char/serial-pci-multi.c b/hw/char/serial-pci-multi.c
> index 54768d3d53..faeb0a9476 100644
> --- a/hw/char/serial-pci-multi.c
> +++ b/hw/char/serial-pci-multi.c
> @@ -38,8 +38,15 @@
>   
>   #define PCI_SERIAL_MAX_PORTS 4
>   
> -typedef struct PCIMultiSerialState {
> +#define TYPE_PCI_MULTISERIAL  "pci-serial"

Ouch, copy/paste mistake, this should be "pci-serial-multi".

> +
> +OBJECT_DECLARE_SIMPLE_TYPE(PCIMultiSerialState, PCI_MULTISERIAL)
> +
> +struct PCIMultiSerialState {
> +    /*< private >*/
>       PCIDevice    dev;
> +    /*< public >*/
> +
>       MemoryRegion iobar;
>       uint32_t     ports;
>       char         *name[PCI_SERIAL_MAX_PORTS];
> @@ -47,7 +54,7 @@ typedef struct PCIMultiSerialState {
>       uint32_t     level[PCI_SERIAL_MAX_PORTS];
>       qemu_irq     *irqs;
>       uint8_t      prog_if;
> -} PCIMultiSerialState;
> +};
>   
>   static void multi_serial_pci_exit(PCIDevice *dev)
>   {
> @@ -191,25 +198,23 @@ static void multi_serial_init(Object *o)
>   
>   static const TypeInfo multi_serial_pci_types[] = {
>       {
> -        .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[]) {
> +        .name           = TYPE_PCI_MULTISERIAL,
> +        .parent         = TYPE_PCI_DEVICE,
> +        .instance_size  = sizeof(PCIMultiSerialState),
> +        .instance_init  = multi_serial_init,
> +        .abstract       = true,
> +        .interfaces     = (InterfaceInfo[]) {
>               { INTERFACE_CONVENTIONAL_PCI_DEVICE },
>               { },
>           },
> +    }, {
> +        .name          = "pci-serial-2x",
> +        .parent        = TYPE_PCI_MULTISERIAL,
> +        .class_init    = multi_2x_serial_pci_class_initfn,
>       }, {
>           .name          = "pci-serial-4x",
> -        .parent        = TYPE_PCI_DEVICE,
> -        .instance_size = sizeof(PCIMultiSerialState),
> -        .instance_init = multi_serial_init,
> +        .parent        = TYPE_PCI_MULTISERIAL,
>           .class_init    = multi_4x_serial_pci_class_initfn,
> -        .interfaces = (InterfaceInfo[]) {
> -            { INTERFACE_CONVENTIONAL_PCI_DEVICE },
> -            { },
> -        },
>       }
>   };
>   

[PATCH v2 04/19] hw/char/serial-pci-multi: Factor multi_serial_class_initfn() out

[Philippe Mathieu-Daudé]

Extract code common to multi_2x_serial_pci_class_initfn() and
multi_4x_serial_pci_class_initfn() to multi_serial_class_initfn().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/char/serial-pci-multi.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/hw/char/serial-pci-multi.c b/hw/char/serial-pci-multi.c
index faeb0a9476..cd5af24bd2 100644
--- a/hw/char/serial-pci-multi.c
+++ b/hw/char/serial-pci-multi.c
@@ -155,14 +155,14 @@ static Property multi_4x_serial_pci_properties[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
-static void multi_2x_serial_pci_class_initfn(ObjectClass *klass, void *data)
+static void multi_serial_class_initfn(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *pc = PCI_DEVICE_CLASS(klass);
+
     pc->realize = multi_serial_pci_realize;
     pc->exit = multi_serial_pci_exit;
     pc->vendor_id = PCI_VENDOR_ID_REDHAT;
-    pc->device_id = PCI_DEVICE_ID_REDHAT_SERIAL2;
     pc->revision = 1;
     pc->class_id = PCI_CLASS_COMMUNICATION_SERIAL;
     dc->vmsd = &vmstate_pci_multi_serial;
@@ -170,19 +170,22 @@ static void multi_2x_serial_pci_class_initfn(ObjectClass *klass, void *data)
     set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
 }
 
+static void multi_2x_serial_pci_class_initfn(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIDeviceClass *pc = PCI_DEVICE_CLASS(klass);
+
+    pc->device_id = PCI_DEVICE_ID_REDHAT_SERIAL2;
+    device_class_set_props(dc, multi_2x_serial_pci_properties);
+}
+
 static void multi_4x_serial_pci_class_initfn(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *pc = PCI_DEVICE_CLASS(klass);
-    pc->realize = multi_serial_pci_realize;
-    pc->exit = multi_serial_pci_exit;
-    pc->vendor_id = PCI_VENDOR_ID_REDHAT;
+
     pc->device_id = PCI_DEVICE_ID_REDHAT_SERIAL4;
-    pc->revision = 1;
-    pc->class_id = PCI_CLASS_COMMUNICATION_SERIAL;
-    dc->vmsd = &vmstate_pci_multi_serial;
     device_class_set_props(dc, multi_4x_serial_pci_properties);
-    set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
 }
 
 static void multi_serial_init(Object *o)
@@ -202,6 +205,7 @@ static const TypeInfo multi_serial_pci_types[] = {
         .parent         = TYPE_PCI_DEVICE,
         .instance_size  = sizeof(PCIMultiSerialState),
         .instance_init  = multi_serial_init,
+        .class_init     = multi_serial_class_initfn,
         .abstract       = true,
         .interfaces     = (InterfaceInfo[]) {
             { INTERFACE_CONVENTIONAL_PCI_DEVICE },
-- 
2.38.1

[PATCH v2 05/19] hw/char/serial-pci-multi: Replace DO_UPCAST() by PCI_MULTISERIAL()

[Philippe Mathieu-Daudé]

Use the PCI_MULTISERIAL() QOM type-checking macro to avoid the few
DO_UPCAST(PCIMultiSerialState) calls.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/char/serial-pci-multi.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/hw/char/serial-pci-multi.c b/hw/char/serial-pci-multi.c
index cd5af24bd2..6f4491210d 100644
--- a/hw/char/serial-pci-multi.c
+++ b/hw/char/serial-pci-multi.c
@@ -58,7 +58,7 @@ struct PCIMultiSerialState {
 
 static void multi_serial_pci_exit(PCIDevice *dev)
 {
-    PCIMultiSerialState *pci = DO_UPCAST(PCIMultiSerialState, dev, dev);
+    PCIMultiSerialState *pci = PCI_MULTISERIAL(dev);
     SerialState *s;
     int i;
 
@@ -97,11 +97,10 @@ static size_t multi_serial_get_port_count(PCIDeviceClass *pc)
     g_assert_not_reached();
 }
 
-
 static void multi_serial_pci_realize(PCIDevice *dev, Error **errp)
 {
     PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
-    PCIMultiSerialState *pci = DO_UPCAST(PCIMultiSerialState, dev, dev);
+    PCIMultiSerialState *pci = PCI_MULTISERIAL(dev);
     SerialState *s;
     size_t i, nports = multi_serial_get_port_count(pc);
 
@@ -190,9 +189,8 @@ static void multi_4x_serial_pci_class_initfn(ObjectClass *klass, void *data)
 
 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));
+    PCIMultiSerialState *pms = PCI_MULTISERIAL(o);
+    size_t i, nports = multi_serial_get_port_count(PCI_DEVICE_GET_CLASS(o));
 
     for (i = 0; i < nports; i++) {
         object_initialize_child(o, "serial[*]", &pms->state[i], TYPE_SERIAL);
-- 
2.38.1

[PATCH v2 06/19] hw/ide/qdev: Replace DO_UPCAST(IDEDevice) by IDE_DEVICE()

[Philippe Mathieu-Daudé]

Use the IDE_DEVICE() QOM type-checking macro to avoid DO_UPCAST().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/ide/qdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 6ae2627a56..1ead62fd18 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -133,7 +133,7 @@ IDEDevice *ide_create_drive(IDEBus *bus, int unit, DriveInfo *drive)
     qdev_prop_set_drive_err(dev, "drive", blk_by_legacy_dinfo(drive),
                             &error_fatal);
     qdev_realize_and_unref(dev, &bus->qbus, &error_fatal);
-    return DO_UPCAST(IDEDevice, qdev, dev);
+    return IDE_DEVICE(dev);
 }
 
 int ide_get_geometry(BusState *bus, int unit,
-- 
2.38.1

[PATCH v2 07/19] hw/ide/qdev: Replace DO_UPCAST(IDEBus) by IDE_BUS()

[Philippe Mathieu-Daudé]

Use the IDE_BUS() QOM type-checking macro to avoid DO_UPCAST().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/ide/qdev.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 1ead62fd18..a168643266 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -90,7 +90,7 @@ static void ide_qdev_realize(DeviceState *qdev, Error **errp)
 {
     IDEDevice *dev = IDE_DEVICE(qdev);
     IDEDeviceClass *dc = IDE_DEVICE_GET_CLASS(dev);
-    IDEBus *bus = DO_UPCAST(IDEBus, qbus, qdev_get_parent_bus(qdev));
+    IDEBus *bus = IDE_BUS(qdev_get_parent_bus(qdev));
 
     if (dev->unit == -1) {
         dev->unit = bus->master ? 1 : 0;
@@ -139,7 +139,7 @@ IDEDevice *ide_create_drive(IDEBus *bus, int unit, DriveInfo *drive)
 int ide_get_geometry(BusState *bus, int unit,
                      int16_t *cyls, int8_t *heads, int8_t *secs)
 {
-    IDEState *s = &DO_UPCAST(IDEBus, qbus, bus)->ifs[unit];
+    IDEState *s = &IDE_BUS(bus)->ifs[unit];
 
     if (s->drive_kind != IDE_HD || !s->blk) {
         return -1;
@@ -153,7 +153,7 @@ int ide_get_geometry(BusState *bus, int unit,
 
 int ide_get_bios_chs_trans(BusState *bus, int unit)
 {
-    return DO_UPCAST(IDEBus, qbus, bus)->ifs[unit].chs_trans;
+    return IDE_BUS(bus)->ifs[unit].chs_trans;
 }
 
 /* --------------------------------- */
@@ -164,7 +164,7 @@ typedef struct IDEDrive {
 
 static void ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind, Error **errp)
 {
-    IDEBus *bus = DO_UPCAST(IDEBus, qbus, qdev_get_parent_bus(DEVICE(dev)));
+    IDEBus *bus = IDE_BUS(qdev_get_parent_bus(DEVICE(dev)));
     IDEState *s = bus->ifs + dev->unit;
     int ret;
 
-- 
2.38.1

[PATCH v2 08/19] hw/net/eepro100: Introduce TYPE_EEPRO100 QOM abstract parent

[Philippe Mathieu-Daudé]

Have all the EEPRO100-based devices share a common (abstract)
QOM parent.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/net/eepro100.c | 40 ++++++++++++++++++++++++++--------------
 1 file changed, 26 insertions(+), 14 deletions(-)

diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
index dc07984ae9..dac42ba17b 100644
--- a/hw/net/eepro100.c
+++ b/hw/net/eepro100.c
@@ -235,8 +235,14 @@ typedef enum {
     ru_ready = 4
 } ru_state_t;
 
-typedef struct {
+#define TYPE_EEPRO100 "eepro100"
+OBJECT_DECLARE_SIMPLE_TYPE(EEPRO100State, EEPRO100)
+
+struct EEPRO100State {
+    /*< private >*/
     PCIDevice dev;
+    /*< public >*/
+
     /* Hash register (multicast mask array, multiple individual addresses). */
     uint8_t mult[8];
     MemoryRegion mmio_bar;
@@ -279,7 +285,7 @@ typedef struct {
     /* Quasi static device properties (no need to save them). */
     uint16_t stats_size;
     bool has_extended_tcb_support;
-} EEPRO100State;
+};
 
 /* Word indices in EEPROM. */
 typedef enum {
@@ -2082,21 +2088,27 @@ static void eepro100_class_init(ObjectClass *klass, void *data)
     k->subsystem_id = info->subsystem_id;
 }
 
+static const TypeInfo eepro100_info = {
+    .name          = TYPE_EEPRO100,
+    .parent        = TYPE_PCI_DEVICE,
+    .class_init    = eepro100_class_init,
+    .abstract      = true,
+    .instance_size = sizeof(EEPRO100State),
+    .instance_init = eepro100_instance_init,
+    .interfaces = (InterfaceInfo[]) {
+        { INTERFACE_CONVENTIONAL_PCI_DEVICE },
+        { },
+    },
+};
+
 static void eepro100_register_types(void)
 {
-    size_t i;
-    for (i = 0; i < ARRAY_SIZE(e100_devices); i++) {
-        TypeInfo type_info = {};
-        E100PCIDeviceInfo *info = &e100_devices[i];
+    type_register_static(&eepro100_info);
 
-        type_info.name = info->name;
-        type_info.parent = TYPE_PCI_DEVICE;
-        type_info.class_init = eepro100_class_init;
-        type_info.instance_size = sizeof(EEPRO100State);
-        type_info.instance_init = eepro100_instance_init;
-        type_info.interfaces = (InterfaceInfo[]) {
-            { INTERFACE_CONVENTIONAL_PCI_DEVICE },
-            { },
+    for (size_t i = 0; i < ARRAY_SIZE(e100_devices); i++) {
+        TypeInfo type_info = {
+            .name   = e100_devices[i].name,
+            .parent = TYPE_EEPRO100,
         };
 
         type_register(&type_info);
-- 
2.38.1

Re: [PATCH v2 08/19] hw/net/eepro100: Introduce TYPE_EEPRO100 QOM abstract parent

[Philippe Mathieu-Daudé]

On 13/2/23 08:08, Philippe Mathieu-Daudé wrote:
> Have all the EEPRO100-based devices share a common (abstract)
> QOM parent.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/net/eepro100.c | 40 ++++++++++++++++++++++++++--------------
>   1 file changed, 26 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
> index dc07984ae9..dac42ba17b 100644
> --- a/hw/net/eepro100.c
> +++ b/hw/net/eepro100.c
> @@ -235,8 +235,14 @@ typedef enum {
>       ru_ready = 4
>   } ru_state_t;
>   
> -typedef struct {
> +#define TYPE_EEPRO100 "eepro100"
> +OBJECT_DECLARE_SIMPLE_TYPE(EEPRO100State, EEPRO100)

Self-NACK, I'll respin also introducing EEPRO100Class for completeness.

Re: [PATCH v2 08/19] hw/net/eepro100: Introduce TYPE_EEPRO100 QOM abstract parent

[Philippe Mathieu-Daudé]

On 13/2/23 09:36, Philippe Mathieu-Daudé wrote:
> On 13/2/23 08:08, Philippe Mathieu-Daudé wrote:
>> Have all the EEPRO100-based devices share a common (abstract)
>> QOM parent.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/net/eepro100.c | 40 ++++++++++++++++++++++++++--------------
>>   1 file changed, 26 insertions(+), 14 deletions(-)
>>
>> diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
>> index dc07984ae9..dac42ba17b 100644
>> --- a/hw/net/eepro100.c
>> +++ b/hw/net/eepro100.c
>> @@ -235,8 +235,14 @@ typedef enum {
>>       ru_ready = 4
>>   } ru_state_t;
>> -typedef struct {
>> +#define TYPE_EEPRO100 "eepro100"
>> +OBJECT_DECLARE_SIMPLE_TYPE(EEPRO100State, EEPRO100)
> 
> Self-NACK, I'll respin also introducing EEPRO100Class for completeness.

Respin posted here:
https://lore.kernel.org/qemu-devel/20230213101048.94519-1-philmd@linaro.org/

[PATCH v2 09/19] hw/net/eepro100: Replace DO_UPCAST(EEPRO100State) by EEPRO100()

[Philippe Mathieu-Daudé]

Use the EEPRO100() QOM type-checking macro to avoid DO_UPCAST().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/net/eepro100.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
index dac42ba17b..915935a818 100644
--- a/hw/net/eepro100.c
+++ b/hw/net/eepro100.c
@@ -1828,7 +1828,7 @@ static const VMStateDescription vmstate_eepro100 = {
 
 static void pci_nic_uninit(PCIDevice *pci_dev)
 {
-    EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, pci_dev);
+    EEPRO100State *s = EEPRO100(pci_dev);
 
     vmstate_unregister(VMSTATE_IF(&pci_dev->qdev), s->vmstate, s);
     g_free(s->vmstate);
@@ -1844,7 +1844,7 @@ static NetClientInfo net_eepro100_info = {
 
 static void e100_nic_realize(PCIDevice *pci_dev, Error **errp)
 {
-    EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, pci_dev);
+    EEPRO100State *s = EEPRO100(pci_dev);
     E100PCIDeviceInfo *info = eepro100_get_class(s);
     Error *local_err = NULL;
 
@@ -1895,7 +1895,7 @@ static void e100_nic_realize(PCIDevice *pci_dev, Error **errp)
 
 static void eepro100_instance_init(Object *obj)
 {
-    EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, PCI_DEVICE(obj));
+    EEPRO100State *s = EEPRO100(obj);
     device_add_bootindex_property(obj, &s->conf.bootindex,
                                   "bootindex", "/ethernet-phy@0",
                                   DEVICE(s));
-- 
2.38.1

[PATCH v2 10/19] hw/net/ne2000-pci: Replace DO_UPCAST(PCINE2000State) by PCI_NE2000()

[Philippe Mathieu-Daudé]

Define TYPE_PCI_NE2000 and the QOM PCI_NE2000() macro.
Use PCI_NE2000() instead of DO_UPCAST().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/net/ne2000-pci.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/hw/net/ne2000-pci.c b/hw/net/ne2000-pci.c
index edc6689d33..0332e7f616 100644
--- a/hw/net/ne2000-pci.c
+++ b/hw/net/ne2000-pci.c
@@ -30,10 +30,16 @@
 #include "ne2000.h"
 #include "sysemu/sysemu.h"
 
-typedef struct PCINE2000State {
+#define TYPE_PCI_NE2000 "ne2k_pci"
+OBJECT_DECLARE_SIMPLE_TYPE(PCINE2000State, PCI_NE2000)
+
+struct PCINE2000State {
+    /*< private >*/
     PCIDevice dev;
+    /*< public >*/
+
     NE2000State ne2000;
-} PCINE2000State;
+};
 
 static const VMStateDescription vmstate_pci_ne2000 = {
     .name = "ne2000",
@@ -54,7 +60,7 @@ static NetClientInfo net_ne2000_info = {
 
 static void pci_ne2000_realize(PCIDevice *pci_dev, Error **errp)
 {
-    PCINE2000State *d = DO_UPCAST(PCINE2000State, dev, pci_dev);
+    PCINE2000State *d = PCI_NE2000(pci_dev);
     NE2000State *s;
     uint8_t *pci_conf;
 
@@ -77,7 +83,7 @@ static void pci_ne2000_realize(PCIDevice *pci_dev, Error **errp)
 
 static void pci_ne2000_exit(PCIDevice *pci_dev)
 {
-    PCINE2000State *d = DO_UPCAST(PCINE2000State, dev, pci_dev);
+    PCINE2000State *d = PCI_NE2000(pci_dev);
     NE2000State *s = &d->ne2000;
 
     qemu_del_nic(s->nic);
@@ -87,7 +93,7 @@ static void pci_ne2000_exit(PCIDevice *pci_dev)
 static void ne2000_instance_init(Object *obj)
 {
     PCIDevice *pci_dev = PCI_DEVICE(obj);
-    PCINE2000State *d = DO_UPCAST(PCINE2000State, dev, pci_dev);
+    PCINE2000State *d = PCI_NE2000(pci_dev);
     NE2000State *s = &d->ne2000;
 
     device_add_bootindex_property(obj, &s->c.bootindex,
@@ -117,7 +123,7 @@ static void ne2000_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo ne2000_info = {
-    .name          = "ne2k_pci",
+    .name          = TYPE_PCI_NE2000,
     .parent        = TYPE_PCI_DEVICE,
     .instance_size = sizeof(PCINE2000State),
     .class_init    = ne2000_class_init,
-- 
2.38.1

[PATCH v2 11/19] hw/net/tulip: Finish QOM conversion

[Philippe Mathieu-Daudé]

Use the TULIP() and DEVICE() QOM type-checking macros.
Remove uses of DO_UPCAST().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/net/tulip.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/hw/net/tulip.c b/hw/net/tulip.c
index 915e5fb595..990507859d 100644
--- a/hw/net/tulip.c
+++ b/hw/net/tulip.c
@@ -19,7 +19,10 @@
 #include "net/eth.h"
 
 struct TULIPState {
+    /*< private >*/
     PCIDevice dev;
+    /*< public >*/
+
     MemoryRegion io;
     MemoryRegion memory;
     NICConf c;
@@ -959,7 +962,7 @@ static void tulip_fill_eeprom(TULIPState *s)
 
 static void pci_tulip_realize(PCIDevice *pci_dev, Error **errp)
 {
-    TULIPState *s = DO_UPCAST(TULIPState, dev, pci_dev);
+    TULIPState *s = TULIP(pci_dev);
     uint8_t *pci_conf;
 
     pci_conf = s->dev.config;
@@ -967,7 +970,7 @@ static void pci_tulip_realize(PCIDevice *pci_dev, Error **errp)
 
     qemu_macaddr_default_if_unset(&s->c.macaddr);
 
-    s->eeprom = eeprom93xx_new(&pci_dev->qdev, 64);
+    s->eeprom = eeprom93xx_new(DEVICE(pci_dev), 64);
     tulip_fill_eeprom(s);
 
     memory_region_init_io(&s->io, OBJECT(&s->dev), &tulip_ops, s,
@@ -983,27 +986,26 @@ static void pci_tulip_realize(PCIDevice *pci_dev, Error **errp)
 
     s->nic = qemu_new_nic(&net_tulip_info, &s->c,
                           object_get_typename(OBJECT(pci_dev)),
-                          pci_dev->qdev.id, s);
+                          DEVICE(pci_dev)->id, s);
     qemu_format_nic_info_str(qemu_get_queue(s->nic), s->c.macaddr.a);
 }
 
 static void pci_tulip_exit(PCIDevice *pci_dev)
 {
-    TULIPState *s = DO_UPCAST(TULIPState, dev, pci_dev);
+    TULIPState *s = TULIP(pci_dev);
 
     qemu_del_nic(s->nic);
     qemu_free_irq(s->irq);
-    eeprom93xx_free(&pci_dev->qdev, s->eeprom);
+    eeprom93xx_free(DEVICE(s), s->eeprom);
 }
 
 static void tulip_instance_init(Object *obj)
 {
-    PCIDevice *pci_dev = PCI_DEVICE(obj);
-    TULIPState *d = DO_UPCAST(TULIPState, dev, pci_dev);
+    TULIPState *s = TULIP(obj);
 
-    device_add_bootindex_property(obj, &d->c.bootindex,
+    device_add_bootindex_property(obj, &s->c.bootindex,
                                   "bootindex", "/ethernet-phy@0",
-                                  &pci_dev->qdev);
+                                  DEVICE(obj));
 }
 
 static Property tulip_properties[] = {
-- 
2.38.1

[PATCH v2 12/19] hw/pci/pci: Replace DO_UPCAST(PCIBus) by PCI_BUS()

[Philippe Mathieu-Daudé]

Use the PCI_BUS() QOM type-checking macro to avoid DO_UPCAST().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/pci/pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 2e785e3aef..ae5c33adb6 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -391,7 +391,7 @@ void pci_device_reset(PCIDevice *dev)
  */
 static void pcibus_reset(BusState *qbus)
 {
-    PCIBus *bus = DO_UPCAST(PCIBus, qbus, qbus);
+    PCIBus *bus = PCI_BUS(qbus);
     int i;
 
     for (i = 0; i < ARRAY_SIZE(bus->devices); ++i) {
-- 
2.38.1

[PATCH v2 13/19] hw/scsi/scsi-bus: Replace DO_UPCAST(SCSIBus) by SCSI_BUS()

[Philippe Mathieu-Daudé]

Use the SCSI_BUS() QOM type-checking macro to avoid DO_UPCAST().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/scsi/scsi-bus.c     | 12 ++++++------
 include/hw/scsi/scsi.h |  2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 7b2a82b335..c4525515ab 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -104,7 +104,7 @@ static void scsi_device_unrealize(SCSIDevice *s)
 int scsi_bus_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf,
                        size_t buf_len, void *hba_private)
 {
-    SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, qdev_get_parent_bus(DEVICE(dev)));
+    SCSIBus *bus = SCSI_BUS(qdev_get_parent_bus(DEVICE(dev)));
     int rc;
 
     assert(cmd->len == 0);
@@ -250,7 +250,7 @@ static bool scsi_bus_check_address(BusState *qbus, DeviceState *qdev, Error **er
 static void scsi_qdev_realize(DeviceState *qdev, Error **errp)
 {
     SCSIDevice *dev = SCSI_DEVICE(qdev);
-    SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, qdev_get_parent_bus(DEVICE(dev)));
+    SCSIBus *bus = SCSI_BUS(qdev_get_parent_bus(qdev));
     bool is_free;
     Error *local_err = NULL;
 
@@ -705,7 +705,7 @@ SCSIRequest *scsi_req_alloc(const SCSIReqOps *reqops, SCSIDevice *d,
 SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
                           uint8_t *buf, size_t buf_len, void *hba_private)
 {
-    SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, qdev_get_parent_bus(DEVICE(d)));
+    SCSIBus *bus = SCSI_BUS(qdev_get_parent_bus(DEVICE(d)));
     const SCSIReqOps *ops;
     SCSIDeviceClass *sc = SCSI_DEVICE_GET_CLASS(d);
     SCSIRequest *req;
@@ -1353,7 +1353,7 @@ int scsi_req_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf,
 
 void scsi_device_report_change(SCSIDevice *dev, SCSISense sense)
 {
-    SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, qdev_get_parent_bus(DEVICE(dev)));
+    SCSIBus *bus = SCSI_BUS(qdev_get_parent_bus(DEVICE(dev)));
 
     scsi_device_set_ua(dev, sense);
     if (bus->info->change) {
@@ -1698,7 +1698,7 @@ static int put_scsi_requests(QEMUFile *f, void *pv, size_t size,
                              const VMStateField *field, JSONWriter *vmdesc)
 {
     SCSIDevice *s = pv;
-    SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, qdev_get_parent_bus(DEVICE(s)));
+    SCSIBus *bus = SCSI_BUS(qdev_get_parent_bus(DEVICE(s)));
     SCSIRequest *req;
 
     QTAILQ_FOREACH(req, &s->requests, next) {
@@ -1726,7 +1726,7 @@ static int get_scsi_requests(QEMUFile *f, void *pv, size_t size,
                              const VMStateField *field)
 {
     SCSIDevice *s = pv;
-    SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, qdev_get_parent_bus(DEVICE(s)));
+    SCSIBus *bus = SCSI_BUS(qdev_get_parent_bus(DEVICE(s)));
     int8_t sbyte;
 
     while ((sbyte = qemu_get_sbyte(f)) > 0) {
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index 843dde8851..eb558c145a 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -177,7 +177,7 @@ static inline void scsi_bus_init(SCSIBus *bus, size_t bus_size,
 
 static inline SCSIBus *scsi_bus_from_device(SCSIDevice *d)
 {
-    return DO_UPCAST(SCSIBus, qbus, qdev_get_parent_bus(DEVICE(d)));
+    return SCSI_BUS(qdev_get_parent_bus(DEVICE(d)));
 }
 
 SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockBackend *blk,
-- 
2.38.1

[PATCH v2 14/19] hw/scsi/scsi-bus: Inline two uses of scsi_bus_from_device()

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/s390x/ipl.c         | 7 ++-----
 hw/scsi/scsi-bus.c     | 2 +-
 include/hw/scsi/scsi.h | 5 -----
 3 files changed, 3 insertions(+), 11 deletions(-)

diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index 8612684d48..4f7f4e60d6 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -366,11 +366,8 @@ static CcwDevice *s390_get_ccw_device(DeviceState *dev_st, int *devtype)
             ccw_dev = CCW_DEVICE(vfio_ccw_dev);
             tmp_dt = CCW_DEVTYPE_VFIO;
         } else {
-            SCSIDevice *sd = (SCSIDevice *)
-                object_dynamic_cast(OBJECT(dev_st),
-                                    TYPE_SCSI_DEVICE);
-            if (sd) {
-                SCSIBus *sbus = scsi_bus_from_device(sd);
+            if (object_dynamic_cast(OBJECT(dev_st), TYPE_SCSI_DEVICE)) {
+                SCSIBus *sbus = SCSI_BUS(qdev_get_parent_bus(dev_st));
                 VirtIODevice *vdev = (VirtIODevice *)
                     object_dynamic_cast(OBJECT(sbus->qbus.parent),
                                         TYPE_VIRTIO_DEVICE);
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index c4525515ab..ee72b86b13 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -679,7 +679,7 @@ SCSIRequest *scsi_req_alloc(const SCSIReqOps *reqops, SCSIDevice *d,
                             uint32_t tag, uint32_t lun, void *hba_private)
 {
     SCSIRequest *req;
-    SCSIBus *bus = scsi_bus_from_device(d);
+    SCSIBus *bus = SCSI_BUS(qdev_get_parent_bus(DEVICE(d)));
     BusState *qbus = BUS(bus);
     const int memset_off = offsetof(SCSIRequest, sense)
                            + sizeof(req->sense);
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index eb558c145a..e3263dec0d 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -175,11 +175,6 @@ static inline void scsi_bus_init(SCSIBus *bus, size_t bus_size,
     scsi_bus_init_named(bus, bus_size, host, info, NULL);
 }
 
-static inline SCSIBus *scsi_bus_from_device(SCSIDevice *d)
-{
-    return SCSI_BUS(qdev_get_parent_bus(DEVICE(d)));
-}
-
 SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockBackend *blk,
                                       int unit, bool removable, int bootindex,
                                       bool share_rw,
-- 
2.38.1

Re: [PATCH v2 14/19] hw/scsi/scsi-bus: Inline two uses of scsi_bus_from_device()

[Eric Farman]

On Mon, 2023-02-13 at 08:08 +0100, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Acked-by: Eric Farman <farman@linux.ibm.com>

> ---
>  hw/s390x/ipl.c         | 7 ++-----
>  hw/scsi/scsi-bus.c     | 2 +-
>  include/hw/scsi/scsi.h | 5 -----
>  3 files changed, 3 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index 8612684d48..4f7f4e60d6 100644
> --- a/hw/s390x/ipl.c
> +++ b/hw/s390x/ipl.c
> @@ -366,11 +366,8 @@ static CcwDevice
> *s390_get_ccw_device(DeviceState *dev_st, int *devtype)
>              ccw_dev = CCW_DEVICE(vfio_ccw_dev);
>              tmp_dt = CCW_DEVTYPE_VFIO;
>          } else {
> -            SCSIDevice *sd = (SCSIDevice *)
> -                object_dynamic_cast(OBJECT(dev_st),
> -                                    TYPE_SCSI_DEVICE);
> -            if (sd) {
> -                SCSIBus *sbus = scsi_bus_from_device(sd);
> +            if (object_dynamic_cast(OBJECT(dev_st),
> TYPE_SCSI_DEVICE)) {
> +                SCSIBus *sbus =
> SCSI_BUS(qdev_get_parent_bus(dev_st));
>                  VirtIODevice *vdev = (VirtIODevice *)
>                      object_dynamic_cast(OBJECT(sbus->qbus.parent),
>                                          TYPE_VIRTIO_DEVICE);
> diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
> index c4525515ab..ee72b86b13 100644
> --- a/hw/scsi/scsi-bus.c
> +++ b/hw/scsi/scsi-bus.c
> @@ -679,7 +679,7 @@ SCSIRequest *scsi_req_alloc(const SCSIReqOps
> *reqops, SCSIDevice *d,
>                              uint32_t tag, uint32_t lun, void
> *hba_private)
>  {
>      SCSIRequest *req;
> -    SCSIBus *bus = scsi_bus_from_device(d);
> +    SCSIBus *bus = SCSI_BUS(qdev_get_parent_bus(DEVICE(d)));
>      BusState *qbus = BUS(bus);
>      const int memset_off = offsetof(SCSIRequest, sense)
>                             + sizeof(req->sense);
> diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
> index eb558c145a..e3263dec0d 100644
> --- a/include/hw/scsi/scsi.h
> +++ b/include/hw/scsi/scsi.h
> @@ -175,11 +175,6 @@ static inline void scsi_bus_init(SCSIBus *bus,
> size_t bus_size,
>      scsi_bus_init_named(bus, bus_size, host, info, NULL);
>  }
>  
> -static inline SCSIBus *scsi_bus_from_device(SCSIDevice *d)
> -{
> -    return SCSI_BUS(qdev_get_parent_bus(DEVICE(d)));
> -}
> -
>  SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockBackend
> *blk,
>                                        int unit, bool removable, int
> bootindex,
>                                        bool share_rw,

[PATCH v2 15/19] hw/s390x/event-facility: Replace DO_UPCAST(SCLPEvent) by SCLP_EVENT()

[Philippe Mathieu-Daudé]

Use the SCLP_EVENT() QOM type-checking macro to avoid DO_UPCAST().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/s390x/event-facility.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
index faa51aa4c7..6891e3cd73 100644
--- a/hw/s390x/event-facility.c
+++ b/hw/s390x/event-facility.c
@@ -64,8 +64,7 @@ static bool event_pending(SCLPEventFacility *ef)
     SCLPEventClass *event_class;
 
     QTAILQ_FOREACH(kid, &ef->sbus.qbus.children, sibling) {
-        DeviceState *qdev = kid->child;
-        event = DO_UPCAST(SCLPEvent, qdev, qdev);
+        event = SCLP_EVENT(kid->child);
         event_class = SCLP_EVENT_GET_CLASS(event);
         if (event->event_pending &&
             event_class->get_send_mask() & ef->receive_mask) {
-- 
2.38.1

Re: [PATCH v2 15/19] hw/s390x/event-facility: Replace DO_UPCAST(SCLPEvent) by SCLP_EVENT()

[Eric Farman]

On Mon, 2023-02-13 at 08:08 +0100, Philippe Mathieu-Daudé wrote:
> Use the SCLP_EVENT() QOM type-checking macro to avoid DO_UPCAST().
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Eric Farman <farman@linux.ibm.com>

> ---
>  hw/s390x/event-facility.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
> index faa51aa4c7..6891e3cd73 100644
> --- a/hw/s390x/event-facility.c
> +++ b/hw/s390x/event-facility.c
> @@ -64,8 +64,7 @@ static bool event_pending(SCLPEventFacility *ef)
>      SCLPEventClass *event_class;
>  
>      QTAILQ_FOREACH(kid, &ef->sbus.qbus.children, sibling) {
> -        DeviceState *qdev = kid->child;
> -        event = DO_UPCAST(SCLPEvent, qdev, qdev);
> +        event = SCLP_EVENT(kid->child);
>          event_class = SCLP_EVENT_GET_CLASS(event);
>          if (event->event_pending &&
>              event_class->get_send_mask() & ef->receive_mask) {

[PATCH v2 16/19] hw/vfio/ccw: Replace DO_UPCAST(VFIOCCWDevice) by VFIO_CCW()

[Philippe Mathieu-Daudé]

Use the VFIO_CCW() QOM type-checking macro to avoid DO_UPCAST().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/vfio/ccw.c | 35 ++++++++++++++++-------------------
 1 file changed, 16 insertions(+), 19 deletions(-)

diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
index 0354737666..a8aa5b48c4 100644
--- a/hw/vfio/ccw.c
+++ b/hw/vfio/ccw.c
@@ -76,8 +76,7 @@ struct VFIODeviceOps vfio_ccw_ops = {
 
 static IOInstEnding vfio_ccw_handle_request(SubchDev *sch)
 {
-    S390CCWDevice *cdev = sch->driver_data;
-    VFIOCCWDevice *vcdev = DO_UPCAST(VFIOCCWDevice, cdev, cdev);
+    VFIOCCWDevice *vcdev = VFIO_CCW(sch->driver_data);
     struct ccw_io_region *region = vcdev->io_region;
     int ret;
 
@@ -125,8 +124,7 @@ again:
 
 static IOInstEnding vfio_ccw_handle_store(SubchDev *sch)
 {
-    S390CCWDevice *cdev = sch->driver_data;
-    VFIOCCWDevice *vcdev = DO_UPCAST(VFIOCCWDevice, cdev, cdev);
+    VFIOCCWDevice *vcdev = VFIO_CCW(sch->driver_data);
     SCHIB *schib = &sch->curr_status;
     struct ccw_schib_region *region = vcdev->schib_region;
     SCHIB *s;
@@ -170,8 +168,7 @@ static IOInstEnding vfio_ccw_handle_store(SubchDev *sch)
 
 static int vfio_ccw_handle_clear(SubchDev *sch)
 {
-    S390CCWDevice *cdev = sch->driver_data;
-    VFIOCCWDevice *vcdev = DO_UPCAST(VFIOCCWDevice, cdev, cdev);
+    VFIOCCWDevice *vcdev = VFIO_CCW(sch->driver_data);
     struct ccw_cmd_region *region = vcdev->async_cmd_region;
     int ret;
 
@@ -210,8 +207,7 @@ again:
 
 static int vfio_ccw_handle_halt(SubchDev *sch)
 {
-    S390CCWDevice *cdev = sch->driver_data;
-    VFIOCCWDevice *vcdev = DO_UPCAST(VFIOCCWDevice, cdev, cdev);
+    VFIOCCWDevice *vcdev = VFIO_CCW(sch->driver_data);
     struct ccw_cmd_region *region = vcdev->async_cmd_region;
     int ret;
 
@@ -252,8 +248,8 @@ again:
 static void vfio_ccw_reset(DeviceState *dev)
 {
     CcwDevice *ccw_dev = DO_UPCAST(CcwDevice, parent_obj, dev);
-    S390CCWDevice *cdev = DO_UPCAST(S390CCWDevice, parent_obj, ccw_dev);
-    VFIOCCWDevice *vcdev = DO_UPCAST(VFIOCCWDevice, cdev, cdev);
+    S390CCWDevice *cdev = S390_CCW_DEVICE(ccw_dev);
+    VFIOCCWDevice *vcdev = VFIO_CCW(cdev);
 
     ioctl(vcdev->vdev.fd, VFIO_DEVICE_RESET);
 }
@@ -588,9 +584,10 @@ static void vfio_ccw_put_device(VFIOCCWDevice *vcdev)
 static void vfio_ccw_get_device(VFIOGroup *group, VFIOCCWDevice *vcdev,
                                 Error **errp)
 {
-    char *name = g_strdup_printf("%x.%x.%04x", vcdev->cdev.hostid.cssid,
-                                 vcdev->cdev.hostid.ssid,
-                                 vcdev->cdev.hostid.devid);
+    S390CCWDevice *cdev = S390_CCW_DEVICE(vcdev);
+    char *name = g_strdup_printf("%x.%x.%04x", cdev->hostid.cssid,
+                                 cdev->hostid.ssid,
+                                 cdev->hostid.devid);
     VFIODevice *vbasedev;
 
     QLIST_FOREACH(vbasedev, &group->device_list, next) {
@@ -611,14 +608,14 @@ static void vfio_ccw_get_device(VFIOGroup *group, VFIOCCWDevice *vcdev,
      */
     vcdev->vdev.ram_block_discard_allowed = true;
 
-    if (vfio_get_device(group, vcdev->cdev.mdevid, &vcdev->vdev, errp)) {
+    if (vfio_get_device(group, cdev->mdevid, &vcdev->vdev, errp)) {
         goto out_err;
     }
 
     vcdev->vdev.ops = &vfio_ccw_ops;
     vcdev->vdev.type = VFIO_DEVICE_TYPE_CCW;
     vcdev->vdev.name = name;
-    vcdev->vdev.dev = &vcdev->cdev.parent_obj.parent_obj;
+    vcdev->vdev.dev = &cdev->parent_obj.parent_obj;
 
     return;
 
@@ -657,9 +654,9 @@ static void vfio_ccw_realize(DeviceState *dev, Error **errp)
 {
     VFIOGroup *group;
     CcwDevice *ccw_dev = DO_UPCAST(CcwDevice, parent_obj, dev);
-    S390CCWDevice *cdev = DO_UPCAST(S390CCWDevice, parent_obj, ccw_dev);
-    VFIOCCWDevice *vcdev = DO_UPCAST(VFIOCCWDevice, cdev, cdev);
+    S390CCWDevice *cdev = S390_CCW_DEVICE(ccw_dev);
     S390CCWDeviceClass *cdc = S390_CCW_DEVICE_GET_CLASS(cdev);
+    VFIOCCWDevice *vcdev = VFIO_CCW(cdev);
     Error *err = NULL;
 
     /* Call the class init function for subchannel. */
@@ -729,9 +726,9 @@ out_err_propagate:
 static void vfio_ccw_unrealize(DeviceState *dev)
 {
     CcwDevice *ccw_dev = DO_UPCAST(CcwDevice, parent_obj, dev);
-    S390CCWDevice *cdev = DO_UPCAST(S390CCWDevice, parent_obj, ccw_dev);
-    VFIOCCWDevice *vcdev = DO_UPCAST(VFIOCCWDevice, cdev, cdev);
+    S390CCWDevice *cdev = S390_CCW_DEVICE(ccw_dev);
     S390CCWDeviceClass *cdc = S390_CCW_DEVICE_GET_CLASS(cdev);
+    VFIOCCWDevice *vcdev = VFIO_CCW(cdev);
     VFIOGroup *group = vcdev->vdev.group;
 
     vfio_ccw_unregister_irq_notifier(vcdev, VFIO_CCW_REQ_IRQ_INDEX);
-- 
2.38.1

Re: [PATCH v2 16/19] hw/vfio/ccw: Replace DO_UPCAST(VFIOCCWDevice) by VFIO_CCW()

[Eric Farman]

On Mon, 2023-02-13 at 08:08 +0100, Philippe Mathieu-Daudé wrote:
> Use the VFIO_CCW() QOM type-checking macro to avoid DO_UPCAST().
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/vfio/ccw.c | 35 ++++++++++++++++-------------------
>  1 file changed, 16 insertions(+), 19 deletions(-)
> 
> diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
> index 0354737666..a8aa5b48c4 100644
> --- a/hw/vfio/ccw.c
> +++ b/hw/vfio/ccw.c

...snip...

> @@ -252,8 +248,8 @@ again:
>  static void vfio_ccw_reset(DeviceState *dev)
>  {
>      CcwDevice *ccw_dev = DO_UPCAST(CcwDevice, parent_obj, dev);

If I'm not mistaken, I believe that this (and (un)realize below) could
be changed to:

   CcwDevice *ccw_dev = CCW_DEVICE(dev);

> -    S390CCWDevice *cdev = DO_UPCAST(S390CCWDevice, parent_obj,
> ccw_dev);
> -    VFIOCCWDevice *vcdev = DO_UPCAST(VFIOCCWDevice, cdev, cdev);
> +    S390CCWDevice *cdev = S390_CCW_DEVICE(ccw_dev);
> +    VFIOCCWDevice *vcdev = VFIO_CCW(cdev);
>  
>      ioctl(vcdev->vdev.fd, VFIO_DEVICE_RESET);
>  }

...snip...

> @@ -657,9 +654,9 @@ static void vfio_ccw_realize(DeviceState *dev,
> Error **errp)
>  {
>      VFIOGroup *group;
>      CcwDevice *ccw_dev = DO_UPCAST(CcwDevice, parent_obj, dev);
> -    S390CCWDevice *cdev = DO_UPCAST(S390CCWDevice, parent_obj,
> ccw_dev);
> -    VFIOCCWDevice *vcdev = DO_UPCAST(VFIOCCWDevice, cdev, cdev);
> +    S390CCWDevice *cdev = S390_CCW_DEVICE(ccw_dev);
>      S390CCWDeviceClass *cdc = S390_CCW_DEVICE_GET_CLASS(cdev);
> +    VFIOCCWDevice *vcdev = VFIO_CCW(cdev);
>      Error *err = NULL;
>  
>      /* Call the class init function for subchannel. */
> @@ -729,9 +726,9 @@ out_err_propagate:
>  static void vfio_ccw_unrealize(DeviceState *dev)
>  {
>      CcwDevice *ccw_dev = DO_UPCAST(CcwDevice, parent_obj, dev);
> -    S390CCWDevice *cdev = DO_UPCAST(S390CCWDevice, parent_obj,
> ccw_dev);
> -    VFIOCCWDevice *vcdev = DO_UPCAST(VFIOCCWDevice, cdev, cdev);
> +    S390CCWDevice *cdev = S390_CCW_DEVICE(ccw_dev);
>      S390CCWDeviceClass *cdc = S390_CCW_DEVICE_GET_CLASS(cdev);
> +    VFIOCCWDevice *vcdev = VFIO_CCW(cdev);
>      VFIOGroup *group = vcdev->vdev.group;
>  
>      vfio_ccw_unregister_irq_notifier(vcdev, VFIO_CCW_REQ_IRQ_INDEX);

Re: [PATCH v2 16/19] hw/vfio/ccw: Replace DO_UPCAST(VFIOCCWDevice) by VFIO_CCW()

[Philippe Mathieu-Daudé]

On 13/2/23 16:29, Eric Farman wrote:
> On Mon, 2023-02-13 at 08:08 +0100, Philippe Mathieu-Daudé wrote:
>> Use the VFIO_CCW() QOM type-checking macro to avoid DO_UPCAST().
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/vfio/ccw.c | 35 ++++++++++++++++-------------------
>>   1 file changed, 16 insertions(+), 19 deletions(-)
>>
>> diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
>> index 0354737666..a8aa5b48c4 100644
>> --- a/hw/vfio/ccw.c
>> +++ b/hw/vfio/ccw.c
> 
> ...snip...
> 
>> @@ -252,8 +248,8 @@ again:
>>   static void vfio_ccw_reset(DeviceState *dev)
>>   {
>>       CcwDevice *ccw_dev = DO_UPCAST(CcwDevice, parent_obj, dev);
> 
> If I'm not mistaken, I believe that this (and (un)realize below) could
> be changed to:
> 
>     CcwDevice *ccw_dev = CCW_DEVICE(dev);

Even ...

>> -    S390CCWDevice *cdev = DO_UPCAST(S390CCWDevice, parent_obj,
>> ccw_dev);
>> -    VFIOCCWDevice *vcdev = DO_UPCAST(VFIOCCWDevice, cdev, cdev);
>> +    S390CCWDevice *cdev = S390_CCW_DEVICE(ccw_dev);
>> +    VFIOCCWDevice *vcdev = VFIO_CCW(cdev);

         VFIOCCWDevice *vcdev = VFIO_CCW(dev);

But I somehow got scared to of removing too many casts...

Are these paths covered by a "make check-qtest" on a s390x host?

>>       ioctl(vcdev->vdev.fd, VFIO_DEVICE_RESET);
>>   }
> 
> ...snip...
> 
>> @@ -657,9 +654,9 @@ static void vfio_ccw_realize(DeviceState *dev,
>> Error **errp)
>>   {
>>       VFIOGroup *group;
>>       CcwDevice *ccw_dev = DO_UPCAST(CcwDevice, parent_obj, dev);
>> -    S390CCWDevice *cdev = DO_UPCAST(S390CCWDevice, parent_obj,
>> ccw_dev);
>> -    VFIOCCWDevice *vcdev = DO_UPCAST(VFIOCCWDevice, cdev, cdev);
>> +    S390CCWDevice *cdev = S390_CCW_DEVICE(ccw_dev);
>>       S390CCWDeviceClass *cdc = S390_CCW_DEVICE_GET_CLASS(cdev);
>> +    VFIOCCWDevice *vcdev = VFIO_CCW(cdev);
>>       Error *err = NULL;
>>   
>>       /* Call the class init function for subchannel. */
>> @@ -729,9 +726,9 @@ out_err_propagate:
>>   static void vfio_ccw_unrealize(DeviceState *dev)
>>   {
>>       CcwDevice *ccw_dev = DO_UPCAST(CcwDevice, parent_obj, dev);
>> -    S390CCWDevice *cdev = DO_UPCAST(S390CCWDevice, parent_obj,
>> ccw_dev);
>> -    VFIOCCWDevice *vcdev = DO_UPCAST(VFIOCCWDevice, cdev, cdev);
>> +    S390CCWDevice *cdev = S390_CCW_DEVICE(ccw_dev);
>>       S390CCWDeviceClass *cdc = S390_CCW_DEVICE_GET_CLASS(cdev);
>> +    VFIOCCWDevice *vcdev = VFIO_CCW(cdev);
>>       VFIOGroup *group = vcdev->vdev.group;
>>   
>>       vfio_ccw_unregister_irq_notifier(vcdev, VFIO_CCW_REQ_IRQ_INDEX);
> 

Re: [PATCH v2 16/19] hw/vfio/ccw: Replace DO_UPCAST(VFIOCCWDevice) by VFIO_CCW()

[Philippe Mathieu-Daudé]

On 13/2/23 16:51, Philippe Mathieu-Daudé wrote:
> On 13/2/23 16:29, Eric Farman wrote:
>> On Mon, 2023-02-13 at 08:08 +0100, Philippe Mathieu-Daudé wrote:
>>> Use the VFIO_CCW() QOM type-checking macro to avoid DO_UPCAST().
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>>   hw/vfio/ccw.c | 35 ++++++++++++++++-------------------
>>>   1 file changed, 16 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
>>> index 0354737666..a8aa5b48c4 100644
>>> --- a/hw/vfio/ccw.c
>>> +++ b/hw/vfio/ccw.c
>>
>> ...snip...
>>
>>> @@ -252,8 +248,8 @@ again:
>>>   static void vfio_ccw_reset(DeviceState *dev)
>>>   {
>>>       CcwDevice *ccw_dev = DO_UPCAST(CcwDevice, parent_obj, dev);
>>
>> If I'm not mistaken, I believe that this (and (un)realize below) could
>> be changed to:
>>
>>     CcwDevice *ccw_dev = CCW_DEVICE(dev);
> 
> Even ...
> 
>>> -    S390CCWDevice *cdev = DO_UPCAST(S390CCWDevice, parent_obj,
>>> ccw_dev);
>>> -    VFIOCCWDevice *vcdev = DO_UPCAST(VFIOCCWDevice, cdev, cdev);
>>> +    S390CCWDevice *cdev = S390_CCW_DEVICE(ccw_dev);
>>> +    VFIOCCWDevice *vcdev = VFIO_CCW(cdev);
> 
>          VFIOCCWDevice *vcdev = VFIO_CCW(dev);
> 
> But I somehow got scared to of removing too many casts...
> 
> Are these paths covered by a "make check-qtest" on a s390x host?

They are covered by the Avocado tests :)

$ avocado --show=app,console run -t arch:s390x tests/avocado

Re: [PATCH v2 16/19] hw/vfio/ccw: Replace DO_UPCAST(VFIOCCWDevice) by VFIO_CCW()

[Eric Farman]

On Mon, 2023-02-13 at 17:10 +0100, Philippe Mathieu-Daudé wrote:
> On 13/2/23 16:51, Philippe Mathieu-Daudé wrote:
> > On 13/2/23 16:29, Eric Farman wrote:
> > > On Mon, 2023-02-13 at 08:08 +0100, Philippe Mathieu-Daudé wrote:
> > > > Use the VFIO_CCW() QOM type-checking macro to avoid
> > > > DO_UPCAST().
> > > > 
> > > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > > > ---
> > > >   hw/vfio/ccw.c | 35 ++++++++++++++++-------------------
> > > >   1 file changed, 16 insertions(+), 19 deletions(-)
> > > > 
> > > > diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
> > > > index 0354737666..a8aa5b48c4 100644
> > > > --- a/hw/vfio/ccw.c
> > > > +++ b/hw/vfio/ccw.c
> > > 
> > > ...snip...
> > > 
> > > > @@ -252,8 +248,8 @@ again:
> > > >   static void vfio_ccw_reset(DeviceState *dev)
> > > >   {
> > > >       CcwDevice *ccw_dev = DO_UPCAST(CcwDevice, parent_obj,
> > > > dev);
> > > 
> > > If I'm not mistaken, I believe that this (and (un)realize below)
> > > could
> > > be changed to:
> > > 
> > >     CcwDevice *ccw_dev = CCW_DEVICE(dev);
> > 
> > Even ...
> > 
> > > > -    S390CCWDevice *cdev = DO_UPCAST(S390CCWDevice, parent_obj,
> > > > ccw_dev);
> > > > -    VFIOCCWDevice *vcdev = DO_UPCAST(VFIOCCWDevice, cdev,
> > > > cdev);
> > > > +    S390CCWDevice *cdev = S390_CCW_DEVICE(ccw_dev);
> > > > +    VFIOCCWDevice *vcdev = VFIO_CCW(cdev);
> > 
> >          VFIOCCWDevice *vcdev = VFIO_CCW(dev);

Ha, I didn't look to see if we cared about the intermediary ones, but
this is true here. (Realize cares a bit, but that's easy enough.)

> > 
> > But I somehow got scared to of removing too many casts...
> > 
> > Are these paths covered by a "make check-qtest" on a s390x host?
> 
> They are covered by the Avocado tests :)
> 
> $ avocado --show=app,console run -t arch:s390x tests/avocado
> 

Woo! Then I'm happy with the big squash then.

Reviewed-by: Eric Farman <farman@linux.ibm.com>

Re: [PATCH v2 16/19] hw/vfio/ccw: Replace DO_UPCAST(VFIOCCWDevice) by VFIO_CCW()

[Philippe Mathieu-Daudé]

On 13/2/23 17:24, Eric Farman wrote:
> On Mon, 2023-02-13 at 17:10 +0100, Philippe Mathieu-Daudé wrote:
>> On 13/2/23 16:51, Philippe Mathieu-Daudé wrote:
>>> On 13/2/23 16:29, Eric Farman wrote:
>>>> On Mon, 2023-02-13 at 08:08 +0100, Philippe Mathieu-Daudé wrote:
>>>>> Use the VFIO_CCW() QOM type-checking macro to avoid
>>>>> DO_UPCAST().
>>>>>
>>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>>> ---
>>>>>    hw/vfio/ccw.c | 35 ++++++++++++++++-------------------
>>>>>    1 file changed, 16 insertions(+), 19 deletions(-)

>>>>>        CcwDevice *ccw_dev = DO_UPCAST(CcwDevice, parent_obj,
>>>>> dev);
>>>>
>>>> If I'm not mistaken, I believe that this (and (un)realize below)
>>>> could
>>>> be changed to:
>>>>
>>>>      CcwDevice *ccw_dev = CCW_DEVICE(dev);
>>>
>>> Even ...

>>>           VFIOCCWDevice *vcdev = VFIO_CCW(dev);
> 
> Ha, I didn't look to see if we cared about the intermediary ones, but
> this is true here. (Realize cares a bit, but that's easy enough.)
> 
>>>
>>> But I somehow got scared to of removing too many casts...
>>>
>>> Are these paths covered by a "make check-qtest" on a s390x host?
>>
>> They are covered by the Avocado tests :)
>>
>> $ avocado --show=app,console run -t arch:s390x tests/avocado
>>
> 
> Woo! Then I'm happy with the big squash then.
> 
> Reviewed-by: Eric Farman <farman@linux.ibm.com>

Thanks! Posted cleaned v3 here:
https://lore.kernel.org/qemu-devel/20230213170145.45666-1-philmd@linaro.org/

[PATCH v2 17/19] hw/usb/dev-hub: Use QOM USB_HUB() macro instead of casting

[Philippe Mathieu-Daudé]

Use the safer USB_HUB() QOM type-checking macro instead of casts.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/usb/dev-hub.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/usb/dev-hub.c b/hw/usb/dev-hub.c
index a6b50dbc8d..4734700e3e 100644
--- a/hw/usb/dev-hub.c
+++ b/hw/usb/dev-hub.c
@@ -350,7 +350,7 @@ static const char *feature_name(int feature)
 static void usb_hub_handle_control(USBDevice *dev, USBPacket *p,
                int request, int value, int index, int length, uint8_t *data)
 {
-    USBHubState *s = (USBHubState *)dev;
+    USBHubState *s = USB_HUB(dev);
     int ret;
 
     trace_usb_hub_control(s->dev.addr, request, value, index, length);
@@ -523,7 +523,7 @@ static void usb_hub_handle_control(USBDevice *dev, USBPacket *p,
 
 static void usb_hub_handle_data(USBDevice *dev, USBPacket *p)
 {
-    USBHubState *s = (USBHubState *)dev;
+    USBHubState *s = USB_HUB(dev);
 
     switch(p->pid) {
     case USB_TOKEN_IN:
@@ -568,7 +568,7 @@ static void usb_hub_handle_data(USBDevice *dev, USBPacket *p)
 
 static void usb_hub_unrealize(USBDevice *dev)
 {
-    USBHubState *s = (USBHubState *)dev;
+    USBHubState *s = USB_HUB(dev);
     int i;
 
     for (i = 0; i < s->num_ports; i++) {
-- 
2.38.1

[PATCH v2 18/19] hw/usb: Replace DO_UPCAST(USBBus) by USB_BUS()

[Philippe Mathieu-Daudé]

Use the USB_BUS() QOM type-checking macro to avoid DO_UPCAST().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/usb.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/hw/usb.h b/include/hw/usb.h
index b2111bb1c7..f743a5e945 100644
--- a/include/hw/usb.h
+++ b/include/hw/usb.h
@@ -520,7 +520,7 @@ void usb_check_attach(USBDevice *dev, Error **errp);
 
 static inline USBBus *usb_bus_from_device(USBDevice *d)
 {
-    return DO_UPCAST(USBBus, qbus, qdev_get_parent_bus(DEVICE(d)));
+    return USB_BUS(qdev_get_parent_bus(DEVICE(d)));
 }
 
 extern const VMStateDescription vmstate_usb_device;
-- 
2.38.1

[RFC PATCH v2 19/19] hw/usb: Inline usb_bus_from_device()

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
RFC Other devices don't use such helper. Maybe it should
    be the other way around, introduce more bus_from_device()
    helpers?
---
 hw/usb/bus.c        | 10 +++++-----
 hw/usb/core.c       |  6 +++---
 hw/usb/dev-hub.c    |  4 ++--
 hw/usb/dev-serial.c | 10 +++++-----
 hw/usb/hcd-xhci.c   |  2 +-
 include/hw/usb.h    |  5 -----
 6 files changed, 16 insertions(+), 21 deletions(-)

diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index d7c3c71435..4a1b67761c 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -427,7 +427,7 @@ void usb_unregister_port(USBBus *bus, USBPort *port)
 
 void usb_claim_port(USBDevice *dev, Error **errp)
 {
-    USBBus *bus = usb_bus_from_device(dev);
+    USBBus *bus = USB_BUS(qdev_get_parent_bus(DEVICE(dev)));
     USBPort *port;
     USBDevice *hub;
 
@@ -473,7 +473,7 @@ void usb_claim_port(USBDevice *dev, Error **errp)
 
 void usb_release_port(USBDevice *dev)
 {
-    USBBus *bus = usb_bus_from_device(dev);
+    USBBus *bus = USB_BUS(qdev_get_parent_bus(DEVICE(dev)));
     USBPort *port = dev->port;
 
     assert(port != NULL);
@@ -517,7 +517,7 @@ static void usb_mask_to_str(char *dest, size_t size,
 
 void usb_check_attach(USBDevice *dev, Error **errp)
 {
-    USBBus *bus = usb_bus_from_device(dev);
+    USBBus *bus = USB_BUS(qdev_get_parent_bus(DEVICE(dev)));
     USBPort *port = dev->port;
     char devspeed[32], portspeed[32];
 
@@ -555,7 +555,7 @@ void usb_device_attach(USBDevice *dev, Error **errp)
 
 int usb_device_detach(USBDevice *dev)
 {
-    USBBus *bus = usb_bus_from_device(dev);
+    USBBus *bus = USB_BUS(qdev_get_parent_bus(DEVICE(dev)));
     USBPort *port = dev->port;
 
     assert(port != NULL);
@@ -583,7 +583,7 @@ static const char *usb_speed(unsigned int speed)
 static void usb_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent)
 {
     USBDevice *dev = USB_DEVICE(qdev);
-    USBBus *bus = usb_bus_from_device(dev);
+    USBBus *bus = USB_BUS(qdev_get_parent_bus(qdev));
 
     monitor_printf(mon, "%*saddr %d.%d, port %s, speed %s, name %s%s\n",
                    indent, "", bus->busnr, dev->addr,
diff --git a/hw/usb/core.c b/hw/usb/core.c
index 975f76250a..f358f0313a 100644
--- a/hw/usb/core.c
+++ b/hw/usb/core.c
@@ -95,7 +95,7 @@ void usb_device_reset(USBDevice *dev)
 void usb_wakeup(USBEndpoint *ep, unsigned int stream)
 {
     USBDevice *dev = ep->dev;
-    USBBus *bus = usb_bus_from_device(dev);
+    USBBus *bus = USB_BUS(qdev_get_parent_bus(DEVICE(dev)));
 
     if (!phase_check(PHASE_MACHINE_READY)) {
         /*
@@ -556,7 +556,7 @@ void usb_packet_check_state(USBPacket *p, USBPacketState expected)
         return;
     }
     dev = p->ep->dev;
-    bus = usb_bus_from_device(dev);
+    bus = USB_BUS(qdev_get_parent_bus(DEVICE(dev)));
     trace_usb_packet_state_fault(bus->busnr, dev->port->path, p->ep->nr, p,
                                  usb_packet_state_name(p->state),
                                  usb_packet_state_name(expected));
@@ -567,7 +567,7 @@ void usb_packet_set_state(USBPacket *p, USBPacketState state)
 {
     if (p->ep) {
         USBDevice *dev = p->ep->dev;
-        USBBus *bus = usb_bus_from_device(dev);
+        USBBus *bus = USB_BUS(qdev_get_parent_bus(DEVICE(dev)));
         trace_usb_packet_state_change(bus->busnr, dev->port->path, p->ep->nr, p,
                                       usb_packet_state_name(p->state),
                                       usb_packet_state_name(state));
diff --git a/hw/usb/dev-hub.c b/hw/usb/dev-hub.c
index 4734700e3e..4a0bcc4093 100644
--- a/hw/usb/dev-hub.c
+++ b/hw/usb/dev-hub.c
@@ -572,7 +572,7 @@ static void usb_hub_unrealize(USBDevice *dev)
     int i;
 
     for (i = 0; i < s->num_ports; i++) {
-        usb_unregister_port(usb_bus_from_device(dev),
+        usb_unregister_port(USB_BUS(qdev_get_parent_bus(DEVICE(dev))),
                             &s->ports[i].port);
     }
 
@@ -611,7 +611,7 @@ static void usb_hub_realize(USBDevice *dev, Error **errp)
     s->intr = usb_ep_get(dev, USB_TOKEN_IN, 1);
     for (i = 0; i < s->num_ports; i++) {
         port = &s->ports[i];
-        usb_register_port(usb_bus_from_device(dev),
+        usb_register_port(USB_BUS(qdev_get_parent_bus(DEVICE(dev))),
                           &port->port, s, i, &usb_hub_port_ops,
                           USB_SPEED_MASK_LOW | USB_SPEED_MASK_FULL);
         usb_port_location(&port->port, dev->port, i+1);
diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
index 63047d79cf..0194bb541b 100644
--- a/hw/usb/dev-serial.c
+++ b/hw/usb/dev-serial.c
@@ -190,7 +190,7 @@ static void usb_serial_set_flow_control(USBSerialState *s,
                                         uint8_t flow_control)
 {
     USBDevice *dev = USB_DEVICE(s);
-    USBBus *bus = usb_bus_from_device(dev);
+    USBBus *bus = USB_BUS(qdev_get_parent_bus(DEVICE(dev)));
 
     /* TODO: ioctl */
     s->flow_control = flow_control;
@@ -200,7 +200,7 @@ static void usb_serial_set_flow_control(USBSerialState *s,
 static void usb_serial_set_xonxoff(USBSerialState *s, int xonxoff)
 {
     USBDevice *dev = USB_DEVICE(s);
-    USBBus *bus = usb_bus_from_device(dev);
+    USBBus *bus = USB_BUS(qdev_get_parent_bus(DEVICE(dev)));
 
     s->xon = xonxoff & 0xff;
     s->xoff = (xonxoff >> 8) & 0xff;
@@ -221,7 +221,7 @@ static void usb_serial_reset(USBSerialState *s)
 static void usb_serial_handle_reset(USBDevice *dev)
 {
     USBSerialState *s = USB_SERIAL(dev);
-    USBBus *bus = usb_bus_from_device(dev);
+    USBBus *bus = USB_BUS(qdev_get_parent_bus(DEVICE(dev)));
 
     trace_usb_serial_reset(bus->busnr, dev->addr);
 
@@ -261,7 +261,7 @@ static void usb_serial_handle_control(USBDevice *dev, USBPacket *p,
                                       int length, uint8_t *data)
 {
     USBSerialState *s = USB_SERIAL(dev);
-    USBBus *bus = usb_bus_from_device(dev);
+    USBBus *bus = USB_BUS(qdev_get_parent_bus(DEVICE(dev)));
     int ret;
 
     trace_usb_serial_handle_control(bus->busnr, dev->addr, request, value);
@@ -479,7 +479,7 @@ static void usb_serial_token_in(USBSerialState *s, USBPacket *p)
 static void usb_serial_handle_data(USBDevice *dev, USBPacket *p)
 {
     USBSerialState *s = USB_SERIAL(dev);
-    USBBus *bus = usb_bus_from_device(dev);
+    USBBus *bus = USB_BUS(qdev_get_parent_bus(DEVICE(dev)));
     uint8_t devep = p->ep->nr;
     struct iovec *iov;
     int i;
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index b89b618ec2..94c2e58aaf 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -3268,7 +3268,7 @@ static void xhci_complete(USBPort *port, USBPacket *packet)
 
 static void xhci_child_detach(USBPort *uport, USBDevice *child)
 {
-    USBBus *bus = usb_bus_from_device(child);
+    USBBus *bus = USB_BUS(qdev_get_parent_bus(DEVICE(child)));
     XHCIState *xhci = container_of(bus, XHCIState, bus);
 
     xhci_detach_slot(xhci, child->port);
diff --git a/include/hw/usb.h b/include/hw/usb.h
index f743a5e945..4a2987c477 100644
--- a/include/hw/usb.h
+++ b/include/hw/usb.h
@@ -518,11 +518,6 @@ void usb_device_attach(USBDevice *dev, Error **errp);
 int usb_device_detach(USBDevice *dev);
 void usb_check_attach(USBDevice *dev, Error **errp);
 
-static inline USBBus *usb_bus_from_device(USBDevice *d)
-{
-    return USB_BUS(qdev_get_parent_bus(DEVICE(d)));
-}
-
 extern const VMStateDescription vmstate_usb_device;
 
 #define VMSTATE_USB_DEVICE(_field, _state) {                         \
-- 
2.38.1

Re: [RFC PATCH v2 19/19] hw/usb: Inline usb_bus_from_device()

[Thomas Huth]

On 13/02/2023 08.08, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> RFC Other devices don't use such helper. Maybe it should
>      be the other way around, introduce more bus_from_device()
>      helpers?
> ---
>   hw/usb/bus.c        | 10 +++++-----
>   hw/usb/core.c       |  6 +++---
>   hw/usb/dev-hub.c    |  4 ++--
>   hw/usb/dev-serial.c | 10 +++++-----
>   hw/usb/hcd-xhci.c   |  2 +-
>   include/hw/usb.h    |  5 -----
>   6 files changed, 16 insertions(+), 21 deletions(-)
> 
> diff --git a/hw/usb/bus.c b/hw/usb/bus.c
> index d7c3c71435..4a1b67761c 100644
> --- a/hw/usb/bus.c
> +++ b/hw/usb/bus.c
> @@ -427,7 +427,7 @@ void usb_unregister_port(USBBus *bus, USBPort *port)
>   
>   void usb_claim_port(USBDevice *dev, Error **errp)
>   {
> -    USBBus *bus = usb_bus_from_device(dev);
> +    USBBus *bus = USB_BUS(qdev_get_parent_bus(DEVICE(dev)));

You're certainly missing a proper justification in the patch description 
here. The "other devices don't use such a helper" does not sound like a real 
justification to me, since the code lines rather get longer this way. Thus 
this rather looks like unnecessary code churn to me --> rather drop the patch?

  Thomas

Re: [RFC PATCH v2 19/19] hw/usb: Inline usb_bus_from_device()

[Philippe Mathieu-Daudé]

On 13/2/23 09:11, Thomas Huth wrote:
> On 13/02/2023 08.08, Philippe Mathieu-Daudé wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> RFC Other devices don't use such helper. Maybe it should
>>      be the other way around, introduce more bus_from_device()
>>      helpers?
>> ---
>>   hw/usb/bus.c        | 10 +++++-----
>>   hw/usb/core.c       |  6 +++---
>>   hw/usb/dev-hub.c    |  4 ++--
>>   hw/usb/dev-serial.c | 10 +++++-----
>>   hw/usb/hcd-xhci.c   |  2 +-
>>   include/hw/usb.h    |  5 -----
>>   6 files changed, 16 insertions(+), 21 deletions(-)
>>
>> diff --git a/hw/usb/bus.c b/hw/usb/bus.c
>> index d7c3c71435..4a1b67761c 100644
>> --- a/hw/usb/bus.c
>> +++ b/hw/usb/bus.c
>> @@ -427,7 +427,7 @@ void usb_unregister_port(USBBus *bus, USBPort *port)
>>   void usb_claim_port(USBDevice *dev, Error **errp)
>>   {
>> -    USBBus *bus = usb_bus_from_device(dev);
>> +    USBBus *bus = USB_BUS(qdev_get_parent_bus(DEVICE(dev)));
> 
> You're certainly missing a proper justification in the patch description 
> here. The "other devices don't use such a helper" does not sound like a 
> real justification to me, since the code lines rather get longer this 
> way. Thus this rather looks like unnecessary code churn to me --> rather 
> drop the patch?

The idea is to avoid having 7 different ways of implementing something
with 3 different APIs and 2 unfinished API conversions in flight.

I'm wondering if the QOM DECLARE_xxx() macros could also define some
xxx_BUS_FROM_DEV() or xxx_PARENT_BUS() macros. So here it would become:

     USBBus *bus = USB_PARENT_BUS(dev);

Re: [RFC PATCH v2 19/19] hw/usb: Inline usb_bus_from_device()

[Thomas Huth]

On 13/02/2023 09.44, Philippe Mathieu-Daudé wrote:
> On 13/2/23 09:11, Thomas Huth wrote:
>> On 13/02/2023 08.08, Philippe Mathieu-Daudé wrote:
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>> RFC Other devices don't use such helper. Maybe it should
>>>      be the other way around, introduce more bus_from_device()
>>>      helpers?
>>> ---
>>>   hw/usb/bus.c        | 10 +++++-----
>>>   hw/usb/core.c       |  6 +++---
>>>   hw/usb/dev-hub.c    |  4 ++--
>>>   hw/usb/dev-serial.c | 10 +++++-----
>>>   hw/usb/hcd-xhci.c   |  2 +-
>>>   include/hw/usb.h    |  5 -----
>>>   6 files changed, 16 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/hw/usb/bus.c b/hw/usb/bus.c
>>> index d7c3c71435..4a1b67761c 100644
>>> --- a/hw/usb/bus.c
>>> +++ b/hw/usb/bus.c
>>> @@ -427,7 +427,7 @@ void usb_unregister_port(USBBus *bus, USBPort *port)
>>>   void usb_claim_port(USBDevice *dev, Error **errp)
>>>   {
>>> -    USBBus *bus = usb_bus_from_device(dev);
>>> +    USBBus *bus = USB_BUS(qdev_get_parent_bus(DEVICE(dev)));
>>
>> You're certainly missing a proper justification in the patch description 
>> here. The "other devices don't use such a helper" does not sound like a 
>> real justification to me, since the code lines rather get longer this way. 
>> Thus this rather looks like unnecessary code churn to me --> rather drop 
>> the patch?
> 
> The idea is to avoid having 7 different ways of implementing something
> with 3 different APIs and 2 unfinished API conversions in flight.

Ok, then please add such information to the patch description.

> I'm wondering if the QOM DECLARE_xxx() macros could also define some
> xxx_BUS_FROM_DEV() or xxx_PARENT_BUS() macros. So here it would become:
> 
>      USBBus *bus = USB_PARENT_BUS(dev);

Sounds more readable at a first glance, but when looking at the output of:

   grep -r '(qdev_get_parent_bus' hw/

it seems like there aren't that many other places using this pattern (many 
places rather use BUS() instead), so it's maybe hard to justify such a 
change. Thus I think your patch here is likely the better solution right now 
(when you add a proper patch description).

  Thomas

Re: [RFC PATCH v2 19/19] hw/usb: Inline usb_bus_from_device()

[Philippe Mathieu-Daudé]

On 13/2/23 10:11, Thomas Huth wrote:
> On 13/02/2023 09.44, Philippe Mathieu-Daudé wrote:
>> On 13/2/23 09:11, Thomas Huth wrote:
>>> On 13/02/2023 08.08, Philippe Mathieu-Daudé wrote:
>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>> ---
>>>> RFC Other devices don't use such helper. Maybe it should
>>>>      be the other way around, introduce more bus_from_device()
>>>>      helpers?
>>>> ---
>>>>   hw/usb/bus.c        | 10 +++++-----
>>>>   hw/usb/core.c       |  6 +++---
>>>>   hw/usb/dev-hub.c    |  4 ++--
>>>>   hw/usb/dev-serial.c | 10 +++++-----
>>>>   hw/usb/hcd-xhci.c   |  2 +-
>>>>   include/hw/usb.h    |  5 -----
>>>>   6 files changed, 16 insertions(+), 21 deletions(-)
>>>>
>>>> diff --git a/hw/usb/bus.c b/hw/usb/bus.c
>>>> index d7c3c71435..4a1b67761c 100644
>>>> --- a/hw/usb/bus.c
>>>> +++ b/hw/usb/bus.c
>>>> @@ -427,7 +427,7 @@ void usb_unregister_port(USBBus *bus, USBPort 
>>>> *port)
>>>>   void usb_claim_port(USBDevice *dev, Error **errp)
>>>>   {
>>>> -    USBBus *bus = usb_bus_from_device(dev);
>>>> +    USBBus *bus = USB_BUS(qdev_get_parent_bus(DEVICE(dev)));
>>>
>>> You're certainly missing a proper justification in the patch 
>>> description here. The "other devices don't use such a helper" does 
>>> not sound like a real justification to me, since the code lines 
>>> rather get longer this way. Thus this rather looks like unnecessary 
>>> code churn to me --> rather drop the patch?
>>
>> The idea is to avoid having 7 different ways of implementing something
>> with 3 different APIs and 2 unfinished API conversions in flight.
> 
> Ok, then please add such information to the patch description.
> 
>> I'm wondering if the QOM DECLARE_xxx() macros could also define some
>> xxx_BUS_FROM_DEV() or xxx_PARENT_BUS() macros. So here it would become:
>>
>>      USBBus *bus = USB_PARENT_BUS(dev);
> 
> Sounds more readable at a first glance, but when looking at the output of:
> 
>    grep -r '(qdev_get_parent_bus' hw/
> 
> it seems like there aren't that many other places using this pattern 
> (many places rather use BUS() instead), so it's maybe hard to justify 
> such a change.

There is another helper, scsi_bus_from_device(), but is only used
twice. The previous patch "hw/scsi/scsi-bus: Inline two uses of
scsi_bus_from_device()" remove it.

 > Thus I think your patch here is likely the better
 > solution right now (when you add a proper patch description).

OK, thanks!