Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1659280 > unrolled thread

RISC-V Linux Port v2

Started byPalmer Dabbelt <palmer@dabbelt.com>
First post2017-06-07 01:10 +0200
Last post2017-06-09 10:20 +0200
Articles 6 on this page of 46 — 13 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  RISC-V Linux Port v2 Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-07 01:10 +0200
    [PATCH 06/17] pci: Add generic pcibios_{fixup_bus,align_resource} Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-07 01:10 +0200
      Re: [PATCH 06/17] pci: Add generic pcibios_{fixup_bus,align_resource} Geert Uytterhoeven <geert@linux-m68k.org> - 2017-06-07 09:30 +0200
        Re: [PATCH 06/17] pci: Add generic pcibios_{fixup_bus,align_resource} Arnd Bergmann <arnd@arndb.de> - 2017-06-07 10:10 +0200
        Re: [PATCH 06/17] pci: Add generic pcibios_{fixup_bus,align_resource} Christoph Hellwig <hch@infradead.org> - 2017-06-08 10:20 +0200
          Re: [PATCH 06/17] pci: Add generic pcibios_{fixup_bus,align_resource} Arnd Bergmann <arnd@arndb.de> - 2017-06-08 10:40 +0200
    [PATCH 08/17] dts: include documentation for the RISC-V interrupt controllers Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-07 01:10 +0200
      Re: [PATCH 08/17] dts: include documentation for the RISC-V interrupt controllers Geert Uytterhoeven <geert@linux-m68k.org> - 2017-06-07 09:20 +0200
        Re: [PATCH 08/17] dts: include documentation for the RISC-V  interrupt controllers Mark Rutland <mark.rutland@arm.com> - 2017-06-07 12:20 +0200
          Re: [PATCH 08/17] dts: include documentation for the RISC-V interrupt controllers Wesley Terpstra <wesley@sifive.com> - 2017-06-07 21:00 +0200
            Re: [PATCH 08/17] dts: include documentation for the RISC-V interrupt controllers Rob Herring <robh+dt@kernel.org> - 2017-06-07 22:00 +0200
              Re: [PATCH 08/17] dts: include documentation for the RISC-V interrupt controllers Wesley Terpstra <wesley@sifive.com> - 2017-06-07 22:40 +0200
            Re: [PATCH 08/17] dts: include documentation for the RISC-V  interrupt controllers Mark Rutland <mark.rutland@arm.com> - 2017-06-08 13:00 +0200
              Re: [PATCH 08/17] dts: include documentation for the RISC-V interrupt controllers Wesley Terpstra <wesley@sifive.com> - 2017-06-09 23:50 +0200
              Re: [PATCH 08/17] dts: include documentation for the RISC-V interrupt controllers Wesley Terpstra <wesley@sifive.com> - 2017-06-10 00:00 +0200
                Re: [PATCH 08/17] dts: include documentation for the RISC-V  interrupt controllers Mark Rutland <mark.rutland@arm.com> - 2017-06-19 16:40 +0200
      Re: [PATCH 08/17] dts: include documentation for the RISC-V interrupt controllers "Luis R. Rodriguez" <mcgrof@kernel.org> - 2017-06-08 00:30 +0200
    [PATCH 11/17] irqchip: RISC-V Local Interrupt Controller Driver Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-07 01:10 +0200
      Re: [PATCH 11/17] irqchip: RISC-V Local Interrupt Controller Driver Geert Uytterhoeven <geert@linux-m68k.org> - 2017-06-07 09:20 +0200
    [PATCH 01/17] drivers: support PCIe in RISCV Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-07 01:10 +0200
      Re: [PATCH 01/17] drivers: support PCIe in RISCV Geert Uytterhoeven <geert@linux-m68k.org> - 2017-06-07 09:20 +0200
      Re: [PATCH 01/17] drivers: support PCIe in RISCV Christoph Hellwig <hch@infradead.org> - 2017-06-07 16:30 +0200
        Re: [PATCH 01/17] drivers: support PCIe in RISCV Olof Johansson <olof@lixom.net> - 2017-06-07 19:50 +0200
    [PATCH 15/17] RISC-V: Add mm subdirectory Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-07 01:10 +0200
    [PATCH 09/17] clocksource/timer-riscv: New RISC-V Clocksource Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-07 01:10 +0200
      Re: [PATCH 09/17] clocksource/timer-riscv: New RISC-V Clocksource Geert Uytterhoeven <geert@linux-m68k.org> - 2017-06-07 09:20 +0200
        Re: [PATCH 09/17] clocksource/timer-riscv: New RISC-V Clocksource Arnd Bergmann <arnd@arndb.de> - 2017-06-07 09:30 +0200
      Re: [PATCH 09/17] clocksource/timer-riscv: New RISC-V Clocksource Marc Zyngier <marc.zyngier@arm.com> - 2017-06-07 11:50 +0200
    Re: RISC-V Linux Port v2 David Howells <dhowells@redhat.com> - 2017-06-07 09:30 +0200
      Re: RISC-V Linux Port v2 Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-08 00:00 +0200
    Re: [PATCH 13/17] RISC-V: Add include subdirectory Arnd Bergmann <arnd@arndb.de> - 2017-06-07 10:20 +0200
    Re: RISC-V Linux Port v2 Will Deacon <will.deacon@arm.com> - 2017-06-07 11:30 +0200
      Re: RISC-V Linux Port v2 Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-08 00:00 +0200
        Re: RISC-V Linux Port v2 Will Deacon <will.deacon@arm.com> - 2017-06-08 12:30 +0200
          Re: RISC-V Linux Port v2 Palmer Dabbelt <palmer@dabbelt.com> - 2017-06-08 20:20 +0200
    Re: [PATCH 13/17] RISC-V: Add include subdirectory Peter Zijlstra <peterz@infradead.org> - 2017-06-07 14:00 +0200
      Re: [PATCH 13/17] RISC-V: Add include subdirectory Peter Zijlstra <peterz@infradead.org> - 2017-06-07 14:30 +0200
    Re: [PATCH 13/17] RISC-V: Add include subdirectory Peter Zijlstra <peterz@infradead.org> - 2017-06-07 14:10 +0200
      Re: [PATCH 13/17] RISC-V: Add include subdirectory Peter Zijlstra <peterz@infradead.org> - 2017-06-07 14:30 +0200
      Re: [PATCH 13/17] RISC-V: Add include subdirectory Peter Zijlstra <peterz@infradead.org> - 2017-06-07 14:40 +0200
        Re: [PATCH 13/17] RISC-V: Add include subdirectory Peter Zijlstra <peterz@infradead.org> - 2017-06-07 15:00 +0200
          Re: [PATCH 13/17] RISC-V: Add include subdirectory Will Deacon <will.deacon@arm.com> - 2017-06-07 15:20 +0200
          Re: [PATCH 13/17] RISC-V: Add include subdirectory Peter Zijlstra <peterz@infradead.org> - 2017-06-07 18:40 +0200
    Re: [PATCH 13/17] RISC-V: Add include subdirectory Peter Zijlstra <peterz@infradead.org> - 2017-06-07 14:50 +0200
    Re: [PATCH 13/17] RISC-V: Add include subdirectory Peter Zijlstra <peterz@infradead.org> - 2017-06-07 15:20 +0200
      Re: [PATCH 13/17] RISC-V: Add include subdirectory Peter Zijlstra <peterz@infradead.org> - 2017-06-09 10:20 +0200

Page 3 of 3 — ← Prev page 1 2 [3]

#1659775 — Re: [PATCH 13/17] RISC-V: Add include subdirectory

FromPeter Zijlstra <peterz@infradead.org>
Date2017-06-07 15:00 +0200
SubjectRe: [PATCH 13/17] RISC-V: Add include subdirectory
Message-ID<tPIJ4-6JB-21@gated-at.bofh.it>
In reply to#1659754
On Wed, Jun 07, 2017 at 02:36:27PM +0200, Peter Zijlstra wrote:
> Which (pending the sub confusion) will generate the entire set of:
> 
>  atomic_add, atomic_add_return{_relaxed,_acquire,_release,} atomic_fetch_add{_relaxed,_acquire,_release,}
>  atomic_sub, atomic_sub_return{_relaxed,_acquire,_release,} atomic_fetch_sub{_relaxed,_acquire,_release,}
> 
>  atomic_and, atomic_fetch_and{_relaxed,_acquire,_release,}
>  atomic_or,  atomic_fetch_or{_relaxed,_acquire,_release,}
>  atomic_xor, atomic_fetch_xor{_relaxed,_acquire,_release,}
> 

Another approach would be to override __atomic_op_{acquire,release} and
use things like:

	"FENCE r,rw" -- (load) ACQUIRE
	"FENCE rw,w" -- (store) RELEASE

And then you only need to provide _relaxed atomics.

Also, and I didn't check for that, you need to provide:

smp_load_acquire(), smp_store_release(), atomic_read_acquire(),
atomic_store_release().

[toc] | [prev] | [next] | [standalone]

#1659783 — Re: [PATCH 13/17] RISC-V: Add include subdirectory

FromWill Deacon <will.deacon@arm.com>
Date2017-06-07 15:20 +0200
SubjectRe: [PATCH 13/17] RISC-V: Add include subdirectory
Message-ID<tPJ2q-77i-11@gated-at.bofh.it>
In reply to#1659775
[sorry, jumping in here because it's the only mail I have relating to
 patch 13]

On Wed, Jun 07, 2017 at 02:58:50PM +0200, Peter Zijlstra wrote:
> On Wed, Jun 07, 2017 at 02:36:27PM +0200, Peter Zijlstra wrote:
> > Which (pending the sub confusion) will generate the entire set of:
> > 
> >  atomic_add, atomic_add_return{_relaxed,_acquire,_release,} atomic_fetch_add{_relaxed,_acquire,_release,}
> >  atomic_sub, atomic_sub_return{_relaxed,_acquire,_release,} atomic_fetch_sub{_relaxed,_acquire,_release,}
> > 
> >  atomic_and, atomic_fetch_and{_relaxed,_acquire,_release,}
> >  atomic_or,  atomic_fetch_or{_relaxed,_acquire,_release,}
> >  atomic_xor, atomic_fetch_xor{_relaxed,_acquire,_release,}
> > 
> 
> Another approach would be to override __atomic_op_{acquire,release} and
> use things like:
> 
> 	"FENCE r,rw" -- (load) ACQUIRE
> 	"FENCE rw,w" -- (store) RELEASE
> 
> And then you only need to provide _relaxed atomics.
> 
> Also, and I didn't check for that, you need to provide:
> 
> smp_load_acquire(), smp_store_release(), atomic_read_acquire(),
> atomic_store_release().

Is there an up-to-date specification for the RISC-V memory model? I looked
at:

https://github.com/riscv/riscv-isa-manual/releases/download/riscv-user-2.2/riscv-spec-v2.2.pdf

but it says:

| 2.7 Memory Model
| This section is out of date as the RISC-V memory model is
| currently under revision to ensure it can efficiently support current
| programming language memory models. The revised base mem- ory model will
| contain further ordering constraints, including at least that loads to the
| same address from the same hart cannot be reordered, and that syntactic data
| dependencies between instructions are respected

which, on the one hand is reassuring (because ignoring dependency ordering is
plain broken), but on the other it doesn't go quite far enough in defining
exactly what constitutes a "syntactic data dependency". The cumulativity of
your fences also needs defining, because I think this was up in the air at some
point and the document above doesn't seem to tackle it (it doesn't seem to
describe what constitutes being a memory of the predecessor or successor sets)

Could you shed some light on this please? We've started relying on RW control
dependencies in semi-recent history, so it's important to get this nailed down.

Thanks,

Will

P.S. You should also totally get your architects to write a formal model ;)

[toc] | [prev] | [next] | [standalone]

#1660008 — Re: [PATCH 13/17] RISC-V: Add include subdirectory

FromPeter Zijlstra <peterz@infradead.org>
Date2017-06-07 18:40 +0200
SubjectRe: [PATCH 13/17] RISC-V: Add include subdirectory
Message-ID<tPM9X-CA-15@gated-at.bofh.it>
In reply to#1659775
On Wed, Jun 07, 2017 at 02:58:50PM +0200, Peter Zijlstra wrote:
> On Wed, Jun 07, 2017 at 02:36:27PM +0200, Peter Zijlstra wrote:
> > Which (pending the sub confusion) will generate the entire set of:
> > 
> >  atomic_add, atomic_add_return{_relaxed,_acquire,_release,} atomic_fetch_add{_relaxed,_acquire,_release,}
> >  atomic_sub, atomic_sub_return{_relaxed,_acquire,_release,} atomic_fetch_sub{_relaxed,_acquire,_release,}
> > 
> >  atomic_and, atomic_fetch_and{_relaxed,_acquire,_release,}
> >  atomic_or,  atomic_fetch_or{_relaxed,_acquire,_release,}
> >  atomic_xor, atomic_fetch_xor{_relaxed,_acquire,_release,}
> > 
> 
> Another approach would be to override __atomic_op_{acquire,release} and
> use things like:
> 
> 	"FENCE r,rw" -- (load) ACQUIRE
> 	"FENCE rw,w" -- (store) RELEASE
> 
> And then you only need to provide _relaxed atomics.
> 
> Also, and I didn't check for that, you need to provide:
> 
> smp_load_acquire(), smp_store_release(), atomic_read_acquire(),
> atomic_store_release().

Also, you probably need to provide smp_mb__before_spinlock(), but also
see:

  https://lkml.kernel.org/r/20170607161501.819948352@infradead.org

[toc] | [prev] | [next] | [standalone]

#1659769 — Re: [PATCH 13/17] RISC-V: Add include subdirectory

FromPeter Zijlstra <peterz@infradead.org>
Date2017-06-07 14:50 +0200
SubjectRe: [PATCH 13/17] RISC-V: Add include subdirectory
Message-ID<tPIzn-6Gk-3@gated-at.bofh.it>
In reply to#1659280
On Tue, Jun 06, 2017 at 04:00:03PM -0700, Palmer Dabbelt wrote:
> diff --git a/arch/riscv/include/asm/cmpxchg.h b/arch/riscv/include/asm/cmpxchg.h
> new file mode 100644
> index 000000000000..c7ee1321ac18
> --- /dev/null
> +++ b/arch/riscv/include/asm/cmpxchg.h
> @@ -0,0 +1,124 @@
> +/*
> + * Copyright (C) 2014 Regents of the University of California
> + *
> + *   This program is free software; you can redistribute it and/or
> + *   modify it under the terms of the GNU General Public License
> + *   as published by the Free Software Foundation, version 2.
> + *
> + *   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.
> + */
> +
> +#ifndef _ASM_RISCV_CMPXCHG_H
> +#define _ASM_RISCV_CMPXCHG_H
> +
> +#include <linux/bug.h>
> +
> +#ifdef CONFIG_ISA_A
> +
> +#include <asm/barrier.h>
> +
> +#define __xchg(new, ptr, size)					\
> +({								\
> +	__typeof__(ptr) __ptr = (ptr);				\
> +	__typeof__(new) __new = (new);				\
> +	__typeof__(*(ptr)) __ret;				\
> +	switch (size) {						\
> +	case 4:							\
> +		__asm__ __volatile__ (				\
> +			"amoswap.w %0, %2, %1"			\
> +			: "=r" (__ret), "+A" (*__ptr)		\
> +			: "r" (__new));				\
> +		break;						\
> +	case 8:							\
> +		__asm__ __volatile__ (				\
> +			"amoswap.d %0, %2, %1"			\
> +			: "=r" (__ret), "+A" (*__ptr)		\
> +			: "r" (__new));				\
> +		break;						\
> +	default:						\
> +		BUILD_BUG();					\
> +	}							\
> +	__ret;							\
> +})
> +
> +#define xchg(ptr, x)    (__xchg((x), (ptr), sizeof(*(ptr))))

our xchg() is fully ordered, and thus you need to use "amoswap.aq.rl"

> +
> +
> +/*
> + * Atomic compare and exchange.  Compare OLD with MEM, if identical,
> + * store NEW in MEM.  Return the initial value in MEM.  Success is
> + * indicated by comparing RETURN with OLD.
> + */
> +#define __cmpxchg(ptr, old, new, size)					\
> +({									\
> +	__typeof__(ptr) __ptr = (ptr);					\
> +	__typeof__(old) __old = (old);					\
> +	__typeof__(new) __new = (new);					\
> +	__typeof__(*(ptr)) __ret;					\
> +	register unsigned int __rc;					\
> +	switch (size) {							\
> +	case 4:								\
> +		__asm__ __volatile__ (					\
> +		"0:"							\
> +			"lr.w %0, %2\n"					\
> +			"bne  %0, %z3, 1f\n"				\
> +			"sc.w %1, %z4, %2\n"				\
> +			"bnez %1, 0b\n"					\
> +		"1:"							\
> +			: "=&r" (__ret), "=&r" (__rc), "+A" (*__ptr)	\
> +			: "rJ" (__old), "rJ" (__new));			\
> +		break;							\
> +	case 8:								\
> +		__asm__ __volatile__ (					\
> +		"0:"							\
> +			"lr.d %0, %2\n"					\
> +			"bne  %0, %z3, 1f\n"				\
> +			"sc.d %1, %z4, %2\n"				\
> +			"bnez %1, 0b\n"					\
> +		"1:"							\
> +			: "=&r" (__ret), "=&r" (__rc), "+A" (*__ptr)	\
> +			: "rJ" (__old), "rJ" (__new));			\
> +		break;							\
> +	default:							\
> +		BUILD_BUG();						\
> +	}								\
> +	__ret;								\
> +})
> +
> +#define __cmpxchg_mb(ptr, old, new, size)			\
> +({								\
> +	__typeof__(*(ptr)) __ret;				\
> +	smp_mb();						\
> +	__ret = __cmpxchg((ptr), (old), (new), (size));		\
> +	smp_mb();						\
> +	__ret;							\
> +})

Your ISA of course, but wouldn't setting the AQ and RL bits on LR/SC be
cheaper than doing two full barriers around the thing?

Note that cmpxchg() doesn't need to provide ordering on failure.

Further note that we have:

	{atomic_,}cmpxchg{_relaxed,_acquire,_release,}()

and recently:

	{atomic_,}try_cmpxchg{_relaxed,_acquire,_release,}()

[toc] | [prev] | [next] | [standalone]

#1659787 — Re: [PATCH 13/17] RISC-V: Add include subdirectory

FromPeter Zijlstra <peterz@infradead.org>
Date2017-06-07 15:20 +0200
SubjectRe: [PATCH 13/17] RISC-V: Add include subdirectory
Message-ID<tPJ2q-77i-23@gated-at.bofh.it>
In reply to#1659280
On Tue, Jun 06, 2017 at 04:00:03PM -0700, Palmer Dabbelt wrote:
> diff --git a/arch/riscv/include/asm/spinlock.h b/arch/riscv/include/asm/spinlock.h
> new file mode 100644
> index 000000000000..9736f5714e54
> --- /dev/null
> +++ b/arch/riscv/include/asm/spinlock.h
> @@ -0,0 +1,155 @@
> +/*
> + * Copyright (C) 2015 Regents of the University of California
> + *
> + *   This program is free software; you can redistribute it and/or
> + *   modify it under the terms of the GNU General Public License
> + *   as published by the Free Software Foundation, version 2.
> + *
> + *   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.
> + */
> +
> +#ifndef _ASM_RISCV_SPINLOCK_H
> +#define _ASM_RISCV_SPINLOCK_H
> +
> +#include <linux/kernel.h>
> +#include <asm/current.h>
> +
> +/*
> + * Simple spin lock operations.  These provide no fairness guarantees.
> + */

Any reason to use a test-and-set spinlock at all?

> +
> +#define arch_spin_lock_flags(lock, flags) arch_spin_lock(lock)
> +#define arch_spin_is_locked(x)	((x)->lock != 0)
> +#define arch_spin_unlock_wait(x) \
> +		do { cpu_relax(); } while ((x)->lock)

Hehe, yeah, no ;-) There are ordering constraints on that.

> +
> +static inline void arch_spin_unlock(arch_spinlock_t *lock)
> +{
> +	__asm__ __volatile__ (
> +		"amoswap.w.rl x0, x0, %0"
> +		: "=A" (lock->lock)
> +		:: "memory");
> +}
> +
> +static inline int arch_spin_trylock(arch_spinlock_t *lock)
> +{
> +	int tmp = 1, busy;
> +
> +	__asm__ __volatile__ (
> +		"amoswap.w.aq %0, %2, %1"
> +		: "=r" (busy), "+A" (lock->lock)
> +		: "r" (tmp)
> +		: "memory");
> +
> +	return !busy;
> +}
> +
> +static inline void arch_spin_lock(arch_spinlock_t *lock)
> +{
> +	while (1) {
> +		if (arch_spin_is_locked(lock))
> +			continue;
> +
> +		if (arch_spin_trylock(lock))
> +			break;
> +	}
> +}

[toc] | [prev] | [next] | [standalone]

#1662078 — Re: [PATCH 13/17] RISC-V: Add include subdirectory

FromPeter Zijlstra <peterz@infradead.org>
Date2017-06-09 10:20 +0200
SubjectRe: [PATCH 13/17] RISC-V: Add include subdirectory
Message-ID<tQnjc-7y0-5@gated-at.bofh.it>
In reply to#1659787
On Wed, Jun 07, 2017 at 03:17:27PM +0200, Peter Zijlstra wrote:

> > +static inline void arch_spin_unlock(arch_spinlock_t *lock)
> > +{
> > +	__asm__ __volatile__ (
> > +		"amoswap.w.rl x0, x0, %0"
> > +		: "=A" (lock->lock)
> > +		:: "memory");
> > +}
> > +
> > +static inline int arch_spin_trylock(arch_spinlock_t *lock)
> > +{
> > +	int tmp = 1, busy;
> > +
> > +	__asm__ __volatile__ (
> > +		"amoswap.w.aq %0, %2, %1"
> > +		: "=r" (busy), "+A" (lock->lock)
> > +		: "r" (tmp)
> > +		: "memory");
> > +
> > +	return !busy;
> > +}

One other thing, you need to describe the acquire/release semantics for
your platform. Is the above lock RCpc or RCsc ? If RCpc, you need to
look into adding smp_mb__after_unlock_lock().

[toc] | [prev] | [standalone]

Page 3 of 3 — ← Prev page 1 2 [3]

Back to top | Article view | linux.kernel

csiph-web