[PATCH 01/37] iommu: Introduce Shared Virtual Addressing API

[Tian, Kevin]

> From: Jean-Philippe Brucker
> Sent: Tuesday, February 13, 2018 8:40 PM
> 
> 
> [...]
> >> +
> >> +/**
> >> + * iommu_sva_device_init() - Initialize Shared Virtual Addressing for a
> >> device
> >> + * @dev: the device
> >> + * @features: bitmask of features that need to be initialized
> >> + * @max_pasid: max PASID value supported by the device
> >> + *
> >> + * Users of the bind()/unbind() API must call this function to initialize all
> >> + * features required for SVA.
> >> + *
> >> + * - If the device should support multiple address spaces (e.g. PCI
> PASID),
> >> + *   IOMMU_SVA_FEAT_PASID must be requested.
> >
> > I think it is by default assumed when using this API, based on definition of
> > SVA. Can you elaborate the situation where this flag can be cleared?
> 
> When passing a device to userspace, you could also share its non-pasid
> address space with the process. It requires a new domain type so is left
> as a TODO in patch 2/37. I did get requests for this feature, though I
> think it was mostly for prototyping. I guess I could remove the flag, and
> reintroduce it as IOMMU_SVA_FEAT_NO_PASID later on.

sorry I still didn't get the definition of non-pasid address space. 
Did you mean the GPA/IOVA address space and no_pasid implies
actually some default PASID associated?

> 
> [...]
> >> +	ret = domain->ops->sva_device_init(dev, features, &min_pasid,
> >> +					   &max_pasid);
> >> +	if (ret)
> >> +		return ret;
> >> +
> >> +	/* FIXME: racy. Next version should have a mutex (same as fault
> >> handler) */
> >> +	dev_param->sva_features = features;
> >> +	dev_param->min_pasid = min_pasid;
> >> +	dev_param->max_pasid = max_pasid;
> >
> > what's the point of min_pasid here?
> 
> Arm SMMUv3 uses entry 0 of the PASID table for the default (non-pasid)
> context, so it needs to set min_pasid to 1. AMD IOMMU recently added a
> similar feature (GIoSup), if I understood correctly.
> 

just for such purpose maybe we should just define a reserved_pasid
otherwise there will be some waste if an implementation allows it
non-zero.

Thanks
Kevin