Am 06.09.2018 um 14:45 schrieb Jean-Philippe Brucker:
On 06/09/2018 12:12, Christian König wrote:
Am 06.09.2018 um 13:09 schrieb Jean-Philippe Brucker:
Hi Eric,

Thanks for reviewing

On 05/09/2018 12:29, Auger Eric wrote:
+int iommu_sva_device_init(struct device *dev, unsigned long features,
+              unsigned int max_pasid)
what about min_pasid?
No one asked for it... The max_pasid parameter is here for drivers that
have vendor-specific PASID size limits, such as AMD KFD (see
kfd_iommu_device_init and
https://patchwork.kernel.org/patch/9989307/#21389571). But in most cases
the PASID size will only depend on the PCI PASID capability and the
IOMMU limits, both known by the IOMMU driver, so device drivers won't
have to set max_pasid.

IOMMU drivers need to set min_pasid in the sva_device_init callback
because it may be either 1 (e.g. Arm where PASID #0 is reserved) or 0
(Intel Vt-d rev2), but at the moment I can't see a reason for device
drivers to override min_pasid
Sorry to ruin your day, but if I'm not completely mistaken PASID zero is
reserved in the AMD KFD as well.
Heh, fair enough. I'll add the min_pasid parameter

I will take this as an opportunity to summarize some of the requirements we have for PASID management from the amdgpu driver point of view:

1. We need to be able to allocate PASID between 1 and some maximum. Zero is reserved as far as I know, but we don't necessary need a minimum.

2. We need to be able to allocate PASIDs without a process address space backing it. E.g. our hardware uses PASIDs even without Shared Virtual Addressing enabled to distinct clients from each other.         Would be a pity if we need to still have a separate PASID handling because the system wide is only available when IOMMU is turned on.

3. Even after destruction of a process address space we need some grace period before a PASID is reused because it can be that the specific PASID is still in some hardware queues etc...         At bare minimum all device drivers using process binding need to explicitly note to the core when they are done with a PASID.

4. It would be nice to have to be able to set a "void *" for each PASID/device combination while binding to a process which then can be queried later on based on the PASID.         E.g. when you have a per PASID/device structure around anyway, just add an extra field.

5. It would be nice to have to allocate multiple PASIDs for the same process address space.         E.g. some teams at AMD want to use a separate GPU address space for their userspace client library. I'm still trying to avoid that, but it is perfectly possible that we are going to need that.         Additional to that it is sometimes quite useful for debugging to isolate where exactly an incorrect access (segfault) is coming from.

Let me know if there are some problems with that, especially I want to know if there is pushback on #5 so that I can forward that :)

Thanks,
Christian.


Thanks,
Jean

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Reply via email to