Re: [Qemu-devel] [PATCH 6/8] pci: Simpler implementation of primary PCI bus

[David Gibson]

On Wed, May 29, 2013 at 01:17:13PM +0300, Michael S. Tsirkin wrote:
> On Wed, May 29, 2013 at 08:06:42PM +1000, David Gibson wrote:
> > On Wed, May 29, 2013 at 12:55:53PM +0300, Michael S. Tsirkin wrote:
> > > On Wed, May 29, 2013 at 07:43:41PM +1000, David Gibson wrote:
> > > > On Thu, May 23, 2013 at 10:16:27PM +1000, David Gibson wrote:
> > > > > On Thu, May 23, 2013 at 02:22:30PM +0300, Michael S. Tsirkin wrote:
> > > > > > On Thu, May 09, 2013 at 10:31:10AM +1000, David Gibson wrote:
> > > > > > > Currently pci_get_primary_bus() searches the list of root buses 
> > > > > > > for one
> > > > > > > with domain 0.  But since host buses are always registered with 
> > > > > > > domain 0,
> > > > > > > this just amounts to finding the only PCI host bus.
> > > > > > > 
> > > > > > > This simplifies the implementation by defining the primary PCI 
> > > > > > > bus to
> > > > > > > be the first one registered, using a global variable to track it.
> > > > > > > 
> > > > > > > Signed-off-by: David Gibson <address@hidden>
> > > > > > 
> > > > > > Or better: can we just fail if there is more than
> > > > > > one root?
> > > > > 
> > > > > That might work, I'll look into doing that.
> > > > 
> > > > So, the difficulty with this is that then any machine with multiple
> > > > PCI bridges could not use pci_nic_init(), since it calls
> > > > pci_get_bus_devfn() which calls pci_find_primary_bus() which would
> > > > always fail.  And using pci_nic_init() is more or less mandatory in
> > > > the machine_init function to support old-style nic configuration.
> > > > 
> > > > Suggestions?
> > > 
> > > You mean multiple PCI roots?
> > > Well, there are no legacy machines with multiple roots to support, are
> > > there?  So why do we need to support legacy flags for these new
> > > configurations?
> > 
> > Because people expect them.
> 
> People can learn, somehow they will learn to add a new root, so they can
> learn to use -device too.

Hrm.  I'd kind of like a second (third?) opinion on that.  Anthony?

> So let's make it fail on multiple roots, and output a message along the
> lines of "please use -device virtio-net-pci instead".

How to produce a meaningful error like that isn't totally obvious,
since the test for multiple roots is down in find_primary_pci_bus()
(or whatever), and once we get back up to pci_nic_init() we just know
that pci_get_bus_devfn() failed for some reason.

> > Plus on spapr we already support the
> > legacy nic options; it would be very strange for them to suddenly
> > break when we add a second host bridge.
> 
> Not sure who "we" is here. IMHO user should ask for a new
> machine type with two roots explicitly.

You seem to be thinking of the number of host bridges as a fixed
property of the platform, which it isn't on spapr.  PCI host bridges
are just another device.  Large scale real hardware can easily have
dozens of them.  In qemu we create one always as a convenience, but
users can (and will have to, for vfio) create additional ones
trivially with -device.

[Which raises another complication as a tangent.  People (and libvirt)
don't generally expect -nodefaults to remove the PCI bridge, but
arguably it should on spapr, since a PAPR guest with no PCI is
perfectly viable but there's currently no way to specify such a
thing.]

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: Digital signature