[Nios2-dev] [PATCH v2] nios2: memset: use the right constraint modifier for the %4 output operand
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
%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
@@ -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 */
More information about the Nios2-dev