qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Richard Henderson
Subject: Re: [PATCH v2 05/14] target/riscv: Calculate address according to XLEN
Date: Wed, 10 Nov 2021 15:40:52 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0

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~



reply via email to

[Prev in Thread] Current Thread [Next in Thread]