Re: [PATCH] accel/tcg: Allow overlapping reads in record_save

On 8/14/26 07:21, Ilya Chichkov wrote:
> record_save() assumed that a target reads the bytes of an insn as a
> strictly ascending sequence of adjacent chunks, and asserted that each
> read begins exactly where the previous one ended.
> 
> That assumption no longer holds for riscv.  Since f9eaa1542b
> ("target/riscv: support atomic instruction fetch (Ziccif)"),
> decode_opc() loads a full aligned word whenever pc is 4-byte aligned,
> even when the insn turns out to be a 2-byte compressed one, so the
> record may already hold bytes past the end of the insn being
> translated.  When such a compressed insn sits at page offset 0xffc,
> pc_next becomes 0xffe, which is within MAX_INSN_LEN of the end of the
> page, and riscv_tr_translate_insn() probes the next insn to decide
> whether it would cross the page boundary.  That probe reads at offset
> 2 while the record already covers [0,4), and the assert fires:
> 
>    qemu-system-riscv32: accel/tcg/translator.c:395: record_save:
>    Assertion `offset == db->record_start + db->record_len' failed.
> 
> record_save() is only reached when the insn is fetched from MMIO, so
> this is visible on boards that execute code from a region created with
> memory_region_init_io(), such as an XIP flash window mapped over a
> serial flash controller.
> 
> Both sides of the collision are correct: the wide fetch is required for
> Ziccif atomicity, and the probe is required for correct fault reporting
> at a page boundary, per 00c07344fa ("target/riscv: Make translator stop
> before the end of a page").  Unlike a86d3352ab ("target/riscv: do not
> use translator_ldl in opcode_at"), where a non-translation caller had
> no business using translator_ld*, the probe here is a genuine
> translation read whose bytes must be recorded.
> 
> Relax the invariant instead.  Keep requiring that a read neither moves
> backwards nor leaves a gap, but let a read overlapping the recorded
> range extend it only by the bytes past its end.
> 
> Fixes: f9eaa1542b ("target/riscv: support atomic instruction fetch (Ziccif)")
> Signed-off-by: Ilya Chichkov <[email protected]>
> ---
>   accel/tcg/translator.c | 14 +++++++++++---
>   1 file changed, 11 insertions(+), 3 deletions(-)

Reviewed-by: Richard Henderson <[email protected]>

Applied to tcg-next.

r~