Re: [PATCH v3 03/12] target/loongarch: Reduce TLB flush with helper_tlbwr

[Richard Henderson <richard.henderson@linaro.org>]

On 9/3/25 10:48, Bibo Mao wrote:
> With function helper_tlbwr(), specified LoongArch TLB entry will be
> updated. There are two PTE pages in one TLB entry called even/odd
> pages. Supposing even/odd page is normal/none state, when odd page
> is added, TLB entry is changed as normal/normal state and even page
> keeps unchanged.
> 
> In this situation, it is not necessary to flush QEMU TLB since even
> page keep unchanged and odd page is newly changed. Here check whether
> PTE page is the same or not, TLB flush can be skipped if both are the
> same or newly added.
> 
> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> ---
>   target/loongarch/tcg/tlb_helper.c | 33 ++++++++++++++++++++++++++-----
>   1 file changed, 28 insertions(+), 5 deletions(-)
> 
> diff --git a/target/loongarch/tcg/tlb_helper.c b/target/loongarch/tcg/tlb_helper.c
> index fcd03ca320..331b485b1a 100644
> --- a/target/loongarch/tcg/tlb_helper.c
> +++ b/target/loongarch/tcg/tlb_helper.c
> @@ -302,16 +302,39 @@ void helper_tlbrd(CPULoongArchState *env)
>   void helper_tlbwr(CPULoongArchState *env)
>   {
>       int index = FIELD_EX64(env->CSR_TLBIDX, CSR_TLBIDX, INDEX);
> +    LoongArchTLB *old, new;

Perhaps "new = { }", then ...

> +    new.tlb_misc = 0;
> +    new.tlb_entry0 = 0;
> +    new.tlb_entry1 = 0;

... this is unnecessary.

> +    fill_tlb_entry(env, &new);
> +    /* Check whether ASID/VPPN is the same */
> +    if (old->tlb_misc == new.tlb_misc) {
> +        /* Check whether both even/odd pages is the same or invalid */
> +        tlb_v0 = FIELD_EX64(old->tlb_entry0, TLBENTRY, V);
> +        tlb_v1 = FIELD_EX64(old->tlb_entry1, TLBENTRY, V);
> +        if ((!tlb_v0 || new.tlb_entry0 == old->tlb_entry0) &&
> +            (!tlb_v1 || new.tlb_entry1 == old->tlb_entry1)) {
> +            skip_inv = true;
> +        }
> +    }
> +
> +    /* flush tlb before updating the entry */
> +    if (!skip_inv) {
> +        invalidate_tlb(env, index);
> +    }
> +    old->tlb_misc = new.tlb_misc;
> +    old->tlb_entry0 = new.tlb_entry0;
> +    old->tlb_entry1 = new.tlb_entry1;

Perhaps better as "*old = new".

Anyway,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~