On Thu, 5 May 2022 12:27:05 +0200
Nico Boehr <nrb@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx>
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;
> }
