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

[chen huacai <zltjiangshi@gmail.com>]

Hi, Xianglai,

Generally, the subject should be "Fix tlb huge page loading issue"
rather than "Fixed tlb huge page loading issue".

On Thu, Mar 14, 2024 at 9:34 AM Xianglai Li <lixianglai@loongson.cn> wrote:
>
> When we use qemu tcg simulation, the page size of bios is 4KB.
> When using the level 2 super large page (page size is 1G) to create the page table,
> it is found that the content of the corresponding address space is abnormal,
> resulting in the bios can not start the operating system and graphical interface normally.
>
> The lddir and ldpte instruction emulation has
> a problem with the use of super large page processing above level 2.
> The page size is not correctly calculated,
> resulting in the wrong page size of the table entry found by tlb.
>
> Cc: maobibo@loongson.cn
> Cc: Song Gao <gaosong@loongson.cn>
> Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn>
> Cc: zhaotianrui@loongson.cn
> Cc: yijun@loongson.cn
> Cc: wuruiyang@loongson.cn
>
> Signed-off-by: Xianglai Li <lixianglai@loongson.cn>
> ---
>  target/loongarch/cpu-csr.h        |   3 +
>  target/loongarch/internals.h      |   5 --
>  target/loongarch/tcg/tlb_helper.c | 105 ++++++++++++++++++++----------
>  3 files changed, 74 insertions(+), 39 deletions(-)
>
> Changes log:
> V3->V4:
> Optimize the huge page calculation method,
> use the FIELD macro for bit calculation.
>
> V2->V3:
> Delete the intermediate variable LDDIR_PS, and implement lddir and ldpte
> huge pages by referring to the latest architecture reference manual.
>
> V1->V2:
> Modified the patch title format and Enrich the commit mesg description
>
> diff --git a/target/loongarch/cpu-csr.h b/target/loongarch/cpu-csr.h
> index c59d7a9fcb..b0775cf6bf 100644
> --- a/target/loongarch/cpu-csr.h
> +++ b/target/loongarch/cpu-csr.h
> @@ -67,6 +67,9 @@ FIELD(TLBENTRY, D, 1, 1)
>  FIELD(TLBENTRY, PLV, 2, 2)
>  FIELD(TLBENTRY, MAT, 4, 2)
>  FIELD(TLBENTRY, G, 6, 1)
> +FIELD(TLBENTRY, HUGE, 6, 1)
> +FIELD(TLBENTRY, HG, 12, 1)
> +FIELD(TLBENTRY, LEVEL, 13, 2)
>  FIELD(TLBENTRY_32, PPN, 8, 24)
>  FIELD(TLBENTRY_64, PPN, 12, 36)
>  FIELD(TLBENTRY_64, NR, 61, 1)
> diff --git a/target/loongarch/internals.h b/target/loongarch/internals.h
> index a2fc54c8a7..944153b180 100644
> --- a/target/loongarch/internals.h
> +++ b/target/loongarch/internals.h
> @@ -16,11 +16,6 @@
>  #define TARGET_PHYS_MASK MAKE_64BIT_MASK(0, TARGET_PHYS_ADDR_SPACE_BITS)
>  #define TARGET_VIRT_MASK MAKE_64BIT_MASK(0, TARGET_VIRT_ADDR_SPACE_BITS)
>
> -/* Global bit used for lddir/ldpte */
> -#define LOONGARCH_PAGE_HUGE_SHIFT   6
> -/* Global bit for huge page */
> -#define LOONGARCH_HGLOBAL_SHIFT     12
> -
>  void loongarch_translate_init(void);
>
>  void loongarch_cpu_dump_state(CPUState *cpu, FILE *f, int flags);
> diff --git a/target/loongarch/tcg/tlb_helper.c b/target/loongarch/tcg/tlb_helper.c
> index 22be031ac7..b9a8633791 100644
> --- a/target/loongarch/tcg/tlb_helper.c
> +++ b/target/loongarch/tcg/tlb_helper.c
> @@ -17,6 +17,34 @@
>  #include "exec/log.h"
>  #include "cpu-csr.h"
>
> +static void get_dir_base_width(CPULoongArchState *env, uint64_t *dir_base,
> +                               uint64_t *dir_width, target_ulong level)
> +{
> +    switch (level) {
> +    case 1:
> +        *dir_base = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR1_BASE);
> +        *dir_width = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR1_WIDTH);
> +        break;
> +    case 2:
> +        *dir_base = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR2_BASE);
> +        *dir_width = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR2_WIDTH);
> +        break;
> +    case 3:
> +        *dir_base = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR3_BASE);
> +        *dir_width = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR3_WIDTH);
> +        break;
> +    case 4:
> +        *dir_base = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR4_BASE);
> +        *dir_width = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR4_WIDTH);
> +        break;
> +    default:
> +        /* level may be zero for ldpte */
> +        *dir_base = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, PTBASE);
> +        *dir_width = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, PTWIDTH);
> +        break;
> +    }
> +}
> +
>  static void raise_mmu_exception(CPULoongArchState *env, target_ulong address,
>                                  MMUAccessType access_type, int tlb_error)
>  {
> @@ -485,7 +513,23 @@ target_ulong helper_lddir(CPULoongArchState *env, target_ulong base,
>      target_ulong badvaddr, index, phys, ret;
>      int shift;
>      uint64_t dir_base, dir_width;
> -    bool huge = (base >> LOONGARCH_PAGE_HUGE_SHIFT) & 0x1;
> +
> +    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);
> +        }
> +    }
>
>      badvaddr = env->CSR_TLBRBADV;
>      base = base & TARGET_PHYS_MASK;
> @@ -494,33 +538,12 @@ target_ulong helper_lddir(CPULoongArchState *env, target_ulong base,
>      shift = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, PTEWIDTH);
>      shift = (shift + 1) * 3;
>
> -    if (huge) {
> -        return base;
> -    }
> -    switch (level) {
> -    case 1:
> -        dir_base = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR1_BASE);
> -        dir_width = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR1_WIDTH);
> -        break;
> -    case 2:
> -        dir_base = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR2_BASE);
> -        dir_width = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR2_WIDTH);
> -        break;
> -    case 3:
> -        dir_base = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR3_BASE);
> -        dir_width = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR3_WIDTH);
> -        break;
> -    case 4:
> -        dir_base = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR4_BASE);
> -        dir_width = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR4_WIDTH);
> -        break;
> -    default:
> -        do_raise_exception(env, EXCCODE_INE, GETPC());
> -        return 0;
> -    }
> +    get_dir_base_width(env, &dir_base, &dir_width, level);
> +
>      index = (badvaddr >> dir_base) & ((1 << dir_width) - 1);
>      phys = base | index << shift;
>      ret = ldq_phys(cs->as, phys) & TARGET_PHYS_MASK;
> +
>      return ret;
>  }
>
> @@ -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);
> +        }
>
> -    if (huge) {
> -        /* Huge Page. base is paddr */
> -        tmp0 = base ^ (1 << LOONGARCH_PAGE_HUGE_SHIFT);
> -        /* Move Global bit */
> -        tmp0 = ((tmp0 & (1 << LOONGARCH_HGLOBAL_SHIFT))  >>
> -                LOONGARCH_HGLOBAL_SHIFT) << R_TLBENTRY_G_SHIFT |
> -                (tmp0 & (~(1 << LOONGARCH_HGLOBAL_SHIFT)));
> -        ps = ptbase + ptwidth - 1;
> +        /*
> +         * Huge pages are evenly split into parity pages
> +         * when loaded into the tlb,
> +         * so the tlb page size needs to be divided by 2.
> +         */
> +        ps = dir_base + dir_width - 1;
> +        tmp0 = base;
>          if (odd) {
>              tmp0 += MAKE_64BIT_MASK(ps, 1);
>          }
> --
> 2.39.1
>
>


-- 
Huacai Chen