Re: [PATCH] iommu/virtio: Detach domain on endpoint release

[Jason Gunthorpe <jgg@nvidia.com>]

On Wed, May 10, 2023 at 09:11:57AM +0100, Jean-Philippe Brucker wrote:

> > diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
> > index 5b8fe9bfa9a5..3d3d4462359e 100644
> > --- a/drivers/iommu/virtio-iommu.c
> > +++ b/drivers/iommu/virtio-iommu.c
> > @@ -788,6 +788,28 @@ static int viommu_attach_dev(struct iommu_domain *domain, struct device *dev)
> >  	return 0;
> >  }
> >  
> > +static void viommu_detach_dev(struct viommu_endpoint *vdev)
> > +{
> > +	int i;
> > +	struct virtio_iommu_req_detach req;
> > +	struct viommu_domain *vdomain = vdev->vdomain;
> > +	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(vdev->dev);
> > +
> > +	if (!vdomain)
> > +		return;
> > +
> > +	req = (struct virtio_iommu_req_detach) {
> > +		.head.type	= VIRTIO_IOMMU_T_DETACH,
> > +		.domain		= cpu_to_le32(vdomain->id),
> > +	};
> > +
> > +	for (i = 0; i < fwspec->num_ids; i++) {
> > +		req.endpoint = cpu_to_le32(fwspec->ids[i]);
> > +		WARN_ON(viommu_send_req_sync(vdev->viommu, &req, sizeof(req)));
> > +	}
> > +	vdev->vdomain = NULL;

Not for this patch, but something to work on..

I assume detach disconnects the container on the VFIO side and puts it
into a BLOCKED state?

Can you wrapper this into a BLOCKED domain like we are moving drivers
toward, and then attach the blocked domain instead of introducing this
special case?

I've been thinking about having some core code support to do fairly
common pattern of 'parking' the iommu at some well defined translation
mode, BLOCKED in this case.

Thanks,
Jason