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

[Sebastian Ott <sebott@redhat.com>]

On Mon, 27 Oct 2025, Peter Maydell wrote:
> On Thu, 11 Sept 2025 at 15:49, Sebastian Ott <sebott@redhat.com> 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
>>
>> Signed-off-by: Sebastian Ott <sebott@redhat.com>
>> ---
>>  docs/system/arm/cpu-features.rst |  5 +++
>>  target/arm/cpu.h                 |  6 +++
>>  target/arm/kvm.c                 | 70 +++++++++++++++++++++++++++++++-
>>  3 files changed, 80 insertions(+), 1 deletion(-)
>>
>> diff --git a/docs/system/arm/cpu-features.rst b/docs/system/arm/cpu-features.rst
>> index 37d5dfd15b..1d32ce0fee 100644
>> --- a/docs/system/arm/cpu-features.rst
>> +++ b/docs/system/arm/cpu-features.rst
>> @@ -204,6 +204,11 @@ the list of KVM VCPU features and their descriptions.
>>    the guest scheduler behavior and/or be exposed to the guest
>>    userspace.
>>
>> +``kvm-psci-version``
>> +  Override the default (as of kernel v6.13 that would be PSCI v1.3)
>> +  PSCI version emulated by the kernel. Current valid values are:
>> +  0.1, 0.2, 1.0, 1.1, 1.2, and 1.3
>> +
>>  TCG VCPU Features
>>  =================
>>
>> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
>> index c15d79a106..44292aab32 100644
>> --- a/target/arm/cpu.h
>> +++ b/target/arm/cpu.h
>> @@ -974,6 +974,12 @@ struct ArchCPU {
>>       */
>>      uint32_t psci_version;
>>
>> +    /*
>> +     * Intermediate value used during property parsing.
>> +     * Once finalized, the value should be read from psci_version.
>> +     */
>> +    uint32_t prop_psci_version;
>> +
>>      /* Current power state, access guarded by BQL */
>>      ARMPSCIState power_state;
>>
>> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
>> index 6672344855..bc6073f395 100644
>> --- a/target/arm/kvm.c
>> +++ b/target/arm/kvm.c
>> @@ -483,6 +483,59 @@ 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;
>>  }
>>
>> +static char *kvm_get_psci_version(Object *obj, Error **errp)
>> +{
>> +    ARMCPU *cpu = ARM_CPU(obj);
>> +    const char *val;
>> +
>> +    switch (cpu->prop_psci_version) {
>> +    case QEMU_PSCI_VERSION_0_1:
>> +        val = "0.1";
>> +        break;
>> +    case QEMU_PSCI_VERSION_0_2:
>> +        val = "0.2";
>> +        break;
>> +    case QEMU_PSCI_VERSION_1_0:
>> +        val = "1.0";
>> +        break;
>> +    case QEMU_PSCI_VERSION_1_1:
>> +        val = "1.1";
>> +        break;
>> +    case QEMU_PSCI_VERSION_1_2:
>> +        val = "1.2";
>> +        break;
>> +    case QEMU_PSCI_VERSION_1_3:
>> +        val = "1.3";
>> +        break;
>> +    default:
>> +        val = "0.2";
>> +        break;
>> +    }
>> +    return g_strdup(val);
>> +}
>> +
>> +static void kvm_set_psci_version(Object *obj, const char *value, Error **errp)
>> +{
>> +    ARMCPU *cpu = ARM_CPU(obj);
>> +
>> +    if (!strcmp(value, "0.1")) {
>> +        cpu->prop_psci_version = QEMU_PSCI_VERSION_0_1;
>> +    } else if (!strcmp(value, "0.2")) {
>> +        cpu->prop_psci_version = QEMU_PSCI_VERSION_0_2;
>> +    } else if (!strcmp(value, "1.0")) {
>> +        cpu->prop_psci_version = QEMU_PSCI_VERSION_1_0;
>> +    } else if (!strcmp(value, "1.1")) {
>> +        cpu->prop_psci_version = QEMU_PSCI_VERSION_1_1;
>> +    } else if (!strcmp(value, "1.2")) {
>> +        cpu->prop_psci_version = QEMU_PSCI_VERSION_1_2;
>> +    } else if (!strcmp(value, "1.3")) {
>> +        cpu->prop_psci_version = QEMU_PSCI_VERSION_1_3;
>
> We already have six values here and it's not implausible
> we might end up with more in future; maybe we should make the
> mapping between string and constant data-driven rather
> than having code written out longhand in the get and set
> functions?

Yes, sure. I'll send out a V2.

Thanks!
Sebastian