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(-)

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;
-- 
2.45.2

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