On Thu, Mar 05, 2015 at 03:56:03PM +0100, Michael Mueller wrote:
> On Wed, 4 Mar 2015 16:19:25 -0300
> Eduardo Habkost <ehabkost@xxxxxxxxxx> 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 <afaerber@xxxxxxx> wrote:
> > >
> > > > Am 02.03.2015 um 17:43 schrieb Michael Mueller:
> > > > > On Mon, 02 Mar 2015 14:57:21 +0100
> > > > > Andreas Färber <afaerber@xxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
