* [PATCH 2/6] target/ppc: Factor out 4xx ppcemb_tlb_t flushing
2024-01-17 15:12 [PATCH 1/6] target/ppc: Fix 440 tlbwe TLB invalidation gaps Nicholas Piggin
@ 2024-01-17 15:12 ` Nicholas Piggin
2024-01-25 10:39 ` Cédric Le Goater
2024-01-17 15:12 ` [PATCH 3/6] target/ppc: 4xx don't flush TLB for a newly written software TLB entry Nicholas Piggin
` (5 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Nicholas Piggin @ 2024-01-17 15:12 UTC (permalink / raw)
To: qemu-ppc
Cc: Nicholas Piggin, Daniel Henrique Barboza, Cédric Le Goater,
Harsh Prateek Bora, BALATON Zoltan, qemu-devel
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 2/6] target/ppc: Factor out 4xx ppcemb_tlb_t flushing
2024-01-17 15:12 ` [PATCH 2/6] target/ppc: Factor out 4xx ppcemb_tlb_t flushing Nicholas Piggin
@ 2024-01-25 10:39 ` Cédric Le Goater
0 siblings, 0 replies; 13+ messages in thread
From: Cédric Le Goater @ 2024-01-25 10:39 UTC (permalink / raw)
To: Nicholas Piggin, qemu-ppc
Cc: Daniel Henrique Barboza, Harsh Prateek Bora, BALATON Zoltan,
qemu-devel
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);
> }
> }
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/6] target/ppc: 4xx don't flush TLB for a newly written software TLB entry
2024-01-17 15:12 [PATCH 1/6] target/ppc: Fix 440 tlbwe TLB invalidation gaps Nicholas Piggin
2024-01-17 15:12 ` [PATCH 2/6] target/ppc: Factor out 4xx ppcemb_tlb_t flushing Nicholas Piggin
@ 2024-01-17 15:12 ` Nicholas Piggin
2024-01-25 10:44 ` Cédric Le Goater
2024-01-17 15:12 ` [PATCH 4/6] target/ppc: 4xx optimise tlbwe_lo TLB flushing Nicholas Piggin
` (4 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Nicholas Piggin @ 2024-01-17 15:12 UTC (permalink / raw)
To: qemu-ppc
Cc: Nicholas Piggin, Daniel Henrique Barboza, Cédric Le Goater,
Harsh Prateek Bora, BALATON Zoltan, qemu-devel
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 3/6] target/ppc: 4xx don't flush TLB for a newly written software TLB entry
2024-01-17 15:12 ` [PATCH 3/6] target/ppc: 4xx don't flush TLB for a newly written software TLB entry Nicholas Piggin
@ 2024-01-25 10:44 ` Cédric Le Goater
0 siblings, 0 replies; 13+ messages in thread
From: Cédric Le Goater @ 2024-01-25 10:44 UTC (permalink / raw)
To: Nicholas Piggin, qemu-ppc
Cc: Daniel Henrique Barboza, Harsh Prateek Bora, BALATON Zoltan,
qemu-devel
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,
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 4/6] target/ppc: 4xx optimise tlbwe_lo TLB flushing
2024-01-17 15:12 [PATCH 1/6] target/ppc: Fix 440 tlbwe TLB invalidation gaps Nicholas Piggin
2024-01-17 15:12 ` [PATCH 2/6] target/ppc: Factor out 4xx ppcemb_tlb_t flushing Nicholas Piggin
2024-01-17 15:12 ` [PATCH 3/6] target/ppc: 4xx don't flush TLB for a newly written software TLB entry Nicholas Piggin
@ 2024-01-17 15:12 ` Nicholas Piggin
2024-01-25 10:44 ` Cédric Le Goater
2024-01-17 15:12 ` [PATCH 5/6] target/ppc: 440 optimise tlbwe " Nicholas Piggin
` (3 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Nicholas Piggin @ 2024-01-17 15:12 UTC (permalink / raw)
To: qemu-ppc
Cc: Nicholas Piggin, Daniel Henrique Barboza, Cédric Le Goater,
Harsh Prateek Bora, BALATON Zoltan, qemu-devel
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 4/6] target/ppc: 4xx optimise tlbwe_lo TLB flushing
2024-01-17 15:12 ` [PATCH 4/6] target/ppc: 4xx optimise tlbwe_lo TLB flushing Nicholas Piggin
@ 2024-01-25 10:44 ` Cédric Le Goater
0 siblings, 0 replies; 13+ messages in thread
From: Cédric Le Goater @ 2024-01-25 10:44 UTC (permalink / raw)
To: Nicholas Piggin, qemu-ppc
Cc: Daniel Henrique Barboza, Harsh Prateek Bora, BALATON Zoltan,
qemu-devel
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)
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 5/6] target/ppc: 440 optimise tlbwe TLB flushing
2024-01-17 15:12 [PATCH 1/6] target/ppc: Fix 440 tlbwe TLB invalidation gaps Nicholas Piggin
` (2 preceding siblings ...)
2024-01-17 15:12 ` [PATCH 4/6] target/ppc: 4xx optimise tlbwe_lo TLB flushing Nicholas Piggin
@ 2024-01-17 15:12 ` Nicholas Piggin
2024-01-25 10:44 ` Cédric Le Goater
2024-01-17 15:12 ` [PATCH 6/6] target/ppc: optimise ppcemb_tlb_t flushing Nicholas Piggin
` (2 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Nicholas Piggin @ 2024-01-17 15:12 UTC (permalink / raw)
To: qemu-ppc
Cc: Nicholas Piggin, Daniel Henrique Barboza, Cédric Le Goater,
Harsh Prateek Bora, BALATON Zoltan, qemu-devel
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 5/6] target/ppc: 440 optimise tlbwe TLB flushing
2024-01-17 15:12 ` [PATCH 5/6] target/ppc: 440 optimise tlbwe " Nicholas Piggin
@ 2024-01-25 10:44 ` Cédric Le Goater
0 siblings, 0 replies; 13+ messages in thread
From: Cédric Le Goater @ 2024-01-25 10:44 UTC (permalink / raw)
To: Nicholas Piggin, qemu-ppc
Cc: Daniel Henrique Barboza, Harsh Prateek Bora, BALATON Zoltan,
qemu-devel
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) {
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 6/6] target/ppc: optimise ppcemb_tlb_t flushing
2024-01-17 15:12 [PATCH 1/6] target/ppc: Fix 440 tlbwe TLB invalidation gaps Nicholas Piggin
` (3 preceding siblings ...)
2024-01-17 15:12 ` [PATCH 5/6] target/ppc: 440 optimise tlbwe " Nicholas Piggin
@ 2024-01-17 15:12 ` Nicholas Piggin
2024-01-25 10:45 ` Cédric Le Goater
2024-01-25 10:38 ` [PATCH 1/6] target/ppc: Fix 440 tlbwe TLB invalidation gaps Cédric Le Goater
2024-02-16 13:28 ` BALATON Zoltan
6 siblings, 1 reply; 13+ messages in thread
From: Nicholas Piggin @ 2024-01-17 15:12 UTC (permalink / raw)
To: qemu-ppc
Cc: Nicholas Piggin, Daniel Henrique Barboza, Cédric Le Goater,
Harsh Prateek Bora, BALATON Zoltan, qemu-devel
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 6/6] target/ppc: optimise ppcemb_tlb_t flushing
2024-01-17 15:12 ` [PATCH 6/6] target/ppc: optimise ppcemb_tlb_t flushing Nicholas Piggin
@ 2024-01-25 10:45 ` Cédric Le Goater
0 siblings, 0 replies; 13+ messages in thread
From: Cédric Le Goater @ 2024-01-25 10:45 UTC (permalink / raw)
To: Nicholas Piggin, qemu-ppc
Cc: Daniel Henrique Barboza, Harsh Prateek Bora, BALATON Zoltan,
qemu-devel
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);
> }
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/6] target/ppc: Fix 440 tlbwe TLB invalidation gaps
2024-01-17 15:12 [PATCH 1/6] target/ppc: Fix 440 tlbwe TLB invalidation gaps Nicholas Piggin
` (4 preceding siblings ...)
2024-01-17 15:12 ` [PATCH 6/6] target/ppc: optimise ppcemb_tlb_t flushing Nicholas Piggin
@ 2024-01-25 10:38 ` Cédric Le Goater
2024-02-16 13:28 ` BALATON Zoltan
6 siblings, 0 replies; 13+ messages in thread
From: Cédric Le Goater @ 2024-01-25 10:38 UTC (permalink / raw)
To: Nicholas Piggin, qemu-ppc
Cc: Daniel Henrique Barboza, Harsh Prateek Bora, BALATON Zoltan,
qemu-devel
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);
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/6] target/ppc: Fix 440 tlbwe TLB invalidation gaps
2024-01-17 15:12 [PATCH 1/6] target/ppc: Fix 440 tlbwe TLB invalidation gaps Nicholas Piggin
` (5 preceding siblings ...)
2024-01-25 10:38 ` [PATCH 1/6] target/ppc: Fix 440 tlbwe TLB invalidation gaps Cédric Le Goater
@ 2024-02-16 13:28 ` BALATON Zoltan
6 siblings, 0 replies; 13+ messages in thread
From: BALATON Zoltan @ 2024-02-16 13:28 UTC (permalink / raw)
To: Nicholas Piggin
Cc: qemu-ppc, Daniel Henrique Barboza, Cédric Le Goater,
Harsh Prateek Bora, qemu-devel
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
^ permalink raw reply [flat|nested] 13+ messages in thread