On Tue, 2 Dec 2025 at 16:09, Sebastian Ott <[email protected]> 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 <[email protected]> > Reviewed-by: Eric Auger <[email protected]> > --- > docs/system/arm/cpu-features.rst | 5 +++ > target/arm/cpu.h | 6 +++ > target/arm/kvm.c | 64 +++++++++++++++++++++++++++++++- > 3 files changed, 74 insertions(+), 1 deletion(-)
Hi; this patch seems generally reasonable to me as a way to handle this kind of "control" register; for more discussion I wrote a longer email in reply to Eric's series handling other kinds of migration failure: https://lore.kernel.org/qemu-devel/cafeaca-gi42jobojlupudabx8wrdwf5szsbkzu-bd06ecf1...@mail.gmail.com/T/#me03ebff8dbd8f58189cd98c3a21812781693277e Unless discussion in that thread reveals that we have so many of this kind of "control" knob that we would prefer a generic solution rather than per-knob user-friendly names and values, I'm OK with taking this patch without completely resolving the design discussion on that other series first. My review comments below are fairly minor, and patch 1 of this series has already been applied upstream now. > 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 I think we could be a little more detailed here. ``kvm-psci-version`` Set the Power State Coordination Interface (PSCI) firmware ABI version that KVM provides to the guest. By default KVM will use the newest version that it knows about (which is PSCI v1.3 in Linux v6.13). You only need to set this if you want to be able to migrate this VM to a host machine running an older kernel that does not recognize the PSCI version that this host's kernel defaults to. 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 39f2b2e54d..e2b6b587ea 100644 > --- a/target/arm/cpu.h > +++ b/target/arm/cpu.h > @@ -1035,6 +1035,12 @@ struct ArchCPU { > bool kvm_vtime_dirty; > uint64_t kvm_vtime; > > + /* > + * Intermediate value used during property parsing. > + * Once finalized, the value should be read from psci_version. > + */ > + uint32_t kvm_prop_psci_version; > + > /* KVM steal time */ > OnOffAuto kvm_steal_time; > > diff --git a/target/arm/kvm.c b/target/arm/kvm.c > index 0d57081e69..cf2de87287 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 { Nit: our coding style says struct type names should be in CamelCase, and structs should have a typedef rather than being used as "struct foo". > + 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 }, > +}; > + > +static char *kvm_get_psci_version(Object *obj, Error **errp) > +{ > + ARMCPU *cpu = ARM_CPU(obj); > + const struct psci_version *ver; > + > + for (ver = psci_versions; ver->number != -1; ver++) { Using ARRAY_SIZE() to set the loop bound is nicer than requiring a sentinel value at the end of the array. > + if (ver->number == cpu->psci_version) > + return g_strdup(ver->str); > + } > + > + return g_strdup_printf("Unknown PSCI-version: %x", cpu->psci_version); Is this ever possible? > +} > + > +static void kvm_set_psci_version(Object *obj, const char *value, Error > **errp) > +{ > + ARMCPU *cpu = ARM_CPU(obj); > + const struct psci_version *ver; > + > + for (ver = psci_versions; ver->number != -1; ver++) { > + if (!strcmp(value, ver->str)) { > + cpu->kvm_prop_psci_version = ver->number; > + return; > + } > + } > + > + error_setg(errp, "Invalid PSCI-version value"); "PSCI version". > +} > + > /* KVM VCPU properties should be prefixed with "kvm-". */ > void kvm_arm_add_vcpu_properties(ARMCPU *cpu) > { > @@ -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"); > } > > bool kvm_arm_pmu_supported(void) > @@ -1959,7 +2008,12 @@ int kvm_arch_init_vcpu(CPUState *cs) > if (cs->start_powered_off) { > cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_POWER_OFF; > } > - if (kvm_check_extension(cs->kvm_state, KVM_CAP_ARM_PSCI_0_2)) { > + if (cpu->kvm_prop_psci_version != QEMU_PSCI_VERSION_0_1 && > + kvm_check_extension(cs->kvm_state, KVM_CAP_ARM_PSCI_0_2)) { > + /* > + * Versions >= v0.2 are backward compatible with v0.2 > + * omit the feature flag for v0.1 . > + */ > cpu->psci_version = QEMU_PSCI_VERSION_0_2; > cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_PSCI_0_2; > } > @@ -1998,6 +2052,14 @@ int kvm_arch_init_vcpu(CPUState *cs) > } > } > > + if (cpu->kvm_prop_psci_version) { > + psciver = cpu->kvm_prop_psci_version; > + ret = kvm_set_one_reg(cs, KVM_REG_ARM_PSCI_VERSION, &psciver); > + if (ret) { > + error_report("PSCI version %"PRIx64" is not supported by KVM", > psciver); Could we make the PSCI version human-readable rather than hex here? If hex, we need the leading 0x. We can also suggest to the user the solution to this problem: error_report("KVM in this kernel does not support PSCI version 0x%" PRIx64 "); error_printf("Consider setting the kvm-psci-version property on the migration source.\n") (watch out that error_report() strings end without a trailing \n but error_printf() ones must have a \n.) thanks -- PMM
