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

Markus Armbruster posted 29 patches 6 years, 6 months ago
Maintainers: Giuseppe Lettieri <g.lettieri@iet.unipi.it>, Andrzej Zaborowski <balrogg@gmail.com>, Stafford Horne <shorne@gmail.com>, Xiao Guangrong <xiaoguangrong.eric@gmail.com>, Aleksandar Markovic <amarkovic@wavecomp.com>, Riku Voipio <riku.voipio@iki.fi>, Michael Walle <michael@walle.cc>, Juan Quintela <quintela@redhat.com>, Pierre Morel <pmorel@linux.ibm.com>, Bastian Koppelmann <kbastian@mail.uni-paderborn.de>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>, Markus Armbruster <armbru@redhat.com>, BALATON Zoltan <balaton@eik.bme.hu>, Stefan Weil <sw@weilnetz.de>, Corey Minyard <minyard@acm.org>, Leif Lindholm <leif.lindholm@linaro.org>, Beniamino Galvani <b.galvani@gmail.com>, "Michael S. Tsirkin" <mst@redhat.com>, Amit Shah <amit@kernel.org>, Sagar Karandikar <sagark@eecs.berkeley.edu>, Collin Walling <walling@linux.ibm.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Yuval Shaia <yuval.shaia@oracle.com>, Liu Yuan <namei.unix@gmail.com>, Alex Williamson <alex.williamson@redhat.com>, Peter Chubb <peter.chubb@nicta.com.au>, Anthony Perard <anthony.perard@citrix.com>, Andrew Jeffery <andrew@aj.id.au>, Alistair Francis <Alistair.Francis@wdc.com>, Richard Henderson <rth@twiddle.net>, Luigi Rizzo <rizzo@iet.unipi.it>, Anthony Green <green@moxielogic.com>, Shannon Zhao <shannon.zhaosl@gmail.com>, Peter Lieven <pl@kamp.de>, Jason Wang <jasowang@redhat.com>, Laurent Vivier <laurent@vivier.eu>, Corey Minyard <cminyard@mvista.com>, Vincenzo Maffione <v.maffione@gmail.com>, James Hogan <jhogan@kernel.org>, Gonglei <arei.gonglei@huawei.com>, Christian Borntraeger <borntraeger@de.ibm.com>, Fabien Chouteau <chouteau@adacore.com>, "Cédric Le Goater" <clg@kaod.org>, David Gibson <david@gibson.dropbear.id.au>, Marek Vasut <marex@denx.de>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Alberto Garcia <berto@igalia.com>, Laurent Vivier <lvivier@redhat.com>, Max Filippov <jcmvbkbc@gmail.com>, KONRAD Frederic <frederic.konrad@adacore.com>, "Hervé Poussineau" <hpoussin@reactos.org>, Keith Busch <keith.busch@intel.com>, Stefano Stabellini <sstabellini@kernel.org>, zhanghailiang <zhang.zhanghailiang@huawei.com>, Eric Blake <eblake@redhat.com>, David Hildenbrand <david@redhat.com>, Paul Burton <pburton@wavecomp.com>, Jan Kiszka <jan.kiszka@web.de>, Igor Mitsyanko <i.mitsyanko@gmail.com>, Joel Stanley <joel@jms.id.au>, Stefan Berger <stefanb@linux.ibm.com>, Peter Maydell <peter.maydell@linaro.org>, Jia Liu <proljc@gmail.com>, Igor Mammedov <imammedo@redhat.com>, Helge Deller <deller@gmx.de>, Fam Zheng <fam@euphon.net>, Jiri Pirko <jiri@resnulli.us>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Magnus Damm <magnus.damm@gmail.com>, Samuel Thibault <samuel.thibault@ens-lyon.org>, Stefan Hajnoczi <stefanha@redhat.com>, Xie Changlong <xiechanglong.d@gmail.com>, Thomas Huth <huth@tuxfamily.org>, Marcelo Tosatti <mtosatti@redhat.com>, "Philippe Mathieu-Daudé" <philmd@redhat.com>, John Snow <jsnow@redhat.com>, Guan Xuetao <gxt@mprc.pku.edu.cn>, Palmer Dabbelt <palmer@sifive.com>, Gerd Hoffmann <kraxel@redhat.com>, Andrey Smirnov <andrew.smirnov@gmail.com>, Thomas Huth <thuth@redhat.com>, Hannes Reinecke <hare@suse.com>, Jiri Slaby <jslaby@suse.cz>, Kevin Wolf <kwolf@redhat.com>, Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>, Subbaraya Sundeep <sundeep.lkml@gmail.com>, Andrew Baumann <Andrew.Baumann@microsoft.com>, Alistair Francis <alistair@alistair23.me>, Rob Herring <robh@kernel.org>, Halil Pasic <pasic@linux.ibm.com>, Paolo Bonzini <pbonzini@redhat.com>, Aurelien Jarno <aurelien@aurel32.net>, Dmitry Fleytman <dmitry.fleytman@gmail.com>, Cornelia Huck <cohuck@redhat.com>, "Alex Bennée" <alex.bennee@linaro.org>, Eduardo Habkost <ehabkost@redhat.com>, Radoslaw Biernacki <radoslaw.biernacki@linaro.org>, Eric Farman <farman@linux.ibm.com>, Wen Congyang <wencongyang2@huawei.com>, Max Reitz <mreitz@redhat.com>, Chris Wulff <crwulff@gmail.com>, Eric Auger <eric.auger@redhat.com>, Greg Kurz <groug@kaod.org>, Paul Durrant <paul.durrant@citrix.com>, Antony Pavlov <antonynpavlov@gmail.com>, Michael Roth <mdroth@linux.vnet.ibm.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Jean-Christophe Dubois <jcd@tribudubois.net>, Tony Krowiak <akrowiak@linux.ibm.com>, Ben Warren <ben@skyportsystems.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Aleksandar Rikalo <arikalo@wavecomp.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Laszlo Ersek <lersek@redhat.com>, Artyom Tarasenko <atar4qemu@gmail.com>
There is a newer version of this series
[Qemu-devel] [PATCH v2 04/29] memory: Fix type of IOMMUMemoryRegionClass member @parent_class
Posted by Markus Armbruster 6 years, 6 months ago
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>
Reviewed-by: Philippe Mathieu-Daudé <philmd@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 5d8c9a9234..09d9b254fd 100644
--- a/memory.c
+++ b/memory.c
@@ -3245,6 +3245,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,
-- 
2.21.0


Re: [Qemu-devel] [PATCH v2 04/29] memory: Fix type of IOMMUMemoryRegionClass member @parent_class
Posted by Philippe Mathieu-Daudé 6 years, 6 months ago
On 8/6/19 5:14 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>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Tested-by: Philippe Mathieu-Daudé <philmd@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 5d8c9a9234..09d9b254fd 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -3245,6 +3245,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,
> 

Re: [Qemu-devel] [PATCH v2 04/29] memory: Fix type of IOMMUMemoryRegionClass member @parent_class
Posted by Paolo Bonzini 6 years, 6 months ago
On 07/08/19 12:11, Philippe Mathieu-Daudé wrote:
> On 8/6/19 5:14 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>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Acked-by: Paolo Bonzini <pbonzini@redhat.com>