[PATCH 0/3] vfio: memory leak fix and code cleanup

[PATCH 0/3] vfio: memory leak fix and code cleanup

[Zhenzhong Duan]

Hi,

This trivial patchset fixes a incremental memory leak in rare case,
and some cleanup on ap/ccw.

This patchset is based on vfio-next.

Thanks
Zhenzhong

Zhenzhong Duan (3):
  vfio/pci: Fix a potential memory leak in vfio_listener_region_add
  vfio/ap: Remove pointless apdev variable
  vfio/ccw: Remove redundant definition of TYPE_VFIO_CCW

 hw/vfio/ap.c                | 9 +++------
 hw/vfio/common.c            | 2 +-
 include/hw/s390x/vfio-ccw.h | 2 --
 3 files changed, 4 insertions(+), 9 deletions(-)

-- 
2.34.1

[PATCH 1/3] vfio/pci: Fix a potential memory leak in vfio_listener_region_add

[Zhenzhong Duan]

When there is an failure in vfio_listener_region_add() and the section
belongs to a ram device, there is an inaccurate error report which should
never be related to vfio_dma_map failure. The memory holding err is also
incrementally leaked in each failure.

Fix it by reporting the real error and free it.

Fixes: 567b5b309ab ("vfio/pci: Relax DMA map errors for MMIO regions")
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 hw/vfio/common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 9e61de03ee..5ff5acf1d8 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -763,7 +763,7 @@ static void vfio_listener_region_add(MemoryListener *listener,
 
 fail:
     if (memory_region_is_ram_device(section->mr)) {
-        error_report("failed to vfio_dma_map. pci p2p may not work");
+        error_reportf_err(err, "PCI p2p may not work: ");
         return;
     }
     /*
-- 
2.34.1

[PATCH 2/3] vfio/ap: Remove pointless apdev variable

[Zhenzhong Duan]

No need to double-cast, call VFIO_AP_DEVICE() on DeviceState.

No functional changes.

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 hw/vfio/ap.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
index 22e564f4f7..e083a19eac 100644
--- a/hw/vfio/ap.c
+++ b/hw/vfio/ap.c
@@ -156,8 +156,7 @@ static void vfio_ap_realize(DeviceState *dev, Error **errp)
 {
     int ret;
     Error *err = NULL;
-    APDevice *apdev = AP_DEVICE(dev);
-    VFIOAPDevice *vapdev = VFIO_AP_DEVICE(apdev);
+    VFIOAPDevice *vapdev = VFIO_AP_DEVICE(dev);
     VFIODevice *vbasedev = &vapdev->vdev;
 
     vbasedev->name = g_path_get_basename(vbasedev->sysfsdev);
@@ -195,8 +194,7 @@ error:
 
 static void vfio_ap_unrealize(DeviceState *dev)
 {
-    APDevice *apdev = AP_DEVICE(dev);
-    VFIOAPDevice *vapdev = VFIO_AP_DEVICE(apdev);
+    VFIOAPDevice *vapdev = VFIO_AP_DEVICE(dev);
 
     vfio_ap_unregister_irq_notifier(vapdev, VFIO_AP_REQ_IRQ_INDEX);
     vfio_detach_device(&vapdev->vdev);
@@ -211,8 +209,7 @@ static Property vfio_ap_properties[] = {
 static void vfio_ap_reset(DeviceState *dev)
 {
     int ret;
-    APDevice *apdev = AP_DEVICE(dev);
-    VFIOAPDevice *vapdev = VFIO_AP_DEVICE(apdev);
+    VFIOAPDevice *vapdev = VFIO_AP_DEVICE(dev);
 
     ret = ioctl(vapdev->vdev.fd, VFIO_DEVICE_RESET);
     if (ret) {
-- 
2.34.1

[PATCH 3/3] vfio/ccw: Remove redundant definition of TYPE_VFIO_CCW

[Zhenzhong Duan]

No functional changes.

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 include/hw/s390x/vfio-ccw.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/include/hw/s390x/vfio-ccw.h b/include/hw/s390x/vfio-ccw.h
index 63a909eb7e..4209d27657 100644
--- a/include/hw/s390x/vfio-ccw.h
+++ b/include/hw/s390x/vfio-ccw.h
@@ -22,6 +22,4 @@
 #define TYPE_VFIO_CCW "vfio-ccw"
 OBJECT_DECLARE_SIMPLE_TYPE(VFIOCCWDevice, VFIO_CCW)
 
-#define TYPE_VFIO_CCW "vfio-ccw"
-
 #endif
-- 
2.34.1

Re: [PATCH 2/3] vfio/ap: Remove pointless apdev variable

[Philippe Mathieu-Daudé]

On 9/10/23 04:20, Zhenzhong Duan wrote:
> No need to double-cast, call VFIO_AP_DEVICE() on DeviceState.
> 
> No functional changes.
> 
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>   hw/vfio/ap.c | 9 +++------
>   1 file changed, 3 insertions(+), 6 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 3/3] vfio/ccw: Remove redundant definition of TYPE_VFIO_CCW

[Philippe Mathieu-Daudé]

On 9/10/23 04:20, Zhenzhong Duan wrote:
> No functional changes.
> 
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>   include/hw/s390x/vfio-ccw.h | 2 --
>   1 file changed, 2 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 0/3] vfio: memory leak fix and code cleanup

[Cédric Le Goater]

On 10/9/23 04:20, Zhenzhong Duan wrote:
> Hi,
> 
> This trivial patchset fixes a incremental memory leak in rare case,
> and some cleanup on ap/ccw.
> 
> This patchset is based on vfio-next.

Will this apply on the future v5 of Eric ?

Thanks,

C.


> 
> Thanks
> Zhenzhong
> 
> Zhenzhong Duan (3):
>    vfio/pci: Fix a potential memory leak in vfio_listener_region_add
>    vfio/ap: Remove pointless apdev variable
>    vfio/ccw: Remove redundant definition of TYPE_VFIO_CCW
> 
>   hw/vfio/ap.c                | 9 +++------
>   hw/vfio/common.c            | 2 +-
>   include/hw/s390x/vfio-ccw.h | 2 --
>   3 files changed, 4 insertions(+), 9 deletions(-)
> 

RE: [PATCH 0/3] vfio: memory leak fix and code cleanup

[Duan, Zhenzhong]

>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Sent: Monday, October 9, 2023 2:32 PM
>Subject: Re: [PATCH 0/3] vfio: memory leak fix and code cleanup
>
>On 10/9/23 04:20, Zhenzhong Duan wrote:
>> Hi,
>>
>> This trivial patchset fixes a incremental memory leak in rare case,
>> and some cleanup on ap/ccw.
>>
>> This patchset is based on vfio-next.
>
>Will this apply on the future v5 of Eric ?

I think so, though this patchset is applied on v4 of Eric. Let me know if
you want a new version without basing on the prerequisite patchset,
it’s easy to rebase.

Thanks
Zhenzhong

Re: [PATCH 0/3] vfio: memory leak fix and code cleanup

[Cédric Le Goater]

On 10/9/23 04:20, Zhenzhong Duan wrote:
> Hi,
> 
> This trivial patchset fixes a incremental memory leak in rare case,
> and some cleanup on ap/ccw.
> 
> This patchset is based on vfio-next.

Applied on vfio-next.

Thanks,

C.

Re: [PATCH 2/3] vfio/ap: Remove pointless apdev variable

[Tony Krowiak]

Reviewed-by: Tony Krowiak <akrowiak@linux.ibm.com>

On 10/8/23 22:20, Zhenzhong Duan wrote:
> No need to double-cast, call VFIO_AP_DEVICE() on DeviceState.
> 
> No functional changes.
> 
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>   hw/vfio/ap.c | 9 +++------
>   1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
> index 22e564f4f7..e083a19eac 100644
> --- a/hw/vfio/ap.c
> +++ b/hw/vfio/ap.c
> @@ -156,8 +156,7 @@ static void vfio_ap_realize(DeviceState *dev, Error **errp)
>   {
>       int ret;
>       Error *err = NULL;
> -    APDevice *apdev = AP_DEVICE(dev);
> -    VFIOAPDevice *vapdev = VFIO_AP_DEVICE(apdev);
> +    VFIOAPDevice *vapdev = VFIO_AP_DEVICE(dev);
>       VFIODevice *vbasedev = &vapdev->vdev;
>   
>       vbasedev->name = g_path_get_basename(vbasedev->sysfsdev);
> @@ -195,8 +194,7 @@ error:
>   
>   static void vfio_ap_unrealize(DeviceState *dev)
>   {
> -    APDevice *apdev = AP_DEVICE(dev);
> -    VFIOAPDevice *vapdev = VFIO_AP_DEVICE(apdev);
> +    VFIOAPDevice *vapdev = VFIO_AP_DEVICE(dev);
>   
>       vfio_ap_unregister_irq_notifier(vapdev, VFIO_AP_REQ_IRQ_INDEX);
>       vfio_detach_device(&vapdev->vdev);
> @@ -211,8 +209,7 @@ static Property vfio_ap_properties[] = {
>   static void vfio_ap_reset(DeviceState *dev)
>   {
>       int ret;
> -    APDevice *apdev = AP_DEVICE(dev);
> -    VFIOAPDevice *vapdev = VFIO_AP_DEVICE(apdev);
> +    VFIOAPDevice *vapdev = VFIO_AP_DEVICE(dev);
>   
>       ret = ioctl(vapdev->vdev.fd, VFIO_DEVICE_RESET);
>       if (ret) {

Re: [PATCH 3/3] vfio/ccw: Remove redundant definition of TYPE_VFIO_CCW

[Eric Farman]

On Mon, 2023-10-09 at 10:20 +0800, Zhenzhong Duan wrote:
> No functional changes.
> 
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>

I see Cedric has already queued this, but FWIW:

Reviewed-by: Eric Farman <farman@linux.ibm.com>

> ---
>  include/hw/s390x/vfio-ccw.h | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/include/hw/s390x/vfio-ccw.h b/include/hw/s390x/vfio-
> ccw.h
> index 63a909eb7e..4209d27657 100644
> --- a/include/hw/s390x/vfio-ccw.h
> +++ b/include/hw/s390x/vfio-ccw.h
> @@ -22,6 +22,4 @@
>  #define TYPE_VFIO_CCW "vfio-ccw"
>  OBJECT_DECLARE_SIMPLE_TYPE(VFIOCCWDevice, VFIO_CCW)
>  
> -#define TYPE_VFIO_CCW "vfio-ccw"
> -
>  #endif

Re: [PATCH 3/3] vfio/ccw: Remove redundant definition of TYPE_VFIO_CCW

[Cédric Le Goater]

On 10/9/23 20:58, Eric Farman wrote:
> On Mon, 2023-10-09 at 10:20 +0800, Zhenzhong Duan wrote:
>> No functional changes.
>>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> 
> I see Cedric has already queued this, but FWIW:
> 
> Reviewed-by: Eric Farman <farman@linux.ibm.com>

I will take the new tags when I rebase.

Thanks,

C.


> 
>> ---
>>   include/hw/s390x/vfio-ccw.h | 2 --
>>   1 file changed, 2 deletions(-)
>>
>> diff --git a/include/hw/s390x/vfio-ccw.h b/include/hw/s390x/vfio-
>> ccw.h
>> index 63a909eb7e..4209d27657 100644
>> --- a/include/hw/s390x/vfio-ccw.h
>> +++ b/include/hw/s390x/vfio-ccw.h
>> @@ -22,6 +22,4 @@
>>   #define TYPE_VFIO_CCW "vfio-ccw"
>>   OBJECT_DECLARE_SIMPLE_TYPE(VFIOCCWDevice, VFIO_CCW)
>>   
>> -#define TYPE_VFIO_CCW "vfio-ccw"
>> -
>>   #endif
>