Skip to content
  • Paul Burton's avatar
    MIPS: Fix cmpxchg on 32b signed ints for 64b kernel with !kernel_uses_llsc · 133d68e0
    Paul Burton authored
    Commit 8263db4d
    
     ("MIPS: cmpxchg: Implement __cmpxchg() as a
    function") refactored our implementation of __cmpxchg() to be a function
    rather than a macro, with the aim of making it easier to read & modify.
    Unfortunately the commit breaks use of cmpxchg() for signed 32 bit
    values when we have a 64 bit kernel with kernel_uses_llsc == false,
    because:
    
     - In cmpxchg_local() we cast the old value to the type the pointer
       points to, and then to an unsigned long. If the pointer points to a
       signed type smaller than 64 bits then the old value will be sign
       extended to 64 bits. That is, bits beyond the size of the pointed to
       type will be set to 1 if the old value is negative. In the case of a
       signed 32 bit integer with a negative value, bits 63:32 will all be
       set.
    
     - In __cmpxchg_asm() we load the value from memory, ie. dereference the
       pointer, and store the value as an unsigned integer (__ret) whose
       size matches the pointer. For a 32 bit cmpxchg() this means we store
       the value in a u32, because the pointer provided to __cmpxchg_asm()
       by __cmpxchg() is of type volatile u32 *.
    
     - __cmpxchg_asm() then checks whether the value in memory (__ret)
       matches the provided old value, by comparing the two values. This
       results in the u32 being promoted to a 64 bit unsigned long to match
       the old argument - however because both types are unsigned the value
       is zero extended, which does not match the sign extension performed
       on the old value in cmpxchg_local() earlier.
    
    This mismatch means that unfortunate cmpxchg() calls can incorrectly
    fail for 64 bit kernels with kernel_uses_llsc == false. This is the case
    on at least non-SMP Cavium Octeon kernels, which hardcode
    kernel_uses_llsc in their cpu-feature-overrides.h header. Using a
    v4.13-rc7 kernel configured using cavium_octeon_defconfig with SMP
    manually disabled, this presents itself as oddity when we reach
    userland - for example:
    
      can't run '/bin/mount': Text file busy
      can't run '/bin/mkdir': Text file busy
      can't run '/bin/mkdir': Text file busy
      can't run '/bin/mount': Text file busy
      can't run '/bin/hostname': Text file busy
      can't run '/etc/init.d/rcS': Text file busy
      can't run '/sbin/getty': Text file busy
      can't run '/sbin/getty': Text file busy
    
    It appears that some part of the init process, which is in this case
    buildroot's busybox init, is running successfully. It never manages to
    reach the login prompt though, and complains about /sbin/getty being
    busy repeatedly and indefinitely.
    
    Fix this by casting the old value provided to __cmpxchg_asm() to an
    appropriately sized unsigned integer, such that we consistently
    zero-extend avoiding the mismatch. The __cmpxchg_small() case for 8 & 16
    bit values is unaffected because __cmpxchg_small() already masks
    provided values appropriately.
    
    Signed-off-by: default avatarPaul Burton <paul.burton@imgtec.com>
    Fixes: 8263db4d ("MIPS: cmpxchg: Implement __cmpxchg() as a function")
    Cc: linux-mips@linux-mips.org
    Patchwork: https://patchwork.linux-mips.org/patch/17226/
    
    
    Cc: linux-mips@linux-mips.org
    Signed-off-by: default avatarRalf Baechle <ralf@linux-mips.org>
    133d68e0