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

-----Original Message----- From: Lu Baolu baolu.lu@linux.intel.com Sent: Thursday, October 26, 2023 3:49 AM To: Jason Gunthorpe jgg@ziepe.ca; Kevin Tian kevin.tian@intel.com; Joerg Roedel joro@8bytes.org; Will Deacon will@kernel.org; Robin Murphy robin.murphy@arm.com; Jean-Philippe Brucker jean-philippe@linaro.org; Nicolin Chen nicolinc@nvidia.com; Yi Liu yi.l.liu@intel.com; Jacob Pan jacob.jun.pan@linux.intel.com Cc: iommu@lists.linux.dev; linux-kselftest@vger.kernel.org; virtualization@lists.linux-foundation.org; linux-kernel@vger.kernel.org; Lu Baolu baolu.lu@linux.intel.com Subject: [PATCH v2 4/6] iommufd: Deliver fault messages to user space

[...]

Hi,

+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;
  

• }
  

• /*
  

•  * Get the hw page table that this response was generated for.
  

•  * It must match the one stored in the fault data.
  

•  */
  

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

• 				       response.hwpt_id,
  

IOMMUFD_OBJ_HW_PAGETABLE),

• 		    struct iommufd_hw_pagetable, obj);
  

• if (IS_ERR(hwpt)) {
  

• 	iommufd_put_object(&idev->obj);
  

• 	rc = PTR_ERR(hwpt);
  

• 	break;
  

• }
  

• if (hwpt != fault->hwpt) {
  

• 	rc = -EINVAL;
  

• 	goto put_obj;
  

• }
  

• group = NULL;
  

• list_for_each_entry(iter, &fault->response, node) {
  

• 	if (response.grpid != iter->last_fault.fault.prm.grpid)
  

• 		continue;
  

• 	if (idev->dev != iter->dev)
  

• 		continue;
  

• 	if ((iter->last_fault.fault.prm.flags &
  

• 	     IOMMU_FAULT_PAGE_REQUEST_PASID_VALID) &&
  

• 	    response.pasid != iter->last_fault.fault.prm.pasid)
  

• 		continue;
  

I am trying to get vSVA working with this series and got hit by the above check. On ARM platforms, page responses to stall events(CMD_RESUME) do not have an associated pasid. I think, either we need to check here using IOMMU_FAULT_PAGE_RESPONSE_NEEDS_PASID or remove the check as it will be eventually done in iommu_page_response().

Thanks, Shameer