Re: [PATCH] target/riscv: add satp mode for kvm host cpu

[Andrew Jones <ajones@ventanamicro.com>]

On Sun, Apr 27, 2025 at 09:25:57PM +0800, Meng Zhuo wrote:
> This patch adds host satp mode while kvm/host cpu satp mode is not
> set.

Huh, the KVM side[1] was written for this purpose, but it appears we never
got a QEMU side merged.

[1] commit 2776421e6839 ("RISC-V: KVM: provide UAPI for host SATP mode")

> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2931
> Signed-off-by: Meng Zhuo <mengzhuo@iscas.ac.cn>
> ---
>  target/riscv/kvm/kvm-cpu.c | 27 ++++++++++++++++++++++++++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index 5315134e08..942f942b25 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -953,6 +953,21 @@ static void kvm_riscv_destroy_scratch_vcpu(KVMScratchCPU *scratch)
>      close(scratch->kvmfd);
>  }
>  
> +static void kvm_riscv_init_satp_mode(RISCVCPU *cpu, KVMScratchCPU *kvmcpu)
> +{
> +    CPURISCVState *env = &cpu->env;
> +    struct kvm_one_reg reg;
> +    int ret;
> +    uint64_t val;

Please add a blank line here.

> +    reg.id = RISCV_CONFIG_REG(env, satp_mode);
> +    reg.addr = (uint64_t)&val;
> +    ret = ioctl(kvmcpu->cpufd, KVM_GET_ONE_REG, &reg);
> +    if (ret != 0) {
> +        error_report("Unable to retrieve satp from host, error %d", ret);
> +    }
> +    env->satp = 1 << val;

We need to expose set_satp_mode_max_supported() and then call it here
instead of setting env->satp. At this phase we're just figuring out what's
supported by KVM. riscv_cpu_finalize_features() will then sort out what's
supported by KVM and what's selected by the user (if anything) in order
to determine what mode should be used.

> +}
> +
>  static void kvm_riscv_init_machine_ids(RISCVCPU *cpu, KVMScratchCPU *kvmcpu)
>  {
>      CPURISCVState *env = &cpu->env;
> @@ -1212,6 +1227,7 @@ static void riscv_init_kvm_registers(Object *cpu_obj)
>      kvm_riscv_init_machine_ids(cpu, &kvmcpu);
>      kvm_riscv_init_misa_ext_mask(cpu, &kvmcpu);
>      kvm_riscv_init_multiext_cfg(cpu, &kvmcpu);
> +    kvm_riscv_init_satp_mode(cpu, &kvmcpu);
>  
>      kvm_riscv_destroy_scratch_vcpu(&kvmcpu);
>  }
> @@ -1891,7 +1907,16 @@ static bool kvm_cpu_realize(CPUState *cs, Error **errp)
>          }
>      }
>  
> -   return true;
> +    RISCVSATPMap *satp_mode = &cpu->cfg.satp_mode;
> +    CPURISCVState *env = &cpu->env;
> +
> +    if (!satp_mode->init && env->satp) {
> +        satp_mode->init = env->satp;
> +        satp_mode->map = env->satp;
> +        satp_mode->supported = env->satp;
> +    }
> +
> +    return true;

Other than the indentation fix, none of the above hunk is correct or
needed.

Thanks,
drew

>  }
>  
>  void riscv_kvm_cpu_finalize_features(RISCVCPU *cpu, Error **errp)
> -- 
> 2.39.5
> 
>