Re: [PATCH v4 3/5] iommu/vt-d: Disable non-recoverable fault processing before unbind
From: Lu Baolu
Date: Thu May 07 2020 - 22:53:02 EST
On 5/8/20 10:12 AM, Tian, Kevin wrote:
From: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
Sent: Thursday, May 7, 2020 9:23 PM
Hi Kevin,
On 2020/5/7 13:45, Tian, Kevin wrote:
From: Lu Baolu<baolu.lu@xxxxxxxxxxxxxxx>
Sent: Thursday, May 7, 2020 8:56 AM
When a PASID is used for SVA by the device, it's possible that the PASID
entry is cleared before the device flushes all ongoing DMA requests. The
IOMMU should ignore the non-recoverable faults caused by these
requests.
Intel VT-d provides such function through the FPD bit of the PASID entry.
This sets FPD bit when PASID entry is cleared in the mm notifier and
clear it when the pasid is unbound.
Signed-off-by: Lu Baolu<baolu.lu@xxxxxxxxxxxxxxx>
---
drivers/iommu/intel-iommu.c | 4 ++--
drivers/iommu/intel-pasid.c | 26 +++++++++++++++++++++-----
drivers/iommu/intel-pasid.h | 3 ++-
drivers/iommu/intel-svm.c | 9 ++++++---
4 files changed, 31 insertions(+), 11 deletions(-)
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index d1866c0905b1..7811422b5a68 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -5352,7 +5352,7 @@ static void
__dmar_remove_one_dev_info(struct
device_domain_info *info)
if (info->dev) {
if (dev_is_pci(info->dev) && sm_supported(iommu))
intel_pasid_tear_down_entry(iommu, info->dev,
- PASID_RID2PASID);
+ PASID_RID2PASID, false);
iommu_disable_dev_iotlb(info);
domain_context_clear(iommu, info->dev);
@@ -5587,7 +5587,7 @@ static void aux_domain_remove_dev(struct
dmar_domain *domain,
auxiliary_unlink_device(domain, dev);
spin_lock(&iommu->lock);
- intel_pasid_tear_down_entry(iommu, dev, domain->default_pasid);
+ intel_pasid_tear_down_entry(iommu, dev, domain->default_pasid,
false);
domain_detach_iommu(domain, iommu);
spin_unlock(&iommu->lock);
diff --git a/drivers/iommu/intel-pasid.c b/drivers/iommu/intel-pasid.c
index 7969e3dac2ad..11aef6c12972 100644
--- a/drivers/iommu/intel-pasid.c
+++ b/drivers/iommu/intel-pasid.c
@@ -292,7 +292,20 @@ static inline void pasid_clear_entry(struct
pasid_entry *pe)
WRITE_ONCE(pe->val[7], 0);
}
-static void intel_pasid_clear_entry(struct device *dev, int pasid)
+static inline void pasid_clear_entry_with_fpd(struct pasid_entry *pe)
+{
+ WRITE_ONCE(pe->val[0], PASID_PTE_FPD);
+ WRITE_ONCE(pe->val[1], 0);
+ WRITE_ONCE(pe->val[2], 0);
+ WRITE_ONCE(pe->val[3], 0);
+ WRITE_ONCE(pe->val[4], 0);
+ WRITE_ONCE(pe->val[5], 0);
+ WRITE_ONCE(pe->val[6], 0);
+ WRITE_ONCE(pe->val[7], 0);
+}
+
+static void
+intel_pasid_clear_entry(struct device *dev, int pasid, bool pf_ignore)
Hi, Baolu,
Just curious whether it makes sense to always set FPD here. Yes, SVA is
one known example that non-recoverable fault associated with a PASID
entry might be caused after the entry is cleared and those are considered
benign. But even in a general context (w/o SVA) why do we care about
such faults after a PASID entry is torn down?
First level page tables are also used for DMA protection. For example,
thunderbolt peripherals are always untrusted and should be protected
with IOMMU. IOMMU should always report unrecoverable faults generated
by those device to detect possible DMA attacks.
when untrusted devices are protected by IOMMU, I don't think PASID
entry (of RID2PASID) will have present bit cleared.
I mean, protect the system from malicious devices. In any case, IOMMU
should report and log the unrecoverable faults caused by the untrusted
devices.
Best regards,
baolu
ATS/PRI devices are always trusted devices, hence we could tolerate
setting FPD bit in the time window between mm_release notifier and
unbind().
Best regards,
baolu