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

Lu Baolu posted 5 patches 2 years, 6 months ago
[PATCH v2 2/5] iommu/vt-d: Allow SVA with device-specific IOPF
Posted by Lu Baolu 2 years, 6 months ago
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@linux.intel.com>
---
 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;
+
+	/* 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);
-- 
2.34.1
Re: [PATCH v2 2/5] iommu/vt-d: Allow SVA with device-specific IOPF
Posted by Jacob Pan 2 years, 5 months ago
Hi BaoLu,

On Thu,  9 Mar 2023 10:56:36 +0800, Lu Baolu <baolu.lu@linux.intel.com>
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@linux.intel.com>
> ---
>  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
Re: [PATCH v2 2/5] iommu/vt-d: Allow SVA with device-specific IOPF
Posted by Baolu Lu 2 years, 5 months ago
On 3/21/23 12:00 AM, Jacob Pan wrote:
> Hi BaoLu,

Hi Jacob,

> 
> On Thu,  9 Mar 2023 10:56:36 +0800, Lu Baolu<baolu.lu@linux.intel.com>
> 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@linux.intel.com>
>> ---
>>   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?

Even for device specific IOPF, PASID/ATS are still required on the IOMMU
side.

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

PCI PRI and device specific IOPF *should* be equivalent. But the IOMMU
side has no means to check the capability of device-specific IOPF.
Therefore, IOMMU can only default that if the device driver enables SVA
on a non-PRI device, it will handle IOPF in its own way.

Best regards,
baolu
RE: [PATCH v2 2/5] iommu/vt-d: Allow SVA with device-specific IOPF
Posted by Tian, Kevin 2 years, 5 months ago
> From: Lu Baolu <baolu.lu@linux.intel.com>
> Sent: Thursday, March 9, 2023 10:57 AM
> 
> @@ -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;
> +

I think you still want to check ats_enabled even for device specific IOPF.
Re: [PATCH v2 2/5] iommu/vt-d: Allow SVA with device-specific IOPF
Posted by Baolu Lu 2 years, 5 months ago
On 2023/3/16 15:09, Tian, Kevin wrote:
>> From: Lu Baolu <baolu.lu@linux.intel.com>
>> Sent: Thursday, March 9, 2023 10:57 AM
>>
>> @@ -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;
>> +
> 
> I think you still want to check ats_enabled even for device specific IOPF.

Yeah! Updated.

Best regards,
baolu