Re: [PATCH v4 02/16] accel/tcg: Fix check for page writeability in load_atomic16_or_exit

[Peter Maydell]

On Tue, 30 May 2023 at 16:09, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 5/30/23 07:48, Peter Maydell wrote:
> > On Tue, 30 May 2023 at 15:29, Richard Henderson
> > <richard.henderson@linaro.org> wrote:
> >>
> >> On 5/30/23 07:06, Peter Maydell wrote:
> >>>> This is about falling through to the cmpxchg below: if !PAGE_WRITE_ORG, 
> >>>> then the page is
> >>>> really not writable, we will SIGSEGV, and handle_sigsegv_accerr_write 
> >>>> will kill the process.
> >>>
> >>> Right, but if !PAGE_WRITE_ORG then that implies also !PAGE_WRITE,
> >>> so we do that even without this change ?
> >>
> >> But !PAGE_WRITE does not imply !PAGE_WRITE_ORG.
> >
> > Hmm. In what situation do we mark a page writeable when the
> > guest didn't ask for it to be writeable ?
>
> I don't know -- it seems backward, I know.
>
> I *think* it's a race condition, where PAGE_WRITE changes.
> That's what the test case is trying to provoke, anyway.

That sounds like the theory I had earlier, that we
don't have the mmap lock, so the other thread can
get in and turn the RO-only-because-of-the-JIT page
back to RW, so we don't want to do the non-atomic
access for the "RO-only-because-of-JIT" cases.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM