1. Patchew
  2. QEMU
  3. [PATCH v3 0/2] vfio/spapr: Fix L2 crash with PCI device passthrough
  4. View patch

[PATCH v3 1/2] vfio/spapr: Enhance error handling in vfio_spapr_create_window()

Amit Machhiwal posted 2 patches 8 months, 2 weeks ago
Maintainers: Nicholas Piggin <npiggin@gmail.com>, Daniel Henrique Barboza <danielhb413@gmail.com>, Harsh Prateek Bora <harshpb@linux.ibm.com>, Alex Williamson <alex.williamson@redhat.com>, "Cédric Le Goater" <clg@redhat.com>
[PATCH v3 1/2] vfio/spapr: Enhance error handling in vfio_spapr_create_window()
Posted by Amit Machhiwal 8 months, 2 weeks ago 
Introduce an Error ** parameter to vfio_spapr_create_window() to enable
structured error reporting. This allows the function to propagate
detailed errors back to callers.

Suggested-by: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Amit Machhiwal <amachhiw@linux.ibm.com>
---
 hw/vfio/spapr.c | 33 ++++++++++++++++-----------------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c
index 1a5d1611f2cd..dd9207679dbe 100644
--- a/hw/vfio/spapr.c
+++ b/hw/vfio/spapr.c
@@ -230,9 +230,9 @@ static int vfio_spapr_remove_window(VFIOContainer *container,
     return 0;
 }
 
-static int vfio_spapr_create_window(VFIOContainer *container,
+static bool vfio_spapr_create_window(VFIOContainer *container,
                                     MemoryRegionSection *section,
-                                    hwaddr *pgsize)
+                                    hwaddr *pgsize, Error **errp)
 {
     int ret = 0;
     VFIOContainerBase *bcontainer = &container->bcontainer;
@@ -252,11 +252,11 @@ static int vfio_spapr_create_window(VFIOContainer *container,
     pgmask = bcontainer->pgsizes & (pagesize | (pagesize - 1));
     pagesize = pgmask ? (1ULL << (63 - clz64(pgmask))) : 0;
     if (!pagesize) {
-        error_report("Host doesn't support page size 0x%"PRIx64
-                     ", the supported mask is 0x%lx",
-                     memory_region_iommu_get_min_page_size(iommu_mr),
-                     bcontainer->pgsizes);
-        return -EINVAL;
+        error_setg_errno(errp, EINVAL, "Host doesn't support page size 0x%"PRIx64
+                         ", the supported mask is 0x%lx",
+                         memory_region_iommu_get_min_page_size(iommu_mr),
+                         bcontainer->pgsizes);
+        return false;
     }
 
     /*
@@ -302,17 +302,17 @@ static int vfio_spapr_create_window(VFIOContainer *container,
         }
     }
     if (ret) {
-        error_report("Failed to create a window, ret = %d (%m)", ret);
-        return -errno;
+        error_setg_errno(errp, errno, "Failed to create a window, ret = %d", ret);
+        return false;
     }
 
     if (create.start_addr != section->offset_within_address_space) {
         vfio_spapr_remove_window(container, create.start_addr);
 
-        error_report("Host doesn't support DMA window at %"HWADDR_PRIx", must be %"PRIx64,
-                     section->offset_within_address_space,
-                     (uint64_t)create.start_addr);
-        return -EINVAL;
+        error_setg_errno(errp, EINVAL, "Host doesn't support DMA window at %"HWADDR_PRIx
+                         ", must be %"PRIx64, section->offset_within_address_space,
+                         (uint64_t)create.start_addr);
+        return false;
     }
     trace_vfio_spapr_create_window(create.page_shift,
                                    create.levels,
@@ -320,7 +320,7 @@ static int vfio_spapr_create_window(VFIOContainer *container,
                                    create.start_addr);
     *pgsize = pagesize;
 
-    return 0;
+    return true;
 }
 
 static bool
@@ -377,9 +377,8 @@ vfio_spapr_container_add_section_window(VFIOContainerBase *bcontainer,
         }
     }
 
-    ret = vfio_spapr_create_window(container, section, &pgsize);
-    if (ret) {
-        error_setg_errno(errp, -ret, "Failed to create SPAPR window");
+    ret = vfio_spapr_create_window(container, section, &pgsize, errp);
+    if (!ret) {
         return false;
     }
 
-- 
2.49.0
Re: [PATCH v3 1/2] vfio/spapr: Enhance error handling in vfio_spapr_create_window()
Posted by Cédric Le Goater 8 months, 2 weeks ago 
On 4/8/25 14:40, Amit Machhiwal wrote:
> Introduce an Error ** parameter to vfio_spapr_create_window() to enable
> structured error reporting. This allows the function to propagate
> detailed errors back to callers.
> 
> Suggested-by: Cédric Le Goater <clg@redhat.com>
> Signed-off-by: Amit Machhiwal <amachhiw@linux.ibm.com>
> --->   hw/vfio/spapr.c | 33 ++++++++++++++++-----------------
>   1 file changed, 16 insertions(+), 17 deletions(-)
> 
> diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c
> index 1a5d1611f2cd..dd9207679dbe 100644
> --- a/hw/vfio/spapr.c
> +++ b/hw/vfio/spapr.c
> @@ -230,9 +230,9 @@ static int vfio_spapr_remove_window(VFIOContainer *container,
>       return 0;
>   }
>   
> -static int vfio_spapr_create_window(VFIOContainer *container,
> +static bool vfio_spapr_create_window(VFIOContainer *container,
>                                       MemoryRegionSection *section,
> -                                    hwaddr *pgsize)
> +                                    hwaddr *pgsize, Error **errp)
>   {
>       int ret = 0;
>       VFIOContainerBase *bcontainer = &container->bcontainer;
> @@ -252,11 +252,11 @@ static int vfio_spapr_create_window(VFIOContainer *container,
>       pgmask = bcontainer->pgsizes & (pagesize | (pagesize - 1));
>       pagesize = pgmask ? (1ULL << (63 - clz64(pgmask))) : 0;
>       if (!pagesize) {
> -        error_report("Host doesn't support page size 0x%"PRIx64
> -                     ", the supported mask is 0x%lx",
> -                     memory_region_iommu_get_min_page_size(iommu_mr),
> -                     bcontainer->pgsizes);
> -        return -EINVAL;
> +        error_setg_errno(errp, EINVAL, "Host doesn't support page size 0x%"PRIx64
> +                         ", the supported mask is 0x%lx",
> +                         memory_region_iommu_get_min_page_size(iommu_mr),
> +                         bcontainer->pgsizes);
> +        return false;
>       }
>   
>       /*
> @@ -302,17 +302,17 @@ static int vfio_spapr_create_window(VFIOContainer *container,
>           }
>       }
>       if (ret) {
> -        error_report("Failed to create a window, ret = %d (%m)", ret);
> -        return -errno;
> +        error_setg_errno(errp, errno, "Failed to create a window, ret = %d", ret);
> +        return false;
>       }
>   
>       if (create.start_addr != section->offset_within_address_space) {
>           vfio_spapr_remove_window(container, create.start_addr);
>   
> -        error_report("Host doesn't support DMA window at %"HWADDR_PRIx", must be %"PRIx64,
> -                     section->offset_within_address_space,
> -                     (uint64_t)create.start_addr);
> -        return -EINVAL;
> +        error_setg_errno(errp, EINVAL, "Host doesn't support DMA window at %"HWADDR_PRIx
> +                         ", must be %"PRIx64, section->offset_within_address_space,
> +                         (uint64_t)create.start_addr);
> +        return false;
>       }
>       trace_vfio_spapr_create_window(create.page_shift,
>                                      create.levels,
> @@ -320,7 +320,7 @@ static int vfio_spapr_create_window(VFIOContainer *container,
>                                      create.start_addr);
>       *pgsize = pagesize;
>   
> -    return 0;
> +    return true;
>   }
>   
>   static bool
> @@ -377,9 +377,8 @@ vfio_spapr_container_add_section_window(VFIOContainerBase *bcontainer,
>           }
>       }
>   
> -    ret = vfio_spapr_create_window(container, section, &pgsize);
> -    if (ret) {
> -        error_setg_errno(errp, -ret, "Failed to create SPAPR window");
> +    ret = vfio_spapr_create_window(container, section, &pgsize, errp);
> +    if (!ret) {
>           return false;
>       }
>   

ret is not needed. Minor.


Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.