From: "Liu, Yi L" <yi.l.liu@intel.com> To: Lu Baolu <baolu.lu@linux.intel.com>, "joro@8bytes.org" <joro@8bytes.org>, "will@kernel.org" <will@kernel.org> Cc: "Tian, Kevin" <kevin.tian@intel.com>, "jacob.jun.pan@linux.intel.com" <jacob.jun.pan@linux.intel.com>, "Raj, Ashok" <ashok.raj@intel.com>, "Tian, Jun J" <jun.j.tian@intel.com>, "Sun, Yi Y" <yi.y.sun@intel.com>, "iommu@lists.linux-foundation.org" <iommu@lists.linux-foundation.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org> Subject: RE: [PATCH v3 3/3] iommu/vt-d: Fix ineffective devTLB invalidation for subdevices Date: Tue, 5 Jan 2021 05:50:22 +0000 [thread overview] Message-ID: <DM5PR11MB1435E814408F19A947263C07C3D10@DM5PR11MB1435.namprd11.prod.outlook.com> (raw) In-Reply-To: <c109eb64-2805-5e87-2283-b52c5704a31f@linux.intel.com> Hi Baolu, > From: Lu Baolu <baolu.lu@linux.intel.com> > Sent: Tuesday, December 29, 2020 4:42 PM > > Hi Yi, > > On 2020/12/29 11:25, Liu Yi L wrote: > > iommu_flush_dev_iotlb() is called to invalidate caches on device. It only > > loops the devices which are full-attached to the domain. For sub-devices, > > this is ineffective. This results in invalid caching entries left on the > > device. Fix it by adding loop for subdevices as well. Also, the domain-> > > has_iotlb_device needs to be updated when attaching to subdevices. > > > > Fixes: 67b8e02b5e761 ("iommu/vt-d: Aux-domain specific domain > attach/detach") > > Signed-off-by: Liu Yi L <yi.l.liu@intel.com> > > --- > > drivers/iommu/intel/iommu.c | 53 ++++++++++++++++++++++++++--------- > -- > > 1 file changed, 37 insertions(+), 16 deletions(-) > > > > diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c > > index d7720a836268..d48a60b61ba6 100644 > > --- a/drivers/iommu/intel/iommu.c > > +++ b/drivers/iommu/intel/iommu.c > > @@ -719,6 +719,8 @@ static int domain_update_device_node(struct > dmar_domain *domain) > > return nid; > > } > > > > +static void domain_update_iotlb(struct dmar_domain *domain); > > + > > /* Some capabilities may be different across iommus */ > > static void domain_update_iommu_cap(struct dmar_domain *domain) > > { > > @@ -744,6 +746,8 @@ static void domain_update_iommu_cap(struct > dmar_domain *domain) > > domain->domain.geometry.aperture_end = > __DOMAIN_MAX_ADDR(domain->gaw - 1); > > else > > domain->domain.geometry.aperture_end = > __DOMAIN_MAX_ADDR(domain->gaw); > > + > > + domain_update_iotlb(domain); > > } > > > > struct context_entry *iommu_context_addr(struct intel_iommu *iommu, > u8 bus, > > @@ -1464,17 +1468,22 @@ static void domain_update_iotlb(struct > dmar_domain *domain) > > > > assert_spin_locked(&device_domain_lock); > > > > - list_for_each_entry(info, &domain->devices, link) { > > - struct pci_dev *pdev; > > - > > - if (!info->dev || !dev_is_pci(info->dev)) > > - continue; > > - > > - pdev = to_pci_dev(info->dev); > > - if (pdev->ats_enabled) { > > + list_for_each_entry(info, &domain->devices, link) > > + if (info && info->ats_enabled) { > > has_iotlb_device = true; > > break; > > } > > + > > + if (!has_iotlb_device) { > > + struct subdev_domain_info *sinfo; > > + > > + list_for_each_entry(sinfo, &domain->subdevices, > link_domain) { > > + info = get_domain_info(sinfo->pdev); > > + if (info && info->ats_enabled) { > > + has_iotlb_device = true; > > + break; > > + } > > + } > > } > > > > domain->has_iotlb_device = has_iotlb_device; > > @@ -1555,25 +1564,37 @@ static void iommu_disable_dev_iotlb(struct > device_domain_info *info) > > #endif > > } > > > > +static void __iommu_flush_dev_iotlb(struct device_domain_info *info, > > + u64 addr, unsigned int mask) > > +{ > > + u16 sid, qdep; > > + > > + if (!info || !info->ats_enabled) > > + return; > > + > > + sid = info->bus << 8 | info->devfn; > > + qdep = info->ats_qdep; > > + qi_flush_dev_iotlb(info->iommu, sid, info->pfsid, > > + qdep, addr, mask); > > +} > > + > > static void iommu_flush_dev_iotlb(struct dmar_domain *domain, > > u64 addr, unsigned mask) > > { > > - u16 sid, qdep; > > unsigned long flags; > > struct device_domain_info *info; > > + struct subdev_domain_info *sinfo; > > > > if (!domain->has_iotlb_device) > > return; > > > > spin_lock_irqsave(&device_domain_lock, flags); > > - list_for_each_entry(info, &domain->devices, link) { > > - if (!info->ats_enabled) > > - continue; > > + list_for_each_entry(info, &domain->devices, link) > > + __iommu_flush_dev_iotlb(info, addr, mask); > > > > - sid = info->bus << 8 | info->devfn; > > - qdep = info->ats_qdep; > > - qi_flush_dev_iotlb(info->iommu, sid, info->pfsid, > > - qdep, addr, mask); > > + list_for_each_entry(sinfo, &domain->subdevices, link_domain) { > > + __iommu_flush_dev_iotlb(get_domain_info(sinfo->pdev), > > + addr, mask); > > } > > Nit: > list_for_each_entry(sinfo, &domain->subdevices, link_domain) { > info = get_domain_info(sinfo->pdev); > __iommu_flush_dev_iotlb(info, addr, mask); > } you are right. this should be better. > Others look good to me. > > Acked-by: Lu Baolu <baolu.lu@linux.intel.com> > > Best regards, > baolu Regards, Yi Liu > > spin_unlock_irqrestore(&device_domain_lock, flags); > > } > >
WARNING: multiple messages have this Message-ID (diff)
From: "Liu, Yi L" <yi.l.liu@intel.com> To: Lu Baolu <baolu.lu@linux.intel.com>, "joro@8bytes.org" <joro@8bytes.org>, "will@kernel.org" <will@kernel.org> Cc: "Tian, Kevin" <kevin.tian@intel.com>, "Raj, Ashok" <ashok.raj@intel.com>, "Tian, Jun J" <jun.j.tian@intel.com>, "iommu@lists.linux-foundation.org" <iommu@lists.linux-foundation.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "Sun, Yi Y" <yi.y.sun@intel.com> Subject: RE: [PATCH v3 3/3] iommu/vt-d: Fix ineffective devTLB invalidation for subdevices Date: Tue, 5 Jan 2021 05:50:22 +0000 [thread overview] Message-ID: <DM5PR11MB1435E814408F19A947263C07C3D10@DM5PR11MB1435.namprd11.prod.outlook.com> (raw) In-Reply-To: <c109eb64-2805-5e87-2283-b52c5704a31f@linux.intel.com> Hi Baolu, > From: Lu Baolu <baolu.lu@linux.intel.com> > Sent: Tuesday, December 29, 2020 4:42 PM > > Hi Yi, > > On 2020/12/29 11:25, Liu Yi L wrote: > > iommu_flush_dev_iotlb() is called to invalidate caches on device. It only > > loops the devices which are full-attached to the domain. For sub-devices, > > this is ineffective. This results in invalid caching entries left on the > > device. Fix it by adding loop for subdevices as well. Also, the domain-> > > has_iotlb_device needs to be updated when attaching to subdevices. > > > > Fixes: 67b8e02b5e761 ("iommu/vt-d: Aux-domain specific domain > attach/detach") > > Signed-off-by: Liu Yi L <yi.l.liu@intel.com> > > --- > > drivers/iommu/intel/iommu.c | 53 ++++++++++++++++++++++++++--------- > -- > > 1 file changed, 37 insertions(+), 16 deletions(-) > > > > diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c > > index d7720a836268..d48a60b61ba6 100644 > > --- a/drivers/iommu/intel/iommu.c > > +++ b/drivers/iommu/intel/iommu.c > > @@ -719,6 +719,8 @@ static int domain_update_device_node(struct > dmar_domain *domain) > > return nid; > > } > > > > +static void domain_update_iotlb(struct dmar_domain *domain); > > + > > /* Some capabilities may be different across iommus */ > > static void domain_update_iommu_cap(struct dmar_domain *domain) > > { > > @@ -744,6 +746,8 @@ static void domain_update_iommu_cap(struct > dmar_domain *domain) > > domain->domain.geometry.aperture_end = > __DOMAIN_MAX_ADDR(domain->gaw - 1); > > else > > domain->domain.geometry.aperture_end = > __DOMAIN_MAX_ADDR(domain->gaw); > > + > > + domain_update_iotlb(domain); > > } > > > > struct context_entry *iommu_context_addr(struct intel_iommu *iommu, > u8 bus, > > @@ -1464,17 +1468,22 @@ static void domain_update_iotlb(struct > dmar_domain *domain) > > > > assert_spin_locked(&device_domain_lock); > > > > - list_for_each_entry(info, &domain->devices, link) { > > - struct pci_dev *pdev; > > - > > - if (!info->dev || !dev_is_pci(info->dev)) > > - continue; > > - > > - pdev = to_pci_dev(info->dev); > > - if (pdev->ats_enabled) { > > + list_for_each_entry(info, &domain->devices, link) > > + if (info && info->ats_enabled) { > > has_iotlb_device = true; > > break; > > } > > + > > + if (!has_iotlb_device) { > > + struct subdev_domain_info *sinfo; > > + > > + list_for_each_entry(sinfo, &domain->subdevices, > link_domain) { > > + info = get_domain_info(sinfo->pdev); > > + if (info && info->ats_enabled) { > > + has_iotlb_device = true; > > + break; > > + } > > + } > > } > > > > domain->has_iotlb_device = has_iotlb_device; > > @@ -1555,25 +1564,37 @@ static void iommu_disable_dev_iotlb(struct > device_domain_info *info) > > #endif > > } > > > > +static void __iommu_flush_dev_iotlb(struct device_domain_info *info, > > + u64 addr, unsigned int mask) > > +{ > > + u16 sid, qdep; > > + > > + if (!info || !info->ats_enabled) > > + return; > > + > > + sid = info->bus << 8 | info->devfn; > > + qdep = info->ats_qdep; > > + qi_flush_dev_iotlb(info->iommu, sid, info->pfsid, > > + qdep, addr, mask); > > +} > > + > > static void iommu_flush_dev_iotlb(struct dmar_domain *domain, > > u64 addr, unsigned mask) > > { > > - u16 sid, qdep; > > unsigned long flags; > > struct device_domain_info *info; > > + struct subdev_domain_info *sinfo; > > > > if (!domain->has_iotlb_device) > > return; > > > > spin_lock_irqsave(&device_domain_lock, flags); > > - list_for_each_entry(info, &domain->devices, link) { > > - if (!info->ats_enabled) > > - continue; > > + list_for_each_entry(info, &domain->devices, link) > > + __iommu_flush_dev_iotlb(info, addr, mask); > > > > - sid = info->bus << 8 | info->devfn; > > - qdep = info->ats_qdep; > > - qi_flush_dev_iotlb(info->iommu, sid, info->pfsid, > > - qdep, addr, mask); > > + list_for_each_entry(sinfo, &domain->subdevices, link_domain) { > > + __iommu_flush_dev_iotlb(get_domain_info(sinfo->pdev), > > + addr, mask); > > } > > Nit: > list_for_each_entry(sinfo, &domain->subdevices, link_domain) { > info = get_domain_info(sinfo->pdev); > __iommu_flush_dev_iotlb(info, addr, mask); > } you are right. this should be better. > Others look good to me. > > Acked-by: Lu Baolu <baolu.lu@linux.intel.com> > > Best regards, > baolu Regards, Yi Liu > > spin_unlock_irqrestore(&device_domain_lock, flags); > > } > > _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
next prev parent reply other threads:[~2021-01-05 5:51 UTC|newest] Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-12-29 3:25 [PATCH v3 0/3] iommu/vt-d: Misc fixes on scalable mode Liu Yi L 2020-12-29 3:25 ` Liu Yi L 2020-12-29 3:25 ` [PATCH v3 1/3] iommu/vt-d: Move intel_iommu info from struct intel_svm to struct intel_svm_dev Liu Yi L 2020-12-29 3:25 ` Liu Yi L 2020-12-29 8:34 ` Lu Baolu 2020-12-29 8:34 ` Lu Baolu 2020-12-29 3:25 ` [PATCH v3 2/3] iommu/vt-d: Track device aux-attach with subdevice_domain_info Liu Yi L 2020-12-29 3:25 ` Liu Yi L 2020-12-29 8:38 ` Lu Baolu 2020-12-29 8:38 ` Lu Baolu 2021-01-05 5:58 ` Liu, Yi L 2021-01-05 5:58 ` Liu, Yi L 2020-12-29 3:25 ` [PATCH v3 3/3] iommu/vt-d: Fix ineffective devTLB invalidation for subdevices Liu Yi L 2020-12-29 3:25 ` Liu Yi L 2020-12-29 8:41 ` Lu Baolu 2020-12-29 8:41 ` Lu Baolu 2021-01-05 5:50 ` Liu, Yi L [this message] 2021-01-05 5:50 ` Liu, Yi L 2021-01-05 17:23 ` Will Deacon 2021-01-05 17:23 ` Will Deacon 2021-01-07 5:22 ` Liu, Yi L 2021-01-07 5:22 ` Liu, Yi L 2021-01-05 12:14 ` Dan Carpenter 2021-01-05 12:14 ` Dan Carpenter 2021-01-05 12:14 ` Dan Carpenter 2020-12-29 8:42 kernel test robot
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=DM5PR11MB1435E814408F19A947263C07C3D10@DM5PR11MB1435.namprd11.prod.outlook.com \ --to=yi.l.liu@intel.com \ --cc=ashok.raj@intel.com \ --cc=baolu.lu@linux.intel.com \ --cc=iommu@lists.linux-foundation.org \ --cc=jacob.jun.pan@linux.intel.com \ --cc=joro@8bytes.org \ --cc=jun.j.tian@intel.com \ --cc=kevin.tian@intel.com \ --cc=linux-kernel@vger.kernel.org \ --cc=will@kernel.org \ --cc=yi.y.sun@intel.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.