| Message ID | 20250408124042.2695955-2-amachhiw@linux.ibm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | vfio/spapr: Fix L2 crash with PCI device passthrough | expand |
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.
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; }
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(-)