esp32s3: SIGSEGV in generated code under -accel tcg,thread=multi (softmmu TLB addend read as zero) (QEMU-314)

[ProlificLab]

Summary

With two TCG translation threads (-accel tcg,thread=multi, the default when
-icount is not used), qemu-system-xtensa -machine esp32s3 dies with SIGSEGV
a few seconds into a guest boot, roughly one run in fifteen, but only when the
host is busy. The fault is inside generated code and the address it dies on is
a guest address, which points at a softmmu TLB entry being used while its
addend is zero.

With -accel tcg,thread=single it does not happen at all. That is what we run
with now, so this is not blocking us; it is filed because the underlying defect
is still there for anyone who does not.

Environment

• Tag esp-develop-9.2.2-20260417 (QEMU 9.2.2), -machine esp32s3.
• Host: x86-64 Linux (Ubuntu 24.04 container), 14 cores.
• Guest: dual-core ESP32-S3 running an unmodified Arduino/ESP-IDF firmware
  (Meshtastic 2.7.10 for the LilyGO T-Deck), 8 MB octal PSRAM.

What the fault looks like

Four captures, from separate processes:

=== signal 11 at address 0x3fcb45eb, faulting pc 0x740e800763bd ===
=== signal 11 at address 0x3fcb45eb, faulting pc 0x70050807693d ===
=== signal 11 at address 0x3fcb45eb, faulting pc 0x733e7007643d ===
=== signal 11 at address 0x3fcf14db, faulting pc 0x61a102c3de3b ===

Two things stand out.

si_addr is a guest address — 0x3fcb45eb and 0x3fcf14db are both in the
S3's internal SRAM (0x3FC88000+). It repeats across processes, so it is not a
wild host pointer; ASLR would have moved that. The softmmu fast path reaches
host memory as guest_addr + entry->addend, so faulting on the guest address
itself means addend was 0 at the moment the access was made — the entry was
read while it held nothing.

The faulting PC is outside the binary's executable segment
(0x374000–0x9f612d), i.e. in the generated-code buffer, which is consistent
with the access coming from a TLB fast path rather than a helper.

Reproduction

It needs load. Fourteen concurrent boots, each with its own build directory:

for i in $(seq 1 14); do
    qemu-system-xtensa -machine esp32s3 -accel tcg,thread=multi \
        -drive file=flash$i.bin,if=mtd,format=raw \
        -m 8M -display none -no-reboot ... &
done
wait

• 14 parallel runs, thread=multi: ~1 fault per 15 runs (2/28, then 3/56).
• The same runs sequentially on an idle host: 0 faults in 28.
• Under gdb: 0 faults in 12+ runs. The timing shift closes the window, which
  is why this went unexamined for a long time here.

Since the crash does not survive being watched, the captures above come from a
SIGSEGV handler installed inside the process that prints si_addr and the
ucontext RIP, rather than from a debugger or a core file.

Caveat on the reproduction

I should be straight about this: our boot uses several out-of-tree devices of
our own (a bus bridge that forwards unmodelled MMIO to a host process over a
socket, plus GPIO/SPI/I2C/I2S models). They are not in your tree, so this is not
a reproduction you can run as-is, and I cannot rule out that they are involved.

Two things argue they are not the mechanism:

• They only ever call memory_region_add_subregion_overlap at device realize.
  Nothing of ours changes a mapping, enables/disables a region, or flushes a
  TLB while the machine is running.
• The fault is on plain internal SRAM, not on any address our devices claim.

What I could not do is a clean A/B: with our devices removed the guest only
reaches about half as far into its boot before the harness stops it, so "no
crash without them" would not have meant anything.

What I ruled out

hw/misc/esp32s3_cache.c writes guest RAM directly through
memory_region_get_ram_ptr() in two places — filling a page from flash via
blk_pread(), and stamping a discarded page with 0xdeadbeef — without
telling QEMU the memory changed. That is a real hazard on its own (any code
already translated from those pages stays live over contents that no longer
exist), and it was my best candidate.

It is not this defect. Declaring both writes with memory_region_set_dirty()
changed the crash rate not at all: 3 faults in 56 runs, against the same rate
before. I reverted it rather than carry a patch that fixes nothing, but the
missing invalidation is probably still worth a separate look.

Workaround

-accel tcg,thread=single. On this workload it costs nothing measurable — the
guest's peripherals are serialised through a single socket anyway, so a second
translation thread mostly contends — and in one paired measurement it was
several times faster.

[ProlificLab]

Found it: tcg_iommu_unmap_notify flushes another CPU's TLB from the wrong thread

The crash I reported here is a cross-thread TLB flush, and it is reachable on
any esp32s3 machine with -accel tcg,thread=multi.

The mechanism

tlb_flush() is not a request; in accel/tcg/cputlb.c it performs the flush
on the calling thread:

void tlb_flush_by_mmuidx(CPUState *cpu, uint16_t idxmap)
{
    assert_cpu_is_self(cpu);
    tlb_flush_by_mmuidx_async_work(cpu, RUN_ON_CPU_HOST_INT(idxmap));
}

despite the callee's name. It is correct only when the caller is cpu's own
thread, and assert_cpu_is_self() is compiled out unless DEBUG_TLB is
defined, so a violation is silent in any normal build.

system/physmem.c violates it:

static void tcg_iommu_unmap_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
{
    ...
    tlb_flush(notifier->cpu);
    notifier->active = false;
}

There is one TCGIOMMUNotifier per CPU per IOMMU, and
memory_region_notify_iommu() walks all of them on whichever thread wrote the
IOMMU's registers. hw/misc/esp32s3_cache.c models the flash/PSRAM cache as an
IOMMU region, so every guest write to an MMU entry notifies it — and with one
TCG thread per vCPU, core 0's write clears core 1's TLB from core 0's
thread, while core 1 is executing translated code that has already loaded an
entry out of it.

It is a frequent path, not a rare one

A diagnostic in that function, reporting when notifier->cpu is not the
calling thread, on one ordinary firmware boot with thread=multi:

=== iommu notify: thread of cpu 0 flushing cpu 1 (cross 1, self 20)
=== iommu notify: thread of cpu 0 flushing cpu 1 (cross 2, self 22)
=== iommu notify: thread of cpu 0 flushing cpu 1 (cross 3, self 23)

Three cross-thread whole-TLB flushes per boot, against twenty-three legitimate
local ones. What makes the crash rare is only the width of the window inside
the flush, which is why it needs a loaded machine and never fires under gdb.

It explains the signature

The fault is inside generated code, at a guest address in internal RAM
(0x3fcb45eb, 0x3fcf14db) — i.e. the entry's addend was read as zero. Note that
internal RAM is never remapped by the cache: tlb_flush() clears every MMU
index, not the page that moved, so the region that faults is not the region
whose mapping changed. That is why my earlier guesses at a stale cache mapping
went nowhere.

Fix

Hand the flush to the CPU that owns the table — the pattern flush_all_helper()
in the same subsystem already uses:

diff --git a/system/physmem.c b/system/physmem.c
index 7538906..5c6c35c 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -587,6 +587,11 @@ typedef struct TCGIOMMUNotifier {
     bool active;
 } TCGIOMMUNotifier;
 
+static void tcg_iommu_tlb_flush_async(CPUState *cpu, run_on_cpu_data data)
+{
+    tlb_flush(cpu);
+}
+
 static void tcg_iommu_unmap_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
 {
     TCGIOMMUNotifier *notifier = container_of(n, TCGIOMMUNotifier, n);
@@ -594,7 +599,20 @@ static void tcg_iommu_unmap_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
     if (!notifier->active) {
         return;
     }
-    tlb_flush(notifier->cpu);
+    /* tlb_flush() performs the flush on the calling thread, against the table
+     * of whichever CPU it is handed, so it is only safe for the CPU whose
+     * thread this is. There is one notifier per CPU and the notify arrives on
+     * whichever thread wrote the IOMMU's registers, so with a TCG thread per
+     * vCPU this would clear another CPU's table while that CPU is executing
+     * translated code that has already read an entry out of it. Leave the
+     * flush to the CPU that owns the table.
+     */
+    if (qemu_cpu_is_self(notifier->cpu)) {
+        tlb_flush(notifier->cpu);
+    } else {
+        async_run_on_cpu(notifier->cpu, tcg_iommu_tlb_flush_async,
+                         RUN_ON_CPU_NULL);
+    }
     notifier->active = false;
     /* We leave the notifier struct on the list to avoid reallocating it later.
      * Generally the number of IOMMUs a CPU deals with will be small.

async_run_on_cpu() takes the target's own work mutex rather than the BQL, so
it is safe from a device write on any thread, and it kicks the target so the
flush lands promptly. The queued function deliberately takes only the CPU and
not the notifier, since notifiers are freed in tcg_exec_unrealizefn().

Measured against the reproduction in this issue — fourteen concurrent boots,
thread=multi. The plain rate is about one boot in a hundred (2 faults in 224
runs), which is too rare to demonstrate a fix, so I also built with a patch
that repeats a wrong-thread flush to widen the window, and compared the two
builds at the same setting with that amplifier in both:

build	runs	faults
amplified, no fix	84	5
amplified, with this patch	84	0

Note that the rate I first reported in this issue — about one in fifteen — was
too high; re-measuring gives nearer one in a hundred.

Note for whoever picks this up

This is upstream code and the bug is not ESP32-specific — any TCG target with
an IOMMU region and thread=multi can hit it. What makes the esp32s3 machine
a reliable trigger is that its cache model rewrites MMU entries during ordinary
flash access, so the notify fires constantly.

Two things I did not change, for the record:

• notifier->active is still written from the notifying thread and read and
  written from the target's, with no synchronisation. Losing that store would
  skip a later flush and leave a stale mapping. I have not observed it
  misbehave, so I have not touched it.
• esp32s3_cache.c writes guest RAM directly through
  memory_region_get_ram_ptr() (filling a page with blk_pread(), and
  stamping a discarded page with 0xdeadbeef) without invalidating translated
  code for those pages. That is a separate hazard; declaring the writes dirty
  changed this crash's rate not at all, so it is not this bug.