| Message ID | 20240912130427.10119-3-yi.l.liu@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | Make set_dev_pasid op supporting domain replacement | expand |
On 9/12/24 9:04 PM, Yi Liu wrote: > diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c > index b51fc268dc84..ceb9c5274a39 100644 > --- a/drivers/iommu/intel/pasid.c > +++ b/drivers/iommu/intel/pasid.c > @@ -236,8 +236,13 @@ devtlb_invalidation_with_pasid(struct intel_iommu *iommu, > qi_flush_dev_iotlb_pasid(iommu, sid, pfsid, pasid, qdep, 0, 64 - VTD_PAGE_SHIFT); > } > > +/* > + * Not all PASID entry destroy requires PRQ drain as it can be handled in > + * the remove_dev_pasid path. Caller should be clear about it and set the > + * @flags properly. > + */ > void intel_pasid_tear_down_entry(struct intel_iommu *iommu, struct device *dev, > - u32 pasid, bool fault_ignore) > + u32 pasid, u32 flags) As a generic opt-in bit flags, why not using 'unsigned int'? Thanks, baolu
On 2024/9/12 21:22, Baolu Lu wrote: > On 2024/9/12 21:04, Yi Liu wrote: >> Draining PRQ is mostly conjuncted with pasid teardown, and with more >> callers coming, >> move it into it in the intel_pasid_tear_down_entry(). But there is >> scenario that only >> teardown pasid entry but no PRQ drain, so passing a flag to mark it. > > Is it a reasonable case where PRI needs to be drained but the pasid > entry won't be torn down? For example, could this happen when a PRI is > disabled? in concept, yes. But it seems no more than a debugging method in my opinion. I cannot map it to a usage so far. >> >> Signed-off-by: Yi Liu<yi.l.liu@intel.com> >> --- >> drivers/iommu/intel/iommu.c | 8 ++++---- >> drivers/iommu/intel/pasid.c | 13 +++++++++++-- >> drivers/iommu/intel/pasid.h | 8 +++++--- >> drivers/iommu/intel/svm.c | 3 ++- >> 4 files changed, 22 insertions(+), 10 deletions(-) > > Thanks, > baolu
On 2024/9/13 10:11, Baolu Lu wrote: > On 9/12/24 9:04 PM, Yi Liu wrote: >> diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c >> index b51fc268dc84..ceb9c5274a39 100644 >> --- a/drivers/iommu/intel/pasid.c >> +++ b/drivers/iommu/intel/pasid.c >> @@ -236,8 +236,13 @@ devtlb_invalidation_with_pasid(struct intel_iommu >> *iommu, >> qi_flush_dev_iotlb_pasid(iommu, sid, pfsid, pasid, qdep, 0, 64 >> - VTD_PAGE_SHIFT); >> } >> +/* >> + * Not all PASID entry destroy requires PRQ drain as it can be handled in >> + * the remove_dev_pasid path. Caller should be clear about it and set the >> + * @flags properly. >> + */ >> void intel_pasid_tear_down_entry(struct intel_iommu *iommu, struct >> device *dev, >> - u32 pasid, bool fault_ignore) >> + u32 pasid, u32 flags) > > As a generic opt-in bit flags, why not using 'unsigned int'? aha, I can use 'unsigned int'.
> From: Liu, Yi L <yi.l.liu@intel.com> > Sent: Thursday, September 12, 2024 9:04 PM > > +/* > + * Not all PASID entry destroy requires PRQ drain as it can be handled in > + * the remove_dev_pasid path. Caller should be clear about it and set the > + * @flags properly. > + */ /* * Caller can request to drain PRQ in this helper if it hasn't done so, * e.g. in a path which doesn't follow remove_dev_pasid(). */ Reviewed-by: Kevin Tian <kevin.tian@intel.com>
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c index 1a2a5cf4ef60..80b587de226d 100644 --- a/drivers/iommu/intel/iommu.c +++ b/drivers/iommu/intel/iommu.c @@ -3376,7 +3376,7 @@ void device_block_translation(struct device *dev) if (!dev_is_real_dma_subdevice(dev)) { if (sm_supported(iommu)) intel_pasid_tear_down_entry(iommu, dev, - IOMMU_NO_PASID, false); + IOMMU_NO_PASID, 0); else domain_context_clear(info); } @@ -4258,7 +4258,7 @@ static void intel_iommu_remove_dev_pasid(struct device *dev, ioasid_t pasid, unsigned long flags; if (domain->type == IOMMU_DOMAIN_IDENTITY) { - intel_pasid_tear_down_entry(iommu, dev, pasid, false); + intel_pasid_tear_down_entry(iommu, dev, pasid, 0); return; } @@ -4278,8 +4278,8 @@ static void intel_iommu_remove_dev_pasid(struct device *dev, ioasid_t pasid, domain_detach_iommu(dmar_domain, iommu); intel_iommu_debugfs_remove_dev_pasid(dev_pasid); kfree(dev_pasid); - intel_pasid_tear_down_entry(iommu, dev, pasid, false); - intel_drain_pasid_prq(dev, pasid); + intel_pasid_tear_down_entry(iommu, dev, pasid, + INTEL_PASID_TEARDOWN_DRAIN_PRQ); } static int intel_iommu_set_dev_pasid(struct iommu_domain *domain, diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c index b51fc268dc84..ceb9c5274a39 100644 --- a/drivers/iommu/intel/pasid.c +++ b/drivers/iommu/intel/pasid.c @@ -236,8 +236,13 @@ devtlb_invalidation_with_pasid(struct intel_iommu *iommu, qi_flush_dev_iotlb_pasid(iommu, sid, pfsid, pasid, qdep, 0, 64 - VTD_PAGE_SHIFT); } +/* + * Not all PASID entry destroy requires PRQ drain as it can be handled in + * the remove_dev_pasid path. Caller should be clear about it and set the + * @flags properly. + */ void intel_pasid_tear_down_entry(struct intel_iommu *iommu, struct device *dev, - u32 pasid, bool fault_ignore) + u32 pasid, u32 flags) { struct pasid_entry *pte; u16 did, pgtt; @@ -251,7 +256,8 @@ void intel_pasid_tear_down_entry(struct intel_iommu *iommu, struct device *dev, did = pasid_get_domain_id(pte); pgtt = pasid_pte_get_pgtt(pte); - intel_pasid_clear_entry(dev, pasid, fault_ignore); + intel_pasid_clear_entry(dev, pasid, + flags & INTEL_PASID_TEARDOWN_IGNORE_FAULT); spin_unlock(&iommu->lock); if (!ecap_coherent(iommu->ecap)) @@ -267,6 +273,9 @@ void intel_pasid_tear_down_entry(struct intel_iommu *iommu, struct device *dev, /* Device IOTLB doesn't need to be flushed in caching mode. */ if (!cap_caching_mode(iommu->cap)) devtlb_invalidation_with_pasid(iommu, dev, pasid); + + if (flags & INTEL_PASID_TEARDOWN_DRAIN_PRQ) + intel_drain_pasid_prq(dev, pasid); } /* diff --git a/drivers/iommu/intel/pasid.h b/drivers/iommu/intel/pasid.h index dde6d3ba5ae0..6eb849ec5fb8 100644 --- a/drivers/iommu/intel/pasid.h +++ b/drivers/iommu/intel/pasid.h @@ -303,9 +303,11 @@ int intel_pasid_setup_pass_through(struct intel_iommu *iommu, struct device *dev, u32 pasid); int intel_pasid_setup_nested(struct intel_iommu *iommu, struct device *dev, u32 pasid, struct dmar_domain *domain); -void intel_pasid_tear_down_entry(struct intel_iommu *iommu, - struct device *dev, u32 pasid, - bool fault_ignore); + +#define INTEL_PASID_TEARDOWN_IGNORE_FAULT (1U << 0) +#define INTEL_PASID_TEARDOWN_DRAIN_PRQ (1U << 1) +void intel_pasid_tear_down_entry(struct intel_iommu *iommu, struct device *dev, + u32 pasid, u32 flags); void intel_pasid_setup_page_snoop_control(struct intel_iommu *iommu, struct device *dev, u32 pasid); int intel_pasid_setup_sm_context(struct device *dev); diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c index 5ae1df7598b7..3c1e105b9da6 100644 --- a/drivers/iommu/intel/svm.c +++ b/drivers/iommu/intel/svm.c @@ -176,7 +176,8 @@ static void intel_mm_release(struct mmu_notifier *mn, struct mm_struct *mm) list_for_each_entry(dev_pasid, &domain->dev_pasids, link_domain) { info = dev_iommu_priv_get(dev_pasid->dev); intel_pasid_tear_down_entry(info->iommu, dev_pasid->dev, - dev_pasid->pasid, true); + dev_pasid->pasid, + INTEL_PASID_TEARDOWN_IGNORE_FAULT); } spin_unlock_irqrestore(&domain->lock, flags);
Draining PRQ is mostly conjuncted with pasid teardown, and with more callers coming, move it into it in the intel_pasid_tear_down_entry(). But there is scenario that only teardown pasid entry but no PRQ drain, so passing a flag to mark it. Signed-off-by: Yi Liu <yi.l.liu@intel.com> --- drivers/iommu/intel/iommu.c | 8 ++++---- drivers/iommu/intel/pasid.c | 13 +++++++++++-- drivers/iommu/intel/pasid.h | 8 +++++--- drivers/iommu/intel/svm.c | 3 ++- 4 files changed, 22 insertions(+), 10 deletions(-)