On Thu, 12 Dec 2024 15:50:50 -0500
Yunxiang Li <Yunxiang.Li@xxxxxxx> wrote:
> If ROM bar is missing for any reason, we can fallback to using pdev->rom
> to expose the ROM content to the guest. This fixes some passthrough use
> cases where the upstream bridge does not have enough address window.
>
> Signed-off-by: Yunxiang Li <Yunxiang.Li@xxxxxxx>
> ---
> drivers/vfio/pci/vfio_pci_config.c | 4 ++++
> drivers/vfio/pci/vfio_pci_core.c | 35 +++++++++++++++---------------
> drivers/vfio/pci/vfio_pci_rdwr.c | 14 ++++++++++--
> 3 files changed, 34 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c
> index e41c3a965663e..4c673d842fb35 100644
> --- a/drivers/vfio/pci/vfio_pci_config.c
> +++ b/drivers/vfio/pci/vfio_pci_config.c
> @@ -511,6 +511,10 @@ static void vfio_bar_fixup(struct vfio_pci_core_device *vdev)
> mask = ~(pci_resource_len(pdev, PCI_ROM_RESOURCE) - 1);
> mask |= PCI_ROM_ADDRESS_ENABLE;
> *vbar &= cpu_to_le32((u32)mask);
> + } else if (pdev->rom && pdev->romlen) {
> + mask = ~(roundup_pow_of_two(pdev->romlen) - 1);
> + mask |= PCI_ROM_ADDRESS_ENABLE;
> + *vbar &= cpu_to_le32((u32)mask);
> } else {
> *vbar = 0;
> }
> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> index b49dd9cdc072a..3120c1e9f22cb 100644
> --- a/drivers/vfio/pci/vfio_pci_core.c
> +++ b/drivers/vfio/pci/vfio_pci_core.c
> @@ -1049,30 +1049,31 @@ static int vfio_pci_ioctl_get_region_info(struct vfio_pci_core_device *vdev,
> break;
> case VFIO_PCI_ROM_REGION_INDEX: {
> void __iomem *io;
> - size_t size;
> + size_t dont_care;
It still receives the size even if we don't consume it.
> u16 cmd;
>
> info.offset = VFIO_PCI_INDEX_TO_OFFSET(info.index);
> info.flags = 0;
> + info.size = 0;
>
> - /* Report the BAR size, not the ROM size */
> - info.size = pci_resource_len(pdev, info.index);
> - if (!info.size)
> - break;
> -
> - /*
> - * Is it really there? Enable memory decode for implicit access
> - * in pci_map_rom().
> - */
> - cmd = vfio_pci_memory_lock_and_enable(vdev);
> - io = pci_map_rom(pdev, &size);
> - if (io) {
> + if (pci_resource_start(pdev, PCI_ROM_RESOURCE)) {
> + /* Check ROM content is valid. Need to enable memory
Incorrect comment style.
> + * decode for ROM access in pci_map_rom().
> + */
> + cmd = vfio_pci_memory_lock_and_enable(vdev);
> + io = pci_map_rom(pdev, &dont_care);
> + if (io) {
> + info.flags = VFIO_REGION_INFO_FLAG_READ;
> + /* Report the BAR size, not the ROM size. */
> + info.size = pci_resource_len(pdev, PCI_ROM_RESOURCE);
> + pci_unmap_rom(pdev, io);
> + }
> + vfio_pci_memory_unlock_and_restore(vdev, cmd);
> + } else if (pdev->rom && pdev->romlen) {
> info.flags = VFIO_REGION_INFO_FLAG_READ;
> - pci_unmap_rom(pdev, io);
> - } else {
> - info.size = 0;
> + /* Report BAR size as power of two. */
> + info.size = roundup_pow_of_two(pdev->romlen);
> }
> - vfio_pci_memory_unlock_and_restore(vdev, cmd);
>
> break;
> }
> diff --git a/drivers/vfio/pci/vfio_pci_rdwr.c b/drivers/vfio/pci/vfio_pci_rdwr.c
> index 4bed9fd5af50f..4ea983cf499d9 100644
> --- a/drivers/vfio/pci/vfio_pci_rdwr.c
> +++ b/drivers/vfio/pci/vfio_pci_rdwr.c
> @@ -243,6 +243,8 @@ ssize_t vfio_pci_bar_rw(struct vfio_pci_core_device *vdev, char __user *buf,
>
> if (pci_resource_start(pdev, bar))
> end = pci_resource_len(pdev, bar);
> + else if (bar == PCI_ROM_RESOURCE && pdev->rom && pdev->romlen)
> + end = roundup_pow_of_two(pdev->romlen);
> else
> return -EINVAL;
>
> @@ -259,7 +261,12 @@ ssize_t vfio_pci_bar_rw(struct vfio_pci_core_device *vdev, char __user *buf,
> * excluded range at the end of the actual ROM. This makes
> * filling large ROM BARs much faster.
> */
> - io = pci_map_rom(pdev, &x_start);
> + if (pci_resource_start(pdev, bar)) {
> + io = pci_map_rom(pdev, &x_start);
> + } else {
> + io = ioremap(pdev->rom, pdev->romlen);
> + x_start = pdev->romlen;
> + }
> if (!io)
> return -ENOMEM;
> x_end = end;
> @@ -267,7 +274,10 @@ ssize_t vfio_pci_bar_rw(struct vfio_pci_core_device *vdev, char __user *buf,
> done = vfio_pci_core_do_io_rw(vdev, 1, io, buf, pos,
> count, x_start, x_end, 0);
>
> - pci_unmap_rom(pdev, io);
> + if (pci_resource_start(pdev, bar))
> + pci_unmap_rom(pdev, io);
> + else
> + iounmap(io);
> } else {
> done = vfio_pci_core_setup_barmap(vdev, bar);
> if (done)
It's not clear why the refactoring of the previous patch was really
necessary simply to have an alternate map and unmap path. Thanks,
Alex
