Re: [PATCH v8 26/29] vfio-pci: Register an iommu fault handler

Auger Eric
2019-06-07T14:19:51Z

Hi Jean,

On 6/7/19 2:48 PM, Jean-Philippe Brucker wrote:
> On 26/05/2019 17:10, Eric Auger wrote:
>> +int vfio_pci_iommu_dev_fault_handler(struct iommu_fault_event *evt, void *data)
>> +{
>> + struct vfio_pci_device *vdev = (struct vfio_pci_device *) data;
>> + struct vfio_region_fault_prod *prod_region =
>> + (struct vfio_region_fault_prod *)vdev->fault_pages;
>> + struct vfio_region_fault_cons *cons_region =
>> + (struct vfio_region_fault_cons *)(vdev->fault_pages + 2 * PAGE_SIZE);
>> + struct iommu_fault *new =
>> + (struct iommu_fault *)(vdev->fault_pages + prod_region->offset +
>> + prod_region->prod * prod_region->entry_size);
>> + int prod, cons, size;
>> +
>> + mutex_lock(&vdev->fault_queue_lock);
>> +
>> + if (!vdev->fault_abi)
>> + goto unlock;
>> +
>> + prod = prod_region->prod;
>> + cons = cons_region->cons;
>> + size = prod_region->nb_entries;
>> +
>> + if (CIRC_SPACE(prod, cons, size) < 1)
>> + goto unlock;
>> +
>> + *new = evt->fault;
>
> Could you check fault.type and return an error if it's not UNRECOV here?
> If the fault is recoverable (very unlikely since the PRI capability is
> disabled, but allowed) and we return an error here, then the caller
> takes care of completing the fault. If we forward it to the guest
> instead, the producer will wait indefinitely for a response.
Sure I will add that check in the next version.

Thanks

Eric
>
> Thanks,
> Jean
>
>> + prod = (prod + 1) % size;
>> + prod_region->prod = prod;
>> + mutex_unlock(&vdev->fault_queue_lock);
>> +
>> + mutex_lock(&vdev->igate);
>> + if (vdev->dma_fault_trigger)
>> + eventfd_signal(vdev->dma_fault_trigger, 1);
>> + mutex_unlock(&vdev->igate);
>> + return 0;
>> +
>> +unlock:
>> + mutex_unlock(&vdev->fault_queue_lock);
>> + return -EINVAL;
>> +}