[Qemu-devel] [PATCH] spapr-pci: change the default PCI bus naming

[Qemu-devel] [PATCH] spapr-pci: change the default PCI bus naming

[Alexey Kardashevskiy]

Previously libvirt required the first/default PCI bus to have name "pci".
Since QEMU can support multiple buses now, libvirt wants "pci.0" now.

This removes custom busname and lets QEMU make up default names.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---

I tested this with:
  -netdev tap,id=id0,ifname=tapqemu-tap-00,script=ifup.sh,downscript=ifdown.sh \
  -device e1000,id=id1,netdev=id0,mac=C0:41:49:4b:00:00 \
  -device \
  spapr-pci-host-bridge,index=5,id=aikbus \
  -netdev tap,id=id2,ifname=tap-1,script=ifup.sh,downscript=ifdown.sh \
  -device rtl8139,id=id3,netdev=id2,bus=aikbus.0,mac=C0:41:49:4b:00:01 \
  -device spapr-pci-vfio-host-bridge,id=id4,index=10,iommu=4 \

This creates a default PCI, an additional emulated PCI bus (named aikbus,
if I omit the name, it is pci.1 which is also fine) and VFIO bus (which is
not in upstream yet but still), this all works fine and I cannot see any flaw.


---
 hw/ppc/spapr_pci.c | 23 ++---------------------
 1 file changed, 2 insertions(+), 21 deletions(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 66ddf10..7ecb3df 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -499,7 +499,6 @@ static int spapr_phb_init(SysBusDevice *s)
     DeviceState *dev = DEVICE(s);
     sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(s);
     PCIHostState *phb = PCI_HOST_BRIDGE(s);
-    const char *busname;
     char *namebuf;
     int i;
     PCIBus *bus;
@@ -583,26 +582,8 @@ static int spapr_phb_init(SysBusDevice *s)
                              get_system_io(), 0, SPAPR_PCI_IO_WIN_SIZE);
     memory_region_add_subregion(get_system_memory(), sphb->io_win_addr,
                                 &sphb->iowindow);
-    /*
-     * Selecting a busname is more complex than you'd think, due to
-     * interacting constraints.  If the user has specified an id
-     * explicitly for the phb , then we want to use the qdev default
-     * of naming the bus based on the bridge device (so the user can
-     * then assign devices to it in the way they expect).  For the
-     * first / default PCI bus (index=0) we want to use just "pci"
-     * because libvirt expects there to be a bus called, simply,
-     * "pci".  Otherwise, we use the same name as in the device tree,
-     * since it's unique by construction, and makes the guest visible
-     * BUID clear.
-     */
-    if (dev->id) {
-        busname = NULL;
-    } else if (sphb->index == 0) {
-        busname = "pci";
-    } else {
-        busname = sphb->dtbusname;
-    }
-    bus = pci_register_bus(dev, busname,
+
+    bus = pci_register_bus(dev, NULL,
                            pci_spapr_set_irq, pci_spapr_map_irq, sphb,
                            &sphb->memspace, &sphb->iospace,
                            PCI_DEVFN(0, 0), PCI_NUM_PINS, TYPE_PCI_BUS);
-- 
1.8.4.rc4

Re: [Qemu-devel] [PATCH] spapr-pci: change the default PCI bus naming

[Paolo Bonzini]

Il 06/03/2014 04:11, Alexey Kardashevskiy ha scritto:
> Previously libvirt required the first/default PCI bus to have name "pci".
> Since QEMU can support multiple buses now, libvirt wants "pci.0" now.
>
> This removes custom busname and lets QEMU make up default names.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>
> I tested this with:
>   -netdev tap,id=id0,ifname=tapqemu-tap-00,script=ifup.sh,downscript=ifdown.sh \
>   -device e1000,id=id1,netdev=id0,mac=C0:41:49:4b:00:00 \
>   -device \
>   spapr-pci-host-bridge,index=5,id=aikbus \
>   -netdev tap,id=id2,ifname=tap-1,script=ifup.sh,downscript=ifdown.sh \
>   -device rtl8139,id=id3,netdev=id2,bus=aikbus.0,mac=C0:41:49:4b:00:01 \
>   -device spapr-pci-vfio-host-bridge,id=id4,index=10,iommu=4 \
>
> This creates a default PCI, an additional emulated PCI bus (named aikbus,
> if I omit the name, it is pci.1 which is also fine) and VFIO bus (which is
> not in upstream yet but still), this all works fine and I cannot see any flaw.

Thanks!

Andreas, are you taking this patch?

Paolo

>
> ---
>  hw/ppc/spapr_pci.c | 23 ++---------------------
>  1 file changed, 2 insertions(+), 21 deletions(-)
>
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 66ddf10..7ecb3df 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -499,7 +499,6 @@ static int spapr_phb_init(SysBusDevice *s)
>      DeviceState *dev = DEVICE(s);
>      sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(s);
>      PCIHostState *phb = PCI_HOST_BRIDGE(s);
> -    const char *busname;
>      char *namebuf;
>      int i;
>      PCIBus *bus;
> @@ -583,26 +582,8 @@ static int spapr_phb_init(SysBusDevice *s)
>                               get_system_io(), 0, SPAPR_PCI_IO_WIN_SIZE);
>      memory_region_add_subregion(get_system_memory(), sphb->io_win_addr,
>                                  &sphb->iowindow);
> -    /*
> -     * Selecting a busname is more complex than you'd think, due to
> -     * interacting constraints.  If the user has specified an id
> -     * explicitly for the phb , then we want to use the qdev default
> -     * of naming the bus based on the bridge device (so the user can
> -     * then assign devices to it in the way they expect).  For the
> -     * first / default PCI bus (index=0) we want to use just "pci"
> -     * because libvirt expects there to be a bus called, simply,
> -     * "pci".  Otherwise, we use the same name as in the device tree,
> -     * since it's unique by construction, and makes the guest visible
> -     * BUID clear.
> -     */
> -    if (dev->id) {
> -        busname = NULL;
> -    } else if (sphb->index == 0) {
> -        busname = "pci";
> -    } else {
> -        busname = sphb->dtbusname;
> -    }
> -    bus = pci_register_bus(dev, busname,
> +
> +    bus = pci_register_bus(dev, NULL,
>                             pci_spapr_set_irq, pci_spapr_map_irq, sphb,
>                             &sphb->memspace, &sphb->iospace,
>                             PCI_DEVFN(0, 0), PCI_NUM_PINS, TYPE_PCI_BUS);
>

Re: [Qemu-devel] [PATCH] spapr-pci: change the default PCI bus naming

[Daniel P. Berrange]

On Thu, Mar 06, 2014 at 12:47:28PM +0100, Paolo Bonzini wrote:
> Il 06/03/2014 04:11, Alexey Kardashevskiy ha scritto:
> >Previously libvirt required the first/default PCI bus to have name "pci".
> >Since QEMU can support multiple buses now, libvirt wants "pci.0" now.
> >
> >This removes custom busname and lets QEMU make up default names.
> >
> >Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> >---
> >
> >I tested this with:
> >  -netdev tap,id=id0,ifname=tapqemu-tap-00,script=ifup.sh,downscript=ifdown.sh \
> >  -device e1000,id=id1,netdev=id0,mac=C0:41:49:4b:00:00 \
> >  -device \
> >  spapr-pci-host-bridge,index=5,id=aikbus \
> >  -netdev tap,id=id2,ifname=tap-1,script=ifup.sh,downscript=ifdown.sh \
> >  -device rtl8139,id=id3,netdev=id2,bus=aikbus.0,mac=C0:41:49:4b:00:01 \
> >  -device spapr-pci-vfio-host-bridge,id=id4,index=10,iommu=4 \
> >
> >This creates a default PCI, an additional emulated PCI bus (named aikbus,
> >if I omit the name, it is pci.1 which is also fine) and VFIO bus (which is
> >not in upstream yet but still), this all works fine and I cannot see any flaw.
> 
> Thanks!
> 
> Andreas, are you taking this patch?

I'd like to see this change done across all machines, not just this
ppc one, so that libvirt can do 1 single version check and know that
the new naming applies for everything from that point.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH] spapr-pci: change the default PCI bus naming

[Paolo Bonzini]

Il 06/03/2014 12:50, Daniel P. Berrange ha scritto:
> I'd like to see this change done across all machines, not just this
> ppc one, so that libvirt can do 1 single version check and know that
> the new naming applies for everything from that point.

Yes, I'll take care of the other machines.

Paolo

Re: [Qemu-devel] [PATCH] spapr-pci: change the default PCI bus naming

[Andreas Färber]

Am 06.03.2014 04:11, schrieb Alexey Kardashevskiy:
> Previously libvirt required the first/default PCI bus to have name "pci".
> Since QEMU can support multiple buses now, libvirt wants "pci.0" now.
> 
> This removes custom busname and lets QEMU make up default names.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> 
> I tested this with:
>   -netdev tap,id=id0,ifname=tapqemu-tap-00,script=ifup.sh,downscript=ifdown.sh \
>   -device e1000,id=id1,netdev=id0,mac=C0:41:49:4b:00:00 \
>   -device \
>   spapr-pci-host-bridge,index=5,id=aikbus \
>   -netdev tap,id=id2,ifname=tap-1,script=ifup.sh,downscript=ifdown.sh \
>   -device rtl8139,id=id3,netdev=id2,bus=aikbus.0,mac=C0:41:49:4b:00:01 \
>   -device spapr-pci-vfio-host-bridge,id=id4,index=10,iommu=4 \
> 
> This creates a default PCI, an additional emulated PCI bus (named aikbus,

aikbus.0 for the bus according to example and info qtree, but yeah ;)

> if I omit the name, it is pci.1 which is also fine) and VFIO bus (which is
> not in upstream yet but still), this all works fine and I cannot see any flaw.
> 
> 
> ---
>  hw/ppc/spapr_pci.c | 23 ++---------------------
>  1 file changed, 2 insertions(+), 21 deletions(-)

Thanks, in absence of Alex applying to qom-next:
https://github.com/afaerber/qemu-cpu/commits/qom-next

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] spapr-pci: change the default PCI bus naming

[Alexey Kardashevskiy]

On 03/07/2014 03:18 AM, Andreas Färber wrote:
> Am 06.03.2014 04:11, schrieb Alexey Kardashevskiy:
>> Previously libvirt required the first/default PCI bus to have name "pci".
>> Since QEMU can support multiple buses now, libvirt wants "pci.0" now.
>>
>> This removes custom busname and lets QEMU make up default names.



Ufff. Does anyone know any workaround how to tell libvirt to use these
custom names? Since there is no "pci" (and there is "pci.0"), libvirt
cannot put any device onto default PCI bus and I do not see any way to
workaround it in XML, am I missing something here?


>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>>
>> I tested this with:
>>   -netdev tap,id=id0,ifname=tapqemu-tap-00,script=ifup.sh,downscript=ifdown.sh \
>>   -device e1000,id=id1,netdev=id0,mac=C0:41:49:4b:00:00 \
>>   -device \
>>   spapr-pci-host-bridge,index=5,id=aikbus \
>>   -netdev tap,id=id2,ifname=tap-1,script=ifup.sh,downscript=ifdown.sh \
>>   -device rtl8139,id=id3,netdev=id2,bus=aikbus.0,mac=C0:41:49:4b:00:01 \
>>   -device spapr-pci-vfio-host-bridge,id=id4,index=10,iommu=4 \
>>
>> This creates a default PCI, an additional emulated PCI bus (named aikbus,
> 
> aikbus.0 for the bus according to example and info qtree, but yeah ;)
> 
>> if I omit the name, it is pci.1 which is also fine) and VFIO bus (which is
>> not in upstream yet but still), this all works fine and I cannot see any flaw.
>>
>>
>> ---
>>  hw/ppc/spapr_pci.c | 23 ++---------------------
>>  1 file changed, 2 insertions(+), 21 deletions(-)
> 
> Thanks, in absence of Alex applying to qom-next:
> https://github.com/afaerber/qemu-cpu/commits/qom-next
> 
> Andreas
> 


-- 
Alexey

Re: [Qemu-devel] [PATCH] spapr-pci: change the default PCI bus naming

[Paolo Bonzini]

Il 03/04/2014 10:44, Alexey Kardashevskiy ha scritto:
>
> Ufff. Does anyone know any workaround how to tell libvirt to use these
> custom names? Since there is no "pci" (and there is "pci.0"), libvirt
> cannot put any device onto default PCI bus and I do not see any way to
> workaround it in XML, am I missing something here?

Can you just change libvirt (and break libvirt compatibility with QEMU < 
2.0)?

Paolo

Re: [Qemu-devel] [PATCH] spapr-pci: change the default PCI bus naming

[Alexey Kardashevskiy]

On 04/03/2014 10:09 PM, Paolo Bonzini wrote:
> Il 03/04/2014 10:44, Alexey Kardashevskiy ha scritto:
>>
>> Ufff. Does anyone know any workaround how to tell libvirt to use these
>> custom names? Since there is no "pci" (and there is "pci.0"), libvirt
>> cannot put any device onto default PCI bus and I do not see any way to
>> workaround it in XML, am I missing something here?
> 
> Can you just change libvirt (and break libvirt compatibility with QEMU < 2.0)?

I can change qemu (cheaper for me) but I cannot make others doing the same
thing :)


-- 
Alexey

Re: [Qemu-devel] [PATCH] spapr-pci: change the default PCI bus naming

[Hong-Hua.Yin]

libvirt need set QEMU_CAPS_PCI_MULTIBUS for PowerPC for QEMU>=2.0.

if (qemuCaps->arch == VIR_ARCH_PPC || qemuCaps->arch == VIR_ARCH_PPC64)
        virQEMUCapsSet(qemuCaps, QEMU_CAPS_PCI_MULTIBUS);

But for QEMU <2.0, how should we judge the machines with different PCI bus name?

BTW, libvirt maintainer would like a general solution for all the architecture 
not limited to PowerPC.

Olivia

> -----Original Message-----
> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> Sent: Thursday, April 03, 2014 7:10 PM
> To: Alexey Kardashevskiy; Andreas Färber; qemu-devel@nongnu.org
> Cc: Yin Olivia-R63875; Alexander Graf; qemu-ppc@nongnu.org
> Subject: Re: [Qemu-devel] [PATCH] spapr-pci: change the default PCI bus
> naming
> 
> Il 03/04/2014 10:44, Alexey Kardashevskiy ha scritto:
> >
> > Ufff. Does anyone know any workaround how to tell libvirt to use these
> > custom names? Since there is no "pci" (and there is "pci.0"), libvirt
> > cannot put any device onto default PCI bus and I do not see any way to
> > workaround it in XML, am I missing something here?
> 
> Can you just change libvirt (and break libvirt compatibility with QEMU <
> 2.0)?
> 
> Paolo
>