qemu-riscv
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 3/3] target/riscv: kvm: Support selecting VCPU extensions


From: Bin Meng
Subject: Re: [PATCH v2 3/3] target/riscv: kvm: Support selecting VCPU extensions
Date: Mon, 5 Dec 2022 23:37:32 +0800

On Mon, Dec 5, 2022 at 6:37 PM Mayuresh Chitale
<mchitale@ventanamicro.com> wrote:
>
> Set the state of each ISA extension on the vcpu depending on what
> is set in the CPU property and what is allowed by KVM for that extension.
>
> Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  target/riscv/cpu.c       | 11 ++++-
>  target/riscv/kvm.c       | 88 ++++++++++++++++++++++++++++++++++------
>  target/riscv/kvm_riscv.h |  2 +-
>  3 files changed, 87 insertions(+), 14 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 8c8f085a80..4e0e96ce71 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1205,10 +1205,19 @@ static void riscv_isa_string_ext(RISCVCPU *cpu, char 
> **isa_str)
>  {
>      char *old = *isa_str;
>      char *new = *isa_str;
> -    int i;
> +    int i, offset;
>
>      for (i = 0; i < ARRAY_SIZE(isa_edata_arr); i++) {
>          if (isa_ext_is_enabled(cpu, &isa_edata_arr[i])) {
> +            offset = isa_edata_arr[i].ext_enable_offset;
> +            if (kvm_enabled() && !kvm_riscv_ext_supported(offset)) {
> +#ifndef CONFIG_USER_ONLY
> +                info_report("disabling %s extension for hart 0x%lx because "

Use TARGET_FMT_lx for mhartid otherwise it will lose accuracy on
32-bit hosts for rv64.

> +                            "kvm does not support it", isa_edata_arr[i].name,
> +                            (unsigned long)cpu->env.mhartid);
> +#endif
> +                    continue;

incorrect indentation level

> +            }
>              if (isa_edata_arr[i].multi_letter) {
>                  if (cpu->cfg.short_isa_string) {
>                      continue;
> diff --git a/target/riscv/kvm.c b/target/riscv/kvm.c
> index 30f21453d6..ea0715c9e4 100644
> --- a/target/riscv/kvm.c
> +++ b/target/riscv/kvm.c
> @@ -42,6 +42,29 @@
>  #include "migration/migration.h"
>  #include "sysemu/runstate.h"
>
> +struct isa_ext_info {
> +    const char *name;
> +    target_ulong misa_bit;
> +    int ext_enable_offset;
> +};
> +
> +#define ISA_EXT_DATA_ENTRY(_name, _bit, _prop) \
> +    {#_name, _bit, offsetof(struct RISCVCPUConfig, _prop)}
> +
> +static const struct isa_ext_info isa_info_arr[] = {
> +    ISA_EXT_DATA_ENTRY(a, RVA, ext_a),
> +    ISA_EXT_DATA_ENTRY(c, RVC, ext_c),
> +    ISA_EXT_DATA_ENTRY(d, RVD, ext_d),
> +    ISA_EXT_DATA_ENTRY(f, RVF, ext_f),
> +    ISA_EXT_DATA_ENTRY(h, RVH, ext_h),
> +    ISA_EXT_DATA_ENTRY(i, RVI, ext_i),
> +    ISA_EXT_DATA_ENTRY(m, RVM, ext_m),
> +    ISA_EXT_DATA_ENTRY(svpbmt, 0, ext_svpbmt),
> +    ISA_EXT_DATA_ENTRY(sstc, 0, ext_sstc),
> +    ISA_EXT_DATA_ENTRY(svinval, 0, ext_svinval),
> +    ISA_EXT_DATA_ENTRY(zihintpause, 0, ext_zihintpause),
> +};
> +
>  static uint64_t kvm_riscv_reg_id(CPURISCVState *env, uint64_t type,
>                                   uint64_t idx)
>  {
> @@ -394,25 +417,66 @@ void kvm_arch_init_irq_routing(KVMState *s)
>  {
>  }
>
> +bool kvm_riscv_ext_supported(int offset)
> +{
> +    int i;
> +
> +    for (i = 0; i < KVM_RISCV_ISA_EXT_MAX; ++i) {

Use ARRAY_SIZE(isa_info_arr) otherwise if there is potential mismatch,
it will cause out-of-bound access

> +        if (isa_info_arr[i].ext_enable_offset == offset) {
> +            return true;
> +        }
> +    }
> +    return false;
> +}
> +
> +static void kvm_riscv_set_isa_ext(CPUState *cs, CPURISCVState *env)
> +{
> +    RISCVCPU *cpu = RISCV_CPU(cs);
> +    unsigned long isa_ext_out;
> +    bool *ext_state;
> +    uint64_t id;
> +    int i, ret;
> +
> +    env->misa_ext = 0;
> +    for (i = 0; i < ARRAY_SIZE(isa_info_arr); i++) {
> +        ext_state = (void *)&cpu->cfg + isa_info_arr[i].ext_enable_offset;
> +        id = kvm_riscv_reg_id(env, KVM_REG_RISCV_ISA_EXT, i);
> +        ret = kvm_get_one_reg(cs, id, &isa_ext_out);
> +        if (ret) {
> +            warn_report("Disabling ext %s due to failure.",

Please report the "ret" in the warning message too.

nits: remove ending .

> +                        isa_info_arr[i].name);
> +            *ext_state = false;
> +            continue;
> +        }
> +        if (isa_ext_out != (*ext_state)) {
> +            isa_ext_out = *ext_state;
> +            ret = kvm_set_one_reg(cs, id, &isa_ext_out);
> +            if (ret) {
> +                warn_report("Could not %s ext %s.",
> +                            (isa_ext_out ? "enable" : "disable"),
> +                            isa_info_arr[i].name);

Please report the "ret" in the warning message too.

nits: remove ending .

> +                *ext_state = !isa_ext_out;
> +            }
> +        }
> +        /*
> +         * If the sigle letter extension is supported by KVM then set

typo: sigle

> +         * the corresponding misa bit for the guest vcpu.
> +         */
> +        if (isa_info_arr[i].misa_bit && (*ext_state)) {
> +            env->misa_ext |= isa_info_arr[i].misa_bit;
> +        }
> +    }
> +}
> +
>  int kvm_arch_init_vcpu(CPUState *cs)
>  {
> -    int ret = 0;
> -    target_ulong isa;
>      RISCVCPU *cpu = RISCV_CPU(cs);
>      CPURISCVState *env = &cpu->env;
> -    uint64_t id;
>
>      qemu_add_vm_change_state_handler(kvm_riscv_vm_state_change, cs);
>
> -    id = kvm_riscv_reg_id(env, KVM_REG_RISCV_CONFIG,
> -                          KVM_REG_RISCV_CONFIG_REG(isa));
> -    ret = kvm_get_one_reg(cs, id, &isa);
> -    if (ret) {
> -        return ret;
> -    }
> -    env->misa_ext = isa;
> -
> -    return ret;
> +    kvm_riscv_set_isa_ext(cs, env);
> +    return 0;
>  }
>
>  int kvm_arch_msi_data_to_gsi(uint32_t data)
> diff --git a/target/riscv/kvm_riscv.h b/target/riscv/kvm_riscv.h
> index ed281bdce0..bdcccc0da4 100644
> --- a/target/riscv/kvm_riscv.h
> +++ b/target/riscv/kvm_riscv.h
> @@ -21,5 +21,5 @@
>
>  void kvm_riscv_reset_vcpu(RISCVCPU *cpu);
>  void kvm_riscv_set_irq(RISCVCPU *cpu, int irq, int level);
> -
> +bool kvm_riscv_ext_supported(int offset);
>  #endif
> --

Regards,
Bin



reply via email to

[Prev in Thread] Current Thread [Next in Thread]