RE: [RFC v1 03/18] hw/pci: introduce PCIPASIDOps to PCIDevice

["Liu, Yi L" <yi.l.liu@intel.com>]

> From: Peter Xu [mailto:zhexu@redhat.com]
> Sent: Tuesday, July 9, 2019 10:12 AM
> To: Liu, Yi L <yi.l.liu@intel.com>
> 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