Re: [PATCH V4 1/1] target/loongarch: Fixed tlb huge page loading issue

[Richard Henderson]

On 3/13/24 15:33, Xianglai Li wrote:
> +    if (unlikely((level == 0) || (level > 4))) {
> +        return base;
> +    }
> +
> +    if (FIELD_EX64(base, TLBENTRY, HUGE)) {
> +        if (FIELD_EX64(base, TLBENTRY, LEVEL)) {
> +            return base;
> +        } else {
> +            return  FIELD_DP64(base, TLBENTRY, LEVEL, level);
> +        }
> +
> +        if (unlikely(level == 4)) {
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                          "Attempted use of level %lu huge page\n", level);
> +        }

This block is unreachable, because you've already returned.
Perhaps it would be worthwhile to add another for the level==0 or > 4 case 
above?

> @@ -530,20 +553,34 @@ void helper_ldpte(CPULoongArchState *env, target_ulong 
> base, target_ulong odd,
>       CPUState *cs = env_cpu(env);
>       target_ulong phys, tmp0, ptindex, ptoffset0, ptoffset1, ps, badv;
>       int shift;
> -    bool huge = (base >> LOONGARCH_PAGE_HUGE_SHIFT) & 0x1;
>       uint64_t ptbase = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, PTBASE);
>       uint64_t ptwidth = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, PTWIDTH);
> +    uint64_t dir_base, dir_width;
>   
>       base = base & TARGET_PHYS_MASK;
> +    if (FIELD_EX64(base, TLBENTRY, HUGE)) {
> +        /*
> +         * Gets the huge page level and Gets huge page size
> +         * Clears the huge page level information in the address
> +         * Clears huge page bit
> +         */
> +        get_dir_base_width(env, &dir_base, &dir_width,
> +                           FIELD_EX64(base, TLBENTRY, LEVEL));
> +
> +        FIELD_DP64(base, TLBENTRY, LEVEL, 0);
> +        FIELD_DP64(base, TLBENTRY, HUGE, 0);
> +        if (FIELD_EX64(base, TLBENTRY, HG)) {
> +            FIELD_DP64(base, TLBENTRY, HG, 0);
> +            FIELD_DP64(base, TLBENTRY, G, 1);

FIELD_DP64 returns a value.  You need

    base = FIELD_DP64(base, ...);


r~