diff mbox

[v3,01/16] Introduce probe mode for machine type none

Message ID 1425300248-40277-2-git-send-email-mimu@linux.vnet.ibm.com
State New
Headers show

Commit Message

Michael Mueller March 2, 2015, 12:43 p.m. UTC
QEMU now switches into "probe mode" when the selected machine is "none" and no
specific accelerator(s) has been requested (i.e.: "-machine none").

In probe mode a by "<ARCH>_CONFIG" defines predefined list of accelerators run
their init() methods.

Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
---
 accel.c              | 31 +++++++++++++++++++++++++------
 include/hw/boards.h  |  1 +
 include/sysemu/kvm.h | 10 ++++++++++
 kvm-all.c            |  3 +++
 4 files changed, 39 insertions(+), 6 deletions(-)

Comments

Andreas Färber March 2, 2015, 1:57 p.m. UTC | #1
Am 02.03.2015 um 13:43 schrieb Michael Mueller:
> QEMU now switches into "probe mode" when the selected machine is "none" and no
> specific accelerator(s) has been requested (i.e.: "-machine none").
> 
> In probe mode a by "<ARCH>_CONFIG" defines predefined list of accelerators run
> their init() methods.
> 
> Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
> ---
>  accel.c              | 31 +++++++++++++++++++++++++------
>  include/hw/boards.h  |  1 +
>  include/sysemu/kvm.h | 10 ++++++++++
>  kvm-all.c            |  3 +++
>  4 files changed, 39 insertions(+), 6 deletions(-)

Edgar/Peter, isn't Xilinx using -machine none in TCG mode?

> @@ -78,20 +83,30 @@ static int accel_init_machine(AccelClass *acc, MachineState *ms)
>  
>  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.

Regards,
Andreas

>  
> -    while (!accel_initialised && *p != '\0') {
> +    while ((probe_mode || !accel_initialised) && *p != '\0') {
>          if (*p == ':') {
>              p++;
>          }
Michael Mueller March 2, 2015, 4:43 p.m. UTC | #2
On Mon, 02 Mar 2015 14:57:21 +0100
Andreas Färber <afaerber@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

The return code of configure_accelerator() is ignored anyway.

Thanks,
Michael
Andreas Färber March 2, 2015, 4:57 p.m. UTC | #3
Am 02.03.2015 um 17:43 schrieb Michael Mueller:
> On Mon, 02 Mar 2015 14:57:21 +0100
> Andreas Färber <afaerber@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
you need both? If we have -probe, isn't that independent of which
machine we specify? Who is going to call either, with which respective goal?

I think that changing the semantics of an absent ,accel=foo parameter to
mean something else than its longtime default of tcg is a bad idea.

Have you testing qtest with it? Doesn't -qtest imply accel=qtest or is
that always passed explicitly?

Regards,
Andreas
Eduardo Habkost March 2, 2015, 7:17 p.m. UTC | #4
On Mon, Mar 02, 2015 at 01:43:53PM +0100, Michael Mueller wrote:
> QEMU now switches into "probe mode" when the selected machine is "none" and no
> specific accelerator(s) has been requested (i.e.: "-machine none").
> 
> In probe mode a by "<ARCH>_CONFIG" defines predefined list of accelerators run
> their init() methods.
> 
> Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
> ---
[...]
>  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;

I don't fully understand the purpose of this patch yet (I will discuss
it in a reply to the cover letter). But if you really want -machine none
to trigger different behavior, why you didn't add a probe_mode field
to MachineClass, so you can set it in the mahine_none class code?

> +        } else {
> +            /* Use the default "accelerator", tcg */
> +            p = "tcg";
> +        }
>      }
[...]
Michael Mueller March 3, 2015, 10:23 a.m. UTC | #5
On Mon, 2 Mar 2015 16:17:33 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> > +        if (probe_mode) {
> > +            /* Use these accelerators in probe mode, tcg should be last */
> > +            p = probe_mode_accels;  
> 
> I don't fully understand the purpose of this patch yet (I will discuss
> it in a reply to the cover letter). But if you really want -machine none
> to trigger different behavior, why you didn't add a probe_mode field
> to MachineClass, so you can set it in the mahine_none class code?

I initially had this machine attribute but, when I remember correctly, but wasn't able to
communicate it down into the target code. But anyhow, the "mode" is eventually obsolete in light
of the temporarily constructed accelerators.. I will further comment on this as reply to your
comment on the cover letter to keep it in place.

Michael
Michael Mueller March 3, 2015, 10:55 a.m. UTC | #6
On Mon, 02 Mar 2015 17:57:01 +0100
Andreas Färber <afaerber@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 <afaerber@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.

Thanks,
Michael

> 
> I think that changing the semantics of an absent ,accel=foo parameter to
> mean something else than its longtime default of tcg is a bad idea.
>
> Have you testing qtest with it? Doesn't -qtest imply accel=qtest or is
> that always passed explicitly?
> 
> Regards,
> Andreas
>
Eduardo Habkost March 4, 2015, 7:19 p.m. UTC | #7
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@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 <afaerber@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

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
Michael Mueller March 5, 2015, 2:56 p.m. UTC | #8
On Wed, 4 Mar 2015 16:19:25 -0300
Eduardo Habkost <ehabkost@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 <afaerber@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 <afaerber@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?


> 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 Habkost March 5, 2015, 3:07 p.m. UTC | #9
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@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 <afaerber@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 <afaerber@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
> > 
>
diff mbox

Patch

diff --git a/accel.c b/accel.c
index 74e41da..260b009 100644
--- a/accel.c
+++ b/accel.c
@@ -36,6 +36,9 @@ 
 
 int tcg_tb_size;
 static bool tcg_allowed = true;
+static const char *probe_mode_accels =
+    "kvm:"
+    "tcg";
 
 static int tcg_init(MachineState *ms)
 {
@@ -59,13 +62,15 @@  static AccelClass *accel_find(const char *opt_name)
     return ac;
 }
 
-static int accel_init_machine(AccelClass *acc, MachineState *ms)
+static int accel_init_machine(AccelClass *acc, MachineState *ms,
+                              bool probe_mode)
 {
     ObjectClass *oc = OBJECT_CLASS(acc);
     const char *cname = object_class_get_name(oc);
     AccelState *accel = ACCEL(object_new(cname));
     int ret;
     ms->accelerator = accel;
+    ms->probe_mode = probe_mode;
     *(acc->allowed) = true;
     ret = acc->init_machine(ms);
     if (ret < 0) {
@@ -78,20 +83,30 @@  static int accel_init_machine(AccelClass *acc, MachineState *ms)
 
 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";
+        }
     }
 
-    while (!accel_initialised && *p != '\0') {
+    while ((probe_mode || !accel_initialised) && *p != '\0') {
         if (*p == ':') {
             p++;
         }
@@ -106,7 +121,7 @@  int configure_accelerator(MachineState *ms)
                    acc->name);
             continue;
         }
-        ret = accel_init_machine(acc, ms);
+        ret = accel_init_machine(acc, ms, probe_mode);
         if (ret < 0) {
             init_failed = true;
             fprintf(stderr, "failed to initialize %s: %s\n",
@@ -128,6 +143,10 @@  int configure_accelerator(MachineState *ms)
         fprintf(stderr, "Back to %s accelerator.\n", acc->name);
     }
 
+    if (probe_mode) {
+        accel_initialised = false;
+    }
+
     return !accel_initialised;
 }
 
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 3ddc449..3253fa5 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -135,6 +135,7 @@  struct MachineState {
     bool usb;
     char *firmware;
     bool iommu;
+    bool probe_mode;
 
     ram_addr_t ram_size;
     ram_addr_t maxram_size;
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 30cb84d..fbc18c8 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -50,6 +50,7 @@  extern bool kvm_msi_via_irqfd_allowed;
 extern bool kvm_gsi_routing_allowed;
 extern bool kvm_gsi_direct_mapping;
 extern bool kvm_readonly_mem_allowed;
+extern bool kvm_probe_mode;
 
 #if defined CONFIG_KVM || !defined NEED_CPU_H
 #define kvm_enabled()           (kvm_allowed)
@@ -143,6 +144,15 @@  extern bool kvm_readonly_mem_allowed;
  */
 #define kvm_readonly_mem_enabled() (kvm_readonly_mem_allowed)
 
+/**
+ * kvm_probe_mode_enabled:
+ *
+ * Returns: true if KVM is initialized for a machine type that
+ * has its probe_mode attribute set (ie QEMU was started in probe
+ * mode)
+ */
+#define kvm_probe_mode_enabled() (kvm_probe_mode)
+
 #else
 #define kvm_enabled()           (0)
 #define kvm_irqchip_in_kernel() (false)
diff --git a/kvm-all.c b/kvm-all.c
index 05a79c2..f9e4434 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -126,6 +126,7 @@  bool kvm_gsi_routing_allowed;
 bool kvm_gsi_direct_mapping;
 bool kvm_allowed;
 bool kvm_readonly_mem_allowed;
+bool kvm_probe_mode;
 
 static const KVMCapabilityInfo kvm_required_capabilites[] = {
     KVM_CAP_INFO(USER_MEMORY),
@@ -1471,6 +1472,8 @@  static int kvm_init(MachineState *ms)
         goto err;
     }
 
+    kvm_probe_mode = ms->probe_mode;
+
     s->nr_slots = kvm_check_extension(s, KVM_CAP_NR_MEMSLOTS);
 
     /* If unspecified, use the default value */