[PATCH] vfio/pci: read ROM via ACPI _ROM when hardware ROM is unreadable

[PATCH] vfio/pci: read ROM via ACPI _ROM when hardware ROM is unreadable

[Michael S. Tsirkin]

Some GPUs (e.g. NVIDIA Quadro P2000 Mobile in a laptop) have a ROM
BAR that exists but is unreadable because the GPU was never POSTed
by the system BIOS. On these systems the VBIOS is available via the
ACPI _ROM method provided by the platform firmware.

Add an ACPI _ROM fallback in the VFIO PCI ROM region handler. When
both pci_map_rom() and the platform ROM (pdev->rom) fail, evaluate
the device's ACPI _ROM method to read the ROM.

Assisted-by: LLM
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

Hi Alex,
this makes this box I found in the dumpster go brr) Pls take a look.


 drivers/vfio/pci/vfio_pci_core.c |   9 +++
 drivers/vfio/pci/vfio_pci_priv.h |   9 +++
 drivers/vfio/pci/vfio_pci_rdwr.c | 124 +++++++++++++++++++++++++++++++
 3 files changed, 142 insertions(+)

diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index 6757054e9d87..53f53653421e 100644
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -1193,6 +1193,15 @@ int vfio_pci_ioctl_get_region_info(struct vfio_device *core_vdev,
 			info->size = roundup_pow_of_two(pdev->romlen);
 		}
 
+		if (!info->size) {
+			size_t acpi_size = vfio_pci_acpi_rom_size(pdev);
+
+			if (acpi_size) {
+				info->flags = VFIO_REGION_INFO_FLAG_READ;
+				info->size = roundup_pow_of_two(acpi_size);
+			}
+		}
+
 		break;
 	}
 	case VFIO_PCI_VGA_REGION_INDEX:
diff --git a/drivers/vfio/pci/vfio_pci_priv.h b/drivers/vfio/pci/vfio_pci_priv.h
index 4e7162234a2e..07c23507832c 100644
--- a/drivers/vfio/pci/vfio_pci_priv.h
+++ b/drivers/vfio/pci/vfio_pci_priv.h
@@ -44,6 +44,15 @@ ssize_t vfio_pci_config_rw_single(struct vfio_pci_core_device *vdev,
 ssize_t vfio_pci_bar_rw(struct vfio_pci_core_device *vdev, char __user *buf,
 			size_t count, loff_t *ppos, bool iswrite);
 
+#ifdef CONFIG_ACPI
+size_t vfio_pci_acpi_rom_size(struct pci_dev *pdev);
+#else
+static inline size_t vfio_pci_acpi_rom_size(struct pci_dev *pdev)
+{
+	return 0;
+}
+#endif
+
 #ifdef CONFIG_VFIO_PCI_VGA
 ssize_t vfio_pci_vga_rw(struct vfio_pci_core_device *vdev, char __user *buf,
 			size_t count, loff_t *ppos, bool iswrite);
diff --git a/drivers/vfio/pci/vfio_pci_rdwr.c b/drivers/vfio/pci/vfio_pci_rdwr.c
index 7f14dd46de17..654af45e8edc 100644
--- a/drivers/vfio/pci/vfio_pci_rdwr.c
+++ b/drivers/vfio/pci/vfio_pci_rdwr.c
@@ -10,6 +10,7 @@
  * Author: Tom Lyon, pugs@cisco.com
  */
 
+#include <linux/acpi.h>
 #include <linux/fs.h>
 #include <linux/pci.h>
 #include <linux/uaccess.h>
@@ -198,6 +199,112 @@ ssize_t vfio_pci_core_do_io_rw(struct vfio_pci_core_device *vdev, bool test_mem,
 }
 EXPORT_SYMBOL_GPL(vfio_pci_core_do_io_rw);
 
+#ifdef CONFIG_ACPI
+static int vfio_pci_acpi_rom_read(struct pci_dev *pdev, void *buf,
+				  loff_t offset, size_t length)
+{
+	acpi_handle handle, rom_handle;
+	acpi_status status;
+	union acpi_object rom_arg[2];
+	struct acpi_object_list rom_args;
+	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+	union acpi_object *obj;
+
+	handle = ACPI_HANDLE(&pdev->dev);
+	if (!handle)
+		return -ENODEV;
+
+	status = acpi_get_handle(handle, "_ROM", &rom_handle);
+	if (ACPI_FAILURE(status))
+		return -ENODEV;
+
+	rom_args.count = 2;
+	rom_args.pointer = rom_arg;
+	rom_arg[0].type = ACPI_TYPE_INTEGER;
+	rom_arg[0].integer.value = offset;
+	rom_arg[1].type = ACPI_TYPE_INTEGER;
+	rom_arg[1].integer.value = length;
+
+	status = acpi_evaluate_object(rom_handle, NULL, &rom_args, &buffer);
+	if (ACPI_FAILURE(status))
+		return -EIO;
+
+	obj = buffer.pointer;
+	if (obj->type != ACPI_TYPE_BUFFER) {
+		kfree(buffer.pointer);
+		return -EIO;
+	}
+	length = min(length, (size_t)obj->buffer.length);
+	memcpy(buf, obj->buffer.pointer, length);
+	kfree(buffer.pointer);
+	return length;
+}
+
+static ssize_t vfio_pci_acpi_rom_rw(struct vfio_pci_core_device *vdev,
+				    char __user *buf, size_t count,
+				    loff_t pos, bool iswrite)
+{
+	struct pci_dev *pdev = vdev->pdev;
+	void *chunk;
+	ssize_t done = 0;
+	int ret;
+
+	if (iswrite)
+		return -EINVAL;
+
+	chunk = kmalloc(0x1000, GFP_KERNEL);
+	if (!chunk)
+		return -ENOMEM;
+
+	while (count) {
+		size_t n = min(count, (size_t)0x1000);
+
+		ret = vfio_pci_acpi_rom_read(pdev, chunk, pos, n);
+		if (ret <= 0) {
+			if (ret < 0 && !done)
+				done = ret;
+			break;
+		}
+
+		if (copy_to_user(buf, chunk, ret)) {
+			done = done ? done : -EFAULT;
+			break;
+		}
+
+		buf += ret;
+		pos += ret;
+		done += ret;
+		count -= ret;
+
+		if (ret < n)
+			break;
+	}
+
+	kfree(chunk);
+	return done;
+}
+
+size_t vfio_pci_acpi_rom_size(struct pci_dev *pdev)
+{
+	void *buf;
+	size_t size = 0;
+	int ret;
+
+	buf = kmalloc(0x1000, GFP_KERNEL);
+	if (!buf)
+		return 0;
+
+	do {
+		ret = vfio_pci_acpi_rom_read(pdev, buf, size, 0x1000);
+		if (ret > 0)
+			size += ret;
+	} while (ret == 0x1000);
+
+	kfree(buf);
+	return size;
+}
+#endif /* CONFIG_ACPI */
+
 ssize_t vfio_pci_bar_rw(struct vfio_pci_core_device *vdev, char __user *buf,
 			size_t count, loff_t *ppos, bool iswrite)
 {
@@ -215,6 +322,14 @@ ssize_t vfio_pci_bar_rw(struct vfio_pci_core_device *vdev, char __user *buf,
 		end = pci_resource_len(pdev, bar);
 	else if (bar == PCI_ROM_RESOURCE && pdev->rom && pdev->romlen)
 		end = roundup_pow_of_two(pdev->romlen);
+#ifdef CONFIG_ACPI
+	else if (bar == PCI_ROM_RESOURCE) {
+		end = vfio_pci_acpi_rom_size(pdev);
+		if (!end)
+			return -EINVAL;
+		end = roundup_pow_of_two(end);
+	}
+#endif
 	else
 		return -EINVAL;
 
@@ -235,6 +350,15 @@ ssize_t vfio_pci_bar_rw(struct vfio_pci_core_device *vdev, char __user *buf,
 			io = ioremap(pdev->rom, pdev->romlen);
 			x_start = pdev->romlen;
 		}
+#ifdef CONFIG_ACPI
+		if (!io) {
+			done = vfio_pci_acpi_rom_rw(vdev, buf, count,
+						    pos, iswrite);
+			if (done >= 0)
+				*ppos += done;
+			return done;
+		}
+#endif
 		if (!io)
 			return -ENOMEM;
 		x_end = end;
-- 
MST

Re: [PATCH] vfio/pci: read ROM via ACPI _ROM when hardware ROM is unreadable

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] vfio/pci: O(N^2) overhead in ACPI ROM read via recalculating size
- [High] vfio/pci: infinite loop in ACPI ROM size calculation
- [High] pci: NULL pointer dereference in vfio_pci_acpi_rom_read() when _ROM returns no object
- [Medium] vfio/pci: ACPI ROM reads truncated by unreadable physical BAR size
- [Medium] vfio/pci: ACPI ROM reads missing 0xFF padding
--

commit ef01db8fb34fde6cd15f6043dd6655f51df61bf4
Author: Michael S. Tsirkin <mst@redhat.com>

vfio/pci: read ROM via ACPI _ROM when hardware ROM is unreadable

Some GPUs (e.g. NVIDIA Quadro P2000 Mobile in a laptop) have a ROM
BAR that exists but is unreadable because the GPU was never POSTed
by the system BIOS. On these systems the VBIOS is available via the
ACPI _ROM method provided by the platform firmware.

> diff --git a/drivers/vfio/pci/vfio_pci_rdwr.c b/drivers/vfio/pci/vfio_pci_rdwr.c
> index df88480876815..9bc0a7df4fd69 100644
> --- a/drivers/vfio/pci/vfio_pci_rdwr.c
> +++ b/drivers/vfio/pci/vfio_pci_rdwr.c
[ ... ]
> @@ -198,6 +199,112 @@ ssize_t vfio_pci_core_do_io_rw(struct vfio_pci_core_device *vdev, bool test_mem,
>  }
>  EXPORT_SYMBOL_GPL(vfio_pci_core_do_io_rw);
>  
> +#ifdef CONFIG_ACPI
> +static int vfio_pci_acpi_rom_read(struct pci_dev *pdev, void *buf,
> +				  loff_t offset, size_t length)
> +{
[ ... ]
> +	status = acpi_evaluate_object(rom_handle, NULL, &rom_args, &buffer);
> +	if (ACPI_FAILURE(status))
> +		return -EIO;
> +
> +	obj = buffer.pointer;
> +	if (obj->type != ACPI_TYPE_BUFFER) {

[Severity: High]
If a system's ACPI BIOS is flawed and the _ROM method executes successfully
but returns no object, acpi_evaluate_object() returns AE_OK but leaves
buffer.pointer as NULL.

Will this unconditionally assign and dereference a NULL pointer via
obj->type, crashing the kernel when vfio_pci_acpi_rom_size() is called
during region info ioctl?

> +		kfree(buffer.pointer);
> +		return -EIO;
> +	}
[ ... ]
> +size_t vfio_pci_acpi_rom_size(struct pci_dev *pdev)
> +{
> +	void *buf;
> +	size_t size = 0;
> +	int ret;
> +
> +	buf = kmalloc(0x1000, GFP_KERNEL);
> +	if (!buf)
> +		return 0;
> +
> +	do {
> +		ret = vfio_pci_acpi_rom_read(pdev, buf, size, 0x1000);
> +		if (ret > 0)
> +			size += ret;
> +	} while (ret == 0x1000);

[Severity: High]
Does this un-bounded loop rely entirely on firmware behavior to terminate?
If a buggy or malicious ACPI _ROM method ignores the offset parameter and
always returns a full 4KB buffer, will this cause an infinite loop and a
kernel soft lockup?

Additionally, this loop evaluates the ACPI method repeatedly in 4KB chunks
to read the entire ROM just to determine its total size.  Since
vfio_pci_acpi_rom_size() is called in vfio_pci_bar_rw() on every read,
will this cause severe performance degradation?  For example, reading a
128KB ROM in 4-byte chunks would execute the full ROM reading loop
via ACPI 32,768 times.

> +
> +	kfree(buf);
> +	return size;
> +}
> +#endif /* CONFIG_ACPI */
> +
>  ssize_t vfio_pci_bar_rw(struct vfio_pci_core_device *vdev, char __user *buf,
>  			size_t count, loff_t *ppos, bool iswrite)
>  {
> @@ -215,6 +322,14 @@ ssize_t vfio_pci_bar_rw(struct vfio_pci_core_device *vdev, char __user *buf,
>  		end = pci_resource_len(pdev, bar);

[Severity: Medium]
This sets the hard limit for reads based on the physical PCI resource length.

If pci_map_rom() fails later in this function, we fallback to ACPI ROM.
However, if the ACPI ROM is larger than the unreadable physical ROM BAR size,
will reads beyond the physical BAR size be improperly rejected with -EINVAL,
even though vfio_pci_ioctl_get_region_info() correctly reported the larger
acpi_size?

>  	else if (bar == PCI_ROM_RESOURCE && pdev->rom && pdev->romlen)
>  		end = roundup_pow_of_two(pdev->romlen);
> +#ifdef CONFIG_ACPI
> +	else if (bar == PCI_ROM_RESOURCE) {
> +		end = vfio_pci_acpi_rom_size(pdev);

[Severity: High]
As mentioned above, is it too expensive to invoke this size calculation
for every user read?  Should the ROM size be cached during region
initialization instead?

> +		if (!end)
> +			return -EINVAL;
> +		end = roundup_pow_of_two(end);
> +	}
> +#endif
>  	else
>  		return -EINVAL;
>  
[ ... ]
> @@ -235,6 +350,15 @@ ssize_t vfio_pci_bar_rw(struct vfio_pci_core_device *vdev, char __user *buf,
>  			io = ioremap(pdev->rom, pdev->romlen);
>  			x_start = pdev->romlen;
>  		}
> +#ifdef CONFIG_ACPI
> +		if (!io) {
> +			done = vfio_pci_acpi_rom_rw(vdev, buf, count,
> +						    pos, iswrite);
> +			if (done >= 0)
> +				*ppos += done;
> +			return done;
> +		}
> +#endif

[Severity: Medium]
When reading past the actual size of the ACPI ROM but within the reported
roundup_pow_of_two region size, does this bypass the standard VFIO
padding logic?

If vfio_pci_acpi_rom_rw() returns a short read or 0 (EOF), will this skip
the vfio_pci_core_do_io_rw() logic that safely pads remaining space with
0xFF, potentially breaking virtualization software expecting standard
hardware behavior?

>  		if (!io)
>  			return -ENOMEM;
>  		x_end = end;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/31aa2fb273d7ff7ea535c74e0780e279aa9a119c.1791933554.git.mst@redhat.com?part=1

Re: [PATCH] vfio/pci: read ROM via ACPI _ROM when hardware ROM is unreadable

[Michael S. Tsirkin]

On Tue, Oct 13, 2026 at 11:36:05PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
> - [High] vfio/pci: O(N^2) overhead in ACPI ROM read via recalculating size
> - [High] vfio/pci: infinite loop in ACPI ROM size calculation
> - [High] pci: NULL pointer dereference in vfio_pci_acpi_rom_read() when _ROM returns no object
> - [Medium] vfio/pci: ACPI ROM reads truncated by unreadable physical BAR size
> - [Medium] vfio/pci: ACPI ROM reads missing 0xFF padding
> --
> 
> commit ef01db8fb34fde6cd15f6043dd6655f51df61bf4
> Author: Michael S. Tsirkin <mst@redhat.com>
> 
> vfio/pci: read ROM via ACPI _ROM when hardware ROM is unreadable
> 
> Some GPUs (e.g. NVIDIA Quadro P2000 Mobile in a laptop) have a ROM
> BAR that exists but is unreadable because the GPU was never POSTed
> by the system BIOS. On these systems the VBIOS is available via the
> ACPI _ROM method provided by the platform firmware.
> 
> > diff --git a/drivers/vfio/pci/vfio_pci_rdwr.c b/drivers/vfio/pci/vfio_pci_rdwr.c
> > index df88480876815..9bc0a7df4fd69 100644
> > --- a/drivers/vfio/pci/vfio_pci_rdwr.c
> > +++ b/drivers/vfio/pci/vfio_pci_rdwr.c
> [ ... ]
> > @@ -198,6 +199,112 @@ ssize_t vfio_pci_core_do_io_rw(struct vfio_pci_core_device *vdev, bool test_mem,
> >  }
> >  EXPORT_SYMBOL_GPL(vfio_pci_core_do_io_rw);
> >  
> > +#ifdef CONFIG_ACPI
> > +static int vfio_pci_acpi_rom_read(struct pci_dev *pdev, void *buf,
> > +				  loff_t offset, size_t length)
> > +{
> [ ... ]
> > +	status = acpi_evaluate_object(rom_handle, NULL, &rom_args, &buffer);
> > +	if (ACPI_FAILURE(status))
> > +		return -EIO;
> > +
> > +	obj = buffer.pointer;
> > +	if (obj->type != ACPI_TYPE_BUFFER) {
> 
> [Severity: High]
> If a system's ACPI BIOS is flawed and the _ROM method executes successfully
> but returns no object, acpi_evaluate_object() returns AE_OK but leaves
> buffer.pointer as NULL.
> 
> Will this unconditionally assign and dereference a NULL pointer via
> obj->type, crashing the kernel when vfio_pci_acpi_rom_size() is called
> during region info ioctl?

i can harden it sure.

> > +		kfree(buffer.pointer);
> > +		return -EIO;
> > +	}
> [ ... ]
> > +size_t vfio_pci_acpi_rom_size(struct pci_dev *pdev)
> > +{
> > +	void *buf;
> > +	size_t size = 0;
> > +	int ret;
> > +
> > +	buf = kmalloc(0x1000, GFP_KERNEL);
> > +	if (!buf)
> > +		return 0;
> > +
> > +	do {
> > +		ret = vfio_pci_acpi_rom_read(pdev, buf, size, 0x1000);
> > +		if (ret > 0)
> > +			size += ret;
> > +	} while (ret == 0x1000);
> 
> [Severity: High]
> Does this un-bounded loop rely entirely on firmware behavior to terminate?
> If a buggy or malicious ACPI _ROM method ignores the offset parameter and
> always returns a full 4KB buffer, will this cause an infinite loop and a
> kernel soft lockup?

i can limit to 16M as per pci spec.

> Additionally, this loop evaluates the ACPI method repeatedly in 4KB chunks
> to read the entire ROM just to determine its total size.

i will cache it.

>  Since
> vfio_pci_acpi_rom_size() is called in vfio_pci_bar_rw() on every read,
> will this cause severe performance degradation?  For example, reading a
> 128KB ROM in 4-byte chunks would execute the full ROM reading loop
> via ACPI 32,768 times.
> 
> > +
> > +	kfree(buf);
> > +	return size;
> > +}
> > +#endif /* CONFIG_ACPI */
> > +
> >  ssize_t vfio_pci_bar_rw(struct vfio_pci_core_device *vdev, char __user *buf,
> >  			size_t count, loff_t *ppos, bool iswrite)
> >  {
> > @@ -215,6 +322,14 @@ ssize_t vfio_pci_bar_rw(struct vfio_pci_core_device *vdev, char __user *buf,
> >  		end = pci_resource_len(pdev, bar);
> 
> [Severity: Medium]
> This sets the hard limit for reads based on the physical PCI resource length.
> If pci_map_rom() fails later in this function, we fallback to ACPI ROM.
> However, if the ACPI ROM is larger than the unreadable physical ROM BAR size,
> will reads beyond the physical BAR size be improperly rejected with -EINVAL,
> even though vfio_pci_ioctl_get_region_info() correctly reported the larger
> acpi_size?

end will be 0 then.


> 
> >  	else if (bar == PCI_ROM_RESOURCE && pdev->rom && pdev->romlen)
> >  		end = roundup_pow_of_two(pdev->romlen);
> > +#ifdef CONFIG_ACPI
> > +	else if (bar == PCI_ROM_RESOURCE) {
> > +		end = vfio_pci_acpi_rom_size(pdev);
> 
> [Severity: High]
> As mentioned above, is it too expensive to invoke this size calculation
> for every user read?  Should the ROM size be cached during region
> initialization instead?


it should.

> 
> > +		if (!end)
> > +			return -EINVAL;
> > +		end = roundup_pow_of_two(end);
> > +	}
> > +#endif
> >  	else
> >  		return -EINVAL;
> >  
> [ ... ]
> > @@ -235,6 +350,15 @@ ssize_t vfio_pci_bar_rw(struct vfio_pci_core_device *vdev, char __user *buf,
> >  			io = ioremap(pdev->rom, pdev->romlen);
> >  			x_start = pdev->romlen;
> >  		}
> > +#ifdef CONFIG_ACPI
> > +		if (!io) {
> > +			done = vfio_pci_acpi_rom_rw(vdev, buf, count,
> > +						    pos, iswrite);
> > +			if (done >= 0)
> > +				*ppos += done;
> > +			return done;
> > +		}
> > +#endif
> 
> [Severity: Medium]
> When reading past the actual size of the ACPI ROM but within the reported
> roundup_pow_of_two region size, does this bypass the standard VFIO
> padding logic?
> 
> If vfio_pci_acpi_rom_rw() returns a short read or 0 (EOF), will this skip
> the vfio_pci_core_do_io_rw() logic that safely pads remaining space with
> 0xFF, potentially breaking virtualization software expecting standard
> hardware behavior?

right better copy

> >  		if (!io)
> >  			return -ENOMEM;
> >  		x_end = end;
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/31aa2fb273d7ff7ea535c74e0780e279aa9a119c.1791933554.git.mst@redhat.com?part=1