* [kvm-unit-tests PATCH v3] s390x: Add strict mode to specification exception interpretation test
@ 2022-07-05 11:17 Janis Schoetterl-Glausch
2022-08-25 7:37 ` Janosch Frank
0 siblings, 1 reply; 3+ messages in thread
From: Janis Schoetterl-Glausch @ 2022-07-05 11:17 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 3931,8562,8561,3907,3906,2965,2964`
will enable it for models z16 - 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.
`--strict !` will enable it always.
Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
---
v2 -> v3
* rebase on master
* global strict bool
* fix style issue
Range-diff against v2:
1: e9c36970 ! 1: c707481c s390x: Add strict mode to specification exception interpretation test
@@ Commit message
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.
+ `--strict 3931,8562,8561,3907,3906,2965,2964`
+ will enable it for models z16 - 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`
@@ s390x/spec_ex-sie.c
#include <sclp.h>
#include <asm/page.h>
#include <asm/arch_def.h>
+ #include <alloc_page.h>
+ #include <sie.h>
+ #include <snippet.h>
++#include <hardware.h>
+
+ static struct vm vm;
+ extern const char SNIPPET_NAME_START(c, spec_ex)[];
+ extern const char SNIPPET_NAME_END(c, spec_ex)[];
++static bool strict;
+
+ static void setup_guest(void)
+ {
@@ s390x/spec_ex-sie.c: static void reset_guest(void)
- vm.sblk->icptcode = 0;
- }
--static void test_spec_ex_sie(void)
-+static void test_spec_ex_sie(bool strict)
+ static void test_spec_ex_sie(void)
{
+ const char *msg;
+
@@ s390x/spec_ex-sie.c: static void test_spec_ex_sie(void)
+ if (list[0] == '!') {
+ ret = true;
+ list++;
-+ } else
++ } else {
+ ret = false;
++ }
+ while (true) {
+ long input = 0;
+
@@ s390x/spec_ex-sie.c: static void test_spec_ex_sie(void)
+
int main(int argc, char **argv)
{
++ strict = parse_strict(argc - 1, argv + 1);
if (!sclp_facilities.has_sief2) {
-@@ s390x/spec_ex-sie.c: int main(int argc, char **argv)
+ report_skip("SIEF2 facility unavailable");
goto out;
- }
-
-- test_spec_ex_sie();
-+ test_spec_ex_sie(parse_strict(argc - 1, argv + 1));
- out:
- return report_summary();
- }
s390x/spec_ex-sie.c | 53 +++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 51 insertions(+), 2 deletions(-)
diff --git a/s390x/spec_ex-sie.c b/s390x/spec_ex-sie.c
index d8e25e75..e5f39451 100644
--- a/s390x/spec_ex-sie.c
+++ b/s390x/spec_ex-sie.c
@@ -7,16 +7,19 @@
* specification exception interpretation is off/on.
*/
#include <libcflat.h>
+#include <stdlib.h>
#include <sclp.h>
#include <asm/page.h>
#include <asm/arch_def.h>
#include <alloc_page.h>
#include <sie.h>
#include <snippet.h>
+#include <hardware.h>
static struct vm vm;
extern const char SNIPPET_NAME_START(c, spec_ex)[];
extern const char SNIPPET_NAME_END(c, spec_ex)[];
+static bool strict;
static void setup_guest(void)
{
@@ -37,6 +40,8 @@ static void reset_guest(void)
static void test_spec_ex_sie(void)
{
+ const char *msg;
+
setup_guest();
report_prefix_push("SIE spec ex interpretation");
@@ -60,16 +65,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");
+ 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)
{
+ strict = parse_strict(argc - 1, argv + 1);
if (!sclp_facilities.has_sief2) {
report_skip("SIEF2 facility unavailable");
goto out;
base-commit: ca85dda2671e88d34acfbca6de48a9ab32b1810d
--
2.36.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [kvm-unit-tests PATCH v3] s390x: Add strict mode to specification exception interpretation test
2022-07-05 11:17 [kvm-unit-tests PATCH v3] s390x: Add strict mode to specification exception interpretation test Janis Schoetterl-Glausch
@ 2022-08-25 7:37 ` Janosch Frank
2022-08-25 10:57 ` Thomas Huth
0 siblings, 1 reply; 3+ messages in thread
From: Janosch Frank @ 2022-08-25 7:37 UTC (permalink / raw)
To: Janis Schoetterl-Glausch, Thomas Huth, Claudio Imbrenda
Cc: David Hildenbrand, kvm, linux-s390
On 7/5/22 13:17, 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.
> `--strict` takes a list of machine types (as reported by STIDP)
> for which to enable strict mode, for example
> `--strict 3931,8562,8561,3907,3906,2965,2964`
> will enable it for models z16 - 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.
> `--strict !` will enable it always.
>
> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
GCC 11.2.0 isn't happy
s390x/spec_ex-sie.c: In function ‘test_spec_ex_sie’:
s390x/spec_ex-sie.c:70:17: error: format not a string literal and no
format arguments [-Werror=format-security]
70 | report(vm.sblk->gpsw.addr == 0xdeadbeee, msg);
| ^~~~~~
s390x/spec_ex-sie.c:72:17: error: format not a string literal and no
format arguments [-Werror=format-security]
72 | report_info(msg);
| ^~~~~~~~~~~
cc1: all warnings being treated as errors
make: *** [<builtin>: s390x/spec_ex-sie.o] Error 1
Other than that the code looks ok to me.
I have to page in the discussion again to know how this fits into the
picture. Either that or Thomas tells me it's exactly what he wants and
I'll add it to my queue once the compile problem has been fixed one way
or another.
> ---
> v2 -> v3
> * rebase on master
> * global strict bool
> * fix style issue
>
> Range-diff against v2:
> 1: e9c36970 ! 1: c707481c s390x: Add strict mode to specification exception interpretation test
> @@ Commit message
> 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.
> + `--strict 3931,8562,8561,3907,3906,2965,2964`
> + will enable it for models z16 - 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`
> @@ s390x/spec_ex-sie.c
> #include <sclp.h>
> #include <asm/page.h>
> #include <asm/arch_def.h>
> + #include <alloc_page.h>
> + #include <sie.h>
> + #include <snippet.h>
> ++#include <hardware.h>
> +
> + static struct vm vm;
> + extern const char SNIPPET_NAME_START(c, spec_ex)[];
> + extern const char SNIPPET_NAME_END(c, spec_ex)[];
> ++static bool strict;
> +
> + static void setup_guest(void)
> + {
> @@ s390x/spec_ex-sie.c: static void reset_guest(void)
> - vm.sblk->icptcode = 0;
> - }
>
> --static void test_spec_ex_sie(void)
> -+static void test_spec_ex_sie(bool strict)
> + static void test_spec_ex_sie(void)
> {
> + const char *msg;
> +
> @@ s390x/spec_ex-sie.c: static void test_spec_ex_sie(void)
> + if (list[0] == '!') {
> + ret = true;
> + list++;
> -+ } else
> ++ } else {
> + ret = false;
> ++ }
> + while (true) {
> + long input = 0;
> +
> @@ s390x/spec_ex-sie.c: static void test_spec_ex_sie(void)
> +
> int main(int argc, char **argv)
> {
> ++ strict = parse_strict(argc - 1, argv + 1);
> if (!sclp_facilities.has_sief2) {
> -@@ s390x/spec_ex-sie.c: int main(int argc, char **argv)
> + report_skip("SIEF2 facility unavailable");
> goto out;
> - }
> -
> -- test_spec_ex_sie();
> -+ test_spec_ex_sie(parse_strict(argc - 1, argv + 1));
> - out:
> - return report_summary();
> - }
>
> s390x/spec_ex-sie.c | 53 +++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 51 insertions(+), 2 deletions(-)
>
> diff --git a/s390x/spec_ex-sie.c b/s390x/spec_ex-sie.c
> index d8e25e75..e5f39451 100644
> --- a/s390x/spec_ex-sie.c
> +++ b/s390x/spec_ex-sie.c
> @@ -7,16 +7,19 @@
> * specification exception interpretation is off/on.
> */
> #include <libcflat.h>
> +#include <stdlib.h>
> #include <sclp.h>
> #include <asm/page.h>
> #include <asm/arch_def.h>
> #include <alloc_page.h>
> #include <sie.h>
> #include <snippet.h>
> +#include <hardware.h>
>
> static struct vm vm;
> extern const char SNIPPET_NAME_START(c, spec_ex)[];
> extern const char SNIPPET_NAME_END(c, spec_ex)[];
> +static bool strict;
>
> static void setup_guest(void)
> {
> @@ -37,6 +40,8 @@ static void reset_guest(void)
>
> static void test_spec_ex_sie(void)
> {
> + const char *msg;
> +
> setup_guest();
>
> report_prefix_push("SIE spec ex interpretation");
> @@ -60,16 +65,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");
> + 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)
> {
> + strict = parse_strict(argc - 1, argv + 1);
> if (!sclp_facilities.has_sief2) {
> report_skip("SIEF2 facility unavailable");
> goto out;
>
> base-commit: ca85dda2671e88d34acfbca6de48a9ab32b1810d
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [kvm-unit-tests PATCH v3] s390x: Add strict mode to specification exception interpretation test
2022-08-25 7:37 ` Janosch Frank
@ 2022-08-25 10:57 ` Thomas Huth
0 siblings, 0 replies; 3+ messages in thread
From: Thomas Huth @ 2022-08-25 10:57 UTC (permalink / raw)
To: Janosch Frank, Janis Schoetterl-Glausch, Claudio Imbrenda
Cc: David Hildenbrand, kvm, linux-s390
On 25/08/2022 09.37, Janosch Frank wrote:
> On 7/5/22 13:17, 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.
>> `--strict` takes a list of machine types (as reported by STIDP)
>> for which to enable strict mode, for example
>> `--strict 3931,8562,8561,3907,3906,2965,2964`
>> will enable it for models z16 - 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.
>> `--strict !` will enable it always.
>>
>> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
>
> GCC 11.2.0 isn't happy
>
> s390x/spec_ex-sie.c: In function ‘test_spec_ex_sie’:
> s390x/spec_ex-sie.c:70:17: error: format not a string literal and no format
> arguments [-Werror=format-security]
> 70 | report(vm.sblk->gpsw.addr == 0xdeadbeee, msg);
> | ^~~~~~
> s390x/spec_ex-sie.c:72:17: error: format not a string literal and no format
> arguments [-Werror=format-security]
> 72 | report_info(msg);
> | ^~~~~~~~~~~
> cc1: all warnings being treated as errors
> make: *** [<builtin>: s390x/spec_ex-sie.o] Error 1
Too bad that GCC isn't smart enough to see that it is a constant string... I
guess we have to add an artifical "%s" format string here now?
> I have to page in the discussion again to know how this fits into the
> picture. Either that or Thomas tells me it's exactly what he wants and I'll
> add it to my queue once the compile problem has been fixed one way or another.
The point was that we wanted to use this k-u-t to automatically test a
backport of the related kernel commit. Without the strict mode that is not
possible since the test does not fail if the kernel backport is missing.
I just also noticed that I never replied to this v3 ... so FWIW, code looks
fine to me (with the "%s" fixup on top of it), thus:
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-08-25 10:57 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-05 11:17 [kvm-unit-tests PATCH v3] s390x: Add strict mode to specification exception interpretation test Janis Schoetterl-Glausch
2022-08-25 7:37 ` Janosch Frank
2022-08-25 10:57 ` Thomas Huth
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).