On Fri, 30 Jan 2026 09:33:44 +0000 Shameer Kolothum Thodi <[email protected]> wrote:
> > -----Original Message----- > > From: Ankit Agrawal <[email protected]> > > Sent: 30 January 2026 04:07 > > To: Ankit Agrawal <[email protected]>; Vikram Sethi <[email protected]>; > > Jason Gunthorpe <[email protected]>; Shameer Kolothum Thodi > > <[email protected]>; [email protected]; [email protected] > > Cc: Aniket Agashe <[email protected]>; Neo Jia <[email protected]>; Kirti > > Wankhede <[email protected]>; Tarun Gupta (SW-GPU) > > <[email protected]>; Zhi Wang <[email protected]>; Matt Ochs > > <[email protected]>; Krishnakant Jaju <[email protected]>; qemu- > > [email protected] > > Subject: [PATCH v1 1/2] hw/vfio: sort and validate sparse mmap regions by > > offset > > > > From: Ankit Agrawal <[email protected]> > > > > Sort sparse mmap regions by offset during region setup to ensure > > predictable mapping order, avoid overlaps and a proper handling > > of the gaps between sub-regions. > > > > Add validation to detect overlapping sparse regions early during > > setup before any mapping operations begin. > > > > The sorting is performed on the subregions ranges during > > vfio_setup_region_sparse_mmaps(). This also ensures that subsequent > > mapping code can rely on subregions being in ascending offset order. > > > > This is preparatory work for alignment adjustments needed to support > > hugepfnmap on systems where device memory (e.g., Grace-based systems) > > may have non-power-of-2 sizes. > > > > cc: Alex Williamson <[email protected]> > > Signed-off-by: Ankit Agrawal <[email protected]> > > --- > > hw/vfio/region.c | 45 ++++++++++++++++++++++++++++++++++++++++++++- > > 1 file changed, 44 insertions(+), 1 deletion(-) > > > > diff --git a/hw/vfio/region.c b/hw/vfio/region.c > > index ab39d77574..7622ae5683 100644 > > --- a/hw/vfio/region.c > > +++ b/hw/vfio/region.c > > @@ -149,6 +149,19 @@ static const MemoryRegionOps vfio_region_ops = { > > }, > > }; > > > > +static int vfio_mmap_compare_offset(const void *a, const void *b) > > +{ > > + const VFIOMmap *mmap_a = a; > > + const VFIOMmap *mmap_b = b; > > + > > + if (mmap_a->offset < mmap_b->offset) { > > + return -1; > > + } else if (mmap_a->offset > mmap_b->offset) { > > + return 1; > > + } > > + return 0; > > +} > > + > > static int vfio_setup_region_sparse_mmaps(VFIORegion *region, > > struct vfio_region_info *info) > > { > > @@ -182,6 +195,34 @@ static int > > vfio_setup_region_sparse_mmaps(VFIORegion *region, > > region->nr_mmaps = j; > > region->mmaps = g_realloc(region->mmaps, j * sizeof(VFIOMmap)); > > > > + /* > > + * Sort sparse mmaps by offset to ensure proper handling of gaps > > + * and predictable mapping order in vfio_region_mmap(). > > + */ > > + if (region->nr_mmaps > 1) { > > + qsort(region->mmaps, region->nr_mmaps, sizeof(VFIOMmap), > > + vfio_mmap_compare_offset); > > + > > + /* > > + * Validate that sparse regions dont overlap after sorting. > > + */ > > + for (i = 1; i < region->nr_mmaps; i++) { > > + off_t prev_end = region->mmaps[i - 1].offset + > > + region->mmaps[i - 1].size; > > + if (prev_end > region->mmaps[i].offset) { > > + error_report("%s: overlapping sparse mmap regions detected > > " > > + "in region %d: [0x%lx-0x%lx] overlaps with > > [0x%lx-0x%lx]", > > + __func__, region->nr, region->mmaps[i - > > 1].offset, > > + prev_end - 1, region->mmaps[i].offset, > > + region->mmaps[i].offset + > > region->mmaps[i].size - 1); > > + g_free(region->mmaps); > > + region->mmaps = NULL; > > + region->nr_mmaps = 0; > > + return -EINVAL; > > + } > > + } > > + } > > + > > return 0; > > } > > > > @@ -213,11 +254,13 @@ int vfio_region_setup(Object *obj, VFIODevice > > *vbasedev, VFIORegion *region, > > > > ret = vfio_setup_region_sparse_mmaps(region, info); > > > > - if (ret) { > > + if (ret == -ENODEV) { > > region->nr_mmaps = 1; > > region->mmaps = g_new0(VFIOMmap, region->nr_mmaps); > > region->mmaps[0].offset = 0; > > region->mmaps[0].size = region->size; > > + } else { > > + return ret; > > } > > Is this now returns without the trace_xxx , right? > > With that fixed: > Reviewed-by: Shameer Kolothum <[email protected]> Right, I think this was probably meant to be 'else if (ret)' so the newly added -EINVAL generates an error w/o trace while the non-error path should fall through. Thanks, Alex
