From patchwork Tue Dec 2 15:41:36 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Cornelia Huck X-Patchwork-Id: 5420821 Return-Path: X-Original-To: patchwork-kvm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 13C799F1C5 for ; Tue, 2 Dec 2014 15:41:56 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id E900620270 for ; Tue, 2 Dec 2014 15:41:52 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id A09672024F for ; Tue, 2 Dec 2014 15:41:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932366AbaLBPlt (ORCPT ); Tue, 2 Dec 2014 10:41:49 -0500 Received: from e06smtp14.uk.ibm.com ([195.75.94.110]:45071 "EHLO e06smtp14.uk.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932236AbaLBPlr (ORCPT ); Tue, 2 Dec 2014 10:41:47 -0500 Received: from /spool/local by e06smtp14.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 2 Dec 2014 15:41:45 -0000 Received: from d06dlp03.portsmouth.uk.ibm.com (9.149.20.15) by e06smtp14.uk.ibm.com (192.168.101.144) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Tue, 2 Dec 2014 15:41:42 -0000 Received: from b06cxnps3075.portsmouth.uk.ibm.com (d06relay10.portsmouth.uk.ibm.com [9.149.109.195]) by d06dlp03.portsmouth.uk.ibm.com (Postfix) with ESMTP id 48EAD1B08049 for ; Tue, 2 Dec 2014 15:42:00 +0000 (GMT) Received: from d06av08.portsmouth.uk.ibm.com (d06av08.portsmouth.uk.ibm.com [9.149.37.249]) by b06cxnps3075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id sB2FfgBY58982510 for ; Tue, 2 Dec 2014 15:41:42 GMT Received: from d06av08.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av08.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id sB2FffR2012987 for ; Tue, 2 Dec 2014 08:41:42 -0700 Received: from gondolin (dyn-9-152-224-210.boeblingen.de.ibm.com [9.152.224.210]) by d06av08.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id sB2FffFE012952; Tue, 2 Dec 2014 08:41:41 -0700 Date: Tue, 2 Dec 2014 16:41:36 +0100 From: Cornelia Huck To: "Michael S. Tsirkin" Cc: virtualization@lists.linux-foundation.org, qemu-devel@nongnu.org, kvm@vger.kernel.org, rusty@rustcorp.com.au, thuth@linux.vnet.ibm.com Subject: Re: [PATCH RFC v5 07/19] virtio: allow virtio-1 queue layout Message-ID: <20141202164136.1a57f358.cornelia.huck@de.ibm.com> In-Reply-To: <20141202155444.0731c48f.cornelia.huck@de.ibm.com> References: <1417525227-14051-1-git-send-email-cornelia.huck@de.ibm.com> <1417525227-14051-8-git-send-email-cornelia.huck@de.ibm.com> <20141202144628.GA2974@redhat.com> <20141202155444.0731c48f.cornelia.huck@de.ibm.com> Organization: IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz =?UTF-8?B?R2VzY2jDpGZ0c2bDvGhydW5nOg==?= Dirk Wittkopp Sitz der Gesellschaft: =?UTF-8?B?QsO2Ymxpbmdlbg==?= Registergericht: Amtsgericht Stuttgart, HRB 243294 X-Mailer: Claws Mail 3.8.0 (GTK+ 2.24.10; i686-pc-linux-gnu) Mime-Version: 1.0 X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 14120215-0017-0000-0000-0000020F4F54 Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Tue, 2 Dec 2014 15:54:44 +0100 Cornelia Huck wrote: > On Tue, 2 Dec 2014 16:46:28 +0200 > "Michael S. Tsirkin" wrote: > > pa == -1ULL tricks look quite ugly. > > Can't we set desc/avail/used unconditionally, and drop > > the pa value? > > And have virtio_queue_get_addr() return desc? Let me see if I can come > up with a patch. I came up with the following (untested) patch, which should hopefully suit mmio as well. I haven't cared about migration yet. --- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 8f69ffa..ac3c615 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -69,7 +69,6 @@ typedef struct VRing struct VirtQueue { VRing vring; - hwaddr pa; uint16_t last_avail_idx; /* Last used index value we have signalled on */ uint16_t signalled_used; @@ -92,12 +91,13 @@ struct VirtQueue }; /* virt queue functions */ -static void virtqueue_init(VirtQueue *vq) +static void virtqueue_update_rings(VirtQueue *vq) { - hwaddr pa = vq->pa; - - vq->vring.desc = pa; - vq->vring.avail = pa + vq->vring.num * sizeof(VRingDesc); + if (!vq->vring.desc) { + /* not yet setup -> nothing to do */ + return; + } + vq->vring.avail = vq->vring.desc + vq->vring.num * sizeof(VRingDesc); vq->vring.used = vring_align(vq->vring.avail + offsetof(VRingAvail, ring[vq->vring.num]), vq->vring.align); @@ -605,7 +605,6 @@ void virtio_reset(void *opaque) vdev->vq[i].vring.avail = 0; vdev->vq[i].vring.used = 0; vdev->vq[i].last_avail_idx = 0; - vdev->vq[i].pa = 0; vdev->vq[i].vector = VIRTIO_NO_VECTOR; vdev->vq[i].signalled_used = 0; vdev->vq[i].signalled_used_valid = false; @@ -708,17 +707,34 @@ void virtio_config_writel(VirtIODevice *vdev, uint32_t addr, uint32_t data) void virtio_queue_set_addr(VirtIODevice *vdev, int n, hwaddr addr) { - vdev->vq[n].pa = addr; - virtqueue_init(&vdev->vq[n]); + vdev->vq[n].vring.desc = addr; + virtqueue_update_rings(&vdev->vq[n]); } hwaddr virtio_queue_get_addr(VirtIODevice *vdev, int n) { - return vdev->vq[n].pa; + return vdev->vq[n].vring.desc; +} + +void virtio_queue_set_rings(VirtIODevice *vdev, int n, hwaddr desc, + hwaddr avail, hwaddr used) +{ + vdev->vq[n].vring.desc = desc; + vdev->vq[n].vring.avail = avail; + vdev->vq[n].vring.used = used; } void virtio_queue_set_num(VirtIODevice *vdev, int n, int num) { + /* + * For virtio-1 devices, the number of buffers may only be + * updated if the ring addresses have not yet been set up. + */ + if (virtio_has_feature(vdev, VIRTIO_F_VERSION_1) && + vdev->vq[n].vring.desc) { + error_report("tried to modify buffer num for virtio-1 device"); + return; + } /* Don't allow guest to flip queue between existent and * nonexistent states, or to set it to an invalid size. */ @@ -728,7 +744,7 @@ void virtio_queue_set_num(VirtIODevice *vdev, int n, int num) return; } vdev->vq[n].vring.num = num; - virtqueue_init(&vdev->vq[n]); + virtqueue_update_rings(&vdev->vq[n]); } int virtio_queue_get_num(VirtIODevice *vdev, int n) @@ -748,6 +764,11 @@ void virtio_queue_set_align(VirtIODevice *vdev, int n, int align) BusState *qbus = qdev_get_parent_bus(DEVICE(vdev)); VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); + /* virtio-1 compliant devices cannot change the aligment */ + if (virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) { + error_report("tried to modify queue alignment for virtio-1 device"); + return; + } /* Check that the transport told us it was going to do this * (so a buggy transport will immediately assert rather than * silently failing to migrate this state) @@ -755,7 +776,7 @@ void virtio_queue_set_align(VirtIODevice *vdev, int n, int align) assert(k->has_variable_vring_alignment); vdev->vq[n].vring.align = align; - virtqueue_init(&vdev->vq[n]); + virtqueue_update_rings(&vdev->vq[n]); } void virtio_queue_notify_vq(VirtQueue *vq) @@ -949,7 +970,8 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f) if (k->has_variable_vring_alignment) { qemu_put_be32(f, vdev->vq[i].vring.align); } - qemu_put_be64(f, vdev->vq[i].pa); + /* XXX virtio-1 devices */ + qemu_put_be64(f, vdev->vq[i].vring.desc); qemu_put_be16s(f, &vdev->vq[i].last_avail_idx); if (k->save_queue) { k->save_queue(qbus->parent, i, f); @@ -1044,13 +1066,14 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id) if (k->has_variable_vring_alignment) { vdev->vq[i].vring.align = qemu_get_be32(f); } - vdev->vq[i].pa = qemu_get_be64(f); + vdev->vq[i].vring.desc = qemu_get_be64(f); qemu_get_be16s(f, &vdev->vq[i].last_avail_idx); vdev->vq[i].signalled_used_valid = false; vdev->vq[i].notification = true; - if (vdev->vq[i].pa) { - virtqueue_init(&vdev->vq[i]); + if (vdev->vq[i].vring.desc) { + /* XXX virtio-1 devices */ + virtqueue_update_rings(&vdev->vq[i]); } else if (vdev->vq[i].last_avail_idx) { error_report("VQ %d address 0x0 " "inconsistent with Host index 0x%x", @@ -1084,7 +1107,7 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id) } for (i = 0; i < num; i++) { - if (vdev->vq[i].pa) { + if (vdev->vq[i].vring.desc) { uint16_t nheads; nheads = vring_avail_idx(&vdev->vq[i]) - vdev->vq[i].last_avail_idx; /* Check it isn't doing strange things with descriptor numbers. */ diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index 68c40db..80ee313 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -224,6 +224,8 @@ void virtio_queue_set_addr(VirtIODevice *vdev, int n, hwaddr addr); hwaddr virtio_queue_get_addr(VirtIODevice *vdev, int n); void virtio_queue_set_num(VirtIODevice *vdev, int n, int num); int virtio_queue_get_num(VirtIODevice *vdev, int n); +void virtio_queue_set_rings(VirtIODevice *vdev, int n, hwaddr desc, + hwaddr avail, hwaddr used); void virtio_queue_set_align(VirtIODevice *vdev, int n, int align); void virtio_queue_notify(VirtIODevice *vdev, int n); uint16_t virtio_queue_vector(VirtIODevice *vdev, int n);