Re: [PATCH v3 07/10] iommu: Add a page fault handler

From: Jacob Pan <jacob.jun.pan@linux.intel.com>
To: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
Cc: iommu@lists.linux-foundation.org, joro@8bytes.org,
	linux-pci@vger.kernel.org, jcrouse@codeaurora.org,
	alex.williamson@redhat.com, Jonathan.Cameron@huawei.com,
	christian.koenig@amd.com, eric.auger@redhat.com,
	kevin.tian@intel.com, yi.l.liu@intel.com, andrew.murray@arm.com,
	will.deacon@arm.com, robin.murphy@arm.com, ashok.raj@intel.com,
	baolu.lu@linux.intel.com, xuzaibo@huawei.com,
	liguozhu@hisilicon.com, okaya@codeaurora.org,
	bharatku@xilinx.com, ilias.apalodimas@linaro.org,
	shunyong.yang@hxt-semitech.com, jacob.jun.pan@linux.intel.com
Subject: Re: [PATCH v3 07/10] iommu: Add a page fault handler
Date: Thu, 27 Sep 2018 13:37:46 -0700	[thread overview]
Message-ID: <20180927133746.319e19b8@jacob-builder> (raw)
In-Reply-To: <20180920170046.20154-8-jean-philippe.brucker@arm.com>

On Thu, 20 Sep 2018 18:00:43 +0100
Jean-Philippe Brucker <jean-philippe.brucker@arm.com> wrote:

> +	/*
> +	 * When removing a PASID, the device driver tells the device
> to stop
> +	 * using it, and flush any pending fault to the IOMMU. In
> this flush
> +	 * callback, the IOMMU driver makes sure that there are no
> such faults
> +	 * left in the low-level queue.
> +	 */
> +	queue->flush(queue->flush_arg, dev, pasid);
> +
> +	/*
> +	 * If at some point the low-level fault queue overflowed and
> the IOMMU
> +	 * device had to auto-respond to a 'last' page fault, other
> faults from
> +	 * the same Page Request Group may still be stuck in the
> partial list.
> +	 * We need to make sure that the next address space using
> the PASID
> +	 * doesn't receive them.
> +	 */
Trying to understand the intended use case under queue full condition.
1 model specific iommu driver register a flush callback to handle
  internal PRQ drain

2 IOMMU HW detects queue full and auto respond with 'SUCCESS' code to
  all device and PASID, raise interrupt

3 model specific iommu driver detects queue full and call
iopf_queue_flush_dev()

4 call queue->flush() callback to drain PRQ in-flight inside IOMMU HW
5.Shoot down partial list for all PASIDs

If the above understanding is correct, don't we need to shoot down all
partial groups? instead of just one PASID. At least for VT-d, we need
to do that.

> +	mutex_lock(&param->lock);
> +	list_for_each_entry_safe(fault, next,
> &param->iopf_param->partial, head) {
> +		if (fault->evt.pasid == pasid || pasid ==
> IOMMU_PASID_INVALID) {
> +			list_del(&fault->head);
> +			kfree(fault);
> +		}
> +	}
> +	mutex_unlock(&param->lock);
> +
> +	flush_workqueue(queue->wq);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(iopf_queue_flush_dev);
> +
[Jacob Pan]