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; >> +}