On 13/04/21 23:36, Peter Xu wrote:
This patch closes this race by allowing the main thread to give the vcpu thread chance to do a VMENTER to complete that write operation. It's done by adding a vcpu loop counter (must be defined as volatile as main thread will do read loop), then the main thread can guarantee the vcpu got at least another VMENTER by making sure the guest_vcpu_loops increases by 2. Dirty ring does not need this since dirty_ring_last_page would already help avoid this specific race condition.
Just a nit, the comment and commit message should mention KVM_RUN rather than vmentry; it's possible to be preempted many times in vcpu_enter_guest without making progress, but those wouldn't return to userspace and thus would not update guest_vcpu_loops.
Also, volatile is considered harmful even in userspace/test code[1]. Technically rather than volatile one should use an atomic load (even a relaxed one), but in practice it's okay to use volatile too *for this specific use* (READ_ONCE/WRITE_ONCE are volatile reads and writes as well). If the selftests gained 32-bit support, one should not use volatile because neither reads or writes to uint64_t variables would be guaranteed to be atomic.
Queued, thanks. Paolo [1] Documentation/process/volatile-considered-harmful.rst
