On Wed, Jul 10, 2019 at 11:08:15AM +0000, Liu, Yi L wrote: > > From: Peter Xu [mailto:[email protected]] > > Sent: Tuesday, July 9, 2019 10:12 AM > > To: Liu, Yi L <[email protected]> > > 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.
That doesn't seem conceptually right. IIUC, the pasids "belong" to a
sort of SVM context. It seems to be the alloc should be on that
object - and that object would already have some connection to any
relevant vfio containers. At the vfio level this seems like it should
be a container operation rather than a device operation.
> > > +{
> > > + 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
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
