[kvm-unit-tests PATCH 0/7] more migration enhancements and tests

[kvm-unit-tests PATCH 0/7] more migration enhancements and tests

[Nicholas Piggin]

This series applies on top of the multi-migration patches and just
has assorted things I've been collecting.

- New migrate_skip() command that tidies up the wart of using
  migrate_once() to skip migration.
- New "continuous migration" mode where the harness just
  migrates the test contually while it is running.
- Put some migration tests in gitlab CI for powerpc and s390.
- Add a test case that can reproduce the QEMU TCG dirty bitmap
  migration bug.

Nicholas Piggin (7):
  arch-run: Keep infifo open
  migration: Add a migrate_skip command
  (arm|s390): Use migrate_skip in test cases
  powerpc: add asm/time.h header with delay and get_clock_us/ms
  arch-run: Add a "continuous" migration option for tests
  gitlab-ci: Run migration selftest on s390x and powerpc
  common: add memory dirtying vs migration test

 .gitlab-ci.yml              |  18 ++++---
 arm/gic.c                   |  21 ++++----
 common/memory-verify.c      |  48 +++++++++++++++++
 common/selftest-migration.c |  26 ++++++---
 lib/migrate.c               |  37 ++++++++++++-
 lib/migrate.h               |   5 ++
 lib/powerpc/asm/processor.h |  21 --------
 lib/powerpc/asm/time.h      |  30 +++++++++++
 lib/powerpc/processor.c     |  11 ++++
 lib/powerpc/smp.c           |   1 +
 lib/ppc64/asm/time.h        |   1 +
 powerpc/Makefile.common     |   1 +
 powerpc/memory-verify.c     |   1 +
 powerpc/spapr_vpa.c         |   1 +
 powerpc/sprs.c              |   1 +
 powerpc/tm.c                |   1 +
 powerpc/unittests.cfg       |  13 +++++
 s390x/Makefile              |   1 +
 s390x/memory-verify.c       |   1 +
 s390x/migration-cmm.c       |   8 +--
 s390x/migration-skey.c      |   4 +-
 s390x/migration.c           |   1 +
 s390x/unittests.cfg         |  11 ++++
 scripts/arch-run.bash       | 104 +++++++++++++++++++++++++++++-------
 24 files changed, 299 insertions(+), 68 deletions(-)
 create mode 100644 common/memory-verify.c
 create mode 100644 lib/powerpc/asm/time.h
 create mode 100644 lib/ppc64/asm/time.h
 create mode 120000 powerpc/memory-verify.c
 create mode 120000 s390x/memory-verify.c

-- 
2.42.0

[kvm-unit-tests PATCH 1/7] arch-run: Keep infifo open

[Nicholas Piggin]

The infifo fifo that is used to send characters to QEMU console is
only able to receive one character before the cat process exits.
Supporting interactions between test and harness involving multiple
characters requires the fifo to remain open.

This also allows us to let the cat out of the bag, simplifying the
input pipeline.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 scripts/arch-run.bash | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index 6daef3218..e5b36a07b 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -158,6 +158,11 @@ run_migration ()
 	mkfifo ${src_outfifo}
 	mkfifo ${dst_outfifo}
 
+	# Holding both ends of the input fifo open prevents opens from
+	# blocking and readers getting EOF when a writer closes it.
+	mkfifo ${dst_infifo}
+	exec {dst_infifo_fd}<>${dst_infifo}
+
 	eval "$migcmdline" \
 		-chardev socket,id=mon,path=${src_qmp},server=on,wait=off \
 		-mon chardev=mon,mode=control > ${src_outfifo} &
@@ -191,14 +196,10 @@ run_migration ()
 
 do_migration ()
 {
-	# We have to use cat to open the named FIFO, because named FIFO's,
-	# unlike pipes, will block on open() until the other end is also
-	# opened, and that totally breaks QEMU...
-	mkfifo ${dst_infifo}
 	eval "$migcmdline" \
 		-chardev socket,id=mon,path=${dst_qmp},server=on,wait=off \
 		-mon chardev=mon,mode=control -incoming unix:${dst_incoming} \
-		< <(cat ${dst_infifo}) > ${dst_outfifo} &
+		< ${dst_infifo} > ${dst_outfifo} &
 	incoming_pid=$!
 	cat ${dst_outfifo} | tee ${dst_out} | filter_quiet_msgs &
 
@@ -245,7 +246,6 @@ do_migration ()
 
 	# keypress to dst so getchar completes and test continues
 	echo > ${dst_infifo}
-	rm ${dst_infifo}
 
 	# Ensure the incoming socket is removed, ready for next destination
 	if [ -S ${dst_incoming} ] ; then
-- 
2.42.0

Re: [kvm-unit-tests PATCH 1/7] arch-run: Keep infifo open

[Thomas Huth]

On 26/02/2024 10.38, Nicholas Piggin wrote:
> The infifo fifo that is used to send characters to QEMU console is
> only able to receive one character before the cat process exits.
> Supporting interactions between test and harness involving multiple
> characters requires the fifo to remain open.
> 
> This also allows us to let the cat out of the bag, simplifying the
> input pipeline.

LOL, we rather let the cat out of the subshell now, but I like the play on 
words :-)

> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   scripts/arch-run.bash | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> index 6daef3218..e5b36a07b 100644
> --- a/scripts/arch-run.bash
> +++ b/scripts/arch-run.bash
> @@ -158,6 +158,11 @@ run_migration ()
>   	mkfifo ${src_outfifo}
>   	mkfifo ${dst_outfifo}
>   
> +	# Holding both ends of the input fifo open prevents opens from
> +	# blocking and readers getting EOF when a writer closes it.
> +	mkfifo ${dst_infifo}
> +	exec {dst_infifo_fd}<>${dst_infifo}
> +
>   	eval "$migcmdline" \
>   		-chardev socket,id=mon,path=${src_qmp},server=on,wait=off \
>   		-mon chardev=mon,mode=control > ${src_outfifo} &
> @@ -191,14 +196,10 @@ run_migration ()
>   
>   do_migration ()
>   {
> -	# We have to use cat to open the named FIFO, because named FIFO's,
> -	# unlike pipes, will block on open() until the other end is also
> -	# opened, and that totally breaks QEMU...
> -	mkfifo ${dst_infifo}
>   	eval "$migcmdline" \
>   		-chardev socket,id=mon,path=${dst_qmp},server=on,wait=off \
>   		-mon chardev=mon,mode=control -incoming unix:${dst_incoming} \
> -		< <(cat ${dst_infifo}) > ${dst_outfifo} &
> +		< ${dst_infifo} > ${dst_outfifo} &
>   	incoming_pid=$!
>   	cat ${dst_outfifo} | tee ${dst_out} | filter_quiet_msgs &
>   
> @@ -245,7 +246,6 @@ do_migration ()
>   
>   	# keypress to dst so getchar completes and test continues
>   	echo > ${dst_infifo}
> -	rm ${dst_infifo}

I assume it will not get deleted by the trap handler? ... sounds fine to me, 
so I dare to say:

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [kvm-unit-tests PATCH 1/7] arch-run: Keep infifo open

[Nicholas Piggin]

On Fri Mar 1, 2024 at 11:32 PM AEST, Thomas Huth wrote:
> On 26/02/2024 10.38, Nicholas Piggin wrote:
> > The infifo fifo that is used to send characters to QEMU console is
> > only able to receive one character before the cat process exits.
> > Supporting interactions between test and harness involving multiple
> > characters requires the fifo to remain open.
> > 
> > This also allows us to let the cat out of the bag, simplifying the
> > input pipeline.
>
> LOL, we rather let the cat out of the subshell now, but I like the play on 
> words :-)

It was a bit of a stretch, but I'm glad you liked it :) I may
incorporate your suggestion to improve it.

>
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >   scripts/arch-run.bash | 12 ++++++------
> >   1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> > index 6daef3218..e5b36a07b 100644
> > --- a/scripts/arch-run.bash
> > +++ b/scripts/arch-run.bash
> > @@ -158,6 +158,11 @@ run_migration ()
> >   	mkfifo ${src_outfifo}
> >   	mkfifo ${dst_outfifo}
> >   
> > +	# Holding both ends of the input fifo open prevents opens from
> > +	# blocking and readers getting EOF when a writer closes it.
> > +	mkfifo ${dst_infifo}
> > +	exec {dst_infifo_fd}<>${dst_infifo}
> > +
> >   	eval "$migcmdline" \
> >   		-chardev socket,id=mon,path=${src_qmp},server=on,wait=off \
> >   		-mon chardev=mon,mode=control > ${src_outfifo} &
> > @@ -191,14 +196,10 @@ run_migration ()
> >   
> >   do_migration ()
> >   {
> > -	# We have to use cat to open the named FIFO, because named FIFO's,
> > -	# unlike pipes, will block on open() until the other end is also
> > -	# opened, and that totally breaks QEMU...
> > -	mkfifo ${dst_infifo}
> >   	eval "$migcmdline" \
> >   		-chardev socket,id=mon,path=${dst_qmp},server=on,wait=off \
> >   		-mon chardev=mon,mode=control -incoming unix:${dst_incoming} \
> > -		< <(cat ${dst_infifo}) > ${dst_outfifo} &
> > +		< ${dst_infifo} > ${dst_outfifo} &
> >   	incoming_pid=$!
> >   	cat ${dst_outfifo} | tee ${dst_out} | filter_quiet_msgs &
> >   
> > @@ -245,7 +246,6 @@ do_migration ()
> >   
> >   	# keypress to dst so getchar completes and test continues
> >   	echo > ${dst_infifo}
> > -	rm ${dst_infifo}
>
> I assume it will not get deleted by the trap handler? ... sounds fine to me, 
> so I dare to say:

Yep, deleted by trap handler.

>
> Reviewed-by: Thomas Huth <thuth@redhat.com>

Thanks,
Nick

[kvm-unit-tests PATCH 2/7] migration: Add a migrate_skip command

[Nicholas Piggin]

Tests that are run with MIGRATION=yes but skip due to some requirement
not being met will show as a failure due to the harness requirement to
see one successful migration. The workaround for this is to migrate in
test's skip path. Add a new command that just tells the harness to not
expect a migration.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 common/selftest-migration.c | 14 ++++++++-----
 lib/migrate.c               | 19 ++++++++++++++++-
 lib/migrate.h               |  2 ++
 powerpc/unittests.cfg       |  6 ++++++
 s390x/unittests.cfg         |  5 +++++
 scripts/arch-run.bash       | 41 +++++++++++++++++++++++++++++--------
 6 files changed, 73 insertions(+), 14 deletions(-)

diff --git a/common/selftest-migration.c b/common/selftest-migration.c
index 54b5d6b2d..0afd8581c 100644
--- a/common/selftest-migration.c
+++ b/common/selftest-migration.c
@@ -14,14 +14,18 @@
 
 int main(int argc, char **argv)
 {
-	int i = 0;
-
 	report_prefix_push("migration");
 
-	for (i = 0; i < NR_MIGRATIONS; i++)
-		migrate_quiet();
+	if (argc > 1 && !strcmp(argv[1], "skip")) {
+		migrate_skip();
+		report(true, "migration skipping");
+	} else {
+		int i;
 
-	report(true, "simple harness stress test");
+		for (i = 0; i < NR_MIGRATIONS; i++)
+			migrate_quiet();
+		report(true, "simple harness stress");
+	}
 
 	report_prefix_pop();
 
diff --git a/lib/migrate.c b/lib/migrate.c
index 92d1d957d..1d22196b7 100644
--- a/lib/migrate.c
+++ b/lib/migrate.c
@@ -39,7 +39,24 @@ void migrate_once(void)
 
 	if (migrated)
 		return;
-
 	migrated = true;
+
 	migrate();
 }
+
+/*
+ * When the test has been started in migration mode, but the test case is
+ * skipped and no migration point is reached, this can be used to tell the
+ * harness not to mark it as a failure to migrate.
+ */
+void migrate_skip(void)
+{
+	static bool did_migrate_skip;
+
+	if (did_migrate_skip)
+		return;
+	did_migrate_skip = true;
+
+	puts("Skipped VM migration (quiet)\n");
+	(void)getchar();
+}
diff --git a/lib/migrate.h b/lib/migrate.h
index 95b9102b0..db6e0c501 100644
--- a/lib/migrate.h
+++ b/lib/migrate.h
@@ -9,3 +9,5 @@
 void migrate(void);
 void migrate_quiet(void);
 void migrate_once(void);
+
+void migrate_skip(void);
diff --git a/powerpc/unittests.cfg b/powerpc/unittests.cfg
index 7ce57de02..89abf2095 100644
--- a/powerpc/unittests.cfg
+++ b/powerpc/unittests.cfg
@@ -40,6 +40,12 @@ groups = selftest
 file = selftest-migration.elf
 groups = selftest migration
 
+[selftest-migration-skip]
+file = selftest-migration.elf
+machine = pseries
+groups = selftest migration
+extra_params = -append "skip"
+
 [spapr_hcall]
 file = spapr_hcall.elf
 
diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
index a7ad522ca..f613602d3 100644
--- a/s390x/unittests.cfg
+++ b/s390x/unittests.cfg
@@ -28,6 +28,11 @@ extra_params = -append 'test 123'
 file = selftest-migration.elf
 groups = selftest migration
 
+[selftest-migration-skip]
+file = selftest-migration.elf
+groups = selftest migration
+extra_params = -append "skip"
+
 [intercept]
 file = intercept.elf
 
diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index e5b36a07b..d0f6f098f 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -124,12 +124,17 @@ qmp_events ()
 
 filter_quiet_msgs ()
 {
-	grep -v "Now migrate the VM (quiet)"
+	grep -v "Now migrate the VM (quiet)" |
+	grep -v "Skipped VM migration (quiet)"
 }
 
 seen_migrate_msg ()
 {
-	grep -q -e "Now migrate the VM" < $1
+	if [ $skip_migration -eq 1 ]; then
+		grep -q -e "Now migrate the VM" < $1
+	else
+		grep -q -e "Now migrate the VM" -e "Skipped VM migration" < $1
+	fi
 }
 
 run_migration ()
@@ -142,7 +147,7 @@ run_migration ()
 	migcmdline=$@
 
 	trap 'trap - TERM ; kill 0 ; exit 2' INT TERM
-	trap 'rm -f ${src_out} ${dst_out} ${src_outfifo} ${dst_outfifo} ${dst_incoming} ${src_qmp} ${dst_qmp} ${dst_infifo}' RETURN EXIT
+	trap 'rm -f ${src_out} ${dst_out} ${src_outfifo} ${dst_outfifo} ${dst_incoming} ${src_qmp} ${dst_qmp} ${src_infifo} ${dst_infifo}' RETURN EXIT
 
 	dst_incoming=$(mktemp -u -t mig-helper-socket-incoming.XXXXXXXXXX)
 	src_out=$(mktemp -t mig-helper-stdout1.XXXXXXXXXX)
@@ -151,21 +156,26 @@ run_migration ()
 	dst_outfifo=$(mktemp -u -t mig-helper-fifo-stdout2.XXXXXXXXXX)
 	src_qmp=$(mktemp -u -t mig-helper-qmp1.XXXXXXXXXX)
 	dst_qmp=$(mktemp -u -t mig-helper-qmp2.XXXXXXXXXX)
-	dst_infifo=$(mktemp -u -t mig-helper-fifo-stdin.XXXXXXXXXX)
+	src_infifo=$(mktemp -u -t mig-helper-fifo-stdin1.XXXXXXXXXX)
+	dst_infifo=$(mktemp -u -t mig-helper-fifo-stdin2.XXXXXXXXXX)
 	src_qmpout=/dev/null
 	dst_qmpout=/dev/null
+	skip_migration=0
 
 	mkfifo ${src_outfifo}
 	mkfifo ${dst_outfifo}
 
 	# Holding both ends of the input fifo open prevents opens from
 	# blocking and readers getting EOF when a writer closes it.
+	mkfifo ${src_infifo}
 	mkfifo ${dst_infifo}
+	exec {src_infifo_fd}<>${src_infifo}
 	exec {dst_infifo_fd}<>${dst_infifo}
 
 	eval "$migcmdline" \
 		-chardev socket,id=mon,path=${src_qmp},server=on,wait=off \
-		-mon chardev=mon,mode=control > ${src_outfifo} &
+		-mon chardev=mon,mode=control \
+		< ${src_infifo} > ${src_outfifo} &
 	live_pid=$!
 	cat ${src_outfifo} | tee ${src_out} | filter_quiet_msgs &
 
@@ -179,8 +189,11 @@ run_migration ()
 		# Wait for test exit or further migration messages.
 		if ! seen_migrate_msg ${src_out} ;  then
 			sleep 0.1
-		else
+		elif grep -q "Now migrate the VM" < ${src_out} ; then
 			do_migration || return $?
+		elif [ $skip_migration -eq 0 ] && grep -q "Skipped VM migration" < ${src_out} ; then
+			echo > ${src_infifo} # Resume src and carry on.
+			break;
 		fi
 	done
 
@@ -207,10 +220,10 @@ do_migration ()
 	# "Now migrate VM" or similar console message.
 	while ! seen_migrate_msg ${src_out} ; do
 		if ! ps -p ${live_pid} > /dev/null ; then
-			echo "ERROR: Test exit before migration point." >&2
 			echo > ${dst_infifo}
-			qmp ${src_qmp} '"quit"'> ${src_qmpout} 2>/dev/null
 			qmp ${dst_qmp} '"quit"'> ${dst_qmpout} 2>/dev/null
+			echo "ERROR: Test exit before migration point." >&2
+			qmp ${src_qmp} '"quit"'> ${src_qmpout} 2>/dev/null
 			return 3
 		fi
 		sleep 0.1
@@ -220,6 +233,15 @@ do_migration ()
 	while ! [ -S ${dst_incoming} ] ; do sleep 0.1 ; done
 	while ! [ -S ${dst_qmp} ] ; do sleep 0.1 ; done
 
+	if [ $skip_migration -eq 0 ] && grep -q "Skipped VM migration" < ${src_out} ; then
+		# May not get any migrations, exit to main loop for now...
+		echo > ${dst_infifo}
+		qmp ${dst_qmp} '"quit"'> ${dst_qmpout} 2>/dev/null
+		echo > ${src_infifo} # Resume src and carry on.
+		skip_migration=1
+		return 0
+	fi
+
 	qmp ${src_qmp} '"migrate", "arguments": { "uri": "unix:'${dst_incoming}'" }' > ${src_qmpout}
 
 	# Wait for the migration to complete
@@ -263,6 +285,9 @@ do_migration ()
 	tmp=${src_out}
 	src_out=${dst_out}
 	dst_out=${tmp}
+	tmp=${src_infifo}
+	src_infifo=${dst_infifo}
+	dst_infifo=${tmp}
 	tmp=${src_outfifo}
 	src_outfifo=${dst_outfifo}
 	dst_outfifo=${tmp}
-- 
2.42.0

Re: [kvm-unit-tests PATCH 2/7] migration: Add a migrate_skip command

[Thomas Huth]

On 26/02/2024 10.38, Nicholas Piggin wrote:
> Tests that are run with MIGRATION=yes but skip due to some requirement
> not being met will show as a failure due to the harness requirement to
> see one successful migration. The workaround for this is to migrate in
> test's skip path. Add a new command that just tells the harness to not
> expect a migration.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   common/selftest-migration.c | 14 ++++++++-----
>   lib/migrate.c               | 19 ++++++++++++++++-
>   lib/migrate.h               |  2 ++
>   powerpc/unittests.cfg       |  6 ++++++
>   s390x/unittests.cfg         |  5 +++++
>   scripts/arch-run.bash       | 41 +++++++++++++++++++++++++++++--------
>   6 files changed, 73 insertions(+), 14 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>

[kvm-unit-tests PATCH 3/7] (arm|s390): Use migrate_skip in test cases

[Nicholas Piggin]

Have tests use the new migrate_skip command in skip paths, rather than
calling migrate_once to prevent harness reporting an error.

s390x/migration.c adds a new command that looks like it was missing
previously.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arm/gic.c              | 21 ++++++++++++---------
 s390x/migration-cmm.c  |  8 ++++----
 s390x/migration-skey.c |  4 +++-
 s390x/migration.c      |  1 +
 4 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/arm/gic.c b/arm/gic.c
index c950b0d15..bbf828f17 100644
--- a/arm/gic.c
+++ b/arm/gic.c
@@ -782,13 +782,15 @@ static void test_its_migration(void)
 	struct its_device *dev2, *dev7;
 	cpumask_t mask;
 
-	if (its_setup1())
+	if (its_setup1()) {
+		migrate_skip();
 		return;
+	}
 
 	dev2 = its_get_device(2);
 	dev7 = its_get_device(7);
 
-	migrate_once();
+	migrate();
 
 	stats_reset();
 	cpumask_clear(&mask);
@@ -819,8 +821,10 @@ static void test_migrate_unmapped_collection(void)
 	int pe0 = 0;
 	u8 config;
 
-	if (its_setup1())
+	if (its_setup1()) {
+		migrate_skip();
 		return;
+	}
 
 	if (!errata(ERRATA_UNMAPPED_COLLECTIONS)) {
 		report_skip("Skipping test, as this test hangs without the fix. "
@@ -836,7 +840,7 @@ static void test_migrate_unmapped_collection(void)
 	its_send_mapti(dev2, 8192, 0, col);
 	gicv3_lpi_set_config(8192, LPI_PROP_DEFAULT);
 
-	migrate_once();
+	migrate();
 
 	/* on the destination, map the collection */
 	its_send_mapc(col, true);
@@ -875,8 +879,10 @@ static void test_its_pending_migration(void)
 	void *ptr;
 	int i;
 
-	if (its_prerequisites(4))
+	if (its_prerequisites(4)) {
+		migrate_skip();
 		return;
+	}
 
 	dev = its_create_device(2 /* dev id */, 8 /* nb_ites */);
 	its_send_mapd(dev, true);
@@ -923,7 +929,7 @@ static void test_its_pending_migration(void)
 	gicv3_lpi_rdist_enable(pe0);
 	gicv3_lpi_rdist_enable(pe1);
 
-	migrate_once();
+	migrate();
 
 	/* let's wait for the 256 LPIs to be handled */
 	mdelay(1000);
@@ -970,17 +976,14 @@ int main(int argc, char **argv)
 	} else if (!strcmp(argv[1], "its-migration")) {
 		report_prefix_push(argv[1]);
 		test_its_migration();
-		migrate_once();
 		report_prefix_pop();
 	} else if (!strcmp(argv[1], "its-pending-migration")) {
 		report_prefix_push(argv[1]);
 		test_its_pending_migration();
-		migrate_once();
 		report_prefix_pop();
 	} else if (!strcmp(argv[1], "its-migrate-unmapped-collection")) {
 		report_prefix_push(argv[1]);
 		test_migrate_unmapped_collection();
-		migrate_once();
 		report_prefix_pop();
 	} else if (strcmp(argv[1], "its-introspection") == 0) {
 		report_prefix_push(argv[1]);
diff --git a/s390x/migration-cmm.c b/s390x/migration-cmm.c
index 43673f18e..b4043a80e 100644
--- a/s390x/migration-cmm.c
+++ b/s390x/migration-cmm.c
@@ -55,12 +55,12 @@ int main(void)
 {
 	report_prefix_push("migration-cmm");
 
-	if (!check_essa_available())
+	if (!check_essa_available()) {
 		report_skip("ESSA is not available");
-	else
+		migrate_skip();
+	} else {
 		test_migration();
-
-	migrate_once();
+	}
 
 	report_prefix_pop();
 	return report_summary();
diff --git a/s390x/migration-skey.c b/s390x/migration-skey.c
index 8d6d8ecfe..1a196ae1e 100644
--- a/s390x/migration-skey.c
+++ b/s390x/migration-skey.c
@@ -169,6 +169,7 @@ static void test_skey_migration_parallel(void)
 
 	if (smp_query_num_cpus() == 1) {
 		report_skip("need at least 2 cpus for this test");
+		migrate_skip();
 		goto error;
 	}
 
@@ -233,6 +234,7 @@ int main(int argc, char **argv)
 
 	if (test_facility(169)) {
 		report_skip("storage key removal facility is active");
+		migrate_skip();
 		goto error;
 	}
 
@@ -247,11 +249,11 @@ int main(int argc, char **argv)
 		break;
 	default:
 		print_usage();
+		migrate_skip();
 		break;
 	}
 
 error:
-	migrate_once();
 	report_prefix_pop();
 	return report_summary();
 }
diff --git a/s390x/migration.c b/s390x/migration.c
index 269e272de..115afb731 100644
--- a/s390x/migration.c
+++ b/s390x/migration.c
@@ -164,6 +164,7 @@ int main(void)
 
 	if (smp_query_num_cpus() == 1) {
 		report_skip("need at least 2 cpus for this test");
+		migrate_skip();
 		goto done;
 	}
 
-- 
2.42.0

Re: [kvm-unit-tests PATCH 3/7] (arm|s390): Use migrate_skip in test cases

[Thomas Huth]

On 26/02/2024 10.38, Nicholas Piggin wrote:
> Have tests use the new migrate_skip command in skip paths, rather than
> calling migrate_once to prevent harness reporting an error.
> 
> s390x/migration.c adds a new command that looks like it was missing
> previously.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   arm/gic.c              | 21 ++++++++++++---------
>   s390x/migration-cmm.c  |  8 ++++----
>   s390x/migration-skey.c |  4 +++-
>   s390x/migration.c      |  1 +
>   4 files changed, 20 insertions(+), 14 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>

[kvm-unit-tests PATCH 4/7] powerpc: add asm/time.h header with delay and get_clock_us/ms

[Nicholas Piggin]

This matches s390x clock and delay APIs, so common test code can start
using time facilities.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 lib/powerpc/asm/processor.h | 21 ---------------------
 lib/powerpc/asm/time.h      | 30 ++++++++++++++++++++++++++++++
 lib/powerpc/processor.c     | 11 +++++++++++
 lib/powerpc/smp.c           |  1 +
 lib/ppc64/asm/time.h        |  1 +
 powerpc/spapr_vpa.c         |  1 +
 powerpc/sprs.c              |  1 +
 powerpc/tm.c                |  1 +
 8 files changed, 46 insertions(+), 21 deletions(-)
 create mode 100644 lib/powerpc/asm/time.h
 create mode 100644 lib/ppc64/asm/time.h

diff --git a/lib/powerpc/asm/processor.h b/lib/powerpc/asm/processor.h
index 4ad6612b3..fe1052939 100644
--- a/lib/powerpc/asm/processor.h
+++ b/lib/powerpc/asm/processor.h
@@ -43,25 +43,4 @@ static inline void mtmsr(uint64_t msr)
 	asm volatile ("mtmsrd %[msr]" :: [msr] "r" (msr) : "memory");
 }
 
-static inline uint64_t get_tb(void)
-{
-	return mfspr(SPR_TB);
-}
-
-extern void delay(uint64_t cycles);
-extern void udelay(uint64_t us);
-extern void sleep_tb(uint64_t cycles);
-extern void usleep(uint64_t us);
-
-static inline void mdelay(uint64_t ms)
-{
-	while (ms--)
-		udelay(1000);
-}
-
-static inline void msleep(uint64_t ms)
-{
-	usleep(ms * 1000);
-}
-
 #endif /* _ASMPOWERPC_PROCESSOR_H_ */
diff --git a/lib/powerpc/asm/time.h b/lib/powerpc/asm/time.h
new file mode 100644
index 000000000..72fcb1bd0
--- /dev/null
+++ b/lib/powerpc/asm/time.h
@@ -0,0 +1,30 @@
+#ifndef _ASMPOWERPC_TIME_H_
+#define _ASMPOWERPC_TIME_H_
+
+#include <libcflat.h>
+#include <asm/processor.h>
+
+static inline uint64_t get_tb(void)
+{
+	return mfspr(SPR_TB);
+}
+
+extern uint64_t get_clock_us(void);
+extern uint64_t get_clock_ms(void);
+extern void delay(uint64_t cycles);
+extern void udelay(uint64_t us);
+extern void sleep_tb(uint64_t cycles);
+extern void usleep(uint64_t us);
+
+static inline void mdelay(uint64_t ms)
+{
+	while (ms--)
+		udelay(1000);
+}
+
+static inline void msleep(uint64_t ms)
+{
+	usleep(ms * 1000);
+}
+
+#endif /* _ASMPOWERPC_TIME_H_ */
diff --git a/lib/powerpc/processor.c b/lib/powerpc/processor.c
index b224fc8eb..ad0d95666 100644
--- a/lib/powerpc/processor.c
+++ b/lib/powerpc/processor.c
@@ -7,6 +7,7 @@
 
 #include <libcflat.h>
 #include <asm/processor.h>
+#include <asm/time.h>
 #include <asm/ptrace.h>
 #include <asm/setup.h>
 #include <asm/barrier.h>
@@ -54,6 +55,16 @@ void do_handle_exception(struct pt_regs *regs)
 	abort();
 }
 
+uint64_t get_clock_us(void)
+{
+	return get_tb() * 1000000 / tb_hz;
+}
+
+uint64_t get_clock_ms(void)
+{
+	return get_tb() * 1000 / tb_hz;
+}
+
 void delay(uint64_t cycles)
 {
 	uint64_t start = get_tb();
diff --git a/lib/powerpc/smp.c b/lib/powerpc/smp.c
index afe436179..3e211eba8 100644
--- a/lib/powerpc/smp.c
+++ b/lib/powerpc/smp.c
@@ -7,6 +7,7 @@
  */
 
 #include <devicetree.h>
+#include <asm/time.h>
 #include <asm/setup.h>
 #include <asm/rtas.h>
 #include <asm/smp.h>
diff --git a/lib/ppc64/asm/time.h b/lib/ppc64/asm/time.h
new file mode 100644
index 000000000..326d2887a
--- /dev/null
+++ b/lib/ppc64/asm/time.h
@@ -0,0 +1 @@
+#include "../../powerpc/asm/time.h"
diff --git a/powerpc/spapr_vpa.c b/powerpc/spapr_vpa.c
index 6a3fe5e3f..c2075e157 100644
--- a/powerpc/spapr_vpa.c
+++ b/powerpc/spapr_vpa.c
@@ -10,6 +10,7 @@
 #include <util.h>
 #include <alloc.h>
 #include <asm/processor.h>
+#include <asm/time.h>
 #include <asm/setup.h>
 #include <asm/hcall.h>
 #include <asm/vpa.h>
diff --git a/powerpc/sprs.c b/powerpc/sprs.c
index 57e487ceb..285976488 100644
--- a/powerpc/sprs.c
+++ b/powerpc/sprs.c
@@ -26,6 +26,7 @@
 #include <asm/handlers.h>
 #include <asm/hcall.h>
 #include <asm/processor.h>
+#include <asm/time.h>
 #include <asm/barrier.h>
 
 uint64_t before[1024], after[1024];
diff --git a/powerpc/tm.c b/powerpc/tm.c
index 7fa916366..6b1ceeb6e 100644
--- a/powerpc/tm.c
+++ b/powerpc/tm.c
@@ -8,6 +8,7 @@
 #include <libcflat.h>
 #include <asm/hcall.h>
 #include <asm/processor.h>
+#include <asm/time.h>
 #include <asm/handlers.h>
 #include <asm/smp.h>
 #include <asm/setup.h>
-- 
2.42.0

Re: [kvm-unit-tests PATCH 4/7] powerpc: add asm/time.h header with delay and get_clock_us/ms

[Thomas Huth]

On 26/02/2024 10.38, Nicholas Piggin wrote:
> This matches s390x clock and delay APIs, so common test code can start
> using time facilities.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   lib/powerpc/asm/processor.h | 21 ---------------------
>   lib/powerpc/asm/time.h      | 30 ++++++++++++++++++++++++++++++
>   lib/powerpc/processor.c     | 11 +++++++++++
>   lib/powerpc/smp.c           |  1 +
>   lib/ppc64/asm/time.h        |  1 +
>   powerpc/spapr_vpa.c         |  1 +
>   powerpc/sprs.c              |  1 +
>   powerpc/tm.c                |  1 +
>   8 files changed, 46 insertions(+), 21 deletions(-)
>   create mode 100644 lib/powerpc/asm/time.h
>   create mode 100644 lib/ppc64/asm/time.h

Reviewed-by: Thomas Huth <thuth@redhat.com>

[kvm-unit-tests PATCH 5/7] arch-run: Add a "continuous" migration option for tests

[Nicholas Piggin]

The cooperative migration protocol is very good to control precise
pre and post conditions for a migration event. However in some cases
its intrusiveness to the test program, can mask problems and make
analysis more difficult.

For example to stress test migration vs concurrent complicated
memory access, including TLB refill, ram dirtying, etc., then the
tight spin at getchar() and resumption of the workload after
migration is unhelpful.

This adds a continuous migration mode that directs the harness to
perform migrations continually. This is added to the migration
selftests, which also sees cooperative migration iterations reduced
to avoid increasing test time too much.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 common/selftest-migration.c | 16 +++++++++--
 lib/migrate.c               | 18 ++++++++++++
 lib/migrate.h               |  3 ++
 scripts/arch-run.bash       | 55 ++++++++++++++++++++++++++++++++-----
 4 files changed, 82 insertions(+), 10 deletions(-)

diff --git a/common/selftest-migration.c b/common/selftest-migration.c
index 0afd8581c..9a9b61835 100644
--- a/common/selftest-migration.c
+++ b/common/selftest-migration.c
@@ -9,12 +9,13 @@
  */
 #include <libcflat.h>
 #include <migrate.h>
+#include <asm/time.h>
 
-#define NR_MIGRATIONS 30
+#define NR_MIGRATIONS 15
 
 int main(int argc, char **argv)
 {
-	report_prefix_push("migration");
+	report_prefix_push("migration harness");
 
 	if (argc > 1 && !strcmp(argv[1], "skip")) {
 		migrate_skip();
@@ -24,7 +25,16 @@ int main(int argc, char **argv)
 
 		for (i = 0; i < NR_MIGRATIONS; i++)
 			migrate_quiet();
-		report(true, "simple harness stress");
+		report(true, "cooperative migration");
+
+		migrate_begin_continuous();
+		mdelay(2000);
+		migrate_end_continuous();
+		mdelay(1000);
+		migrate_begin_continuous();
+		mdelay(2000);
+		migrate_end_continuous();
+		report(true, "continuous migration");
 	}
 
 	report_prefix_pop();
diff --git a/lib/migrate.c b/lib/migrate.c
index 1d22196b7..770f76d5c 100644
--- a/lib/migrate.c
+++ b/lib/migrate.c
@@ -60,3 +60,21 @@ void migrate_skip(void)
 	puts("Skipped VM migration (quiet)\n");
 	(void)getchar();
 }
+
+void migrate_begin_continuous(void)
+{
+	puts("Begin continuous migration\n");
+	(void)getchar();
+}
+
+void migrate_end_continuous(void)
+{
+	/*
+	 * Migration can split this output between source and dest QEMU
+	 * output files, print twice and match once to always cope with
+	 * a split.
+	 */
+	puts("End continuous migration\n");
+	puts("End continuous migration (quiet)\n");
+	(void)getchar();
+}
diff --git a/lib/migrate.h b/lib/migrate.h
index db6e0c501..35b6703a2 100644
--- a/lib/migrate.h
+++ b/lib/migrate.h
@@ -11,3 +11,6 @@ void migrate_quiet(void);
 void migrate_once(void);
 
 void migrate_skip(void);
+
+void migrate_begin_continuous(void);
+void migrate_end_continuous(void);
diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index d0f6f098f..5c7e72036 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -125,15 +125,17 @@ qmp_events ()
 filter_quiet_msgs ()
 {
 	grep -v "Now migrate the VM (quiet)" |
+	grep -v "Begin continuous migration (quiet)" |
+	grep -v "End continuous migration (quiet)" |
 	grep -v "Skipped VM migration (quiet)"
 }
 
 seen_migrate_msg ()
 {
 	if [ $skip_migration -eq 1 ]; then
-		grep -q -e "Now migrate the VM" < $1
+	        grep -q -e "Now migrate the VM" -e "Begin continuous migration" < $1
 	else
-		grep -q -e "Now migrate the VM" -e "Skipped VM migration" < $1
+	        grep -q -e "Now migrate the VM" -e "Begin continuous migration" -e "Skipped VM migration" < $1
 	fi
 }
 
@@ -161,6 +163,7 @@ run_migration ()
 	src_qmpout=/dev/null
 	dst_qmpout=/dev/null
 	skip_migration=0
+	continuous_migration=0
 
 	mkfifo ${src_outfifo}
 	mkfifo ${dst_outfifo}
@@ -186,9 +189,12 @@ run_migration ()
 	do_migration || return $?
 
 	while ps -p ${live_pid} > /dev/null ; do
-		# Wait for test exit or further migration messages.
-		if ! seen_migrate_msg ${src_out} ;  then
+		if [[ ${continuous_migration} -eq 1 ]] ; then
+			do_migration || return $?
+		elif ! seen_migrate_msg ${src_out} ;  then
 			sleep 0.1
+		elif grep -q "Begin continuous migration" < ${src_out} ; then
+			do_migration || return $?
 		elif grep -q "Now migrate the VM" < ${src_out} ; then
 			do_migration || return $?
 		elif [ $skip_migration -eq 0 ] && grep -q "Skipped VM migration" < ${src_out} ; then
@@ -218,7 +224,7 @@ do_migration ()
 
 	# The test must prompt the user to migrate, so wait for the
 	# "Now migrate VM" or similar console message.
-	while ! seen_migrate_msg ${src_out} ; do
+	while [[ ${continuous_migration} -eq 0 ]] && ! seen_migrate_msg ${src_out} ; do
 		if ! ps -p ${live_pid} > /dev/null ; then
 			echo > ${dst_infifo}
 			qmp ${dst_qmp} '"quit"'> ${dst_qmpout} 2>/dev/null
@@ -229,12 +235,32 @@ do_migration ()
 		sleep 0.1
 	done
 
+	if grep -q "Begin continuous migration" < ${src_out} ; then
+		if [[ ${continuous_migration} -eq 1 ]] ; then
+			echo > ${dst_infifo}
+			qmp ${dst_qmp} '"quit"'> ${dst_qmpout} 2>/dev/null
+			echo "ERROR: Continuous migration already begun." >&2
+			qmp ${src_qmp} '"quit"'> ${src_qmpout} 2>/dev/null
+			return 3
+		fi
+		continuous_migration=1
+		echo > ${src_infifo}
+	fi
+
 	# Wait until the destination has created the incoming and qmp sockets
 	while ! [ -S ${dst_incoming} ] ; do sleep 0.1 ; done
 	while ! [ -S ${dst_qmp} ] ; do sleep 0.1 ; done
 
 	if [ $skip_migration -eq 0 ] && grep -q "Skipped VM migration" < ${src_out} ; then
 		# May not get any migrations, exit to main loop for now...
+		# No migrations today, shut down dst in an orderly manner...
+		if [[ ${continuous_migration} -eq 1 ]] ; then
+			echo > ${dst_infifo}
+			qmp ${dst_qmp} '"quit"'> ${dst_qmpout} 2>/dev/null
+			echo "ERROR: Can't skip in continuous migration." >&2
+			qmp ${src_qmp} '"quit"'> ${src_qmpout} 2>/dev/null
+			return 3
+		fi
 		echo > ${dst_infifo}
 		qmp ${dst_qmp} '"quit"'> ${dst_qmpout} 2>/dev/null
 		echo > ${src_infifo} # Resume src and carry on.
@@ -266,8 +292,23 @@ do_migration ()
 
 	qmp ${src_qmp} '"quit"'> ${src_qmpout} 2>/dev/null
 
-	# keypress to dst so getchar completes and test continues
-	echo > ${dst_infifo}
+	# Should we end continuous migration?
+	if grep -q "End continuous migration" < ${src_out} ; then
+		if [[ ${continuous_migration} -eq 0 ]] ; then
+			echo "ERROR: Can't end continuous migration when not started." >&2
+			echo > ${dst_infifo}
+			qmp ${dst_qmp} '"quit"'> ${dst_qmpout} 2>/dev/null
+			qmp ${src_qmp} '"quit"'> ${src_qmpout} 2>/dev/null
+			return 3
+		fi
+		continuous_migration=0
+		echo > ${src_infifo}
+	fi
+
+	if [[ ${continuous_migration} -eq 0 ]]; then
+		# keypress to dst so getchar completes and test continues
+		echo > ${dst_infifo}
+	fi
 
 	# Ensure the incoming socket is removed, ready for next destination
 	if [ -S ${dst_incoming} ] ; then
-- 
2.42.0

Re: [kvm-unit-tests PATCH 5/7] arch-run: Add a "continuous" migration option for tests

[Thomas Huth]

On 26/02/2024 10.38, Nicholas Piggin wrote:
> The cooperative migration protocol is very good to control precise
> pre and post conditions for a migration event. However in some cases
> its intrusiveness to the test program, can mask problems and make
> analysis more difficult.
> 
> For example to stress test migration vs concurrent complicated
> memory access, including TLB refill, ram dirtying, etc., then the
> tight spin at getchar() and resumption of the workload after
> migration is unhelpful.
> 
> This adds a continuous migration mode that directs the harness to
> perform migrations continually. This is added to the migration
> selftests, which also sees cooperative migration iterations reduced
> to avoid increasing test time too much.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   common/selftest-migration.c | 16 +++++++++--
>   lib/migrate.c               | 18 ++++++++++++
>   lib/migrate.h               |  3 ++
>   scripts/arch-run.bash       | 55 ++++++++++++++++++++++++++++++++-----
>   4 files changed, 82 insertions(+), 10 deletions(-)
> 
> diff --git a/common/selftest-migration.c b/common/selftest-migration.c
> index 0afd8581c..9a9b61835 100644
> --- a/common/selftest-migration.c
> +++ b/common/selftest-migration.c
> @@ -9,12 +9,13 @@
>    */
>   #include <libcflat.h>
>   #include <migrate.h>
> +#include <asm/time.h>
>   
> -#define NR_MIGRATIONS 30
> +#define NR_MIGRATIONS 15
>   
>   int main(int argc, char **argv)
>   {
> -	report_prefix_push("migration");
> +	report_prefix_push("migration harness");
>   
>   	if (argc > 1 && !strcmp(argv[1], "skip")) {
>   		migrate_skip();
> @@ -24,7 +25,16 @@ int main(int argc, char **argv)
>   
>   		for (i = 0; i < NR_MIGRATIONS; i++)
>   			migrate_quiet();
> -		report(true, "simple harness stress");
> +		report(true, "cooperative migration");
> +
> +		migrate_begin_continuous();
> +		mdelay(2000);
> +		migrate_end_continuous();
> +		mdelay(1000);
> +		migrate_begin_continuous();
> +		mdelay(2000);
> +		migrate_end_continuous();
> +		report(true, "continuous migration");
>   	}
>   
>   	report_prefix_pop();
> diff --git a/lib/migrate.c b/lib/migrate.c
> index 1d22196b7..770f76d5c 100644
> --- a/lib/migrate.c
> +++ b/lib/migrate.c
> @@ -60,3 +60,21 @@ void migrate_skip(void)
>   	puts("Skipped VM migration (quiet)\n");
>   	(void)getchar();
>   }
> +
> +void migrate_begin_continuous(void)
> +{
> +	puts("Begin continuous migration\n");
> +	(void)getchar();
> +}
> +
> +void migrate_end_continuous(void)
> +{
> +	/*
> +	 * Migration can split this output between source and dest QEMU
> +	 * output files, print twice and match once to always cope with
> +	 * a split.
> +	 */
> +	puts("End continuous migration\n");
> +	puts("End continuous migration (quiet)\n");
> +	(void)getchar();
> +}
> diff --git a/lib/migrate.h b/lib/migrate.h
> index db6e0c501..35b6703a2 100644
> --- a/lib/migrate.h
> +++ b/lib/migrate.h
> @@ -11,3 +11,6 @@ void migrate_quiet(void);
>   void migrate_once(void);
>   
>   void migrate_skip(void);
> +
> +void migrate_begin_continuous(void);
> +void migrate_end_continuous(void);
> diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> index d0f6f098f..5c7e72036 100644
> --- a/scripts/arch-run.bash
> +++ b/scripts/arch-run.bash
> @@ -125,15 +125,17 @@ qmp_events ()
>   filter_quiet_msgs ()
>   {
>   	grep -v "Now migrate the VM (quiet)" |
> +	grep -v "Begin continuous migration (quiet)" |
> +	grep -v "End continuous migration (quiet)" |
>   	grep -v "Skipped VM migration (quiet)"
>   }
>   
>   seen_migrate_msg ()
>   {
>   	if [ $skip_migration -eq 1 ]; then
> -		grep -q -e "Now migrate the VM" < $1
> +	        grep -q -e "Now migrate the VM" -e "Begin continuous migration" < $1
>   	else
> -		grep -q -e "Now migrate the VM" -e "Skipped VM migration" < $1
> +	        grep -q -e "Now migrate the VM" -e "Begin continuous migration" -e "Skipped VM migration" < $1
>   	fi
>   }
>   
> @@ -161,6 +163,7 @@ run_migration ()
>   	src_qmpout=/dev/null
>   	dst_qmpout=/dev/null
>   	skip_migration=0
> +	continuous_migration=0
>   
>   	mkfifo ${src_outfifo}
>   	mkfifo ${dst_outfifo}
> @@ -186,9 +189,12 @@ run_migration ()
>   	do_migration || return $?
>   
>   	while ps -p ${live_pid} > /dev/null ; do
> -		# Wait for test exit or further migration messages.
> -		if ! seen_migrate_msg ${src_out} ;  then
> +		if [[ ${continuous_migration} -eq 1 ]] ; then

Here you're using "[[" for testing ...

> +			do_migration || return $?
> +		elif ! seen_migrate_msg ${src_out} ;  then
>   			sleep 0.1
> +		elif grep -q "Begin continuous migration" < ${src_out} ; then
> +			do_migration || return $?
>   		elif grep -q "Now migrate the VM" < ${src_out} ; then
>   			do_migration || return $?
>   		elif [ $skip_migration -eq 0 ] && grep -q "Skipped VM migration" < ${src_out} ; then

... while the other code seems to use "[" for testing values. Can we try to 
stick to one style, please (unless it's really required to use "[[" somewhere)?

  Thanks,
   Thomas

Re: [kvm-unit-tests PATCH 5/7] arch-run: Add a "continuous" migration option for tests

[Andrew Jones]

On Mon, Mar 04, 2024 at 07:17:35AM +0100, Thomas Huth wrote:
> On 26/02/2024 10.38, Nicholas Piggin wrote:
> > The cooperative migration protocol is very good to control precise
> > pre and post conditions for a migration event. However in some cases
> > its intrusiveness to the test program, can mask problems and make
> > analysis more difficult.
> > 
> > For example to stress test migration vs concurrent complicated
> > memory access, including TLB refill, ram dirtying, etc., then the
> > tight spin at getchar() and resumption of the workload after
> > migration is unhelpful.
> > 
> > This adds a continuous migration mode that directs the harness to
> > perform migrations continually. This is added to the migration
> > selftests, which also sees cooperative migration iterations reduced
> > to avoid increasing test time too much.
> > 
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >   common/selftest-migration.c | 16 +++++++++--
> >   lib/migrate.c               | 18 ++++++++++++
> >   lib/migrate.h               |  3 ++
> >   scripts/arch-run.bash       | 55 ++++++++++++++++++++++++++++++++-----
> >   4 files changed, 82 insertions(+), 10 deletions(-)
> > 
> > diff --git a/common/selftest-migration.c b/common/selftest-migration.c
> > index 0afd8581c..9a9b61835 100644
> > --- a/common/selftest-migration.c
> > +++ b/common/selftest-migration.c
> > @@ -9,12 +9,13 @@
> >    */
> >   #include <libcflat.h>
> >   #include <migrate.h>
> > +#include <asm/time.h>
> > -#define NR_MIGRATIONS 30
> > +#define NR_MIGRATIONS 15
> >   int main(int argc, char **argv)
> >   {
> > -	report_prefix_push("migration");
> > +	report_prefix_push("migration harness");
> >   	if (argc > 1 && !strcmp(argv[1], "skip")) {
> >   		migrate_skip();
> > @@ -24,7 +25,16 @@ int main(int argc, char **argv)
> >   		for (i = 0; i < NR_MIGRATIONS; i++)
> >   			migrate_quiet();
> > -		report(true, "simple harness stress");
> > +		report(true, "cooperative migration");
> > +
> > +		migrate_begin_continuous();
> > +		mdelay(2000);
> > +		migrate_end_continuous();
> > +		mdelay(1000);
> > +		migrate_begin_continuous();
> > +		mdelay(2000);
> > +		migrate_end_continuous();
> > +		report(true, "continuous migration");
> >   	}
> >   	report_prefix_pop();
> > diff --git a/lib/migrate.c b/lib/migrate.c
> > index 1d22196b7..770f76d5c 100644
> > --- a/lib/migrate.c
> > +++ b/lib/migrate.c
> > @@ -60,3 +60,21 @@ void migrate_skip(void)
> >   	puts("Skipped VM migration (quiet)\n");
> >   	(void)getchar();
> >   }
> > +
> > +void migrate_begin_continuous(void)
> > +{
> > +	puts("Begin continuous migration\n");
> > +	(void)getchar();
> > +}
> > +
> > +void migrate_end_continuous(void)
> > +{
> > +	/*
> > +	 * Migration can split this output between source and dest QEMU
> > +	 * output files, print twice and match once to always cope with
> > +	 * a split.
> > +	 */
> > +	puts("End continuous migration\n");
> > +	puts("End continuous migration (quiet)\n");
> > +	(void)getchar();
> > +}
> > diff --git a/lib/migrate.h b/lib/migrate.h
> > index db6e0c501..35b6703a2 100644
> > --- a/lib/migrate.h
> > +++ b/lib/migrate.h
> > @@ -11,3 +11,6 @@ void migrate_quiet(void);
> >   void migrate_once(void);
> >   void migrate_skip(void);
> > +
> > +void migrate_begin_continuous(void);
> > +void migrate_end_continuous(void);
> > diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> > index d0f6f098f..5c7e72036 100644
> > --- a/scripts/arch-run.bash
> > +++ b/scripts/arch-run.bash
> > @@ -125,15 +125,17 @@ qmp_events ()
> >   filter_quiet_msgs ()
> >   {
> >   	grep -v "Now migrate the VM (quiet)" |
> > +	grep -v "Begin continuous migration (quiet)" |
> > +	grep -v "End continuous migration (quiet)" |
> >   	grep -v "Skipped VM migration (quiet)"
> >   }
> >   seen_migrate_msg ()
> >   {
> >   	if [ $skip_migration -eq 1 ]; then
> > -		grep -q -e "Now migrate the VM" < $1
> > +	        grep -q -e "Now migrate the VM" -e "Begin continuous migration" < $1
> >   	else
> > -		grep -q -e "Now migrate the VM" -e "Skipped VM migration" < $1
> > +	        grep -q -e "Now migrate the VM" -e "Begin continuous migration" -e "Skipped VM migration" < $1
> >   	fi
> >   }
> > @@ -161,6 +163,7 @@ run_migration ()
> >   	src_qmpout=/dev/null
> >   	dst_qmpout=/dev/null
> >   	skip_migration=0
> > +	continuous_migration=0
> >   	mkfifo ${src_outfifo}
> >   	mkfifo ${dst_outfifo}
> > @@ -186,9 +189,12 @@ run_migration ()
> >   	do_migration || return $?
> >   	while ps -p ${live_pid} > /dev/null ; do
> > -		# Wait for test exit or further migration messages.
> > -		if ! seen_migrate_msg ${src_out} ;  then
> > +		if [[ ${continuous_migration} -eq 1 ]] ; then
> 
> Here you're using "[[" for testing ...
> 
> > +			do_migration || return $?
> > +		elif ! seen_migrate_msg ${src_out} ;  then
> >   			sleep 0.1
> > +		elif grep -q "Begin continuous migration" < ${src_out} ; then
> > +			do_migration || return $?
> >   		elif grep -q "Now migrate the VM" < ${src_out} ; then
> >   			do_migration || return $?
> >   		elif [ $skip_migration -eq 0 ] && grep -q "Skipped VM migration" < ${src_out} ; then
> 
> ... while the other code seems to use "[" for testing values. Can we try to
> stick to one style, please (unless it's really required to use "[["
> somewhere)?
>

We should decide on a Bash coding style and on preferences like [[ and
then write a document for it, as well as create a set of shellcheck
includes/excludes to test it. Then, using shellcheck we'd change all our
current Bash code and also require shellcheck to pass on all new code
before merge. Any volunteers for that effort? For the style selection
we can take inspiration from other projects or even just adopt their
style guides. Google has some guidance[1][2] and googling for Bash style
pops up other hits.

[1] https://google.github.io/styleguide/shellguide.html
[2] https://chromium.googlesource.com/chromiumos/docs/+/master/styleguide/shell.md

Thanks,
drew

Re: [kvm-unit-tests PATCH 5/7] arch-run: Add a "continuous" migration option for tests

[Nicholas Piggin]

On Mon Mar 4, 2024 at 7:19 PM AEST, Andrew Jones wrote:
> On Mon, Mar 04, 2024 at 07:17:35AM +0100, Thomas Huth wrote:
> > On 26/02/2024 10.38, Nicholas Piggin wrote:
> > > The cooperative migration protocol is very good to control precise
> > > pre and post conditions for a migration event. However in some cases
> > > its intrusiveness to the test program, can mask problems and make
> > > analysis more difficult.
> > > 
> > > For example to stress test migration vs concurrent complicated
> > > memory access, including TLB refill, ram dirtying, etc., then the
> > > tight spin at getchar() and resumption of the workload after
> > > migration is unhelpful.
> > > 
> > > This adds a continuous migration mode that directs the harness to
> > > perform migrations continually. This is added to the migration
> > > selftests, which also sees cooperative migration iterations reduced
> > > to avoid increasing test time too much.
> > > 
> > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > > ---
> > >   common/selftest-migration.c | 16 +++++++++--
> > >   lib/migrate.c               | 18 ++++++++++++
> > >   lib/migrate.h               |  3 ++
> > >   scripts/arch-run.bash       | 55 ++++++++++++++++++++++++++++++++-----
> > >   4 files changed, 82 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/common/selftest-migration.c b/common/selftest-migration.c
> > > index 0afd8581c..9a9b61835 100644
> > > --- a/common/selftest-migration.c
> > > +++ b/common/selftest-migration.c
> > > @@ -9,12 +9,13 @@
> > >    */
> > >   #include <libcflat.h>
> > >   #include <migrate.h>
> > > +#include <asm/time.h>
> > > -#define NR_MIGRATIONS 30
> > > +#define NR_MIGRATIONS 15
> > >   int main(int argc, char **argv)
> > >   {
> > > -	report_prefix_push("migration");
> > > +	report_prefix_push("migration harness");
> > >   	if (argc > 1 && !strcmp(argv[1], "skip")) {
> > >   		migrate_skip();
> > > @@ -24,7 +25,16 @@ int main(int argc, char **argv)
> > >   		for (i = 0; i < NR_MIGRATIONS; i++)
> > >   			migrate_quiet();
> > > -		report(true, "simple harness stress");
> > > +		report(true, "cooperative migration");
> > > +
> > > +		migrate_begin_continuous();
> > > +		mdelay(2000);
> > > +		migrate_end_continuous();
> > > +		mdelay(1000);
> > > +		migrate_begin_continuous();
> > > +		mdelay(2000);
> > > +		migrate_end_continuous();
> > > +		report(true, "continuous migration");
> > >   	}
> > >   	report_prefix_pop();
> > > diff --git a/lib/migrate.c b/lib/migrate.c
> > > index 1d22196b7..770f76d5c 100644
> > > --- a/lib/migrate.c
> > > +++ b/lib/migrate.c
> > > @@ -60,3 +60,21 @@ void migrate_skip(void)
> > >   	puts("Skipped VM migration (quiet)\n");
> > >   	(void)getchar();
> > >   }
> > > +
> > > +void migrate_begin_continuous(void)
> > > +{
> > > +	puts("Begin continuous migration\n");
> > > +	(void)getchar();
> > > +}
> > > +
> > > +void migrate_end_continuous(void)
> > > +{
> > > +	/*
> > > +	 * Migration can split this output between source and dest QEMU
> > > +	 * output files, print twice and match once to always cope with
> > > +	 * a split.
> > > +	 */
> > > +	puts("End continuous migration\n");
> > > +	puts("End continuous migration (quiet)\n");
> > > +	(void)getchar();
> > > +}
> > > diff --git a/lib/migrate.h b/lib/migrate.h
> > > index db6e0c501..35b6703a2 100644
> > > --- a/lib/migrate.h
> > > +++ b/lib/migrate.h
> > > @@ -11,3 +11,6 @@ void migrate_quiet(void);
> > >   void migrate_once(void);
> > >   void migrate_skip(void);
> > > +
> > > +void migrate_begin_continuous(void);
> > > +void migrate_end_continuous(void);
> > > diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> > > index d0f6f098f..5c7e72036 100644
> > > --- a/scripts/arch-run.bash
> > > +++ b/scripts/arch-run.bash
> > > @@ -125,15 +125,17 @@ qmp_events ()
> > >   filter_quiet_msgs ()
> > >   {
> > >   	grep -v "Now migrate the VM (quiet)" |
> > > +	grep -v "Begin continuous migration (quiet)" |
> > > +	grep -v "End continuous migration (quiet)" |
> > >   	grep -v "Skipped VM migration (quiet)"
> > >   }
> > >   seen_migrate_msg ()
> > >   {
> > >   	if [ $skip_migration -eq 1 ]; then
> > > -		grep -q -e "Now migrate the VM" < $1
> > > +	        grep -q -e "Now migrate the VM" -e "Begin continuous migration" < $1
> > >   	else
> > > -		grep -q -e "Now migrate the VM" -e "Skipped VM migration" < $1
> > > +	        grep -q -e "Now migrate the VM" -e "Begin continuous migration" -e "Skipped VM migration" < $1
> > >   	fi
> > >   }
> > > @@ -161,6 +163,7 @@ run_migration ()
> > >   	src_qmpout=/dev/null
> > >   	dst_qmpout=/dev/null
> > >   	skip_migration=0
> > > +	continuous_migration=0
> > >   	mkfifo ${src_outfifo}
> > >   	mkfifo ${dst_outfifo}
> > > @@ -186,9 +189,12 @@ run_migration ()
> > >   	do_migration || return $?
> > >   	while ps -p ${live_pid} > /dev/null ; do
> > > -		# Wait for test exit or further migration messages.
> > > -		if ! seen_migrate_msg ${src_out} ;  then
> > > +		if [[ ${continuous_migration} -eq 1 ]] ; then
> > 
> > Here you're using "[[" for testing ...
> > 
> > > +			do_migration || return $?
> > > +		elif ! seen_migrate_msg ${src_out} ;  then
> > >   			sleep 0.1
> > > +		elif grep -q "Begin continuous migration" < ${src_out} ; then
> > > +			do_migration || return $?
> > >   		elif grep -q "Now migrate the VM" < ${src_out} ; then
> > >   			do_migration || return $?
> > >   		elif [ $skip_migration -eq 0 ] && grep -q "Skipped VM migration" < ${src_out} ; then
> > 
> > ... while the other code seems to use "[" for testing values. Can we try to
> > stick to one style, please (unless it's really required to use "[["
> > somewhere)?
> >
>
> We should decide on a Bash coding style and on preferences like [[ and
> then write a document for it, as well as create a set of shellcheck
> includes/excludes to test it. Then, using shellcheck we'd change all our
> current Bash code and also require shellcheck to pass on all new code
> before merge.

Seems like a good idea.

> Any volunteers for that effort? For the style selection
> we can take inspiration from other projects or even just adopt their
> style guides. Google has some guidance[1][2] and googling for Bash style
> pops up other hits.
>
> [1] https://google.github.io/styleguide/shellguide.html
> [2] https://chromium.googlesource.com/chromiumos/docs/+/master/styleguide/shell.md

My bash skills consit of mashing the keyboard until the errors quieten
down, so I may not be the best to decide on this stuff. I could take a
look at getting the checker running in make and post an RFC though. No
promises if it turns out to be harder than it looks...

Thanks,
Nick

Re: [kvm-unit-tests PATCH 5/7] arch-run: Add a "continuous" migration option for tests

[Nicholas Piggin]

On Mon Mar 4, 2024 at 4:17 PM AEST, Thomas Huth wrote:
> On 26/02/2024 10.38, Nicholas Piggin wrote:
> > The cooperative migration protocol is very good to control precise
> > pre and post conditions for a migration event. However in some cases
> > its intrusiveness to the test program, can mask problems and make
> > analysis more difficult.
> > 
> > For example to stress test migration vs concurrent complicated
> > memory access, including TLB refill, ram dirtying, etc., then the
> > tight spin at getchar() and resumption of the workload after
> > migration is unhelpful.
> > 
> > This adds a continuous migration mode that directs the harness to
> > perform migrations continually. This is added to the migration
> > selftests, which also sees cooperative migration iterations reduced
> > to avoid increasing test time too much.
> > 
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >   common/selftest-migration.c | 16 +++++++++--
> >   lib/migrate.c               | 18 ++++++++++++
> >   lib/migrate.h               |  3 ++
> >   scripts/arch-run.bash       | 55 ++++++++++++++++++++++++++++++++-----
> >   4 files changed, 82 insertions(+), 10 deletions(-)
> > 
> > diff --git a/common/selftest-migration.c b/common/selftest-migration.c
> > index 0afd8581c..9a9b61835 100644
> > --- a/common/selftest-migration.c
> > +++ b/common/selftest-migration.c
> > @@ -9,12 +9,13 @@
> >    */
> >   #include <libcflat.h>
> >   #include <migrate.h>
> > +#include <asm/time.h>
> >   
> > -#define NR_MIGRATIONS 30
> > +#define NR_MIGRATIONS 15
> >   
> >   int main(int argc, char **argv)
> >   {
> > -	report_prefix_push("migration");
> > +	report_prefix_push("migration harness");
> >   
> >   	if (argc > 1 && !strcmp(argv[1], "skip")) {
> >   		migrate_skip();
> > @@ -24,7 +25,16 @@ int main(int argc, char **argv)
> >   
> >   		for (i = 0; i < NR_MIGRATIONS; i++)
> >   			migrate_quiet();
> > -		report(true, "simple harness stress");
> > +		report(true, "cooperative migration");
> > +
> > +		migrate_begin_continuous();
> > +		mdelay(2000);
> > +		migrate_end_continuous();
> > +		mdelay(1000);
> > +		migrate_begin_continuous();
> > +		mdelay(2000);
> > +		migrate_end_continuous();
> > +		report(true, "continuous migration");
> >   	}
> >   
> >   	report_prefix_pop();
> > diff --git a/lib/migrate.c b/lib/migrate.c
> > index 1d22196b7..770f76d5c 100644
> > --- a/lib/migrate.c
> > +++ b/lib/migrate.c
> > @@ -60,3 +60,21 @@ void migrate_skip(void)
> >   	puts("Skipped VM migration (quiet)\n");
> >   	(void)getchar();
> >   }
> > +
> > +void migrate_begin_continuous(void)
> > +{
> > +	puts("Begin continuous migration\n");
> > +	(void)getchar();
> > +}
> > +
> > +void migrate_end_continuous(void)
> > +{
> > +	/*
> > +	 * Migration can split this output between source and dest QEMU
> > +	 * output files, print twice and match once to always cope with
> > +	 * a split.
> > +	 */
> > +	puts("End continuous migration\n");
> > +	puts("End continuous migration (quiet)\n");
> > +	(void)getchar();
> > +}
> > diff --git a/lib/migrate.h b/lib/migrate.h
> > index db6e0c501..35b6703a2 100644
> > --- a/lib/migrate.h
> > +++ b/lib/migrate.h
> > @@ -11,3 +11,6 @@ void migrate_quiet(void);
> >   void migrate_once(void);
> >   
> >   void migrate_skip(void);
> > +
> > +void migrate_begin_continuous(void);
> > +void migrate_end_continuous(void);
> > diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> > index d0f6f098f..5c7e72036 100644
> > --- a/scripts/arch-run.bash
> > +++ b/scripts/arch-run.bash
> > @@ -125,15 +125,17 @@ qmp_events ()
> >   filter_quiet_msgs ()
> >   {
> >   	grep -v "Now migrate the VM (quiet)" |
> > +	grep -v "Begin continuous migration (quiet)" |
> > +	grep -v "End continuous migration (quiet)" |
> >   	grep -v "Skipped VM migration (quiet)"
> >   }
> >   
> >   seen_migrate_msg ()
> >   {
> >   	if [ $skip_migration -eq 1 ]; then
> > -		grep -q -e "Now migrate the VM" < $1
> > +	        grep -q -e "Now migrate the VM" -e "Begin continuous migration" < $1
> >   	else
> > -		grep -q -e "Now migrate the VM" -e "Skipped VM migration" < $1
> > +	        grep -q -e "Now migrate the VM" -e "Begin continuous migration" -e "Skipped VM migration" < $1
> >   	fi
> >   }
> >   
> > @@ -161,6 +163,7 @@ run_migration ()
> >   	src_qmpout=/dev/null
> >   	dst_qmpout=/dev/null
> >   	skip_migration=0
> > +	continuous_migration=0
> >   
> >   	mkfifo ${src_outfifo}
> >   	mkfifo ${dst_outfifo}
> > @@ -186,9 +189,12 @@ run_migration ()
> >   	do_migration || return $?
> >   
> >   	while ps -p ${live_pid} > /dev/null ; do
> > -		# Wait for test exit or further migration messages.
> > -		if ! seen_migrate_msg ${src_out} ;  then
> > +		if [[ ${continuous_migration} -eq 1 ]] ; then
>
> Here you're using "[[" for testing ...
>
> > +			do_migration || return $?
> > +		elif ! seen_migrate_msg ${src_out} ;  then
> >   			sleep 0.1
> > +		elif grep -q "Begin continuous migration" < ${src_out} ; then
> > +			do_migration || return $?
> >   		elif grep -q "Now migrate the VM" < ${src_out} ; then
> >   			do_migration || return $?
> >   		elif [ $skip_migration -eq 0 ] && grep -q "Skipped VM migration" < ${src_out} ; then
>
> ... while the other code seems to use "[" for testing values. Can we try to 
> stick to one style, please (unless it's really required to use "[[" somewhere)?

Good point. Will do.

Thanks,
Nick

[kvm-unit-tests PATCH 6/7] gitlab-ci: Run migration selftest on s390x and powerpc

[Nicholas Piggin]

The migration harness is complicated and easy to break so CI will
be helpful.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 .gitlab-ci.yml | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 71d986e98..61f196d5d 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -64,26 +64,28 @@ build-arm:
 build-ppc64be:
  extends: .outoftree_template
  script:
- - dnf install -y qemu-system-ppc gcc-powerpc64-linux-gnu
+ - dnf install -y qemu-system-ppc gcc-powerpc64-linux-gnu nmap-ncat
  - mkdir build
  - cd build
  - ../configure --arch=ppc64 --endian=big --cross-prefix=powerpc64-linux-gnu-
  - make -j2
  - ACCEL=tcg ./run_tests.sh
-     selftest-setup spapr_hcall rtas-get-time-of-day rtas-get-time-of-day-base
-     rtas-set-time-of-day emulator
+     selftest-setup selftest-migration selftest-migration-skip spapr_hcall
+     rtas-get-time-of-day rtas-get-time-of-day-base rtas-set-time-of-day
+     emulator
      | tee results.txt
  - if grep -q FAIL results.txt ; then exit 1 ; fi
 
 build-ppc64le:
  extends: .intree_template
  script:
- - dnf install -y qemu-system-ppc gcc-powerpc64-linux-gnu
+ - dnf install -y qemu-system-ppc gcc-powerpc64-linux-gnu nmap-ncat
  - ./configure --arch=ppc64 --endian=little --cross-prefix=powerpc64-linux-gnu-
  - make -j2
  - ACCEL=tcg ./run_tests.sh
-     selftest-setup spapr_hcall rtas-get-time-of-day rtas-get-time-of-day-base
-     rtas-set-time-of-day emulator
+     selftest-setup selftest-migration selftest-migration-skip spapr_hcall
+     rtas-get-time-of-day rtas-get-time-of-day-base rtas-set-time-of-day
+     emulator
      | tee results.txt
  - if grep -q FAIL results.txt ; then exit 1 ; fi
 
@@ -107,7 +109,7 @@ build-riscv64:
 build-s390x:
  extends: .outoftree_template
  script:
- - dnf install -y qemu-system-s390x gcc-s390x-linux-gnu
+ - dnf install -y qemu-system-s390x gcc-s390x-linux-gnu nmap-ncat
  - mkdir build
  - cd build
  - ../configure --arch=s390x --cross-prefix=s390x-linux-gnu-
@@ -133,6 +135,8 @@ build-s390x:
       sclp-1g
       sclp-3g
       selftest-setup
+      selftest-migration
+      selftest-migration-skip
       sieve
       smp
       stsi
-- 
2.42.0

Re: [kvm-unit-tests PATCH 6/7] gitlab-ci: Run migration selftest on s390x and powerpc

[Thomas Huth]

On 26/02/2024 10.38, Nicholas Piggin wrote:
> The migration harness is complicated and easy to break so CI will
> be helpful.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   .gitlab-ci.yml | 18 +++++++++++-------
>   1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index 71d986e98..61f196d5d 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -64,26 +64,28 @@ build-arm:
>   build-ppc64be:
>    extends: .outoftree_template
>    script:
> - - dnf install -y qemu-system-ppc gcc-powerpc64-linux-gnu
> + - dnf install -y qemu-system-ppc gcc-powerpc64-linux-gnu nmap-ncat
>    - mkdir build
>    - cd build
>    - ../configure --arch=ppc64 --endian=big --cross-prefix=powerpc64-linux-gnu-
>    - make -j2
>    - ACCEL=tcg ./run_tests.sh
> -     selftest-setup spapr_hcall rtas-get-time-of-day rtas-get-time-of-day-base
> -     rtas-set-time-of-day emulator
> +     selftest-setup selftest-migration selftest-migration-skip spapr_hcall
> +     rtas-get-time-of-day rtas-get-time-of-day-base rtas-set-time-of-day
> +     emulator
>        | tee results.txt
>    - if grep -q FAIL results.txt ; then exit 1 ; fi
>   
>   build-ppc64le:
>    extends: .intree_template
>    script:
> - - dnf install -y qemu-system-ppc gcc-powerpc64-linux-gnu
> + - dnf install -y qemu-system-ppc gcc-powerpc64-linux-gnu nmap-ncat
>    - ./configure --arch=ppc64 --endian=little --cross-prefix=powerpc64-linux-gnu-
>    - make -j2
>    - ACCEL=tcg ./run_tests.sh
> -     selftest-setup spapr_hcall rtas-get-time-of-day rtas-get-time-of-day-base
> -     rtas-set-time-of-day emulator
> +     selftest-setup selftest-migration selftest-migration-skip spapr_hcall
> +     rtas-get-time-of-day rtas-get-time-of-day-base rtas-set-time-of-day
> +     emulator
>        | tee results.txt
>    - if grep -q FAIL results.txt ; then exit 1 ; fi
>   
> @@ -107,7 +109,7 @@ build-riscv64:
>   build-s390x:
>    extends: .outoftree_template
>    script:
> - - dnf install -y qemu-system-s390x gcc-s390x-linux-gnu
> + - dnf install -y qemu-system-s390x gcc-s390x-linux-gnu nmap-ncat
>    - mkdir build
>    - cd build
>    - ../configure --arch=s390x --cross-prefix=s390x-linux-gnu-
> @@ -133,6 +135,8 @@ build-s390x:
>         sclp-1g
>         sclp-3g
>         selftest-setup
> +      selftest-migration
> +      selftest-migration-skip
>         sieve
>         smp
>         stsi

While I can update the qemu binary for the s390x-kvm job, the build-* jobs 
run in a container with a normal QEMU from the corresponding distros, so I 
think this has to wait 'til we get distros that contain your QEMU TCG 
migration fix.

  Thomas

Re: [kvm-unit-tests PATCH 6/7] gitlab-ci: Run migration selftest on s390x and powerpc

[Nicholas Piggin]

On Sat Mar 2, 2024 at 12:16 AM AEST, Thomas Huth wrote:
> On 26/02/2024 10.38, Nicholas Piggin wrote:
> > The migration harness is complicated and easy to break so CI will
> > be helpful.
> > 
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >   .gitlab-ci.yml | 18 +++++++++++-------
> >   1 file changed, 11 insertions(+), 7 deletions(-)
> > 
> > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> > index 71d986e98..61f196d5d 100644
> > --- a/.gitlab-ci.yml
> > +++ b/.gitlab-ci.yml
> > @@ -64,26 +64,28 @@ build-arm:
> >   build-ppc64be:
> >    extends: .outoftree_template
> >    script:
> > - - dnf install -y qemu-system-ppc gcc-powerpc64-linux-gnu
> > + - dnf install -y qemu-system-ppc gcc-powerpc64-linux-gnu nmap-ncat
> >    - mkdir build
> >    - cd build
> >    - ../configure --arch=ppc64 --endian=big --cross-prefix=powerpc64-linux-gnu-
> >    - make -j2
> >    - ACCEL=tcg ./run_tests.sh
> > -     selftest-setup spapr_hcall rtas-get-time-of-day rtas-get-time-of-day-base
> > -     rtas-set-time-of-day emulator
> > +     selftest-setup selftest-migration selftest-migration-skip spapr_hcall
> > +     rtas-get-time-of-day rtas-get-time-of-day-base rtas-set-time-of-day
> > +     emulator
> >        | tee results.txt
> >    - if grep -q FAIL results.txt ; then exit 1 ; fi
> >   
> >   build-ppc64le:
> >    extends: .intree_template
> >    script:
> > - - dnf install -y qemu-system-ppc gcc-powerpc64-linux-gnu
> > + - dnf install -y qemu-system-ppc gcc-powerpc64-linux-gnu nmap-ncat
> >    - ./configure --arch=ppc64 --endian=little --cross-prefix=powerpc64-linux-gnu-
> >    - make -j2
> >    - ACCEL=tcg ./run_tests.sh
> > -     selftest-setup spapr_hcall rtas-get-time-of-day rtas-get-time-of-day-base
> > -     rtas-set-time-of-day emulator
> > +     selftest-setup selftest-migration selftest-migration-skip spapr_hcall
> > +     rtas-get-time-of-day rtas-get-time-of-day-base rtas-set-time-of-day
> > +     emulator
> >        | tee results.txt
> >    - if grep -q FAIL results.txt ; then exit 1 ; fi
> >   
> > @@ -107,7 +109,7 @@ build-riscv64:
> >   build-s390x:
> >    extends: .outoftree_template
> >    script:
> > - - dnf install -y qemu-system-s390x gcc-s390x-linux-gnu
> > + - dnf install -y qemu-system-s390x gcc-s390x-linux-gnu nmap-ncat
> >    - mkdir build
> >    - cd build
> >    - ../configure --arch=s390x --cross-prefix=s390x-linux-gnu-
> > @@ -133,6 +135,8 @@ build-s390x:
> >         sclp-1g
> >         sclp-3g
> >         selftest-setup
> > +      selftest-migration
> > +      selftest-migration-skip
> >         sieve
> >         smp
> >         stsi
>
> While I can update the qemu binary for the s390x-kvm job, the build-* jobs 
> run in a container with a normal QEMU from the corresponding distros, so I 
> think this has to wait 'til we get distros that contain your QEMU TCG 
> migration fix.

Okay. powerpc *could* run into the TCG bug too, in practice it has not.
We could try enable it there to get migration into CI, and revert it if
it starts showing random failures?

Thanks,
Nick

Re: [kvm-unit-tests PATCH 6/7] gitlab-ci: Run migration selftest on s390x and powerpc

[Thomas Huth]

On 05/03/2024 03.38, Nicholas Piggin wrote:
> On Sat Mar 2, 2024 at 12:16 AM AEST, Thomas Huth wrote:
>> On 26/02/2024 10.38, Nicholas Piggin wrote:
>>> The migration harness is complicated and easy to break so CI will
>>> be helpful.
>>>
>>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>>> ---
>>>    .gitlab-ci.yml | 18 +++++++++++-------
>>>    1 file changed, 11 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
>>> index 71d986e98..61f196d5d 100644
>>> --- a/.gitlab-ci.yml
>>> +++ b/.gitlab-ci.yml
>>> @@ -64,26 +64,28 @@ build-arm:
>>>    build-ppc64be:
>>>     extends: .outoftree_template
>>>     script:
>>> - - dnf install -y qemu-system-ppc gcc-powerpc64-linux-gnu
>>> + - dnf install -y qemu-system-ppc gcc-powerpc64-linux-gnu nmap-ncat
>>>     - mkdir build
>>>     - cd build
>>>     - ../configure --arch=ppc64 --endian=big --cross-prefix=powerpc64-linux-gnu-
>>>     - make -j2
>>>     - ACCEL=tcg ./run_tests.sh
>>> -     selftest-setup spapr_hcall rtas-get-time-of-day rtas-get-time-of-day-base
>>> -     rtas-set-time-of-day emulator
>>> +     selftest-setup selftest-migration selftest-migration-skip spapr_hcall
>>> +     rtas-get-time-of-day rtas-get-time-of-day-base rtas-set-time-of-day
>>> +     emulator
>>>         | tee results.txt
>>>     - if grep -q FAIL results.txt ; then exit 1 ; fi
>>>    
>>>    build-ppc64le:
>>>     extends: .intree_template
>>>     script:
>>> - - dnf install -y qemu-system-ppc gcc-powerpc64-linux-gnu
>>> + - dnf install -y qemu-system-ppc gcc-powerpc64-linux-gnu nmap-ncat
>>>     - ./configure --arch=ppc64 --endian=little --cross-prefix=powerpc64-linux-gnu-
>>>     - make -j2
>>>     - ACCEL=tcg ./run_tests.sh
>>> -     selftest-setup spapr_hcall rtas-get-time-of-day rtas-get-time-of-day-base
>>> -     rtas-set-time-of-day emulator
>>> +     selftest-setup selftest-migration selftest-migration-skip spapr_hcall
>>> +     rtas-get-time-of-day rtas-get-time-of-day-base rtas-set-time-of-day
>>> +     emulator
>>>         | tee results.txt
>>>     - if grep -q FAIL results.txt ; then exit 1 ; fi
>>>    
>>> @@ -107,7 +109,7 @@ build-riscv64:
>>>    build-s390x:
>>>     extends: .outoftree_template
>>>     script:
>>> - - dnf install -y qemu-system-s390x gcc-s390x-linux-gnu
>>> + - dnf install -y qemu-system-s390x gcc-s390x-linux-gnu nmap-ncat
>>>     - mkdir build
>>>     - cd build
>>>     - ../configure --arch=s390x --cross-prefix=s390x-linux-gnu-
>>> @@ -133,6 +135,8 @@ build-s390x:
>>>          sclp-1g
>>>          sclp-3g
>>>          selftest-setup
>>> +      selftest-migration
>>> +      selftest-migration-skip
>>>          sieve
>>>          smp
>>>          stsi
>>
>> While I can update the qemu binary for the s390x-kvm job, the build-* jobs
>> run in a container with a normal QEMU from the corresponding distros, so I
>> think this has to wait 'til we get distros that contain your QEMU TCG
>> migration fix.
> 
> Okay. powerpc *could* run into the TCG bug too, in practice it has not.
> We could try enable it there to get migration into CI, and revert it if
> it starts showing random failures?

Fine for me.

  Thomas

[kvm-unit-tests PATCH 7/7] common: add memory dirtying vs migration test

[Nicholas Piggin]

This test stores to a bunch of pages and verifies previous stores,
while being continually migrated. This can fail due to a QEMU TCG
physical memory dirty bitmap bug.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 common/memory-verify.c  | 48 +++++++++++++++++++++++++++++++++++++++++
 powerpc/Makefile.common |  1 +
 powerpc/memory-verify.c |  1 +
 powerpc/unittests.cfg   |  7 ++++++
 s390x/Makefile          |  1 +
 s390x/memory-verify.c   |  1 +
 s390x/unittests.cfg     |  6 ++++++
 7 files changed, 65 insertions(+)
 create mode 100644 common/memory-verify.c
 create mode 120000 powerpc/memory-verify.c
 create mode 120000 s390x/memory-verify.c

diff --git a/common/memory-verify.c b/common/memory-verify.c
new file mode 100644
index 000000000..7c4ec087b
--- /dev/null
+++ b/common/memory-verify.c
@@ -0,0 +1,48 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Simple memory verification test, used to exercise dirty memory migration.
+ *
+ */
+#include <libcflat.h>
+#include <migrate.h>
+#include <alloc.h>
+#include <asm/page.h>
+#include <asm/time.h>
+
+#define NR_PAGES 32
+
+int main(int argc, char **argv)
+{
+	void *mem = malloc(NR_PAGES*PAGE_SIZE);
+	bool success = true;
+	uint64_t ms;
+	long i;
+
+	report_prefix_push("memory");
+
+	memset(mem, 0, NR_PAGES*PAGE_SIZE);
+
+	migrate_begin_continuous();
+	ms = get_clock_ms();
+	i = 0;
+	do {
+		int j;
+
+		for (j = 0; j < NR_PAGES*PAGE_SIZE; j += PAGE_SIZE) {
+			if (*(volatile long *)(mem + j) != i) {
+				success = false;
+				goto out;
+			}
+			*(volatile long *)(mem + j) = i + 1;
+		}
+		i++;
+	} while (get_clock_ms() - ms < 5000);
+out:
+	migrate_end_continuous();
+
+	report(success, "memory verification stress test");
+
+	report_prefix_pop();
+
+	return report_summary();
+}
diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
index da4a7bbb8..1e181da69 100644
--- a/powerpc/Makefile.common
+++ b/powerpc/Makefile.common
@@ -7,6 +7,7 @@
 tests-common = \
 	$(TEST_DIR)/selftest.elf \
 	$(TEST_DIR)/selftest-migration.elf \
+	$(TEST_DIR)/memory-verify.elf \
 	$(TEST_DIR)/spapr_hcall.elf \
 	$(TEST_DIR)/rtas.elf \
 	$(TEST_DIR)/emulator.elf \
diff --git a/powerpc/memory-verify.c b/powerpc/memory-verify.c
new file mode 120000
index 000000000..5985c730f
--- /dev/null
+++ b/powerpc/memory-verify.c
@@ -0,0 +1 @@
+../common/memory-verify.c
\ No newline at end of file
diff --git a/powerpc/unittests.cfg b/powerpc/unittests.cfg
index 89abf2095..fadd8dde6 100644
--- a/powerpc/unittests.cfg
+++ b/powerpc/unittests.cfg
@@ -46,6 +46,13 @@ machine = pseries
 groups = selftest migration
 extra_params = -append "skip"
 
+# This fails due to a QEMU TCG bug so KVM-only until QEMU is fixed upstream
+[migration-memory]
+file = memory-verify.elf
+accel = kvm
+machine = pseries
+groups = migration
+
 [spapr_hcall]
 file = spapr_hcall.elf
 
diff --git a/s390x/Makefile b/s390x/Makefile
index 344d46d68..ddc0969f3 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -1,5 +1,6 @@
 tests = $(TEST_DIR)/selftest.elf
 tests += $(TEST_DIR)/selftest-migration.elf
+tests += $(TEST_DIR)/memory-verify.elf
 tests += $(TEST_DIR)/intercept.elf
 tests += $(TEST_DIR)/emulator.elf
 tests += $(TEST_DIR)/sieve.elf
diff --git a/s390x/memory-verify.c b/s390x/memory-verify.c
new file mode 120000
index 000000000..5985c730f
--- /dev/null
+++ b/s390x/memory-verify.c
@@ -0,0 +1 @@
+../common/memory-verify.c
\ No newline at end of file
diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
index f613602d3..a88fe9e79 100644
--- a/s390x/unittests.cfg
+++ b/s390x/unittests.cfg
@@ -33,6 +33,12 @@ file = selftest-migration.elf
 groups = selftest migration
 extra_params = -append "skip"
 
+# This fails due to a QEMU TCG bug so KVM-only until QEMU is fixed upstream
+[migration-memory]
+file = memory-verify.elf
+accel = kvm
+groups = migration
+
 [intercept]
 file = intercept.elf
 
-- 
2.42.0

Re: [kvm-unit-tests PATCH 7/7] common: add memory dirtying vs migration test

[Thomas Huth]

On 26/02/2024 10.38, Nicholas Piggin wrote:
> This test stores to a bunch of pages and verifies previous stores,
> while being continually migrated. This can fail due to a QEMU TCG
> physical memory dirty bitmap bug.

Good idea, but could we then please drop "continuous" test from 
selftest-migration.c again? ... having two common tests to exercise the 
continuous migration that take quite a bunch of seconds to finish sounds 
like a waste of time in the long run to me.

> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   common/memory-verify.c  | 48 +++++++++++++++++++++++++++++++++++++++++
>   powerpc/Makefile.common |  1 +
>   powerpc/memory-verify.c |  1 +
>   powerpc/unittests.cfg   |  7 ++++++
>   s390x/Makefile          |  1 +
>   s390x/memory-verify.c   |  1 +
>   s390x/unittests.cfg     |  6 ++++++
>   7 files changed, 65 insertions(+)
>   create mode 100644 common/memory-verify.c
>   create mode 120000 powerpc/memory-verify.c
>   create mode 120000 s390x/memory-verify.c
> 
> diff --git a/common/memory-verify.c b/common/memory-verify.c
> new file mode 100644
> index 000000000..7c4ec087b
> --- /dev/null
> +++ b/common/memory-verify.c
> @@ -0,0 +1,48 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Simple memory verification test, used to exercise dirty memory migration.
> + *
> + */
> +#include <libcflat.h>
> +#include <migrate.h>
> +#include <alloc.h>
> +#include <asm/page.h>
> +#include <asm/time.h>
> +
> +#define NR_PAGES 32
> +
> +int main(int argc, char **argv)
> +{
> +	void *mem = malloc(NR_PAGES*PAGE_SIZE);
> +	bool success = true;
> +	uint64_t ms;
> +	long i;
> +
> +	report_prefix_push("memory");
> +
> +	memset(mem, 0, NR_PAGES*PAGE_SIZE);
> +
> +	migrate_begin_continuous();
> +	ms = get_clock_ms();
> +	i = 0;
> +	do {
> +		int j;
> +
> +		for (j = 0; j < NR_PAGES*PAGE_SIZE; j += PAGE_SIZE) {
> +			if (*(volatile long *)(mem + j) != i) {
> +				success = false;
> +				goto out;
> +			}
> +			*(volatile long *)(mem + j) = i + 1;
> +		}
> +		i++;
> +	} while (get_clock_ms() - ms < 5000);

Maybe add a parameter so that the user can use different values for the 
runtime than always doing 5 seconds?

  Thomas

> +out:
> +	migrate_end_continuous();
> +
> +	report(success, "memory verification stress test");
> +
> +	report_prefix_pop();
> +
> +	return report_summary();
> +}

Re: [kvm-unit-tests PATCH 7/7] common: add memory dirtying vs migration test

[Nicholas Piggin]

On Mon Mar 4, 2024 at 4:22 PM AEST, Thomas Huth wrote:
> On 26/02/2024 10.38, Nicholas Piggin wrote:
> > This test stores to a bunch of pages and verifies previous stores,
> > while being continually migrated. This can fail due to a QEMU TCG
> > physical memory dirty bitmap bug.
>
> Good idea, but could we then please drop "continuous" test from 
> selftest-migration.c again? ... having two common tests to exercise the 
> continuous migration that take quite a bunch of seconds to finish sounds 
> like a waste of time in the long run to me.

Yeah if you like. I could shorten them up a bit. I did want to have
the selftests for just purely testing the harness with as little
"test" code as possible.

>
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >   common/memory-verify.c  | 48 +++++++++++++++++++++++++++++++++++++++++
> >   powerpc/Makefile.common |  1 +
> >   powerpc/memory-verify.c |  1 +
> >   powerpc/unittests.cfg   |  7 ++++++
> >   s390x/Makefile          |  1 +
> >   s390x/memory-verify.c   |  1 +
> >   s390x/unittests.cfg     |  6 ++++++
> >   7 files changed, 65 insertions(+)
> >   create mode 100644 common/memory-verify.c
> >   create mode 120000 powerpc/memory-verify.c
> >   create mode 120000 s390x/memory-verify.c
> > 
> > diff --git a/common/memory-verify.c b/common/memory-verify.c
> > new file mode 100644
> > index 000000000..7c4ec087b
> > --- /dev/null
> > +++ b/common/memory-verify.c
> > @@ -0,0 +1,48 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Simple memory verification test, used to exercise dirty memory migration.
> > + *
> > + */
> > +#include <libcflat.h>
> > +#include <migrate.h>
> > +#include <alloc.h>
> > +#include <asm/page.h>
> > +#include <asm/time.h>
> > +
> > +#define NR_PAGES 32
> > +
> > +int main(int argc, char **argv)
> > +{
> > +	void *mem = malloc(NR_PAGES*PAGE_SIZE);
> > +	bool success = true;
> > +	uint64_t ms;
> > +	long i;
> > +
> > +	report_prefix_push("memory");
> > +
> > +	memset(mem, 0, NR_PAGES*PAGE_SIZE);
> > +
> > +	migrate_begin_continuous();
> > +	ms = get_clock_ms();
> > +	i = 0;
> > +	do {
> > +		int j;
> > +
> > +		for (j = 0; j < NR_PAGES*PAGE_SIZE; j += PAGE_SIZE) {
> > +			if (*(volatile long *)(mem + j) != i) {
> > +				success = false;
> > +				goto out;
> > +			}
> > +			*(volatile long *)(mem + j) = i + 1;
> > +		}
> > +		i++;
> > +	} while (get_clock_ms() - ms < 5000);
>
> Maybe add a parameter so that the user can use different values for the 
> runtime than always doing 5 seconds?

Sure.

Thanks,
Nick

Re: [kvm-unit-tests PATCH 7/7] common: add memory dirtying vs migration test

[Thomas Huth]

On 05/03/2024 03.50, Nicholas Piggin wrote:
> On Mon Mar 4, 2024 at 4:22 PM AEST, Thomas Huth wrote:
>> On 26/02/2024 10.38, Nicholas Piggin wrote:
>>> This test stores to a bunch of pages and verifies previous stores,
>>> while being continually migrated. This can fail due to a QEMU TCG
>>> physical memory dirty bitmap bug.
>>
>> Good idea, but could we then please drop "continuous" test from
>> selftest-migration.c again? ... having two common tests to exercise the
>> continuous migration that take quite a bunch of seconds to finish sounds
>> like a waste of time in the long run to me.
> 
> Yeah if you like. I could shorten them up a bit. I did want to have
> the selftests for just purely testing the harness with as little
> "test" code as possible.

Ok, but then please shorten the selftest to ~ 2 seconds if possible, please.

  Thomas