On 18/02/2019 13:54, Eric Auger wrote: [...]> +/** > + * iommu_register_device_fault_handler() - Register a device fault handler > + * @dev: the device > + * @handler: the fault handler > + * @data: private data passed as argument to the handler > + * > + * When an IOMMU fault event is received, call this handler with the fault > event > + * and data as argument. The handler should return 0 on success. If the > fault is > + * recoverable (IOMMU_FAULT_PAGE_REQ), the handler can also complete > + * the fault by calling iommu_page_response() with one of the following > + * response code: > + * - IOMMU_PAGE_RESP_SUCCESS: retry the translation > + * - IOMMU_PAGE_RESP_INVALID: terminate the fault > + * - IOMMU_PAGE_RESP_FAILURE: terminate the fault and stop reporting > + * page faults if possible.
The comment refers to function and values that haven't been defined yet. Either the page_response() patch should come before, or we need to split this patch. Something I missed before: if the handler fails (returns != 0) it should complete the fault by calling iommu_page_response(), if we're not doing it in iommu_report_device_fault(). It should be indicated in this comment. It's safe for the handler to call page_response() since we're not holding fault_param->lock when calling the handler. > + * > + * Return 0 if the fault handler was installed successfully, or an error. > + */ [...] > +/** > + * iommu_report_device_fault() - Report fault event to device > + * @dev: the device > + * @evt: fault event data > + * > + * Called by IOMMU model specific drivers when fault is detected, typically > + * in a threaded IRQ handler. > + * > + * Return 0 on success, or an error. > + */ > +int iommu_report_device_fault(struct device *dev, struct iommu_fault_event > *evt) > +{ > + int ret = 0; > + struct iommu_fault_event *evt_pending; > + struct iommu_fault_param *fparam; > + > + /* iommu_param is allocated when device is added to group */ > + if (!dev->iommu_param | !evt) Typo: || Thanks, Jean > + return -EINVAL; > + /* we only report device fault if there is a handler registered */ > + mutex_lock(&dev->iommu_param->lock); > + if (!dev->iommu_param->fault_param || > + !dev->iommu_param->fault_param->handler) { > + ret = -EINVAL; > + goto done_unlock; > + } > + fparam = dev->iommu_param->fault_param; > + if (evt->fault.type == IOMMU_FAULT_PAGE_REQ && > + evt->fault.prm.flags & IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE) { > + evt_pending = kmemdup(evt, sizeof(struct iommu_fault_event), > + GFP_KERNEL); > + if (!evt_pending) { > + ret = -ENOMEM; > + goto done_unlock; > + } > + mutex_lock(&fparam->lock); > + list_add_tail(&evt_pending->list, &fparam->faults); > + mutex_unlock(&fparam->lock); > + } > + ret = fparam->handler(evt, fparam->data); > +done_unlock: > + mutex_unlock(&dev->iommu_param->lock); > + return ret; > +} > +EXPORT_SYMBOL_GPL(iommu_report_device_fault); [...] _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm