[Qemu-devel] [PATCH] Name the default PCI bus "pci.0" on all architectures

[Qemu-devel] [PATCH] Name the default PCI bus "pci.0" on all architectures

[Daniel P. Berrange]

The system emulators for each arch are using inconsistent
naming for the default PCI bus "pci" vs "pci.0". Since it
is conceivable we'll have multiple PCI buses in the future
standardize on "pci.0" for all architectures. This ensures
mgmt apps can rely on a name when assigning PCI devices an
address on the bus using eg '-device e1000,bus=pci.0,addr=3'

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 hw/grackle_pci.c   |    2 +-
 hw/gt64xxx.c       |    2 +-
 hw/ppc4xx_pci.c    |    2 +-
 hw/ppce500_pci.c   |    2 +-
 hw/prep_pci.c      |    2 +-
 hw/sh_pci.c        |    2 +-
 hw/unin_pci.c      |    4 ++--
 hw/versatile_pci.c |    2 +-
 8 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/hw/grackle_pci.c b/hw/grackle_pci.c
index aa0c51b..8444a35 100644
--- a/hw/grackle_pci.c
+++ b/hw/grackle_pci.c
@@ -88,7 +88,7 @@ PCIBus *pci_grackle_init(uint32_t base, qemu_irq *pic)
     qdev_init_nofail(dev);
     s = sysbus_from_qdev(dev);
     d = FROM_SYSBUS(GrackleState, s);
-    d->host_state.bus = pci_register_bus(&d->busdev.qdev, "pci",
+    d->host_state.bus = pci_register_bus(&d->busdev.qdev, "pci.0",
                                          pci_grackle_set_irq,
                                          pci_grackle_map_irq,
                                          pic, 0, 4);
diff --git a/hw/gt64xxx.c b/hw/gt64xxx.c
index 55971b9..756e1bf 100644
--- a/hw/gt64xxx.c
+++ b/hw/gt64xxx.c
@@ -1113,7 +1113,7 @@ PCIBus *pci_gt64120_init(qemu_irq *pic)
     s = qemu_mallocz(sizeof(GT64120State));
     s->pci = qemu_mallocz(sizeof(GT64120PCIState));
 
-    s->pci->bus = pci_register_bus(NULL, "pci",
+    s->pci->bus = pci_register_bus(NULL, "pci.0",
                                    pci_gt64120_set_irq, pci_gt64120_map_irq,
                                    pic, 144, 4);
     s->ISD_handle = cpu_register_io_memory(gt64120_read, gt64120_write, s);
diff --git a/hw/ppc4xx_pci.c b/hw/ppc4xx_pci.c
index c9e3279..dc1d2f8 100644
--- a/hw/ppc4xx_pci.c
+++ b/hw/ppc4xx_pci.c
@@ -357,7 +357,7 @@ PCIBus *ppc4xx_pci_init(CPUState *env, qemu_irq pci_irqs[4],
 
     controller = qemu_mallocz(sizeof(PPC4xxPCIState));
 
-    controller->pci_state.bus = pci_register_bus(NULL, "pci",
+    controller->pci_state.bus = pci_register_bus(NULL, "pci.0",
                                                  ppc4xx_pci_set_irq,
                                                  ppc4xx_pci_map_irq,
                                                  pci_irqs, 0, 4);
diff --git a/hw/ppce500_pci.c b/hw/ppce500_pci.c
index 336d284..fa4387a 100644
--- a/hw/ppce500_pci.c
+++ b/hw/ppce500_pci.c
@@ -276,7 +276,7 @@ PCIBus *ppce500_pci_init(qemu_irq pci_irqs[4], target_phys_addr_t registers)
 
     controller = qemu_mallocz(sizeof(PPCE500PCIState));
 
-    controller->pci_state.bus = pci_register_bus(NULL, "pci",
+    controller->pci_state.bus = pci_register_bus(NULL, "pci.0",
                                                  mpc85xx_pci_set_irq,
                                                  mpc85xx_pci_map_irq,
                                                  pci_irqs, 0x88, 4);
diff --git a/hw/prep_pci.c b/hw/prep_pci.c
index 144fde0..7ea7ca5 100644
--- a/hw/prep_pci.c
+++ b/hw/prep_pci.c
@@ -117,7 +117,7 @@ PCIBus *pci_prep_init(qemu_irq *pic)
     int PPC_io_memory;
 
     s = qemu_mallocz(sizeof(PREPPCIState));
-    s->bus = pci_register_bus(NULL, "pci",
+    s->bus = pci_register_bus(NULL, "pci.0",
                               prep_set_irq, prep_map_irq, pic, 0, 4);
 
     pci_host_conf_register_ioport(0xcf8, s);
diff --git a/hw/sh_pci.c b/hw/sh_pci.c
index cc2f190..0e138ed 100644
--- a/hw/sh_pci.c
+++ b/hw/sh_pci.c
@@ -98,7 +98,7 @@ PCIBus *sh_pci_register_bus(pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
     int reg;
 
     p = qemu_mallocz(sizeof(SHPCIC));
-    p->bus = pci_register_bus(NULL, "pci",
+    p->bus = pci_register_bus(NULL, "pci.0",
                               set_irq, map_irq, opaque, devfn_min, nirq);
 
     p->dev = pci_register_device(p->bus, "SH PCIC", sizeof(PCIDevice),
diff --git a/hw/unin_pci.c b/hw/unin_pci.c
index f0a773d..57c56e0 100644
--- a/hw/unin_pci.c
+++ b/hw/unin_pci.c
@@ -226,7 +226,7 @@ PCIBus *pci_pmac_init(qemu_irq *pic)
     qdev_init_nofail(dev);
     s = sysbus_from_qdev(dev);
     d = FROM_SYSBUS(UNINState, s);
-    d->host_state.bus = pci_register_bus(&d->busdev.qdev, "pci",
+    d->host_state.bus = pci_register_bus(&d->busdev.qdev, "pci.0",
                                          pci_unin_set_irq, pci_unin_map_irq,
                                          pic, 11 << 3, 4);
 
@@ -278,7 +278,7 @@ PCIBus *pci_pmac_u3_init(qemu_irq *pic)
     s = sysbus_from_qdev(dev);
     d = FROM_SYSBUS(UNINState, s);
 
-    d->host_state.bus = pci_register_bus(&d->busdev.qdev, "pci",
+    d->host_state.bus = pci_register_bus(&d->busdev.qdev, "pci.0",
                                          pci_unin_set_irq, pci_unin_map_irq,
                                          pic, 11 << 3, 4);
 
diff --git a/hw/versatile_pci.c b/hw/versatile_pci.c
index 199bc19..d0469ff 100644
--- a/hw/versatile_pci.c
+++ b/hw/versatile_pci.c
@@ -125,7 +125,7 @@ static int pci_vpb_init(SysBusDevice *dev)
     for (i = 0; i < 4; i++) {
         sysbus_init_irq(dev, &s->irq[i]);
     }
-    bus = pci_register_bus(&dev->qdev, "pci",
+    bus = pci_register_bus(&dev->qdev, "pci.0",
                            pci_vpb_set_irq, pci_vpb_map_irq, s->irq,
                            11 << 3, 4);
 
-- 
1.6.6.1

Re: [Qemu-devel] [PATCH] Name the default PCI bus "pci.0" on all architectures

[Blue Swirl]

On 5/19/10, Daniel P. Berrange <berrange@redhat.com> wrote:
> The system emulators for each arch are using inconsistent
>  naming for the default PCI bus "pci" vs "pci.0". Since it
>  is conceivable we'll have multiple PCI buses in the future
>  standardize on "pci.0" for all architectures. This ensures
>  mgmt apps can rely on a name when assigning PCI devices an
>  address on the bus using eg '-device e1000,bus=pci.0,addr=3'
>
>  Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
>  ---
>   hw/grackle_pci.c   |    2 +-
>   hw/gt64xxx.c       |    2 +-
>   hw/ppc4xx_pci.c    |    2 +-
>   hw/ppce500_pci.c   |    2 +-
>   hw/prep_pci.c      |    2 +-
>   hw/sh_pci.c        |    2 +-
>   hw/unin_pci.c      |    4 ++--
>   hw/versatile_pci.c |    2 +-

Missing hw/apb_pci.c.

>   8 files changed, 9 insertions(+), 9 deletions(-)
>
>  diff --git a/hw/grackle_pci.c b/hw/grackle_pci.c
>  index aa0c51b..8444a35 100644
>  --- a/hw/grackle_pci.c
>  +++ b/hw/grackle_pci.c
>  @@ -88,7 +88,7 @@ PCIBus *pci_grackle_init(uint32_t base, qemu_irq *pic)
>      qdev_init_nofail(dev);
>      s = sysbus_from_qdev(dev);
>      d = FROM_SYSBUS(GrackleState, s);
>  -    d->host_state.bus = pci_register_bus(&d->busdev.qdev, "pci",
>  +    d->host_state.bus = pci_register_bus(&d->busdev.qdev, "pci.0",
>                                           pci_grackle_set_irq,
>                                           pci_grackle_map_irq,
>                                           pic, 0, 4);
>  diff --git a/hw/gt64xxx.c b/hw/gt64xxx.c
>  index 55971b9..756e1bf 100644
>  --- a/hw/gt64xxx.c
>  +++ b/hw/gt64xxx.c
>  @@ -1113,7 +1113,7 @@ PCIBus *pci_gt64120_init(qemu_irq *pic)
>      s = qemu_mallocz(sizeof(GT64120State));
>      s->pci = qemu_mallocz(sizeof(GT64120PCIState));
>
>  -    s->pci->bus = pci_register_bus(NULL, "pci",
>  +    s->pci->bus = pci_register_bus(NULL, "pci.0",
>                                     pci_gt64120_set_irq, pci_gt64120_map_irq,
>                                     pic, 144, 4);
>      s->ISD_handle = cpu_register_io_memory(gt64120_read, gt64120_write, s);
>  diff --git a/hw/ppc4xx_pci.c b/hw/ppc4xx_pci.c
>  index c9e3279..dc1d2f8 100644
>  --- a/hw/ppc4xx_pci.c
>  +++ b/hw/ppc4xx_pci.c
>  @@ -357,7 +357,7 @@ PCIBus *ppc4xx_pci_init(CPUState *env, qemu_irq pci_irqs[4],
>
>      controller = qemu_mallocz(sizeof(PPC4xxPCIState));
>
>  -    controller->pci_state.bus = pci_register_bus(NULL, "pci",
>  +    controller->pci_state.bus = pci_register_bus(NULL, "pci.0",
>                                                   ppc4xx_pci_set_irq,
>                                                   ppc4xx_pci_map_irq,
>                                                   pci_irqs, 0, 4);
>  diff --git a/hw/ppce500_pci.c b/hw/ppce500_pci.c
>  index 336d284..fa4387a 100644
>  --- a/hw/ppce500_pci.c
>  +++ b/hw/ppce500_pci.c
>  @@ -276,7 +276,7 @@ PCIBus *ppce500_pci_init(qemu_irq pci_irqs[4], target_phys_addr_t registers)
>
>      controller = qemu_mallocz(sizeof(PPCE500PCIState));
>
>  -    controller->pci_state.bus = pci_register_bus(NULL, "pci",
>  +    controller->pci_state.bus = pci_register_bus(NULL, "pci.0",
>                                                   mpc85xx_pci_set_irq,
>                                                   mpc85xx_pci_map_irq,
>                                                   pci_irqs, 0x88, 4);
>  diff --git a/hw/prep_pci.c b/hw/prep_pci.c
>  index 144fde0..7ea7ca5 100644
>  --- a/hw/prep_pci.c
>  +++ b/hw/prep_pci.c
>  @@ -117,7 +117,7 @@ PCIBus *pci_prep_init(qemu_irq *pic)
>      int PPC_io_memory;
>
>      s = qemu_mallocz(sizeof(PREPPCIState));
>  -    s->bus = pci_register_bus(NULL, "pci",
>  +    s->bus = pci_register_bus(NULL, "pci.0",
>                                prep_set_irq, prep_map_irq, pic, 0, 4);
>
>      pci_host_conf_register_ioport(0xcf8, s);
>  diff --git a/hw/sh_pci.c b/hw/sh_pci.c
>  index cc2f190..0e138ed 100644
>  --- a/hw/sh_pci.c
>  +++ b/hw/sh_pci.c
>  @@ -98,7 +98,7 @@ PCIBus *sh_pci_register_bus(pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
>      int reg;
>
>      p = qemu_mallocz(sizeof(SHPCIC));
>  -    p->bus = pci_register_bus(NULL, "pci",
>  +    p->bus = pci_register_bus(NULL, "pci.0",
>                                set_irq, map_irq, opaque, devfn_min, nirq);
>
>      p->dev = pci_register_device(p->bus, "SH PCIC", sizeof(PCIDevice),
>  diff --git a/hw/unin_pci.c b/hw/unin_pci.c
>  index f0a773d..57c56e0 100644
>  --- a/hw/unin_pci.c
>  +++ b/hw/unin_pci.c
>  @@ -226,7 +226,7 @@ PCIBus *pci_pmac_init(qemu_irq *pic)
>      qdev_init_nofail(dev);
>      s = sysbus_from_qdev(dev);
>      d = FROM_SYSBUS(UNINState, s);
>  -    d->host_state.bus = pci_register_bus(&d->busdev.qdev, "pci",
>  +    d->host_state.bus = pci_register_bus(&d->busdev.qdev, "pci.0",
>                                           pci_unin_set_irq, pci_unin_map_irq,
>                                           pic, 11 << 3, 4);
>
>  @@ -278,7 +278,7 @@ PCIBus *pci_pmac_u3_init(qemu_irq *pic)
>      s = sysbus_from_qdev(dev);
>      d = FROM_SYSBUS(UNINState, s);
>
>  -    d->host_state.bus = pci_register_bus(&d->busdev.qdev, "pci",
>  +    d->host_state.bus = pci_register_bus(&d->busdev.qdev, "pci.0",
>                                           pci_unin_set_irq, pci_unin_map_irq,
>                                           pic, 11 << 3, 4);
>
>  diff --git a/hw/versatile_pci.c b/hw/versatile_pci.c
>  index 199bc19..d0469ff 100644
>  --- a/hw/versatile_pci.c
>  +++ b/hw/versatile_pci.c
>  @@ -125,7 +125,7 @@ static int pci_vpb_init(SysBusDevice *dev)
>      for (i = 0; i < 4; i++) {
>          sysbus_init_irq(dev, &s->irq[i]);
>      }
>  -    bus = pci_register_bus(&dev->qdev, "pci",
>  +    bus = pci_register_bus(&dev->qdev, "pci.0",
>                             pci_vpb_set_irq, pci_vpb_map_irq, s->irq,
>                             11 << 3, 4);
>
>
>  --
>  1.6.6.1
>
>
>

Re: [Qemu-devel] [PATCH] Name the default PCI bus "pci.0" on all architectures

[Daniel P. Berrange]

On Wed, May 19, 2010 at 10:19:06PM +0300, Blue Swirl wrote:
> On 5/19/10, Daniel P. Berrange <berrange@redhat.com> wrote:
> > The system emulators for each arch are using inconsistent
> >  naming for the default PCI bus "pci" vs "pci.0". Since it
> >  is conceivable we'll have multiple PCI buses in the future
> >  standardize on "pci.0" for all architectures. This ensures
> >  mgmt apps can rely on a name when assigning PCI devices an
> >  address on the bus using eg '-device e1000,bus=pci.0,addr=3'
> >
> >  Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> >  ---
> >   hw/grackle_pci.c   |    2 +-
> >   hw/gt64xxx.c       |    2 +-
> >   hw/ppc4xx_pci.c    |    2 +-
> >   hw/ppce500_pci.c   |    2 +-
> >   hw/prep_pci.c      |    2 +-
> >   hw/sh_pci.c        |    2 +-
> >   hw/unin_pci.c      |    4 ++--
> >   hw/versatile_pci.c |    2 +-
> 
> Missing hw/apb_pci.c.

Ah yes, don't know how I missed that. Have posted another version
of this patch with that included

Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [Qemu-devel] [PATCH] Name the default PCI bus "pci.0" on all architectures

[Paul Brook]

> The system emulators for each arch are using inconsistent
> naming for the default PCI bus "pci" vs "pci.0". Since it
> is conceivable we'll have multiple PCI buses in the future
> standardize on "pci.0" for all architectures. This ensures
> mgmt apps can rely on a name when assigning PCI devices an
> address on the bus using eg '-device e1000,bus=pci.0,addr=3'

No. Bus names are local to the parent device.  None of the host bridges 
support multiple bridges, so the ".0" suffix makes no sense.  The parent 
device has no idea whether it owns the "default" pci bus or not.
If you have multiple PCI busses then you can identify them by the device path.

Paul

Re: [Qemu-devel] [PATCH] Name the default PCI bus "pci.0" on all architectures

[Markus Armbruster]

Paul Brook <paul@codesourcery.com> writes:

>> The system emulators for each arch are using inconsistent
>> naming for the default PCI bus "pci" vs "pci.0". Since it
>> is conceivable we'll have multiple PCI buses in the future
>> standardize on "pci.0" for all architectures. This ensures
>> mgmt apps can rely on a name when assigning PCI devices an
>> address on the bus using eg '-device e1000,bus=pci.0,addr=3'
>
> No. Bus names are local to the parent device.  None of the host bridges 
> support multiple bridges, so the ".0" suffix makes no sense.  The parent 
> device has no idea whether it owns the "default" pci bus or not.
> If you have multiple PCI busses then you can identify them by the device path.

>From qbus_create_inplace():

    if (name) {
        /* use supplied name */
        bus->name = qemu_strdup(name);
    } else if (parent && parent->id) {
        /* parent device has id -> use it for bus name */
        len = strlen(parent->id) + 16;
        buf = qemu_malloc(len);
        snprintf(buf, len, "%s.%d", parent->id, parent->num_child_bus);
        bus->name = buf;
    } else {
        /* no id -> use lowercase bus type for bus name */
        len = strlen(info->name) + 16;
        buf = qemu_malloc(len);
        len = snprintf(buf, len, "%s.%d", info->name,
                       parent ? parent->num_child_bus : 0);
        for (i = 0; i < len; i++)
            buf[i] = qemu_tolower(buf[i]);
        bus->name = buf;
    }

If appending ".0" really makes no sense when the device has just one
bus, then we shouldn't append it in cases 2 & 3.

But I'd simply append it always.  One bus is just as countable as many.

Re: [Qemu-devel] [PATCH] Name the default PCI bus "pci.0" on all architectures

[Paul Brook]

> Paul Brook <paul@codesourcery.com> writes:
> >> The system emulators for each arch are using inconsistent
> >> naming for the default PCI bus "pci" vs "pci.0". Since it
> >> is conceivable we'll have multiple PCI buses in the future
> >> standardize on "pci.0" for all architectures. This ensures
> >> mgmt apps can rely on a name when assigning PCI devices an
> >> address on the bus using eg '-device e1000,bus=pci.0,addr=3'
> > 
> > No. Bus names are local to the parent device.  None of the host bridges
> > support multiple bridges, so the ".0" suffix makes no sense.  The parent
> > device has no idea whether it owns the "default" pci bus or not.
> > If you have multiple PCI busses then you can identify them by the device
> > path.
> 
> From qbus_create_inplace():
> 
>     if (name) {
>         /* use supplied name */
>         bus->name = qemu_strdup(name);
>     } else if (parent && parent->id) {
>         /* parent device has id -> use it for bus name */
>         len = strlen(parent->id) + 16;
>         buf = qemu_malloc(len);
>         snprintf(buf, len, "%s.%d", parent->id, parent->num_child_bus);
>         bus->name = buf;
>     } else {
>         /* no id -> use lowercase bus type for bus name */
>         len = strlen(info->name) + 16;
>         buf = qemu_malloc(len);
>         len = snprintf(buf, len, "%s.%d", info->name,
>                        parent ? parent->num_child_bus : 0);
>         for (i = 0; i < len; i++)
>             buf[i] = qemu_tolower(buf[i]);
>         bus->name = buf;
>     }
> 
> If appending ".0" really makes no sense when the device has just one
> bus, then we shouldn't append it in cases 2 & 3.

IMO the code you quote is completely bogus.  All devices should specify names 
for their child busses, failure to do so is a bug.

These bus names are local to the parent device. Trying to use them as global 
identifiers is just plain wrong.  A given device [type] should always have the 
same set of properties/child busses regardless of where it occurs in the 
device tree.
Using a single counter for all busses is also wrong. The order of bus creation 
is a device implementation detail, under no circumstances should it be part of 
the user visible API.  Consider a device that has both a PCI and I2C bus. It 
makes no sense to call there pci.0 and i2c.1.

Paul

Re: [Qemu-devel] [PATCH] Name the default PCI bus "pci.0" on all architectures

[Markus Armbruster]

Paul Brook <paul@codesourcery.com> writes:

>> Paul Brook <paul@codesourcery.com> writes:
>> >> The system emulators for each arch are using inconsistent
>> >> naming for the default PCI bus "pci" vs "pci.0". Since it
>> >> is conceivable we'll have multiple PCI buses in the future
>> >> standardize on "pci.0" for all architectures. This ensures
>> >> mgmt apps can rely on a name when assigning PCI devices an
>> >> address on the bus using eg '-device e1000,bus=pci.0,addr=3'
>> > 
>> > No. Bus names are local to the parent device.  None of the host bridges
>> > support multiple bridges, so the ".0" suffix makes no sense.  The parent
>> > device has no idea whether it owns the "default" pci bus or not.
>> > If you have multiple PCI busses then you can identify them by the device
>> > path.
>> 
>> From qbus_create_inplace():
>> 
>>     if (name) {
>>         /* use supplied name */
>>         bus->name = qemu_strdup(name);
>>     } else if (parent && parent->id) {
>>         /* parent device has id -> use it for bus name */
>>         len = strlen(parent->id) + 16;
>>         buf = qemu_malloc(len);
>>         snprintf(buf, len, "%s.%d", parent->id, parent->num_child_bus);
>>         bus->name = buf;
>>     } else {
>>         /* no id -> use lowercase bus type for bus name */
>>         len = strlen(info->name) + 16;
>>         buf = qemu_malloc(len);
>>         len = snprintf(buf, len, "%s.%d", info->name,
>>                        parent ? parent->num_child_bus : 0);
>>         for (i = 0; i < len; i++)
>>             buf[i] = qemu_tolower(buf[i]);
>>         bus->name = buf;
>>     }
>> 
>> If appending ".0" really makes no sense when the device has just one
>> bus, then we shouldn't append it in cases 2 & 3.
>
> IMO the code you quote is completely bogus.  All devices should specify names 
> for their child busses, failure to do so is a bug.
>
> These bus names are local to the parent device. Trying to use them as global 
> identifiers is just plain wrong.  A given device [type] should always have the 
> same set of properties/child busses regardless of where it occurs in the 
> device tree.

I agree.

Case in point: For a bug fix I'm working on, I need to find the drives
connected to an IDE controller.  Should be easy enough: go from
controller qdev to its qbus, iterate over the qdevs on that bus.  I need
to call qdev_get_child_bus() for the first step, obviously.  But what to
pass as name argument?  -EMAGIC.

Note that I *only* object to case 2 (derive bus name from parent
device's ID).  Case 3 (derive bus name from bus type) is fine with me.

> Using a single counter for all busses is also wrong. The order of bus creation 
> is a device implementation detail, under no circumstances should it be part of 
> the user visible API.  Consider a device that has both a PCI and I2C bus. It 
> makes no sense to call there pci.0 and i2c.1.

Yes, that's a bit ugly.

Re: [Qemu-devel] [PATCH] Name the default PCI bus "pci.0" on all architectures

[Paul Brook]

> Note that I *only* object to case 2 (derive bus name from parent
> device's ID).  Case 3 (derive bus name from bus type) is fine with me.
> 
> > Using a single counter for all busses is also wrong. The order of bus
> > creation is a device implementation detail, under no circumstances
> > should it be part of the user visible API.  Consider a device that has
> > both a PCI and I2C bus. It makes no sense to call there pci.0 and i2c.1.
> 
> Yes, that's a bit ugly.

I'd accept deriving bus name from bus type (case 3) if we remove the 
numbering.

If there are multiple busses of the same type then the parent device must 
provide a name. In this case I don't believe generic code has enough knowledge 
to make sensible or consistent choices. A user-visible interface defined by 
internal implementation details (order of creation) makes me twitchy.

Take a graphics card as an example of something with multiple I2C busses.
IMO it would make much more sense to call them things like ".ddc" and 
".sensors" than ".0" and ".1".

Paul

Re: [Qemu-devel] [PATCH] Name the default PCI bus "pci.0" on all architectures

[Daniel P. Berrange]

On Fri, May 28, 2010 at 08:39:53PM +0100, Paul Brook wrote:
> > The system emulators for each arch are using inconsistent
> > naming for the default PCI bus "pci" vs "pci.0". Since it
> > is conceivable we'll have multiple PCI buses in the future
> > standardize on "pci.0" for all architectures. This ensures
> > mgmt apps can rely on a name when assigning PCI devices an
> > address on the bus using eg '-device e1000,bus=pci.0,addr=3'
> 
> No. Bus names are local to the parent device.  None of the host bridges 
> support multiple bridges, so the ".0" suffix makes no sense.  The parent 
> device has no idea whether it owns the "default" pci bus or not.
> If you have multiple PCI busses then you can identify them by the device 
> path.

The problem is that the ID names of default devices in machines are ABI
sensitive. Management apps need to know what the ID of these default
devices are. The x86 machines have already used 'pci.0' as their name
in the previous 0.12 release and libvirt is using this naming. We later
discovered many non-x86 archs have a name of just 'pci'. We need a single
consistent naming across all arches, hence this patch whcih standardizes
on 'pci.0'. The '.N' convention is used extensively in QEMU and is more 
futureproof as & when QEMU supports multiple buses, without requiring 
apps to use the more verbose device paths to ensure uniquness.

Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [Qemu-devel] [PATCH] Name the default PCI bus "pci.0" on all architectures

[Paul Brook]

> The problem is that the ID names of default devices in machines are ABI
> sensitive. Management apps need to know what the ID of these default
> devices are. The x86 machines have already used 'pci.0' as their name
> in the previous 0.12 release and libvirt is using this naming. We later
> discovered many non-x86 archs have a name of just 'pci'. We need a single
> consistent naming across all arches, hence this patch whcih standardizes
> on 'pci.0'. 

I think you'll find x86 qdev conversion is both incomplete and incorrect.  
Specifically i440fx_int breaks several abstraction boundaries.  I raised this 
issue at the time, but was told this was only temporary, and would be fixed. 
x86 should be made to match the arches that have been converted properly.

> The '.N' convention is used extensively in QEMU and is more
> futureproof as & when QEMU supports multiple buses, without requiring
> apps to use the more verbose device paths to ensure uniquness.

I disagree.  Anything that depends on device creation order is fundamentally 
broken. If you want to create globally unique user-friendly tags for devices 
or busses then that is a completely different problem, and should be done via 
explicit aliases. qemu currently has no concept of a "default bus".

Paul

Re: [Qemu-devel] [PATCH] Name the default PCI bus "pci.0" on all architectures

[Gerd Hoffmann]

   Hi,

> I disagree.  Anything that depends on device creation order is fundamentally
> broken. If you want to create globally unique user-friendly tags for devices
> or busses then that is a completely different problem, and should be done via
> explicit aliases.

For anything created via -device the id does the job.  The device gets 
tagged with the supplied id, and any child busses of that device carry 
the id too, i.e.

   -device lsi,id=foo

creates a lsi scsi hostadapter with id 'foo' and a scsi bus with the 
name 'foo.0'.  A (theoretical) scsi hba with two scsi busses would have 
'foo.0' and 'foo.1' child busses.  If you don't specify a id you'll get 
'scsi.$nr'.  Numbers are per device, not global.  So if you add two lsi 
adapters without id you'll get two 'scsi.0' busses, so better don't do 
that if you want be able to address them via bus= ...

For devices created by machine->init() the names are more or less 
hard-coded in qemu though (and hopefully some day in some machine 
description file).  'pci.0' is the default name for a pci bus and IMHO a 
good choice for the primary pci bus.  secondary busses created by 
machine->init() (sparc64 does this I think) should get some other name.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH] Name the default PCI bus "pci.0" on all architectures

[Paul Brook]

>    Hi,
> 
> > I disagree.  Anything that depends on device creation order is
> > fundamentally broken. If you want to create globally unique
> > user-friendly tags for devices or busses then that is a completely
> > different problem, and should be done via explicit aliases.
> 
> For anything created via -device the id does the job.  The device gets
> tagged with the supplied id, and any child busses of that device carry
> the id too, i.e.
> 
>    -device lsi,id=foo
> 
> creates a lsi scsi hostadapter with id 'foo' and a scsi bus with the
> name 'foo.0'.  A (theoretical) scsi hba with two scsi busses would have
> 'foo.0' and 'foo.1' child busses.  If you don't specify a id you'll get
> 'scsi.$nr'.  Numbers are per device, not global.  So if you add two lsi
> adapters without id you'll get two 'scsi.0' busses, so better don't do
> that if you want be able to address them via bus= ...

IMO the name of the bus should not depend on the id of the device. Bus names 
should be used to distinguish between different child busses within the same 
device.  The parent device (plus local bus name if necessary) is more than 
sufficient to identify the bus.  One of the key features of the bus/device 
framework is that it gives us a tree structure and avoids the need for 
globally unique names. The real bug here is that the bus specifier should be a 
tree address. Once you do that you don't need the weird bus naming, and any 
aliases you create for a device can be trivially used to locate its child 
busses.

As mentioned previously, I think devices should be specifying the names of 
their child busses, and anything that passes NULL to qbus_create is just plain 
wrong. Trying to automagically allocate names is IMO fundamentally flawed.

Paul

Re: [Qemu-devel] [PATCH] Name the default PCI bus "pci.0" on all architectures

[Andreas Färber]

Am 02.06.2010 um 16:12 schrieb Daniel P. Berrange:

> On Fri, May 28, 2010 at 08:39:53PM +0100, Paul Brook wrote:
>>> The system emulators for each arch are using inconsistent
>>> naming for the default PCI bus "pci" vs "pci.0". Since it
>>> is conceivable we'll have multiple PCI buses in the future
>>> standardize on "pci.0" for all architectures. This ensures
>>> mgmt apps can rely on a name when assigning PCI devices an
>>> address on the bus using eg '-device e1000,bus=pci.0,addr=3'
>>
>> No. Bus names are local to the parent device.  None of the host  
>> bridges
>> support multiple bridges, so the ".0" suffix makes no sense.  The  
>> parent
>> device has no idea whether it owns the "default" pci bus or not.
>> If you have multiple PCI busses then you can identify them by the  
>> device
>> path.
>
> The problem is that the ID names of default devices in machines are  
> ABI
> sensitive. Management apps need to know what the ID of these default
> devices are. The x86 machines have already used 'pci.0' as their name
> in the previous 0.12 release and libvirt is using this naming. We  
> later
> discovered many non-x86 archs have a name of just 'pci'. We need a  
> single
> consistent naming across all arches, hence this patch whcih  
> standardizes
> on 'pci.0'.

Iiuc sparc and ppc try to follow the IEEE1275 OpenFirmware naming  
conventions. Those should not blindly be patched to some random  
convention just to make them more x86-like. OpenFirmware uses  
pci@80000000 on my ppc machine. In other places, e.g. disks, numbering  
appears to be done locally via @0, @1, etc. See `show-devs` for a  
complete listing.

As suggested by Paul there are device aliases, so that in `qemu-system- 
ppc -cdrom /dev/null` you can run `dev pci` followed by `pwd` to see  
the real device name.
Lacking a working `devalias`, see `dev /aliases .properties` for some  
more: screen, nvram, cd, cdrom, scca, sccb, adb-keyboard, adb-mouse

Andreas

> The '.N' convention is used extensively in QEMU and is more
> futureproof as & when QEMU supports multiple buses, without requiring
> apps to use the more verbose device paths to ensure uniquness.
>
> Daniel
> -- 
> |: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ 
>  :|
> |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org 
>  :|
> |: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ 
>  :|
> |: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742  
> 7D3B 9505 :|
>