Re: [PATCH v9 1/2] Set KVM initial privilege mode and mp_state

["Radim Krčmář" <rkrcmar@ventanamicro.com>]

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.