[PATCH v4 3/5] iommu/vt-d: Disable non-recoverable fault processing before unbind
Lu Baolu
baolu.lu at linux.intel.com
Fri May 8 02:49:36 UTC 2020
On 5/8/20 10:12 AM, Tian, Kevin wrote:
>> From: Lu Baolu <baolu.lu at 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 at 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 at 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.
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
More information about the iommu
mailing list