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

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

[Philippe Mathieu-Daudé]

v3:
- Corrected TYPE_PCI_MULTISERIAL string
- Split EEPRO100 series out:
  https://lore.kernel.org/qemu-devel/20230213101048.94519-1-philmd@linaro.org/
- Split VFIO_CCW series out:
  https://lore.kernel.org/qemu-devel/20230213170145.45666-1-philmd@linaro.org/
- Removed "Inline usb_bus_from_device()" RFC patch
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é (14):
  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/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/usb/dev-hub: Use QOM USB_HUB() macro instead of casting
  hw/usb: Replace DO_UPCAST(USBBus) by USB_BUS()

 hw/char/serial-pci-multi.c | 93 +++++++++++++++++++-------------------
 hw/char/serial-pci.c       |  7 ++-
 hw/ide/qdev.c              | 10 ++--
 hw/net/ne2000-pci.c        | 18 +++++---
 hw/net/tulip.c             | 20 ++++----
 hw/pci/pci.c               |  2 +-
 hw/s390x/ipl.c             |  7 +--
 hw/scsi/scsi-bus.c         | 14 +++---
 hw/usb/dev-hub.c           |  6 +--
 include/hw/scsi/scsi.h     |  5 --
 include/hw/usb.h           |  2 +-
 11 files changed, 94 insertions(+), 90 deletions(-)

-- 
2.38.1

[PATCH v3 01/14] 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

Re: [PATCH v3 01/14] hw/char/serial-pci: Replace DO_UPCAST(PCISerialState) by PCI_SERIAL()

[Thomas Huth]

On 13/02/2023 19.43, Philippe Mathieu-Daudé wrote:
> 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 >*/
> +

I'm not sure about this part of the patch. It does not seem to be related to 
the other changes at all, and are you sure about which parts are really 
"public" and which parts are "private"? If so, I'd like to see a description 
about this in the commit message, preferably in a separate patch. Also, why 
an empty line after the "public" comment?

>       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));

Ack for the DO_UPCAST removal.

  Thomas

Re: [PATCH v3 01/14] hw/char/serial-pci: Replace DO_UPCAST(PCISerialState) by PCI_SERIAL()

[Philippe Mathieu-Daudé]

On 16/2/23 12:20, Thomas Huth wrote:
> On 13/02/2023 19.43, Philippe Mathieu-Daudé wrote:
>> 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 >*/
>> +
> 
> I'm not sure about this part of the patch. It does not seem to be 
> related to the other changes at all, and are you sure about which parts 
> are really "public" and which parts are "private"? If so, I'd like to 
> see a description about this in the commit message, preferably in a 
> separate patch. Also, why an empty line after the "public" comment?

This is how QOM style separates the object 'private' part -- the
inherited parent, used by the QOM-cast macros -- and the fields
specific to this object.
The private field *must* be the first one in the structure for the
cast macros to work.

Maybe this isn't a convention and we could make one, to unify the
API style. I'm open to better suggestion :)

I suppose I got custom to see it to distinct the QOM hierarchy and
now it helps me to detect what is QOM and what isn't.
Anyway I'll remove from this patch.


> Ack for the DO_UPCAST removal.

Thanks!

Re: [PATCH v3 01/14] hw/char/serial-pci: Replace DO_UPCAST(PCISerialState) by PCI_SERIAL()

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 2612 bytes --]

On Thu, 16 Feb 2023, Philippe Mathieu-Daudé wrote:
> On 16/2/23 12:20, Thomas Huth wrote:
>> On 13/02/2023 19.43, Philippe Mathieu-Daudé wrote:
>>> 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 >*/
>>> +
>> 
>> I'm not sure about this part of the patch. It does not seem to be related 
>> to the other changes at all, and are you sure about which parts are really 
>> "public" and which parts are "private"? If so, I'd like to see a 
>> description about this in the commit message, preferably in a separate 
>> patch. Also, why an empty line after the "public" comment?
>
> This is how QOM style separates the object 'private' part -- the
> inherited parent, used by the QOM-cast macros -- and the fields
> specific to this object.
> The private field *must* be the first one in the structure for the
> cast macros to work.
>
> Maybe this isn't a convention and we could make one, to unify the
> API style. I'm open to better suggestion :)
>
> I suppose I got custom to see it to distinct the QOM hierarchy and
> now it helps me to detect what is QOM and what isn't.
> Anyway I'll remove from this patch.

I also dislike these comments and empty lines in these struct definitions. 
I think it should be enough to document this QOM convention in the docs 
saying that each QOM object state has to have it's parent's state as first 
member and you're not supposed to access it directly (except maybe from 
very closely related sub class) but do a QOM cast instead. If this is 
clearly stated in the docs then there's no need to add comments about this 
in every object. You could tell QOM objects from other structs by the 
first member also being a QOM object and usually called parent or similar 
but sometimes just dev. If you really want to get fancy maybe you could 
hide it in a macro, something like:

OBJECT_STATE(PCISerialState, PCIDevice)
...
END_OBJECT_STATE

but I'm not sure I like that because it has to hide the braces in the 
macro so it's not clear it's just a struct. So just describing it in the 
docs if it's not already is probably enough.

Regards,
BALATON Zoltan

Re: [PATCH v3 01/14] hw/char/serial-pci: Replace DO_UPCAST(PCISerialState) by PCI_SERIAL()

[Philippe Mathieu-Daudé]

On 16/2/23 15:17, BALATON Zoltan wrote:
> On Thu, 16 Feb 2023, Philippe Mathieu-Daudé wrote:
>> On 16/2/23 12:20, Thomas Huth wrote:
>>> On 13/02/2023 19.43, Philippe Mathieu-Daudé wrote:
>>>> 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 >*/
>>>> +
>>>
>>> I'm not sure about this part of the patch. It does not seem to be 
>>> related to the other changes at all, and are you sure about which 
>>> parts are really "public" and which parts are "private"? If so, I'd 
>>> like to see a description about this in the commit message, 
>>> preferably in a separate patch. Also, why an empty line after the 
>>> "public" comment?
>>
>> This is how QOM style separates the object 'private' part -- the
>> inherited parent, used by the QOM-cast macros -- and the fields
>> specific to this object.
>> The private field *must* be the first one in the structure for the
>> cast macros to work.
>>
>> Maybe this isn't a convention and we could make one, to unify the
>> API style. I'm open to better suggestion :)
>>
>> I suppose I got custom to see it to distinct the QOM hierarchy and
>> now it helps me to detect what is QOM and what isn't.
>> Anyway I'll remove from this patch.
> 
> I also dislike these comments and empty lines in these struct 
> definitions. 

Well this and the new line somehow helps to not reorder this field
elsewhere in the structure.

> I think it should be enough to document this QOM convention 
> in the docs saying that each QOM object state has to have it's parent's 
> state as first member and you're not supposed to access it directly 
> (except maybe from very closely related sub class) but do a QOM cast 
> instead. If this is clearly stated in the docs then there's no need to 
> add comments about this in every object. You could tell QOM objects from 
> other structs by the first member also being a QOM object and usually 
> called parent or similar but sometimes just dev.

Maybe "first_field_is_always_the_qom_parent" so nobody will be tempted
to access it directly? :)

> If you really want to 
> get fancy maybe you could hide it in a macro, something like:
> 
> OBJECT_STATE(PCISerialState, PCIDevice)
> ...
> END_OBJECT_STATE
> 
> but I'm not sure I like that because it has to hide the braces in the 
> macro so it's not clear it's just a struct. So just describing it in the 
> docs if it's not already is probably enough.
> 
> Regards,
> BALATON Zoltan

[PATCH v3 02/14] 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 v3 03/14] 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..e56c0bc841 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-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 },
-            { },
-        },
     }
 };
 
-- 
2.38.1

[PATCH v3 04/14] 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 e56c0bc841..704be5c294 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

Re: [PATCH v3 04/14] hw/char/serial-pci-multi: Factor multi_serial_class_initfn() out

[Bernhard Beschow]

[-- Attachment #1: Type: text/plain, Size: 6291 bytes --]

On Mon, Feb 13, 2023 at 7:46 PM Philippe Mathieu-Daudé <philmd@linaro.org>
wrote:

> 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 e56c0bc841..704be5c294 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
>
>
>
This patch hits an assert for me:

    qemu-system-x86_64: ../src/qom/object.c:1279:
object_class_property_add: Assertion `!object_class_property_find(klass,
name)' failed.

with the following backtrace:

Thread 1 "qemu-system-x86" received signal SIGABRT, Aborted.
__pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6,
no_tid=no_tid@entry=0) at pthread_kill.c:44
44            return INTERNAL_SYSCALL_ERROR_P (ret) ?
INTERNAL_SYSCALL_ERRNO (ret) : 0;
(gdb) bt
#0  __pthread_kill_implementation (threadid=<optimized out>,
signo=signo@entry=6, no_tid=no_tid@entry=0) at pthread_kill.c:44
#1  0x00007ffff6c91953 in __pthread_kill_internal (signo=6,
threadid=<optimized out>) at pthread_kill.c:78
#2  0x00007ffff6c42ea8 in __GI_raise (sig=sig@entry=6) at
../sysdeps/posix/raise.c:26
#3  0x00007ffff6c2c53d in __GI_abort () at abort.c:79
#4  0x00007ffff6c2c45c in __assert_fail_base
    (fmt=0x7ffff6da5d68 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n",
assertion=0x5555560b9998 "!object_class_property_find(klass, name)",
file=0x5555560b94be "../src/qom/object.c", line=1279, function=<optimized
out>) at assert.c:92
#5  0x00007ffff6c3b9f6 in __assert_fail
    (assertion=assertion@entry=0x5555560b9998
"!object_class_property_find(klass, name)", file=file@entry=0x5555560b94be
"../src/qom/object.c", line=line@entry=1279,
function=function@entry=0x5555560b9d00
<__PRETTY_FUNCTION__.21> "object_class_property_add") at assert.c:101
#6  0x0000555555dbb690 in object_class_property_add
    (klass=klass@entry=0x5555569af820, name=name@entry=0x555555fbe04b
"chardev1", type=0x5555561230dd "str", get=0x555555db2ad0 <field_prop_get>,
set=0x555555db36d0 <field_prop_set>, release=0x5555559f8f90 <release_chr>,
opaque=0x5555563cc900 <multi_4x_serial_pci_properties>) at
../src/qom/object.c:1279
#7  0x0000555555db3e6d in qdev_class_add_property (prop=0x5555563cc900
<multi_4x_serial_pci_properties>, name=0x555555fbe04b "chardev1",
klass=0x5555569af820)
    at ../src/hw/core/qdev-properties.c:889
#8  device_class_set_props (dc=0x5555569af820, props=<optimized out>) at
../src/hw/core/qdev-properties.c:955
#9  0x0000555555dba590 in type_initialize (ti=0x5555567f4840) at
../src/qom/object.c:1094
#10 object_class_foreach_tramp (key=<optimized out>, value=0x5555567f4840,
opaque=0x7fffffffe260) at ../src/qom/object.c:1081
#11 0x00007ffff70bcda8 in g_hash_table_foreach (hash_table=0x5555567b92a0 =
{...}, func=0x555555dba530 <object_class_foreach_tramp>,
user_data=0x7fffffffe260)
    at ../glib/glib/ghash.c:2098
#12 0x0000555555dbaac6 in object_class_foreach (opaque=0x7fffffffe258,
include_abstract=false, implements_type=<optimized out>, fn=0x555555db8a70
<object_class_get_list_tramp>)
    at ../src/qom/object.c:87
#13 object_class_get_list (implements_type=implements_type@entry=0x55555603bb98
"machine", include_abstract=include_abstract@entry=false) at
../src/qom/object.c:1160
#14 0x0000555555b68df7 in select_machine (errp=<optimized out>,
qdict=0x55555684a4e0) at ../src/softmmu/vl.c:1580
#15 qemu_create_machine (qdict=0x55555684a4e0) at ../src/softmmu/vl.c:2015
#16 qemu_init (argc=<optimized out>, argv=0x7fffffffe548) at
../src/softmmu/vl.c:3542
#17 0x000055555596f07e in main (argc=<optimized out>, argv=<optimized out>)
at ../src/softmmu/main.c:47

[-- Attachment #2: Type: text/html, Size: 7368 bytes --]

[PATCH v3 05/14] 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 704be5c294..d660b6fa97 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 v3 06/14] 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 v3 07/14] 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 v3 08/14] 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

Re: [PATCH v3 08/14] hw/net/ne2000-pci: Replace DO_UPCAST(PCINE2000State) by PCI_NE2000()

[Philippe Mathieu-Daudé]

Hi Jason, do you Ack this patch?

On 13/2/23 19:43, Philippe Mathieu-Daudé wrote:
> 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,

[PATCH v3 09/14] 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

Re: [PATCH v3 09/14] hw/net/tulip: Finish QOM conversion

[Philippe Mathieu-Daudé]

Hi Jason, do you Ack this patch?

On 13/2/23 19:43, Philippe Mathieu-Daudé wrote:
> 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[] = {

Re: [PATCH v3 09/14] hw/net/tulip: Finish QOM conversion

[Jason Wang]

On Tue, Feb 28, 2023 at 9:39 PM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> Hi Jason, do you Ack this patch?

Yes.

Acked-by: Jason Wang <jasowang@redhat.com>

Thanks

>
> On 13/2/23 19:43, Philippe Mathieu-Daudé wrote:
> > 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[] = {
>

[PATCH v3 10/14] 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 v3 11/14] 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 v3 12/14] hw/scsi/scsi-bus: Inline two uses of scsi_bus_from_device()

[Philippe Mathieu-Daudé]

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

[PATCH v3 13/14] 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 v3 14/14] 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

Re: [PATCH v3 00/14] hw: Use QOM macros and remove DO_UPCAST() uses

[Philippe Mathieu-Daudé]

On 13/2/23 19:43, Philippe Mathieu-Daudé wrote:

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

> Philippe Mathieu-Daudé (14):
>    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/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/usb/dev-hub: Use QOM USB_HUB() macro instead of casting
>    hw/usb: Replace DO_UPCAST(USBBus) by USB_BUS()

I'll respin as tiny series to help maintainers catch which patches
belong to their areas.

Re: [PATCH v3 00/14] hw: Use QOM macros and remove DO_UPCAST() uses

[Philippe Mathieu-Daudé]

(meanwhile, forgot to Cc Alex)

On 28/2/23 14:43, Philippe Mathieu-Daudé wrote:
> On 13/2/23 19:43, Philippe Mathieu-Daudé wrote:
> 
>> 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.
> 
>> Philippe Mathieu-Daudé (14):
>>    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/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/usb/dev-hub: Use QOM USB_HUB() macro instead of casting
>>    hw/usb: Replace DO_UPCAST(USBBus) by USB_BUS()
> 
> I'll respin as tiny series to help maintainers catch which patches
> belong to their areas.