From patchwork Tue Dec 8 18:07:48 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Michael S. Tsirkin" X-Patchwork-Id: 40667 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [199.232.76.165]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id DF5BAB7BE8 for ; Wed, 9 Dec 2009 06:02:40 +1100 (EST) Received: from localhost ([127.0.0.1]:56729 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NI5KW-0005M1-VF for incoming@patchwork.ozlabs.org; Tue, 08 Dec 2009 14:02:37 -0500 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NI4WG-0004R7-Mo for qemu-devel@nongnu.org; Tue, 08 Dec 2009 13:10:40 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NI4WC-0004LX-6R for qemu-devel@nongnu.org; Tue, 08 Dec 2009 13:10:40 -0500 Received: from [199.232.76.173] (port=52423 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NI4WB-0004L9-Or for qemu-devel@nongnu.org; Tue, 08 Dec 2009 13:10:35 -0500 Received: from mx1.redhat.com ([209.132.183.28]:17973) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NI4WB-00079c-Fl for qemu-devel@nongnu.org; Tue, 08 Dec 2009 13:10:35 -0500 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id nB8IAWnG006672 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 8 Dec 2009 13:10:32 -0500 Received: from redhat.com (vpn2-9-36.ams2.redhat.com [10.36.9.36]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with SMTP id nB8IAUGC026731; Tue, 8 Dec 2009 13:10:31 -0500 Date: Tue, 8 Dec 2009 20:07:48 +0200 From: "Michael S. Tsirkin" To: qemu-devel@nongnu.org, Anthony Liguori Message-ID: <20091208180748.GA32709@redhat.com> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.19 (2009-01-05) X-Scanned-By: MIMEDefang 2.67 on 10.5.11.11 X-detected-operating-system: by monty-python.gnu.org: Genre and OS details not recognized. Cc: Subject: [Qemu-devel] [PATCH] virtio: verify features on load X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org migrating between hosts which have different features might break silently, if the migration destination does not support some features supported by source. Prevent this from happening by comparing acked feature bits with the mask supported by the device. Signed-off-by: Michael S. Tsirkin --- Please consider the following for both master and 0.12. BTW: is there some reason why syborg does not support indirect buffers? I am guessing that this is just and oversight. We can prevent this from happening in future by moving the list of standard virtio flags to an inline function. Comments? hw/syborg_virtio.c | 12 ++++++++++-- hw/virtio-pci.c | 14 +++++++++++--- hw/virtio.c | 11 ++++++++++- hw/virtio.h | 6 ++++++ 4 files changed, 37 insertions(+), 6 deletions(-) diff --git a/hw/syborg_virtio.c b/hw/syborg_virtio.c index 6cf5a15..a84206a 100644 --- a/hw/syborg_virtio.c +++ b/hw/syborg_virtio.c @@ -87,7 +87,7 @@ static uint32_t syborg_virtio_readl(void *opaque, target_phys_addr_t offset) break; case SYBORG_VIRTIO_HOST_FEATURES: ret = vdev->get_features(vdev); - ret |= (1 << VIRTIO_F_NOTIFY_ON_EMPTY); + ret |= vdev->binding->get_features(s); break; case SYBORG_VIRTIO_GUEST_FEATURES: ret = vdev->features; @@ -242,8 +242,16 @@ static void syborg_virtio_update_irq(void *opaque, uint16_t vector) qemu_set_irq(proxy->irq, level != 0); } +static unsigned syborg_virtio_get_features(void *opaque) +{ + unsigned ret = 0; + ret |= (1 << VIRTIO_F_NOTIFY_ON_EMPTY); + return ret; +} + static VirtIOBindings syborg_virtio_bindings = { - .notify = syborg_virtio_update_irq + .notify = syborg_virtio_update_irq, + .get_features = syborg_virtio_get_features, }; static int syborg_virtio_init(SyborgVirtIOProxy *proxy, VirtIODevice *vdev) diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c index d222ce0..4500130 100644 --- a/hw/virtio-pci.c +++ b/hw/virtio-pci.c @@ -236,9 +236,7 @@ static uint32_t virtio_ioport_read(VirtIOPCIProxy *proxy, uint32_t addr) switch (addr) { case VIRTIO_PCI_HOST_FEATURES: ret = vdev->get_features(vdev); - ret |= (1 << VIRTIO_F_NOTIFY_ON_EMPTY); - ret |= (1 << VIRTIO_RING_F_INDIRECT_DESC); - ret |= (1 << VIRTIO_F_BAD_FEATURE); + ret |= vdev->binding->get_features(proxy); break; case VIRTIO_PCI_GUEST_FEATURES: ret = vdev->features; @@ -382,12 +380,22 @@ static void virtio_write_config(PCIDevice *pci_dev, uint32_t address, msix_write_config(pci_dev, address, val, len); } +static unsigned virtio_pci_get_features(void *opaque) +{ + unsigned ret = 0; + ret |= (1 << VIRTIO_F_NOTIFY_ON_EMPTY); + ret |= (1 << VIRTIO_RING_F_INDIRECT_DESC); + ret |= (1 << VIRTIO_F_BAD_FEATURE); + return ret; +} + static const VirtIOBindings virtio_pci_bindings = { .notify = virtio_pci_notify, .save_config = virtio_pci_save_config, .load_config = virtio_pci_load_config, .save_queue = virtio_pci_save_queue, .load_queue = virtio_pci_load_queue, + .get_features = virtio_pci_get_features, }; static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev, diff --git a/hw/virtio.c b/hw/virtio.c index bbd7b51..bb78ca8 100644 --- a/hw/virtio.c +++ b/hw/virtio.c @@ -649,6 +649,9 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f) int virtio_load(VirtIODevice *vdev, QEMUFile *f) { int num, i, ret; + uint32_t features; + uint32_t supported_features = vdev->get_features(vdev) | + vdev->binding->get_features(vdev->binding_opaque); if (vdev->binding->load_config) { ret = vdev->binding->load_config(vdev->binding_opaque, f); @@ -659,7 +662,13 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f) qemu_get_8s(f, &vdev->status); qemu_get_8s(f, &vdev->isr); qemu_get_be16s(f, &vdev->queue_sel); - qemu_get_be32s(f, &vdev->features); + qemu_get_be32s(f, &features); + if (features & ~supported_features) { + fprintf(stderr, "Features 0x%x unsupported. Allowed features: 0x%x\n", + features, supported_features); + return -1; + } + vdev->features = features; vdev->config_len = qemu_get_be32(f); qemu_get_buffer(f, vdev->config, vdev->config_len); diff --git a/hw/virtio.h b/hw/virtio.h index 15ad910..35532a6 100644 --- a/hw/virtio.h +++ b/hw/virtio.h @@ -31,6 +31,11 @@ /* We've given up on this device. */ #define VIRTIO_CONFIG_S_FAILED 0x80 +/* Some virtio feature bits (currently bits 28 through 31) are reserved for the + * transport being used (eg. virtio_ring), the rest are per-device feature bits. */ +#define VIRTIO_TRANSPORT_F_START 28 +#define VIRTIO_TRANSPORT_F_END 32 + /* We notify when the ring is completely used, even if the guest is suppressing * callbacks */ #define VIRTIO_F_NOTIFY_ON_EMPTY 24 @@ -82,6 +87,7 @@ typedef struct { void (*save_queue)(void * opaque, int n, QEMUFile *f); int (*load_config)(void * opaque, QEMUFile *f); int (*load_queue)(void * opaque, int n, QEMUFile *f); + unsigned (*get_features)(void * opaque); } VirtIOBindings; #define VIRTIO_PCI_QUEUE_MAX 16