Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [RESEND PATCH v2 1/9] h8300: Don't include linux/kernel.h in asm/atomic.h
From: Will Deacon @ 2018-06-19 12:53 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1529412794-17720-1-git-send-email-will.deacon@arm.com>

linux/kernel.h isn't needed by asm/atomic.h and will result in circular
dependencies when the asm-generic atomic bitops are built around the
tomic_long_t interface.

Remove the broad include and replace it with linux/compiler.h for
READ_ONCE() etc and asm/irqflags.h for arch_local_irq_save() etc.

Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/h8300/include/asm/atomic.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/h8300/include/asm/atomic.h b/arch/h8300/include/asm/atomic.h
index 941e7554e886..b174dec099bf 100644
--- a/arch/h8300/include/asm/atomic.h
+++ b/arch/h8300/include/asm/atomic.h
@@ -2,8 +2,10 @@
 #ifndef __ARCH_H8300_ATOMIC__
 #define __ARCH_H8300_ATOMIC__
 
+#include <linux/compiler.h>
 #include <linux/types.h>
 #include <asm/cmpxchg.h>
+#include <asm/irqflags.h>
 
 /*
  * Atomic operations that C can't guarantee us.  Useful for
@@ -15,8 +17,6 @@
 #define atomic_read(v)		READ_ONCE((v)->counter)
 #define atomic_set(v, i)	WRITE_ONCE(((v)->counter), (i))
 
-#include <linux/kernel.h>
-
 #define ATOMIC_OP_RETURN(op, c_op)				\
 static inline int atomic_##op##_return(int i, atomic_t *v)	\
 {								\
-- 
2.1.4

^ permalink raw reply related

* [RESEND PATCH v2 2/9] m68k: Don't use asm-generic/bitops/lock.h
From: Will Deacon @ 2018-06-19 12:53 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1529412794-17720-1-git-send-email-will.deacon@arm.com>

asm-generic/bitops/lock.h is shortly going to be built on top of the
atomic_long_* API, which introduces a nasty circular dependency for
m68k where linux/atomic.h pulls in linux/bitops.h via:

	linux/atomic.h
	asm/atomic.h
	linux/irqflags.h
	asm/irqflags.h
	linux/preempt.h
	asm/preempt.h
	asm-generic/preempt.h
	linux/thread_info.h
	asm/thread_info.h
	asm/page.h
	asm-generic/getorder.h
	linux/log2.h
	linux/bitops.h

Since m68k isn't SMP and doesn't support ACQUIRE/RELEASE barriers, we
can just define the lock bitops in terms of the atomic bitops in the
asm/bitops.h header.

Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/m68k/include/asm/bitops.h | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/m68k/include/asm/bitops.h b/arch/m68k/include/asm/bitops.h
index 93b47b1f6fb4..18193419f97d 100644
--- a/arch/m68k/include/asm/bitops.h
+++ b/arch/m68k/include/asm/bitops.h
@@ -515,12 +515,16 @@ static inline int __fls(int x)
 
 #endif
 
+/* Simple test-and-set bit locks */
+#define test_and_set_bit_lock	test_and_set_bit
+#define clear_bit_unlock	clear_bit
+#define __clear_bit_unlock	clear_bit_unlock
+
 #include <asm-generic/bitops/ext2-atomic.h>
 #include <asm-generic/bitops/le.h>
 #include <asm-generic/bitops/fls64.h>
 #include <asm-generic/bitops/sched.h>
 #include <asm-generic/bitops/hweight.h>
-#include <asm-generic/bitops/lock.h>
 #endif /* __KERNEL__ */
 
 #endif /* _M68K_BITOPS_H */
-- 
2.1.4

^ permalink raw reply related

* [RESEND PATCH v2 3/9] asm-generic: Move some macros from linux/bitops.h to a new bits.h file
From: Will Deacon @ 2018-06-19 12:53 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1529412794-17720-1-git-send-email-will.deacon@arm.com>

In preparation for implementing the asm-generic atomic bitops in terms
of atomic_long_*, we need to prevent asm/atomic.h implementations from
pulling in linux/bitops.h. A common reason for this include is for the
BITS_PER_BYTE definition, so move this and some other BIT() and masking
macros into a new header file, linux/bits.h

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 include/linux/bitops.h | 22 +---------------------
 include/linux/bits.h   | 26 ++++++++++++++++++++++++++
 2 files changed, 27 insertions(+), 21 deletions(-)
 create mode 100644 include/linux/bits.h

diff --git a/include/linux/bitops.h b/include/linux/bitops.h
index 4cac4e1a72ff..af419012d77d 100644
--- a/include/linux/bitops.h
+++ b/include/linux/bitops.h
@@ -2,29 +2,9 @@
 #ifndef _LINUX_BITOPS_H
 #define _LINUX_BITOPS_H
 #include <asm/types.h>
+#include <linux/bits.h>
 
-#ifdef	__KERNEL__
-#define BIT(nr)			(1UL << (nr))
-#define BIT_ULL(nr)		(1ULL << (nr))
-#define BIT_MASK(nr)		(1UL << ((nr) % BITS_PER_LONG))
-#define BIT_WORD(nr)		((nr) / BITS_PER_LONG)
-#define BIT_ULL_MASK(nr)	(1ULL << ((nr) % BITS_PER_LONG_LONG))
-#define BIT_ULL_WORD(nr)	((nr) / BITS_PER_LONG_LONG)
-#define BITS_PER_BYTE		8
 #define BITS_TO_LONGS(nr)	DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(long))
-#endif
-
-/*
- * Create a contiguous bitmask starting@bit position @l and ending at
- * position @h. For example
- * GENMASK_ULL(39, 21) gives us the 64bit vector 0x000000ffffe00000.
- */
-#define GENMASK(h, l) \
-	(((~0UL) - (1UL << (l)) + 1) & (~0UL >> (BITS_PER_LONG - 1 - (h))))
-
-#define GENMASK_ULL(h, l) \
-	(((~0ULL) - (1ULL << (l)) + 1) & \
-	 (~0ULL >> (BITS_PER_LONG_LONG - 1 - (h))))
 
 extern unsigned int __sw_hweight8(unsigned int w);
 extern unsigned int __sw_hweight16(unsigned int w);
diff --git a/include/linux/bits.h b/include/linux/bits.h
new file mode 100644
index 000000000000..2b7b532c1d51
--- /dev/null
+++ b/include/linux/bits.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __LINUX_BITS_H
+#define __LINUX_BITS_H
+#include <asm/bitsperlong.h>
+
+#define BIT(nr)			(1UL << (nr))
+#define BIT_ULL(nr)		(1ULL << (nr))
+#define BIT_MASK(nr)		(1UL << ((nr) % BITS_PER_LONG))
+#define BIT_WORD(nr)		((nr) / BITS_PER_LONG)
+#define BIT_ULL_MASK(nr)	(1ULL << ((nr) % BITS_PER_LONG_LONG))
+#define BIT_ULL_WORD(nr)	((nr) / BITS_PER_LONG_LONG)
+#define BITS_PER_BYTE		8
+
+/*
+ * Create a contiguous bitmask starting@bit position @l and ending at
+ * position @h. For example
+ * GENMASK_ULL(39, 21) gives us the 64bit vector 0x000000ffffe00000.
+ */
+#define GENMASK(h, l) \
+	(((~0UL) - (1UL << (l)) + 1) & (~0UL >> (BITS_PER_LONG - 1 - (h))))
+
+#define GENMASK_ULL(h, l) \
+	(((~0ULL) - (1ULL << (l)) + 1) & \
+	 (~0ULL >> (BITS_PER_LONG_LONG - 1 - (h))))
+
+#endif	/* __LINUX_BITS_H */
-- 
2.1.4

^ permalink raw reply related

* [RESEND PATCH v2 4/9] openrisc: Don't pull in all of linux/bitops.h in asm/cmpxchg.h
From: Will Deacon @ 2018-06-19 12:53 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1529412794-17720-1-git-send-email-will.deacon@arm.com>

The openrisc implementation of asm/cmpxchg.h pulls in linux/bitops.h
so that it can refer to BITS_PER_BYTE. It also transitively relies on
this pulling in linux/compiler.h for READ_ONCE().

Replace the #include with linux/bits.h and linux/compiler.h

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/openrisc/include/asm/cmpxchg.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/openrisc/include/asm/cmpxchg.h b/arch/openrisc/include/asm/cmpxchg.h
index d29f7db53906..f9cd43a39d72 100644
--- a/arch/openrisc/include/asm/cmpxchg.h
+++ b/arch/openrisc/include/asm/cmpxchg.h
@@ -16,8 +16,9 @@
 #ifndef __ASM_OPENRISC_CMPXCHG_H
 #define __ASM_OPENRISC_CMPXCHG_H
 
+#include  <linux/bits.h>
+#include  <linux/compiler.h>
 #include  <linux/types.h>
-#include  <linux/bitops.h>
 
 #define __HAVE_ARCH_CMPXCHG 1
 
-- 
2.1.4

^ permalink raw reply related

* [RESEND PATCH v2 5/9] sh: Don't pull in all of linux/bitops.h in asm/cmpxchg-xchg.h
From: Will Deacon @ 2018-06-19 12:53 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1529412794-17720-1-git-send-email-will.deacon@arm.com>

The sh implementation of asm/cmpxchg-xchg.h pulls in linux/bitops.h
so that it can refer to BITS_PER_BYTE. It also transitively relies on
this pulling in linux/compiler.h for READ_ONCE().

Replace the #include with linux/bits.h and linux/compiler.h

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/sh/include/asm/cmpxchg-xchg.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/sh/include/asm/cmpxchg-xchg.h b/arch/sh/include/asm/cmpxchg-xchg.h
index 1e881f5db659..593a9704782b 100644
--- a/arch/sh/include/asm/cmpxchg-xchg.h
+++ b/arch/sh/include/asm/cmpxchg-xchg.h
@@ -8,7 +8,8 @@
  * This work is licensed under the terms of the GNU GPL, version 2.  See the
  * file "COPYING" in the main directory of this archive for more details.
  */
-#include <linux/bitops.h>
+#include <linux/bits.h>
+#include <linux/compiler.h>
 #include <asm/byteorder.h>
 
 /*
-- 
2.1.4

^ permalink raw reply related

* [RESEND PATCH v2 6/9] asm-generic/bitops/atomic.h: Rewrite using atomic_*
From: Will Deacon @ 2018-06-19 12:53 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1529412794-17720-1-git-send-email-will.deacon@arm.com>

The atomic bitops can actually be implemented pretty efficiently using
the atomic_* ops, rather than explicit use of spinlocks.

Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 include/asm-generic/bitops/atomic.h | 188 +++++++-----------------------------
 1 file changed, 33 insertions(+), 155 deletions(-)

diff --git a/include/asm-generic/bitops/atomic.h b/include/asm-generic/bitops/atomic.h
index 04deffaf5f7d..dd90c9792909 100644
--- a/include/asm-generic/bitops/atomic.h
+++ b/include/asm-generic/bitops/atomic.h
@@ -2,189 +2,67 @@
 #ifndef _ASM_GENERIC_BITOPS_ATOMIC_H_
 #define _ASM_GENERIC_BITOPS_ATOMIC_H_
 
-#include <asm/types.h>
-#include <linux/irqflags.h>
-
-#ifdef CONFIG_SMP
-#include <asm/spinlock.h>
-#include <asm/cache.h>		/* we use L1_CACHE_BYTES */
-
-/* Use an array of spinlocks for our atomic_ts.
- * Hash function to index into a different SPINLOCK.
- * Since "a" is usually an address, use one spinlock per cacheline.
- */
-#  define ATOMIC_HASH_SIZE 4
-#  define ATOMIC_HASH(a) (&(__atomic_hash[ (((unsigned long) a)/L1_CACHE_BYTES) & (ATOMIC_HASH_SIZE-1) ]))
-
-extern arch_spinlock_t __atomic_hash[ATOMIC_HASH_SIZE] __lock_aligned;
-
-/* Can't use raw_spin_lock_irq because of #include problems, so
- * this is the substitute */
-#define _atomic_spin_lock_irqsave(l,f) do {	\
-	arch_spinlock_t *s = ATOMIC_HASH(l);	\
-	local_irq_save(f);			\
-	arch_spin_lock(s);			\
-} while(0)
-
-#define _atomic_spin_unlock_irqrestore(l,f) do {	\
-	arch_spinlock_t *s = ATOMIC_HASH(l);		\
-	arch_spin_unlock(s);				\
-	local_irq_restore(f);				\
-} while(0)
-
-
-#else
-#  define _atomic_spin_lock_irqsave(l,f) do { local_irq_save(f); } while (0)
-#  define _atomic_spin_unlock_irqrestore(l,f) do { local_irq_restore(f); } while (0)
-#endif
+#include <linux/atomic.h>
+#include <linux/compiler.h>
+#include <asm/barrier.h>
 
 /*
- * NMI events can occur at any time, including when interrupts have been
- * disabled by *_irqsave().  So you can get NMI events occurring while a
- * *_bit function is holding a spin lock.  If the NMI handler also wants
- * to do bit manipulation (and they do) then you can get a deadlock
- * between the original caller of *_bit() and the NMI handler.
- *
- * by Keith Owens
+ * Implementation of atomic bitops using atomic-fetch ops.
+ * See Documentation/atomic_bitops.txt for details.
  */
 
-/**
- * set_bit - Atomically set a bit in memory
- * @nr: the bit to set
- * @addr: the address to start counting from
- *
- * This function is atomic and may not be reordered.  See __set_bit()
- * if you do not require the atomic guarantees.
- *
- * Note: there are no guarantees that this function will not be reordered
- * on non x86 architectures, so if you are writing portable code,
- * make sure not to rely on its reordering guarantees.
- *
- * Note that @nr may be almost arbitrarily large; this function is not
- * restricted to acting on a single-word quantity.
- */
-static inline void set_bit(int nr, volatile unsigned long *addr)
+static inline void set_bit(unsigned int nr, volatile unsigned long *p)
 {
-	unsigned long mask = BIT_MASK(nr);
-	unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr);
-	unsigned long flags;
-
-	_atomic_spin_lock_irqsave(p, flags);
-	*p  |= mask;
-	_atomic_spin_unlock_irqrestore(p, flags);
+	p += BIT_WORD(nr);
+	atomic_long_or(BIT_MASK(nr), (atomic_long_t *)p);
 }
 
-/**
- * clear_bit - Clears a bit in memory
- * @nr: Bit to clear
- * @addr: Address to start counting from
- *
- * clear_bit() is atomic and may not be reordered.  However, it does
- * not contain a memory barrier, so if it is used for locking purposes,
- * you should call smp_mb__before_atomic() and/or smp_mb__after_atomic()
- * in order to ensure changes are visible on other processors.
- */
-static inline void clear_bit(int nr, volatile unsigned long *addr)
+static inline void clear_bit(unsigned int nr, volatile unsigned long *p)
 {
-	unsigned long mask = BIT_MASK(nr);
-	unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr);
-	unsigned long flags;
-
-	_atomic_spin_lock_irqsave(p, flags);
-	*p &= ~mask;
-	_atomic_spin_unlock_irqrestore(p, flags);
+	p += BIT_WORD(nr);
+	atomic_long_andnot(BIT_MASK(nr), (atomic_long_t *)p);
 }
 
-/**
- * change_bit - Toggle a bit in memory
- * @nr: Bit to change
- * @addr: Address to start counting from
- *
- * change_bit() is atomic and may not be reordered. It may be
- * reordered on other architectures than x86.
- * Note that @nr may be almost arbitrarily large; this function is not
- * restricted to acting on a single-word quantity.
- */
-static inline void change_bit(int nr, volatile unsigned long *addr)
+static inline void change_bit(unsigned int nr, volatile unsigned long *p)
 {
-	unsigned long mask = BIT_MASK(nr);
-	unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr);
-	unsigned long flags;
-
-	_atomic_spin_lock_irqsave(p, flags);
-	*p ^= mask;
-	_atomic_spin_unlock_irqrestore(p, flags);
+	p += BIT_WORD(nr);
+	atomic_long_xor(BIT_MASK(nr), (atomic_long_t *)p);
 }
 
-/**
- * test_and_set_bit - Set a bit and return its old value
- * @nr: Bit to set
- * @addr: Address to count from
- *
- * This operation is atomic and cannot be reordered.
- * It may be reordered on other architectures than x86.
- * It also implies a memory barrier.
- */
-static inline int test_and_set_bit(int nr, volatile unsigned long *addr)
+static inline int test_and_set_bit(unsigned int nr, volatile unsigned long *p)
 {
+	long old;
 	unsigned long mask = BIT_MASK(nr);
-	unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr);
-	unsigned long old;
-	unsigned long flags;
 
-	_atomic_spin_lock_irqsave(p, flags);
-	old = *p;
-	*p = old | mask;
-	_atomic_spin_unlock_irqrestore(p, flags);
+	p += BIT_WORD(nr);
+	if (READ_ONCE(*p) & mask)
+		return 1;
 
-	return (old & mask) != 0;
+	old = atomic_long_fetch_or(mask, (atomic_long_t *)p);
+	return !!(old & mask);
 }
 
-/**
- * test_and_clear_bit - Clear a bit and return its old value
- * @nr: Bit to clear
- * @addr: Address to count from
- *
- * This operation is atomic and cannot be reordered.
- * It can be reorderdered on other architectures other than x86.
- * It also implies a memory barrier.
- */
-static inline int test_and_clear_bit(int nr, volatile unsigned long *addr)
+static inline int test_and_clear_bit(unsigned int nr, volatile unsigned long *p)
 {
+	long old;
 	unsigned long mask = BIT_MASK(nr);
-	unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr);
-	unsigned long old;
-	unsigned long flags;
 
-	_atomic_spin_lock_irqsave(p, flags);
-	old = *p;
-	*p = old & ~mask;
-	_atomic_spin_unlock_irqrestore(p, flags);
+	p += BIT_WORD(nr);
+	if (!(READ_ONCE(*p) & mask))
+		return 0;
 
-	return (old & mask) != 0;
+	old = atomic_long_fetch_andnot(mask, (atomic_long_t *)p);
+	return !!(old & mask);
 }
 
-/**
- * test_and_change_bit - Change a bit and return its old value
- * @nr: Bit to change
- * @addr: Address to count from
- *
- * This operation is atomic and cannot be reordered.
- * It also implies a memory barrier.
- */
-static inline int test_and_change_bit(int nr, volatile unsigned long *addr)
+static inline int test_and_change_bit(unsigned int nr, volatile unsigned long *p)
 {
+	long old;
 	unsigned long mask = BIT_MASK(nr);
-	unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr);
-	unsigned long old;
-	unsigned long flags;
-
-	_atomic_spin_lock_irqsave(p, flags);
-	old = *p;
-	*p = old ^ mask;
-	_atomic_spin_unlock_irqrestore(p, flags);
 
-	return (old & mask) != 0;
+	p += BIT_WORD(nr);
+	old = atomic_long_fetch_xor(mask, (atomic_long_t *)p);
+	return !!(old & mask);
 }
 
 #endif /* _ASM_GENERIC_BITOPS_ATOMIC_H */
-- 
2.1.4

^ permalink raw reply related

* [RESEND PATCH v2 7/9] asm-generic/bitops/lock.h: Rewrite using atomic_fetch_*
From: Will Deacon @ 2018-06-19 12:53 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1529412794-17720-1-git-send-email-will.deacon@arm.com>

The lock bitops can be implemented more efficiently using the atomic_fetch_*
ops, which provide finer-grained control over the memory ordering semantics
than the bitops.

Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 include/asm-generic/bitops/lock.h | 68 ++++++++++++++++++++++++++++++++-------
 1 file changed, 56 insertions(+), 12 deletions(-)

diff --git a/include/asm-generic/bitops/lock.h b/include/asm-generic/bitops/lock.h
index 67ab280ad134..3ae021368f48 100644
--- a/include/asm-generic/bitops/lock.h
+++ b/include/asm-generic/bitops/lock.h
@@ -2,6 +2,10 @@
 #ifndef _ASM_GENERIC_BITOPS_LOCK_H_
 #define _ASM_GENERIC_BITOPS_LOCK_H_
 
+#include <linux/atomic.h>
+#include <linux/compiler.h>
+#include <asm/barrier.h>
+
 /**
  * test_and_set_bit_lock - Set a bit and return its old value, for lock
  * @nr: Bit to set
@@ -11,7 +15,20 @@
  * the returned value is 0.
  * It can be used to implement bit locks.
  */
-#define test_and_set_bit_lock(nr, addr)	test_and_set_bit(nr, addr)
+static inline int test_and_set_bit_lock(unsigned int nr,
+					volatile unsigned long *p)
+{
+	long old;
+	unsigned long mask = BIT_MASK(nr);
+
+	p += BIT_WORD(nr);
+	if (READ_ONCE(*p) & mask)
+		return 1;
+
+	old = atomic_long_fetch_or_acquire(mask, (atomic_long_t *)p);
+	return !!(old & mask);
+}
+
 
 /**
  * clear_bit_unlock - Clear a bit in memory, for unlock
@@ -20,11 +37,11 @@
  *
  * This operation is atomic and provides release barrier semantics.
  */
-#define clear_bit_unlock(nr, addr)	\
-do {					\
-	smp_mb__before_atomic();	\
-	clear_bit(nr, addr);		\
-} while (0)
+static inline void clear_bit_unlock(unsigned int nr, volatile unsigned long *p)
+{
+	p += BIT_WORD(nr);
+	atomic_long_fetch_andnot_release(BIT_MASK(nr), (atomic_long_t *)p);
+}
 
 /**
  * __clear_bit_unlock - Clear a bit in memory, for unlock
@@ -37,11 +54,38 @@ do {					\
  *
  * See for example x86's implementation.
  */
-#define __clear_bit_unlock(nr, addr)	\
-do {					\
-	smp_mb__before_atomic();	\
-	clear_bit(nr, addr);		\
-} while (0)
+static inline void __clear_bit_unlock(unsigned int nr,
+				      volatile unsigned long *p)
+{
+	unsigned long old;
 
-#endif /* _ASM_GENERIC_BITOPS_LOCK_H_ */
+	p += BIT_WORD(nr);
+	old = READ_ONCE(*p);
+	old &= ~BIT_MASK(nr);
+	atomic_long_set_release((atomic_long_t *)p, old);
+}
+
+/**
+ * clear_bit_unlock_is_negative_byte - Clear a bit in memory and test if bottom
+ *                                     byte is negative, for unlock.
+ * @nr: the bit to clear
+ * @addr: the address to start counting from
+ *
+ * This is a bit of a one-trick-pony for the filemap code, which clears
+ * PG_locked and tests PG_waiters,
+ */
+#ifndef clear_bit_unlock_is_negative_byte
+static inline bool clear_bit_unlock_is_negative_byte(unsigned int nr,
+						     volatile unsigned long *p)
+{
+	long old;
+	unsigned long mask = BIT_MASK(nr);
+
+	p += BIT_WORD(nr);
+	old = atomic_long_fetch_andnot_release(mask, (atomic_long_t *)p);
+	return !!(old & BIT(7));
+}
+#define clear_bit_unlock_is_negative_byte clear_bit_unlock_is_negative_byte
+#endif
 
+#endif /* _ASM_GENERIC_BITOPS_LOCK_H_ */
-- 
2.1.4

^ permalink raw reply related

* [RESEND PATCH v2 8/9] arm64: Replace our atomic/lock bitop implementations with asm-generic
From: Will Deacon @ 2018-06-19 12:53 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1529412794-17720-1-git-send-email-will.deacon@arm.com>

The asm-generic/bitops/{atomic,lock}.h implementations are built around
the atomic-fetch ops, which we implement efficiently for both LSE and
LL/SC systems. Use that instead of our hand-rolled, out-of-line bitops.S.

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/include/asm/bitops.h | 14 ++------
 arch/arm64/lib/Makefile         |  2 +-
 arch/arm64/lib/bitops.S         | 76 -----------------------------------------
 3 files changed, 3 insertions(+), 89 deletions(-)
 delete mode 100644 arch/arm64/lib/bitops.S

diff --git a/arch/arm64/include/asm/bitops.h b/arch/arm64/include/asm/bitops.h
index 9c19594ce7cb..13501460be6b 100644
--- a/arch/arm64/include/asm/bitops.h
+++ b/arch/arm64/include/asm/bitops.h
@@ -17,22 +17,11 @@
 #define __ASM_BITOPS_H
 
 #include <linux/compiler.h>
-#include <asm/barrier.h>
 
 #ifndef _LINUX_BITOPS_H
 #error only <linux/bitops.h> can be included directly
 #endif
 
-/*
- * Little endian assembly atomic bitops.
- */
-extern void set_bit(int nr, volatile unsigned long *p);
-extern void clear_bit(int nr, volatile unsigned long *p);
-extern void change_bit(int nr, volatile unsigned long *p);
-extern int test_and_set_bit(int nr, volatile unsigned long *p);
-extern int test_and_clear_bit(int nr, volatile unsigned long *p);
-extern int test_and_change_bit(int nr, volatile unsigned long *p);
-
 #include <asm-generic/bitops/builtin-__ffs.h>
 #include <asm-generic/bitops/builtin-ffs.h>
 #include <asm-generic/bitops/builtin-__fls.h>
@@ -44,8 +33,9 @@ extern int test_and_change_bit(int nr, volatile unsigned long *p);
 
 #include <asm-generic/bitops/sched.h>
 #include <asm-generic/bitops/hweight.h>
-#include <asm-generic/bitops/lock.h>
 
+#include <asm-generic/bitops/atomic.h>
+#include <asm-generic/bitops/lock.h>
 #include <asm-generic/bitops/non-atomic.h>
 #include <asm-generic/bitops/le.h>
 
diff --git a/arch/arm64/lib/Makefile b/arch/arm64/lib/Makefile
index 137710f4dac3..68755fd70dcf 100644
--- a/arch/arm64/lib/Makefile
+++ b/arch/arm64/lib/Makefile
@@ -1,5 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
-lib-y		:= bitops.o clear_user.o delay.o copy_from_user.o	\
+lib-y		:= clear_user.o delay.o copy_from_user.o		\
 		   copy_to_user.o copy_in_user.o copy_page.o		\
 		   clear_page.o memchr.o memcpy.o memmove.o memset.o	\
 		   memcmp.o strcmp.o strncmp.o strlen.o strnlen.o	\
diff --git a/arch/arm64/lib/bitops.S b/arch/arm64/lib/bitops.S
deleted file mode 100644
index 43ac736baa5b..000000000000
--- a/arch/arm64/lib/bitops.S
+++ /dev/null
@@ -1,76 +0,0 @@
-/*
- * Based on arch/arm/lib/bitops.h
- *
- * Copyright (C) 2013 ARM Ltd.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- */
-
-#include <linux/linkage.h>
-#include <asm/assembler.h>
-#include <asm/lse.h>
-
-/*
- * x0: bits 5:0  bit offset
- *     bits 31:6 word offset
- * x1: address
- */
-	.macro	bitop, name, llsc, lse
-ENTRY(	\name	)
-	and	w3, w0, #63		// Get bit offset
-	eor	w0, w0, w3		// Clear low bits
-	mov	x2, #1
-	add	x1, x1, x0, lsr #3	// Get word offset
-alt_lse "	prfm	pstl1strm, [x1]",	"nop"
-	lsl	x3, x2, x3		// Create mask
-
-alt_lse	"1:	ldxr	x2, [x1]",		"\lse	x3, [x1]"
-alt_lse	"	\llsc	x2, x2, x3",		"nop"
-alt_lse	"	stxr	w0, x2, [x1]",		"nop"
-alt_lse	"	cbnz	w0, 1b",		"nop"
-
-	ret
-ENDPROC(\name	)
-	.endm
-
-	.macro	testop, name, llsc, lse
-ENTRY(	\name	)
-	and	w3, w0, #63		// Get bit offset
-	eor	w0, w0, w3		// Clear low bits
-	mov	x2, #1
-	add	x1, x1, x0, lsr #3	// Get word offset
-alt_lse "	prfm	pstl1strm, [x1]",	"nop"
-	lsl	x4, x2, x3		// Create mask
-
-alt_lse	"1:	ldxr	x2, [x1]",		"\lse	x4, x2, [x1]"
-	lsr	x0, x2, x3
-alt_lse	"	\llsc	x2, x2, x4",		"nop"
-alt_lse	"	stlxr	w5, x2, [x1]",		"nop"
-alt_lse	"	cbnz	w5, 1b",		"nop"
-alt_lse	"	dmb	ish",			"nop"
-
-	and	x0, x0, #1
-	ret
-ENDPROC(\name	)
-	.endm
-
-/*
- * Atomic bit operations.
- */
-	bitop	change_bit, eor, steor
-	bitop	clear_bit, bic, stclr
-	bitop	set_bit, orr, stset
-
-	testop	test_and_change_bit, eor, ldeoral
-	testop	test_and_clear_bit, bic, ldclral
-	testop	test_and_set_bit, orr, ldsetal
-- 
2.1.4

^ permalink raw reply related

* [RESEND PATCH v2 9/9] arm64: bitops: Include <asm-generic/bitops/ext2-atomic-setbit.h>
From: Will Deacon @ 2018-06-19 12:53 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1529412794-17720-1-git-send-email-will.deacon@arm.com>

asm-generic/bitops/ext2-atomic-setbit.h provides the ext2 atomic bitop
definitions, so we don't need to define our own.

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/include/asm/bitops.h | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/arch/arm64/include/asm/bitops.h b/arch/arm64/include/asm/bitops.h
index 13501460be6b..10d536b1af74 100644
--- a/arch/arm64/include/asm/bitops.h
+++ b/arch/arm64/include/asm/bitops.h
@@ -38,11 +38,6 @@
 #include <asm-generic/bitops/lock.h>
 #include <asm-generic/bitops/non-atomic.h>
 #include <asm-generic/bitops/le.h>
-
-/*
- * Ext2 is defined to use little-endian byte ordering.
- */
-#define ext2_set_bit_atomic(lock, nr, p)	test_and_set_bit_le(nr, p)
-#define ext2_clear_bit_atomic(lock, nr, p)	test_and_clear_bit_le(nr, p)
+#include <asm-generic/bitops/ext2-atomic-setbit.h>
 
 #endif /* __ASM_BITOPS_H */
-- 
2.1.4

^ permalink raw reply related

* [PATCH 02/11] tpm/tpm_i2c_infineon: switch to i2c_lock_segment
From: Jarkko Sakkinen @ 2018-06-19 12:56 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180615101506.8012-3-peda@axentia.se>

On Fri, Jun 15, 2018 at 12:14:57PM +0200, Peter Rosin wrote:
> Locking the root adapter for __i2c_transfer will deadlock if the
> device sits behind a mux-locked I2C mux. Switch to the finer-grained
> i2c_lock_segment. If the device does not sit behind a mux-locked mux,
> the two locking variants are equivalent.
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>

Can you quickly explain (or give a reference) the difference with these
functions? Not an expert in this area. Thanks.

/Jarkko

^ permalink raw reply

* [PATCH v2 0/5] crypto: ccree: cleanup, fixes and R-Car enabling
From: Geert Uytterhoeven @ 2018-06-19 12:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527171551-21979-1-git-send-email-gilad@benyossef.com>

Hi Gilad,

On Thu, May 24, 2018 at 4:19 PM Gilad Ben-Yossef <gilad@benyossef.com> wrote:
> The patch set enables the use of CryptoCell found in some Renesas R-Car
> Salvator-X boards and fixes some driver issues uncovered that prevented
> to work properly.

With DEBUG enabled on R-Car H3, I see lots of

    ccree e6601000.crypto: IRR includes unknown cause bits (0x00000098)
    ccree e6601000.crypto: IRR includes unknown cause bits (0x000000C0)
    ccree e6601000.crypto: IRR includes unknown cause bits (0x000000D0)
    ccree e6601000.crypto: IRR includes unknown cause bits (0x000000D8)
    ccree e6601000.crypto: IRR includes unknown cause bits (0x000000E0)
    ccree e6601000.crypto: IRR includes unknown cause bits (0x000000F0)
    ccree e6601000.crypto: IRR includes unknown cause bits (0x000000F8)

during boot. Is that expected?

Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [PATCH] arm: Hook up SYNC_CORE functionality for sys_membarrier()
From: Will Deacon @ 2018-06-19 12:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1431651959.17300.1529412602455.JavaMail.zimbra@efficios.com>

Hi Mathieu,

On Tue, Jun 19, 2018 at 08:50:02AM -0400, Mathieu Desnoyers wrote:
> ----- On Jun 19, 2018, at 8:22 AM, Will Deacon will.deacon at arm.com wrote:
> 
> > Exception return implies context synchronization, so we can hook up the
> > SYNC_CORE option to sys_membarrier() simply by selecting the Kconfig option,
> > just like we've done for arm64 already.
> > 
> > Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> > Cc: Orion Hodson <oth@google.com>
> > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > ---
> > arch/arm/Kconfig | 1 +
> > 1 file changed, 1 insertion(+)
> > 
> > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > index 54eeb8d00bc6..b0ac18547370 100644
> > --- a/arch/arm/Kconfig
> > +++ b/arch/arm/Kconfig
> > @@ -9,6 +9,7 @@ config ARM
> > 	select ARCH_HAS_ELF_RANDOMIZE
> > 	select ARCH_HAS_FORTIFY_SOURCE
> > 	select ARCH_HAS_KCOV
> > +	select ARCH_HAS_MEMBARRIER_SYNC_CORE
> 
> In addition to this, we added this comment in arch/arm64/kernel/entry.S:
> 
> +       /*
> +        * ARCH_HAS_MEMBARRIER_SYNC_CORE rely on eret context synchronization
> +        * when returning from IPI handler, and when returning to user-space.
> +        */
> 
> So I would expect a similar comment in arch/arm/kernel/entry-header.S, within
> svc_exit and svc_exit_via_fiq:
> 
>         /*
>          * ARCH_HAS_MEMBARRIER_SYNC_CORE rely on [insn] context synchronization
>          * when returning from IPI handler, and when returning to user-space.
>          */

Bah, you know I hate that comment ;) I should update arch-support.txt,
though. Diff below.

> Which instruction exactly is responsible for context synchronization on
> arm32 ?

It's the act of doing an exception return, so there are multiple instruction
sequences to do that on 32-bit arm.

Will

--->8

diff --git a/Documentation/features/sched/membarrier-sync-core/arch-support.txt b/Documentation/features/sched/membarrier-sync-core/arch-support.txt
index dbdf62907703..c7858dd1ea8f 100644
--- a/Documentation/features/sched/membarrier-sync-core/arch-support.txt
+++ b/Documentation/features/sched/membarrier-sync-core/arch-support.txt
@@ -5,10 +5,10 @@
 #
 # Architecture requirements
 #
-# * arm64
+# * arm/arm64
 #
-# Rely on eret context synchronization when returning from IPI handler, and
-# when returning to user-space.
+# Rely on implicit context synchronization as a result of exception return
+# when returning from IPI handler, and when returning to user-space.
 #
 # * x86
 #
@@ -31,7 +31,7 @@
     -----------------------
     |       alpha: | TODO |
     |         arc: | TODO |
-    |         arm: | TODO |
+    |         arm: |  ok  |
     |       arm64: |  ok  |
     |         c6x: | TODO |
     |       h8300: | TODO |

^ permalink raw reply related

* [PATCH] serial: mps2-uart: Initialize early console
From: Guenter Roeck @ 2018-06-19 13:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <173551dd-3632-08a6-ec42-2ee416ccb9ad@arm.com>

On 06/19/2018 02:07 AM, Vladimir Murzin wrote:
> Hi Guenter,
> 
> On 19/06/18 05:54, Guenter Roeck wrote:
>> The early console code for mps2-uart assumes that the serial hardware is
>> enabled for transmit when the system boots. However, this is not the case
>> after reset. This results in a hang in mps2_early_putchar() if the serial
>> transmitter is not enabled by a boot loader or ROM monitor.
> 
> I was under impression that for earlycon there is an assumption/requirement
> that the serial port must already be setup and configured. For instance, I
> see such requirement for pl011. So it looks like boot code's fault not to
> enable serial (for mps2 it needs to setup BAUDDIV as well).
> 

Good to know. Fine with me as well; I wasn't aware that such a requirement
existed.

Guenter

> I'm not against the patch per se, but I'd like to hear if my understanding of
> earlycon requirements is correct or not.
> 
> Cheers
> Vladimir
> 
>>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> ---
>>   drivers/tty/serial/mps2-uart.c | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/tty/serial/mps2-uart.c b/drivers/tty/serial/mps2-uart.c
>> index 9f8f63719126..0743a0551ce1 100644
>> --- a/drivers/tty/serial/mps2-uart.c
>> +++ b/drivers/tty/serial/mps2-uart.c
>> @@ -448,6 +448,14 @@ static struct console mps2_uart_console = {
>>   
>>   #define MPS2_SERIAL_CONSOLE (&mps2_uart_console)
>>   
>> +static void mps2_early_init(struct uart_port *port)
>> +{
>> +	u8 control = readb(port->membase + UARTn_CTRL);
>> +
>> +	control |= UARTn_CTRL_TX_ENABLE;
>> +	writeb(control, port->membase + UARTn_CTRL);
>> +}
>> +
>>   static void mps2_early_putchar(struct uart_port *port, int ch)
>>   {
>>   	while (readb(port->membase + UARTn_STATE) & UARTn_STATE_TX_FULL)
>> @@ -469,6 +477,7 @@ static int __init mps2_early_console_setup(struct earlycon_device *device,
>>   	if (!device->port.membase)
>>   		return -ENODEV;
>>   
>> +	mps2_early_init(&device->port);
>>   	device->con->write = mps2_early_write;
>>   
>>   	return 0;
>>
> 
> 

^ permalink raw reply

* [PATCH] iommu/io-pgtable-arm-v7s: Abort allocation when table address overflows the PTE
From: Will Deacon @ 2018-06-19 13:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180619125224.1008-1-jean-philippe.brucker@arm.com>

On Tue, Jun 19, 2018 at 01:52:24PM +0100, Jean-Philippe Brucker wrote:
> When run on a 64-bit system in selftest, the v7s driver may obtain page
> table with physical addresses larger than 32-bit. Level-2 tables are 1KB
> and are are allocated with slab, which doesn't accept the GFP_DMA32
> flag. Currently map() truncates the address written in the PTE, causing
> iova_to_phys() or unmap() to access invalid memory. Kasan reports it as
> a use-after-free. To avoid any nasty surprise, test if the physical
> address fits in a PTE before returning a new table. 32-bit systems,
> which are the main users of this page table format, shouldn't see any
> difference.
> 
> Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
> ---
>  drivers/iommu/io-pgtable-arm-v7s.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)

Thanks, I'll queue this too.

It would be nice if we could use GFP_DMA32 instead of failing the request,
but that doesn't work at all with the kmem_cache so we'd have to roll our
own l2 allocator if we wanted to support this.

Will

^ permalink raw reply

* [PATCH] arm: Hook up SYNC_CORE functionality for sys_membarrier()
From: Mathieu Desnoyers @ 2018-06-19 13:03 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180619125849.GH13984@arm.com>

----- On Jun 19, 2018, at 8:58 AM, Will Deacon will.deacon at arm.com wrote:

> Hi Mathieu,
> 
> On Tue, Jun 19, 2018 at 08:50:02AM -0400, Mathieu Desnoyers wrote:
>> ----- On Jun 19, 2018, at 8:22 AM, Will Deacon will.deacon at arm.com wrote:
>> 
>> > Exception return implies context synchronization, so we can hook up the
>> > SYNC_CORE option to sys_membarrier() simply by selecting the Kconfig option,
>> > just like we've done for arm64 already.
>> > 
>> > Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>> > Cc: Orion Hodson <oth@google.com>
>> > Signed-off-by: Will Deacon <will.deacon@arm.com>
>> > ---
>> > arch/arm/Kconfig | 1 +
>> > 1 file changed, 1 insertion(+)
>> > 
>> > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> > index 54eeb8d00bc6..b0ac18547370 100644
>> > --- a/arch/arm/Kconfig
>> > +++ b/arch/arm/Kconfig
>> > @@ -9,6 +9,7 @@ config ARM
>> > 	select ARCH_HAS_ELF_RANDOMIZE
>> > 	select ARCH_HAS_FORTIFY_SOURCE
>> > 	select ARCH_HAS_KCOV
>> > +	select ARCH_HAS_MEMBARRIER_SYNC_CORE
>> 
>> In addition to this, we added this comment in arch/arm64/kernel/entry.S:
>> 
>> +       /*
>> +        * ARCH_HAS_MEMBARRIER_SYNC_CORE rely on eret context synchronization
>> +        * when returning from IPI handler, and when returning to user-space.
>> +        */
>> 
>> So I would expect a similar comment in arch/arm/kernel/entry-header.S, within
>> svc_exit and svc_exit_via_fiq:
>> 
>>         /*
>>          * ARCH_HAS_MEMBARRIER_SYNC_CORE rely on [insn] context synchronization
>>          * when returning from IPI handler, and when returning to user-space.
>>          */
> 
> Bah, you know I hate that comment ;) I should update arch-support.txt,
> though. Diff below.

It works for me with this update, thanks!

Mathieu

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

^ permalink raw reply

* [PATCH 02/11] tpm/tpm_i2c_infineon: switch to i2c_lock_segment
From: Peter Rosin @ 2018-06-19 13:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180619125618.GB5609@linux.intel.com>

On 2018-06-19 14:56, Jarkko Sakkinen wrote:
> On Fri, Jun 15, 2018 at 12:14:57PM +0200, Peter Rosin wrote:
>> Locking the root adapter for __i2c_transfer will deadlock if the
>> device sits behind a mux-locked I2C mux. Switch to the finer-grained
>> i2c_lock_segment. If the device does not sit behind a mux-locked mux,
>> the two locking variants are equivalent.
>>
>> Signed-off-by: Peter Rosin <peda@axentia.se>
> 
> Can you quickly explain (or give a reference) the difference with these
> functions? Not an expert in this area. Thanks.

There are some words in the cover letter. If you need more, there's
always Documentation/i2c/i2c-topology. Hope that helps, otherwise I'll
try to explain better...

Cheers,
Peter

^ permalink raw reply

* [PATCHv3 10/19] arm64: convert native/compat syscall entry to C
From: Mark Rutland @ 2018-06-19 13:15 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180619121814.GR22983@e103592.cambridge.arm.com>

On Tue, Jun 19, 2018 at 01:18:17PM +0100, Dave Martin wrote:
> On Mon, Jun 18, 2018 at 01:03:01PM +0100, Mark Rutland wrote:
> > +static inline void sve_user_reset(void)
> > +{
> 
> Can we call this "sve_user_discard" please?
> 
> "Reset" is a reasonable name for the concept, but the "discard"
> terminology has been used elsewhere.

Sure; done.

> > +	if (!system_supports_sve())
> > +		return;
> > +
> > +	/*
> > +	 * task_fpsimd_load() won't be called to update CPACR_EL1 in
> > +	 * ret_to_user unless TIF_FOREIGN_FPSTATE is still set, which only
> > +	 * happens if a context switch or kernel_neon_begin() or context
> > +	 * modification (sigreturn, ptrace) intervenes.
> > +	 * So, ensure that CPACR_EL1 is already correct for the fast-path case.
> > +	 */
> 
> This comment should go after clear_thead_flag(), since it describes not
> the purpose of this function but the presence of sve_user_disable().
> 
> > +	clear_thread_flag(TIF_SVE);
> > +	sve_user_disable();
> > +}

Good point. I've moved the clear_thread_flag(TIF_SVE) above the comment
(with a blank line before the comment).

Thanks,
Mark.

^ permalink raw reply

* Dynamic ftrace self test broken on ARM
From: Steven Rostedt @ 2018-06-19 13:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <b9f269ed2143688c38cdcbba7367b101@agner.ch>

On Tue, 19 Jun 2018 10:16:19 +0200
Stefan Agner <stefan@agner.ch> wrote:

> > I'm guessing that it boots fine with CONFIG_FTRACE_STARTUP_TEST=n? Can
> > you try disable the tracers to see if it's the function graph or
> > function tracer that is causing the issue? That is, turn off
> > CONFIG_FUNCTION_GRAPH_TRACER and test it again, and if that crashes,
> > turn off CONFIG_FUNCTION_TRACER to make sure the crash goes away there
> > too.  
> 
> Without CONFIG_FTRACE_STARTUP_TEST the kernel boots fine.
> 
> CONFIG_FUNCTION_TRACER=y
> # CONFIG_FUNCTION_GRAPH_TRACER is not set
> # CONFIG_SCHED_TRACER is not set
> CONFIG_FTRACE_STARTUP_TEST=y

OK, so it's not a graph tracer issue, but a function tracer issue.

> 
> Crashes with the same stack trace.
> 
> # CONFIG_FUNCTION_TRACER is not set
> CONFIG_SCHED_TRACER=y
> CONFIG_FTRACE_STARTUP_TEST=y
> 
> Runs tracer tests and boots fine.

Thanks. Did this ever work? And if so, perhaps you have time to perform
a bisect. If you have a ktest setup, you can have it run the bisect for
you (over night).

-- Steve

^ permalink raw reply

* [PATCH] dmaengine: pl330: report BURST residue granularity
From: Marek Szyprowski @ 2018-06-19 13:20 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CGME20180619132105eucas1p2d65cbd799e60a8ab1849c4ae716275c4@eucas1p2.samsung.com>

The reported residue is already calculated in BURST unit granularity, so
advertise this capability properly to other devices in the system.

Fixes: aee4d1fac887 ("dmaengine: pl330: improve pl330_tx_status() function")
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/dma/pl330.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index defcdde4d358..de0957fe9668 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -3033,7 +3033,7 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id)
 	pd->src_addr_widths = PL330_DMA_BUSWIDTHS;
 	pd->dst_addr_widths = PL330_DMA_BUSWIDTHS;
 	pd->directions = BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV);
-	pd->residue_granularity = DMA_RESIDUE_GRANULARITY_SEGMENT;
+	pd->residue_granularity = DMA_RESIDUE_GRANULARITY_BURST;
 	pd->max_burst = ((pl330->quirks & PL330_QUIRK_BROKEN_NO_FLUSHP) ?
 			 1 : PL330_MAX_BURST);
 
-- 
2.17.1

^ permalink raw reply related

* [RFC PATCH] ARM: Use logical or instead of addition for badr address calculation
From: Guenter Roeck @ 2018-06-19 13:29 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAKv+Gu8Byj7G3Awnq4Y90Wot=GJTx0gO5A7-960au9ZkBBL2=A@mail.gmail.com>

Hi Ard,

On 06/19/2018 12:48 AM, Ard Biesheuvel wrote:
> On 19 June 2018 at 07:07, Guenter Roeck <linux@roeck-us.net> wrote:
>> Modern assemblers may take the ISA into account when resolving local
>> symbols. This can result in bad address calculations when using badr
>> in the wrong location since the offset + 1 may be added twice, resulting
>> in an even address target for THUMB instructions. This in turn results
>> in an exception at (destination address + 2).
>>
>> Unhandled exception: IPSR = 00000006 LR = fffffff1
>> CPU: 0 PID: 1 Comm: init Not tainted 4.18.0-rc1-00026-gf773e5bdf0c9 #15
>> Hardware name: MPS2 (Device Tree Support)
>> PC is at ret_fast_syscall+0x2/0x58
>> LR is at tty_ioctl+0x2a5/0x528
>> pc : [<21009002>]    lr : [<210d1535>]    psr: 4000000b
>> sp : 21825fa8  ip : 0000001c  fp : 21a95892
>> r10: 00000000  r9 : 21824000  r8 : 210091c0
>> r7 : 00000036  r6 : 21ae1ee0  r5 : 00000000  r4 : 21ae1f3c
>> r3 : 00000000  r2 : 3d9adc25  r1 : 00000000  r0 : 00000000
>> xPSR: 4000000b
>> CPU: 0 PID: 1 Comm: init Not tainted 4.18.0-rc1-00026-gf773e5bdf0c9 #15
>> Hardware name: MPS2 (Device Tree Support)
>> [<2100bd8d>] (unwind_backtrace) from [<2100b13b>] (show_stack+0xb/0xc)
>> [<2100b13b>] (show_stack) from [<2100b87b>] (__invalid_entry+0x4b/0x4c)
>>
>> Fix the problem by using a logical or instead of an addition. This is
>> less efficient but guaranteed to work.
>>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> ---
>> RFC: I don't really like this, but my ARM assembler knowledge is quite
>> limited. Just dropping the "+ 1" from badr doesn't work because some
>> other code needs it (the image hangs completely if I try that).
>> Ultimately I don't even know if the invoke_syscall macro should just
>> have used adr instead of badr (but then how did this ever work ?).
>>
>> Seen with various toolchains based on gcc 7.x and binutils 2.30 when
>> building and testing MPS2 images.
>>
> 
> Hello Guenter,
> 
> This issue has been discussed before. It appears the binutils people
> suddenly started caring about the thumb annotation of local function
> symbols, resulting in behavior that is not backwards compatible.
> 
> https://marc.info/?l=linux-arm-kernel&m=151143776213636&w=2
> https://sourceware.org/bugzilla/show_bug.cgi?id=21458
> 
> Can you try the fix below please?
> 
>>   arch/arm/include/asm/assembler.h | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
>> index 0cd4dccbae78..24c87ff2060f 100644
>> --- a/arch/arm/include/asm/assembler.h
>> +++ b/arch/arm/include/asm/assembler.h
>> @@ -195,7 +195,8 @@
>>          .irp    c,,eq,ne,cs,cc,mi,pl,vs,vc,hi,ls,ge,lt,gt,le,hs,lo
>>          .macro  badr\c, rd, sym
>>   #ifdef CONFIG_THUMB2_KERNEL
>> -       adr\c   \rd, \sym + 1
>> +       adr\c   \rd, \sym
>> +       orr     \rd, #1
>>   #else
>>          adr\c   \rd, \sym
>>   #endif
>> --
>> 2.7.4
> 
> ----------8<------------
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Date: Tue, 16 Jan 2018 12:12:45 +0000
> Subject: [PATCH] ARM: assembler: prevent ADR from setting the Thumb bit twice
> 
> To work around recent issues where ADR references to Thumb function
> symbols may or may not have the Thumb bit set already when they are
> resolved by GAS, reference the symbol indirectly via a local symbol
> typed as 'object', stripping the ARM/Thumb annotation.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> 
> diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
> index 6ae42ad29518..dd2ff94ad90b 100644
> --- a/arch/arm/include/asm/assembler.h
> +++ b/arch/arm/include/asm/assembler.h
> @@ -195,13 +195,19 @@
>          .irp    c,,eq,ne,cs,cc,mi,pl,vs,vc,hi,ls,ge,lt,gt,le,hs,lo
>          .macro  badr\c, rd, sym
>   #ifdef CONFIG_THUMB2_KERNEL
> -       adr\c   \rd, \sym + 1
> +       __badr  \c, \rd, \sym
>   #else
>          adr\c   \rd, \sym
>   #endif
>          .endm
>          .endr
> 
> +       /* this needs to be a separate macro or \@ does not work correctly */
> +       .macro  __badr, c, rd, sym
> +       .eqv    .Lsym\@, \sym
> +       adr\c   \rd, .Lsym\@ + 1

Wild shot, but the following works for me.

	.eqv    .Lsym\@, \sym + 1
	adr\c	\rd, .Lsym\@

Does it make sense ?

Thanks,
Guenter

^ permalink raw reply

* [PATCH 1/3] arm64: Avoid flush_icache_range() in alternatives patching code
From: Mark Rutland @ 2018-06-19 13:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1529412495-17525-2-git-send-email-will.deacon@arm.com>

On Tue, Jun 19, 2018 at 01:48:13PM +0100, Will Deacon wrote:
> The implementation of flush_icache_range() includes instruction sequences
> which are themselves patched at runtime, so it is not safe to call from
> the patching framework.
> 
> This patch reworks the alternatives cache-flushing code so that it rolls
> its own internal D-cache maintenance using DC CIVAC before invalidating
> the entire I-cache after all alternatives have been applied at boot.
> Modules don't cause any issues, since flush_icache_range() is safe to
> call by the time they are loaded.
> 
> Reported-by: Rohit Khanna <rokhanna@nvidia.com>
> Cc: Alexander Van Brunt <avanbrunt@nvidia.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
>  arch/arm64/include/asm/alternative.h |  7 +++++-
>  arch/arm64/kernel/alternative.c      | 46 ++++++++++++++++++++++++++++++------
>  arch/arm64/kernel/module.c           |  5 ++--
>  3 files changed, 47 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/alternative.h b/arch/arm64/include/asm/alternative.h
> index a91933b1e2e6..4b650ec1d7dd 100644
> --- a/arch/arm64/include/asm/alternative.h
> +++ b/arch/arm64/include/asm/alternative.h
> @@ -28,7 +28,12 @@ typedef void (*alternative_cb_t)(struct alt_instr *alt,
>  				 __le32 *origptr, __le32 *updptr, int nr_inst);
>  
>  void __init apply_alternatives_all(void);
> -void apply_alternatives(void *start, size_t length);
> +
> +#ifdef CONFIG_MODULES
> +void apply_alternatives_module(void *start, size_t length);
> +#else
> +static inline void apply_alternatives_module(void *start, size_t length) { }
> +#endif
>  
>  #define ALTINSTR_ENTRY(feature,cb)					      \
>  	" .word 661b - .\n"				/* label           */ \
> diff --git a/arch/arm64/kernel/alternative.c b/arch/arm64/kernel/alternative.c
> index 5c4bce4ac381..4f3dcc15a5b2 100644
> --- a/arch/arm64/kernel/alternative.c
> +++ b/arch/arm64/kernel/alternative.c
> @@ -122,7 +122,25 @@ static void patch_alternative(struct alt_instr *alt,
>  	}
>  }
>  

It might be worth a comment here, e.g.

/*
 * Since the regular dcache maintenance functions are patched with
 * alteratives, we use an unpatched copy to apply the alternatives
 * safely.
 */

... so as to avoid any helpful cleanup patches in future moving back to
the "optimized" the asm functions.

Regardless:

Acked-by: Mark Rutland <mark.rutland@arm.com>

Mark.

> -static void __apply_alternatives(void *alt_region, bool use_linear_alias)
> +static void clean_dcache_range_nopatch(u64 start, u64 end)
> +{
> +	u64 cur, d_size, ctr_el0;
> +
> +	ctr_el0 = read_sanitised_ftr_reg(SYS_CTR_EL0);
> +	d_size = 4 << cpuid_feature_extract_unsigned_field(ctr_el0,
> +							   CTR_DMINLINE_SHIFT);
> +	cur = start & ~(d_size - 1);
> +	do {
> +		/*
> +		 * We must clean+invalidate to the PoC in order to avoid
> +		 * Cortex-A53 errata 826319, 827319, 824069 and 819472
> +		 * (this corresponds to ARM64_WORKAROUND_CLEAN_CACHE)
> +		 */
> +		asm volatile("dc civac, %0" : : "r" (cur) : "memory");
> +	} while (cur += d_size, cur < end);
> +}
> +
> +static void __apply_alternatives(void *alt_region, bool is_module)
>  {
>  	struct alt_instr *alt;
>  	struct alt_region *region = alt_region;
> @@ -145,7 +163,7 @@ static void __apply_alternatives(void *alt_region, bool use_linear_alias)
>  		pr_info_once("patching kernel code\n");
>  
>  		origptr = ALT_ORIG_PTR(alt);
> -		updptr = use_linear_alias ? lm_alias(origptr) : origptr;
> +		updptr = is_module ? origptr : lm_alias(origptr);
>  		nr_inst = alt->orig_len / AARCH64_INSN_SIZE;
>  
>  		if (alt->cpufeature < ARM64_CB_PATCH)
> @@ -155,8 +173,20 @@ static void __apply_alternatives(void *alt_region, bool use_linear_alias)
>  
>  		alt_cb(alt, origptr, updptr, nr_inst);
>  
> -		flush_icache_range((uintptr_t)origptr,
> -				   (uintptr_t)(origptr + nr_inst));
> +		if (!is_module) {
> +			clean_dcache_range_nopatch((u64)origptr,
> +						   (u64)(origptr + nr_inst));
> +		}
> +	}
> +
> +	/*
> +	 * The core module code takes care of cache maintenance in
> +	 * flush_module_icache().
> +	 */
> +	if (!is_module) {
> +		dsb(ish);
> +		__flush_icache_all();
> +		isb();
>  	}
>  }
>  
> @@ -178,7 +208,7 @@ static int __apply_alternatives_multi_stop(void *unused)
>  		isb();
>  	} else {
>  		BUG_ON(alternatives_applied);
> -		__apply_alternatives(&region, true);
> +		__apply_alternatives(&region, false);
>  		/* Barriers provided by the cache flushing */
>  		WRITE_ONCE(alternatives_applied, 1);
>  	}
> @@ -192,12 +222,14 @@ void __init apply_alternatives_all(void)
>  	stop_machine(__apply_alternatives_multi_stop, NULL, cpu_online_mask);
>  }
>  
> -void apply_alternatives(void *start, size_t length)
> +#ifdef CONFIG_MODULES
> +void apply_alternatives_module(void *start, size_t length)
>  {
>  	struct alt_region region = {
>  		.begin	= start,
>  		.end	= start + length,
>  	};
>  
> -	__apply_alternatives(&region, false);
> +	__apply_alternatives(&region, true);
>  }
> +#endif
> diff --git a/arch/arm64/kernel/module.c b/arch/arm64/kernel/module.c
> index 155fd91e78f4..f0f27aeefb73 100644
> --- a/arch/arm64/kernel/module.c
> +++ b/arch/arm64/kernel/module.c
> @@ -448,9 +448,8 @@ int module_finalize(const Elf_Ehdr *hdr,
>  	const char *secstrs = (void *)hdr + sechdrs[hdr->e_shstrndx].sh_offset;
>  
>  	for (s = sechdrs, se = sechdrs + hdr->e_shnum; s < se; s++) {
> -		if (strcmp(".altinstructions", secstrs + s->sh_name) == 0) {
> -			apply_alternatives((void *)s->sh_addr, s->sh_size);
> -		}
> +		if (strcmp(".altinstructions", secstrs + s->sh_name) == 0)
> +			apply_alternatives_module((void *)s->sh_addr, s->sh_size);
>  #ifdef CONFIG_ARM64_MODULE_PLTS
>  		if (IS_ENABLED(CONFIG_DYNAMIC_FTRACE) &&
>  		    !strcmp(".text.ftrace_trampoline", secstrs + s->sh_name))
> -- 
> 2.1.4
> 

^ permalink raw reply

* [PATCHv3 08/19] arm64: convert raw syscall invocation to C
From: Catalin Marinas @ 2018-06-19 13:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180618120310.39527-9-mark.rutland@arm.com>

On Mon, Jun 18, 2018 at 01:02:59PM +0100, Mark Rutland wrote:
> As a first step towards invoking syscalls with a pt_regs argument,
> convert the raw syscall invocation logic to C. We end up with a bit more
> register shuffling, but the unified invocation logic means we can unify
> the tracing paths, too.
> 
> Previously, assembly had to open-code calls to ni_sys() when the system
> call number was out-of-bounds for the relevant syscall table. This case
> is now handled by invoke_syscall(), and the assembly no longer need to
> handle this case explicitly. This allows the tracing paths to be
> simplfiied and unified, as we no longer need the __ni_sys_trace path and

'simplified'

> the __sys_trace_return label.
> 
> This only converts the invocation of the syscall. The rest of the
> syscall triage and tracing is left in assembly for now, and will be
> converted in subsequent patches.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

^ permalink raw reply

* [PATCH 2/3] arm64: Remove unnecessary ISBs from set_{pte, pmd, pud}
From: Mark Rutland @ 2018-06-19 13:34 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1529412495-17525-3-git-send-email-will.deacon@arm.com>

On Tue, Jun 19, 2018 at 01:48:14PM +0100, Will Deacon wrote:
> Commit 7f0b1bf04511 ("arm64: Fix barriers used for page table modifications")
> fixed a reported issue with fixmap page-table entries not being visible
> to the walker due to a missing DSB instruction. At the same time, it added
> ISB instructions to the arm64 set_{pte,pmd,pud} functions, which are not
> required by the architecture and make little sense in isolation.
> 
> Remove the redundant ISBs.
> 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Steve Capper <steve.capper@linaro.org>
> Signed-off-by: Will Deacon <will.deacon@arm.com>

Acked-by: Mark Rutland <mark.rutland@arm.com>

Mark.

> ---
>  arch/arm64/include/asm/pgtable.h | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> index 9f82d6b53851..1bdeca8918a6 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -224,10 +224,8 @@ static inline void set_pte(pte_t *ptep, pte_t pte)
>  	 * Only if the new pte is valid and kernel, otherwise TLB maintenance
>  	 * or update_mmu_cache() have the necessary barriers.
>  	 */
> -	if (pte_valid_not_user(pte)) {
> +	if (pte_valid_not_user(pte))
>  		dsb(ishst);
> -		isb();
> -	}
>  }
>  
>  extern void __sync_icache_dcache(pte_t pteval);
> @@ -434,7 +432,6 @@ static inline void set_pmd(pmd_t *pmdp, pmd_t pmd)
>  {
>  	WRITE_ONCE(*pmdp, pmd);
>  	dsb(ishst);
> -	isb();
>  }
>  
>  static inline void pmd_clear(pmd_t *pmdp)
> @@ -485,7 +482,6 @@ static inline void set_pud(pud_t *pudp, pud_t pud)
>  {
>  	WRITE_ONCE(*pudp, pud);
>  	dsb(ishst);
> -	isb();
>  }
>  
>  static inline void pud_clear(pud_t *pudp)
> -- 
> 2.1.4
> 

^ permalink raw reply

* [RFC PATCH] ARM: Use logical or instead of addition for badr address calculation
From: Ard Biesheuvel @ 2018-06-19 13:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <b8bff03d-91a7-0e53-a44e-d1924beb382d@roeck-us.net>

On 19 June 2018 at 15:29, Guenter Roeck <linux@roeck-us.net> wrote:
> Hi Ard,
>
>
> On 06/19/2018 12:48 AM, Ard Biesheuvel wrote:
>>
>> On 19 June 2018 at 07:07, Guenter Roeck <linux@roeck-us.net> wrote:
>>>
>>> Modern assemblers may take the ISA into account when resolving local
>>> symbols. This can result in bad address calculations when using badr
>>> in the wrong location since the offset + 1 may be added twice, resulting
>>> in an even address target for THUMB instructions. This in turn results
>>> in an exception at (destination address + 2).
>>>
>>> Unhandled exception: IPSR = 00000006 LR = fffffff1
>>> CPU: 0 PID: 1 Comm: init Not tainted 4.18.0-rc1-00026-gf773e5bdf0c9 #15
>>> Hardware name: MPS2 (Device Tree Support)
>>> PC is at ret_fast_syscall+0x2/0x58
>>> LR is at tty_ioctl+0x2a5/0x528
>>> pc : [<21009002>]    lr : [<210d1535>]    psr: 4000000b
>>> sp : 21825fa8  ip : 0000001c  fp : 21a95892
>>> r10: 00000000  r9 : 21824000  r8 : 210091c0
>>> r7 : 00000036  r6 : 21ae1ee0  r5 : 00000000  r4 : 21ae1f3c
>>> r3 : 00000000  r2 : 3d9adc25  r1 : 00000000  r0 : 00000000
>>> xPSR: 4000000b
>>> CPU: 0 PID: 1 Comm: init Not tainted 4.18.0-rc1-00026-gf773e5bdf0c9 #15
>>> Hardware name: MPS2 (Device Tree Support)
>>> [<2100bd8d>] (unwind_backtrace) from [<2100b13b>] (show_stack+0xb/0xc)
>>> [<2100b13b>] (show_stack) from [<2100b87b>] (__invalid_entry+0x4b/0x4c)
>>>
>>> Fix the problem by using a logical or instead of an addition. This is
>>> less efficient but guaranteed to work.
>>>
>>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>>> ---
>>> RFC: I don't really like this, but my ARM assembler knowledge is quite
>>> limited. Just dropping the "+ 1" from badr doesn't work because some
>>> other code needs it (the image hangs completely if I try that).
>>> Ultimately I don't even know if the invoke_syscall macro should just
>>> have used adr instead of badr (but then how did this ever work ?).
>>>
>>> Seen with various toolchains based on gcc 7.x and binutils 2.30 when
>>> building and testing MPS2 images.
>>>
>>
>> Hello Guenter,
>>
>> This issue has been discussed before. It appears the binutils people
>> suddenly started caring about the thumb annotation of local function
>> symbols, resulting in behavior that is not backwards compatible.
>>
>> https://marc.info/?l=linux-arm-kernel&m=151143776213636&w=2
>> https://sourceware.org/bugzilla/show_bug.cgi?id=21458
>>
>> Can you try the fix below please?
>>
>>>   arch/arm/include/asm/assembler.h | 3 ++-
>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm/include/asm/assembler.h
>>> b/arch/arm/include/asm/assembler.h
>>> index 0cd4dccbae78..24c87ff2060f 100644
>>> --- a/arch/arm/include/asm/assembler.h
>>> +++ b/arch/arm/include/asm/assembler.h
>>> @@ -195,7 +195,8 @@
>>>          .irp    c,,eq,ne,cs,cc,mi,pl,vs,vc,hi,ls,ge,lt,gt,le,hs,lo
>>>          .macro  badr\c, rd, sym
>>>   #ifdef CONFIG_THUMB2_KERNEL
>>> -       adr\c   \rd, \sym + 1
>>> +       adr\c   \rd, \sym
>>> +       orr     \rd, #1
>>>   #else
>>>          adr\c   \rd, \sym
>>>   #endif
>>> --
>>> 2.7.4
>>
>>
>> ----------8<------------
>> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Date: Tue, 16 Jan 2018 12:12:45 +0000
>> Subject: [PATCH] ARM: assembler: prevent ADR from setting the Thumb bit
>> twice
>>
>> To work around recent issues where ADR references to Thumb function
>> symbols may or may not have the Thumb bit set already when they are
>> resolved by GAS, reference the symbol indirectly via a local symbol
>> typed as 'object', stripping the ARM/Thumb annotation.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>
>> diff --git a/arch/arm/include/asm/assembler.h
>> b/arch/arm/include/asm/assembler.h
>> index 6ae42ad29518..dd2ff94ad90b 100644
>> --- a/arch/arm/include/asm/assembler.h
>> +++ b/arch/arm/include/asm/assembler.h
>> @@ -195,13 +195,19 @@
>>          .irp    c,,eq,ne,cs,cc,mi,pl,vs,vc,hi,ls,ge,lt,gt,le,hs,lo
>>          .macro  badr\c, rd, sym
>>   #ifdef CONFIG_THUMB2_KERNEL
>> -       adr\c   \rd, \sym + 1
>> +       __badr  \c, \rd, \sym
>>   #else
>>          adr\c   \rd, \sym
>>   #endif
>>          .endm
>>          .endr
>>
>> +       /* this needs to be a separate macro or \@ does not work correctly
>> */
>> +       .macro  __badr, c, rd, sym
>> +       .eqv    .Lsym\@, \sym
>> +       adr\c   \rd, .Lsym\@ + 1
>
>
> Wild shot, but the following works for me.
>
>         .eqv    .Lsym\@, \sym + 1
>         adr\c   \rd, .Lsym\@
>
> Does it make sense ?
>

Interesting. Do you mean this works with your 2.30 binutils that
triggers the original issue?

If so, then yes, it makes sense, although it still seems fragile,
since we are relying on \sym being resolved without the Thumb bit in
the context of the .eqv pseudo op. But if this works across all
implementations we care about, I would be fine with it.

Russell?

^ permalink raw reply

* [PATCH v2 1/4] arm64: export memblock_reserve()d regions via /proc/iomem
From: Dave Kleikamp @ 2018-06-19 13:37 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180619064424.6642-2-takahiro.akashi@linaro.org>

On 06/19/2018 01:44 AM, AKASHI Takahiro wrote:

> +static int __init reserve_memblock_reserved_regions(void)
> +{
> +	phys_addr_t start, end, roundup_end = 0;
> +	struct resource *mem, *res;
> +	u64 i;
> +
> +	for_each_reserved_mem_region(i, &start, &end) {
> +		if (end <= roundup_end)
> +			continue; /* done already */
> +
> +		start = __pfn_to_phys(PFN_DOWN(start));
> +		end = __pfn_to_phys(PFN_UP(end)) - 1;
> +		roundup_end = end;
> +
> +		res = kzalloc(sizeof(*res), GFP_ATOMIC);
> +		if (WARN_ON(!res))
> +			return -ENOMEM;
> +		res->start = start;
> +		res->end = end;
> +		res->name  = "reserved";
> +		res->flags = IORESOURCE_MEM;
> +
> +		mem = request_resource_conflict(&iomem_resource, res);
> +		/*
> +		 * We expected memblock_reserve() regions to conflict with
> +		 * memory created by request_standard_resources().
> +		 */
> +		if (WARN_ON_ONCE(!mem))
> +			continue;
> +		kfree(res);

Why is kfree() after the conditional continue? This is a memory leak.

> +
> +		reserve_region_with_split(mem, start, end, "reserved");
> +	}
> +
> +	return 0;
> +}
> +arch_initcall(reserve_memblock_reserved_regions);
> +
>  u64 __cpu_logical_map[NR_CPUS] = { [0 ... NR_CPUS-1] = INVALID_HWID };
>  
>  void __init setup_arch(char **cmdline_p)
> 

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox