[PATCH 1/6] target-arm: kvm: save/restore mp state

[Peter Maydell]

On 26 February 2015 at 01:02, Alex Bennée <alex.bennee at linaro.org> wrote:
> This adds the saving and restore of the current Multi-Processing state
> of the machine. While the KVM_GET/SET_MP_STATE API exposes a number of
> potential states for x86 we only use two for ARM. Either the process is
> running or not.

By this you mean "is the CPU in the PSCI powered down state or not",
right?

> Signed-off-by: Alex Bennée <alex.bennee at linaro.org>
>
> diff --git a/target-arm/kvm.c b/target-arm/kvm.c
> index 23cefe9..8732854 100644
> --- a/target-arm/kvm.c
> +++ b/target-arm/kvm.c
> @@ -25,6 +25,7 @@
>  #include "hw/arm/arm.h"
>
>  const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
> +    KVM_CAP_INFO(MP_STATE),

Does this really want to be a required cap? I haven't checked,
but assuming 'required' means what it says this presumably
means we'll stop working on host kernels we previously ran
fine on (even if migration didn't work there).

>      KVM_CAP_LAST_INFO
>  };
>
> diff --git a/target-arm/machine.c b/target-arm/machine.c
> index 9446e5a..70b1bc4 100644
> --- a/target-arm/machine.c
> +++ b/target-arm/machine.c
> @@ -161,6 +161,34 @@ static const VMStateInfo vmstate_cpsr = {
>      .put = put_cpsr,
>  };
>
> +#if defined CONFIG_KVM
> +static int get_mpstate(QEMUFile *f, void *opaque, size_t size)
> +{
> +    ARMCPU *cpu = opaque;
> +    struct kvm_mp_state mp_state = { .mp_state =  qemu_get_be32(f)};
> +    return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MP_STATE, &mp_state);
> +}

Won't this break if you're running a QEMU built with KVM
support in TCG mode on an aarch64 host?

In any case, for that configuration we should be migrating the
TCG cpu->powered_off flag, which is where we keep the PSCI
power state for TCG.

> +
> +static void put_mpstate(QEMUFile *f, void *opaque, size_t size)
> +{
> +    ARMCPU *cpu = opaque;
> +    struct kvm_mp_state mp_state;
> +    int ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_MP_STATE, &mp_state);
> +    if (ret) {
> +        fprintf(stderr, "%s: failed to get MP_STATE %d/%s\n",
> +                __func__, ret, strerror(ret));
> +        abort();
> +    }
> +    qemu_put_be32(f, mp_state.mp_state);
> +}
> +
> +static const VMStateInfo vmstate_mpstate = {
> +    .name = "mpstate",
> +    .get = get_mpstate,
> +    .put = put_mpstate,
> +};
> +#endif
> +
>  static void cpu_pre_save(void *opaque)
>  {
>      ARMCPU *cpu = opaque;
> @@ -244,6 +272,16 @@ const VMStateDescription vmstate_arm_cpu = {
>          VMSTATE_UINT32_ARRAY(env.regs, ARMCPU, 16),
>          VMSTATE_UINT64_ARRAY(env.xregs, ARMCPU, 32),
>          VMSTATE_UINT64(env.pc, ARMCPU),
> +#if defined CONFIG_KVM
> +        {
> +            .name = "mp_state",
> +            .version_id = 0,
> +            .size = sizeof(uint32_t),
> +            .info = &vmstate_mpstate,
> +            .flags = VMS_SINGLE,
> +            .offset = 0,
> +        },
> +#endif

This means the migration format will be different on different
build configurations, which seems like a really bad idea.

Have you considered having the KVM state sync functions just
sync the kernel's MP state into cpu->powered_off, and then
migrating that flag here unconditionally?

>          {
>              .name = "cpsr",
>              .version_id = 0,
> --
> 2.3.0
>

-- PMM