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

[Jean-Philippe Brucker <jean-philippe.brucker-5wv7dgnIgG8@public.gmane.org>]

On 22/10/2018 11:07, Raj, Ashok wrote:
>>     For my own convenience I've been using the SVA infrastructure since
>>     I already had the locking and IOMMU ops in place. The proposed
>>     interface is also missing min_pasid and max_pasid parameters, which
>>     could be needed by device drivers to enforce PASID limits.
> 
> Variable PASID limits per-device is hard to manage. I suppose we can play
> some games to get this right, but I suspect that wont be very useful in 
> the long run.

Devices may have PASID limits that are not discoverable with the PCI
PASID capability (https://patchwork.kernel.org/patch/9989307/#21389571).
Even if we simply reject devices that don't support enough PASIDs, I
think it's still better to return an error on bind or init_device than
to return a valid PASID that they can't use

> #1: Given that PASID is a system wide resource, during boot iommu driver needs 
> to scan and set the max PASID to be no more than min(iommu_supported_max_pasid) 
> of all available IOMMU's. 
> 
> #2: In addition if any device supports less than the choosen system max PASID
> we should simply disable PASID on that device.
> 
> The reason is if the process were to bind to 2 or more accelerators and
> the second device has a limit smaller than the first that the application
> already attached, the second attemt to bind would fail. (Because
> we would use the same PASID for a single process)

But that's not reason enough to completely disable PASID for the device,
it could be the only one bound to that process, or PASID could be only
used privately by the host device driver.

> In addition for Intel IOMMU's PASID is a system wide resource. We have
> a virtual capability for vIOMMU's to allocate PASID's. At the time of
> allocation we don't know what device this PASID is even being allocated 
> for. 

Ah, I had missed that part, I thought the PV allocation had the device
ID as well. That's a significant change.

I was still hoping that we could go back to per-device PASID spaces in
the host, since I still haven't seen any convincing argument in favor of
system-wide PASID. Get rid of #1 in order to solve #2, and as a bonus
support more PASIDs in total. Until now PASID was a per-device resource
except for choices made when writing the IOMMU driver.

> Certainly we could add other intelligence to pass a hint for 
> max_pasid in the virtiual interface. 
> 
> I suppose when this becomes real, most serious accelerators will 
> support the full width.

I don't know if that's obvious from the device perspective. If I had to
implement one, I would simply size my PASIDs to the number of contexts
supported by my device (which might be especially small in the embedded
space), given that technically nothing prevents software from supporting
that and the specification allows me to choose a width.

This was the intended model for PCI, but thankfully version 4.0 added an
implementation note (6.20.2.1 - PASID Field) advising against this
approach, and to instead support 20 bits for interoperability. Maybe it
will set a trend for non-PCI devices as well.

Thanks,
Jean