Re: [PATCH v2 05/14] target/riscv: Calculate address according to XLEN

[LIU Zhiwei]

On 2021/11/10 下午10:40, Richard Henderson wrote:

On 11/10/21 2:44 PM, LIU Zhiwei wrote:

I suspect the extend should come before the pointer mask and not after, but this is is a weakness in the current RVJ spec that it does not specify how the extension interacts with UXL.  (The reverse ordering would allow a 64-bit os to place a 32-bit application at a base address above 4gb, which could allow address separation without paging enabled.)

Agree. Should we adjust currently, or just add a TODO comment here?

Let's add a todo comment for sure.

I do think we should merge gen_pm_adjust_address into this function so that we only create one new temporary.

I think custom instructions will be added in the future. And possibly there will be  some custom load/store instructions. If we merge gen_pm_adjust_address,  we may have to split it once again at that time.

I don't think so.  We're simply having one function to compute a canonical address from a register plus offset plus mods.

Also, patch 10 combines pm-mask with zero-extension, so we shouldn't need to do both here.  The checks should be combined like

    tcg_gen_addi_tl(addr, src1, imm);
    if (ctx->pm_enabled) {
        tcg_gen_and_tl(addr, addr, pm_mask);
        tcg_gen_or_tl(addr, addr, pm_base);
    } else if (get_xl(ctx) == MXL_RV32) {
        tcg_gen_ext32u_tl(addr, addr);
    }

and could possibly be extended to

    if (ctx->pm_mask_enabled) {
        tcg_gen_and_tl(addr, addr, pm_mask);
    } else if (get_xl(ctx) == MXL_RV32) {
        tcg_gen_ext32u_tl(addr, addr);
    }
    if (ctx->pm_base_enabled) {
        tcg_gen_or_tl(addr, addr, pm_base);
    }

Can we just use

 tcg_gen_and_tl(addr, addr, pm_mask);
 tcg_gen_or_tl(addr, addr, pm_base);

Therefore we can remove all PM flags in TB_FLAGS.

Thanks,
Zhiwei

with one more bit in TB_FLAGS, e.g.

    if (env->cur_pm_mask < (xl == MVL_RV32 ? UINT32_MAX : UINT64_MAX)) {
        flags = FIELD_DP32(flags, TB_FLAGS, PM_MASK_ENABLED, 1);
    }
    if (env->cur_pm_base != 0) {
        flags = FIELD_DP32(flags, TB_FLAGS, PM_BASE_ENABLED, 1);
    }


r~