qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Marcel Apfelbaum <marcel.a@redhat.com>
To: Gerd Hoffmann <kraxel@redhat.com>
Cc: qemu-devel@nongnu.org, "Michael S. Tsirkin" <mst@redhat.com>
Subject: Re: [Qemu-devel] [PATCH] pci: fix pci bridge fw path
Date: Mon, 09 Dec 2013 11:24:12 +0200	[thread overview]
Message-ID: <1386581052.10879.22.camel@localhost.localdomain> (raw)
In-Reply-To: <1386573270.8684.11.camel@nilsson.home.kraxel.org>

On Mon, 2013-12-09 at 08:14 +0100, Gerd Hoffmann wrote:
> On So, 2013-12-08 at 13:31 +0200, Michael S. Tsirkin wrote:
> > On Fri, Dec 06, 2013 at 12:24:40PM +0100, Gerd Hoffmann wrote:
> > > qemu uses "pci" as name for pci bridges in the firmware device path.
> > > seabios expects "pci-bridge".  Result is that bootorder is broken for
> > > devices behind pci bridges.
> > > 
> > > Some googling suggests that "pci-bridge" is the correct one.  At least
> > > PPC-based Apple machines are using this.  See question "How do I boot
> > > from a device attached to a PCI card" here:
> > > 	http://www.netbsd.org/ports/macppc/faq.html
> > > 
> > > So lets change qemu to use "pci-bridge" too.
> > > 
> > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > 
> > Sounds good but this needs to be enabled for
> > new machine types only - otherwise guest will read
> > corrupted data since bootindex is in FW CFG.
> 
> Come on, that is seriously over-engineering.
Does this change affects migration in any way?

> 
> First, the chance that you'll find setup in the field where this can
> actually happen in practice is very low as bootorder for devices behind
> pci bridges does not work at all without this patch.
> 
> Second, the race window is so tiny that nobody has ever seen that happen
> in testing.
> 
> > Alternatively, extend
> > 04920fc0faa4760f9c4fc0e73b992b768099be70
> > to work for all fw cfg file entries.
> > 
> > I think the whole boot index thing needs
> > to be reworked: we also have the long-standing feature request to allow
> > booting from a device added by hot add.
> 
> I'm not going to rewrite bootorder just to get a one-liner bugfix
> merged.
Why? You could rewrite Qemu in C++, then rewrite the 
bootorder (easier, because of C++) and finally get your bugfix in...
but again, your patch would look exactly the same :)

Thanks,
Marcel

> 
> cheers,
>   Gerd
> 
> 

  reply	other threads:[~2013-12-09  9:24 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-06 11:24 [Qemu-devel] [PATCH] pci: fix pci bridge fw path Gerd Hoffmann
2013-12-08 11:31 ` Michael S. Tsirkin
2013-12-09  7:14   ` Gerd Hoffmann
2013-12-09  9:24     ` Marcel Apfelbaum [this message]
2013-12-09 10:00       ` Gerd Hoffmann
2013-12-09 10:22     ` Michael S. Tsirkin
2013-12-09 10:23 ` 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=1386581052.10879.22.camel@localhost.localdomain \
    --to=marcel.a@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=mst@redhat.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).