Re: [PATCH v3 2/2] target/arm/kvm: add kvm-psci-version vcpu property

[Sebastian Ott <sebott@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2863 bytes --]

Hi Philippe,

On Wed, 12 Nov 2025, Philippe Mathieu-Daudé wrote:
> On 12/11/25 19:13, Sebastian Ott wrote:
>>  Provide a kvm specific vcpu property to override the default
>>  (as of kernel v6.13 that would be PSCI v1.3) PSCI version emulated
>>  by kvm. Current valid values are: 0.1, 0.2, 1.0, 1.1, 1.2, and 1.3
>>
>>  Note: in order to support PSCI v0.1 we need to drop vcpu
>>  initialization with KVM_CAP_ARM_PSCI_0_2 in that case.
>>
>>  Signed-off-by: Sebastian Ott <sebott@redhat.com>
>>  ---
>>    docs/system/arm/cpu-features.rst |  5 +++
>>    target/arm/cpu.h                 |  6 +++
>>    target/arm/kvm.c                 | 64 +++++++++++++++++++++++++++++++-
>>    3 files changed, 74 insertions(+), 1 deletion(-)
>
>
>>  diff --git a/target/arm/kvm.c b/target/arm/kvm.c
>>  index 0d57081e69..e91b1abfb8 100644
>>  --- a/target/arm/kvm.c
>>  +++ b/target/arm/kvm.c
>>  @@ -484,6 +484,49 @@ static void kvm_steal_time_set(Object *obj, bool
>>  value, Error **errp)
>>        ARM_CPU(obj)->kvm_steal_time = value ? ON_OFF_AUTO_ON :
>>    ON_OFF_AUTO_OFF;
>>    }
>>
>>  +struct psci_version {
>>  +    uint32_t number;
>>  +    const char *str;
>>  +};
>>  +
>>  +static const struct psci_version psci_versions[] = {
>>  +    { QEMU_PSCI_VERSION_0_1, "0.1" },
>>  +    { QEMU_PSCI_VERSION_0_2, "0.2" },
>>  +    { QEMU_PSCI_VERSION_1_0, "1.0" },
>>  +    { QEMU_PSCI_VERSION_1_1, "1.1" },
>>  +    { QEMU_PSCI_VERSION_1_2, "1.2" },
>>  +    { QEMU_PSCI_VERSION_1_3, "1.3" },
>>  +    { -1, NULL },
>>  +};
>
>
>>  @@ -505,6 +548,12 @@ void kvm_arm_add_vcpu_properties(ARMCPU *cpu)
>>                                 kvm_steal_time_set);
>>        object_property_set_description(obj, "kvm-steal-time",
>>                                        "Set off to disable KVM steal
>>  time.");
>>  +
>>  +    object_property_add_str(obj, "kvm-psci-version",
>>  kvm_get_psci_version,
>>  +                            kvm_set_psci_version);
>>  +    object_property_set_description(obj, "kvm-psci-version",
>>  +                                    "Set PSCI version. "
>>  +                                    "Valid values are 0.1, 0.2, 1.0, 1.1,
>>  1.2, 1.3");
>
> Could we enumerate from psci_versions[] here?
>

Hm, we'd need to concatenate these. Either manually:
"Valid values are " psci_versions[0].str ", " psci_versions[1].str ", " ... 
which is not pretty and still needs to be touched for a new version.

Or by a helper function that puts these in a new array and uses smth like
g_strjoinv(", ", array);
But that's quite a bit of extra code that needs to be maintained without
much gain.

Or we shy away from the issue and rephrase that to:
"Valid values include 1.0, 1.1, 1.2, 1.3"

Since the intended use case is via machine types and I don't expect a
lot of users setting the psci version manually - I vote for option 3.

Opinions?

Sebastian