On Mon, 10 Mar 2025 at 20:36, Joe Komlodi <koml...@google.com> wrote: > > On ARM hosts with CTR_EL0.DIC and CTR_EL0.IDC set, this would only cause > an ISB to be executed during cache maintenance, which could lead to QEMU > executing TBs containing garbage instructions. > > This seems to be because the ISB finishes executing instructions and > flushes the pipeline, but the ISB doesn't guarantee that writes from the > executed instructions are committed. If a small enough TB is created, it's > possible that the writes setting up the TB aren't committed by the time the > TB is executed.
Yes; we need the DSB to ensure that the stores have completed and are visible to subsequent icache fills; and then we need the ISB to ensure that any instructions that we execute after this are done with an instruction fetch that happens after the ISB (i.e. the CPU hasn't already speculatively fetched the insn before we forced the store to complete). > This function is intended to be a port of the gcc implementation > (https://github.com/gcc-mirror/gcc/blob/85b46d0795ac76bc192cb8f88b646a647acf98c1/libgcc/config/aarch64/sync-cache.c#L67) > which makes the first DSB unconditional, so we can fix the synchronization > issue by doing that as well. > > Signed-off-by: Joe Komlodi <koml...@google.com> We should add: Cc: qemu-sta...@nongnu.org Fixes: 664a79735e4deb1 ("util: Specialize flush_idcache_range for aarch64") > --- > util/cacheflush.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/util/cacheflush.c b/util/cacheflush.c > index a08906155a..1d12899a39 100644 > --- a/util/cacheflush.c > +++ b/util/cacheflush.c > @@ -279,9 +279,11 @@ void flush_idcache_range(uintptr_t rx, uintptr_t rw, > size_t len) > for (p = rw & -dcache_lsize; p < rw + len; p += dcache_lsize) { > asm volatile("dc\tcvau, %0" : : "r" (p) : "memory"); > } > - asm volatile("dsb\tish" : : : "memory"); > } > > + /* DSB unconditionally to ensure any outstanding writes are committed. */ > + asm volatile("dsb\tish" : : : "memory"); > + > /* > * If CTR_EL0.DIC is enabled, Instruction cache cleaning to the Point > * of Unification is not required for instruction to data coherence. Reviewed-by: Peter Maydell <peter.mayd...@linaro.org> Richard, are you doing a TCG pullreq for rc0? If not, I can put this into target-arm.next. thanks -- PMM
