On Fri, 26 May 2023 at 01:24, Richard Henderson <richard.hender...@linaro.org> wrote: > > PAGE_WRITE is current writability, as modified by TB protection; > PAGE_WRITE_ORG is the original page writability. > > Fixes: cdfac37be0d ("accel/tcg: Honor atomicity of loads") > Signed-off-by: Richard Henderson <richard.hender...@linaro.org> > --- > accel/tcg/ldst_atomicity.c.inc | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/accel/tcg/ldst_atomicity.c.inc b/accel/tcg/ldst_atomicity.c.inc > index 0f6b3f8ab6..57163f5ca2 100644 > --- a/accel/tcg/ldst_atomicity.c.inc > +++ b/accel/tcg/ldst_atomicity.c.inc > @@ -191,7 +191,7 @@ static Int128 load_atomic16_or_exit(CPUArchState *env, > uintptr_t ra, void *pv) > * another process, because the fallback start_exclusive solution > * provides no protection across processes. > */ > - if (!page_check_range(h2g(p), 16, PAGE_WRITE)) { > + if (!page_check_range(h2g(p), 16, PAGE_WRITE_ORG)) { > return *p; > } > #endif > -- > 2.34.1
load_atomic8_or_exit() has a similar condition, so we should change either both or neither. So, if I understand this correctly, !PAGE_WRITE_ORG is a stricter test than !PAGE_WRITE, so we're saying "don't do a simple non-atomic load if the page was only read-only because we've translated code out of it". Why is it not OK to do the non-atomic load in that case? I guess because we don't have the mmap lock, so some other thread might nip in and do an access that causes us to invalidate the TBs and move the page back to writeable? thanks -- PMM