On Mon, 23 Apr 2018 12:30:13 +0100 Jean-Philippe Brucker <jean-philippe.bruc...@arm.com> wrote:
> On Mon, Apr 16, 2018 at 10:49:01PM +0100, Jacob Pan wrote: > [...] > > +int iommu_register_device_fault_handler(struct device *dev, > > + iommu_dev_fault_handler_t > > handler, > > + void *data) > > +{ > > + struct iommu_param *param = dev->iommu_param; > > + > > + /* > > + * Device iommu_param should have been allocated when > > device is > > + * added to its iommu_group. > > + */ > > + if (!param) > > + return -EINVAL; > > + > > + /* Only allow one fault handler registered for each device > > */ > > + if (param->fault_param) > > + return -EBUSY; > > + > > + mutex_lock(¶m->lock); > > + get_device(dev); > > + param->fault_param = > > + kzalloc(sizeof(struct iommu_fault_param), > > GFP_ATOMIC); > > This can be GFP_KERNEL > yes, will change. > [...] > > +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) > > + 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->type == IOMMU_FAULT_PAGE_REQ && evt->last_req) { > > + evt_pending = kzalloc(sizeof(*evt_pending), > > GFP_ATOMIC); > > We're expecting caller to be a thread at the moment, so this could be > GFP_KERNEL too. You could also use kmemdup to remove the memcpy below > good idea. will do. > [...] > > +static inline int iommu_register_device_fault_handler(struct > > device *dev, > > + > > iommu_dev_fault_handler_t handler, > > + void *data) > > +{ > > + return 0; > > Should return -ENODEV > right. thanks. > Thanks, > Jean [Jacob Pan]