[kvm-unit-tests PATCH v2 0/5] s390x: Dirty cc before executing tested instructions

[kvm-unit-tests PATCH v2 0/5] s390x: Dirty cc before executing tested instructions

[Janosch Frank]

A recent s390 KVM fixpatch [1] showed us that checking the cc is not
enough when emulation code forgets to set the cc. There might just be
the correct cc in the PSW which would make the cc check succeed.

This series intentionally dirties the cc for sigp, uvc, some io
instructions and sclp to make cc setting errors more apparent. I had a
cursory look through the tested instructions and those are the most
prominent ones with defined cc values.

Since the issue appeared in PQAP my AP test series is now dependent on
this series.

[1] https://lore.kernel.org/kvm/20231201181657.1614645-1-farman@linux.ibm.com/

v2:
	* Moved from spm to tmll (thanks Nina)

Janosch Frank (5):
  lib: s390x: sigp: Dirty CC before sigp execution
  lib: s390x: uv: Dirty CC before uvc execution
  lib: s390x: css: Dirty CC before css instructions
  s390x: mvpg: Dirty CC before mvpg execution
  s390x: sclp: Dirty CC before sclp execution

 lib/s390x/asm/sigp.h |  6 +++++-
 lib/s390x/asm/uv.h   |  4 +++-
 lib/s390x/css.h      | 16 ++++++++++++----
 s390x/mvpg.c         |  6 ++++--
 s390x/sclp.c         |  5 ++++-
 5 files changed, 28 insertions(+), 9 deletions(-)

-- 
2.40.1

[kvm-unit-tests PATCH v2 1/5] lib: s390x: sigp: Dirty CC before sigp execution

[Janosch Frank]

Dirtying the CC allows us to find missing CC changes when sigp is
emulated.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 lib/s390x/asm/sigp.h | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/lib/s390x/asm/sigp.h b/lib/s390x/asm/sigp.h
index 61d2c625..4eae95d0 100644
--- a/lib/s390x/asm/sigp.h
+++ b/lib/s390x/asm/sigp.h
@@ -49,13 +49,17 @@ static inline int sigp(uint16_t addr, uint8_t order, unsigned long parm,
 		       uint32_t *status)
 {
 	register unsigned long reg1 asm ("1") = parm;
+	uint64_t bogus_cc = SIGP_CC_NOT_OPERATIONAL;
 	int cc;
 
 	asm volatile(
+		"	tmll	%[bogus_cc],3\n"
 		"	sigp	%1,%2,0(%3)\n"
 		"	ipm	%0\n"
 		"	srl	%0,28\n"
-		: "=d" (cc), "+d" (reg1) : "d" (addr), "a" (order) : "cc");
+		: "=d" (cc), "+d" (reg1)
+		: "d" (addr), "a" (order), [bogus_cc] "d" (bogus_cc)
+		: "cc");
 	if (status)
 		*status = reg1;
 	return cc;
-- 
2.40.1

Re: [kvm-unit-tests PATCH v2 1/5] lib: s390x: sigp: Dirty CC before sigp execution

[Claudio Imbrenda]

On Wed, 31 Jan 2024 07:44:23 +0000
Janosch Frank <frankja@linux.ibm.com> wrote:

> Dirtying the CC allows us to find missing CC changes when sigp is
> emulated.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  lib/s390x/asm/sigp.h | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/s390x/asm/sigp.h b/lib/s390x/asm/sigp.h
> index 61d2c625..4eae95d0 100644
> --- a/lib/s390x/asm/sigp.h
> +++ b/lib/s390x/asm/sigp.h
> @@ -49,13 +49,17 @@ static inline int sigp(uint16_t addr, uint8_t order, unsigned long parm,
>  		       uint32_t *status)
>  {
>  	register unsigned long reg1 asm ("1") = parm;
> +	uint64_t bogus_cc = SIGP_CC_NOT_OPERATIONAL;
>  	int cc;
>  
>  	asm volatile(
> +		"	tmll	%[bogus_cc],3\n"
>  		"	sigp	%1,%2,0(%3)\n"
>  		"	ipm	%0\n"
>  		"	srl	%0,28\n"
> -		: "=d" (cc), "+d" (reg1) : "d" (addr), "a" (order) : "cc");
> +		: "=d" (cc), "+d" (reg1)
> +		: "d" (addr), "a" (order), [bogus_cc] "d" (bogus_cc)
> +		: "cc");

since you are doing changes in this inline asm, could you put names for
all the parameters? that way it will be more consistent and readable.

also in all the other patches in the series (except patch 2)

>  	if (status)
>  		*status = reg1;
>  	return cc;

Re: [kvm-unit-tests PATCH v2 1/5] lib: s390x: sigp: Dirty CC before sigp execution

[Janosch Frank]

On 1/31/24 12:56, Claudio Imbrenda wrote:
> On Wed, 31 Jan 2024 07:44:23 +0000
> Janosch Frank <frankja@linux.ibm.com> wrote:
> 
>> Dirtying the CC allows us to find missing CC changes when sigp is
>> emulated.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>   lib/s390x/asm/sigp.h | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/s390x/asm/sigp.h b/lib/s390x/asm/sigp.h
>> index 61d2c625..4eae95d0 100644
>> --- a/lib/s390x/asm/sigp.h
>> +++ b/lib/s390x/asm/sigp.h
>> @@ -49,13 +49,17 @@ static inline int sigp(uint16_t addr, uint8_t order, unsigned long parm,
>>   		       uint32_t *status)
>>   {
>>   	register unsigned long reg1 asm ("1") = parm;
>> +	uint64_t bogus_cc = SIGP_CC_NOT_OPERATIONAL;
>>   	int cc;
>>   
>>   	asm volatile(
>> +		"	tmll	%[bogus_cc],3\n"
>>   		"	sigp	%1,%2,0(%3)\n"
>>   		"	ipm	%0\n"
>>   		"	srl	%0,28\n"
>> -		: "=d" (cc), "+d" (reg1) : "d" (addr), "a" (order) : "cc");
>> +		: "=d" (cc), "+d" (reg1)
>> +		: "d" (addr), "a" (order), [bogus_cc] "d" (bogus_cc)
>> +		: "cc");
> 
> since you are doing changes in this inline asm, could you put names for
> all the parameters? that way it will be more consistent and readable.
> 

NACK, not in this series.

[kvm-unit-tests PATCH v2 2/5] lib: s390x: uv: Dirty CC before uvc execution

[Janosch Frank]

Dirtying the CC allows us to find missing CC changes.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 lib/s390x/asm/uv.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/s390x/asm/uv.h b/lib/s390x/asm/uv.h
index e9fb19af..611dcd3f 100644
--- a/lib/s390x/asm/uv.h
+++ b/lib/s390x/asm/uv.h
@@ -216,14 +216,16 @@ struct uv_cb_ssc {
 
 static inline int uv_call_once(unsigned long r1, unsigned long r2)
 {
+	uint64_t bogus_cc = 1;
 	int cc;
 
 	asm volatile(
+		"	tmll    %[bogus_cc],3\n"
 		"0:	.insn rrf,0xB9A40000,%[r1],%[r2],0,0\n"
 		"		ipm	%[cc]\n"
 		"		srl	%[cc],28\n"
 		: [cc] "=d" (cc)
-		: [r1] "a" (r1), [r2] "a" (r2)
+		: [r1] "a" (r1), [r2] "a" (r2), [bogus_cc] "d" (bogus_cc)
 		: "memory", "cc");
 
 	if (UVC_ERR_DEBUG && cc == 1)
-- 
2.40.1

Re: [kvm-unit-tests PATCH v2 2/5] lib: s390x: uv: Dirty CC before uvc execution

[Claudio Imbrenda]

On Wed, 31 Jan 2024 07:44:24 +0000
Janosch Frank <frankja@linux.ibm.com> wrote:

> Dirtying the CC allows us to find missing CC changes.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>

Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

> ---
>  lib/s390x/asm/uv.h | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/s390x/asm/uv.h b/lib/s390x/asm/uv.h
> index e9fb19af..611dcd3f 100644
> --- a/lib/s390x/asm/uv.h
> +++ b/lib/s390x/asm/uv.h
> @@ -216,14 +216,16 @@ struct uv_cb_ssc {
>  
>  static inline int uv_call_once(unsigned long r1, unsigned long r2)
>  {
> +	uint64_t bogus_cc = 1;
>  	int cc;
>  
>  	asm volatile(
> +		"	tmll    %[bogus_cc],3\n"
>  		"0:	.insn rrf,0xB9A40000,%[r1],%[r2],0,0\n"
>  		"		ipm	%[cc]\n"
>  		"		srl	%[cc],28\n"
>  		: [cc] "=d" (cc)
> -		: [r1] "a" (r1), [r2] "a" (r2)
> +		: [r1] "a" (r1), [r2] "a" (r2), [bogus_cc] "d" (bogus_cc)
>  		: "memory", "cc");
>  
>  	if (UVC_ERR_DEBUG && cc == 1)

[kvm-unit-tests PATCH v2 3/5] lib: s390x: css: Dirty CC before css instructions

[Janosch Frank]

Dirtying the CC allows us to find missing CC changes when css
instructions are emulated.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 lib/s390x/css.h | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/lib/s390x/css.h b/lib/s390x/css.h
index 0a19324b..504b3f14 100644
--- a/lib/s390x/css.h
+++ b/lib/s390x/css.h
@@ -147,14 +147,16 @@ static inline int ssch(unsigned long schid, struct orb *addr)
 static inline int stsch(unsigned long schid, struct schib *addr)
 {
 	register unsigned long reg1 asm ("1") = schid;
+	uint64_t bogus_cc = 1;
 	int cc;
 
 	asm volatile(
+		"       tmll    %[bogus_cc],3\n"
 		"	stsch	0(%3)\n"
 		"	ipm	%0\n"
 		"	srl	%0,28"
 		: "=d" (cc), "=m" (*addr)
-		: "d" (reg1), "a" (addr)
+		: "d" (reg1), "a" (addr), [bogus_cc] "d" (bogus_cc)
 		: "cc");
 	return cc;
 }
@@ -177,14 +179,16 @@ static inline int msch(unsigned long schid, struct schib *addr)
 static inline int tsch(unsigned long schid, struct irb *addr)
 {
 	register unsigned long reg1 asm ("1") = schid;
+	uint64_t bogus_cc = 2;
 	int cc;
 
 	asm volatile(
+		"       tmll    %[bogus_cc],3\n"
 		"	tsch	0(%3)\n"
 		"	ipm	%0\n"
 		"	srl	%0,28"
 		: "=d" (cc), "=m" (*addr)
-		: "d" (reg1), "a" (addr)
+		: "d" (reg1), "a" (addr), [bogus_cc] "d" (bogus_cc)
 		: "cc");
 	return cc;
 }
@@ -252,28 +256,32 @@ static inline int rsch(unsigned long schid)
 static inline int rchp(unsigned long chpid)
 {
 	register unsigned long reg1 asm("1") = chpid;
+	uint64_t bogus_cc = 1;
 	int cc;
 
 	asm volatile(
+		"       tmll    %[bogus_cc],3\n"
 		"	rchp\n"
 		"	ipm	%0\n"
 		"	srl	%0,28"
 		: "=d" (cc)
-		: "d" (reg1)
+		: "d" (reg1), [bogus_cc] "d" (bogus_cc)
 		: "cc");
 	return cc;
 }
 
 static inline int stcrw(uint32_t *crw)
 {
+	uint64_t bogus_cc = 1;
 	int cc;
 
 	asm volatile(
+		"       tmll    %[bogus_cc],3\n"
 		"	stcrw	%[crw]\n"
 		"	ipm	%[cc]\n"
 		"	srl	%[cc],28"
 		: [cc] "=d" (cc)
-		: [crw] "Q" (*crw)
+		: [crw] "Q" (*crw), [bogus_cc] "d" (bogus_cc)
 		: "cc", "memory");
 	return cc;
 }
-- 
2.40.1

[kvm-unit-tests PATCH v2 4/5] s390x: mvpg: Dirty CC before mvpg execution

[Janosch Frank]

Dirtying the CC allows us to find missing CC changes when mvpg is
emulated.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 s390x/mvpg.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/s390x/mvpg.c b/s390x/mvpg.c
index 296338d4..62d42e36 100644
--- a/s390x/mvpg.c
+++ b/s390x/mvpg.c
@@ -40,12 +40,14 @@ static uint8_t *fresh;
 static inline int mvpg(unsigned long r0, void *dest, void *src)
 {
 	register unsigned long reg0 asm ("0") = r0;
+	uint64_t bogus_cc = 3;
 	int cc;
 
-	asm volatile("	mvpg    %1,%2\n"
+	asm volatile("	tmll	%[bogus_cc],3\n"
+		     "	mvpg    %1,%2\n"
 		     "	ipm     %0\n"
 		     "	srl     %0,28"
-		: "=&d" (cc) : "a" (dest), "a" (src), "d" (reg0)
+		: "=&d" (cc) : "a" (dest), "a" (src), "d" (reg0), [bogus_cc] "d" (bogus_cc)
 		: "memory", "cc");
 	return cc;
 }
-- 
2.40.1

[kvm-unit-tests PATCH v2 5/5] s390x: sclp: Dirty CC before sclp execution

[Janosch Frank]

Dirtying the CC allows us to find missing CC changes when sclp is
emulated.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 s390x/sclp.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/s390x/sclp.c b/s390x/sclp.c
index ccbaa913..53fce0cf 100644
--- a/s390x/sclp.c
+++ b/s390x/sclp.c
@@ -399,6 +399,7 @@ out:
 static void test_instbits(void)
 {
 	SCCBHeader *h = (SCCBHeader *)pagebuf;
+	uint64_t bogus_cc = 1;
 	int cc;
 
 	sclp_mark_busy();
@@ -406,10 +407,12 @@ static void test_instbits(void)
 	sclp_setup_int();
 
 	asm volatile(
+		"	tmll	%[bogus_cc],3\n"
 		"       .insn   rre,0xb2204200,%1,%2\n"  /* servc %1,%2 */
 		"       ipm     %0\n"
 		"       srl     %0,28"
-		: "=&d" (cc) : "d" (valid_code), "a" (__pa(pagebuf))
+		: "=&d" (cc)
+		: "d" (valid_code), "a" (__pa(pagebuf)), [bogus_cc] "d" (bogus_cc)
 		: "cc", "memory");
 	/* No exception, but also no command accepted, so no interrupt is
 	 * expected. We need to clear the flag manually otherwise we will
-- 
2.40.1

Re: [kvm-unit-tests PATCH v2 0/5] s390x: Dirty cc before executing tested instructions

[Claudio Imbrenda]

On Wed, 31 Jan 2024 07:44:22 +0000
Janosch Frank <frankja@linux.ibm.com> wrote:

> A recent s390 KVM fixpatch [1] showed us that checking the cc is not
> enough when emulation code forgets to set the cc. There might just be
> the correct cc in the PSW which would make the cc check succeed.
> 
> This series intentionally dirties the cc for sigp, uvc, some io
> instructions and sclp to make cc setting errors more apparent. I had a
> cursory look through the tested instructions and those are the most
> prominent ones with defined cc values.
> 
> Since the issue appeared in PQAP my AP test series is now dependent on
> this series.
> 
> [1] https://lore.kernel.org/kvm/20231201181657.1614645-1-farman@linux.ibm.com/

whole series:
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>


> 
> v2:
> 	* Moved from spm to tmll (thanks Nina)
> 
> Janosch Frank (5):
>   lib: s390x: sigp: Dirty CC before sigp execution
>   lib: s390x: uv: Dirty CC before uvc execution
>   lib: s390x: css: Dirty CC before css instructions
>   s390x: mvpg: Dirty CC before mvpg execution
>   s390x: sclp: Dirty CC before sclp execution
> 
>  lib/s390x/asm/sigp.h |  6 +++++-
>  lib/s390x/asm/uv.h   |  4 +++-
>  lib/s390x/css.h      | 16 ++++++++++++----
>  s390x/mvpg.c         |  6 ++++--
>  s390x/sclp.c         |  5 ++++-
>  5 files changed, 28 insertions(+), 9 deletions(-)
>