[PATCH] ARM: Add PI/robust mutexes support for SMP kernels

[PATCH] ARM: Add PI/robust mutexes support for SMP kernels

[Anton Vorontsov]

To support PI or robust mutexes, the kernel needs to perform some
operations atomically on userspace addresses, and yet ARM lacked
the support for the SMP case.

ARMv6 adds exclusive access variants of ldr and str instructions,
which means that support for PI/robust mutexes should now be
relatively straightforward.

Note that with this patch, if we run ARMv6 or greater, we'll use
atomic instructions no matter if it's SMP or UP kernel (just as we
do in atomic.h).

This has been tested on an ARM11 MPCore machine (ARMv6K), but I
also plan to test it with some Cortex-A9 (ARMv7) soon.

Signed-off-by: Anton Vorontsov <avorontsov@mvista.com>
---
 arch/arm/include/asm/futex.h |  125 +++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 122 insertions(+), 3 deletions(-)

diff --git a/arch/arm/include/asm/futex.h b/arch/arm/include/asm/futex.h
index 540a044..acdbebb 100644
--- a/arch/arm/include/asm/futex.h
+++ b/arch/arm/include/asm/futex.h
@@ -3,17 +3,134 @@
 
 #ifdef __KERNEL__
 
-#ifdef CONFIG_SMP
+#if defined(CONFIG_SMP) && __LINUX_ARM_ARCH__ < 6
 
 #include <asm-generic/futex.h>
 
-#else /* !SMP, we can work around lack of atomic ops by disabling preemption */
+#else
 
 #include <linux/futex.h>
 #include <linux/preempt.h>
 #include <linux/uaccess.h>
 #include <asm/errno.h>
 
+#if __LINUX_ARM_ARCH__ >= 6
+
+#define __futex_atomic_op(insn, res, ret, oldval, uaddr, oparg)	\
+	__asm__ __volatile__(					\
+	"1:	ldrex	%2, [%3]\n"				\
+	"	" insn "\n"					\
+	"2:	strex	%0, %1, [%3]\n"				\
+	"	teq	%0, #0\n"				\
+	"	bne 1b\n"					\
+	"	mov	%1, #0\n"				\
+	"3:\n"							\
+	"	.pushsection __ex_table,\"a\"\n"		\
+	"	.align	3\n"					\
+	"	.long	1b, 4f, 2b, 4f\n"			\
+	"	.popsection\n"					\
+	"	.pushsection .fixup,\"ax\"\n"			\
+	"4:	mov	%1, %5\n"				\
+	"	b	3b\n"					\
+	"	.popsection"					\
+	: "=&r" (res), "=&r" (ret), "=&r" (oldval)		\
+	: "r" (uaddr), "r" (oparg), "Ir" (-EFAULT)		\
+	: "cc", "memory")
+
+static inline int futex_atomic_op_inuser(int encoded_op, int __user *uaddr)
+{
+	int op = (encoded_op >> 28) & 7;
+	int cmp = (encoded_op >> 24) & 15;
+	int oparg = (encoded_op << 8) >> 20;
+	int cmparg = (encoded_op << 20) >> 20;
+	int oldval = 0, ret;
+	unsigned long res;
+
+	if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28))
+		oparg = 1 << oparg;
+
+	if (!access_ok(VERIFY_WRITE, uaddr, sizeof(int)))
+		return -EFAULT;
+
+	pagefault_disable();	/* implies preempt_disable() */
+
+	switch (op) {
+	case FUTEX_OP_SET:
+		__futex_atomic_op("mov	%1, %4", res, ret, oldval, uaddr, oparg);
+		break;
+	case FUTEX_OP_ADD:
+		__futex_atomic_op("add	%1, %2, %4", res, ret, oldval, uaddr, oparg);
+		break;
+	case FUTEX_OP_OR:
+		__futex_atomic_op("orr	%1, %2, %4", res, ret, oldval, uaddr, oparg);
+		break;
+	case FUTEX_OP_ANDN:
+		__futex_atomic_op("and	%1, %2, %4", res, ret, oldval, uaddr, ~oparg);
+		break;
+	case FUTEX_OP_XOR:
+		__futex_atomic_op("eor	%1, %2, %4", res, ret, oldval, uaddr, oparg);
+		break;
+	default:
+		ret = -ENOSYS;
+	}
+
+	pagefault_enable();	/* subsumes preempt_enable() */
+
+	if (!ret) {
+		switch (cmp) {
+		case FUTEX_OP_CMP_EQ: ret = (oldval == cmparg); break;
+		case FUTEX_OP_CMP_NE: ret = (oldval != cmparg); break;
+		case FUTEX_OP_CMP_LT: ret = (oldval < cmparg); break;
+		case FUTEX_OP_CMP_GE: ret = (oldval >= cmparg); break;
+		case FUTEX_OP_CMP_LE: ret = (oldval <= cmparg); break;
+		case FUTEX_OP_CMP_GT: ret = (oldval > cmparg); break;
+		default: ret = -ENOSYS;
+		}
+	}
+	return ret;
+}
+
+static inline int
+futex_atomic_cmpxchg_inatomic(int __user *uaddr, int oldval, int newval)
+{
+	unsigned long ret;
+	unsigned long res;
+
+	if (!access_ok(VERIFY_WRITE, uaddr, sizeof(int)))
+		return -EFAULT;
+
+	smp_mb();
+
+	do {
+		asm volatile("@futex_atomic_cmpxchg_inatomic\n"
+		"1:	ldrex	%1, [%2]\n"
+		"	mov	%0, #0\n"
+		"	teq	%1, %3\n"
+		"	it      eq @ explicit IT needed for the 2b label\n"
+		"2:	strexeq %0, %4, [%2]\n"
+		"3:\n"
+		"	.pushsection __ex_table,\"a\"\n"
+		"	.align	3\n"
+		"	.long	1b, 4f, 2b, 4f\n"
+		"	.popsection\n"
+		"	.pushsection .fixup,\"ax\"\n"
+		"4:	mov	%0, #0\n"
+		"	mov	%1, %5\n"
+		"	b	3b\n"
+		"	.popsection"
+			: "=&r" (res), "=&r" (ret)
+			: "r" (uaddr), "Ir" (oldval), "r" (newval),
+			  "Ir" (-EFAULT)
+			: "memory", "cc");
+	} while (res);
+
+	smp_mb();
+
+	return ret;
+}
+
+#else /* we can work around lack of atomic ops by disabling preemption */
+
 #define __futex_atomic_op(insn, ret, oldval, uaddr, oparg)	\
 	__asm__ __volatile__(					\
 	"1:	ldrt	%1, [%2]\n"				\
@@ -119,7 +236,9 @@ futex_atomic_cmpxchg_inatomic(int __user *uaddr, int oldval, int newval)
 	return val;
 }
 
-#endif /* !SMP */
+#endif
+
+#endif
 
 #endif /* __KERNEL__ */
 #endif /* _ASM_ARM_FUTEX_H */
-- 
1.7.0.5

[PATCH] ARM: Add PI/robust mutexes support for SMP kernels

[Russell King - ARM Linux]

On Mon, Jun 07, 2010 at 09:36:30PM +0400, Anton Vorontsov wrote:
> To support PI or robust mutexes, the kernel needs to perform some
> operations atomically on userspace addresses, and yet ARM lacked
> the support for the SMP case.
> 
> ARMv6 adds exclusive access variants of ldr and str instructions,
> which means that support for PI/robust mutexes should now be
> relatively straightforward.

It isn't this straight forward.  You're now bypassing the MMU protections
in that 'strex' can bypass the read-only protection of the user page.
This can result in the zero BSS page being corrupted, or worse corruption
to page cache pages.

[PATCH] ARM: Add PI/robust mutexes support for SMP kernels

[Anton Vorontsov]

On Mon, Jun 07, 2010 at 08:44:57PM +0100, Russell King - ARM Linux wrote:
> On Mon, Jun 07, 2010 at 09:36:30PM +0400, Anton Vorontsov wrote:
> > To support PI or robust mutexes, the kernel needs to perform some
> > operations atomically on userspace addresses, and yet ARM lacked
> > the support for the SMP case.
> > 
> > ARMv6 adds exclusive access variants of ldr and str instructions,
> > which means that support for PI/robust mutexes should now be
> > relatively straightforward.
> 
> It isn't this straight forward.  You're now bypassing the MMU protections
> in that 'strex' can bypass the read-only protection of the user page.
> This can result in the zero BSS page being corrupted, or worse corruption
> to page cache pages.

Interesting. I don't pretend I understand all MMU details, but arm.com
says "If a processor does an STR on a memory region that it has already
marked as exclusive, this does not clear the tag."

So, can we solve this by

ldrex
...
strt newval
   ^ may cause exception, but doesn't clear the tag, so we're still atomic
strex <- clears the tag

.fixup
	strex oldval <- just clear the tag
	return -EFAULT;

Thanks!

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

[PATCH] ARM: Add PI/robust mutexes support for SMP kernels

[Nicolas Pitre]

On Tue, 8 Jun 2010, Anton Vorontsov wrote:

> So, can we solve this by
> 
> ldrex
> ...
> strt newval
>    ^ may cause exception, but doesn't clear the tag, so we're still atomic
> strex <- clears the tag

No, you're not atomic anymore, as some other ldrex/strex might have come 
through by the time the strt is executed, and while this strex wouldn't 
be executed as the tag would be cleared@that point, the strt has no 
such concerns with atomic tags and would indeed corrupt whatever new 
value has been stored in the mean time.



Nicolas

[PATCH] ARM: Add PI/robust mutexes support for SMP kernels

[Anton Vorontsov]

On Mon, Jun 07, 2010 at 04:40:01PM -0400, Nicolas Pitre wrote:
> On Tue, 8 Jun 2010, Anton Vorontsov wrote:
> 
> > So, can we solve this by
> > 
> > ldrex
> > ...
> > strt newval
> >    ^ may cause exception, but doesn't clear the tag, so we're still atomic
> > strex <- clears the tag
> 
> No, you're not atomic anymore, as some other ldrex/strex might have come 
> through by the time the strt is executed, and while this strex wouldn't 
> be executed as the tag would be cleared at that point, the strt has no 
> such concerns with atomic tags and would indeed corrupt whatever new 
> value has been stored in the mean time.

Ugh, surely right you are, thanks!

It seems Catalin's "Remove the domain switching" patch comes handy
here.

-- 
Anton Vorontsov
email: cbouatmailru at gmail.com
irc://irc.freenode.net/bd2

[PATCH] ARM: Add PI/robust mutexes support for SMP kernels

[Catalin Marinas]

On Mon, 2010-06-07 at 22:35 +0100, Anton Vorontsov wrote:
> On Mon, Jun 07, 2010 at 04:40:01PM -0400, Nicolas Pitre wrote:
> > On Tue, 8 Jun 2010, Anton Vorontsov wrote:
> >
> > > So, can we solve this by
> > >
> > > ldrex
> > > ...
> > > strt newval
> > >    ^ may cause exception, but doesn't clear the tag, so we're still atomic
> > > strex <- clears the tag
> >
> > No, you're not atomic anymore, as some other ldrex/strex might have come
> > through by the time the strt is executed, and while this strex wouldn't
> > be executed as the tag would be cleared at that point, the strt has no
> > such concerns with atomic tags and would indeed corrupt whatever new
> > value has been stored in the mean time.
> 
> Ugh, surely right you are, thanks!
> 
> It seems Catalin's "Remove the domain switching" patch comes handy
> here.

As I said, not all the domains patch is needed for your specific case,
just the parts dealing with page permissions.

-- 
Catalin

[PATCH] ARM: Add PI/robust mutexes support for SMP kernels

[Catalin Marinas]

On Mon, 2010-06-07 at 21:27 +0100, Anton Vorontsov wrote:
> On Mon, Jun 07, 2010 at 08:44:57PM +0100, Russell King - ARM Linux wrote:
> > On Mon, Jun 07, 2010 at 09:36:30PM +0400, Anton Vorontsov wrote:
> > > To support PI or robust mutexes, the kernel needs to perform some
> > > operations atomically on userspace addresses, and yet ARM lacked
> > > the support for the SMP case.
> > >
> > > ARMv6 adds exclusive access variants of ldr and str instructions,
> > > which means that support for PI/robust mutexes should now be
> > > relatively straightforward.
> >
> > It isn't this straight forward.  You're now bypassing the MMU protections
> > in that 'strex' can bypass the read-only protection of the user page.
> > This can result in the zero BSS page being corrupted, or worse corruption
> > to page cache pages.
> 
> Interesting. I don't pretend I understand all MMU details, but arm.com
> says "If a processor does an STR on a memory region that it has already
> marked as exclusive, this does not clear the tag."

Russell is probably referring to a STR or STREX in kernel mode that
writes to a read-only user page (currently mapped as kernel read/write).
Such write would succeed and it breaks the copy-on-write mechanism that
Linux uses for duplicating various pages. The original futex code uses
STRT which is executed with user permissions even if in kernel mode, so
it won't succeed (but raise a data abort which is handled by the
kernel).

But as I said, we already have patches to change the kernel R/W
permission for RO user pages.

-- 
Catalin

[PATCH] ARM: Add PI/robust mutexes support for SMP kernels

[Russell King - ARM Linux]

On Mon, Jun 07, 2010 at 10:05:57PM +0100, Catalin Marinas wrote:
> But as I said, we already have patches to change the kernel R/W
> permission for RO user pages.

... but they introduce their own problem.

[PATCH] ARM: Add PI/robust mutexes support for SMP kernels

[Catalin Marinas]

On Mon, 2010-06-07 at 22:52 +0100, Russell King - ARM Linux wrote:
> On Mon, Jun 07, 2010 at 10:05:57PM +0100, Catalin Marinas wrote:
> > But as I said, we already have patches to change the kernel R/W
> > permission for RO user pages.
> 
> ... but they introduce their own problem.

And these problem are ...

-- 
Catalin

[PATCH] ARM: Add PI/robust mutexes support for SMP kernels

[Russell King - ARM Linux]

On Tue, Jun 08, 2010 at 11:04:15AM +0100, Catalin Marinas wrote:
> On Mon, 2010-06-07 at 22:52 +0100, Russell King - ARM Linux wrote:
> > On Mon, Jun 07, 2010 at 10:05:57PM +0100, Catalin Marinas wrote:
> > > But as I said, we already have patches to change the kernel R/W
> > > permission for RO user pages.
> > 
> > ... but they introduce their own problem.
> 
> And these problem are ...

As pointed out in May.

[PATCH] ARM: Add PI/robust mutexes support for SMP kernels

[Catalin Marinas]

On Tue, 2010-06-08 at 11:26 +0100, Russell King - ARM Linux wrote:
> On Tue, Jun 08, 2010 at 11:04:15AM +0100, Catalin Marinas wrote:
> > On Mon, 2010-06-07 at 22:52 +0100, Russell King - ARM Linux wrote:
> > > On Mon, Jun 07, 2010 at 10:05:57PM +0100, Catalin Marinas wrote:
> > > > But as I said, we already have patches to change the kernel R/W
> > > > permission for RO user pages.
> > >
> > > ... but they introduce their own problem.
> >
> > And these problem are ...
> 
> As pointed out in May.

Your first point was FIQ handlers and I updated the patch.

Your second point was I/D cache coherency and I also replied saying that
it doesn't matter on non-aliasing VIPT caches. There was no further
reply from you.

-- 
Catalin

[PATCH] ARM: Add PI/robust mutexes support for SMP kernels

[Catalin Marinas]

On Mon, 2010-06-07 at 20:44 +0100, Russell King - ARM Linux wrote:
> On Mon, Jun 07, 2010 at 09:36:30PM +0400, Anton Vorontsov wrote:
> > To support PI or robust mutexes, the kernel needs to perform some
> > operations atomically on userspace addresses, and yet ARM lacked
> > the support for the SMP case.
> >
> > ARMv6 adds exclusive access variants of ldr and str instructions,
> > which means that support for PI/robust mutexes should now be
> > relatively straightforward.
> 
> It isn't this straight forward.  You're now bypassing the MMU protections
> in that 'strex' can bypass the read-only protection of the user page.
> This can result in the zero BSS page being corrupted, or worse corruption
> to page cache pages.

Do you mean STREX in SVC mode targeting user pages? I keep posting a
patch for more than a year :) which removes the domains switching and
makes user pages read-only for both kernel and user space.

Making user RO / kernel RO pages doesn't necessarily require the full
domains patch, just the parts that deal with the vectors page and FIQ
setting (though we really need the whole domains patch in the kernel for
many reasons).

Such permissions are also required for Leif's patch which handles SWP
emulation via STREX/LDREX.

-- 
Catalin

[PATCH] ARM: Add PI/robust mutexes support for SMP kernels

[Anton Vorontsov]

On Mon, Jun 07, 2010 at 09:56:10PM +0100, Catalin Marinas wrote:
> On Mon, 2010-06-07 at 20:44 +0100, Russell King - ARM Linux wrote:
> > On Mon, Jun 07, 2010 at 09:36:30PM +0400, Anton Vorontsov wrote:
> > > To support PI or robust mutexes, the kernel needs to perform some
> > > operations atomically on userspace addresses, and yet ARM lacked
> > > the support for the SMP case.
> > >
> > > ARMv6 adds exclusive access variants of ldr and str instructions,
> > > which means that support for PI/robust mutexes should now be
> > > relatively straightforward.
> > 
> > It isn't this straight forward.  You're now bypassing the MMU protections
> > in that 'strex' can bypass the read-only protection of the user page.
> > This can result in the zero BSS page being corrupted, or worse corruption
> > to page cache pages.
> 
> Do you mean STREX in SVC mode targeting user pages? I keep posting a
> patch for more than a year :) which removes the domains switching and
> makes user pages read-only for both kernel and user space.

Wow. Then I should just make PI/robust mutexes depend on
!CONFIG_CPU_USE_DOMAINS (which is actually always true for SMP :-).

Thanks!

-- 
Anton Vorontsov
email: cbouatmailru at gmail.com
irc://irc.freenode.net/bd2