> From: Peter Xu [mailto:zhexu@xxxxxxxxxx] > Sent: Tuesday, July 9, 2019 10:12 AM > To: Liu, Yi L <yi.l.liu@xxxxxxxxx> > Subject: Re: [RFC v1 03/18] hw/pci: introduce PCIPASIDOps to PCIDevice > > On Fri, Jul 05, 2019 at 07:01:36PM +0800, Liu Yi L wrote: > > +void pci_setup_pasid_ops(PCIDevice *dev, PCIPASIDOps *ops) > > +{ > > + assert(ops && !dev->pasid_ops); > > + dev->pasid_ops = ops; > > +} > > + > > +bool pci_device_is_ops_set(PCIBus *bus, int32_t devfn) > > Name should be "pci_device_is_pasid_ops_set". Or maybe you can simply > drop this function because as long as you check it in helper functions > like [1] below always then it seems even unecessary. yes, the name should be "pci_device_is_pasid_ops_set". I noticed your comments on the necessity in another, let's talk in that thread. :-) > > +{ > > + PCIDevice *dev; > > + > > + if (!bus) { > > + return false; > > + } > > + > > + dev = bus->devices[devfn]; > > + return !!(dev && dev->pasid_ops); > > +} > > + > > +int pci_device_request_pasid_alloc(PCIBus *bus, int32_t devfn, > > + uint32_t min_pasid, uint32_t max_pasid) > > From VT-d spec I see that the virtual command "allocate pasid" does > not have bdf information so it's global, but here we've got bus/devfn. > I'm curious is that reserved for ARM or some other arch? You are right. VT-d spec doesn’t have bdf info. But we need to pass the allocation request via vfio. So this function has bdf info. In vIOMMU side, it should select a vfio-pci device and invoke this callback when it wants to request PASID alloc/free. > > +{ > > + PCIDevice *dev; > > + > > + if (!bus) { > > + return -1; > > + } > > + > > + dev = bus->devices[devfn]; > > + if (dev && dev->pasid_ops && dev->pasid_ops->alloc_pasid) { > > [1] > > > + return dev->pasid_ops->alloc_pasid(bus, devfn, min_pasid, max_pasid); Thanks, Yi Liu