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 <coh...@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 <coh...@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
