linux-kernel - RE: [PATCH v2 4/5] vt-d/iommu: Refactor quirk_extra_dev_tlb

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: <MW5PR11MB588170EA147F7CEC6F94090189BA2@MW5PR11MB5881.namprd11.prod.outlook.com>
Date: Fri, 9 Aug 2024 09:10:00 +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 4/5] vt-d/iommu: Refactor quirk_extra_dev_tlb_flush()

Hi Baolu,

> -----Original Message-----
> From: Baolu Lu <baolu.lu@...ux.intel.com>
> Sent: Friday, August 9, 2024 4:26 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 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@...el.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