[Nios2-dev] [PATCH v2] nios2: memset: use the right constraint modifier for the %4 output operand

Romain Perier romain.perier at free-electrons.com
Wed Apr 13 15:01:38 UTC 2016


Depending on the size of the area to be memset'ed, the nios2 memset
implementation either uses a naive loop (for buffers smaller or equal than
8 bytes) or a more optimized implementation (for buffers larger than 8
bytes). This implementation does 4-byte stores rather than 1-byte stores
to speed up memset.

However, we discovered that on our nios2 platform, memset() was not
properly setting the buffer to the expected value. A memset of 0xff
would not set the entire buffer to 0xff, but to:

0xff 0x00 0xff 0x00 0xff 0x00 0xff 0x00 ...

Which is obviously incorrect. Our investigation has revealed that the
problem lies in the incorrect constraints used in the inline assembly.

The following piece of assembly, from the nios2 memset implementation,
is supposed to create a 4-byte value that repeats 4 times the 1-byte
pattern passed as memset argument:

/* fill8 %3, %5 (c & 0xff) */
"       slli    %4, %5, 8\n"
"       or      %4, %4, %5\n"
"       slli    %3, %4, 16\n"
"       or      %3, %3, %4\n"

However, depending on the compiler and optimization level, this code
might be compiled as:

34:	280a923a 	slli	r5,r5,8
38:	294ab03a 	or	r5,r5,r5
3c:	2808943a 	slli	r4,r5,16
40:	2148b03a 	or	r4,r4,r5

This is wrong because r5 gets used both for %5 and %4, which leads to the
final pattern stored in r4 to be 0xff00ff00 rather than the expected
0xffffffff.

%4 is defined with the "=r" constraint, i.e as an output operand. However,
as explained in http://www.ethernut.de/en/documents/arm-inline-asm.html,
this does not prevent gcc from using the same register for an output
operand (%4) and input operand (%5). By using the constraint modifier '&',
we indicate that the register should be used for output only. With this
change, we get the following assembly output:

34:	2810923a 	slli	r8,r5,8
38:	4150b03a 	or	r8,r8,r5
3c:	400e943a 	slli	r7,r8,16
40:	3a0eb03a 	or	r7,r7,r8

Which correctly produces the 0xffffffff pattern when 0xff is passed as
the memset() pattern.

It is worth mentioning the observed consequence of this bug: we were
hitting the kernel BUG() in mm/bootmem.c:__free() that verifies when
marking a page as free that it was previously marked as occupied (i.e
that the bit was set to 1). The entire bootmem bitmap is set to 0xff
bit via a memset() during the bootmem initialization. The
bootmem_free() call right after the initialization was finding some
bits to be set to 0, which didn't make sense since the bitmap has just
been memset'ed to 0xff. Except that due to the bug explained above, the
bitmap was in fact initialized to 0xff00ff00.

Thanks to Marek Vasut for his help and feedback.

Signed-off-by: Romain Perier <romain.perier at free-electrons.com>
---

Changes in v2:
 - Fixed "we discovered than" -> "we discovered that"
 - Fixed warnings for scripts/checkpatch.pl

 arch/nios2/lib/memset.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/nios2/lib/memset.c b/arch/nios2/lib/memset.c
index c2cfcb1..2fcefe7 100644
--- a/arch/nios2/lib/memset.c
+++ b/arch/nios2/lib/memset.c
@@ -68,7 +68,7 @@ void *memset(void *s, int c, size_t count)
 		  "=r" (charcnt),	/* %1  Output */
 		  "=r" (dwordcnt),	/* %2  Output */
 		  "=r" (fill8reg),	/* %3  Output */
-		  "=r" (wrkrega)	/* %4  Output */
+		  "=&r" (wrkrega)	/* %4  Output only */
 		: "r" (c),		/* %5  Input */
 		  "0" (s),		/* %0  Input/Output */
 		  "1" (count)		/* %1  Input/Output */
-- 
2.8.0



More information about the Nios2-dev mailing list