[PATCH 1/6] target/ppc: Fix 440 tlbwe TLB invalidation gaps

[PATCH 1/6] target/ppc: Fix 440 tlbwe TLB invalidation gaps

[Nicholas Piggin]

The 440 software TLB write entry misses several cases that must flush
the TCG TLB:
- If the new size is smaller than the existing size, the EA no longer
  covered should be flushed. This looks like an inverted inequality test.
- If the TLB PID changes.
- If the TLB attr bit 0 (translation address space) changes.
- If low prot (access control) bits change.

Fix this by removing tricks to avoid TLB flushes, and just invalidate
the TLB if any valid entry is being changed, similarly to 4xx.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/ppc/mmu_helper.c | 35 ++++++++++-------------------------
 1 file changed, 10 insertions(+), 25 deletions(-)

diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
index f87d35379a..c140f3c96d 100644
--- a/target/ppc/mmu_helper.c
+++ b/target/ppc/mmu_helper.c
@@ -855,49 +855,34 @@ void helper_440_tlbwe(CPUPPCState *env, uint32_t word, target_ulong entry,
                       target_ulong value)
 {
     ppcemb_tlb_t *tlb;
-    target_ulong EPN, RPN, size;
-    int do_flush_tlbs;
 
     qemu_log_mask(CPU_LOG_MMU, "%s word %d entry %d value " TARGET_FMT_lx "\n",
                   __func__, word, (int)entry, value);
-    do_flush_tlbs = 0;
     entry &= 0x3F;
     tlb = &env->tlb.tlbe[entry];
+
+    /* Invalidate previous TLB (if it's valid) */
+    if (tlb->prot & PAGE_VALID) {
+        tlb_flush(env_cpu(env));
+    }
+
     switch (word) {
     default:
         /* Just here to please gcc */
     case 0:
-        EPN = value & 0xFFFFFC00;
-        if ((tlb->prot & PAGE_VALID) && EPN != tlb->EPN) {
-            do_flush_tlbs = 1;
-        }
-        tlb->EPN = EPN;
-        size = booke_tlb_to_page_size((value >> 4) & 0xF);
-        if ((tlb->prot & PAGE_VALID) && tlb->size < size) {
-            do_flush_tlbs = 1;
-        }
-        tlb->size = size;
+        tlb->EPN = value & 0xFFFFFC00;
+        tlb->size = booke_tlb_to_page_size((value >> 4) & 0xF);
         tlb->attr &= ~0x1;
         tlb->attr |= (value >> 8) & 1;
         if (value & 0x200) {
             tlb->prot |= PAGE_VALID;
         } else {
-            if (tlb->prot & PAGE_VALID) {
-                tlb->prot &= ~PAGE_VALID;
-                do_flush_tlbs = 1;
-            }
+            tlb->prot &= ~PAGE_VALID;
         }
         tlb->PID = env->spr[SPR_440_MMUCR] & 0x000000FF;
-        if (do_flush_tlbs) {
-            tlb_flush(env_cpu(env));
-        }
         break;
     case 1:
-        RPN = value & 0xFFFFFC0F;
-        if ((tlb->prot & PAGE_VALID) && tlb->RPN != RPN) {
-            tlb_flush(env_cpu(env));
-        }
-        tlb->RPN = RPN;
+        tlb->RPN = value & 0xFFFFFC0F;
         break;
     case 2:
         tlb->attr = (tlb->attr & 0x1) | (value & 0x0000FF00);
-- 
2.42.0

[PATCH 2/6] target/ppc: Factor out 4xx ppcemb_tlb_t flushing

[Nicholas Piggin]

Flushing the TCG TLB pages that cache a software TLB is a common
operation, factor it into its own function.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/ppc/mmu_helper.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
index c140f3c96d..949ae87f4f 100644
--- a/target/ppc/mmu_helper.c
+++ b/target/ppc/mmu_helper.c
@@ -749,12 +749,20 @@ target_ulong helper_4xx_tlbre_lo(CPUPPCState *env, target_ulong entry)
     return ret;
 }
 
+static void ppcemb_tlb_flush(CPUState *cs, ppcemb_tlb_t *tlb)
+{
+    target_ulong ea;
+
+    for (ea = tlb->EPN; ea < tlb->EPN + tlb->size; ea += TARGET_PAGE_SIZE) {
+        tlb_flush_page(cs, ea);
+    }
+}
+
 void helper_4xx_tlbwe_hi(CPUPPCState *env, target_ulong entry,
                          target_ulong val)
 {
     CPUState *cs = env_cpu(env);
     ppcemb_tlb_t *tlb;
-    target_ulong page, end;
 
     qemu_log_mask(CPU_LOG_MMU, "%s entry %d val " TARGET_FMT_lx "\n",
                   __func__, (int)entry,
@@ -763,13 +771,10 @@ void helper_4xx_tlbwe_hi(CPUPPCState *env, target_ulong entry,
     tlb = &env->tlb.tlbe[entry];
     /* Invalidate previous TLB (if it's valid) */
     if (tlb->prot & PAGE_VALID) {
-        end = tlb->EPN + tlb->size;
         qemu_log_mask(CPU_LOG_MMU, "%s: invalidate old TLB %d start "
                       TARGET_FMT_lx " end " TARGET_FMT_lx "\n", __func__,
-                      (int)entry, tlb->EPN, end);
-        for (page = tlb->EPN; page < end; page += TARGET_PAGE_SIZE) {
-            tlb_flush_page(cs, page);
-        }
+                      (int)entry, tlb->EPN, tlb->EPN + tlb->size);
+        ppcemb_tlb_flush(cs, tlb);
     }
     tlb->size = booke_tlb_to_page_size((val >> PPC4XX_TLBHI_SIZE_SHIFT)
                                        & PPC4XX_TLBHI_SIZE_MASK);
@@ -805,13 +810,10 @@ void helper_4xx_tlbwe_hi(CPUPPCState *env, target_ulong entry,
                   tlb->prot & PAGE_VALID ? 'v' : '-', (int)tlb->PID);
     /* Invalidate new TLB (if valid) */
     if (tlb->prot & PAGE_VALID) {
-        end = tlb->EPN + tlb->size;
         qemu_log_mask(CPU_LOG_MMU, "%s: invalidate TLB %d start "
                       TARGET_FMT_lx " end " TARGET_FMT_lx "\n", __func__,
-                      (int)entry, tlb->EPN, end);
-        for (page = tlb->EPN; page < end; page += TARGET_PAGE_SIZE) {
-            tlb_flush_page(cs, page);
-        }
+                      (int)entry, tlb->EPN, tlb->EPN + tlb->size);
+        ppcemb_tlb_flush(cs, tlb);
     }
 }
 
-- 
2.42.0

[PATCH 3/6] target/ppc: 4xx don't flush TLB for a newly written software TLB entry

[Nicholas Piggin]

BookE software TLB is implemented by flushing old translations from the
relevant TCG TLB whenever software TLB entries change. This means a new
software TLB entry should not have any corresponding cached TCG TLB
translations, so there is nothing to flush. The exception is multiple
software TLBs that cover the same address and address space, but that
is a programming error and results in undefined behaviour, and flushing
does not give an obviously better outcome in that case either.

Remove the unnecessary flush of a newly written software TLB entry.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/ppc/mmu_helper.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
index 949ae87f4f..68632bf54e 100644
--- a/target/ppc/mmu_helper.c
+++ b/target/ppc/mmu_helper.c
@@ -808,13 +808,6 @@ void helper_4xx_tlbwe_hi(CPUPPCState *env, target_ulong entry,
                   tlb->prot & PAGE_WRITE ? 'w' : '-',
                   tlb->prot & PAGE_EXEC ? 'x' : '-',
                   tlb->prot & PAGE_VALID ? 'v' : '-', (int)tlb->PID);
-    /* Invalidate new TLB (if valid) */
-    if (tlb->prot & PAGE_VALID) {
-        qemu_log_mask(CPU_LOG_MMU, "%s: invalidate TLB %d start "
-                      TARGET_FMT_lx " end " TARGET_FMT_lx "\n", __func__,
-                      (int)entry, tlb->EPN, tlb->EPN + tlb->size);
-        ppcemb_tlb_flush(cs, tlb);
-    }
 }
 
 void helper_4xx_tlbwe_lo(CPUPPCState *env, target_ulong entry,
-- 
2.42.0

[PATCH 4/6] target/ppc: 4xx optimise tlbwe_lo TLB flushing

[Nicholas Piggin]

Rather than tlbwe_lo always flushing all TCG TLBs, have it flush just
those corresponding to the old software TLB, and only if it was valid.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/ppc/mmu_helper.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
index 68632bf54e..923779d052 100644
--- a/target/ppc/mmu_helper.c
+++ b/target/ppc/mmu_helper.c
@@ -813,12 +813,20 @@ void helper_4xx_tlbwe_hi(CPUPPCState *env, target_ulong entry,
 void helper_4xx_tlbwe_lo(CPUPPCState *env, target_ulong entry,
                          target_ulong val)
 {
+    CPUState *cs = env_cpu(env);
     ppcemb_tlb_t *tlb;
 
     qemu_log_mask(CPU_LOG_MMU, "%s entry %i val " TARGET_FMT_lx "\n",
                   __func__, (int)entry, val);
     entry &= PPC4XX_TLB_ENTRY_MASK;
     tlb = &env->tlb.tlbe[entry];
+    /* Invalidate previous TLB (if it's valid) */
+    if (tlb->prot & PAGE_VALID) {
+        qemu_log_mask(CPU_LOG_MMU, "%s: invalidate old TLB %d start "
+                      TARGET_FMT_lx " end " TARGET_FMT_lx "\n", __func__,
+                      (int)entry, tlb->EPN, tlb->EPN + tlb->size);
+        ppcemb_tlb_flush(cs, tlb);
+    }
     tlb->attr = val & PPC4XX_TLBLO_ATTR_MASK;
     tlb->RPN = val & PPC4XX_TLBLO_RPN_MASK;
     tlb->prot = PAGE_READ;
@@ -836,8 +844,6 @@ void helper_4xx_tlbwe_lo(CPUPPCState *env, target_ulong entry,
                   tlb->prot & PAGE_WRITE ? 'w' : '-',
                   tlb->prot & PAGE_EXEC ? 'x' : '-',
                   tlb->prot & PAGE_VALID ? 'v' : '-', (int)tlb->PID);
-
-    env->tlb_need_flush |= TLB_NEED_LOCAL_FLUSH;
 }
 
 target_ulong helper_4xx_tlbsx(CPUPPCState *env, target_ulong address)
-- 
2.42.0

[PATCH 5/6] target/ppc: 440 optimise tlbwe TLB flushing

[Nicholas Piggin]

Have 440 tlbwe flush only the range corresponding to the addresses
covered by the software TLB entry being modified rather than the
entire TLB. This matches what 4xx does.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/ppc/mmu_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
index 923779d052..ba965f1779 100644
--- a/target/ppc/mmu_helper.c
+++ b/target/ppc/mmu_helper.c
@@ -864,7 +864,7 @@ void helper_440_tlbwe(CPUPPCState *env, uint32_t word, target_ulong entry,
 
     /* Invalidate previous TLB (if it's valid) */
     if (tlb->prot & PAGE_VALID) {
-        tlb_flush(env_cpu(env));
+        ppcemb_tlb_flush(env_cpu(env), tlb);
     }
 
     switch (word) {
-- 
2.42.0

[PATCH 6/6] target/ppc: optimise ppcemb_tlb_t flushing

[Nicholas Piggin]

Filter TLB flushing by PID and mmuidx.

Zoltan reports that, together with the previous TLB flush changes,
performance of a sam460ex machine running lame to convert a wav to mp3
is improved nearly 10%:

                  CPU time    TLB partial flushes  TLB elided flushes
Before            37s         508238               7680722
After             34s             73                  1143

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/ppc/mmu_helper.c | 43 +++++++++++++++++++++++++++++++++++------
 1 file changed, 37 insertions(+), 6 deletions(-)

diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
index ba965f1779..c071b4d5e2 100644
--- a/target/ppc/mmu_helper.c
+++ b/target/ppc/mmu_helper.c
@@ -751,11 +751,20 @@ target_ulong helper_4xx_tlbre_lo(CPUPPCState *env, target_ulong entry)
 
 static void ppcemb_tlb_flush(CPUState *cs, ppcemb_tlb_t *tlb)
 {
-    target_ulong ea;
+    unsigned mmu_idx = 0;
 
-    for (ea = tlb->EPN; ea < tlb->EPN + tlb->size; ea += TARGET_PAGE_SIZE) {
-        tlb_flush_page(cs, ea);
+    if (tlb->prot & 0xf) {
+        mmu_idx |= 0x1;
     }
+    if ((tlb->prot >> 4) & 0xf) {
+        mmu_idx |= 0x2;
+    }
+    if (tlb->attr & 1) {
+        mmu_idx <<= 2;
+    }
+
+    tlb_flush_range_by_mmuidx(cs, tlb->EPN, tlb->size, mmu_idx,
+                              TARGET_LONG_BITS);
 }
 
 void helper_4xx_tlbwe_hi(CPUPPCState *env, target_ulong entry,
@@ -770,7 +779,7 @@ void helper_4xx_tlbwe_hi(CPUPPCState *env, target_ulong entry,
     entry &= PPC4XX_TLB_ENTRY_MASK;
     tlb = &env->tlb.tlbe[entry];
     /* Invalidate previous TLB (if it's valid) */
-    if (tlb->prot & PAGE_VALID) {
+    if ((tlb->prot & PAGE_VALID) && tlb->PID == env->spr[SPR_40x_PID]) {
         qemu_log_mask(CPU_LOG_MMU, "%s: invalidate old TLB %d start "
                       TARGET_FMT_lx " end " TARGET_FMT_lx "\n", __func__,
                       (int)entry, tlb->EPN, tlb->EPN + tlb->size);
@@ -821,7 +830,7 @@ void helper_4xx_tlbwe_lo(CPUPPCState *env, target_ulong entry,
     entry &= PPC4XX_TLB_ENTRY_MASK;
     tlb = &env->tlb.tlbe[entry];
     /* Invalidate previous TLB (if it's valid) */
-    if (tlb->prot & PAGE_VALID) {
+    if ((tlb->prot & PAGE_VALID) && tlb->PID == env->spr[SPR_40x_PID]) {
         qemu_log_mask(CPU_LOG_MMU, "%s: invalidate old TLB %d start "
                       TARGET_FMT_lx " end " TARGET_FMT_lx "\n", __func__,
                       (int)entry, tlb->EPN, tlb->EPN + tlb->size);
@@ -851,6 +860,25 @@ target_ulong helper_4xx_tlbsx(CPUPPCState *env, target_ulong address)
     return ppcemb_tlb_search(env, address, env->spr[SPR_40x_PID]);
 }
 
+static bool mmubooke_pid_match(CPUPPCState *env, ppcemb_tlb_t *tlb)
+{
+    if (tlb->PID == env->spr[SPR_BOOKE_PID]) {
+        return true;
+    }
+    if (!env->nb_pids) {
+        return false;
+    }
+
+    if (env->spr[SPR_BOOKE_PID1] && tlb->PID == env->spr[SPR_BOOKE_PID1]) {
+        return true;
+    }
+    if (env->spr[SPR_BOOKE_PID2] && tlb->PID == env->spr[SPR_BOOKE_PID2]) {
+        return true;
+    }
+
+    return false;
+}
+
 /* PowerPC 440 TLB management */
 void helper_440_tlbwe(CPUPPCState *env, uint32_t word, target_ulong entry,
                       target_ulong value)
@@ -863,7 +891,10 @@ void helper_440_tlbwe(CPUPPCState *env, uint32_t word, target_ulong entry,
     tlb = &env->tlb.tlbe[entry];
 
     /* Invalidate previous TLB (if it's valid) */
-    if (tlb->prot & PAGE_VALID) {
+    if ((tlb->prot & PAGE_VALID) && mmubooke_pid_match(env, tlb)) {
+        qemu_log_mask(CPU_LOG_MMU, "%s: invalidate old TLB %d start "
+                      TARGET_FMT_lx " end " TARGET_FMT_lx "\n", __func__,
+                      (int)entry, tlb->EPN, tlb->EPN + tlb->size);
         ppcemb_tlb_flush(env_cpu(env), tlb);
     }
 
-- 
2.42.0

Re: [PATCH 1/6] target/ppc: Fix 440 tlbwe TLB invalidation gaps

[Cédric Le Goater]

On 1/17/24 16:12, Nicholas Piggin wrote:
> The 440 software TLB write entry misses several cases that must flush
> the TCG TLB:
> - If the new size is smaller than the existing size, the EA no longer
>    covered should be flushed. This looks like an inverted inequality test.
> - If the TLB PID changes.
> - If the TLB attr bit 0 (translation address space) changes.
> - If low prot (access control) bits change.
> 
> Fix this by removing tricks to avoid TLB flushes, and just invalidate
> the TLB if any valid entry is being changed, similarly to 4xx.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>


Acked-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

PS: A cover letter would have been nice :) I couldn't find it.


> ---
>   target/ppc/mmu_helper.c | 35 ++++++++++-------------------------
>   1 file changed, 10 insertions(+), 25 deletions(-)
> 
> diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
> index f87d35379a..c140f3c96d 100644
> --- a/target/ppc/mmu_helper.c
> +++ b/target/ppc/mmu_helper.c
> @@ -855,49 +855,34 @@ void helper_440_tlbwe(CPUPPCState *env, uint32_t word, target_ulong entry,
>                         target_ulong value)
>   {
>       ppcemb_tlb_t *tlb;
> -    target_ulong EPN, RPN, size;
> -    int do_flush_tlbs;
>   
>       qemu_log_mask(CPU_LOG_MMU, "%s word %d entry %d value " TARGET_FMT_lx "\n",
>                     __func__, word, (int)entry, value);
> -    do_flush_tlbs = 0;
>       entry &= 0x3F;
>       tlb = &env->tlb.tlbe[entry];
> +
> +    /* Invalidate previous TLB (if it's valid) */
> +    if (tlb->prot & PAGE_VALID) {
> +        tlb_flush(env_cpu(env));
> +    }
> +
>       switch (word) {
>       default:
>           /* Just here to please gcc */
>       case 0:
> -        EPN = value & 0xFFFFFC00;
> -        if ((tlb->prot & PAGE_VALID) && EPN != tlb->EPN) {
> -            do_flush_tlbs = 1;
> -        }
> -        tlb->EPN = EPN;
> -        size = booke_tlb_to_page_size((value >> 4) & 0xF);
> -        if ((tlb->prot & PAGE_VALID) && tlb->size < size) {
> -            do_flush_tlbs = 1;
> -        }
> -        tlb->size = size;
> +        tlb->EPN = value & 0xFFFFFC00;
> +        tlb->size = booke_tlb_to_page_size((value >> 4) & 0xF);
>           tlb->attr &= ~0x1;
>           tlb->attr |= (value >> 8) & 1;
>           if (value & 0x200) {
>               tlb->prot |= PAGE_VALID;
>           } else {
> -            if (tlb->prot & PAGE_VALID) {
> -                tlb->prot &= ~PAGE_VALID;
> -                do_flush_tlbs = 1;
> -            }
> +            tlb->prot &= ~PAGE_VALID;
>           }
>           tlb->PID = env->spr[SPR_440_MMUCR] & 0x000000FF;
> -        if (do_flush_tlbs) {
> -            tlb_flush(env_cpu(env));
> -        }
>           break;
>       case 1:
> -        RPN = value & 0xFFFFFC0F;
> -        if ((tlb->prot & PAGE_VALID) && tlb->RPN != RPN) {
> -            tlb_flush(env_cpu(env));
> -        }
> -        tlb->RPN = RPN;
> +        tlb->RPN = value & 0xFFFFFC0F;
>           break;
>       case 2:
>           tlb->attr = (tlb->attr & 0x1) | (value & 0x0000FF00);

Re: [PATCH 2/6] target/ppc: Factor out 4xx ppcemb_tlb_t flushing

[Cédric Le Goater]

On 1/17/24 16:12, Nicholas Piggin wrote:
> Flushing the TCG TLB pages that cache a software TLB is a common
> operation, factor it into its own function.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>


Acked-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.


> ---
>   target/ppc/mmu_helper.c | 24 +++++++++++++-----------
>   1 file changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
> index c140f3c96d..949ae87f4f 100644
> --- a/target/ppc/mmu_helper.c
> +++ b/target/ppc/mmu_helper.c
> @@ -749,12 +749,20 @@ target_ulong helper_4xx_tlbre_lo(CPUPPCState *env, target_ulong entry)
>       return ret;
>   }
>   
> +static void ppcemb_tlb_flush(CPUState *cs, ppcemb_tlb_t *tlb)
> +{
> +    target_ulong ea;
> +
> +    for (ea = tlb->EPN; ea < tlb->EPN + tlb->size; ea += TARGET_PAGE_SIZE) {
> +        tlb_flush_page(cs, ea);
> +    }
> +}
> +
>   void helper_4xx_tlbwe_hi(CPUPPCState *env, target_ulong entry,
>                            target_ulong val)
>   {
>       CPUState *cs = env_cpu(env);
>       ppcemb_tlb_t *tlb;
> -    target_ulong page, end;
>   
>       qemu_log_mask(CPU_LOG_MMU, "%s entry %d val " TARGET_FMT_lx "\n",
>                     __func__, (int)entry,
> @@ -763,13 +771,10 @@ void helper_4xx_tlbwe_hi(CPUPPCState *env, target_ulong entry,
>       tlb = &env->tlb.tlbe[entry];
>       /* Invalidate previous TLB (if it's valid) */
>       if (tlb->prot & PAGE_VALID) {
> -        end = tlb->EPN + tlb->size;
>           qemu_log_mask(CPU_LOG_MMU, "%s: invalidate old TLB %d start "
>                         TARGET_FMT_lx " end " TARGET_FMT_lx "\n", __func__,
> -                      (int)entry, tlb->EPN, end);
> -        for (page = tlb->EPN; page < end; page += TARGET_PAGE_SIZE) {
> -            tlb_flush_page(cs, page);
> -        }
> +                      (int)entry, tlb->EPN, tlb->EPN + tlb->size);
> +        ppcemb_tlb_flush(cs, tlb);
>       }
>       tlb->size = booke_tlb_to_page_size((val >> PPC4XX_TLBHI_SIZE_SHIFT)
>                                          & PPC4XX_TLBHI_SIZE_MASK);
> @@ -805,13 +810,10 @@ void helper_4xx_tlbwe_hi(CPUPPCState *env, target_ulong entry,
>                     tlb->prot & PAGE_VALID ? 'v' : '-', (int)tlb->PID);
>       /* Invalidate new TLB (if valid) */
>       if (tlb->prot & PAGE_VALID) {
> -        end = tlb->EPN + tlb->size;
>           qemu_log_mask(CPU_LOG_MMU, "%s: invalidate TLB %d start "
>                         TARGET_FMT_lx " end " TARGET_FMT_lx "\n", __func__,
> -                      (int)entry, tlb->EPN, end);
> -        for (page = tlb->EPN; page < end; page += TARGET_PAGE_SIZE) {
> -            tlb_flush_page(cs, page);
> -        }
> +                      (int)entry, tlb->EPN, tlb->EPN + tlb->size);
> +        ppcemb_tlb_flush(cs, tlb);
>       }
>   }
>   

Re: [PATCH 3/6] target/ppc: 4xx don't flush TLB for a newly written software TLB entry

[Cédric Le Goater]

On 1/17/24 16:12, Nicholas Piggin wrote:
> BookE software TLB is implemented by flushing old translations from the
> relevant TCG TLB whenever software TLB entries change. This means a new
> software TLB entry should not have any corresponding cached TCG TLB
> translations, so there is nothing to flush. The exception is multiple
> software TLBs that cover the same address and address space, but that
> is a programming error and results in undefined behaviour, and flushing
> does not give an obviously better outcome in that case either.
> 
> Remove the unnecessary flush of a newly written software TLB entry.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>


Acked-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.


> ---
>   target/ppc/mmu_helper.c | 7 -------
>   1 file changed, 7 deletions(-)
> 
> diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
> index 949ae87f4f..68632bf54e 100644
> --- a/target/ppc/mmu_helper.c
> +++ b/target/ppc/mmu_helper.c
> @@ -808,13 +808,6 @@ void helper_4xx_tlbwe_hi(CPUPPCState *env, target_ulong entry,
>                     tlb->prot & PAGE_WRITE ? 'w' : '-',
>                     tlb->prot & PAGE_EXEC ? 'x' : '-',
>                     tlb->prot & PAGE_VALID ? 'v' : '-', (int)tlb->PID);
> -    /* Invalidate new TLB (if valid) */
> -    if (tlb->prot & PAGE_VALID) {
> -        qemu_log_mask(CPU_LOG_MMU, "%s: invalidate TLB %d start "
> -                      TARGET_FMT_lx " end " TARGET_FMT_lx "\n", __func__,
> -                      (int)entry, tlb->EPN, tlb->EPN + tlb->size);
> -        ppcemb_tlb_flush(cs, tlb);
> -    }
>   }
>   
>   void helper_4xx_tlbwe_lo(CPUPPCState *env, target_ulong entry,

Re: [PATCH 4/6] target/ppc: 4xx optimise tlbwe_lo TLB flushing

[Cédric Le Goater]

On 1/17/24 16:12, Nicholas Piggin wrote:
> Rather than tlbwe_lo always flushing all TCG TLBs, have it flush just
> those corresponding to the old software TLB, and only if it was valid.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>



Acked-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
>   target/ppc/mmu_helper.c | 10 ++++++++--
>   1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
> index 68632bf54e..923779d052 100644
> --- a/target/ppc/mmu_helper.c
> +++ b/target/ppc/mmu_helper.c
> @@ -813,12 +813,20 @@ void helper_4xx_tlbwe_hi(CPUPPCState *env, target_ulong entry,
>   void helper_4xx_tlbwe_lo(CPUPPCState *env, target_ulong entry,
>                            target_ulong val)
>   {
> +    CPUState *cs = env_cpu(env);
>       ppcemb_tlb_t *tlb;
>   
>       qemu_log_mask(CPU_LOG_MMU, "%s entry %i val " TARGET_FMT_lx "\n",
>                     __func__, (int)entry, val);
>       entry &= PPC4XX_TLB_ENTRY_MASK;
>       tlb = &env->tlb.tlbe[entry];
> +    /* Invalidate previous TLB (if it's valid) */
> +    if (tlb->prot & PAGE_VALID) {
> +        qemu_log_mask(CPU_LOG_MMU, "%s: invalidate old TLB %d start "
> +                      TARGET_FMT_lx " end " TARGET_FMT_lx "\n", __func__,
> +                      (int)entry, tlb->EPN, tlb->EPN + tlb->size);
> +        ppcemb_tlb_flush(cs, tlb);
> +    }
>       tlb->attr = val & PPC4XX_TLBLO_ATTR_MASK;
>       tlb->RPN = val & PPC4XX_TLBLO_RPN_MASK;
>       tlb->prot = PAGE_READ;
> @@ -836,8 +844,6 @@ void helper_4xx_tlbwe_lo(CPUPPCState *env, target_ulong entry,
>                     tlb->prot & PAGE_WRITE ? 'w' : '-',
>                     tlb->prot & PAGE_EXEC ? 'x' : '-',
>                     tlb->prot & PAGE_VALID ? 'v' : '-', (int)tlb->PID);
> -
> -    env->tlb_need_flush |= TLB_NEED_LOCAL_FLUSH;
>   }
>   
>   target_ulong helper_4xx_tlbsx(CPUPPCState *env, target_ulong address)

Re: [PATCH 5/6] target/ppc: 440 optimise tlbwe TLB flushing

[Cédric Le Goater]

On 1/17/24 16:12, Nicholas Piggin wrote:
> Have 440 tlbwe flush only the range corresponding to the addresses
> covered by the software TLB entry being modified rather than the
> entire TLB. This matches what 4xx does.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>


Acked-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.


> ---
>   target/ppc/mmu_helper.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
> index 923779d052..ba965f1779 100644
> --- a/target/ppc/mmu_helper.c
> +++ b/target/ppc/mmu_helper.c
> @@ -864,7 +864,7 @@ void helper_440_tlbwe(CPUPPCState *env, uint32_t word, target_ulong entry,
>   
>       /* Invalidate previous TLB (if it's valid) */
>       if (tlb->prot & PAGE_VALID) {
> -        tlb_flush(env_cpu(env));
> +        ppcemb_tlb_flush(env_cpu(env), tlb);
>       }
>   
>       switch (word) {

Re: [PATCH 6/6] target/ppc: optimise ppcemb_tlb_t flushing

[Cédric Le Goater]

On 1/17/24 16:12, Nicholas Piggin wrote:
> Filter TLB flushing by PID and mmuidx.
> 
> Zoltan reports that, together with the previous TLB flush changes,
> performance of a sam460ex machine running lame to convert a wav to mp3
> is improved nearly 10%:
> 
>                    CPU time    TLB partial flushes  TLB elided flushes
> Before            37s         508238               7680722
> After             34s             73                  1143
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>



Acked-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.


> ---
>   target/ppc/mmu_helper.c | 43 +++++++++++++++++++++++++++++++++++------
>   1 file changed, 37 insertions(+), 6 deletions(-)
> 
> diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
> index ba965f1779..c071b4d5e2 100644
> --- a/target/ppc/mmu_helper.c
> +++ b/target/ppc/mmu_helper.c
> @@ -751,11 +751,20 @@ target_ulong helper_4xx_tlbre_lo(CPUPPCState *env, target_ulong entry)
>   
>   static void ppcemb_tlb_flush(CPUState *cs, ppcemb_tlb_t *tlb)
>   {
> -    target_ulong ea;
> +    unsigned mmu_idx = 0;
>   
> -    for (ea = tlb->EPN; ea < tlb->EPN + tlb->size; ea += TARGET_PAGE_SIZE) {
> -        tlb_flush_page(cs, ea);
> +    if (tlb->prot & 0xf) {
> +        mmu_idx |= 0x1;
>       }
> +    if ((tlb->prot >> 4) & 0xf) {
> +        mmu_idx |= 0x2;
> +    }
> +    if (tlb->attr & 1) {
> +        mmu_idx <<= 2;
> +    }
> +
> +    tlb_flush_range_by_mmuidx(cs, tlb->EPN, tlb->size, mmu_idx,
> +                              TARGET_LONG_BITS);
>   }
>   
>   void helper_4xx_tlbwe_hi(CPUPPCState *env, target_ulong entry,
> @@ -770,7 +779,7 @@ void helper_4xx_tlbwe_hi(CPUPPCState *env, target_ulong entry,
>       entry &= PPC4XX_TLB_ENTRY_MASK;
>       tlb = &env->tlb.tlbe[entry];
>       /* Invalidate previous TLB (if it's valid) */
> -    if (tlb->prot & PAGE_VALID) {
> +    if ((tlb->prot & PAGE_VALID) && tlb->PID == env->spr[SPR_40x_PID]) {
>           qemu_log_mask(CPU_LOG_MMU, "%s: invalidate old TLB %d start "
>                         TARGET_FMT_lx " end " TARGET_FMT_lx "\n", __func__,
>                         (int)entry, tlb->EPN, tlb->EPN + tlb->size);
> @@ -821,7 +830,7 @@ void helper_4xx_tlbwe_lo(CPUPPCState *env, target_ulong entry,
>       entry &= PPC4XX_TLB_ENTRY_MASK;
>       tlb = &env->tlb.tlbe[entry];
>       /* Invalidate previous TLB (if it's valid) */
> -    if (tlb->prot & PAGE_VALID) {
> +    if ((tlb->prot & PAGE_VALID) && tlb->PID == env->spr[SPR_40x_PID]) {
>           qemu_log_mask(CPU_LOG_MMU, "%s: invalidate old TLB %d start "
>                         TARGET_FMT_lx " end " TARGET_FMT_lx "\n", __func__,
>                         (int)entry, tlb->EPN, tlb->EPN + tlb->size);
> @@ -851,6 +860,25 @@ target_ulong helper_4xx_tlbsx(CPUPPCState *env, target_ulong address)
>       return ppcemb_tlb_search(env, address, env->spr[SPR_40x_PID]);
>   }
>   
> +static bool mmubooke_pid_match(CPUPPCState *env, ppcemb_tlb_t *tlb)
> +{
> +    if (tlb->PID == env->spr[SPR_BOOKE_PID]) {
> +        return true;
> +    }
> +    if (!env->nb_pids) {
> +        return false;
> +    }
> +
> +    if (env->spr[SPR_BOOKE_PID1] && tlb->PID == env->spr[SPR_BOOKE_PID1]) {
> +        return true;
> +    }
> +    if (env->spr[SPR_BOOKE_PID2] && tlb->PID == env->spr[SPR_BOOKE_PID2]) {
> +        return true;
> +    }
> +
> +    return false;
> +}
> +
>   /* PowerPC 440 TLB management */
>   void helper_440_tlbwe(CPUPPCState *env, uint32_t word, target_ulong entry,
>                         target_ulong value)
> @@ -863,7 +891,10 @@ void helper_440_tlbwe(CPUPPCState *env, uint32_t word, target_ulong entry,
>       tlb = &env->tlb.tlbe[entry];
>   
>       /* Invalidate previous TLB (if it's valid) */
> -    if (tlb->prot & PAGE_VALID) {
> +    if ((tlb->prot & PAGE_VALID) && mmubooke_pid_match(env, tlb)) {
> +        qemu_log_mask(CPU_LOG_MMU, "%s: invalidate old TLB %d start "
> +                      TARGET_FMT_lx " end " TARGET_FMT_lx "\n", __func__,
> +                      (int)entry, tlb->EPN, tlb->EPN + tlb->size);
>           ppcemb_tlb_flush(env_cpu(env), tlb);
>       }
>   

Re: [PATCH 1/6] target/ppc: Fix 440 tlbwe TLB invalidation gaps

[BALATON Zoltan]

On Thu, 18 Jan 2024, Nicholas Piggin wrote:
> The 440 software TLB write entry misses several cases that must flush
> the TCG TLB:
> - If the new size is smaller than the existing size, the EA no longer
>  covered should be flushed. This looks like an inverted inequality test.
> - If the TLB PID changes.
> - If the TLB attr bit 0 (translation address space) changes.
> - If low prot (access control) bits change.
>
> Fix this by removing tricks to avoid TLB flushes, and just invalidate
> the TLB if any valid entry is being changed, similarly to 4xx.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

This series was missing a cover letter so patchew did not pick it up 
correctly. However this improves the sam460ex performance a lot so I'd 
like this to be included in 9.0 release. Nick, maybe it's time to start 
merging patches and send a pull request to avoid getting conflicts in last 
minute that could cause series to miss release. So an early pull request 
would help to get everybody on the same page.

Regards,
BALATON Zoltan