public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
* [kvm-unit-tests PATCH v2 0/4] lib: add function to request migration
@ 2022-12-09 13:48 Nico Boehr
  2022-12-09 13:48 ` [kvm-unit-tests PATCH v2 1/4] " Nico Boehr
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Nico Boehr @ 2022-12-09 13:48 UTC (permalink / raw)
  To: kvm; +Cc: frankja, imbrenda, thuth, pbonzini, andrew.jones, lvivier

v1->v2:
---
* arm: commit message gib->gic (thanks Andrew)
* arm: remove unneeded {} (thanks Andrew)
* s390x: make patch less intrusive (thanks Claudio)

With this series, I pick up a suggestion Claudio has brought up in my
CMM-migration series[1].

Migration tests can ask migrate_cmd to migrate them to a new QEMU
process. Requesting migration and waiting for completion is hence a
common pattern which is repeated all over the code base. Add a function
which does all of that to avoid repetition.

Since migrate_cmd currently can only migrate exactly once, this function
is called migrate_once() and is a no-op when it has been called before.
This can simplify the control flow, especially when tests are skipped.

[1] https://lore.kernel.org/kvm/20221125154646.5974cb52@p-imbrenda/

Nico Boehr (4):
  lib: add function to request migration
  powerpc: use migrate_once() in migration tests
  s390x: use migrate_once() in migration tests
  arm: use migrate_once() in migration tests

 arm/Makefile.common     |  1 +
 powerpc/Makefile.common |  1 +
 s390x/Makefile          |  1 +
 lib/migrate.h           |  9 ++++++++
 lib/migrate.c           | 34 ++++++++++++++++++++++++++++
 arm/debug.c             | 17 +++++---------
 arm/gic.c               | 49 ++++++++++++-----------------------------
 powerpc/sprs.c          |  4 ++--
 s390x/migration-cmm.c   | 25 +++++++--------------
 s390x/migration-sck.c   |  4 ++--
 s390x/migration-skey.c  | 15 +++++--------
 s390x/migration.c       |  7 ++----
 12 files changed, 84 insertions(+), 83 deletions(-)
 create mode 100644 lib/migrate.h
 create mode 100644 lib/migrate.c

-- 
2.36.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [kvm-unit-tests PATCH v2 1/4] lib: add function to request migration
  2022-12-09 13:48 [kvm-unit-tests PATCH v2 0/4] lib: add function to request migration Nico Boehr
@ 2022-12-09 13:48 ` Nico Boehr
  2022-12-09 15:41   ` Claudio Imbrenda
  2022-12-09 13:48 ` [kvm-unit-tests PATCH v2 2/4] powerpc: use migrate_once() in migration tests Nico Boehr
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 7+ messages in thread
From: Nico Boehr @ 2022-12-09 13:48 UTC (permalink / raw)
  To: kvm; +Cc: frankja, imbrenda, thuth, pbonzini, andrew.jones, lvivier

Migration tests can ask migrate_cmd to migrate them to a new QEMU
process. Requesting migration and waiting for completion is hence a
common pattern which is repeated all over the code base. Add a function
which does all of that to avoid repeating the same pattern.

Since migrate_cmd currently can only migrate exactly once, this function
is called migrate_once() and is a no-op when it has been called before.
This can simplify the control flow, especially when tests are skipped.

Suggested-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
---
 lib/migrate.h |  9 +++++++++
 lib/migrate.c | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+)
 create mode 100644 lib/migrate.h
 create mode 100644 lib/migrate.c

diff --git a/lib/migrate.h b/lib/migrate.h
new file mode 100644
index 000000000000..3c94e6af761c
--- /dev/null
+++ b/lib/migrate.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Migration-related functions
+ *
+ * Copyright IBM Corp. 2022
+ * Author: Nico Boehr <nrb@linux.ibm.com>
+ */
+
+void migrate_once(void);
diff --git a/lib/migrate.c b/lib/migrate.c
new file mode 100644
index 000000000000..527e63ae189b
--- /dev/null
+++ b/lib/migrate.c
@@ -0,0 +1,34 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Migration-related functions
+ *
+ * Copyright IBM Corp. 2022
+ * Author: Nico Boehr <nrb@linux.ibm.com>
+ */
+#include <libcflat.h>
+#include "migrate.h"
+
+/* static for now since we only support migrating exactly once per test. */
+static void migrate(void)
+{
+	puts("Now migrate the VM, then press a key to continue...\n");
+	(void)getchar();
+	report_info("Migration complete");
+}
+
+/*
+ * Initiate migration and wait for it to complete.
+ * If this function is called more than once, it is a no-op.
+ * Since migrate_cmd can only migrate exactly once this function can
+ * simplify the control flow, especially when skipping tests.
+ */
+void migrate_once(void)
+{
+	static bool migrated;
+
+	if (migrated)
+		return;
+
+	migrated = true;
+	migrate();
+}
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [kvm-unit-tests PATCH v2 2/4] powerpc: use migrate_once() in migration tests
  2022-12-09 13:48 [kvm-unit-tests PATCH v2 0/4] lib: add function to request migration Nico Boehr
  2022-12-09 13:48 ` [kvm-unit-tests PATCH v2 1/4] " Nico Boehr
@ 2022-12-09 13:48 ` Nico Boehr
  2022-12-09 13:48 ` [kvm-unit-tests PATCH v2 3/4] s390x: " Nico Boehr
  2022-12-09 13:48 ` [kvm-unit-tests PATCH v2 4/4] arm: " Nico Boehr
  3 siblings, 0 replies; 7+ messages in thread
From: Nico Boehr @ 2022-12-09 13:48 UTC (permalink / raw)
  To: kvm; +Cc: frankja, imbrenda, thuth, pbonzini, andrew.jones, lvivier

Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 powerpc/Makefile.common | 1 +
 powerpc/sprs.c          | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
index 12c280c15fff..8ce00340b6be 100644
--- a/powerpc/Makefile.common
+++ b/powerpc/Makefile.common
@@ -36,6 +36,7 @@ cflatobjs += lib/getchar.o
 cflatobjs += lib/alloc_phys.o
 cflatobjs += lib/alloc.o
 cflatobjs += lib/devicetree.o
+cflatobjs += lib/migrate.o
 cflatobjs += lib/powerpc/io.o
 cflatobjs += lib/powerpc/hcall.o
 cflatobjs += lib/powerpc/setup.o
diff --git a/powerpc/sprs.c b/powerpc/sprs.c
index d3c8780e8376..5cc1cd16cfda 100644
--- a/powerpc/sprs.c
+++ b/powerpc/sprs.c
@@ -21,6 +21,7 @@
  */
 #include <libcflat.h>
 #include <util.h>
+#include <migrate.h>
 #include <alloc.h>
 #include <asm/handlers.h>
 #include <asm/hcall.h>
@@ -285,8 +286,7 @@ int main(int argc, char **argv)
 	get_sprs(before);
 
 	if (pause) {
-		puts("Now migrate the VM, then press a key to continue...\n");
-		(void) getchar();
+		migrate_once();
 	} else {
 		puts("Sleeping...\n");
 		handle_exception(0x900, &dec_except_handler, &decr);
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [kvm-unit-tests PATCH v2 3/4] s390x: use migrate_once() in migration tests
  2022-12-09 13:48 [kvm-unit-tests PATCH v2 0/4] lib: add function to request migration Nico Boehr
  2022-12-09 13:48 ` [kvm-unit-tests PATCH v2 1/4] " Nico Boehr
  2022-12-09 13:48 ` [kvm-unit-tests PATCH v2 2/4] powerpc: use migrate_once() in migration tests Nico Boehr
@ 2022-12-09 13:48 ` Nico Boehr
  2022-12-09 15:43   ` Claudio Imbrenda
  2022-12-09 13:48 ` [kvm-unit-tests PATCH v2 4/4] arm: " Nico Boehr
  3 siblings, 1 reply; 7+ messages in thread
From: Nico Boehr @ 2022-12-09 13:48 UTC (permalink / raw)
  To: kvm; +Cc: frankja, imbrenda, thuth, pbonzini, andrew.jones, lvivier

migrate_once() can simplify the control flow in migration-skey and
migration-cmm.

Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
---
 s390x/Makefile         |  1 +
 s390x/migration-cmm.c  | 25 ++++++++-----------------
 s390x/migration-sck.c  |  4 ++--
 s390x/migration-skey.c | 15 +++++----------
 s390x/migration.c      |  7 ++-----
 5 files changed, 18 insertions(+), 34 deletions(-)

diff --git a/s390x/Makefile b/s390x/Makefile
index bf1504f9d58c..52a9d821974e 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -85,6 +85,7 @@ cflatobjs += lib/alloc_page.o
 cflatobjs += lib/vmalloc.o
 cflatobjs += lib/alloc_phys.o
 cflatobjs += lib/getchar.o
+cflatobjs += lib/migrate.o
 cflatobjs += lib/s390x/io.o
 cflatobjs += lib/s390x/stack.o
 cflatobjs += lib/s390x/sclp.o
diff --git a/s390x/migration-cmm.c b/s390x/migration-cmm.c
index aa7910ca76bf..eef45f5f8fb7 100644
--- a/s390x/migration-cmm.c
+++ b/s390x/migration-cmm.c
@@ -9,6 +9,7 @@
  */
 
 #include <libcflat.h>
+#include <migrate.h>
 #include <asm/interrupt.h>
 #include <asm/page.h>
 #include <asm/cmm.h>
@@ -39,8 +40,7 @@ static void test_migration(void)
 		essa(ESSA_SET_POT_VOLATILE, (unsigned long)pagebuf[i + 3]);
 	}
 
-	puts("Please migrate me, then press return\n");
-	(void)getchar();
+	migrate_once();
 
 	for (i = 0; i < NUM_PAGES; i++) {
 		actual_state = essa(ESSA_GET_STATE, (unsigned long)pagebuf[i]);
@@ -53,25 +53,16 @@ static void test_migration(void)
 
 int main(void)
 {
-	bool has_essa = check_essa_available();
-
 	report_prefix_push("migration-cmm");
-	if (!has_essa) {
-		report_skip("ESSA is not available");
-
-		/*
-		 * If we just exit and don't ask migrate_cmd to migrate us, it
-		 * will just hang forever. Hence, also ask for migration when we
-		 * skip this test alltogether.
-		 */
-		puts("Please migrate me, then press return\n");
-		(void)getchar();
 
-		goto done;
+	if (!check_essa_available()) {
+		report_skip("ESSA is not available");
+	} else {
+		test_migration();
 	}
 
-	test_migration();
-done:
+	migrate_once();
+
 	report_prefix_pop();
 	return report_summary();
 }
diff --git a/s390x/migration-sck.c b/s390x/migration-sck.c
index 2d9a195ab4c4..2a9c87071643 100644
--- a/s390x/migration-sck.c
+++ b/s390x/migration-sck.c
@@ -9,6 +9,7 @@
  */
 
 #include <libcflat.h>
+#include <migrate.h>
 #include <asm/time.h>
 
 static void test_sck_migration(void)
@@ -30,8 +31,7 @@ static void test_sck_migration(void)
 	report(!cc, "clock running after set");
 	report(now_after_set >= time_to_set, "TOD clock value is larger than what has been set");
 
-	puts("Please migrate me, then press return\n");
-	(void)getchar();
+	migrate_once();
 
 	cc = stckf(&now_after_migration);
 	report(!cc, "clock still set");
diff --git a/s390x/migration-skey.c b/s390x/migration-skey.c
index b7bd82581abe..438a4be95702 100644
--- a/s390x/migration-skey.c
+++ b/s390x/migration-skey.c
@@ -9,6 +9,7 @@
  */
 
 #include <libcflat.h>
+#include <migrate.h>
 #include <asm/facility.h>
 #include <asm/page.h>
 #include <asm/mem.h>
@@ -35,8 +36,7 @@ static void test_migration(void)
 		set_storage_key(pagebuf[i], key_to_set, 1);
 	}
 
-	puts("Please migrate me, then press return\n");
-	(void)getchar();
+	migrate_once();
 
 	for (i = 0; i < NUM_PAGES; i++) {
 		actual_key.val = get_storage_key(pagebuf[i]);
@@ -64,20 +64,15 @@ static void test_migration(void)
 int main(void)
 {
 	report_prefix_push("migration-skey");
+
 	if (test_facility(169)) {
 		report_skip("storage key removal facility is active");
-
-		/*
-		 * If we just exit and don't ask migrate_cmd to migrate us, it
-		 * will just hang forever. Hence, also ask for migration when we
-		 * skip this test altogether.
-		 */
-		puts("Please migrate me, then press return\n");
-		(void)getchar();
 	} else {
 		test_migration();
 	}
 
+	migrate_once();
+
 	report_prefix_pop();
 	return report_summary();
 }
diff --git a/s390x/migration.c b/s390x/migration.c
index a45296374cd8..fe6ea8369edb 100644
--- a/s390x/migration.c
+++ b/s390x/migration.c
@@ -8,6 +8,7 @@
  *  Nico Boehr <nrb@linux.ibm.com>
  */
 #include <libcflat.h>
+#include <migrate.h>
 #include <asm/arch_def.h>
 #include <asm/vector.h>
 #include <asm/barrier.h>
@@ -178,11 +179,7 @@ int main(void)
 		mb();
 	flag_thread_complete = 0;
 
-	/* ask migrate_cmd to migrate (it listens for 'migrate') */
-	puts("Please migrate me, then press return\n");
-
-	/* wait for migration to finish, we will read a newline */
-	(void)getchar();
+	migrate_once();
 
 	flag_migration_complete = 1;
 
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [kvm-unit-tests PATCH v2 4/4] arm: use migrate_once() in migration tests
  2022-12-09 13:48 [kvm-unit-tests PATCH v2 0/4] lib: add function to request migration Nico Boehr
                   ` (2 preceding siblings ...)
  2022-12-09 13:48 ` [kvm-unit-tests PATCH v2 3/4] s390x: " Nico Boehr
@ 2022-12-09 13:48 ` Nico Boehr
  3 siblings, 0 replies; 7+ messages in thread
From: Nico Boehr @ 2022-12-09 13:48 UTC (permalink / raw)
  To: kvm; +Cc: frankja, imbrenda, thuth, pbonzini, andrew.jones, lvivier

Some tests already shipped with their own do_migrate() function, remove
it and instead use the new migrate_once() function. The control flow in
the gic tests can be simplified due to migrate_once().

Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
Reviewed-by: Andrew Jones <andrew.jones@linux.dev>
---
 arm/Makefile.common |  1 +
 arm/debug.c         | 17 +++++-----------
 arm/gic.c           | 49 +++++++++++++--------------------------------
 3 files changed, 20 insertions(+), 47 deletions(-)

diff --git a/arm/Makefile.common b/arm/Makefile.common
index 38385e0c558e..1bbec64f2342 100644
--- a/arm/Makefile.common
+++ b/arm/Makefile.common
@@ -38,6 +38,7 @@ cflatobjs += lib/alloc_page.o
 cflatobjs += lib/vmalloc.o
 cflatobjs += lib/alloc.o
 cflatobjs += lib/devicetree.o
+cflatobjs += lib/migrate.o
 cflatobjs += lib/pci.o
 cflatobjs += lib/pci-host-generic.o
 cflatobjs += lib/pci-testdev.o
diff --git a/arm/debug.c b/arm/debug.c
index e9f805632db7..b3e9749c598f 100644
--- a/arm/debug.c
+++ b/arm/debug.c
@@ -1,4 +1,5 @@
 #include <libcflat.h>
+#include <migrate.h>
 #include <errata.h>
 #include <asm/setup.h>
 #include <asm/processor.h>
@@ -257,13 +258,6 @@ static void reset_debug_state(void)
 	isb();
 }
 
-static void do_migrate(void)
-{
-	puts("Now migrate the VM, then press a key to continue...\n");
-	(void)getchar();
-	report_info("Migration complete");
-}
-
 static noinline void test_hw_bp(bool migrate)
 {
 	extern unsigned char hw_bp0;
@@ -291,7 +285,7 @@ static noinline void test_hw_bp(bool migrate)
 	isb();
 
 	if (migrate) {
-		do_migrate();
+		migrate_once();
 		report(num_bp == get_num_hw_bp(), "brps match after migrate");
 	}
 
@@ -335,7 +329,7 @@ static noinline void test_wp(bool migrate)
 	isb();
 
 	if (migrate) {
-		do_migrate();
+		migrate_once();
 		report(num_wp == get_num_wp(), "wrps match after migrate");
 	}
 
@@ -368,9 +362,8 @@ static noinline void test_ss(bool migrate)
 	write_sysreg(mdscr, mdscr_el1);
 	isb();
 
-	if (migrate) {
-		do_migrate();
-	}
+	if (migrate)
+		migrate_once();
 
 	asm volatile("msr daifclr, #8");
 
diff --git a/arm/gic.c b/arm/gic.c
index 60457e29e73a..c950b0d1597c 100644
--- a/arm/gic.c
+++ b/arm/gic.c
@@ -12,6 +12,7 @@
  * This work is licensed under the terms of the GNU LGPL, version 2.
  */
 #include <libcflat.h>
+#include <migrate.h>
 #include <errata.h>
 #include <asm/setup.h>
 #include <asm/processor.h>
@@ -779,23 +780,15 @@ static void test_its_trigger(void)
 static void test_its_migration(void)
 {
 	struct its_device *dev2, *dev7;
-	bool test_skipped = false;
 	cpumask_t mask;
 
-	if (its_setup1()) {
-		test_skipped = true;
-		goto do_migrate;
-	}
+	if (its_setup1())
+		return;
 
 	dev2 = its_get_device(2);
 	dev7 = its_get_device(7);
 
-do_migrate:
-	puts("Now migrate the VM, then press a key to continue...\n");
-	(void)getchar();
-	report_info("Migration complete");
-	if (test_skipped)
-		return;
+	migrate_once();
 
 	stats_reset();
 	cpumask_clear(&mask);
@@ -822,21 +815,17 @@ static void test_migrate_unmapped_collection(void)
 {
 	struct its_collection *col = NULL;
 	struct its_device *dev2 = NULL, *dev7 = NULL;
-	bool test_skipped = false;
 	cpumask_t mask;
 	int pe0 = 0;
 	u8 config;
 
-	if (its_setup1()) {
-		test_skipped = true;
-		goto do_migrate;
-	}
+	if (its_setup1())
+		return;
 
 	if (!errata(ERRATA_UNMAPPED_COLLECTIONS)) {
 		report_skip("Skipping test, as this test hangs without the fix. "
 			    "Set %s=y to enable.", ERRATA_UNMAPPED_COLLECTIONS);
-		test_skipped = true;
-		goto do_migrate;
+		return;
 	}
 
 	col = its_create_collection(pe0, pe0);
@@ -847,12 +836,7 @@ static void test_migrate_unmapped_collection(void)
 	its_send_mapti(dev2, 8192, 0, col);
 	gicv3_lpi_set_config(8192, LPI_PROP_DEFAULT);
 
-do_migrate:
-	puts("Now migrate the VM, then press a key to continue...\n");
-	(void)getchar();
-	report_info("Migration complete");
-	if (test_skipped)
-		return;
+	migrate_once();
 
 	/* on the destination, map the collection */
 	its_send_mapc(col, true);
@@ -887,15 +871,12 @@ static void test_its_pending_migration(void)
 	struct its_collection *collection[2];
 	int *expected = calloc(nr_cpus, sizeof(int));
 	int pe0 = nr_cpus - 1, pe1 = nr_cpus - 2;
-	bool test_skipped = false;
 	u64 pendbaser;
 	void *ptr;
 	int i;
 
-	if (its_prerequisites(4)) {
-		test_skipped = true;
-		goto do_migrate;
-	}
+	if (its_prerequisites(4))
+		return;
 
 	dev = its_create_device(2 /* dev id */, 8 /* nb_ites */);
 	its_send_mapd(dev, true);
@@ -942,12 +923,7 @@ static void test_its_pending_migration(void)
 	gicv3_lpi_rdist_enable(pe0);
 	gicv3_lpi_rdist_enable(pe1);
 
-do_migrate:
-	puts("Now migrate the VM, then press a key to continue...\n");
-	(void)getchar();
-	report_info("Migration complete");
-	if (test_skipped)
-		return;
+	migrate_once();
 
 	/* let's wait for the 256 LPIs to be handled */
 	mdelay(1000);
@@ -994,14 +970,17 @@ 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]);
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [kvm-unit-tests PATCH v2 1/4] lib: add function to request migration
  2022-12-09 13:48 ` [kvm-unit-tests PATCH v2 1/4] " Nico Boehr
@ 2022-12-09 15:41   ` Claudio Imbrenda
  0 siblings, 0 replies; 7+ messages in thread
From: Claudio Imbrenda @ 2022-12-09 15:41 UTC (permalink / raw)
  To: Nico Boehr; +Cc: kvm, frankja, thuth, pbonzini, andrew.jones, lvivier

On Fri,  9 Dec 2022 14:48:06 +0100
Nico Boehr <nrb@linux.ibm.com> wrote:

> Migration tests can ask migrate_cmd to migrate them to a new QEMU
> process. Requesting migration and waiting for completion is hence a
> common pattern which is repeated all over the code base. Add a function
> which does all of that to avoid repeating the same pattern.
> 
> Since migrate_cmd currently can only migrate exactly once, this function
> is called migrate_once() and is a no-op when it has been called before.
> This can simplify the control flow, especially when tests are skipped.
> 
> Suggested-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> Signed-off-by: Nico Boehr <nrb@linux.ibm.com>

Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

> ---
>  lib/migrate.h |  9 +++++++++
>  lib/migrate.c | 34 ++++++++++++++++++++++++++++++++++
>  2 files changed, 43 insertions(+)
>  create mode 100644 lib/migrate.h
>  create mode 100644 lib/migrate.c
> 
> diff --git a/lib/migrate.h b/lib/migrate.h
> new file mode 100644
> index 000000000000..3c94e6af761c
> --- /dev/null
> +++ b/lib/migrate.h
> @@ -0,0 +1,9 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Migration-related functions
> + *
> + * Copyright IBM Corp. 2022
> + * Author: Nico Boehr <nrb@linux.ibm.com>
> + */
> +
> +void migrate_once(void);
> diff --git a/lib/migrate.c b/lib/migrate.c
> new file mode 100644
> index 000000000000..527e63ae189b
> --- /dev/null
> +++ b/lib/migrate.c
> @@ -0,0 +1,34 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Migration-related functions
> + *
> + * Copyright IBM Corp. 2022
> + * Author: Nico Boehr <nrb@linux.ibm.com>
> + */
> +#include <libcflat.h>
> +#include "migrate.h"
> +
> +/* static for now since we only support migrating exactly once per test. */
> +static void migrate(void)
> +{
> +	puts("Now migrate the VM, then press a key to continue...\n");
> +	(void)getchar();
> +	report_info("Migration complete");
> +}
> +
> +/*
> + * Initiate migration and wait for it to complete.
> + * If this function is called more than once, it is a no-op.
> + * Since migrate_cmd can only migrate exactly once this function can
> + * simplify the control flow, especially when skipping tests.
> + */
> +void migrate_once(void)
> +{
> +	static bool migrated;
> +
> +	if (migrated)
> +		return;
> +
> +	migrated = true;
> +	migrate();
> +}


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [kvm-unit-tests PATCH v2 3/4] s390x: use migrate_once() in migration tests
  2022-12-09 13:48 ` [kvm-unit-tests PATCH v2 3/4] s390x: " Nico Boehr
@ 2022-12-09 15:43   ` Claudio Imbrenda
  0 siblings, 0 replies; 7+ messages in thread
From: Claudio Imbrenda @ 2022-12-09 15:43 UTC (permalink / raw)
  To: Nico Boehr; +Cc: kvm, frankja, thuth, pbonzini, andrew.jones, lvivier

On Fri,  9 Dec 2022 14:48:08 +0100
Nico Boehr <nrb@linux.ibm.com> wrote:

> migrate_once() can simplify the control flow in migration-skey and
> migration-cmm.
> 
> Signed-off-by: Nico Boehr <nrb@linux.ibm.com>

looks very good, just some small nits

> ---
>  s390x/Makefile         |  1 +
>  s390x/migration-cmm.c  | 25 ++++++++-----------------
>  s390x/migration-sck.c  |  4 ++--
>  s390x/migration-skey.c | 15 +++++----------
>  s390x/migration.c      |  7 ++-----
>  5 files changed, 18 insertions(+), 34 deletions(-)
> 
> diff --git a/s390x/Makefile b/s390x/Makefile
> index bf1504f9d58c..52a9d821974e 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -85,6 +85,7 @@ cflatobjs += lib/alloc_page.o
>  cflatobjs += lib/vmalloc.o
>  cflatobjs += lib/alloc_phys.o
>  cflatobjs += lib/getchar.o
> +cflatobjs += lib/migrate.o
>  cflatobjs += lib/s390x/io.o
>  cflatobjs += lib/s390x/stack.o
>  cflatobjs += lib/s390x/sclp.o
> diff --git a/s390x/migration-cmm.c b/s390x/migration-cmm.c
> index aa7910ca76bf..eef45f5f8fb7 100644
> --- a/s390x/migration-cmm.c
> +++ b/s390x/migration-cmm.c
> @@ -9,6 +9,7 @@
>   */
>  
>  #include <libcflat.h>
> +#include <migrate.h>
>  #include <asm/interrupt.h>
>  #include <asm/page.h>
>  #include <asm/cmm.h>
> @@ -39,8 +40,7 @@ static void test_migration(void)
>  		essa(ESSA_SET_POT_VOLATILE, (unsigned long)pagebuf[i + 3]);
>  	}
>  
> -	puts("Please migrate me, then press return\n");
> -	(void)getchar();
> +	migrate_once();
>  
>  	for (i = 0; i < NUM_PAGES; i++) {
>  		actual_state = essa(ESSA_GET_STATE, (unsigned long)pagebuf[i]);
> @@ -53,25 +53,16 @@ static void test_migration(void)
>  
>  int main(void)
>  {
> -	bool has_essa = check_essa_available();
> -
>  	report_prefix_push("migration-cmm");
> -	if (!has_essa) {
> -		report_skip("ESSA is not available");
> -
> -		/*
> -		 * If we just exit and don't ask migrate_cmd to migrate us, it
> -		 * will just hang forever. Hence, also ask for migration when we
> -		 * skip this test alltogether.
> -		 */
> -		puts("Please migrate me, then press return\n");
> -		(void)getchar();
>  
> -		goto done;
> +	if (!check_essa_available()) {
> +		report_skip("ESSA is not available");
> +	} else {
> +		test_migration();
>  	}

I think you don't need the {} any longer here?

>  
> -	test_migration();
> -done:
> +	migrate_once();
> +
>  	report_prefix_pop();
>  	return report_summary();
>  }
> diff --git a/s390x/migration-sck.c b/s390x/migration-sck.c
> index 2d9a195ab4c4..2a9c87071643 100644
> --- a/s390x/migration-sck.c
> +++ b/s390x/migration-sck.c
> @@ -9,6 +9,7 @@
>   */
>  
>  #include <libcflat.h>
> +#include <migrate.h>
>  #include <asm/time.h>
>  
>  static void test_sck_migration(void)
> @@ -30,8 +31,7 @@ static void test_sck_migration(void)
>  	report(!cc, "clock running after set");
>  	report(now_after_set >= time_to_set, "TOD clock value is larger than what has been set");
>  
> -	puts("Please migrate me, then press return\n");
> -	(void)getchar();
> +	migrate_once();
>  
>  	cc = stckf(&now_after_migration);
>  	report(!cc, "clock still set");
> diff --git a/s390x/migration-skey.c b/s390x/migration-skey.c
> index b7bd82581abe..438a4be95702 100644
> --- a/s390x/migration-skey.c
> +++ b/s390x/migration-skey.c
> @@ -9,6 +9,7 @@
>   */
>  
>  #include <libcflat.h>
> +#include <migrate.h>
>  #include <asm/facility.h>
>  #include <asm/page.h>
>  #include <asm/mem.h>
> @@ -35,8 +36,7 @@ static void test_migration(void)
>  		set_storage_key(pagebuf[i], key_to_set, 1);
>  	}
>  
> -	puts("Please migrate me, then press return\n");
> -	(void)getchar();
> +	migrate_once();
>  
>  	for (i = 0; i < NUM_PAGES; i++) {
>  		actual_key.val = get_storage_key(pagebuf[i]);
> @@ -64,20 +64,15 @@ static void test_migration(void)
>  int main(void)
>  {
>  	report_prefix_push("migration-skey");
> +
>  	if (test_facility(169)) {
>  		report_skip("storage key removal facility is active");
> -
> -		/*
> -		 * If we just exit and don't ask migrate_cmd to migrate us, it
> -		 * will just hang forever. Hence, also ask for migration when we
> -		 * skip this test altogether.
> -		 */
> -		puts("Please migrate me, then press return\n");
> -		(void)getchar();
>  	} else {
>  		test_migration();
>  	}

same here {} ^

>  
> +	migrate_once();
> +
>  	report_prefix_pop();
>  	return report_summary();
>  }
> diff --git a/s390x/migration.c b/s390x/migration.c
> index a45296374cd8..fe6ea8369edb 100644
> --- a/s390x/migration.c
> +++ b/s390x/migration.c
> @@ -8,6 +8,7 @@
>   *  Nico Boehr <nrb@linux.ibm.com>
>   */
>  #include <libcflat.h>
> +#include <migrate.h>
>  #include <asm/arch_def.h>
>  #include <asm/vector.h>
>  #include <asm/barrier.h>
> @@ -178,11 +179,7 @@ int main(void)
>  		mb();
>  	flag_thread_complete = 0;
>  
> -	/* ask migrate_cmd to migrate (it listens for 'migrate') */
> -	puts("Please migrate me, then press return\n");
> -
> -	/* wait for migration to finish, we will read a newline */
> -	(void)getchar();
> +	migrate_once();
>  
>  	flag_migration_complete = 1;
>  


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-12-09 15:43 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-09 13:48 [kvm-unit-tests PATCH v2 0/4] lib: add function to request migration Nico Boehr
2022-12-09 13:48 ` [kvm-unit-tests PATCH v2 1/4] " Nico Boehr
2022-12-09 15:41   ` Claudio Imbrenda
2022-12-09 13:48 ` [kvm-unit-tests PATCH v2 2/4] powerpc: use migrate_once() in migration tests Nico Boehr
2022-12-09 13:48 ` [kvm-unit-tests PATCH v2 3/4] s390x: " Nico Boehr
2022-12-09 15:43   ` Claudio Imbrenda
2022-12-09 13:48 ` [kvm-unit-tests PATCH v2 4/4] arm: " Nico Boehr

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox