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

[Richard Henderson]

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);
    }

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~