Re: [PATCH v2 18/18] target/riscv/kvm.c: read/write (cbom|cboz)_blocksize in KVM

[Andrew Jones]

On Tue, Jun 13, 2023 at 05:58:57PM -0300, Daniel Henrique Barboza wrote:
> If we don't set a proper cbom_blocksize|cboz_blocksize in the FDT the
> Linux Kernel will fail to detect the availability of the CBOM/CBOZ
> extensions, regardless of the contents of the 'riscv,isa' DT prop.
> 
> The FDT is being written using the cpu->cfg.cbom|z_blocksize attributes,
> so let's expose them as user properties like it is already done with
> TCG.
> 
> This will also require us to determine proper blocksize values during
> init() time since the FDT is already created during realize(). We'll
> take a ride in kvm_riscv_init_multiext_cfg() to do it. Note that we
> don't need to fetch both cbom and cboz blocksizes every time: check for
> their parent extensions (icbom and icboz) and only read the blocksizes
> if needed.
> 
> In contrast with cbom/z_blocksize properties from TCG, the user is not
> able to set any value that is different from the 'host' value when
> running KVM. KVM can be particularly harsh dealing with it: a ENOTSUPP
> can be thrown for the mere attempt of executing kvm_set_one_reg() for
> these 2 regs.
> 
> Hopefully, we don't need to call kvm_set_one_reg() for these regs.
> We'll check if the user input matches the host value in
> kvm_cpu_set_cbomz_blksize(), the set() accessor for both blocksize
> properties. We'll fail fast since it's already known to not be
> supported.
> 
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
>  target/riscv/kvm.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 70 insertions(+)
> 
> diff --git a/target/riscv/kvm.c b/target/riscv/kvm.c
> index 3a9f7b0722..ccd2375d8d 100644
> --- a/target/riscv/kvm.c
> +++ b/target/riscv/kvm.c
> @@ -283,6 +283,42 @@ static void kvm_cpu_set_multi_ext_cfg(Object *obj, 
> Visitor *v,
>      kvm_cpu_cfg_set(cpu, multi_ext_cfg, value);
>  }
>  
> +static KVMCPUConfig kvm_cbom_blocksize = {
> +    .name = "cbom_blocksize",
> +    .offset = CPUCFG(cbom_blocksize),
> +    .kvm_reg_id = KVM_REG_RISCV_CONFIG_REG(zicbom_block_size)
> +};
> +
> +static KVMCPUConfig kvm_cboz_blocksize = {
> +    .name = "cboz_blocksize",
> +    .offset = CPUCFG(cboz_blocksize),
> +    .kvm_reg_id = KVM_REG_RISCV_CONFIG_REG(zicboz_block_size)
> +};
> +
> +static void kvm_cpu_set_cbomz_blksize(Object *obj, Visitor *v,
> +                                      const char *name,
> +                                      void *opaque, Error **errp)
> +{
> +    KVMCPUConfig *cbomz_cfg = opaque;
> +    RISCVCPU *cpu = RISCV_CPU(obj);
> +    uint16_t value, *host_val;
> +
> +    if (!visit_type_uint16(v, name, &value, errp)) {
> +        return;
> +    }
> +
> +    host_val = kvmconfig_get_cfg_addr(cpu, cbomz_cfg);
> +
> +    if (value != *host_val) {
> +        error_report("Unable to set %s to a different value than "
> +                     "the host (%u)",
> +                     cbomz_cfg->name, *host_val);
> +        exit(EXIT_FAILURE);
> +    }
> +
> +    cbomz_cfg->user_set = true;
> +}
> +
>  static void kvm_riscv_update_cpu_cfg_isa_ext(RISCVCPU *cpu, CPUState *cs)
>  {
>      CPURISCVState *env = &cpu->env;
> @@ -332,6 +368,14 @@ static void kvm_riscv_add_cpu_user_properties(Object 
> *cpu_obj)
>                              kvm_cpu_set_multi_ext_cfg,
>                              NULL, multi_cfg);
>      }
> +
> +    object_property_add(cpu_obj, "cbom_blocksize", "uint16",
> +                        NULL, kvm_cpu_set_cbomz_blksize,
> +                        NULL, &kvm_cbom_blocksize);
> +
> +    object_property_add(cpu_obj, "cboz_blocksize", "uint16",
> +                        NULL, kvm_cpu_set_cbomz_blksize,
> +                        NULL, &kvm_cboz_blocksize);
>  }
>  
>  static int kvm_riscv_get_regs_core(CPUState *cs)
> @@ -647,6 +691,24 @@ static void kvm_riscv_init_misa_ext_mask(RISCVCPU *cpu,
>      env->misa_ext = env->misa_ext_mask;
>  }
>  
> +static void kvm_riscv_read_cbomz_blksize(RISCVCPU *cpu, KVMScratchCPU 
> *kvmcpu,
> +                                         KVMCPUConfig *cbomz_cfg)
> +{
> +    CPURISCVState *env = &cpu->env;
> +    struct kvm_one_reg reg;
> +    int ret;
> +
> +    reg.id = kvm_riscv_reg_id(env, KVM_REG_RISCV_CONFIG,
> +                              cbomz_cfg->kvm_reg_id);
> +    reg.addr = (uint64_t)kvmconfig_get_cfg_addr(cpu, cbomz_cfg);
> +    ret = ioctl(kvmcpu->cpufd, KVM_GET_ONE_REG, &reg);
> +    if (ret != 0) {
> +        error_report("Unable to read KVM reg %s, error %d",
> +                     cbomz_cfg->name, ret);
> +        exit(EXIT_FAILURE);
> +    }
> +}
> +
>  static void kvm_riscv_init_multiext_cfg(RISCVCPU *cpu, KVMScratchCPU *kvmcpu)
>  {
>      CPURISCVState *env = &cpu->env;
> @@ -678,6 +740,14 @@ static void kvm_riscv_init_multiext_cfg(RISCVCPU *cpu, 
> KVMScratchCPU *kvmcpu)
>  
>          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);
> +    }
>  }
>  
>  void kvm_riscv_init_user_properties(Object *cpu_obj)
> -- 
> 2.40.1
>

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