[PATCH] MIPS: Add 'memory' clobber to csum_ipv6_magic() inline assembler

[PATCH] MIPS: Add 'memory' clobber to csum_ipv6_magic() inline assembler

[Guenter Roeck]

After 'lib: checksum: Use aligned accesses for ip_fast_csum and
csum_ipv6_magic tests' was applied, the test_csum_ipv6_magic unit test
started failing for all mips platforms, both little and bit endian.
Oddly enough, adding debug code into test_csum_ipv6_magic() made the
problem disappear.

The gcc manual says:

"The "memory" clobber tells the compiler that the assembly code performs
 memory reads or writes to items other than those listed in the input
 and output operands (for example, accessing the memory pointed to by one
 of the input parameters)
"

This is definitely the case for csum_ipv6_magic(). Indeed, adding the
'memory' clobber fixes the problem.

Cc: Charlie Jenkins <charlie@rivosinc.com>
Cc: Palmer Dabbelt <palmer@rivosinc.com>
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 arch/mips/include/asm/checksum.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/mips/include/asm/checksum.h b/arch/mips/include/asm/checksum.h
index 4044eaf989ac..0921ddda11a4 100644
--- a/arch/mips/include/asm/checksum.h
+++ b/arch/mips/include/asm/checksum.h
@@ -241,7 +241,8 @@ static __inline__ __sum16 csum_ipv6_magic(const struct in6_addr *saddr,
 	"	.set	pop"
 	: "=&r" (sum), "=&r" (tmp)
 	: "r" (saddr), "r" (daddr),
-	  "0" (htonl(len)), "r" (htonl(proto)), "r" (sum));
+	  "0" (htonl(len)), "r" (htonl(proto)), "r" (sum)
+	: "memory");
 
 	return csum_fold(sum);
 }
-- 
2.39.2

Re: [PATCH] MIPS: Add 'memory' clobber to csum_ipv6_magic() inline assembler

[Charlie Jenkins]

On Sun, Feb 11, 2024 at 08:08:37AM -0800, Guenter Roeck wrote:
> After 'lib: checksum: Use aligned accesses for ip_fast_csum and
> csum_ipv6_magic tests' was applied, the test_csum_ipv6_magic unit test
> started failing for all mips platforms, both little and bit endian.
> Oddly enough, adding debug code into test_csum_ipv6_magic() made the
> problem disappear.
> 
> The gcc manual says:
> 
> "The "memory" clobber tells the compiler that the assembly code performs
>  memory reads or writes to items other than those listed in the input
>  and output operands (for example, accessing the memory pointed to by one
>  of the input parameters)
> "
> 
> This is definitely the case for csum_ipv6_magic(). Indeed, adding the
> 'memory' clobber fixes the problem.
> 
> Cc: Charlie Jenkins <charlie@rivosinc.com>
> Cc: Palmer Dabbelt <palmer@rivosinc.com>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
>  arch/mips/include/asm/checksum.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/mips/include/asm/checksum.h b/arch/mips/include/asm/checksum.h
> index 4044eaf989ac..0921ddda11a4 100644
> --- a/arch/mips/include/asm/checksum.h
> +++ b/arch/mips/include/asm/checksum.h
> @@ -241,7 +241,8 @@ static __inline__ __sum16 csum_ipv6_magic(const struct in6_addr *saddr,
>  	"	.set	pop"
>  	: "=&r" (sum), "=&r" (tmp)
>  	: "r" (saddr), "r" (daddr),
> -	  "0" (htonl(len)), "r" (htonl(proto)), "r" (sum));
> +	  "0" (htonl(len)), "r" (htonl(proto)), "r" (sum)
> +	: "memory");
>  
>  	return csum_fold(sum);
>  }
> -- 
> 2.39.2
> 

Thank you for looking into this. It fixed the failure on my mips64el
qemu setup.

Reviewed-by: Charlie Jenkins <charlie@rivosinc.com>

Re: [PATCH] MIPS: Add 'memory' clobber to csum_ipv6_magic() inline assembler

[Thomas Bogendoerfer]

On Sun, Feb 11, 2024 at 08:08:37AM -0800, Guenter Roeck wrote:
> After 'lib: checksum: Use aligned accesses for ip_fast_csum and
> csum_ipv6_magic tests' was applied, the test_csum_ipv6_magic unit test
> started failing for all mips platforms, both little and bit endian.
> Oddly enough, adding debug code into test_csum_ipv6_magic() made the
> problem disappear.
> 
> The gcc manual says:
> 
> "The "memory" clobber tells the compiler that the assembly code performs
>  memory reads or writes to items other than those listed in the input
>  and output operands (for example, accessing the memory pointed to by one
>  of the input parameters)
> "
> 
> This is definitely the case for csum_ipv6_magic(). Indeed, adding the
> 'memory' clobber fixes the problem.
> 
> Cc: Charlie Jenkins <charlie@rivosinc.com>
> Cc: Palmer Dabbelt <palmer@rivosinc.com>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
>  arch/mips/include/asm/checksum.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/mips/include/asm/checksum.h b/arch/mips/include/asm/checksum.h
> index 4044eaf989ac..0921ddda11a4 100644
> --- a/arch/mips/include/asm/checksum.h
> +++ b/arch/mips/include/asm/checksum.h
> @@ -241,7 +241,8 @@ static __inline__ __sum16 csum_ipv6_magic(const struct in6_addr *saddr,
>  	"	.set	pop"
>  	: "=&r" (sum), "=&r" (tmp)
>  	: "r" (saddr), "r" (daddr),
> -	  "0" (htonl(len)), "r" (htonl(proto)), "r" (sum));
> +	  "0" (htonl(len)), "r" (htonl(proto)), "r" (sum)
> +	: "memory");
>  
>  	return csum_fold(sum);
>  }
> -- 
> 2.39.2

applied to mips-fixes.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]