[PATCH v4 0/5] Fix definition of classes with wrong parent

[PATCH v4 0/5] Fix definition of classes with wrong parent

[Zhenzhong Duan]

Hi,

This fix definition of some classes with wrong parent.
Used below script to get a list, hoping all are addressed.

find "$directory" -type d \( -name "roms" -o -name "build" \) -prune -o -type f \( -name "*.c" -o -name "*.h" \) -print | while read -r file; do
    awk '/^(struct |typedef struct )/ { 
        first = $0;
        getline second; 
        getline third; 
        if (first ~ /[Cc]lass/ && !(second ~ /Class/) && !(third ~ /Class/)) {
            print FILENAME ":" NR-2 ": " first; 
            print FILENAME ":" NR-1 ": " second; 
            print FILENAME ":" NR ": " third;
        }
    }' "$file"
done

patch4/5 are more like cleanups instead of fixes as those class_size are not
initialized so class definition doesn't take effect, so no Fixes tag for them.

Thanks
Zhenzhong

Changelog:
v4:
- add comments 'this isn't catastrophic only because sizeof(VirtIODevice) >
  sizeof(VirtioDeviceClass).' to patch1/2/3 (Markus)
- collect RB for patch5

v3:
- s/Suggested-by/Reported-by on patch2 (David)
- add Closes: tag (David)
- collect RB

v2:
- add more fixes per David

Zhenzhong Duan (5):
  virtio-mem: Fix definition of VirtIOMEMClass
  virtio-pmem: Fix definition of VirtIOPMEMClass
  hw/gpio/aspeed: Fix definition of AspeedGPIOClass
  hw/char/sh_serial: Remove dummy definition of SH_SERIAL class
  hw/riscv/riscv-iommu: Remove definition of RISCVIOMMU[Pci|Sys]Class

 include/hw/gpio/aspeed_gpio.h   | 2 +-
 include/hw/riscv/iommu.h        | 6 ++----
 include/hw/virtio/virtio-mem.h  | 2 +-
 include/hw/virtio/virtio-pmem.h | 2 +-
 hw/char/sh_serial.c             | 4 +---
 hw/riscv/riscv-iommu-pci.c      | 6 ------
 hw/riscv/riscv-iommu-sys.c      | 6 ------
 7 files changed, 6 insertions(+), 22 deletions(-)

-- 
2.34.1

[PATCH v4 1/5] virtio-mem: Fix definition of VirtIOMEMClass

[Zhenzhong Duan]

Parent of VirtIOMEMClass is VirtioDeviceClass rather than VirtIODevice.
This isn't catastrophic only because sizeof(VirtIODevice) >
sizeof(VirtioDeviceClass).

Fixes: 910b25766b33 ("virtio-mem: Paravirtualized memory hot(un)plug")
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/virtio/virtio-mem.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/hw/virtio/virtio-mem.h b/include/hw/virtio/virtio-mem.h
index bc4f787772..e0ab31b45a 100644
--- a/include/hw/virtio/virtio-mem.h
+++ b/include/hw/virtio/virtio-mem.h
@@ -134,7 +134,7 @@ struct VirtioMemSystemReset {
 
 struct VirtIOMEMClass {
     /* private */
-    VirtIODevice parent;
+    VirtioDeviceClass parent_class;
 
     /* public */
     void (*fill_device_info)(const VirtIOMEM *vmen, VirtioMEMDeviceInfo *vi);
-- 
2.34.1

[PATCH v4 2/5] virtio-pmem: Fix definition of VirtIOPMEMClass

[Zhenzhong Duan]

VirtIOPMEMClass's parent is VirtioDeviceClass rather than VirtIODevice.
This isn't catastrophic only because sizeof(VirtIODevice) >
sizeof(VirtioDeviceClass).

Fixes: 5f503cd9f388 ("virtio-pmem: add virtio device")
Closes: https://lists.gnu.org/archive/html/qemu-devel/2025-06/msg00586.html
Reported-by: David Hildenbrand <david@redhat.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 include/hw/virtio/virtio-pmem.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/hw/virtio/virtio-pmem.h b/include/hw/virtio/virtio-pmem.h
index fc4fd1f7fe..9cce600d0b 100644
--- a/include/hw/virtio/virtio-pmem.h
+++ b/include/hw/virtio/virtio-pmem.h
@@ -36,7 +36,7 @@ struct VirtIOPMEM {
 
 struct VirtIOPMEMClass {
     /* private */
-    VirtIODevice parent;
+    VirtioDeviceClass parent_class;
 
     /* public */
     void (*fill_device_info)(const VirtIOPMEM *pmem, VirtioPMEMDeviceInfo *vi);
-- 
2.34.1

[PATCH v4 3/5] hw/gpio/aspeed: Fix definition of AspeedGPIOClass

[Zhenzhong Duan]

AspeedGPIOClass's parent is SysBusDeviceClass rather than SysBusDevice.
This isn't catastrophic only because sizeof(SysBusDevice) >
sizeof(SysBusDeviceClass).

Fixes: 4b7f956862dc ("hw/gpio: Add basic Aspeed GPIO model for AST2400 and AST2500")
Closes: https://lists.gnu.org/archive/html/qemu-devel/2025-06/msg00586.html
Suggested-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 include/hw/gpio/aspeed_gpio.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/hw/gpio/aspeed_gpio.h b/include/hw/gpio/aspeed_gpio.h
index e1e6c54333..e6b2fe71b5 100644
--- a/include/hw/gpio/aspeed_gpio.h
+++ b/include/hw/gpio/aspeed_gpio.h
@@ -70,7 +70,7 @@ typedef struct AspeedGPIOReg {
 } AspeedGPIOReg;
 
 struct AspeedGPIOClass {
-    SysBusDevice parent_obj;
+    SysBusDeviceClass parent_class;
     const GPIOSetProperties *props;
     uint32_t nr_gpio_pins;
     uint32_t nr_gpio_sets;
-- 
2.34.1

[PATCH v4 4/5] hw/char/sh_serial: Remove dummy definition of SH_SERIAL class

[Zhenzhong Duan]

SH_SERIAL is declared with OBJECT_DECLARE_SIMPLE_TYPE but defined with
OBJECT_DEFINE_TYPE, SHSerialStateClass is also a dummy class which
missed its parent.

Change to use OBJECT_DEFINE_SIMPLE_TYPE and remove SHSerialStateClass.

Closes: https://lists.gnu.org/archive/html/qemu-devel/2025-06/msg00586.html
Suggested-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 hw/char/sh_serial.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/char/sh_serial.c b/hw/char/sh_serial.c
index 6abd80386f..8ccc2234ba 100644
--- a/hw/char/sh_serial.c
+++ b/hw/char/sh_serial.c
@@ -78,9 +78,7 @@ struct SHSerialState {
     qemu_irq bri;
 };
 
-typedef struct {} SHSerialStateClass;
-
-OBJECT_DEFINE_TYPE(SHSerialState, sh_serial, SH_SERIAL, SYS_BUS_DEVICE)
+OBJECT_DEFINE_SIMPLE_TYPE(SHSerialState, sh_serial, SH_SERIAL, SYS_BUS_DEVICE)
 
 static void sh_serial_clear_fifo(SHSerialState *s)
 {
-- 
2.34.1

Re: [PATCH v4 4/5] hw/char/sh_serial: Remove dummy definition of SH_SERIAL class

[Philippe Mathieu-Daudé]

On 6/6/25 11:24, Zhenzhong Duan wrote:
> SH_SERIAL is declared with OBJECT_DECLARE_SIMPLE_TYPE but defined with
> OBJECT_DEFINE_TYPE, SHSerialStateClass is also a dummy class which
> missed its parent.
> 
> Change to use OBJECT_DEFINE_SIMPLE_TYPE and remove SHSerialStateClass.
> 
> Closes: https://lists.gnu.org/archive/html/qemu-devel/2025-06/msg00586.html
> Suggested-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>   hw/char/sh_serial.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/hw/char/sh_serial.c b/hw/char/sh_serial.c
> index 6abd80386f..8ccc2234ba 100644
> --- a/hw/char/sh_serial.c
> +++ b/hw/char/sh_serial.c
> @@ -78,9 +78,7 @@ struct SHSerialState {
>       qemu_irq bri;
>   };
>   
> -typedef struct {} SHSerialStateClass;
> -
> -OBJECT_DEFINE_TYPE(SHSerialState, sh_serial, SH_SERIAL, SYS_BUS_DEVICE)
> +OBJECT_DEFINE_SIMPLE_TYPE(SHSerialState, sh_serial, SH_SERIAL, SYS_BUS_DEVICE)

I'm surprised this is the first time we use OBJECT_DEFINE_SIMPLE_TYPE.

Re: [PATCH v4 4/5] hw/char/sh_serial: Remove dummy definition of SH_SERIAL class

[Philippe Mathieu-Daudé]

On 6/6/25 12:40, Philippe Mathieu-Daudé wrote:
> On 6/6/25 11:24, Zhenzhong Duan wrote:
>> SH_SERIAL is declared with OBJECT_DECLARE_SIMPLE_TYPE but defined with
>> OBJECT_DEFINE_TYPE, SHSerialStateClass is also a dummy class which
>> missed its parent.
>>
>> Change to use OBJECT_DEFINE_SIMPLE_TYPE and remove SHSerialStateClass.
>>
>> Closes: https://lists.gnu.org/archive/html/qemu-devel/2025-06/ 
>> msg00586.html
>> Suggested-by: David Hildenbrand <david@redhat.com>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>>   hw/char/sh_serial.c | 4 +---
>>   1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/hw/char/sh_serial.c b/hw/char/sh_serial.c
>> index 6abd80386f..8ccc2234ba 100644
>> --- a/hw/char/sh_serial.c
>> +++ b/hw/char/sh_serial.c
>> @@ -78,9 +78,7 @@ struct SHSerialState {
>>       qemu_irq bri;
>>   };
>> -typedef struct {} SHSerialStateClass;
>> -
>> -OBJECT_DEFINE_TYPE(SHSerialState, sh_serial, SH_SERIAL, SYS_BUS_DEVICE)
>> +OBJECT_DEFINE_SIMPLE_TYPE(SHSerialState, sh_serial, SH_SERIAL, 
>> SYS_BUS_DEVICE)
> 
> I'm surprised this is the first time we use OBJECT_DEFINE_SIMPLE_TYPE.

Actually I already posted a patch cleaning that...:

https://lore.kernel.org/qemu-devel/20250124175053.74461-1-philmd@linaro.org/

Also I now notice I never replied to Peter's questions. I'll and
queue the series, along with yours.

Regards,

Phil.

RE: [PATCH v4 4/5] hw/char/sh_serial: Remove dummy definition of SH_SERIAL class

[Duan, Zhenzhong]

>-----Original Message-----
>From: Philippe Mathieu-Daudé <philmd@linaro.org>
>Subject: Re: [PATCH v4 4/5] hw/char/sh_serial: Remove dummy definition of
>SH_SERIAL class
>
>On 6/6/25 12:40, Philippe Mathieu-Daudé wrote:
>> On 6/6/25 11:24, Zhenzhong Duan wrote:
>>> SH_SERIAL is declared with OBJECT_DECLARE_SIMPLE_TYPE but defined with
>>> OBJECT_DEFINE_TYPE, SHSerialStateClass is also a dummy class which
>>> missed its parent.
>>>
>>> Change to use OBJECT_DEFINE_SIMPLE_TYPE and remove SHSerialStateClass.
>>>
>>> Closes: https://lists.gnu.org/archive/html/qemu-devel/2025-06/
>>> msg00586.html
>>> Suggested-by: David Hildenbrand <david@redhat.com>
>>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>>> ---
>>>   hw/char/sh_serial.c | 4 +---
>>>   1 file changed, 1 insertion(+), 3 deletions(-)
>>>
>>> diff --git a/hw/char/sh_serial.c b/hw/char/sh_serial.c
>>> index 6abd80386f..8ccc2234ba 100644
>>> --- a/hw/char/sh_serial.c
>>> +++ b/hw/char/sh_serial.c
>>> @@ -78,9 +78,7 @@ struct SHSerialState {
>>>       qemu_irq bri;
>>>   };
>>> -typedef struct {} SHSerialStateClass;
>>> -
>>> -OBJECT_DEFINE_TYPE(SHSerialState, sh_serial, SH_SERIAL, SYS_BUS_DEVICE)
>>> +OBJECT_DEFINE_SIMPLE_TYPE(SHSerialState, sh_serial, SH_SERIAL,
>>> SYS_BUS_DEVICE)
>>
>> I'm surprised this is the first time we use OBJECT_DEFINE_SIMPLE_TYPE.

Ah, yes.

>
>Actually I already posted a patch cleaning that...:
>
>https://lore.kernel.org/qemu-devel/20250124175053.74461-1-
>philmd@linaro.org/

Good to see.

>
>Also I now notice I never replied to Peter's questions. I'll and
>queue the series, along with yours.
Thanks.

BRs,
Zhenzhong

[PATCH v4 5/5] hw/riscv/riscv-iommu: Remove definition of RISCVIOMMU[Pci|Sys]Class

[Zhenzhong Duan]

RISCVIOMMUPciClass and RISCVIOMMUSysClass are defined with missed
parent class, class_init on them may corrupt their parent class
fields.

It's lucky that parent_realize and parent_phases are not initialized
or used until now, so just remove the definitions. They can be added
back when really necessary.

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
 include/hw/riscv/iommu.h   | 6 ++----
 hw/riscv/riscv-iommu-pci.c | 6 ------
 hw/riscv/riscv-iommu-sys.c | 6 ------
 3 files changed, 2 insertions(+), 16 deletions(-)

diff --git a/include/hw/riscv/iommu.h b/include/hw/riscv/iommu.h
index b03339d75c..8a8acfc3f0 100644
--- a/include/hw/riscv/iommu.h
+++ b/include/hw/riscv/iommu.h
@@ -30,14 +30,12 @@ typedef struct RISCVIOMMUState RISCVIOMMUState;
 typedef struct RISCVIOMMUSpace RISCVIOMMUSpace;
 
 #define TYPE_RISCV_IOMMU_PCI "riscv-iommu-pci"
-OBJECT_DECLARE_TYPE(RISCVIOMMUStatePci, RISCVIOMMUPciClass, RISCV_IOMMU_PCI)
+OBJECT_DECLARE_SIMPLE_TYPE(RISCVIOMMUStatePci, RISCV_IOMMU_PCI)
 typedef struct RISCVIOMMUStatePci RISCVIOMMUStatePci;
-typedef struct RISCVIOMMUPciClass RISCVIOMMUPciClass;
 
 #define TYPE_RISCV_IOMMU_SYS "riscv-iommu-device"
-OBJECT_DECLARE_TYPE(RISCVIOMMUStateSys, RISCVIOMMUSysClass, RISCV_IOMMU_SYS)
+OBJECT_DECLARE_SIMPLE_TYPE(RISCVIOMMUStateSys, RISCV_IOMMU_SYS)
 typedef struct RISCVIOMMUStateSys RISCVIOMMUStateSys;
-typedef struct RISCVIOMMUSysClass RISCVIOMMUSysClass;
 
 #define FDT_IRQ_TYPE_EDGE_LOW 1
 
diff --git a/hw/riscv/riscv-iommu-pci.c b/hw/riscv/riscv-iommu-pci.c
index 1f44eef74e..cdb4a7a8f0 100644
--- a/hw/riscv/riscv-iommu-pci.c
+++ b/hw/riscv/riscv-iommu-pci.c
@@ -68,12 +68,6 @@ typedef struct RISCVIOMMUStatePci {
     RISCVIOMMUState  iommu;   /* common IOMMU state */
 } RISCVIOMMUStatePci;
 
-struct RISCVIOMMUPciClass {
-    /*< public >*/
-    DeviceRealize parent_realize;
-    ResettablePhases parent_phases;
-};
-
 /* interrupt delivery callback */
 static void riscv_iommu_pci_notify(RISCVIOMMUState *iommu, unsigned vector)
 {
diff --git a/hw/riscv/riscv-iommu-sys.c b/hw/riscv/riscv-iommu-sys.c
index 74e76b94a5..e34d00aef6 100644
--- a/hw/riscv/riscv-iommu-sys.c
+++ b/hw/riscv/riscv-iommu-sys.c
@@ -53,12 +53,6 @@ struct RISCVIOMMUStateSys {
     uint8_t *msix_pba;
 };
 
-struct RISCVIOMMUSysClass {
-    /*< public >*/
-    DeviceRealize parent_realize;
-    ResettablePhases parent_phases;
-};
-
 static uint64_t msix_table_mmio_read(void *opaque, hwaddr addr,
                                      unsigned size)
 {
-- 
2.34.1

Re: [PATCH v4 5/5] hw/riscv/riscv-iommu: Remove definition of RISCVIOMMU[Pci|Sys]Class

[Alistair Francis]

On Fri, Jun 6, 2025 at 7:29 PM Zhenzhong Duan <zhenzhong.duan@intel.com> wrote:
>
> RISCVIOMMUPciClass and RISCVIOMMUSysClass are defined with missed
> parent class, class_init on them may corrupt their parent class
> fields.
>
> It's lucky that parent_realize and parent_phases are not initialized
> or used until now, so just remove the definitions. They can be added
> back when really necessary.
>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

Acked-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  include/hw/riscv/iommu.h   | 6 ++----
>  hw/riscv/riscv-iommu-pci.c | 6 ------
>  hw/riscv/riscv-iommu-sys.c | 6 ------
>  3 files changed, 2 insertions(+), 16 deletions(-)
>
> diff --git a/include/hw/riscv/iommu.h b/include/hw/riscv/iommu.h
> index b03339d75c..8a8acfc3f0 100644
> --- a/include/hw/riscv/iommu.h
> +++ b/include/hw/riscv/iommu.h
> @@ -30,14 +30,12 @@ typedef struct RISCVIOMMUState RISCVIOMMUState;
>  typedef struct RISCVIOMMUSpace RISCVIOMMUSpace;
>
>  #define TYPE_RISCV_IOMMU_PCI "riscv-iommu-pci"
> -OBJECT_DECLARE_TYPE(RISCVIOMMUStatePci, RISCVIOMMUPciClass, RISCV_IOMMU_PCI)
> +OBJECT_DECLARE_SIMPLE_TYPE(RISCVIOMMUStatePci, RISCV_IOMMU_PCI)
>  typedef struct RISCVIOMMUStatePci RISCVIOMMUStatePci;
> -typedef struct RISCVIOMMUPciClass RISCVIOMMUPciClass;
>
>  #define TYPE_RISCV_IOMMU_SYS "riscv-iommu-device"
> -OBJECT_DECLARE_TYPE(RISCVIOMMUStateSys, RISCVIOMMUSysClass, RISCV_IOMMU_SYS)
> +OBJECT_DECLARE_SIMPLE_TYPE(RISCVIOMMUStateSys, RISCV_IOMMU_SYS)
>  typedef struct RISCVIOMMUStateSys RISCVIOMMUStateSys;
> -typedef struct RISCVIOMMUSysClass RISCVIOMMUSysClass;
>
>  #define FDT_IRQ_TYPE_EDGE_LOW 1
>
> diff --git a/hw/riscv/riscv-iommu-pci.c b/hw/riscv/riscv-iommu-pci.c
> index 1f44eef74e..cdb4a7a8f0 100644
> --- a/hw/riscv/riscv-iommu-pci.c
> +++ b/hw/riscv/riscv-iommu-pci.c
> @@ -68,12 +68,6 @@ typedef struct RISCVIOMMUStatePci {
>      RISCVIOMMUState  iommu;   /* common IOMMU state */
>  } RISCVIOMMUStatePci;
>
> -struct RISCVIOMMUPciClass {
> -    /*< public >*/
> -    DeviceRealize parent_realize;
> -    ResettablePhases parent_phases;
> -};
> -
>  /* interrupt delivery callback */
>  static void riscv_iommu_pci_notify(RISCVIOMMUState *iommu, unsigned vector)
>  {
> diff --git a/hw/riscv/riscv-iommu-sys.c b/hw/riscv/riscv-iommu-sys.c
> index 74e76b94a5..e34d00aef6 100644
> --- a/hw/riscv/riscv-iommu-sys.c
> +++ b/hw/riscv/riscv-iommu-sys.c
> @@ -53,12 +53,6 @@ struct RISCVIOMMUStateSys {
>      uint8_t *msix_pba;
>  };
>
> -struct RISCVIOMMUSysClass {
> -    /*< public >*/
> -    DeviceRealize parent_realize;
> -    ResettablePhases parent_phases;
> -};
> -
>  static uint64_t msix_table_mmio_read(void *opaque, hwaddr addr,
>                                       unsigned size)
>  {
> --
> 2.34.1
>
>