From patchwork Thu Jul 4 13:09:20 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Markus Armbruster X-Patchwork-Id: 256897 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)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 4A47E2C008F for ; Thu, 4 Jul 2013 23:12:31 +1000 (EST) Received: from localhost ([::1]:36479 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UujKn-0000UT-8S for incoming@patchwork.ozlabs.org; Thu, 04 Jul 2013 09:12:29 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58799) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UujHw-0003vy-4g for qemu-devel@nongnu.org; Thu, 04 Jul 2013 09:09:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UujHr-00006k-Ph for qemu-devel@nongnu.org; Thu, 04 Jul 2013 09:09:31 -0400 Received: from oxygen.pond.sub.org ([2a01:4f8:121:10e4::3]:36900) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UujHr-00005y-Go for qemu-devel@nongnu.org; Thu, 04 Jul 2013 09:09:27 -0400 Received: from blackfin.pond.sub.org (p5B32B473.dip0.t-ipconnect.de [91.50.180.115]) by oxygen.pond.sub.org (Postfix) with ESMTPA id C1BD2A5C6B; Thu, 4 Jul 2013 15:09:24 +0200 (CEST) Received: by blackfin.pond.sub.org (Postfix, from userid 1000) id 10A78200B8; Thu, 4 Jul 2013 15:09:24 +0200 (CEST) From: Markus Armbruster To: qemu-devel@nongnu.org Date: Thu, 4 Jul 2013 15:09:20 +0200 Message-Id: <1372943363-24081-5-git-send-email-armbru@redhat.com> X-Mailer: git-send-email 1.7.11.7 In-Reply-To: <1372943363-24081-1-git-send-email-armbru@redhat.com> References: <1372943363-24081-1-git-send-email-armbru@redhat.com> X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2a01:4f8:121:10e4::3 Cc: aliguori@us.ibm.com Subject: [Qemu-devel] [PATCH 4/7] Fix -machine options accel, kernel_irqchip, kvm_shadow_mem 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 Multiple -machine options with the same ID are merged. All but the one without an ID are to be silently ignored. In most places, we query these options with a null ID. This is correct. In some places, we instead query whatever options come first in the list. This is wrong. When the -machine processed first happens to have an ID, options are taken from that ID, and the ones specified without ID are silently ignored. Example: $ upstream-qemu -nodefaults -S -display none -monitor stdio -machine id=foo -machine accel=kvm,usb=on $ upstream-qemu -nodefaults -S -display none -monitor stdio -machine id=foo,accel=kvm,usb=on -machine accel=xen $ upstream-qemu -nodefaults -S -display none -monitor stdio -machine accel=xen -machine id=foo,accel=kvm,usb=on $ qemu-system-x86_64 -nodefaults -S -display none -monitor stdio -machine accel=kvm,usb=on QEMU 1.5.50 monitor - type 'help' for more information (qemu) info kvm kvm support: enabled (qemu) info usb (qemu) q $ qemu-system-x86_64 -nodefaults -S -display none -monitor stdio -machine id=foo -machine accel=kvm,usb=on QEMU 1.5.50 monitor - type 'help' for more information (qemu) info kvm kvm support: disabled (qemu) info usb (qemu) q $ qemu-system-x86_64 -nodefaults -S -display none -monitor stdio -machine id=foo,accel=kvm,usb=on -machine accel=xen QEMU 1.5.50 monitor - type 'help' for more information (qemu) info kvm kvm support: enabled (qemu) info usb USB support not enabled (qemu) q $ qemu-system-x86_64 -nodefaults -S -display none -monitor stdio -machine accel=xen -machine id=foo,accel=kvm,usb=on xc: error: Could not obtain handle on privileged command interface (2 = No such file or directory): Internal error xen be core: can't open xen interface failed to initialize Xen: Operation not permitted Option usb is queried correctly, and the one without an ID wins, regardless of option order. Option accel is queried incorrectly, and which one wins depends on option order and ID. Affected options are accel (and its sugared forms -enable-kvm and -no-kvm), kernel_irqchip, kvm_shadow_mem. Additionally, option kernel_irqchip is normally on by default, except it's off when no -machine options are given. Bug can't bite, because kernel_irqchip is used only when KVM is enabled, KVM is off by default, and enabling always creates -machine options. Downstreams that enable KVM by default do get bitten, though. Use qemu_get_machine_opts() to fix these bugs. Signed-off-by: Markus Armbruster --- hw/ppc/e500.c | 13 ++++--------- kvm-all.c | 5 +---- target-i386/kvm.c | 17 +++++++---------- vl.c | 8 ++------ 4 files changed, 14 insertions(+), 29 deletions(-) diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c index 38f7990..5c02713 100644 --- a/hw/ppc/e500.c +++ b/hw/ppc/e500.c @@ -528,7 +528,6 @@ static DeviceState *ppce500_init_mpic_kvm(PPCE500Params *params, static qemu_irq *ppce500_init_mpic(PPCE500Params *params, MemoryRegion *ccsr, qemu_irq **irqs) { - QemuOptsList *list; qemu_irq *mpic; DeviceState *dev = NULL; SysBusDevice *s; @@ -537,15 +536,11 @@ static qemu_irq *ppce500_init_mpic(PPCE500Params *params, MemoryRegion *ccsr, mpic = g_new(qemu_irq, 256); if (kvm_enabled()) { - bool irqchip_allowed = true, irqchip_required = false; - - list = qemu_find_opts("machine"); - if (!QTAILQ_EMPTY(&list->head)) { - irqchip_allowed = qemu_opt_get_bool(QTAILQ_FIRST(&list->head), + QemuOpts *machine_opts = qemu_get_machine_opts(); + bool irqchip_allowed = qemu_opt_get_bool(machine_opts, "kernel_irqchip", true); - irqchip_required = qemu_opt_get_bool(QTAILQ_FIRST(&list->head), - "kernel_irqchip", false); - } + bool irqchip_required = qemu_opt_get_bool(machine_opts, + "kernel_irqchip", false); if (irqchip_allowed) { dev = ppce500_init_mpic_kvm(params, irqs); diff --git a/kvm-all.c b/kvm-all.c index c757dd2..526b3c0 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -1283,12 +1283,9 @@ int kvm_irqchip_remove_irqfd_notifier(KVMState *s, EventNotifier *n, int virq) static int kvm_irqchip_create(KVMState *s) { - QemuOptsList *list = qemu_find_opts("machine"); int ret; - if (QTAILQ_EMPTY(&list->head) || - !qemu_opt_get_bool(QTAILQ_FIRST(&list->head), - "kernel_irqchip", true) || + if (!qemu_opt_get_bool(qemu_get_machine_opts(), "kernel_irqchip", true) || !kvm_check_extension(s, KVM_CAP_IRQCHIP)) { return 0; } diff --git a/target-i386/kvm.c b/target-i386/kvm.c index 39f4fbb..0a2310d 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -741,7 +741,6 @@ static int kvm_get_supported_msrs(KVMState *s) int kvm_arch_init(KVMState *s) { - QemuOptsList *list = qemu_find_opts("machine"); uint64_t identity_base = 0xfffbc000; uint64_t shadow_mem; int ret; @@ -790,15 +789,13 @@ int kvm_arch_init(KVMState *s) } qemu_register_reset(kvm_unpoison_all, NULL); - if (!QTAILQ_EMPTY(&list->head)) { - shadow_mem = qemu_opt_get_size(QTAILQ_FIRST(&list->head), - "kvm_shadow_mem", -1); - if (shadow_mem != -1) { - shadow_mem /= 4096; - ret = kvm_vm_ioctl(s, KVM_SET_NR_MMU_PAGES, shadow_mem); - if (ret < 0) { - return ret; - } + shadow_mem = qemu_opt_get_size(qemu_get_machine_opts(), + "kvm_shadow_mem", -1); + if (shadow_mem != -1) { + shadow_mem /= 4096; + ret = kvm_vm_ioctl(s, KVM_SET_NR_MMU_PAGES, shadow_mem); + if (ret < 0) { + return ret; } } return 0; diff --git a/vl.c b/vl.c index e68d19c..6678765 100644 --- a/vl.c +++ b/vl.c @@ -2691,17 +2691,13 @@ static struct { static int configure_accelerator(void) { - const char *p = NULL; + const char *p; char buf[10]; int i, ret; bool accel_initialised = false; bool init_failed = false; - QemuOptsList *list = qemu_find_opts("machine"); - if (!QTAILQ_EMPTY(&list->head)) { - p = qemu_opt_get(QTAILQ_FIRST(&list->head), "accel"); - } - + p = qemu_opt_get(qemu_get_machine_opts(), "accel"); if (p == NULL) { /* Use the default "accelerator", tcg */ p = "tcg";