Re: [Qemu-devel] [PATCH 04/28] memory: Fix type of IOMMUMemoryRegionClass member @parent_class

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

On 7/26/19 2:05 PM, Markus Armbruster wrote:
> TYPE_IOMMU_MEMORY_REGION is a direct subtype of TYPE_MEMORY_REGION.
> Its instance struct is IOMMUMemoryRegion, and its first member is a
> MemoryRegion.  Correct.  Its class struct is IOMMUMemoryRegionClass,
> and its first member is a DeviceClass.  Wrong.  Messed up when commit
> 1221a474676 introduced the QOM type.  It even included hw/qdev-core.h
> just for that.
> 
> TYPE_MEMORY_REGION doesn't bother to define a class struct.  This is
> fine, it simply defaults to its super-type TYPE_OBJECT's class struct
> ObjectClass.  Changing IOMMUMemoryRegionClass's first member's type to
> ObjectClass would be a minimal fix, if a bit brittle: if
> TYPE_MEMORY_REGION ever acquired own class struct, we'd have to update
> IOMMUMemoryRegionClass to use it.
> 
> Fix it the clean and robust way instead: give TYPE_MEMORY_REGION its
> own class struct MemoryRegionClass now, and use it for
> IOMMUMemoryRegionClass's first member.
> 
> Revert the include of hw/qdev-core.h, and fix the few files that have
> come to rely on it.
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/display/vga-isa-mm.c | 1 +
>  hw/net/pcnet.h          | 1 +
>  include/exec/memory.h   | 9 +++++++--
>  memory.c                | 1 +
>  4 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/display/vga-isa-mm.c b/hw/display/vga-isa-mm.c
> index 215e649719..a790f69b6d 100644
> --- a/hw/display/vga-isa-mm.c
> +++ b/hw/display/vga-isa-mm.c
> @@ -22,6 +22,7 @@
>   * THE SOFTWARE.
>   */
>  #include "qemu/osdep.h"
> +#include "qemu/bitops.h"
>  #include "qemu/units.h"
>  #include "hw/hw.h"
>  #include "hw/display/vga.h"
> diff --git a/hw/net/pcnet.h b/hw/net/pcnet.h
> index 40831a7845..28d19a5c6f 100644
> --- a/hw/net/pcnet.h
> +++ b/hw/net/pcnet.h
> @@ -8,6 +8,7 @@
>  #define PCNET_LOOPTEST_NOCRC	2
>  
>  #include "exec/memory.h"
> +#include "hw/irq.h"
>  
>  /* BUS CONFIGURATION REGISTERS */
>  #define BCR_MSRDA    0
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index bb0961ddb9..238370a2ff 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -25,7 +25,6 @@
>  #include "qemu/notify.h"
>  #include "qom/object.h"
>  #include "qemu/rcu.h"
> -#include "hw/qdev-core.h"
>  
>  #define RAM_ADDR_INVALID (~(ram_addr_t)0)
>  
> @@ -205,6 +204,12 @@ struct MemoryRegionOps {
>      } impl;
>  };
>  
> +typedef struct MemoryRegionClass {
> +    /* private */
> +    ObjectClass parent_class;
> +} MemoryRegionClass;
> +
> +
>  enum IOMMUMemoryRegionAttr {
>      IOMMU_ATTR_SPAPR_TCE_FD
>  };
> @@ -237,7 +242,7 @@ enum IOMMUMemoryRegionAttr {
>   */
>  typedef struct IOMMUMemoryRegionClass {
>      /* private */
> -    struct DeviceClass parent_class;
> +    MemoryRegionClass parent_class;
>  
>      /*
>       * Return a TLB entry that contains a given address.
> diff --git a/memory.c b/memory.c
> index d4579bbaec..bf108b596e 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -3223,6 +3223,7 @@ void memory_region_init_rom_device(MemoryRegion *mr,
>  static const TypeInfo memory_region_info = {
>      .parent             = TYPE_OBJECT,
>      .name               = TYPE_MEMORY_REGION,
> +    .class_size         = sizeof(MemoryRegionClass),
>      .instance_size      = sizeof(MemoryRegion),
>      .instance_init      = memory_region_initfn,
>      .instance_finalize  = memory_region_finalize,
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>