From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50207) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aId8c-0000l1-BO for qemu-devel@nongnu.org; Mon, 11 Jan 2016 09:08:08 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aId8W-0008F8-9Q for qemu-devel@nongnu.org; Mon, 11 Jan 2016 09:08:02 -0500 Received: from mx1.redhat.com ([209.132.183.28]:60325) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aId8W-0008F4-4i for qemu-devel@nongnu.org; Mon, 11 Jan 2016 09:07:56 -0500 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (Postfix) with ESMTPS id DFD80A37AA for ; Mon, 11 Jan 2016 14:07:55 +0000 (UTC) Date: Mon, 11 Jan 2016 15:07:53 +0100 From: Igor Mammedov Message-ID: <20160111150753.324bd37b@nial.brq.redhat.com> In-Reply-To: <1452515063-5615-1-git-send-email-marcel@redhat.com> References: <1452515063-5615-1-git-send-email-marcel@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] hw/pci: do not update the PCI mappings while Decode (I/O or memory) bit is not set in the Command register List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Marcel Apfelbaum Cc: lersek@redhat.com, qemu-devel@nongnu.org, mst@redhat.com On Mon, 11 Jan 2016 14:24:23 +0200 Marcel Apfelbaum wrote: > Two reasons: > - PCI Spec indicates that while the bit is not set > the memory sizing is not finished. > - pci_bar_address will return PCI_BAR_UNMAPPED > and a previous value can be accidentally overridden > if the command register is modified (and not the BAR). > > Signed-off-by: Marcel Apfelbaum > --- > > Hi, > > I found this when trying to use multiple root complexes with OVMF. > > When trying to attach a device to the pxb-pcie device as Integrated > Device it did not receive the IO/MEM resources. > > The reason is that OVMF is working like that: > 1. It disables the Decode (I/O or memory) bit in the Command register > 2. It configures the device BARS > 3. Makes some tests on the Command register > 4. ... > 5. Enables the Decode (I/O or memory) at some point. > > On step 3 all the BARS are overridden to 0xffffffff by QEMU. > > Since QEMU uses the device BARs to compute the new host bridge resources > it now gets garbage. > > Laszlo, this also solves the SHPC problem for the pci-2-pci bridge inside the pxb. > Now we can enable the SHPC for it too. What about migration case? Shouldn't mappings be updated to match source even if bit isn't set? > > Thanks, > Marcel > > hw/pci/pci.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > index 168b9cc..f9127dc 100644 > --- a/hw/pci/pci.c > +++ b/hw/pci/pci.c > @@ -1148,6 +1148,7 @@ static void pci_update_mappings(PCIDevice *d) > PCIIORegion *r; > int i; > pcibus_t new_addr; > + uint16_t cmd = pci_get_word(d->config + PCI_COMMAND); > > for(i = 0; i < PCI_NUM_REGIONS; i++) { > r = &d->io_regions[i]; > @@ -1156,6 +1157,22 @@ static void pci_update_mappings(PCIDevice *d) > if (!r->size) > continue; > > + /* > + * Do not update the mappings until the command register's > + * Decode (I/O or memory) bit is not set. Two reasons: > + * - PCI Spec indicates that while the bit is not set > + * the memory sizing is not finished. > + * - pci_bar_address will return PCI_BAR_UNMAPPED > + * and a previous value can be accidentally overridden > + * if the command register is modified (and not the BAR). > + * */ > + if (((r->type & PCI_BASE_ADDRESS_SPACE_IO) && > + !(cmd & PCI_COMMAND_IO)) || > + ((r->type != PCI_BASE_ADDRESS_SPACE_IO) && > + !(cmd & PCI_COMMAND_MEMORY))) { > + continue; > + } > + > new_addr = pci_bar_address(d, i, r->type, r->size); > > /* This bar isn't changed */