-----Original Message----- From: Jason Gunthorpe jgg@ziepe.ca Sent: Monday, January 15, 2024 4:47 PM To: Shameerali Kolothum Thodi shameerali.kolothum.thodi@huawei.com Cc: Lu Baolu baolu.lu@linux.intel.com; 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; iommu@lists.linux.dev; linux-kselftest@vger.kernel.org; virtualization@lists.linux-foundation.org; linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 4/6] iommufd: Deliver fault messages to user space
On Fri, Jan 12, 2024 at 05:46:13PM +0000, Shameerali Kolothum Thodi wrote:
-----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 generatedfor.
* 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().
That doesn't sound right..
The PASID is the only information we have for userspace to identify the domain that is being faulted. It cannot be optional on the request side.
Yes, it is valid on the request side. But this is on the response side.
If it is valid when userspace does read() then it should be valid when userspace does write() too.
It is the only way the kernel can actually match request and response here.
The kernel currently checks the pasid only if IOMMU_FAULT_PAGE_RESPONSE_NEEDS_PASID is set.
https://lore.kernel.org/linux-iommu/20200616144712.748818-1-jean-philippe@li...
So, I think you have a userspace issue to not provide the right pasid??
This is not just ARM stall resume case, but for some PCI devices as well as per the above commit log. So do we really need to track this in userspace ?
Thanks, Shameer