Re: [PATCH v2 4/6] iommufd: Deliver fault messages to user space

On Thu, Oct 26, 2023 at 10:49:28AM +0800, Lu Baolu wrote:

+static ssize_t hwpt_fault_fops_write(struct file *filep,

• 		     const char __user *buf,
  

• 		     size_t count, loff_t *ppos)
  

+{

• size_t response_size = sizeof(struct iommu_hwpt_page_response);
• struct hw_pgtable_fault *fault = filep->private_data;
• struct iommu_hwpt_page_response response;
• struct iommufd_hw_pagetable *hwpt;
• struct iopf_group *iter, *group;
• struct iommufd_device *idev;
• size_t done = 0;
• int rc = 0;
• if (*ppos || count % response_size)

• return -ESPIPE;
  

• mutex_lock(&fault->mutex);
• while (!list_empty(&fault->response) && count > done) {

• rc = copy_from_user(&response, buf + done, response_size);
  

• if (rc)
  

• 	break;
  

• /* Get the device that this response targets at. */
  

• idev = container_of(iommufd_get_object(fault->ictx,
  

• 				       response.dev_id,
  

• 				       IOMMUFD_OBJ_DEVICE),
  

• 		    struct iommufd_device, obj);
  

• if (IS_ERR(idev)) {
  

• 	rc = PTR_ERR(idev);
  

• 	break;
  

• }
  

See here it might be better to have a per-fd list of outstanding faults per-fd and then the cookie would just index that list, then you get everything in one shot instead of having to do a xarray looking and then a linear list search

+static const struct file_operations hwpt_fault_fops = {

• .owner = THIS_MODULE,
• .read = hwpt_fault_fops_read,
• .write = hwpt_fault_fops_write,

+};

nonseekable_open() behavior should be integrated into this

+static int hw_pagetable_get_fault_fd(struct hw_pgtable_fault *fault) +{

• struct file *filep;
• int fdno;
• fdno = get_unused_fd_flags(O_CLOEXEC);
• if (fdno < 0)

• return fdno;
  

• filep = anon_inode_getfile("[iommufd-pgfault]", &hwpt_fault_fops,

• 		   fault, O_RDWR);
  

• if (IS_ERR(filep)) {

• put_unused_fd(fdno);
  

• return PTR_ERR(filep);
  

• }
• fd_install(fdno, filep);
• fault->fault_file = filep;
• fault->fault_fd = fdno;

fd_install must be the very last thing before returning success from a system call because we cannot undo it.

There are other failure paths before here and the final return

Jason