qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Gavin Shan" <gshan@redhat.com>, "Peter Xu" <peterx@redhat.com>,
	"Philippe Mathieu-Daudé" <f4bug@amsat.org>
Subject: [PULL 01/21] KVM: use store-release to mark dirty pages as harvested
Date: Mon, 19 Sep 2022 19:34:29 +0200	[thread overview]
Message-ID: <20220919173449.5864-2-pbonzini@redhat.com> (raw)
In-Reply-To: <20220919173449.5864-1-pbonzini@redhat.com>

The following scenario can happen if QEMU sets more RESET flags while
the KVM_RESET_DIRTY_RINGS ioctl is ongoing on another host CPU:

    CPU0                     CPU1               CPU2
    ------------------------ ------------------ ------------------------
                                                fill gfn0
                                                store-rel flags for gfn0
                                                fill gfn1
                                                store-rel flags for gfn1
    load-acq flags for gfn0
    set RESET for gfn0
    load-acq flags for gfn1
    set RESET for gfn1
    do ioctl! ----------->
                             ioctl(RESET_RINGS)
                                                fill gfn2
                                                store-rel flags for gfn2
    load-acq flags for gfn2
    set RESET for gfn2
                             process gfn0
                             process gfn1
                             process gfn2
    do ioctl!
    etc.

The three load-acquire in CPU0 synchronize with the three store-release
in CPU2, but CPU0 and CPU1 are only synchronized up to gfn1 and CPU1
may miss gfn2's fields other than flags.

The kernel must be able to cope with invalid values of the fields, and
userspace *will* invoke the ioctl once more.  However, once the RESET flag
is cleared on gfn2, it is lost forever, therefore in the above scenario
CPU1 must read the correct value of gfn2's fields.

Therefore RESET must be set with a store-release, that will synchronize
with KVM's load-acquire in CPU1.

Cc: Gavin Shan <gshan@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 accel/kvm/kvm-all.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 136c8eaed3..7c8ce18bdd 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -728,7 +728,23 @@ static bool dirty_gfn_is_dirtied(struct kvm_dirty_gfn *gfn)
 
 static void dirty_gfn_set_collected(struct kvm_dirty_gfn *gfn)
 {
-    gfn->flags = KVM_DIRTY_GFN_F_RESET;
+    /*
+     * Use a store-release so that the CPU that executes KVM_RESET_DIRTY_RINGS
+     * sees the full content of the ring:
+     *
+     * CPU0                     CPU1                         CPU2
+     * ------------------------------------------------------------------------------
+     *                                                       fill gfn0
+     *                                                       store-rel flags for gfn0
+     * load-acq flags for gfn0
+     * store-rel RESET for gfn0
+     *                          ioctl(RESET_RINGS)
+     *                            load-acq flags for gfn0
+     *                            check if flags have RESET
+     *
+     * The synchronization goes from CPU2 to CPU0 to CPU1.
+     */
+    qatomic_store_release(&gfn->flags, KVM_DIRTY_GFN_F_RESET);
 }
 
 /*
-- 
2.37.2



  reply	other threads:[~2022-09-19 17:54 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-19 17:34 [PULL 00/21] Misc patches for 2022-09-19 Paolo Bonzini
2022-09-19 17:34 ` Paolo Bonzini [this message]
2022-09-19 17:34 ` [PULL 02/21] target/i386: Raise #GP on unaligned m128 accesses when required Paolo Bonzini
2022-09-19 17:34 ` [PULL 03/21] kvm: fix memory leak on failure to read stats descriptors Paolo Bonzini
2022-09-19 17:34 ` [PULL 04/21] spapr_pci: fix leak in spapr_phb_vfio_get_loc_code Paolo Bonzini
2022-09-19 17:34 ` [PULL 05/21] coverity: add new RISC-V component Paolo Bonzini
2022-09-19 17:34 ` [PULL 06/21] coverity: put NUBus under m68k component Paolo Bonzini
2022-09-19 17:34 ` [PULL 07/21] smbios: sanitize type from external type before checking have_fields_bitmap Paolo Bonzini
2022-09-19 17:34 ` [PULL 08/21] tests: unit: simplify test-visitor-serialization list tests Paolo Bonzini
2022-09-19 17:34 ` [PULL 09/21] tests: test-qga: close socket on failure to connect Paolo Bonzini
2022-09-19 17:34 ` [PULL 10/21] tests: unit: add NULL-pointer check Paolo Bonzini
2022-09-19 17:34 ` [PULL 11/21] tests/tcg: i386: fix typos in 3DNow! instructions Paolo Bonzini
2022-09-19 17:34 ` [PULL 12/21] tests/tcg: i386: add MMX and 3DNow! tests Paolo Bonzini
2022-09-19 17:34 ` [PULL 13/21] tests/tcg: refine MMX support in SSE tests Paolo Bonzini
2022-09-19 17:34 ` [PULL 14/21] tests/tcg: remove old " Paolo Bonzini
2022-09-19 17:34 ` [PULL 15/21] audio: add help option for -audio and -audiodev Paolo Bonzini
2022-09-19 17:34 ` [PULL 16/21] target/i386: correctly mask SSE4a bit indices in register operands Paolo Bonzini
2022-09-19 17:34 ` [PULL 17/21] target/i386: fix INSERTQ implementation Paolo Bonzini
2022-09-19 17:34 ` [PULL 18/21] target/i386: REPZ and REPNZ are mutually exclusive Paolo Bonzini
2022-09-19 17:34 ` [PULL 19/21] target/i386: introduce insn_get_addr Paolo Bonzini
2022-09-19 17:34 ` [PULL 20/21] build: remove extra parentheses causing missing rebuilds Paolo Bonzini
2022-09-19 17:34 ` [PULL 21/21] qboot: update to latest submodule Paolo Bonzini
2022-09-21 17:33 ` [PULL 00/21] Misc patches for 2022-09-19 Stefan Hajnoczi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220919173449.5864-2-pbonzini@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=f4bug@amsat.org \
    --cc=gshan@redhat.com \
    --cc=peterx@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).