[PATCH 0/2] Fixes for vfio region cache

[PATCH 0/2] Fixes for vfio region cache

[John Levon]

John Levon (2):
  vfio: rename field to "num_initial_regions"
  vfio: only check region info cache for initial regions

 include/hw/vfio/vfio-device.h |  2 +-
 hw/vfio-user/device.c         |  2 +-
 hw/vfio/ccw.c                 |  4 ++--
 hw/vfio/device.c              | 39 ++++++++++++++++++++++-------------
 hw/vfio/iommufd.c             |  3 ++-
 hw/vfio/pci.c                 |  4 ++--
 6 files changed, 33 insertions(+), 21 deletions(-)

-- 
2.43.0

[PATCH 1/2] vfio: rename field to "num_initial_regions"

[John Levon]

We set VFIODevice::num_regions at initialization time, and do not
otherwise refresh it. As it is valid in theory for a VFIO device to
later increase the number of supported regions, rename the field to
"num_initial_regions" to better reflect its semantics.

Signed-off-by: John Levon <john.levon@nutanix.com>
---
 include/hw/vfio/vfio-device.h |  2 +-
 hw/vfio-user/device.c         |  2 +-
 hw/vfio/ccw.c                 |  4 ++--
 hw/vfio/device.c              | 12 ++++++------
 hw/vfio/iommufd.c             |  3 ++-
 hw/vfio/pci.c                 |  4 ++--
 6 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/include/hw/vfio/vfio-device.h b/include/hw/vfio/vfio-device.h
index 7e9aed6d3c..0fe6c60ba2 100644
--- a/include/hw/vfio/vfio-device.h
+++ b/include/hw/vfio/vfio-device.h
@@ -74,7 +74,7 @@ typedef struct VFIODevice {
     VFIODeviceOps *ops;
     VFIODeviceIOOps *io_ops;
     unsigned int num_irqs;
-    unsigned int num_regions;
+    unsigned int num_initial_regions;
     unsigned int flags;
     VFIOMigration *migration;
     Error *migration_blocker;
diff --git a/hw/vfio-user/device.c b/hw/vfio-user/device.c
index 0609a7dc25..64ef35b320 100644
--- a/hw/vfio-user/device.c
+++ b/hw/vfio-user/device.c
@@ -134,7 +134,7 @@ static int vfio_user_device_io_get_region_info(VFIODevice *vbasedev,
     VFIOUserFDs fds = { 0, 1, fd};
     int ret;
 
-    if (info->index > vbasedev->num_regions) {
+    if (info->index > vbasedev->num_initial_regions) {
         return -EINVAL;
     }
 
diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
index 9560b8d851..4d9588e7aa 100644
--- a/hw/vfio/ccw.c
+++ b/hw/vfio/ccw.c
@@ -484,9 +484,9 @@ static bool vfio_ccw_get_region(VFIOCCWDevice *vcdev, Error **errp)
      * We always expect at least the I/O region to be present. We also
      * may have a variable number of regions governed by capabilities.
      */
-    if (vdev->num_regions < VFIO_CCW_CONFIG_REGION_INDEX + 1) {
+    if (vdev->num_initial_regions < VFIO_CCW_CONFIG_REGION_INDEX + 1) {
         error_setg(errp, "vfio: too few regions (%u), expected at least %u",
-                   vdev->num_regions, VFIO_CCW_CONFIG_REGION_INDEX + 1);
+                   vdev->num_initial_regions, VFIO_CCW_CONFIG_REGION_INDEX + 1);
         return false;
     }
 
diff --git a/hw/vfio/device.c b/hw/vfio/device.c
index 64f8750389..52079f4cf5 100644
--- a/hw/vfio/device.c
+++ b/hw/vfio/device.c
@@ -257,7 +257,7 @@ int vfio_device_get_region_info_type(VFIODevice *vbasedev, uint32_t type,
 {
     int i;
 
-    for (i = 0; i < vbasedev->num_regions; i++) {
+    for (i = 0; i < vbasedev->num_initial_regions; i++) {
         struct vfio_info_cap_header *hdr;
         struct vfio_region_info_cap_type *cap_type;
 
@@ -466,7 +466,7 @@ void vfio_device_prepare(VFIODevice *vbasedev, VFIOContainer *bcontainer,
     int i;
 
     vbasedev->num_irqs = info->num_irqs;
-    vbasedev->num_regions = info->num_regions;
+    vbasedev->num_initial_regions = info->num_regions;
     vbasedev->flags = info->flags;
     vbasedev->reset_works = !!(info->flags & VFIO_DEVICE_FLAGS_RESET);
 
@@ -476,10 +476,10 @@ void vfio_device_prepare(VFIODevice *vbasedev, VFIOContainer *bcontainer,
     QLIST_INSERT_HEAD(&vfio_device_list, vbasedev, global_next);
 
     vbasedev->reginfo = g_new0(struct vfio_region_info *,
-                               vbasedev->num_regions);
+                               vbasedev->num_initial_regions);
     if (vbasedev->use_region_fds) {
-        vbasedev->region_fds = g_new0(int, vbasedev->num_regions);
-        for (i = 0; i < vbasedev->num_regions; i++) {
+        vbasedev->region_fds = g_new0(int, vbasedev->num_initial_regions);
+        for (i = 0; i < vbasedev->num_initial_regions; i++) {
             vbasedev->region_fds[i] = -1;
         }
     }
@@ -489,7 +489,7 @@ void vfio_device_unprepare(VFIODevice *vbasedev)
 {
     int i;
 
-    for (i = 0; i < vbasedev->num_regions; i++) {
+    for (i = 0; i < vbasedev->num_initial_regions; i++) {
         g_free(vbasedev->reginfo[i]);
         if (vbasedev->region_fds != NULL && vbasedev->region_fds[i] != -1) {
             close(vbasedev->region_fds[i]);
diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
index 68470d552e..10fc065d20 100644
--- a/hw/vfio/iommufd.c
+++ b/hw/vfio/iommufd.c
@@ -663,7 +663,8 @@ found_container:
     vfio_iommufd_cpr_register_device(vbasedev);
 
     trace_iommufd_cdev_device_info(vbasedev->name, devfd, vbasedev->num_irqs,
-                                   vbasedev->num_regions, vbasedev->flags);
+                                   vbasedev->num_initial_regions,
+                                   vbasedev->flags);
     return true;
 
 err_listener_register:
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 06b06afc2b..8b8bc5a421 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2975,9 +2975,9 @@ bool vfio_pci_populate_device(VFIOPCIDevice *vdev, Error **errp)
         return false;
     }
 
-    if (vbasedev->num_regions < VFIO_PCI_CONFIG_REGION_INDEX + 1) {
+    if (vbasedev->num_initial_regions < VFIO_PCI_CONFIG_REGION_INDEX + 1) {
         error_setg(errp, "unexpected number of io regions %u",
-                   vbasedev->num_regions);
+                   vbasedev->num_initial_regions);
         return false;
     }
 
-- 
2.43.0

[PATCH 2/2] vfio: only check region info cache for initial regions

[John Levon]

It is semantically valid for a VFIO device to increase the number of
regions after initialization. In this case, we'd attempt to check for
cached region info past the size of the ->reginfo array. Check for the
region index and skip the cache in these cases.

This also works around some VGPU use cases which appear to be a bug,
where VFIO_DEVICE_QUERY_GFX_PLANE returns a region index beyond the
reported ->num_regions.

Fixes: 95cdb024 ("vfio: add region info cache")
Signed-off-by: John Levon <john.levon@nutanix.com>
---
 hw/vfio/device.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/hw/vfio/device.c b/hw/vfio/device.c
index 52079f4cf5..8b63e765ac 100644
--- a/hw/vfio/device.c
+++ b/hw/vfio/device.c
@@ -205,10 +205,19 @@ int vfio_device_get_region_info(VFIODevice *vbasedev, int index,
     int fd = -1;
     int ret;
 
-    /* check cache */
-    if (vbasedev->reginfo[index] != NULL) {
-        *info = vbasedev->reginfo[index];
-        return 0;
+    /*
+     * We only set up the region info cache for the initial number of regions.
+     *
+     * Since a VFIO device may later increase the number of regions then use
+     * such regions with an index past ->num_initial_regions, don't attempt to
+     * use the info cache in those cases.
+     */
+    if (index < vbasedev->num_initial_regions) {
+        /* check cache */
+        if (vbasedev->reginfo[index] != NULL) {
+            *info = vbasedev->reginfo[index];
+            return 0;
+        }
     }
 
     *info = g_malloc0(argsz);
@@ -236,10 +245,12 @@ retry:
         goto retry;
     }
 
-    /* fill cache */
-    vbasedev->reginfo[index] = *info;
-    if (vbasedev->region_fds != NULL) {
-        vbasedev->region_fds[index] = fd;
+    if (index < vbasedev->num_initial_regions) {
+        /* fill cache */
+        vbasedev->reginfo[index] = *info;
+        if (vbasedev->region_fds != NULL) {
+            vbasedev->region_fds[index] = fd;
+        }
     }
 
     return 0;
-- 
2.43.0

Re: [PATCH 0/2] Fixes for vfio region cache

[Alex Williamson]

On Tue, 14 Oct 2025 17:12:25 +0200
John Levon <john.levon@nutanix.com> wrote:

> John Levon (2):
>   vfio: rename field to "num_initial_regions"
>   vfio: only check region info cache for initial regions
> 
>  include/hw/vfio/vfio-device.h |  2 +-
>  hw/vfio-user/device.c         |  2 +-
>  hw/vfio/ccw.c                 |  4 ++--
>  hw/vfio/device.c              | 39 ++++++++++++++++++++++-------------
>  hw/vfio/iommufd.c             |  3 ++-
>  hw/vfio/pci.c                 |  4 ++--
>  6 files changed, 33 insertions(+), 21 deletions(-)
> 

Reviewed-by: Alex Williamson <alex@shazbot.org>

Re: [PATCH 1/2] vfio: rename field to "num_initial_regions"

[Cédric Le Goater]

On 10/14/25 17:12, John Levon wrote:
> We set VFIODevice::num_regions at initialization time, and do not
> otherwise refresh it. As it is valid in theory for a VFIO device to
> later increase the number of supported regions, rename the field to
> "num_initial_regions" to better reflect its semantics.
> 
> Signed-off-by: John Levon <john.levon@nutanix.com>


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

Thanks,

C.


> ---
>   include/hw/vfio/vfio-device.h |  2 +-
>   hw/vfio-user/device.c         |  2 +-
>   hw/vfio/ccw.c                 |  4 ++--
>   hw/vfio/device.c              | 12 ++++++------
>   hw/vfio/iommufd.c             |  3 ++-
>   hw/vfio/pci.c                 |  4 ++--
>   6 files changed, 14 insertions(+), 13 deletions(-)
> 
> diff --git a/include/hw/vfio/vfio-device.h b/include/hw/vfio/vfio-device.h
> index 7e9aed6d3c..0fe6c60ba2 100644
> --- a/include/hw/vfio/vfio-device.h
> +++ b/include/hw/vfio/vfio-device.h
> @@ -74,7 +74,7 @@ typedef struct VFIODevice {
>       VFIODeviceOps *ops;
>       VFIODeviceIOOps *io_ops;
>       unsigned int num_irqs;
> -    unsigned int num_regions;
> +    unsigned int num_initial_regions;
>       unsigned int flags;
>       VFIOMigration *migration;
>       Error *migration_blocker;
> diff --git a/hw/vfio-user/device.c b/hw/vfio-user/device.c
> index 0609a7dc25..64ef35b320 100644
> --- a/hw/vfio-user/device.c
> +++ b/hw/vfio-user/device.c
> @@ -134,7 +134,7 @@ static int vfio_user_device_io_get_region_info(VFIODevice *vbasedev,
>       VFIOUserFDs fds = { 0, 1, fd};
>       int ret;
>   
> -    if (info->index > vbasedev->num_regions) {
> +    if (info->index > vbasedev->num_initial_regions) {
>           return -EINVAL;
>       }
>   
> diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
> index 9560b8d851..4d9588e7aa 100644
> --- a/hw/vfio/ccw.c
> +++ b/hw/vfio/ccw.c
> @@ -484,9 +484,9 @@ static bool vfio_ccw_get_region(VFIOCCWDevice *vcdev, Error **errp)
>        * We always expect at least the I/O region to be present. We also
>        * may have a variable number of regions governed by capabilities.
>        */
> -    if (vdev->num_regions < VFIO_CCW_CONFIG_REGION_INDEX + 1) {
> +    if (vdev->num_initial_regions < VFIO_CCW_CONFIG_REGION_INDEX + 1) {
>           error_setg(errp, "vfio: too few regions (%u), expected at least %u",
> -                   vdev->num_regions, VFIO_CCW_CONFIG_REGION_INDEX + 1);
> +                   vdev->num_initial_regions, VFIO_CCW_CONFIG_REGION_INDEX + 1);
>           return false;
>       }
>   
> diff --git a/hw/vfio/device.c b/hw/vfio/device.c
> index 64f8750389..52079f4cf5 100644
> --- a/hw/vfio/device.c
> +++ b/hw/vfio/device.c
> @@ -257,7 +257,7 @@ int vfio_device_get_region_info_type(VFIODevice *vbasedev, uint32_t type,
>   {
>       int i;
>   
> -    for (i = 0; i < vbasedev->num_regions; i++) {
> +    for (i = 0; i < vbasedev->num_initial_regions; i++) {
>           struct vfio_info_cap_header *hdr;
>           struct vfio_region_info_cap_type *cap_type;
>   
> @@ -466,7 +466,7 @@ void vfio_device_prepare(VFIODevice *vbasedev, VFIOContainer *bcontainer,
>       int i;
>   
>       vbasedev->num_irqs = info->num_irqs;
> -    vbasedev->num_regions = info->num_regions;
> +    vbasedev->num_initial_regions = info->num_regions;
>       vbasedev->flags = info->flags;
>       vbasedev->reset_works = !!(info->flags & VFIO_DEVICE_FLAGS_RESET);
>   
> @@ -476,10 +476,10 @@ void vfio_device_prepare(VFIODevice *vbasedev, VFIOContainer *bcontainer,
>       QLIST_INSERT_HEAD(&vfio_device_list, vbasedev, global_next);
>   
>       vbasedev->reginfo = g_new0(struct vfio_region_info *,
> -                               vbasedev->num_regions);
> +                               vbasedev->num_initial_regions);
>       if (vbasedev->use_region_fds) {
> -        vbasedev->region_fds = g_new0(int, vbasedev->num_regions);
> -        for (i = 0; i < vbasedev->num_regions; i++) {
> +        vbasedev->region_fds = g_new0(int, vbasedev->num_initial_regions);
> +        for (i = 0; i < vbasedev->num_initial_regions; i++) {
>               vbasedev->region_fds[i] = -1;
>           }
>       }
> @@ -489,7 +489,7 @@ void vfio_device_unprepare(VFIODevice *vbasedev)
>   {
>       int i;
>   
> -    for (i = 0; i < vbasedev->num_regions; i++) {
> +    for (i = 0; i < vbasedev->num_initial_regions; i++) {
>           g_free(vbasedev->reginfo[i]);
>           if (vbasedev->region_fds != NULL && vbasedev->region_fds[i] != -1) {
>               close(vbasedev->region_fds[i]);
> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
> index 68470d552e..10fc065d20 100644
> --- a/hw/vfio/iommufd.c
> +++ b/hw/vfio/iommufd.c
> @@ -663,7 +663,8 @@ found_container:
>       vfio_iommufd_cpr_register_device(vbasedev);
>   
>       trace_iommufd_cdev_device_info(vbasedev->name, devfd, vbasedev->num_irqs,
> -                                   vbasedev->num_regions, vbasedev->flags);
> +                                   vbasedev->num_initial_regions,
> +                                   vbasedev->flags);
>       return true;
>   
>   err_listener_register:
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 06b06afc2b..8b8bc5a421 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -2975,9 +2975,9 @@ bool vfio_pci_populate_device(VFIOPCIDevice *vdev, Error **errp)
>           return false;
>       }
>   
> -    if (vbasedev->num_regions < VFIO_PCI_CONFIG_REGION_INDEX + 1) {
> +    if (vbasedev->num_initial_regions < VFIO_PCI_CONFIG_REGION_INDEX + 1) {
>           error_setg(errp, "unexpected number of io regions %u",
> -                   vbasedev->num_regions);
> +                   vbasedev->num_initial_regions);
>           return false;
>       }
>   

Re: [PATCH 2/2] vfio: only check region info cache for initial regions

[Cédric Le Goater]

On 10/14/25 17:12, John Levon wrote:
> It is semantically valid for a VFIO device to increase the number of
> regions after initialization. In this case, we'd attempt to check for
> cached region info past the size of the ->reginfo array. Check for the
> region index and skip the cache in these cases.
> 
> This also works around some VGPU use cases which appear to be a bug,
> where VFIO_DEVICE_QUERY_GFX_PLANE returns a region index beyond the
> reported ->num_regions.
> 
> Fixes: 95cdb024 ("vfio: add region info cache")
> Signed-off-by: John Levon <john.levon@nutanix.com>


Cc: qemu-stable@nongnu.org

> ---
>   hw/vfio/device.c | 27 +++++++++++++++++++--------
>   1 file changed, 19 insertions(+), 8 deletions(-)
> 

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

Thanks,

C.

Re: [PATCH 0/2] Fixes for vfio region cache

[Cédric Le Goater]

+ Mario

On 10/14/25 17:12, John Levon wrote:
> 
> 
> John Levon (2):
>    vfio: rename field to "num_initial_regions"
>    vfio: only check region info cache for initial regions
> 
>   include/hw/vfio/vfio-device.h |  2 +-
>   hw/vfio-user/device.c         |  2 +-
>   hw/vfio/ccw.c                 |  4 ++--
>   hw/vfio/device.c              | 39 ++++++++++++++++++++++-------------
>   hw/vfio/iommufd.c             |  3 ++-
>   hw/vfio/pci.c                 |  4 ++--
>   6 files changed, 33 insertions(+), 21 deletions(-)
> 

That was quick ! Thanks John. I will build a version based
on upstream for internal testing.

C.

Re: [PATCH 0/2] Fixes for vfio region cache

[Cédric Le Goater]

On 10/14/25 21:20, Cédric Le Goater wrote:
> + Mario
> 
> On 10/14/25 17:12, John Levon wrote:
>>
>>
>> John Levon (2):
>>    vfio: rename field to "num_initial_regions"
>>    vfio: only check region info cache for initial regions
>>
>>   include/hw/vfio/vfio-device.h |  2 +-
>>   hw/vfio-user/device.c         |  2 +-
>>   hw/vfio/ccw.c                 |  4 ++--
>>   hw/vfio/device.c              | 39 ++++++++++++++++++++++-------------
>>   hw/vfio/iommufd.c             |  3 ++-
>>   hw/vfio/pci.c                 |  4 ++--
>>   6 files changed, 33 insertions(+), 21 deletions(-)
>>
> 
> That was quick ! Thanks John. I will build a version based
> on upstream for internal testing.
All good. Thanks to Mario.

Applied to vfio-next.

C.