Messages in this thread | | | Subject | Re: [PATCH v5 13/23] iommu: introduce device fault report API | From | Jean-Philippe Brucker <> | Date | Tue, 25 Sep 2018 15:58:41 +0100 |
| |
Hi Jacob,
Just two minor things below, that I noticed while using fault handlers for SVA. From my perspective the series is fine otherwise
On 11/05/2018 21:54, Jacob Pan wrote: > +int iommu_unregister_device_fault_handler(struct device *dev) > +{ > + struct iommu_param *param = dev->iommu_param; > + int ret = 0; > + > + if (!param) > + return -EINVAL; > + > + mutex_lock(¶m->lock);
Could we check that param->fault_param isn't NULL here, so that the driver can call this function unconditionally in a cleanup path?
> + /* we cannot unregister handler if there are pending faults */ > + if (!list_empty(¶m->fault_param->faults)) { > + ret = -EBUSY; > + goto unlock; > + } > + > + kfree(param->fault_param); > + param->fault_param = NULL; > + put_device(dev); > +unlock: > + mutex_unlock(¶m->lock); > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(iommu_unregister_device_fault_handler); > + > + > +/** > + * 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)
Should probably be ||
Thanks, Jean
| |