From patchwork Thu May 19 14:46:42 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Dave Martin X-Patchwork-Id: 1539 Return-Path: Delivered-To: unknown Received: from imap.gmail.com (74.125.159.109) by localhost6.localdomain6 with IMAP4-SSL; 08 Jun 2011 14:52:51 -0000 Delivered-To: patches@linaro.org Received: by 10.224.54.134 with SMTP id q6cs118137qag; Thu, 19 May 2011 07:46:52 -0700 (PDT) Received: by 10.227.152.132 with SMTP id g4mr3158037wbw.24.1305816411084; Thu, 19 May 2011 07:46:51 -0700 (PDT) Received: from mail-ww0-f50.google.com (mail-ww0-f50.google.com [74.125.82.50]) by mx.google.com with ESMTPS id q14si5312320wby.120.2011.05.19.07.46.50 (version=TLSv1/SSLv3 cipher=OTHER); Thu, 19 May 2011 07:46:51 -0700 (PDT) Received-SPF: neutral (google.com: 74.125.82.50 is neither permitted nor denied by best guess record for domain of dave.martin@linaro.org) client-ip=74.125.82.50; Authentication-Results: mx.google.com; spf=neutral (google.com: 74.125.82.50 is neither permitted nor denied by best guess record for domain of dave.martin@linaro.org) smtp.mail=dave.martin@linaro.org Received: by wwc33 with SMTP id 33so2862094wwc.31 for ; Thu, 19 May 2011 07:46:50 -0700 (PDT) Received: by 10.227.199.19 with SMTP id eq19mr3240884wbb.75.1305816409949; Thu, 19 May 2011 07:46:49 -0700 (PDT) Received: from e200948.peterhouse.linaro.org (fw-lnat.cambridge.arm.com [217.140.96.63]) by mx.google.com with ESMTPS id k2sm1664797wby.34.2011.05.19.07.46.48 (version=SSLv3 cipher=OTHER); Thu, 19 May 2011 07:46:49 -0700 (PDT) From: Dave Martin To: linux-arm-kernel@lists.infradead.org Cc: patches@linaro.org, Nicolas Pitre Subject: [RFC PATCH] ARM: kuser helpers: comment cleanup Date: Thu, 19 May 2011 15:46:42 +0100 Message-Id: <1305816402-9036-1-git-send-email-dave.martin@linaro.org> X-Mailer: git-send-email 1.7.4.1 MIME-Version: 1.0 The comments accompanying the kuser helpers contain assembler code which is no longer really needed, since the compiler will do a decent job of calling the helpers if the suggested C declarations are used. Also, the existing code uses old‐style calling techniquies which do not interwork properly if used in Thumb code, and which are likely to be suboptimally branch‐predicted on recent CPUs. Letting the compiler generate the appropriate function call boilerplate avoids these problems without needing to write multiple versions of the code. For these reasons, this patch removes those example assembler code snippets. Similarly, there’s no real reason why the example of how to implement atomic add needs to be assembler, so I have replaced it with a simpler C version. Comments have also been added clarifying how and when __kernel_helper_version should be checked. Signed-off-by: Dave Martin --- Note that this is just an RFC: In particular, I haven't tested the C atomic_add example yet -- it just shows the kind of thing we _could_ document. Instead of (or in addition to) this patch, we could also: * Put the kuser helper C declarations in an actual header file: this would mean that people writing userspace code could just use the helpers directly via C instead of having to write their own assembler wrappers. * Move the explanatory test into Documentation/arm/ The proposed comments on checking __kernel_helper_version may be overkill, particularly for the helpers which have always been there. On the other hand, encouraging people into good habits of always checking seems harmless at worst, and beneficial at best, particularly if we add more helpers over time, such as the 64-bit cpmxchg helper which has been mooted. Any views on this? Cheers ---Dave arch/arm/kernel/entry-armv.S | 55 ++++++++++++++++++++--------------------- 1 files changed, 27 insertions(+), 28 deletions(-) diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S index e8d8856..5d444b5 100644 --- a/arch/arm/kernel/entry-armv.S +++ b/arch/arm/kernel/entry-armv.S @@ -779,6 +779,11 @@ ENDPROC(__switch_to) * earlier processors just for the sake of not using these kernel helpers * if your compiled code is not going to use the new instructions for other * purpose. + * + * New helpers may be added over time, so an older kernel may be missing + * some helpers present in a newer kernel. For this reason, programs + * must check the value of __kernel_helper_version (see below) before + * assuming that it is safe to call any particular helper. */ THUMB( .arm ) @@ -819,11 +824,7 @@ __kuser_helper_start: * Apply any needed memory barrier to preserve consistency with data modified * manually and __kuser_cmpxchg usage. * - * This could be used as follows: - * - * #define __kernel_dmb() \ - * asm volatile ( "mov r0, #0xffff0fff; mov lr, pc; sub pc, r0, #95" \ - * : : : "r0", "lr","cc" ) + * Do not attempt to call this function unless __kernel_helper_version >= 3. */ __kuser_memory_barrier: @ 0xffff0fa0 @@ -855,7 +856,8 @@ __kuser_memory_barrier: @ 0xffff0fa0 * * Definition and user space usage example: * - * typedef int (__kernel_cmpxchg_t)(int oldval, int newval, int *ptr); + * typedef int (__kernel_cmpxchg_t)(int oldval, int newval, + * int volatile *ptr); * #define __kernel_cmpxchg (*(__kernel_cmpxchg_t *)0xffff0fc0) * * Atomically store newval in *ptr if *ptr is equal to oldval for user space. @@ -863,27 +865,25 @@ __kuser_memory_barrier: @ 0xffff0fa0 * The C flag is also set if *ptr was changed to allow for assembly * optimization in the calling code. * + * Do not attempt to call this function unless __kernel_helper_version >= 2. + * * Notes: * * - This routine already includes memory barriers as needed. * * For example, a user space atomic_add implementation could look like this: * - * #define atomic_add(ptr, val) \ - * ({ register unsigned int *__ptr asm("r2") = (ptr); \ - * register unsigned int __result asm("r1"); \ - * asm volatile ( \ - * "1: @ atomic_add\n\t" \ - * "ldr r0, [r2]\n\t" \ - * "mov r3, #0xffff0fff\n\t" \ - * "add lr, pc, #4\n\t" \ - * "add r1, r0, %2\n\t" \ - * "add pc, r3, #(0xffff0fc0 - 0xffff0fff)\n\t" \ - * "bcc 1b" \ - * : "=&r" (__result) \ - * : "r" (__ptr), "rIL" (val) \ - * : "r0","r3","ip","lr","cc","memory" ); \ - * __result; }) + * int atomic_add(int volatile *ptr, int val) + * { + * int newval; + * do { + * int oldval = *ptr; + * + * newval = oldval + val; + * while(__kernel_cmpxchg(oldval, newval, ptr)); + * + * return newval; + * } */ __kuser_cmpxchg: @ 0xffff0fc0 @@ -983,13 +983,7 @@ kuser_cmpxchg_fixup: * * Get the TLS value as previously set via the __ARM_NR_set_tls syscall. * - * This could be used as follows: - * - * #define __kernel_get_tls() \ - * ({ register unsigned int __val asm("r0"); \ - * asm( "mov r0, #0xffff0fff; mov lr, pc; sub pc, r0, #31" \ - * : "=r" (__val) : : "lr","cc" ); \ - * __val; }) + * Do not attempt to call this function unless __kernel_helper_version >= 1. */ __kuser_get_tls: @ 0xffff0fe0 @@ -1011,6 +1005,11 @@ __kuser_get_tls: @ 0xffff0fe0 * * User space may read this to determine the curent number of helpers * available. + * + * User space may assume that the value of this field never changes + * during the lifetime of any single process. This means that this + * field can be read once during the initialisation of a library or + * startup phase of a program. */ __kuser_helper_version: @ 0xffff0ffc