Re: [PATCH RFC 1/1] tcg: Always pass the full write size to notdirty_write()

[Ilya Leoshkevich <iii@linux.ibm.com>]

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 <iii@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.