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.
Jason