2014-07-23 21:20:09

by Stefan Kristiansson

[permalink] [raw]
Subject: [PATCH 0/6] openrisc: support for l.lwa and l.swa atomic instructions

The latest revision (v1.1-0) of the OpenRISC 1000 architecture
specification [1], adds definitions for load-linked/store-conditional
type of atomic instructions.

There are already a couple of implementations available that has support
for these instructions implemented.
E.g. a hardware implementation - mor1kx and a functional simulator - or1ksim.

This series of patches takes these new instructions into use to create hardware
assisted versions of atomic/bitops/cmpxchg.
It also enables proper support for futexes, something that hasn't been available
previously.

[1] https://github.com/openrisc/doc/blob/master/openrisc-arch-1.1-rev0.pdf?raw=true

Stefan Kristiansson (6):
openrisc: add Kconfig for l.lwa and l.swa atomic instructions
openrisc: add atomic bitops
openrisc: add cmpxchg and xchg implementations
openrisc: add atomic operations implementations
openrisc: include l.swa in check for write data pagefault
openrisc: add futex_atomic_* implementations

arch/openrisc/Kconfig | 7 ++
arch/openrisc/include/asm/Kbuild | 4 -
arch/openrisc/include/asm/atomic.h | 88 +++++++++++++++++++
arch/openrisc/include/asm/bitops.h | 2 +-
arch/openrisc/include/asm/bitops/atomic.h | 109 +++++++++++++++++++++++
arch/openrisc/include/asm/cmpxchg.h | 83 ++++++++++++++++++
arch/openrisc/include/asm/futex.h | 140 ++++++++++++++++++++++++++++++
arch/openrisc/kernel/entry.S | 2 +-
8 files changed, 429 insertions(+), 6 deletions(-)
create mode 100644 arch/openrisc/include/asm/atomic.h
create mode 100644 arch/openrisc/include/asm/bitops/atomic.h
create mode 100644 arch/openrisc/include/asm/cmpxchg.h
create mode 100644 arch/openrisc/include/asm/futex.h

--
1.8.3.2


2014-07-23 21:20:17

by Stefan Kristiansson

[permalink] [raw]
Subject: [PATCH 1/6] openrisc: add Kconfig for l.lwa and l.swa atomic instructions

Not all OpenRISC cpus have support for the l.lwa and l.swa,
this adds a config knob to opt them in and out.

Signed-off-by: Stefan Kristiansson <[email protected]>
---
arch/openrisc/Kconfig | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/arch/openrisc/Kconfig b/arch/openrisc/Kconfig
index 848bf96..e8065a3 100644
--- a/arch/openrisc/Kconfig
+++ b/arch/openrisc/Kconfig
@@ -103,6 +103,13 @@ config OPENRISC_HAVE_INST_DIV
default y
help
Select this if your implementation has a hardware divide instruction
+
+config OPENRISC_HAVE_INST_LWA_SWA
+ bool "Have instruction l.lwa and l.swa"
+ help
+ Select this if your implementation have l.lwa and l.swa atomic
+ instructions.
+
endmenu


--
1.8.3.2

2014-07-23 21:20:21

by Stefan Kristiansson

[permalink] [raw]
Subject: [PATCH 2/6] openrisc: add atomic bitops

This utilize the load-link/store-conditional l.lwa and l.swa
instructions to implement the atomic bitops.
When those instructions are not available, a fallback to the
generic implementation is provided.

Signed-off-by: Stefan Kristiansson <[email protected]>
---
arch/openrisc/include/asm/bitops.h | 2 +-
arch/openrisc/include/asm/bitops/atomic.h | 109 ++++++++++++++++++++++++++++++
2 files changed, 110 insertions(+), 1 deletion(-)
create mode 100644 arch/openrisc/include/asm/bitops/atomic.h

diff --git a/arch/openrisc/include/asm/bitops.h b/arch/openrisc/include/asm/bitops.h
index 2c64f22..08271bc 100644
--- a/arch/openrisc/include/asm/bitops.h
+++ b/arch/openrisc/include/asm/bitops.h
@@ -52,7 +52,7 @@
#include <asm-generic/bitops/hweight.h>
#include <asm-generic/bitops/lock.h>

-#include <asm-generic/bitops/atomic.h>
+#include <asm/bitops/atomic.h>
#include <asm-generic/bitops/non-atomic.h>
#include <asm-generic/bitops/le.h>
#include <asm-generic/bitops/ext2-atomic.h>
diff --git a/arch/openrisc/include/asm/bitops/atomic.h b/arch/openrisc/include/asm/bitops/atomic.h
new file mode 100644
index 0000000..ef25cff
--- /dev/null
+++ b/arch/openrisc/include/asm/bitops/atomic.h
@@ -0,0 +1,109 @@
+/*
+ * Copyright (C) 2014 Stefan Kristiansson <[email protected]>
+ *
+ * This file is licensed under the terms of the GNU General Public License
+ * version 2. This program is licensed "as is" without any warranty of any
+ * kind, whether express or implied.
+ */
+
+#ifndef __ASM_OPENRISC_BITOPS_ATOMIC_H
+#define __ASM_OPENRISC_BITOPS_ATOMIC_H
+
+#ifdef CONFIG_OPENRISC_HAVE_INST_LWA_SWA
+
+static inline void set_bit(int nr, volatile unsigned long *addr)
+{
+ unsigned long mask = BIT_MASK(nr);
+ unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr);
+ unsigned long tmp;
+
+ __asm__ __volatile__(
+ "1: l.lwa %0,0(%1) \n"
+ " l.or %0,%0,%2 \n"
+ " l.swa 0(%1),%0 \n"
+ " l.bnf 1b \n"
+ " l.nop \n"
+ : "=&r"(tmp)
+ : "r"(p), "r"(mask)
+ : "cc", "memory");
+}
+
+static inline void clear_bit(int nr, volatile unsigned long *addr)
+{
+ unsigned long mask = BIT_MASK(nr);
+ unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr);
+ unsigned long tmp;
+
+ __asm__ __volatile__(
+ "1: l.lwa %0,0(%1) \n"
+ " l.and %0,%0,%2 \n"
+ " l.swa 0(%1),%0 \n"
+ " l.bnf 1b \n"
+ " l.nop \n"
+ : "=&r"(tmp)
+ : "r"(p), "r"(~mask)
+ : "cc", "memory");
+}
+
+static inline void change_bit(int nr, volatile unsigned long *addr)
+{
+ unsigned long mask = BIT_MASK(nr);
+ unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr);
+ unsigned long tmp;
+
+ __asm__ __volatile__(
+ "1: l.lwa %0,0(%1) \n"
+ " l.xor %0,%0,%2 \n"
+ " l.swa 0(%1),%0 \n"
+ " l.bnf 1b \n"
+ " l.nop \n"
+ : "=&r"(tmp)
+ : "r"(p), "r"(mask)
+ : "cc", "memory");
+}
+
+static inline int test_and_set_bit(int nr, volatile unsigned long *addr)
+{
+ unsigned long mask = BIT_MASK(nr);
+ unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr);
+ unsigned long old;
+ unsigned long tmp;
+
+ __asm__ __volatile__(
+ "1: l.lwa %0,0(%2) \n"
+ " l.or %1,%0,%3 \n"
+ " l.swa 0(%2),%1 \n"
+ " l.bnf 1b \n"
+ " l.nop \n"
+ : "=&r"(old), "=&r"(tmp)
+ : "r"(p), "r"(mask)
+ : "cc", "memory");
+
+ return (old & mask) != 0;
+}
+
+static inline int test_and_clear_bit(int nr, volatile unsigned long *addr)
+{
+ unsigned long mask = BIT_MASK(nr);
+ unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr);
+ unsigned long old;
+ unsigned long tmp;
+
+ __asm__ __volatile__(
+ "1: l.lwa %0,0(%2) \n"
+ " l.and %1,%0,%3 \n"
+ " l.swa 0(%2),%1 \n"
+ " l.bnf 1b \n"
+ " l.nop \n"
+ : "=&r"(old), "=&r"(tmp)
+ : "r"(p), "r"(~mask)
+ : "cc", "memory");
+
+ return (old & mask) != 0;
+}
+
+#else
+#include <asm-generic/bitops/atomic.h>
+#endif
+
+#endif /* __ASM_OPENRISC_BITOPS_ATOMIC_H */
--
1.8.3.2

2014-07-23 21:20:29

by Stefan Kristiansson

[permalink] [raw]
Subject: [PATCH 6/6] openrisc: add futex_atomic_* implementations

Support for the futex_atomic_* operations by using the
load-link/store-conditional l.lwa/l.swa instructions.

A fallback to the generic implementation is provided when those
implementations aren't available.
The generic implementation however only provides stubs for the
functions, so to obtain proper futex support, the implementation
that is added here is needed.

Signed-off-by: Stefan Kristiansson <[email protected]>
---
arch/openrisc/include/asm/Kbuild | 1 -
arch/openrisc/include/asm/futex.h | 140 ++++++++++++++++++++++++++++++++++++++
2 files changed, 140 insertions(+), 1 deletion(-)
create mode 100644 arch/openrisc/include/asm/futex.h

diff --git a/arch/openrisc/include/asm/Kbuild b/arch/openrisc/include/asm/Kbuild
index 1e759d5..5bc2246 100644
--- a/arch/openrisc/include/asm/Kbuild
+++ b/arch/openrisc/include/asm/Kbuild
@@ -20,7 +20,6 @@ generic-y += exec.h
generic-y += fb.h
generic-y += fcntl.h
generic-y += ftrace.h
-generic-y += futex.h
generic-y += hardirq.h
generic-y += hash.h
generic-y += hw_irq.h
diff --git a/arch/openrisc/include/asm/futex.h b/arch/openrisc/include/asm/futex.h
new file mode 100644
index 0000000..f4b654f
--- /dev/null
+++ b/arch/openrisc/include/asm/futex.h
@@ -0,0 +1,140 @@
+#ifndef __ASM_OPENRISC_FUTEX_H
+#define __ASM_OPENRISC_FUTEX_H
+
+#ifdef CONFIG_OPENRISC_HAVE_INST_LWA_SWA
+#ifdef __KERNEL__
+
+#include <linux/futex.h>
+#include <linux/uaccess.h>
+#include <asm/errno.h>
+
+#define __futex_atomic_op(insn, ret, oldval, uaddr, oparg) \
+({ \
+ __asm__ __volatile__ ( \
+ "1: l.lwa %0, %2 \n" \
+ insn "\n" \
+ "2: l.swa %2, %1 \n" \
+ " l.bnf 1b \n" \
+ " l.ori %1, r0, 0 \n" \
+ "3: \n" \
+ ".section .fixup,\"ax\" \n" \
+ "4: l.j 3b \n" \
+ " l.addi %1, r0, %3 \n" \
+ ".previous \n" \
+ ".section __ex_table,\"a\" \n" \
+ ".word 1b,4b,2b,4b \n" \
+ ".previous \n" \
+ : "=&r" (oldval), "=&r" (ret), "+m" (*uaddr) \
+ : "i" (-EFAULT), "r" (oparg) \
+ : "cc", "memory" \
+ ); \
+})
+
+static inline int
+futex_atomic_op_inuser(int encoded_op, u32 __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;
+
+ if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28))
+ oparg = 1 << oparg;
+
+ if (!access_ok(VERIFY_WRITE, uaddr, sizeof(u32)))
+ return -EFAULT;
+
+ pagefault_disable();
+
+ switch (op) {
+ case FUTEX_OP_SET:
+ __futex_atomic_op("l.or %1,%4,%4", ret, oldval, uaddr, oparg);
+ break;
+ case FUTEX_OP_ADD:
+ __futex_atomic_op("l.add %1,%0,%4", ret, oldval, uaddr, oparg);
+ break;
+ case FUTEX_OP_OR:
+ __futex_atomic_op("l.or %1,%0,%4", ret, oldval, uaddr, oparg);
+ break;
+ case FUTEX_OP_ANDN:
+ __futex_atomic_op("l.and %1,%0,%4", ret, oldval, uaddr, ~oparg);
+ break;
+ case FUTEX_OP_XOR:
+ __futex_atomic_op("l.xor %1,%0,%4", ret, oldval, uaddr, oparg);
+ break;
+ default:
+ ret = -ENOSYS;
+ }
+
+ pagefault_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(u32 *uval, u32 __user *uaddr,
+ u32 oldval, u32 newval)
+{
+ int ret = 0;
+ u32 prev;
+
+ if (!access_ok(VERIFY_WRITE, uaddr, sizeof(u32)))
+ return -EFAULT;
+
+ __asm__ __volatile__ ( \
+ "1: l.lwa %1, %2 \n" \
+ " l.sfeq %1, %3 \n" \
+ " l.bnf 3f \n" \
+ " l.nop \n" \
+ "2: l.swa %2, %4 \n" \
+ " l.bnf 1b \n" \
+ " l.nop \n" \
+ "3: \n" \
+ ".section .fixup,\"ax\" \n" \
+ "4: l.j 3b \n" \
+ " l.addi %0, r0, %5 \n" \
+ ".previous \n" \
+ ".section __ex_table,\"a\" \n" \
+ ".word 1b,4b,2b,4b \n" \
+ ".previous \n" \
+ : "+r" (ret), "=&r" (prev), "+m" (*uaddr) \
+ : "r" (oldval), "r" (newval), "i" (-EFAULT) \
+ : "cc", "memory" \
+ );
+
+ *uval = prev;
+ return ret;
+}
+
+#endif /* __KERNEL__ */
+
+#else
+#include <asm-generic/futex.h>
+#endif
+
+#endif /* __ASM_OPENRISC_FUTEX_H */
--
1.8.3.2

2014-07-23 21:20:25

by Stefan Kristiansson

[permalink] [raw]
Subject: [PATCH 4/6] openrisc: add atomic operations implementations

Optimized version that make use of the l.lwa and l.swa instructions
if available.

Signed-off-by: Stefan Kristiansson <[email protected]>
---
arch/openrisc/include/asm/Kbuild | 1 -
arch/openrisc/include/asm/atomic.h | 88 ++++++++++++++++++++++++++++++++++++++
2 files changed, 88 insertions(+), 1 deletion(-)
create mode 100644 arch/openrisc/include/asm/atomic.h

diff --git a/arch/openrisc/include/asm/Kbuild b/arch/openrisc/include/asm/Kbuild
index 013b64a..1e759d5 100644
--- a/arch/openrisc/include/asm/Kbuild
+++ b/arch/openrisc/include/asm/Kbuild
@@ -1,7 +1,6 @@

header-y += ucontext.h

-generic-y += atomic.h
generic-y += auxvec.h
generic-y += barrier.h
generic-y += bitsperlong.h
diff --git a/arch/openrisc/include/asm/atomic.h b/arch/openrisc/include/asm/atomic.h
new file mode 100644
index 0000000..253edcc
--- /dev/null
+++ b/arch/openrisc/include/asm/atomic.h
@@ -0,0 +1,88 @@
+/*
+ * Copyright (C) 2014 Stefan Kristiansson <[email protected]>
+ *
+ * This file is licensed under the terms of the GNU General Public License
+ * version 2. This program is licensed "as is" without any warranty of any
+ * kind, whether express or implied.
+ */
+
+#ifndef __ASM_OPENRISC_ATOMIC_H
+#define __ASM_OPENRISC_ATOMIC_H
+
+#include <linux/types.h>
+
+#ifdef CONFIG_OPENRISC_HAVE_INST_LWA_SWA
+
+static inline int atomic_add_return(int i, atomic_t *v)
+{
+ int tmp;
+
+ __asm__ __volatile__(
+ "1: l.lwa %0,0(%1) \n"
+ " l.add %0,%0,%2 \n"
+ " l.swa 0(%1),%0 \n"
+ " l.bnf 1b \n"
+ " l.nop \n"
+ : "=&r"(tmp)
+ : "r"(&v->counter), "r"(i)
+ : "cc", "memory");
+
+ return tmp;
+}
+
+static inline int atomic_sub_return(int i, atomic_t *v)
+{
+ int tmp;
+
+ __asm__ __volatile__(
+ "1: l.lwa %0,0(%1) \n"
+ " l.sub %0,%0,%2 \n"
+ " l.swa 0(%1),%0 \n"
+ " l.bnf 1b \n"
+ " l.nop \n"
+ : "=&r"(tmp)
+ : "r"(&v->counter), "r"(i)
+ : "cc", "memory");
+
+ return tmp;
+}
+
+static inline void atomic_clear_mask(unsigned long mask, atomic_t *v)
+{
+ unsigned long tmp;
+
+ __asm__ __volatile__(
+ "1: l.lwa %0,0(%1) \n"
+ " l.and %0,%0,%2 \n"
+ " l.swa 0(%1),%0 \n"
+ " l.bnf 1b \n"
+ " l.nop \n"
+ : "=&r"(tmp)
+ : "r"(&v->counter), "r"(mask)
+ : "cc", "memory");
+}
+
+static inline void atomic_set_mask(unsigned long mask, atomic_t *v)
+{
+ unsigned long tmp;
+
+ __asm__ __volatile__(
+ "1: l.lwa %0,0(%1) \n"
+ " l.or %0,%0,%2 \n"
+ " l.swa 0(%1),%0 \n"
+ " l.bnf 1b \n"
+ " l.nop \n"
+ : "=&r"(tmp)
+ : "r"(&v->counter), "r"(mask)
+ : "cc", "memory");
+}
+
+#define atomic_add_return atomic_add_return
+#define atomic_sub_return atomic_sub_return
+#define atomic_clear_mask atomic_clear_mask
+#define atomic_set_mask atomic_set_mask
+
+#endif
+#include <asm-generic/atomic.h>
+
+#endif /* __ASM_OPENRISC_ATOMIC_H */
--
1.8.3.2

2014-07-23 21:20:55

by Stefan Kristiansson

[permalink] [raw]
Subject: [PATCH 5/6] openrisc: include l.swa in check for write data pagefault

The way to determine if a data pagefault was caused
by a write is to read the instruction at the PC that caused
the fault and check if it's a store instruction.
To recognize pagefaults caused by the l.swa instruction with
opcode 0x33, has to be included in the check.

Signed-off-by: Stefan Kristiansson <[email protected]>
---
arch/openrisc/kernel/entry.S | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/openrisc/kernel/entry.S b/arch/openrisc/kernel/entry.S
index 572d223..aac0bde 100644
--- a/arch/openrisc/kernel/entry.S
+++ b/arch/openrisc/kernel/entry.S
@@ -264,7 +264,7 @@ EXCEPTION_ENTRY(_data_page_fault_handler)
l.srli r6,r6,26 // check opcode for write access
#endif

- l.sfgeui r6,0x34 // check opcode for write access
+ l.sfgeui r6,0x33 // check opcode for write access
l.bnf 1f
l.sfleui r6,0x37
l.bnf 1f
--
1.8.3.2

2014-07-23 21:21:15

by Stefan Kristiansson

[permalink] [raw]
Subject: [PATCH 3/6] openrisc: add cmpxchg and xchg implementations

Optimized version that make use of the l.lwa and l.swa instructions
if available.

Signed-off-by: Stefan Kristiansson <[email protected]>
---
arch/openrisc/include/asm/Kbuild | 2 -
arch/openrisc/include/asm/cmpxchg.h | 83 +++++++++++++++++++++++++++++++++++++
2 files changed, 83 insertions(+), 2 deletions(-)
create mode 100644 arch/openrisc/include/asm/cmpxchg.h

diff --git a/arch/openrisc/include/asm/Kbuild b/arch/openrisc/include/asm/Kbuild
index 17f1e83..013b64a 100644
--- a/arch/openrisc/include/asm/Kbuild
+++ b/arch/openrisc/include/asm/Kbuild
@@ -10,8 +10,6 @@ generic-y += bugs.h
generic-y += cacheflush.h
generic-y += checksum.h
generic-y += clkdev.h
-generic-y += cmpxchg-local.h
-generic-y += cmpxchg.h
generic-y += cputime.h
generic-y += current.h
generic-y += device.h
diff --git a/arch/openrisc/include/asm/cmpxchg.h b/arch/openrisc/include/asm/cmpxchg.h
new file mode 100644
index 0000000..4e40411
--- /dev/null
+++ b/arch/openrisc/include/asm/cmpxchg.h
@@ -0,0 +1,83 @@
+/*
+ * Copyright (C) 2014 Stefan Kristiansson <[email protected]>
+ *
+ * This file is licensed under the terms of the GNU General Public License
+ * version 2. This program is licensed "as is" without any warranty of any
+ * kind, whether express or implied.
+ */
+
+#ifndef __ASM_OPENRISC_CMPXCHG_H
+#define __ASM_OPENRISC_CMPXCHG_H
+
+#include <linux/types.h>
+
+#ifdef CONFIG_OPENRISC_HAVE_INST_LWA_SWA
+
+/*
+ * This function doesn't exist, so you'll get a linker error
+ * if something tries to do an invalid cmpxchg().
+ */
+extern void __cmpxchg_called_with_bad_pointer(void);
+
+#define __HAVE_ARCH_CMPXCHG 1
+
+static inline unsigned long
+__cmpxchg(volatile void *ptr, unsigned long old, unsigned long new, int size)
+{
+ if (size != 4) {
+ __cmpxchg_called_with_bad_pointer();
+ return old;
+ }
+
+ __asm__ __volatile__(
+ "1: l.lwa %0, 0(%1) \n"
+ " l.sfeq %0, %2 \n"
+ " l.bnf 1f \n"
+ " l.nop \n"
+ " l.swa 0(%1), %3 \n"
+ " l.bnf 1b \n"
+ "1: l.nop \n"
+ : "=&r"(old)
+ : "r"(ptr), "r"(old), "r"(new)
+ : "cc", "memory");
+
+ return old;
+}
+
+#define cmpxchg(ptr, o, n) ((typeof(*(ptr)))__cmpxchg((ptr), \
+ (unsigned long)(o), (unsigned long)(n), sizeof(*(ptr))))
+
+/*
+ * This function doesn't exist, so you'll get a linker error if
+ * something tries to do an invalidly-sized xchg().
+ */
+extern void __xchg_called_with_bad_pointer(void);
+
+static inline unsigned long __xchg(unsigned long val, volatile void *ptr,
+ int size)
+{
+ if (size != 4) {
+ __xchg_called_with_bad_pointer();
+ return val;
+ }
+
+ __asm__ __volatile__(
+ "1: l.lwa %0, 0(%1) \n"
+ " l.swa 0(%1), %2 \n"
+ " l.bnf 1b \n"
+ " l.nop \n"
+ : "=&r"(val)
+ : "r"(ptr), "r"(val)
+ : "cc", "memory");
+
+ return val;
+}
+
+#define xchg(ptr, with) \
+ ((typeof(*(ptr)))__xchg((unsigned long)(with), (ptr), sizeof(*(ptr))))
+
+#else
+#include <asm-generic/cmpxchg.h>
+#endif
+
+#endif /* __ASM_OPENRISC_CMPXCHG_H */
--
1.8.3.2

2014-07-23 21:26:29

by Alan Cox

[permalink] [raw]
Subject: Re: [PATCH 1/6] openrisc: add Kconfig for l.lwa and l.swa atomic instructions

> +config OPENRISC_HAVE_INST_LWA_SWA
> + bool "Have instruction l.lwa and l.swa"
> + help
> + Select this if your implementation have l.lwa and l.swa atomic
> + instructions.
> +
> endmenu

Please do everyone a favour - sort all these out at runtime, add an
architectural feature bits ROM address or catch the traps and use the
alterntatives logic or something.

Alan

2014-07-24 19:53:32

by Stefan Kristiansson

[permalink] [raw]
Subject: Re: [PATCH 1/6] openrisc: add Kconfig for l.lwa and l.swa atomic instructions

On Wed, Jul 23, 2014 at 10:26:01PM +0100, One Thousand Gnomes wrote:
> > +config OPENRISC_HAVE_INST_LWA_SWA
> > + bool "Have instruction l.lwa and l.swa"
> > + help
> > + Select this if your implementation have l.lwa and l.swa atomic
> > + instructions.
> > +
> > endmenu
>
> Please do everyone a favour - sort all these out at runtime, add an
> architectural feature bits ROM address or catch the traps and use the
> alterntatives logic or something.
>

Detecting whether the instruction exists is of course trivial,
the hard part is what to do with this information at runtime.
My initial intention was to add emulation for the l.lwa and l.swa
instructions in the kernel, so a CPU without them would still
be able to run a kernel with CONFIG_OPENRISC_HAVE_INST_LWA_SWA enabled,
but inefficiently.

I will in either case add the kernel emulation,
since it's still needed to handle userspace occurences of these
instructions, perhaps that patch belong to this series as well.

I wasn't familiar with the alternatives logic before, so I had
to look it up.
Correct me if I'm wrong, but from what I gathered, the approach
that takes is especially suitable to fixup instructions that
are similar to each others but have slightly different properties
(the lock prefix to cmpxchg being the canonical example).

I guess it'd be *possible* to still use this method to implement
alternatives that would work in the UP case by disabling
context-switch and perform normal loads and stores in place
of the l.lwa and l.swa instructions, but is it really a good approach?

Stefan

2014-07-24 20:35:06

by Alan Cox

[permalink] [raw]
Subject: Re: [PATCH 1/6] openrisc: add Kconfig for l.lwa and l.swa atomic instructions

> I wasn't familiar with the alternatives logic before, so I had
> to look it up.
> Correct me if I'm wrong, but from what I gathered, the approach
> that takes is especially suitable to fixup instructions that
> are similar to each others but have slightly different properties
> (the lock prefix to cmpxchg being the canonical example).

You need some similarity or in some cases it makes sense to swap the
instruction with a call to a helper out of line, rather than taking the
hit on the traps and emulation.

> I guess it'd be *possible* to still use this method to implement
> alternatives that would work in the UP case by disabling
> context-switch and perform normal loads and stores in place
> of the l.lwa and l.swa instructions, but is it really a good approach?

We actually did that on the 386 when it was still supported as 386
lacked some of the more obscure atomic instructions. Some other platforms
use constructs of the form

do {
i = reschedule_count
do repeatable ops
}
while(reschedule_count != i)

for non irq safe locking ops rather than irq off/on. Another approach is
to use a section to gather all the out of line "atomic" operations - ie
all the helpers - and not reschedule if the return address is in the
"atomic" section.

Most of the other architectures we ask these questions and generate code
for "at least this feature set" but then also hide the detail, although I
imagine most openrisc users are smarter than the average.

Alan