On Mon, 2023-08-07 at 11:21 -0700, Richard Henderson wrote:
> On 8/7/23 06:56, Ilya Leoshkevich wrote:
> > One of notdirty_write()'s responsibilities is detecting self-
> > modifying
> > code. Some functions pass the full size of a write to it, some pass
> > 1.
> > When a write to a code section begins before a TB start, but then
> > overlaps the TB, the paths that pass 1 don't flush a TB and don't
> > return to the translator loop.
> >
> > This may be masked, one example being HELPER(vstl). There,
> > probe_write_access() ultimately calls notdirty_write() with a size
> > of
> > 1 and misses self-modifying code. However, cpu_stq_be_data_ra()
> > ultimately calls mmu_watch_or_dirty(), which in turn calls
> > notdirty_write() with the full size.
> >
> > It's still worth improving this, because there may still be
> > user-visible adverse effects in other helpers.
> >
> > Signed-off-by: Ilya Leoshkevich <i...@linux.ibm.com>
>
> IIRC there are some uses of probe_access_* that set size == 0.
> Should we adjust addr+size to cover the whole page for that case?
> That seems to be the intent, anyway.
There is a comment that says we shouldn't do watchpoint/smc detection
in this case:
/* Per the interface, size == 0 merely faults the access. */
if (size == 0) {
return NULL;
}
Come to think of it, qemu-user works this way too: SMC is detected on
the actual access, not the probe:
helper_vstl()
cpu_stq_be_data_ra()
...
stq_he_p()
<signal handler called>
host_signal_handler()
handle_sigsegv_accerr_write()
page_unprotect()
tb_invalidate_phys_page_unwind()
cpu_loop_exit_noexc()
So all this is probably fine, I now think it's better to leave the code
as is, especially given that I cannot reproduce the original problem
anymore.
