From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43657) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aiTKf-0001Ej-Mb for qemu-devel@nongnu.org; Tue, 22 Mar 2016 16:55:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aiTKc-0004on-DT for qemu-devel@nongnu.org; Tue, 22 Mar 2016 16:55:17 -0400 Received: from mx1.redhat.com ([209.132.183.28]:55151) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aiTKa-0004nG-5Z for qemu-devel@nongnu.org; Tue, 22 Mar 2016 16:55:13 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (Postfix) with ESMTPS id 72512711D1 for ; Tue, 22 Mar 2016 20:55:11 +0000 (UTC) From: Bandan Das References: <20160321163441.29684cbf@t450s.home> <20160321183052.59e87b29@ul30vt.home> <20160321201600.7073b441@ul30vt.home> <20160322133124.54cbe3d2@t450s.home> Date: Tue, 22 Mar 2016 16:55:09 -0400 In-Reply-To: <20160322133124.54cbe3d2@t450s.home> (Alex Williamson's message of "Tue, 22 Mar 2016 13:31:24 -0600") Message-ID: MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH] vfio: add check for memory region overflow condition List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Williamson Cc: qemu-devel Alex Williamson writes: ... >> >> And it does. If we fix this assert, then vfio_dma_map() attempts mapping >> this direct mapped address range starting from 0 and prints a >> warning message; happens for the whole range and goes on for ever. >> The overflow check seemed to me like something we should fix, but now >> I am more confused then ever! > > Is the MemoryRegion memory_region_is_iommu() such that you're calling > vfio_dma_map() from vfio_iommu_map_notify()? If so then we should Yes, that is correct. This all started after we added the iommu mapping replay changes but I was wrong about the vfio_dma_map part. Please see below. > probably be using 128bit helpers for doing sanity checking and go ahead > and let something assert if we get to the vfio_dma_map() in > vfio_listener_region_add() with a 2^64 size. Then if you're taking the > memory_region_is_iommu() path, vfio_dma_map() is going to be called > with translations within that 2^64 bit address space, not mapping the > entire space, right? Thanks, The 128 bit operations make sense... The error message comes from: if (!memory_region_is_ram(mr)) { error_report("iommu map to non memory area %"HWADDR_PRIx"", xlat); goto out; } in vfio_iommu_map_notify() before we even get to vfio_dma_map(). This gets attempted for the entire range because dmar isn't enabled yet and vtd_iommu_translate() does this direct mapping in 4k increments in the translate path : ... if (!s->dmar_enabled) { /* DMAR disabled, passthrough, use 4k-page*/ ret.iova = addr & VTD_PAGE_MASK_4K; ret.translated_addr = addr & VTD_PAGE_MASK_4K; ret.addr_mask = ~VTD_PAGE_MASK_4K; ret.perm = IOMMU_RW; return ret; } I am not sure yet who actually uses it though. memory_region_iommu_replay() does the whole iteration if perm != IOMMU_NONE: void memory_region_iommu_replay(MemoryRegion *mr, Notifier *n, hwaddr granularity, bool is_write) { hwaddr addr; IOMMUTLBEntry iotlb; for (addr = 0; addr < memory_region_size(mr); addr += granularity) { iotlb = mr->iommu_ops->translate(mr, addr, is_write); if (iotlb.perm != IOMMU_NONE) { n->notify(n, &iotlb); } ... > Alex