qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Gerd Hoffmann <kraxel@redhat.com>
Cc: kevin@koconnor.net, seabios@seabios.org, qemu-devel@nongnu.org,
	Marcel Apfelbaum <marcel.a@redhat.com>
Subject: Re: [Qemu-devel] [SeaBIOS] [PATCH] hw/pci: reserve IO and mem for pci-2-pci bridges with no devices attached
Date: Tue, 8 Apr 2014 11:54:54 +0300	[thread overview]
Message-ID: <20140408085454.GB4880@redhat.com> (raw)
In-Reply-To: <1396937123.22660.19.camel@nilsson.home.kraxel.org>

On Tue, Apr 08, 2014 at 08:05:23AM +0200, Gerd Hoffmann wrote:
> On Mo, 2014-04-07 at 16:34 +0300, Michael S. Tsirkin wrote:
> > On Mon, Apr 07, 2014 at 02:44:06PM +0200, Gerd Hoffmann wrote:
> > >   Hi,
> > > 
> > > > > +        u8 shpc_cap = pci_find_capability(s->bus_dev, PCI_CAP_ID_SHPC);
> > > 
> > > > One thing I'd do is maybe check that the relevant memory type is
> > > > enabled in the bridge (probably just by writing fff to base and reading
> > > > it back).
> > > 
> > > > This will give hypervisors an option to avoid wasting resources:
> > > > e.g. it's uncommon for express devices to claim IO.
> > > 
> > > I don't think we'll need that for the SHPC bridge.
> > 
> > Why not?
> 
> With typical use cases for the shpc bridge you likely need the io window
> anyway.

That won't be true after virtio 1.0 is out.

> > I'm referring to this text in the bridge specification:
> > 
> > 	The I/O Base and I/O Limit registers are optional and define an address
> > 	range that is used
> > 	by the bridge to determine when to forward I/O transactions from one
> > 	interface to the
> > 	other.
> > 	If a bridge does not implement an I/O address range, then both the I/O
> > 	Base and I/O
> > 	Limit registers must be implemented as read-only registers that return
> > 	zero when read. If a
> > 	bridge supports an I/O address range, then these registers must be
> > 	initialized by
> > 	configuration software so default states are not specified.
> > 
> > So we should probe bridge for I/O support before wasting I/O resources on it.
> 
> Yes, makes sense from a correctness point of view.

So that's all I'm arguing for, for now: let's implement the spec
correctly.
Whether we should emulate such devices is a separate question.

> I suspect you'll have a hard time to find such bridges in the x86 world
> though, so I'm not sure it is a good idea to emulate this in qemu.
> Guests might not handle it correctly.

We'll have to test this.
The handling is mostly done by BIOS so I don't expect a lot of problems.

> > > For express it indeed makes sense to avoid claiming IO address space.
> > > I'd try to find something more automatic though, where you don't need
> > > some kind of "disable io for this express port" config option.
> > 
> > Won't same trick as above work?
> 
> Yes, it will work.
> 
> But as we probably want support io on express devices (because it is
> used in practice, even though being strongly discouraged in the pci
> express specs).  So doing it that way would require a config switch on
> the qemu side to turn on/off io address space support for express
> switches/ports.
> 
> cheers,
>   Gerd

True but I don't see a way around this anyway.
At least this way we won't need PV.


-- 
MST

      reply	other threads:[~2014-04-08  8:54 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-07 10:59 [Qemu-devel] [SeaBIOS] [PATCH] hw/pci: reserve IO and mem for pci-2-pci bridges with no devices attached Marcel Apfelbaum
2014-04-07 12:01 ` Gerd Hoffmann
2014-04-07 12:11   ` Michael S. Tsirkin
2014-04-07 12:18     ` Marcel Apfelbaum
2014-04-07 12:15 ` Michael S. Tsirkin
2014-04-07 12:44   ` Gerd Hoffmann
2014-04-07 12:51     ` Marcel Apfelbaum
2014-04-07 13:34     ` Michael S. Tsirkin
2014-04-07 13:51       ` Marcel Apfelbaum
2014-04-07 13:55         ` Marcel Apfelbaum
2014-04-07 14:09         ` Michael S. Tsirkin
2014-04-07 14:16           ` Marcel Apfelbaum
2014-04-07 16:22             ` Michael S. Tsirkin
2014-04-08  6:05       ` Gerd Hoffmann
2014-04-08  8:54         ` Michael S. Tsirkin [this message]

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=20140408085454.GB4880@redhat.com \
    --to=mst@redhat.com \
    --cc=kevin@koconnor.net \
    --cc=kraxel@redhat.com \
    --cc=marcel.a@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=seabios@seabios.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).