[v1,04/24] vfio-user: add region cache

Commit Message

John Johnson Nov. 8, 2022, 11:13 p.m. UTC

cache VFIO_DEVICE_GET_REGION_INFO results to reduce
memory alloc/free cycles and as prep work for vfio-user

Signed-off-by: John G Johnson <[e-mail]>
Signed-off-by: Elena Ufimtseva <[e-mail]>
Signed-off-by: Jagannathan Raman <[e-mail]>
---
 hw/vfio/ccw.c                 |  5 -----
 hw/vfio/common.c              | 41 +++++++++++++++++++++++++++++++++++------
 hw/vfio/igd.c                 | 23 +++++++++--------------
 hw/vfio/migration.c           |  2 --
 hw/vfio/pci-quirks.c          | 19 +++++--------------
 hw/vfio/pci.c                 |  8 --------
 include/hw/vfio/vfio-common.h |  2 ++
 7 files changed, 51 insertions(+), 49 deletions(-)

Comments

John Levon Dec. 9, 2022, 2:15 p.m. UTC | #1

On Tue, Nov 08, 2022 at 03:13:26PM -0800, John Johnson wrote:

> cache VFIO_DEVICE_GET_REGION_INFO results to reduce
> memory alloc/free cycles and as prep work for vfio-user

LGTM

Reviewed-by: John Levon <[e-mail]>

regards
john

Cédric Le Goater Dec. 12, 2022, 7:44 a.m. UTC | #2

On 11/9/22 00:13, John Johnson wrote:
> cache VFIO_DEVICE_GET_REGION_INFO results to reduce
> memory alloc/free cycles and as prep work for vfio-user
> 
> Signed-off-by: John G Johnson <[e-mail]>
> Signed-off-by: Elena Ufimtseva <[e-mail]>
> Signed-off-by: Jagannathan Raman <[e-mail]>


LGTM,

Reviewed-by: Cédric Le Goater <[e-mail]>

Thanks,

C.

> ---
>   hw/vfio/ccw.c                 |  5 -----
>   hw/vfio/common.c              | 41 +++++++++++++++++++++++++++++++++++------
>   hw/vfio/igd.c                 | 23 +++++++++--------------
>   hw/vfio/migration.c           |  2 --
>   hw/vfio/pci-quirks.c          | 19 +++++--------------
>   hw/vfio/pci.c                 |  8 --------
>   include/hw/vfio/vfio-common.h |  2 ++
>   7 files changed, 51 insertions(+), 49 deletions(-)
> 
> diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
> index 0354737..06b588c 100644
> --- a/hw/vfio/ccw.c
> +++ b/hw/vfio/ccw.c
> @@ -517,7 +517,6 @@ static void vfio_ccw_get_region(VFIOCCWDevice *vcdev, Error **errp)
>   
>       vcdev->io_region_offset = info->offset;
>       vcdev->io_region = g_malloc0(info->size);
> -    g_free(info);
>   
>       /* check for the optional async command region */
>       ret = vfio_get_dev_region_info(vdev, VFIO_REGION_TYPE_CCW,
> @@ -530,7 +529,6 @@ static void vfio_ccw_get_region(VFIOCCWDevice *vcdev, Error **errp)
>           }
>           vcdev->async_cmd_region_offset = info->offset;
>           vcdev->async_cmd_region = g_malloc0(info->size);
> -        g_free(info);
>       }
>   
>       ret = vfio_get_dev_region_info(vdev, VFIO_REGION_TYPE_CCW,
> @@ -543,7 +541,6 @@ static void vfio_ccw_get_region(VFIOCCWDevice *vcdev, Error **errp)
>           }
>           vcdev->schib_region_offset = info->offset;
>           vcdev->schib_region = g_malloc(info->size);
> -        g_free(info);
>       }
>   
>       ret = vfio_get_dev_region_info(vdev, VFIO_REGION_TYPE_CCW,
> @@ -557,7 +554,6 @@ static void vfio_ccw_get_region(VFIOCCWDevice *vcdev, Error **errp)
>           }
>           vcdev->crw_region_offset = info->offset;
>           vcdev->crw_region = g_malloc(info->size);
> -        g_free(info);
>       }
>   
>       return;
> @@ -567,7 +563,6 @@ out_err:
>       g_free(vcdev->schib_region);
>       g_free(vcdev->async_cmd_region);
>       g_free(vcdev->io_region);
> -    g_free(info);
>       return;
>   }
>   
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 83d69b9..dd9104f 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -1600,8 +1600,6 @@ int vfio_region_setup(Object *obj, VFIODevice *vbasedev, VFIORegion *region,
>           }
>       }
>   
> -    g_free(info);
> -
>       trace_vfio_region_setup(vbasedev->name, index, name,
>                               region->flags, region->fd_offset, region->size);
>       return 0;
> @@ -2357,6 +2355,16 @@ void vfio_put_group(VFIOGroup *group)
>       }
>   }
>   
> +void vfio_get_all_regions(VFIODevice *vbasedev)
> +{
> +    struct vfio_region_info *info;
> +    int i;
> +
> +    for (i = 0; i < vbasedev->num_regions; i++) {
> +        vfio_get_region_info(vbasedev, i, &info);
> +    }
> +}
> +
>   int vfio_get_device(VFIOGroup *group, const char *name,
>                       VFIODevice *vbasedev, Error **errp)
>   {
> @@ -2412,12 +2420,23 @@ int vfio_get_device(VFIOGroup *group, const char *name,
>       trace_vfio_get_device(name, dev_info.flags, dev_info.num_regions,
>                             dev_info.num_irqs);
>   
> +    vfio_get_all_regions(vbasedev);
>       vbasedev->reset_works = !!(dev_info.flags & VFIO_DEVICE_FLAGS_RESET);
>       return 0;
>   }
>   
>   void vfio_put_base_device(VFIODevice *vbasedev)
>   {
> +    if (vbasedev->regions != NULL) {
> +        int i;
> +
> +        for (i = 0; i < vbasedev->num_regions; i++) {
> +            g_free(vbasedev->regions[i]);
> +        }
> +        g_free(vbasedev->regions);
> +        vbasedev->regions = NULL;
> +    }
> +
>       if (!vbasedev->group) {
>           return;
>       }
> @@ -2432,6 +2451,17 @@ int vfio_get_region_info(VFIODevice *vbasedev, int index,
>   {
>       size_t argsz = sizeof(struct vfio_region_info);
>   
> +    /* create region cache */
> +    if (vbasedev->regions == NULL) {
> +        vbasedev->regions = g_new0(struct vfio_region_info *,
> +                                   vbasedev->num_regions);
> +    }
> +    /* check cache */
> +    if (vbasedev->regions[index] != NULL) {
> +        *info = vbasedev->regions[index];
> +        return 0;
> +    }
> +
>       *info = g_malloc0(argsz);
>   
>       (*info)->index = index;
> @@ -2451,6 +2481,9 @@ retry:
>           goto retry;
>       }
>   
> +    /* fill cache */
> +    vbasedev->regions[index] = *info;
> +
>       return 0;
>   }
>   
> @@ -2469,7 +2502,6 @@ int vfio_get_dev_region_info(VFIODevice *vbasedev, uint32_t type,
>   
>           hdr = vfio_get_region_info_cap(*info, VFIO_REGION_INFO_CAP_TYPE);
>           if (!hdr) {
> -            g_free(*info);
>               continue;
>           }
>   
> @@ -2481,8 +2513,6 @@ int vfio_get_dev_region_info(VFIODevice *vbasedev, uint32_t type,
>           if (cap_type->type == type && cap_type->subtype == subtype) {
>               return 0;
>           }
> -
> -        g_free(*info);
>       }
>   
>       *info = NULL;
> @@ -2498,7 +2528,6 @@ bool vfio_has_region_cap(VFIODevice *vbasedev, int region, uint16_t cap_type)
>           if (vfio_get_region_info_cap(info, cap_type)) {
>               ret = true;
>           }
> -        g_free(info);
>       }
>   
>       return ret;
> diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
> index afe3fe7..22efa1a 100644
> --- a/hw/vfio/igd.c
> +++ b/hw/vfio/igd.c
> @@ -425,7 +425,7 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
>       if ((ret || !rom->size) && !vdev->pdev.romfile) {
>           error_report("IGD device %s has no ROM, legacy mode disabled",
>                        vdev->vbasedev.name);
> -        goto out;
> +        return;
>       }
>   
>       /*
> @@ -436,7 +436,7 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
>           error_report("IGD device %s hotplugged, ROM disabled, "
>                        "legacy mode disabled", vdev->vbasedev.name);
>           vdev->rom_read_failed = true;
> -        goto out;
> +        return;
>       }
>   
>       /*
> @@ -449,7 +449,7 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
>       if (ret) {
>           error_report("IGD device %s does not support OpRegion access,"
>                        "legacy mode disabled", vdev->vbasedev.name);
> -        goto out;
> +        return;
>       }
>   
>       ret = vfio_get_dev_region_info(&vdev->vbasedev,
> @@ -458,7 +458,7 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
>       if (ret) {
>           error_report("IGD device %s does not support host bridge access,"
>                        "legacy mode disabled", vdev->vbasedev.name);
> -        goto out;
> +        return;
>       }
>   
>       ret = vfio_get_dev_region_info(&vdev->vbasedev,
> @@ -467,7 +467,7 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
>       if (ret) {
>           error_report("IGD device %s does not support LPC bridge access,"
>                        "legacy mode disabled", vdev->vbasedev.name);
> -        goto out;
> +        return;
>       }
>   
>       gmch = vfio_pci_read_config(&vdev->pdev, IGD_GMCH, 4);
> @@ -481,7 +481,7 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
>           error_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
>           error_report("IGD device %s failed to enable VGA access, "
>                        "legacy mode disabled", vdev->vbasedev.name);
> -        goto out;
> +        return;
>       }
>   
>       /* Create our LPC/ISA bridge */
> @@ -489,7 +489,7 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
>       if (ret) {
>           error_report("IGD device %s failed to create LPC bridge, "
>                        "legacy mode disabled", vdev->vbasedev.name);
> -        goto out;
> +        return;
>       }
>   
>       /* Stuff some host values into the VM PCI host bridge */
> @@ -497,7 +497,7 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
>       if (ret) {
>           error_report("IGD device %s failed to modify host bridge, "
>                        "legacy mode disabled", vdev->vbasedev.name);
> -        goto out;
> +        return;
>       }
>   
>       /* Setup OpRegion access */
> @@ -505,7 +505,7 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
>       if (ret) {
>           error_append_hint(&err, "IGD legacy mode disabled\n");
>           error_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
> -        goto out;
> +        return;
>       }
>   
>       /* Setup our quirk to munge GTT addresses to the VM allocated buffer */
> @@ -608,9 +608,4 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
>   
>       trace_vfio_pci_igd_bdsm_enabled(vdev->vbasedev.name, ggms_mb + gms_mb);
>   
> -out:
> -    g_free(rom);
> -    g_free(opregion);
> -    g_free(host);
> -    g_free(lpc);
>   }
> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> index a6ad1f8..397be43 100644
> --- a/hw/vfio/migration.c
> +++ b/hw/vfio/migration.c
> @@ -879,13 +879,11 @@ int vfio_migration_probe(VFIODevice *vbasedev, Error **errp)
>       }
>   
>       trace_vfio_migration_probe(vbasedev->name, info->index);
> -    g_free(info);
>       return 0;
>   
>   add_blocker:
>       error_setg(&vbasedev->migration_blocker,
>                  "VFIO device doesn't support migration");
> -    g_free(info);
>   
>       ret = migrate_add_blocker(vbasedev->migration_blocker, errp);
>       if (ret < 0) {
> diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
> index f0147a0..c04ee19 100644
> --- a/hw/vfio/pci-quirks.c
> +++ b/hw/vfio/pci-quirks.c
> @@ -1585,16 +1585,14 @@ int vfio_pci_nvidia_v100_ram_init(VFIOPCIDevice *vdev, Error **errp)
>   
>       hdr = vfio_get_region_info_cap(nv2reg, VFIO_REGION_INFO_CAP_NVLINK2_SSATGT);
>       if (!hdr) {
> -        ret = -ENODEV;
> -        goto free_exit;
> +        return -ENODEV;
>       }
>       cap = (void *) hdr;
>   
>       p = mmap(NULL, nv2reg->size, PROT_READ | PROT_WRITE,
>                MAP_SHARED, vdev->vbasedev.fd, nv2reg->offset);
>       if (p == MAP_FAILED) {
> -        ret = -errno;
> -        goto free_exit;
> +        return -errno;
>       }
>   
>       quirk = vfio_quirk_alloc(1);
> @@ -1607,8 +1605,6 @@ int vfio_pci_nvidia_v100_ram_init(VFIOPCIDevice *vdev, Error **errp)
>                                      OBJ_PROP_FLAG_READ);
>       trace_vfio_pci_nvidia_gpu_setup_quirk(vdev->vbasedev.name, cap->tgt,
>                                             nv2reg->size);
> -free_exit:
> -    g_free(nv2reg);
>   
>       return ret;
>   }
> @@ -1635,16 +1631,14 @@ int vfio_pci_nvlink2_init(VFIOPCIDevice *vdev, Error **errp)
>       hdr = vfio_get_region_info_cap(atsdreg,
>                                      VFIO_REGION_INFO_CAP_NVLINK2_SSATGT);
>       if (!hdr) {
> -        ret = -ENODEV;
> -        goto free_exit;
> +        return -ENODEV;
>       }
>       captgt = (void *) hdr;
>   
>       hdr = vfio_get_region_info_cap(atsdreg,
>                                      VFIO_REGION_INFO_CAP_NVLINK2_LNKSPD);
>       if (!hdr) {
> -        ret = -ENODEV;
> -        goto free_exit;
> +        return -ENODEV;
>       }
>       capspeed = (void *) hdr;
>   
> @@ -1653,8 +1647,7 @@ int vfio_pci_nvlink2_init(VFIOPCIDevice *vdev, Error **errp)
>           p = mmap(NULL, atsdreg->size, PROT_READ | PROT_WRITE,
>                    MAP_SHARED, vdev->vbasedev.fd, atsdreg->offset);
>           if (p == MAP_FAILED) {
> -            ret = -errno;
> -            goto free_exit;
> +            return -errno;
>           }
>   
>           quirk = vfio_quirk_alloc(1);
> @@ -1674,8 +1667,6 @@ int vfio_pci_nvlink2_init(VFIOPCIDevice *vdev, Error **errp)
>                                      OBJ_PROP_FLAG_READ);
>       trace_vfio_pci_nvlink2_setup_quirk_lnkspd(vdev->vbasedev.name,
>                                                 capspeed->link_speed);
> -free_exit:
> -    g_free(atsdreg);
>   
>       return ret;
>   }
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 60acde5..1c7618d 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -836,8 +836,6 @@ static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
>       vdev->rom_size = size = reg_info->size;
>       vdev->rom_offset = reg_info->offset;
>   
> -    g_free(reg_info);
> -
>       if (!vdev->rom_size) {
>           vdev->rom_read_failed = true;
>           error_report("vfio-pci: Cannot read device rom at "
> @@ -2564,7 +2562,6 @@ int vfio_populate_vga(VFIOPCIDevice *vdev, Error **errp)
>           error_setg(errp, "unexpected VGA info, flags 0x%lx, size 0x%lx",
>                      (unsigned long)reg_info->flags,
>                      (unsigned long)reg_info->size);
> -        g_free(reg_info);
>           return -EINVAL;
>       }
>   
> @@ -2573,8 +2570,6 @@ int vfio_populate_vga(VFIOPCIDevice *vdev, Error **errp)
>       vdev->vga->fd_offset = reg_info->offset;
>       vdev->vga->fd = vdev->vbasedev.fd;
>   
> -    g_free(reg_info);
> -
>       vdev->vga->region[QEMU_PCI_VGA_MEM].offset = QEMU_PCI_VGA_MEM_BASE;
>       vdev->vga->region[QEMU_PCI_VGA_MEM].nr = QEMU_PCI_VGA_MEM;
>       QLIST_INIT(&vdev->vga->region[QEMU_PCI_VGA_MEM].quirks);
> @@ -2669,8 +2664,6 @@ static void vfio_populate_device(VFIOPCIDevice *vdev, Error **errp)
>       }
>       vdev->config_offset = reg_info->offset;
>   
> -    g_free(reg_info);
> -
>       if (vdev->features & VFIO_FEATURE_ENABLE_VGA) {
>           ret = vfio_populate_vga(vdev, errp);
>           if (ret) {
> @@ -3079,7 +3072,6 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
>           }
>   
>           ret = vfio_pci_igd_opregion_init(vdev, opregion, errp);
> -        g_free(opregion);
>           if (ret) {
>               goto out_teardown;
>           }
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index 6fd40f1..a1db165 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -145,6 +145,7 @@ typedef struct VFIODevice {
>       VFIOMigration *migration;
>       Error *migration_blocker;
>       OnOffAuto pre_copy_dirty_page_tracking;
> +    struct vfio_region_info **regions;
>   } VFIODevice;
>   
>   struct VFIODeviceOps {
> @@ -258,6 +259,7 @@ int vfio_get_region_info(VFIODevice *vbasedev, int index,
>                            struct vfio_region_info **info);
>   int vfio_get_dev_region_info(VFIODevice *vbasedev, uint32_t type,
>                                uint32_t subtype, struct vfio_region_info **info);
> +void vfio_get_all_regions(VFIODevice *vbasedev);
>   bool vfio_has_region_cap(VFIODevice *vbasedev, int region, uint16_t cap_type);
>   struct vfio_info_cap_header *
>   vfio_get_region_info_cap(struct vfio_region_info *info, uint16_t id);

Philippe Mathieu-Daudé Dec. 12, 2022, 11:42 a.m. UTC | #3

On 9/11/22 00:13, John Johnson wrote:
> cache VFIO_DEVICE_GET_REGION_INFO results to reduce
> memory alloc/free cycles and as prep work for vfio-user
> 
> Signed-off-by: John G Johnson <[e-mail]>
> Signed-off-by: Elena Ufimtseva <[e-mail]>
> Signed-off-by: Jagannathan Raman <[e-mail]>
> ---
>   hw/vfio/ccw.c                 |  5 -----
>   hw/vfio/common.c              | 41 +++++++++++++++++++++++++++++++++++------
>   hw/vfio/igd.c                 | 23 +++++++++--------------
>   hw/vfio/migration.c           |  2 --
>   hw/vfio/pci-quirks.c          | 19 +++++--------------
>   hw/vfio/pci.c                 |  8 --------
>   include/hw/vfio/vfio-common.h |  2 ++
>   7 files changed, 51 insertions(+), 49 deletions(-)


>   void vfio_put_base_device(VFIODevice *vbasedev)
>   {
> +    if (vbasedev->regions != NULL) {
> +        int i;
> +
> +        for (i = 0; i < vbasedev->num_regions; i++) {
> +            g_free(vbasedev->regions[i]);
> +        }
> +        g_free(vbasedev->regions);
> +        vbasedev->regions = NULL;
> +    }
> +
>       if (!vbasedev->group) {
>           return;
>       }
> @@ -2432,6 +2451,17 @@ int vfio_get_region_info(VFIODevice *vbasedev, int index,
>   {
>       size_t argsz = sizeof(struct vfio_region_info);
>   
> +    /* create region cache */
> +    if (vbasedev->regions == NULL) {
> +        vbasedev->regions = g_new0(struct vfio_region_info *,
> +                                   vbasedev->num_regions);
> +    }
> +    /* check cache */
> +    if (vbasedev->regions[index] != NULL) {
> +        *info = vbasedev->regions[index];
> +        return 0;
> +    }

What about simply allocating/releasing once regions[] in
vfio_instance_init / vfio_instance_finalize?

John Johnson Feb. 2, 2023, 5:21 a.m. UTC | #4

> On Dec 12, 2022, at 3:42 AM, Philippe Mathieu-Daudé <[e-mail]> wrote:
> 
> On 9/11/22 00:13, John Johnson wrote:
>> cache VFIO_DEVICE_GET_REGION_INFO results to reduce
>> memory alloc/free cycles and as prep work for vfio-user
>> Signed-off-by: John G Johnson <[e-mail]>
>> Signed-off-by: Elena Ufimtseva <[e-mail]>
>> Signed-off-by: Jagannathan Raman <[e-mail]>
>> ---
>>  hw/vfio/ccw.c                 |  5 -----
>>  hw/vfio/common.c              | 41 +++++++++++++++++++++++++++++++++++------
>>  hw/vfio/igd.c                 | 23 +++++++++--------------
>>  hw/vfio/migration.c           |  2 --
>>  hw/vfio/pci-quirks.c          | 19 +++++--------------
>>  hw/vfio/pci.c                 |  8 --------
>>  include/hw/vfio/vfio-common.h |  2 ++
>>  7 files changed, 51 insertions(+), 49 deletions(-)
> 
> 
>>  void vfio_put_base_device(VFIODevice *vbasedev)
>>  {
>> +    if (vbasedev->regions != NULL) {
>> +        int i;
>> +
>> +        for (i = 0; i < vbasedev->num_regions; i++) {
>> +            g_free(vbasedev->regions[i]);
>> +        }
>> +        g_free(vbasedev->regions);
>> +        vbasedev->regions = NULL;
>> +    }
>> +
>>      if (!vbasedev->group) {
>>          return;
>>      }
>> @@ -2432,6 +2451,17 @@ int vfio_get_region_info(VFIODevice *vbasedev, int index,
>>  {
>>      size_t argsz = sizeof(struct vfio_region_info);
>>  +    /* create region cache */
>> +    if (vbasedev->regions == NULL) {
>> +        vbasedev->regions = g_new0(struct vfio_region_info *,
>> +                                   vbasedev->num_regions);
>> +    }
>> +    /* check cache */
>> +    if (vbasedev->regions[index] != NULL) {
>> +        *info = vbasedev->regions[index];
>> +        return 0;
>> +    }
> 
> What about simply allocating/releasing once regions[] in
> vfio_instance_init / vfio_instance_finalize?


	I think this is done already, except with _realize and _instance_finalize.

e.g,, vfio_realize() -> vfio_get_device() -> vfio_get_all_regions  allocates

and

vfio_instance_finalize -> vfio_put_device -> vfio_put_base_device  deallocates

								JJ

Patch

diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
index 0354737..06b588c 100644
--- a/hw/vfio/ccw.c
+++ b/hw/vfio/ccw.c
@@ -517,7 +517,6 @@  static void vfio_ccw_get_region(VFIOCCWDevice *vcdev, Error **errp)
 
     vcdev->io_region_offset = info->offset;
     vcdev->io_region = g_malloc0(info->size);
-    g_free(info);
 
     /* check for the optional async command region */
     ret = vfio_get_dev_region_info(vdev, VFIO_REGION_TYPE_CCW,
@@ -530,7 +529,6 @@  static void vfio_ccw_get_region(VFIOCCWDevice *vcdev, Error **errp)
         }
         vcdev->async_cmd_region_offset = info->offset;
         vcdev->async_cmd_region = g_malloc0(info->size);
-        g_free(info);
     }
 
     ret = vfio_get_dev_region_info(vdev, VFIO_REGION_TYPE_CCW,
@@ -543,7 +541,6 @@  static void vfio_ccw_get_region(VFIOCCWDevice *vcdev, Error **errp)
         }
         vcdev->schib_region_offset = info->offset;
         vcdev->schib_region = g_malloc(info->size);
-        g_free(info);
     }
 
     ret = vfio_get_dev_region_info(vdev, VFIO_REGION_TYPE_CCW,
@@ -557,7 +554,6 @@  static void vfio_ccw_get_region(VFIOCCWDevice *vcdev, Error **errp)
         }
         vcdev->crw_region_offset = info->offset;
         vcdev->crw_region = g_malloc(info->size);
-        g_free(info);
     }
 
     return;
@@ -567,7 +563,6 @@  out_err:
     g_free(vcdev->schib_region);
     g_free(vcdev->async_cmd_region);
     g_free(vcdev->io_region);
-    g_free(info);
     return;
 }
 
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 83d69b9..dd9104f 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -1600,8 +1600,6 @@  int vfio_region_setup(Object *obj, VFIODevice *vbasedev, VFIORegion *region,
         }
     }
 
-    g_free(info);
-
     trace_vfio_region_setup(vbasedev->name, index, name,
                             region->flags, region->fd_offset, region->size);
     return 0;
@@ -2357,6 +2355,16 @@  void vfio_put_group(VFIOGroup *group)
     }
 }
 
+void vfio_get_all_regions(VFIODevice *vbasedev)
+{
+    struct vfio_region_info *info;
+    int i;
+
+    for (i = 0; i < vbasedev->num_regions; i++) {
+        vfio_get_region_info(vbasedev, i, &info);
+    }
+}
+
 int vfio_get_device(VFIOGroup *group, const char *name,
                     VFIODevice *vbasedev, Error **errp)
 {
@@ -2412,12 +2420,23 @@  int vfio_get_device(VFIOGroup *group, const char *name,
     trace_vfio_get_device(name, dev_info.flags, dev_info.num_regions,
                           dev_info.num_irqs);
 
+    vfio_get_all_regions(vbasedev);
     vbasedev->reset_works = !!(dev_info.flags & VFIO_DEVICE_FLAGS_RESET);
     return 0;
 }
 
 void vfio_put_base_device(VFIODevice *vbasedev)
 {
+    if (vbasedev->regions != NULL) {
+        int i;
+
+        for (i = 0; i < vbasedev->num_regions; i++) {
+            g_free(vbasedev->regions[i]);
+        }
+        g_free(vbasedev->regions);
+        vbasedev->regions = NULL;
+    }
+
     if (!vbasedev->group) {
         return;
     }
@@ -2432,6 +2451,17 @@  int vfio_get_region_info(VFIODevice *vbasedev, int index,
 {
     size_t argsz = sizeof(struct vfio_region_info);
 
+    /* create region cache */
+    if (vbasedev->regions == NULL) {
+        vbasedev->regions = g_new0(struct vfio_region_info *,
+                                   vbasedev->num_regions);
+    }
+    /* check cache */
+    if (vbasedev->regions[index] != NULL) {
+        *info = vbasedev->regions[index];
+        return 0;
+    }
+
     *info = g_malloc0(argsz);
 
     (*info)->index = index;
@@ -2451,6 +2481,9 @@  retry:
         goto retry;
     }
 
+    /* fill cache */
+    vbasedev->regions[index] = *info;
+
     return 0;
 }
 
@@ -2469,7 +2502,6 @@  int vfio_get_dev_region_info(VFIODevice *vbasedev, uint32_t type,
 
         hdr = vfio_get_region_info_cap(*info, VFIO_REGION_INFO_CAP_TYPE);
         if (!hdr) {
-            g_free(*info);
             continue;
         }
 
@@ -2481,8 +2513,6 @@  int vfio_get_dev_region_info(VFIODevice *vbasedev, uint32_t type,
         if (cap_type->type == type && cap_type->subtype == subtype) {
             return 0;
         }
-
-        g_free(*info);
     }
 
     *info = NULL;
@@ -2498,7 +2528,6 @@  bool vfio_has_region_cap(VFIODevice *vbasedev, int region, uint16_t cap_type)
         if (vfio_get_region_info_cap(info, cap_type)) {
             ret = true;
         }
-        g_free(info);
     }
 
     return ret;
diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
index afe3fe7..22efa1a 100644
--- a/hw/vfio/igd.c
+++ b/hw/vfio/igd.c
@@ -425,7 +425,7 @@  void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
     if ((ret || !rom->size) && !vdev->pdev.romfile) {
         error_report("IGD device %s has no ROM, legacy mode disabled",
                      vdev->vbasedev.name);
-        goto out;
+        return;
     }
 
     /*
@@ -436,7 +436,7 @@  void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
         error_report("IGD device %s hotplugged, ROM disabled, "
                      "legacy mode disabled", vdev->vbasedev.name);
         vdev->rom_read_failed = true;
-        goto out;
+        return;
     }
 
     /*
@@ -449,7 +449,7 @@  void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
     if (ret) {
         error_report("IGD device %s does not support OpRegion access,"
                      "legacy mode disabled", vdev->vbasedev.name);
-        goto out;
+        return;
     }
 
     ret = vfio_get_dev_region_info(&vdev->vbasedev,
@@ -458,7 +458,7 @@  void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
     if (ret) {
         error_report("IGD device %s does not support host bridge access,"
                      "legacy mode disabled", vdev->vbasedev.name);
-        goto out;
+        return;
     }
 
     ret = vfio_get_dev_region_info(&vdev->vbasedev,
@@ -467,7 +467,7 @@  void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
     if (ret) {
         error_report("IGD device %s does not support LPC bridge access,"
                      "legacy mode disabled", vdev->vbasedev.name);
-        goto out;
+        return;
     }
 
     gmch = vfio_pci_read_config(&vdev->pdev, IGD_GMCH, 4);
@@ -481,7 +481,7 @@  void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
         error_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
         error_report("IGD device %s failed to enable VGA access, "
                      "legacy mode disabled", vdev->vbasedev.name);
-        goto out;
+        return;
     }
 
     /* Create our LPC/ISA bridge */
@@ -489,7 +489,7 @@  void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
     if (ret) {
         error_report("IGD device %s failed to create LPC bridge, "
                      "legacy mode disabled", vdev->vbasedev.name);
-        goto out;
+        return;
     }
 
     /* Stuff some host values into the VM PCI host bridge */
@@ -497,7 +497,7 @@  void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
     if (ret) {
         error_report("IGD device %s failed to modify host bridge, "
                      "legacy mode disabled", vdev->vbasedev.name);
-        goto out;
+        return;
     }
 
     /* Setup OpRegion access */
@@ -505,7 +505,7 @@  void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
     if (ret) {
         error_append_hint(&err, "IGD legacy mode disabled\n");
         error_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
-        goto out;
+        return;
     }
 
     /* Setup our quirk to munge GTT addresses to the VM allocated buffer */
@@ -608,9 +608,4 @@  void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
 
     trace_vfio_pci_igd_bdsm_enabled(vdev->vbasedev.name, ggms_mb + gms_mb);
 
-out:
-    g_free(rom);
-    g_free(opregion);
-    g_free(host);
-    g_free(lpc);
 }
diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index a6ad1f8..397be43 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -879,13 +879,11 @@  int vfio_migration_probe(VFIODevice *vbasedev, Error **errp)
     }
 
     trace_vfio_migration_probe(vbasedev->name, info->index);
-    g_free(info);
     return 0;
 
 add_blocker:
     error_setg(&vbasedev->migration_blocker,
                "VFIO device doesn't support migration");
-    g_free(info);
 
     ret = migrate_add_blocker(vbasedev->migration_blocker, errp);
     if (ret < 0) {
diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
index f0147a0..c04ee19 100644
--- a/hw/vfio/pci-quirks.c
+++ b/hw/vfio/pci-quirks.c
@@ -1585,16 +1585,14 @@  int vfio_pci_nvidia_v100_ram_init(VFIOPCIDevice *vdev, Error **errp)
 
     hdr = vfio_get_region_info_cap(nv2reg, VFIO_REGION_INFO_CAP_NVLINK2_SSATGT);
     if (!hdr) {
-        ret = -ENODEV;
-        goto free_exit;
+        return -ENODEV;
     }
     cap = (void *) hdr;
 
     p = mmap(NULL, nv2reg->size, PROT_READ | PROT_WRITE,
              MAP_SHARED, vdev->vbasedev.fd, nv2reg->offset);
     if (p == MAP_FAILED) {
-        ret = -errno;
-        goto free_exit;
+        return -errno;
     }
 
     quirk = vfio_quirk_alloc(1);
@@ -1607,8 +1605,6 @@  int vfio_pci_nvidia_v100_ram_init(VFIOPCIDevice *vdev, Error **errp)
                                    OBJ_PROP_FLAG_READ);
     trace_vfio_pci_nvidia_gpu_setup_quirk(vdev->vbasedev.name, cap->tgt,
                                           nv2reg->size);
-free_exit:
-    g_free(nv2reg);
 
     return ret;
 }
@@ -1635,16 +1631,14 @@  int vfio_pci_nvlink2_init(VFIOPCIDevice *vdev, Error **errp)
     hdr = vfio_get_region_info_cap(atsdreg,
                                    VFIO_REGION_INFO_CAP_NVLINK2_SSATGT);
     if (!hdr) {
-        ret = -ENODEV;
-        goto free_exit;
+        return -ENODEV;
     }
     captgt = (void *) hdr;
 
     hdr = vfio_get_region_info_cap(atsdreg,
                                    VFIO_REGION_INFO_CAP_NVLINK2_LNKSPD);
     if (!hdr) {
-        ret = -ENODEV;
-        goto free_exit;
+        return -ENODEV;
     }
     capspeed = (void *) hdr;
 
@@ -1653,8 +1647,7 @@  int vfio_pci_nvlink2_init(VFIOPCIDevice *vdev, Error **errp)
         p = mmap(NULL, atsdreg->size, PROT_READ | PROT_WRITE,
                  MAP_SHARED, vdev->vbasedev.fd, atsdreg->offset);
         if (p == MAP_FAILED) {
-            ret = -errno;
-            goto free_exit;
+            return -errno;
         }
 
         quirk = vfio_quirk_alloc(1);
@@ -1674,8 +1667,6 @@  int vfio_pci_nvlink2_init(VFIOPCIDevice *vdev, Error **errp)
                                    OBJ_PROP_FLAG_READ);
     trace_vfio_pci_nvlink2_setup_quirk_lnkspd(vdev->vbasedev.name,
                                               capspeed->link_speed);
-free_exit:
-    g_free(atsdreg);
 
     return ret;
 }
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 60acde5..1c7618d 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -836,8 +836,6 @@  static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
     vdev->rom_size = size = reg_info->size;
     vdev->rom_offset = reg_info->offset;
 
-    g_free(reg_info);
-
     if (!vdev->rom_size) {
         vdev->rom_read_failed = true;
         error_report("vfio-pci: Cannot read device rom at "
@@ -2564,7 +2562,6 @@  int vfio_populate_vga(VFIOPCIDevice *vdev, Error **errp)
         error_setg(errp, "unexpected VGA info, flags 0x%lx, size 0x%lx",
                    (unsigned long)reg_info->flags,
                    (unsigned long)reg_info->size);
-        g_free(reg_info);
         return -EINVAL;
     }
 
@@ -2573,8 +2570,6 @@  int vfio_populate_vga(VFIOPCIDevice *vdev, Error **errp)
     vdev->vga->fd_offset = reg_info->offset;
     vdev->vga->fd = vdev->vbasedev.fd;
 
-    g_free(reg_info);
-
     vdev->vga->region[QEMU_PCI_VGA_MEM].offset = QEMU_PCI_VGA_MEM_BASE;
     vdev->vga->region[QEMU_PCI_VGA_MEM].nr = QEMU_PCI_VGA_MEM;
     QLIST_INIT(&vdev->vga->region[QEMU_PCI_VGA_MEM].quirks);
@@ -2669,8 +2664,6 @@  static void vfio_populate_device(VFIOPCIDevice *vdev, Error **errp)
     }
     vdev->config_offset = reg_info->offset;
 
-    g_free(reg_info);
-
     if (vdev->features & VFIO_FEATURE_ENABLE_VGA) {
         ret = vfio_populate_vga(vdev, errp);
         if (ret) {
@@ -3079,7 +3072,6 @@  static void vfio_realize(PCIDevice *pdev, Error **errp)
         }
 
         ret = vfio_pci_igd_opregion_init(vdev, opregion, errp);
-        g_free(opregion);
         if (ret) {
             goto out_teardown;
         }
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index 6fd40f1..a1db165 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -145,6 +145,7 @@  typedef struct VFIODevice {
     VFIOMigration *migration;
     Error *migration_blocker;
     OnOffAuto pre_copy_dirty_page_tracking;
+    struct vfio_region_info **regions;
 } VFIODevice;
 
 struct VFIODeviceOps {
@@ -258,6 +259,7 @@  int vfio_get_region_info(VFIODevice *vbasedev, int index,
                          struct vfio_region_info **info);
 int vfio_get_dev_region_info(VFIODevice *vbasedev, uint32_t type,
                              uint32_t subtype, struct vfio_region_info **info);
+void vfio_get_all_regions(VFIODevice *vbasedev);
 bool vfio_has_region_cap(VFIODevice *vbasedev, int region, uint16_t cap_type);
 struct vfio_info_cap_header *
 vfio_get_region_info_cap(struct vfio_region_info *info, uint16_t id);