Messages in this thread | | | Date | Tue, 19 Dec 2023 15:27:14 +0800 | Subject | Re: [PATCH v3 2/2] iommu/vt-d: don's issue devTLB flush request when device is disconnected | From | Ethan Zhao <> |
| |
Baolu,
On 12/19/2023 3:16 PM, Ethan Zhao wrote: > > 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 ! > > May I use your reviewed-by sign in next version after that ?
Thanks,
Ethan
> Thanks, > > Ethan > >> >> If you move this up before ATS checking, then you need to check >> dev_is_pci(). >> >> Best regards, >> baolu >> >
| |