| Message ID | 0fdddb94bb0e31b7da129a809a308d91c10c0b5e.1606941224.git.maciej.szmigiero@oracle.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 0c55f867f0c96dff93d4e0b5973975d65afb26d8 |
| Headers | show |
| Series | selftests: kvm/set_memory_region_test: Fix race in move region test | expand |
On 02/12/20 21:35, Maciej S. Szmigiero wrote: > From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com> > > The current memory region move test correctly handles the situation that > the second (realigning) memslot move operation would temporarily trigger > MMIO until it completes, however it does not handle the case in which the > first (misaligning) move operation does this, too. > This results in false test assertions in case it does so. > > Fix this by handling temporary MMIO from the first memslot move operation > in the test guest code, too. > > Fixes: 8a0639fe9201 ("KVM: sefltests: Add explicit synchronization to move mem region test") > Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com> > --- > The race is pretty hard to trigger on the current KVM memslot code, > to trigger it reliably an extra delay in memslot move op is needed: > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -1173,7 +1173,7 @@ static struct kvm_memslots *kvm_dup_memslots(struct kvm_memslots *old, > > return slots; > } > - > +#include <linux/delay.h> > static int kvm_set_memslot(struct kvm *kvm, > const struct kvm_userspace_memory_region *mem, > struct kvm_memory_slot *old, > @@ -1212,6 +1212,8 @@ static int kvm_set_memslot(struct kvm *kvm, > * - kvm_is_visible_gfn (mmu_check_root) > */ > kvm_arch_flush_shadow_memslot(kvm, slot); > + > + if (change == KVM_MR_MOVE) mdelay(100); > } > > r = kvm_arch_prepare_memory_region(kvm, new, mem, change); > > .../selftests/kvm/set_memory_region_test.c | 17 +++++++++++++---- > 1 file changed, 13 insertions(+), 4 deletions(-) > > diff --git a/tools/testing/selftests/kvm/set_memory_region_test.c b/tools/testing/selftests/kvm/set_memory_region_test.c > index b3ece55a2da6..6f441dd9f33c 100644 > --- a/tools/testing/selftests/kvm/set_memory_region_test.c > +++ b/tools/testing/selftests/kvm/set_memory_region_test.c > @@ -156,14 +156,23 @@ static void guest_code_move_memory_region(void) > GUEST_SYNC(0); > > /* > - * Spin until the memory region is moved to a misaligned address. This > - * may or may not trigger MMIO, as the window where the memslot is > - * invalid is quite small. > + * Spin until the memory region starts getting moved to a > + * misaligned address. > + * Every region move may or may not trigger MMIO, as the > + * window where the memslot is invalid is usually quite small. > */ > val = guest_spin_on_val(0); > GUEST_ASSERT_1(val == 1 || val == MMIO_VAL, val); > > - /* Spin until the memory region is realigned. */ > + /* Spin until the misaligning memory region move completes. */ > + val = guest_spin_on_val(MMIO_VAL); > + GUEST_ASSERT_1(val == 1 || val == 0, val); > + > + /* Spin until the memory region starts to get re-aligned. */ > + val = guest_spin_on_val(0); > + GUEST_ASSERT_1(val == 1 || val == MMIO_VAL, val); > + > + /* Spin until the re-aligning memory region move completes. */ > val = guest_spin_on_val(MMIO_VAL); > GUEST_ASSERT_1(val == 1, val); > > Queued, thanks. paolo
diff --git a/tools/testing/selftests/kvm/set_memory_region_test.c b/tools/testing/selftests/kvm/set_memory_region_test.c index b3ece55a2da6..6f441dd9f33c 100644 --- a/tools/testing/selftests/kvm/set_memory_region_test.c +++ b/tools/testing/selftests/kvm/set_memory_region_test.c @@ -156,14 +156,23 @@ static void guest_code_move_memory_region(void) GUEST_SYNC(0); /* - * Spin until the memory region is moved to a misaligned address. This - * may or may not trigger MMIO, as the window where the memslot is - * invalid is quite small. + * Spin until the memory region starts getting moved to a + * misaligned address. + * Every region move may or may not trigger MMIO, as the + * window where the memslot is invalid is usually quite small. */ val = guest_spin_on_val(0); GUEST_ASSERT_1(val == 1 || val == MMIO_VAL, val); - /* Spin until the memory region is realigned. */ + /* Spin until the misaligning memory region move completes. */ + val = guest_spin_on_val(MMIO_VAL); + GUEST_ASSERT_1(val == 1 || val == 0, val); + + /* Spin until the memory region starts to get re-aligned. */ + val = guest_spin_on_val(0); + GUEST_ASSERT_1(val == 1 || val == MMIO_VAL, val); + + /* Spin until the re-aligning memory region move completes. */ val = guest_spin_on_val(MMIO_VAL); GUEST_ASSERT_1(val == 1, val);