1. Patchew
  2. QEMU
  3. View series

[PATCH] vfio/pci: Initialize rom_read_failed in vfio_pci_load_rom()

Mario Casquero posted 1 patch 6 days, 14 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260601144339.339275-1-mcasquer@redhat.com
Maintainers: Alex Williamson <alex@shazbot.org>, "Cédric Le Goater" <clg@redhat.com>
There is a newer version of this series
hw/vfio/pci.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
[PATCH] vfio/pci: Initialize rom_read_failed in vfio_pci_load_rom()
Posted by Mario Casquero 6 days, 14 hours ago 
When vfio_device_get_region_info() fails in vfio_pci_load_rom(),
the function returns without setting vdev->rom_read_failed to true,
and without allocating vdev->rom. This leaves vdev->rom as NULL.

Signed-off-by: Mario Casquero <mcasquer@redhat.com>
---
 hw/vfio/pci.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 9c06b25e63..d73614d3de 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1027,7 +1027,7 @@ static void vfio_update_msi(VFIOPCIDevice *vdev)
     }
 }

-static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
+static bool vfio_pci_load_rom(VFIOPCIDevice *vdev)
 {
     VFIODevice *vbasedev = &vdev->vbasedev;
     struct vfio_region_info *reg_info = NULL;
@@ -1041,7 +1041,7 @@ static void vfio_pci_load_rom(VFIOPCIDevice *vdev)

     if (ret != 0) {
         error_report("vfio: Error getting ROM info: %s", strerror(-ret));
-        return;
+        return false;
     }

     trace_vfio_pci_load_rom(vbasedev->name, (unsigned long)reg_info->size,
@@ -1057,7 +1057,7 @@ static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
         error_printf("Device option ROM contents are probably invalid "
                     "(check dmesg).\nSkip option ROM probe with rombar=0, "
                     "or load from file with romfile=\n");
-        return;
+        return false;
     }

     vdev->rom = g_malloc(size);
@@ -1113,6 +1113,8 @@ static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
             data[6] = -csum;
         }
     }
+
+    return true;
 }

 /* "Raw" read of underlying config space. */
@@ -1146,7 +1148,7 @@ static uint64_t vfio_rom_read(void *opaque, hwaddr addr, unsigned size)

     /* Load the ROM lazily when the guest tries to read it */
     if (unlikely(!vdev->rom && !vdev->rom_read_failed)) {
-        vfio_pci_load_rom(vdev);
+        vdev->rom_read_failed = !vfio_pci_load_rom(vdev);
     }

     memcpy(&val, vdev->rom + addr,
--
2.54.0
Re: [PATCH] vfio/pci: Initialize rom_read_failed in vfio_pci_load_rom()
Posted by Cédric Le Goater 6 days, 14 hours ago 
On 6/1/26 16:43, Mario Casquero wrote:
> When vfio_device_get_region_info() fails in vfio_pci_load_rom(),
> the function returns without setting vdev->rom_read_failed to true,
> and without allocating vdev->rom. This leaves vdev->rom as NULL.
> 
> Signed-off-by: Mario Casquero <mcasquer@redhat.com>
> ---
>   hw/vfio/pci.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 9c06b25e63..d73614d3de 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -1027,7 +1027,7 @@ static void vfio_update_msi(VFIOPCIDevice *vdev)
>       }
>   }
> 
> -static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
> +static bool vfio_pci_load_rom(VFIOPCIDevice *vdev)
>   {
>       VFIODevice *vbasedev = &vdev->vbasedev;
>       struct vfio_region_info *reg_info = NULL;
> @@ -1041,7 +1041,7 @@ static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
> 
>       if (ret != 0) {
>           error_report("vfio: Error getting ROM info: %s", strerror(-ret));
> -        return;
> +        return false;
>       }
> 
>       trace_vfio_pci_load_rom(vbasedev->name, (unsigned long)reg_info->size,
> @@ -1057,7 +1057,7 @@ static void vfio_pci_load_rom(VFIOPCIDevice *vdev)

One little nit, you could remove the vdev->rom_read_failed assignment here :
  
         vdev->rom_read_failed = true;

Could you send a v2 please ?

Thanks,

C.

>           error_printf("Device option ROM contents are probably invalid "
>                       "(check dmesg).\nSkip option ROM probe with rombar=0, "
>                       "or load from file with romfile=\n");
> -        return;
> +        return false;
>       }
> 
>       vdev->rom = g_malloc(size);
> @@ -1113,6 +1113,8 @@ static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
>               data[6] = -csum;
>           }
>       }
> +
> +    return true;
>   }
> 
>   /* "Raw" read of underlying config space. */
> @@ -1146,7 +1148,7 @@ static uint64_t vfio_rom_read(void *opaque, hwaddr addr, unsigned size)
> 
>       /* Load the ROM lazily when the guest tries to read it */
>       if (unlikely(!vdev->rom && !vdev->rom_read_failed)) {
> -        vfio_pci_load_rom(vdev);
> +        vdev->rom_read_failed = !vfio_pci_load_rom(vdev);
>       }
> 
>       memcpy(&val, vdev->rom + addr,
> --
> 2.54.0
>
Re: [PATCH] vfio/pci: Initialize rom_read_failed in vfio_pci_load_rom()
Posted by Cédric Le Goater 6 days, 14 hours ago 
On 6/1/26 16:43, Mario Casquero wrote:
> When vfio_device_get_region_info() fails in vfio_pci_load_rom(),
> the function returns without setting vdev->rom_read_failed to true,
> and without allocating vdev->rom. This leaves vdev->rom as NULL.
> 
> Signed-off-by: Mario Casquero <mcasquer@redhat.com>
> ---
>   hw/vfio/pci.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 9c06b25e63..d73614d3de 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -1027,7 +1027,7 @@ static void vfio_update_msi(VFIOPCIDevice *vdev)
>       }
>   }
> 
> -static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
> +static bool vfio_pci_load_rom(VFIOPCIDevice *vdev)
>   {
>       VFIODevice *vbasedev = &vdev->vbasedev;
>       struct vfio_region_info *reg_info = NULL;
> @@ -1041,7 +1041,7 @@ static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
> 
>       if (ret != 0) {
>           error_report("vfio: Error getting ROM info: %s", strerror(-ret));
> -        return;
> +        return false;
>       }
> 
>       trace_vfio_pci_load_rom(vbasedev->name, (unsigned long)reg_info->size,
> @@ -1057,7 +1057,7 @@ static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
>           error_printf("Device option ROM contents are probably invalid "
>                       "(check dmesg).\nSkip option ROM probe with rombar=0, "
>                       "or load from file with romfile=\n");
> -        return;
> +        return false;
>       }
> 
>       vdev->rom = g_malloc(size);
> @@ -1113,6 +1113,8 @@ static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
>               data[6] = -csum;
>           }
>       }
> +
> +    return true;
>   }
> 
>   /* "Raw" read of underlying config space. */
> @@ -1146,7 +1148,7 @@ static uint64_t vfio_rom_read(void *opaque, hwaddr addr, unsigned size)
> 
>       /* Load the ROM lazily when the guest tries to read it */
>       if (unlikely(!vdev->rom && !vdev->rom_read_failed)) {
> -        vfio_pci_load_rom(vdev);
> +        vdev->rom_read_failed = !vfio_pci_load_rom(vdev);
>       }
> 
>       memcpy(&val, vdev->rom + addr,

In a followup patch, I would add a guest error log when the guest attempts
to read from a ROM that failed to load.

> --
> 2.54.0
> 

Anyhow,


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

Thanks !

C.

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.