All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: David Gibson <david@gibson.dropbear.id.au>
Cc: aliguori@us.ibm.com, qemu-devel@nongnu.org,
	David Gibson <dwg@au1.ibm.com>
Subject: Re: [Qemu-devel] [PATCH 6/8] pci: Simpler implementation of primary PCI bus
Date: Wed, 29 May 2013 15:22:29 +0300	[thread overview]
Message-ID: <20130529122229.GS4472@redhat.com> (raw)
In-Reply-To: <20130529110400.GK5489@boomeroo.fritz.box>

On Wed, May 29, 2013 at 09:04:00PM +1000, David Gibson wrote:
> 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 <david@gibson.dropbear.id.au>
> > > > > > > 
> > > > > > > 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.

What other possible reason for it to fail?

> > > 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.

Absolutely. I'm not thinking of it as fixed.
I'm thinking of the *default* number of pci host bridges
as fixed. If a user is smart enough to use -device to create
a host bridge, said user can learn about -device for creating
a nic.

> In qemu we create one always as a convenience, but
> users can (and will have to, for vfio) create additional ones
> trivially with -device.

So they know about -device then.

> [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.]

I guess the problem is not what they expect generally,
but specifically that some users might rely on spapr with
-nodefaults having PCI?

I don't have any ideas besides introducing a new machine type
that is same as spapr but without the default PCI host bridge.


> -- 
> 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

  reply	other threads:[~2013-05-29 12:22 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-09  0:31 [Qemu-devel] [0/8] Clean up PCI code to allow for multiple root buses David Gibson
2013-05-09  0:31 ` [Qemu-devel] [PATCH 1/8] pci: Cleanup configuration for pci-hotplug.c David Gibson
2013-05-23 11:11   ` Michael S. Tsirkin
2013-05-23 12:23     ` David Gibson
2013-05-24  7:44     ` David Gibson
2013-05-23 14:54   ` Paolo Bonzini
2013-05-24  7:46     ` David Gibson
2013-05-09  0:31 ` [Qemu-devel] [PATCH 2/8] pci: Move pci_read_devaddr to pci-hotplug-old.c David Gibson
2013-05-09  0:31 ` [Qemu-devel] [PATCH 3/8] pci: Abolish pci_find_root_bus() David Gibson
2013-05-23 11:26   ` Michael S. Tsirkin
2013-05-09  0:31 ` [Qemu-devel] [PATCH 4/8] pci: Use helper o find device's root bus in pci_find_domain() David Gibson
2013-05-09  0:31 ` [Qemu-devel] [PATCH 5/8] pci: Replace pci_find_domain() with more general pci_root_bus_path() David Gibson
2013-05-23 11:04   ` Michael S. Tsirkin
2013-05-23 12:21     ` David Gibson
2013-05-23 14:51     ` Paolo Bonzini
2013-05-23 14:57       ` Michael S. Tsirkin
2013-05-23 15:00         ` Paolo Bonzini
2013-05-23 15:06           ` Michael S. Tsirkin
2013-05-24  7:40           ` David Gibson
2013-05-09  0:31 ` [Qemu-devel] [PATCH 6/8] pci: Simpler implementation of primary PCI bus David Gibson
2013-05-23 11:01   ` Michael S. Tsirkin
2013-05-23 12:16     ` David Gibson
2013-05-23 14:39       ` Michael S. Tsirkin
2013-05-23 11:22   ` Michael S. Tsirkin
2013-05-23 12:16     ` David Gibson
2013-05-29  9:43       ` David Gibson
2013-05-29  9:47         ` David Gibson
2013-05-29  9:55         ` Michael S. Tsirkin
2013-05-29 10:06           ` David Gibson
2013-05-29 10:17             ` Michael S. Tsirkin
2013-05-29 11:04               ` David Gibson
2013-05-29 12:22                 ` Michael S. Tsirkin [this message]
2013-05-30  3:34                   ` David Gibson
2013-05-30  5:02                     ` Michael S. Tsirkin
2013-06-06  7:39                       ` David Gibson
2013-06-06  8:18                         ` Michael S. Tsirkin
2013-05-09  0:31 ` [Qemu-devel] [PATCH 7/8] pci: Remove domain from PCIHostBus David Gibson
2013-05-09  0:31 ` [Qemu-devel] [PATCH 8/8] pci: Fold host_buses list into PCIHostState functionality David Gibson
2013-05-14 10:53 ` [Qemu-devel] [0/8] Clean up PCI code to allow for multiple root buses Michael S. Tsirkin
2013-05-23 11:12 ` Michael S. Tsirkin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130529122229.GS4472@redhat.com \
    --to=mst@redhat.com \
    --cc=aliguori@us.ibm.com \
    --cc=david@gibson.dropbear.id.au \
    --cc=dwg@au1.ibm.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.