[kvm-unit-tests PATCH v1] s390x: verify EQBS/SQBS is unavailable

[kvm-unit-tests PATCH v1] s390x: verify EQBS/SQBS is unavailable

[Nico Boehr]

QEMU doesn't provide EQBS/SQBS instructions, so we should check they
result in an exception.

Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
---
 s390x/intercept.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/s390x/intercept.c b/s390x/intercept.c
index 9e826b6c79ad..73b06b5fc6e8 100644
--- a/s390x/intercept.c
+++ b/s390x/intercept.c
@@ -197,6 +197,55 @@ static void test_diag318(void)
 
 }
 
+static inline int sqbs(u64 token)
+{
+	unsigned long _token = token;
+	int cc;
+
+	asm volatile(
+		"	lgr 1,%[token]\n"
+		"	.insn   rsy,0xeb000000008a,0,0,0(0)\n"
+		"	ipm %[cc]\n"
+		"	srl %[cc],28\n"
+		: [cc] "=&d" (cc)
+		: [token] "d" (_token)
+		: "memory", "cc", "1");
+
+	return cc;
+}
+
+static inline int eqbs(u64 token)
+{
+	unsigned long _token = token;
+	int cc;
+
+	asm volatile(
+		"	lgr 1,%[token]\n"
+		"	.insn   rrf,0xb99c0000,0,0,0,0\n"
+		"	ipm %[cc]\n"
+		"	srl %[cc],28\n"
+		: [cc] "=&d" (cc)
+		: [token] "d" (_token)
+		: "memory", "cc", "1");
+
+	return cc;
+}
+
+static void test_qbs(void)
+{
+	report_prefix_push("sqbs");
+	expect_pgm_int();
+	sqbs(0xffffffdeadbeefULL);
+	check_pgm_int_code(PGM_INT_CODE_OPERATION);
+	report_prefix_pop();
+
+	report_prefix_push("eqbs");
+	expect_pgm_int();
+	eqbs(0xffffffdeadbeefULL);
+	check_pgm_int_code(PGM_INT_CODE_OPERATION);
+	report_prefix_pop();
+}
+
 struct {
 	const char *name;
 	void (*func)(void);
@@ -208,6 +257,7 @@ struct {
 	{ "stidp", test_stidp, false },
 	{ "testblock", test_testblock, false },
 	{ "diag318", test_diag318, false },
+	{ "qbs", test_qbs, false },
 	{ NULL, NULL, false }
 };
 
-- 
2.36.1

Re: [kvm-unit-tests PATCH v1] s390x: verify EQBS/SQBS is unavailable

[Claudio Imbrenda]

On Tue,  2 Aug 2022 16:51:02 +0200
Nico Boehr <nrb@linux.ibm.com> wrote:

> QEMU doesn't provide EQBS/SQBS instructions, so we should check they
> result in an exception.
> 
> Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
> ---
>  s390x/intercept.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 50 insertions(+)
> 
> diff --git a/s390x/intercept.c b/s390x/intercept.c
> index 9e826b6c79ad..73b06b5fc6e8 100644
> --- a/s390x/intercept.c
> +++ b/s390x/intercept.c
> @@ -197,6 +197,55 @@ static void test_diag318(void)
>  
>  }
>  
> +static inline int sqbs(u64 token)
> +{
> +	unsigned long _token = token;
> +	int cc;
> +
> +	asm volatile(
> +		"	lgr 1,%[token]\n"
> +		"	.insn   rsy,0xeb000000008a,0,0,0(0)\n"
> +		"	ipm %[cc]\n"
> +		"	srl %[cc],28\n"
> +		: [cc] "=&d" (cc)

do you really need all those extra things?

can't you just reduce this whole function to:

asm volatile("	.insn   rsy,0xeb000000008a,0,0,0(0)\n");

in the end we don't care what happens, we only want it to fail with an
operation exception

(ok maybe you need to add some clobbers to make sure things work as
they should in case the instruction is actually executed)

> +		: [token] "d" (_token)
> +		: "memory", "cc", "1");
> +
> +	return cc;
> +}
> +
> +static inline int eqbs(u64 token)
> +{
> +	unsigned long _token = token;
> +	int cc;
> +
> +	asm volatile(
> +		"	lgr 1,%[token]\n"
> +		"	.insn   rrf,0xb99c0000,0,0,0,0\n"

same here

> +		"	ipm %[cc]\n"
> +		"	srl %[cc],28\n"
> +		: [cc] "=&d" (cc)
> +		: [token] "d" (_token)
> +		: "memory", "cc", "1");
> +
> +	return cc;
> +}
> +
> +static void test_qbs(void)
> +{
> +	report_prefix_push("sqbs");
> +	expect_pgm_int();
> +	sqbs(0xffffffdeadbeefULL);
> +	check_pgm_int_code(PGM_INT_CODE_OPERATION);
> +	report_prefix_pop();
> +
> +	report_prefix_push("eqbs");
> +	expect_pgm_int();
> +	eqbs(0xffffffdeadbeefULL);
> +	check_pgm_int_code(PGM_INT_CODE_OPERATION);
> +	report_prefix_pop();
> +}

we expect those to fail only in qemu, right?
maybe this should be fenced and skip the tests when running in other
environments

> +
>  struct {
>  	const char *name;
>  	void (*func)(void);
> @@ -208,6 +257,7 @@ struct {
>  	{ "stidp", test_stidp, false },
>  	{ "testblock", test_testblock, false },
>  	{ "diag318", test_diag318, false },
> +	{ "qbs", test_qbs, false },
>  	{ NULL, NULL, false }
>  };
>  

Re: [kvm-unit-tests PATCH v1] s390x: verify EQBS/SQBS is unavailable

[Nico Boehr]

Quoting Claudio Imbrenda (2022-08-02 18:14:20)
[...]
> > diff --git a/s390x/intercept.c b/s390x/intercept.c
> > index 9e826b6c79ad..73b06b5fc6e8 100644
> > --- a/s390x/intercept.c
> > +++ b/s390x/intercept.c
> > @@ -197,6 +197,55 @@ static void test_diag318(void)
> >  
> >  }
> >  
> > +static inline int sqbs(u64 token)
> > +{
> > +     unsigned long _token = token;
> > +     int cc;
> > +
> > +     asm volatile(
> > +             "       lgr 1,%[token]\n"
> > +             "       .insn   rsy,0xeb000000008a,0,0,0(0)\n"
> > +             "       ipm %[cc]\n"
> > +             "       srl %[cc],28\n"
> > +             : [cc] "=&d" (cc)
> 
> do you really need all those extra things?
> 
> can't you just reduce this whole function to:
> 
> asm volatile("  .insn   rsy,0xeb000000008a,0,0,0(0)\n");
> 
> in the end we don't care what happens, we only want it to fail with an
> operation exception
> 
> (ok maybe you need to add some clobbers to make sure things work as
> they should in case the instruction is actually executed)

I don't mind changing that, will do.

[...]
> > +static void test_qbs(void)
> > +{
> > +     report_prefix_push("sqbs");
> > +     expect_pgm_int();
> > +     sqbs(0xffffffdeadbeefULL);
> > +     check_pgm_int_code(PGM_INT_CODE_OPERATION);
> > +     report_prefix_pop();
> > +
> > +     report_prefix_push("eqbs");
> > +     expect_pgm_int();
> > +     eqbs(0xffffffdeadbeefULL);
> > +     check_pgm_int_code(PGM_INT_CODE_OPERATION);
> > +     report_prefix_pop();
> > +}
> 
> we expect those to fail only in qemu, right?
> maybe this should be fenced and skip the tests when running in other
> environments

OK will do.