[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <MWHPR11MB18866988310787FE573763878C529@MWHPR11MB1886.namprd11.prod.outlook.com>
Date: Wed, 12 May 2021 07:46:05 +0000
From: "Tian, Kevin" <kevin.tian@...el.com>
To: Jason Gunthorpe <jgg@...dia.com>,
Lu Baolu <baolu.lu@...ux.intel.com>
CC: Christoph Hellwig <hch@....de>, Joerg Roedel <joro@...tes.org>,
"David Woodhouse" <dwmw2@...radead.org>,
Alex Williamson <alex.williamson@...hat.com>,
Kirti Wankhede <kwankhede@...dia.com>,
"Raj, Ashok" <ashok.raj@...el.com>,
"Kumar, Sanjay K" <sanjay.k.kumar@...el.com>,
"Pan, Jacob jun" <jacob.jun.pan@...el.com>,
Jean-Philippe Brucker <jean-philippe.brucker@....com>,
"Liu, Yi L" <yi.l.liu@...el.com>, "Sun, Yi Y" <yi.y.sun@...el.com>,
"peterx@...hat.com" <peterx@...hat.com>,
"tiwei.bie@...el.com" <tiwei.bie@...el.com>,
"Zeng, Xin" <xin.zeng@...el.com>,
"iommu@...ts.linux-foundation.org" <iommu@...ts.linux-foundation.org>,
"kvm@...r.kernel.org" <kvm@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Jacob Pan <jacob.jun.pan@...ux.intel.com>
Subject: RE: [PATCH v8 7/9] vfio/mdev: Add iommu related member in mdev_device
> From: Jason Gunthorpe <jgg@...dia.com>
> Sent: Wednesday, May 12, 2021 1:37 AM
>
> On Tue, May 11, 2021 at 02:56:05PM +0800, Lu Baolu wrote:
>
> > > 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 the
> > 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?
>
> 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.
>
> 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 support
non-pci devices?
>
> /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.
>
I assume the so-called connection here implies using iommu_attach_device
to attach a device to an iommu domain. Did you suggest this connection
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
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 == 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 layer.
/dev/ioasid can accept pgtable cmd on this ioasid and associated domain.
Thanks
Kevin
Powered by blists - more mailing lists