From patchwork Fri Feb 15 10:32:38 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Daniel_P=2E_Berrang=C3=A9?= X-Patchwork-Id: 10814481 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id A93DA6C2 for ; Fri, 15 Feb 2019 10:36:33 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id AE10E2E73F for ; Fri, 15 Feb 2019 10:36:33 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id A20172EC10; Fri, 15 Feb 2019 10:36:33 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id EEC952E779 for ; Fri, 15 Feb 2019 10:36:32 +0000 (UTC) Received: from localhost ([127.0.0.1]:36271 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1guar9-0000uX-V7 for patchwork-qemu-devel@patchwork.kernel.org; Fri, 15 Feb 2019 05:36:32 -0500 Received: from eggs.gnu.org ([209.51.188.92]:60593) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1guann-00086E-TR for qemu-devel@nongnu.org; Fri, 15 Feb 2019 05:33:06 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1guanl-0004Ta-EK for qemu-devel@nongnu.org; Fri, 15 Feb 2019 05:33:03 -0500 Received: from mx1.redhat.com ([209.132.183.28]:55948) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1guanl-000440-0N for qemu-devel@nongnu.org; Fri, 15 Feb 2019 05:33:01 -0500 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id ABCB38666D; Fri, 15 Feb 2019 10:32:52 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-112-65.ams2.redhat.com [10.36.112.65]) by smtp.corp.redhat.com (Postfix) with ESMTP id DC9DB611B0; Fri, 15 Feb 2019 10:32:49 +0000 (UTC) From: =?utf-8?q?Daniel_P=2E_Berrang=C3=A9?= To: qemu-devel@nongnu.org Date: Fri, 15 Feb 2019 10:32:38 +0000 Message-Id: <20190215103239.28640-2-berrange@redhat.com> In-Reply-To: <20190215103239.28640-1-berrange@redhat.com> References: <20190215103239.28640-1-berrange@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Fri, 15 Feb 2019 10:32:52 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v2 1/2] hw: report invalid disable-legacy|modern usage for virtio-1-only devs X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Eduardo Habkost , "Michael S. Tsirkin" , Gonglei , Gerd Hoffmann , =?utf-8?q?Andreas_F=C3=A4rber?= Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP A number of virtio devices (gpu, crypto, mouse, keyboard, tablet) only support the virtio-1 (aka modern) mode. Currently if the user launches QEMU, setting those devices to enable legacy mode, QEMU will silently create them in modern mode, ignoring the user's (mistaken) request. This patch introduces proper data validation so that an attempt to configure a virtio-1-only devices in legacy mode gets reported as an error to the user. Checking this required introduction of a new field to explicitly track what operating model is to be used for a device, separately from the disable_modern and disable_legacy fields that record the user's requested configuration. Signed-off-by: Daniel P. Berrangé Reviewed-by: Eduardo Habkost --- hw/core/machine.c | 23 ++++++++++++++++++++--- hw/display/virtio-gpu-pci.c | 4 +++- hw/display/virtio-vga.c | 4 +++- hw/virtio/virtio-crypto-pci.c | 4 +++- hw/virtio/virtio-input-pci.c | 4 +++- hw/virtio/virtio-pci.c | 26 ++++++++++++++++---------- hw/virtio/virtio-pci.h | 31 +++++++++++++++++++++++++------ 7 files changed, 73 insertions(+), 23 deletions(-) diff --git a/hw/core/machine.c b/hw/core/machine.c index 077fbd182a..61fb791e6a 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -96,9 +96,26 @@ const size_t hw_compat_2_7_len = G_N_ELEMENTS(hw_compat_2_7); GlobalProperty hw_compat_2_6[] = { { "virtio-mmio", "format_transport_address", "off" }, - /* Optional because not all virtio-pci devices support legacy mode */ - { "virtio-pci", "disable-modern", "on", .optional = true }, - { "virtio-pci", "disable-legacy", "off", .optional = true }, + /* + * don't include devices which are modern-only + * ie keyboard, mouse, tablet, gpu, vga & crypto + */ + { "virtio-9p-pci", "disable-modern", "on" }, + { "virtio-9p-pci", "disable-legacy", "off" }, + { "virtio-balloon-pci", "disable-modern", "on" }, + { "virtio-balloon-pci", "disable-legacy", "off" }, + { "virtio-blk-pci", "disable-modern", "on" }, + { "virtio-blk-pci", "disable-legacy", "off" }, + { "virtio-input-host-pci", "disable-modern", "on" }, + { "virtio-input-host-pci", "disable-legacy", "off" }, + { "virtio-net-pci", "disable-modern", "on" }, + { "virtio-net-pci", "disable-legacy", "off" }, + { "virtio-rng-pci", "disable-modern", "on" }, + { "virtio-rng-pci", "disable-legacy", "off" }, + { "virtio-scsi-pci", "disable-modern", "on" }, + { "virtio-scsi-pci", "disable-legacy", "off" }, + { "virtio-serial-pci", "disable-modern", "on" }, + { "virtio-serial-pci", "disable-legacy", "off" }, }; const size_t hw_compat_2_6_len = G_N_ELEMENTS(hw_compat_2_6); diff --git a/hw/display/virtio-gpu-pci.c b/hw/display/virtio-gpu-pci.c index bdcd33c925..0bc4d9d424 100644 --- a/hw/display/virtio-gpu-pci.c +++ b/hw/display/virtio-gpu-pci.c @@ -47,7 +47,9 @@ static void virtio_gpu_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp) Error *local_error = NULL; qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus)); - virtio_pci_force_virtio_1(vpci_dev); + if (!virtio_pci_force_virtio_1(vpci_dev, errp)) { + return; + } object_property_set_bool(OBJECT(vdev), true, "realized", &local_error); if (local_error) { diff --git a/hw/display/virtio-vga.c b/hw/display/virtio-vga.c index 1e48009b74..d63fe8c345 100644 --- a/hw/display/virtio-vga.c +++ b/hw/display/virtio-vga.c @@ -145,7 +145,9 @@ static void virtio_vga_realize(VirtIOPCIProxy *vpci_dev, Error **errp) /* init virtio bits */ qdev_set_parent_bus(DEVICE(g), BUS(&vpci_dev->bus)); - virtio_pci_force_virtio_1(vpci_dev); + if (!virtio_pci_force_virtio_1(vpci_dev, errp)) { + return; + } object_property_set_bool(OBJECT(g), true, "realized", &err); if (err) { error_propagate(errp, err); diff --git a/hw/virtio/virtio-crypto-pci.c b/hw/virtio/virtio-crypto-pci.c index 90a6e0dc2e..13807e538b 100644 --- a/hw/virtio/virtio-crypto-pci.c +++ b/hw/virtio/virtio-crypto-pci.c @@ -51,7 +51,9 @@ static void virtio_crypto_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp) } qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus)); - virtio_pci_force_virtio_1(vpci_dev); + if (!virtio_pci_force_virtio_1(vpci_dev, errp)) { + return; + } object_property_set_bool(OBJECT(vdev), true, "realized", errp); object_property_set_link(OBJECT(vcrypto), OBJECT(vcrypto->vdev.conf.cryptodev), "cryptodev", diff --git a/hw/virtio/virtio-input-pci.c b/hw/virtio/virtio-input-pci.c index 2c1397842b..28477729a3 100644 --- a/hw/virtio/virtio-input-pci.c +++ b/hw/virtio/virtio-input-pci.c @@ -48,7 +48,9 @@ static void virtio_input_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp) DeviceState *vdev = DEVICE(&vinput->vdev); qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus)); - virtio_pci_force_virtio_1(vpci_dev); + if (!virtio_pci_force_virtio_1(vpci_dev, errp)) { + return; + } object_property_set_bool(OBJECT(vdev), true, "realized", errp); } diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index e978bfe760..ec31ec6cf3 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -1721,16 +1721,22 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error **errp) /* PCI BAR regions must be powers of 2 */ pow2ceil(proxy->notify.offset + proxy->notify.size)); - if (proxy->disable_legacy == ON_OFF_AUTO_AUTO) { - proxy->disable_legacy = pcie_port ? ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF; - } - - if (!virtio_pci_modern(proxy) && !virtio_pci_legacy(proxy)) { - error_setg(errp, "device cannot work as neither modern nor legacy mode" - " is enabled"); - error_append_hint(errp, "Set either disable-modern or disable-legacy" - " to off\n"); - return; + if ((proxy->disable_legacy == ON_OFF_AUTO_ON) || + ((proxy->disable_legacy == ON_OFF_AUTO_AUTO) && pcie_port)) { + if (proxy->disable_modern) { + error_setg(errp, "device cannot work as neither modern nor " + "legacy mode is enabled"); + error_append_hint(errp, "Set either disable-modern or " + "disable-legacy to off\n"); + return; + } + proxy->mode = VIRTIO_PCI_MODE_MODERN; + } else { + if (proxy->disable_modern) { + proxy->mode = VIRTIO_PCI_MODE_LEGACY; + } else { + proxy->mode = VIRTIO_PCI_MODE_TRANSITIONAL; + } } if (pcie_port && pci_is_express(pci_dev)) { diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h index bd223a6e3b..16ef4c0a3f 100644 --- a/hw/virtio/virtio-pci.h +++ b/hw/virtio/virtio-pci.h @@ -15,6 +15,7 @@ #ifndef QEMU_VIRTIO_PCI_H #define QEMU_VIRTIO_PCI_H +#include "qapi/error.h" #include "hw/pci/msi.h" #include "hw/virtio/virtio-bus.h" @@ -118,6 +119,12 @@ typedef struct VirtIOPCIQueue { uint32_t used[2]; } VirtIOPCIQueue; +typedef enum { + VIRTIO_PCI_MODE_LEGACY, + VIRTIO_PCI_MODE_TRANSITIONAL, + VIRTIO_PCI_MODE_MODERN, +} VirtIOPCIMode; + struct VirtIOPCIProxy { PCIDevice pci_dev; MemoryRegion bar; @@ -142,6 +149,7 @@ struct VirtIOPCIProxy { bool disable_modern; bool ignore_backend_features; OnOffAuto disable_legacy; + VirtIOPCIMode mode; uint32_t class_code; uint32_t nvectors; uint32_t dfselect; @@ -156,23 +164,34 @@ struct VirtIOPCIProxy { static inline bool virtio_pci_modern(VirtIOPCIProxy *proxy) { - return !proxy->disable_modern; + return proxy->mode != VIRTIO_PCI_MODE_LEGACY; } static inline bool virtio_pci_legacy(VirtIOPCIProxy *proxy) { - return proxy->disable_legacy == ON_OFF_AUTO_OFF; + return proxy->mode != VIRTIO_PCI_MODE_MODERN; } -static inline void virtio_pci_force_virtio_1(VirtIOPCIProxy *proxy) +static inline bool virtio_pci_force_virtio_1(VirtIOPCIProxy *proxy, + Error **errp) { - proxy->disable_modern = false; - proxy->disable_legacy = ON_OFF_AUTO_ON; + if (proxy->disable_legacy == ON_OFF_AUTO_OFF) { + error_setg(errp, "Unable to set disable-legacy=off on a virtio-1.0 " + "only device"); + return false; + } + if (proxy->disable_modern == true) { + error_setg(errp, "Unable to set disable-modern=on on a virtio-1.0 " + "only device"); + return false; + } + proxy->mode = VIRTIO_PCI_MODE_MODERN; + return true; } static inline void virtio_pci_disable_modern(VirtIOPCIProxy *proxy) { - proxy->disable_modern = true; + proxy->mode = VIRTIO_PCI_MODE_LEGACY; } /* From patchwork Fri Feb 15 10:32:39 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Daniel_P=2E_Berrang=C3=A9?= X-Patchwork-Id: 10814479 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 315221575 for ; Fri, 15 Feb 2019 10:34:12 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 37C69284B1 for ; Fri, 15 Feb 2019 10:34:12 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 2BE962851E; Fri, 15 Feb 2019 10:34:12 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id D048D284B1 for ; Fri, 15 Feb 2019 10:34:11 +0000 (UTC) Received: from localhost ([127.0.0.1]:36226 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1guaot-0000Iz-2R for patchwork-qemu-devel@patchwork.kernel.org; Fri, 15 Feb 2019 05:34:11 -0500 Received: from eggs.gnu.org ([209.51.188.92]:60551) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1guanm-000861-US for qemu-devel@nongnu.org; Fri, 15 Feb 2019 05:33:04 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1guanl-0004U8-IA for qemu-devel@nongnu.org; Fri, 15 Feb 2019 05:33:02 -0500 Received: from mx1.redhat.com ([209.132.183.28]:60723) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1guanl-0004CI-7A for qemu-devel@nongnu.org; Fri, 15 Feb 2019 05:33:01 -0500 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 15F65C0B2A2E; Fri, 15 Feb 2019 10:32:56 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-112-65.ams2.redhat.com [10.36.112.65]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1B0A5611B0; Fri, 15 Feb 2019 10:32:52 +0000 (UTC) From: =?utf-8?q?Daniel_P=2E_Berrang=C3=A9?= To: qemu-devel@nongnu.org Date: Fri, 15 Feb 2019 10:32:39 +0000 Message-Id: <20190215103239.28640-3-berrange@redhat.com> In-Reply-To: <20190215103239.28640-1-berrange@redhat.com> References: <20190215103239.28640-1-berrange@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Fri, 15 Feb 2019 10:32:56 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v2 2/2] Revert "globals: Allow global properties to be optional" X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Eduardo Habkost , "Michael S. Tsirkin" , Gonglei , Gerd Hoffmann , =?utf-8?q?Andreas_F=C3=A4rber?= Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP This reverts commit d7741743f4f3d2683d1bb6938f88dc0167c21afa. Relying on setting properties on parents types which may not be relevant to certain sub-classes had unexpected side-effects causing bugs in device config defaults. It is preferrable to be explicit about which devices get which properties, even if this needs repetition. Signed-off-by: Daniel P. Berrangé Reviewed-by: Eduardo Habkost --- include/hw/qdev-core.h | 3 --- qom/object.c | 3 --- 2 files changed, 6 deletions(-) diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index 0a84c42756..9614f76ae6 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -250,8 +250,6 @@ struct PropertyInfo { /** * GlobalProperty: * @used: Set to true if property was used when initializing a device. - * @optional: If set to true, GlobalProperty will be skipped without errors - * if the property doesn't exist. * * An error is fatal for non-hotplugged devices, when the global is applied. */ @@ -260,7 +258,6 @@ typedef struct GlobalProperty { const char *property; const char *value; bool used; - bool optional; } GlobalProperty; static inline void diff --git a/qom/object.c b/qom/object.c index b8c732063b..4e5226ca12 100644 --- a/qom/object.c +++ b/qom/object.c @@ -385,9 +385,6 @@ void object_apply_global_props(Object *obj, const GPtrArray *props, Error **errp if (object_dynamic_cast(obj, p->driver) == NULL) { continue; } - if (p->optional && !object_property_find(obj, p->property, NULL)) { - continue; - } p->used = true; object_property_parse(obj, p->value, p->property, &err); if (err != NULL) {