diff mbox series

[kvm-unit-tests] lib: s390x: css: Name inline assembly arguments and clean them up

Message ID 20250204100339.28158-1-frankja@linux.ibm.com (mailing list archive)
State New
Headers show
Series [kvm-unit-tests] lib: s390x: css: Name inline assembly arguments and clean them up | expand

Commit Message

Janosch Frank Feb. 4, 2025, 9:51 a.m. UTC 
Less need to count the operands makes the code easier to read.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---

This one has been gathering dust for a while.
rfc->v1: Moved to Q constraint (thanks Heiko)

---
 lib/s390x/css.h | 76 ++++++++++++++++++++++++-------------------------
 1 file changed, 38 insertions(+), 38 deletions(-)

Comments

Nico Boehr Feb. 4, 2025, 12:55 p.m. UTC | #1 
On Tue Feb 4, 2025 at 10:51 AM CET, Janosch Frank wrote:
> @@ -215,9 +215,9 @@ static inline int xsch(unsigned long schid)
>  
>  	asm volatile(
>  		"	xsch\n"
> -		"	ipm	%0\n"
> -		"	srl	%0,28"
> -		: "=d" (cc)
> +		"	ipm	%[cc]\n"
> +		"	srl	%cc,28"

Should this be:
		"	srl	%[cc],28"
instead?

With that fixed (if it needs fixing):

Reviewed-by: Nico Boehr <nrb@linux.ibm.com>
Janosch Frank Feb. 5, 2025, 8:17 a.m. UTC | #2 
On 2/4/25 1:55 PM, Nico Boehr wrote:
> On Tue Feb 4, 2025 at 10:51 AM CET, Janosch Frank wrote:
>> @@ -215,9 +215,9 @@ static inline int xsch(unsigned long schid)
>>   
>>   	asm volatile(
>>   		"	xsch\n"
>> -		"	ipm	%0\n"
>> -		"	srl	%0,28"
>> -		: "=d" (cc)
>> +		"	ipm	%[cc]\n"
>> +		"	srl	%cc,28"
> 
> Should this be:
> 		"	srl	%[cc],28"
> instead?
> 
> With that fixed (if it needs fixing):
> 
> Reviewed-by: Nico Boehr <nrb@linux.ibm.com>
> 

Thanks!
Fixed and applied.
Claudio Imbrenda Feb. 5, 2025, 10:25 a.m. UTC | #3 
On Tue,  4 Feb 2025 09:51:33 +0000
Janosch Frank <frankja@linux.ibm.com> wrote:

> Less need to count the operands makes the code easier to read.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
> 
> This one has been gathering dust for a while.
> rfc->v1: Moved to Q constraint (thanks Heiko)
> 
> ---

[...]

>  	asm volatile(" .insn   rre,0xb25f0000,%2,0\n"
> -		     " ipm     %0\n"
> -		     " srl     %0,28\n"
> -		     : "=d" (cc), "=m" (p)
> +		     " ipm     %[cc]\n"
> +		     " srl     %[cc],28\n"
> +		     : [cc] "=d" (cc), "=m" (p)
>  		     : "d" (p), "m" (p)

this bit (which you did not touch) is actually the most confusing to me.
what's the point of separately specifying both "d" and "m" constraints
for (p) ? (and it also has a "=m" in the output clobberlist)

>  		     : "cc");
>
Janosch Frank Feb. 5, 2025, 12:08 p.m. UTC | #4 
On 2/5/25 11:25 AM, Claudio Imbrenda wrote:
> On Tue,  4 Feb 2025 09:51:33 +0000
> Janosch Frank <frankja@linux.ibm.com> wrote:
> 
>> Less need to count the operands makes the code easier to read.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>
>> This one has been gathering dust for a while.
>> rfc->v1: Moved to Q constraint (thanks Heiko)
>>
>> ---
> 
> [...]
> 
>>   	asm volatile(" .insn   rre,0xb25f0000,%2,0\n"
>> -		     " ipm     %0\n"
>> -		     " srl     %0,28\n"
>> -		     : "=d" (cc), "=m" (p)
>> +		     " ipm     %[cc]\n"
>> +		     " srl     %[cc],28\n"
>> +		     : [cc] "=d" (cc), "=m" (p)
>>   		     : "d" (p), "m" (p)
> 
> this bit (which you did not touch) is actually the most confusing to me.
> what's the point of separately specifying both "d" and "m" constraints
> for (p) ? (and it also has a "=m" in the output clobberlist)

I'll add a second patch to bring this into line with the kernel's 
ioasm.c implementation.
diff mbox series

Patch

diff --git a/lib/s390x/css.h b/lib/s390x/css.h
index 504b3f14..42c5830c 100644
--- a/lib/s390x/css.h
+++ b/lib/s390x/css.h
@@ -135,11 +135,11 @@  static inline int ssch(unsigned long schid, struct orb *addr)
 	int cc;
 
 	asm volatile(
-		"	ssch	0(%2)\n"
-		"	ipm	%0\n"
-		"	srl	%0,28\n"
-		: "=d" (cc)
-		: "d" (reg1), "a" (addr), "m" (*addr)
+		"	ssch	%[addr]\n"
+		"	ipm	%[cc]\n"
+		"	srl	%[cc],28\n"
+		: [cc] "=d" (cc)
+		: "d" (reg1), [addr] "Q" (*addr)
 		: "cc", "memory");
 	return cc;
 }
@@ -152,11 +152,11 @@  static inline int stsch(unsigned long schid, struct schib *addr)
 
 	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), [bogus_cc] "d" (bogus_cc)
+		"	stsch	%[addr]\n"
+		"	ipm	%[cc]\n"
+		"	srl	%[cc],28"
+		: [cc] "=d" (cc), [addr] "=Q" (*addr)
+		: "d" (reg1), [bogus_cc] "d" (bogus_cc)
 		: "cc");
 	return cc;
 }
@@ -167,11 +167,11 @@  static inline int msch(unsigned long schid, struct schib *addr)
 	int cc;
 
 	asm volatile(
-		"	msch	0(%3)\n"
-		"	ipm	%0\n"
-		"	srl	%0,28"
-		: "=d" (cc)
-		: "d" (reg1), "m" (*addr), "a" (addr)
+		"	msch	%[addr]\n"
+		"	ipm	%[cc]\n"
+		"	srl	%[cc],28"
+		: [cc] "=d" (cc)
+		: "d" (reg1), [addr] "Q" (*addr)
 		: "cc");
 	return cc;
 }
@@ -184,11 +184,11 @@  static inline int tsch(unsigned long schid, struct irb *addr)
 
 	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), [bogus_cc] "d" (bogus_cc)
+		"	tsch	%[addr]\n"
+		"	ipm	%[cc]\n"
+		"	srl	%[cc],28"
+		: [cc] "=d" (cc), [addr] "=Q" (*addr)
+		: "d" (reg1), [bogus_cc] "d" (bogus_cc)
 		: "cc");
 	return cc;
 }
@@ -200,9 +200,9 @@  static inline int hsch(unsigned long schid)
 
 	asm volatile(
 		"	hsch\n"
-		"	ipm	%0\n"
-		"	srl	%0,28"
-		: "=d" (cc)
+		"	ipm	%[cc]\n"
+		"	srl	%[cc],28"
+		: [cc] "=d" (cc)
 		: "d" (reg1)
 		: "cc");
 	return cc;
@@ -215,9 +215,9 @@  static inline int xsch(unsigned long schid)
 
 	asm volatile(
 		"	xsch\n"
-		"	ipm	%0\n"
-		"	srl	%0,28"
-		: "=d" (cc)
+		"	ipm	%[cc]\n"
+		"	srl	%cc,28"
+		: [cc] "=d" (cc)
 		: "d" (reg1)
 		: "cc");
 	return cc;
@@ -230,9 +230,9 @@  static inline int csch(unsigned long schid)
 
 	asm volatile(
 		"	csch\n"
-		"	ipm	%0\n"
-		"	srl	%0,28"
-		: "=d" (cc)
+		"	ipm	%[cc]\n"
+		"	srl	%[cc],28"
+		: [cc] "=d" (cc)
 		: "d" (reg1)
 		: "cc");
 	return cc;
@@ -245,9 +245,9 @@  static inline int rsch(unsigned long schid)
 
 	asm volatile(
 		"	rsch\n"
-		"	ipm	%0\n"
-		"	srl	%0,28"
-		: "=d" (cc)
+		"	ipm	%[cc]\n"
+		"	srl	%[cc],28"
+		: [cc] "=d" (cc)
 		: "d" (reg1)
 		: "cc");
 	return cc;
@@ -262,9 +262,9 @@  static inline int rchp(unsigned long chpid)
 	asm volatile(
 		"       tmll    %[bogus_cc],3\n"
 		"	rchp\n"
-		"	ipm	%0\n"
-		"	srl	%0,28"
-		: "=d" (cc)
+		"	ipm	%[cc]\n"
+		"	srl	%[cc],28"
+		: [cc] "=d" (cc)
 		: "d" (reg1), [bogus_cc] "d" (bogus_cc)
 		: "cc");
 	return cc;
@@ -369,9 +369,9 @@  static inline int _chsc(void *p)
 	int cc;
 
 	asm volatile(" .insn   rre,0xb25f0000,%2,0\n"
-		     " ipm     %0\n"
-		     " srl     %0,28\n"
-		     : "=d" (cc), "=m" (p)
+		     " ipm     %[cc]\n"
+		     " srl     %[cc],28\n"
+		     : [cc] "=d" (cc), "=m" (p)
 		     : "d" (p), "m" (p)
 		     : "cc");