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

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 20/11/25 11:10, Eric Auger wrote:
> 
> 
> On 11/13/25 1:05 PM, Sebastian Ott wrote:
>> 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"
> Personally I would vote for keeping it as is

OK.

> (by the way why did you
> moit 0.1 and 0.2 above?)
> 
> Eric
>>
>> 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
>