From mboxrd@z Thu Jan 1 00:00:00 1970 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 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= 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 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: [Qemu-devel] [PATCH v2 1/2] hw: report invalid disable-legacy|modern usage for virtio-1-only devs List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: "Michael S. Tsirkin" , Gonglei , Marcel Apfelbaum , Eduardo Habkost , Gerd Hoffmann , =?UTF-8?q?Andreas=20F=C3=A4rber?= , =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= 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=C3=A9 --- 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 =3D G_N_ELEMENTS(hw_com= pat_2_7); =20 GlobalProperty hw_compat_2_6[] =3D { { "virtio-mmio", "format_transport_address", "off" }, - /* Optional because not all virtio-pci devices support legacy mode *= / - { "virtio-pci", "disable-modern", "on", .optional =3D true }, - { "virtio-pci", "disable-legacy", "off", .optional =3D 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 =3D G_N_ELEMENTS(hw_compat_2_6); =20 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 =3D NULL; =20 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_erro= r); =20 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_d= ev, Error **errp) =20 /* 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 *v= pci_dev, Error **errp) } =20 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 *vp= ci_dev, Error **errp) DeviceState *vdev =3D DEVICE(&vinput->vdev); =20 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); } =20 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.siz= e)); =20 - if (proxy->disable_legacy =3D=3D ON_OFF_AUTO_AUTO) { - proxy->disable_legacy =3D pcie_port ? ON_OFF_AUTO_ON : ON_OFF_AU= TO_OFF; - } - - if (!virtio_pci_modern(proxy) && !virtio_pci_legacy(proxy)) { - error_setg(errp, "device cannot work as neither modern nor legac= y mode" - " is enabled"); - error_append_hint(errp, "Set either disable-modern or disable-le= gacy" - " to off\n"); - return; + if ((proxy->disable_legacy =3D=3D ON_OFF_AUTO_ON) || + ((proxy->disable_legacy =3D=3D 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 =3D VIRTIO_PCI_MODE_MODERN; + } else { + if (proxy->disable_modern) { + proxy->mode =3D VIRTIO_PCI_MODE_LEGACY; + } else { + proxy->mode =3D VIRTIO_PCI_MODE_TRANSITIONAL; + } } =20 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 =20 +#include "qapi/error.h" #include "hw/pci/msi.h" #include "hw/virtio/virtio-bus.h" =20 @@ -118,6 +119,12 @@ typedef struct VirtIOPCIQueue { uint32_t used[2]; } VirtIOPCIQueue; =20 +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 { =20 static inline bool virtio_pci_modern(VirtIOPCIProxy *proxy) { - return !proxy->disable_modern; + return proxy->mode !=3D VIRTIO_PCI_MODE_LEGACY; } =20 static inline bool virtio_pci_legacy(VirtIOPCIProxy *proxy) { - return proxy->disable_legacy =3D=3D ON_OFF_AUTO_OFF; + return proxy->mode !=3D VIRTIO_PCI_MODE_MODERN; } =20 -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 =3D false; - proxy->disable_legacy =3D ON_OFF_AUTO_ON; + if (proxy->disable_legacy =3D=3D ON_OFF_AUTO_OFF) { + error_setg(errp, "Unable to set disable-legacy=3Doff on a virtio= -1.0 " + "only device"); + return false; + } + if (proxy->disable_modern =3D=3D true) { + error_setg(errp, "Unable to set disable-modern=3Don on a virtio-= 1.0 " + "only device"); + return false; + } + proxy->mode =3D VIRTIO_PCI_MODE_MODERN; + return true; } =20 static inline void virtio_pci_disable_modern(VirtIOPCIProxy *proxy) { - proxy->disable_modern =3D true; + proxy->mode =3D VIRTIO_PCI_MODE_LEGACY; } =20 /* --=20 2.20.1 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-9.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 425A4C04AAF for ; Thu, 16 May 2019 12:22:27 +0000 (UTC) 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.kernel.org (Postfix) with ESMTPS id 0422120848 for ; Thu, 16 May 2019 12:22:27 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0422120848 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([127.0.0.1]:53771 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hRFOv-0002g9-Cg for qemu-devel@archiver.kernel.org; Thu, 16 May 2019 08:22:21 -0400 Received: from eggs.gnu.org ([209.51.188.92]:48436) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hRFKa-0007nJ-8Y for qemu-devel@nongnu.org; Thu, 16 May 2019 08:17:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hRFKX-0000tM-7g for qemu-devel@nongnu.org; Thu, 16 May 2019 08:17:52 -0400 Received: from mail-qt1-f194.google.com ([209.85.160.194]:38712) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1hRFKX-0000sp-1Q for qemu-devel@nongnu.org; Thu, 16 May 2019 08:17:49 -0400 Received: by mail-qt1-f194.google.com with SMTP id d13so3560483qth.5 for ; Thu, 16 May 2019 05:17:49 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to; bh=Mwj59QAF+ETU+yRYVN8P1wjLxiufoyyiRXyVtu+O3hU=; b=D5o6RnuvCZlEbU0//CIrio5fZdO2Os/enu1/BiCFK7R2xE5+ZF6BdoY+MgStiaPlFV r2kc7oyivc/iTvafjT56T6v6MUd6fwkdW+iq4cdmEmZAkmaRzogzCECDOOzw1JqWJtPK YJQ1uGirQz03yRXnMV4qUvyBk3D+n4NDsAJ8LSeUojgea0dn8BAyb9wRZT+gVSvkbrWB 2ddwyQTS7KoK3c0j2Z+azcyO5Uxe9FdAi0P4YIUT9Yuclpv6gbZrDD3RUwa0yfgxXqeW RB+btI6NX8GOe8v8OQrkbAYYnFCsyaSbXQY4uCaJSNLkO9BxqWp/F+vnw+we0/fYnDAS i+Kg== X-Gm-Message-State: APjAAAWzHeG3qW0V/mgBpvu98nT2J6fuBFIH7zCvpssSbkZMu2x6u5BH 4QpsaJafEDskOa/+84xiHiUJdeFNqhQ= X-Google-Smtp-Source: APXvYqxlAwbxfneZwEj8zmYzxPGkBcum8jVBEl0sc9jBTZsqdD0hCJ9k8pUfyhrHG89E+g9d4SfO7A== X-Received: by 2002:a0c:c14d:: with SMTP id i13mr23632494qvh.244.1558009068065; Thu, 16 May 2019 05:17:48 -0700 (PDT) Received: from redhat.com ([185.54.206.10]) by smtp.gmail.com with ESMTPSA id y12sm2911083qtk.51.2019.05.16.05.17.45 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Thu, 16 May 2019 05:17:47 -0700 (PDT) Date: Thu, 16 May 2019 08:17:43 -0400 From: "Michael S. Tsirkin" To: qemu-devel@nongnu.org Message-ID: <20190215103239.28640-2-berrange@redhat.com> References: <20190515121146.7248-1-mst@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20190515121146.7248-1-mst@redhat.com> X-Mailer: git-send-email 2.17.1.1206.gb667731e2e.dirty X-Mutt-Fcc: =sent X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.85.160.194 Subject: [Qemu-devel] [PULL 01/37] 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: Peter Maydell , Daniel =?utf-8?B?UC4gQmVycmFuZ8Op?= , Eduardo Habkost , Gonglei , Gerd Hoffmann Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" Message-ID: <20190516121743.sVIz6I2NXll-SI0cpXohH85cMnoD6UeCmJWOE4570N8@z> From: Daniel P. Berrangé 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é Message-Id: <20190215103239.28640-2-berrange@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/virtio/virtio-pci.h | 31 +++++++++++++++++++++++++------ 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 ++++++++++++++++---------- 7 files changed, 73 insertions(+), 23 deletions(-) diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h index 18581854ca..bfea2892a5 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; } /* diff --git a/hw/core/machine.c b/hw/core/machine.c index 5d046a43e3..934c1bcceb 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -102,9 +102,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 a2b803b75f..5d57bf5b0c 100644 --- a/hw/display/virtio-vga.c +++ b/hw/display/virtio-vga.c @@ -154,7 +154,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 cb44e19b67..509c1ff555 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)) { -- MST From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-9.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id EC73FC04AAC for ; Mon, 20 May 2019 23:12:31 +0000 (UTC) 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.kernel.org (Postfix) with ESMTPS id B23CD2171F for ; Mon, 20 May 2019 23:12:31 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B23CD2171F Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([127.0.0.1]:43330 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hSrSI-0008Qy-Q0 for qemu-devel@archiver.kernel.org; Mon, 20 May 2019 19:12:30 -0400 Received: from eggs.gnu.org ([209.51.188.92]:44749) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hSrQQ-0006th-56 for qemu-devel@nongnu.org; Mon, 20 May 2019 19:10:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hSrQO-0007ol-CC for qemu-devel@nongnu.org; Mon, 20 May 2019 19:10:34 -0400 Received: from mail-qk1-f196.google.com ([209.85.222.196]:33289) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1hSrQO-0007oP-6n for qemu-devel@nongnu.org; Mon, 20 May 2019 19:10:32 -0400 Received: by mail-qk1-f196.google.com with SMTP id p18so9358891qkk.0 for ; Mon, 20 May 2019 16:10:32 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to; bh=Mwj59QAF+ETU+yRYVN8P1wjLxiufoyyiRXyVtu+O3hU=; b=tA7GnkFLsvJoQB7LZuFkOq2KUL6sgIkRStc+6XIaLk87DE4r2rDr3A40i1bWV8vxbZ VHJu9IYzJfLMoS5oAM05/MBMw46Obxnxim7LCFs+flTbB/5d6n90kANbc4lHPx67qcna vIWVunMD4OBJBvK+AIMlvDKF143SHzWryNTD6P70Dvo06yco9YF1hcGBtX9utkKp+BW9 RxOKhOOCFfiL3PJmHgloW0CnZ5JRrXxOhvcViERhU+2cvv6d4eWmO1M9uvgP9uOx5a/U PoO1S9bnO5hmgtpf+Umai/h7xp8ubbWCoRHC6hq7WhUtWFlVoC41QfDIxnNh1V5sEXdS 7WZg== X-Gm-Message-State: APjAAAVTi4qjIaaqM85G/dP2/NB03SVCPOwiTUqViz6tLE7B6KgGSIad 54w51ezKM3zdVvpPXpqI7VZpZhuIUiI= X-Google-Smtp-Source: APXvYqwLath+U8Ovgq8LzzwgyrwSpW9BAGI2M6RhsVCFR52MGptQpFDwN2YtWquJgu1KtZ/70XRnYA== X-Received: by 2002:a05:620a:1108:: with SMTP id o8mr40557707qkk.126.1558393830511; Mon, 20 May 2019 16:10:30 -0700 (PDT) Received: from redhat.com (pool-173-76-105-71.bstnma.fios.verizon.net. [173.76.105.71]) by smtp.gmail.com with ESMTPSA id y47sm4875692qtb.55.2019.05.20.16.10.29 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Mon, 20 May 2019 16:10:29 -0700 (PDT) Date: Mon, 20 May 2019 19:10:28 -0400 From: "Michael S. Tsirkin" To: qemu-devel@nongnu.org Message-ID: <20190215103239.28640-2-berrange@redhat.com> References: <20190520231008.20140-1-mst@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20190520231008.20140-1-mst@redhat.com> X-Mailer: git-send-email 2.17.1.1206.gb667731e2e.dirty X-Mutt-Fcc: =sent X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.85.222.196 Subject: [Qemu-devel] [PULL v2 01/36] 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: Peter Maydell , Daniel =?utf-8?B?UC4gQmVycmFuZ8Op?= , Eduardo Habkost , Gonglei , Gerd Hoffmann Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" Message-ID: <20190520231028.aUXEuCezSG3CbzmIUsB1HtsWahRbrstzITStXFhHEZg@z> From: Daniel P. Berrangé 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é Message-Id: <20190215103239.28640-2-berrange@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/virtio/virtio-pci.h | 31 +++++++++++++++++++++++++------ 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 ++++++++++++++++---------- 7 files changed, 73 insertions(+), 23 deletions(-) diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h index 18581854ca..bfea2892a5 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; } /* diff --git a/hw/core/machine.c b/hw/core/machine.c index 5d046a43e3..934c1bcceb 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -102,9 +102,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 a2b803b75f..5d57bf5b0c 100644 --- a/hw/display/virtio-vga.c +++ b/hw/display/virtio-vga.c @@ -154,7 +154,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 cb44e19b67..509c1ff555 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)) { -- MST