Re: [PATCH v5 01/10] target/riscv/tcg: add 'zic64b' support

[Andrew Jones]

On Wed, Oct 25, 2023 at 08:44:50PM -0300, Daniel Henrique Barboza wrote:
> zic64b is defined in the RVA22U64 profile [1] as a named feature for
> "Cache blocks must be 64 bytes in size, naturally aligned in the address
> space".  It's a fantasy name for 64 bytes cache blocks. RVA22U64
> mandates this feature, meaning that applications using it expects 64
> bytes cache blocks.
> 
> In theory we're already compliant to it since we're using 64 bytes cache
> block sizes by default, but nothing is stopping users from enabling a
> profile and changing the cache block size at the same time.
> 
> We'll add zic64b as a 'named feature', not a regular extension, in a
> sense that we won't write it in riscv,isa. It'll be used solely to track
> whether the user changed cache sizes and if we should warn about it.
> 
> zic64b is default to 'true' since we're already using 64 bytes blocks.
> If any cache block size (cbom_blocksize or cboz_blocksize) is changed to
> something different than 64, zic64b is set to 'false' and, if zic64b was
> set to 'true' in the command line, also throw an user warning.
> 
> Our profile implementation set mandatory extensions as if users enabled
> them in the command line, so this logic will extend to the future RVA22U64
> implementation as well.
> 
> [1] 
> https://github.com/riscv/riscv-profiles/releases/download/v1.0/profiles.pdf
> 
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
>  target/riscv/cpu.c         | 12 ++++++++++--
>  target/riscv/cpu.h         |  3 +++
>  target/riscv/cpu_cfg.h     |  1 +
>  target/riscv/tcg/tcg-cpu.c | 26 ++++++++++++++++++++++++++
>  4 files changed, 40 insertions(+), 2 deletions(-)
> 
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index f40da4c661..5095f093ba 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1394,6 +1394,12 @@ const RISCVCPUMultiExtConfig 
> riscv_cpu_experimental_exts[] = {
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> +const RISCVCPUMultiExtConfig riscv_cpu_named_features[] = {
> +    MULTI_EXT_CFG_BOOL("zic64b", zic64b, true),
> +
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
>  /* Deprecated entries marked for future removal */
>  const RISCVCPUMultiExtConfig riscv_cpu_deprecated_exts[] = {
>      MULTI_EXT_CFG_BOOL("Zifencei", ext_zifencei, true),
> @@ -1423,8 +1429,10 @@ Property riscv_cpu_options[] = {
>      DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
>      DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
>  
> -    DEFINE_PROP_UINT16("cbom_blocksize", RISCVCPU, cfg.cbom_blocksize, 64),
> -    DEFINE_PROP_UINT16("cboz_blocksize", RISCVCPU, cfg.cboz_blocksize, 64),
> +    DEFINE_PROP_UINT16("cbom_blocksize", RISCVCPU,
> +                       cfg.cbom_blocksize, CB_DEF_VALUE),
> +    DEFINE_PROP_UINT16("cboz_blocksize", RISCVCPU,
> +                       cfg.cboz_blocksize, CB_DEF_VALUE),
>  
>      DEFINE_PROP_END_OF_LIST(),
>  };
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 8efc4d83ec..ee9abe61d6 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -64,6 +64,8 @@ extern const uint32_t misa_bits[];
>  const char *riscv_get_misa_ext_name(uint32_t bit);
>  const char *riscv_get_misa_ext_description(uint32_t bit);
>  
> +#define CB_DEF_VALUE 64
> +
>  #define CPU_CFG_OFFSET(_prop) offsetof(struct RISCVCPUConfig, _prop)
>  
>  /* Privileged specification version */
> @@ -745,6 +747,7 @@ typedef struct RISCVCPUMultiExtConfig {
>  extern const RISCVCPUMultiExtConfig riscv_cpu_extensions[];
>  extern const RISCVCPUMultiExtConfig riscv_cpu_vendor_exts[];
>  extern const RISCVCPUMultiExtConfig riscv_cpu_experimental_exts[];
> +extern const RISCVCPUMultiExtConfig riscv_cpu_named_features[];

We should add a line like

  riscv_obj_add_multiext_props(obj, qdict_out, riscv_cpu_named_features);

to cpu-model-expansion. zic64b is actually exactly what we need there in
order to describe the blocksize with a boolean, since we don't have any
way to expose the blocksize right now with that query.

Thanks,
drew