I apologize for the delay.
Daniel Henrique Barboza <dbarb...@ventanamicro.com> writes:
> We're not honouring KVM options that are provided by any -accel option
> aside from the first. In this example:
>
> qemu-system-riscv64 -accel kvm,riscv-aia=emul (...) \
> -accel kvm,riscv-aia=hwaccel
>
> 'riscv-aia' will be set to 'emul', ignoring the last occurrence of the
> option that set 'riscv-aia' to 'hwaccel'.
The way you phrase this, it sounds like a bug. But as far as I know,
-accel is meant to have fallback semantics: we use the first one that
works.
Perhaps:
-accel has fallback semantics, i.e. we try accelerators in order until
we find one that works. Any remainder is ignored.
Because of that, you can't override properties like this:
qemu-system-riscv64 -accel kvm,riscv-aia=emul (...) \
-accel kvm,riscv-aia=hwaccel
When KVM is available, 'riscv-aia' will be set to 'emul', and the
second -accel is ignored. When KVM is not available, neither option
works, and the command fails.
Why would you want to override accelerator properties?
Aside: not diagnosing obvious errors in fallback options we don't use is
not nice. Not this patch's problem.
> A failed attempt to solve this issue by setting 'merge_lists' can be
> found in [1].
I guess this failed because it destroyed the fallback semantics.
Correct?
> A different approach was suggested in [2]: enable global
> properties for accelerators. This allows an user to override any accel
> setting by using '-global' and we won't need to change how '-accel' opts
> are handled.
>
> This is done by calling object_prop_set_globals() in
> accel_init_machine(). As done in device_post_init(), user's global
> props take precedence over compat props so object_prop_set_globals() is
> called after object_set_accelerator_compat_props().
>
> object_prop_check_globals() is then changed to recognize TYPE_ACCEL
> globals as valid.
Would it make sense to enable -global for user-creatable objects
(-object), too?
> Re-using the fore-mentioned example we'll be able to set
> riscv-aia=hwaccel by doing the following:
>
> qemu-system-riscv64 -accel kvm,riscv-aia=emul (...) \
> -global kvm-accel.riscv-aia=hwaccel
I'm confused.
-accel kvm,riscv-aia=emul creates accelerator kvm (which is an instance
of class "kvm-accel") and sets its property "riscv-aia" to "emul".
-global kvm-accel,riscv-aia=hwaccel changes the (global) default value
of class "kvm-accel" property "riscv-aia".
Are you *sure* your example sets "riscv-aia" to "hwaccel"?
> [1]
> https://lore.kernel.org/qemu-devel/20240701133038.1489043-1-dbarb...@ventanamicro.com/
> [2]
> https://lore.kernel.org/qemu-devel/CABgObfbVJkCdpHV2uA7LGTbYEaoaizrbeG0vNo_T1ua5b=j...@mail.gmail.com/
>
> Reported-by: Andrew Jones <ajo...@ventanamicro.com>
> Suggested-by: Paolo Bonzini <pbonz...@redhat.com>
> Signed-off-by: Daniel Henrique Barboza <dbarb...@ventanamicro.com>
