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

[Baolu Lu <baolu.lu@linux.intel.com>]

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