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

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

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 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 what do you think about that approach?

Regards,

	Joerg