From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:39109) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UFmHZ-0006px-AK for qemu-devel@nongnu.org; Wed, 13 Mar 2013 10:04:00 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UFmHS-0005Cw-Iz for qemu-devel@nongnu.org; Wed, 13 Mar 2013 10:03:53 -0400 Received: from e23smtp07.au.ibm.com ([202.81.31.140]:55338) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UFmHS-0005CM-22 for qemu-devel@nongnu.org; Wed, 13 Mar 2013 10:03:46 -0400 Received: from /spool/local by e23smtp07.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 13 Mar 2013 23:55:38 +1000 Received: from d23relay04.au.ibm.com (d23relay04.au.ibm.com [9.190.234.120]) by d23dlp03.au.ibm.com (Postfix) with ESMTP id 1173C3578051 for ; Thu, 14 Mar 2013 01:03:39 +1100 (EST) Received: from d23av03.au.ibm.com (d23av03.au.ibm.com [9.190.234.97]) by d23relay04.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r2DDorgX6226338 for ; Thu, 14 Mar 2013 00:50:54 +1100 Received: from d23av03.au.ibm.com (loopback [127.0.0.1]) by d23av03.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r2DE3bRB022257 for ; Thu, 14 Mar 2013 01:03:37 +1100 From: Anthony Liguori In-Reply-To: <20130313112659.GE2309@dhcp-200-207.str.redhat.com> References: <1362491612-19226-1-git-send-email-aliguori@us.ibm.com> <1362491612-19226-2-git-send-email-aliguori@us.ibm.com> <20130313112659.GE2309@dhcp-200-207.str.redhat.com> Date: Wed, 13 Mar 2013 09:03:33 -0500 Message-ID: <87mwu7s8ga.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Subject: Re: [Qemu-devel] [RFC PATCH 1/8] qtest: add libqos List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: qemu-devel@nongnu.org, Stefan Hajnoczi Kevin Wolf writes: > Am 05.03.2013 um 14:53 hat Anthony Liguori geschrieben: >> This includes basic PCI support. >> >> Signed-off-by: Anthony Liguori > >> +static void *qpci_pc_iomap(QPCIBus *bus, QPCIDevice *dev, int barno) >> +{ >> + QPCIBusPC *s = container_of(bus, QPCIBusPC, bus); >> + static const int bar_reg_map[] = { >> + PCI_BASE_ADDRESS_0, PCI_BASE_ADDRESS_1, PCI_BASE_ADDRESS_2, >> + PCI_BASE_ADDRESS_3, PCI_BASE_ADDRESS_4, PCI_BASE_ADDRESS_5, >> + }; >> + int bar_reg; >> + uint32_t addr; >> + uint64_t size; >> + >> + g_assert(barno >= 0 && barno <= 5); >> + bar_reg = bar_reg_map[barno]; >> + >> + qpci_config_writel(dev, bar_reg, 0xFFFFFFFF); >> + addr = qpci_config_readl(dev, bar_reg); >> + >> + size = (1ULL << ctol(addr)); > > This doesn't look right. It should be something like: > > size = (1ULL << ctzl(addr & ~0x3)); > > In fact, what must be masked out differs for I/O (0x3) and memory > (0xf). You are correct, it should look something like: if (addr & PCI_BASE_ADDRESS_SPACE_IO) { size = (1ULL << ctzl(addr & PCI_BASE_ADDRESS_IO_MASK)); } else { size = (1ULL << ctzl(addr & PCI_BASE_ADDRESS_MEM_MASK)); } This doesn't deal with 64-bit bars though. I'll update in the next round. I think this has worked for me because I have only done single device testing. I did switch from ffz when I rebased but I think ffz would still have this problem. >> +QPCIDevice *qpci_device_find(QPCIBus *bus, int devfn) >> +{ >> + QPCIDevice *dev; >> + >> + dev = g_malloc0(sizeof(*dev)); > > Where is the matching free? I can't seem to destroy a device I > once queried. It's missing, I'll add it in the next round. >> + dev->bus = bus; >> + dev->devfn = devfn; >> + >> + if (qpci_config_readw(dev, PCI_VENDOR_ID) == 0xFFFF) { >> + printf("vendor id is %x\n", qpci_config_readw(dev, PCI_VENDOR_ID)); >> + g_free(dev); >> + return NULL; >> + } >> + >> + return dev; >> +} >> + >> +void qpci_device_enable(QPCIDevice *dev) >> +{ >> + uint16_t cmd; >> + >> + /* FIXME -- does this need to be a bus callout? */ >> + cmd = qpci_config_readw(dev, PCI_COMMAND); >> + cmd |= PCI_COMMAND_IO | PCI_COMMAND_MEMORY; >> + qpci_config_writew(dev, PCI_COMMAND, cmd); >> +} > > Wouldn't it make sense to enable bus mastering here as well? Forgetting > to do this manually is a trap that's easy to fall in... Indeed, thanks for pointing it out. Regards, Anthony Liguori > > Kevin