| Message ID | 20240117151238.93323-1-npiggin@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/6] target/ppc: Fix 440 tlbwe TLB invalidation gaps | expand |
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);
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
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);
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(-)