Re: [Qemu-devel] [RFC PATCH V5 6/8] target-arm: Enable KVM_ARM_VCPU_PSCI_0_2 feature when possible

[Peter Maydell <peter.maydell@linaro.org>]

On 12 May 2014 13:37, Pranavkumar Sawargaonkar <pranavkumar@linaro.org> wrote:
> Latest linux kernel supports in-kernel emulation of PSCI v0.2 but
> to enable it we need to select KVM_ARM_VCPU_PSCI_0_2 feature using
> KVM_ARM_VCPU_INIT ioctl.
>
> Also, we can use KVM_ARM_VCPU_PSCI_0_2 feature for VCPU only when
> linux kernel has KVM_CAP_ARM_PSCI_0_2 capability.
>
> This patch updates kvm_arch_init_vcpu() to enable KVM_ARM_VCPU_PSCI_0_2
> feature for VCPU when KVM ARM/ARM64 has KVM_CAP_ARM_PSCI_0_2 capability.

I was dubious in previous versions of this patchset about
whether we could reasonably just set the CPU feature if the
kernel reports the extension. However after a discussion on
#qemu with Paolo I'm now happier that it's OK.

The effect is that we will (silently) break migration between
hosts where one end supports PSCI 0.2 and the other does not.
However it's not unreasonable at this point in time to just
say that migration requires matching kernel versions.

At a later date we might want to introduce support for CPU
feature enable/disable subflags, at which point a command line
switch would allow running a QEMU with PSCI 0.1 even on a 0.2
supporting host kernel, to allow for back-compatible migration.
(Or it might turn out that by that point PSCI-0.1-only kernels
are a distant memory...)

> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
> Signed-off-by: Anup Patel <anup.patel@linaro.org>
> ---
>  target-arm/kvm32.c |    3 +++
>  target-arm/kvm64.c |    3 +++
>  2 files changed, 6 insertions(+)
>
> diff --git a/target-arm/kvm32.c b/target-arm/kvm32.c
> index cc9cdfa..afc248b 100644
> --- a/target-arm/kvm32.c
> +++ b/target-arm/kvm32.c
> @@ -179,6 +179,9 @@ int kvm_arch_init_vcpu(CPUState *cs)
>      if (cpu->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)) {
> +        cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_PSCI_0_2;
> +    }
>
>      /* Do KVM_ARM_VCPU_INIT ioctl */
>      ret = kvm_arm_vcpu_init(cs);
> diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
> index 528536f..917d256 100644
> --- a/target-arm/kvm64.c
> +++ b/target-arm/kvm64.c
> @@ -91,6 +91,9 @@ int kvm_arch_init_vcpu(CPUState *cs)
>      if (cpu->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)) {
> +        cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_PSCI_0_2;
> +    }
>
>      /* Do KVM_ARM_VCPU_INIT ioctl */
>      ret = kvm_arm_vcpu_init(cs);
> --
> 1.7.9.5

See also my review comments on patch 8: this patch is the place
to add the psci_version field to the ARMCPU struct, and set it
appropriately based on the KVM capabilities the kernel reports.

thanks
-- PMM