| Message ID | 1475580648-6470-2-git-send-email-lvivier@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Oct 04, 2016 at 01:30:47PM +0200, Laurent Vivier wrote: > By default, QEMU uses 'tcg' or the one provided with the "accel" > property. > > But sometime, user wants to use a real accelerator without knowing > if he really can, with, for instance accel=kvm:tcg. > In this case, and if the accelerator is not available we > have a noisy "XXX accelerator not found". IMHO it would be better to just remove that warning message and continue using existing accel=kvm:tcg syntax to specify the preference of which accelerators to use. Only emit "XXX accelerator not found", if there are not further accelerators listed. eg accel=kvm:tcg should not print a "KVM accelerator not found" warning when it falls back to tcg, but a accel=kvm would print a warning, since no fallback is given. Regards, Daniel
On 4 October 2016 at 13:00, Daniel P. Berrange <berrange@redhat.com> wrote: > On Tue, Oct 04, 2016 at 01:30:47PM +0200, Laurent Vivier wrote: >> By default, QEMU uses 'tcg' or the one provided with the "accel" >> property. >> >> But sometime, user wants to use a real accelerator without knowing >> if he really can, with, for instance accel=kvm:tcg. >> In this case, and if the accelerator is not available we >> have a noisy "XXX accelerator not found". > > IMHO it would be better to just remove that warning message > and continue using existing accel=kvm:tcg syntax to specify > the preference of which accelerators to use. Oddly we don't give a warning message for what is probably the most common user error: forgetting to specify -enable-kvm and thus getting TCG only even though the h/w supports KVM... thanks -- PMM
On 04/10/2016 13:30, Laurent Vivier wrote: > But sometime, user wants to use a real accelerator without knowing > if he really can, with, for instance accel=kvm:tcg. > In this case, and if the accelerator is not available we > have a noisy "XXX accelerator not found". > > By allowing the user to ask the "best" accelerator for the given > target, we can avoid this problem. > > This patch introduces a new parameter for the "accel" property, the > "best" keyword. > > You can ask to use the best accelerator with "-M accel=best", > or if you want to use your favorite accelerator and if it is not > available, the best one, you can use, for instance > "-M accel=kvm:best". I don't think there's a single definition of a "best" accelerator. For example, some "-cpu" features may be available only with TCG. In that case, "kvm:tcg" has a clear meaning ("kvm" if it exists, otherwise "tcg") but "best" doesn't. I agree with Daniel that unit tests should use "tcg" exclusively, at least as a default. Paolo
On 04.10.2016 14:41, Paolo Bonzini wrote: > > > On 04/10/2016 13:30, Laurent Vivier wrote: >> But sometime, user wants to use a real accelerator without knowing >> if he really can, with, for instance accel=kvm:tcg. >> In this case, and if the accelerator is not available we >> have a noisy "XXX accelerator not found". >> >> By allowing the user to ask the "best" accelerator for the given >> target, we can avoid this problem. >> >> This patch introduces a new parameter for the "accel" property, the >> "best" keyword. >> >> You can ask to use the best accelerator with "-M accel=best", >> or if you want to use your favorite accelerator and if it is not >> available, the best one, you can use, for instance >> "-M accel=kvm:best". > > I don't think there's a single definition of a "best" accelerator. For > example, some "-cpu" features may be available only with TCG. In that > case, "kvm:tcg" has a clear meaning ("kvm" if it exists, otherwise > "tcg") but "best" doesn't. > > I agree with Daniel that unit tests should use "tcg" exclusively, at > least as a default. Using only tcg has also some disadvantages: For some tests, it's interesting to know whether they also work properly with KVM (e.g. migration tests), and only using tcg by default slows down the "make check" quite a bit - which might become an issue now that we're adding more and more tests. So maybe we need something like a qtest_get_accel() function that returns "tcg" by default, but if the user set an environment variable like QTEST_ACCEL, it uses that value instead? Thomas
On Tue, Oct 04, 2016 at 02:49:21PM +0200, Thomas Huth wrote: > On 04.10.2016 14:41, Paolo Bonzini wrote: > > > > > > On 04/10/2016 13:30, Laurent Vivier wrote: > >> But sometime, user wants to use a real accelerator without knowing > >> if he really can, with, for instance accel=kvm:tcg. > >> In this case, and if the accelerator is not available we > >> have a noisy "XXX accelerator not found". > >> > >> By allowing the user to ask the "best" accelerator for the given > >> target, we can avoid this problem. > >> > >> This patch introduces a new parameter for the "accel" property, the > >> "best" keyword. > >> > >> You can ask to use the best accelerator with "-M accel=best", > >> or if you want to use your favorite accelerator and if it is not > >> available, the best one, you can use, for instance > >> "-M accel=kvm:best". > > > > I don't think there's a single definition of a "best" accelerator. For > > example, some "-cpu" features may be available only with TCG. In that > > case, "kvm:tcg" has a clear meaning ("kvm" if it exists, otherwise > > "tcg") but "best" doesn't. > > > > I agree with Daniel that unit tests should use "tcg" exclusively, at > > least as a default. > > Using only tcg has also some disadvantages: For some tests, it's > interesting to know whether they also work properly with KVM (e.g. > migration tests), and only using tcg by default slows down the "make > check" quite a bit - which might become an issue now that we're adding > more and more tests. Which tests are you seeing a slow-down for ? make check-unit doesn't show any difference and for 'make check-qtest' the difference was negligible (1m47 for KVM vs 1m57 for TCG). We shouldn't be running extensive guest workloads in unit tests, so I'd be surprised to see a major hit in unit test time. Regards, Daniel
On 04.10.2016 15:31, Daniel P. Berrange wrote: > On Tue, Oct 04, 2016 at 02:49:21PM +0200, Thomas Huth wrote: >> On 04.10.2016 14:41, Paolo Bonzini wrote: >>> >>> >>> On 04/10/2016 13:30, Laurent Vivier wrote: >>>> But sometime, user wants to use a real accelerator without knowing >>>> if he really can, with, for instance accel=kvm:tcg. >>>> In this case, and if the accelerator is not available we >>>> have a noisy "XXX accelerator not found". >>>> >>>> By allowing the user to ask the "best" accelerator for the given >>>> target, we can avoid this problem. >>>> >>>> This patch introduces a new parameter for the "accel" property, the >>>> "best" keyword. >>>> >>>> You can ask to use the best accelerator with "-M accel=best", >>>> or if you want to use your favorite accelerator and if it is not >>>> available, the best one, you can use, for instance >>>> "-M accel=kvm:best". >>> >>> I don't think there's a single definition of a "best" accelerator. For >>> example, some "-cpu" features may be available only with TCG. In that >>> case, "kvm:tcg" has a clear meaning ("kvm" if it exists, otherwise >>> "tcg") but "best" doesn't. >>> >>> I agree with Daniel that unit tests should use "tcg" exclusively, at >>> least as a default. >> >> Using only tcg has also some disadvantages: For some tests, it's >> interesting to know whether they also work properly with KVM (e.g. >> migration tests), and only using tcg by default slows down the "make >> check" quite a bit - which might become an issue now that we're adding >> more and more tests. > > Which tests are you seeing a slow-down for ? Well, everything that is using accel=tcg in tests/ could be accelerated. For example, the new ipv6/ppc64 unit test is quite slow with TCG: sudo QTEST_QEMU_BINARY=ppc64-softmmu/qemu-system-ppc64 time tests/pxe-test 48.46user 0.07system 0:48.48elapsed If I replace the "accel=tcg" with "accel=kvm" in pxe-test.c, the test runs much faster: 11.94user 0.38system 0:12.28elapsed Thomas
On 04/10/2016 16:18, Thomas Huth wrote: >>> >> Using only tcg has also some disadvantages: For some tests, it's >>> >> interesting to know whether they also work properly with KVM (e.g. >>> >> migration tests), and only using tcg by default slows down the "make >>> >> check" quite a bit - which might become an issue now that we're adding >>> >> more and more tests. >> > >> > Which tests are you seeing a slow-down for ? > Well, everything that is using accel=tcg in tests/ could be accelerated. > For example, the new ipv6/ppc64 unit test is quite slow with TCG: > > sudo QTEST_QEMU_BINARY=ppc64-softmmu/qemu-system-ppc64 time tests/pxe-test > > 48.46user 0.07system 0:48.48elapsed Could that point to a firmware bug? 10 network-bound seconds for a boot makes some sense, but 10 CPU-bound seconds don't... Paolo
On Tue, Oct 04, 2016 at 07:16:49PM +0200, Paolo Bonzini wrote: > > > On 04/10/2016 16:18, Thomas Huth wrote: > >>> >> Using only tcg has also some disadvantages: For some tests, it's > >>> >> interesting to know whether they also work properly with KVM (e.g. > >>> >> migration tests), and only using tcg by default slows down the "make > >>> >> check" quite a bit - which might become an issue now that we're adding > >>> >> more and more tests. > >> > > >> > Which tests are you seeing a slow-down for ? > > Well, everything that is using accel=tcg in tests/ could be accelerated. > > For example, the new ipv6/ppc64 unit test is quite slow with TCG: > > > > sudo QTEST_QEMU_BINARY=ppc64-softmmu/qemu-system-ppc64 time tests/pxe-test > > > > 48.46user 0.07system 0:48.48elapsed > > Could that point to a firmware bug? 10 network-bound seconds for a boot > makes some sense, but 10 CPU-bound seconds don't... > > Paolo AFAIK pxe firmware typically can't handle interrupts so it polls for packets.
On 04.10.2016 19:16, Paolo Bonzini wrote: > > > On 04/10/2016 16:18, Thomas Huth wrote: >>>>>> Using only tcg has also some disadvantages: For some tests, it's >>>>>> interesting to know whether they also work properly with KVM (e.g. >>>>>> migration tests), and only using tcg by default slows down the "make >>>>>> check" quite a bit - which might become an issue now that we're adding >>>>>> more and more tests. >>>> >>>> Which tests are you seeing a slow-down for ? >> Well, everything that is using accel=tcg in tests/ could be accelerated. >> For example, the new ipv6/ppc64 unit test is quite slow with TCG: >> >> sudo QTEST_QEMU_BINARY=ppc64-softmmu/qemu-system-ppc64 time tests/pxe-test >> >> 48.46user 0.07system 0:48.48elapsed > > Could that point to a firmware bug? 10 network-bound seconds for a boot > makes some sense, but 10 CPU-bound seconds don't... SLOF is incredibly slow with TCG - most parts are written in Forth that gets interpreted during runtime, and that seems to perform quite badly with TCG for some reasons. We're in progress of speeding up the boot process of SLOF a little bit (see https://github.com/aik/SLOF/commit/b3fde41bc75269df2 for example), but it will likely always be slower than a firmware that has been written in C only. Thomas
On 05/10/2016 03:13, Michael S. Tsirkin wrote: > On Tue, Oct 04, 2016 at 07:16:49PM +0200, Paolo Bonzini wrote: >> >> >> On 04/10/2016 16:18, Thomas Huth wrote: >>>>>>> Using only tcg has also some disadvantages: For some tests, it's >>>>>>> interesting to know whether they also work properly with KVM (e.g. >>>>>>> migration tests), and only using tcg by default slows down the "make >>>>>>> check" quite a bit - which might become an issue now that we're adding >>>>>>> more and more tests. >>>>> >>>>> Which tests are you seeing a slow-down for ? >>> Well, everything that is using accel=tcg in tests/ could be accelerated. >>> For example, the new ipv6/ppc64 unit test is quite slow with TCG: >>> >>> sudo QTEST_QEMU_BINARY=ppc64-softmmu/qemu-system-ppc64 time tests/pxe-test >>> >>> 48.46user 0.07system 0:48.48elapsed >> >> Could that point to a firmware bug? 10 network-bound seconds for a boot >> makes some sense, but 10 CPU-bound seconds don't... > > AFAIK pxe firmware typically can't handle interrupts > so it polls for packets. Still wouldn't be appreciably slower when emulating. Paolo
On 05/10/2016 09:00, Thomas Huth wrote: > On 04.10.2016 19:16, Paolo Bonzini wrote: >> >> >> On 04/10/2016 16:18, Thomas Huth wrote: >>>>>>> Using only tcg has also some disadvantages: For some tests, it's >>>>>>> interesting to know whether they also work properly with KVM (e.g. >>>>>>> migration tests), and only using tcg by default slows down the "make >>>>>>> check" quite a bit - which might become an issue now that we're adding >>>>>>> more and more tests. >>>>> >>>>> Which tests are you seeing a slow-down for ? >>> Well, everything that is using accel=tcg in tests/ could be accelerated. >>> For example, the new ipv6/ppc64 unit test is quite slow with TCG: >>> >>> sudo QTEST_QEMU_BINARY=ppc64-softmmu/qemu-system-ppc64 time tests/pxe-test >>> >>> 48.46user 0.07system 0:48.48elapsed >> >> Could that point to a firmware bug? 10 network-bound seconds for a boot >> makes some sense, but 10 CPU-bound seconds don't... > > SLOF is incredibly slow with TCG - most parts are written in Forth that > gets interpreted during runtime, and that seems to perform quite badly > with TCG for some reasons. We're in progress of speeding up the boot > process of SLOF a little bit (see > https://github.com/aik/SLOF/commit/b3fde41bc75269df2 for example), but > it will likely always be slower than a firmware that has been written in > C only. For what it's worth, just adding "-nodefaults -serial mon:stdio -net nic,model=virtio -net user" saves 7 seconds (30%) on the time needed to reach the OpenFirmware prompt. SCSI seems to be the slowest device to scan. Paolo
diff --git a/accel.c b/accel.c index 403eb5e..07a738d 100644 --- a/accel.c +++ b/accel.c @@ -60,6 +60,38 @@ static AccelClass *accel_find(const char *opt_name) return ac; } +static void accel_filter_best(ObjectClass *klass, void *opaque) +{ + AccelClass **best = opaque; + AccelClass *ac = ACCEL_CLASS(klass); + + if (ac->available && !ac->available()) { + return; + } + + if (ac->priority < 0) { + return; + } + + if (*best == NULL) { + *best = ac; + return; + } + + if (ac->priority > (*best)->priority) { + *best = ac; + } +} + +static AccelClass *accel_find_best(void) +{ + AccelClass *best = NULL; + + object_class_foreach(accel_filter_best, TYPE_ACCEL, false, &best); + + return ACCEL_CLASS(best); +} + static int accel_init_machine(AccelClass *acc, MachineState *ms) { ObjectClass *oc = OBJECT_CLASS(acc); @@ -97,15 +129,22 @@ void configure_accelerator(MachineState *ms) p++; } p = get_opt_name(buf, sizeof(buf), p, ':'); - acc = accel_find(buf); - if (!acc) { - fprintf(stderr, "\"%s\" accelerator not found.\n", buf); - continue; - } - if (acc->available && !acc->available()) { - printf("%s not supported for this target\n", - acc->name); - continue; + if (strcmp(buf, "best") == 0) { + acc = accel_find_best(); + if (acc == NULL) { + break; + } + } else { + acc = accel_find(buf); + if (!acc) { + fprintf(stderr, "\"%s\" accelerator not found.\n", buf); + continue; + } + if (acc->available && !acc->available()) { + printf("%s not supported for this target\n", + acc->name); + continue; + } } ret = accel_init_machine(acc, ms); if (ret < 0) { @@ -137,6 +176,7 @@ static void tcg_accel_class_init(ObjectClass *oc, void *data) ac->name = "tcg"; ac->init_machine = tcg_init; ac->allowed = &tcg_allowed; + ac->priority = 10; } #define TYPE_TCG_ACCEL ACCEL_CLASS_NAME("tcg") diff --git a/include/sysemu/accel.h b/include/sysemu/accel.h index 15944c1..5f2d7c9 100644 --- a/include/sysemu/accel.h +++ b/include/sysemu/accel.h @@ -40,6 +40,7 @@ typedef struct AccelClass { int (*available)(void); int (*init_machine)(MachineState *ms); bool *allowed; + int priority; } AccelClass; #define TYPE_ACCEL "accel" diff --git a/kvm-all.c b/kvm-all.c index fc2898a..1b7d82a 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -2460,6 +2460,7 @@ static void kvm_accel_class_init(ObjectClass *oc, void *data) ac->name = "KVM"; ac->init_machine = kvm_init; ac->allowed = &kvm_allowed; + ac->priority = 100; } static const TypeInfo kvm_accel_type = { diff --git a/qemu-options.hx b/qemu-options.hx index 01f01df..8ed7454 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -55,7 +55,9 @@ available machines. Supported machine properties are: This is used to enable an accelerator. Depending on the target architecture, kvm, xen, or tcg can be available. By default, tcg is used. If there is more than one accelerator specified, the next one is used if the previous one fails -to initialize. +to initialize. You can ask to use the best accelerator with "accel=best". If +you want to use one accelerator and if it is not available, the best one, you +can use, for instance, "accel=kvm:best". @item kernel_irqchip=on|off Controls in-kernel irqchip support for the chosen accelerator when available. @item gfx_passthru=on|off diff --git a/qtest.c b/qtest.c index 22482cc..4915f51 100644 --- a/qtest.c +++ b/qtest.c @@ -698,6 +698,7 @@ static void qtest_accel_class_init(ObjectClass *oc, void *data) ac->available = qtest_available; ac->init_machine = qtest_init_accel; ac->allowed = &qtest_allowed; + ac->priority = -1; } #define TYPE_QTEST_ACCEL ACCEL_CLASS_NAME("qtest") diff --git a/xen-common.c b/xen-common.c index e641ad1..19848be 100644 --- a/xen-common.c +++ b/xen-common.c @@ -140,6 +140,7 @@ static void xen_accel_class_init(ObjectClass *oc, void *data) ac->name = "Xen"; ac->init_machine = xen_init; ac->allowed = &xen_allowed; + ac->priority = 100; } #define TYPE_XEN_ACCEL ACCEL_CLASS_NAME("xen")
By default, QEMU uses 'tcg' or the one provided with the "accel" property. But sometime, user wants to use a real accelerator without knowing if he really can, with, for instance accel=kvm:tcg. In this case, and if the accelerator is not available we have a noisy "XXX accelerator not found". By allowing the user to ask the "best" accelerator for the given target, we can avoid this problem. This patch introduces a new parameter for the "accel" property, the "best" keyword. You can ask to use the best accelerator with "-M accel=best", or if you want to use your favorite accelerator and if it is not available, the best one, you can use, for instance "-M accel=kvm:best". Signed-off-by: Laurent Vivier <lvivier@redhat.com> --- accel.c | 58 ++++++++++++++++++++++++++++++++++++++++++-------- include/sysemu/accel.h | 1 + kvm-all.c | 1 + qemu-options.hx | 4 +++- qtest.c | 1 + xen-common.c | 1 + 6 files changed, 56 insertions(+), 10 deletions(-)