Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: Svgy6PaEQIHDuFq1Kg1WSXtWYG5XRLpr0iQ8KelueiRsk8m2N1Qd63Op13qsyXuupAjvAiTVlJ
 z9AWYCNyd3nA==
IronPort-SDR: EUSc8OFbHcR7H9eqzZG4seteHK/SB2/lTP+jbwfnNgg9d5Otvvq4Yyltk1za7TpruXaa/5lHyP
 TyUeTXA9X1Ng==
From:   "Tian, Kevin" <kevin.tian@intel.com>
To:     Jason Gunthorpe <jgg@nvidia.com>,
        Lu Baolu <baolu.lu@linux.intel.com>
CC:     Christoph Hellwig <hch@lst.de>, Joerg Roedel <joro@8bytes.org>,
        "David Woodhouse" <dwmw2@infradead.org>,
        Alex Williamson <alex.williamson@redhat.com>,
        Kirti Wankhede <kwankhede@nvidia.com>,
        "Raj, Ashok" <ashok.raj@intel.com>,
        "Kumar, Sanjay K" <sanjay.k.kumar@intel.com>,
        "Pan, Jacob jun" <jacob.jun.pan@intel.com>,
        Jean-Philippe Brucker <jean-philippe.brucker@arm.com>,
        "Liu, Yi L" <yi.l.liu@intel.com>, "Sun, Yi Y" <yi.y.sun@intel.com>,
        "peterx@redhat.com" <peterx@redhat.com>,
        "tiwei.bie@intel.com" <tiwei.bie@intel.com>,
        "Zeng, Xin" <xin.zeng@intel.com>,
        "iommu@lists.linux-foundation.org" <iommu@lists.linux-foundation.org>,
        "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Jacob Pan <jacob.jun.pan@linux.intel.com>
Subject: RE: [PATCH v8 7/9] vfio/mdev: Add iommu related member in mdev_device
Thread-Topic: [PATCH v8 7/9] vfio/mdev: Add iommu related member in
 mdev_device
Thread-Index: AQHXKx+RnarxvSHJQ0qqACGXvCESaareDy2AgACzFYCAANYSMA==
Date:   Wed, 12 May 2021 07:46:05 +0000
Message-ID: <MWHPR11MB18866988310787FE573763878C529@MWHPR11MB1886.namprd11.prod.outlook.com>
References: <20190325013036.18400-1-baolu.lu@linux.intel.com>
 <20190325013036.18400-8-baolu.lu@linux.intel.com>
 <20210406200030.GA425310@nvidia.com>
 <2d6d3c70-0c6f-2430-3982-2705bfe9f5a6@linux.intel.com>
 <20210511173703.GO1002214@nvidia.com>
In-Reply-To: <20210511173703.GO1002214@nvidia.com>
Accept-Language: en-US
Content-Language: en-US
dlp-version: 11.5.1.3
dlp-product: dlpe-windows
dlp-reaction: no-action
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: bd6f46dd-067a-4e8c-eb20-08d91519ffb8
X-MS-Exchange-CrossTenant-originalarrivaltime: 12 May 2021 07:46:05.3652
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: OnbAgVzO5TlpBcoGtIf6rQ7mwbEzC/UTR+cQJpetyuC9WuVUi/4z0GmE9Put+mrBt3fHtevkXhSACxzdqa8Q9Q==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR11MB1823
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Wednesday, May 12, 2021 1:37 AM
>=20
> On Tue, May 11, 2021 at 02:56:05PM +0800, Lu Baolu wrote:
>=20
> > >     After my next series the mdev drivers will have direct access to
> > >     the vfio_device. So an alternative to using the struct device, or
> > >     adding 'if mdev' is to add an API to the vfio_device world to
> > >     inject what iommu configuration is needed from that direction
> > >     instead of trying to discover it from a struct device.
> >
> > Just want to make sure that I understand you correctly.
> >
> > We should use the existing IOMMU in-kernel APIs to connect mdev with th=
e
> > iommu subsystem, so that the upper lays don't need to use something
> > like (if dev_is_mdev) to handle mdev differently. Do I get you
> > correctly?
>=20
> After going through all the /dev/ioasid stuff I'm pretty convinced
> that none of the PASID use cases for mdev should need any iommu
> connection from the mdev_device - this is an artifact of trying to
> cram the vfio container and group model into the mdev world and is not
> good design.
>=20
> The PASID interfaces for /dev/ioasid should use the 'struct
> pci_device' for everything and never pass in a mdev_device to the
> iommu layer.

'struct pci_device' -> 'struct device' since /dev/ioasid also needs to supp=
ort
non-pci devices?

>=20
> /dev/ioasid should be designed to support this operation and is why I
> strongly want to see the actual vfio_device implementation handle the
> connection to the iommu layer and not keep trying to hack through
> building what is actually a vfio_device specific connection through
> the type1 container code.
>=20

I assume the so-called connection here implies using iommu_attach_device=20
to attach a device to an iommu domain. Did you suggest this connection=20
must be done by the mdev driver which implements vfio_device and then
passing iommu domain to /dev/ioasid when attaching the device to an
IOASID? sort of like:
	ioctl(device_fd, VFIO_ATTACH_IOASID, ioasid, domain);

If yes, this conflicts with one design in /dev/ioasid proposal that we're
working on. In earlier discussion we agreed that each ioasid is associated
to a singleton iommu domain and all devices that are attached to this=20
ioasid with compatible iommu capabilities just share this domain. It
implies that iommu domain is allocated at ATTACH_IOASID phase (e.g.
when the 1st device is attached to an ioasid). Pre-allocating domain by
vfio_device driver means that every device (SR-IOV or mdev) has its own
domain thus cannot share ioasid then.

Did I misunderstand your intention?

Baolu and I discussed below draft proposal to avoid passing mdev_device
to the iommu layer. Please check whether it makes sense:

// for every device attached to an ioasid
// mdev is represented by pasid (allocated by mdev driver)
// pf/vf has INVALID_IOASID in pasid
struct dev_info {
	struct list_head		next;
	struct device		*device;
	u32			pasid;
}

// for every allocated ioasid
struct ioasid_info {
	// the handle to convey iommu operations
	struct iommu_domain	*domain;
	// metadata for map/unmap
	struct rb_node		dma_list;
	// the list of attached device
	struct dev_info		*dev_list;
	...
}

// called by VFIO/VDPA
int ioasid_attach_device(struct *device, u32 ioasid, u32 pasid)
{
	// allocate a new dev_info, filled with device/pasid
	// allocate iommu domain if it's the 1st attached device
	// check iommu compatibility if an domain already exists

	// attach the device to the iommu domain
	if (pasid =3D=3D INVALID_IOASID)
		iommu_attach_device(domain, device);
	else
		iommu_aux_attach_device(domain, device, pasid);

	// add dev_info to the dev_list of ioasid_info
}

// when attaching PF/VF to an ioasid
ioctl(device_fd, VFIO_ATTACH_IOASID, ioasid);
-> get vfio_device of device_fd
-> ioasid_attach_device(vfio_device->dev, ioasid, INVALID_IOASID);

// when attaching a mdev to an ioasid
ioctl(device_fd, VFIO_ATTACH_IOASID, ioasid);
-> get vfio_device of device_fd
-> find mdev_parent of vfio_device
-> find pasid allocated to this mdev
-> ioasid_attach_device(parent->dev, ioasid, pasid);

starting from this point the vfio device has been connected to the iommu la=
yer.
/dev/ioasid can accept pgtable cmd on this ioasid and associated domain.

Thanks
Kevin