On Thu, Oct 30, 2025 at 02:38:36PM -0600, Alex Williamson wrote:
On Mon, 13 Oct 2025 18:26:11 +0300 Leon Romanovsky leon@kernel.org wrote:
diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c index fe247d0e2831..56b1320238a9 100644 --- a/drivers/vfio/pci/vfio_pci_core.c +++ b/drivers/vfio/pci/vfio_pci_core.c @@ -1511,6 +1520,19 @@ int vfio_pci_core_ioctl_feature(struct vfio_device *device, u32 flags, return vfio_pci_core_pm_exit(vdev, flags, arg, argsz); case VFIO_DEVICE_FEATURE_PCI_VF_TOKEN: return vfio_pci_core_feature_token(vdev, flags, arg, argsz);
- case VFIO_DEVICE_FEATURE_DMA_BUF:
if (device->ops->ioctl != vfio_pci_core_ioctl)/** Devices that overwrite general .ioctl() callback* usually do it to implement their own* VFIO_DEVICE_GET_REGION_INFO handlerm and they presentTypo, "handlerm"
Thanks, this part of code is going to be different in v6.
<...>
@@ -2482,6 +2506,10 @@ static int vfio_pci_dev_set_hot_reset(struct vfio_device_set *dev_set, ret = pci_reset_bus(pdev);
- list_for_each_entry(vdev, &dev_set->device_list, vdev.dev_set_list)
if (__vfio_pci_memory_enabled(vdev))vfio_pci_dma_buf_move(vdev, false);- vdev = list_last_entry(&dev_set->device_list, struct vfio_pci_core_device, vdev.dev_set_list);
This needs to be placed in the existing undo loop with the up_write(), otherwise it can be missed in the error case.
I'll move, but it caused me to wonder what did you want to achieve with this "vdev = list_last_entry ..." line? vdev is overwritten immediately after that line.
diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c new file mode 100644 index 000000000000..eaba010777f3 --- /dev/null +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c +static unsigned int calc_sg_nents(struct vfio_pci_dma_buf *priv,
struct dma_iova_state *state)+{
- struct phys_vec *phys_vec = priv->phys_vec;
- unsigned int nents = 0;
- u32 i;
- if (!state || !dma_use_iova(state))
for (i = 0; i < priv->nr_ranges; i++)nents += DIV_ROUND_UP(phys_vec[i].len, UINT_MAX);- else
/** In IOVA case, there is only one SG entry which spans* for whole IOVA address space, but we need to make sure* that it fits sg->length, maybe we need more.*/nents = DIV_ROUND_UP(priv->size, UINT_MAX);I think we're arguably running afoul of the coding style standard here that this is not a single simple statement and should use braces.
<...>
+err_unmap_dma:
- if (!i || !state)
; /* Do nothing */- else if (dma_use_iova(state))
dma_iova_destroy(attachment->dev, state, mapped_len, dir,attrs);- else
for_each_sgtable_dma_sg(sgt, sgl, i)dma_unmap_phys(attachment->dev, sg_dma_address(sgl),sg_dma_len(sgl), dir, attrs);Same, here for braces.
<...>
- if (!state)
; /* Do nothing */- else if (dma_use_iova(state))
dma_iova_destroy(attachment->dev, state, priv->size, dir,attrs);- else
for_each_sgtable_dma_sg(sgt, sgl, i)dma_unmap_phys(attachment->dev, sg_dma_address(sgl),sg_dma_len(sgl), dir, attrs);Here too.
I will change it, but it is worth to admit that I'm consistent in my coding style.
- sg_free_table(sgt);
- kfree(sgt);
+}
...
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h index 75100bf009ba..63214467c875 100644 --- a/include/uapi/linux/vfio.h +++ b/include/uapi/linux/vfio.h @@ -1478,6 +1478,31 @@ struct vfio_device_feature_bus_master { }; #define VFIO_DEVICE_FEATURE_BUS_MASTER 10 +/**
- Upon VFIO_DEVICE_FEATURE_GET create a dma_buf fd for the
- regions selected.
- open_flags are the typical flags passed to open(2), eg O_RDWR, O_CLOEXEC,
- etc. offset/length specify a slice of the region to create the dmabuf from.
- nr_ranges is the total number of (P2P DMA) ranges that comprise the dmabuf.
Probably worth noting that .flags should be zero, I see we enforce that. Thanks,
Added, thanks
Alex
linaro-mm-sig@lists.linaro.org
-
Leon Romanovsky