Re: [Qemu-devel] Re: [PATCH] PCI: Bus number from the bridge, not the device

[Alex Williamson]

On Wed, 2010-12-15 at 11:56 +0200, Michael S. Tsirkin wrote:
> On Tue, Dec 14, 2010 at 11:34:53AM -0700, Alex Williamson wrote:
> > On Tue, 2010-12-14 at 14:26 +0200, Michael S. Tsirkin wrote:
> > > On Mon, Dec 13, 2010 at 10:04:24PM -0700, Alex Williamson wrote:
> > > > 
> > > > I've only ever seen config[PCI_SECONDARY_BUS] be non-zero for an
> > > > assigned device, so I'm pretty sure we're not going to hurt migration,
> > > > but the code is clearly wrong and I'd like to make sure we don't trip on
> > > > a migration failure for a minor device config space change.
> > > 
> > > Which reminds me: maybe just mark nested bridges as non-migrateable
> > > for now?  Care writing such a patch?
> > 
> > Hmm, this is trickier than it sounds.
> 
> Hmm, since 0 is put in the path instead of the bridge number,
> will the correct bridge be restored?
> 
> >  We're really only broken wrt
> > migration if a device under a bridge calls qemu_ram_alloc.
> 
> I guess there's more broken-ness. What exactly breaks qemu_ram_alloc?

You're right, it's more broken than that.  Anything that calls
get_dev_path is broken for migration of bridges since the path is
determined before the guest updates bus numbers.  That includes
qemu_ram_alloc and vmstate.  I was only looking at the qemu_ram_alloc
side.  So perhaps the right answer, for the moment, is to block
migration if there's a p2p bridge.

Alex

> >  Any device
> > is free to do this, but typically it only happens via
> > pci_add_option_rom() (not counting vga as typical).  So maybe the better
> > approach for now is to prevent the problem by disallowing option ROMs
> > for devices below a bridge.  We obviously risk devices coming along that
> > allocate RAM on their own, but we could still allow the most common
> > issue with almost no lost functionality (assuming no one wants to boot
> > off that nested device).  Thoughts?  Thanks,
> > 
> > Alex