From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42509) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aXnxi-0005Ez-7P for qemu-devel@nongnu.org; Mon, 22 Feb 2016 05:43:31 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aXnxe-0000zO-St for qemu-devel@nongnu.org; Mon, 22 Feb 2016 05:43:30 -0500 Received: from mx1.redhat.com ([209.132.183.28]:45065) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aXnxe-0000zE-Lj for qemu-devel@nongnu.org; Mon, 22 Feb 2016 05:43:26 -0500 References: <1450284917-10508-1-git-send-email-apyrgio@arrikto.com> <1450284917-10508-2-git-send-email-apyrgio@arrikto.com> <56BC6DC0.4070204@redhat.com> <56C70168.2050408@arrikto.com> From: Paolo Bonzini Message-ID: <56CAE64B.4070608@redhat.com> Date: Mon, 22 Feb 2016 11:43:23 +0100 MIME-Version: 1.0 In-Reply-To: <56C70168.2050408@arrikto.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 1/9] dma-helpers: Expose the sg mapping logic List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Pyrgiotis Cc: qemu-devel@nongnu.org On 19/02/2016 12:50, Alex Pyrgiotis wrote: > QEMU/Hardware space: > 5. The SCSI controller code will create a QEMUSGList that points to > the memory regions of the SCSI request. This QEMUSGList will also > include the MMIO regions. > 6. The QEMU device implementation, e.g. scsi-block, chooses to use > the dma_* interface. > 7. The dma_blk_read/write() code will ultimately attempt to map all the > memory regions pointed by the QEMUSGList in order to create a > QEMUIOVector. > 8. At some point during the mapping loop, the code will encounter an > MMIO region. Since reading and writing from/to an MMIO region > requires special handling, e.g., we need to call > MemoryRegion->ops->write(), we cannot include it in our read/write > system call to the host kernel. > 9. This leads to a partial read/write and the mapping loop will resume > once the partial read/write() has finished. >=20 > Are we in the same page so far? Yes. > Are the above OK? If so, I have some questions: >=20 > a) Is an MMIO region one of the reasons why we can't map an sg? Yes, the only one pretty much. > b) At which point will the relevant ops->write() method for the MMIO > region be called when we have to DMA into the region?? Is it handled > implicitly in dma_memory_map()? It's in address_space_unmap: if (is_write) { address_space_write(as, bounce.addr, MEMTXATTRS_UNSPECIFIED, bounce.buffer, access_len); } Likewise, address_space_map does the ops->read call through address_space_read. > c) I'm not quite sure about the logic of the "nothing mapped" section. > Correct me if I'm wrong, but what I think it does is that it > registers a callback (reschedule_dma) once some sort of mapping has > completed. What kind of mapping is this? Is there anything more to > it? Once something (presumably a concurrent user of dma-helpers.c) calls address_space_unmap to free the mapping (the bounce.buffer in the above address_space_write call), reschedule_dma is called. >> However, it is not possible to do the same for ioctls. This is actual= ly >> the reason why no one has ever tried to make scsi-generic do anything >> but bounce-buffering. I think that your code breaks horribly in this >> case, and I don't see a way to fix it, except for reverting to bounce >> buffering. >> >> This would require major changes in your patches, and I'm not sure >> whether they are worth it for the single use case of tape devices... >=20 > Well, I wouldn't narrow it down to tape devices. The scsi-generic > interface is the universal interface for all SCSI devices and the only > interface that is fully passthrough. Sure, but what's the advantage of a fully passthrough interface over scsi-block? > So this patch would effectively > boost the baseline performance of SCSI devices. I think it's worth a tr= y. I think the first step is understanding what to do about the weird "& ~BDRV_SECTOR_MASK" case, then. Paolo