From: Ankit Agrawal <ankita@nvidia.com>
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 <alex@shazbot.org>
Reviewed-by: Shameer Kolothum <skolothumtho@nvidia.com>
Signed-off-by: Ankit Agrawal <ankita@nvidia.com>
---
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..177494c379 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 if (ret) {
+ return ret;
}
}
}
--
2.34.1On 2/11/26 04:06, ankita@nvidia.com wrote:
> From: Ankit Agrawal <ankita@nvidia.com>
>
> 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 <alex@shazbot.org>
> Reviewed-by: Shameer Kolothum <skolothumtho@nvidia.com>
> Signed-off-by: Ankit Agrawal <ankita@nvidia.com>
> ---
> 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..177494c379 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]",
please use PRIx64
An extra 'Error **' parameter to vfio_region_setup() could also be
considered to propagate the error to vfio_pci_realize(). It shouldn't
be too complex to add.
Thanks,
C.
> + __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 if (ret) {
> + return ret;
> }
> }
> }
> + */
> + 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]",
>
> please use PRIx64
>
>
> An extra 'Error **' parameter to vfio_region_setup() could also be
> considered to propagate the error to vfio_pci_realize(). It shouldn't
> be too complex to add.
Thanks Cedric for the comment, I've posted v3 addressing the additional
Error ** param. Please take a look.
https://lore.kernel.org/all/20260215084950.4657-1-ankita@nvidia.com/
I missed to address to use the PRIx64 in error_report. I'll consolidate that
with any additional comments that I get in that version.
Hello Ankit,
On 2/16/26 08:17, Ankit Agrawal wrote:
>
>> + */
>> + 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]",
>>
>> please use PRIx64
>>
>>
>> An extra 'Error **' parameter to vfio_region_setup() could also be
>> considered to propagate the error to vfio_pci_realize(). It shouldn't
>> be too complex to add.
>
> Thanks Cedric for the comment, I've posted v3 addressing the additional
> Error ** param. Please take a look.
> https://lore.kernel.org/all/20260215084950.4657-1-ankita@nvidia.com/
vfio_setup_region_sparse_mmaps() can take 'Error ** errp' parameter
too.
> I missed to address to use the PRIx64 in error_report. I'll consolidate that
> with any additional comments that I get in that version.
A v4 is required indeed bc of PRIx64. I will take a look.
Thanks,
C.
© 2016 - 2026 Red Hat, Inc.