qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH RFC] accel: default to an actually available accelerator
@ 2017-09-06  9:49 Cornelia Huck
  2017-09-06 11:29 ` Cornelia Huck
  2017-09-06 14:04 ` Richard Henderson
  0 siblings, 2 replies; 14+ messages in thread
From: Cornelia Huck @ 2017-09-06  9:49 UTC (permalink / raw)
  To: qemu-devel; +Cc: lvivier, pbonzini, thuth, mreitz, kwolf, Cornelia Huck

configure_accelerator() falls back to tcg if no accelerator has
been specified. Formerly, we could be sure that tcg is always
available; however, with --disable-tcg, this is not longer true,
and you are not able to start qemu without explicitly specifying
another accelerator on those builds.

Instead, choose an accelerator in the order tcg->kvm->xen->hax.

Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---

RFC mainly because this breaks iotest 186 in a different way on a
tcg-less x86_64 build: Before, it fails with "-machine accel=tcg: No
accelerator found"; afterwards, there seems to be a difference in
output due to different autogenerated devices. Not sure how to handle
that.

cc:ing some hopefully interested folks (-ENOMAINTAINER again).

---
 accel/accel.c              | 22 ++++++++++++++++++++--
 arch_init.c                | 17 +++++++++++++++++
 include/sysemu/arch_init.h |  2 ++
 qemu-options.hx            |  6 ++++--
 4 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/accel/accel.c b/accel/accel.c
index 8ae40e1e13..26a3f32627 100644
--- a/accel/accel.c
+++ b/accel/accel.c
@@ -68,6 +68,25 @@ static int accel_init_machine(AccelClass *acc, MachineState *ms)
     return ret;
 }
 
+static const char *default_accelerator(void)
+{
+    if (tcg_available()) {
+        return "tcg";
+    }
+    if (kvm_available()) {
+        return "kvm";
+    }
+    if (xen_available()) {
+        return "xen";
+    }
+    if (hax_available()) {
+        return "hax";
+    }
+    /* configure makes sure we have at least one accelerator */
+    g_assert(false);
+    return "";
+}
+
 void configure_accelerator(MachineState *ms)
 {
     const char *accel, *p;
@@ -79,8 +98,7 @@ void configure_accelerator(MachineState *ms)
 
     accel = qemu_opt_get(qemu_get_machine_opts(), "accel");
     if (accel == NULL) {
-        /* Use the default "accelerator", tcg */
-        accel = "tcg";
+        accel = default_accelerator();
     }
 
     p = accel;
diff --git a/arch_init.c b/arch_init.c
index a0b8ed6167..1d84eca14d 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -103,6 +103,23 @@ int xen_available(void)
 #endif
 }
 
+int tcg_available(void)
+{
+#ifdef CONFIG_TCG
+    return 1;
+#else
+    return 0;
+#endif
+}
+
+int hax_available(void)
+{
+#ifdef CONFIG_HAX
+    return 1;
+#else
+    return 0;
+#endif
+}
 
 TargetInfo *qmp_query_target(Error **errp)
 {
diff --git a/include/sysemu/arch_init.h b/include/sysemu/arch_init.h
index 8751c468ed..43e515c233 100644
--- a/include/sysemu/arch_init.h
+++ b/include/sysemu/arch_init.h
@@ -30,6 +30,8 @@ extern const uint32_t arch_type;
 
 int kvm_available(void);
 int xen_available(void);
+int tcg_available(void);
+int hax_available(void);
 
 CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp);
 CpuModelExpansionInfo *arch_query_cpu_model_expansion(CpuModelExpansionType type,
diff --git a/qemu-options.hx b/qemu-options.hx
index 9f6e2adfff..386e6e945d 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -66,7 +66,8 @@ Supported machine properties are:
 @table @option
 @item accel=@var{accels1}[:@var{accels2}[:...]]
 This is used to enable an accelerator. Depending on the target architecture,
-kvm, xen, hax or tcg can be available. By default, tcg is used. If there is
+kvm, xen, hax or tcg can be available. By default, the first one available
+out of tcg, kvm, xen, hax (in that order) is used. If there is
 more than one accelerator specified, the next one is used if the previous one
 fails to initialize.
 @item kernel_irqchip=on|off
@@ -126,7 +127,8 @@ STEXI
 @item -accel @var{name}[,prop=@var{value}[,...]]
 @findex -accel
 This is used to enable an accelerator. Depending on the target architecture,
-kvm, xen, hax or tcg can be available. By default, tcg is used. If there is
+kvm, xen, hax or tcg can be available. By default, the first one available
+out of tcg, kvm, xen, hax (in that order) is used. If there is
 more than one accelerator specified, the next one is used if the previous one
 fails to initialize.
 @table @option
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH RFC] accel: default to an actually available accelerator
  2017-09-06  9:49 [Qemu-devel] [PATCH RFC] accel: default to an actually available accelerator Cornelia Huck
@ 2017-09-06 11:29 ` Cornelia Huck
  2017-09-06 14:35   ` Peter Maydell
  2017-09-07  8:11   ` Kevin Wolf
  2017-09-06 14:04 ` Richard Henderson
  1 sibling, 2 replies; 14+ messages in thread
From: Cornelia Huck @ 2017-09-06 11:29 UTC (permalink / raw)
  To: qemu-devel; +Cc: lvivier, pbonzini, thuth, mreitz, kwolf

On Wed,  6 Sep 2017 11:49:27 +0200
Cornelia Huck <cohuck@redhat.com> wrote:

> configure_accelerator() falls back to tcg if no accelerator has
> been specified. Formerly, we could be sure that tcg is always
> available; however, with --disable-tcg, this is not longer true,
> and you are not able to start qemu without explicitly specifying
> another accelerator on those builds.
> 
> Instead, choose an accelerator in the order tcg->kvm->xen->hax.
> 
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> ---
> 
> RFC mainly because this breaks iotest 186 in a different way on a
> tcg-less x86_64 build: Before, it fails with "-machine accel=tcg: No
> accelerator found"; afterwards, there seems to be a difference in
> output due to different autogenerated devices. Not sure how to handle
> that.
> 
> cc:ing some hopefully interested folks (-ENOMAINTAINER again).
> 
> ---
>  accel/accel.c              | 22 ++++++++++++++++++++--
>  arch_init.c                | 17 +++++++++++++++++
>  include/sysemu/arch_init.h |  2 ++
>  qemu-options.hx            |  6 ++++--
>  4 files changed, 43 insertions(+), 4 deletions(-)
> 
> diff --git a/accel/accel.c b/accel/accel.c
> index 8ae40e1e13..26a3f32627 100644
> --- a/accel/accel.c
> +++ b/accel/accel.c
> @@ -68,6 +68,25 @@ static int accel_init_machine(AccelClass *acc, MachineState *ms)
>      return ret;
>  }
>  
> +static const char *default_accelerator(void)
> +{
> +    if (tcg_available()) {
> +        return "tcg";
> +    }
> +    if (kvm_available()) {
> +        return "kvm";
> +    }
> +    if (xen_available()) {
> +        return "xen";
> +    }
> +    if (hax_available()) {
> +        return "hax";
> +    }
> +    /* configure makes sure we have at least one accelerator */
> +    g_assert(false);
> +    return "";
> +}
> +
>  void configure_accelerator(MachineState *ms)
>  {
>      const char *accel, *p;
> @@ -79,8 +98,7 @@ void configure_accelerator(MachineState *ms)
>  
>      accel = qemu_opt_get(qemu_get_machine_opts(), "accel");
>      if (accel == NULL) {
> -        /* Use the default "accelerator", tcg */
> -        accel = "tcg";
> +        accel = default_accelerator();

It actually may be easier to just switch the default to
"tcg:kvm:xen:hax". Haven't tested that, though.

>      }
>  
>      p = accel;

> diff --git a/qemu-options.hx b/qemu-options.hx
> index 9f6e2adfff..386e6e945d 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -66,7 +66,8 @@ Supported machine properties are:
>  @table @option
>  @item accel=@var{accels1}[:@var{accels2}[:...]]
>  This is used to enable an accelerator. Depending on the target architecture,
> -kvm, xen, hax or tcg can be available. By default, tcg is used. If there is
> +kvm, xen, hax or tcg can be available. By default, the first one available
> +out of tcg, kvm, xen, hax (in that order) is used. If there is
>  more than one accelerator specified, the next one is used if the previous one
>  fails to initialize.
>  @item kernel_irqchip=on|off
> @@ -126,7 +127,8 @@ STEXI
>  @item -accel @var{name}[,prop=@var{value}[,...]]
>  @findex -accel
>  This is used to enable an accelerator. Depending on the target architecture,
> -kvm, xen, hax or tcg can be available. By default, tcg is used. If there is
> +kvm, xen, hax or tcg can be available. By default, the first one available
> +out of tcg, kvm, xen, hax (in that order) is used. If there is
>  more than one accelerator specified, the next one is used if the previous one
>  fails to initialize.
>  @table @option

These changes would still apply, as would the question about what to do
with the iotest.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH RFC] accel: default to an actually available accelerator
  2017-09-06  9:49 [Qemu-devel] [PATCH RFC] accel: default to an actually available accelerator Cornelia Huck
  2017-09-06 11:29 ` Cornelia Huck
@ 2017-09-06 14:04 ` Richard Henderson
  1 sibling, 0 replies; 14+ messages in thread
From: Richard Henderson @ 2017-09-06 14:04 UTC (permalink / raw)
  To: Cornelia Huck, qemu-devel; +Cc: lvivier, kwolf, thuth, mreitz, pbonzini

On 09/06/2017 02:49 AM, Cornelia Huck wrote:
> +    /* configure makes sure we have at least one accelerator */
> +    g_assert(false);
> +    return "";

g_assert_not_reached();

Though I do like the follow-up idea of "t:k:x:h".


r~

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH RFC] accel: default to an actually available accelerator
  2017-09-06 11:29 ` Cornelia Huck
@ 2017-09-06 14:35   ` Peter Maydell
  2017-09-06 15:54     ` Cornelia Huck
  2017-09-11 11:48     ` Paolo Bonzini
  2017-09-07  8:11   ` Kevin Wolf
  1 sibling, 2 replies; 14+ messages in thread
From: Peter Maydell @ 2017-09-06 14:35 UTC (permalink / raw)
  To: Cornelia Huck
  Cc: QEMU Developers, Laurent Vivier, Paolo Bonzini, Thomas Huth,
	Kevin Wolf, Max Reitz

On 6 September 2017 at 12:29, Cornelia Huck <cohuck@redhat.com> wrote:
> On Wed,  6 Sep 2017 11:49:27 +0200
> Cornelia Huck <cohuck@redhat.com> wrote:
>> @@ -79,8 +98,7 @@ void configure_accelerator(MachineState *ms)
>>
>>      accel = qemu_opt_get(qemu_get_machine_opts(), "accel");
>>      if (accel == NULL) {
>> -        /* Use the default "accelerator", tcg */
>> -        accel = "tcg";
>> +        accel = default_accelerator();
>
> It actually may be easier to just switch the default to
> "tcg:kvm:xen:hax". Haven't tested that, though.

Does it make sense to include Xen in the default list?
I don't know much about Xen but I was under the impression
that it's a special purpose thing that you can only use
as part of a Xen setup, whereas tcg, kvm, hax are all
more-or-less interchangeable ways to run a VM under a
Linux/etc host. Do I have the wrong end of the Xen stick?

thanks
-- PMM

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH RFC] accel: default to an actually available accelerator
  2017-09-06 14:35   ` Peter Maydell
@ 2017-09-06 15:54     ` Cornelia Huck
  2017-09-11 11:48     ` Paolo Bonzini
  1 sibling, 0 replies; 14+ messages in thread
From: Cornelia Huck @ 2017-09-06 15:54 UTC (permalink / raw)
  To: Peter Maydell
  Cc: QEMU Developers, Laurent Vivier, Paolo Bonzini, Thomas Huth,
	Kevin Wolf, Max Reitz

On Wed, 6 Sep 2017 15:35:16 +0100
Peter Maydell <peter.maydell@linaro.org> wrote:

> On 6 September 2017 at 12:29, Cornelia Huck <cohuck@redhat.com> wrote:
> > On Wed,  6 Sep 2017 11:49:27 +0200
> > Cornelia Huck <cohuck@redhat.com> wrote:  
> >> @@ -79,8 +98,7 @@ void configure_accelerator(MachineState *ms)
> >>
> >>      accel = qemu_opt_get(qemu_get_machine_opts(), "accel");
> >>      if (accel == NULL) {
> >> -        /* Use the default "accelerator", tcg */
> >> -        accel = "tcg";
> >> +        accel = default_accelerator();  
> >
> > It actually may be easier to just switch the default to
> > "tcg:kvm:xen:hax". Haven't tested that, though.  
> 
> Does it make sense to include Xen in the default list?
> I don't know much about Xen but I was under the impression
> that it's a special purpose thing that you can only use
> as part of a Xen setup, whereas tcg, kvm, hax are all
> more-or-less interchangeable ways to run a VM under a
> Linux/etc host. Do I have the wrong end of the Xen stick?

I'm unfortunately not familiar with xen either.

I was going with what configure considers as an available accelerator
(in supported_target()). FWIW, I can build x86_64 with xen as the only
available accelerator, but have not been able to get it to work (in all
of the 5 minutes I tried). I can't get it to build with hax as the only
accelerator (although configure does not complain; I might be missing
something).

Switching the default from "tcg" to "tcg:kvm" would already fix the
problem for s390x ;), but maybe someone else has a better idea?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH RFC] accel: default to an actually available accelerator
  2017-09-06 11:29 ` Cornelia Huck
  2017-09-06 14:35   ` Peter Maydell
@ 2017-09-07  8:11   ` Kevin Wolf
  2017-09-07  8:14     ` Thomas Huth
  2017-09-11 11:51     ` Paolo Bonzini
  1 sibling, 2 replies; 14+ messages in thread
From: Kevin Wolf @ 2017-09-07  8:11 UTC (permalink / raw)
  To: Cornelia Huck; +Cc: qemu-devel, lvivier, pbonzini, thuth, mreitz

Am 06.09.2017 um 13:29 hat Cornelia Huck geschrieben:
> On Wed,  6 Sep 2017 11:49:27 +0200
> Cornelia Huck <cohuck@redhat.com> wrote:
> 
> > configure_accelerator() falls back to tcg if no accelerator has
> > been specified. Formerly, we could be sure that tcg is always
> > available; however, with --disable-tcg, this is not longer true,
> > and you are not able to start qemu without explicitly specifying
> > another accelerator on those builds.
> > 
> > Instead, choose an accelerator in the order tcg->kvm->xen->hax.
> > 
> > Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> > ---
> > 
> > RFC mainly because this breaks iotest 186 in a different way on a
> > tcg-less x86_64 build: Before, it fails with "-machine accel=tcg: No
> > accelerator found"; afterwards, there seems to be a difference in
> > output due to different autogenerated devices. Not sure how to handle
> > that.
> > 
> > cc:ing some hopefully interested folks (-ENOMAINTAINER again).
> > 
> > ---
> >  accel/accel.c              | 22 ++++++++++++++++++++--
> >  arch_init.c                | 17 +++++++++++++++++
> >  include/sysemu/arch_init.h |  2 ++
> >  qemu-options.hx            |  6 ++++--
> >  4 files changed, 43 insertions(+), 4 deletions(-)
> > 
> > diff --git a/accel/accel.c b/accel/accel.c
> > index 8ae40e1e13..26a3f32627 100644
> > --- a/accel/accel.c
> > +++ b/accel/accel.c
> > @@ -68,6 +68,25 @@ static int accel_init_machine(AccelClass *acc, MachineState *ms)
> >      return ret;
> >  }
> >  
> > +static const char *default_accelerator(void)
> > +{
> > +    if (tcg_available()) {
> > +        return "tcg";
> > +    }
> > +    if (kvm_available()) {
> > +        return "kvm";
> > +    }
> > +    if (xen_available()) {
> > +        return "xen";
> > +    }
> > +    if (hax_available()) {
> > +        return "hax";
> > +    }
> > +    /* configure makes sure we have at least one accelerator */
> > +    g_assert(false);
> > +    return "";
> > +}
> > +
> >  void configure_accelerator(MachineState *ms)
> >  {
> >      const char *accel, *p;
> > @@ -79,8 +98,7 @@ void configure_accelerator(MachineState *ms)
> >  
> >      accel = qemu_opt_get(qemu_get_machine_opts(), "accel");
> >      if (accel == NULL) {
> > -        /* Use the default "accelerator", tcg */
> > -        accel = "tcg";
> > +        accel = default_accelerator();
> 
> It actually may be easier to just switch the default to
> "tcg:kvm:xen:hax". Haven't tested that, though.

This would have been my first thought and looks a bit simpler, so if
it works, I'd go for it.

But the real reason why I'm replying: Should we add changing the default
to "kvm:tcg" to the list of planned 3.0 changes? I am part of the group
that intentionally uses TCG occasionally, but I think the majority of
users wants to use KVM (or whatever the fastest option is on their
system) whenever it is available.

Kevin

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH RFC] accel: default to an actually available accelerator
  2017-09-07  8:11   ` Kevin Wolf
@ 2017-09-07  8:14     ` Thomas Huth
  2017-09-07  8:25       ` Cornelia Huck
  2017-09-11 11:51     ` Paolo Bonzini
  1 sibling, 1 reply; 14+ messages in thread
From: Thomas Huth @ 2017-09-07  8:14 UTC (permalink / raw)
  To: Kevin Wolf, Cornelia Huck; +Cc: qemu-devel, lvivier, pbonzini, mreitz

On 07.09.2017 10:11, Kevin Wolf wrote:
[...]
> But the real reason why I'm replying: Should we add changing the default
> to "kvm:tcg" to the list of planned 3.0 changes? I am part of the group
> that intentionally uses TCG occasionally, but I think the majority of
> users wants to use KVM (or whatever the fastest option is on their
> system) whenever it is available.

If you consider how often people are getting this wrong (they want to
use KVM but end up with TCG in the first try), I think that's a good idea.

Maybe we should start a Wiki page where we collect ideas for QEMU 3.0?

 Thomas

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH RFC] accel: default to an actually available accelerator
  2017-09-07  8:14     ` Thomas Huth
@ 2017-09-07  8:25       ` Cornelia Huck
  2017-09-07  8:45         ` Kevin Wolf
  0 siblings, 1 reply; 14+ messages in thread
From: Cornelia Huck @ 2017-09-07  8:25 UTC (permalink / raw)
  To: Thomas Huth; +Cc: Kevin Wolf, qemu-devel, lvivier, pbonzini, mreitz

On Thu, 7 Sep 2017 10:14:27 +0200
Thomas Huth <thuth@redhat.com> wrote:

> On 07.09.2017 10:11, Kevin Wolf wrote:
> [...]
> > But the real reason why I'm replying: Should we add changing the default
> > to "kvm:tcg" to the list of planned 3.0 changes? I am part of the group
> > that intentionally uses TCG occasionally, but I think the majority of
> > users wants to use KVM (or whatever the fastest option is on their
> > system) whenever it is available.  
> 
> If you consider how often people are getting this wrong (they want to
> use KVM but end up with TCG in the first try), I think that's a good idea.

Agreed.

I'm wondering what that means for our tests, though. Some of them work
slightly different under tcg or kvm (cf. iotest 186, as referenced in
the original mail), and sometimes you'll probably explicitly want to
exercise tcg. That does not speak against the change, but we probably
need to look at what we want in more detail.

> Maybe we should start a Wiki page where we collect ideas for QEMU 3.0?

Also a good idea.

[Do we already have any idea about the timeframe for 3.0?]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH RFC] accel: default to an actually available accelerator
  2017-09-07  8:25       ` Cornelia Huck
@ 2017-09-07  8:45         ` Kevin Wolf
  0 siblings, 0 replies; 14+ messages in thread
From: Kevin Wolf @ 2017-09-07  8:45 UTC (permalink / raw)
  To: Cornelia Huck; +Cc: Thomas Huth, qemu-devel, lvivier, pbonzini, mreitz

Am 07.09.2017 um 10:25 hat Cornelia Huck geschrieben:
> On Thu, 7 Sep 2017 10:14:27 +0200
> Thomas Huth <thuth@redhat.com> wrote:
> 
> > On 07.09.2017 10:11, Kevin Wolf wrote:
> > [...]
> > > But the real reason why I'm replying: Should we add changing the default
> > > to "kvm:tcg" to the list of planned 3.0 changes? I am part of the group
> > > that intentionally uses TCG occasionally, but I think the majority of
> > > users wants to use KVM (or whatever the fastest option is on their
> > > system) whenever it is available.  
> > 
> > If you consider how often people are getting this wrong (they want to
> > use KVM but end up with TCG in the first try), I think that's a good idea.
> 
> Agreed.
> 
> I'm wondering what that means for our tests, though. Some of them work
> slightly different under tcg or kvm (cf. iotest 186, as referenced in
> the original mail), and sometimes you'll probably explicitly want to
> exercise tcg. That does not speak against the change, but we probably
> need to look at what we want in more detail.

This is a bug in test 186, and probably 172, too. Normally, we use the
options from ./common:

    export QEMU_OPTIONS="-nodefaults -machine accel=qtest"

However, these two test cases overwrite QEMU_OPTIONS and neglect to add
a '-machine accel=qtest' option manually.

Kevin

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH RFC] accel: default to an actually available accelerator
  2017-09-06 14:35   ` Peter Maydell
  2017-09-06 15:54     ` Cornelia Huck
@ 2017-09-11 11:48     ` Paolo Bonzini
  2017-09-11 11:51       ` Cornelia Huck
  1 sibling, 1 reply; 14+ messages in thread
From: Paolo Bonzini @ 2017-09-11 11:48 UTC (permalink / raw)
  To: Peter Maydell, Cornelia Huck
  Cc: QEMU Developers, Laurent Vivier, Thomas Huth, Kevin Wolf,
	Max Reitz

On 06/09/2017 16:35, Peter Maydell wrote:
>>>      accel = qemu_opt_get(qemu_get_machine_opts(), "accel");
>>>      if (accel == NULL) {
>>> -        /* Use the default "accelerator", tcg */
>>> -        accel = "tcg";
>>> +        accel = default_accelerator();
>> It actually may be easier to just switch the default to
>> "tcg:kvm:xen:hax". Haven't tested that, though.
> Does it make sense to include Xen in the default list?
> I don't know much about Xen but I was under the impression
> that it's a special purpose thing that you can only use
> as part of a Xen setup, whereas tcg, kvm, hax are all
> more-or-less interchangeable ways to run a VM under a
> Linux/etc host. Do I have the wrong end of the Xen stick?

Yes, that is correct (in fact, -xen-domid is required too).

Paolo

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH RFC] accel: default to an actually available accelerator
  2017-09-11 11:48     ` Paolo Bonzini
@ 2017-09-11 11:51       ` Cornelia Huck
  2017-09-11 11:53         ` Paolo Bonzini
  0 siblings, 1 reply; 14+ messages in thread
From: Cornelia Huck @ 2017-09-11 11:51 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Peter Maydell, QEMU Developers, Laurent Vivier, Thomas Huth,
	Kevin Wolf, Max Reitz

On Mon, 11 Sep 2017 13:48:46 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 06/09/2017 16:35, Peter Maydell wrote:
> >>>      accel = qemu_opt_get(qemu_get_machine_opts(), "accel");
> >>>      if (accel == NULL) {
> >>> -        /* Use the default "accelerator", tcg */
> >>> -        accel = "tcg";
> >>> +        accel = default_accelerator();  
> >> It actually may be easier to just switch the default to
> >> "tcg:kvm:xen:hax". Haven't tested that, though.  
> > Does it make sense to include Xen in the default list?
> > I don't know much about Xen but I was under the impression
> > that it's a special purpose thing that you can only use
> > as part of a Xen setup, whereas tcg, kvm, hax are all
> > more-or-less interchangeable ways to run a VM under a
> > Linux/etc host. Do I have the wrong end of the Xen stick?  
> 
> Yes, that is correct (in fact, -xen-domid is required too).

OK, so we should use "tcg:kvm:hax"?

(Not sure how useful the hax statement is, I'm not familiar with that
one.)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH RFC] accel: default to an actually available accelerator
  2017-09-07  8:11   ` Kevin Wolf
  2017-09-07  8:14     ` Thomas Huth
@ 2017-09-11 11:51     ` Paolo Bonzini
  2017-09-22 18:15       ` Eduardo Habkost
  1 sibling, 1 reply; 14+ messages in thread
From: Paolo Bonzini @ 2017-09-11 11:51 UTC (permalink / raw)
  To: Kevin Wolf, Cornelia Huck
  Cc: qemu-devel, lvivier, thuth, mreitz, Eduardo Habkost

On 07/09/2017 10:11, Kevin Wolf wrote:
> Am 06.09.2017 um 13:29 hat Cornelia Huck geschrieben:
>> On Wed,  6 Sep 2017 11:49:27 +0200
>> Cornelia Huck <cohuck@redhat.com> wrote:
>>
>>> configure_accelerator() falls back to tcg if no accelerator has
>>> been specified. Formerly, we could be sure that tcg is always
>>> available; however, with --disable-tcg, this is not longer true,
>>> and you are not able to start qemu without explicitly specifying
>>> another accelerator on those builds.
>>>
>>> Instead, choose an accelerator in the order tcg->kvm->xen->hax.
>>>
>>> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
>>> ---
>>>
>>> RFC mainly because this breaks iotest 186 in a different way on a
>>> tcg-less x86_64 build: Before, it fails with "-machine accel=tcg: No
>>> accelerator found"; afterwards, there seems to be a difference in
>>> output due to different autogenerated devices. Not sure how to handle
>>> that.
>>>
>>> cc:ing some hopefully interested folks (-ENOMAINTAINER again).
>>>
>>> ---
>>>  accel/accel.c              | 22 ++++++++++++++++++++--
>>>  arch_init.c                | 17 +++++++++++++++++
>>>  include/sysemu/arch_init.h |  2 ++
>>>  qemu-options.hx            |  6 ++++--
>>>  4 files changed, 43 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/accel/accel.c b/accel/accel.c
>>> index 8ae40e1e13..26a3f32627 100644
>>> --- a/accel/accel.c
>>> +++ b/accel/accel.c
>>> @@ -68,6 +68,25 @@ static int accel_init_machine(AccelClass *acc, MachineState *ms)
>>>      return ret;
>>>  }
>>>  
>>> +static const char *default_accelerator(void)
>>> +{
>>> +    if (tcg_available()) {
>>> +        return "tcg";
>>> +    }
>>> +    if (kvm_available()) {
>>> +        return "kvm";
>>> +    }
>>> +    if (xen_available()) {
>>> +        return "xen";
>>> +    }
>>> +    if (hax_available()) {
>>> +        return "hax";
>>> +    }
>>> +    /* configure makes sure we have at least one accelerator */
>>> +    g_assert(false);
>>> +    return "";
>>> +}
>>> +
>>>  void configure_accelerator(MachineState *ms)
>>>  {
>>>      const char *accel, *p;
>>> @@ -79,8 +98,7 @@ void configure_accelerator(MachineState *ms)
>>>  
>>>      accel = qemu_opt_get(qemu_get_machine_opts(), "accel");
>>>      if (accel == NULL) {
>>> -        /* Use the default "accelerator", tcg */
>>> -        accel = "tcg";
>>> +        accel = default_accelerator();
>>
>> It actually may be easier to just switch the default to
>> "tcg:kvm:xen:hax". Haven't tested that, though.
> 
> This would have been my first thought and looks a bit simpler, so if
> it works, I'd go for it.
> 
> But the real reason why I'm replying: Should we add changing the default
> to "kvm:tcg" to the list of planned 3.0 changes? I am part of the group
> that intentionally uses TCG occasionally, but I think the majority of
> users wants to use KVM (or whatever the fastest option is on their
> system) whenever it is available.

Yes, the only thing to be clarified is the default family/model/stepping
for "-accel kvm".  "-cpu qemu64" with KVM uses an AMD f/m/s and Intel as
the vendor, and some software (IIRC the GMP testsuite or something like
that) doesn't like it.  We should probably change qemu64 to a core2
f/m/s or something like that when running under KVM.  Eduardo?

Paolo

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH RFC] accel: default to an actually available accelerator
  2017-09-11 11:51       ` Cornelia Huck
@ 2017-09-11 11:53         ` Paolo Bonzini
  0 siblings, 0 replies; 14+ messages in thread
From: Paolo Bonzini @ 2017-09-11 11:53 UTC (permalink / raw)
  To: Cornelia Huck
  Cc: Peter Maydell, QEMU Developers, Laurent Vivier, Thomas Huth,
	Kevin Wolf, Max Reitz

On 11/09/2017 13:51, Cornelia Huck wrote:
> On Mon, 11 Sep 2017 13:48:46 +0200
> Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
>> On 06/09/2017 16:35, Peter Maydell wrote:
>>>>>      accel = qemu_opt_get(qemu_get_machine_opts(), "accel");
>>>>>      if (accel == NULL) {
>>>>> -        /* Use the default "accelerator", tcg */
>>>>> -        accel = "tcg";
>>>>> +        accel = default_accelerator();  
>>>> It actually may be easier to just switch the default to
>>>> "tcg:kvm:xen:hax". Haven't tested that, though.  
>>> Does it make sense to include Xen in the default list?
>>> I don't know much about Xen but I was under the impression
>>> that it's a special purpose thing that you can only use
>>> as part of a Xen setup, whereas tcg, kvm, hax are all
>>> more-or-less interchangeable ways to run a VM under a
>>> Linux/etc host. Do I have the wrong end of the Xen stick?  
>>
>> Yes, that is correct (in fact, -xen-domid is required too).
> 
> OK, so we should use "tcg:kvm:hax"?
> 
> (Not sure how useful the hax statement is, I'm not familiar with that
> one.)

Yes.  When we move KVM to the front, however, HAX and the upcoming HVF
accelerator probably should stay in the back because they are less
tested than TCG (e.g. HAX doesn't support -cpu, HVF will not support
live migration in the first iteration, etc.).

Paolo

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH RFC] accel: default to an actually available accelerator
  2017-09-11 11:51     ` Paolo Bonzini
@ 2017-09-22 18:15       ` Eduardo Habkost
  0 siblings, 0 replies; 14+ messages in thread
From: Eduardo Habkost @ 2017-09-22 18:15 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Kevin Wolf, Cornelia Huck, lvivier, thuth, qemu-devel, mreitz

On Mon, Sep 11, 2017 at 01:51:54PM +0200, Paolo Bonzini wrote:
> On 07/09/2017 10:11, Kevin Wolf wrote:
> > Am 06.09.2017 um 13:29 hat Cornelia Huck geschrieben:
> >> On Wed,  6 Sep 2017 11:49:27 +0200
> >> Cornelia Huck <cohuck@redhat.com> wrote:
> >>
> >>> configure_accelerator() falls back to tcg if no accelerator has
> >>> been specified. Formerly, we could be sure that tcg is always
> >>> available; however, with --disable-tcg, this is not longer true,
> >>> and you are not able to start qemu without explicitly specifying
> >>> another accelerator on those builds.
> >>>
> >>> Instead, choose an accelerator in the order tcg->kvm->xen->hax.
> >>>
> >>> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> >>> ---
> >>>
> >>> RFC mainly because this breaks iotest 186 in a different way on a
> >>> tcg-less x86_64 build: Before, it fails with "-machine accel=tcg: No
> >>> accelerator found"; afterwards, there seems to be a difference in
> >>> output due to different autogenerated devices. Not sure how to handle
> >>> that.
> >>>
> >>> cc:ing some hopefully interested folks (-ENOMAINTAINER again).
> >>>
> >>> ---
> >>>  accel/accel.c              | 22 ++++++++++++++++++++--
> >>>  arch_init.c                | 17 +++++++++++++++++
> >>>  include/sysemu/arch_init.h |  2 ++
> >>>  qemu-options.hx            |  6 ++++--
> >>>  4 files changed, 43 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/accel/accel.c b/accel/accel.c
> >>> index 8ae40e1e13..26a3f32627 100644
> >>> --- a/accel/accel.c
> >>> +++ b/accel/accel.c
> >>> @@ -68,6 +68,25 @@ static int accel_init_machine(AccelClass *acc, MachineState *ms)
> >>>      return ret;
> >>>  }
> >>>  
> >>> +static const char *default_accelerator(void)
> >>> +{
> >>> +    if (tcg_available()) {
> >>> +        return "tcg";
> >>> +    }
> >>> +    if (kvm_available()) {
> >>> +        return "kvm";
> >>> +    }
> >>> +    if (xen_available()) {
> >>> +        return "xen";
> >>> +    }
> >>> +    if (hax_available()) {
> >>> +        return "hax";
> >>> +    }
> >>> +    /* configure makes sure we have at least one accelerator */
> >>> +    g_assert(false);
> >>> +    return "";
> >>> +}
> >>> +
> >>>  void configure_accelerator(MachineState *ms)
> >>>  {
> >>>      const char *accel, *p;
> >>> @@ -79,8 +98,7 @@ void configure_accelerator(MachineState *ms)
> >>>  
> >>>      accel = qemu_opt_get(qemu_get_machine_opts(), "accel");
> >>>      if (accel == NULL) {
> >>> -        /* Use the default "accelerator", tcg */
> >>> -        accel = "tcg";
> >>> +        accel = default_accelerator();
> >>
> >> It actually may be easier to just switch the default to
> >> "tcg:kvm:xen:hax". Haven't tested that, though.
> > 
> > This would have been my first thought and looks a bit simpler, so if
> > it works, I'd go for it.
> > 
> > But the real reason why I'm replying: Should we add changing the default
> > to "kvm:tcg" to the list of planned 3.0 changes? I am part of the group
> > that intentionally uses TCG occasionally, but I think the majority of
> > users wants to use KVM (or whatever the fastest option is on their
> > system) whenever it is available.
> 
> Yes, the only thing to be clarified is the default family/model/stepping
> for "-accel kvm".  "-cpu qemu64" with KVM uses an AMD f/m/s and Intel as
> the vendor, and some software (IIRC the GMP testsuite or something like
> that) doesn't like it.  We should probably change qemu64 to a core2
> f/m/s or something like that when running under KVM.  Eduardo?

The current f/m/s was supposed to make sense for both AMD and
Intel CPUs, to avoid requiring per-cpu-vendor defaults.  If we
find a more recent f/m/s combination that still makes some sense
for both vendors, changing it will be very simple.

Long term, however, we should probably add per-cpu-vendor
defaults to make this more flexible.

-- 
Eduardo

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2017-09-22 18:16 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-06  9:49 [Qemu-devel] [PATCH RFC] accel: default to an actually available accelerator Cornelia Huck
2017-09-06 11:29 ` Cornelia Huck
2017-09-06 14:35   ` Peter Maydell
2017-09-06 15:54     ` Cornelia Huck
2017-09-11 11:48     ` Paolo Bonzini
2017-09-11 11:51       ` Cornelia Huck
2017-09-11 11:53         ` Paolo Bonzini
2017-09-07  8:11   ` Kevin Wolf
2017-09-07  8:14     ` Thomas Huth
2017-09-07  8:25       ` Cornelia Huck
2017-09-07  8:45         ` Kevin Wolf
2017-09-11 11:51     ` Paolo Bonzini
2017-09-22 18:15       ` Eduardo Habkost
2017-09-06 14:04 ` Richard Henderson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).