RE: [PATCH 1/2] iommu/vt-d: Support batching IOTLB/dev-IOTLB invalidation commands

[Zhang, Tina]

Hi Baolu,

> -----Original Message-----
> From: Baolu Lu <baolu.lu@xxxxxxxxxxxxxxx>
> Sent: Sunday, May 19, 2024 5:43 PM
> To: Zhang, Tina <tina.zhang@xxxxxxxxx>; Tian, Kevin <kevin.tian@xxxxxxxxx>
> Cc: baolu.lu@xxxxxxxxxxxxxxx; iommu@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH 1/2] iommu/vt-d: Support batching IOTLB/dev-IOTLB
> invalidation commands
> 
> On 5/17/24 8:37 AM, Tina Zhang wrote:
> > Introduce a new parameter batch_desc to the QI based IOTLB/dev-IOTLB
> > invalidation operations to support batching invalidation descriptors.
> > This batch_desc is a pointer to the descriptor entry in a batch cmds
> > buffer. If the batch_desc is NULL, it indicates that batch submission
> > is not being used, and descriptors will be submitted individually.
> >
> > Also fix an issue reported by checkpatch about "unsigned mask":
> >          "Prefer 'unsigned int' to bare use of 'unsigned'"
> >
> > Signed-off-by: Tina Zhang<tina.zhang@xxxxxxxxx>
> > ---
> >   drivers/iommu/intel/cache.c | 33 +++++++++++-------
> >   drivers/iommu/intel/dmar.c  | 67 ++++++++++++++++++++-----------------
> >   drivers/iommu/intel/iommu.c | 27 +++++++++------
> >   drivers/iommu/intel/iommu.h | 21 ++++++++----
> >   drivers/iommu/intel/pasid.c | 20 ++++++-----
> >   5 files changed, 100 insertions(+), 68 deletions(-)
> >
> > diff --git a/drivers/iommu/intel/cache.c b/drivers/iommu/intel/cache.c
> > index e8418cdd8331..dcf5e0e6af17 100644
> > --- a/drivers/iommu/intel/cache.c
> > +++ b/drivers/iommu/intel/cache.c
> > @@ -278,7 +278,7 @@ void cache_tag_flush_range(struct dmar_domain
> *domain, unsigned long start,
> >   		case CACHE_TAG_NESTING_IOTLB:
> >   			if (domain->use_first_level) {
> >   				qi_flush_piotlb(iommu, tag->domain_id,
> > -						tag->pasid, addr, pages, ih);
> > +						tag->pasid, addr, pages, ih,
> NULL);
> >   			} else {
> 
> I'd like to have all batched descriptors code inside this file to make it easier for
> maintenance. Perhaps we can add the below infrastructure in the
> dmar_domain structure together with the cache tag.

Does it suggest we need to add a batch version of qi_flush_iotlb/qi_flush_dev_iotlb/qi_flush_piotlb/qi_flush_dev_iotlb_pasid() in the cache.c file? It doesn't sound like an easy to maintain those functions, does it?

In this patch, we reuse the current qi_flush_xxx() for both batching and non-batching processing, so that we don't need to duplicate the logic of qi_flush_xxx() in two places with one for batching processing and the other one for non-batching processing. What do you think?

Regards,
-Tina