diff mbox series

[1/5] watchdog: diag288_wdt: get rid of register asm

Message ID 20230203073958.1585738-2-egorenar@linux.ibm.com
State New
Headers show
Series diag288 watchdog fixes and improvements | expand

Commit Message

Alexander Egorenkov Feb. 3, 2023, 7:39 a.m. UTC 
Using register asm statements has been proven to be very error prone,
especially when using code instrumentation where gcc may add function
calls, which clobbers register contents in an unexpected way.

Therefore, get rid of register asm statements in watchdog code, and make
sure this bug class cannot happen.

Moreover, remove the register r1 from the clobber list because this
register is not changed by DIAG 288.

Reviewed-by: Heiko Carstens <hca@linux.ibm.com>
Signed-off-by: Alexander Egorenkov <egorenar@linux.ibm.com>
---
 drivers/watchdog/diag288_wdt.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

Comments

Guenter Roeck Feb. 3, 2023, 6:17 p.m. UTC | #1 
On Fri, Feb 03, 2023 at 08:39:54AM +0100, Alexander Egorenkov wrote:
> Using register asm statements has been proven to be very error prone,
> especially when using code instrumentation where gcc may add function
> calls, which clobbers register contents in an unexpected way.
> 
> Therefore, get rid of register asm statements in watchdog code, and make
> sure this bug class cannot happen.
> 
> Moreover, remove the register r1 from the clobber list because this
> register is not changed by DIAG 288.
> 
> Reviewed-by: Heiko Carstens <hca@linux.ibm.com>
> Signed-off-by: Alexander Egorenkov <egorenar@linux.ibm.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/watchdog/diag288_wdt.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/watchdog/diag288_wdt.c b/drivers/watchdog/diag288_wdt.c
> index 6ca5d9515d85..07ebbb709af4 100644
> --- a/drivers/watchdog/diag288_wdt.c
> +++ b/drivers/watchdog/diag288_wdt.c
> @@ -73,20 +73,19 @@ MODULE_ALIAS("vmwatchdog");
>  static int __diag288(unsigned int func, unsigned int timeout,
>  		     unsigned long action, unsigned int len)
>  {
> -	register unsigned long __func asm("2") = func;
> -	register unsigned long __timeout asm("3") = timeout;
> -	register unsigned long __action asm("4") = action;
> -	register unsigned long __len asm("5") = len;
> +	union register_pair r1 = { .even = func, .odd = timeout, };
> +	union register_pair r3 = { .even = action, .odd = len, };
>  	int err;
>  
>  	err = -EINVAL;
>  	asm volatile(
> -		"	diag	%1, %3, 0x288\n"
> -		"0:	la	%0, 0\n"
> +		"	diag	%[r1],%[r3],0x288\n"
> +		"0:	la	%[err],0\n"
>  		"1:\n"
>  		EX_TABLE(0b, 1b)
> -		: "+d" (err) : "d"(__func), "d"(__timeout),
> -		  "d"(__action), "d"(__len) : "1", "cc", "memory");
> +		: [err] "+d" (err)
> +		: [r1] "d" (r1.pair), [r3] "d" (r3.pair)
> +		: "cc", "memory");
>  	return err;
>  }
>
diff mbox series

Patch

diff --git a/drivers/watchdog/diag288_wdt.c b/drivers/watchdog/diag288_wdt.c
index 6ca5d9515d85..07ebbb709af4 100644
--- a/drivers/watchdog/diag288_wdt.c
+++ b/drivers/watchdog/diag288_wdt.c
@@ -73,20 +73,19 @@  MODULE_ALIAS("vmwatchdog");
 static int __diag288(unsigned int func, unsigned int timeout,
 		     unsigned long action, unsigned int len)
 {
-	register unsigned long __func asm("2") = func;
-	register unsigned long __timeout asm("3") = timeout;
-	register unsigned long __action asm("4") = action;
-	register unsigned long __len asm("5") = len;
+	union register_pair r1 = { .even = func, .odd = timeout, };
+	union register_pair r3 = { .even = action, .odd = len, };
 	int err;
 
 	err = -EINVAL;
 	asm volatile(
-		"	diag	%1, %3, 0x288\n"
-		"0:	la	%0, 0\n"
+		"	diag	%[r1],%[r3],0x288\n"
+		"0:	la	%[err],0\n"
 		"1:\n"
 		EX_TABLE(0b, 1b)
-		: "+d" (err) : "d"(__func), "d"(__timeout),
-		  "d"(__action), "d"(__len) : "1", "cc", "memory");
+		: [err] "+d" (err)
+		: [r1] "d" (r1.pair), [r3] "d" (r3.pair)
+		: "cc", "memory");
 	return err;
 }