Messages in this thread | | | Subject | Re: [PATCH v10 05/11] vfio/pci: Register an iommu fault handler | From | Zenghui Yu <> | Date | Thu, 24 Sep 2020 16:49:53 +0800 |
| |
Hi Eric,
On 2020/3/21 0:19, Eric Auger wrote: > Register an IOMMU fault handler which records faults in > the DMA FAULT region ring buffer. In a subsequent patch, we > will add the signaling of a specific eventfd to allow the > userspace to be notified whenever a new fault as shown up. > > Signed-off-by: Eric Auger <eric.auger@redhat.com>
[...]
> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c > index 586b89debed5..69595c240baf 100644 > --- a/drivers/vfio/pci/vfio_pci.c > +++ b/drivers/vfio/pci/vfio_pci.c > @@ -27,6 +27,7 @@ > #include <linux/vfio.h> > #include <linux/vgaarb.h> > #include <linux/nospec.h> > +#include <linux/circ_buf.h> > > #include "vfio_pci_private.h" > > @@ -283,6 +284,38 @@ static const struct vfio_pci_regops vfio_pci_dma_fault_regops = { > .add_capability = vfio_pci_dma_fault_add_capability, > }; > > +int vfio_pci_iommu_dev_fault_handler(struct iommu_fault *fault, void *data) > +{ > + struct vfio_pci_device *vdev = (struct vfio_pci_device *)data; > + struct vfio_region_dma_fault *reg = > + (struct vfio_region_dma_fault *)vdev->fault_pages; > + struct iommu_fault *new = > + (struct iommu_fault *)(vdev->fault_pages + reg->offset + > + reg->head * reg->entry_size);
Shouldn't 'reg->head' be protected under the fault_queue_lock? Otherwise things may change behind our backs...
We shouldn't take any assumption about how IOMMU driver would report the fault (serially or in parallel), I think.
> + int head, tail, size; > + int ret = 0; > + > + if (fault->type != IOMMU_FAULT_DMA_UNRECOV) > + return -ENOENT; > + > + mutex_lock(&vdev->fault_queue_lock); > + > + head = reg->head; > + tail = reg->tail; > + size = reg->nb_entries; > + > + if (CIRC_SPACE(head, tail, size) < 1) { > + ret = -ENOSPC; > + goto unlock; > + } > + > + *new = *fault; > + reg->head = (head + 1) % size; > +unlock: > + mutex_unlock(&vdev->fault_queue_lock); > + return ret; > +} > + > #define DMA_FAULT_RING_LENGTH 512 > > static int vfio_pci_init_dma_fault_region(struct vfio_pci_device *vdev) > @@ -317,6 +350,13 @@ static int vfio_pci_init_dma_fault_region(struct vfio_pci_device *vdev) > header->entry_size = sizeof(struct iommu_fault); > header->nb_entries = DMA_FAULT_RING_LENGTH; > header->offset = sizeof(struct vfio_region_dma_fault); > + > + ret = iommu_register_device_fault_handler(&vdev->pdev->dev, > + vfio_pci_iommu_dev_fault_handler, > + vdev); > + if (ret) > + goto out; > + > return 0; > out: > kfree(vdev->fault_pages);
Thanks, Zenghui
| |