[PATCH] powerpc32: memcpy: only use dcbz once cache is enabled

[PATCH] powerpc32: memcpy: only use dcbz once cache is enabled

[Christophe Leroy]

memcpy() uses instruction dcbz to speed up copy by not wasting time
loading cache line with data that will be overwritten.
Some platform like mpc52xx do no have cache active at startup and
can therefore not use memcpy(). Allthough no part of the code
explicitly uses memcpy(), GCC makes calls to it.

This patch modifies memcpy() such that at startup, the 'dcbz'
instruction is replaced by 'dcbt' which is harmless if cache is not
enabled, and which helps a bit (allthough not as much as dcbz) if
cache is already enabled.

Once the initial MMU is setup, in machine_init() we patch memcpy()
by replacing the temporary 'dcbt' by 'dcbz'

Reported-by: Michal Sojka <sojkam1@fel.cvut.cz>
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
@Michal, can you please test it ?

 arch/powerpc/kernel/setup_32.c | 12 ++++++++++++
 arch/powerpc/lib/copy_32.S     | 11 ++++++++++-
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/setup_32.c b/arch/powerpc/kernel/setup_32.c
index 07831ed..93715f3 100644
--- a/arch/powerpc/kernel/setup_32.c
+++ b/arch/powerpc/kernel/setup_32.c
@@ -39,6 +39,7 @@
 #include <asm/udbg.h>
 #include <asm/mmu_context.h>
 #include <asm/epapr_hcalls.h>
+#include <asm/code-patching.h>
 
 #define DBG(fmt...)
 
@@ -108,6 +109,15 @@ notrace unsigned long __init early_init(unsigned long dt_ptr)
 	return KERNELBASE + offset;
 }
 
+extern unsigned int ppc32_memcpy_dcbz;
+notrace void __init patch_memcpy(void)
+{
+	unsigned int instr = ppc32_memcpy_dcbz;
+
+	instr &= 0x001ff800; /* keep only RA and RB */
+	instr |= 0x7c0007ec; /* dcbz */
+	patch_instruction(&ppc32_memcpy_dcbz, instr);
+}
 
 /*
  * Find out what kind of machine we're on and save any data we need
@@ -122,6 +132,8 @@ notrace void __init machine_init(u64 dt_ptr)
 	/* Enable early debugging if any specified (see udbg.h) */
 	udbg_early_init();
 
+	patch_memcpy();
+
 	/* Do some early initialization based on the flat device tree */
 	early_init_devtree(__va(dt_ptr));
 
diff --git a/arch/powerpc/lib/copy_32.S b/arch/powerpc/lib/copy_32.S
index 2ef50c6..05b3096 100644
--- a/arch/powerpc/lib/copy_32.S
+++ b/arch/powerpc/lib/copy_32.S
@@ -172,7 +172,16 @@ _GLOBAL(memcpy)
 	mtctr	r0
 	beq	63f
 53:
-	dcbz	r11,r6
+	/*
+	 * During early init, cache might not be active yet, so dcbz cannot be
+	 * used. We put dcbt instead of dcbz. If cache is not active, it's just
+	 * like a not. If cache is active, at least it prefetchs the line to be
+	 * overwritten.
+	 * Will be replaced by dcbz in machine_init()
+	 */
+_GLOBAL(ppc32_memcpy_dcbz)
+	dcbt	r11,r6
+
 	COPY_16_BYTES
 #if L1_CACHE_BYTES >= 32
 	COPY_16_BYTES
-- 
2.1.0

Re: [PATCH] powerpc32: memcpy: only use dcbz once cache is enabled

[Michal Sojka]

On Mon, Sep 07 2015, Christophe Leroy wrote:
> memcpy() uses instruction dcbz to speed up copy by not wasting time
> loading cache line with data that will be overwritten.
> Some platform like mpc52xx do no have cache active at startup and
> can therefore not use memcpy(). Allthough no part of the code
> explicitly uses memcpy(), GCC makes calls to it.
>
> This patch modifies memcpy() such that at startup, the 'dcbz'
> instruction is replaced by 'dcbt' which is harmless if cache is not
> enabled, and which helps a bit (allthough not as much as dcbz) if
> cache is already enabled.
>
> Once the initial MMU is setup, in machine_init() we patch memcpy()
> by replacing the temporary 'dcbt' by 'dcbz'
>
> Reported-by: Michal Sojka <sojkam1@fel.cvut.cz>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> @Michal, can you please test it ?

Yes, it works.

Tested-by: Michal Sojka <sojkam1@fel.cvut.cz>

-Michal

RE: [PATCH] powerpc32: memcpy: only use dcbz once cache is enabled

[David Laight]

RnJvbTogQ2hyaXN0b3BoZSBMZXJveQ0KPiBTZW50OiAwNyBTZXB0ZW1iZXIgMjAxNSAxNToyNQ0K
Li4uDQo+IGRpZmYgLS1naXQgYS9hcmNoL3Bvd2VycGMvbGliL2NvcHlfMzIuUyBiL2FyY2gvcG93
ZXJwYy9saWIvY29weV8zMi5TDQo+IGluZGV4IDJlZjUwYzYuLjA1YjMwOTYgMTAwNjQ0DQo+IC0t
LSBhL2FyY2gvcG93ZXJwYy9saWIvY29weV8zMi5TDQo+ICsrKyBiL2FyY2gvcG93ZXJwYy9saWIv
Y29weV8zMi5TDQo+IEBAIC0xNzIsNyArMTcyLDE2IEBAIF9HTE9CQUwobWVtY3B5KQ0KPiAgCW10
Y3RyCXIwDQo+ICAJYmVxCTYzZg0KPiAgNTM6DQo+IC0JZGNieglyMTEscjYNCj4gKwkvKg0KPiAr
CSAqIER1cmluZyBlYXJseSBpbml0LCBjYWNoZSBtaWdodCBub3QgYmUgYWN0aXZlIHlldCwgc28g
ZGNieiBjYW5ub3QgYmUNCj4gKwkgKiB1c2VkLiBXZSBwdXQgZGNidCBpbnN0ZWFkIG9mIGRjYnou
IElmIGNhY2hlIGlzIG5vdCBhY3RpdmUsIGl0J3MganVzdA0KPiArCSAqIGxpa2UgYSBub3QuIElm
IGNhY2hlIGlzIGFjdGl2ZSwgYXQgbGVhc3QgaXQgcHJlZmV0Y2hzIHRoZSBsaW5lIHRvIGJlDQog
ICAgICAgICAgICAgICAgXl5eIG5vcCA/Pw0KDQoJRGF2aWQNCg0KPiArCSAqIG92ZXJ3cml0dGVu
Lg0KPiArCSAqIFdpbGwgYmUgcmVwbGFjZWQgYnkgZGNieiBpbiBtYWNoaW5lX2luaXQoKQ0KPiAr
CSAqLw0KPiArX0dMT0JBTChwcGMzMl9tZW1jcHlfZGNieikNCj4gKwlkY2J0CXIxMSxyNg0KPiAr
DQo+ICAJQ09QWV8xNl9CWVRFUw0KPiAgI2lmIEwxX0NBQ0hFX0JZVEVTID49IDMyDQo+ICAJQ09Q
WV8xNl9CWVRFUw0KPiAtLQ0KPiAyLjEuMA0KDQo=

Re: [PATCH] powerpc32: memcpy: only use dcbz once cache is enabled

[Michael Ellerman]

On Mon, 2015-09-07 at 16:24 +0200, Christophe Leroy wrote:
> memcpy() uses instruction dcbz to speed up copy by not wasting time
> loading cache line with data that will be overwritten.
> Some platform like mpc52xx do no have cache active at startup and
> can therefore not use memcpy(). Allthough no part of the code
> explicitly uses memcpy(), GCC makes calls to it.
> 
> This patch modifies memcpy() such that at startup, the 'dcbz'
> instruction is replaced by 'dcbt' which is harmless if cache is not
> enabled, and which helps a bit (allthough not as much as dcbz) if
> cache is already enabled.
> 
> Once the initial MMU is setup, in machine_init() we patch memcpy()
> by replacing the temporary 'dcbt' by 'dcbz'
> 
> Reported-by: Michal Sojka <sojkam1@fel.cvut.cz>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>


Is there some reason you can't use the normal cpu feature sections? See
arch/powerpc/lib/memcpy_64.S for an example of what I mean.

cheers