From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52818) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bWizA-0003FO-Do for qemu-devel@nongnu.org; Mon, 08 Aug 2016 07:44:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bWiz4-0008UB-Pu for qemu-devel@nongnu.org; Mon, 08 Aug 2016 07:44:47 -0400 Received: from mail-wm0-x242.google.com ([2a00:1450:400c:c09::242]:32924) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bWiz4-0008U6-FL for qemu-devel@nongnu.org; Mon, 08 Aug 2016 07:44:42 -0400 Received: by mail-wm0-x242.google.com with SMTP id o80so14924120wme.0 for ; Mon, 08 Aug 2016 04:44:42 -0700 (PDT) References: <1470146790-6168-1-git-send-email-paulinaszubarczyk@gmail.com> <1470146790-6168-3-git-send-email-paulinaszubarczyk@gmail.com> From: Paulina Szubarczyk Message-ID: <57A870A6.5070306@gmail.com> Date: Mon, 8 Aug 2016 13:44:38 +0200 MIME-Version: 1.0 In-Reply-To: <1470146790-6168-3-git-send-email-paulinaszubarczyk@gmail.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v4 2/2] qdisk - hw/block/xen_disk: grant copy implementation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: xen-devel@lists.xenproject.org Cc: roger.pau@citrix.com, wei.liu2@citrix.com, ian.jackson@eu.citrix.com, david.vrabel@citrix.com, sstabellini@kernel.org, anthony.perard@citrix.com, qemu-devel@nongnu.org On 08/02/2016 04:06 PM, Paulina Szubarczyk wrote: > Copy data operated on during request from/to local buffers to/from > the grant references. > > Before grant copy operation local buffers must be allocated what is > done by calling ioreq_init_copy_buffers. For the 'read' operation, > first, the qemu device invokes the read operation on local buffers > and on the completion grant copy is called and buffers are freed. > For the 'write' operation grant copy is performed before invoking > write by qemu device. > > A new value 'feature_grant_copy' is added to recognize when the > grant copy operation is supported by a guest. > > Signed-off-by: Paulina Szubarczyk > --- > Changes since v3: > - qemu_memalign/qemu_free is used instead function allocating > memory from xc. > - removed the get_buffer function instead there is a direct call > to qemu_memalign. > - moved ioreq_copy for write operation to ioreq_runio_qemu_aio. > - added struct xengnttab_grant_copy_segment_t and stub in > xen_common.h for version of xen earlier then 480. > - added checking for version 480 to configure. The test repeats > all the operation that are required for version < 480 and > checks if xengnttab_grant_copy() is implemented. > > * I did not change the way of testing if grant_copy operation is > implemented. As far as I understand if the code from > gnttab_unimp.c is used then the gnttab device is unavailable > and the handler to gntdev would be invalid. But if the handler > is valid then the ioctl should return operation unimplemented > if the gntdev does not implement the operation. > --- > configure | 56 +++++++++++++++++ > hw/block/xen_disk.c | 142 ++++++++++++++++++++++++++++++++++++++++++-- > include/hw/xen/xen_common.h | 25 ++++++++ > 3 files changed, 218 insertions(+), 5 deletions(-) > /* qemu block driver */ > DriveInfo *dinfo; > BlockBackend *blk; > @@ -489,6 +492,95 @@ static int ioreq_map(struct ioreq *ioreq) > return 0; > } > static void qemu_aio_complete(void *opaque, int ret) > @@ -511,8 +603,29 @@ static void qemu_aio_complete(void *opaque, int ret) > return; > } > > + if (ioreq->blkdev->feature_grant_copy) { > + switch (ioreq->req.operation) { > + case BLKIF_OP_READ: > + /* in case of failure ioreq->aio_errors is increased */ > + ioreq_copy(ioreq); I would add a condition to invoke the grant copy only if the ret argument with which the callback from BlockBackend 'qemu_aio_complete(void *opaque, int ret)' is called is equal to 0 to not unnecessary copy invalid data. > + free_buffers(ioreq); > + break; > + case BLKIF_OP_WRITE: > + case BLKIF_OP_FLUSH_DISKCACHE: > + if (!ioreq->req.nr_segments) { > + break; > + } > + free_buffers(ioreq); > + break; > + default: > + break; > + } > + } > + > ioreq->status = ioreq->aio_errors ? BLKIF_RSP_ERROR : BLKIF_RSP_OKAY; > - ioreq_unmap(ioreq); > + if (!ioreq->blkdev->feature_grant_copy) { > + ioreq_unmap(ioreq); > + } > ioreq_finish(ioreq); > switch (ioreq->req.operation) { > case BLKIF_OP_WRITE: Paulina