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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <e82c300e-9d68-4270-a4ea-10bea20b1169@linux.intel.com>
Date: Fri, 12 Sep 2025 10:35:12 +0800
From: Baolu Lu <baolu.lu@...ux.intel.com>
To: Joel Granados <joel.granados@...nel.org>,
 David Woodhouse <dwmw2@...radead.org>, Joerg Roedel <joro@...tes.org>,
 Will Deacon <will@...nel.org>, Robin Murphy <robin.murphy@....com>
Cc: iommu@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] intel/vt-d: Send Page Request Drain only if supported

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@...nel.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