Boolean properties are preferrable in comparision to string properties
since they don't require a string parsing.
Add three bools that represents the available kvm-aia mode:
riscv-aia-emul, riscv-aia-hwaccel, riscv-aia-auto. They work like the
existing riscv-aia string property, i.e. if no bool is set we'll default
to riscv-aia-auto, if the host supports it.
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
target/riscv/kvm/kvm-cpu.c | 77 ++++++++++++++++++++++++++++++++++++++
1 file changed, 77 insertions(+)
diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index 32f3dd6a43..e256e3fc48 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -1671,6 +1671,62 @@ static void riscv_set_kvm_aia(Object *obj, const char *val, Error **errp)
}
}
+static void riscv_set_kvm_aia_bool(uint32_t aia_bool, bool val)
+{
+ bool default_aia_mode = KVM_DEV_RISCV_AIA_MODE_AUTO;
+
+ g_assert(aia_bool <= KVM_DEV_RISCV_AIA_MODE_AUTO);
+
+ if (val) {
+ aia_mode = aia_bool;
+ return;
+ }
+
+ /*
+ * Setting an aia_bool to 'false' does nothing if
+ * aia_mode isn't set to aia_bool.
+ */
+ if (aia_mode != aia_bool) {
+ return;
+ }
+
+ /*
+ * Return to default value if we're disabling the
+ * current set aia_mode.
+ */
+ aia_mode = default_aia_mode;
+}
+
+static bool riscv_get_kvm_aia_emul(Object *obj, Error **errp)
+{
+ return aia_mode == KVM_DEV_RISCV_AIA_MODE_EMUL;
+}
+
+static void riscv_set_kvm_aia_emul(Object *obj, bool val, Error **errp)
+{
+ riscv_set_kvm_aia_bool(KVM_DEV_RISCV_AIA_MODE_EMUL, val);
+}
+
+static bool riscv_get_kvm_aia_hwaccel(Object *obj, Error **errp)
+{
+ return aia_mode == KVM_DEV_RISCV_AIA_MODE_HWACCEL;
+}
+
+static void riscv_set_kvm_aia_hwaccel(Object *obj, bool val, Error **errp)
+{
+ riscv_set_kvm_aia_bool(KVM_DEV_RISCV_AIA_MODE_HWACCEL, val);
+}
+
+static bool riscv_get_kvm_aia_auto(Object *obj, Error **errp)
+{
+ return aia_mode == KVM_DEV_RISCV_AIA_MODE_AUTO;
+}
+
+static void riscv_set_kvm_aia_auto(Object *obj, bool val, Error **errp)
+{
+ riscv_set_kvm_aia_bool(KVM_DEV_RISCV_AIA_MODE_AUTO, val);
+}
+
void kvm_arch_accel_class_init(ObjectClass *oc)
{
object_class_property_add_str(oc, "riscv-aia", riscv_get_kvm_aia,
@@ -1681,6 +1737,27 @@ void kvm_arch_accel_class_init(ObjectClass *oc)
"if the host supports it");
object_property_set_default_str(object_class_property_find(oc, "riscv-aia"),
"auto");
+
+ object_class_property_add_bool(oc, "riscv-aia-emul",
+ riscv_get_kvm_aia_emul,
+ riscv_set_kvm_aia_emul);
+ object_class_property_set_description(oc, "riscv-aia-emul",
+ "Set KVM AIA mode to 'emul'. Changing KVM AIA modes relies on host "
+ "support. Default mode is 'auto' if the host supports it");
+
+ object_class_property_add_bool(oc, "riscv-aia-hwaccel",
+ riscv_get_kvm_aia_hwaccel,
+ riscv_set_kvm_aia_hwaccel);
+ object_class_property_set_description(oc, "riscv-aia-hwaccel",
+ "Set KVM AIA mode to 'hwaccel'. Changing KVM AIA modes relies on host "
+ "support. Default mode is 'auto' if the host supports it");
+
+ object_class_property_add_bool(oc, "riscv-aia-auto",
+ riscv_get_kvm_aia_auto,
+ riscv_set_kvm_aia_auto);
+ object_class_property_set_description(oc, "riscv-aia-auto",
+ "Set KVM AIA mode to 'auto'. Changing KVM AIA modes "
+ "relies on host support");
}
void kvm_riscv_aia_create(MachineState *machine, uint64_t group_shift,
--
2.45.2On Tue, Sep 24, 2024 at 10:46 PM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> Boolean properties are preferrable in comparision to string properties
> since they don't require a string parsing.
>
> Add three bools that represents the available kvm-aia mode:
> riscv-aia-emul, riscv-aia-hwaccel, riscv-aia-auto. They work like the
> existing riscv-aia string property, i.e. if no bool is set we'll default
> to riscv-aia-auto, if the host supports it.
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
> target/riscv/kvm/kvm-cpu.c | 77 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 77 insertions(+)
>
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index 32f3dd6a43..e256e3fc48 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -1671,6 +1671,62 @@ static void riscv_set_kvm_aia(Object *obj, const char *val, Error **errp)
> }
> }
>
> +static void riscv_set_kvm_aia_bool(uint32_t aia_bool, bool val)
> +{
> + bool default_aia_mode = KVM_DEV_RISCV_AIA_MODE_AUTO;
> +
> + g_assert(aia_bool <= KVM_DEV_RISCV_AIA_MODE_AUTO);
> +
> + if (val) {
> + aia_mode = aia_bool;
> + return;
> + }
> +
> + /*
> + * Setting an aia_bool to 'false' does nothing if
> + * aia_mode isn't set to aia_bool.
> + */
> + if (aia_mode != aia_bool) {
> + return;
> + }
> +
> + /*
> + * Return to default value if we're disabling the
> + * current set aia_mode.
> + */
> + aia_mode = default_aia_mode;
> +}
> +
> +static bool riscv_get_kvm_aia_emul(Object *obj, Error **errp)
> +{
> + return aia_mode == KVM_DEV_RISCV_AIA_MODE_EMUL;
> +}
> +
> +static void riscv_set_kvm_aia_emul(Object *obj, bool val, Error **errp)
> +{
> + riscv_set_kvm_aia_bool(KVM_DEV_RISCV_AIA_MODE_EMUL, val);
> +}
> +
> +static bool riscv_get_kvm_aia_hwaccel(Object *obj, Error **errp)
> +{
> + return aia_mode == KVM_DEV_RISCV_AIA_MODE_HWACCEL;
> +}
> +
> +static void riscv_set_kvm_aia_hwaccel(Object *obj, bool val, Error **errp)
> +{
> + riscv_set_kvm_aia_bool(KVM_DEV_RISCV_AIA_MODE_HWACCEL, val);
> +}
> +
> +static bool riscv_get_kvm_aia_auto(Object *obj, Error **errp)
> +{
> + return aia_mode == KVM_DEV_RISCV_AIA_MODE_AUTO;
> +}
> +
> +static void riscv_set_kvm_aia_auto(Object *obj, bool val, Error **errp)
> +{
> + riscv_set_kvm_aia_bool(KVM_DEV_RISCV_AIA_MODE_AUTO, val);
> +}
> +
> void kvm_arch_accel_class_init(ObjectClass *oc)
> {
> object_class_property_add_str(oc, "riscv-aia", riscv_get_kvm_aia,
> @@ -1681,6 +1737,27 @@ void kvm_arch_accel_class_init(ObjectClass *oc)
> "if the host supports it");
> object_property_set_default_str(object_class_property_find(oc, "riscv-aia"),
> "auto");
> +
> + object_class_property_add_bool(oc, "riscv-aia-emul",
> + riscv_get_kvm_aia_emul,
> + riscv_set_kvm_aia_emul);
> + object_class_property_set_description(oc, "riscv-aia-emul",
> + "Set KVM AIA mode to 'emul'. Changing KVM AIA modes relies on host "
> + "support. Default mode is 'auto' if the host supports it");
> +
> + object_class_property_add_bool(oc, "riscv-aia-hwaccel",
> + riscv_get_kvm_aia_hwaccel,
> + riscv_set_kvm_aia_hwaccel);
> + object_class_property_set_description(oc, "riscv-aia-hwaccel",
> + "Set KVM AIA mode to 'hwaccel'. Changing KVM AIA modes relies on host "
> + "support. Default mode is 'auto' if the host supports it");
> +
> + object_class_property_add_bool(oc, "riscv-aia-auto",
> + riscv_get_kvm_aia_auto,
> + riscv_set_kvm_aia_auto);
> + object_class_property_set_description(oc, "riscv-aia-auto",
> + "Set KVM AIA mode to 'auto'. Changing KVM AIA modes "
> + "relies on host support");
This seems more confusing. What should happen if a user sets multiple to true?
Alistair
> }
>
> void kvm_riscv_aia_create(MachineState *machine, uint64_t group_shift,
> --
> 2.45.2
>
>
On 10/10/24 10:57 PM, Alistair Francis wrote:
> On Tue, Sep 24, 2024 at 10:46 PM Daniel Henrique Barboza
> <dbarboza@ventanamicro.com> wrote:
>>
>> Boolean properties are preferrable in comparision to string properties
>> since they don't require a string parsing.
>>
>> Add three bools that represents the available kvm-aia mode:
>> riscv-aia-emul, riscv-aia-hwaccel, riscv-aia-auto. They work like the
>> existing riscv-aia string property, i.e. if no bool is set we'll default
>> to riscv-aia-auto, if the host supports it.
>>
>> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
>> ---
>> target/riscv/kvm/kvm-cpu.c | 77 ++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 77 insertions(+)
>>
>> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
>> index 32f3dd6a43..e256e3fc48 100644
>> --- a/target/riscv/kvm/kvm-cpu.c
>> +++ b/target/riscv/kvm/kvm-cpu.c
>> @@ -1671,6 +1671,62 @@ static void riscv_set_kvm_aia(Object *obj, const char *val, Error **errp)
>> }
>> }
>>
>> +static void riscv_set_kvm_aia_bool(uint32_t aia_bool, bool val)
>> +{
>> + bool default_aia_mode = KVM_DEV_RISCV_AIA_MODE_AUTO;
>> +
>> + g_assert(aia_bool <= KVM_DEV_RISCV_AIA_MODE_AUTO);
>> +
>> + if (val) {
>> + aia_mode = aia_bool;
>> + return;
>> + }
>> +
>> + /*
>> + * Setting an aia_bool to 'false' does nothing if
>> + * aia_mode isn't set to aia_bool.
>> + */
>> + if (aia_mode != aia_bool) {
>> + return;
>> + }
>> +
>> + /*
>> + * Return to default value if we're disabling the
>> + * current set aia_mode.
>> + */
>> + aia_mode = default_aia_mode;
>> +}
>> +
>> +static bool riscv_get_kvm_aia_emul(Object *obj, Error **errp)
>> +{
>> + return aia_mode == KVM_DEV_RISCV_AIA_MODE_EMUL;
>> +}
>> +
>> +static void riscv_set_kvm_aia_emul(Object *obj, bool val, Error **errp)
>> +{
>> + riscv_set_kvm_aia_bool(KVM_DEV_RISCV_AIA_MODE_EMUL, val);
>> +}
>> +
>> +static bool riscv_get_kvm_aia_hwaccel(Object *obj, Error **errp)
>> +{
>> + return aia_mode == KVM_DEV_RISCV_AIA_MODE_HWACCEL;
>> +}
>> +
>> +static void riscv_set_kvm_aia_hwaccel(Object *obj, bool val, Error **errp)
>> +{
>> + riscv_set_kvm_aia_bool(KVM_DEV_RISCV_AIA_MODE_HWACCEL, val);
>> +}
>> +
>> +static bool riscv_get_kvm_aia_auto(Object *obj, Error **errp)
>> +{
>> + return aia_mode == KVM_DEV_RISCV_AIA_MODE_AUTO;
>> +}
>> +
>> +static void riscv_set_kvm_aia_auto(Object *obj, bool val, Error **errp)
>> +{
>> + riscv_set_kvm_aia_bool(KVM_DEV_RISCV_AIA_MODE_AUTO, val);
>> +}
>> +
>> void kvm_arch_accel_class_init(ObjectClass *oc)
>> {
>> object_class_property_add_str(oc, "riscv-aia", riscv_get_kvm_aia,
>> @@ -1681,6 +1737,27 @@ void kvm_arch_accel_class_init(ObjectClass *oc)
>> "if the host supports it");
>> object_property_set_default_str(object_class_property_find(oc, "riscv-aia"),
>> "auto");
>> +
>> + object_class_property_add_bool(oc, "riscv-aia-emul",
>> + riscv_get_kvm_aia_emul,
>> + riscv_set_kvm_aia_emul);
>> + object_class_property_set_description(oc, "riscv-aia-emul",
>> + "Set KVM AIA mode to 'emul'. Changing KVM AIA modes relies on host "
>> + "support. Default mode is 'auto' if the host supports it");
>> +
>> + object_class_property_add_bool(oc, "riscv-aia-hwaccel",
>> + riscv_get_kvm_aia_hwaccel,
>> + riscv_set_kvm_aia_hwaccel);
>> + object_class_property_set_description(oc, "riscv-aia-hwaccel",
>> + "Set KVM AIA mode to 'hwaccel'. Changing KVM AIA modes relies on host "
>> + "support. Default mode is 'auto' if the host supports it");
>> +
>> + object_class_property_add_bool(oc, "riscv-aia-auto",
>> + riscv_get_kvm_aia_auto,
>> + riscv_set_kvm_aia_auto);
>> + object_class_property_set_description(oc, "riscv-aia-auto",
>> + "Set KVM AIA mode to 'auto'. Changing KVM AIA modes "
>> + "relies on host support");
>
> This seems more confusing. What should happen if a user sets multiple to true?
It'll work like most options in QEMU: the last setting will overwrite the previous
ones. "-accel kvm,riscv-aia-hwaccel=true,riscv-aia-emul=true" will set the mode
to 'emul'. This is the same behavior that we have with the existing 'riscv-aia'
string option.
In case someone tries it out with multiple -accel options, this doesn't work. Only
the first '-accel <type>' are parsed. This happens due to a known command line
parsing/accel globals issue that I tried to fix in [1] and [2].
For now, using the existing 'riscv-aia' string option:
-accel kvm,riscv-aia=emul -accel kvm,riscv-aia=hwaccel -accel kvm,riscv-aia=auto
This will set riscv-aia to "emul" because all other "-accel kvm" options aren't
being parsed. You can do silly stuff like:
-accel kvm,riscv-aia=emul -accel kvm,riscv-aia=this_is_not_an_option
And the guest will boot normally, setting riscv-aia to 'emul'.
Thanks,
Daniel
[1] "[PATCH 0/2] system/vl.c: parse all '-accel' opts"
https://lore.kernel.org/qemu-devel/20240701133038.1489043-1-dbarboza@ventanamicro.com/
[2] "[PATCH v2 0/2] object,accel-system: allow Accel type globals"
https://lore.kernel.org/qemu-devel/20240703204149.1957136-1-dbarboza@ventanamicro.com/
>
> Alistair
>
>> }
>>
>> void kvm_riscv_aia_create(MachineState *machine, uint64_t group_shift,
>> --
>> 2.45.2
>>
>>
© 2016 - 2024 Red Hat, Inc.