linux-kernel - Re: [PATCH v2 10/19] iommufd/viommu: Add vdev

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: <ZwQZbYiCdf6WVGG5@Asurada-Nvidia>
Date: Mon, 7 Oct 2024 10:25:01 -0700
From: Nicolin Chen <nicolinc@...dia.com>
To: Jason Gunthorpe <jgg@...dia.com>
CC: <kevin.tian@...el.com>, <will@...nel.org>, <joro@...tes.org>,
	<suravee.suthikulpanit@....com>, <robin.murphy@....com>,
	<dwmw2@...radead.org>, <baolu.lu@...ux.intel.com>, <shuah@...nel.org>,
	<linux-kernel@...r.kernel.org>, <iommu@...ts.linux.dev>,
	<linux-arm-kernel@...ts.infradead.org>, <linux-kselftest@...r.kernel.org>,
	<eric.auger@...hat.com>, <jean-philippe@...aro.org>, <mdf@...nel.org>,
	<mshavit@...gle.com>, <shameerali.kolothum.thodi@...wei.com>,
	<smostafa@...gle.com>, <yi.l.liu@...el.com>
Subject: Re: [PATCH v2 10/19] iommufd/viommu: Add vdev_id helpers for IOMMU
 drivers

On Mon, Oct 07, 2024 at 02:11:19PM -0300, Jason Gunthorpe wrote:
> On Mon, Oct 07, 2024 at 09:36:18AM -0700, Nicolin Chen wrote:
> > On Mon, Oct 07, 2024 at 12:38:37PM -0300, Jason Gunthorpe wrote:
> > > On Fri, Oct 04, 2024 at 10:19:43PM -0700, Nicolin Chen wrote:
> > > > I tried exposing the struct iommufd_viommu to drivers, and was
> > > > able to drop a couple of helpers, except these two:
> > > > 
> > > > struct device *vdev_to_dev(struct iommufd_vdevice *vdev)
> > > > {
> > > > 	return vdev ? vdev->idev->dev : NULL;
> > > > } // Without it, we need to expose struct iommufd_device.
> > > > 
> > > > struct iommu_domain *
> > > > iommufd_viommu_to_parent_domain(struct iommufd_viommu *viommu)
> > > > {
> > > > 	if (!viommu || !viommu->hwpt)
> > > > 		return NULL;
> > > > 	return viommu->hwpt->common.domain;
> > > > } // Without it, we need to expose struct iommufd_hwpt_page.
> > > 
> > > It seems OK, there isn't really locking entanglements or performance
> > > path on this stuff?
> > 
> > -----
> > The typical use case of the first one is like:
> > 	dev = vdev_to_dev(xa_load(&viommu->vdevs, (unsigned long)vdev_id));
> > so I am asking for:
> > /* Caller should lock via viommu->vdevs_rwsem with proper permission */
> 
> Why would vdev_to_dev need that locking? The viommu cannot change hwpt
> during its lifecycle?

This is for vdev/dev v.s. hwpt. We need the lock for viommu's
vdev xarray.

Yet, giving a 2nd thought, I feel the lock would be useless if
a driver tries to refer the returned vdev (with this helper)
after the vdev object is destroyed..

We could only note something similar that caller must be aware
of the life cycle of vdev itself..

Nicolin