Re: [PATCH] powerpc: Remove eieio in _memcpy_fromio

[Christophe Leroy <christophe.leroy@csgroup.eu>]

Le 28/01/2025 à 14:57, Julian Vetter a écrit :
> Remove the eieio() calls in _memcpy_fromio, to bring its implementation
> closer to the one from lib/iomem_copy.c. These eieio() calls don't seem
> to be necessary, because the _memcpy_toio completely omits them. Also
> the legacy code from ppc was not doing them.

What do you mean exactly by "legacy code" ?

As far as I can see they were already there before commit 68a64357d15a 
("[POWERPC] Merge 32 and 64 bits asm-powerpc/io.h"):

-static inline void eeh_memcpy_fromio(void *dest, const volatile void 
__iomem *src,
+static inline void eeh_memcpy_fromio(void *dest, const
+				     volatile void __iomem *src,
  				     unsigned long n)
  {
-	void *vsrc = (void __force *) src;
-	void *destsave = dest;
-	unsigned long nsave = n;
-
-	__asm__ __volatile__ ("sync" : : : "memory");
-	while(n && (!EEH_CHECK_ALIGN(vsrc, 4) || !EEH_CHECK_ALIGN(dest, 4))) {
-		*((u8 *)dest) = *((volatile u8 *)vsrc);
-		__asm__ __volatile__ ("eieio" : : : "memory");
-		vsrc++;
-		dest++;
-		n--;
-	}
-	while(n > 4) {
-		*((u32 *)dest) = *((volatile u32 *)vsrc);
-		__asm__ __volatile__ ("eieio" : : : "memory");
-		vsrc += 4;
-		dest += 4;
-		n -= 4;
-	}
-	while(n) {
-		*((u8 *)dest) = *((volatile u8 *)vsrc);
-		__asm__ __volatile__ ("eieio" : : : "memory");
-		vsrc++;
-		dest++;
-		n--;
-	}
-	__asm__ __volatile__ ("sync" : : : "memory");
+	_memcpy_fromio(dest, src, n);

  	/* Look for ffff's here at dest[n].  Assume that at least 4 bytes
  	 * were copied. Check all four bytes.
  	 */
-	if ((nsave >= 4) &&
-		(EEH_POSSIBLE_ERROR((*((u32 *) destsave+nsave-4)), u32))) {
-		eeh_check_failure(src, (*((u32 *) destsave+nsave-4)));
-	}
+	if (n >= 4 && EEH_POSSIBLE_ERROR(*((u32 *)(dest + n - 4)), u32))
+		eeh_check_failure(src, *((u32 *)(dest + n - 4)));
  }



> 
> Signed-off-by: Julian Vetter <julian@outer-limits.org>
> ---
>   arch/powerpc/kernel/io.c | 3 ---
>   1 file changed, 3 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/io.c b/arch/powerpc/kernel/io.c
> index 6af535905984..81e5d54260a1 100644
> --- a/arch/powerpc/kernel/io.c
> +++ b/arch/powerpc/kernel/io.c
> @@ -155,21 +155,18 @@ void _memcpy_fromio(void *dest, const volatile void __iomem *src,
>          __asm__ __volatile__ ("sync" : : : "memory");
>          while(n && (!IO_CHECK_ALIGN(vsrc, 4) || !IO_CHECK_ALIGN(dest, 4))) {
>                  *((u8 *)dest) = *((volatile u8 *)vsrc);
> -               eieio();
>                  vsrc++;
>                  dest++;
>                  n--;
>          }
>          while(n >= 4) {
>                  *((u32 *)dest) = *((volatile u32 *)vsrc);
> -               eieio();
>                  vsrc += 4;
>                  dest += 4;
>                  n -= 4;
>          }
>          while(n) {
>                  *((u8 *)dest) = *((volatile u8 *)vsrc);
> -               eieio();
>                  vsrc++;
>                  dest++;
>                  n--;
> --
> 2.34.1
>