> From: kvm-owner@xxxxxxxxxxxxxxx [mailto:kvm-owner@xxxxxxxxxxxxxxx] On Behalf > Of David Gibson > Sent: Tuesday, July 23, 2019 11:58 AM > To: Liu, Yi L <yi.l.liu@xxxxxxxxx> > Subject: Re: [RFC v1 05/18] vfio/pci: add pasid alloc/free implementation > > On Mon, Jul 22, 2019 at 07:02:51AM +0000, Liu, Yi L wrote: > > > From: kvm-owner@xxxxxxxxxxxxxxx [mailto:kvm-owner@xxxxxxxxxxxxxxx] > > > On Behalf Of David Gibson > > > Sent: Wednesday, July 17, 2019 11:07 AM > > > To: Liu, Yi L <yi.l.liu@xxxxxxxxx> > > > Subject: Re: [RFC v1 05/18] vfio/pci: add pasid alloc/free > > > implementation > > > > > > On Tue, Jul 16, 2019 at 10:25:55AM +0000, Liu, Yi L wrote: > > > > > From: kvm-owner@xxxxxxxxxxxxxxx > > > > > [mailto:kvm-owner@xxxxxxxxxxxxxxx] On > > > Behalf > > > > > Of David Gibson > > > > > Sent: Monday, July 15, 2019 10:55 AM > > > > > To: Liu, Yi L <yi.l.liu@xxxxxxxxx> > > > > > Subject: Re: [RFC v1 05/18] vfio/pci: add pasid alloc/free > > > > > implementation > > > > > > > > > > On Fri, Jul 05, 2019 at 07:01:38PM +0800, Liu Yi L wrote: > > > > > > This patch adds vfio implementation PCIPASIDOps.alloc_pasid/free_pasid(). > > > > > > These two functions are used to propagate guest pasid > > > > > > allocation and free requests to host via vfio container ioctl. > > > > > > > > > > As I said in an earlier comment, I think doing this on the > > > > > device is conceptually incorrect. I think we need an explcit > > > > > notion of an SVM context (i.e. the namespace in which all the > > > > > PASIDs live) - which will IIUC usually be shared amongst > > > > > multiple devices. The create and free PASID requests should be on that object. > > > > > > > > Actually, the allocation is not doing on this device. System wide, > > > > it is done on a container. So not sure if it is the API interface > > > > gives you a sense that this is done on device. > > > > > > Sorry, I should have been clearer. I can see that at the VFIO level > > > it is done on the container. However the function here takes a bus > > > and devfn, so this qemu internal interface is per-device, which > > > doesn't really make sense. > > > > Got it. The reason here is to pass the bus and devfn info, so that > > VFIO can figure out a container for the operation. So far in QEMU, > > there is no good way to connect the vIOMMU emulator and VFIO regards > > to SVM. > > Right, and I think that's an indication that we're not modelling something in qemu > that we should be. > > > hw/pci layer is a choice based on some previous discussion. But yes, I > > agree with you that we may need to have an explicit notion for SVM. Do > > you think it is good to introduce a new abstract layer for SVM (may > > name as SVMContext). > > I think so, yes. > > If nothing else, I expect we'll need this concept if we ever want to be able to > implement SVM for emulated devices (which could be useful for debugging, even if > it's not something you'd do in production). > > > The idea would be that vIOMMU maintain the SVMContext instances and > > expose explicit interface for VFIO to get it. Then VFIO register > > notifiers on to the SVMContext. When vIOMMU emulator wants to do PASID > > alloc/free, it fires the corresponding notifier. After call into VFIO, > > the notifier function itself figure out the container it is bound. In > > this way, it's the duty of vIOMMU emulator to figure out a proper > > notifier to fire. From interface point of view, it is no longer > > per-device. > > Exactly. Cool, let me prepare another version with the ideas. Thanks for your review. :-) Regards, Yi Liu > > Also, it leaves the PASID management details to vIOMMU emulator as it > > can be vendor specific. Does it make sense? > > Also, I'd like to know if you have any other idea on it. That would > > surely be helpful. :-) > > > > > > Also, curious on the SVM context > > > > concept, do you mean it a per-VM context or a per-SVM usage context? > > > > May you elaborate a little more. :-) > > > > > > Sorry, I'm struggling to find a good term for this. By "context" I > > > mean a namespace containing a bunch of PASID address spaces, those > > > PASIDs are then visible to some group of devices. > > > > I see. May be the SVMContext instance above can include multiple PASID > > address spaces. And again, I think this relationship should be > > maintained in vIOMMU emulator.