Re: [PATCH v3 4/4] iommu/vt-d: Introduce batched cache invalidation

From: Baolu Lu
Date: Mon Aug 19 2024 - 22:09:55 EST


On 8/19/24 11:40 PM, Jacob Pan wrote:
On Sat, 17 Aug 2024 11:28:21 +0800
Baolu Lu<baolu.lu@xxxxxxxxxxxxxxx> wrote:

On 2024/8/17 0:38, Jacob Pan wrote:
On Thu, 15 Aug 2024 14:52:21 +0800
Tina Zhang<tina.zhang@xxxxxxxxx> wrote:
@@ -270,7 +343,8 @@ static void cache_tag_flush_iotlb(struct
dmar_domain *domain, struct cache_tag * u64 type =
DMA_TLB_PSI_FLUSH;
if (domain->use_first_level) {
- qi_flush_piotlb(iommu, tag->domain_id, tag->pasid,
addr, pages, ih);
+ qi_batch_add_piotlb(iommu, tag->domain_id,
tag->pasid, addr,
+ pages, ih, domain->qi_batch);
return;
}
@@ -287,7 +361,8 @@ static void cache_tag_flush_iotlb(struct
dmar_domain *domain, struct cache_tag * }
if (ecap_qis(iommu->ecap))
- qi_flush_iotlb(iommu, tag->domain_id, addr | ih,
mask, type);
+ qi_batch_add_iotlb(iommu, tag->domain_id, addr |
ih, mask, type,
+ domain->qi_batch);
If I understand this correctly, IOTLB flush maybe deferred until the
batch array is full, right? If so, is there a security gap where
callers think the mapping is gone after the call returns?
No. All related caches are flushed before function return. A domain
can have multiple cache tags. Previously, we sent individual cache
invalidation requests to hardware. This change combines all necessary
invalidation requests into a single batch and raise them to hardware
together to make it more efficient.
I was looking at the code below, if the index does not reach
QI_MAX_BATCHED_DESC_COUNT. There will be no flush after
cache_tag_flush_iotlb() returns, right?

No. qi_batch_flush_descs() is called explicitly before return.

@@ -341,6 +417,7 @@ static void cache_tag_flush_devtlb_all(struct dmar_domain *domain, struct cache_
void cache_tag_flush_range(struct dmar_domain *domain, unsigned long start,
unsigned long end, int ih)
{
+ struct intel_iommu *iommu = NULL;
unsigned long pages, mask, addr;
struct cache_tag *tag;
unsigned long flags;
@@ -349,6 +426,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) {
+ if (iommu && iommu != tag->iommu)
+ qi_batch_flush_descs(iommu, domain->qi_batch);
+ iommu = tag->iommu;
+
switch (tag->type) {
case CACHE_TAG_IOTLB:
case CACHE_TAG_NESTING_IOTLB:
@@ -372,6 +453,7 @@ void cache_tag_flush_range(struct dmar_domain *domain, unsigned long start,

trace_cache_tag_flush_range(tag, start, end, addr, pages, mask);
}
+ qi_batch_flush_descs(iommu, domain->qi_batch);
spin_unlock_irqrestore(&domain->cache_lock, flags);
}

Thanks,
baolu