[PATCH 3/3] target/loongarch: Remove unnecessary page size validity checking

Bibo Mao posted 3 patches 5 months ago
Maintainers: Song Gao <gaosong@loongson.cn>
[PATCH 3/3] target/loongarch: Remove unnecessary page size validity checking
Posted by Bibo Mao 5 months ago
Page size of TLB entry comes from CSR STLBPS and pwcl register. With
huge page, it is dir_base + dir_width from pwcl register. With normal
page, it is field of PTBASE from pwcl register.

So it is ok to check validity in function helper_ldpte() and function
helper_csrwr_stlbps(). And it is unnecessary in tlb entry fill path.

Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
 target/loongarch/tcg/tlb_helper.c | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/target/loongarch/tcg/tlb_helper.c b/target/loongarch/tcg/tlb_helper.c
index dc48b0f4d2..21381ba59f 100644
--- a/target/loongarch/tcg/tlb_helper.c
+++ b/target/loongarch/tcg/tlb_helper.c
@@ -173,12 +173,6 @@ static void fill_tlb_entry(CPULoongArchState *env, int index)
         lo1 = env->CSR_TLBELO1;
     }
 
-    /*check csr_ps */
-    if (!check_ps(env, csr_ps)) {
-        qemu_log_mask(LOG_GUEST_ERROR, "csr_ps %d is illegal\n", csr_ps);
-        return;
-    }
-
     /* Only MTLB has the ps fields */
     if (index >= LOONGARCH_STLB) {
         tlb->tlb_misc = FIELD_DP64(tlb->tlb_misc, TLB_MISC, PS, csr_ps);
@@ -340,23 +334,16 @@ void helper_tlbfill(CPULoongArchState *env)
 
     if (FIELD_EX64(env->CSR_TLBRERA, CSR_TLBRERA, ISTLBR)) {
         entryhi = env->CSR_TLBREHI;
+        /* Validity of pagesize is checked in helper_ldpte() */
         pagesize = FIELD_EX64(env->CSR_TLBREHI, CSR_TLBREHI, PS);
     } else {
         entryhi = env->CSR_TLBEHI;
+        /* Validity of pagesize is checked in helper_tlbrd() */
         pagesize = FIELD_EX64(env->CSR_TLBIDX, CSR_TLBIDX, PS);
     }
 
-    if (!check_ps(env, pagesize)) {
-        qemu_log_mask(LOG_GUEST_ERROR, "pagesize %d is illegal\n", pagesize);
-        return;
-    }
-
+    /* Validity of stlb_ps is checked in helper_csrwr_stlbps() */
     stlb_ps = FIELD_EX64(env->CSR_STLBPS, CSR_STLBPS, PS);
-    if (!check_ps(env, stlb_ps)) {
-        qemu_log_mask(LOG_GUEST_ERROR, "stlb_ps %d is illegal\n", stlb_ps);
-        return;
-    }
-
     if (pagesize == stlb_ps) {
         /* Only write into STLB bits [47:13] */
         address = entryhi & ~MAKE_64BIT_MASK(0, R_CSR_TLBEHI_64_VPPN_SHIFT);
@@ -651,6 +638,11 @@ void helper_ldpte(CPULoongArchState *env, target_ulong base, target_ulong odd,
         if (odd) {
             tmp0 += MAKE_64BIT_MASK(ps, 1);
         }
+
+        if (!check_ps(env, ps)) {
+            qemu_log_mask(LOG_GUEST_ERROR, "Illegal huge pagesize %ld\n", ps);
+            return;
+        }
     } else {
         badv = env->CSR_TLBRBADV;
 
-- 
2.39.3
Re: [PATCH 3/3] target/loongarch: Remove unnecessary page size validity checking
Posted by gaosong 4 months, 2 weeks ago
在 2025/6/18 下午2:26, Bibo Mao 写道:
> Page size of TLB entry comes from CSR STLBPS and pwcl register. With
> huge page, it is dir_base + dir_width from pwcl register. With normal
> page, it is field of PTBASE from pwcl register.
>
> So it is ok to check validity in function helper_ldpte() and function
> helper_csrwr_stlbps(). And it is unnecessary in tlb entry fill path.
>
> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> ---
>   target/loongarch/tcg/tlb_helper.c | 24 ++++++++----------------
>   1 file changed, 8 insertions(+), 16 deletions(-)
Reviewed-by: Song Gao <gaosong@loongson.cn>

Thanks.
Song Gao
> diff --git a/target/loongarch/tcg/tlb_helper.c b/target/loongarch/tcg/tlb_helper.c
> index dc48b0f4d2..21381ba59f 100644
> --- a/target/loongarch/tcg/tlb_helper.c
> +++ b/target/loongarch/tcg/tlb_helper.c
> @@ -173,12 +173,6 @@ static void fill_tlb_entry(CPULoongArchState *env, int index)
>           lo1 = env->CSR_TLBELO1;
>       }
>   
> -    /*check csr_ps */
> -    if (!check_ps(env, csr_ps)) {
> -        qemu_log_mask(LOG_GUEST_ERROR, "csr_ps %d is illegal\n", csr_ps);
> -        return;
> -    }
> -
>       /* Only MTLB has the ps fields */
>       if (index >= LOONGARCH_STLB) {
>           tlb->tlb_misc = FIELD_DP64(tlb->tlb_misc, TLB_MISC, PS, csr_ps);
> @@ -340,23 +334,16 @@ void helper_tlbfill(CPULoongArchState *env)
>   
>       if (FIELD_EX64(env->CSR_TLBRERA, CSR_TLBRERA, ISTLBR)) {
>           entryhi = env->CSR_TLBREHI;
> +        /* Validity of pagesize is checked in helper_ldpte() */
>           pagesize = FIELD_EX64(env->CSR_TLBREHI, CSR_TLBREHI, PS);
>       } else {
>           entryhi = env->CSR_TLBEHI;
> +        /* Validity of pagesize is checked in helper_tlbrd() */
>           pagesize = FIELD_EX64(env->CSR_TLBIDX, CSR_TLBIDX, PS);
>       }
>   
> -    if (!check_ps(env, pagesize)) {
> -        qemu_log_mask(LOG_GUEST_ERROR, "pagesize %d is illegal\n", pagesize);
> -        return;
> -    }
> -
> +    /* Validity of stlb_ps is checked in helper_csrwr_stlbps() */
>       stlb_ps = FIELD_EX64(env->CSR_STLBPS, CSR_STLBPS, PS);
> -    if (!check_ps(env, stlb_ps)) {
> -        qemu_log_mask(LOG_GUEST_ERROR, "stlb_ps %d is illegal\n", stlb_ps);
> -        return;
> -    }
> -
>       if (pagesize == stlb_ps) {
>           /* Only write into STLB bits [47:13] */
>           address = entryhi & ~MAKE_64BIT_MASK(0, R_CSR_TLBEHI_64_VPPN_SHIFT);
> @@ -651,6 +638,11 @@ void helper_ldpte(CPULoongArchState *env, target_ulong base, target_ulong odd,
>           if (odd) {
>               tmp0 += MAKE_64BIT_MASK(ps, 1);
>           }
> +
> +        if (!check_ps(env, ps)) {
> +            qemu_log_mask(LOG_GUEST_ERROR, "Illegal huge pagesize %ld\n", ps);
> +            return;
> +        }
>       } else {
>           badv = env->CSR_TLBRBADV;
>