[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5 01/10] target/riscv/tcg: add 'zic64b' support
From: |
Andrew Jones |
Subject: |
Re: [PATCH v5 01/10] target/riscv/tcg: add 'zic64b' support |
Date: |
Thu, 26 Oct 2023 14:04:53 +0200 |
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[];
> 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 6eef4a51ea..b6693320d3 100644
> --- a/target/riscv/cpu_cfg.h
> +++ b/target/riscv/cpu_cfg.h
> @@ -107,6 +107,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..ac5f65a757 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -264,6 +264,27 @@ static void
> riscv_cpu_disable_priv_spec_isa_exts(RISCVCPU *cpu)
> }
> }
>
> +static void riscv_cpu_validate_zic64b(RISCVCPU *cpu)
> +{
> + const char *warn = "zic64b set to 'true' but %s is set to %u. "
> + "zic64b changed to 'false'";
> + bool send_warn = cpu_cfg_ext_is_user_set(CPU_CFG_OFFSET(zic64b));
> +
> + if (cpu->cfg.cbom_blocksize != CB_DEF_VALUE) {
> + cpu->cfg.zic64b = false;
> + if (send_warn) {
> + warn_report(warn, "cbom_blocksize", cpu->cfg.cbom_blocksize);
> + }
> + }
> +
> + if (cpu->cfg.cboz_blocksize != CB_DEF_VALUE) {
> + cpu->cfg.zic64b = false;
> + if (send_warn) {
> + warn_report(warn, "cboz_blocksize", cpu->cfg.cboz_blocksize);
> + }
I think if a user does X=1,X=2 and expects X to be the superposition of 1
and 2 they should get an error instead of a warning, and that's what
zic64b=on,cbom_blocksize=128 would effectively be asking to do, so let's
error out here instead of warn.
> + }
> +}
> +
> /*
> * Check consistency between chosen extensions while setting
> * cpu->cfg accordingly.
> @@ -394,6 +415,10 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu,
> Error **errp)
> return;
> }
>
> + if (cpu->cfg.zic64b) {
> + riscv_cpu_validate_zic64b(cpu);
> + }
We set this on by default, so if a user just gives cbom_blocksize=128,
then we'll get the warning (or failure). They'd have to do
zic64b=off,cbom_blocksize=128 instead to avoid it. I think zic64b needs
to be off by default and then automatically turn on during cpu finalize
if all blocksize properties are 64.
> +
> if (cpu->cfg.ext_zvfh) {
> cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zvfhmin), true);
> }
> @@ -889,6 +914,7 @@ static void riscv_cpu_add_user_properties(Object *obj)
> riscv_cpu_add_multiext_prop_array(obj, riscv_cpu_extensions);
> riscv_cpu_add_multiext_prop_array(obj, riscv_cpu_vendor_exts);
> riscv_cpu_add_multiext_prop_array(obj, riscv_cpu_experimental_exts);
> + riscv_cpu_add_multiext_prop_array(obj, riscv_cpu_named_features);
>
> riscv_cpu_add_multiext_prop_array(obj, riscv_cpu_deprecated_exts);
>
> --
> 2.41.0
>
Thanks,
drew
- [PATCH v5 00/10] RVA22U64 profile support, Daniel Henrique Barboza, 2023/10/25
- [PATCH v5 03/10] target/riscv/kvm: add 'rva22u64' flag as unavailable, Daniel Henrique Barboza, 2023/10/25
- [PATCH v5 02/10] target/riscv: add rva22u64 profile definition, Daniel Henrique Barboza, 2023/10/25
- [PATCH v5 04/10] target/riscv/tcg: add user flag for profile support, Daniel Henrique Barboza, 2023/10/25
- [PATCH v5 06/10] target/riscv/tcg: add riscv_cpu_write_misa_bit(), Daniel Henrique Barboza, 2023/10/25
- [PATCH v5 05/10] target/riscv/tcg: add MISA user options hash, Daniel Henrique Barboza, 2023/10/25
- [PATCH v5 01/10] target/riscv/tcg: add 'zic64b' support, Daniel Henrique Barboza, 2023/10/25
- [PATCH v5 08/10] target/riscv/tcg: add hash table insert helpers, Daniel Henrique Barboza, 2023/10/25
- [PATCH v5 07/10] target/riscv/tcg: handle profile MISA bits, Daniel Henrique Barboza, 2023/10/25
- [PATCH v5 10/10] target/riscv/tcg: warn if profile exts are disabled, Daniel Henrique Barboza, 2023/10/25
- [PATCH v5 09/10] target/riscv/tcg: honor user choice for G MISA bits, Daniel Henrique Barboza, 2023/10/25