Messages in this thread | | | Date | Tue, 19 Dec 2023 15:16:50 +0800 | Subject | Re: [PATCH v3 2/2] iommu/vt-d: don's issue devTLB flush request when device is disconnected | From | Ethan Zhao <> |
| |
On 12/19/2023 2:57 PM, Baolu Lu wrote: > 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().
Fair engough !
Thanks,
Ethan
> > If you move this up before ATS checking, then you need to check > dev_is_pci(). > > Best regards, > baolu >
| |