From patchwork Mon Nov 24 18:44:45 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefano Stabellini X-Patchwork-Id: 414063 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 573EC140145 for ; Tue, 25 Nov 2014 05:45:26 +1100 (AEDT) Received: from localhost ([::1]:53980 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XsydY-0003s4-0z for incoming@patchwork.ozlabs.org; Mon, 24 Nov 2014 13:45:24 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58577) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XsydA-0003Vs-FW for qemu-devel@nongnu.org; Mon, 24 Nov 2014 13:45:06 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Xsyd4-0007M5-8W for qemu-devel@nongnu.org; Mon, 24 Nov 2014 13:45:00 -0500 Received: from smtp02.citrix.com ([66.165.176.63]:39035) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xsyd4-0007Lt-1E for qemu-devel@nongnu.org; Mon, 24 Nov 2014 13:44:54 -0500 X-IronPort-AV: E=Sophos;i="5.07,450,1413244800"; d="scan'208";a="196214454" Received: from ukmail1.uk.xensource.com (10.80.16.128) by smtprelay.citrix.com (10.13.107.80) with Microsoft SMTP Server id 14.3.181.6; Mon, 24 Nov 2014 13:44:49 -0500 Received: from kaball.uk.xensource.com ([10.80.2.59]) by ukmail1.uk.xensource.com with esmtp (Exim 4.69) (envelope-from ) id 1Xsycz-0005JL-2g; Mon, 24 Nov 2014 18:44:49 +0000 Date: Mon, 24 Nov 2014 18:44:45 +0000 From: Stefano Stabellini X-X-Sender: sstabellini@kaball.uk.xensource.com To: In-Reply-To: Message-ID: References: <547290D7.2020506@cn.fujitsu.com> <5472F1DA.4080508@m2r.biz> <5472F980.6030208@cn.fujitsu.com> User-Agent: Alpine 2.02 (DEB 1266 2009-07-14) MIME-Version: 1.0 X-DLP: MIA1 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 66.165.176.63 Cc: mst@redhat.com, Stefano Stabellini , xen devel , Fabio Fantoni , aliguori@amazon.com, anthony PERARD , Paolo Bonzini Subject: [Qemu-devel] virtio leaks cpu mappings, was: qemu crash with virtio on Xen domUs (backtrace included) X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org On Mon, 24 Nov 2014, Stefano Stabellini wrote: > On Mon, 24 Nov 2014, Stefano Stabellini wrote: > > CC'ing Paolo. > > > > > > Wen, > > thanks for the logs. > > > > I investigated a little bit and it seems to me that the bug occurs when > > QEMU tries to unmap only a portion of a memory region previously mapped. > > That doesn't work with xen-mapcache. > > > > See these logs for example: > > > > DEBUG address_space_map phys_addr=78ed8b44 vaddr=7fab50afbb68 len=0xa > > DEBUG address_space_unmap vaddr=7fab50afbb68 len=0x6 > > Sorry the logs don't quite match, it was supposed to be: > > DEBUG address_space_map phys_addr=78ed8b44 vaddr=7fab50afbb64 len=0xa > DEBUG address_space_unmap vaddr=7fab50afbb68 len=0x6 It looks like the problem is caused by iov_discard_front, called by virtio_net_handle_ctrl. By changing iov_base after the sg has already been mapped (cpu_physical_memory_map), it causes a leak in the mapping because the corresponding cpu_physical_memory_unmap will only unmap a portion of the original sg. On Xen the problem is worse because xen-mapcache aborts. diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 2ac6ce5..b2b5c2d 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -775,7 +775,7 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) struct iovec *iov; unsigned int iov_cnt; - while (virtqueue_pop(vq, &elem)) { + while (virtqueue_pop_nomap(vq, &elem)) { if (iov_size(elem.in_sg, elem.in_num) < sizeof(status) || iov_size(elem.out_sg, elem.out_num) < sizeof(ctrl)) { error_report("virtio-net ctrl missing headers"); @@ -784,8 +784,12 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) iov = elem.out_sg; iov_cnt = elem.out_num; - s = iov_to_buf(iov, iov_cnt, 0, &ctrl, sizeof(ctrl)); iov_discard_front(&iov, &iov_cnt, sizeof(ctrl)); + + virtqueue_map_sg(elem.in_sg, elem.in_addr, elem.in_num, 1); + virtqueue_map_sg(elem.out_sg, elem.out_addr, elem.out_num, 0); + + s = iov_to_buf(iov, iov_cnt, 0, &ctrl, sizeof(ctrl)); if (s != sizeof(ctrl)) { status = VIRTIO_NET_ERR; } else if (ctrl.class == VIRTIO_NET_CTRL_RX) { diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 3e4b70c..6a4bd3a 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -446,7 +446,7 @@ void virtqueue_map_sg(struct iovec *sg, hwaddr *addr, } } -int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem) +int virtqueue_pop_nomap(VirtQueue *vq, VirtQueueElement *elem) { unsigned int i, head, max; hwaddr desc_pa = vq->vring.desc; @@ -505,9 +505,6 @@ int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem) } } while ((i = virtqueue_next_desc(desc_pa, i, max)) != max); - /* Now map what we have collected */ - virtqueue_map_sg(elem->in_sg, elem->in_addr, elem->in_num, 1); - virtqueue_map_sg(elem->out_sg, elem->out_addr, elem->out_num, 0); elem->index = head; @@ -517,6 +514,16 @@ int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem) return elem->in_num + elem->out_num; } +int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem) +{ + int rc = virtqueue_pop_nomap(vq, elem); + if (rc > 0) { + virtqueue_map_sg(elem->in_sg, elem->in_addr, elem->in_num, 1); + virtqueue_map_sg(elem->out_sg, elem->out_addr, elem->out_num, 0); + } + return rc; +} + /* virtio device */ static void virtio_notify_vector(VirtIODevice *vdev, uint16_t vector) { diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index 3e54e90..40a3977 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -174,6 +174,7 @@ void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem, void virtqueue_map_sg(struct iovec *sg, hwaddr *addr, size_t num_sg, int is_write); int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem); +int virtqueue_pop_nomap(VirtQueue *vq, VirtQueueElement *elem); int virtqueue_avail_bytes(VirtQueue *vq, unsigned int in_bytes, unsigned int out_bytes); void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int *in_bytes,