lkml.org 
[lkml]   [2023]   [Dec]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v3 2/2] iommu/vt-d: don's issue devTLB flush request when device is disconnected
From
On 2023/12/19 14:49, Ethan Zhao wrote:
>>> diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
>>> index 74e8e4c17e81..182eb5df244d 100644
>>> --- a/drivers/iommu/intel/pasid.c
>>> +++ b/drivers/iommu/intel/pasid.c
>>> @@ -476,6 +476,23 @@ devtlb_invalidation_with_pasid(struct
>>> intel_iommu *iommu,
>>>   {
>>>       struct device_domain_info *info;
>>>       u16 sid, qdep, pfsid;
>>> +    struct pci_dev *pdev;
>>> +
>>> +    pdev = to_pci_dev(dev);
>>> +    if (!pdev)
>>> +        return;
>>> +
>>> +    /*
>>> +     * If endpoint device's link was brough down by user's pci
>>> configuration
>>> +     * access to it's hotplug capable slot link control register, as
>>> sequence
>>> +     * response for DLLSC, pciehp_ist() will set the device
>>> error_state to
>>> +     * pci_channel_io_perm_failure. Checking device's state here to
>>> avoid
>>> +     * issuing meaningless devTLB flush request to it, that might
>>> cause lockup
>>> +     * warning or deadlock because too long time waiting in
>>> interrupt context.
>>> +     */
>>> +
>>> +    if (pci_dev_is_disconnected(pdev))
>>> +        return;
>>>         info = dev_iommu_priv_get(dev);
>>>       if (!info || !info->ats_enabled)
>>
>> It's likely better to check the device status after verifying
>> ats_enabled. How about below change?
>>
>> diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
>> index 74e8e4c17e81..fa19c6cdfd8b 100644
>> --- a/drivers/iommu/intel/pasid.c
>> +++ b/drivers/iommu/intel/pasid.c
>> @@ -481,6 +481,9 @@ devtlb_invalidation_with_pasid(struct intel_iommu
>> *iommu,
>>         if (!info || !info->ats_enabled)
>>                 return;
>>
>> +       if (pci_dev_is_disconnected(to_pci_dev(dev)))
>
> I like this kind of simplicity, but rationalist always brings me back to
> the no-error(ugly)
>
> style.  🙂

The rational is that Intel IOMMU driver only supports PCI ATS. So if
device is marked as ATS supported, then it must be a PCI device.
Therefore, it's safe to convert it to pci_device with to_pci_dev().

If you move this up before ATS checking, then you need to check
dev_is_pci().

Best regards,
baolu

\
 
 \ /
  Last update: 2023-12-19 07:58    [W:0.052 / U:1.684 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site