From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60125) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WX8Rg-00007W-Bk for qemu-devel@nongnu.org; Mon, 07 Apr 2014 08:14:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WX8Ra-0005t6-4E for qemu-devel@nongnu.org; Mon, 07 Apr 2014 08:14:36 -0400 Received: from mx1.redhat.com ([209.132.183.28]:27199) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WX8RZ-0005sR-Sl for qemu-devel@nongnu.org; Mon, 07 Apr 2014 08:14:30 -0400 Date: Mon, 7 Apr 2014 15:15:08 +0300 From: "Michael S. Tsirkin" Message-ID: <20140407121508.GD16369@redhat.com> References: <1396868342-12005-1-git-send-email-marcel.a@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1396868342-12005-1-git-send-email-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 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Marcel Apfelbaum Cc: kevin@koconnor.net, seabios@seabios.org, qemu-devel@nongnu.org On Mon, Apr 07, 2014 at 01:59:02PM +0300, Marcel Apfelbaum wrote: > If a pci-2-pci bridge supports hot-plug functionality but there are no devices > connected to it, reserve IO/mem in order to be able to attach devices > later. Do not waste space, use minimum allowed. > > Signed-off-by: Marcel Apfelbaum > --- > src/fw/pciinit.c | 3 +++ > src/hw/pci.c | 17 +++++++++++++++++ > src/hw/pci.h | 1 + > 3 files changed, 21 insertions(+) > > diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c > index 64f1d41..9b5d7ad 100644 > --- a/src/fw/pciinit.c > +++ b/src/fw/pciinit.c > @@ -677,12 +677,15 @@ static int pci_bios_check_devices(struct pci_bus *busses) > continue; > struct pci_bus *parent = &busses[pci_bdf_to_bus(s->bus_dev->bdf)]; > int type; > + u8 shpc_cap = pci_find_capability(s->bus_dev, PCI_CAP_ID_SHPC); > for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) { > u64 align = (type == PCI_REGION_TYPE_IO) ? > PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN; > if (pci_region_align(&s->r[type]) > align) > align = pci_region_align(&s->r[type]); > u64 sum = pci_region_sum(&s->r[type]); > + if (!sum && shpc_cap) > + sum = align; /* reserve min size for hot-plug */ > u64 size = ALIGN(sum, align); > int is64 = pci_bios_bridge_region_is64(&s->r[type], > s->bus_dev, type); 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. > diff --git a/src/hw/pci.c b/src/hw/pci.c > index caf9265..ee8a7f1 100644 > --- a/src/hw/pci.c > +++ b/src/hw/pci.c > @@ -225,6 +225,23 @@ pci_find_init_device(const struct pci_device_id *ids, void *arg) > return NULL; > } > > +u8 pci_find_capability(struct pci_device *pci, u8 cap_id) > +{ > + u8 cap; > + u16 status = pci_config_readw(pci->bdf, PCI_STATUS); > + > + if (!(status & PCI_STATUS_CAP_LIST)) > + return 0; > + > + 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. > + > + return 0; > +} > + > + > void > pci_reboot(void) > { > diff --git a/src/hw/pci.h b/src/hw/pci.h > index 167a027..e828225 100644 > --- a/src/hw/pci.h > +++ b/src/hw/pci.h > @@ -116,6 +116,7 @@ int pci_init_device(const struct pci_device_id *ids > , struct pci_device *pci, void *arg); > struct pci_device *pci_find_init_device(const struct pci_device_id *ids > , void *arg); > +u8 pci_find_capability(struct pci_device *pci, u8 cap_id); > void pci_reboot(void); > > #endif > -- > 1.8.3.1