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

Jean-Philippe Brucker jean-philippe.brucker at arm.com
Tue Dec 11 13:35:23 UTC 2018


On 07/12/2018 10:29, 'joro at 8bytes.org' wrote:
> Hi,
> 
> On Mon, Nov 26, 2018 at 07:29:45AM +0000, Tian, Kevin wrote:
>> btw Baolu just reminded me one thing which is worthy of noting here.
>> 'primary' vs. 'aux' concept makes sense only when we look from a device
>> p.o.v. That binding relationship is not (*should not be*) carry-and-forwarded
>> cross devices. every domain must be explicitly attached to other devices
>> (instead of implicitly attached being above example), and new primary/aux
>> attribute on another device will be decided at attach time.
> 
> Okay, so after all the discussions we had I learned a few more things
> about the scalable mode feature and thought a bit longer about how to
> best support it in the IOMMU-API.
> 
> The concept of sub-domains I initially proposed certainly makes no
> sense, but scalable-mode specific attach/detach functions do. So instead
> of a sub-domain mode, I'd like to propose device-feature sets.
> 
> The posted patch-set already includes this as device-attributes, but I
> don't like this naming as we are really talking about additional
> feature sets of a device. So how about we introduce this:
> 
> 	enum iommu_dev_features {
> 		/* ... */
> 		IOMMU_DEV_FEAT_AUX,
> 		IOMMU_DEV_FEAT_SVA,
> 		/* ... */
> 	};
> 
> 	/* Check if a device supports a given feature of the IOMMU-API */
> 	bool iommu_dev_has_feature(struct device *dev, enum iommu_dev_features *feat);
> 
> 	/*
> 	 * Only works if iommu_dev_has_feature(dev, IOMMU_DEV_FEAT_AUX)
> 	 * returns true
> 	 *
> 	 * Also, as long as domains are attached to a device through
> 	 * this interface, any trys to call iommu_attach_device() should
> 	 * fail (iommu_detach_device() can't fail, so we fail on the
> 	 * tryint to re-attach). This should make us safe against a
> 	 * device being attached to a guest as a whole while there are
> 	 * still pasid users on it (aux and sva).
> 	 */
> 	int iommu_aux_attach_device(struct iommu_domain *domain,
> 				    struct device *dev);
> 
> 	int iommu_aux_detach_device(struct iommu_domain *domain,
> 				    struct device *dev);
> 	/*
> 	 * I know we are targeting a system-wide pasid-space, so that
> 	 * the pasid would be the same for one domain on all devices,
> 	 * let's just keep the option open to have different
> 	 * pasid-spaces in one system. Also this way we can use it to
> 	 * check whether the domain is attached to this device at all.
> 	 *
> 	 * returns pasid or <0 if domain has no pasid on that device.
> 	 */
> 	int iommu_aux_get_pasid(struct iommu_domain *domain,
> 				struct device *dev);>
> 	/* 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()

> 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?

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)

Thanks,
Jean


More information about the iommu mailing list