• Romain Perier's avatar
    nios2: memset: use the right constraint modifier for the %4 output operand · a8950e49
    Romain Perier authored
    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: default avatarRomain Perier <romain.perier@free-electrons.com>
    Acked-by: default avatarMarek Vasut <marex@denx.de>
    Acked-by: default avatarLey Foon Tan <lftan@altera.com>
    a8950e49
memset.c 1.84 KB