linux-kernel - RE: [PATCH v2 3/5] iommu/vt-d: Introduce interfaces for QI batching operations

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <MW5PR11MB5881A8659273103114CF46E089BA2@MW5PR11MB5881.namprd11.prod.outlook.com>
Date: Fri, 9 Aug 2024 08:59:39 +0000
From: "Zhang, Tina" <tina.zhang@...el.com>
To: Baolu Lu <baolu.lu@...ux.intel.com>, "Tian, Kevin" <kevin.tian@...el.com>
CC: "iommu@...ts.linux.dev" <iommu@...ts.linux.dev>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v2 3/5] iommu/vt-d: Introduce interfaces for QI batching
 operations

Hi Baolu,

> -----Original Message-----
> From: Baolu Lu <baolu.lu@...ux.intel.com>
> Sent: Friday, August 9, 2024 4:16 PM
> To: Zhang, Tina <tina.zhang@...el.com>; Tian, Kevin <kevin.tian@...el.com>
> Cc: baolu.lu@...ux.intel.com; iommu@...ts.linux.dev; linux-
> kernel@...r.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@...el.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