On Fri, Jun 07, 2024 at 09:17:28AM +0000, Tian, Kevin wrote:
> > From: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
> > Sent: Monday, May 27, 2024 12:05 PM
> >
> > +static ssize_t iommufd_fault_fops_read(struct file *filep, char __user *buf,
> > + size_t count, loff_t *ppos)
> > +{
> > + size_t fault_size = sizeof(struct iommu_hwpt_pgfault);
> > + struct iommufd_fault *fault = filep->private_data;
> > + struct iommu_hwpt_pgfault data;
> > + struct iommufd_device *idev;
> > + struct iopf_group *group;
> > + struct iopf_fault *iopf;
> > + size_t done = 0;
> > + int rc = 0;
> > +
> > + if (*ppos || count % fault_size)
> > + return -ESPIPE;
>
> the man page says:
>
> "If count is zero, read() returns zero and has no other results."
The above does that? 0 % X == 0
> > +
> > + mutex_lock(&fault->mutex);
> > + while (!list_empty(&fault->deliver) && count > done) {
> > + group = list_first_entry(&fault->deliver,
> > + struct iopf_group, node);
> > +
> > + if (group->fault_count * fault_size > count - done)
> > + break;
> > +
> > + rc = xa_alloc(&fault->response, &group->cookie, group,
> > + xa_limit_32b, GFP_KERNEL);
> > + if (rc)
> > + break;
> > +
> > + idev = to_iommufd_handle(group->attach_handle)->idev;
> > + list_for_each_entry(iopf, &group->faults, list) {
> > + iommufd_compose_fault_message(&iopf->fault,
> > + &data, idev,
> > + group->cookie);
> > + rc = copy_to_user(buf + done, &data, fault_size);
> > + if (rc) {
>
> 'rc' should be converted to -EFAULT.
Yes
> > + xa_erase(&fault->response, group->cookie);
> > + break;
> > + }
> > + done += fault_size;
> > + }
> > +
> > + list_del(&group->node);
> > + }
> > + mutex_unlock(&fault->mutex);
> > +
> > + return done == 0 ? rc : done;
>
> again this doesn't match the manual:
>
> "On error, -1 is returned, and errno is set appropriately. "
>
> it doesn't matter whether 'done' is 0.
It is setup so that once the list_del() below happens it is guarenteed
that the system call will return a positive result so that the
list_del'd items are always returned to userspace.
If we hit any fault here on the Nth item we should still return the
prior items and ignore the fault.
If we hit a fault on the first item then we should return the fault.
Jason
