From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1N8Zmd-0007c2-3i for qemu-devel@nongnu.org; Thu, 12 Nov 2009 08:32:19 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1N8ZmX-0007St-E6 for qemu-devel@nongnu.org; Thu, 12 Nov 2009 08:32:18 -0500 Received: from [199.232.76.173] (port=54450 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1N8ZmX-0007SZ-5L for qemu-devel@nongnu.org; Thu, 12 Nov 2009 08:32:13 -0500 Received: from mx1.redhat.com ([209.132.183.28]:54671) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1N8ZmW-000096-M1 for qemu-devel@nongnu.org; Thu, 12 Nov 2009 08:32:12 -0500 Date: Thu, 12 Nov 2009 15:29:39 +0200 From: "Michael S. Tsirkin" Message-ID: <20091112132939.GA1551@redhat.com> References: <1258005528-25383-1-git-send-email-yamahata@valinux.co.jp> <1258005528-25383-17-git-send-email-yamahata@valinux.co.jp> <20091112120622.GB11106@redhat.com> <20091112131207.GF2240%yamahata@valinux.co.jp> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20091112131207.GF2240%yamahata@valinux.co.jp> Subject: [Qemu-devel] Re: [PATCH 16/20] pci: kill goto in pci_update_mappings() List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Isaku Yamahata Cc: qemu-devel@nongnu.org On Thu, Nov 12, 2009 at 10:12:07PM +0900, Isaku Yamahata wrote: > On Thu, Nov 12, 2009 at 02:06:22PM +0200, Michael S. Tsirkin wrote: > > On Thu, Nov 12, 2009 at 02:58:44PM +0900, Isaku Yamahata wrote: > > > This patch kills nasty goto in pci_update_mappings(). > > > > > > Signed-off-by: Isaku Yamahata > > > --- > > > hw/pci.c | 54 ++++++++++++++++++++++++++++-------------------------- > > > 1 files changed, 28 insertions(+), 26 deletions(-) > > > > > > diff --git a/hw/pci.c b/hw/pci.c > > > index cae3d53..2eff7fe 100644 > > > --- a/hw/pci.c > > > +++ b/hw/pci.c > > > @@ -756,35 +756,37 @@ static void pci_update_mappings(PCIDevice *d) > > > new_addr = pci_get_long(d->config + pci_bar(d, i)); > > > } > > > /* the ROM slot has a specific enable bit */ > > > - if (i == PCI_ROM_SLOT && !(new_addr & PCI_ROM_ADDRESS_ENABLE)) > > > - goto no_mem_map; > > > - new_addr = new_addr & ~(r->size - 1); > > > - last_addr = new_addr + r->size - 1; > > > - /* NOTE: we do not support wrapping */ > > > - /* XXX: as we cannot support really dynamic > > > - mappings, we handle specific values as invalid > > > - mappings. */ > > > - if (last_addr <= new_addr || new_addr == 0 || > > > > By the way, any idea why do we need new_addr == 0 > > and last_addr == PCI_BAR_UNMAPPED? > > As for new_addr == 0, since default BAR value is zero, plural BARs can > overlap with each other. I think qemu can't handle BAR overlap anyway. > > As for last_addr == PCI_BAR_UNMAPPED, it avoid to map > around 4GB as discussed before. So it should be ~0x0ull and not PCI_BAR_UNMAPPED, PCI_BAR_UNMAPPED could be any value e.g. 0x1 would do as well. > I observed that guest doesn't boot > without the check. However I didn't track it down further. > Now it's 64bit though. I really think we need to move mapping regions out of devices and into pci.c. Then we can finally support overlapping BARs (by being careful not to map common regions). -- MST