All of lore.kernel.org
 help / color / mirror / Atom feed
* [kvm-unit-tests PATCH] s390x: Add strict mode to specification exception interpretation test
@ 2022-02-25 17:23 Janis Schoetterl-Glausch
  2022-02-28 13:27 ` Claudio Imbrenda
  0 siblings, 1 reply; 6+ messages in thread
From: Janis Schoetterl-Glausch @ 2022-02-25 17:23 UTC (permalink / raw)
  To: Thomas Huth, Janosch Frank, Claudio Imbrenda
  Cc: Janis Schoetterl-Glausch, David Hildenbrand, kvm, linux-s390

While specification exception interpretation is not required to occur,
it can be useful for automatic regression testing to fail the test if it
does not occur.
Add a `--strict` argument to enable this.
`--strict` takes a list of machine types (as reported by STIDP)
for which to enable strict mode, for example
`--strict 8562,8561,3907,3906,2965,2964`
will enable it for models z15 - z13.
Alternatively, strict mode can be enabled for all but the listed machine
types by prefixing the list with a `!`, for example
`--strict !1090,1091,2064,2066,2084,2086,2094,2096,2097,2098,2817,2818,2827,2828`
will enable it for z/Architecture models except those older than z13.

Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
---
Range-diff:
1:  5d91f693 < -:  -------- s390x: Add specification exception interception test
2:  950eafd7 ! 1:  e9c36970 s390x: Add strict mode to specification exception interpretation test
    @@ s390x/spec_ex-sie.c: static void reset_guest(void)
     -static void test_spec_ex_sie(void)
     +static void test_spec_ex_sie(bool strict)
      {
    ++	const char *msg;
    ++
      	setup_guest();
      
    + 	report_prefix_push("SIE spec ex interpretation");
     @@ s390x/spec_ex-sie.c: static void test_spec_ex_sie(void)
      	report(vm.sblk->icptcode == ICPT_PROGI
      	       && vm.sblk->iprcc == PGM_INT_CODE_SPECIFICATION,
      	       "Received specification exception intercept");
     -	if (vm.sblk->gpsw.addr == 0xdeadbeee)
     -		report_info("Interpreted initial exception, intercepted invalid program new PSW exception");
    --	else
    --		report_info("Did not interpret initial exception");
    -+	{
    -+		const char *msg;
    -+
    -+		msg = "Interpreted initial exception, intercepted invalid program new PSW exception";
    -+		if (strict)
    -+			report(vm.sblk->gpsw.addr == 0xdeadbeee, msg);
    -+		else if (vm.sblk->gpsw.addr == 0xdeadbeee)
    -+			report_info(msg);
    -+		else
    -+			report_info("Did not interpret initial exception");
    -+	}
    ++	msg = "Interpreted initial exception, intercepted invalid program new PSW exception";
    ++	if (strict)
    ++		report(vm.sblk->gpsw.addr == 0xdeadbeee, msg);
    ++	else if (vm.sblk->gpsw.addr == 0xdeadbeee)
    ++		report_info(msg);
    + 	else
    + 		report_info("Did not interpret initial exception");
      	report_prefix_pop();
      	report_prefix_pop();
      }
      
    -+static bool parse_strict(char **argv)
    ++static bool parse_strict(int argc, char **argv)
     +{
     +	uint16_t machine_id;
     +	char *list;
     +	bool ret;
     +
    -+	if (!*argv)
    ++	if (argc < 1)
     +		return false;
    -+	if (strcmp("--strict", *argv))
    ++	if (strcmp("--strict", argv[0]))
     +		return false;
     +
     +	machine_id = get_machine_id();
    -+	list = argv[1];
    -+	if (!list) {
    ++	if (argc < 2) {
     +		printf("No argument to --strict, ignoring\n");
     +		return false;
     +	}
    ++	list = argv[1];
     +	if (list[0] == '!') {
     +		ret = true;
     +		list++;
    @@ s390x/spec_ex-sie.c: int main(int argc, char **argv)
      	}
      
     -	test_spec_ex_sie();
    -+	test_spec_ex_sie(parse_strict(argv + 1));
    ++	test_spec_ex_sie(parse_strict(argc - 1, argv + 1));
      out:
      	return report_summary();
      }

 s390x/spec_ex-sie.c | 53 +++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 49 insertions(+), 4 deletions(-)

diff --git a/s390x/spec_ex-sie.c b/s390x/spec_ex-sie.c
index 5dea4115..071110e3 100644
--- a/s390x/spec_ex-sie.c
+++ b/s390x/spec_ex-sie.c
@@ -7,6 +7,7 @@
  * specification exception interpretation is off/on.
  */
 #include <libcflat.h>
+#include <stdlib.h>
 #include <sclp.h>
 #include <asm/page.h>
 #include <asm/arch_def.h>
@@ -36,8 +37,10 @@ static void reset_guest(void)
 	vm.sblk->icptcode = 0;
 }
 
-static void test_spec_ex_sie(void)
+static void test_spec_ex_sie(bool strict)
 {
+	const char *msg;
+
 	setup_guest();
 
 	report_prefix_push("SIE spec ex interpretation");
@@ -61,14 +64,56 @@ static void test_spec_ex_sie(void)
 	report(vm.sblk->icptcode == ICPT_PROGI
 	       && vm.sblk->iprcc == PGM_INT_CODE_SPECIFICATION,
 	       "Received specification exception intercept");
-	if (vm.sblk->gpsw.addr == 0xdeadbeee)
-		report_info("Interpreted initial exception, intercepted invalid program new PSW exception");
+	msg = "Interpreted initial exception, intercepted invalid program new PSW exception";
+	if (strict)
+		report(vm.sblk->gpsw.addr == 0xdeadbeee, msg);
+	else if (vm.sblk->gpsw.addr == 0xdeadbeee)
+		report_info(msg);
 	else
 		report_info("Did not interpret initial exception");
 	report_prefix_pop();
 	report_prefix_pop();
 }
 
+static bool parse_strict(int argc, char **argv)
+{
+	uint16_t machine_id;
+	char *list;
+	bool ret;
+
+	if (argc < 1)
+		return false;
+	if (strcmp("--strict", argv[0]))
+		return false;
+
+	machine_id = get_machine_id();
+	if (argc < 2) {
+		printf("No argument to --strict, ignoring\n");
+		return false;
+	}
+	list = argv[1];
+	if (list[0] == '!') {
+		ret = true;
+		list++;
+	} else
+		ret = false;
+	while (true) {
+		long input = 0;
+
+		if (strlen(list) == 0)
+			return ret;
+		input = strtol(list, &list, 16);
+		if (*list == ',')
+			list++;
+		else if (*list != '\0')
+			break;
+		if (input == machine_id)
+			return !ret;
+	}
+	printf("Invalid --strict argument \"%s\", ignoring\n", list);
+	return ret;
+}
+
 int main(int argc, char **argv)
 {
 	if (!sclp_facilities.has_sief2) {
@@ -76,7 +121,7 @@ int main(int argc, char **argv)
 		goto out;
 	}
 
-	test_spec_ex_sie();
+	test_spec_ex_sie(parse_strict(argc - 1, argv + 1));
 out:
 	return report_summary();
 }

base-commit: 257c962f3d1b2d0534af59de4ad18764d734903a
-- 
2.33.1


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

* Re: [kvm-unit-tests PATCH] s390x: Add strict mode to specification exception interpretation test
  2022-02-25 17:23 [kvm-unit-tests PATCH] s390x: Add strict mode to specification exception interpretation test Janis Schoetterl-Glausch
@ 2022-02-28 13:27 ` Claudio Imbrenda
  2022-03-03  9:43   ` Janis Schoetterl-Glausch
  0 siblings, 1 reply; 6+ messages in thread
From: Claudio Imbrenda @ 2022-02-28 13:27 UTC (permalink / raw)
  To: Janis Schoetterl-Glausch
  Cc: Thomas Huth, Janosch Frank, David Hildenbrand, kvm, linux-s390

On Fri, 25 Feb 2022 18:23:55 +0100
Janis Schoetterl-Glausch <scgl@linux.ibm.com> wrote:

> While specification exception interpretation is not required to occur,
> it can be useful for automatic regression testing to fail the test if it
> does not occur.
> Add a `--strict` argument to enable this.
> `--strict` takes a list of machine types (as reported by STIDP)
> for which to enable strict mode, for example
> `--strict 8562,8561,3907,3906,2965,2964`
> will enable it for models z15 - z13.
> Alternatively, strict mode can be enabled for all but the listed machine
> types by prefixing the list with a `!`, for example
> `--strict !1090,1091,2064,2066,2084,2086,2094,2096,2097,2098,2817,2818,2827,2828`
> will enable it for z/Architecture models except those older than z13.
> 
> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
> ---

[...]

> +static bool parse_strict(int argc, char **argv)
> +{
> +	uint16_t machine_id;
> +	char *list;
> +	bool ret;
> +
> +	if (argc < 1)
> +		return false;
> +	if (strcmp("--strict", argv[0]))
> +		return false;
> +
> +	machine_id = get_machine_id();
> +	if (argc < 2) {
> +		printf("No argument to --strict, ignoring\n");
> +		return false;
> +	}
> +	list = argv[1];
> +	if (list[0] == '!') {
> +		ret = true;
> +		list++;
> +	} else
> +		ret = false;
> +	while (true) {
> +		long input = 0;
> +
> +		if (strlen(list) == 0)
> +			return ret;
> +		input = strtol(list, &list, 16);
> +		if (*list == ',')
> +			list++;
> +		else if (*list != '\0')
> +			break;
> +		if (input == machine_id)
> +			return !ret;
> +	}
> +	printf("Invalid --strict argument \"%s\", ignoring\n", list);
> +	return ret;
> +}

probably I should write a few parsing functions for command line
arguments, so we don't have to re-invent the wheel every time

> +
>  int main(int argc, char **argv)
>  {
>  	if (!sclp_facilities.has_sief2) {
> @@ -76,7 +121,7 @@ int main(int argc, char **argv)
>  		goto out;
>  	}
>  
> -	test_spec_ex_sie();
> +	test_spec_ex_sie(parse_strict(argc - 1, argv + 1));

hmmm... maybe it would be more readable and more uniform with the other
tests to parse the command line during initialization of the unit test,
and set a global flag.

>  out:
>  	return report_summary();
>  }
> 
> base-commit: 257c962f3d1b2d0534af59de4ad18764d734903a


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

* Re: [kvm-unit-tests PATCH] s390x: Add strict mode to specification exception interpretation test
  2022-02-28 13:27 ` Claudio Imbrenda
@ 2022-03-03  9:43   ` Janis Schoetterl-Glausch
  0 siblings, 0 replies; 6+ messages in thread
From: Janis Schoetterl-Glausch @ 2022-03-03  9:43 UTC (permalink / raw)
  To: Claudio Imbrenda
  Cc: Thomas Huth, Janosch Frank, David Hildenbrand, kvm, linux-s390

On 2/28/22 14:27, Claudio Imbrenda wrote:
> On Fri, 25 Feb 2022 18:23:55 +0100
> Janis Schoetterl-Glausch <scgl@linux.ibm.com> wrote:
> 
>> While specification exception interpretation is not required to occur,
>> it can be useful for automatic regression testing to fail the test if it
>> does not occur.
>> Add a `--strict` argument to enable this.
>> `--strict` takes a list of machine types (as reported by STIDP)
>> for which to enable strict mode, for example
>> `--strict 8562,8561,3907,3906,2965,2964`
>> will enable it for models z15 - z13.
>> Alternatively, strict mode can be enabled for all but the listed machine
>> types by prefixing the list with a `!`, for example
>> `--strict !1090,1091,2064,2066,2084,2086,2094,2096,2097,2098,2817,2818,2827,2828`
>> will enable it for z/Architecture models except those older than z13.
>>
>> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
>> ---
> 
> [...]
> 
>> +static bool parse_strict(int argc, char **argv)
>> +{
>> +	uint16_t machine_id;
>> +	char *list;
>> +	bool ret;
>> +
>> +	if (argc < 1)
>> +		return false;
>> +	if (strcmp("--strict", argv[0]))
>> +		return false;
>> +
>> +	machine_id = get_machine_id();
>> +	if (argc < 2) {
>> +		printf("No argument to --strict, ignoring\n");
>> +		return false;
>> +	}
>> +	list = argv[1];
>> +	if (list[0] == '!') {
>> +		ret = true;
>> +		list++;
>> +	} else
>> +		ret = false;
>> +	while (true) {
>> +		long input = 0;
>> +
>> +		if (strlen(list) == 0)
>> +			return ret;
>> +		input = strtol(list, &list, 16);
>> +		if (*list == ',')
>> +			list++;
>> +		else if (*list != '\0')
>> +			break;
>> +		if (input == machine_id)
>> +			return !ret;
>> +	}
>> +	printf("Invalid --strict argument \"%s\", ignoring\n", list);
>> +	return ret;
>> +}
> 
> probably I should write a few parsing functions for command line
> arguments, so we don't have to re-invent the wheel every time

Maybe, would depend on what you have in mind, I'm not sure most
use cases can be covered by a reasonable set of abstractions.
> 
>> +
>>  int main(int argc, char **argv)
>>  {
>>  	if (!sclp_facilities.has_sief2) {
>> @@ -76,7 +121,7 @@ int main(int argc, char **argv)
>>  		goto out;
>>  	}
>>  
>> -	test_spec_ex_sie();
>> +	test_spec_ex_sie(parse_strict(argc - 1, argv + 1));
> 
> hmmm... maybe it would be more readable and more uniform with the other
> tests to parse the command line during initialization of the unit test,
> and set a global flag.

More uniform maybe, but I tend to dislike globals from a readability point
of view. I'm inclined to keep it as is.
> 
>>  out:
>>  	return report_summary();
>>  }
>>
>> base-commit: 257c962f3d1b2d0534af59de4ad18764d734903a
> 


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

* Re: [kvm-unit-tests PATCH] s390x: Add strict mode to specification exception interpretation test
  2021-12-03 11:15   ` Thomas Huth
@ 2021-12-03 11:46     ` Janis Schoetterl-Glausch
  0 siblings, 0 replies; 6+ messages in thread
From: Janis Schoetterl-Glausch @ 2021-12-03 11:46 UTC (permalink / raw)
  To: Thomas Huth, Janis Schoetterl-Glausch, Janosch Frank, Claudio Imbrenda
  Cc: David Hildenbrand, kvm, linux-s390

On 12/3/21 12:15, Thomas Huth wrote:
> On 25/11/2021 15.47, Janis Schoetterl-Glausch wrote:
>> While specification exception interpretation is not required to occur,
>> it can be useful for automatic regression testing to fail the test if it
>> does not occur.
>> Add a `--strict` argument to enable this.
> 
> Thank you very much for adding this!

Sure :)
> 
> Some comments below...
> 
>> `--strict` takes a list of machine types (as reported by STIDP)
>> for which to enable strict mode, for example
>> `--strict 8562,8561,3907,3906,2965,2964`
>> will enable it for models z15 - z13.
>> Alternatively, strict mode can be enabled for all but the listed machine
>> types by prefixing the list with a `!`, for example
>> `--strict !1090,1091,2064,2066,2084,2086,2094,2096,2097,2098,2817,2818,2827,2828`
>> will enable it for z/Architecture models except those older than z13.
>>
>> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
>> ---
>>
>> Apparently my message with inline patch did not make it to the mailing
>> list for some reason, so here's the patch again.
>>
>>   s390x/spec_ex-sie.c | 59 ++++++++++++++++++++++++++++++++++++++++-----
>>   1 file changed, 53 insertions(+), 6 deletions(-)
>>
>> diff --git a/s390x/spec_ex-sie.c b/s390x/spec_ex-sie.c
>> index 5dea411..9a063f9 100644
>> --- a/s390x/spec_ex-sie.c
>> +++ b/s390x/spec_ex-sie.c
>> @@ -7,6 +7,7 @@
>>    * specification exception interpretation is off/on.
>>    */
>>   #include <libcflat.h>
>> +#include <stdlib.h>
>>   #include <sclp.h>
>>   #include <asm/page.h>
>>   #include <asm/arch_def.h>
>> @@ -36,7 +37,7 @@ static void reset_guest(void)
>>       vm.sblk->icptcode = 0;
>>   }
>>   -static void test_spec_ex_sie(void)
>> +static void test_spec_ex_sie(bool strict)
>>   {
>>       setup_guest();
>>   @@ -61,14 +62,60 @@ static void test_spec_ex_sie(void)
>>       report(vm.sblk->icptcode == ICPT_PROGI
>>              && vm.sblk->iprcc == PGM_INT_CODE_SPECIFICATION,
>>              "Received specification exception intercept");
>> -    if (vm.sblk->gpsw.addr == 0xdeadbeee)
>> -        report_info("Interpreted initial exception, intercepted invalid program new PSW exception");
>> -    else
>> -        report_info("Did not interpret initial exception");
>> +    {
>> +        const char *msg;
> 
> Could you please move the variable declaration to the beginning of the function? Then you could get rid of the curly brackets and one level of indentation.

Yes.
> 
>> +        msg = "Interpreted initial exception, intercepted invalid program new PSW exception";
>> +        if (strict)
>> +            report(vm.sblk->gpsw.addr == 0xdeadbeee, msg);
>> +        else if (vm.sblk->gpsw.addr == 0xdeadbeee)
>> +            report_info(msg);
>> +        else
>> +            report_info("Did not interpret initial exception");
>> +    }
>>       report_prefix_pop();
>>       report_prefix_pop();
>>   }
>>   +static bool parse_strict(char **argv)
>> +{
>> +    uint16_t machine_id;
>> +    char *list;
>> +    bool ret;
>> +
>> +    if (!*argv)
>> +        return false;
> 
> I think this works ok with out current implementation of argv, but that's an "inofficial" implementation detail, so in case this ever gets changed, it might be better to check argc first before dereferencing argv here ... so could you please add a check for argc, too?

This is required by POSIX, isn't it? But then whether or not we comply with it is another question.
> 
>> +    if (strcmp("--strict", *argv))
>> +        return false;
>> +
>> +    machine_id = get_machine_id();
>> +    list = argv[1];

This needs to be covered by the argc check too, then.

>> +    if (!list) {
>> +        printf("No argument to --strict, ignoring\n");
>> +        return false;
> 
> You could also support --strict without arguments - that could turn on the strict mode unconditionally, I think.

--strict ! works. Granted it's not the best UI, but it is more consistent,
you could do --strict $(get list of ids from somewhere) and if the list is empty the semantics stay the same. 
> 
>> +    }
>> +    if (list[0] == '!') {
>> +        ret = true;
>> +        list++;
>> +    } else
>> +        ret = false;
>> +    while (true) {
>> +        long input = 0;
>> +
>> +        if (strlen(list) == 0)
>> +            return ret;
>> +        input = strtol(list, &list, 16);
>> +        if (*list == ',')
>> +            list++;
>> +        else if (*list != '\0')
>> +            break;
>> +        if (input == machine_id)
>> +            return !ret;
>> +    }
>> +    printf("Invalid --strict argument \"%s\", ignoring\n", list);
>> +    return ret;
>> +}
>> +
>>   int main(int argc, char **argv)
>>   {
>>       if (!sclp_facilities.has_sief2) {
>> @@ -76,7 +123,7 @@ int main(int argc, char **argv)
>>           goto out;
>>       }
>>   -    test_spec_ex_sie();
>> +    test_spec_ex_sie(parse_strict(argv + 1));
>>   out:
>>       return report_summary();
>>   }
>>
> 
>  Thomas
> 


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

* Re: [kvm-unit-tests PATCH] s390x: Add strict mode to specification exception interpretation test
  2021-11-25 14:47 ` [kvm-unit-tests PATCH] s390x: Add strict mode to specification exception interpretation test Janis Schoetterl-Glausch
@ 2021-12-03 11:15   ` Thomas Huth
  2021-12-03 11:46     ` Janis Schoetterl-Glausch
  0 siblings, 1 reply; 6+ messages in thread
From: Thomas Huth @ 2021-12-03 11:15 UTC (permalink / raw)
  To: Janis Schoetterl-Glausch, Janosch Frank, Claudio Imbrenda
  Cc: David Hildenbrand, kvm, linux-s390

On 25/11/2021 15.47, Janis Schoetterl-Glausch wrote:
> While specification exception interpretation is not required to occur,
> it can be useful for automatic regression testing to fail the test if it
> does not occur.
> Add a `--strict` argument to enable this.

Thank you very much for adding this!

Some comments below...

> `--strict` takes a list of machine types (as reported by STIDP)
> for which to enable strict mode, for example
> `--strict 8562,8561,3907,3906,2965,2964`
> will enable it for models z15 - z13.
> Alternatively, strict mode can be enabled for all but the listed machine
> types by prefixing the list with a `!`, for example
> `--strict !1090,1091,2064,2066,2084,2086,2094,2096,2097,2098,2817,2818,2827,2828`
> will enable it for z/Architecture models except those older than z13.
> 
> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
> ---
> 
> Apparently my message with inline patch did not make it to the mailing
> list for some reason, so here's the patch again.
> 
>   s390x/spec_ex-sie.c | 59 ++++++++++++++++++++++++++++++++++++++++-----
>   1 file changed, 53 insertions(+), 6 deletions(-)
> 
> diff --git a/s390x/spec_ex-sie.c b/s390x/spec_ex-sie.c
> index 5dea411..9a063f9 100644
> --- a/s390x/spec_ex-sie.c
> +++ b/s390x/spec_ex-sie.c
> @@ -7,6 +7,7 @@
>    * specification exception interpretation is off/on.
>    */
>   #include <libcflat.h>
> +#include <stdlib.h>
>   #include <sclp.h>
>   #include <asm/page.h>
>   #include <asm/arch_def.h>
> @@ -36,7 +37,7 @@ static void reset_guest(void)
>   	vm.sblk->icptcode = 0;
>   }
>   
> -static void test_spec_ex_sie(void)
> +static void test_spec_ex_sie(bool strict)
>   {
>   	setup_guest();
>   
> @@ -61,14 +62,60 @@ static void test_spec_ex_sie(void)
>   	report(vm.sblk->icptcode == ICPT_PROGI
>   	       && vm.sblk->iprcc == PGM_INT_CODE_SPECIFICATION,
>   	       "Received specification exception intercept");
> -	if (vm.sblk->gpsw.addr == 0xdeadbeee)
> -		report_info("Interpreted initial exception, intercepted invalid program new PSW exception");
> -	else
> -		report_info("Did not interpret initial exception");
> +	{
> +		const char *msg;

Could you please move the variable declaration to the beginning of the 
function? Then you could get rid of the curly brackets and one level of 
indentation.

> +		msg = "Interpreted initial exception, intercepted invalid program new PSW exception";
> +		if (strict)
> +			report(vm.sblk->gpsw.addr == 0xdeadbeee, msg);
> +		else if (vm.sblk->gpsw.addr == 0xdeadbeee)
> +			report_info(msg);
> +		else
> +			report_info("Did not interpret initial exception");
> +	}
>   	report_prefix_pop();
>   	report_prefix_pop();
>   }
>   
> +static bool parse_strict(char **argv)
> +{
> +	uint16_t machine_id;
> +	char *list;
> +	bool ret;
> +
> +	if (!*argv)
> +		return false;

I think this works ok with out current implementation of argv, but that's an 
"inofficial" implementation detail, so in case this ever gets changed, it 
might be better to check argc first before dereferencing argv here ... so 
could you please add a check for argc, too?

> +	if (strcmp("--strict", *argv))
> +		return false;
> +
> +	machine_id = get_machine_id();
> +	list = argv[1];
> +	if (!list) {
> +		printf("No argument to --strict, ignoring\n");
> +		return false;

You could also support --strict without arguments - that could turn on the 
strict mode unconditionally, I think.

> +	}
> +	if (list[0] == '!') {
> +		ret = true;
> +		list++;
> +	} else
> +		ret = false;
> +	while (true) {
> +		long input = 0;
> +
> +		if (strlen(list) == 0)
> +			return ret;
> +		input = strtol(list, &list, 16);
> +		if (*list == ',')
> +			list++;
> +		else if (*list != '\0')
> +			break;
> +		if (input == machine_id)
> +			return !ret;
> +	}
> +	printf("Invalid --strict argument \"%s\", ignoring\n", list);
> +	return ret;
> +}
> +
>   int main(int argc, char **argv)
>   {
>   	if (!sclp_facilities.has_sief2) {
> @@ -76,7 +123,7 @@ int main(int argc, char **argv)
>   		goto out;
>   	}
>   
> -	test_spec_ex_sie();
> +	test_spec_ex_sie(parse_strict(argv + 1));
>   out:
>   	return report_summary();
>   }
> 

  Thomas


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

* [kvm-unit-tests PATCH] s390x: Add strict mode to specification exception interpretation test
  2021-11-11  7:47 [kvm-unit-tests PATCH v3 1/1] s390x: Add specification exception interception test Thomas Huth
@ 2021-11-25 14:47 ` Janis Schoetterl-Glausch
  2021-12-03 11:15   ` Thomas Huth
  0 siblings, 1 reply; 6+ messages in thread
From: Janis Schoetterl-Glausch @ 2021-11-25 14:47 UTC (permalink / raw)
  To: Thomas Huth, Janosch Frank, Claudio Imbrenda
  Cc: Janis Schoetterl-Glausch, David Hildenbrand, kvm, linux-s390

While specification exception interpretation is not required to occur,
it can be useful for automatic regression testing to fail the test if it
does not occur.
Add a `--strict` argument to enable this.
`--strict` takes a list of machine types (as reported by STIDP)
for which to enable strict mode, for example
`--strict 8562,8561,3907,3906,2965,2964`
will enable it for models z15 - z13.
Alternatively, strict mode can be enabled for all but the listed machine
types by prefixing the list with a `!`, for example
`--strict !1090,1091,2064,2066,2084,2086,2094,2096,2097,2098,2817,2818,2827,2828`
will enable it for z/Architecture models except those older than z13.

Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
---

Apparently my message with inline patch did not make it to the mailing
list for some reason, so here's the patch again.

 s390x/spec_ex-sie.c | 59 ++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 53 insertions(+), 6 deletions(-)

diff --git a/s390x/spec_ex-sie.c b/s390x/spec_ex-sie.c
index 5dea411..9a063f9 100644
--- a/s390x/spec_ex-sie.c
+++ b/s390x/spec_ex-sie.c
@@ -7,6 +7,7 @@
  * specification exception interpretation is off/on.
  */
 #include <libcflat.h>
+#include <stdlib.h>
 #include <sclp.h>
 #include <asm/page.h>
 #include <asm/arch_def.h>
@@ -36,7 +37,7 @@ static void reset_guest(void)
 	vm.sblk->icptcode = 0;
 }
 
-static void test_spec_ex_sie(void)
+static void test_spec_ex_sie(bool strict)
 {
 	setup_guest();
 
@@ -61,14 +62,60 @@ static void test_spec_ex_sie(void)
 	report(vm.sblk->icptcode == ICPT_PROGI
 	       && vm.sblk->iprcc == PGM_INT_CODE_SPECIFICATION,
 	       "Received specification exception intercept");
-	if (vm.sblk->gpsw.addr == 0xdeadbeee)
-		report_info("Interpreted initial exception, intercepted invalid program new PSW exception");
-	else
-		report_info("Did not interpret initial exception");
+	{
+		const char *msg;
+
+		msg = "Interpreted initial exception, intercepted invalid program new PSW exception";
+		if (strict)
+			report(vm.sblk->gpsw.addr == 0xdeadbeee, msg);
+		else if (vm.sblk->gpsw.addr == 0xdeadbeee)
+			report_info(msg);
+		else
+			report_info("Did not interpret initial exception");
+	}
 	report_prefix_pop();
 	report_prefix_pop();
 }
 
+static bool parse_strict(char **argv)
+{
+	uint16_t machine_id;
+	char *list;
+	bool ret;
+
+	if (!*argv)
+		return false;
+	if (strcmp("--strict", *argv))
+		return false;
+
+	machine_id = get_machine_id();
+	list = argv[1];
+	if (!list) {
+		printf("No argument to --strict, ignoring\n");
+		return false;
+	}
+	if (list[0] == '!') {
+		ret = true;
+		list++;
+	} else
+		ret = false;
+	while (true) {
+		long input = 0;
+
+		if (strlen(list) == 0)
+			return ret;
+		input = strtol(list, &list, 16);
+		if (*list == ',')
+			list++;
+		else if (*list != '\0')
+			break;
+		if (input == machine_id)
+			return !ret;
+	}
+	printf("Invalid --strict argument \"%s\", ignoring\n", list);
+	return ret;
+}
+
 int main(int argc, char **argv)
 {
 	if (!sclp_facilities.has_sief2) {
@@ -76,7 +123,7 @@ int main(int argc, char **argv)
 		goto out;
 	}
 
-	test_spec_ex_sie();
+	test_spec_ex_sie(parse_strict(argv + 1));
 out:
 	return report_summary();
 }
-- 
2.31.1


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

end of thread, other threads:[~2022-03-03  9:43 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-25 17:23 [kvm-unit-tests PATCH] s390x: Add strict mode to specification exception interpretation test Janis Schoetterl-Glausch
2022-02-28 13:27 ` Claudio Imbrenda
2022-03-03  9:43   ` Janis Schoetterl-Glausch
  -- strict thread matches above, loose matches on Subject: below --
2021-11-11  7:47 [kvm-unit-tests PATCH v3 1/1] s390x: Add specification exception interception test Thomas Huth
2021-11-25 14:47 ` [kvm-unit-tests PATCH] s390x: Add strict mode to specification exception interpretation test Janis Schoetterl-Glausch
2021-12-03 11:15   ` Thomas Huth
2021-12-03 11:46     ` Janis Schoetterl-Glausch

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.