On 20/11/2017 19:08, Peter Maydell wrote:
> To do a write to memory that is marked as notdirty, we need
> to invalidate any TBs we have cached for that memory, and
> update the cpu physical memory dirty flags for VGA and migration.
> The slowpath code in notdirty_mem_write() does all this correctly,
> but the new atomic handling code in atomic_mmu_lookup() doesn't
> do anything at all, it just clears the dirty bit in the TLB.
>
> The effect of this bug is that if the first write to a notdirty
> page for which we have cached TBs is by a guest atomic access,
> we fail to invalidate the TBs and subsequently will execute
> incorrect code. This can be seen by trying to run 'javac' on AArch64.
>
> The first patch here refactors notdirty_mem_write() to pull out
> the "correctly handle dirty bit updates" parts of the code into
> two new functions memory_notdirty_write_prepare() and
> memory_notdirty_write_complete(). The second patch then uses
> those functions to fix the atomic helpers.
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Thanks!
Paolo
> Changes v1->v2:
> * add the 'bool active;' flag to NotDirtyInfo in patch 1
> * change the comment on NotDirtyInfo in patch 1 to document
> the active flag (and to fix incorrect references to function
> names that I forgot to update when I decided on the names for
> the prepare/complete functions)
> * in patch 2, don't call prepare unless the TLB was notdirty
> * in patch 2, use the active flag to track whether we need to
> call complete or not
>
> thanks
> -- PMM
>
> Peter Maydell (2):
> exec.c: Factor out before/after actions for notdirty memory writes
> accel/tcg: Handle atomic accesses to notdirty memory correctly
>
> accel/tcg/atomic_template.h | 12 ++++++++
> include/exec/memory-internal.h | 62 ++++++++++++++++++++++++++++++++++++++++
> accel/tcg/cputlb.c | 38 +++++++++++++++---------
> accel/tcg/user-exec.c | 1 +
> exec.c | 65 ++++++++++++++++++++++++++++--------------
> 5 files changed, 144 insertions(+), 34 deletions(-)
>