1. Patchew
  2. QEMU
  3. View series

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

Meng Zhuo posted 1 patch 5 months, 4 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20250520104103.89736-1-mengzhuo@iscas.ac.cn
Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>
There is a newer version of this series
target/riscv/cpu.c         |  3 +--
target/riscv/cpu.h         |  1 +
target/riscv/kvm/kvm-cpu.c | 20 +++++++++++++++++++-
3 files changed, 21 insertions(+), 3 deletions(-)
[PATCH v3] target/riscv/kvm: add satp mode for host cpu
Posted by Meng Zhuo 5 months, 4 weeks ago 
This patch adds host satp mode while kvm/host cpu satp mode is not
set.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2931
Signed-off-by: Meng Zhuo <mengzhuo@iscas.ac.cn>
---
 target/riscv/cpu.c         |  3 +--
 target/riscv/cpu.h         |  1 +
 target/riscv/kvm/kvm-cpu.c | 20 +++++++++++++++++++-
 3 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index d92874baa0..a84edd3a3b 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -433,8 +433,7 @@ const char *satp_mode_str(uint8_t satp_mode, bool is_32_bit)
     g_assert_not_reached();
 }
 
-static void set_satp_mode_max_supported(RISCVCPU *cpu,
-                                        uint8_t satp_mode)
+void set_satp_mode_max_supported(RISCVCPU *cpu, uint8_t satp_mode)
 {
     bool rv32 = riscv_cpu_mxl(&cpu->env) == MXL_RV32;
     const bool *valid_vm = rv32 ? valid_vm_1_10_32 : valid_vm_1_10_64;
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index b56d3afa69..d7136f1d72 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -915,6 +915,7 @@ char *riscv_cpu_get_name(RISCVCPU *cpu);
 
 void riscv_cpu_finalize_features(RISCVCPU *cpu, Error **errp);
 void riscv_add_satp_mode_properties(Object *obj);
+void set_satp_mode_max_supported(RISCVCPU *cpu, uint8_t satp_mode);
 bool riscv_cpu_accelerator_compatible(RISCVCPU *cpu);
 
 /* CSR function table */
diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index 82f9728636..18fbca1a08 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -999,6 +999,23 @@ 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;
+
+    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);
+    }
+
+    set_satp_mode_max_supported(cpu, val);
+}
+
 static void kvm_riscv_init_machine_ids(RISCVCPU *cpu, KVMScratchCPU *kvmcpu)
 {
     struct kvm_one_reg reg;
@@ -1302,6 +1319,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_cfg(cpu, &kvmcpu);
+    kvm_riscv_init_satp_mode(cpu, &kvmcpu);
 
     kvm_riscv_destroy_scratch_vcpu(&kvmcpu);
 }
@@ -1980,7 +1998,7 @@ static bool kvm_cpu_realize(CPUState *cs, Error **errp)
         }
     }
 
-   return true;
+    return true;
 }
 
 void riscv_kvm_cpu_finalize_features(RISCVCPU *cpu, Error **errp)
-- 
2.39.5
Re: [PATCH v3] target/riscv/kvm: add satp mode for host cpu
Posted by Andrew Jones 5 months, 4 weeks ago 
On Tue, May 20, 2025 at 06:41:03PM +0800, Meng Zhuo wrote:
> This patch adds host satp mode while kvm/host cpu satp mode is not
> set.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2931
> Signed-off-by: Meng Zhuo <mengzhuo@iscas.ac.cn>
> ---

Please provide a changelog here. I had to go reread the comments on v1 to
guess at what might have changed.

>  target/riscv/cpu.c         |  3 +--
>  target/riscv/cpu.h         |  1 +
>  target/riscv/kvm/kvm-cpu.c | 20 +++++++++++++++++++-
>  3 files changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index d92874baa0..a84edd3a3b 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -433,8 +433,7 @@ const char *satp_mode_str(uint8_t satp_mode, bool is_32_bit)
>      g_assert_not_reached();
>  }
>  
> -static void set_satp_mode_max_supported(RISCVCPU *cpu,
> -                                        uint8_t satp_mode)
> +void set_satp_mode_max_supported(RISCVCPU *cpu, uint8_t satp_mode)

In v1 I suggested we add some sanity checking to this function to ensure
we can never set an invalid mode (one that's higher than the max
supported).

>  {
>      bool rv32 = riscv_cpu_mxl(&cpu->env) == MXL_RV32;
>      const bool *valid_vm = rv32 ? valid_vm_1_10_32 : valid_vm_1_10_64;
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index b56d3afa69..d7136f1d72 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -915,6 +915,7 @@ char *riscv_cpu_get_name(RISCVCPU *cpu);
>  
>  void riscv_cpu_finalize_features(RISCVCPU *cpu, Error **errp);
>  void riscv_add_satp_mode_properties(Object *obj);
> +void set_satp_mode_max_supported(RISCVCPU *cpu, uint8_t satp_mode);
>  bool riscv_cpu_accelerator_compatible(RISCVCPU *cpu);
>  
>  /* CSR function table */
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index 82f9728636..18fbca1a08 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -999,6 +999,23 @@ 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;
> +
> +    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);

This will output "Unable..., error -1" on error. We should output at least
the errno and preferably the errno string. I see that this is just a copy
of a pattern used throughout this file though, so we need a patch fixing
all of the ioctl call error_reports.

> +    }
> +
> +    set_satp_mode_max_supported(cpu, val);
> +}
> +
>  static void kvm_riscv_init_machine_ids(RISCVCPU *cpu, KVMScratchCPU *kvmcpu)
>  {
>      struct kvm_one_reg reg;
> @@ -1302,6 +1319,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_cfg(cpu, &kvmcpu);
> +    kvm_riscv_init_satp_mode(cpu, &kvmcpu);
>  
>      kvm_riscv_destroy_scratch_vcpu(&kvmcpu);
>  }
> @@ -1980,7 +1998,7 @@ static bool kvm_cpu_realize(CPUState *cs, Error **errp)
>          }
>      }
>  
> -   return true;
> +    return true;
>  }
>  
>  void riscv_kvm_cpu_finalize_features(RISCVCPU *cpu, Error **errp)
> -- 
> 2.39.5
> 
>

Otherwise,

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Re: [PATCH v3] target/riscv/kvm: add satp mode for host cpu
Posted by Andrew Jones 5 months, 4 weeks ago 
On Tue, May 20, 2025 at 01:58:45PM +0200, Andrew Jones wrote:
> On Tue, May 20, 2025 at 06:41:03PM +0800, Meng Zhuo wrote:
> > This patch adds host satp mode while kvm/host cpu satp mode is not
> > set.
> > 
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2931
> > Signed-off-by: Meng Zhuo <mengzhuo@iscas.ac.cn>
> > ---
> 
> Please provide a changelog here. I had to go reread the comments on v1 to
> guess at what might have changed.
> 
> >  target/riscv/cpu.c         |  3 +--
> >  target/riscv/cpu.h         |  1 +
> >  target/riscv/kvm/kvm-cpu.c | 20 +++++++++++++++++++-
> >  3 files changed, 21 insertions(+), 3 deletions(-)
> > 
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index d92874baa0..a84edd3a3b 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -433,8 +433,7 @@ const char *satp_mode_str(uint8_t satp_mode, bool is_32_bit)
> >      g_assert_not_reached();
> >  }
> >  
> > -static void set_satp_mode_max_supported(RISCVCPU *cpu,
> > -                                        uint8_t satp_mode)
> > +void set_satp_mode_max_supported(RISCVCPU *cpu, uint8_t satp_mode)
> 
> In v1 I suggested we add some sanity checking to this function to ensure
> we can never set an invalid mode (one that's higher than the max
> supported).

It appears [1], which I just learned exists (that series doesn't CC
qemu-riscv) adds an assert for the check (among other changes).

[1] https://lore.kernel.org/all/20250406070254.274797-3-pbonzini@redhat.com/

Thanks,
drew

> 
> >  {
> >      bool rv32 = riscv_cpu_mxl(&cpu->env) == MXL_RV32;
> >      const bool *valid_vm = rv32 ? valid_vm_1_10_32 : valid_vm_1_10_64;
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index b56d3afa69..d7136f1d72 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -915,6 +915,7 @@ char *riscv_cpu_get_name(RISCVCPU *cpu);
> >  
> >  void riscv_cpu_finalize_features(RISCVCPU *cpu, Error **errp);
> >  void riscv_add_satp_mode_properties(Object *obj);
> > +void set_satp_mode_max_supported(RISCVCPU *cpu, uint8_t satp_mode);
> >  bool riscv_cpu_accelerator_compatible(RISCVCPU *cpu);
> >  
> >  /* CSR function table */
> > diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> > index 82f9728636..18fbca1a08 100644
> > --- a/target/riscv/kvm/kvm-cpu.c
> > +++ b/target/riscv/kvm/kvm-cpu.c
> > @@ -999,6 +999,23 @@ 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;
> > +
> > +    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);
> 
> This will output "Unable..., error -1" on error. We should output at least
> the errno and preferably the errno string. I see that this is just a copy
> of a pattern used throughout this file though, so we need a patch fixing
> all of the ioctl call error_reports.
> 
> > +    }
> > +
> > +    set_satp_mode_max_supported(cpu, val);
> > +}
> > +
> >  static void kvm_riscv_init_machine_ids(RISCVCPU *cpu, KVMScratchCPU *kvmcpu)
> >  {
> >      struct kvm_one_reg reg;
> > @@ -1302,6 +1319,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_cfg(cpu, &kvmcpu);
> > +    kvm_riscv_init_satp_mode(cpu, &kvmcpu);
> >  
> >      kvm_riscv_destroy_scratch_vcpu(&kvmcpu);
> >  }
> > @@ -1980,7 +1998,7 @@ static bool kvm_cpu_realize(CPUState *cs, Error **errp)
> >          }
> >      }
> >  
> > -   return true;
> > +    return true;
> >  }
> >  
> >  void riscv_kvm_cpu_finalize_features(RISCVCPU *cpu, Error **errp)
> > -- 
> > 2.39.5
> > 
> >
> 
> Otherwise,
> 
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.