RE: [PATCH v4 10/10] vfio/pci: Add dma-buf export support for MMIO regions

[Shameer Kolothum]

> -----Original Message-----
> From: Leon Romanovsky <leon@xxxxxxxxxx>
> Sent: 30 September 2025 10:01
> To: Alex Williamson <alex.williamson@xxxxxxxxxx>
> Cc: Jason Gunthorpe <jgg@xxxxxxxxxx>; Andrew Morton <akpm@linux-
> foundation.org>; Bjorn Helgaas <bhelgaas@xxxxxxxxxx>; Christian König
> <christian.koenig@xxxxxxx>; dri-devel@xxxxxxxxxxxxxxxxxxxxx;
> iommu@xxxxxxxxxxxxxxx; Jens Axboe <axboe@xxxxxxxxx>; Joerg Roedel
> <joro@xxxxxxxxxx>; kvm@xxxxxxxxxxxxxxx; linaro-mm-sig@xxxxxxxxxxxxxxxx;
> linux-block@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-
> media@xxxxxxxxxxxxxxx; linux-mm@xxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx;
> Logan Gunthorpe <logang@xxxxxxxxxxxx>; Marek Szyprowski
> <m.szyprowski@xxxxxxxxxxx>; Robin Murphy <robin.murphy@xxxxxxx>;
> Sumit Semwal <sumit.semwal@xxxxxxxxxx>; Vivek Kasireddy
> <vivek.kasireddy@xxxxxxxxx>; Will Deacon <will@xxxxxxxxxx>
> Subject: Re: [PATCH v4 10/10] vfio/pci: Add dma-buf export support for
> MMIO regions
> 
> External email: Use caution opening links or attachments
> 
> 
> On Mon, Sep 29, 2025 at 03:17:49PM -0600, Alex Williamson wrote:
> > On Sun, 28 Sep 2025 17:50:20 +0300
> > Leon Romanovsky <leon@xxxxxxxxxx> wrote:
> > > +static int validate_dmabuf_input(struct vfio_pci_core_device *vdev,
> > > +                            struct vfio_device_feature_dma_buf *dma_buf,
> > > +                            struct vfio_region_dma_range *dma_ranges,
> > > +                            struct p2pdma_provider **provider)
> > > +{
> > > +   struct pci_dev *pdev = vdev->pdev;
> > > +   u32 bar = dma_buf->region_index;
> > > +   resource_size_t bar_size;
> > > +   u64 sum;
> > > +   int i;
> > > +
> > > +   if (dma_buf->flags)
> > > +           return -EINVAL;
> > > +   /*
> > > +    * For PCI the region_index is the BAR number like  everything else.
> > > +    */
> > > +   if (bar >= VFIO_PCI_ROM_REGION_INDEX)
> > > +           return -ENODEV;
> > > +
> > > +   *provider = pcim_p2pdma_provider(pdev, bar);
> > > +   if (!provider)
> >
> > This needs to be IS_ERR_OR_NULL() or the function needs to settle on a
> > consistent error return value regardless of CONFIG_PCI_P2PDMA.
> 
> pcim_p2pdma_provider() doesn't return errors after split to _init() and _get().
> The more accurate check needs to be if (!*provider) and not what is written.
> 
> >
> > > +           return -EINVAL;
> > > +
> > > +   bar_size = pci_resource_len(pdev, bar);
> >
> > We get to this feature via vfio_pci_core_ioctl_feature(), which is used
> > by several variant drivers, some of which mangle the BAR size exposed
> > to the user, ex. hisi_acc.  I'm afraid this might actually be giving
> > dmabuf access to a portion of the BAR that isn't exposed otherwise.
> 
> Doe you mean that part?
> 
>   1185 static int hisi_acc_vf_qm_init(struct hisi_acc_vf_core_device
> *hisi_acc_vdev)
>   1186 {
> ...
>   1204          * Also the HiSilicon ACC VF devices supported by this driver on
>   1205          * HiSilicon hardware platforms are integrated end point devices
>   1206          * and the platform lacks the capability to perform any PCIe P2P
>   1207          * between these devices.
>   1208          */
>   1209
>   1210         vf_qm->io_base =
>   1211                 ioremap(pci_resource_start(vf_dev,
> VFIO_PCI_BAR2_REGION_INDEX),
>   1212                         pci_resource_len(vf_dev,
> VFIO_PCI_BAR2_REGION_INDEX));
>   1213         if (!vf_qm->io_base)
>   1214                 return -EIO;
>   1215

This is where hisi_acc reports a different BAR size as it tries to hide
the migration control region from Guest access.

static long hisi_acc_vfio_pci_ioctl(struct vfio_device *core_vdev, unsigned int cmd,
				    unsigned long arg)
{
	...
		if (info.index == VFIO_PCI_BAR2_REGION_INDEX) {
			info.offset = VFIO_PCI_INDEX_TO_OFFSET(info.index);

			/*
			 * ACC VF dev BAR2 region consists of both functional
			 * register space and migration control register space.
			 * Report only the functional region to Guest.
			 */
			info.size = pci_resource_len(pdev, info.index) / 2;

			info.flags = VFIO_REGION_INFO_FLAG_READ |
					VFIO_REGION_INFO_FLAG_WRITE |
					VFIO_REGION_INFO_FLAG_MMAP;

			return copy_to_user((void __user *)arg, &info, minsz) ?
					    -EFAULT : 0;
		}
	}
	return vfio_pci_core_ioctl(core_vdev, cmd, arg);
}

> According to the comment, it doesn't support p2p and in any case we will
> fail that platform in vfio_pci_dma_buf_attach() by taking "default" case:

Yes. No P2P for this device. But variant drivers can override size exposed to
userspace like this one.

Thanks,
Shameer