From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:40797)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <kraxel@redhat.com>) id 1WX8uQ-0000Qj-C4
	for qemu-devel@nongnu.org; Mon, 07 Apr 2014 08:44:24 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <kraxel@redhat.com>) id 1WX8uK-0008Rl-1M
	for qemu-devel@nongnu.org; Mon, 07 Apr 2014 08:44:18 -0400
Received: from mx1.redhat.com ([209.132.183.28]:50050)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <kraxel@redhat.com>) id 1WX8uJ-0008RS-Pr
	for qemu-devel@nongnu.org; Mon, 07 Apr 2014 08:44:11 -0400
Message-ID: <1396874646.5001.76.camel@nilsson.home.kraxel.org>
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Mon, 07 Apr 2014 14:44:06 +0200
In-Reply-To: <20140407121508.GD16369@redhat.com>
References: <1396868342-12005-1-git-send-email-marcel.a@redhat.com>
	<20140407121508.GD16369@redhat.com>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [SeaBIOS] [PATCH] hw/pci: reserve IO and mem for
 pci-2-pci bridges with no devices attached
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: kevin@koconnor.net, seabios@seabios.org, qemu-devel@nongnu.org, Marcel Apfelbaum <marcel.a@redhat.com>

  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.

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.

For express ports which can only have a single device underneath we can
check whenever we have a device and if one is present already don't
bother claiming extra resources for hotplug.

> > +    for (cap = pci_config_readb(pci->bdf, PCI_CAPABILITY_LIST); cap;
> > +                cap = pci_config_readb(pci->bdf, cap + PCI_CAP_LIST_NEXT))
> > +        if (pci_config_readb(pci->bdf, cap + PCI_CAP_LIST_ID) == cap_id)
> > +            return cap;
> 
> I would also limit this to 256 iterations, to make sure
> we dont' get into an infinite loop with a broken device.

Good point.

cheers,
  Gerd