From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:35051) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1THADH-0002Vt-1M for qemu-devel@nongnu.org; Thu, 27 Sep 2012 05:17:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1THADB-0000D7-27 for qemu-devel@nongnu.org; Thu, 27 Sep 2012 05:16:54 -0400 Received: from mx1.redhat.com ([209.132.183.28]:46694) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1THADA-0000D1-Lq for qemu-devel@nongnu.org; Thu, 27 Sep 2012 05:16:48 -0400 Message-ID: <50641976.3020405@redhat.com> Date: Thu, 27 Sep 2012 11:16:38 +0200 From: Avi Kivity MIME-Version: 1.0 References: <50597D1F.3070607@redhat.com> <505991A2.6090709@siemens.com> <5059954A.50408@redhat.com> <50600F7B.5080106@redhat.com> <50602B0A.1020403@redhat.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [big lock] Discussion about the convention of device's DMA each other after breaking down biglock List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: liu ping fan Cc: Jan Kiszka , Marcelo Tosatti , "qemu-devel@nongnu.org" , Anthony Liguori , Paolo Bonzini On 09/27/2012 05:13 AM, liu ping fan wrote: > On Mon, Sep 24, 2012 at 5:42 PM, Avi Kivity wrote: >> On 09/24/2012 10:32 AM, liu ping fan wrote: >>> On Mon, Sep 24, 2012 at 3:44 PM, Avi Kivity wrote: >>>> On 09/24/2012 08:33 AM, liu ping fan wrote: >>>>> On Wed, Sep 19, 2012 at 5:50 PM, Avi Kivity wrote: >>>>> > On 09/19/2012 12:34 PM, Jan Kiszka wrote: >>>>> >> >>>>> >> What about the following: >>>>> >> >>>>> >> What we really need to support in practice is MMIO access trigge= rs RAM >>>>> >> access of device model. Scenarios where a device access triggers= another >>>>> >> MMIO access could likely just be rejected without causing troubl= es. >>>>> >> >>>>> >> So, when we dispatch a request to a device, we mark that the cur= rent >>>>> >> thread is in a MMIO dispatch and reject any follow-up c_p_m_rw t= hat does >>>>> >> _not_ target RAM, ie. is another, nested MMIO request - independ= ent of >>>>> >> its destination. How much of the known issues would this solve? = And what >>>>> >> would remain open? >>>>> > >>>>> > Various iommu-like devices re-dispatch I/O, like changing endiann= ess or >>>>> > bitband. I don't know whether it targets I/O rather than RAM. >>>>> > >>>>> Have not found the exact code. But I think the call chain may look >>>>> like this: dev mmio-handler --> c_p_m_rw() --> iommu mmio-handler -= -> >>>>> c_p_m_rw() >>>>> And I think you worry about the case for "c_p_m_rw() --> iommu >>>>> mmio-handler". Right? How about introduce an member can_nest for >>>>> MemoryRegionOps of iommu's mr? >>>>> >>>> >>>> I would rather push the iommu logic into the memory API: >>>> >>>> memory_region_init_iommu(MemoryRegion *mr, const char *name, >>>> MemoryRegion *target, MemoryRegionIOMMUOp= s *ops, >>>> unsigned size) >>>> >>>> struct MemoryRegionIOMMUOps { >>>> target_physical_addr_t (*translate)(target_physical_addr_t add= r, >>>> bool write); >>>> void (*fault)(target_physical_addr_t addr); >>>> }; >>>> >>> So I guess, after introduce this, the code logic in c_p_m_rw() will >>> look like this >>> >>> c_p_m_rw(dev_virt_addr, ...) >>> { >>> mr =3D phys_page_lookup(); >>> if (mr->iommu_ops) >>> real_addr =3D translate(dev_virt_addr,..)=EF=BC=9B >>> >>> ptr =3D qemu_get_ram_ptr(real_addr); >>> memcpy(buf, ptr, sz); >>> } >>> >> >> Something like that. It will be a while loop, to allow for iommus >> strung in series. >> > Will model the system like the following: >=20 > --.Introduce iommu address space. It will be the container of the > regions which are put under the management of iommu. > --.In the system address space, using alias-iommu-mrX with priority=3D1 > to expose iommu address space and obscure the overlapped regions. > -- Device's access to address manged by alias-iommu-mrX > c_p_m_rw(target_physical_addr_t addrA, ..) > { > while (len > 0) { > mr =3D phys_page_lookup(); > if (mr->iommu_ops) > addrB =3D translate(addrA,..)=EF=BC=9B >=20 > ptr =3D qemu_get_ram_ptr(addrB); > memcpy(buf, ptr, sz); > } > } >=20 > Is it correct? iommus only apply to device accesses, not cpu accesses (as in cpu_p_m_w()). So we will need a generic dma function: typedef struct MemoryAddressSpace { MemoryRegion *root; PhysPageEntry phys_map; ... // linked list entry for list of all MemoryAddressSpaces } void memory_address_space_rw(MemoryAddressSpace *mas, ...) { look up mas->phys_map dispatch } void cpu_physical_memory_rw(...) { memory_address_space_rw(&system_memory, ...); } The snippet if (mr->iommu_ops) addrB =3D translate(addrA,..)=EF=BC=9B needs to be a little more complicated. After translation, we need to look up the address again in a different phys_map. So a MemoryRegion that is an iommu needs to hold its own phys_map pointer for the lookup. But let's ignore the problem for now, we have too much on our plate. With a recursive big lock, there is no problem with iommus, yes? So as long as there is no intersection between converted devices and platforms with iommus, we're safe. --=20 error compiling committee.c: too many arguments to function