From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758262AbbCDTTs (ORCPT ); Wed, 4 Mar 2015 14:19:48 -0500 Received: from mx1.redhat.com ([209.132.183.28]:60222 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753962AbbCDTTq (ORCPT ); Wed, 4 Mar 2015 14:19:46 -0500 Date: Wed, 4 Mar 2015 16:19:25 -0300 From: Eduardo Habkost To: Michael Mueller Cc: Andreas =?iso-8859-1?Q?F=E4rber?= , linux-s390@vger.kernel.org, Peter Crosthwaite , kvm@vger.kernel.org, Gleb Natapov , linux-kernel@vger.kernel.org, qemu-devel@nongnu.org, Alexander Graf , Christian Borntraeger , "Jason J. Herne" , Paolo Bonzini , Cornelia Huck , "Edgar E. Iglesias" , Alistair Francis , Richard Henderson Subject: Re: [Qemu-devel] [PATCH v3 01/16] Introduce probe mode for machine type none Message-ID: <20150304191925.GD13041@thinpad.lan.raisama.net> References: <1425300248-40277-1-git-send-email-mimu@linux.vnet.ibm.com> <1425300248-40277-2-git-send-email-mimu@linux.vnet.ibm.com> <54F46C41.3000201@suse.de> <20150302174352.3715f1e4@bee> <54F4965D.7000701@suse.de> <20150303115524.40bd40b0@bee> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20150303115524.40bd40b0@bee> X-Fnord: you can see the fnord User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Mar 03, 2015 at 11:55:24AM +0100, Michael Mueller wrote: > On Mon, 02 Mar 2015 17:57:01 +0100 > Andreas Färber wrote: > > > Am 02.03.2015 um 17:43 schrieb Michael Mueller: > > > On Mon, 02 Mar 2015 14:57:21 +0100 > > > Andreas Färber wrote: > > > > > >>> int configure_accelerator(MachineState *ms) > > >>> { > > >>> - const char *p; > > >>> + const char *p, *name; > > >>> char buf[10]; > > >>> int ret; > > >>> bool accel_initialised = false; > > >>> bool init_failed = false; > > >>> AccelClass *acc = NULL; > > >>> + ObjectClass *oc; > > >>> + bool probe_mode = false; > > >>> > > >>> p = qemu_opt_get(qemu_get_machine_opts(), "accel"); > > >>> if (p == NULL) { > > >>> - /* Use the default "accelerator", tcg */ > > >>> - p = "tcg"; > > >>> + oc = (ObjectClass *) MACHINE_GET_CLASS(current_machine); > > >>> + name = object_class_get_name(oc); > > >>> + probe_mode = !strcmp(name, "none" TYPE_MACHINE_SUFFIX); > > >>> + if (probe_mode) { > > >>> + /* Use these accelerators in probe mode, tcg should be last */ > > >>> + p = probe_mode_accels; > > >>> + } else { > > >>> + /* Use the default "accelerator", tcg */ > > >>> + p = "tcg"; > > >>> + } > > >>> } > > >> > > >> Can't we instead use an explicit ,accel=probe or ,accel=auto? > > >> That would then obsolete the next patch. > > > > > > How would you express the following with the accel= approach? > > > > > > -probe -machine s390-ccw,accel=kvm > > > > > > Using machine "none" as default with tcg as last accelerator initialized should not break > > > anything. > > > > > > -M none > > > > Let me ask differently: What does -machine none or -M none have to do > > with probing? It reads as if you are introducing two probe modes. Why do > > The machine none? nothing directly, I guess. What are real world use cases for that > machine type? > > > you need both? If we have -probe, isn't that independent of which > > It is just two different means to switch on the same mode. > > > machine we specify? Who is going to call either, with which respective goal? > > -probe itself would be sufficient but I currently do not want to enforce the use of > a new parameter. Best would be not to have that mode at all if possible. > > The intended use case is driven by management interfaces that need to draw decisions > on, in this particular case runnable cpu models, with information originated by qemu. > > Let me walk through Eduardo's suggestion first and crosscheck it with my requirements > before we enter in a maybe afterwards obsolete discussion. I have been working on some changes to implement x86 CPU probing code that creates accel objects on the fly, that may be useful. See: https://github.com/ehabkost/qemu-hacks/tree/work/user-accel-init Especially the commit: kvm: Move /dev/kvm opening/closing to open/close methods The next steps I plan are: * Create AccelState object on TCG too, and somehow pass it as argument to cpu_x86_init() * Change all kvm_enabled() occurrences on target-i386/cpu.c to use the provided accel object (including x86_cpu_get_supported_feature_word() and x86_cpu_filter_features()) * Use the new x86_cpu_get_supported_feature_word()/x86_cpu_filter_features() code to implement a is_runnable(X86CPUClass*, AccelState*) check * Use the new is_runnable() check to extend query-cpu-definitions for x86 too * Add -cpu string and machine-type arguments to the is_runnable() check -- Eduardo