| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 2 Nov 2015 17:26:31 +0530
From: Vineet Gupta <Vineet.Gupta1@...opsys.com>
To: Noam Camus <noamc@...hip.com>, <linux-snps-arc@...ts.infradead.org>
CC: <linux-kernel@...r.kernel.org>, <talz@...hip.com>,
<gilf@...hip.com>, <cmetcalf@...hip.com>,
Peter Zijlstra <peterz@...radead.org>,
"Gilad Ben Yossef" <giladb@...hip.com>
Subject: Re: [PATCH v1 13/20] ARC: [plat-eznps] Use dedicated
bitops/atomic/cmpxchg
+CC Peter, Gilad
On Saturday 31 October 2015 06:45 PM, Noam Camus wrote:
> From: Noam Camus <noamc@...hip.com>
>
> We need our own implementaions since we lack of LLSC.
> Our extended ISA provided with optimized solution for all 32bit
> operations we see in those three headers.
> Signed-off-by: Noam Camus <noamc@...hip.com>
> ---
> arch/arc/include/asm/atomic.h | 69 +++++++++++++++++++++++++++++++
> arch/arc/include/asm/bitops.h | 49 ++++++++++++++++++++++
> arch/arc/include/asm/cmpxchg.h | 49 ++++++++++++++++++++++
> arch/arc/plat-eznps/include/plat/ctop.h | 1 +
> 4 files changed, 168 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arc/include/asm/atomic.h b/arch/arc/include/asm/atomic.h
> index c3ecda0..ca318c2 100644
> --- a/arch/arc/include/asm/atomic.h
> +++ b/arch/arc/include/asm/atomic.h
> @@ -17,6 +17,74 @@
> #include <asm/barrier.h>
> #include <asm/smp.h>
>
> +#ifdef CONFIG_ARC_PLAT_EZNPS
Could you please move nps specific code to end in all the 3 files - just because
it is least general use case.
#ifdef CONFIG_ARC_HAS_LLSC
..
-#else /* !CONFIG_ARC_HAS_LLSC */
+#elif !defined(CONFIG_ARC_PLAT_EZNPS)
+#else /* CONFIG_ARC_PLAT_EZNPS */
+
+ nps code here
+
+#endif
> +static inline int atomic_read(const atomic_t *v)
> +{
> + int temp;
> +
> + __asm__ __volatile__(
> + " ld.di %0, [%1]"
> + : "=r"(temp)
> + : "r"(&v->counter)
> + : "memory");
> + return temp;
> +}
> +
> +static inline void atomic_set(atomic_t *v, int i)
> +{
> + __asm__ __volatile__(
> + " st.di %0,[%1]"
> + :
> + : "r"(i), "r"(&v->counter)
> + : "memory");
> +}
> +
> +#define ATOMIC_OP(op, c_op, asm_op) \
> +static inline void atomic_##op(int i, atomic_t *v) \
> +{ \
> + __asm__ __volatile__( \
> + " mov r2, %0\n" \
> + " mov r3, %1\n" \
> + " .word %2\n" \
> + : \
> + : "r"(i), "r"(&v->counter), "i"(asm_op) \
> + : "r2", "r3", "memory"); \
> +} \
> +
> +#define ATOMIC_OP_RETURN(op, c_op, asm_op) \
> +static inline int atomic_##op##_return(int i, atomic_t *v) \
> +{ \
> + unsigned int temp = i; \
> + \
> + __asm__ __volatile__( \
> + " mov r2, %0\n" \
> + " mov r3, %1\n" \
> + " .word %2\n" \
> + " mov %0, r2" \
> + : "+r"(temp) \
> + : "r"(&v->counter), "i"(asm_op) \
> + : "r2", "r3", "memory"); \
> + \
> + return v->counter; \
> +}
atomic_op_return APIs require full barrier semantics (look at ARC normal
versions). So you need to add the smp_mb() calls both above and below the op
itself. This assumes that despite lack of enabling dcache for kernel, the current
hardware scheduling instructions (CTOP_INST_SCHD*) do imply some sort of barrier
semantics. If not then you don't need them as backend of smp_*() API.
> +
> +#define ATOMIC_OPS(op, c_op, asm_op) \
> + ATOMIC_OP(op, c_op, asm_op) \
> + ATOMIC_OP_RETURN(op, c_op, asm_op)
> +
+#ifndef CONFIG_ARC_PLAT_EZNPS
.... existing ATOMIC_*
+#else
> +ATOMIC_OPS(add, +=, CTOP_INST_AADD_DI_R2_R2_R3)
> +#define atomic_sub(i, v) atomic_add(-(i), (v))
> +#define atomic_sub_return(i, v) atomic_add_return(-(i), (v))
> +
> +ATOMIC_OP(and, &=, CTOP_INST_AAND_DI_R2_R2_R3)
> +#define atomic_andnot(mask, v) atomic_and(~(mask), (v))
> +ATOMIC_OP(or, |=, CTOP_INST_AOR_DI_R2_R2_R3)
> +ATOMIC_OP(xor, ^=, CTOP_INST_AXOR_DI_R2_R2_R3)
+#endif
> +
> +#undef ATOMIC_OPS
> +#undef ATOMIC_OP_RETURN
> +#undef ATOMIC_OP
> +#else /* CONFIG_ARC_PLAT_EZNPS */
This need not be copied.
> #define atomic_read(v) ((v)->counter)
>
> #ifdef CONFIG_ARC_HAS_LLSC
> @@ -186,6 +254,7 @@ ATOMIC_OP(xor, ^=, xor)
> #undef SCOND_FAIL_RETRY_VAR_DEF
> #undef SCOND_FAIL_RETRY_ASM
> #undef SCOND_FAIL_RETRY_VARS
> +#endif /* CONFIG_ARC_PLAT_EZNPS */
>
> /**
> * __atomic_add_unless - add unless the number is a given value
> diff --git a/arch/arc/include/asm/bitops.h b/arch/arc/include/asm/bitops.h
> index 57c1f33..54ecbe4 100644
> --- a/arch/arc/include/asm/bitops.h
> +++ b/arch/arc/include/asm/bitops.h
> @@ -22,6 +22,48 @@
> #include <asm/smp.h>
> #endif
>
> +#ifdef CONFIG_ARC_PLAT_EZNPS
Ditto - please move it to end !
> +#define BIT_OP(op, c_op, asm_op) \
> +static inline void op##_bit(unsigned long nr, volatile unsigned long *m)\
> +{ \
> + m += nr >> 5; \
> + \
> + nr = (1UL << (nr & 0x1f)); \
> + if (asm_op == CTOP_INST_AAND_DI_R2_R2_R3) \
> + nr = ~nr; \
> + \
> + __asm__ __volatile__( \
> + " mov r2, %0\n" \
> + " mov r3, %1\n" \
> + " .word %2\n" \
> + : \
> + : "r"(nr), "r"(m), "i"(asm_op) \
> + : "r2", "r3", "memory"); \
> +}
> +
> +#define TEST_N_BIT_OP(op, c_op, asm_op) \
> +static inline int test_and_##op##_bit(unsigned long nr, volatile unsigned long *m)\
> +{ \
> + unsigned long old; \
> + \
> + m += nr >> 5; \
smp_mb() needed here !
> + \
> + old = (1UL << (nr & 0x1f)); \
So you are reusing @old: for input bitmask as well as output value which is kind
of confusing but that's how the instructions works it seems.
> + if (asm_op == CTOP_INST_AAND_DI_R2_R2_R3) \
> + old = ~old; \
> + \
> + __asm__ __volatile__( \
> + " mov r2, %0\n" \
> + " mov r3, %1\n" \
> + " .word %2\n" \
> + " mov %0, r2" \
> + : "+r"(old) \
> + : "r"(m), "i"(asm_op) \
> + : "r2", "r3", "memory"); \
smp_mb() needed here !
> + \
> + return (old & (1 << nr)) != 0; \
> +}
> +#else /* CONFIG_ARC_PLAT_EZNPS */
> #if defined(CONFIG_ARC_HAS_LLSC)
>
> /*
> @@ -155,6 +197,7 @@ static inline int test_and_##op##_bit(unsigned long nr, volatile unsigned long *
> }
>
> #endif /* CONFIG_ARC_HAS_LLSC */
> +#endif /* CONFIG_ARC_PLAT_EZNPS */
>
> /***************************************
> * Non atomic variants
> @@ -196,9 +239,15 @@ static inline int __test_and_##op##_bit(unsigned long nr, volatile unsigned long
> /* __test_and_set_bit(), __test_and_clear_bit(), __test_and_change_bit() */\
> __TEST_N_BIT_OP(op, c_op, asm_op)
>
> +#ifdef CONFIG_ARC_PLAT_EZNPS
> +BIT_OPS(set, |, CTOP_INST_AOR_DI_R2_R2_R3)
> +BIT_OPS(clear, & ~, CTOP_INST_AAND_DI_R2_R2_R3)
> +BIT_OPS(change, ^, CTOP_INST_AXOR_DI_R2_R2_R3)
> +#else
> BIT_OPS(set, |, bset)
> BIT_OPS(clear, & ~, bclr)
> BIT_OPS(change, ^, bxor)
> +#endif
Here again, please swap them around !
>
> /*
> * This routine doesn't need to be atomic.
> diff --git a/arch/arc/include/asm/cmpxchg.h b/arch/arc/include/asm/cmpxchg.h
> index af7a2db..8578586 100644
> --- a/arch/arc/include/asm/cmpxchg.h
> +++ b/arch/arc/include/asm/cmpxchg.h
> @@ -14,6 +14,53 @@
> #include <asm/barrier.h>
> #include <asm/smp.h>
>
> +#ifdef CONFIG_ARC_PLAT_EZNPS
> +static inline unsigned long
> +__cmpxchg(volatile void *ptr, unsigned long expected, unsigned long new)
> +{
> + write_aux_reg(CTOP_AUX_GPA1, expected);
> +
smp_mb()
> + __asm__ __volatile__(
> + " mov r2, %0\n"
> + " mov r3, %1\n"
> + " .word %2\n"
> + " mov %0, r2"
> + : "+r"(new)
> + : "r"(ptr), "i"(CTOP_INST_EXC_DI_R2_R2_R3)
> + : "r2", "r3", "memory");
smp_mb()
> +
> + return new;
> +}
> +
> +#define cmpxchg(ptr, o, n) ((typeof(*(ptr)))__cmpxchg((ptr), \
> + (unsigned long)(o), (unsigned long)(n)))
> +#define atomic_cmpxchg(v, o, n) ((int)cmpxchg(&((v)->counter), (o), (n)))
Do these have to be duplicated !
> +
> +static inline unsigned long __xchg(unsigned long val, volatile void *ptr,
> + int size)
> +{
> + extern unsigned long __xchg_bad_pointer(void);
> +
> + switch (size) {
> + case 4:
smp_mb()
> + __asm__ __volatile__(
> + " mov r2, %0\n"
> + " mov r3, %1\n"
> + " .word %2\n"
> + " mov %0, r2\n"
> + : "+r"(val)
> + : "r"(ptr), "i"(CTOP_INST_XEX_DI_R2_R2_R3)
> + : "r2", "r3", "memory");
> +
smp_mb()
> + return val;
> + }
> + return __xchg_bad_pointer();
> +}
> +
> +#define xchg(ptr, with) ((typeof(*(ptr)))__xchg((unsigned long)(with), (ptr), \
> + sizeof(*(ptr))))
> +#define atomic_xchg(v, new) (xchg(&((v)->counter), new))
> +#else /* CONFIG_ARC_PLAT_EZNPS */
> #ifdef CONFIG_ARC_HAS_LLSC
>
> static inline unsigned long
> @@ -158,4 +205,6 @@ static inline unsigned long __xchg(unsigned long val, volatile void *ptr,
> */
> #define atomic_xchg(v, new) (xchg(&((v)->counter), new))
>
> +#endif /* CONFIG_ARC_PLAT_EZNPS */
> +
> #endif
> diff --git a/arch/arc/plat-eznps/include/plat/ctop.h b/arch/arc/plat-eznps/include/plat/ctop.h
> index b708f9f..655a860 100644
> --- a/arch/arc/plat-eznps/include/plat/ctop.h
> +++ b/arch/arc/plat-eznps/include/plat/ctop.h
> @@ -34,6 +34,7 @@
> #define AUX_REG_TSI1 (CTOP_AUX_BASE + 0x050)
> #define CTOP_AUX_EFLAGS (CTOP_AUX_BASE + 0x080)
> #define CTOP_AUX_IACK (CTOP_AUX_BASE + 0x088)
> +#define CTOP_AUX_GPA1 (CTOP_AUX_BASE + 0x08C)
This one is standing out - can u not squash this hunk with the patch which
introduces CTOP_INST_EXC* or one which adds various CTOP_AUX_*
> #define CTOP_AUX_UDMC (CTOP_AUX_BASE + 0x300)
>
> /* EZchip core instructions */
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists