From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Mv6bZ-0002CA-UE for qemu-devel@nongnu.org; Tue, 06 Oct 2009 05:45:13 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Mv6bV-0002Ac-Ub for qemu-devel@nongnu.org; Tue, 06 Oct 2009 05:45:13 -0400 Received: from [199.232.76.173] (port=43460 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Mv6bV-0002AR-Ol for qemu-devel@nongnu.org; Tue, 06 Oct 2009 05:45:09 -0400 Received: from mx1.redhat.com ([209.132.183.28]:42421) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1Mv6bV-00010x-3M for qemu-devel@nongnu.org; Tue, 06 Oct 2009 05:45:09 -0400 Date: Tue, 6 Oct 2009 11:43:06 +0200 From: "Michael S. Tsirkin" Message-ID: <20091006094306.GA8899@redhat.com> References: <1254737223-16129-1-git-send-email-yamahata@valinux.co.jp> <1254737223-16129-13-git-send-email-yamahata@valinux.co.jp> <20091005120630.GB30799@redhat.com> <20091006093830.GB32367%yamahata@valinux.co.jp> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20091006093830.GB32367%yamahata@valinux.co.jp> Subject: [Qemu-devel] Re: [PATCH 12/23] pci: 64bit bar support. 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 Tue, Oct 06, 2009 at 06:38:30PM +0900, Isaku Yamahata wrote: > On Mon, Oct 05, 2009 at 02:06:30PM +0200, Michael S. Tsirkin wrote: > > On Mon, Oct 05, 2009 at 07:06:52PM +0900, Isaku Yamahata wrote: > > > implemented pci 64bit bar support. > > > > > > Signed-off-by: Isaku Yamahata > > > --- > > > hw/pci.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++-------- > > > hw/pci.h | 2 ++ > > > 2 files changed, 48 insertions(+), 8 deletions(-) > > > > > > diff --git a/hw/pci.c b/hw/pci.c > > > index 21565f5..09a6816 100644 > > > --- a/hw/pci.c > > > +++ b/hw/pci.c > > > @@ -395,6 +395,13 @@ int pci_unregister_device(PCIDevice *pci_dev) > > > return 0; > > > } > > > > > > +static inline int pci_bar_is_mem64(const PCIIORegion *r) > > > +{ > > > + return !(r->type & PCI_ADDRESS_SPACE_IO) && > > > + (r->type & PCI_ADDRESS_SPACE_MEM_TYPE_MASK) == > > > + PCI_ADDRESS_SPACE_MEM_TYPE_64; > > > +} > > > + > > > > why are we checking PCI_ADDRESS_SPACE_IO? Let's not. > > Because bar handling logic for io bar and 32bit memory bar is same. > For example, pci_register_bar() does. Yes, but 64 bit handling does not care about this: if you see a 64 bit bar, you can handle it as 64 bit. No need to check the I/O bit. > > > > void pci_register_bar(PCIDevice *pci_dev, int region_num, > > > pcibus_t size, int type, > > > PCIMapIORegionFunc *map_func) > > > @@ -427,8 +434,13 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num, > > > addr = 0x10 + region_num * 4; > > > } > > > pci_set_long(pci_dev->config + addr, type); > > > - pci_set_long(pci_dev->wmask + addr, wmask & 0xffffffff); > > > - pci_set_long(pci_dev->cmask + addr, 0xffffffff); > > > + if (pci_bar_is_mem64(r)) { > > > + pci_set_quad(pci_dev->wmask + addr, wmask); > > > + pci_set_quad(pci_dev->cmask + addr, ~0ULL); > > > + } else { > > > + pci_set_long(pci_dev->wmask + addr, wmask & 0xffffffff); > > > + pci_set_long(pci_dev->cmask + addr, 0xffffffff); > > > + } > > > } > > > > > > static void pci_update_mappings(PCIDevice *d) > > > @@ -462,7 +474,11 @@ static void pci_update_mappings(PCIDevice *d) > > > } > > > } else { > > > if (cmd & PCI_COMMAND_MEMORY) { > > > - new_addr = pci_get_long(d->config + config_ofs); > > > + if (pci_bar_is_mem64(r)) { > > > + new_addr = pci_get_quad(d->config + config_ofs); > > > + } else { > > > + new_addr = pci_get_long(d->config + config_ofs); > > > + } > > > /* the ROM slot has a specific enable bit */ > > > if (i == PCI_ROM_SLOT && !(new_addr & 1)) > > > goto no_mem_map; > > > @@ -473,11 +489,24 @@ static void pci_update_mappings(PCIDevice *d) > > > mappings, we handle specific values as invalid > > > mappings. */ > > > if (last_addr <= new_addr || new_addr == 0 || > > > - last_addr == PCI_BAR_UNMAPPED || > > > + last_addr == PCI_BAR_UNMAPPED) { > > > + new_addr = PCI_BAR_UNMAPPED; > > > + } > > > > > > - /* keep old behaviour > > > - * without this, PC ide doesn't work well. */ > > > - last_addr >= UINT32_MAX) { > > > + /* > > > + * work around: without this, PC ide and other devices > > > + * don't work well. > > > + * OS writes all 1 bits to 32 BAR to find its size > > > > Isn't memory disabled then? PCI spec says it should be ... > > > > > + * resulting in setting UINT32_MAX - alignemnt, > > > > typo > > > > > + * and then OS sets the BAR to where they really want > > > + * the BAR to sit. > > > + * On the other hand, there are some important areas > > > + * blow 4G on i386/x86_64. So setting BAR over those area > > > > typo > > > > > + * below 4G causes troubles. > > > + * We work around the issues by prohibitting BAR > > > > typo. > > > > > + * from sitting right blow 4G. > > > > typo > > > > > + */ > > > + if (!pci_bar_is_mem64(r) && last_addr >= UINT32_MAX) { > > > new_addr = PCI_BAR_UNMAPPED; > > > } > > > } else { > > > @@ -736,7 +765,16 @@ static void pci_info_device(PCIDevice *d) > > > monitor_printf(mon, "I/O at 0x%04"FMT_PCIBUS" [0x%04"FMT_PCIBUS"].\n", > > > r->addr, r->addr + r->size - 1); > > > } else { > > > - monitor_printf(mon, "32 bit memory at 0x%08"FMT_PCIBUS" [0x%08"FMT_PCIBUS"].\n", > > > + const char *type = pci_bar_is_mem64(r)? "64 bit": "32 bit"; > > > + const char *prefetch = ""; > > > + > > > + if (r->type & PCI_ADDRESS_SPACE_MEM_PREFETCH) { > > > + prefetch = " prefetchable"; > > > + } > > > + > > > + monitor_printf(mon, "%s%s memory at " > > > + "0x%08"FMT_PCIBUS" [0x%08"FMT_PCIBUS"].\n", > > > + type, prefetch, > > > r->addr, r->addr + r->size - 1); > > > } > > > } > > > diff --git a/hw/pci.h b/hw/pci.h > > > index cbf80c0..b65ce03 100644 > > > --- a/hw/pci.h > > > +++ b/hw/pci.h > > > @@ -84,6 +84,8 @@ typedef int PCIUnregisterFunc(PCIDevice *pci_dev); > > > > > > #define PCI_ADDRESS_SPACE_MEM 0x00 > > > #define PCI_ADDRESS_SPACE_IO 0x01 > > > +#define PCI_ADDRESS_SPACE_MEM_TYPE_MASK 0x06 > > > +#define PCI_ADDRESS_SPACE_MEM_TYPE_64 0x04 /* 64 bit address */ > > > > something wrong with text alignment here. pls align with tabs as the > > rest of surrounding code does. > > > > And I thought we were switching to names from pci_regs.h? > > > > > #define PCI_ADDRESS_SPACE_MEM_PREFETCH 0x08 > > > > > > typedef struct PCIIORegion { > > > > -- > yamahata