> -----Original Message-----
> From: Leon Romanovsky <[email protected]>
> Sent: 30 September 2025 10:01
> To: Alex Williamson <[email protected]>
> Cc: Jason Gunthorpe <[email protected]>; Andrew Morton <akpm@linux-
> foundation.org>; Bjorn Helgaas <[email protected]>; Christian König
> <[email protected]>; [email protected];
> [email protected]; Jens Axboe <[email protected]>; Joerg Roedel
> <[email protected]>; [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; [email protected];
> Logan Gunthorpe <[email protected]>; Marek Szyprowski
> <[email protected]>; Robin Murphy <[email protected]>;
> Sumit Semwal <[email protected]>; Vivek Kasireddy
> <[email protected]>; Will Deacon <[email protected]>
> 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 <[email protected]> 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
