From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1M9KnQ-0007fX-IW for qemu-devel@nongnu.org; Wed, 27 May 2009 11:12:00 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1M9KnM-0007dY-T7 for qemu-devel@nongnu.org; Wed, 27 May 2009 11:12:00 -0400 Received: from [199.232.76.173] (port=49951 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1M9KnM-0007dT-Ot for qemu-devel@nongnu.org; Wed, 27 May 2009 11:11:56 -0400 Received: from e1.ny.us.ibm.com ([32.97.182.141]:42731) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1M9KnL-0003zX-NN for qemu-devel@nongnu.org; Wed, 27 May 2009 11:11:56 -0400 Received: from d01relay02.pok.ibm.com (d01relay02.pok.ibm.com [9.56.227.234]) by e1.ny.us.ibm.com (8.13.1/8.13.1) with ESMTP id n4RF7n7k019947 for ; Wed, 27 May 2009 11:07:49 -0400 Received: from d01av03.pok.ibm.com (d01av03.pok.ibm.com [9.56.224.217]) by d01relay02.pok.ibm.com (8.13.8/8.13.8/NCO v9.2) with ESMTP id n4RFBrfE231380 for ; Wed, 27 May 2009 11:11:53 -0400 Received: from d01av03.pok.ibm.com (loopback [127.0.0.1]) by d01av03.pok.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id n4RFBqLA028097 for ; Wed, 27 May 2009 11:11:52 -0400 Message-ID: <4A1D5837.3010705@us.ibm.com> Date: Wed, 27 May 2009 10:11:51 -0500 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 2/3] Add PCI memory region registration References: <1243157375-14329-1-git-send-email-avi@redhat.com> <1243157375-14329-3-git-send-email-avi@redhat.com> <4A1D4961.1010903@us.ibm.com> <4A1D5604.60003@redhat.com> In-Reply-To: <4A1D5604.60003@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Avi Kivity Cc: qemu-devel@nongnu.org Avi Kivity wrote: > Anthony Liguori wrote: >>> >>> +PCIIORegionComponent *pci_register_physical_memory(PCIDevice *pci_dev, >>> + int region_num, >>> + >>> target_phys_addr_t size, >>> + ram_addr_t >>> ram_addr); >>> >> >> PCI devices should not care about physical memory. They should only >> care about IO regions. It ought to look something like: >> >> enum { >> PCI_REGION_IO, >> PCI_REGION_MEM, >> }; >> >> typedef struct PCIIOFunction >> { >> uint32_t (*read)(void *opaque, uint64_t offset, int size); >> void (*write)(void *opaque, uint64_t offset, int size, uint64_t >> value); >> } PCIIOFunction; > > That doesn't work for direct-mapped memory like the cirrus framebuffer > or host devices. That's because it's an internal performance hack. We should just avoid the PCI routines for that device, if we can, although that suggests we need a map hook which is ugly. Clever ideas are welcome. The above API is needed to sanely deal with PCI busses that do either IOMMU translation or endianness conversion. Whether we use the existing IO memory functions verses a new set is not important to me. What I care most about is that we have a region vs. memory API, we don't have knowledge of where the region is mapped (so we can get rid of map), and that we provide a read/write indirection layer that is hookable by the PCI bus. Direct mapping is a very special case that can be handled differently. We shouldn't design the API around it. Regards, Anthony Liguori