| Message ID | 20240701133038.1489043-2-dbarboza@ventanamicro.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | system/vl.c: parse all '-accel' opts | expand |
On 7/1/24 06:30, Daniel Henrique Barboza wrote: > We're allowing multiple -accel options to be used with different > accelerators, even though we don't have any machine that supports mixed > acceleration. > > In fact, it will only parse the first occurence of -accel and, aside > from a help option (e.g. -accel help) that will always cause the process > to print the help text, it will accept every other accel option > regardless of being correct or not. E.g. this: > > qemu-system-x86_64 -accel kvm -accel tcg -accel IamNotAnAccel (...) > > will happily boot a x86_64 KVM guest. > > Do not allow for different accelerators to be used when multiple > instances of -accel are present. > > Cc: Paolo Bonzini<pbonzini@redhat.com> > Cc: Thomas Huth<thuth@redhat.com> > Signed-off-by: Daniel Henrique Barboza<dbarboza@ventanamicro.com> > --- > system/vl.c | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) We use '-accel kvm -accel tcg' to allow kvm to fail (e.g. no /dev/kvm permission) and proceed with tcg. This patch will cause testsuite failures. r~
On 7/1/24 12:23 PM, Richard Henderson wrote: > On 7/1/24 06:30, Daniel Henrique Barboza wrote: >> We're allowing multiple -accel options to be used with different >> accelerators, even though we don't have any machine that supports mixed >> acceleration. >> >> In fact, it will only parse the first occurence of -accel and, aside >> from a help option (e.g. -accel help) that will always cause the process >> to print the help text, it will accept every other accel option >> regardless of being correct or not. E.g. this: >> >> qemu-system-x86_64 -accel kvm -accel tcg -accel IamNotAnAccel (...) >> >> will happily boot a x86_64 KVM guest. >> >> Do not allow for different accelerators to be used when multiple >> instances of -accel are present. >> >> Cc: Paolo Bonzini<pbonzini@redhat.com> >> Cc: Thomas Huth<thuth@redhat.com> >> Signed-off-by: Daniel Henrique Barboza<dbarboza@ventanamicro.com> >> --- >> system/vl.c | 15 ++++++++++++++- >> 1 file changed, 14 insertions(+), 1 deletion(-) > > We use '-accel kvm -accel tcg' to allow kvm to fail (e.g. no /dev/kvm permission) and proceed with tcg. > > This patch will cause testsuite failures. For the issue I want to fix patch 2 alone is enough. I'll re-send. Thanks, Daniel > > > r~
On Mon, Jul 1, 2024 at 5:53 PM Daniel Henrique Barboza <dbarboza@ventanamicro.com> wrote: > > We use '-accel kvm -accel tcg' to allow kvm to fail (e.g. no /dev/kvm permission) and proceed with tcg. > > > > This patch will cause testsuite failures. > > For the issue I want to fix patch 2 alone is enough. I'll re-send. It doesn't; it effectively changes '-accel kvm -accel tcg' to just '-accel tcg'. This is why you didn't see any failures, I think. Paolo
diff --git a/system/vl.c b/system/vl.c index bdd2f6ecf6..32602e68b7 100644 --- a/system/vl.c +++ b/system/vl.c @@ -3318,10 +3318,22 @@ void qemu_init(int argc, char **argv) } break; } - case QEMU_OPTION_accel: + case QEMU_OPTION_accel: { + QemuOptsList *list = qemu_find_opts("accel"); + const char *prev_accel = qemu_opt_get(QTAILQ_LAST(&list->head), + "accel"); + accel_opts = qemu_opts_parse_noisily(qemu_find_opts("accel"), optarg, true); optarg = qemu_opt_get(accel_opts, "accel"); + + if (prev_accel && optarg && strcmp(prev_accel, optarg)) { + printf("Unable to mix accelerators with multiple " + "-accel options (have: '%s' and '%s')\n", + prev_accel, optarg); + exit(1); + } + if (!optarg || is_help_option(optarg)) { printf("Accelerators supported in QEMU binary:\n"); GSList *el, *accel_list = object_class_get_list(TYPE_ACCEL, @@ -3343,6 +3355,7 @@ void qemu_init(int argc, char **argv) exit(0); } break; + } case QEMU_OPTION_usb: qdict_put_str(machine_opts_dict, "usb", "on"); break;
We're allowing multiple -accel options to be used with different accelerators, even though we don't have any machine that supports mixed acceleration. In fact, it will only parse the first occurence of -accel and, aside from a help option (e.g. -accel help) that will always cause the process to print the help text, it will accept every other accel option regardless of being correct or not. E.g. this: qemu-system-x86_64 -accel kvm -accel tcg -accel IamNotAnAccel (...) will happily boot a x86_64 KVM guest. Do not allow for different accelerators to be used when multiple instances of -accel are present. Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Thomas Huth <thuth@redhat.com> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> --- system/vl.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)