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

[Jacob Pan <jacob.pan@linux.microsoft.com>]

Hi Yi,

On Mon, 19 Aug 2024 15:58:35 +0800
Yi Liu <yi.l.liu@intel.com> wrote:

> On 2024/8/17 11:28, Baolu Lu wrote:
> > On 2024/8/17 0:38, Jacob Pan wrote:  
> >> On Thu, 15 Aug 2024 14:52:21 +0800
> >> Tina Zhang <tina.zhang@intel.com> 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.  
> 
> Hi Jacob,
> 
> Do you mean the configuration that iommu.strict==0? :) As the above
> explanation from Baolu, this patch is not for that although it uses
> the term "batched". Also, it would reduce the VMExits that due to the
> IOTLB/DevTLB invalidation a lot in the virtualization environment.
> 
No, I understand this is a different "batch", not for deferred flush.
I am asking why it has to gather QI_MAX_BATCHED_DESC_COUNT before it
does the flush. See my other reply.

Thanks,

Jacob