[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 5/6] target/riscv: Gate hardware A/D PTE bit updating
From: |
Alistair Francis |
Subject: |
Re: [PATCH v3 5/6] target/riscv: Gate hardware A/D PTE bit updating |
Date: |
Thu, 15 Feb 2024 15:46:36 +1000 |
On Sat, Feb 3, 2024 at 1:22 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> From: Andrew Jones <ajones@ventanamicro.com>
>
> Gate hardware A/D PTE bit updating on {m,h}envcfg.ADUE and only
> enable menvcfg.ADUE on reset if svade has not been selected. Now
> that we also consider svade, we have four possible configurations:
>
> 1) !svade && !svadu
> use hardware updating and there's no way to disable it
> (the default, which maintains past behavior. Maintaining
> the default, even with !svadu is a change that fixes [1])
>
> 2) !svade && svadu
> use hardware updating, but also provide {m,h}envcfg.ADUE,
> allowing software to switch to exception mode
> (being able to switch is a change which fixes [1])
>
> 3) svade && !svadu
> use exception mode and there's no way to switch to hardware
> updating
> (this behavior change fixes [2])
>
> 4) svade && svadu
> use exception mode, but also provide {m,h}envcfg.ADUE,
> allowing software to switch to hardware updating
> (this behavior change fixes [2])
>
> Fixes: 0af3f115e68e ("target/riscv: Add *envcfg.HADE related check in address
> translation") [1]
> Fixes: 48531f5adb2a ("target/riscv: implement svade") [2]
> Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> target/riscv/cpu.c | 3 ++-
> target/riscv/cpu_helper.c | 19 +++++++++++++++----
> target/riscv/tcg/tcg-cpu.c | 15 +++++----------
> 3 files changed, 22 insertions(+), 15 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 9045f87481..50ac7845a8 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -960,7 +960,8 @@ static void riscv_cpu_reset_hold(Object *obj)
> env->two_stage_lookup = false;
>
> env->menvcfg = (cpu->cfg.ext_svpbmt ? MENVCFG_PBMTE : 0) |
> - (cpu->cfg.ext_svadu ? MENVCFG_ADUE : 0);
> + (!cpu->cfg.ext_svade && cpu->cfg.ext_svadu ?
> + MENVCFG_ADUE : 0);
> env->henvcfg = 0;
>
> /* Initialized default priorities of local interrupts. */
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 8da9104da4..3a440833f8 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -907,7 +907,9 @@ static int get_physical_address(CPURISCVState *env,
> hwaddr *physical,
> }
>
> bool pbmte = env->menvcfg & MENVCFG_PBMTE;
> - bool adue = env->menvcfg & MENVCFG_ADUE;
> + bool svade = riscv_cpu_cfg(env)->ext_svade;
> + bool svadu = riscv_cpu_cfg(env)->ext_svadu;
> + bool adue = svadu ? env->menvcfg & MENVCFG_ADUE : !svade;
>
> if (first_stage && two_stage && env->virt_enabled) {
> pbmte = pbmte && (env->henvcfg & HENVCFG_PBMTE);
> @@ -1082,9 +1084,18 @@ restart:
> return TRANSLATE_FAIL;
> }
>
> - /* If necessary, set accessed and dirty bits. */
> - target_ulong updated_pte = pte | PTE_A |
> - (access_type == MMU_DATA_STORE ? PTE_D : 0);
> + target_ulong updated_pte = pte;
> +
> + /*
> + * If ADUE is enabled, set accessed and dirty bits.
> + * Otherwise raise an exception if necessary.
> + */
> + if (adue) {
> + updated_pte |= PTE_A | (access_type == MMU_DATA_STORE ? PTE_D : 0);
> + } else if (!(pte & PTE_A) ||
> + (access_type == MMU_DATA_STORE && !(pte & PTE_D))) {
> + return TRANSLATE_FAIL;
> + }
>
> /* Page table updates need to be atomic with MTTCG enabled */
> if (updated_pte != pte && !is_debug) {
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index 673097c6e4..43c32b4a15 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -196,17 +196,14 @@ static bool cpu_cfg_offset_is_named_feat(uint32_t
> ext_offset)
>
> static void riscv_cpu_enable_named_feat(RISCVCPU *cpu, uint32_t feat_offset)
> {
> - switch (feat_offset) {
> - case CPU_CFG_OFFSET(ext_zic64b):
> + /*
> + * All other named features are already enabled
> + * in riscv_tcg_cpu_instance_init().
> + */
> + if (feat_offset == CPU_CFG_OFFSET(ext_zic64b)) {
> cpu->cfg.cbom_blocksize = 64;
> cpu->cfg.cbop_blocksize = 64;
> cpu->cfg.cboz_blocksize = 64;
> - break;
> - case CPU_CFG_OFFSET(ext_svade):
> - cpu->cfg.ext_svadu = false;
> - break;
> - default:
> - g_assert_not_reached();
> }
> }
>
> @@ -348,8 +345,6 @@ static void riscv_cpu_update_named_features(RISCVCPU *cpu)
> cpu->cfg.ext_zic64b = cpu->cfg.cbom_blocksize == 64 &&
> cpu->cfg.cbop_blocksize == 64 &&
> cpu->cfg.cboz_blocksize == 64;
> -
> - cpu->cfg.ext_svade = !cpu->cfg.ext_svadu;
> }
>
> static void riscv_cpu_validate_g(RISCVCPU *cpu)
> --
> 2.43.0
>
>
- Re: [PATCH v3 3/6] target/riscv: add remaining named features, (continued)
- Re: [PATCH v3 3/6] target/riscv: add remaining named features, Andrew Jones, 2024/02/15
- Re: [PATCH v3 3/6] target/riscv: add remaining named features, Andrew Jones, 2024/02/15
- Re: [PATCH v3 3/6] target/riscv: add remaining named features, Conor Dooley, 2024/02/15
- Re: [PATCH v3 3/6] target/riscv: add remaining named features, Andrew Jones, 2024/02/15
- Re: [PATCH v3 3/6] target/riscv: add remaining named features, Conor Dooley, 2024/02/15
- Re: [PATCH v3 3/6] target/riscv: add remaining named features, Alistair Francis, 2024/02/15
- Re: [PATCH v3 3/6] target/riscv: add remaining named features, Conor Dooley, 2024/02/16
[PATCH v3 6/6] target/riscv: Promote svade to a normal extension, Daniel Henrique Barboza, 2024/02/02
[PATCH v3 5/6] target/riscv: Gate hardware A/D PTE bit updating, Daniel Henrique Barboza, 2024/02/02
- Re: [PATCH v3 5/6] target/riscv: Gate hardware A/D PTE bit updating,
Alistair Francis <=
[PATCH v3 4/6] target/riscv: Reset henvcfg to zero, Daniel Henrique Barboza, 2024/02/02
Re: [PATCH v3 0/6] riscv: named features riscv,isa, 'svade' rework, Alistair Francis, 2024/02/15