Re: [PATCH v2 1/7] iommu/vt-d: Refactor parameters for qi_submit_sync()

Wed, 15 Apr 2020 01:33:27 -0700 

On 2020/4/15 16:02, Tian, Kevin wrote:

From: Lu Baolu <[email protected]>
Sent: Wednesday, April 15, 2020 1:26 PM

Current qi_submit_sync() supports single invalidation descriptor
per submission and appends wait descriptor after each submission
to poll hardware completion. This patch adjusts the parameters
of this function so that multiple descriptors per submission can
be supported.

Signed-off-by: Jacob Pan <[email protected]>
Signed-off-by: Lu Baolu <[email protected]>
---
  drivers/iommu/dmar.c                | 24 ++++++++++++++----------
  drivers/iommu/intel-pasid.c         |  4 ++--
  drivers/iommu/intel-svm.c           |  6 +++---
  drivers/iommu/intel_irq_remapping.c |  2 +-
  include/linux/intel-iommu.h         |  8 +++++++-
  5 files changed, 27 insertions(+), 17 deletions(-)

diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index d9dc787feef7..bb42177e2369 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -1225,10 +1225,14 @@ static int qi_check_fault(struct intel_iommu
*iommu, int index)
  }

  /*
- * Submit the queued invalidation descriptor to the remapping
- * hardware unit and wait for its completion.
+ * Function to submit invalidation descriptors of all types to the queued
+ * invalidation interface(QI). Multiple descriptors can be submitted at a
+ * time, a wait descriptor will be appended to each submission to ensure
+ * hardware has completed the invalidation before return. Wait descriptors
+ * can be part of the submission but it will not be polled for completion.
   */
-int qi_submit_sync(struct qi_desc *desc, struct intel_iommu *iommu)
+int qi_submit_sync(struct intel_iommu *iommu, struct qi_desc *desc,
+                  unsigned int count, unsigned long options)


Adding parameter w/o actually using them is not typical way of splitting
patches. Better squash this with 2/7 together.


My original thought was to make it easier for code review. No particular
preference. Both are okay to me. :-)

Best regards,
baolu




  {
        int rc;
        struct q_inval *qi = iommu->qi;
@@ -1318,7 +1322,7 @@ void qi_global_iec(struct intel_iommu *iommu)
        desc.qw3 = 0;

        /* should never fail */
-       qi_submit_sync(&desc, iommu);
+       qi_submit_sync(iommu, &desc, 1, 0);
  }

  void qi_flush_context(struct intel_iommu *iommu, u16 did, u16 sid, u8 fm,
@@ -1332,7 +1336,7 @@ void qi_flush_context(struct intel_iommu *iommu,
u16 did, u16 sid, u8 fm,
        desc.qw2 = 0;
        desc.qw3 = 0;

-       qi_submit_sync(&desc, iommu);
+       qi_submit_sync(iommu, &desc, 1, 0);
  }

  void qi_flush_iotlb(struct intel_iommu *iommu, u16 did, u64 addr,
@@ -1356,7 +1360,7 @@ void qi_flush_iotlb(struct intel_iommu *iommu,
u16 did, u64 addr,
        desc.qw2 = 0;
        desc.qw3 = 0;

-       qi_submit_sync(&desc, iommu);
+       qi_submit_sync(iommu, &desc, 1, 0);
  }

  void qi_flush_dev_iotlb(struct intel_iommu *iommu, u16 sid, u16 pfsid,
@@ -1378,7 +1382,7 @@ void qi_flush_dev_iotlb(struct intel_iommu
*iommu, u16 sid, u16 pfsid,
        desc.qw2 = 0;
        desc.qw3 = 0;

-       qi_submit_sync(&desc, iommu);
+       qi_submit_sync(iommu, &desc, 1, 0);
  }

  /* PASID-based IOTLB invalidation */
@@ -1419,7 +1423,7 @@ void qi_flush_piotlb(struct intel_iommu *iommu,
u16 did, u32 pasid, u64 addr,
                                QI_EIOTLB_AM(mask);
        }

-       qi_submit_sync(&desc, iommu);
+       qi_submit_sync(iommu, &desc, 1, 0);
  }

  /* PASID-based device IOTLB Invalidate */
@@ -1448,7 +1452,7 @@ void qi_flush_dev_iotlb_pasid(struct intel_iommu
*iommu, u16 sid, u16 pfsid,
        if (size_order)
                desc.qw1 |= QI_DEV_EIOTLB_SIZE;

-       qi_submit_sync(&desc, iommu);
+       qi_submit_sync(iommu, &desc, 1, 0);
  }

  void qi_flush_pasid_cache(struct intel_iommu *iommu, u16 did,
@@ -1458,7 +1462,7 @@ void qi_flush_pasid_cache(struct intel_iommu
*iommu, u16 did,

        desc.qw0 = QI_PC_PASID(pasid) | QI_PC_DID(did) |
                        QI_PC_GRAN(granu) | QI_PC_TYPE;
-       qi_submit_sync(&desc, iommu);
+       qi_submit_sync(iommu, &desc, 1, 0);
  }

  /*
diff --git a/drivers/iommu/intel-pasid.c b/drivers/iommu/intel-pasid.c
index 48cc9ca5f3dc..7969e3dac2ad 100644
--- a/drivers/iommu/intel-pasid.c
+++ b/drivers/iommu/intel-pasid.c
@@ -498,7 +498,7 @@ pasid_cache_invalidation_with_pasid(struct
intel_iommu *iommu,
        desc.qw2 = 0;
        desc.qw3 = 0;

-       qi_submit_sync(&desc, iommu);
+       qi_submit_sync(iommu, &desc, 1, 0);
  }

  static void
@@ -512,7 +512,7 @@ iotlb_invalidation_with_pasid(struct intel_iommu
*iommu, u16 did, u32 pasid)
        desc.qw2 = 0;
        desc.qw3 = 0;

-       qi_submit_sync(&desc, iommu);
+       qi_submit_sync(iommu, &desc, 1, 0);
  }

  static void
diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
index e9f4e979a71f..83dc4319f661 100644
--- a/drivers/iommu/intel-svm.c
+++ b/drivers/iommu/intel-svm.c
@@ -138,7 +138,7 @@ static void intel_flush_svm_range_dev (struct
intel_svm *svm, struct intel_svm_d
        }
        desc.qw2 = 0;
        desc.qw3 = 0;
-       qi_submit_sync(&desc, svm->iommu);
+       qi_submit_sync(svm->iommu, &desc, 1, 0);

        if (sdev->dev_iotlb) {
                desc.qw0 = QI_DEV_EIOTLB_PASID(svm->pasid) |
@@ -162,7 +162,7 @@ static void intel_flush_svm_range_dev (struct
intel_svm *svm, struct intel_svm_d
                }
                desc.qw2 = 0;
                desc.qw3 = 0;
-               qi_submit_sync(&desc, svm->iommu);
+               qi_submit_sync(svm->iommu, &desc, 1, 0);
        }
  }

@@ -850,7 +850,7 @@ static irqreturn_t prq_event_thread(int irq, void *d)
                                       sizeof(req->priv_data));
                        resp.qw2 = 0;
                        resp.qw3 = 0;
-                       qi_submit_sync(&resp, iommu);
+                       qi_submit_sync(iommu, &resp, 1, 0);
                }
                head = (head + sizeof(*req)) & PRQ_RING_MASK;
        }
diff --git a/drivers/iommu/intel_irq_remapping.c
b/drivers/iommu/intel_irq_remapping.c
index 81e43c1df7ec..a042f123b091 100644
--- a/drivers/iommu/intel_irq_remapping.c
+++ b/drivers/iommu/intel_irq_remapping.c
@@ -151,7 +151,7 @@ static int qi_flush_iec(struct intel_iommu *iommu, int
index, int mask)
        desc.qw2 = 0;
        desc.qw3 = 0;

-       return qi_submit_sync(&desc, iommu);
+       return qi_submit_sync(iommu, &desc, 1, 0);
  }

  static int modify_irte(struct irq_2_iommu *irq_iommu,
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index cfe720f10112..ee2d5cdd8339 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -710,7 +710,13 @@ void qi_flush_dev_iotlb_pasid(struct intel_iommu
*iommu, u16 sid, u16 pfsid,
  void qi_flush_pasid_cache(struct intel_iommu *iommu, u16 did, u64 granu,
                          int pasid);

-extern int qi_submit_sync(struct qi_desc *desc, struct intel_iommu *iommu);
+int qi_submit_sync(struct intel_iommu *iommu, struct qi_desc *desc,
+                  unsigned int count, unsigned long options);
+/*
+ * Options used in qi_submit_sync:
+ * QI_OPT_WAIT_DRAIN - Wait for PRQ drain completion, spec 6.5.2.8.
+ */
+#define QI_OPT_WAIT_DRAIN              BIT(0)


no one uses this flag in this patch



  extern int dmar_ir_support(void);

--
2.17.1


Thanks
Kevin


_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Reply via email to