linux-kernel - RE: [PATCH v2 5/8] iommu/vt-d: Make device pasid attachment explicit

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: <BN9PR11MB5276BCF726D0B813046479A18C889@BN9PR11MB5276.namprd11.prod.outlook.com>
Date:   Tue, 28 Mar 2023 07:44:52 +0000
From:   "Tian, Kevin" <kevin.tian@...el.com>
To:     Baolu Lu <baolu.lu@...ux.intel.com>,
        Jacob Pan <jacob.jun.pan@...ux.intel.com>,
        LKML <linux-kernel@...r.kernel.org>,
        "iommu@...ts.linux.dev" <iommu@...ts.linux.dev>,
        Jason Gunthorpe <jgg@...dia.com>,
        Joerg Roedel <joro@...tes.org>,
        "dmaengine@...r.kernel.org" <dmaengine@...r.kernel.org>,
        "vkoul@...nel.org" <vkoul@...nel.org>
CC:     Robin Murphy <robin.murphy@....com>, Will Deacon <will@...nel.org>,
        "David Woodhouse" <dwmw2@...radead.org>,
        "Raj, Ashok" <ashok.raj@...el.com>,
        "Liu, Yi L" <yi.l.liu@...el.com>,
        "Yu, Fenghua" <fenghua.yu@...el.com>,
        "Jiang, Dave" <dave.jiang@...el.com>,
        "Luck, Tony" <tony.luck@...el.com>,
        "Zanussi, Tom" <tom.zanussi@...el.com>
Subject: RE: [PATCH v2 5/8] iommu/vt-d: Make device pasid attachment explicit

> From: Baolu Lu <baolu.lu@...ux.intel.com>
> Sent: Tuesday, March 28, 2023 1:49 PM
> 
> On 3/28/23 7:21 AM, Jacob Pan wrote:
> > diff --git a/drivers/iommu/intel/iommu.h b/drivers/iommu/intel/iommu.h
> > index 65b15be72878..b6c26f25d1ba 100644
> > --- a/drivers/iommu/intel/iommu.h
> > +++ b/drivers/iommu/intel/iommu.h
> > @@ -595,6 +595,7 @@ struct dmar_domain {
> >
> >   	spinlock_t lock;		/* Protect device tracking lists */
> >   	struct list_head devices;	/* all devices' list */
> > +	struct list_head dev_pasids;	/* all attached pasids */
> >
> >   	struct dma_pte	*pgd;		/* virtual address */
> >   	int		gaw;		/* max guest address width */
> > @@ -708,6 +709,7 @@ struct device_domain_info {
> >   	u8 ats_supported:1;
> >   	u8 ats_enabled:1;
> >   	u8 dtlb_extra_inval:1;	/* Quirk for devices need extra flush */
> > +	u8 dev_attached:1;	/* Device context activated */
> >   	u8 ats_qdep;
> >   	struct device *dev; /* it's NULL for PCIe-to-PCI bridge */
> >   	struct intel_iommu *iommu; /* IOMMU used by this device */
> > @@ -715,6 +717,12 @@ struct device_domain_info {
> >   	struct pasid_table *pasid_table; /* pasid table */
> >   };
> >
> > +struct device_pasid_info {
> > +	struct list_head link_domain;	/* link to domain siblings */
> > +	struct device *dev;		/* physical device derived from */
> > +	ioasid_t pasid;			/* PASID on physical device */
> > +};
> 
> The dev_pasids list seems to be duplicate with iommu_group::pasid_array.
> 
> The pasid_array is de facto per-device as the PCI subsystem requires ACS
> to be enabled on the upstream path to the root port.
> 
> pci_enable_pasid():
> 385         if (!pci_acs_path_enabled(pdev, NULL, PCI_ACS_RR | PCI_ACS_UF))
> 386                 return -EINVAL;
> 
> For such PCI topology, pci_device_group() always assigns an exclusive
> iommu group (a.k.a. singleton group).
> 
> So, how about moving the pasid_array from struct iommu_group to struct
> dev_iommu? With this refactoring, the individual iommu driver has no
> need to create their own pasid array or list.
> 
> Instead of using iommu_group::mutex, perhaps the pasid_array needs its
> own lock in struct dev_iommu after moving.
> 

What you suggested is a right thing and more friendly to pasid attach
in iommufd [1].

but dev_pasids list here is a different thing. It tracks which [device, pasid]
is attached to the domain. w/o this information you'll have to walk the
pasid_array of every attached device under the domain and search for
every pasid entry pointing to the said domain. It's very inefficient. 

of course if this can be done more generally it'd be nice.😊

[1] https://lore.kernel.org/linux-iommu/ZAjbDxSzxYPqSCjo@nvidia.com/