On Thu, Mar 05, 2015 at 03:56:03PM +0100, Michael Mueller wrote: > On Wed, 4 Mar 2015 16:19:25 -0300 > Eduardo Habkost <ehabk...@redhat.com> wrote: > > > 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 <afaer...@suse.de> wrote: > > > > > > > Am 02.03.2015 um 17:43 schrieb Michael Mueller: > > > > > On Mon, 02 Mar 2015 14:57:21 +0100 > > > > > Andreas Färber <afaer...@suse.de> 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=<pseudo-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 > > > > So the idea is to use kvm_open/close() in the query-cpu-definitions callback > on the fly without > to disturb the KVM-side data structures for the machine probe instead of > going through kvm_init() > during accelerator configuration?
If by KVM-side data structures you mean globals like kvm_state, yes. The idea is to not disturb any global state during probe (including kvm_state). In the branch above, the open/close methods will affect only the local AccelState object. Code that will affect MachineState or other global state will be in the machine_init method. > > > > 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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html