On Thu, 2015-09-17 at 15:41 +0300, Marcel Apfelbaum wrote: > On 09/17/2015 03:12 PM, Knut Omang wrote: > > On Thu, 2015-09-17 at 14:11 +0300, Marcel Apfelbaum wrote: > > > On 09/12/2015 03:36 PM, Knut Omang wrote: > > > > Without this, the devfn argument to pci_create_*() > > > > does not affect the assigned devfn. > > > > > > > > Needed to support (VF_STRIDE,VF_OFFSET) values other than (1,1) > > > > for SR/IOV. > > > > > > > > Signed-off-by: Knut Omang <knut.om...@oracle.com> > > > > --- > > > > hw/pci/pci.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > > > > index ccea628..a5cc015 100644 > > > > --- a/hw/pci/pci.c > > > > +++ b/hw/pci/pci.c > > > > @@ -1840,7 +1840,7 @@ static void pci_qdev_realize(DeviceState > > > > *qdev, Error **errp) > > > > bus = PCI_BUS(qdev_get_parent_bus(qdev)); > > > > pci_dev = do_pci_register_device(pci_dev, bus, > > > > > > > > object_get_typename(OBJECT(qdev)), > > > > - pci_dev->devfn, errp); > > > > + > > > > object_property_get_int(OBJECT(qdev), "addr", NULL), errp); > > > Hi, > > > > > > I don't get this, using object_property_get_int on "addr" should > > > return the value of pci_dev->devfn, > > > can you please tell what exactly is not working? > > > > The problem is that at that point pci_dev->devfn has not been set > > yet - > > have commented on this before somewhere.. > > > > But "addr" property has the right value? Is indeed strange because it > should > get the value from pci_dev->devfn.
In the current version, in pci_qdev_realize() the devfn argument to do_pci_register_device() is set to pci_dev->devfn. Then inside do_pci_register_device that devfn argument is again used to set pci_dev->devfn. When the SR/IOV code tries to add a second device (the first VF) at (devfn + offset + stride) to the bus, it fails because devfn is 0 and that hits the PF in the devices[] array. > Don't get me wrong, this patch is OK. > I just want to understand if we have a hidden bug somewhere. Yes, havent dived into the details for a while but it probably works because the pci_dev->devfn argument is often -1 (don't care) and then the code will just search for an available function number. > Anyway, > Reviewed-by: Marcel Apfelbaum <mar...@redhat.com> Thanks, Knut > > Knut > > > > > Thanks, > > > Marcel > > > > > > > > > > > > > if (pci_dev == NULL) > > > > return; > > > > > > > > > > > > >
