qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v9 0/2] target/riscv:Fix riscv64 kvm migration
@ 2025-09-15  7:08 Xie Bo
  2025-09-15  7:08 ` [PATCH v9 1/2] Set KVM initial privilege mode and mp_state Xie Bo
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Xie Bo @ 2025-09-15  7:08 UTC (permalink / raw)
  To: qemu-devel
  Cc: ajones, qemu-riscv, alistair23, pbonzini, anup, alistair.francis,
	rkrcmar, palmer, xiamy, Xie Bo

This is v9 of the series. Compared to v8, the patches are now based on
the 'riscv-to-apply.next' branch from Alistair's repository:

https://github.com/alistair23/qemu/tree/riscv-to-apply.next

Changes since v8:
- Rebased the series onto [alistair/riscv-to-apply.next]
- Removed the previous 'Reviewed-by' tags due to the rebase
  * The changes are purely mechanical; no code logic was altered *
- Added 'Cc: qemu-stable@nongnu.org'

Xie Bo (2):
  Set KVM initial privilege mode and mp_state
  Fix VM resume after QEMU+KVM migration

 target/riscv/cpu.c           | 17 +++++++++-
 target/riscv/cpu.h           |  2 ++
 target/riscv/kvm/kvm-cpu.c   | 60 ++++++++++++++++++++++++++++--------
 target/riscv/kvm/kvm_riscv.h |  3 +-
 target/riscv/machine.c       |  5 +--
 5 files changed, 70 insertions(+), 17 deletions(-)

-- 
2.43.0



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v9 1/2] Set KVM initial privilege mode and mp_state
  2025-09-15  7:08 [PATCH v9 0/2] target/riscv:Fix riscv64 kvm migration Xie Bo
@ 2025-09-15  7:08 ` Xie Bo
  2025-09-16  8:00   ` Radim Krčmář
                     ` (2 more replies)
  2025-09-15  7:08 ` [PATCH v9 2/2] Fix VM resume after QEMU+KVM migration Xie Bo
                   ` (2 subsequent siblings)
  3 siblings, 3 replies; 10+ messages in thread
From: Xie Bo @ 2025-09-15  7:08 UTC (permalink / raw)
  To: qemu-devel
  Cc: ajones, qemu-riscv, alistair23, pbonzini, anup, alistair.francis,
	rkrcmar, palmer, xiamy, Xie Bo

For KVM mode, the privilege mode should not include M-mode, and the
initial value should be set to S-mode. Additionally, a following patch
adds the implementation of putting the vCPU privilege mode to KVM.
When the vCPU runs for the first time, QEMU will first put the privilege
state to KVM. If the initial value is set to M-mode, KVM will encounter
an error.

In addition, this patch introduces the 'mp_state' field to RISC-V
vCPUs, following the convention used by KVM on x86. The 'mp_state'
reflects the multiprocessor state of a vCPU, and is used to control
whether the vCPU is runnable by KVM. Randomly select one CPU as the
boot CPU. Since each CPU executes the riscv_cpu_reset_hold() function
and CPU0 executes first, only CPU0 randomly selects the boot CPU.

Signed-off-by: Xie Bo <xb@ultrarisc.com>
---
 target/riscv/cpu.c | 17 ++++++++++++++++-
 target/riscv/cpu.h |  2 ++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index d055ddf462..55892a2fc7 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -37,6 +37,7 @@
 #include "kvm/kvm_riscv.h"
 #include "tcg/tcg-cpu.h"
 #include "tcg/tcg.h"
+#include "hw/boards.h"
 
 /* RISC-V CPU definitions */
 static const char riscv_single_letter_exts[] = "IEMAFDQCBPVH";
@@ -685,18 +686,32 @@ static void riscv_cpu_reset_hold(Object *obj, ResetType type)
 #ifndef CONFIG_USER_ONLY
     uint8_t iprio;
     int i, irq, rdzero;
+    static int boot_cpu_index;
 #endif
     CPUState *cs = CPU(obj);
     RISCVCPU *cpu = RISCV_CPU(cs);
     RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(obj);
     CPURISCVState *env = &cpu->env;
+    MachineState *ms = MACHINE(qdev_get_machine());
 
     if (mcc->parent_phases.hold) {
         mcc->parent_phases.hold(obj, type);
     }
 #ifndef CONFIG_USER_ONLY
     env->misa_mxl = mcc->def->misa_mxl_max;
-    env->priv = PRV_M;
+    if (kvm_enabled()) {
+        env->priv = PRV_S;
+    } else {
+        env->priv = PRV_M;
+    }
+    if (cs->cpu_index == 0) {
+        boot_cpu_index = g_random_int_range(0, ms->smp.cpus);
+    }
+    if (cs->cpu_index == boot_cpu_index) {
+        env->mp_state = KVM_MP_STATE_RUNNABLE;
+    } else {
+        env->mp_state = KVM_MP_STATE_STOPPED;
+    }
     env->mstatus &= ~(MSTATUS_MIE | MSTATUS_MPRV);
     if (env->misa_mxl > MXL_RV32) {
         /*
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 738e68fa6e..7ea4859de7 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -269,6 +269,8 @@ struct CPUArchState {
 #endif
 
     target_ulong priv;
+    /* Current multiprocessor state of this vCPU. */
+    uint32_t mp_state;
     /* CSRs for execution environment configuration */
     uint64_t menvcfg;
     target_ulong senvcfg;
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v9 2/2] Fix VM resume after QEMU+KVM migration
  2025-09-15  7:08 [PATCH v9 0/2] target/riscv:Fix riscv64 kvm migration Xie Bo
  2025-09-15  7:08 ` [PATCH v9 1/2] Set KVM initial privilege mode and mp_state Xie Bo
@ 2025-09-15  7:08 ` Xie Bo
  2025-09-16  8:03   ` Radim Krčmář
  2025-09-15 15:31 ` [PATCH v9 0/2] target/riscv:Fix riscv64 kvm migration Andrew Jones
  2025-09-29  1:43 ` Alistair Francis
  3 siblings, 1 reply; 10+ messages in thread
From: Xie Bo @ 2025-09-15  7:08 UTC (permalink / raw)
  To: qemu-devel
  Cc: ajones, qemu-riscv, alistair23, pbonzini, anup, alistair.francis,
	rkrcmar, palmer, xiamy, Xie Bo

Fix two migration issues for virtual machines in KVM mode:
1.It saves and restores the vCPU's privilege mode to ensure that the
vCPU's privilege mode is correct after migration.
2.It saves and restores the vCPU's mp_state (runnable or stopped) and
includes this state in the migration sequence, upgrading the vmstate
version to ensure that the vCPU's mp_state is correct after migration.

KVM_PUT_RUNTIME_STATE only synchronizes the vCPU’s runtime-modified
state (such as registers), whereas mp_state is related to system boot,
multi-core initialization, and is not modified during normal operation.
Therefore, mp_state is only synchronized to KVM during KVM_PUT_RESET_STATE
and KVM_PUT_FULL_STATE.

Signed-off-by: Xie Bo <xb@ultrarisc.com>
---
 target/riscv/kvm/kvm-cpu.c   | 60 ++++++++++++++++++++++++++++--------
 target/riscv/kvm/kvm_riscv.h |  3 +-
 target/riscv/machine.c       |  5 +--
 3 files changed, 52 insertions(+), 16 deletions(-)

diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index 5c19062c19..1fa755bd96 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -594,6 +594,12 @@ static int kvm_riscv_get_regs_core(CPUState *cs)
     }
     env->pc = reg;
 
+    ret = kvm_get_one_reg(cs, RISCV_CORE_REG(mode), &reg);
+    if (ret) {
+        return ret;
+    }
+    env->priv = reg;
+
     for (i = 1; i < 32; i++) {
         uint64_t id = KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_CORE, i);
         ret = kvm_get_one_reg(cs, id, &reg);
@@ -619,6 +625,12 @@ static int kvm_riscv_put_regs_core(CPUState *cs)
         return ret;
     }
 
+    reg = env->priv;
+    ret = kvm_set_one_reg(cs, RISCV_CORE_REG(mode), &reg);
+    if (ret) {
+        return ret;
+    }
+
     for (i = 1; i < 32; i++) {
         uint64_t id = KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_CORE, i);
         reg = env->gpr[i];
@@ -1348,25 +1360,52 @@ 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)
 {
+    int ret = 0;
+    CPURISCVState *env = &RISCV_CPU(cs)->env;
+
     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);
+        ret = kvm_vcpu_ioctl(cs, KVM_SET_MP_STATE, &mp_state);
         if (ret) {
-            fprintf(stderr, "%s: failed to sync MP_STATE %d/%s\n",
+            fprintf(stderr, "%s: failed to sync MP_STATE to KVM %d/%s\n",
                     __func__, ret, strerror(-ret));
-            return -1;
         }
     }
 
-    return 0;
+    return ret;
+}
+
+int kvm_riscv_sync_mpstate_to_qemu(CPUState *cs)
+{
+    int ret = 0;
+    CPURISCVState *env = &RISCV_CPU(cs)->env;
+
+    if (cap_has_mp_state) {
+        struct kvm_mp_state mp_state;
+
+        ret = kvm_vcpu_ioctl(cs, KVM_GET_MP_STATE, &mp_state);
+        if (ret) {
+            fprintf(stderr, "%s: failed to sync MP_STATE to QEMU %d/%s\n",
+                    __func__, ret, strerror(-ret));
+            return ret;
+        }
+        env->mp_state = mp_state.mp_state;
+    }
+
+    return ret;
 }
 
 int kvm_arch_put_registers(CPUState *cs, int level, Error **errp)
@@ -1393,13 +1432,8 @@ int kvm_arch_put_registers(CPUState *cs, int level, Error **errp)
         return ret;
     }
 
-    if (KVM_PUT_RESET_STATE == level) {
-        RISCVCPU *cpu = RISCV_CPU(cs);
-        if (cs->cpu_index == 0) {
-            ret = kvm_riscv_sync_mpstate_to_kvm(cpu, KVM_MP_STATE_RUNNABLE);
-        } else {
-            ret = kvm_riscv_sync_mpstate_to_kvm(cpu, KVM_MP_STATE_STOPPED);
-        }
+    if (level >= KVM_PUT_RESET_STATE) {
+        ret = kvm_riscv_sync_mpstate_to_kvm(cs);
         if (ret) {
             return ret;
         }
diff --git a/target/riscv/kvm/kvm_riscv.h b/target/riscv/kvm/kvm_riscv.h
index b2bcd1041f..953db94160 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 51e0567ed3..6e5be17c27 100644
--- a/target/riscv/machine.c
+++ b/target/riscv/machine.c
@@ -426,8 +426,8 @@ static const VMStateDescription vmstate_sstc = {
 
 const VMStateDescription vmstate_riscv_cpu = {
     .name = "cpu",
-    .version_id = 10,
-    .minimum_version_id = 10,
+    .version_id = 11,
+    .minimum_version_id = 11,
     .post_load = riscv_cpu_post_load,
     .fields = (const VMStateField[]) {
         VMSTATE_UINTTL_ARRAY(env.gpr, RISCVCPU, 32),
@@ -447,6 +447,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),
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v9 0/2] target/riscv:Fix riscv64 kvm migration
  2025-09-15  7:08 [PATCH v9 0/2] target/riscv:Fix riscv64 kvm migration Xie Bo
  2025-09-15  7:08 ` [PATCH v9 1/2] Set KVM initial privilege mode and mp_state Xie Bo
  2025-09-15  7:08 ` [PATCH v9 2/2] Fix VM resume after QEMU+KVM migration Xie Bo
@ 2025-09-15 15:31 ` Andrew Jones
  2025-09-29  1:43 ` Alistair Francis
  3 siblings, 0 replies; 10+ messages in thread
From: Andrew Jones @ 2025-09-15 15:31 UTC (permalink / raw)
  To: Xie Bo
  Cc: qemu-devel, qemu-riscv, alistair23, pbonzini, anup,
	alistair.francis, rkrcmar, palmer, xiamy

On Mon, Sep 15, 2025 at 03:08:06PM +0800, Xie Bo wrote:
> This is v9 of the series. Compared to v8, the patches are now based on
> the 'riscv-to-apply.next' branch from Alistair's repository:
> 
> https://github.com/alistair23/qemu/tree/riscv-to-apply.next
> 
> Changes since v8:
> - Rebased the series onto [alistair/riscv-to-apply.next]
> - Removed the previous 'Reviewed-by' tags due to the rebase

Please don't do that. If the rebase doesn't require a rework of the logic,
then the patches don't change in any significant way, so reviewers
shouldn't need to look at them again.

Anyway, for the series (since I can't see any difference from before)

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

Thanks,
drew

>   * The changes are purely mechanical; no code logic was altered *
> - Added 'Cc: qemu-stable@nongnu.org'
> 
> Xie Bo (2):
>   Set KVM initial privilege mode and mp_state
>   Fix VM resume after QEMU+KVM migration
> 
>  target/riscv/cpu.c           | 17 +++++++++-
>  target/riscv/cpu.h           |  2 ++
>  target/riscv/kvm/kvm-cpu.c   | 60 ++++++++++++++++++++++++++++--------
>  target/riscv/kvm/kvm_riscv.h |  3 +-
>  target/riscv/machine.c       |  5 +--
>  5 files changed, 70 insertions(+), 17 deletions(-)
> 
> -- 
> 2.43.0
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v9 1/2] Set KVM initial privilege mode and mp_state
  2025-09-15  7:08 ` [PATCH v9 1/2] Set KVM initial privilege mode and mp_state Xie Bo
@ 2025-09-16  8:00   ` Radim Krčmář
  2025-09-29  1:48   ` Alistair Francis
  2025-10-22 11:47   ` Daniel Henrique Barboza
  2 siblings, 0 replies; 10+ messages in thread
From: Radim Krčmář @ 2025-09-16  8:00 UTC (permalink / raw)
  To: Xie Bo, qemu-devel
  Cc: ajones, qemu-riscv, alistair23, pbonzini, anup, alistair.francis,
	palmer, xiamy, qemu-riscv-bounces+qemu-riscv=archiver.kernel.org

2025-09-15T15:08:07+08:00, Xie Bo <xb@ultrarisc.com>:
> For KVM mode, the privilege mode should not include M-mode, and the
> initial value should be set to S-mode. Additionally, a following patch
> adds the implementation of putting the vCPU privilege mode to KVM.
> When the vCPU runs for the first time, QEMU will first put the privilege
> state to KVM. If the initial value is set to M-mode, KVM will encounter
> an error.
>
> In addition, this patch introduces the 'mp_state' field to RISC-V
> vCPUs, following the convention used by KVM on x86. The 'mp_state'
> reflects the multiprocessor state of a vCPU, and is used to control
> whether the vCPU is runnable by KVM. Randomly select one CPU as the
> boot CPU. Since each CPU executes the riscv_cpu_reset_hold() function
> and CPU0 executes first, only CPU0 randomly selects the boot CPU.

This could really be two patches, as changing the boot 

> Signed-off-by: Xie Bo <xb@ultrarisc.com>
> ---
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> @@ -685,18 +686,32 @@ static void riscv_cpu_reset_hold(Object *obj, ResetType type)
> +    static int boot_cpu_index;
       ^^^^^^
!

> +    if (kvm_enabled()) {
> +        env->priv = PRV_S;
> +    } else {
> +        env->priv = PRV_M;
> +    }

(I think changing the priv belongs to a separate patch.)

> +    if (cs->cpu_index == 0) {
> +        boot_cpu_index = g_random_int_range(0, ms->smp.cpus);
> +    }

This adds an assumption that vcpu_index == 0 is executed first.
Is that always going to be true?

If we reset the VCPUs in a different order (or in parallel), we might
also online zero or two VCPUs.

Performing the selection just once, in the reset initiator, would allow
us to avoid the dreaded static variable by putting it in machine arch
state.

> +    if (cs->cpu_index == boot_cpu_index) {
> +        env->mp_state = KVM_MP_STATE_RUNNABLE;
> +    } else {
> +        env->mp_state = KVM_MP_STATE_STOPPED;
> +    }

Thanks.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v9 2/2] Fix VM resume after QEMU+KVM migration
  2025-09-15  7:08 ` [PATCH v9 2/2] Fix VM resume after QEMU+KVM migration Xie Bo
@ 2025-09-16  8:03   ` Radim Krčmář
  0 siblings, 0 replies; 10+ messages in thread
From: Radim Krčmář @ 2025-09-16  8:03 UTC (permalink / raw)
  To: Xie Bo, qemu-devel
  Cc: ajones, qemu-riscv, alistair23, pbonzini, anup, alistair.francis,
	palmer, xiamy, qemu-riscv-bounces+qemu-riscv=archiver.kernel.org

2025-09-15T15:08:08+08:00, Xie Bo <xb@ultrarisc.com>:
> Fix two migration issues for virtual machines in KVM mode:
> 1.It saves and restores the vCPU's privilege mode to ensure that the
> vCPU's privilege mode is correct after migration.
> 2.It saves and restores the vCPU's mp_state (runnable or stopped) and
> includes this state in the migration sequence, upgrading the vmstate
> version to ensure that the vCPU's mp_state is correct after migration.

This could be two patches, each doing just one thing:
  * fix mode
  * fix mp_state

And the mode hunk that I pointed out in [v9 1/2] could be put in the
patch that fixes mode.

I jumped quite late into the review, so if others are fine with the
current split,

Reviewed-by: Radim Krčmář <rkrcmar@ventanamicro.com>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v9 0/2] target/riscv:Fix riscv64 kvm migration
  2025-09-15  7:08 [PATCH v9 0/2] target/riscv:Fix riscv64 kvm migration Xie Bo
                   ` (2 preceding siblings ...)
  2025-09-15 15:31 ` [PATCH v9 0/2] target/riscv:Fix riscv64 kvm migration Andrew Jones
@ 2025-09-29  1:43 ` Alistair Francis
  3 siblings, 0 replies; 10+ messages in thread
From: Alistair Francis @ 2025-09-29  1:43 UTC (permalink / raw)
  To: Xie Bo
  Cc: qemu-devel, ajones, qemu-riscv, pbonzini, anup, alistair.francis,
	rkrcmar, palmer, xiamy

On Mon, Sep 15, 2025 at 5:08 PM Xie Bo <xb@ultrarisc.com> wrote:
>
> This is v9 of the series. Compared to v8, the patches are now based on
> the 'riscv-to-apply.next' branch from Alistair's repository:
>
> https://github.com/alistair23/qemu/tree/riscv-to-apply.next
>
> Changes since v8:
> - Rebased the series onto [alistair/riscv-to-apply.next]
> - Removed the previous 'Reviewed-by' tags due to the rebase
>   * The changes are purely mechanical; no code logic was altered *
> - Added 'Cc: qemu-stable@nongnu.org'
>
> Xie Bo (2):
>   Set KVM initial privilege mode and mp_state
>   Fix VM resume after QEMU+KVM migration

Thanks!

Applied to riscv-to-apply.next

Alistair

>
>  target/riscv/cpu.c           | 17 +++++++++-
>  target/riscv/cpu.h           |  2 ++
>  target/riscv/kvm/kvm-cpu.c   | 60 ++++++++++++++++++++++++++++--------
>  target/riscv/kvm/kvm_riscv.h |  3 +-
>  target/riscv/machine.c       |  5 +--
>  5 files changed, 70 insertions(+), 17 deletions(-)
>
> --
> 2.43.0
>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v9 1/2] Set KVM initial privilege mode and mp_state
  2025-09-15  7:08 ` [PATCH v9 1/2] Set KVM initial privilege mode and mp_state Xie Bo
  2025-09-16  8:00   ` Radim Krčmář
@ 2025-09-29  1:48   ` Alistair Francis
  2025-09-29 17:36     ` Radim Krčmář
  2025-10-22 11:47   ` Daniel Henrique Barboza
  2 siblings, 1 reply; 10+ messages in thread
From: Alistair Francis @ 2025-09-29  1:48 UTC (permalink / raw)
  To: Xie Bo
  Cc: qemu-devel, ajones, qemu-riscv, pbonzini, anup, alistair.francis,
	rkrcmar, palmer, xiamy

On Mon, Sep 15, 2025 at 5:08 PM Xie Bo <xb@ultrarisc.com> wrote:
>
> For KVM mode, the privilege mode should not include M-mode, and the
> initial value should be set to S-mode. Additionally, a following patch
> adds the implementation of putting the vCPU privilege mode to KVM.
> When the vCPU runs for the first time, QEMU will first put the privilege
> state to KVM. If the initial value is set to M-mode, KVM will encounter
> an error.
>
> In addition, this patch introduces the 'mp_state' field to RISC-V
> vCPUs, following the convention used by KVM on x86. The 'mp_state'
> reflects the multiprocessor state of a vCPU, and is used to control
> whether the vCPU is runnable by KVM. Randomly select one CPU as the
> boot CPU. Since each CPU executes the riscv_cpu_reset_hold() function
> and CPU0 executes first, only CPU0 randomly selects the boot CPU.
>
> Signed-off-by: Xie Bo <xb@ultrarisc.com>
> ---
>  target/riscv/cpu.c | 17 ++++++++++++++++-
>  target/riscv/cpu.h |  2 ++
>  2 files changed, 18 insertions(+), 1 deletion(-)

This fails to build with the following error, it seems an include is missing

../target/riscv/cpu.c: In function ‘riscv_cpu_reset_hold’:
../target/riscv/cpu.c:711:25: error: ‘KVM_MP_STATE_RUNNABLE’
undeclared (first use in this function)
 711 |         env->mp_state = KVM_MP_STATE_RUNNABLE;
     |                         ^~~~~~~~~~~~~~~~~~~~~
../target/riscv/cpu.c:711:25: note: each undeclared identifier is
reported only once for each function it appears in
../target/riscv/cpu.c:713:25: error: ‘KVM_MP_STATE_STOPPED’ undeclared
(first use in this function); did you mean ‘S390_CPU_STATE_STOPPED’?
 713 |         env->mp_state = KVM_MP_STATE_STOPPED;
     |                         ^~~~~~~~~~~~~~~~~~~~
     |                         S390_CPU_STATE_STO

Alistair

>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index d055ddf462..55892a2fc7 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -37,6 +37,7 @@
>  #include "kvm/kvm_riscv.h"
>  #include "tcg/tcg-cpu.h"
>  #include "tcg/tcg.h"
> +#include "hw/boards.h"
>
>  /* RISC-V CPU definitions */
>  static const char riscv_single_letter_exts[] = "IEMAFDQCBPVH";
> @@ -685,18 +686,32 @@ static void riscv_cpu_reset_hold(Object *obj, ResetType type)
>  #ifndef CONFIG_USER_ONLY
>      uint8_t iprio;
>      int i, irq, rdzero;
> +    static int boot_cpu_index;
>  #endif
>      CPUState *cs = CPU(obj);
>      RISCVCPU *cpu = RISCV_CPU(cs);
>      RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(obj);
>      CPURISCVState *env = &cpu->env;
> +    MachineState *ms = MACHINE(qdev_get_machine());
>
>      if (mcc->parent_phases.hold) {
>          mcc->parent_phases.hold(obj, type);
>      }
>  #ifndef CONFIG_USER_ONLY
>      env->misa_mxl = mcc->def->misa_mxl_max;
> -    env->priv = PRV_M;
> +    if (kvm_enabled()) {
> +        env->priv = PRV_S;
> +    } else {
> +        env->priv = PRV_M;
> +    }
> +    if (cs->cpu_index == 0) {
> +        boot_cpu_index = g_random_int_range(0, ms->smp.cpus);
> +    }
> +    if (cs->cpu_index == boot_cpu_index) {
> +        env->mp_state = KVM_MP_STATE_RUNNABLE;
> +    } else {
> +        env->mp_state = KVM_MP_STATE_STOPPED;
> +    }
>      env->mstatus &= ~(MSTATUS_MIE | MSTATUS_MPRV);
>      if (env->misa_mxl > MXL_RV32) {
>          /*
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 738e68fa6e..7ea4859de7 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -269,6 +269,8 @@ struct CPUArchState {
>  #endif
>
>      target_ulong priv;
> +    /* Current multiprocessor state of this vCPU. */
> +    uint32_t mp_state;
>      /* CSRs for execution environment configuration */
>      uint64_t menvcfg;
>      target_ulong senvcfg;
> --
> 2.43.0
>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v9 1/2] Set KVM initial privilege mode and mp_state
  2025-09-29  1:48   ` Alistair Francis
@ 2025-09-29 17:36     ` Radim Krčmář
  0 siblings, 0 replies; 10+ messages in thread
From: Radim Krčmář @ 2025-09-29 17:36 UTC (permalink / raw)
  To: Alistair Francis, Xie Bo
  Cc: qemu-devel, ajones, qemu-riscv, pbonzini, anup, alistair.francis,
	palmer, xiamy, qemu-riscv-bounces+qemu-riscv=archiver.kernel.org

2025-09-29T11:48:08+10:00, Alistair Francis <alistair23@gmail.com>:
> On Mon, Sep 15, 2025 at 5:08 PM Xie Bo <xb@ultrarisc.com> wrote:
>>
>> For KVM mode, the privilege mode should not include M-mode, and the
>> initial value should be set to S-mode. Additionally, a following patch
>> adds the implementation of putting the vCPU privilege mode to KVM.
>> When the vCPU runs for the first time, QEMU will first put the privilege
>> state to KVM. If the initial value is set to M-mode, KVM will encounter
>> an error.
>>
>> In addition, this patch introduces the 'mp_state' field to RISC-V
>> vCPUs, following the convention used by KVM on x86. The 'mp_state'
>> reflects the multiprocessor state of a vCPU, and is used to control
>> whether the vCPU is runnable by KVM. Randomly select one CPU as the
>> boot CPU. Since each CPU executes the riscv_cpu_reset_hold() function
>> and CPU0 executes first, only CPU0 randomly selects the boot CPU.
>>
>> Signed-off-by: Xie Bo <xb@ultrarisc.com>
>> ---
>>  target/riscv/cpu.c | 17 ++++++++++++++++-
>>  target/riscv/cpu.h |  2 ++
>>  2 files changed, 18 insertions(+), 1 deletion(-)
>
> This fails to build with the following error, it seems an include is missing
>
> ../target/riscv/cpu.c: In function ‘riscv_cpu_reset_hold’:
> ../target/riscv/cpu.c:711:25: error: ‘KVM_MP_STATE_RUNNABLE’
> undeclared (first use in this function)
>  711 |         env->mp_state = KVM_MP_STATE_RUNNABLE;
>      |                         ^~~~~~~~~~~~~~~~~~~~~
> ../target/riscv/cpu.c:711:25: note: each undeclared identifier is
> reported only once for each function it appears in
> ../target/riscv/cpu.c:713:25: error: ‘KVM_MP_STATE_STOPPED’ undeclared
> (first use in this function); did you mean ‘S390_CPU_STATE_STOPPED’?
>  713 |         env->mp_state = KVM_MP_STATE_STOPPED;
>      |                         ^~~~~~~~~~~~~~~~~~~~
>      |                         S390_CPU_STATE_STO

I think this is because the code belongs in kvm_riscv_reset_vcpu().

>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> +    static int boot_cpu_index;
>> +    MachineState *ms = MACHINE(qdev_get_machine());
>> +    if (cs->cpu_index == 0) {
>> +        boot_cpu_index = g_random_int_range(0, ms->smp.cpus);
>> +    }

If we're already touching the code, can we also express boot_cpu_index
in a VM state?  Using a static variable is pretty scary.

Thanks.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v9 1/2] Set KVM initial privilege mode and mp_state
  2025-09-15  7:08 ` [PATCH v9 1/2] Set KVM initial privilege mode and mp_state Xie Bo
  2025-09-16  8:00   ` Radim Krčmář
  2025-09-29  1:48   ` Alistair Francis
@ 2025-10-22 11:47   ` Daniel Henrique Barboza
  2 siblings, 0 replies; 10+ messages in thread
From: Daniel Henrique Barboza @ 2025-10-22 11:47 UTC (permalink / raw)
  To: Xie Bo, qemu-devel
  Cc: ajones, qemu-riscv, alistair23, pbonzini, anup, alistair.francis,
	rkrcmar, palmer, xiamy

Hi,

On 9/15/25 4:08 AM, Xie Bo wrote:
> For KVM mode, the privilege mode should not include M-mode, and the
> initial value should be set to S-mode. Additionally, a following patch
> adds the implementation of putting the vCPU privilege mode to KVM.
> When the vCPU runs for the first time, QEMU will first put the privilege
> state to KVM. If the initial value is set to M-mode, KVM will encounter
> an error.
> 
> In addition, this patch introduces the 'mp_state' field to RISC-V
> vCPUs, following the convention used by KVM on x86. The 'mp_state'
> reflects the multiprocessor state of a vCPU, and is used to control
> whether the vCPU is runnable by KVM. Randomly select one CPU as the
> boot CPU. Since each CPU executes the riscv_cpu_reset_hold() function
> and CPU0 executes first, only CPU0 randomly selects the boot CPU.
> 
> Signed-off-by: Xie Bo <xb@ultrarisc.com>
> ---
>   target/riscv/cpu.c | 17 ++++++++++++++++-
>   target/riscv/cpu.h |  2 ++
>   2 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index d055ddf462..55892a2fc7 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -37,6 +37,7 @@
>   #include "kvm/kvm_riscv.h"
>   #include "tcg/tcg-cpu.h"
>   #include "tcg/tcg.h"
> +#include "hw/boards.h"
>   
>   /* RISC-V CPU definitions */
>   static const char riscv_single_letter_exts[] = "IEMAFDQCBPVH";
> @@ -685,18 +686,32 @@ static void riscv_cpu_reset_hold(Object *obj, ResetType type)
>   #ifndef CONFIG_USER_ONLY
>       uint8_t iprio;
>       int i, irq, rdzero;
> +    static int boot_cpu_index;
>   #endif
>       CPUState *cs = CPU(obj);
>       RISCVCPU *cpu = RISCV_CPU(cs);
>       RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(obj);
>       CPURISCVState *env = &cpu->env;
> +    MachineState *ms = MACHINE(qdev_get_machine());
>   
>       if (mcc->parent_phases.hold) {
>           mcc->parent_phases.hold(obj, type);
>       }
>   #ifndef CONFIG_USER_ONLY
>       env->misa_mxl = mcc->def->misa_mxl_max;
> -    env->priv = PRV_M;
> +    if (kvm_enabled()) {
> +        env->priv = PRV_S;
> +    } else {
> +        env->priv = PRV_M;
> +    }

Sorry for the late v9 review. riscv_cpu_reset_hold() is reserved for TCG initialization
only. All kvm specific initialization is done in kvm_riscv_reset_vcpu(), called in the
end of this function:

     if (kvm_enabled()) {
         kvm_riscv_reset_vcpu(cpu);
     }

Everything that is KVM specific should be put in kvm_riscv_reset_vcpu() and other KVM
related helpers in target/riscv/kvm/kvm-cpu.c.


As for the except above, I just sent a patch that is fixing it inside kvm_riscv_reset_regs_csr():

("[PATCH] target/riscv/kvm: fix env->priv setting in reset_regs_csr()")
https://lore.kernel.org/qemu-riscv/20251022111105.483992-1-dbarboza@ventanamicro.com/


The reason I went ahead with that patch is because it is related to an opened Gitlab
issue (https://gitlab.com/qemu-project/qemu/-/issues/2991) and we already missed the
window for 10.1 to fix it.

In case you send another version of this series you can either drop this particular
change or move it to kvm_riscv_reset_regs_csr(). We'll keep whatever is merged first.


Thanks,

Daniel



> +    if (cs->cpu_index == 0) {
> +        boot_cpu_index = g_random_int_range(0, ms->smp.cpus);
> +    }
> +    if (cs->cpu_index == boot_cpu_index) {
> +        env->mp_state = KVM_MP_STATE_RUNNABLE;
> +    } else {
> +        env->mp_state = KVM_MP_STATE_STOPPED;
> +    }
>       env->mstatus &= ~(MSTATUS_MIE | MSTATUS_MPRV);
>       if (env->misa_mxl > MXL_RV32) {
>           /*
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 738e68fa6e..7ea4859de7 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -269,6 +269,8 @@ struct CPUArchState {
>   #endif
>   
>       target_ulong priv;
> +    /* Current multiprocessor state of this vCPU. */
> +    uint32_t mp_state;
>       /* CSRs for execution environment configuration */
>       uint64_t menvcfg;
>       target_ulong senvcfg;



^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2025-10-22 11:47 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-15  7:08 [PATCH v9 0/2] target/riscv:Fix riscv64 kvm migration Xie Bo
2025-09-15  7:08 ` [PATCH v9 1/2] Set KVM initial privilege mode and mp_state Xie Bo
2025-09-16  8:00   ` Radim Krčmář
2025-09-29  1:48   ` Alistair Francis
2025-09-29 17:36     ` Radim Krčmář
2025-10-22 11:47   ` Daniel Henrique Barboza
2025-09-15  7:08 ` [PATCH v9 2/2] Fix VM resume after QEMU+KVM migration Xie Bo
2025-09-16  8:03   ` Radim Krčmář
2025-09-15 15:31 ` [PATCH v9 0/2] target/riscv:Fix riscv64 kvm migration Andrew Jones
2025-09-29  1:43 ` Alistair Francis

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).