On 26/05/18 01:35, Jacob Pan wrote:

quoted

quoted

quoted

Not exactly, it's the IOMMU driver that makes sure all LPIG in its
queues are submitted by the above flush call. In more details the
flow is:

* Either device driver calls unbind()/sva_device_shutdown(), or the
process exits.
* If the device driver called, then it already told the device to
stop using the PASID. Otherwise we use the mm_exit() callback to
tell the device driver to stop using the PASID.

Sorry I still need more clarification. For the PASID termination
initiated by vfio unbind, I don't see device driver given a chance to
stop PASID. Seems just call __iommu_sva_unbind_device() which already
assume device stopped issuing DMA with the PASID.
So it is the vfio unbind caller responsible for doing driver callback
to stop DMA on a given PASID?

Yes, but I don't know how to implement this. Since PCI doesn't formalize
the PASID stop mechanism and the device doesn't have a kernel driver,
VFIO would need help from the userspace driver for stopping PASID
(notify the userspace driver when an other process exits).

quoted

quoted

quoted

* In either case, when receiving a stop request from the driver,
the device sends the LPIGs to the IOMMU queue.
* Then, the flush call above ensures that the IOMMU reports the
LPIG with iommu_report_device_fault.
* While submitting all LPIGs for this PASID to the work queue,
ipof_queue_fault also picked up all partial faults, so the partial
list is clean.

Maybe I should improve this comment?
 

thanks for explaining. LPIG submission is done by device
asynchronously w.r.t. driver stopping/decommission PASID.  

Hmm, it should really be synchronous, otherwise there is no way to
know when the PASID can be decommissioned. We need a guarantee such
as the one in 6.20.1 of the PCIe spec, "Managing PASID TLP Prefix
Usage":

"When the stop request mechanism indicates completion, the Function
has:
* Completed all Non-Posted Requests associated with this PASID.
* Flushed to the host all Posted Requests addressing host memory in
all TCs that were used by the PASID."

That's in combination with "The function shall [...] finish
transmitting any multi-page Page Request Messages for this PASID
(i.e. send the Page Request Message with the L bit Set)." from the
ATS spec.

I am not contesting on the device side, what I meant was from the
host IOMMU driver perspective, LPIG is received via IOMMU host queue,
therefore asynchronous. Not sure about ARM, but on VT-d LPIG submission
could meet queue full condition. So per VT-d spec, iommu will generate a
successful auto response to the device. At this point, assume we
already stopped the given PASID on the device, there might not be
another LPIG sent for the device. Therefore, you could have a partial
list. I think we can just drop the requests in the partial list for
that PASID until the PASID gets re-allocated.

Indeed, I'll add this in next version. For a complete solution to the
queue-full condition (which seems to behave the same way on ARM) I was
thinking the IOMMU driver should also have a method for removing all
partial faults when detecting a queue overflow. Since it doesn't know
which PRGs did receive an auto-response, all it can do is remove all
partial faults, for all devices using this queue. But freeing the stuck
partial faults in flush() and remove_device() should be good enough

Thanks,
Jean