Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

translate: missing memory sync callback in translate-a64.c for atomic special case. #89

Open
UlisesLuzius opened this issue Apr 11, 2023 · 2 comments

Comments

@UlisesLuzius
Copy link
Collaborator

In disas_ldst_atomic, in case o3_opc == 014, then we are missing the qflex_post_mem:

    GEN_QFLEX_HELPER(qflex_mem_trace_gen_helper(), GEN_HELPER(qflex_pre_mem)( 
					 cpu_env, clean_addr, tcg_const_i32(MMU_DATA_LOAD), tcg_const_i32(1 << size)));
    GEN_QFLEX_HELPER(qflex_mem_trace_gen_helper(), GEN_HELPER(qflex_pre_mem)( 
					 cpu_env, clean_addr, tcg_const_i32(MMU_DATA_STORE), tcg_const_i32(1 << size)));

    if (o3_opc == 014) {
        /*
         * LDAPR* are a special case because they are a simple load, not a
         * fetch-and-do-something op.
         * The architectural consistency requirements here are weaker than
         * full load-acquire (we only need "load-acquire processor consistent"),
         * but we choose to implement them as full LDAQ.
         */
        do_gpr_ld(s, cpu_reg(s, rt), clean_addr, size, false,
                  true, rt, disas_ldst_compute_iss_sf(size, false, 0), true);
        tcg_gen_mb(TCG_MO_ALL | TCG_BAR_LDAQ);
        return;
    }

    tcg_rs = read_cpu_reg(s, rs, true);
    tcg_rt = cpu_reg(s, rt);

    if (o3_opc == 1) { /* LDCLR */
        tcg_gen_not_i64(tcg_rs, tcg_rs);
    }

    /* The tcg atomic primitives are all full barriers.  Therefore we
     * can ignore the Acquire and Release bits of this instruction.
     */
    fn(tcg_rt, clean_addr, tcg_rs, get_mem_index(s), mop);

    if ((mop & MO_SIGN) && size != MO_64) {
        tcg_gen_ext32u_i64(tcg_rt, tcg_rt);
    }
    GEN_QFLEX_HELPER(qflex_mem_trace_gen_helper(), GEN_HELPER(qflex_post_mem)( 
					 cpu_env, clean_addr, tcg_const_i32(MMU_DATA_LOAD), tcg_const_i32(1 << size)));
    GEN_QFLEX_HELPER(qflex_mem_trace_gen_helper(), GEN_HELPER(qflex_post_mem)( 
					 cpu_env, clean_addr, tcg_const_i32(MMU_DATA_STORE), tcg_const_i32(1 << size)));
@UlisesLuzius
Copy link
Collaborator Author

Also check whenever gen_compare_and_swap_pair the qflex_post_mem and qflex_pre_mem callbacks size argument is too small: clean_addr is size + 1.

static void gen_compare_and_swap_pair(DisasContext *s, int rs, int rt,
                                      int rn, int size)
{
    TCGv_i64 s1 = cpu_reg(s, rs);
    TCGv_i64 s2 = cpu_reg(s, rs + 1);
    TCGv_i64 t1 = cpu_reg(s, rt);
    TCGv_i64 t2 = cpu_reg(s, rt + 1);
    TCGv_i64 clean_addr;
    int memidx = get_mem_index(s);

    if (rn == 31) {
        gen_check_sp_alignment(s);
    }

    /* This is a single atomic access, despite the "pair". */
    clean_addr = gen_mte_check1(s, cpu_reg_sp(s, rn), true, rn != 31, size + 1);

    GEN_QFLEX_HELPER(qflex_mem_trace_gen_helper(), GEN_HELPER(qflex_pre_mem)( 
					 cpu_env, clean_addr, tcg_const_i32(MMU_DATA_LOAD), tcg_const_i32(1 << size)));
    GEN_QFLEX_HELPER(qflex_mem_trace_gen_helper(), GEN_HELPER(qflex_pre_mem)( 
					 cpu_env, clean_addr, tcg_const_i32(MMU_DATA_STORE), tcg_const_i32(1 << size)));
. . .

@UlisesLuzius
Copy link
Collaborator Author

UlisesLuzius commented Apr 11, 2023

gen_load_exclusive and gen_store_exclusive size is not calculated correctly for is_pair case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant