Re: [RFC PATCH 0/6] Auxiliary IOMMU domains and Arm SMMUv3

["'joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org'" <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>]

On Tue, Dec 11, 2018 at 01:35:23PM +0000, Jean-Philippe Brucker wrote:
> > 	/* So we need a iommu_aux_detach_all()? */
> 
> This could be useful for device drivers that want to do bulk cleanup on
> device removal. If they rely on Function Level Reset to disable PASID
> states for example, they could also cleanup with a detach_all(). But
> most will probably need to clean up individual PASID states (for example
> free all mdevs) and therefore don't need detach_all()

Yeah, so the more I think about it, the more dangerous it seems to have
this function. It creates subtle error cases for the users of SVA and
aux-domains because their mapping goes away without notice. It is
certainly better to force an orderly shutdown of all users before the
device can be re-assigned.

> > This concept can also be easily extended for supporting SVA in parallel
> > on the same device, with the same constraints regarding the behavior of
> > iommu_attach_device()/iommu_detach_device().
> 
> So this would be without the initial step of attaching an "ext" domain
> to the device in order to enable SVA/PASID?

No, I don't think anymore we should introduce a special domain type to
enable these features. Separate enable/disable functions per feature
seem to be a better choice.

> If I understood it correctly, I agree with the
> iommu_attach/detach_device() behavior for SVA as well. If a device is
> bound to an mm, then the device cannot be attached to another domain
> with iommu_attach_device(). This prevents leaks and forces the device
> driver to clean up PASID states when switching to a different driver
> (e.g. vfio-pci)

Right, the API should ensure we are safe on that side.

Thanks,

	Joerg