* [kvm-unit-tests PATCH v1] s390x: migration: don't run tests when facilities are not there
@ 2022-05-05 10:27 Nico Boehr
2022-05-05 11:54 ` Claudio Imbrenda
0 siblings, 1 reply; 3+ messages in thread
From: Nico Boehr @ 2022-05-05 10:27 UTC (permalink / raw)
To: kvm, linux-s390; +Cc: frankja, imbrenda, thuth
In the new migration tests, there was no check whether guarded-storage and
vector facilities are there. This may lead to crashes, especially under TCG
where guarded-storage is not supported.
Add checks for the respective facilities. Since it is possible neither
guarded-storage nor vector are there, add an additional report_pass() such that
least one PASS is reported.
Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
---
s390x/migration.c | 154 +++++++++++++++++++++++++++-------------------
1 file changed, 90 insertions(+), 64 deletions(-)
diff --git a/s390x/migration.c b/s390x/migration.c
index 2f31fc53bf68..a45296374cd8 100644
--- a/s390x/migration.c
+++ b/s390x/migration.c
@@ -11,6 +11,7 @@
#include <asm/arch_def.h>
#include <asm/vector.h>
#include <asm/barrier.h>
+#include <asm/facility.h>
#include <gs.h>
#include <bitops.h>
#include <smp.h>
@@ -50,6 +51,16 @@ static void check_gs_regs(void)
report_prefix_pop();
}
+static bool have_vector_facility(void)
+{
+ return test_facility(129);
+}
+
+static bool have_guarded_storage_facility(void)
+{
+ return test_facility(133);
+}
+
static void test_func(void)
{
uint8_t expected_vec_contents[VEC_REGISTER_NUM][VEC_REGISTER_SIZE];
@@ -58,74 +69,89 @@ static void test_func(void)
int i;
int vec_result = 0;
- for (i = 0; i < VEC_REGISTER_NUM; i++) {
- vec_reg = &expected_vec_contents[i][0];
- /* i+1 to avoid zero content */
- memset(vec_reg, i + 1, VEC_REGISTER_SIZE);
+ if (have_guarded_storage_facility()) {
+ ctl_set_bit(2, CTL2_GUARDED_STORAGE);
+
+ write_gs_regs();
}
- ctl_set_bit(0, CTL0_VECTOR);
- ctl_set_bit(2, CTL2_GUARDED_STORAGE);
-
- write_gs_regs();
-
- /*
- * It is important loading the vector/floating point registers and
- * comparing their contents occurs in the same inline assembly block.
- * Otherwise, the compiler is allowed to re-use the registers for
- * something else in between.
- * For this very reason, this also runs on a second CPU, so all the
- * complex console stuff can be done in C on the first CPU and here we
- * just need to wait for it to set the flag.
- */
- asm inline(
- " .machine z13\n"
- /* load vector registers: vlm handles at most 16 registers at a time */
- " vlm 0,15, 0(%[expected_vec_reg])\n"
- " vlm 16,31, 256(%[expected_vec_reg])\n"
- /* inform CPU0 we are done, it will request migration */
- " mvhi %[flag_thread_complete], 1\n"
- /* wait for migration to finish */
- "0: clfhsi %[flag_migration_complete], 1\n"
- " jnz 0b\n"
- /*
- * store vector register contents in actual_vec_reg: vstm
- * handles at most 16 registers at a time
- */
- " vstm 0,15, 0(%[actual_vec_reg])\n"
- " vstm 16,31, 256(%[actual_vec_reg])\n"
+ if (have_vector_facility()) {
+ for (i = 0; i < VEC_REGISTER_NUM; i++) {
+ vec_reg = &expected_vec_contents[i][0];
+ /* i+1 to avoid zero content */
+ memset(vec_reg, i + 1, VEC_REGISTER_SIZE);
+ }
+
+ ctl_set_bit(0, CTL0_VECTOR);
+
/*
- * compare the contents in expected_vec_reg with actual_vec_reg:
- * clc handles at most 256 bytes at a time
+ * It is important loading the vector/floating point registers and
+ * comparing their contents occurs in the same inline assembly block.
+ * Otherwise, the compiler is allowed to re-use the registers for
+ * something else in between.
+ * For this very reason, this also runs on a second CPU, so all the
+ * complex console stuff can be done in C on the first CPU and here we
+ * just need to wait for it to set the flag.
*/
- " clc 0(256, %[expected_vec_reg]), 0(%[actual_vec_reg])\n"
- " jnz 1f\n"
- " clc 256(256, %[expected_vec_reg]), 256(%[actual_vec_reg])\n"
- " jnz 1f\n"
- /* success */
- " mvhi %[vec_result], 1\n"
- "1:"
- :
- : [expected_vec_reg] "a"(expected_vec_contents),
- [actual_vec_reg] "a"(actual_vec_contents),
- [flag_thread_complete] "Q"(flag_thread_complete),
- [flag_migration_complete] "Q"(flag_migration_complete),
- [vec_result] "Q"(vec_result)
- : "v0", "v1", "v2", "v3", "v4", "v5", "v6", "v7", "v8", "v9",
- "v10", "v11", "v12", "v13", "v14", "v15", "v16", "v17", "v18",
- "v19", "v20", "v21", "v22", "v23", "v24", "v25", "v26", "v27",
- "v28", "v29", "v30", "v31", "cc", "memory"
- );
-
- report(vec_result, "vector contents match");
-
- check_gs_regs();
-
- report(stctg(0) & BIT(CTL0_VECTOR), "ctl0 vector bit set");
- report(stctg(2) & BIT(CTL2_GUARDED_STORAGE), "ctl2 guarded-storage bit set");
-
- ctl_clear_bit(0, CTL0_VECTOR);
- ctl_clear_bit(2, CTL2_GUARDED_STORAGE);
+ asm inline(
+ " .machine z13\n"
+ /* load vector registers: vlm handles at most 16 registers at a time */
+ " vlm 0,15, 0(%[expected_vec_reg])\n"
+ " vlm 16,31, 256(%[expected_vec_reg])\n"
+ /* inform CPU0 we are done, it will request migration */
+ " mvhi %[flag_thread_complete], 1\n"
+ /* wait for migration to finish */
+ "0: clfhsi %[flag_migration_complete], 1\n"
+ " jnz 0b\n"
+ /*
+ * store vector register contents in actual_vec_reg: vstm
+ * handles at most 16 registers at a time
+ */
+ " vstm 0,15, 0(%[actual_vec_reg])\n"
+ " vstm 16,31, 256(%[actual_vec_reg])\n"
+ /*
+ * compare the contents in expected_vec_reg with actual_vec_reg:
+ * clc handles at most 256 bytes at a time
+ */
+ " clc 0(256, %[expected_vec_reg]), 0(%[actual_vec_reg])\n"
+ " jnz 1f\n"
+ " clc 256(256, %[expected_vec_reg]), 256(%[actual_vec_reg])\n"
+ " jnz 1f\n"
+ /* success */
+ " mvhi %[vec_result], 1\n"
+ "1:"
+ :
+ : [expected_vec_reg] "a"(expected_vec_contents),
+ [actual_vec_reg] "a"(actual_vec_contents),
+ [flag_thread_complete] "Q"(flag_thread_complete),
+ [flag_migration_complete] "Q"(flag_migration_complete),
+ [vec_result] "Q"(vec_result)
+ : "v0", "v1", "v2", "v3", "v4", "v5", "v6", "v7", "v8", "v9",
+ "v10", "v11", "v12", "v13", "v14", "v15", "v16", "v17", "v18",
+ "v19", "v20", "v21", "v22", "v23", "v24", "v25", "v26", "v27",
+ "v28", "v29", "v30", "v31", "cc", "memory"
+ );
+
+ report(vec_result, "vector contents match");
+
+ report(stctg(0) & BIT(CTL0_VECTOR), "ctl0 vector bit set");
+
+ ctl_clear_bit(0, CTL0_VECTOR);
+ } else {
+ flag_thread_complete = 1;
+ while(!flag_migration_complete)
+ mb();
+ }
+
+ report_pass("Migrated");
+
+ if (have_guarded_storage_facility()) {
+ check_gs_regs();
+
+ report(stctg(2) & BIT(CTL2_GUARDED_STORAGE), "ctl2 guarded-storage bit set");
+
+ ctl_clear_bit(2, CTL2_GUARDED_STORAGE);
+ }
flag_thread_complete = 1;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [kvm-unit-tests PATCH v1] s390x: migration: don't run tests when facilities are not there
2022-05-05 10:27 [kvm-unit-tests PATCH v1] s390x: migration: don't run tests when facilities are not there Nico Boehr
@ 2022-05-05 11:54 ` Claudio Imbrenda
2022-05-05 12:02 ` Nico Boehr
0 siblings, 1 reply; 3+ messages in thread
From: Claudio Imbrenda @ 2022-05-05 11:54 UTC (permalink / raw)
To: Nico Boehr; +Cc: kvm, linux-s390, frankja, thuth
On Thu, 5 May 2022 12:27:05 +0200
Nico Boehr <nrb@linux.ibm.com> wrote:
> In the new migration tests, there was no check whether guarded-storage and
> vector facilities are there. This may lead to crashes, especially under TCG
> where guarded-storage is not supported.
>
> Add checks for the respective facilities. Since it is possible neither
> guarded-storage nor vector are there, add an additional report_pass() such that
> least one PASS is reported.
>
> Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
Ooops, we missed this when reviewing the other patch
I think this patch should actually be folded in the previous one, I'll
do that unless there are objections
> ---
> s390x/migration.c | 154 +++++++++++++++++++++++++++-------------------
> 1 file changed, 90 insertions(+), 64 deletions(-)
>
> diff --git a/s390x/migration.c b/s390x/migration.c
> index 2f31fc53bf68..a45296374cd8 100644
> --- a/s390x/migration.c
> +++ b/s390x/migration.c
> @@ -11,6 +11,7 @@
> #include <asm/arch_def.h>
> #include <asm/vector.h>
> #include <asm/barrier.h>
> +#include <asm/facility.h>
> #include <gs.h>
> #include <bitops.h>
> #include <smp.h>
> @@ -50,6 +51,16 @@ static void check_gs_regs(void)
> report_prefix_pop();
> }
>
> +static bool have_vector_facility(void)
> +{
> + return test_facility(129);
> +}
> +
> +static bool have_guarded_storage_facility(void)
> +{
> + return test_facility(133);
> +}
> +
> static void test_func(void)
> {
> uint8_t expected_vec_contents[VEC_REGISTER_NUM][VEC_REGISTER_SIZE];
> @@ -58,74 +69,89 @@ static void test_func(void)
> int i;
> int vec_result = 0;
>
> - for (i = 0; i < VEC_REGISTER_NUM; i++) {
> - vec_reg = &expected_vec_contents[i][0];
> - /* i+1 to avoid zero content */
> - memset(vec_reg, i + 1, VEC_REGISTER_SIZE);
> + if (have_guarded_storage_facility()) {
> + ctl_set_bit(2, CTL2_GUARDED_STORAGE);
> +
> + write_gs_regs();
> }
>
> - ctl_set_bit(0, CTL0_VECTOR);
> - ctl_set_bit(2, CTL2_GUARDED_STORAGE);
> -
> - write_gs_regs();
> -
> - /*
> - * It is important loading the vector/floating point registers and
> - * comparing their contents occurs in the same inline assembly block.
> - * Otherwise, the compiler is allowed to re-use the registers for
> - * something else in between.
> - * For this very reason, this also runs on a second CPU, so all the
> - * complex console stuff can be done in C on the first CPU and here we
> - * just need to wait for it to set the flag.
> - */
> - asm inline(
> - " .machine z13\n"
> - /* load vector registers: vlm handles at most 16 registers at a time */
> - " vlm 0,15, 0(%[expected_vec_reg])\n"
> - " vlm 16,31, 256(%[expected_vec_reg])\n"
> - /* inform CPU0 we are done, it will request migration */
> - " mvhi %[flag_thread_complete], 1\n"
> - /* wait for migration to finish */
> - "0: clfhsi %[flag_migration_complete], 1\n"
> - " jnz 0b\n"
> - /*
> - * store vector register contents in actual_vec_reg: vstm
> - * handles at most 16 registers at a time
> - */
> - " vstm 0,15, 0(%[actual_vec_reg])\n"
> - " vstm 16,31, 256(%[actual_vec_reg])\n"
> + if (have_vector_facility()) {
> + for (i = 0; i < VEC_REGISTER_NUM; i++) {
> + vec_reg = &expected_vec_contents[i][0];
> + /* i+1 to avoid zero content */
> + memset(vec_reg, i + 1, VEC_REGISTER_SIZE);
> + }
> +
> + ctl_set_bit(0, CTL0_VECTOR);
> +
> /*
> - * compare the contents in expected_vec_reg with actual_vec_reg:
> - * clc handles at most 256 bytes at a time
> + * It is important loading the vector/floating point registers and
> + * comparing their contents occurs in the same inline assembly block.
> + * Otherwise, the compiler is allowed to re-use the registers for
> + * something else in between.
> + * For this very reason, this also runs on a second CPU, so all the
> + * complex console stuff can be done in C on the first CPU and here we
> + * just need to wait for it to set the flag.
> */
> - " clc 0(256, %[expected_vec_reg]), 0(%[actual_vec_reg])\n"
> - " jnz 1f\n"
> - " clc 256(256, %[expected_vec_reg]), 256(%[actual_vec_reg])\n"
> - " jnz 1f\n"
> - /* success */
> - " mvhi %[vec_result], 1\n"
> - "1:"
> - :
> - : [expected_vec_reg] "a"(expected_vec_contents),
> - [actual_vec_reg] "a"(actual_vec_contents),
> - [flag_thread_complete] "Q"(flag_thread_complete),
> - [flag_migration_complete] "Q"(flag_migration_complete),
> - [vec_result] "Q"(vec_result)
> - : "v0", "v1", "v2", "v3", "v4", "v5", "v6", "v7", "v8", "v9",
> - "v10", "v11", "v12", "v13", "v14", "v15", "v16", "v17", "v18",
> - "v19", "v20", "v21", "v22", "v23", "v24", "v25", "v26", "v27",
> - "v28", "v29", "v30", "v31", "cc", "memory"
> - );
> -
> - report(vec_result, "vector contents match");
> -
> - check_gs_regs();
> -
> - report(stctg(0) & BIT(CTL0_VECTOR), "ctl0 vector bit set");
> - report(stctg(2) & BIT(CTL2_GUARDED_STORAGE), "ctl2 guarded-storage bit set");
> -
> - ctl_clear_bit(0, CTL0_VECTOR);
> - ctl_clear_bit(2, CTL2_GUARDED_STORAGE);
> + asm inline(
> + " .machine z13\n"
> + /* load vector registers: vlm handles at most 16 registers at a time */
> + " vlm 0,15, 0(%[expected_vec_reg])\n"
> + " vlm 16,31, 256(%[expected_vec_reg])\n"
> + /* inform CPU0 we are done, it will request migration */
> + " mvhi %[flag_thread_complete], 1\n"
> + /* wait for migration to finish */
> + "0: clfhsi %[flag_migration_complete], 1\n"
> + " jnz 0b\n"
> + /*
> + * store vector register contents in actual_vec_reg: vstm
> + * handles at most 16 registers at a time
> + */
> + " vstm 0,15, 0(%[actual_vec_reg])\n"
> + " vstm 16,31, 256(%[actual_vec_reg])\n"
> + /*
> + * compare the contents in expected_vec_reg with actual_vec_reg:
> + * clc handles at most 256 bytes at a time
> + */
> + " clc 0(256, %[expected_vec_reg]), 0(%[actual_vec_reg])\n"
> + " jnz 1f\n"
> + " clc 256(256, %[expected_vec_reg]), 256(%[actual_vec_reg])\n"
> + " jnz 1f\n"
> + /* success */
> + " mvhi %[vec_result], 1\n"
> + "1:"
> + :
> + : [expected_vec_reg] "a"(expected_vec_contents),
> + [actual_vec_reg] "a"(actual_vec_contents),
> + [flag_thread_complete] "Q"(flag_thread_complete),
> + [flag_migration_complete] "Q"(flag_migration_complete),
> + [vec_result] "Q"(vec_result)
> + : "v0", "v1", "v2", "v3", "v4", "v5", "v6", "v7", "v8", "v9",
> + "v10", "v11", "v12", "v13", "v14", "v15", "v16", "v17", "v18",
> + "v19", "v20", "v21", "v22", "v23", "v24", "v25", "v26", "v27",
> + "v28", "v29", "v30", "v31", "cc", "memory"
> + );
> +
> + report(vec_result, "vector contents match");
> +
> + report(stctg(0) & BIT(CTL0_VECTOR), "ctl0 vector bit set");
> +
> + ctl_clear_bit(0, CTL0_VECTOR);
> + } else {
> + flag_thread_complete = 1;
> + while(!flag_migration_complete)
> + mb();
> + }
> +
> + report_pass("Migrated");
> +
> + if (have_guarded_storage_facility()) {
> + check_gs_regs();
> +
> + report(stctg(2) & BIT(CTL2_GUARDED_STORAGE), "ctl2 guarded-storage bit set");
> +
> + ctl_clear_bit(2, CTL2_GUARDED_STORAGE);
> + }
>
> flag_thread_complete = 1;
> }
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [kvm-unit-tests PATCH v1] s390x: migration: don't run tests when facilities are not there
2022-05-05 11:54 ` Claudio Imbrenda
@ 2022-05-05 12:02 ` Nico Boehr
0 siblings, 0 replies; 3+ messages in thread
From: Nico Boehr @ 2022-05-05 12:02 UTC (permalink / raw)
To: Claudio Imbrenda; +Cc: kvm, linux-s390, frankja, thuth
On Thu, 2022-05-05 at 13:54 +0200, Claudio Imbrenda wrote:
> I think this patch should actually be folded in the previous one,
> I'll
> do that unless there are objections
Thanks. Fine for me.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-05-05 12:02 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-05-05 10:27 [kvm-unit-tests PATCH v1] s390x: migration: don't run tests when facilities are not there Nico Boehr
2022-05-05 11:54 ` Claudio Imbrenda
2022-05-05 12:02 ` Nico Boehr
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox