Tian, Baolu, On 8/30/2023 1:13 PM, Tian, Kevin wrote: >> From: Baolu Lu <baolu.lu@xxxxxxxxxxxxxxx> >> Sent: Saturday, August 26, 2023 4:01 PM >> >> On 8/25/23 4:17 PM, Tian, Kevin wrote: >>>> + >>>> /** >>>> * iopf_queue_flush_dev - Ensure that all queued faults have been >>>> processed >>>> * @dev: the endpoint whose faults need to be flushed. >>> Presumably we also need a flush callback per domain given now >>> the use of workqueue is optional then flush_workqueue() might >>> not be sufficient. >>> >> >> The iopf_queue_flush_dev() function flushes all pending faults from the >> IOMMU queue for a specific device. It has no means to flush fault queues >> out of iommu core. >> >> The iopf_queue_flush_dev() function is typically called when a domain is >> detaching from a PASID. Hence it's necessary to flush the pending faults >> from top to bottom. For example, iommufd should flush pending faults in >> its fault queues after detaching the domain from the pasid. >> > > Is there an ordering problem? The last step of intel_svm_drain_prq() > in the detaching path issues a set of descriptors to drain page requests > and responses in hardware. It cannot complete if not all software queues > are drained and it's counter-intuitive to drain a software queue after > the hardware draining has already been completed. > > btw just flushing requests is probably insufficient in iommufd case since > the responses are received asynchronously. It requires an interface to > drain both requests and responses (presumably with timeouts in case > of a malicious guest which never responds) in the detach path. > > it's not a problem for sva as responses are synchrounsly delivered after > handling mm fault. So fine to not touch it in this series but certainly > this area needs more work when moving to support iommufd. 😊 > > btw why is iopf_queue_flush_dev() called only in intel-iommu driver? > Isn't it a common requirement for all sva-capable drivers? I had same question when we did SVA implementation for AMD IOMMU [1]. Currently we call queue_flush from remove_dev_pasid() path. Since PASID can be enabled without ATS/PRI, I thought its individual drivers responsibility. But looking this series, does it make sense to handle queue_flush in core layer? [1] https://lore.kernel.org/linux-iommu/20230823140415.729050-1-vasant.hegde@xxxxxxx/T/#t -Vasant
