MIPS: Fix cmpxchg on 32b signed ints for 64b kernel with !kernel_uses_llsc
authorPaul Burton <paul.burton@imgtec.com>
Fri, 1 Sep 2017 21:46:50 +0000 (14:46 -0700)
committerRalf Baechle <ralf@linux-mips.org>
Mon, 9 Oct 2017 14:31:32 +0000 (16:31 +0200)
commit133d68e0ed4d822ccfa276ff7d2d1753477de1a8
tree8eb862971bd0d3d30abdaa28a67028676c4ba716
parent1b6ad6df8b4fd723d8d98b670b0d7772402e4e34
MIPS: Fix cmpxchg on 32b signed ints for 64b kernel with !kernel_uses_llsc

Commit 8263db4d7768 ("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: Paul Burton <paul.burton@imgtec.com>
Fixes: 8263db4d7768 ("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: Ralf Baechle <ralf@linux-mips.org>
arch/mips/include/asm/cmpxchg.h