Re: [PATCH v2 2/2] target/riscv/kvm: support KVM_GET_REG_LIST

[Andrew Jones]

On Tue, Oct 03, 2023 at 10:21:48AM -0300, Daniel Henrique Barboza wrote:
> KVM for RISC-V started supporting KVM_GET_REG_LIST in Linux 6.6. It
> consists of a KVM ioctl() that retrieves a list of all available regs
> for get_one_reg/set_one_reg. Regs that aren't present in the list aren't
> supported in the host.
> 
> This simplifies our lives when initing the KVM regs since we don't have
> to always attempt a KVM_GET_ONE_REG for all regs QEMU knows. We'll only
> attempt a get_one_reg() if we're sure the reg is supported, i.e. it was
> retrieved by KVM_GET_REG_LIST. Any error in get_one_reg() will then
> always considered fatal, instead of having to handle special error codes
> that might indicate a non-fatal failure.
> 
> Start by moving the current kvm_riscv_init_multiext_cfg() logic into a
> new kvm_riscv_read_multiext_legacy() helper. We'll prioritize using
> KVM_GET_REG_LIST, so check if we have it available and, in case we
> don't, use the legacy() logic.
> 
> Otherwise, retrieve the available reg list and use it to check if the
> host supports our known KVM regs, doing the usual get_one_reg() for
> the supported regs and setting cpu->cfg accordingly.
> 
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
>  target/riscv/kvm/kvm-cpu.c | 96 +++++++++++++++++++++++++++++++++++++-
>  1 file changed, 95 insertions(+), 1 deletion(-)
> 
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index c3daf74fe9..090d617627 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -771,7 +771,8 @@ static void kvm_riscv_read_cbomz_blksize(RISCVCPU *cpu, 
> KVMScratchCPU *kvmcpu,
>      }
>  }
>  
> -static void kvm_riscv_init_multiext_cfg(RISCVCPU *cpu, KVMScratchCPU *kvmcpu)
> +static void kvm_riscv_read_multiext_legacy(RISCVCPU *cpu,
> +                                           KVMScratchCPU *kvmcpu)
>  {
>      CPURISCVState *env = &cpu->env;
>      uint64_t val;
> @@ -812,6 +813,99 @@ static void kvm_riscv_init_multiext_cfg(RISCVCPU *cpu, 
> KVMScratchCPU *kvmcpu)
>      }
>  }
>  
> +static int uint64_cmp(const void *a, const void *b)
> +{
> +    uint64_t val1 = *(const uint64_t *)a;
> +    uint64_t val2 = *(const uint64_t *)b;
> +
> +    if (val1 < val2) {
> +        return -1;
> +    }
> +
> +    if (val1 > val2) {
> +        return 1;
> +    }
> +
> +    return 0;
> +}
> +
> +static void kvm_riscv_init_multiext_cfg(RISCVCPU *cpu, KVMScratchCPU *kvmcpu)
> +{
> +    KVMCPUConfig *multi_ext_cfg;
> +    struct kvm_one_reg reg;
> +    struct kvm_reg_list rl_struct;
> +    struct kvm_reg_list *reglist;
> +    uint64_t val, reg_id, *reg_search;
> +    int i, ret;
> +
> +    rl_struct.n = 0;
> +    ret = ioctl(kvmcpu->cpufd, KVM_GET_REG_LIST, &rl_struct);
> +
> +    /*
> +     * If KVM_GET_REG_LIST isn't supported we'll get errno 22
> +     * (EINVAL). Use read_legacy() in this case.
> +     */
> +    if (errno == EINVAL) {
> +        return kvm_riscv_read_multiext_legacy(cpu, kvmcpu);
> +    } else if (errno != E2BIG) {
> +        /*
> +         * E2BIG is an expected error message for the API since we
> +         * don't know the number of registers. The right amount will
> +         * be written in rl_struct.n.
> +         *
> +         * Error out if we get any other errno.
> +         */
> +        error_report("Error when accessing get-reg-list, code: %s",
> +                     strerrorname_np(errno));
> +        exit(EXIT_FAILURE);
> +    }
> +
> +    reglist = g_malloc(sizeof(struct kvm_reg_list) +
> +                       rl_struct.n * sizeof(uint64_t));
> +    reglist->n = rl_struct.n;
> +    ret = ioctl(kvmcpu->cpufd, KVM_GET_REG_LIST, reglist);
> +    if (ret) {
> +        error_report("Error when reading KVM_GET_REG_LIST, code %s ",
> +                     strerrorname_np(errno));
> +        exit(EXIT_FAILURE);
> +    }
> +
> +    /* sort reglist to use bsearch() */
> +    qsort(&reglist->reg, reglist->n, sizeof(uint64_t), uint64_cmp);
> +
> +    for (i = 0; i < ARRAY_SIZE(kvm_multi_ext_cfgs); i++) {
> +        multi_ext_cfg = &kvm_multi_ext_cfgs[i];
> +        reg_id = kvm_riscv_reg_id(&cpu->env, KVM_REG_RISCV_ISA_EXT,
> +                                  multi_ext_cfg->kvm_reg_id);
> +        reg_search = bsearch(&reg_id, reglist->reg, reglist->n,
> +                             sizeof(uint64_t), uint64_cmp);
> +        if (!reg_search) {
> +            continue;
> +        }
> +
> +        reg.id = reg_id;
> +        reg.addr = (uint64_t)&val;
> +        ret = ioctl(kvmcpu->cpufd, KVM_GET_ONE_REG, &reg);
> +        if (ret != 0) {
> +            error_report("Unable to read ISA_EXT KVM register %s, "
> +                         "error code: %s", multi_ext_cfg->name,
> +                         strerrorname_np(errno));
> +            exit(EXIT_FAILURE);
> +        }
> +
> +        multi_ext_cfg->supported = true;
> +        kvm_cpu_cfg_set(cpu, multi_ext_cfg, val);
> +    }
> +
> +    if (cpu->cfg.ext_icbom) {
> +        kvm_riscv_read_cbomz_blksize(cpu, kvmcpu, &kvm_cbom_blocksize);
> +    }
> +
> +    if (cpu->cfg.ext_icboz) {
> +        kvm_riscv_read_cbomz_blksize(cpu, kvmcpu, &kvm_cboz_blocksize);
> +    }
> +}
> +
>  static void riscv_init_kvm_registers(Object *cpu_obj)
>  {
>      RISCVCPU *cpu = RISCV_CPU(cpu_obj);
> -- 
> 2.41.0
>

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