[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 1/4] pci: Make use of the devfn property when
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH v2 1/4] pci: Make use of the devfn property when registering new devices |
Date: |
Tue, 2 Sep 2014 16:55:49 +0300 |
On Tue, Sep 02, 2014 at 03:44:28PM +0200, Knut Omang wrote:
> On Tue, 2014-09-02 at 16:03 +0300, Michael S. Tsirkin wrote:
> > On Tue, Sep 02, 2014 at 01:00:03PM +0200, 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 <address@hidden>
> >
> > Sorry, I don't understand the explanation exactly.
> > pci_dev->devfn is not set correctly? why?
>
> This probably has been broken by some of the qom adaptation if I
> understand it right: The devfn parameter in pci_create_multifunction()
> is only used to set the "addr" property, which in turn is not used
> anywhere as far as I can see. So the pci_dev->devfn has it's default
> value.
There are a bunch of places that depend on devfn being correct
though so I think this needs to be fixed.
>
> What I observe is that when I enable VFs with the igb example device,
> now with stride = 2 and offset 0x80 (as my real lab hardware implements)
> and enable some VFs, this yields devfn's of 0x80, 0x82, 0x84... sent as
> parameters to pci_create..(), I still end up with this (eg.
> the "natural" next function will be selected instead of the provided
> devfn):
>
> 01:00.0 Ethernet controller: Intel Corporation 82576 Gigabit Network
> Connection (rev 01)
> 01:00.1 Ethernet controller: Intel Corporation 82576 Virtual Function (rev 01)
> 01:00.2 Ethernet controller: Intel Corporation 82576 Virtual Function (rev 01)
> 01:00.3 Ethernet controller: Intel Corporation 82576 Virtual Function (rev 01)
> 01:00.4 Ethernet controller: Intel Corporation 82576 Virtual Function (rev 01)
>
> With this patch applied, I get the expected:
>
> 01:00.0 Ethernet controller: Intel Corporation 82576 Gigabit Network
> Connection (rev 01)
> 01:10.0 Ethernet controller: Intel Corporation 82576 Virtual Function (rev 01)
> 01:10.2 Ethernet controller: Intel Corporation 82576 Virtual Function (rev 01)
> 01:10.4 Ethernet controller: Intel Corporation 82576 Virtual Function (rev 01)
> 01:10.6 Ethernet controller: Intel Corporation 82576 Virtual Function (rev 01)
> > > ---
> > > 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 daeaeac..6b21dee 100644
> > > --- a/hw/pci/pci.c
> > > +++ b/hw/pci/pci.c
> > > @@ -1757,7 +1757,7 @@ static int pci_qdev_init(DeviceState *qdev)
> > > 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);
> > > +
> > > object_property_get_int(OBJECT(qdev), "addr", NULL));
> > > if (pci_dev == NULL)
> > > return -1;
> > >
> > > --
> > > 1.9.0
>