1. Patchew
  2. linux
  3. [PATCH v2 0/5] Batch IOTLB/dev-IOTLB invalidation
  4. View patch

[PATCH v2 4/5] vt-d/iommu: Refactor quirk_extra_dev_tlb_flush()

Tina Zhang posted 5 patches 1 year, 6 months ago
There is a newer version of this series
[PATCH v2 4/5] vt-d/iommu: Refactor quirk_extra_dev_tlb_flush()
Posted by Tina Zhang 1 year, 6 months ago 
Extract the core logic from quirk_extra_dev_tlb_flush() into a new
helper __quirk_extra_dev_tlb_flush(). This helper is for accommodating
for both individual and batched TLB invalidation commands, thereby
streamlining the process for handling device-specific TLB flush quirks.

Signed-off-by: Tina Zhang <tina.zhang@intel.com>
---
 drivers/iommu/intel/iommu.c | 55 +++++++++++++++++++++++++++++--------
 drivers/iommu/intel/iommu.h |  4 +++
 2 files changed, 47 insertions(+), 12 deletions(-)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 9ff8b83c19a3..160d569015b4 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4875,6 +4875,41 @@ static void __init check_tylersburg_isoch(void)
 	       vtisochctrl);
 }
 
+static inline void __quirk_extra_dev_tlb_flush(struct device_domain_info *info,
+					       unsigned long address, unsigned long mask,
+					       u32 pasid, u16 qdep,
+					       struct qi_batch *batch)
+{
+	u16 sid;
+
+	if (likely(!info->dtlb_extra_inval))
+		return;
+
+	sid = PCI_DEVID(info->bus, info->devfn);
+	if (batch == NULL) {
+		if (pasid == IOMMU_NO_PASID)
+			qi_flush_dev_iotlb(info->iommu, sid, info->pfsid,
+					   qdep, address, mask);
+		else
+			qi_flush_dev_iotlb_pasid(info->iommu, sid,
+						 info->pfsid, pasid,
+						 qdep, address, mask);
+	} else {
+		if (pasid == IOMMU_NO_PASID)
+			qi_batch_add_dev_iotlb_desc(info->iommu, sid,
+						    info->pfsid, qdep,
+						    address, mask, batch);
+		else
+			qi_batch_add_dev_iotlb_pasid_desc(info->iommu,
+							  sid,
+							  info->pfsid,
+							  pasid, qdep,
+							  address,
+							  mask,
+							  batch);
+	}
+}
+
 /*
  * Here we deal with a device TLB defect where device may inadvertently issue ATS
  * invalidation completion before posted writes initiated with translated address
@@ -4905,19 +4940,15 @@ void quirk_extra_dev_tlb_flush(struct device_domain_info *info,
 			       unsigned long address, unsigned long mask,
 			       u32 pasid, u16 qdep)
 {
-	u16 sid;
+	__quirk_extra_dev_tlb_flush(info, address, mask, pasid, qdep, NULL);
+}
 
-	if (likely(!info->dtlb_extra_inval))
-		return;
-
-	sid = PCI_DEVID(info->bus, info->devfn);
-	if (pasid == IOMMU_NO_PASID) {
-		qi_flush_dev_iotlb(info->iommu, sid, info->pfsid,
-				   qdep, address, mask);
-	} else {
-		qi_flush_dev_iotlb_pasid(info->iommu, sid, info->pfsid,
-					 pasid, qdep, address, mask);
-	}
+void batch_quirk_extra_dev_tlb_flush(struct device_domain_info *info,
+				     unsigned long address, unsigned long mask,
+				     u32 pasid, u16 qdep,
+				     struct qi_batch *batch)
+{
+	__quirk_extra_dev_tlb_flush(info, address, mask, pasid, qdep, batch);
 }
 
 #define ecmd_get_status_code(res)	(((res) & 0xff) >> 1)
diff --git a/drivers/iommu/intel/iommu.h b/drivers/iommu/intel/iommu.h
index cd7c1d0a01c6..04aa1f200124 100644
--- a/drivers/iommu/intel/iommu.h
+++ b/drivers/iommu/intel/iommu.h
@@ -1109,6 +1109,10 @@ void qi_flush_dev_iotlb_pasid(struct intel_iommu *iommu, u16 sid, u16 pfsid,
 void quirk_extra_dev_tlb_flush(struct device_domain_info *info,
 			       unsigned long address, unsigned long pages,
 			       u32 pasid, u16 qdep);
+void batch_quirk_extra_dev_tlb_flush(struct device_domain_info *info,
+				     unsigned long address, unsigned long mask,
+				     u32 pasid, u16 qdep,
+				     struct qi_batch *batch);
 void qi_flush_pasid_cache(struct intel_iommu *iommu, u16 did, u64 granu,
 			  u32 pasid);
 
-- 
2.43.0
Re: [PATCH v2 4/5] vt-d/iommu: Refactor quirk_extra_dev_tlb_flush()
Posted by Baolu Lu 1 year, 6 months ago 
On 2024/8/9 10:54, Tina Zhang wrote:
> Extract the core logic from quirk_extra_dev_tlb_flush() into a new
> helper __quirk_extra_dev_tlb_flush(). This helper is for accommodating
> for both individual and batched TLB invalidation commands, thereby
> streamlining the process for handling device-specific TLB flush quirks.
> 
> Signed-off-by: Tina Zhang<tina.zhang@intel.com>
> ---
>   drivers/iommu/intel/iommu.c | 55 +++++++++++++++++++++++++++++--------
>   drivers/iommu/intel/iommu.h |  4 +++
>   2 files changed, 47 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 9ff8b83c19a3..160d569015b4 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -4875,6 +4875,41 @@ static void __init check_tylersburg_isoch(void)
>   	       vtisochctrl);
>   }
>   
> +static inline void __quirk_extra_dev_tlb_flush(struct device_domain_info *info,
> +					       unsigned long address, unsigned long mask,
> +					       u32 pasid, u16 qdep,
> +					       struct qi_batch *batch)
> +{
> +	u16 sid;
> +
> +	if (likely(!info->dtlb_extra_inval))
> +		return;
> +
> +	sid = PCI_DEVID(info->bus, info->devfn);
> +	if (batch == NULL) {
> +		if (pasid == IOMMU_NO_PASID)
> +			qi_flush_dev_iotlb(info->iommu, sid, info->pfsid,
> +					   qdep, address, mask);
> +		else
> +			qi_flush_dev_iotlb_pasid(info->iommu, sid,
> +						 info->pfsid, pasid,
> +						 qdep, address, mask);
> +	} else {
> +		if (pasid == IOMMU_NO_PASID)
> +			qi_batch_add_dev_iotlb_desc(info->iommu, sid,
> +						    info->pfsid, qdep,
> +						    address, mask, batch);
> +		else
> +			qi_batch_add_dev_iotlb_pasid_desc(info->iommu,
> +							  sid,
> +							  info->pfsid,
> +							  pasid, qdep,
> +							  address,
> +							  mask,
> +							  batch);
> +	}
> +}

How about moving this helper into cache.c? That's its only or major
consumer, right?

By the way, in which case could 'batch' be a NULL?

Thanks,
baolu
RE: [PATCH v2 4/5] vt-d/iommu: Refactor quirk_extra_dev_tlb_flush()
Posted by Zhang, Tina 1 year, 6 months ago 
Hi Baolu,

> -----Original Message-----
> From: Baolu Lu <baolu.lu@linux.intel.com>
> Sent: Friday, August 9, 2024 4:26 PM
> To: Zhang, Tina <tina.zhang@intel.com>; Tian, Kevin <kevin.tian@intel.com>
> Cc: baolu.lu@linux.intel.com; iommu@lists.linux.dev; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH v2 4/5] vt-d/iommu: Refactor quirk_extra_dev_tlb_flush()
> 
> On 2024/8/9 10:54, Tina Zhang wrote:
> > Extract the core logic from quirk_extra_dev_tlb_flush() into a new
> > helper __quirk_extra_dev_tlb_flush(). This helper is for accommodating
> > for both individual and batched TLB invalidation commands, thereby
> > streamlining the process for handling device-specific TLB flush quirks.
> >
> > Signed-off-by: Tina Zhang<tina.zhang@intel.com>
> > ---
> >   drivers/iommu/intel/iommu.c | 55 +++++++++++++++++++++++++++++-----
> ---
> >   drivers/iommu/intel/iommu.h |  4 +++
> >   2 files changed, 47 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> > index 9ff8b83c19a3..160d569015b4 100644
> > --- a/drivers/iommu/intel/iommu.c
> > +++ b/drivers/iommu/intel/iommu.c
> > @@ -4875,6 +4875,41 @@ static void __init check_tylersburg_isoch(void)
> >   	       vtisochctrl);
> >   }
> >
> > +static inline void __quirk_extra_dev_tlb_flush(struct device_domain_info
> *info,
> > +					       unsigned long address, unsigned
> long mask,
> > +					       u32 pasid, u16 qdep,
> > +					       struct qi_batch *batch)
> > +{
> > +	u16 sid;
> > +
> > +	if (likely(!info->dtlb_extra_inval))
> > +		return;
> > +
> > +	sid = PCI_DEVID(info->bus, info->devfn);
> > +	if (batch == NULL) {
> > +		if (pasid == IOMMU_NO_PASID)
> > +			qi_flush_dev_iotlb(info->iommu, sid, info->pfsid,
> > +					   qdep, address, mask);
> > +		else
> > +			qi_flush_dev_iotlb_pasid(info->iommu, sid,
> > +						 info->pfsid, pasid,
> > +						 qdep, address, mask);
> > +	} else {
> > +		if (pasid == IOMMU_NO_PASID)
> > +			qi_batch_add_dev_iotlb_desc(info->iommu, sid,
> > +						    info->pfsid, qdep,
> > +						    address, mask, batch);
> > +		else
> > +			qi_batch_add_dev_iotlb_pasid_desc(info->iommu,
> > +							  sid,
> > +							  info->pfsid,
> > +							  pasid, qdep,
> > +							  address,
> > +							  mask,
> > +							  batch);
> > +	}
> > +}
> 
> How about moving this helper into cache.c? That's its only or major consumer,
> right?
The quirk_extra_dev_tlb_flush() can also get invoked by pasid.c

> 
> By the way, in which case could 'batch' be a NULL?
In this patch, I move the core logic in quirk_extra_dev_tlb_flush() into this new function __quirk_extra_dev_tlb_flush() and make the invokers, who don’t expect batch processing, invoke __quirk_extra_dev_tlb_flush() with 'batch' be NULL, see:

/*
  * Here we deal with a device TLB defect where device may inadvertently issue ATS
  * invalidation completion before posted writes initiated with translated address
@@ -4905,19 +4940,15 @@ void quirk_extra_dev_tlb_flush(struct device_domain_info *info,
                               unsigned long address, unsigned long mask,
                               u32 pasid, u16 qdep)
 {
-       u16 sid;
+       __quirk_extra_dev_tlb_flush(info, address, mask, pasid, qdep, NULL);
+}

Regards,
-Tina
> 
> Thanks,
> baolu

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.