[Qemu-devel] pci_nic_init_nofail() only works on root PCI buses?

[Qemu-devel] pci_nic_init_nofail() only works on root PCI buses?

[Mark Cave-Ayland]

Hi all,

Playing around with trying to set up PCI bridges for sun4u, I noticed
that I get an assert with the following diff which simply moves the
default NIC behind a PCI bridge rather than being directly on the root
PCI bus:

diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 69f565d..0b17f2e 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -465,7 +465,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
     parallel_hds_isa_init(isa_bus, MAX_PARALLEL_PORTS);

     for(i = 0; i < nb_nics; i++)
-        pci_nic_init_nofail(&nd_table[i], pci_bus, "ne2k_pci", NULL);
+        pci_nic_init_nofail(&nd_table[i], pci_bus3, "ne2k_pci", NULL);

     ide_drive_get(hd, ARRAY_SIZE(hd));


$ ./qemu-system-sparc64 -nographic
qemu-system-sparc64: hw/pci/pci.c:682: pci_get_bus_devfn: Assertion
`!root->parent_dev' failed.
Aborted

Is there any particular reason why the assert() is set to fail if the
NIC isn't being instantiated on the root PCI bus?


ATB,

Mark.

Re: [Qemu-devel] pci_nic_init_nofail() only works on root PCI buses?

[Thomas Huth]

On 11.06.2017 17:08, Mark Cave-Ayland wrote:
> Hi all,
> 
> Playing around with trying to set up PCI bridges for sun4u, I noticed
> that I get an assert with the following diff which simply moves the
> default NIC behind a PCI bridge rather than being directly on the root
> PCI bus:
> 
> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> index 69f565d..0b17f2e 100644
> --- a/hw/sparc64/sun4u.c
> +++ b/hw/sparc64/sun4u.c
> @@ -465,7 +465,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>      parallel_hds_isa_init(isa_bus, MAX_PARALLEL_PORTS);
> 
>      for(i = 0; i < nb_nics; i++)
> -        pci_nic_init_nofail(&nd_table[i], pci_bus, "ne2k_pci", NULL);
> +        pci_nic_init_nofail(&nd_table[i], pci_bus3, "ne2k_pci", NULL);
> 
>      ide_drive_get(hd, ARRAY_SIZE(hd));
> 
> 
> $ ./qemu-system-sparc64 -nographic
> qemu-system-sparc64: hw/pci/pci.c:682: pci_get_bus_devfn: Assertion
> `!root->parent_dev' failed.
> Aborted
> 
> Is there any particular reason why the assert() is set to fail if the
> NIC isn't being instantiated on the root PCI bus?

Sounds strange indeed. At least the patch description of the commit that
introduce the assert() says that using anything else seems to be an error:

http://git.qemu.org/?p=qemu.git;a=commitdiff;h=85c6e4fabb4c26e5cd8a0

Not sure whether that's still true nowadays, since that commit is
already 4 years old...

 Thomas

Re: [Qemu-devel] pci_nic_init_nofail() only works on root PCI buses?

[Michael S. Tsirkin]

On Wed, Jun 21, 2017 at 03:00:31AM +0200, Thomas Huth wrote:
> On 11.06.2017 17:08, Mark Cave-Ayland wrote:
> > Hi all,
> > 
> > Playing around with trying to set up PCI bridges for sun4u, I noticed
> > that I get an assert with the following diff which simply moves the
> > default NIC behind a PCI bridge rather than being directly on the root
> > PCI bus:
> > 
> > diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> > index 69f565d..0b17f2e 100644
> > --- a/hw/sparc64/sun4u.c
> > +++ b/hw/sparc64/sun4u.c
> > @@ -465,7 +465,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
> >      parallel_hds_isa_init(isa_bus, MAX_PARALLEL_PORTS);
> > 
> >      for(i = 0; i < nb_nics; i++)
> > -        pci_nic_init_nofail(&nd_table[i], pci_bus, "ne2k_pci", NULL);
> > +        pci_nic_init_nofail(&nd_table[i], pci_bus3, "ne2k_pci", NULL);
> > 
> >      ide_drive_get(hd, ARRAY_SIZE(hd));
> > 
> > 
> > $ ./qemu-system-sparc64 -nographic
> > qemu-system-sparc64: hw/pci/pci.c:682: pci_get_bus_devfn: Assertion
> > `!root->parent_dev' failed.
> > Aborted
> > 
> > Is there any particular reason why the assert() is set to fail if the
> > NIC isn't being instantiated on the root PCI bus?
> 
> Sounds strange indeed. At least the patch description of the commit that
> introduce the assert() says that using anything else seems to be an error:
> 
> http://git.qemu.org/?p=qemu.git;a=commitdiff;h=85c6e4fabb4c26e5cd8a0
> 
> Not sure whether that's still true nowadays, since that commit is
> already 4 years old...
> 
>  Thomas

pci_get_bus_devfn is not a sensible interface. It uses bus numbers which
are guest assigned in the machine monitor interface.

We keep it around for compatibility reasons.

Just avoid legacy APIs is my advice. New machine types should create
device instances with -device or similar.

-- 
MST