On Sat, Oct 28, 2023 at 05:54:17AM -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. The RVA22U64
> profile mandates this feature, meaning that applications using this
> profile expects 64 bytes cache blocks.
>
> To make the upcoming RVA22U64 implementation complete, we'll zic64b as
> a 'named feature', not a regular extension. This means that:
>
> - it won't be exposed to users;
> - it won't be written in riscv,isa.
>
> This will be extended to other named extensions in the future, so we're
> creating some common boilerplate for them as well.
>
> zic64b is default to 'true' since we're already using 64 bytes blocks.
> If any cache block size (cbo{m,p,z}_blocksize) is changed to something
> different than 64, zic64b is set to 'false'.
>
> Our profile implementation will then be able to check the current state
> of zic64b and take the appropriate action (e.g. throw a warning).
>
> [1]
> https://github.com/riscv/riscv-profiles/releases/download/v1.0/profiles.pdf
>
> Signed-off-by: Daniel Henrique Barboza <dbarb...@ventanamicro.com>
> ---
> target/riscv/cpu.c | 15 ++++++++++++---
> target/riscv/cpu.h | 3 +++
> target/riscv/cpu_cfg.h | 1 +
> target/riscv/tcg/tcg-cpu.c | 14 ++++++++++++++
> 4 files changed, 30 insertions(+), 3 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 6c0050988f..316d468a19 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1396,6 +1396,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),
> @@ -1425,9 +1431,12 @@ 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("cbop_blocksize", RISCVCPU, cfg.cbop_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("cbop_blocksize", RISCVCPU,
> + cfg.cbop_blocksize, CB_DEF_VALUE),
> + DEFINE_PROP_UINT16("cboz_blocksize", RISCVCPU,
> + cfg.cboz_blocksize, CB_DEF_VALUE),
I wouldn't introduce the CB_DEF_VALUE define. I state why below.
>
> 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[];
> extern const RISCVCPUMultiExtConfig riscv_cpu_deprecated_exts[];
> extern Property riscv_cpu_options[];
>
> diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
> index 2203b4c45b..f61a8434c4 100644
> --- a/target/riscv/cpu_cfg.h
> +++ b/target/riscv/cpu_cfg.h
> @@ -108,6 +108,7 @@ struct RISCVCPUConfig {
> bool ext_smepmp;
> bool rvv_ta_all_1s;
> bool rvv_ma_all_1s;
> + bool zic64b;
>
> uint32_t mvendorid;
> uint64_t marchid;
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index 093bda2e75..65d59bc984 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -264,6 +264,18 @@ static void
> riscv_cpu_disable_priv_spec_isa_exts(RISCVCPU *cpu)
> }
> }
>
> +static void riscv_cpu_validate_zic64b(RISCVCPU *cpu)
> +{
> + cpu->cfg.zic64b = cpu->cfg.cbom_blocksize == CB_DEF_VALUE &&
> + cpu->cfg.cbop_blocksize == CB_DEF_VALUE &&
> + cpu->cfg.cboz_blocksize == CB_DEF_VALUE;
The zic64b name has an explicit 64 in it, so CB_DEF_VALUE must be 64,
which implies it should also be named something with an explicit 64
in it. However, there's really no point in doing
#define NUM_64 64
so I'd just drop the define altogether.
> +}
> +
> +static void riscv_cpu_validate_named_features(RISCVCPU *cpu)
> +{
> + riscv_cpu_validate_zic64b(cpu);
> +}
> +
> /*
> * Check consistency between chosen extensions while setting
> * cpu->cfg accordingly.
> @@ -586,6 +598,8 @@ void riscv_tcg_cpu_finalize_features(RISCVCPU *cpu, Error
> **errp)
> return;
> }
>
> + riscv_cpu_validate_named_features(cpu);
> +
> if (cpu->cfg.ext_smepmp && !cpu->cfg.pmp) {
> /*
> * Enhanced PMP should only be available
> --
> 2.41.0
>
Thanks,
drew
