Messages in this thread | | | From | "Tian, Kevin" <> | Subject | RE: [PATCH v4 3/5] iommu/vt-d: Disable non-recoverable fault processing before unbind | Date | Fri, 8 May 2020 02:12:25 +0000 |
| |
> From: Lu Baolu <baolu.lu@linux.intel.com> > Sent: Thursday, May 7, 2020 9:23 PM > > Hi Kevin, > > On 2020/5/7 13:45, Tian, Kevin wrote: > >> From: Lu Baolu<baolu.lu@linux.intel.com> > >> 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@linux.intel.com> > >> --- > >> 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.
> > 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
| |