Messages in this thread | | | Subject | Re: [PATCH v2 1/7] iommu/vt-d: Refactor parameters for qi_submit_sync() | From | Lu Baolu <> | Date | Wed, 15 Apr 2020 16:33:15 +0800 |
| |
On 2020/4/15 16:02, Tian, Kevin wrote: >> From: Lu Baolu <baolu.lu@linux.intel.com> >> 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 <jacob.jun.pan@linux.intel.com> >> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com> >> --- >> 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 >
| |