* [PATCH v2 1/5] iommu/vt-d: Refactor IOTLB/Dev-IOTLB invalidation command logic
2024-08-09 2:54 [PATCH v2 0/5] Batch IOTLB/dev-IOTLB invalidation Tina Zhang
@ 2024-08-09 2:54 ` Tina Zhang
2024-08-09 2:54 ` [PATCH v2 2/5] iommu/vt-d: Refactor IOTLB and Dev-IOTLB flush logic Tina Zhang
` (3 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Tina Zhang @ 2024-08-09 2:54 UTC (permalink / raw)
To: Lu Baolu, Kevin Tian; +Cc: iommu, linux-kernel, Tina Zhang
Separate assembling IOTLB/dev-IOTLB invalidation command logic from
qi_flush interfaces. New qi_desc() functions are introduced for this
purpose. The goal is to facilitate the reuse of these qi_desc()
functions in the qi_batch interfaces which will be introduced in a
subsequent patch.
Signed-off-by: Tina Zhang <tina.zhang@intel.com>
---
drivers/iommu/intel/dmar.c | 203 +++++++++++++++++++++----------------
1 file changed, 116 insertions(+), 87 deletions(-)
diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
index 1c8d3141cb55..64724af1a618 100644
--- a/drivers/iommu/intel/dmar.c
+++ b/drivers/iommu/intel/dmar.c
@@ -1360,6 +1360,116 @@ static int qi_check_fault(struct intel_iommu *iommu, int index, int wait_index)
return 0;
}
+static void qi_desc_iotlb(struct intel_iommu *iommu, u16 did,
+ u64 addr, unsigned int size_order,
+ u64 type, struct qi_desc *desc)
+{
+ u8 dw = 0, dr = 0;
+ int ih = 0;
+
+ if (cap_write_drain(iommu->cap))
+ dw = 1;
+
+ if (cap_read_drain(iommu->cap))
+ dr = 1;
+
+ desc->qw0 = QI_IOTLB_DID(did) | QI_IOTLB_DR(dr) | QI_IOTLB_DW(dw)
+ | QI_IOTLB_GRAN(type) | QI_IOTLB_TYPE;
+ desc->qw1 = QI_IOTLB_ADDR(addr) | QI_IOTLB_IH(ih)
+ | QI_IOTLB_AM(size_order);
+ desc->qw2 = 0;
+ desc->qw3 = 0;
+}
+
+static void qi_desc_dev_iotlb(u16 sid, u16 pfsid,
+ u16 qdep, u64 addr,
+ unsigned int mask,
+ struct qi_desc *desc)
+{
+ if (mask) {
+ addr |= (1ULL << (VTD_PAGE_SHIFT + mask - 1)) - 1;
+ desc->qw1 = QI_DEV_IOTLB_ADDR(addr) | QI_DEV_IOTLB_SIZE;
+ } else
+ desc->qw1 = QI_DEV_IOTLB_ADDR(addr);
+
+ if (qdep >= QI_DEV_IOTLB_MAX_INVS)
+ qdep = 0;
+
+ desc->qw0 = QI_DEV_IOTLB_SID(sid) | QI_DEV_IOTLB_QDEP(qdep) |
+ QI_DIOTLB_TYPE | QI_DEV_IOTLB_PFSID(pfsid);
+ desc->qw2 = 0;
+ desc->qw3 = 0;
+}
+
+static void qi_desc_piotlb(u16 did, u32 pasid, u64 addr,
+ unsigned long npages, bool ih,
+ struct qi_desc *desc)
+{
+ if (npages == -1) {
+ desc->qw0 = QI_EIOTLB_PASID(pasid) |
+ QI_EIOTLB_DID(did) |
+ QI_EIOTLB_GRAN(QI_GRAN_NONG_PASID) |
+ QI_EIOTLB_TYPE;
+ desc->qw1 = 0;
+ } else {
+ int mask = ilog2(__roundup_pow_of_two(npages));
+ unsigned long align = (1ULL << (VTD_PAGE_SHIFT + mask));
+
+ if (WARN_ON_ONCE(!IS_ALIGNED(addr, align)))
+ addr = ALIGN_DOWN(addr, align);
+
+ desc->qw0 = QI_EIOTLB_PASID(pasid) |
+ QI_EIOTLB_DID(did) |
+ QI_EIOTLB_GRAN(QI_GRAN_PSI_PASID) |
+ QI_EIOTLB_TYPE;
+ desc->qw1 = QI_EIOTLB_ADDR(addr) |
+ QI_EIOTLB_IH(ih) |
+ QI_EIOTLB_AM(mask);
+ }
+}
+
+static void qi_desc_dev_iotlb_pasid(u16 sid, u16 pfsid,
+ u32 pasid, u16 qdep, u64 addr,
+ unsigned int size_order,
+ struct qi_desc *desc)
+{
+ unsigned long mask = 1UL << (VTD_PAGE_SHIFT + size_order - 1);
+
+ desc->qw0 = QI_DEV_EIOTLB_PASID(pasid) | QI_DEV_EIOTLB_SID(sid) |
+ QI_DEV_EIOTLB_QDEP(qdep) | QI_DEIOTLB_TYPE |
+ QI_DEV_IOTLB_PFSID(pfsid);
+
+ /*
+ * If S bit is 0, we only flush a single page. If S bit is set,
+ * The least significant zero bit indicates the invalidation address
+ * range. VT-d spec 6.5.2.6.
+ * e.g. address bit 12[0] indicates 8KB, 13[0] indicates 16KB.
+ * size order = 0 is PAGE_SIZE 4KB
+ * Max Invs Pending (MIP) is set to 0 for now until we have DIT in
+ * ECAP.
+ */
+ if (!IS_ALIGNED(addr, VTD_PAGE_SIZE << size_order))
+ pr_warn_ratelimited("Invalidate non-aligned address %llx, order %d\n",
+ addr, size_order);
+
+ /* Take page address */
+ desc->qw1 = QI_DEV_EIOTLB_ADDR(addr);
+
+ if (size_order) {
+ /*
+ * Existing 0s in address below size_order may be the least
+ * significant bit, we must set them to 1s to avoid having
+ * smaller size than desired.
+ */
+ desc->qw1 |= GENMASK_ULL(size_order + VTD_PAGE_SHIFT - 1,
+ VTD_PAGE_SHIFT);
+ /* Clear size_order bit to indicate size */
+ desc->qw1 &= ~mask;
+ /* Set the S bit to indicate flushing more than 1 page */
+ desc->qw1 |= QI_DEV_EIOTLB_SIZE;
+ }
+}
+
/*
* Function to submit invalidation descriptors of all types to the queued
* invalidation interface(QI). Multiple descriptors can be submitted at a
@@ -1520,24 +1630,9 @@ void qi_flush_context(struct intel_iommu *iommu, u16 did, u16 sid, u8 fm,
void qi_flush_iotlb(struct intel_iommu *iommu, u16 did, u64 addr,
unsigned int size_order, u64 type)
{
- u8 dw = 0, dr = 0;
-
struct qi_desc desc;
- int ih = 0;
-
- if (cap_write_drain(iommu->cap))
- dw = 1;
-
- if (cap_read_drain(iommu->cap))
- dr = 1;
-
- desc.qw0 = QI_IOTLB_DID(did) | QI_IOTLB_DR(dr) | QI_IOTLB_DW(dw)
- | QI_IOTLB_GRAN(type) | QI_IOTLB_TYPE;
- desc.qw1 = QI_IOTLB_ADDR(addr) | QI_IOTLB_IH(ih)
- | QI_IOTLB_AM(size_order);
- desc.qw2 = 0;
- desc.qw3 = 0;
+ qi_desc_iotlb(iommu, did, addr, size_order, type, &desc);
qi_submit_sync(iommu, &desc, 1, 0);
}
@@ -1555,20 +1650,7 @@ void qi_flush_dev_iotlb(struct intel_iommu *iommu, u16 sid, u16 pfsid,
if (!(iommu->gcmd & DMA_GCMD_TE))
return;
- if (mask) {
- addr |= (1ULL << (VTD_PAGE_SHIFT + mask - 1)) - 1;
- desc.qw1 = QI_DEV_IOTLB_ADDR(addr) | QI_DEV_IOTLB_SIZE;
- } else
- desc.qw1 = QI_DEV_IOTLB_ADDR(addr);
-
- if (qdep >= QI_DEV_IOTLB_MAX_INVS)
- qdep = 0;
-
- desc.qw0 = QI_DEV_IOTLB_SID(sid) | QI_DEV_IOTLB_QDEP(qdep) |
- QI_DIOTLB_TYPE | QI_DEV_IOTLB_PFSID(pfsid);
- desc.qw2 = 0;
- desc.qw3 = 0;
-
+ qi_desc_dev_iotlb(sid, pfsid, qdep, addr, mask, &desc);
qi_submit_sync(iommu, &desc, 1, 0);
}
@@ -1588,28 +1670,7 @@ void qi_flush_piotlb(struct intel_iommu *iommu, u16 did, u32 pasid, u64 addr,
return;
}
- if (npages == -1) {
- desc.qw0 = QI_EIOTLB_PASID(pasid) |
- QI_EIOTLB_DID(did) |
- QI_EIOTLB_GRAN(QI_GRAN_NONG_PASID) |
- QI_EIOTLB_TYPE;
- desc.qw1 = 0;
- } else {
- int mask = ilog2(__roundup_pow_of_two(npages));
- unsigned long align = (1ULL << (VTD_PAGE_SHIFT + mask));
-
- if (WARN_ON_ONCE(!IS_ALIGNED(addr, align)))
- addr = ALIGN_DOWN(addr, align);
-
- desc.qw0 = QI_EIOTLB_PASID(pasid) |
- QI_EIOTLB_DID(did) |
- QI_EIOTLB_GRAN(QI_GRAN_PSI_PASID) |
- QI_EIOTLB_TYPE;
- desc.qw1 = QI_EIOTLB_ADDR(addr) |
- QI_EIOTLB_IH(ih) |
- QI_EIOTLB_AM(mask);
- }
-
+ qi_desc_piotlb(did, pasid, addr, npages, ih, &desc);
qi_submit_sync(iommu, &desc, 1, 0);
}
@@ -1617,7 +1678,6 @@ void qi_flush_piotlb(struct intel_iommu *iommu, u16 did, u32 pasid, u64 addr,
void qi_flush_dev_iotlb_pasid(struct intel_iommu *iommu, u16 sid, u16 pfsid,
u32 pasid, u16 qdep, u64 addr, unsigned int size_order)
{
- unsigned long mask = 1UL << (VTD_PAGE_SHIFT + size_order - 1);
struct qi_desc desc = {.qw1 = 0, .qw2 = 0, .qw3 = 0};
/*
@@ -1629,40 +1689,9 @@ void qi_flush_dev_iotlb_pasid(struct intel_iommu *iommu, u16 sid, u16 pfsid,
if (!(iommu->gcmd & DMA_GCMD_TE))
return;
- desc.qw0 = QI_DEV_EIOTLB_PASID(pasid) | QI_DEV_EIOTLB_SID(sid) |
- QI_DEV_EIOTLB_QDEP(qdep) | QI_DEIOTLB_TYPE |
- QI_DEV_IOTLB_PFSID(pfsid);
-
- /*
- * If S bit is 0, we only flush a single page. If S bit is set,
- * The least significant zero bit indicates the invalidation address
- * range. VT-d spec 6.5.2.6.
- * e.g. address bit 12[0] indicates 8KB, 13[0] indicates 16KB.
- * size order = 0 is PAGE_SIZE 4KB
- * Max Invs Pending (MIP) is set to 0 for now until we have DIT in
- * ECAP.
- */
- if (!IS_ALIGNED(addr, VTD_PAGE_SIZE << size_order))
- pr_warn_ratelimited("Invalidate non-aligned address %llx, order %d\n",
- addr, size_order);
-
- /* Take page address */
- desc.qw1 = QI_DEV_EIOTLB_ADDR(addr);
-
- if (size_order) {
- /*
- * Existing 0s in address below size_order may be the least
- * significant bit, we must set them to 1s to avoid having
- * smaller size than desired.
- */
- desc.qw1 |= GENMASK_ULL(size_order + VTD_PAGE_SHIFT - 1,
- VTD_PAGE_SHIFT);
- /* Clear size_order bit to indicate size */
- desc.qw1 &= ~mask;
- /* Set the S bit to indicate flushing more than 1 page */
- desc.qw1 |= QI_DEV_EIOTLB_SIZE;
- }
-
+ qi_desc_dev_iotlb_pasid(sid, pfsid, pasid,
+ qdep, addr, size_order,
+ &desc);
qi_submit_sync(iommu, &desc, 1, 0);
}
--
2.43.0
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH v2 2/5] iommu/vt-d: Refactor IOTLB and Dev-IOTLB flush logic
2024-08-09 2:54 [PATCH v2 0/5] Batch IOTLB/dev-IOTLB invalidation Tina Zhang
2024-08-09 2:54 ` [PATCH v2 1/5] iommu/vt-d: Refactor IOTLB/Dev-IOTLB invalidation command logic Tina Zhang
@ 2024-08-09 2:54 ` Tina Zhang
2024-08-09 8:09 ` Baolu Lu
2024-08-09 2:54 ` [PATCH v2 3/5] iommu/vt-d: Introduce interfaces for QI batching operations Tina Zhang
` (2 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Tina Zhang @ 2024-08-09 2:54 UTC (permalink / raw)
To: Lu Baolu, Kevin Tian; +Cc: iommu, linux-kernel, Tina Zhang
Introduce three new helper functions, handle_iotlb_flush(), handle_dev_
tlb_flush() and handle_dev_tlb_flush_all() to encapsulate the logic for
IOTLB and Dev-IOTLB invalidation commands. This refactoring aims to
improve code readability and maintainability by centralizing the handling
of these flush operations.
Signed-off-by: Tina Zhang <tina.zhang@intel.com>
---
drivers/iommu/intel/cache.c | 136 +++++++++++++++++++-----------------
1 file changed, 72 insertions(+), 64 deletions(-)
diff --git a/drivers/iommu/intel/cache.c b/drivers/iommu/intel/cache.c
index 44e92638c0cd..3ae84ccfcfa1 100644
--- a/drivers/iommu/intel/cache.c
+++ b/drivers/iommu/intel/cache.c
@@ -255,6 +255,72 @@ static unsigned long calculate_psi_aligned_address(unsigned long start,
return ALIGN_DOWN(start, VTD_PAGE_SIZE << mask);
}
+static inline void handle_iotlb_flush(struct dmar_domain *domain,
+ struct cache_tag *tag,
+ unsigned long addr,
+ unsigned long pages,
+ unsigned long mask,
+ int ih)
+{
+ struct intel_iommu *iommu = tag->iommu;
+
+ if (domain->use_first_level) {
+ qi_flush_piotlb(iommu, tag->domain_id,
+ tag->pasid, addr, pages, ih);
+ } else {
+ /*
+ * Fallback to domain selective flush if no
+ * PSI support or the size is too big.
+ */
+ if (!cap_pgsel_inv(iommu->cap) ||
+ mask > cap_max_amask_val(iommu->cap) ||
+ pages == -1)
+ iommu->flush.flush_iotlb(iommu, tag->domain_id,
+ 0, 0, DMA_TLB_DSI_FLUSH);
+ else
+ iommu->flush.flush_iotlb(iommu, tag->domain_id,
+ addr | ih, mask,
+ DMA_TLB_PSI_FLUSH);
+ }
+}
+
+static void handle_dev_tlb_flush(struct cache_tag *tag,
+ unsigned long addr,
+ unsigned long mask)
+{
+ struct intel_iommu *iommu = tag->iommu;
+ struct device_domain_info *info;
+ u16 sid;
+
+ info = dev_iommu_priv_get(tag->dev);
+ sid = PCI_DEVID(info->bus, info->devfn);
+
+ if (tag->pasid == IOMMU_NO_PASID)
+ qi_flush_dev_iotlb(iommu, sid, info->pfsid,
+ info->ats_qdep, addr, mask);
+ else
+ qi_flush_dev_iotlb_pasid(iommu, sid, info->pfsid,
+ tag->pasid, info->ats_qdep,
+ addr, mask);
+
+ quirk_extra_dev_tlb_flush(info, addr, mask, tag->pasid, info->ats_qdep);
+}
+
+static void handle_dev_tlb_flush_all(struct cache_tag *tag)
+{
+ struct intel_iommu *iommu = tag->iommu;
+ struct device_domain_info *info;
+ u16 sid;
+
+ info = dev_iommu_priv_get(tag->dev);
+ sid = PCI_DEVID(info->bus, info->devfn);
+
+ qi_flush_dev_iotlb(iommu, sid, info->pfsid, info->ats_qdep,
+ 0, MAX_AGAW_PFN_WIDTH);
+ quirk_extra_dev_tlb_flush(info, 0, MAX_AGAW_PFN_WIDTH,
+ IOMMU_NO_PASID, info->ats_qdep);
+}
+
/*
* Invalidates a range of IOVA from @start (inclusive) to @end (inclusive)
* when the memory mappings in the target domain have been modified.
@@ -270,30 +336,10 @@ void cache_tag_flush_range(struct dmar_domain *domain, unsigned long start,
spin_lock_irqsave(&domain->cache_lock, flags);
list_for_each_entry(tag, &domain->cache_tags, node) {
- struct intel_iommu *iommu = tag->iommu;
- struct device_domain_info *info;
- u16 sid;
-
switch (tag->type) {
case CACHE_TAG_IOTLB:
case CACHE_TAG_NESTING_IOTLB:
- if (domain->use_first_level) {
- qi_flush_piotlb(iommu, tag->domain_id,
- tag->pasid, addr, pages, ih);
- } else {
- /*
- * Fallback to domain selective flush if no
- * PSI support or the size is too big.
- */
- if (!cap_pgsel_inv(iommu->cap) ||
- mask > cap_max_amask_val(iommu->cap))
- iommu->flush.flush_iotlb(iommu, tag->domain_id,
- 0, 0, DMA_TLB_DSI_FLUSH);
- else
- iommu->flush.flush_iotlb(iommu, tag->domain_id,
- addr | ih, mask,
- DMA_TLB_PSI_FLUSH);
- }
+ handle_iotlb_flush(domain, tag, addr, pages, mask, ih);
break;
case CACHE_TAG_NESTING_DEVTLB:
/*
@@ -307,18 +353,7 @@ void cache_tag_flush_range(struct dmar_domain *domain, unsigned long start,
mask = MAX_AGAW_PFN_WIDTH;
fallthrough;
case CACHE_TAG_DEVTLB:
- info = dev_iommu_priv_get(tag->dev);
- sid = PCI_DEVID(info->bus, info->devfn);
-
- if (tag->pasid == IOMMU_NO_PASID)
- qi_flush_dev_iotlb(iommu, sid, info->pfsid,
- info->ats_qdep, addr, mask);
- else
- qi_flush_dev_iotlb_pasid(iommu, sid, info->pfsid,
- tag->pasid, info->ats_qdep,
- addr, mask);
-
- quirk_extra_dev_tlb_flush(info, addr, mask, tag->pasid, info->ats_qdep);
+ handle_dev_tlb_flush(tag, addr, mask);
break;
}
@@ -338,29 +373,14 @@ void cache_tag_flush_all(struct dmar_domain *domain)
spin_lock_irqsave(&domain->cache_lock, flags);
list_for_each_entry(tag, &domain->cache_tags, node) {
- struct intel_iommu *iommu = tag->iommu;
- struct device_domain_info *info;
- u16 sid;
-
switch (tag->type) {
case CACHE_TAG_IOTLB:
case CACHE_TAG_NESTING_IOTLB:
- if (domain->use_first_level)
- qi_flush_piotlb(iommu, tag->domain_id,
- tag->pasid, 0, -1, 0);
- else
- iommu->flush.flush_iotlb(iommu, tag->domain_id,
- 0, 0, DMA_TLB_DSI_FLUSH);
+ handle_iotlb_flush(domain, tag, 0, -1, 0, 0);
break;
case CACHE_TAG_DEVTLB:
case CACHE_TAG_NESTING_DEVTLB:
- info = dev_iommu_priv_get(tag->dev);
- sid = PCI_DEVID(info->bus, info->devfn);
-
- qi_flush_dev_iotlb(iommu, sid, info->pfsid, info->ats_qdep,
- 0, MAX_AGAW_PFN_WIDTH);
- quirk_extra_dev_tlb_flush(info, 0, MAX_AGAW_PFN_WIDTH,
- IOMMU_NO_PASID, info->ats_qdep);
+ handle_dev_tlb_flush_all(tag);
break;
}
@@ -399,20 +419,8 @@ void cache_tag_flush_range_np(struct dmar_domain *domain, unsigned long start,
}
if (tag->type == CACHE_TAG_IOTLB ||
- tag->type == CACHE_TAG_NESTING_IOTLB) {
- /*
- * Fallback to domain selective flush if no
- * PSI support or the size is too big.
- */
- if (!cap_pgsel_inv(iommu->cap) ||
- mask > cap_max_amask_val(iommu->cap))
- iommu->flush.flush_iotlb(iommu, tag->domain_id,
- 0, 0, DMA_TLB_DSI_FLUSH);
- else
- iommu->flush.flush_iotlb(iommu, tag->domain_id,
- addr, mask,
- DMA_TLB_PSI_FLUSH);
- }
+ tag->type == CACHE_TAG_NESTING_IOTLB)
+ handle_iotlb_flush(domain, tag, addr, pages, mask, 0);
trace_cache_tag_flush_range_np(tag, start, end, addr, pages, mask);
}
--
2.43.0
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 2/5] iommu/vt-d: Refactor IOTLB and Dev-IOTLB flush logic
2024-08-09 2:54 ` [PATCH v2 2/5] iommu/vt-d: Refactor IOTLB and Dev-IOTLB flush logic Tina Zhang
@ 2024-08-09 8:09 ` Baolu Lu
0 siblings, 0 replies; 13+ messages in thread
From: Baolu Lu @ 2024-08-09 8:09 UTC (permalink / raw)
To: Tina Zhang, Kevin Tian; +Cc: baolu.lu, iommu, linux-kernel
On 2024/8/9 10:54, Tina Zhang wrote:
> Introduce three new helper functions, handle_iotlb_flush(), handle_dev_
> tlb_flush() and handle_dev_tlb_flush_all() to encapsulate the logic for
> IOTLB and Dev-IOTLB invalidation commands. This refactoring aims to
> improve code readability and maintainability by centralizing the handling
> of these flush operations.
>
> Signed-off-by: Tina Zhang <tina.zhang@intel.com>
> ---
> drivers/iommu/intel/cache.c | 136 +++++++++++++++++++-----------------
> 1 file changed, 72 insertions(+), 64 deletions(-)
>
> diff --git a/drivers/iommu/intel/cache.c b/drivers/iommu/intel/cache.c
> index 44e92638c0cd..3ae84ccfcfa1 100644
> --- a/drivers/iommu/intel/cache.c
> +++ b/drivers/iommu/intel/cache.c
> @@ -255,6 +255,72 @@ static unsigned long calculate_psi_aligned_address(unsigned long start,
> return ALIGN_DOWN(start, VTD_PAGE_SIZE << mask);
> }
>
> +static inline void handle_iotlb_flush(struct dmar_domain *domain,
> + struct cache_tag *tag,
> + unsigned long addr,
> + unsigned long pages,
> + unsigned long mask,
> + int ih)
> +{
> + struct intel_iommu *iommu = tag->iommu;
> +
> + if (domain->use_first_level) {
> + qi_flush_piotlb(iommu, tag->domain_id,
> + tag->pasid, addr, pages, ih);
> + } else {
> + /*
> + * Fallback to domain selective flush if no
> + * PSI support or the size is too big.
> + */
> + if (!cap_pgsel_inv(iommu->cap) ||
> + mask > cap_max_amask_val(iommu->cap) ||
> + pages == -1)
> + iommu->flush.flush_iotlb(iommu, tag->domain_id,
> + 0, 0, DMA_TLB_DSI_FLUSH);
> + else
> + iommu->flush.flush_iotlb(iommu, tag->domain_id,
> + addr | ih, mask,
> + DMA_TLB_PSI_FLUSH);
> + }
> +}
No need to make it inline. Same to other places in this series. If you
really want any inline helper, please add it in the header.
Thanks,
baolu
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 3/5] iommu/vt-d: Introduce interfaces for QI batching operations
2024-08-09 2:54 [PATCH v2 0/5] Batch IOTLB/dev-IOTLB invalidation Tina Zhang
2024-08-09 2:54 ` [PATCH v2 1/5] iommu/vt-d: Refactor IOTLB/Dev-IOTLB invalidation command logic Tina Zhang
2024-08-09 2:54 ` [PATCH v2 2/5] iommu/vt-d: Refactor IOTLB and Dev-IOTLB flush logic Tina Zhang
@ 2024-08-09 2:54 ` Tina Zhang
2024-08-09 8:15 ` Baolu Lu
2024-08-09 2:54 ` [PATCH v2 4/5] vt-d/iommu: Refactor quirk_extra_dev_tlb_flush() Tina Zhang
2024-08-09 2:54 ` [PATCH v2 5/5] vt-d/iommu: Enable batching of IOTLB/Dev-IOTLB invalidations Tina Zhang
4 siblings, 1 reply; 13+ messages in thread
From: Tina Zhang @ 2024-08-09 2:54 UTC (permalink / raw)
To: Lu Baolu, Kevin Tian; +Cc: iommu, linux-kernel, Tina Zhang
Introduces qi_batch_xxx() interfaces to the VT-d driver to enhance the
efficiency of IOTLB and Dev-IOTLB invalidation command processing.
By allowing these commands to be batched together before submission,
the patch aims to minimize the overhead previously incurred when
handling these operations individually.
The addition of qi_batch_add_xxx() functions enable the accumulation of
invalidation commands into a batch, while the qi_batch_flush_descs()
function allows for the collective submission of these commands.
Signed-off-by: Tina Zhang <tina.zhang@intel.com>
---
drivers/iommu/intel/dmar.c | 78 +++++++++++++++++++++++++++++++++++++
drivers/iommu/intel/iommu.h | 39 +++++++++++++++++++
2 files changed, 117 insertions(+)
diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
index 64724af1a618..8d55c49382fc 100644
--- a/drivers/iommu/intel/dmar.c
+++ b/drivers/iommu/intel/dmar.c
@@ -1636,6 +1636,84 @@ void qi_flush_iotlb(struct intel_iommu *iommu, u16 did, u64 addr,
qi_submit_sync(iommu, &desc, 1, 0);
}
+static void qi_batch_increment_index(struct intel_iommu *iommu,
+ struct qi_batch *batch)
+{
+ if (++batch->index == QI_MAX_BATCHED_DESC_COUNT)
+ qi_batch_flush_descs(iommu, batch);
+}
+
+void qi_batch_flush_descs(struct intel_iommu *iommu, struct qi_batch *batch)
+{
+ if (!batch->index)
+ return;
+
+ qi_submit_sync(iommu, batch->descs, batch->index, 0);
+
+ /* Reset the index value and clean the whole batch buffer */
+ memset(batch, 0, sizeof(struct qi_batch));
+}
+
+void qi_batch_add_iotlb_desc(struct intel_iommu *iommu, u16 did, u64 addr,
+ unsigned int size_order, u64 type,
+ struct qi_batch *batch)
+{
+ qi_desc_iotlb(iommu, did, addr, size_order, type, &(batch->descs[batch->index]));
+ qi_batch_increment_index(iommu, batch);
+}
+
+void qi_batch_add_dev_iotlb_desc(struct intel_iommu *iommu, u16 sid,
+ u16 pfsid, u16 qdep, u64 addr,
+ unsigned int mask,
+ struct qi_batch *batch)
+{
+ /*
+ * According to VT-d spec, software is recommended to not submit any Device-TLB
+ * invalidation requests while address remapping hardware is disabled.
+ */
+ if (!(iommu->gcmd & DMA_GCMD_TE))
+ return;
+
+ qi_desc_dev_iotlb(sid, pfsid, qdep, addr, mask, &(batch->descs[batch->index]));
+ qi_batch_increment_index(iommu, batch);
+}
+
+void qi_batch_add_piotlb_desc(struct intel_iommu *iommu, u16 did,
+ u32 pasid, u64 addr,
+ unsigned long npages, bool ih,
+ struct qi_batch *batch)
+{
+ /*
+ * npages == -1 means a PASID-selective invalidation, otherwise,
+ * a positive value for Page-selective-within-PASID invalidation.
+ * 0 is not a valid input.
+ */
+ if (!npages)
+ return;
+
+ qi_desc_piotlb(did, pasid, addr, npages, ih, &(batch->descs[batch->index]));
+ qi_batch_increment_index(iommu, batch);
+}
+
+void qi_batch_add_dev_iotlb_pasid_desc(struct intel_iommu *iommu,
+ u16 sid, u16 pfsid,
+ u32 pasid, u16 qdep,
+ u64 addr, unsigned int size_order,
+ struct qi_batch *batch)
+{
+ /*
+ * According to VT-d spec, software is recommended to not submit any Device-TLB
+ * invalidation requests while address remapping hardware is disabled.
+ */
+ if (!(iommu->gcmd & DMA_GCMD_TE))
+ return;
+
+ qi_desc_dev_iotlb_pasid(sid, pfsid, pasid,
+ qdep, addr, size_order,
+ &(batch->descs[batch->index]));
+ qi_batch_increment_index(iommu, batch);
+}
+
void qi_flush_dev_iotlb(struct intel_iommu *iommu, u16 sid, u16 pfsid,
u16 qdep, u64 addr, unsigned mask)
{
diff --git a/drivers/iommu/intel/iommu.h b/drivers/iommu/intel/iommu.h
index b67c14da1240..cd7c1d0a01c6 100644
--- a/drivers/iommu/intel/iommu.h
+++ b/drivers/iommu/intel/iommu.h
@@ -584,6 +584,22 @@ struct iommu_domain_info {
* to VT-d spec, section 9.3 */
};
+/*
+ * The QI_MAX_BATCHED_DESC_COUNT value is determined by the two considerations:
+ * 1) Maximizing the batching of IOTLB and Dev-IOTLB invalidation commands, which is
+ * especially advantageous in virtualization environments where multiple devices may be
+ * associated with a single virtual IOMMU.
+ * 2) Minimizing unnecessary memory allocation for domains lacking ATS support.
+ *
+ * Future enhancements could include dynamically allocating the batch buffer based on actual
+ * demand, allowing for adjustments to the batch size to better accommodate various use cases.
+ */
+#define QI_MAX_BATCHED_DESC_COUNT 16
+struct qi_batch {
+ struct qi_desc descs[QI_MAX_BATCHED_DESC_COUNT];
+ unsigned int index;
+};
+
struct dmar_domain {
int nid; /* node id */
struct xarray iommu_array; /* Attached IOMMU array */
@@ -1098,6 +1114,29 @@ void qi_flush_pasid_cache(struct intel_iommu *iommu, u16 did, u64 granu,
int qi_submit_sync(struct intel_iommu *iommu, struct qi_desc *desc,
unsigned int count, unsigned long options);
+
+void qi_batch_flush_descs(struct intel_iommu *iommu,
+ struct qi_batch *batch);
+
+void qi_batch_add_iotlb_desc(struct intel_iommu *iommu, u16 did, u64 addr,
+ unsigned int size_order, u64 type,
+ struct qi_batch *batch);
+
+void qi_batch_add_dev_iotlb_desc(struct intel_iommu *iommu, u16 sid,
+ u16 pfsid, u16 qdep, u64 addr,
+ unsigned int mask,
+ struct qi_batch *batch);
+
+void qi_batch_add_piotlb_desc(struct intel_iommu *iommu, u16 did,
+ u32 pasid, u64 addr,
+ unsigned long npages, bool ih,
+ struct qi_batch *batch);
+
+void qi_batch_add_dev_iotlb_pasid_desc(struct intel_iommu *iommu,
+ u16 sid, u16 pfsid,
+ u32 pasid, u16 qdep,
+ u64 addr, unsigned int size_order,
+ struct qi_batch *batch);
/*
* Options used in qi_submit_sync:
* QI_OPT_WAIT_DRAIN - Wait for PRQ drain completion, spec 6.5.2.8.
--
2.43.0
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 3/5] iommu/vt-d: Introduce interfaces for QI batching operations
2024-08-09 2:54 ` [PATCH v2 3/5] iommu/vt-d: Introduce interfaces for QI batching operations Tina Zhang
@ 2024-08-09 8:15 ` Baolu Lu
2024-08-09 8:59 ` Zhang, Tina
0 siblings, 1 reply; 13+ messages in thread
From: Baolu Lu @ 2024-08-09 8:15 UTC (permalink / raw)
To: Tina Zhang, Kevin Tian; +Cc: baolu.lu, iommu, linux-kernel
On 2024/8/9 10:54, Tina Zhang wrote:
> Introduces qi_batch_xxx() interfaces to the VT-d driver to enhance the
> efficiency of IOTLB and Dev-IOTLB invalidation command processing.
> By allowing these commands to be batched together before submission,
> the patch aims to minimize the overhead previously incurred when
> handling these operations individually.
>
> The addition of qi_batch_add_xxx() functions enable the accumulation of
> invalidation commands into a batch, while the qi_batch_flush_descs()
> function allows for the collective submission of these commands.
>
> Signed-off-by: Tina Zhang<tina.zhang@intel.com>
> ---
> drivers/iommu/intel/dmar.c | 78 +++++++++++++++++++++++++++++++++++++
> drivers/iommu/intel/iommu.h | 39 +++++++++++++++++++
> 2 files changed, 117 insertions(+)
>
> diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
> index 64724af1a618..8d55c49382fc 100644
> --- a/drivers/iommu/intel/dmar.c
> +++ b/drivers/iommu/intel/dmar.c
> @@ -1636,6 +1636,84 @@ void qi_flush_iotlb(struct intel_iommu *iommu, u16 did, u64 addr,
> qi_submit_sync(iommu, &desc, 1, 0);
> }
>
> +static void qi_batch_increment_index(struct intel_iommu *iommu,
> + struct qi_batch *batch)
> +{
> + if (++batch->index == QI_MAX_BATCHED_DESC_COUNT)
> + qi_batch_flush_descs(iommu, batch);
> +}
> +
> +void qi_batch_flush_descs(struct intel_iommu *iommu, struct qi_batch *batch)
> +{
> + if (!batch->index)
> + return;
> +
> + qi_submit_sync(iommu, batch->descs, batch->index, 0);
> +
> + /* Reset the index value and clean the whole batch buffer */
> + memset(batch, 0, sizeof(struct qi_batch));
> +}
> +
> +void qi_batch_add_iotlb_desc(struct intel_iommu *iommu, u16 did, u64 addr,
> + unsigned int size_order, u64 type,
> + struct qi_batch *batch)
> +{
> + qi_desc_iotlb(iommu, did, addr, size_order, type, &(batch->descs[batch->index]));
> + qi_batch_increment_index(iommu, batch);
> +}
> +
> +void qi_batch_add_dev_iotlb_desc(struct intel_iommu *iommu, u16 sid,
> + u16 pfsid, u16 qdep, u64 addr,
> + unsigned int mask,
> + struct qi_batch *batch)
> +{
> + /*
> + * According to VT-d spec, software is recommended to not submit any Device-TLB
> + * invalidation requests while address remapping hardware is disabled.
> + */
> + if (!(iommu->gcmd & DMA_GCMD_TE))
> + return;
> +
> + qi_desc_dev_iotlb(sid, pfsid, qdep, addr, mask, &(batch->descs[batch->index]));
> + qi_batch_increment_index(iommu, batch);
> +}
> +
> +void qi_batch_add_piotlb_desc(struct intel_iommu *iommu, u16 did,
> + u32 pasid, u64 addr,
> + unsigned long npages, bool ih,
> + struct qi_batch *batch)
> +{
> + /*
> + * npages == -1 means a PASID-selective invalidation, otherwise,
> + * a positive value for Page-selective-within-PASID invalidation.
> + * 0 is not a valid input.
> + */
> + if (!npages)
> + return;
> +
> + qi_desc_piotlb(did, pasid, addr, npages, ih, &(batch->descs[batch->index]));
> + qi_batch_increment_index(iommu, batch);
> +}
> +
> +void qi_batch_add_dev_iotlb_pasid_desc(struct intel_iommu *iommu,
> + u16 sid, u16 pfsid,
> + u32 pasid, u16 qdep,
> + u64 addr, unsigned int size_order,
> + struct qi_batch *batch)
> +{
> + /*
> + * According to VT-d spec, software is recommended to not submit any Device-TLB
> + * invalidation requests while address remapping hardware is disabled.
> + */
> + if (!(iommu->gcmd & DMA_GCMD_TE))
> + return;
> +
> + qi_desc_dev_iotlb_pasid(sid, pfsid, pasid,
> + qdep, addr, size_order,
> + &(batch->descs[batch->index]));
> + qi_batch_increment_index(iommu, batch);
> +}
How about moving all these helpers into drivers/iommu/intel/cache.c?
It's the only consumer for these helpers, right?
Thanks,
baolu
^ permalink raw reply [flat|nested] 13+ messages in thread* RE: [PATCH v2 3/5] iommu/vt-d: Introduce interfaces for QI batching operations
2024-08-09 8:15 ` Baolu Lu
@ 2024-08-09 8:59 ` Zhang, Tina
0 siblings, 0 replies; 13+ messages in thread
From: Zhang, Tina @ 2024-08-09 8:59 UTC (permalink / raw)
To: Baolu Lu, Tian, Kevin; +Cc: iommu, linux-kernel
Hi Baolu,
> -----Original Message-----
> From: Baolu Lu <baolu.lu@linux.intel.com>
> Sent: Friday, August 9, 2024 4:16 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 3/5] iommu/vt-d: Introduce interfaces for QI batching
> operations
>
> On 2024/8/9 10:54, Tina Zhang wrote:
> > Introduces qi_batch_xxx() interfaces to the VT-d driver to enhance the
> > efficiency of IOTLB and Dev-IOTLB invalidation command processing.
> > By allowing these commands to be batched together before submission,
> > the patch aims to minimize the overhead previously incurred when
> > handling these operations individually.
> >
> > The addition of qi_batch_add_xxx() functions enable the accumulation
> > of invalidation commands into a batch, while the
> > qi_batch_flush_descs() function allows for the collective submission of these
> commands.
> >
> > Signed-off-by: Tina Zhang<tina.zhang@intel.com>
> > ---
> > drivers/iommu/intel/dmar.c | 78
> +++++++++++++++++++++++++++++++++++++
> > drivers/iommu/intel/iommu.h | 39 +++++++++++++++++++
> > 2 files changed, 117 insertions(+)
> >
> > diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
> > index 64724af1a618..8d55c49382fc 100644
> > --- a/drivers/iommu/intel/dmar.c
> > +++ b/drivers/iommu/intel/dmar.c
> > @@ -1636,6 +1636,84 @@ void qi_flush_iotlb(struct intel_iommu *iommu,
> u16 did, u64 addr,
> > qi_submit_sync(iommu, &desc, 1, 0);
> > }
> >
> > +static void qi_batch_increment_index(struct intel_iommu *iommu,
> > + struct qi_batch *batch)
> > +{
> > + if (++batch->index == QI_MAX_BATCHED_DESC_COUNT)
> > + qi_batch_flush_descs(iommu, batch); }
> > +
> > +void qi_batch_flush_descs(struct intel_iommu *iommu, struct qi_batch
> > +*batch) {
> > + if (!batch->index)
> > + return;
> > +
> > + qi_submit_sync(iommu, batch->descs, batch->index, 0);
> > +
> > + /* Reset the index value and clean the whole batch buffer */
> > + memset(batch, 0, sizeof(struct qi_batch)); }
> > +
> > +void qi_batch_add_iotlb_desc(struct intel_iommu *iommu, u16 did, u64
> addr,
> > + unsigned int size_order, u64 type,
> > + struct qi_batch *batch)
> > +{
> > + qi_desc_iotlb(iommu, did, addr, size_order, type, &(batch-
> >descs[batch->index]));
> > + qi_batch_increment_index(iommu, batch); }
> > +
> > +void qi_batch_add_dev_iotlb_desc(struct intel_iommu *iommu, u16 sid,
> > + u16 pfsid, u16 qdep, u64 addr,
> > + unsigned int mask,
> > + struct qi_batch *batch)
> > +{
> > + /*
> > + * According to VT-d spec, software is recommended to not submit any
> Device-TLB
> > + * invalidation requests while address remapping hardware is disabled.
> > + */
> > + if (!(iommu->gcmd & DMA_GCMD_TE))
> > + return;
> > +
> > + qi_desc_dev_iotlb(sid, pfsid, qdep, addr, mask, &(batch->descs[batch-
> >index]));
> > + qi_batch_increment_index(iommu, batch); }
> > +
> > +void qi_batch_add_piotlb_desc(struct intel_iommu *iommu, u16 did,
> > + u32 pasid, u64 addr,
> > + unsigned long npages, bool ih,
> > + struct qi_batch *batch)
> > +{
> > + /*
> > + * npages == -1 means a PASID-selective invalidation, otherwise,
> > + * a positive value for Page-selective-within-PASID invalidation.
> > + * 0 is not a valid input.
> > + */
> > + if (!npages)
> > + return;
> > +
> > + qi_desc_piotlb(did, pasid, addr, npages, ih, &(batch->descs[batch-
> >index]));
> > + qi_batch_increment_index(iommu, batch); }
> > +
> > +void qi_batch_add_dev_iotlb_pasid_desc(struct intel_iommu *iommu,
> > + u16 sid, u16 pfsid,
> > + u32 pasid, u16 qdep,
> > + u64 addr, unsigned int size_order,
> > + struct qi_batch *batch)
> > +{
> > + /*
> > + * According to VT-d spec, software is recommended to not submit any
> Device-TLB
> > + * invalidation requests while address remapping hardware is disabled.
> > + */
> > + if (!(iommu->gcmd & DMA_GCMD_TE))
> > + return;
> > +
> > + qi_desc_dev_iotlb_pasid(sid, pfsid, pasid,
> > + qdep, addr, size_order,
> > + &(batch->descs[batch->index]));
> > + qi_batch_increment_index(iommu, batch); }
>
> How about moving all these helpers into drivers/iommu/intel/cache.c?
> It's the only consumer for these helpers, right?
Well, cache.c is one of the places where these helpers may get called. Another place is quirk_extra_dev_tlb_flush() in drivers/iommu/intel/iommu.c. And the quirk_extra_dev_tlb_flush() gets invoked in both cache.c and pasid.c.
Regards,
-Tina
>
> Thanks,
> baolu
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 4/5] vt-d/iommu: Refactor quirk_extra_dev_tlb_flush()
2024-08-09 2:54 [PATCH v2 0/5] Batch IOTLB/dev-IOTLB invalidation Tina Zhang
` (2 preceding siblings ...)
2024-08-09 2:54 ` [PATCH v2 3/5] iommu/vt-d: Introduce interfaces for QI batching operations Tina Zhang
@ 2024-08-09 2:54 ` Tina Zhang
2024-08-09 8:25 ` Baolu Lu
2024-08-09 2:54 ` [PATCH v2 5/5] vt-d/iommu: Enable batching of IOTLB/Dev-IOTLB invalidations Tina Zhang
4 siblings, 1 reply; 13+ messages in thread
From: Tina Zhang @ 2024-08-09 2:54 UTC (permalink / raw)
To: Lu Baolu, Kevin Tian; +Cc: iommu, linux-kernel, Tina Zhang
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
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 4/5] vt-d/iommu: Refactor quirk_extra_dev_tlb_flush()
2024-08-09 2:54 ` [PATCH v2 4/5] vt-d/iommu: Refactor quirk_extra_dev_tlb_flush() Tina Zhang
@ 2024-08-09 8:25 ` Baolu Lu
2024-08-09 9:10 ` Zhang, Tina
0 siblings, 1 reply; 13+ messages in thread
From: Baolu Lu @ 2024-08-09 8:25 UTC (permalink / raw)
To: Tina Zhang, Kevin Tian; +Cc: baolu.lu, iommu, linux-kernel
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
^ permalink raw reply [flat|nested] 13+ messages in thread* RE: [PATCH v2 4/5] vt-d/iommu: Refactor quirk_extra_dev_tlb_flush()
2024-08-09 8:25 ` Baolu Lu
@ 2024-08-09 9:10 ` Zhang, Tina
0 siblings, 0 replies; 13+ messages in thread
From: Zhang, Tina @ 2024-08-09 9:10 UTC (permalink / raw)
To: Baolu Lu, Tian, Kevin; +Cc: iommu, linux-kernel
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 5/5] vt-d/iommu: Enable batching of IOTLB/Dev-IOTLB invalidations
2024-08-09 2:54 [PATCH v2 0/5] Batch IOTLB/dev-IOTLB invalidation Tina Zhang
` (3 preceding siblings ...)
2024-08-09 2:54 ` [PATCH v2 4/5] vt-d/iommu: Refactor quirk_extra_dev_tlb_flush() Tina Zhang
@ 2024-08-09 2:54 ` Tina Zhang
2024-08-09 8:22 ` Baolu Lu
4 siblings, 1 reply; 13+ messages in thread
From: Tina Zhang @ 2024-08-09 2:54 UTC (permalink / raw)
To: Lu Baolu, Kevin Tian; +Cc: iommu, linux-kernel, Tina Zhang
Enable batch processing for IOTLB/Dev-IOTLB invalidation commands of SVA
domains and default domains with ATS enabled.
Signed-off-by: Tina Zhang <tina.zhang@intel.com>
---
drivers/iommu/intel/cache.c | 145 ++++++++++++++++++++++++++++++++++--
drivers/iommu/intel/iommu.c | 1 +
drivers/iommu/intel/iommu.h | 1 +
drivers/iommu/intel/svm.c | 5 +-
4 files changed, 145 insertions(+), 7 deletions(-)
diff --git a/drivers/iommu/intel/cache.c b/drivers/iommu/intel/cache.c
index 3ae84ccfcfa1..fabb98138760 100644
--- a/drivers/iommu/intel/cache.c
+++ b/drivers/iommu/intel/cache.c
@@ -59,10 +59,19 @@ static int cache_tag_assign(struct dmar_domain *domain, u16 did,
tag->pasid = pasid;
tag->users = 1;
- if (type == CACHE_TAG_DEVTLB || type == CACHE_TAG_NESTING_DEVTLB)
+ if (type == CACHE_TAG_DEVTLB || type == CACHE_TAG_NESTING_DEVTLB) {
tag->dev = dev;
- else
+
+ if (!domain->qi_batch && iommu->qi)
+ /*
+ * It doesn't matter if domain->qi_batch is NULL, as in
+ * this case the commands will be submitted individually.
+ */
+ domain->qi_batch = kzalloc(sizeof(struct qi_batch),
+ GFP_KERNEL);
+ } else {
tag->dev = iommu->iommu.dev;
+ }
spin_lock_irqsave(&domain->cache_lock, flags);
list_for_each_entry(temp, &domain->cache_tags, node) {
@@ -255,6 +264,84 @@ static unsigned long calculate_psi_aligned_address(unsigned long start,
return ALIGN_DOWN(start, VTD_PAGE_SIZE << mask);
}
+static inline void handle_batched_iotlb_descs(struct dmar_domain *domain,
+ struct cache_tag *tag,
+ unsigned long addr,
+ unsigned long pages,
+ unsigned long mask,
+ int ih)
+{
+ struct intel_iommu *iommu = tag->iommu;
+
+ if (domain->use_first_level) {
+ qi_batch_add_piotlb_desc(iommu, tag->domain_id,
+ tag->pasid, addr, pages,
+ ih, domain->qi_batch);
+ } else {
+ /*
+ * Fallback to domain selective flush if no
+ * PSI support or the size is too big.
+ */
+ if (!cap_pgsel_inv(iommu->cap) ||
+ mask > cap_max_amask_val(iommu->cap) ||
+ pages == -1)
+ qi_batch_add_iotlb_desc(iommu, tag->domain_id,
+ 0, 0, DMA_TLB_DSI_FLUSH,
+ domain->qi_batch);
+ else
+ qi_batch_add_iotlb_desc(iommu, tag->domain_id,
+ addr | ih, mask,
+ DMA_TLB_PSI_FLUSH,
+ domain->qi_batch);
+ }
+
+}
+
+static inline void handle_batched_dev_tlb_descs(struct dmar_domain *domain,
+ struct cache_tag *tag,
+ unsigned long addr,
+ unsigned long mask)
+{
+ struct intel_iommu *iommu = tag->iommu;
+ struct device_domain_info *info;
+ u16 sid;
+
+ info = dev_iommu_priv_get(tag->dev);
+ sid = PCI_DEVID(info->bus, info->devfn);
+
+ if (tag->pasid == IOMMU_NO_PASID)
+ qi_batch_add_dev_iotlb_desc(iommu, sid, info->pfsid,
+ info->ats_qdep, addr, mask,
+ domain->qi_batch);
+ else
+ qi_batch_add_dev_iotlb_pasid_desc(iommu, sid, info->pfsid,
+ tag->pasid, info->ats_qdep,
+ addr, mask, domain->qi_batch);
+
+ batch_quirk_extra_dev_tlb_flush(info, addr, mask,
+ tag->pasid,
+ info->ats_qdep,
+ domain->qi_batch);
+}
+
+static void handle_batched_dev_tlb_descs_all(struct dmar_domain *domain,
+ struct cache_tag *tag)
+{
+ struct intel_iommu *iommu = tag->iommu;
+ struct device_domain_info *info;
+ u16 sid;
+
+ info = dev_iommu_priv_get(tag->dev);
+ sid = PCI_DEVID(info->bus, info->devfn);
+
+ qi_batch_add_dev_iotlb_desc(iommu, sid, info->pfsid, info->ats_qdep,
+ 0, MAX_AGAW_PFN_WIDTH, domain->qi_batch);
+ batch_quirk_extra_dev_tlb_flush(info, 0, MAX_AGAW_PFN_WIDTH,
+ IOMMU_NO_PASID, info->ats_qdep,
+ domain->qi_batch);
+
+}
+
static inline void handle_iotlb_flush(struct dmar_domain *domain,
struct cache_tag *tag,
unsigned long addr,
@@ -264,6 +351,12 @@ static inline void handle_iotlb_flush(struct dmar_domain *domain,
{
struct intel_iommu *iommu = tag->iommu;
+ if (domain->qi_batch) {
+ handle_batched_iotlb_descs(domain, tag, addr,
+ pages, mask, ih);
+ return;
+ }
+
if (domain->use_first_level) {
qi_flush_piotlb(iommu, tag->domain_id,
tag->pasid, addr, pages, ih);
@@ -284,7 +377,8 @@ static inline void handle_iotlb_flush(struct dmar_domain *domain,
}
}
-static void handle_dev_tlb_flush(struct cache_tag *tag,
+static void handle_dev_tlb_flush(struct dmar_domain *domain,
+ struct cache_tag *tag,
unsigned long addr,
unsigned long mask)
{
@@ -292,6 +386,11 @@ static void handle_dev_tlb_flush(struct cache_tag *tag,
struct device_domain_info *info;
u16 sid;
+ if (domain->qi_batch) {
+ handle_batched_dev_tlb_descs(domain, tag, addr, mask);
+ return;
+ }
+
info = dev_iommu_priv_get(tag->dev);
sid = PCI_DEVID(info->bus, info->devfn);
@@ -306,12 +405,18 @@ static void handle_dev_tlb_flush(struct cache_tag *tag,
quirk_extra_dev_tlb_flush(info, addr, mask, tag->pasid, info->ats_qdep);
}
-static void handle_dev_tlb_flush_all(struct cache_tag *tag)
+static void handle_dev_tlb_flush_all(struct dmar_domain *domain,
+ struct cache_tag *tag)
{
struct intel_iommu *iommu = tag->iommu;
struct device_domain_info *info;
u16 sid;
+ if (domain->qi_batch) {
+ handle_batched_dev_tlb_descs_all(domain, tag);
+ return;
+ }
+
info = dev_iommu_priv_get(tag->dev);
sid = PCI_DEVID(info->bus, info->devfn);
@@ -329,6 +434,7 @@ void cache_tag_flush_range(struct dmar_domain *domain, unsigned long start,
unsigned long end, int ih)
{
unsigned long pages, mask, addr;
+ struct intel_iommu *iommu = NULL;
struct cache_tag *tag;
unsigned long flags;
@@ -336,6 +442,17 @@ void cache_tag_flush_range(struct dmar_domain *domain, unsigned long start,
spin_lock_irqsave(&domain->cache_lock, flags);
list_for_each_entry(tag, &domain->cache_tags, node) {
+ if (domain->qi_batch && iommu != tag->iommu) {
+ /*
+ * If domain supports batching commands, need to flush the
+ * batch buffer before switching to another iommu.
+ */
+ if (iommu)
+ qi_batch_flush_descs(iommu, domain->qi_batch);
+
+ iommu = tag->iommu;
+ }
+
switch (tag->type) {
case CACHE_TAG_IOTLB:
case CACHE_TAG_NESTING_IOTLB:
@@ -353,12 +470,14 @@ void cache_tag_flush_range(struct dmar_domain *domain, unsigned long start,
mask = MAX_AGAW_PFN_WIDTH;
fallthrough;
case CACHE_TAG_DEVTLB:
- handle_dev_tlb_flush(tag, addr, mask);
+ handle_dev_tlb_flush(domain, tag, addr, mask);
break;
}
trace_cache_tag_flush_range(tag, start, end, addr, pages, mask);
}
+ if (domain->qi_batch && domain->qi_batch->index)
+ qi_batch_flush_descs(iommu, domain->qi_batch);
spin_unlock_irqrestore(&domain->cache_lock, flags);
}
@@ -368,11 +487,23 @@ void cache_tag_flush_range(struct dmar_domain *domain, unsigned long start,
*/
void cache_tag_flush_all(struct dmar_domain *domain)
{
+ struct intel_iommu *iommu = NULL;
struct cache_tag *tag;
unsigned long flags;
spin_lock_irqsave(&domain->cache_lock, flags);
list_for_each_entry(tag, &domain->cache_tags, node) {
+ if (domain->qi_batch && iommu != tag->iommu) {
+ /*
+ * If domain supports batching commands, need to flush the
+ * batch buffer before switching to another iommu.
+ */
+ if (iommu)
+ qi_batch_flush_descs(iommu, domain->qi_batch);
+
+ iommu = tag->iommu;
+ }
+
switch (tag->type) {
case CACHE_TAG_IOTLB:
case CACHE_TAG_NESTING_IOTLB:
@@ -380,12 +511,14 @@ void cache_tag_flush_all(struct dmar_domain *domain)
break;
case CACHE_TAG_DEVTLB:
case CACHE_TAG_NESTING_DEVTLB:
- handle_dev_tlb_flush_all(tag);
+ handle_dev_tlb_flush_all(domain, tag);
break;
}
trace_cache_tag_flush_all(tag);
}
+ if (domain->qi_batch && domain->qi_batch->index)
+ qi_batch_flush_descs(iommu, domain->qi_batch);
spin_unlock_irqrestore(&domain->cache_lock, flags);
}
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 160d569015b4..5907470b9b35 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -1632,6 +1632,7 @@ static void domain_exit(struct dmar_domain *domain)
if (WARN_ON(!list_empty(&domain->devices)))
return;
+ kfree(domain->qi_batch);
kfree(domain);
}
diff --git a/drivers/iommu/intel/iommu.h b/drivers/iommu/intel/iommu.h
index 04aa1f200124..f16ffda48095 100644
--- a/drivers/iommu/intel/iommu.h
+++ b/drivers/iommu/intel/iommu.h
@@ -625,6 +625,7 @@ struct dmar_domain {
spinlock_t cache_lock; /* Protect the cache tag list */
struct list_head cache_tags; /* Cache tag list */
+ struct qi_batch *qi_batch; /* QI descriptors batch */
int iommu_superpage;/* Level of superpages supported:
0 == 4KiB (no superpages), 1 == 2MiB,
diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
index 0e3a9b38bef2..3421813995db 100644
--- a/drivers/iommu/intel/svm.c
+++ b/drivers/iommu/intel/svm.c
@@ -184,7 +184,10 @@ static void intel_mm_release(struct mmu_notifier *mn, struct mm_struct *mm)
static void intel_mm_free_notifier(struct mmu_notifier *mn)
{
- kfree(container_of(mn, struct dmar_domain, notifier));
+ struct dmar_domain *domain = container_of(mn, struct dmar_domain, notifier);
+
+ kfree(domain->qi_batch);
+ kfree(domain);
}
static const struct mmu_notifier_ops intel_mmuops = {
--
2.43.0
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 5/5] vt-d/iommu: Enable batching of IOTLB/Dev-IOTLB invalidations
2024-08-09 2:54 ` [PATCH v2 5/5] vt-d/iommu: Enable batching of IOTLB/Dev-IOTLB invalidations Tina Zhang
@ 2024-08-09 8:22 ` Baolu Lu
2024-08-09 9:24 ` Zhang, Tina
0 siblings, 1 reply; 13+ messages in thread
From: Baolu Lu @ 2024-08-09 8:22 UTC (permalink / raw)
To: Tina Zhang, Kevin Tian; +Cc: baolu.lu, iommu, linux-kernel
On 2024/8/9 10:54, Tina Zhang wrote:
> +static inline void handle_batched_iotlb_descs(struct dmar_domain *domain,
> + struct cache_tag *tag,
> + unsigned long addr,
> + unsigned long pages,
> + unsigned long mask,
> + int ih)
> +{
> + struct intel_iommu *iommu = tag->iommu;
> +
> + if (domain->use_first_level) {
> + qi_batch_add_piotlb_desc(iommu, tag->domain_id,
> + tag->pasid, addr, pages,
> + ih, domain->qi_batch);
> + } else {
> + /*
> + * Fallback to domain selective flush if no
> + * PSI support or the size is too big.
> + */
> + if (!cap_pgsel_inv(iommu->cap) ||
> + mask > cap_max_amask_val(iommu->cap) ||
> + pages == -1)
> + qi_batch_add_iotlb_desc(iommu, tag->domain_id,
> + 0, 0, DMA_TLB_DSI_FLUSH,
> + domain->qi_batch);
> + else
> + qi_batch_add_iotlb_desc(iommu, tag->domain_id,
> + addr | ih, mask,
> + DMA_TLB_PSI_FLUSH,
> + domain->qi_batch);
> + }
> +
> +}
What if the iommu driver is running on an early or emulated hardware
where the queued invalidation is not supported?
Thanks,
baolu
^ permalink raw reply [flat|nested] 13+ messages in thread* RE: [PATCH v2 5/5] vt-d/iommu: Enable batching of IOTLB/Dev-IOTLB invalidations
2024-08-09 8:22 ` Baolu Lu
@ 2024-08-09 9:24 ` Zhang, Tina
0 siblings, 0 replies; 13+ messages in thread
From: Zhang, Tina @ 2024-08-09 9:24 UTC (permalink / raw)
To: Baolu Lu, Tian, Kevin; +Cc: iommu, linux-kernel
Hi Baolu,
> -----Original Message-----
> From: Baolu Lu <baolu.lu@linux.intel.com>
> Sent: Friday, August 9, 2024 4:22 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 5/5] vt-d/iommu: Enable batching of IOTLB/Dev-IOTLB
> invalidations
>
> On 2024/8/9 10:54, Tina Zhang wrote:
> > +static inline void handle_batched_iotlb_descs(struct dmar_domain *domain,
> > + struct cache_tag *tag,
> > + unsigned long addr,
> > + unsigned long pages,
> > + unsigned long mask,
> > + int ih)
> > +{
> > + struct intel_iommu *iommu = tag->iommu;
> > +
> > + if (domain->use_first_level) {
> > + qi_batch_add_piotlb_desc(iommu, tag->domain_id,
> > + tag->pasid, addr, pages,
> > + ih, domain->qi_batch);
> > + } else {
> > + /*
> > + * Fallback to domain selective flush if no
> > + * PSI support or the size is too big.
> > + */
> > + if (!cap_pgsel_inv(iommu->cap) ||
> > + mask > cap_max_amask_val(iommu->cap) ||
> > + pages == -1)
> > + qi_batch_add_iotlb_desc(iommu, tag->domain_id,
> > + 0, 0, DMA_TLB_DSI_FLUSH,
> > + domain->qi_batch);
> > + else
> > + qi_batch_add_iotlb_desc(iommu, tag->domain_id,
> > + addr | ih, mask,
> > + DMA_TLB_PSI_FLUSH,
> > + domain->qi_batch);
> > + }
> > +
> > +}
>
> What if the iommu driver is running on an early or emulated hardware where
> the queued invalidation is not supported?
Yes, this is also taken into consideration.
In this patch, domain->qi_batch will be NULL if the IOMMU doesn't support qi based invalidations (i.e. iommu->qi is NULL), see:
- if (type == CACHE_TAG_DEVTLB || type == CACHE_TAG_NESTING_DEVTLB)
+ if (type == CACHE_TAG_DEVTLB || type == CACHE_TAG_NESTING_DEVTLB) {
tag->dev = dev;
- else
+
+ if (!domain->qi_batch && iommu->qi)
+ /*
+ * It doesn't matter if domain->qi_batch is NULL, as in
+ * this case the commands will be submitted individually.
+ */
+ domain->qi_batch = kzalloc(sizeof(struct qi_batch),
+ GFP_KERNEL);
+ } else {
tag->dev = iommu->iommu.dev;
+ }
Then, when invoking handle_batched_xxx() helpers, the logic, introduced in this patch, would check if domain->qi_batch is valid or not before proceeding batch processing.
Regards,
-Tina
>
> Thanks,
> baolu
^ permalink raw reply [flat|nested] 13+ messages in thread