Re: [PATCH v2 1/2] vfio/pci: fall back to platform ROM when pci_map_rom() fails

[Alex Williamson <alex@shazbot.org>]

On Wed, 21 Oct 2026 04:20:32 -0400
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> When pci_resource_start() returns non-zero but pci_map_rom() fails
> (ROM BAR exists but content is unreadable), the pdev->rom fallback
> is unreachable because it's behind an else-if that only triggers
> when pci_resource_start() is zero.
> 
> Try pdev->rom whenever the ROM BAR didn't produce a mapping, in both
> the region info ioctl and the read path.

Here's the crazy thing, users actually update firmware through
vfio-pci.  Therefore, what's an invalid ROM image on one check, may be
a valid ROM image next round.  If we fall through to a different ROM
provider, we might actually be presenting a changing view of the
device.

I believe this is why we intentionally always report the physical ROM
BAR when present and mapped, regardless of the contents.

QEMU, of course, has the ability to take the ROM as a file and override
whatever is provided through the kernel interface.  This is how I had
previously made use of a secondary GPU on a laptop, dumping the ACPI
ROM to a file to provide directly to QEMU.

Does your GPU support a ROM BAR?  The v1 patch didn't have this and
would have exposed the physical RAM BAR if present, so I'm guessing not.
Thanks,

Alex

 
> Fixes: e021e6cbfb5a ("vfio/pci: Expose setup ROM at ROM bar when needed")
> Cc: Yunxiang Li <Yunxiang.Li@amd.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> 
> Notes (changelog):
>     v1->v2:
>         bound reads by the ROM image rather than the BAR once the
>         pdev->rom fallback is selected, so reads match the reported
>         region size (a pread spanning the image used to fail with
>         -EINVAL past the BAR size)
>         add Fixes: tag
> 
>  drivers/vfio/pci/vfio_pci_core.c |  4 +++-
>  drivers/vfio/pci/vfio_pci_rdwr.c | 32 ++++++++++++++++++++++++--------
>  2 files changed, 27 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> index 050e7542952e..630474d03e3b 100644
> --- a/drivers/vfio/pci/vfio_pci_core.c
> +++ b/drivers/vfio/pci/vfio_pci_core.c
> @@ -1128,7 +1128,9 @@ int vfio_pci_ioctl_get_region_info(struct vfio_device *core_vdev,
>  				pci_unmap_rom(pdev, io);
>  			}
>  			vfio_pci_memory_unlock_and_restore(vdev, cmd);
> -		} else if (pdev->rom && pdev->romlen) {
> +		}
> +
> +		if (!info->size && pdev->rom && pdev->romlen) {
>  			info->flags = VFIO_REGION_INFO_FLAG_READ;
>  			/* Report BAR size as power of two. */
>  			info->size = roundup_pow_of_two(pdev->romlen);
> diff --git a/drivers/vfio/pci/vfio_pci_rdwr.c b/drivers/vfio/pci/vfio_pci_rdwr.c
> index 3bfbb879a005..6e47d529cbb9 100644
> --- a/drivers/vfio/pci/vfio_pci_rdwr.c
> +++ b/drivers/vfio/pci/vfio_pci_rdwr.c
> @@ -219,7 +219,8 @@ ssize_t vfio_pci_bar_rw(struct vfio_pci_core_device *vdev, char __user *buf,
>  	int bar = VFIO_PCI_OFFSET_TO_INDEX(*ppos);
>  	size_t x_start = 0, x_end = 0;
>  	resource_size_t end;
> -	void __iomem *io;
> +	void __iomem *io = NULL;
> +	bool rom_is_mapped = false;
>  	struct resource *res = &vdev->pdev->resource[bar];
>  	ssize_t done;
>  	enum vfio_pci_io_width max_width = VFIO_PCI_IO_WIDTH_8;
> @@ -231,11 +232,6 @@ ssize_t vfio_pci_bar_rw(struct vfio_pci_core_device *vdev, char __user *buf,
>  	else
>  		return -EINVAL;
>  
> -	if (pos >= end)
> -		return -EINVAL;
> -
> -	count = min(count, (size_t)(end - pos));
> -
>  	if (bar == PCI_ROM_RESOURCE) {
>  		/*
>  		 * The ROM can fill less space than the BAR, so we start the
> @@ -244,10 +240,30 @@ ssize_t vfio_pci_bar_rw(struct vfio_pci_core_device *vdev, char __user *buf,
>  		 */
>  		if (pci_resource_start(pdev, bar)) {
>  			io = pci_map_rom(pdev, &x_start);
> -		} else {
> +			if (io)
> +				rom_is_mapped = true;
> +		}
> +
> +		/*
> +		 * The BAR can also exist but be unreadable, e.g. on a GPU
> +		 * that was never POSTed.  Fall back to the ROM image stashed
> +		 * by the boot loader; the region then follows the image size
> +		 * rather than the BAR, so bound the access only after the
> +		 * source of the ROM content is known.
> +		 */
> +		if (!io && pdev->rom && pdev->romlen) {
> +			end = roundup_pow_of_two(pdev->romlen);
>  			io = ioremap(pdev->rom, pdev->romlen);
>  			x_start = pdev->romlen;
>  		}
> +	}
> +
> +	if (pos >= end)
> +		return -EINVAL;
> +
> +	count = min(count, (size_t)(end - pos));
> +
> +	if (bar == PCI_ROM_RESOURCE) {
>  		if (!io)
>  			return -ENOMEM;
>  		x_end = end;
> @@ -283,7 +299,7 @@ ssize_t vfio_pci_bar_rw(struct vfio_pci_core_device *vdev, char __user *buf,
>  		*ppos += done;
>  
>  	if (bar == PCI_ROM_RESOURCE) {
> -		if (pci_resource_start(pdev, bar))
> +		if (rom_is_mapped)
>  			pci_unmap_rom(pdev, io);
>  		else
>  			iounmap(io);