[v5,2/9] pci: insert assert that auto-assigned-address function is single function device.

Auto-assigned-address pci function (passing devfn = -1) is always
single function.
This patch adds assert() to guarantee that auto-assigned-address function
is always single function device at function = 0.

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>

---
changes v2 -> v3
split out into PCI_DEVFN() patch and assert patch.
---
 hw/pci.c |    3 ++-
 hw/pci.h |    1 +
 2 files changed, 3 insertions(+), 1 deletions(-)

On Wed, Jun 23, 2010 at 04:15:26PM +0900, Isaku Yamahata wrote:
> Auto-assigned-address pci function (passing devfn = -1) is always
> single function.
> This patch adds assert() to guarantee that auto-assigned-address function
> is always single function device at function = 0.
> 
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>

So this means we must pass PCI_DEVFN(x, 0) to pci_register_bus?

> ---
> changes v2 -> v3
> split out into PCI_DEVFN() patch and assert patch.
> ---
>  hw/pci.c |    3 ++-
>  hw/pci.h |    1 +
>  2 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/pci.c b/hw/pci.c
> index ef17eb4..1ba209f 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -224,6 +224,7 @@ void pci_bus_new_inplace(PCIBus *bus, DeviceState *parent,
>                           const char *name, int devfn_min)
>  {
>      qbus_create_inplace(&bus->qbus, &pci_bus_info, parent, name);
> +    assert(PCI_FUNC(devfn_min) == 0);
>      bus->devfn_min = devfn_min;
>      QLIST_INIT(&bus->child);
>      vmstate_register(-1, &vmstate_pcibus, bus);
> @@ -601,7 +602,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
>  {
>      if (devfn < 0) {
>          for(devfn = bus->devfn_min ; devfn < ARRAY_SIZE(bus->devices);
> -            devfn += 8) {
> +            devfn += PCI_FUNC_MAX) {
>              if (!bus->devices[devfn])
>                  goto found;
>          }
> diff --git a/hw/pci.h b/hw/pci.h
> index 6a2bc6a..077387d 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -14,6 +14,7 @@
>  #define PCI_DEVFN(slot, func)   ((((slot) & 0x1f) << 3) | ((func) & 0x07))
>  #define PCI_SLOT(devfn)         (((devfn) >> 3) & 0x1f)
>  #define PCI_FUNC(devfn)         ((devfn) & 0x07)
> +#define PCI_FUNC_MAX            8
>  
>  /* Class, Vendor and Device IDs from Linux's pci_ids.h */
>  #include "pci_ids.h"
> -- 
> 1.6.6.1

On Wed, Jun 23, 2010 at 01:11:00PM +0300, Michael S. Tsirkin wrote:
> On Wed, Jun 23, 2010 at 04:15:26PM +0900, Isaku Yamahata wrote:
> > Auto-assigned-address pci function (passing devfn = -1) is always
> > single function.
> > This patch adds assert() to guarantee that auto-assigned-address function
> > is always single function device at function = 0.
> > 
> > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> 
> So this means we must pass PCI_DEVFN(x, 0) to pci_register_bus?

Yes.
devfn_min is used only for devfn auto assignment.
By checking the commit logs, it seems that the changeset
which introduced devfn_min easily chose devfn than dev.


---
commit 30468f786c127fc027d84c0aec6155e3e59475bb
Author: bellard <bellard@c046a42c-6fe2-441c-8c8c-71466251a162>
Date:   Mon Jun 21 19:45:35 2004 +0000

    added PCI bus - added IRQ support for PowerPC bridges - suppressed PREP PCI 
bios init

---
commit 502a53952d574717bdb626b651b16cadacab46f4
Author: pbrook <pbrook@c046a42c-6fe2-441c-8c8c-71466251a162>
Date:   Sat May 13 16:11:23 2006 +0000

    Rearrange PCI host emulation code.
    Add ARM PCI emulation.



> 
> > ---
> > changes v2 -> v3
> > split out into PCI_DEVFN() patch and assert patch.
> > ---
> >  hw/pci.c |    3 ++-
> >  hw/pci.h |    1 +
> >  2 files changed, 3 insertions(+), 1 deletions(-)
> > 
> > diff --git a/hw/pci.c b/hw/pci.c
> > index ef17eb4..1ba209f 100644
> > --- a/hw/pci.c
> > +++ b/hw/pci.c
> > @@ -224,6 +224,7 @@ void pci_bus_new_inplace(PCIBus *bus, DeviceState *parent,
> >                           const char *name, int devfn_min)
> >  {
> >      qbus_create_inplace(&bus->qbus, &pci_bus_info, parent, name);
> > +    assert(PCI_FUNC(devfn_min) == 0);
> >      bus->devfn_min = devfn_min;
> >      QLIST_INIT(&bus->child);
> >      vmstate_register(-1, &vmstate_pcibus, bus);
> > @@ -601,7 +602,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
> >  {
> >      if (devfn < 0) {
> >          for(devfn = bus->devfn_min ; devfn < ARRAY_SIZE(bus->devices);
> > -            devfn += 8) {
> > +            devfn += PCI_FUNC_MAX) {
> >              if (!bus->devices[devfn])
> >                  goto found;
> >          }
> > diff --git a/hw/pci.h b/hw/pci.h
> > index 6a2bc6a..077387d 100644
> > --- a/hw/pci.h
> > +++ b/hw/pci.h
> > @@ -14,6 +14,7 @@
> >  #define PCI_DEVFN(slot, func)   ((((slot) & 0x1f) << 3) | ((func) & 0x07))
> >  #define PCI_SLOT(devfn)         (((devfn) >> 3) & 0x1f)
> >  #define PCI_FUNC(devfn)         ((devfn) & 0x07)
> > +#define PCI_FUNC_MAX            8
> >  
> >  /* Class, Vendor and Device IDs from Linux's pci_ids.h */
> >  #include "pci_ids.h"
> > -- 
> > 1.6.6.1
>