Re: [PATCH v4 for v10.0.0] target/riscv/kvm/kvm-cpu: Fixed the issue of resume after QEMU+KVM migration

[Andrew Jones <ajones@ventanamicro.com>]

On Fri, Aug 01, 2025 at 05:52:20PM +0800, 谢波 wrote:
> This is v4 of this patch to add the function of saving and restoring the running status of vCPU during migration
> 
> This patch fixes two critical bugs in QEMU with KVM:
> Post-Migration Failure in User Mode: When QEMU with KVM is running in user mode, the guest may fail to function correctly after migration.
> Multi-Core Guest Inconsistency: After migration, only the first CPU (core 0) remains functional, while all other cores become unresponsive.
> This patch addresses both problems to ensure stable guest operation after migration.
> 
> Signed-off-by: Xie Bo <xb@ultrarisc.com>
> ---
>  target/riscv/cpu.h           |  1 +
>  target/riscv/kvm/kvm-cpu.c   | 63 ++++++++++++++++++++++++++++++++----
>  target/riscv/kvm/kvm_riscv.h |  3 +-
>  target/riscv/machine.c       |  1 +
>  4 files changed, 61 insertions(+), 7 deletions(-)
> 
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 51e49e03dec..1d7ad598faa 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -256,6 +256,7 @@ struct CPUArchState {
>  #endif
>  
>      target_ulong priv;
> +    uint32_t mp_state;  /*current multiprocessor state of this vCPU*/

This patch has several formatting problems, such as missing spaces between
the '/*' and the text. Also please capitalize first letters of sentences
and use punctuation, such as periods, in comments.

Try running scripts/checkpatch.pl to find formatting issues.

>      /* CSRs for execution environment configuration */
>      uint64_t menvcfg;
>      target_ulong senvcfg;
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index 0f4997a9186..c4c7c606a33 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -576,6 +576,15 @@ static int kvm_riscv_get_regs_core(CPUState *cs)
>      }
>      env->pc = reg;
>  
> +    /*Save the guest's privileged state before migration*/

This comment can be dropped since we know the purpose of get-regs.

> +    ret = kvm_get_one_reg(cs, RISCV_CORE_REG(env, mode), &reg);
> +    if (ret) {
> +        return ret;
> +    }
> +    if(reg != PRV_M) {

Missing space between 'if' and '('. Hopefully checkpatch complains about
stuff like that. Also priv == M should never be true for a KVM guest.

> +        env->priv = reg;
> +    }
> +
>      for (i = 1; i < 32; i++) {
>          uint64_t id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CORE, i);
>          ret = kvm_get_one_reg(cs, id, &reg);
> @@ -601,6 +610,16 @@ static int kvm_riscv_put_regs_core(CPUState *cs)
>          return ret;
>      }
>  
> +    /*Restore the guest's privileged state after migration*/

This comment can be dropped since we know the purpose of put-regs.

> +    reg = env->priv;
> +
> +    if(reg != PRV_M) {

Should never be true.

> +        ret = kvm_set_one_reg(cs, RISCV_CORE_REG(env, mode), &reg);
> +        if (ret) {
> +            return ret;
> +        }
> +    }
> +
>      for (i = 1; i < 32; i++) {
>          uint64_t id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CORE, i);
>          reg = env->gpr[i];
> @@ -1244,22 +1263,46 @@ int kvm_arch_get_registers(CPUState *cs, Error **errp)
>          return ret;
>      }
>  
> +    ret = kvm_riscv_sync_mpstate_to_qemu(cs);
> +    if (ret) {
> +        return ret;
> +    }
> +
>      return ret;
>  }
>  
> -int kvm_riscv_sync_mpstate_to_kvm(RISCVCPU *cpu, int state)
> +int kvm_riscv_sync_mpstate_to_kvm(CPUState *cs)
>  {
> +    CPURISCVState *env = &RISCV_CPU(cs)->env;

Need a blank line here.

>      if (cap_has_mp_state) {
>          struct kvm_mp_state mp_state = {
> -            .mp_state = state
> +            .mp_state = env->mp_state
>          };
>  
> -        int ret = kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MP_STATE, &mp_state);
> +        int ret = kvm_vcpu_ioctl(cs, KVM_SET_MP_STATE, &mp_state);

Put 'int ret' at the top of the function.

> +        if (ret) {
> +            fprintf(stderr, "%s: failed to sync MP_STATE %d/%s\n",

We should add "to KVM" to this error message.

> +                    __func__, ret, strerror(-ret));
> +            return -1;

We should return 'ret' instead of -1. So just change this to

 if (ret) {
     fprintf(...);
 }

 return ret;

> +        }
> +    }
> +
> +    return 0;
> +}
> +
> +int kvm_riscv_sync_mpstate_to_qemu(CPUState *cs)
> +{
> +    CPURISCVState *env = &RISCV_CPU(cs)->env;
> +    if (cap_has_mp_state) {
> +        struct kvm_mp_state mp_state;
> +
> +        int ret = kvm_vcpu_ioctl(cs, KVM_GET_MP_STATE, &mp_state);
>          if (ret) {
>              fprintf(stderr, "%s: failed to sync MP_STATE %d/%s\n",
>                      __func__, ret, strerror(-ret));
>              return -1;
>          }
> +        env->mp_state = mp_state.mp_state;
>      }

All the same comments as above for kvm_riscv_sync_mpstate_to_kvm.

>  
>      return 0;
> @@ -1290,16 +1333,24 @@ int kvm_arch_put_registers(CPUState *cs, int level, Error **errp)
>      }
>  
>      if (KVM_PUT_RESET_STATE == level) {
> -        RISCVCPU *cpu = RISCV_CPU(cs);
> +        CPURISCVState *env = &RISCV_CPU(cs)->env;
>          if (cs->cpu_index == 0) {
> -            ret = kvm_riscv_sync_mpstate_to_kvm(cpu, KVM_MP_STATE_RUNNABLE);
> +            env->mp_state = KVM_MP_STATE_RUNNABLE;
> +            ret = kvm_riscv_sync_mpstate_to_kvm(cs);
>          } else {
> -            ret = kvm_riscv_sync_mpstate_to_kvm(cpu, KVM_MP_STATE_STOPPED);
> +            env->mp_state = KVM_MP_STATE_STOPPED;
> +            ret = kvm_riscv_sync_mpstate_to_kvm(cs);
>          }
>          if (ret) {
>              return ret;
>          }
>      }
> +    else {

The else should be up after the '}'

> +        ret = kvm_riscv_sync_mpstate_to_kvm(cs);
> +        if (ret) {
> +            return ret;
> +        }
> +    }
>  
>      return ret;
>  }
> diff --git a/target/riscv/kvm/kvm_riscv.h b/target/riscv/kvm/kvm_riscv.h
> index b2bcd1041f6..953db941605 100644
> --- a/target/riscv/kvm/kvm_riscv.h
> +++ b/target/riscv/kvm/kvm_riscv.h
> @@ -28,7 +28,8 @@ void kvm_riscv_aia_create(MachineState *machine, uint64_t group_shift,
>                            uint64_t aplic_base, uint64_t imsic_base,
>                            uint64_t guest_num);
>  void riscv_kvm_aplic_request(void *opaque, int irq, int level);
> -int kvm_riscv_sync_mpstate_to_kvm(RISCVCPU *cpu, int state);
> +int kvm_riscv_sync_mpstate_to_kvm(CPUState *cs);
> +int kvm_riscv_sync_mpstate_to_qemu(CPUState *cs);
>  void riscv_kvm_cpu_finalize_features(RISCVCPU *cpu, Error **errp);
>  uint64_t kvm_riscv_get_timebase_frequency(RISCVCPU *cpu);
>  
> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> index 889e2b65701..22edd2dd744 100644
> --- a/target/riscv/machine.c
> +++ b/target/riscv/machine.c
> @@ -422,6 +422,7 @@ const VMStateDescription vmstate_riscv_cpu = {
>          VMSTATE_UNUSED(4),
>          VMSTATE_UINT32(env.misa_ext_mask, RISCVCPU),
>          VMSTATE_UINTTL(env.priv, RISCVCPU),
> +        VMSTATE_UINT32(env.mp_state, RISCVCPU),
>          VMSTATE_BOOL(env.virt_enabled, RISCVCPU),
>          VMSTATE_UINT64(env.resetvec, RISCVCPU),
>          VMSTATE_UINTTL(env.mhartid, RISCVCPU),

This requires a vmstate version bump.

None of the stuff below this point should be in the patch.

Thanks,
drew

> --
> 
> > -----原始邮件-----
> > 发件人: 谢波 <xb@ultrarisc.com>
> > 发送时间:2025-07-11 17:28:10 (星期五)
> > 收件人: qemu-devel@nongnu.org
> > 抄送: alistair.francis@wdc.com, palmer@dabbelt.com, pbonzini@redhat.com, anup@brainfault.org, anup.patel@wdc.com
> > 主题: [PATCH for v10.0.0] target/riscv/kvm/kvm-cpu: Fixed the issue of resume after QEMU+KVM migration
> > 
> > This is v3 of this patch to fix patch format
> > 
> > This patch fixes two critical issues in QEMU with KVM:
> > 1. Post-Migration Failure in User Mode: When QEMU with KVM is running in user mode, the guest may fail to function correctly after migration.
> > 2. Multi-Core Guest Inconsistency: After migration, only the first CPU (core 0) remains functional, while all other cores become unresponsive.
> > 
> > Changes include:
> > - Properly restoring guest privileged state during register synchronization.
> > - Correctly updating multi-core state after migration to ensure all cores are active.
> > 
> > Signed-off-by: Xie Bo <xb@ultrarisc.com>
> > ---
> >  target/riscv/kvm/kvm-cpu.c | 23 +++++++++++++++++++++++
> >  1 file changed, 23 insertions(+)
> > 
> > diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> > index 75724b6af4..a15caa20ce 100644
> > --- a/target/riscv/kvm/kvm-cpu.c
> > +++ b/target/riscv/kvm/kvm-cpu.c
> > @@ -576,6 +576,14 @@ static int kvm_riscv_get_regs_core(CPUState *cs)
> >      }
> >      env->pc = reg;
> > 
> > +    /*Restore the guest's privileged level after migration*/
> > +    ret = kvm_get_one_reg(cs, RISCV_CORE_REG(env, mode), &reg);
> > +    if (ret) {
> > +        return ret;
> > +    }
> > +    if(reg != 3) {
> > +        env->priv = reg;
> > +    }
> >      for (i = 1; i < 32; i++) {
> >          uint64_t id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CORE, i);
> >          ret = kvm_get_one_reg(cs, id, &reg);
> > @@ -601,6 +609,15 @@ static int kvm_riscv_put_regs_core(CPUState *cs)
> >          return ret;
> >      }
> > 
> > +    /*Save guest privilege level before migration*/
> > +    reg = env->priv;
> > +    if(reg != 3) {
> > +        ret = kvm_set_one_reg(cs, RISCV_CORE_REG(env, mode), &reg);
> > +        if (ret) {
> > +            return ret;
> > +        }
> > +    }
> > +
> >      for (i = 1; i < 32; i++) {
> >          uint64_t id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CORE, i);
> >          reg = env->gpr[i];
> > @@ -1289,6 +1306,12 @@ int kvm_arch_put_registers(CPUState *cs, int level, Error **errp)
> >          return ret;
> >      }
> > 
> > +    /*Ensure all non-core 0 CPUs are runnable after migration*/
> > +    if((level == KVM_PUT_FULL_STATE) && (cs->cpu_index != 0)){
> > +        RISCVCPU *cpu = RISCV_CPU(cs);
> > +        ret = kvm_riscv_sync_mpstate_to_kvm(cpu, KVM_MP_STATE_RUNNABLE);
> > +    }
> > +
> >      if (KVM_PUT_RESET_STATE == level) {
> >          RISCVCPU *cpu = RISCV_CPU(cs);
> >          if (cs->cpu_index == 0) {
> > --
> > 2.34.1
> > 
> > 
> > 
> > 
> > > -----原始邮件-----
> > > 发件人: 谢波 <xb@ultrarisc.com>
> > > 发送时间:2025-05-26 15:45:52 (星期一)
> > > 收件人: qemu-devel@nongnu.org
> > > 抄送: alistair.francis@wdc.com, palmer@dabbelt.com, pbonzini@redhat.com, anup@brainfault.org, anup.patel@wdc.com
> > > 主题: Re: [PATCH V2] target/riscv/kvm/kvm-cpu: Fixed the issue of resume after QEMU+KVM migration
> > > 
> > > This is v2 of this patch with no functional changes; adding CC.
> > > 
> > > ---
> > >  target/riscv/kvm/kvm-cpu.c | 23 +++++++++++++++++++++++
> > >  1 file changed, 23 insertions(+)
> > > 
> > > --- a/target/riscv/kvm/kvm-cpu.c
> > > +++ b/target/riscv/kvm/kvm-cpu.c
> > > @@ -576,6 +576,14 @@ static int kvm_riscv_get_regs_core(CPUState *cs)
> > >      }
> > >      env->pc = reg;
> > > 
> > > +    /* Restore guest privilege level after migration */
> > > +    ret = kvm_get_one_reg(cs, RISCV_CORE_REG(env, mode), &reg);
> > > +    if (ret) {
> > > +        return ret;
> > > +    }
> > > +    if (reg != 3) {
> > > +        env->priv = reg;
> > > +    }
> > > 
> > >      for (i = 1; i < 32; i++) {
> > >          uint64_t id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CORE, i);
> > >          ret = kvm_get_one_reg(cs, id, &reg);
> > > @@ -601,6 +609,15 @@ static int kvm_riscv_put_regs_core(CPUState *cs)
> > >          return ret;
> > >      }
> > > 
> > > +    /* Save guest privilege level before migration */
> > > +    reg = env->priv;
> > > +    if (reg != 3) {
> > > +        ret = kvm_set_one_reg(cs, RISCV_CORE_REG(env, mode), &reg);
> > > +        if (ret) {
> > > +            return ret;
> > > +        }
> > > +    }
> > > +
> > >      for (i = 1; i < 32; i++) {
> > >          uint64_t id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CORE, i);
> > >          reg = env->gpr[i];
> > > @@ -1289,6 +1306,12 @@ int kvm_arch_put_registers(CPUState *cs, int level, Error **errp)
> > >          return ret;
> > >      }
> > > 
> > > +    /* Ensure all non-core 0 CPUs are runnable after migration */
> > > +    if ((level == KVM_PUT_FULL_STATE) && (cs->cpu_index != 0)) {
> > > +        RISCVCPU *cpu = RISCV_CPU(cs);
> > > +        ret = kvm_riscv_sync_mpstate_to_kvm(cpu, KVM_MP_STATE_RUNNABLE);
> > > +        if (ret) {
> > > +            return ret;
> > > +        }
> > > +    }
> > > 
> > >      if (KVM_PUT_RESET_STATE == level) {
> > >          RISCVCPU *cpu = RISCV_CPU(cs);
> > >          if (cs->cpu_index == 0) {
> > > -- 
> > > 2.34.1
> > > 
> > > 
> > > 
> > > 
> > > > -----原始邮件-----
> > > > 发件人: 谢波 <xb@ultrarisc.com>
> > > > 发送时间:2025-05-19 17:41:36 (星期一)
> > > > 收件人: qemu-devel@nongnu.org
> > > > 抄送: alistair.francis@wdc.com, palmer@dabbelt.com, pbonzini@redhat.com
> > > > 主题: [PATCH] target/riscv/kvm/kvm-cpu: Fixed the issue of resume after QEMU+KVM migration
> > > > 
> > > > This patch fixes two critical issues in QEMU with KVM:
> > > > 
> > > > 1. Post-Migration Failure in User Mode: When QEMU with KVM is running in user mode, the guest may fail to function correctly after migration due to incorrect privilege state restoration.
> > > > 
> > > > 2. Multi-Core Guest Inconsistency: After migration, only the first CPU (core 0) remains functional, while all other cores become unresponsive. This patch ensures all cores are properly set to runnable state after migration.
> > > > 
> > > > Changes include:
> > > > - Properly restoring guest privileged state during register synchronization.
> > > > - Correctly updating multi-core state after migration to ensure all cores are active.
> > > > 
> > > > Signed-off-by: Xie Bo <xb@ultrarisc.com>
> > > > 
> > > > ---
> > > >  target/riscv/kvm/kvm-cpu.c | 23 +++++++++++++++++++++++
> > > >  1 file changed, 23 insertions(+)
> > > > 
> > > > --- a/target/riscv/kvm/kvm-cpu.c
> > > > +++ b/target/riscv/kvm/kvm-cpu.c
> > > > @@ -576,6 +576,14 @@ static int kvm_riscv_get_regs_core(CPUState *cs)
> > > >      }
> > > >      env->pc = reg;
> > > > 
> > > > +    /* Restore guest privilege level after migration */
> > > > +    ret = kvm_get_one_reg(cs, RISCV_CORE_REG(env, mode), &reg);
> > > > +    if (ret) {
> > > > +        return ret;
> > > > +    }
> > > > +    if (reg != 3) {
> > > > +        env->priv = reg;
> > > > +    }
> > > > 
> > > >      for (i = 1; i < 32; i++) {
> > > >          uint64_t id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CORE, i);
> > > >          ret = kvm_get_one_reg(cs, id, &reg);
> > > > @@ -601,6 +609,15 @@ static int kvm_riscv_put_regs_core(CPUState *cs)
> > > >          return ret;
> > > >      }
> > > > 
> > > > +    /* Save guest privilege level before migration */
> > > > +    reg = env->priv;
> > > > +    if (reg != 3) {
> > > > +        ret = kvm_set_one_reg(cs, RISCV_CORE_REG(env, mode), &reg);
> > > > +        if (ret) {
> > > > +            return ret;
> > > > +        }
> > > > +    }
> > > > +
> > > >      for (i = 1; i < 32; i++) {
> > > >          uint64_t id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CORE, i);
> > > >          reg = env->gpr[i];
> > > > @@ -1289,6 +1306,12 @@ int kvm_arch_put_registers(CPUState *cs, int level, Error **errp)
> > > >          return ret;
> > > >      }
> > > > 
> > > > +    /* Ensure all non-core 0 CPUs are runnable after migration */
> > > > +    if ((level == KVM_PUT_FULL_STATE) && (cs->cpu_index != 0)) {
> > > > +        RISCVCPU *cpu = RISCV_CPU(cs);
> > > > +        ret = kvm_riscv_sync_mpstate_to_kvm(cpu, KVM_MP_STATE_RUNNABLE);
> > > > +        if (ret) {
> > > > +            return ret;
> > > > +        }
> > > > +    }
> > > > 
> > > >      if (KVM_PUT_RESET_STATE == level) {
> > > >          RISCVCPU *cpu = RISCV_CPU(cs);
> > > >          if (cs->cpu_index == 0) {
> > > > -- 
> > > > 2.34.1
> > > > 
> > > > ______________________www.ultrarisc.com
> > > > 重要提示：本邮件包括附件的内容是受法律保护的保密信息，如果您不是指定收件人，请立即将本邮件删除，法律禁止任何非法的披露、复制、传播或以任何方式使用本邮件。本邮件中包含的意见、建议是基于或受到我方表达和定义的条款及条件的限定，如无我方的正式书面澄清或授权，不可被单独作为任何情形下的证据或依据。感谢您的理解与配合。版权所有。IMPORTANT NOTICE: This email, including its attachment if any, is confidential. If you are not the intended recipient, please delete it from your computer immediately. Any disclosure, copying, or distribution of this message, or taking of any action based on it is strictly prohibited.  Any opinions and suggestions contained in this email are subject to the terms and conditions expressed and defined by us and should not be relied upon unconditionally under any circumstances unless they are confirmed in official written clarification or authorization from us.  Thank you for your understanding and cooperation.All rights reserved.
> > > 
> > > 
> > > ______________________www.ultrarisc.com
> > > 重要提示：本邮件包括附件的内容是受法律保护的保密信息，如果您不是指定收件人，请立即将本邮件删除，法律禁止任何非法的披露、复制、传播或以任何方式使用本邮件。本邮件中包含的意见、建议是基于或受到我方表达和定义的条款及条件的限定，如无我方的正式书面澄清或授权，不可被单独作为任何情形下的证据或依据。感谢您的理解与配合。版权所有。IMPORTANT NOTICE: This email, including its attachment if any, is confidential. If you are not the intended recipient, please delete it from your computer immediately. Any disclosure, copying, or distribution of this message, or taking of any action based on it is strictly prohibited.  Any opinions and suggestions contained in this email are subject to the terms and conditions expressed and defined by us and should not be relied upon unconditionally under any circumstances unless they are confirmed in official written clarification or authorization from us.  Thank you for your understanding and cooperation.All rights reserved.
> > 
> > 
> > ______________________www.ultrarisc.com
> > 重要提示：本邮件包括附件的内容是受法律保护的保密信息，如果您不是指定收件人，请立即将本邮件删除，法律禁止任何非法的披露、复制、传播或以任何方式使用本邮件。本邮件中包含的意见、建议是基于或受到我方表达和定义的条款及条件的限定，如无我方的正式书面澄清或授权，不可被单独作为任何情形下的证据或依据。感谢您的理解与配合。版权所有。IMPORTANT NOTICE: This email, including its attachment if any, is confidential. If you are not the intended recipient, please delete it from your computer immediately. Any disclosure, copying, or distribution of this message, or taking of any action based on it is strictly prohibited.  Any opinions and suggestions contained in this email are subject to the terms and conditions expressed and defined by us and should not be relied upon unconditionally under any circumstances unless they are confirmed in official written clarification or authorization from us.  Thank you for your understanding and cooperation.All rights reserved.
> 
> 
> ______________________www.ultrarisc.com
> 重要提示：本邮件包括附件的内容是受法律保护的保密信息，如果您不是指定收件人，请立即将本邮件删除，法律禁止任何非法的披露、复制、传播或以任何方式使用本邮件。本邮件中包含的意见、建议是基于或受到我方表达和定义的条款及条件的限定，如无我方的正式书面澄清或授权，不可被单独作为任何情形下的证据或依据。感谢您的理解与配合。版权所有。IMPORTANT NOTICE: This email, including its attachment if any, is confidential. If you are not the intended recipient, please delete it from your computer immediately. Any disclosure, copying, or distribution of this message, or taking of any action based on it is strictly prohibited.  Any opinions and suggestions contained in this email are subject to the terms and conditions expressed and defined by us and should not be relied upon unconditionally under any circumstances unless they are confirmed in official written clarification or authorization from us.  Thank you for your understanding and cooperation.All rights reserved.