On 12/1/23 10:38 PM, Jason Gunthorpe wrote:
On Thu, Oct 26, 2023 at 10:49:25AM +0800, Lu Baolu wrote:+void *iopf_pasid_cookie_get(struct device *dev, ioasid_t pasid) +{ + struct iommu_fault_param *iopf_param = iopf_get_dev_fault_param(dev); + void *curr; + + if (!iopf_param) + return ERR_PTR(-ENODEV); + + xa_lock(&iopf_param->pasid_cookie); + curr = xa_load(&iopf_param->pasid_cookie, pasid); + xa_unlock(&iopf_param->pasid_cookie);No need for this locking, the caller has to provide some kind of locking to protect the returned pointer. I'm not sure how this can work really.. What iommfd wants is to increment the device object refcount under this xa_lock. I'm not sure this is the right arrangement: Basically you want to have a cookie per domain attachment for iopf domains that is forwarded to the handler. So maybe this entire thing is not quite right, instead of having a generic iopf attached to the domain the iopf should be supplied at domain attach time? Something like: iommu_domain_attach_iopf(struct iommu_domain *, struct device *, ioasid_t pasid, struct iopf *, void *cookie); The per-attach cookie would be passed to the iopf function automatically by the infrastructure. Detach would have the necessary locking to ensure that no handler is running across detach Then the cookie is logically placed in the API and properly protected with natural locking we already need.
Great idea! In a subsequent series, we could arrange the enabling and disabling of IOPF in this API, thereby eliminating the calling of iommu_dev_enable/disable_feature(dev, IOMMU_DEV_FEAT_IOPF) from the device drivers. Best regards, baolu
