Re: [PATCH RFCv2 06/13] iommufd: Make attach_handle generic

[Yi Liu]

On 2025/1/19 04:32, Nicolin Chen wrote:
> On Sat, Jan 18, 2025 at 04:23:22PM +0800, Yi Liu wrote:
> > On 2025/1/11 11:32, Nicolin Chen wrote:
> > > "attach_handle" was added exclusively for the iommufd_fault_iopf_handler()
> > > used by IOPF/PRI use cases, along with the "fault_data". Now, the iommufd
> > > version of sw_msi function will resue the attach_handle and fault_data for
> > > a non-fault case.
> > >
> > > Move the attach_handle part out of the fault.c file to make it generic for
> > > all cases. Simplify the remaining fault specific routine to attach/detach.
> >
> > I guess you can send it separately since both of our series need it. :)
>
> Jason, would you like to take this patch separately? I can send
> it prior to two big series for a quick review after rc1. It'll
> likely impact the vEVENTQ series too.
>
> > > +static int iommufd_hwpt_attach_device(struct iommufd_hw_pagetable *hwpt,
> > > +				      struct iommufd_device *idev)
> > > +{
> > > +	struct iommufd_attach_handle *handle;
> > > +	int rc;
> > > +
> > > +	if (hwpt->fault) {
> > > +		rc = iommufd_fault_domain_attach_dev(hwpt, idev, true);
> > > +		if (rc)
> > > +			return rc;
> > > +	}
> > > +
> > > +	handle = kzalloc(sizeof(*handle), GFP_KERNEL);
> > > +	if (!handle) {
> > > +		rc = -ENOMEM;
> > > +		goto out_fault_detach;
> > > +	}
> > > +
> > > +	handle->idev = idev;
> > > +	rc = iommu_attach_group_handle(hwpt->domain, idev->igroup->group,
> > > +				       &handle->handle);
> > > +	if (rc)
> > > +		goto out_free_handle;
> > > +
> > > +	return 0;
> > > +
> > > +out_free_handle:
> > > +	kfree(handle);
> > > +	handle = NULL;
> > > +out_fault_detach:
> > > +	if (hwpt->fault)
> > > +		iommufd_fault_domain_detach_dev(hwpt, idev, handle, true);
> > > +	return rc;
> > > +}
>
> Here the revert path passes in a handle=NULL..

aha. got it. Perhaps we can allocate handle first. In the below thread, it
is possible that a failed domain may have pending PRIs, it would require
the caller to call the auto response. Although, we are likely to swap the
order, but it is nice to have for the caller to do it.

https://lore.kernel.org/linux-iommu/f685daca-081a-4ede-b1e1-559009fa9ebc@xxxxxxxxx/

--
Regards,
Yi Liu