Re: [PATCH v2 2/5] iommu/vt-d: Allow SVA with device-specific IOPF

[Jacob Pan]

Hi BaoLu,

On Thu,  9 Mar 2023 10:56:36 +0800, Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
wrote:

> Currently enabling SVA requires IOPF support from the IOMMU and device
> PCI PRI. However, some devices can handle IOPF by itself without ever
> sending PCI page requests nor advertising PRI capability.
> 
> Allow SVA support with IOPF handled either by IOMMU (PCI PRI) or device
> driver (device-specific IOPF). As long as IOPF could be handled, SVA
> should continue to work.
> 
> Signed-off-by: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
> ---
>  drivers/iommu/intel/iommu.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 7c2f4bd33582..d2fcab9d8f61 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -4650,7 +4650,18 @@ static int intel_iommu_enable_sva(struct device
> *dev) if (!(iommu->flags & VTD_FLAG_SVM_CAPABLE))
>  		return -ENODEV;
>  
> -	if (!info->pasid_enabled || !info->pri_enabled ||
> !info->ats_enabled)
> +	if (!info->pasid_enabled)
> +		return -EINVAL;
> +
> +	/*
> +	 * Devices having device-specific I/O fault handling should not
> +	 * support PCI/PRI.
> +	 */
> +	if (!info->pri_supported)
> +		return 0;
If you put this check at the very beginning, everything else should it be
the same, right?

Still feel a little weird that, SVA is tied to PRI for PCI PRI but not for
device specific IOPF.

> +	/* Devices supporting ATS/PRI should have it enabled. */
> +	if (!info->pri_enabled || !info->ats_enabled)
>  		return -EINVAL;
>  
>  	ret = iopf_queue_add_device(iommu->iopf_queue, dev);


Thanks,

Jacob