1. Patchew
  2. linux
  3. View series

[PATCH] intel/vt-d: Send Page Request Drain only if supported

Joel Granados posted 1 patch 3 weeks, 2 days ago 
drivers/iommu/intel/prq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] intel/vt-d: Send Page Request Drain only if supported
Posted by Joel Granados 3 weeks, 2 days ago 
The intel_iommu_drain_pasid_prq function sends QI_OPT_WAIT_DRAIN to
hardware without verifying Page Request Drain Support (PDS). According
to VT-d specification section 6.5.2.8, PRQ drain functionality should
only be used when PDS (bit 42) is set in the extended capability
register. Add ecap_pds() check to conditionally use QI_OPT_WAIT_DRAIN
based on hardware capability.

Signed-off-by: Joel Granados <joel.granados@kernel.org>
---
 drivers/iommu/intel/prq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/intel/prq.c b/drivers/iommu/intel/prq.c
index 52570e42a14c05b7492957909568805dc9c7b6ef..f89916de31a3439866f059f5400e45fb362a6a7d 100644
--- a/drivers/iommu/intel/prq.c
+++ b/drivers/iommu/intel/prq.c
@@ -119,7 +119,7 @@ void intel_iommu_drain_pasid_prq(struct device *dev, u32 pasid)
 	}
 qi_retry:
 	reinit_completion(&iommu->prq_complete);
-	qi_submit_sync(iommu, desc, 3, QI_OPT_WAIT_DRAIN);
+	qi_submit_sync(iommu, desc, 3, ecap_pds(iommu->ecap) ? QI_OPT_WAIT_DRAIN : 0);
 	if (readl(iommu->reg + DMAR_PRS_REG) & DMA_PRS_PRO) {
 		wait_for_completion(&iommu->prq_complete);
 		goto qi_retry;

---
base-commit: f777d1112ee597d7f7dd3ca232220873a34ad0c8
change-id: 20250909-jag-pds-7d85b60ad9d2

Best regards,
-- 
Joel Granados <joel.granados@kernel.org>
Re: [PATCH] intel/vt-d: Send Page Request Drain only if supported
Posted by Baolu Lu 2 weeks, 6 days ago 
On 9/9/25 16:58, Joel Granados wrote:
> The intel_iommu_drain_pasid_prq function sends QI_OPT_WAIT_DRAIN to
> hardware without verifying Page Request Drain Support (PDS). According
> to VT-d specification section 6.5.2.8, PRQ drain functionality should
> only be used when PDS (bit 42) is set in the extended capability
> register. Add ecap_pds() check to conditionally use QI_OPT_WAIT_DRAIN
> based on hardware capability.
> 
> Signed-off-by: Joel Granados<joel.granados@kernel.org>
> ---
>   drivers/iommu/intel/prq.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/intel/prq.c b/drivers/iommu/intel/prq.c
> index 52570e42a14c05b7492957909568805dc9c7b6ef..f89916de31a3439866f059f5400e45fb362a6a7d 100644
> --- a/drivers/iommu/intel/prq.c
> +++ b/drivers/iommu/intel/prq.c
> @@ -119,7 +119,7 @@ void intel_iommu_drain_pasid_prq(struct device *dev, u32 pasid)
>   	}
>   qi_retry:
>   	reinit_completion(&iommu->prq_complete);
> -	qi_submit_sync(iommu, desc, 3, QI_OPT_WAIT_DRAIN);
> +	qi_submit_sync(iommu, desc, 3, ecap_pds(iommu->ecap) ? QI_OPT_WAIT_DRAIN : 0);

I'm afraid that draining page requests and responses won't work as
expected without the PDS capability. We should perhaps fail to enable
IOPF if the PDS capability isn't supported.

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 9c3ab9d9f69a..ca6a6eaea62c 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -3983,6 +3983,10 @@ int intel_iommu_enable_iopf(struct device *dev)
         if (!info->pri_enabled)
                 return -ENODEV;

+       /* PDS is required to drain page requests and responses. */
+       if (!ecap_pds(iommu->ecap))
+               return -ENODEV;
+
         /* pri_enabled is protected by the group mutex. */
         iommu_group_mutex_assert(dev);
         if (info->iopf_refcount) {

At the same time, qi_submit_sync() should not set PD bit in the wait
descriptor as the spec Section 6.5.2.9 "Invalidation Wait Descriptor"
states:

  Page-request Drain (PD): Remapping hardware implementations reporting
  Page-request draining as not supported (PDS = 0 in ECAP_REG) treats
  this field as reserved.

therefore,

diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
index ec975c73cfe6..e38af2274032 100644
--- a/drivers/iommu/intel/dmar.c
+++ b/drivers/iommu/intel/dmar.c
@@ -1427,7 +1427,7 @@ int qi_submit_sync(struct intel_iommu *iommu, 
struct qi_desc *desc,

         wait_desc.qw0 = QI_IWD_STATUS_DATA(QI_DONE) |
                         QI_IWD_STATUS_WRITE | QI_IWD_TYPE;
-       if (options & QI_OPT_WAIT_DRAIN)
+       if ((options & QI_OPT_WAIT_DRAIN) && 
!WARN_ON_ONCE(!ecap_pds(iommu->ecap)))
                 wait_desc.qw0 |= QI_IWD_PRQ_DRAIN;
         wait_desc.qw1 = virt_to_phys(&qi->desc_status[wait_index]);
         wait_desc.qw2 = 0;

?

>   	if (readl(iommu->reg + DMAR_PRS_REG) & DMA_PRS_PRO) {
>   		wait_for_completion(&iommu->prq_complete);
>   		goto qi_retry;

Thanks,
baolu
Re: [PATCH] intel/vt-d: Send Page Request Drain only if supported
Posted by Baolu Lu 2 weeks, 3 days ago 
On 9/12/25 10:35, Baolu Lu wrote:
> On 9/9/25 16:58, Joel Granados wrote:
>> The intel_iommu_drain_pasid_prq function sends QI_OPT_WAIT_DRAIN to
>> hardware without verifying Page Request Drain Support (PDS). According
>> to VT-d specification section 6.5.2.8, PRQ drain functionality should
>> only be used when PDS (bit 42) is set in the extended capability
>> register. Add ecap_pds() check to conditionally use QI_OPT_WAIT_DRAIN
>> based on hardware capability.
>>
>> Signed-off-by: Joel Granados<joel.granados@kernel.org>
>> ---
>>   drivers/iommu/intel/prq.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/iommu/intel/prq.c b/drivers/iommu/intel/prq.c
>> index 
>> 52570e42a14c05b7492957909568805dc9c7b6ef..f89916de31a3439866f059f5400e45fb362a6a7d 100644
>> --- a/drivers/iommu/intel/prq.c
>> +++ b/drivers/iommu/intel/prq.c
>> @@ -119,7 +119,7 @@ void intel_iommu_drain_pasid_prq(struct device 
>> *dev, u32 pasid)
>>       }
>>   qi_retry:
>>       reinit_completion(&iommu->prq_complete);
>> -    qi_submit_sync(iommu, desc, 3, QI_OPT_WAIT_DRAIN);
>> +    qi_submit_sync(iommu, desc, 3, ecap_pds(iommu->ecap) ? 
>> QI_OPT_WAIT_DRAIN : 0);
> 
> I'm afraid that draining page requests and responses won't work as
> expected without the PDS capability. We should perhaps fail to enable
> IOPF if the PDS capability isn't supported.
> 
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 9c3ab9d9f69a..ca6a6eaea62c 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -3983,6 +3983,10 @@ int intel_iommu_enable_iopf(struct device *dev)
>          if (!info->pri_enabled)
>                  return -ENODEV;
> 
> +       /* PDS is required to drain page requests and responses. */
> +       if (!ecap_pds(iommu->ecap))
> +               return -ENODEV;
> +
>          /* pri_enabled is protected by the group mutex. */
>          iommu_group_mutex_assert(dev);
>          if (info->iopf_refcount) {
> 
> At the same time, qi_submit_sync() should not set PD bit in the wait
> descriptor as the spec Section 6.5.2.9 "Invalidation Wait Descriptor"
> states:
> 
>   Page-request Drain (PD): Remapping hardware implementations reporting
>   Page-request draining as not supported (PDS = 0 in ECAP_REG) treats
>   this field as reserved.
> 
> therefore,
> 
> diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
> index ec975c73cfe6..e38af2274032 100644
> --- a/drivers/iommu/intel/dmar.c
> +++ b/drivers/iommu/intel/dmar.c
> @@ -1427,7 +1427,7 @@ int qi_submit_sync(struct intel_iommu *iommu, 
> struct qi_desc *desc,
> 
>          wait_desc.qw0 = QI_IWD_STATUS_DATA(QI_DONE) |
>                          QI_IWD_STATUS_WRITE | QI_IWD_TYPE;
> -       if (options & QI_OPT_WAIT_DRAIN)
> +       if ((options & QI_OPT_WAIT_DRAIN) && !WARN_ON_ONCE(! 
> ecap_pds(iommu->ecap)))
>                  wait_desc.qw0 |= QI_IWD_PRQ_DRAIN;
>          wait_desc.qw1 = virt_to_phys(&qi->desc_status[wait_index]);
>          wait_desc.qw2 = 0;
> 
> ?

I submitted a fix patch for this. Feel free to comment on it there.

https://lore.kernel.org/linux-iommu/20250915062946.120196-1-baolu.lu@linux.intel.com/

Thanks,
baolu

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.