On Mon, Jan 22, 2024 at 03:38:57PM +0800, Lu Baolu wrote:
> @@ -215,7 +202,23 @@ static struct iopf_group *iopf_group_alloc(struct iommu_fault_param *iopf_param,
> group = abort_group;
> }
>
> + cookie = iopf_pasid_cookie_get(iopf_param->dev, pasid);
> + if (!cookie && pasid != IOMMU_NO_PASID)
> + cookie = iopf_pasid_cookie_get(iopf_param->dev, IOMMU_NO_PASID);
> + if (IS_ERR(cookie) || !cookie) {
> + /*
> + * The PASID of this device was not attached by an I/O-capable
> + * domain. Ask the caller to abort handling of this fault.
> + * Otherwise, the reference count will be switched to the new
> + * iopf group and will be released in iopf_free_group().
> + */
> + kfree(group);
> + group = abort_group;
> + cookie = NULL;
> + }
If this is the main point of the cookie mechansim - why not just have
a refcount inside the domain itself?
I'm really having a hard time making sense of this cookie thing, we
have a lifetime issue on the domain pointer, why is adding another
structure the answer?
I see we also need to stick a pointer in the domain for iommufd to get
back to the hwpt, but that doesn't seem to need such a big system to
accomplish - just add a void *. It would make sense for the domain to
have some optional pointer to a struct for all the fault related data
that becomes allocated when a PRI domain is created..
Also, I thought we'd basically said that domain detatch is supposed to
flush any open PRIs before returning, what happened to that as a
solution to the lifetime problem?
Regardless I think we should split this into two series - improve the PRI
lifetime model for domains and then put iommufd on top of it..
Jason
