Messages in this thread | | | Date | Tue, 19 Dec 2023 14:57:22 +0800 | Subject | Re: [PATCH v3 2/2] iommu/vt-d: don's issue devTLB flush request when device is disconnected | From | Baolu Lu <> |
| |
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
| |