Re: [PATCH 0/3] target/ppc: fix tlb flushing race

[Nicholas Piggin]

On Thu Mar 28, 2024 at 6:12 PM AEST, Nicholas Piggin wrote:
> On Thu Mar 28, 2024 at 3:31 PM AEST, Nicholas Piggin wrote:
> > ppc broadcast tlb flushes should be synchronised with other vCPUs,
> > like all other architectures that support such operations seem to
> > be doing.
> >
> > Fixing ppc removes the last caller of the non-synced TLB flush
> > variants, we can remove some dead code. I'd like to merge patch 1
> > for 9.0, and hold patches 2 and 3 until 9.1 to avoid churn (unless
> > someone prefers to remove the dead code asap).
>
> Hmm, turns out to not be so simple, this in parts reverts
> the fix in commit 4ddc104689b. Do other architectures
> that use the _synced TLB flush variants have that same problem
> with the TLB flush not actually flushing until the TB ends,
> I wonder?

Huh, I can reproduce that original problem with a little test
case (which I will upstream into kvm-unit-tests).

async_run_on_cpu(this_cpu) seems to flush before the next TB, but
async_safe_run_on_cpu(this_cpu) does not? How does it execute it
without exiting from the TB?

In any case, patch 1 to make it _synced, plus the following,
seems to close both races.

Thanks,
Nick

---

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 93ffec787c..c44e0ce687 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -3495,6 +3495,7 @@ static inline void gen_check_tlb_flush(DisasContext *ctx, 
bool global)
         gen_helper_check_tlb_flush_local(tcg_env);
     }
     gen_set_label(l);
+    ctx->base.is_jmp = DISAS_EXIT_UPDATE;
 }
 #else
 static inline void gen_check_tlb_flush(DisasContext *ctx, bool global) { }
diff --git a/target/ppc/translate/storage-ctrl-impl.c.inc 
b/target/ppc/translate/storage-ctrl-impl.c.inc
index 74c23a4191..673e754404 100644
--- a/target/ppc/translate/storage-ctrl-impl.c.inc
+++ b/target/ppc/translate/storage-ctrl-impl.c.inc
@@ -224,6 +224,9 @@ static bool do_tlbie(DisasContext *ctx, arg_X_tlbie *a, 
bool local)
                                  a->prs << TLBIE_F_PRS_SHIFT |
                                  a->r << TLBIE_F_R_SHIFT |
                                  local << TLBIE_F_LOCAL_SHIFT));
+        if (!local) {
+            ctx->base.is_jmp = DISAS_EXIT_UPDATE;
+        }
         return true;
 #endif