Re: [PATCH 5/5] target/loongarch/tcg: Add hardware page table walker support

[Richard Henderson]

On 7/29/24 11:39, Song Gao wrote:
>       /* Mapped address */
> -    return loongarch_map_address(env, physical, prot, address,
> -                                 access_type, mmu_idx);
> +    ret = loongarch_map_address(env, physical, prot, address,
> +                                access_type, mmu_idx);
> +#ifdef CONFIG_TCG
> +    if (!FIELD_EX32(env->cpucfg[2], CPUCFG2, HPTW)) {
> +        return ret;
> +    }
> +
> +    if (!FIELD_EX32(env->CSR_PWCH, CSR_PWCH, HPTW_EN)) {
> +        return ret;
> +    }
> +
> +    if (do_page_walk(env, address, access_type, ret)) {

When called from loongarch_cpu_get_phys_page_debug, you do not want ...

> +        index = get_random_tlb_index(env, tlbehi, ps);
> +        invalidate_tlb(env, index);
> +        do_fill_tlb_entry(env, vppn, entrylo0, entrylo1, index, ps);

... to modify the TLB.  This will cause gdbstub to modify the behaviour of the guest, 
which you do not want.

> +        entry = ldq_phys(cs->as, tmp0) & TARGET_PHYS_MASK;
> +
> +        if (entry == 0) {
> +            return ret;
> +        }
> +
> +        /* Check entry, and do tlb modify. */
> +        if ((tlb_error == TLBRET_INVALID) &&
> +            (access_type == MMU_DATA_LOAD ||
> +             access_type == MMU_INST_FETCH )) {
> +            if (!(FIELD_EX64(entry, TLBENTRY, PRESENT))) {
> +                break;
> +            }
> +            entry = FIELD_DP64(entry, TLBENTRY, V, 1);
> +        } else if ((tlb_error == TLBRET_INVALID) &&
> +                   access_type == MMU_DATA_STORE) {
> +            if (!((FIELD_EX64(entry, TLBENTRY, PRESENT) &&
> +                  (FIELD_EX64(entry, TLBENTRY, WRITE))))){
> +                break;
> +            }
> +            entry = FIELD_DP64(entry, TLBENTRY, V, 1);
> +            entry = FIELD_DP64(entry, TLBENTRY, D, 1);
> +        } else if (tlb_error ==  TLBRET_DIRTY) {
> +            if (!(FIELD_EX64(entry, TLBENTRY, WRITE))) {
> +                break;
> +            }
> +            entry = FIELD_DP64(entry, TLBENTRY, D, 1);
> +            entry = FIELD_DP64(entry, TLBENTRY, V, 1);
> +        }
> +        stq_phys(cs->as, tmp0, entry);

You certainly want to use a compare and swap here, restarting if the compare 
fails.


r~