Re: [PATCH v2 1/4] target/nios2: Shadow register set

[Richard Henderson]

On 2/24/22 03:48, Amir Gonnen wrote:
> +void helper_eret(CPUNios2State *env, uint32_t new_pc)
> +{
> +    uint32_t crs = cpu_get_crs(env);
> +    if (crs == 0) {
> +        env->regs[CR_STATUS] = env->regs[CR_ESTATUS];
> +    } else {
> +        env->regs[CR_STATUS] = env->regs[R_SSTATUS];
> +    }
> +    cpu_change_reg_set(env, crs, cpu_get_crs(env));

Hmm.  This could probably use a comment that the second computation of cpu_get_crs is 
using the value just restored into CR_STATUS.


> +void helper_wrprs(CPUNios2State *env, uint32_t reg_index, uint32_t value)
> +{
> +    uint32_t prs = cpu_get_prs(env);
> +    env->shadow_regs[prs][reg_index] = value;
> +}
> +
> +uint32_t helper_rdprs(CPUNios2State *env, uint32_t reg_index)
> +{
> +    uint32_t prs = cpu_get_prs(env);
> +    return env->shadow_regs[prs][reg_index];
> +}

These are fairly easy to compute inline, e.g. for rdprs:

    TCGv_i32 t = tcg_temp_i32_new();
    TCGv_ptr p = tcg_temp_ptr_new();

    tcg_gen_extract_i32(t, cpu_R[CR_STATUS],
                        R_CR_STATUS_CRS_SHIFT,
                        R_CR_STATUS_CRS_LENGTH);
    tcg_gen_muli_i32(t, t, sizeof(uint32_t) * NUM_GP_REGS);
    tcg_gen_ext_i32_ptr(p, t);

    tcg_gen_add_ptr(p, p, cpu_env);
    tcg_gen_ld_i32(t, p, offsetof(CPUNios2State, shadow_regs)
                    + sizeof(uint32_t) * instr.a);
    tcg_gen_addi_i32(cpu_R[instr.b], t, instr.imm16.s);

    tcg_temp_free_ptr(p);
    tcg_temp_free_i32(o);

> +static void rdprs(DisasContext *dc, uint32_t code, uint32_t flags)
> +{
> +    I_TYPE(instr, code);
> +    TCGv t = tcg_temp_new();
> +    gen_helper_rdprs(t, cpu_env, tcg_const_i32(instr.a));

You're missing a gen_check_supervisor here and in wrprs.

>  static void eret(DisasContext *dc, uint32_t code, uint32_t flags)
>  {
> -    tcg_gen_mov_tl(cpu_R[CR_STATUS], cpu_R[CR_ESTATUS]);
> -    tcg_gen_mov_tl(cpu_R[R_PC], cpu_R[R_EA]);
> +    gen_helper_eret(cpu_env, cpu_R[R_EA]);

As an existing bug to be fixed by a separate patch, eret should also check for 
supervisor.

> The contents of this email message and any attachments are intended solely for 
> the addressee(s) and may contain confidential and/or privileged information and 
> may be legally protected from disclosure. If you are not the intended recipient 
> of this message or their agent, or if this message has been addressed to you in 
> error, please immediately alert the sender by reply email and then delete this 
> message and any attachments. If you are not the intended recipient, you are 
> hereby notified that any use, dissemination, copying, or storage of this 
> message or its attachments is strictly prohibited.

You really need to suppress these footers when posting to a public mailing list.


r~