[PATCH-for-8.2 0/6] hw: Free DEFINE_PROP_ARRAY()'s arrays in instance_finalize()

[PATCH-for-8.2 0/6] hw: Free DEFINE_PROP_ARRAY()'s arrays in instance_finalize()

[Philippe Mathieu-Daudé]

In few places we forget to free the array allocated by the
DEFINE_PROP_ARRAY() macro handlers. Fix that.

Philippe Mathieu-Daudé (6):
  hw/virtio: Add VirtioPCIDeviceTypeInfo::instance_finalize field
  hw/virtio: Free VirtIOIOMMUPCI::vdev.reserved_regions[] on finalize()
  hw/misc/mps2-scc: Free MPS2SCC::oscclk[] array on finalize()
  hw/nvram/xlnx-efuse: Free XlnxEFuse::ro_bits[] array on finalize()
  hw/nvram/xlnx-efuse-ctrl: Free XlnxVersalEFuseCtrl[] "pg0-lock" array
  hw/input/stellaris_gamepad: Free StellarisGamepad::keycodes[] array

 include/hw/virtio/virtio-pci.h    | 1 +
 hw/input/stellaris_gamepad.c      | 8 ++++++++
 hw/misc/mps2-scc.c                | 8 ++++++++
 hw/nvram/xlnx-efuse.c             | 8 ++++++++
 hw/nvram/xlnx-versal-efuse-ctrl.c | 8 ++++++++
 hw/virtio/virtio-iommu-pci.c      | 8 ++++++++
 hw/virtio/virtio-pci.c            | 1 +
 7 files changed, 42 insertions(+)

-- 
2.41.0

[PATCH-for-8.2? 1/6] hw/virtio: Add VirtioPCIDeviceTypeInfo::instance_finalize field

[Philippe Mathieu-Daudé]

The VirtioPCIDeviceTypeInfo structure, added in commit a4ee4c8baa
("virtio: Helper for registering virtio device types") got extended
in commit 8ea90ee690 ("virtio: add class_size") with the @class_size
field. Do similarly with the @instance_finalize field.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/virtio/virtio-pci.h | 1 +
 hw/virtio/virtio-pci.c         | 1 +
 2 files changed, 2 insertions(+)

diff --git a/include/hw/virtio/virtio-pci.h b/include/hw/virtio/virtio-pci.h
index 5a3f182f99..59d88018c1 100644
--- a/include/hw/virtio/virtio-pci.h
+++ b/include/hw/virtio/virtio-pci.h
@@ -246,6 +246,7 @@ typedef struct VirtioPCIDeviceTypeInfo {
     size_t instance_size;
     size_t class_size;
     void (*instance_init)(Object *obj);
+    void (*instance_finalize)(Object *obj);
     void (*class_init)(ObjectClass *klass, void *data);
     InterfaceInfo *interfaces;
 } VirtioPCIDeviceTypeInfo;
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 205dbf24fb..e433879542 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -2391,6 +2391,7 @@ void virtio_pci_types_register(const VirtioPCIDeviceTypeInfo *t)
         .parent        = t->parent ? t->parent : TYPE_VIRTIO_PCI,
         .instance_size = t->instance_size,
         .instance_init = t->instance_init,
+        .instance_finalize = t->instance_finalize,
         .class_size    = t->class_size,
         .abstract      = true,
         .interfaces    = t->interfaces,
-- 
2.41.0

[PATCH-for-8.2? 2/6] hw/virtio: Free VirtIOIOMMUPCI::vdev.reserved_regions[] on finalize()

[Philippe Mathieu-Daudé]

Commit 0be6bfac62 ("qdev: Implement variable length array properties")
added the DEFINE_PROP_ARRAY() macro with the following comment:

  * It is the responsibility of the device deinit code to free the
  * @_arrayfield memory.

Commit 8077b8e549 added:

  DEFINE_PROP_ARRAY("reserved-regions", VirtIOIOMMUPCI,
                    vdev.nb_reserved_regions, vdev.reserved_regions,
                    qdev_prop_reserved_region, ReservedRegion),

but forgot to free the 'vdev.reserved_regions' array. Do it in the
instance_finalize() handler.

Cc: qemu-stable@nongnu.org
Fixes: 8077b8e549 ("virtio-iommu-pci: Add array of Interval properties") # v5.1.0+
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/virtio/virtio-iommu-pci.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/virtio/virtio-iommu-pci.c b/hw/virtio/virtio-iommu-pci.c
index 9459fbf6ed..cbdfe4c591 100644
--- a/hw/virtio/virtio-iommu-pci.c
+++ b/hw/virtio/virtio-iommu-pci.c
@@ -95,10 +95,18 @@ static void virtio_iommu_pci_instance_init(Object *obj)
                                 TYPE_VIRTIO_IOMMU);
 }
 
+static void virtio_iommu_pci_instance_finalize(Object *obj)
+{
+    VirtIOIOMMUPCI *dev = VIRTIO_IOMMU_PCI(obj);
+
+    g_free(dev->vdev.prop_resv_regions);
+}
+
 static const VirtioPCIDeviceTypeInfo virtio_iommu_pci_info = {
     .generic_name  = TYPE_VIRTIO_IOMMU_PCI,
     .instance_size = sizeof(VirtIOIOMMUPCI),
     .instance_init = virtio_iommu_pci_instance_init,
+    .instance_finalize = virtio_iommu_pci_instance_finalize,
     .class_init    = virtio_iommu_pci_class_init,
 };
 
-- 
2.41.0

[PATCH-for-8.2? 3/6] hw/misc/mps2-scc: Free MPS2SCC::oscclk[] array on finalize()

[Philippe Mathieu-Daudé]

Commit 0be6bfac62 ("qdev: Implement variable length array properties")
added the DEFINE_PROP_ARRAY() macro with the following comment:

  * It is the responsibility of the device deinit code to free the
  * @_arrayfield memory.

Commit 4fb013afcc added:

  DEFINE_PROP_ARRAY("oscclk", MPS2SCC, num_oscclk, oscclk_reset,
                    qdev_prop_uint32, uint32_t),

but forgot to free the 'oscclk_reset' array. Do it in the
instance_finalize() handler.

Cc: qemu-stable@nongnu.org
Fixes: 4fb013afcc ("hw/misc/mps2-scc: Support configurable number of OSCCLK values") # v6.0.0+
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/misc/mps2-scc.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/misc/mps2-scc.c b/hw/misc/mps2-scc.c
index b3b42a792c..fe5034db14 100644
--- a/hw/misc/mps2-scc.c
+++ b/hw/misc/mps2-scc.c
@@ -329,6 +329,13 @@ static void mps2_scc_realize(DeviceState *dev, Error **errp)
     s->oscclk = g_new0(uint32_t, s->num_oscclk);
 }
 
+static void mps2_scc_finalize(Object *obj)
+{
+    MPS2SCC *s = MPS2_SCC(obj);
+
+    g_free(s->oscclk_reset);
+}
+
 static const VMStateDescription mps2_scc_vmstate = {
     .name = "mps2-scc",
     .version_id = 3,
@@ -385,6 +392,7 @@ static const TypeInfo mps2_scc_info = {
     .parent = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(MPS2SCC),
     .instance_init = mps2_scc_init,
+    .instance_finalize = mps2_scc_finalize,
     .class_init = mps2_scc_class_init,
 };
 
-- 
2.41.0

[PATCH-for-8.2? 4/6] hw/nvram/xlnx-efuse: Free XlnxEFuse::ro_bits[] array on finalize()

[Philippe Mathieu-Daudé]

Commit 0be6bfac62 ("qdev: Implement variable length array properties")
added the DEFINE_PROP_ARRAY() macro with the following comment:

  * It is the responsibility of the device deinit code to free the
  * @_arrayfield memory.

Commit 68fbcc344e added:

  DEFINE_PROP_ARRAY("read-only", XlnxEFuse, ro_bits_cnt, ro_bits,
                    qdev_prop_uint32, uint32_t),

but forgot to free the 'ro_bits' array. Do it in the instance_finalize
handler.

Cc: qemu-stable@nongnu.org
Fixes: 68fbcc344e ("hw/nvram: Introduce Xilinx eFuse QOM") # v6.2.0+
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/nvram/xlnx-efuse.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/nvram/xlnx-efuse.c b/hw/nvram/xlnx-efuse.c
index 655c40b8d1..f7b849f7de 100644
--- a/hw/nvram/xlnx-efuse.c
+++ b/hw/nvram/xlnx-efuse.c
@@ -224,6 +224,13 @@ static void efuse_realize(DeviceState *dev, Error **errp)
     }
 }
 
+static void efuse_finalize(Object *obj)
+{
+    XlnxEFuse *s = XLNX_EFUSE(obj);
+
+    g_free(s->ro_bits);
+}
+
 static void efuse_prop_set_drive(Object *obj, Visitor *v, const char *name,
                                  void *opaque, Error **errp)
 {
@@ -280,6 +287,7 @@ static const TypeInfo efuse_info = {
     .name          = TYPE_XLNX_EFUSE,
     .parent        = TYPE_DEVICE,
     .instance_size = sizeof(XlnxEFuse),
+    .instance_finalize = efuse_finalize,
     .class_init    = efuse_class_init,
 };
 
-- 
2.41.0

[PATCH-for-8.2? 5/6] hw/nvram/xlnx-efuse-ctrl: Free XlnxVersalEFuseCtrl[] "pg0-lock" array

[Philippe Mathieu-Daudé]

Commit 0be6bfac62 ("qdev: Implement variable length array properties")
added the DEFINE_PROP_ARRAY() macro with the following comment:

  * It is the responsibility of the device deinit code to free the
  * @_arrayfield memory.

Commit 9e4aa1fafe added:

  DEFINE_PROP_ARRAY("pg0-lock",
                    XlnxVersalEFuseCtrl, extra_pg0_lock_n16,
                    extra_pg0_lock_spec, qdev_prop_uint16, uint16_t),

but forgot to free the 'extra_pg0_lock_spec' array. Do it in the
instance_finalize() handler.

Cc: qemu-stable@nongnu.org
Fixes: 9e4aa1fafe ("hw/nvram: Xilinx Versal eFuse device") # v6.2.0+
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/nvram/xlnx-versal-efuse-ctrl.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/nvram/xlnx-versal-efuse-ctrl.c b/hw/nvram/xlnx-versal-efuse-ctrl.c
index beb5661c35..2480af35e1 100644
--- a/hw/nvram/xlnx-versal-efuse-ctrl.c
+++ b/hw/nvram/xlnx-versal-efuse-ctrl.c
@@ -726,6 +726,13 @@ static void efuse_ctrl_init(Object *obj)
     sysbus_init_irq(sbd, &s->irq_efuse_imr);
 }
 
+static void efuse_ctrl_finalize(Object *obj)
+{
+    XlnxVersalEFuseCtrl *s = XLNX_VERSAL_EFUSE_CTRL(obj);
+
+    g_free(s->extra_pg0_lock_spec);
+}
+
 static const VMStateDescription vmstate_efuse_ctrl = {
     .name = TYPE_XLNX_VERSAL_EFUSE_CTRL,
     .version_id = 1,
@@ -764,6 +771,7 @@ static const TypeInfo efuse_ctrl_info = {
     .instance_size = sizeof(XlnxVersalEFuseCtrl),
     .class_init    = efuse_ctrl_class_init,
     .instance_init = efuse_ctrl_init,
+    .instance_finalize = efuse_ctrl_finalize,
 };
 
 static void efuse_ctrl_register_types(void)
-- 
2.41.0

[PATCH-for-8.2 6/6] hw/input/stellaris_gamepad: Free StellarisGamepad::keycodes[] array

[Philippe Mathieu-Daudé]

Commit 0be6bfac62 ("qdev: Implement variable length array properties")
added the DEFINE_PROP_ARRAY() macro with the following comment:

  * It is the responsibility of the device deinit code to free the
  * @_arrayfield memory.

Commit a75f336b97 added:

  DEFINE_PROP_ARRAY("keycodes", StellarisGamepad, num_buttons,
                    keycodes, qdev_prop_uint32, uint32_t),

but forgot to free the 'keycodes' array. Do it in the instance_finalize
handler.

Fixes: a75f336b97 ("hw/input/stellaris_input: Convert to qdev")
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/input/stellaris_gamepad.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/input/stellaris_gamepad.c b/hw/input/stellaris_gamepad.c
index 06a0c0ce83..9dfa620e29 100644
--- a/hw/input/stellaris_gamepad.c
+++ b/hw/input/stellaris_gamepad.c
@@ -63,6 +63,13 @@ static void stellaris_gamepad_realize(DeviceState *dev, Error **errp)
     qemu_input_handler_register(dev, &stellaris_gamepad_handler);
 }
 
+static void stellaris_gamepad_finalize(Object *obj)
+{
+    StellarisGamepad *s = STELLARIS_GAMEPAD(obj);
+
+    g_free(s->keycodes);
+}
+
 static void stellaris_gamepad_reset_enter(Object *obj, ResetType type)
 {
     StellarisGamepad *s = STELLARIS_GAMEPAD(obj);
@@ -92,6 +99,7 @@ static const TypeInfo stellaris_gamepad_info[] = {
         .name = TYPE_STELLARIS_GAMEPAD,
         .parent = TYPE_SYS_BUS_DEVICE,
         .instance_size = sizeof(StellarisGamepad),
+        .instance_finalize = stellaris_gamepad_finalize,
         .class_init = stellaris_gamepad_class_init,
     },
 };
-- 
2.41.0

Re: [PATCH-for-8.2? 2/6] hw/virtio: Free VirtIOIOMMUPCI::vdev.reserved_regions[] on finalize()

[Eric Auger]

Hi Phil,

On 11/21/23 18:40, Philippe Mathieu-Daudé wrote:
> Commit 0be6bfac62 ("qdev: Implement variable length array properties")
> added the DEFINE_PROP_ARRAY() macro with the following comment:
>
>   * It is the responsibility of the device deinit code to free the
>   * @_arrayfield memory.
>
> Commit 8077b8e549 added:
>
>   DEFINE_PROP_ARRAY("reserved-regions", VirtIOIOMMUPCI,
>                     vdev.nb_reserved_regions, vdev.reserved_regions,
>                     qdev_prop_reserved_region, ReservedRegion),
>
> but forgot to free the 'vdev.reserved_regions' array. Do it in the
> instance_finalize() handler.
>
> Cc: qemu-stable@nongnu.org
> Fixes: 8077b8e549 ("virtio-iommu-pci: Add array of Interval properties") # v5.1.0+
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Eric
> ---
>  hw/virtio/virtio-iommu-pci.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/hw/virtio/virtio-iommu-pci.c b/hw/virtio/virtio-iommu-pci.c
> index 9459fbf6ed..cbdfe4c591 100644
> --- a/hw/virtio/virtio-iommu-pci.c
> +++ b/hw/virtio/virtio-iommu-pci.c
> @@ -95,10 +95,18 @@ static void virtio_iommu_pci_instance_init(Object *obj)
>                                  TYPE_VIRTIO_IOMMU);
>  }
>  
> +static void virtio_iommu_pci_instance_finalize(Object *obj)
> +{
> +    VirtIOIOMMUPCI *dev = VIRTIO_IOMMU_PCI(obj);
> +
> +    g_free(dev->vdev.prop_resv_regions);
> +}
> +
>  static const VirtioPCIDeviceTypeInfo virtio_iommu_pci_info = {
>      .generic_name  = TYPE_VIRTIO_IOMMU_PCI,
>      .instance_size = sizeof(VirtIOIOMMUPCI),
>      .instance_init = virtio_iommu_pci_instance_init,
> +    .instance_finalize = virtio_iommu_pci_instance_finalize,
>      .class_init    = virtio_iommu_pci_class_init,
>  };
>  

Re: [PATCH-for-8.2 0/6] hw: Free DEFINE_PROP_ARRAY()'s arrays in instance_finalize()

[Peter Maydell]

On Tue, 21 Nov 2023 at 17:40, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> In few places we forget to free the array allocated by the
> DEFINE_PROP_ARRAY() macro handlers. Fix that.
>
> Philippe Mathieu-Daudé (6):
>   hw/virtio: Add VirtioPCIDeviceTypeInfo::instance_finalize field
>   hw/virtio: Free VirtIOIOMMUPCI::vdev.reserved_regions[] on finalize()
>   hw/misc/mps2-scc: Free MPS2SCC::oscclk[] array on finalize()
>   hw/nvram/xlnx-efuse: Free XlnxEFuse::ro_bits[] array on finalize()
>   hw/nvram/xlnx-efuse-ctrl: Free XlnxVersalEFuseCtrl[] "pg0-lock" array
>   hw/input/stellaris_gamepad: Free StellarisGamepad::keycodes[] array

Applied to target-arm.next, thanks.

-- PMM

Re: [PATCH-for-8.2? 1/6] hw/virtio: Add VirtioPCIDeviceTypeInfo::instance_finalize field

[Michael Tokarev]

21.11.2023 20:40, Philippe Mathieu-Daudé:
> The VirtioPCIDeviceTypeInfo structure, added in commit a4ee4c8baa
> ("virtio: Helper for registering virtio device types") got extended
> in commit 8ea90ee690 ("virtio: add class_size") with the @class_size
> field. Do similarly with the @instance_finalize field.

Since other patches in this series are Cc'ed to qemu-stable, and all
uses this new field, it smells like this patch should also be picked
up for -stable (doing that now).

/mjt

> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/hw/virtio/virtio-pci.h | 1 +
>   hw/virtio/virtio-pci.c         | 1 +
>   2 files changed, 2 insertions(+)
> 
> diff --git a/include/hw/virtio/virtio-pci.h b/include/hw/virtio/virtio-pci.h
> index 5a3f182f99..59d88018c1 100644
> --- a/include/hw/virtio/virtio-pci.h
> +++ b/include/hw/virtio/virtio-pci.h
> @@ -246,6 +246,7 @@ typedef struct VirtioPCIDeviceTypeInfo {
>       size_t instance_size;
>       size_t class_size;
>       void (*instance_init)(Object *obj);
> +    void (*instance_finalize)(Object *obj);
>       void (*class_init)(ObjectClass *klass, void *data);
>       InterfaceInfo *interfaces;
>   } VirtioPCIDeviceTypeInfo;
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 205dbf24fb..e433879542 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -2391,6 +2391,7 @@ void virtio_pci_types_register(const VirtioPCIDeviceTypeInfo *t)
>           .parent        = t->parent ? t->parent : TYPE_VIRTIO_PCI,
>           .instance_size = t->instance_size,
>           .instance_init = t->instance_init,
> +        .instance_finalize = t->instance_finalize,
>           .class_size    = t->class_size,
>           .abstract      = true,
>           .interfaces    = t->interfaces,

Re: [PATCH-for-8.2? 2/6] hw/virtio: Free VirtIOIOMMUPCI::vdev.reserved_regions[] on finalize()

[Michael Tokarev]

21.11.2023 20:40, Philippe Mathieu-Daudé пишет:
> Commit 0be6bfac62 ("qdev: Implement variable length array properties")
> added the DEFINE_PROP_ARRAY() macro with the following comment:
> 
>    * It is the responsibility of the device deinit code to free the
>    * @_arrayfield memory.
> 
> Commit 8077b8e549 added:
> 
>    DEFINE_PROP_ARRAY("reserved-regions", VirtIOIOMMUPCI,
>                      vdev.nb_reserved_regions, vdev.reserved_regions,
>                      qdev_prop_reserved_region, ReservedRegion),
> 
> but forgot to free the 'vdev.reserved_regions' array. Do it in the
> instance_finalize() handler.

It is interesting that the actual code frees prop_resv_regions, not
reserved_regions as the Subject says.  This is because of commit
v8.1.0-2552-g41cc70cdf5, ""virtio-iommu: Rename reserved_regions
into prop_resv_regions".

:)

/mjt

> Cc: qemu-stable@nongnu.org
> Fixes: 8077b8e549 ("virtio-iommu-pci: Add array of Interval properties") # v5.1.0+
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/virtio/virtio-iommu-pci.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/hw/virtio/virtio-iommu-pci.c b/hw/virtio/virtio-iommu-pci.c
> index 9459fbf6ed..cbdfe4c591 100644
> --- a/hw/virtio/virtio-iommu-pci.c
> +++ b/hw/virtio/virtio-iommu-pci.c
> @@ -95,10 +95,18 @@ static void virtio_iommu_pci_instance_init(Object *obj)
>                                   TYPE_VIRTIO_IOMMU);
>   }
>   
> +static void virtio_iommu_pci_instance_finalize(Object *obj)
> +{
> +    VirtIOIOMMUPCI *dev = VIRTIO_IOMMU_PCI(obj);
> +
> +    g_free(dev->vdev.prop_resv_regions);
> +}
> +
>   static const VirtioPCIDeviceTypeInfo virtio_iommu_pci_info = {
>       .generic_name  = TYPE_VIRTIO_IOMMU_PCI,
>       .instance_size = sizeof(VirtIOIOMMUPCI),
>       .instance_init = virtio_iommu_pci_instance_init,
> +    .instance_finalize = virtio_iommu_pci_instance_finalize,
>       .class_init    = virtio_iommu_pci_class_init,
>   };
>