Linux userland API discussions
 help / color / mirror / Atom feed
* [PATCH 2/7] aio: remove an outdated BUG_ON and comment in aio_complete
From: Christoph Hellwig @ 2018-05-02 21:14 UTC (permalink / raw)
  To: viro; +Cc: Avi Kivity, linux-aio, linux-fsdevel, linux-api, linux-kernel
In-Reply-To: <20180502211448.18276-1-hch@lst.de>

These days we don't treat sync iocbs special in the aio completion code as
they never use it.  Remove the old comment and BUG_ON given that the
current definition of is_sync_kiocb makes it impossible to hit.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/aio.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/fs/aio.c b/fs/aio.c
index add46b06be86..7c1855afd723 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -1107,15 +1107,6 @@ static void aio_complete(struct kiocb *kiocb, long res, long res2)
 		file_end_write(file);
 	}
 
-	/*
-	 * Special case handling for sync iocbs:
-	 *  - events go directly into the iocb for fast handling
-	 *  - the sync task with the iocb in its stack holds the single iocb
-	 *    ref, no other paths have a way to get another ref
-	 *  - the sync task helpfully left a reference to itself in the iocb
-	 */
-	BUG_ON(is_sync_kiocb(kiocb));
-
 	if (iocb->ki_list.next) {
 		unsigned long flags;
 
-- 
2.17.0

^ permalink raw reply related

* [PATCH 1/7] aio: don't print the page size at boot time
From: Christoph Hellwig @ 2018-05-02 21:14 UTC (permalink / raw)
  To: viro; +Cc: Avi Kivity, linux-aio, linux-fsdevel, linux-api, linux-kernel
In-Reply-To: <20180502211448.18276-1-hch@lst.de>

The page size is in no way related to the aio code, and printing it in
the (debug) dmesg at every boot serves no purpose.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Acked-by: Jeff Moyer <jmoyer@redhat.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/aio.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/fs/aio.c b/fs/aio.c
index 88d7927ffbc6..add46b06be86 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -264,9 +264,6 @@ static int __init aio_setup(void)
 
 	kiocb_cachep = KMEM_CACHE(aio_kiocb, SLAB_HWCACHE_ALIGN|SLAB_PANIC);
 	kioctx_cachep = KMEM_CACHE(kioctx,SLAB_HWCACHE_ALIGN|SLAB_PANIC);
-
-	pr_debug("sizeof(struct page) = %zu\n", sizeof(struct page));
-
 	return 0;
 }
 __initcall(aio_setup);
-- 
2.17.0

^ permalink raw reply related

* io_pgetevents & aio fsync V4
From: Christoph Hellwig @ 2018-05-02 21:14 UTC (permalink / raw)
  To: viro; +Cc: Avi Kivity, linux-aio, linux-fsdevel, linux-api, linux-kernel

Hi all,

this patch adds workqueue based fsync offload.  Version of this
patch have been floating around for a couple years, but we now
have a user with seastar used by ScyllaDB (who sponsored this
work) that really wants this in addition to the aio poll support.
More details are in the patch itself.

Because the iocb types have been defined sine day one (and probably
were supported by RHEL3) libaio already supports these calls as-is.

This also pulls in the aio cleanups and io_pgetevents support previously
submitted and review as part of the aio poll series.  The aio poll
series will be resubmitted on top of this series

A git tree is available here:

    git://git.infradead.org/users/hch/vfs.git aio-fsync.4

Gitweb:

    http://git.infradead.org/users/hch/vfs.git/shortlog/refs/heads/aio-fsync.4

Changes since V3:
 - rebased on top of 4.17-rc3
 - improved/added a few comments

Changes since V2:
 - don't introduce a use after free for lockdep sb release tracking
 - set up list for cancellation before I/O submission

Changes since V1:
 - remove a BUG_ON_ONE(is_sync_kiocb(kiocb));
 - moved cancellation patches to the poll series
 - improve a list_empty check

^ permalink raw reply

* Re: [PATCH 2/3] mm: add find_alloc_contig_pages() interface
From: Mike Kravetz @ 2018-05-02 21:13 UTC (permalink / raw)
  To: Vlastimil Babka, linux-mm, linux-kernel, linux-api
  Cc: Reinette Chatre, Michal Hocko, Christopher Lameter, Guy Shattah,
	Anshuman Khandual, Michal Nazarewicz, David Nellans, Laura Abbott,
	Pavel Machek, Dave Hansen, Andrew Morton
In-Reply-To: <deb9dd1d-84bf-75c7-2880-a7bcec880d47@suse.cz>

On 04/21/2018 09:16 AM, Vlastimil Babka wrote:
> On 04/17/2018 04:09 AM, Mike Kravetz wrote:
>> find_alloc_contig_pages() is a new interface that attempts to locate
>> and allocate a contiguous range of pages.  It is provided as a more
>> convenient interface than alloc_contig_range() which is currently
>> used by CMA and gigantic huge pages.
>>
>> When attempting to allocate a range of pages, migration is employed
>> if possible.  There is no guarantee that the routine will succeed.
>> So, the user must be prepared for failure and have a fall back plan.
>>
>> Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
> 
> Hi, just two quick observations, maybe discussion pointers for the
> LSF/MM session:
> - it's weird that find_alloc_contig_pages() takes an order, and
> free_contig_pages() takes a nr_pages. I suspect the interface would be
> more future-proof with both using nr_pages? Perhaps also minimum
> alignment for the allocation side? Order is fine for hugetlb, but what
> about other potential users?

Agreed, and I am changing this to nr_pages and adding alignment.

> - contig_alloc_migratetype_ok() says that MIGRATE_CMA blocks are OK to
> allocate from. This silently assumes that everything allocated by this
> will be migratable itself, or it might eat CMA reserves. Is it the case?
> Also you then call alloc_contig_range() with MIGRATE_MOVABLE, so it will
> skip/fail on MIGRATE_CMA anyway IIRC.

When looking closer at the code, alloc_contig_range currently has comments
saying migratetype must be MIGRATE_MOVABLE or MIGRATE_CMA.  However, this
is not checked/enforced anywhere in the code (that I can see).  The
migratetype passed to alloc_contig_range() will be used to set the migrate
type of all pageblocks in the range.  If there is an error, one side effect
is that some pageblocks may have their migrate type changed to migratetype.
Depending on how far we got before hitting the error, the number of pageblocks
changed is unknown.  This actually can happen at the lower level routine
start_isolate_page_range().

My first thought was to make start_isolate_page_range/set_migratetype_isolate
check that the migrate type of a pageblock was migratetype before isolating.
This would work for CMA, and I could make it work for the new allocator.
However, offline_pages also calls start_isolate_page_range and I believe we
do not want to enforce such a rule (all pageblocks must be of the same migrate
type) for memory hotplug/offline?

Should we be concerned at all about this potential changing of migrate type
on error?  The only way I can think to avoid this is to save the original
migrate type before isolation.

-- 
Mike Kravetz

^ permalink raw reply

* Re: [RFC PATCH for 4.18 00/14] Restartable Sequences
From: Daniel Colascione @ 2018-05-02 20:37 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Mathieu Desnoyers, Paul McKenney, boqun.feng, luto, davejwatson,
	linux-kernel, linux-api, Paul Turner, Andrew Morton, linux, tglx,
	mingo, hpa, Andrew Hunter, andi, cl, bmaurer, rostedt, josh,
	torvalds, catalin.marinas, will.deacon, Michael Kerrisk-manpages,
	Joel Fernandes
In-Reply-To: <20180502202233.GV12217@hirez.programming.kicks-ass.net>

On Wed, May 2, 2018 at 1:23 PM Peter Zijlstra <peterz@infradead.org> wrote:

> On Wed, May 02, 2018 at 06:27:22PM +0000, Daniel Colascione wrote:
> > On Wed, May 2, 2018 at 10:22 AM Peter Zijlstra <peterz@infradead.org>
wrote:
> > >> On Wed, May 02, 2018 at 03:53:47AM +0000, Daniel Colascione wrote:
> > > > Suppose we make a userspace mutex implemented with a lock word
having
> > three
> > > > bits: acquired, sleep_mode, and wait_pending, with the rest of the
word
> > not
> > > > being relevant at the moment.
> >
> > > So ideally we'd kill FUTEX_WAIT/FUTEX_WAKE for mutexes entirely, and
go
> > > with FUTEX_LOCK/FUTEX_UNLOCK that have the same semantics as the
> > > existing FUTEX_LOCK_PI/FUTEX_UNLOCK_PI, namely, the word contains the
> > > owner TID.
> >
> > That doesn't work if you want to use the rest of the word for something
> > else, like a recursion count. With FUTEX_WAIT and FUTEX_WAKE, you can
make
> > a lock with two bits.

> Recursive locks are teh most horrible crap ever. And having the tid in

What happened to providing mechanism, not policy?

You can't wish away recursive locking. It's baked into Java and the CLR,
and it's enshrined in POSIX. It's not going away, and there's no reason not
to support it efficiently.

> the word allows things like kernel based optimistic spins and possibly
> PI related things.

Sure. A lot of people don't want PI though, or at least they want to opt
into it. And we shouldn't require an entry into the kernel for what we can
in principle do efficiently in userspace.

> > > As brought up in the last time we talked about spin loops, why do we
> > > care if the spin loop is in userspace or not? Aside from the whole PTI
> > > thing, the syscall cost was around 150 cycle or so, while a LOCK
CMPXCHG
> > > is around 20 cycles. So ~7 spins gets you the cost of entry.
> >
> > That's pre-KPTI, isn't it?

> Yes, and once the hardware gets sorted, we'll be there again. I don't
> think we should design interfaces for 'broken' hardware.

It would be a mistake to design interfaces under the assumption that
everyone has fast permission level transitions.

^ permalink raw reply

* Re: [RFC PATCH for 4.18 00/14] Restartable Sequences
From: Peter Zijlstra @ 2018-05-02 20:22 UTC (permalink / raw)
  To: Daniel Colascione
  Cc: Mathieu Desnoyers, Paul McKenney, boqun.feng, luto, davejwatson,
	linux-kernel, linux-api, Paul Turner, Andrew Morton, linux, tglx,
	mingo, hpa, Andrew Hunter, andi, cl, bmaurer, rostedt, josh,
	torvalds, catalin.marinas, will.deacon, Michael Kerrisk-manpages,
	Joel Fernandes
In-Reply-To: <CAKOZuesjF8QxfphgsyRKctO89jR9VAM727dspTe7PX1z8KFpyw@mail.gmail.com>

On Wed, May 02, 2018 at 06:27:22PM +0000, Daniel Colascione wrote:
> On Wed, May 2, 2018 at 10:22 AM Peter Zijlstra <peterz@infradead.org> wrote:
> >> On Wed, May 02, 2018 at 03:53:47AM +0000, Daniel Colascione wrote:
> > > Suppose we make a userspace mutex implemented with a lock word having
> three
> > > bits: acquired, sleep_mode, and wait_pending, with the rest of the word
> not
> > > being relevant at the moment.
> 
> > So ideally we'd kill FUTEX_WAIT/FUTEX_WAKE for mutexes entirely, and go
> > with FUTEX_LOCK/FUTEX_UNLOCK that have the same semantics as the
> > existing FUTEX_LOCK_PI/FUTEX_UNLOCK_PI, namely, the word contains the
> > owner TID.
> 
> That doesn't work if you want to use the rest of the word for something
> else, like a recursion count. With FUTEX_WAIT and FUTEX_WAKE, you can make
> a lock with two bits.

Recursive locks are teh most horrible crap ever. And having the tid in
the word allows things like kernel based optimistic spins and possibly
PI related things.

> > As brought up in the last time we talked about spin loops, why do we
> > care if the spin loop is in userspace or not? Aside from the whole PTI
> > thing, the syscall cost was around 150 cycle or so, while a LOCK CMPXCHG
> > is around 20 cycles. So ~7 spins gets you the cost of entry.
> 
> That's pre-KPTI, isn't it?

Yes, and once the hardware gets sorted, we'll be there again. I don't
think we should design interfaces for 'broken' hardware.

^ permalink raw reply

* Re: [RFC PATCH for 4.18 00/14] Restartable Sequences
From: Daniel Colascione @ 2018-05-02 18:27 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Mathieu Desnoyers, Paul McKenney, boqun.feng, luto, davejwatson,
	linux-kernel, linux-api, Paul Turner, Andrew Morton, linux, tglx,
	mingo, hpa, Andrew Hunter, andi, cl, bmaurer, rostedt, josh,
	torvalds, catalin.marinas, will.deacon, Michael Kerrisk-manpages,
	Joel Fernandes
In-Reply-To: <20180502172218.GL12180@hirez.programming.kicks-ass.net>

On Wed, May 2, 2018 at 10:22 AM Peter Zijlstra <peterz@infradead.org> wrote:
>> On Wed, May 02, 2018 at 03:53:47AM +0000, Daniel Colascione wrote:
> > Suppose we make a userspace mutex implemented with a lock word having
three
> > bits: acquired, sleep_mode, and wait_pending, with the rest of the word
not
> > being relevant at the moment.

> So ideally we'd kill FUTEX_WAIT/FUTEX_WAKE for mutexes entirely, and go
> with FUTEX_LOCK/FUTEX_UNLOCK that have the same semantics as the
> existing FUTEX_LOCK_PI/FUTEX_UNLOCK_PI, namely, the word contains the
> owner TID.

That doesn't work if you want to use the rest of the word for something
else, like a recursion count. With FUTEX_WAIT and FUTEX_WAKE, you can make
a lock with two bits.

> As brought up in the last time we talked about spin loops, why do we
> care if the spin loop is in userspace or not? Aside from the whole PTI
> thing, the syscall cost was around 150 cycle or so, while a LOCK CMPXCHG
> is around 20 cycles. So ~7 spins gets you the cost of entry.

That's pre-KPTI, isn't it?

^ permalink raw reply

* Re: [RFC PATCH for 4.18 00/14] Restartable Sequences
From: Peter Zijlstra @ 2018-05-02 17:22 UTC (permalink / raw)
  To: Daniel Colascione
  Cc: mathieu.desnoyers, paulmck, boqun.feng, luto, davejwatson,
	linux-kernel, linux-api, pjt, Andrew Morton, linux, tglx, mingo,
	hpa, Andrew Hunter, andi, cl, bmaurer, rostedt, josh, torvalds,
	catalin.marinas, will.deacon, mtk.manpages, Joel Fernandes
In-Reply-To: <CAKOZuesa338sc_=w6-wvro25idrSN_zx_zrHFyo-bDiORs7FZw@mail.gmail.com>

On Wed, May 02, 2018 at 03:53:47AM +0000, Daniel Colascione wrote:
> Suppose we make a userspace mutex implemented with a lock word having three
> bits: acquired, sleep_mode, and wait_pending, with the rest of the word not
> being relevant at the moment.

So ideally we'd kill FUTEX_WAIT/FUTEX_WAKE for mutexes entirely, and go
with FUTEX_LOCK/FUTEX_UNLOCK that have the same semantics as the
existing FUTEX_LOCK_PI/FUTEX_UNLOCK_PI, namely, the word contains the
owner TID.

As brought up in the last time we talked about spin loops, why do we
care if the spin loop is in userspace or not? Aside from the whole PTI
thing, the syscall cost was around 150 cycle or so, while a LOCK CMPXCHG
is around 20 cycles. So ~7 spins gets you the cost of entry.

^ permalink raw reply

* Re: [RFC PATCH for 4.18 00/14] Restartable Sequences
From: Daniel Colascione @ 2018-05-02 16:55 UTC (permalink / raw)
  To: rostedt
  Cc: Mathieu Desnoyers, Peter Zijlstra, Paul McKenney, boqun.feng,
	luto, davejwatson, linux-kernel, linux-api, Paul Turner,
	Andrew Morton, linux, tglx, mingo, hpa, Andrew Hunter, andi, cl,
	bmaurer, josh, torvalds, catalin.marinas, will.deacon,
	Michael Kerrisk-manpages, Joel Fernandes
In-Reply-To: <20180502124253.145253cb@gandalf.local.home>

On Wed, May 2, 2018 at 9:42 AM Steven Rostedt <rostedt@goodmis.org> wrote:

> On Wed, 02 May 2018 16:07:48 +0000
> Daniel Colascione <dancol@google.com> wrote:

> > Why couldn't we take a page fault just before schedule? The reason we
can't
> > take a page fault in atomic context is that doing so might call
schedule.
> > Here, we're about to call schedule _anyway_, so what harm does it do to
> > call something that might call schedule? If we schedule via that call,
we
> > can skip the manual schedule we were going to perform.

> Another issue is slowing down something that is considered a fast path.

There are two questions: 1) does this feature slow down schedule when
you're not using it? and 2) is schedule unacceptably slow when you are
using this feature?

The answer to #1 is no; rseq already tests current->rseq during task switch
(via rseq_set_notify_resume), so adding a single further branch (which we'd
only test when we follow the current->rseq path anyway) isn't a problem.

Regarding #2: yes, a futex operation will increase path length for that one
task switch, but in the no-page-fault case, only by a tiny amount. We can
run benchmarks of course, but I don't see any reason to suspect that the
proposal would make task switching unacceptably slow. If we *do* take a
page fault, we won't have done much additional work overall, since
*somebody* is going to take that page fault anyway when the lock is
released, and the latency of the task switch shouldn't increase, since the
futex code will very quickly realize that it needs to sleep and call
schedule anyway.

^ permalink raw reply

* Re: [RFC PATCH for 4.18 00/14] Restartable Sequences
From: Steven Rostedt @ 2018-05-02 16:42 UTC (permalink / raw)
  To: Daniel Colascione
  Cc: Mathieu Desnoyers, Peter Zijlstra, Paul McKenney, boqun.feng,
	luto, davejwatson, linux-kernel, linux-api, Paul Turner,
	Andrew Morton, linux, tglx, mingo, hpa, Andrew Hunter, andi, cl,
	bmaurer, josh, torvalds, catalin.marinas, will.deacon,
	Michael Kerrisk-manpages, Joel Fernandes
In-Reply-To: <CAKOZueu_7Ha_WXMyxqMEScXo1aHWr9qYRxqyb-Rpd4k1JP3xHA@mail.gmail.com>

On Wed, 02 May 2018 16:07:48 +0000
Daniel Colascione <dancol@google.com> wrote:

> Why couldn't we take a page fault just before schedule? The reason we can't
> take a page fault in atomic context is that doing so might call schedule.
> Here, we're about to call schedule _anyway_, so what harm does it do to
> call something that might call schedule? If we schedule via that call, we
> can skip the manual schedule we were going to perform.

Another issue is slowing down something that is considered a fast path.

-- Steve

^ permalink raw reply

* Re: [PATCH 0/6 resend] statfs: handle mount propagation
From: Al Viro @ 2018-05-02 16:09 UTC (permalink / raw)
  To: Christian Brauner
  Cc: torvalds, linux-fsdevel, linux-kernel, hch, tglx, kstewart,
	gregkh, pombredanne, linux-api
In-Reply-To: <20180502154239.14013-1-christian.brauner@ubuntu.com>

On Wed, May 02, 2018 at 05:42:33PM +0200, Christian Brauner wrote:
> Hey,
> 
> This is the second resend of this patchset. I'm not sure whether it has
> simply been overlooked but the number of people get_maintainer.pl was
> rather small and seemed a little random so I added Linus and Christoph,
> two people I know that do look at VFS stuff at least from time to time,
> although they weren't listed by get_maintainer.pl. I hope that's ok.
> 
> This little series
> - unifies the definition of constants in statfs.h and fs.h
>   *Note, that Andreas has expressed doubts whether this unification is
>   useful. Please see https://lkml.org/lkml/2018/4/13/571 . I still think
>   it is but I'm happy to drop these two patches if others agree.*
> - extends statfs to handle mount propagation. This will let userspace
>   easily query a given mountpoint for MS_UNBINDABLE, MS_SHARED,
>   MS_PRIVATE and MS_SLAVE without always having to do costly parsing of
>   /proc/<pid>/mountinfo.
>   To this end the flags:
>   - ST_UNBINDABLE
>   - ST_SHARED
>   - ST_PRIVATE
>   - ST_SLAVE
>   are added. They have the same value as their MS_* counterparts.

How about some rationale for that in the first place?  statfs() looks like
a bad match for that - not to mention anything else, there's no way to
get anything beyond "it is a peer of something", not even "do these two
get propagation between them".  What would be using that, what would the
userland side of users look like, etc...

And in any case linux-api should've been Cc'd.  I'm not saying that this
(or something similar) would be an inherently bad idea, but the question
"why this way?" deserves a bit more than "parsing is costly"...

^ permalink raw reply

* Re: [RFC PATCH for 4.18 00/14] Restartable Sequences
From: Daniel Colascione @ 2018-05-02 16:07 UTC (permalink / raw)
  To: Mathieu Desnoyers
  Cc: Peter Zijlstra, Paul McKenney, boqun.feng, luto, davejwatson,
	linux-kernel, linux-api, Paul Turner, Andrew Morton, linux, tglx,
	mingo, hpa, Andrew Hunter, andi, cl, bmaurer, rostedt, josh,
	torvalds, catalin.marinas, will.deacon, Michael Kerrisk-manpages,
	Joel Fernandes
In-Reply-To: <660904075.9201.1525276988842.JavaMail.zimbra@efficios.com>

On Wed, May 2, 2018 at 9:03 AM Mathieu Desnoyers <
mathieu.desnoyers@efficios.com> wrote:

> ----- On May 1, 2018, at 11:53 PM, Daniel Colascione dancol@google.com
wrote:
> [...]
> >
> > I think a small enhancement to rseq would let us build a perfect
userspace
> > mutex, one that spins on lock-acquire only when the lock owner is
running
> > and that sleeps otherwise, freeing userspace from both specifying ad-hoc
> > spin counts and from trying to detect situations in which spinning is
> > generally pointless.
> >
> > It'd work like this: in the per-thread rseq data structure, we'd
include a
> > description of a futex operation for the kernel would perform (in the
> > context of the preempted thread) upon preemption, immediately before
> > schedule(). If the futex operation itself sleeps, that's no problem: we
> > will have still accomplished our goal of running some other thread
instead
> > of the preempted thread.

> Hi Daniel,

> I agree that the problem you are aiming to solve is important. Let's see
> what prevents the proposed rseq implementation from doing what you
envision.

> The main issue here is touching userspace immediately before schedule().
> At that specific point, it's not possible to take a page fault. In the
proposed
> rseq implementation, we get away with it by raising a task struct flag,
and using
> it in a return to userspace notifier (where we can actually take a
fault), where
> we touch the userspace TLS area.

> If we can find a way to solve this limitation, then the rest of your
design
> makes sense to me.

Thanks for taking a look!

Why couldn't we take a page fault just before schedule? The reason we can't
take a page fault in atomic context is that doing so might call schedule.
Here, we're about to call schedule _anyway_, so what harm does it do to
call something that might call schedule? If we schedule via that call, we
can skip the manual schedule we were going to perform.

^ permalink raw reply

* Re: [RFC PATCH for 4.18 00/14] Restartable Sequences
From: Mathieu Desnoyers @ 2018-05-02 16:03 UTC (permalink / raw)
  To: Daniel Colascione
  Cc: Peter Zijlstra, Paul E. McKenney, Boqun Feng, Andy Lutomirski,
	Dave Watson, linux-kernel, linux-api, Paul Turner, Andrew Morton,
	Russell King, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Andrew Hunter, Andi Kleen, Chris Lameter, Ben Maurer, rostedt,
	Josh Triplett, Linus Torvalds, Catalin Marinas <cata>
In-Reply-To: <CAKOZuesa338sc_=w6-wvro25idrSN_zx_zrHFyo-bDiORs7FZw@mail.gmail.com>

----- On May 1, 2018, at 11:53 PM, Daniel Colascione dancol@google.com wrote:
[...]
> 
> I think a small enhancement to rseq would let us build a perfect userspace
> mutex, one that spins on lock-acquire only when the lock owner is running
> and that sleeps otherwise, freeing userspace from both specifying ad-hoc
> spin counts and from trying to detect situations in which spinning is
> generally pointless.
> 
> It'd work like this: in the per-thread rseq data structure, we'd include a
> description of a futex operation for the kernel would perform (in the
> context of the preempted thread) upon preemption, immediately before
> schedule(). If the futex operation itself sleeps, that's no problem: we
> will have still accomplished our goal of running some other thread instead
> of the preempted thread.

Hi Daniel,

I agree that the problem you are aiming to solve is important. Let's see
what prevents the proposed rseq implementation from doing what you envision.

The main issue here is touching userspace immediately before schedule().
At that specific point, it's not possible to take a page fault. In the proposed
rseq implementation, we get away with it by raising a task struct flag, and using
it in a return to userspace notifier (where we can actually take a fault), where
we touch the userspace TLS area.

If we can find a way to solve this limitation, then the rest of your design
makes sense to me.

Thanks!

Mathieu

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

^ permalink raw reply

* Re: [PATCH] mmap.2: MAP_FIXED is okay if the address range has been reserved
From: Michael Kerrisk (man-pages) @ 2018-05-02 13:06 UTC (permalink / raw)
  To: Michal Hocko, Jann Horn
  Cc: mtk.manpages, John Hubbard, linux-man, Andrew Morton, Linux-MM,
	lkml, Linux API
In-Reply-To: <20180416211115.GU17484@dhcp22.suse.cz>

Jann, Micha,

On 04/16/2018 11:11 PM, Michal Hocko wrote:
> On Mon 16-04-18 22:17:40, Jann Horn wrote:
>> On Mon, Apr 16, 2018 at 9:57 PM, Michal Hocko <mhocko@kernel.org> wrote:
>>> On Mon 16-04-18 21:30:09, Jann Horn wrote:
>>>> On Mon, Apr 16, 2018 at 9:18 PM, Michal Hocko <mhocko@kernel.org> wrote:
>>> [...]
>>>>> Yes, reasonably well written application will not have this problem.
>>>>> That, however, requires an external synchronization and that's why
>>>>> called it error prone and racy. I guess that was the main motivation for
>>>>> that part of the man page.
>>>>
>>>> What requires external synchronization? I still don't understand at
>>>> all what you're talking about.
>>>>
>>>> The following code:
>>>>
>>>> void *try_to_alloc_addr(void *hint, size_t len) {
>>>>   char *x = mmap(hint, len, ...);
>>>>   if (x == MAP_FAILED) return NULL;
>>>>   if (x == hint) return x;
>>>
>>> Any other thread can modify the address space at this moment.
>>
>> But not parts of the address space that were returned by this mmap() call.
> ?
>>> Just
>>> consider that another thread would does mmap(x, MAP_FIXED) (or any other
>>> address overlapping [x, x+len] range)
>>
>> If the other thread does that without previously having created a
>> mapping covering the area in question, that would be a bug in the
>> other thread.
> 
> MAP_FIXED is sometimes used without preallocated address ranges.
> 
>> MAP_FIXED on an unmapped address is almost always a bug
>> (excluding single-threaded cases with no library code, and even then
>> it's quite weird) - for example, any malloc() call could also cause
>> libc to start using the memory range you're trying to map with
>> MAP_FIXED.
> 
> Yeah and that's why we there is such a large paragraph in the man page
> ;)
> 
>>> becaus it is seemingly safe as x
>>> != hint.
>>
>> I don't understand this part. Are you talking about a hypothetical
>> scenario in which a programmer attempts to segment the virtual memory
>> space into areas that are exclusively used by threads without creating
>> memory mappings for those areas?
> 
> Yeah, that doesn't sound all that over-exaggerated, right? And yes,
> such a code would be subtle and most probably buggy. I am not trying to
> argue for those hypothetical cases. All I am saying is that MAP_FIXED is
> subtle.
> 
> I _do_ agree that using it solely on the preallocated and _properly_
> managed address ranges is safe. I still maintain my position on error
> prone though. And besides that there are usecases which do not operate
> on preallocated address ranges so people really have to be careful.
> 
> I do not really care what is the form. I find the current wording quite
> informative and showing examples of how things might be broken. I do
> agree with your remark that "MAP_FIXED on preallocated ranges is safe"
> should be added. But MAP_FIXED is dangerous API and should have few big
> fat warnings.

So, at this stage, is any further patch needed to the text describing
MAP_FIXED/MAP_FIXED_REPLACE?

Cheers,

Michael


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

^ permalink raw reply

* Re: [PATCH net-next] modules: allow modprobe load regular elf binaries
From: Jesper Dangaard Brouer @ 2018-05-02  9:12 UTC (permalink / raw)
  To: Chris Mason
  Cc: brouer, Linus Torvalds, Alexei Starovoitov, David Miller,
	Daniel Borkmann, Greg Kroah-Hartman, Luis R. Rodriguez,
	Network Development, Linux Kernel Mailing List, kernel-team,
	Linux API
In-Reply-To: <EED9C7CB-BC5D-4E2D-B0CC-0003F682C73B@fb.com>


On Tue, 6 Mar 2018 15:42:41 -0800 Chris Mason <clm@fb.com> wrote:
> On 6 Mar 2018, at 11:12, Linus Torvalds wrote:
> 
[...]
> >
> > I do *not* want this to be a magical way to hide things.  
> 
> Especially early on, this makes a lot of sense.  But I wanted to plug 
> bps and the hopefully growing set of bpf introspection tools:
> 
> https://github.com/iovisor/bcc/blob/master/introspection/bps_example.txt
> 
> Long term these are probably a good place to tell the admin what's going 
> on.
(related to bpf itself not modprobe subject)

Hi Chris,

I just want to point out that the tool 'bpftool', is currently the
dominating tool for eBPF introspection.  And the 'bps' tool you mention
seems to have gained less (open source) traction.

The bpftool is part of the kernel git-tree: tools/bpf/bpftool
 https://github.com/torvalds/linux/blob/master/tools/bpf/bpftool/

And it even have bash-completion and man-pages in RST format so they
even render nicely when viewed via github:

 https://github.com/torvalds/linux/blob/master/tools/bpf/bpftool/Documentation/bpftool.rst
 https://github.com/torvalds/linux/blob/master/tools/bpf/bpftool/Documentation/bpftool-prog.rst
 https://github.com/torvalds/linux/blob/master/tools/bpf/bpftool/Documentation/bpftool-map.rst

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply

* Re: [RFC PATCH for 4.18 00/14] Restartable Sequences
From: Peter Zijlstra @ 2018-05-02  8:43 UTC (permalink / raw)
  To: Daniel Colascione
  Cc: mathieu.desnoyers, paulmck, boqun.feng, luto, davejwatson,
	linux-kernel, linux-api, pjt, Andrew Morton, linux, tglx, mingo,
	hpa, Andrew Hunter, andi, cl, bmaurer, rostedt, josh, torvalds,
	catalin.marinas, will.deacon, mtk.manpages, Joel Fernandes
In-Reply-To: <CAKOZuesa338sc_=w6-wvro25idrSN_zx_zrHFyo-bDiORs7FZw@mail.gmail.com>

On Wed, May 02, 2018 at 03:53:47AM +0000, Daniel Colascione wrote:
> The usual approach to "better" is an "adaptive mutex". Such a thing, when
> it attempts to acquire a lock another thread owns, spins for some number of
> iterations, then falls back to futex. I guess that's a little better than
> immediately jumping to futex, but it's not optimal. We can still spin when
> the lock owner isn't scheduled, and the spin count is usually some guess
> (either specified manually or estimated statistically) that's not
> guaranteed to produce decent results. Even if we do pick a good spin count,
> we run a very good chance of under- or over-spinning on any given
> lock-acquire. We always want to sleep when spinning would be pointless.

Look for the FUTEX_LOCK patches from Waiman.

^ permalink raw reply

* [RFC PATCH] Add /proc/<pid>/numa_vamaps for numa node information
From: Prakash Sangappa @ 2018-05-02  5:58 UTC (permalink / raw)
  To: linux-kernel, linux-mm, linux-api
  Cc: akpm, mhocko, kirill.shutemov, n-horiguchi, drepper, rientjes,
	prakash.sangappa

For analysis purpose it is useful to have numa node information
corresponding mapped address ranges of the process. Currently
/proc/<pid>/numa_maps provides list of numa nodes from where pages are
allocated per VMA of the process. This is not useful if an user needs to
determine which numa node the mapped pages are allocated from for a
particular address range. It would have helped if the numa node information
presented in /proc/<pid>/numa_maps was broken down by VA ranges showing the
exact numa node from where the pages have been allocated.

The format of /proc/<pid>/numa_maps file content is dependent on
/proc/<pid>/maps file content as mentioned in the manpage. i.e one line
entry for every VMA corresponding to entries in /proc/<pids>/maps file.
Therefore changing the output of /proc/<pid>/numa_maps may not be possible.

Hence, this patch proposes adding file /proc/<pid>/numa_vamaps which will
provide proper break down of VA ranges by numa node id from where the mapped
pages are allocated. For Address ranges not having any pages mapped, a '-'
is printed instead of numa node id. In addition, this file will include most
of the other information currently presented in /proc/<pid>/numa_maps. The
additional information included is for convenience. If this is not
preferred, the patch could be modified to just provide VA range to numa node
information as the rest of the information is already available thru
/proc/<pid>/numa_maps file.

Since the VA range to numa node information does not include page's PFN,
reading this file will not be restricted(i.e requiring CAP_SYS_ADMIN).

Here is the snippet from the new file content showing the format.

00400000-00401000 N0=1 kernelpagesize_kB=4 mapped=1 file=/tmp/hmap2
00600000-00601000 N0=1 kernelpagesize_kB=4 anon=1 dirty=1 file=/tmp/hmap2
00601000-00602000 N0=1 kernelpagesize_kB=4 anon=1 dirty=1 file=/tmp/hmap2
7f0215600000-7f0215800000 N0=1 kernelpagesize_kB=2048 dirty=1 file=/mnt/f1
7f0215800000-7f0215c00000 -  file=/mnt/f1
7f0215c00000-7f0215e00000 N0=1 kernelpagesize_kB=2048 dirty=1 file=/mnt/f1
7f0215e00000-7f0216200000 -  file=/mnt/f1
..
7f0217ecb000-7f0217f20000 N0=85 kernelpagesize_kB=4 mapped=85 mapmax=51
   file=/usr/lib64/libc-2.17.so
7f0217f20000-7f0217f30000 -  file=/usr/lib64/libc-2.17.so
7f0217f30000-7f0217f90000 N0=96 kernelpagesize_kB=4 mapped=96 mapmax=51
   file=/usr/lib64/libc-2.17.so
7f0217f90000-7f0217fb0000 -  file=/usr/lib64/libc-2.17.so
..

The 'pmap' command can be enhanced to include an option to show numa node
information which it can read from this new proc file. This will be a
follow on proposal.

There have been couple of previous patch proposals to provide numa node
information based on pfn or physical address. They seem to have not made
progress. Also it would appear reading numa node information based on PFN
or physical address will require privileges(CAP_SYS_ADMIN) similar to
reading PFN info from /proc/<pid>/pagemap.

See
https://marc.info/?t=139630938200001&r=1&w=2

https://marc.info/?t=139718724400001&r=1&w=2

Signed-off-by: Prakash Sangappa <prakash.sangappa@oracle.com>
---
 fs/proc/base.c     |   2 +
 fs/proc/internal.h |   3 +
 fs/proc/task_mmu.c | 299 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 303 insertions(+), 1 deletion(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index 1b2ede6..8fd7cc5 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -2960,6 +2960,7 @@ static const struct pid_entry tgid_base_stuff[] = {
 	REG("maps",       S_IRUGO, proc_pid_maps_operations),
 #ifdef CONFIG_NUMA
 	REG("numa_maps",  S_IRUGO, proc_pid_numa_maps_operations),
+	REG("numa_vamaps",  S_IRUGO, proc_pid_numa_vamaps_operations),
 #endif
 	REG("mem",        S_IRUSR|S_IWUSR, proc_mem_operations),
 	LNK("cwd",        proc_cwd_link),
@@ -3352,6 +3353,7 @@ static const struct pid_entry tid_base_stuff[] = {
 #endif
 #ifdef CONFIG_NUMA
 	REG("numa_maps", S_IRUGO, proc_tid_numa_maps_operations),
+	REG("numa_vamaps",  S_IRUGO, proc_tid_numa_vamaps_operations),
 #endif
 	REG("mem",       S_IRUSR|S_IWUSR, proc_mem_operations),
 	LNK("cwd",       proc_cwd_link),
diff --git a/fs/proc/internal.h b/fs/proc/internal.h
index 0f1692e..9a3ff80 100644
--- a/fs/proc/internal.h
+++ b/fs/proc/internal.h
@@ -273,6 +273,7 @@ struct proc_maps_private {
 #ifdef CONFIG_NUMA
 	struct mempolicy *task_mempolicy;
 #endif
+	u64 vma_off;
 } __randomize_layout;
 
 struct mm_struct *proc_mem_open(struct inode *inode, unsigned int mode);
@@ -280,7 +281,9 @@ struct mm_struct *proc_mem_open(struct inode *inode, unsigned int mode);
 extern const struct file_operations proc_pid_maps_operations;
 extern const struct file_operations proc_tid_maps_operations;
 extern const struct file_operations proc_pid_numa_maps_operations;
+extern const struct file_operations proc_pid_numa_vamaps_operations;
 extern const struct file_operations proc_tid_numa_maps_operations;
+extern const struct file_operations proc_tid_numa_vamaps_operations;
 extern const struct file_operations proc_pid_smaps_operations;
 extern const struct file_operations proc_pid_smaps_rollup_operations;
 extern const struct file_operations proc_tid_smaps_operations;
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index c486ad4..e3c7d65 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -169,6 +169,13 @@ static void *m_start(struct seq_file *m, loff_t *ppos)
 	hold_task_mempolicy(priv);
 	priv->tail_vma = get_gate_vma(mm);
 
+	if (priv->vma_off) {
+	       vma = find_vma(mm, priv->vma_off);
+	       if (vma)
+		       return vma;
+	}
+
+
 	if (last_addr) {
 		vma = find_vma(mm, last_addr - 1);
 		if (vma && vma->vm_start <= last_addr)
@@ -197,7 +204,18 @@ static void *m_start(struct seq_file *m, loff_t *ppos)
 static void *m_next(struct seq_file *m, void *v, loff_t *pos)
 {
 	struct proc_maps_private *priv = m->private;
-	struct vm_area_struct *next;
+	struct vm_area_struct *next, *vma = v;
+
+	if (priv->vma_off) {
+	       if (vma && vma->vm_start <= priv->vma_off &&
+		    priv->vma_off < vma->vm_end)
+		       next = vma;
+	       else
+		       next = find_vma(priv->mm, priv->vma_off);
+
+	       if (next)
+		       return next;
+	}
 
 	(*pos)++;
 	next = m_next_vma(priv, v);
@@ -1568,9 +1586,14 @@ struct numa_maps {
 	unsigned long mapcount_max;
 	unsigned long dirty;
 	unsigned long swapcache;
+	unsigned long nextaddr;
+		 long nid;
 	unsigned long node[MAX_NUMNODES];
 };
 
+#define	NUMA_VAMAPS_NID_NOPAGES 	(-1)
+#define	NUMA_VAMAPS_NID_NONE 	(-2)
+
 struct numa_maps_private {
 	struct proc_maps_private proc_maps;
 	struct numa_maps md;
@@ -1804,6 +1827,232 @@ static int show_numa_map(struct seq_file *m, void *v, int is_pid)
 	return 0;
 }
 
+/* match node id of page to previous node id, return 0 on match */
+static int vamap_match_nid(struct numa_maps *md, unsigned long addr,
+		struct page *page)
+{
+	if (page) {
+		if (md->nid == NUMA_VAMAPS_NID_NONE ||
+			 page_to_nid(page) == md->nid) {
+			if (md->nid == NUMA_VAMAPS_NID_NONE)
+				md->nid = page_to_nid(page);
+			return 0;
+		}
+	} else {
+		if (md->nid  == NUMA_VAMAPS_NID_NONE ||
+		     md->nid == NUMA_VAMAPS_NID_NOPAGES )  {
+			if (md->nid == NUMA_VAMAPS_NID_NONE)
+				md->nid = NUMA_VAMAPS_NID_NOPAGES;
+			return 0;
+		}
+	}
+	/* Did not match */
+	md->nextaddr = addr;
+	return 1;
+}
+
+static int gather_pte_stats_vamap(pmd_t *pmd, unsigned long addr,
+		unsigned long end, struct mm_walk *walk)
+{
+	struct numa_maps *md = walk->private;
+	struct vm_area_struct *vma = walk->vma;
+	spinlock_t *ptl;
+	pte_t *orig_pte;
+	pte_t *pte;
+	int ret = 0;
+
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+	ptl = pmd_trans_huge_lock(pmd, vma);
+	if (ptl) {
+		struct page *page;
+
+		page = can_gather_numa_stats_pmd(*pmd, vma, addr);
+		ret = vamap_match_nid(md, addr, page);
+		if (page && !ret)
+			gather_stats(page, md, pmd_dirty(*pmd),
+					     HPAGE_PMD_SIZE/PAGE_SIZE);
+		spin_unlock(ptl);
+		return ret;
+	}
+
+	if (pmd_trans_unstable(pmd))
+		return 0;
+#endif
+	orig_pte = pte = pte_offset_map_lock(walk->mm, pmd, addr, &ptl);
+	do {
+		struct page *page = can_gather_numa_stats(*pte, vma, addr);
+		ret = vamap_match_nid(md, addr, page);
+		if (ret)
+			break;
+		if (page)
+			gather_stats(page, md, pte_dirty(*pte), 1);
+
+	} while (pte++, addr += PAGE_SIZE, addr != end);
+	pte_unmap_unlock(orig_pte, ptl);
+	cond_resched();
+	return ret;
+}
+#ifdef CONFIG_HUGETLB_PAGE
+static int gather_hugetlb_stats_vamap(pte_t *pte, unsigned long hmask,
+		unsigned long addr, unsigned long end, struct mm_walk *walk)
+{
+	pte_t huge_pte = huge_ptep_get(pte);
+	struct numa_maps *md;
+	struct page *page;
+
+	md = walk->private;
+	if (!pte_present(huge_pte))
+		return (vamap_match_nid(md, addr, NULL));
+
+	page = pte_page(huge_pte);
+	if (!page)
+		return (vamap_match_nid(md, addr, page));
+
+	if (vamap_match_nid(md, addr, page))
+		return 1;
+	gather_stats(page, md, pte_dirty(huge_pte), 1);
+	return 0;
+}
+
+#else
+static int gather_hugetlb_stats_vamap(pte_t *pte, unsigned long hmask,
+		unsigned long addr, unsigned long end, struct mm_walk *walk)
+{
+	return 0;
+}
+#endif
+
+
+static int gather_hole_info_vamap(unsigned long start, unsigned long end,
+				struct mm_walk *walk)
+{
+	struct numa_maps *md = walk->private;
+	struct vm_area_struct *vma = walk->vma;
+
+	/*
+	 * check if we are still tracking a hole or end the walk.
+	 */
+	if ((md->nid != NUMA_VAMAPS_NID_NOPAGES &&
+	     md->nid != NUMA_VAMAPS_NID_NONE) ||
+	     vma != find_vma(walk->mm, start)) {
+		md->nextaddr = start;
+		return 1;
+	}
+
+	if (md->nid == NUMA_VAMAPS_NID_NONE)
+		md->nid = NUMA_VAMAPS_NID_NOPAGES;
+
+	return 0;
+}
+
+/*
+ * Display pages allocated per node via /proc.
+ */
+static int show_numa_vamap(struct seq_file *m, void *v, int is_pid)
+{
+	struct numa_maps_private *numa_priv = m->private;
+	struct proc_maps_private *proc_priv = &numa_priv->proc_maps;
+	struct vm_area_struct *vma = v;
+	struct numa_maps *md = &numa_priv->md;
+	struct file *file = vma->vm_file;
+	struct mm_struct *mm = vma->vm_mm;
+	struct mm_walk walk = {
+		.hugetlb_entry = gather_hugetlb_stats_vamap,
+		.pmd_entry = gather_pte_stats_vamap,
+		.pte_hole = gather_hole_info_vamap,
+		.private = md,
+		.mm = mm,
+	};
+	unsigned long start_va, next_va;
+
+	if (!mm)
+		return 0;
+
+	start_va = proc_priv->vma_off;
+	if (!start_va)
+		start_va = vma->vm_start;
+
+	if (start_va < vma->vm_end) {
+
+		/* Ensure we start with an empty numa_maps statistics */
+		memset(md, 0, sizeof(*md));
+		md->nid = NUMA_VAMAPS_NID_NONE; /* invalid nodeid at start */
+		md->nextaddr = 0;
+
+		/* mmap_sem is held by m_start() */
+		 if (walk_page_range(start_va, vma->vm_end, &walk) < 0)
+			goto out;
+
+		/*
+		 * If we reached the end of this vma.
+		 */
+		if (md->nextaddr == 0)
+			md->nextaddr = vma->vm_end;
+
+		next_va = md->nextaddr;
+		seq_printf(m, "%08lx-%08lx", start_va, next_va);
+		start_va = next_va;
+
+		if (md->nid != NUMA_VAMAPS_NID_NONE &&
+		    md->nid != NUMA_VAMAPS_NID_NOPAGES && md->node[md->nid]) {
+			seq_printf(m, " N%ld=%lu", md->nid, md->node[md->nid]);
+
+			seq_printf(m, " kernelpagesize_kB=%lu",
+					 vma_kernel_pagesize(vma) >> 10);
+		} else {
+			seq_printf(m, " - ");
+		}
+
+		if (md->anon)
+			seq_printf(m, " anon=%lu", md->anon);
+
+		if (md->dirty)
+			seq_printf(m, " dirty=%lu", md->dirty);
+
+		if (md->pages != md->anon && md->pages != md->dirty)
+			seq_printf(m, " mapped=%lu", md->pages);
+
+		if (md->mapcount_max > 1)
+			seq_printf(m, " mapmax=%lu", md->mapcount_max);
+
+		if (md->swapcache)
+			seq_printf(m, " swapcache=%lu", md->swapcache);
+
+		if (md->active < md->pages && !is_vm_hugetlb_page(vma))
+			seq_printf(m, " active=%lu", md->active);
+
+		if (md->writeback)
+			seq_printf(m, " writeback=%lu", md->writeback);
+
+		if (file) {
+			seq_puts(m, " file=");
+			seq_file_path(m, file, "\n\t= ");
+		} else if (vma->vm_start <= mm->brk &&
+				 vma->vm_end >= mm->start_brk) {
+			seq_puts(m, " heap");
+		} else if (is_stack(vma)) {
+			seq_puts(m, " stack");
+
+		}
+
+		seq_putc(m, '\n');
+	}
+
+	/*
+	 * If buffer has not overflowed update vma_off, otherwise preserve
+	 * previous offest as it will be retried.
+	 */
+	if (!seq_has_overflowed(m)) {
+	       if (md->nextaddr < vma->vm_end)
+		       proc_priv->vma_off = md->nextaddr;
+	       else
+		       proc_priv->vma_off = 0;
+	}
+out:
+	m_cache_vma(m, vma);
+	return 0;
+}
+
 static int show_pid_numa_map(struct seq_file *m, void *v)
 {
 	return show_numa_map(m, v, 1);
@@ -1814,6 +2063,16 @@ static int show_tid_numa_map(struct seq_file *m, void *v)
 	return show_numa_map(m, v, 0);
 }
 
+static int show_pid_numa_vamap(struct seq_file *m, void *v)
+{
+	return show_numa_vamap(m, v, 1);
+}
+
+static int show_tid_numa_vamap(struct seq_file *m, void *v)
+{
+	return show_numa_vamap(m, v, 0);
+}
+
 static const struct seq_operations proc_pid_numa_maps_op = {
 	.start  = m_start,
 	.next   = m_next,
@@ -1828,6 +2087,20 @@ static const struct seq_operations proc_tid_numa_maps_op = {
 	.show   = show_tid_numa_map,
 };
 
+static const struct seq_operations proc_pid_numa_vamaps_op = {
+	.start  = m_start,
+	.next   = m_next,
+	.stop   = m_stop,
+	.show   = show_pid_numa_vamap,
+};
+
+static const struct seq_operations proc_tid_numa_vamaps_op = {
+	.start  = m_start,
+	.next   = m_next,
+	.stop   = m_stop,
+	.show   = show_tid_numa_vamap,
+};
+
 static int numa_maps_open(struct inode *inode, struct file *file,
 			  const struct seq_operations *ops)
 {
@@ -1845,6 +2118,16 @@ static int tid_numa_maps_open(struct inode *inode, struct file *file)
 	return numa_maps_open(inode, file, &proc_tid_numa_maps_op);
 }
 
+static int pid_numa_vamaps_open(struct inode *inode, struct file *file)
+{
+	return numa_maps_open(inode, file, &proc_pid_numa_vamaps_op);
+}
+
+static int tid_numa_vamaps_open(struct inode *inode, struct file *file)
+{
+	return numa_maps_open(inode, file, &proc_tid_numa_vamaps_op);
+}
+
 const struct file_operations proc_pid_numa_maps_operations = {
 	.open		= pid_numa_maps_open,
 	.read		= seq_read,
@@ -1858,4 +2141,18 @@ const struct file_operations proc_tid_numa_maps_operations = {
 	.llseek		= seq_lseek,
 	.release	= proc_map_release,
 };
+
+const struct file_operations proc_pid_numa_vamaps_operations = {
+	.open		= pid_numa_vamaps_open,
+	.read		= seq_read,
+	.llseek		= seq_lseek,
+	.release	= proc_map_release,
+};
+
+const struct file_operations proc_tid_numa_vamaps_operations = {
+	.open		= tid_numa_vamaps_open,
+	.read		= seq_read,
+	.llseek		= seq_lseek,
+	.release	= proc_map_release,
+};
 #endif /* CONFIG_NUMA */
-- 
2.7.4

^ permalink raw reply related

* Re: [RFC PATCH for 4.18 00/14] Restartable Sequences
From: Daniel Colascione @ 2018-05-02  3:53 UTC (permalink / raw)
  To: mathieu.desnoyers
  Cc: Peter Zijlstra, paulmck, boqun.feng, luto, davejwatson,
	linux-kernel, linux-api, pjt, Andrew Morton, linux, tglx, mingo,
	hpa, Andrew Hunter, andi, cl, bmaurer, rostedt, josh, torvalds,
	catalin.marinas, will.deacon, mtk.manpages, Joel Fernandes
In-Reply-To: <20180430224433.17407-1-mathieu.desnoyers@efficios.com>

Hi Mathieu: this work looks very cool. See inline.

On Mon, Apr 30, 2018 at 3:48 PM Mathieu Desnoyers <
mathieu.desnoyers@efficios.com> wrote:

> Hi,

> Here is an updated RFC round of the Restartable Sequences patchset
> based on kernel 4.17-rc3. Based on feedback from Linus, I'm introducing
> only the rseq system call, keeping the rest for later.

> This already enables speeding up the Facebook jemalloc and arm64 PMC
> read from user-space use-cases, as well as speedup of use-cases relying
> on getting the current cpu number from user-space. We'll have to wait
> until a more complete solution is introduced before the LTTng-UST
> tracer can replace its ring buffer atomic instructions with rseq
> though. But let's proceed one step at a time.

I like the general theme of the kernel using its "superpowers" (in this
case, knowledge of preemption) to help userspace do a better job without
userspace code needing to enter the kernel to benefit. The per-CPU data
structures this patch enables help in a lot of use cases, but I think
there's another use case that you might not have considered, one that can
benefit from a extension to your proposed API.

Consider mutexes: in the kernel, for mutual exclusion, we can use a
spinlock, which in the kernel ends up being simpler and (in a lot of
scenarios) more efficient than a mutex: a core that takes a spinlock
promises to keep the lock held for only a very short time, and it disables
interrupts to delay asynchronous work that might unexpectedly lengthen the
critical section. A different core that wants to grab that spinlock can
just spin on the lock word, confident that its spin will be short because
any core owning the lock is guaranteed to release it very quickly. (Long
spins would be very bad for power.) The overall result is a lock that's
much lighter than a mutex. (A spinlock can also be used in places we can't
sleep, but this ability isn't relevant to the discussion below.)

Userspace doesn't have a good equivalent to a lightweight spinlock. While
you can build a spinlock in userspace, the result ends up having serious
problems because of preemption: first, a thread owning such a lock can be
preempted in its critical section, lengthening the critical section
arbitrarily. Second, a thread spinning on a lock will keep spinning even
when the owning thread isn't scheduled anywhere.

Userspace can just implement a mutex as a try-acquire and a FUTEX_WAIT on
failure. This approach works fine when there's no contention, but a system
call is a pretty heavy operation. Why pay for a system call on occasional
light congestion with a short critical section. Can we do better?

The usual approach to "better" is an "adaptive mutex". Such a thing, when
it attempts to acquire a lock another thread owns, spins for some number of
iterations, then falls back to futex. I guess that's a little better than
immediately jumping to futex, but it's not optimal. We can still spin when
the lock owner isn't scheduled, and the spin count is usually some guess
(either specified manually or estimated statistically) that's not
guaranteed to produce decent results. Even if we do pick a good spin count,
we run a very good chance of under- or over-spinning on any given
lock-acquire. We always want to sleep when spinning would be pointless.

One important case of the spin-while-not-scheduled problem is operation on
a uniprocessor system: on such a system, only a single task can be
scheduled at a time, making all spins maximally pointless. The usual
approach to avoiding wasted spins for adaptive mutexes is for the adaptive
mutex library to ask upon initialization "How many cores are in this
system?", and if the answer comes back as "1", disable spinning. This
approach is inadequate: CPU affinity can change at arbitrary times, and CPU
affinity can produce the same spin pessimization that a uniprocessor system
does.

I think a small enhancement to rseq would let us build a perfect userspace
mutex, one that spins on lock-acquire only when the lock owner is running
and that sleeps otherwise, freeing userspace from both specifying ad-hoc
spin counts and from trying to detect situations in which spinning is
generally pointless.

It'd work like this: in the per-thread rseq data structure, we'd include a
description of a futex operation for the kernel would perform (in the
context of the preempted thread) upon preemption, immediately before
schedule(). If the futex operation itself sleeps, that's no problem: we
will have still accomplished our goal of running some other thread instead
of the preempted thread.

Suppose we make a userspace mutex implemented with a lock word having three
bits: acquired, sleep_mode, and wait_pending, with the rest of the word not
being relevant at the moment.

We'd implement lock-acquire the usual way, CASing the acquired bit into the
lock and deeming the lock taken if we're successful. Except that before
attempting the CAS, we'd configure the current thread's rseq area to
bitwise-or the sleep_mode bit into the lock word if the current thread is
scheduled.

Now, imagine that we're a different thread that wants to take the lock
while the first thread owns it. We'll attempt a CAS as before. The CAS will
fail. That's fine --- we'll spin by retrying the CAS. Here's where we
differ from a conventional from a conventional adaptive mutex. A normal
adaptive mutex will execute a fixed maximum number of CAS attempts, then
FUTEX_WAIT. We, instead, keep spinning until we either grab the lock or we
notice the sleep_mode bit set in the lock word. (On every CAS attempt, we
update our local cached copy of the lock word.) When we do notice the
sleep_mode bit set, we'll fall back to the usual sleeping strategy: CAS the
wait_pending bit into the lock word and FUTEX_WAIT.

Back in the owning thread, when we release the model, we'll CAS again to
reset the acquired bit and (if set) sleep_mode bit, and if we see
wait_pending, FUTEX_WAKE any waiters. At this point, we can disable the
rseq registration. (If we're preempted after the unlock but before the rseq
disarm, we'll spuriously set sleep_mode, but that's fine, since we'll reset
it on next lock-acquire.)

This scheme gives us optimal spinning behavior. We spin on lock-acquire
only as long as the owning thread is actually running. We don't spin at all
on uniprocessor machines or in situations where we've set up affinity to
create the moral equivalent of a uniprocessor system. We correctly fall
back to sleeping when the owner itself schedules (which indicates that the
critical section is likely to last a while). And we don't need to choose
some arbitrary constant or use some estimation function to guess how many
times we want to spin. We can stop spinning as soon as we know it'll be
unproductive.

In practice, I think you'd want to impose a maximum spin count anyway to
guard against 1) unexpected non-scheduling critical section lengthening via
bugs, and 2) the possibility that the futex-on-schedule operation sleeps
before setting sleep_mode.

If you don't think the futex-on-schedule thing is a good idea, there are
other ways to accomplish the same basic task. For example, you could add an
is_running field to struct rseq, and the kernel would take of making this
field true only when the task owning the struct rseq is, in fact, running.

A sufficiently clever runtime system could stash the owning thread ID in
the lockword and provide a way to find a thread's struct rseq given its
thread ID. On lock contention, instead of switching to FUTEX_WAIT when we
notice sleep_mode set in the lock word, we'd switch to FUTEX_WAIT when we
notice is_running in the owning thread's struct rseq become false. This
approach is probably simpler, but makes each spin a bit heavier due to the
need to fetch two separate memory locations (the lockword and the
is_running field).

Anyway, I'm sure there are other variations on the general theme of the
rseq mechanism helping to optimize mutex acquisition. What do you think?

^ permalink raw reply

* [PATCH v5 28/28] fpga: dfl: afu: add DFL_FPGA_PORT_DMA_MAP/UNMAP ioctls support
From: Wu Hao @ 2018-05-02  2:50 UTC (permalink / raw)
  To: atull, mdf, linux-fpga, linux-kernel
  Cc: linux-api, luwei.kang, yi.z.zhang, hao.wu, Tim Whisonant,
	Enno Luebbers, Shiva Rao, Christopher Rauer, Xiao Guangrong
In-Reply-To: <1525229431-3087-1-git-send-email-hao.wu@intel.com>

DMA memory regions are required for Accelerated Function Unit (AFU) usage.
These two ioctls allow user space applications to map user memory regions
for dma, and unmap them after use. Iova is returned from driver to user
space application via DFL_FPGA_PORT_DMA_MAP ioctl. Application needs to unmap
it after use, otherwise, driver will unmap them in device file release
operation.

Each AFU has its own rb tree to keep track of its mapped DMA regions.

Ioctl interfaces:
* DFL_FPGA_PORT_DMA_MAP
  Do the dma mapping per user_addr and length which provided by user.
  Return iova in provided struct dfl_fpga_port_dma_map.

* DFL_FPGA_PORT_DMA_UNMAP
  Unmap the dma region per iova provided by user.

Signed-off-by: Tim Whisonant <tim.whisonant@intel.com>
Signed-off-by: Enno Luebbers <enno.luebbers@intel.com>
Signed-off-by: Shiva Rao <shiva.rao@intel.com>
Signed-off-by: Christopher Rauer <christopher.rauer@intel.com>
Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Signed-off-by: Wu Hao <hao.wu@intel.com>
---
v2: moved the code to drivers/fpga folder as suggested by Alan Tull.
    switched to GPLv2 license.
    fixed kbuild warnings.
v3: improved commit description and fixed coding style issues.
    replaced fpga_pdata_to_pcidev with fpga_pdata_to_fpga_cdev
v4: rebase and fix SPDX license issue
v5: rebase, and add DFL_ prefix to IOCTL APIs.
---
 drivers/fpga/Makefile             |   2 +-
 drivers/fpga/dfl-afu-dma-region.c | 463 ++++++++++++++++++++++++++++++++++++++
 drivers/fpga/dfl-afu-main.c       |  61 ++++-
 drivers/fpga/dfl-afu.h            |  31 ++-
 include/uapi/linux/fpga-dfl.h     |  37 +++
 5 files changed, 591 insertions(+), 3 deletions(-)
 create mode 100644 drivers/fpga/dfl-afu-dma-region.c

diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
index 041e3cd1..dd454ab 100644
--- a/drivers/fpga/Makefile
+++ b/drivers/fpga/Makefile
@@ -37,7 +37,7 @@ obj-$(CONFIG_FPGA_DFL_FME_REGION)	+= dfl-fme-region.o
 obj-$(CONFIG_FPGA_DFL_AFU)		+= dfl-afu.o
 
 dfl-fme-objs := dfl-fme-main.o dfl-fme-pr.o
-dfl-afu-objs := dfl-afu-main.o dfl-afu-region.o
+dfl-afu-objs := dfl-afu-main.o dfl-afu-region.o dfl-afu-dma-region.o
 
 # Drivers for FPGAs which implement DFL
 obj-$(CONFIG_FPGA_DFL_PCI)		+= dfl-pci.o
diff --git a/drivers/fpga/dfl-afu-dma-region.c b/drivers/fpga/dfl-afu-dma-region.c
new file mode 100644
index 0000000..600fd0c
--- /dev/null
+++ b/drivers/fpga/dfl-afu-dma-region.c
@@ -0,0 +1,463 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for FPGA Accelerated Function Unit (AFU) DMA Region Management
+ *
+ * Copyright (C) 2017 Intel Corporation, Inc.
+ *
+ * Authors:
+ *   Wu Hao <hao.wu@intel.com>
+ *   Xiao Guangrong <guangrong.xiao@linux.intel.com>
+ */
+
+#include <linux/dma-mapping.h>
+#include <linux/sched/signal.h>
+#include <linux/uaccess.h>
+
+#include "dfl-afu.h"
+
+static void put_all_pages(struct page **pages, int npages)
+{
+	int i;
+
+	for (i = 0; i < npages; i++)
+		if (pages[i])
+			put_page(pages[i]);
+}
+
+void afu_dma_region_init(struct dfl_feature_platform_data *pdata)
+{
+	struct dfl_afu *afu = dfl_fpga_pdata_get_private(pdata);
+
+	afu->dma_regions = RB_ROOT;
+}
+
+/**
+ * afu_dma_adjust_locked_vm - adjust locked memory
+ * @dev: port device
+ * @npages: number of pages
+ * @incr: increase or decrease locked memory
+ *
+ * Increase or decrease the locked memory size with npages input.
+ *
+ * Return 0 on success.
+ * Return -ENOMEM if locked memory size is over the limit and no CAP_IPC_LOCK.
+ */
+static int afu_dma_adjust_locked_vm(struct device *dev, long npages, bool incr)
+{
+	unsigned long locked, lock_limit;
+	int ret = 0;
+
+	/* the task is exiting. */
+	if (!current->mm)
+		return 0;
+
+	down_write(&current->mm->mmap_sem);
+
+	if (incr) {
+		locked = current->mm->locked_vm + npages;
+		lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
+
+		if (locked > lock_limit && !capable(CAP_IPC_LOCK))
+			ret = -ENOMEM;
+		else
+			current->mm->locked_vm += npages;
+	} else {
+		if (WARN_ON_ONCE(npages > current->mm->locked_vm))
+			npages = current->mm->locked_vm;
+		current->mm->locked_vm -= npages;
+	}
+
+	dev_dbg(dev, "[%d] RLIMIT_MEMLOCK %c%ld %ld/%ld%s\n", current->pid,
+		incr ? '+' : '-', npages << PAGE_SHIFT,
+		current->mm->locked_vm << PAGE_SHIFT, rlimit(RLIMIT_MEMLOCK),
+		ret ? "- execeeded" : "");
+
+	up_write(&current->mm->mmap_sem);
+
+	return ret;
+}
+
+/**
+ * afu_dma_pin_pages - pin pages of given dma memory region
+ * @pdata: feature device platform data
+ * @region: dma memory region to be pinned
+ *
+ * Pin all the pages of given dfl_afu_dma_region.
+ * Return 0 for success or negative error code.
+ */
+static int afu_dma_pin_pages(struct dfl_feature_platform_data *pdata,
+			     struct dfl_afu_dma_region *region)
+{
+	int npages = region->length >> PAGE_SHIFT;
+	struct device *dev = &pdata->dev->dev;
+	int ret, pinned;
+
+	ret = afu_dma_adjust_locked_vm(dev, npages, true);
+	if (ret)
+		return ret;
+
+	region->pages = kcalloc(npages, sizeof(struct page *), GFP_KERNEL);
+	if (!region->pages) {
+		ret = -ENOMEM;
+		goto unlock_vm;
+	}
+
+	pinned = get_user_pages_fast(region->user_addr, npages, 1,
+				     region->pages);
+	if (pinned < 0) {
+		ret = pinned;
+		goto put_pages;
+	} else if (pinned != npages) {
+		ret = -EFAULT;
+		goto free_pages;
+	}
+
+	dev_dbg(dev, "%d pages pinned\n", pinned);
+
+	return 0;
+
+put_pages:
+	put_all_pages(region->pages, pinned);
+free_pages:
+	kfree(region->pages);
+unlock_vm:
+	afu_dma_adjust_locked_vm(dev, npages, false);
+	return ret;
+}
+
+/**
+ * afu_dma_unpin_pages - unpin pages of given dma memory region
+ * @pdata: feature device platform data
+ * @region: dma memory region to be unpinned
+ *
+ * Unpin all the pages of given dfl_afu_dma_region.
+ * Return 0 for success or negative error code.
+ */
+static void afu_dma_unpin_pages(struct dfl_feature_platform_data *pdata,
+				struct dfl_afu_dma_region *region)
+{
+	long npages = region->length >> PAGE_SHIFT;
+	struct device *dev = &pdata->dev->dev;
+
+	put_all_pages(region->pages, npages);
+	kfree(region->pages);
+	afu_dma_adjust_locked_vm(dev, npages, false);
+
+	dev_dbg(dev, "%ld pages unpinned\n", npages);
+}
+
+/**
+ * afu_dma_check_continuous_pages - check if pages are continuous
+ * @region: dma memory region
+ *
+ * Return true if pages of given dma memory region have continuous physical
+ * address, otherwise return false.
+ */
+static bool afu_dma_check_continuous_pages(struct dfl_afu_dma_region *region)
+{
+	int npages = region->length >> PAGE_SHIFT;
+	int i;
+
+	for (i = 0; i < npages - 1; i++)
+		if (page_to_pfn(region->pages[i]) + 1 !=
+				page_to_pfn(region->pages[i + 1]))
+			return false;
+
+	return true;
+}
+
+/**
+ * dma_region_check_iova - check if memory area is fully contained in the region
+ * @region: dma memory region
+ * @iova: address of the dma memory area
+ * @size: size of the dma memory area
+ *
+ * Compare the dma memory area defined by @iova and @size with given dma region.
+ * Return true if memory area is fully contained in the region, otherwise false.
+ */
+static bool dma_region_check_iova(struct dfl_afu_dma_region *region,
+				  u64 iova, u64 size)
+{
+	if (!size && region->iova != iova)
+		return false;
+
+	return (region->iova <= iova) &&
+		(region->length + region->iova >= iova + size);
+}
+
+/**
+ * afu_dma_region_add - add given dma region to rbtree
+ * @pdata: feature device platform data
+ * @region: dma region to be added
+ *
+ * Return 0 for success, -EEXIST if dma region has already been added.
+ *
+ * Needs to be called with pdata->lock heold.
+ */
+static int afu_dma_region_add(struct dfl_feature_platform_data *pdata,
+			      struct dfl_afu_dma_region *region)
+{
+	struct dfl_afu *afu = dfl_fpga_pdata_get_private(pdata);
+	struct rb_node **new, *parent = NULL;
+
+	dev_dbg(&pdata->dev->dev, "add region (iova = %llx)\n",
+		(unsigned long long)region->iova);
+
+	new = &afu->dma_regions.rb_node;
+
+	while (*new) {
+		struct dfl_afu_dma_region *this;
+
+		this = container_of(*new, struct dfl_afu_dma_region, node);
+
+		parent = *new;
+
+		if (dma_region_check_iova(this, region->iova, region->length))
+			return -EEXIST;
+
+		if (region->iova < this->iova)
+			new = &((*new)->rb_left);
+		else if (region->iova > this->iova)
+			new = &((*new)->rb_right);
+		else
+			return -EEXIST;
+	}
+
+	rb_link_node(&region->node, parent, new);
+	rb_insert_color(&region->node, &afu->dma_regions);
+
+	return 0;
+}
+
+/**
+ * afu_dma_region_remove - remove given dma region from rbtree
+ * @pdata: feature device platform data
+ * @region: dma region to be removed
+ *
+ * Needs to be called with pdata->lock heold.
+ */
+static void afu_dma_region_remove(struct dfl_feature_platform_data *pdata,
+				  struct dfl_afu_dma_region *region)
+{
+	struct dfl_afu *afu;
+
+	dev_dbg(&pdata->dev->dev, "del region (iova = %llx)\n",
+		(unsigned long long)region->iova);
+
+	afu = dfl_fpga_pdata_get_private(pdata);
+	rb_erase(&region->node, &afu->dma_regions);
+}
+
+/**
+ * afu_dma_region_destroy - destroy all regions in rbtree
+ * @pdata: feature device platform data
+ *
+ * Needs to be called with pdata->lock heold.
+ */
+void afu_dma_region_destroy(struct dfl_feature_platform_data *pdata)
+{
+	struct dfl_afu *afu = dfl_fpga_pdata_get_private(pdata);
+	struct rb_node *node = rb_first(&afu->dma_regions);
+	struct dfl_afu_dma_region *region;
+
+	while (node) {
+		region = container_of(node, struct dfl_afu_dma_region, node);
+
+		dev_dbg(&pdata->dev->dev, "del region (iova = %llx)\n",
+			(unsigned long long)region->iova);
+
+		rb_erase(node, &afu->dma_regions);
+
+		if (region->iova)
+			dma_unmap_page(dfl_fpga_pdata_to_parent(pdata),
+				       region->iova, region->length,
+				       DMA_BIDIRECTIONAL);
+
+		if (region->pages)
+			afu_dma_unpin_pages(pdata, region);
+
+		node = rb_next(node);
+		kfree(region);
+	}
+}
+
+/**
+ * afu_dma_region_find - find the dma region from rbtree based on iova and size
+ * @pdata: feature device platform data
+ * @iova: address of the dma memory area
+ * @size: size of the dma memory area
+ *
+ * It finds the dma region from the rbtree based on @iova and @size:
+ * - if @size == 0, it finds the dma region which starts from @iova
+ * - otherwise, it finds the dma region which fully contains
+ *   [@iova, @iova+size)
+ * If nothing is matched returns NULL.
+ *
+ * Needs to be called with pdata->lock held.
+ */
+struct dfl_afu_dma_region *
+afu_dma_region_find(struct dfl_feature_platform_data *pdata, u64 iova, u64 size)
+{
+	struct dfl_afu *afu = dfl_fpga_pdata_get_private(pdata);
+	struct rb_node *node = afu->dma_regions.rb_node;
+	struct device *dev = &pdata->dev->dev;
+
+	while (node) {
+		struct dfl_afu_dma_region *region;
+
+		region = container_of(node, struct dfl_afu_dma_region, node);
+
+		if (dma_region_check_iova(region, iova, size)) {
+			dev_dbg(dev, "find region (iova = %llx)\n",
+				(unsigned long long)region->iova);
+			return region;
+		}
+
+		if (iova < region->iova)
+			node = node->rb_left;
+		else if (iova > region->iova)
+			node = node->rb_right;
+		else
+			/* the iova region is not fully covered. */
+			break;
+	}
+
+	dev_dbg(dev, "region with iova %llx and size %llx is not found\n",
+		(unsigned long long)iova, (unsigned long long)size);
+
+	return NULL;
+}
+
+/**
+ * afu_dma_region_find_iova - find the dma region from rbtree by iova
+ * @pdata: feature device platform data
+ * @iova: address of the dma region
+ *
+ * Needs to be called with pdata->lock held.
+ */
+static struct dfl_afu_dma_region *
+afu_dma_region_find_iova(struct dfl_feature_platform_data *pdata, u64 iova)
+{
+	return afu_dma_region_find(pdata, iova, 0);
+}
+
+/**
+ * afu_dma_map_region - map memory region for dma
+ * @pdata: feature device platform data
+ * @user_addr: address of the memory region
+ * @length: size of the memory region
+ * @iova: pointer of iova address
+ *
+ * Map memory region defined by @user_addr and @length, and return dma address
+ * of the memory region via @iova.
+ * Return 0 for success, otherwise error code.
+ */
+int afu_dma_map_region(struct dfl_feature_platform_data *pdata,
+		       u64 user_addr, u64 length, u64 *iova)
+{
+	struct dfl_afu_dma_region *region;
+	int ret;
+
+	/*
+	 * Check Inputs, only accept page-aligned user memory region with
+	 * valid length.
+	 */
+	if (!PAGE_ALIGNED(user_addr) || !PAGE_ALIGNED(length) || !length)
+		return -EINVAL;
+
+	/* Check overflow */
+	if (user_addr + length < user_addr)
+		return -EINVAL;
+
+	if (!access_ok(VERIFY_WRITE, (void __user *)(unsigned long)user_addr,
+		       length))
+		return -EINVAL;
+
+	region = kzalloc(sizeof(*region), GFP_KERNEL);
+	if (!region)
+		return -ENOMEM;
+
+	region->user_addr = user_addr;
+	region->length = length;
+
+	/* Pin the user memory region */
+	ret = afu_dma_pin_pages(pdata, region);
+	if (ret) {
+		dev_err(&pdata->dev->dev, "failed to pin memory region\n");
+		goto free_region;
+	}
+
+	/* Only accept continuous pages, return error else */
+	if (!afu_dma_check_continuous_pages(region)) {
+		dev_err(&pdata->dev->dev, "pages are not continuous\n");
+		ret = -EINVAL;
+		goto unpin_pages;
+	}
+
+	/* As pages are continuous then start to do DMA mapping */
+	region->iova = dma_map_page(dfl_fpga_pdata_to_parent(pdata),
+				    region->pages[0], 0,
+				    region->length,
+				    DMA_BIDIRECTIONAL);
+	if (dma_mapping_error(&pdata->dev->dev, region->iova)) {
+		dev_err(&pdata->dev->dev, "failed to map for dma\n");
+		ret = -EFAULT;
+		goto unpin_pages;
+	}
+
+	*iova = region->iova;
+
+	mutex_lock(&pdata->lock);
+	ret = afu_dma_region_add(pdata, region);
+	mutex_unlock(&pdata->lock);
+	if (ret) {
+		dev_err(&pdata->dev->dev, "failed to add dma region\n");
+		goto unmap_dma;
+	}
+
+	return 0;
+
+unmap_dma:
+	dma_unmap_page(dfl_fpga_pdata_to_parent(pdata),
+		       region->iova, region->length, DMA_BIDIRECTIONAL);
+unpin_pages:
+	afu_dma_unpin_pages(pdata, region);
+free_region:
+	kfree(region);
+	return ret;
+}
+
+/**
+ * afu_dma_unmap_region - unmap dma memory region
+ * @pdata: feature device platform data
+ * @iova: dma address of the region
+ *
+ * Unmap dma memory region based on @iova.
+ * Return 0 for success, otherwise error code.
+ */
+int afu_dma_unmap_region(struct dfl_feature_platform_data *pdata, u64 iova)
+{
+	struct dfl_afu_dma_region *region;
+
+	mutex_lock(&pdata->lock);
+	region = afu_dma_region_find_iova(pdata, iova);
+	if (!region) {
+		mutex_unlock(&pdata->lock);
+		return -EINVAL;
+	}
+
+	if (region->in_use) {
+		mutex_unlock(&pdata->lock);
+		return -EBUSY;
+	}
+
+	afu_dma_region_remove(pdata, region);
+	mutex_unlock(&pdata->lock);
+
+	dma_unmap_page(dfl_fpga_pdata_to_parent(pdata),
+		       region->iova, region->length, DMA_BIDIRECTIONAL);
+	afu_dma_unpin_pages(pdata, region);
+	kfree(region);
+
+	return 0;
+}
diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
index e7024dd..56241b8 100644
--- a/drivers/fpga/dfl-afu-main.c
+++ b/drivers/fpga/dfl-afu-main.c
@@ -293,7 +293,11 @@ static int afu_release(struct inode *inode, struct file *filp)
 
 	pdata = dev_get_platdata(&pdev->dev);
 
-	port_reset(pdev);
+	mutex_lock(&pdata->lock);
+	__port_reset(pdev);
+	afu_dma_region_destroy(pdata);
+	mutex_unlock(&pdata->lock);
+
 	dfl_feature_dev_use_end(pdata);
 
 	return 0;
@@ -364,6 +368,55 @@ static long afu_ioctl_get_region_info(struct dfl_feature_platform_data *pdata,
 	return 0;
 }
 
+static long
+afu_ioctl_dma_map(struct dfl_feature_platform_data *pdata, void __user *arg)
+{
+	struct dfl_fpga_port_dma_map map;
+	unsigned long minsz;
+	long ret;
+
+	minsz = offsetofend(struct dfl_fpga_port_dma_map, iova);
+
+	if (copy_from_user(&map, arg, minsz))
+		return -EFAULT;
+
+	if (map.argsz < minsz || map.flags)
+		return -EINVAL;
+
+	ret = afu_dma_map_region(pdata, map.user_addr, map.length, &map.iova);
+	if (ret)
+		return ret;
+
+	if (copy_to_user(arg, &map, sizeof(map))) {
+		afu_dma_unmap_region(pdata, map.iova);
+		return -EFAULT;
+	}
+
+	dev_dbg(&pdata->dev->dev, "dma map: ua=%llx, len=%llx, iova=%llx\n",
+		(unsigned long long)map.user_addr,
+		(unsigned long long)map.length,
+		(unsigned long long)map.iova);
+
+	return 0;
+}
+
+static long
+afu_ioctl_dma_unmap(struct dfl_feature_platform_data *pdata, void __user *arg)
+{
+	struct dfl_fpga_port_dma_unmap unmap;
+	unsigned long minsz;
+
+	minsz = offsetofend(struct dfl_fpga_port_dma_unmap, iova);
+
+	if (copy_from_user(&unmap, arg, minsz))
+		return -EFAULT;
+
+	if (unmap.argsz < minsz || unmap.flags)
+		return -EINVAL;
+
+	return afu_dma_unmap_region(pdata, unmap.iova);
+}
+
 static long afu_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 {
 	struct platform_device *pdev = filp->private_data;
@@ -384,6 +437,10 @@ static long afu_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 		return afu_ioctl_get_info(pdata, (void __user *)arg);
 	case DFL_FPGA_PORT_GET_REGION_INFO:
 		return afu_ioctl_get_region_info(pdata, (void __user *)arg);
+	case DFL_FPGA_PORT_DMA_MAP:
+		return afu_ioctl_dma_map(pdata, (void __user *)arg);
+	case DFL_FPGA_PORT_DMA_UNMAP:
+		return afu_ioctl_dma_unmap(pdata, (void __user *)arg);
 	default:
 		/*
 		 * Let sub-feature's ioctl function to handle the cmd
@@ -460,6 +517,7 @@ static int afu_dev_init(struct platform_device *pdev)
 	mutex_lock(&pdata->lock);
 	dfl_fpga_pdata_set_private(pdata, afu);
 	afu_mmio_region_init(pdata);
+	afu_dma_region_init(pdata);
 	mutex_unlock(&pdata->lock);
 
 	return 0;
@@ -473,6 +531,7 @@ static int afu_dev_destroy(struct platform_device *pdev)
 	mutex_lock(&pdata->lock);
 	afu = dfl_fpga_pdata_get_private(pdata);
 	afu_mmio_region_destroy(pdata);
+	afu_dma_region_destroy(pdata);
 	dfl_fpga_pdata_set_private(pdata, NULL);
 	mutex_unlock(&pdata->lock);
 
diff --git a/drivers/fpga/dfl-afu.h b/drivers/fpga/dfl-afu.h
index 7376af9..42d26b9 100644
--- a/drivers/fpga/dfl-afu.h
+++ b/drivers/fpga/dfl-afu.h
@@ -41,11 +41,31 @@ struct dfl_afu_mmio_region {
 };
 
 /**
+ * struct fpga_afu_dma_region - afu DMA region data structure
+ *
+ * @user_addr: region userspace virtual address.
+ * @length: region length.
+ * @iova: region IO virtual address.
+ * @pages: ptr to pages of this region.
+ * @node: rb tree node.
+ * @in_use: flag to indicate if this region is in_use.
+ */
+struct dfl_afu_dma_region {
+	u64 user_addr;
+	u64 length;
+	u64 iova;
+	struct page **pages;
+	struct rb_node node;
+	bool in_use;
+};
+
+/**
  * struct dfl_afu - afu device data structure
  *
  * @region_cur_offset: current region offset from start to the device fd.
  * @num_regions: num of mmio regions.
  * @regions: the mmio region link list of this afu feature device.
+ * @dma_regions: root of dma regions rb tree.
  * @num_umsgs: num of umsgs.
  * @pdata: afu platform device's pdata.
  */
@@ -54,6 +74,7 @@ struct dfl_afu {
 	int num_regions;
 	u8 num_umsgs;
 	struct list_head regions;
+	struct rb_root dma_regions;
 
 	struct dfl_feature_platform_data *pdata;
 };
@@ -68,4 +89,12 @@ int afu_mmio_region_get_by_index(struct dfl_feature_platform_data *pdata,
 int afu_mmio_region_get_by_offset(struct dfl_feature_platform_data *pdata,
 				  u64 offset, u64 size,
 				  struct dfl_afu_mmio_region *pregion);
-#endif
+void afu_dma_region_init(struct dfl_feature_platform_data *pdata);
+void afu_dma_region_destroy(struct dfl_feature_platform_data *pdata);
+int afu_dma_map_region(struct dfl_feature_platform_data *pdata,
+		       u64 user_addr, u64 length, u64 *iova);
+int afu_dma_unmap_region(struct dfl_feature_platform_data *pdata, u64 iova);
+struct dfl_afu_dma_region *
+afu_dma_region_find(struct dfl_feature_platform_data *pdata,
+		    u64 iova, u64 size);
+#endif /* __DFL_AFU_H */
diff --git a/include/uapi/linux/fpga-dfl.h b/include/uapi/linux/fpga-dfl.h
index 8a7dd5e..77af6c1 100644
--- a/include/uapi/linux/fpga-dfl.h
+++ b/include/uapi/linux/fpga-dfl.h
@@ -113,6 +113,43 @@ struct dfl_fpga_port_region_info {
 
 #define DFL_FPGA_PORT_GET_REGION_INFO	_IO(DFL_FPGA_MAGIC, DFL_PORT_BASE + 2)
 
+/**
+ * DFL_FPGA_PORT_DMA_MAP - _IOWR(DFL_FPGA_MAGIC, DFL_PORT_BASE + 3,
+ *						struct dfl_fpga_port_dma_map)
+ *
+ * Map the dma memory per user_addr and length which are provided by caller.
+ * Driver fills the iova in provided struct afu_port_dma_map.
+ * This interface only accepts page-size aligned user memory for dma mapping.
+ * Return: 0 on success, -errno on failure.
+ */
+struct dfl_fpga_port_dma_map {
+	/* Input */
+	__u32 argsz;		/* Structure length */
+	__u32 flags;		/* Zero for now */
+	__u64 user_addr;        /* Process virtual address */
+	__u64 length;           /* Length of mapping (bytes)*/
+	/* Output */
+	__u64 iova;             /* IO virtual address */
+};
+
+#define DFL_FPGA_PORT_DMA_MAP		_IO(DFL_FPGA_MAGIC, DFL_PORT_BASE + 3)
+
+/**
+ * DFL_FPGA_PORT_DMA_UNMAP - _IOW(FPGA_MAGIC, PORT_BASE + 4,
+ *						struct dfl_fpga_port_dma_unmap)
+ *
+ * Unmap the dma memory per iova provided by caller.
+ * Return: 0 on success, -errno on failure.
+ */
+struct dfl_fpga_port_dma_unmap {
+	/* Input */
+	__u32 argsz;		/* Structure length */
+	__u32 flags;		/* Zero for now */
+	__u64 iova;		/* IO virtual address */
+};
+
+#define DFL_FPGA_PORT_DMA_UNMAP		_IO(DFL_FPGA_MAGIC, DFL_PORT_BASE + 4)
+
 /* IOCTLs for FME file descriptor */
 
 /**
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH v5 27/28] fpga: dfl: afu: add afu sub feature support
From: Wu Hao @ 2018-05-02  2:50 UTC (permalink / raw)
  To: atull, mdf, linux-fpga, linux-kernel
  Cc: linux-api, luwei.kang, yi.z.zhang, hao.wu, Xiao Guangrong,
	Tim Whisonant, Enno Luebbers, Shiva Rao, Christopher Rauer
In-Reply-To: <1525229431-3087-1-git-send-email-hao.wu@intel.com>

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

User Accelerated Function Unit sub feature exposes the MMIO region of
the AFU. After valid PR bitstream is programmed and the port is enabled,
then this MMIO region could be accessed.

This patch adds support to enumerate the AFU MMIO region and expose it
to userspace via mmap file operation. Below interfaces are exposed to user:

Sysfs interface:
* /sys/class/fpga_region/<regionX>/<dfl-port.x>/afu_id
  Read-only. Indicate which PR bitstream is programmed to this AFU.

Ioctl interfaces:
* DFL_FPGA_PORT_GET_INFO
  Provide info to userspace on the number of supported region.
  Only UAFU region is supported now.

* DFL_FPGA_PORT_GET_REGION_INFO
  Provide region information, including access permission, region size,
  offset from the start of device fd.

Signed-off-by: Tim Whisonant <tim.whisonant@intel.com>
Signed-off-by: Enno Luebbers <enno.luebbers@intel.com>
Signed-off-by: Shiva Rao <shiva.rao@intel.com>
Signed-off-by: Christopher Rauer <christopher.rauer@intel.com>
Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Signed-off-by: Wu Hao <hao.wu@intel.com>
---
v2: moved the code to drivers/fpga folder as suggested by Alan Tull.
    switched to GPLv2 license.
    fixed kbuild warnings.
v3: improved commit description and fixed coding style issues.
    replaced fpga_pdata_to_pcidev with fpga_pdata_to_fpga_cdev
v4: rebase and fix SPDX license issue
v5: rebase, due to DFL framework naming changes.
    add DFL_ prefix to IOCTL API.
    remove "user" afu, and replace "green bitstream" terminology.
    add "mmio" to afu region functions and data structures.
---
 Documentation/ABI/testing/sysfs-platform-dfl-port |   9 +
 drivers/fpga/Makefile                             |   2 +-
 drivers/fpga/dfl-afu-main.c                       | 219 +++++++++++++++++++++-
 drivers/fpga/dfl-afu-region.c                     | 166 ++++++++++++++++
 drivers/fpga/dfl-afu.h                            |  71 +++++++
 include/uapi/linux/fpga-dfl.h                     |  48 +++++
 6 files changed, 508 insertions(+), 7 deletions(-)
 create mode 100644 drivers/fpga/dfl-afu-region.c
 create mode 100644 drivers/fpga/dfl-afu.h

diff --git a/Documentation/ABI/testing/sysfs-platform-dfl-port b/Documentation/ABI/testing/sysfs-platform-dfl-port
index eb1f02d..739a3fb 100644
--- a/Documentation/ABI/testing/sysfs-platform-dfl-port
+++ b/Documentation/ABI/testing/sysfs-platform-dfl-port
@@ -5,3 +5,12 @@ Contact:	Wu Hao <hao.wu@intel.com>
 Description:	Read-only. It returns id of this port. One DFL FPGA device
 		may have more than one port. Userspace could use this id to
 		distinguish different ports under same FPGA device.
+
+What:		/sys/bus/platform/devices/dfl-port.0/afu_id
+Date:		May 2018
+KernelVersion:	4.17
+Contact:	Wu Hao <hao.wu@intel.com>
+Description:	Read-only. User can program different PR bitstreams to FPGA
+		Accelerator Function Unit (AFU) for different functions. It
+		returns uuid which could be used to identify which PR bitstream
+		is programmed in this AFU.
diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
index 5c9607b..041e3cd1 100644
--- a/drivers/fpga/Makefile
+++ b/drivers/fpga/Makefile
@@ -37,7 +37,7 @@ obj-$(CONFIG_FPGA_DFL_FME_REGION)	+= dfl-fme-region.o
 obj-$(CONFIG_FPGA_DFL_AFU)		+= dfl-afu.o
 
 dfl-fme-objs := dfl-fme-main.o dfl-fme-pr.o
-dfl-afu-objs := dfl-afu-main.o
+dfl-afu-objs := dfl-afu-main.o dfl-afu-region.o
 
 # Drivers for FPGAs which implement DFL
 obj-$(CONFIG_FPGA_DFL_PCI)		+= dfl-pci.o
diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
index 57bb726..e7024dd 100644
--- a/drivers/fpga/dfl-afu-main.c
+++ b/drivers/fpga/dfl-afu-main.c
@@ -16,18 +16,18 @@
 
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/uaccess.h>
 #include <linux/fpga-dfl.h>
 
-#include "dfl.h"
+#include "dfl-afu.h"
 
 /**
  * port_enable - enable a port
  * @pdev: port platform device.
  *
  * Enable Port by clear the port soft reset bit, which is set by default.
- * The User AFU is unable to respond to any MMIO access while in reset.
- * port_enable function should only be used after port_disable
- * function.
+ * The AFU is unable to respond to any MMIO access while in reset.
+ * port_enable function should only be used after port_disable function.
  */
 static void port_enable(struct platform_device *pdev)
 {
@@ -191,12 +191,75 @@ static void port_hdr_uinit(struct platform_device *pdev,
 	.ioctl = port_hdr_ioctl,
 };
 
+static ssize_t
+afu_id_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
+	void __iomem *base;
+	u64 guidl, guidh;
+
+	base = dfl_get_feature_ioaddr_by_id(dev, PORT_FEATURE_ID_AFU);
+
+	mutex_lock(&pdata->lock);
+	if (pdata->disable_count) {
+		mutex_unlock(&pdata->lock);
+		return -EBUSY;
+	}
+
+	guidl = readq(base + GUID_L);
+	guidh = readq(base + GUID_H);
+	mutex_unlock(&pdata->lock);
+
+	return scnprintf(buf, PAGE_SIZE, "%016llx%016llx\n", guidh, guidl);
+}
+static DEVICE_ATTR_RO(afu_id);
+
+static const struct attribute *port_afu_attrs[] = {
+	&dev_attr_afu_id.attr,
+	NULL
+};
+
+static int port_afu_init(struct platform_device *pdev,
+			 struct dfl_feature *feature)
+{
+	struct resource *res = &pdev->resource[feature->resource_index];
+	int ret;
+
+	dev_dbg(&pdev->dev, "PORT AFU Init.\n");
+
+	ret = afu_mmio_region_add(dev_get_platdata(&pdev->dev),
+				  DFL_PORT_REGION_INDEX_AFU, resource_size(res),
+				  res->start, DFL_PORT_REGION_READ |
+				  DFL_PORT_REGION_WRITE | DFL_PORT_REGION_MMAP);
+	if (ret)
+		return ret;
+
+	return sysfs_create_files(&pdev->dev.kobj, port_afu_attrs);
+}
+
+static void port_afu_uinit(struct platform_device *pdev,
+			   struct dfl_feature *feature)
+{
+	dev_dbg(&pdev->dev, "PORT AFU UInit.\n");
+
+	sysfs_remove_files(&pdev->dev.kobj, port_afu_attrs);
+}
+
+static const struct dfl_feature_ops port_afu_ops = {
+	.init = port_afu_init,
+	.uinit = port_afu_uinit,
+};
+
 static struct dfl_feature_driver port_feature_drvs[] = {
 	{
 		.id = PORT_FEATURE_ID_HEADER,
 		.ops = &port_hdr_ops,
 	},
 	{
+		.id = PORT_FEATURE_ID_AFU,
+		.ops = &port_afu_ops,
+	},
+	{
 		.ops = NULL,
 	}
 };
@@ -243,6 +306,64 @@ static long afu_ioctl_check_extension(struct dfl_feature_platform_data *pdata,
 	return 0;
 }
 
+static long
+afu_ioctl_get_info(struct dfl_feature_platform_data *pdata, void __user *arg)
+{
+	struct dfl_fpga_port_info info;
+	struct dfl_afu *afu;
+	unsigned long minsz;
+
+	minsz = offsetofend(struct dfl_fpga_port_info, num_umsgs);
+
+	if (copy_from_user(&info, arg, minsz))
+		return -EFAULT;
+
+	if (info.argsz < minsz)
+		return -EINVAL;
+
+	mutex_lock(&pdata->lock);
+	afu = dfl_fpga_pdata_get_private(pdata);
+	info.flags = 0;
+	info.num_regions = afu->num_regions;
+	info.num_umsgs = afu->num_umsgs;
+	mutex_unlock(&pdata->lock);
+
+	if (copy_to_user(arg, &info, sizeof(info)))
+		return -EFAULT;
+
+	return 0;
+}
+
+static long afu_ioctl_get_region_info(struct dfl_feature_platform_data *pdata,
+				      void __user *arg)
+{
+	struct dfl_fpga_port_region_info rinfo;
+	struct dfl_afu_mmio_region region;
+	unsigned long minsz;
+	long ret;
+
+	minsz = offsetofend(struct dfl_fpga_port_region_info, offset);
+
+	if (copy_from_user(&rinfo, arg, minsz))
+		return -EFAULT;
+
+	if (rinfo.argsz < minsz || rinfo.padding)
+		return -EINVAL;
+
+	ret = afu_mmio_region_get_by_index(pdata, rinfo.index, &region);
+	if (ret)
+		return ret;
+
+	rinfo.flags = region.flags;
+	rinfo.size = region.size;
+	rinfo.offset = region.offset;
+
+	if (copy_to_user(arg, &rinfo, sizeof(rinfo)))
+		return -EFAULT;
+
+	return 0;
+}
+
 static long afu_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 {
 	struct platform_device *pdev = filp->private_data;
@@ -259,6 +380,10 @@ static long afu_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 		return DFL_FPGA_API_VERSION;
 	case DFL_FPGA_CHECK_EXTENSION:
 		return afu_ioctl_check_extension(pdata, arg);
+	case DFL_FPGA_PORT_GET_INFO:
+		return afu_ioctl_get_info(pdata, (void __user *)arg);
+	case DFL_FPGA_PORT_GET_REGION_INFO:
+		return afu_ioctl_get_region_info(pdata, (void __user *)arg);
 	default:
 		/*
 		 * Let sub-feature's ioctl function to handle the cmd
@@ -277,13 +402,83 @@ static long afu_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 	return -EINVAL;
 }
 
+static int afu_mmap(struct file *filp, struct vm_area_struct *vma)
+{
+	struct platform_device *pdev = filp->private_data;
+	struct dfl_feature_platform_data *pdata;
+	u64 size = vma->vm_end - vma->vm_start;
+	struct dfl_afu_mmio_region region;
+	u64 offset;
+	int ret;
+
+	if (!(vma->vm_flags & VM_SHARED))
+		return -EINVAL;
+
+	pdata = dev_get_platdata(&pdev->dev);
+
+	offset = vma->vm_pgoff << PAGE_SHIFT;
+	ret = afu_mmio_region_get_by_offset(pdata, offset, size, &region);
+	if (ret)
+		return ret;
+
+	if (!(region.flags & DFL_PORT_REGION_MMAP))
+		return -EINVAL;
+
+	if ((vma->vm_flags & VM_READ) && !(region.flags & DFL_PORT_REGION_READ))
+		return -EPERM;
+
+	if ((vma->vm_flags & VM_WRITE) &&
+	    !(region.flags & DFL_PORT_REGION_WRITE))
+		return -EPERM;
+
+	vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
+
+	return remap_pfn_range(vma, vma->vm_start,
+			(region.phys + (offset - region.offset)) >> PAGE_SHIFT,
+			size, vma->vm_page_prot);
+}
+
 static const struct file_operations afu_fops = {
 	.owner = THIS_MODULE,
 	.open = afu_open,
 	.release = afu_release,
 	.unlocked_ioctl = afu_ioctl,
+	.mmap = afu_mmap,
 };
 
+static int afu_dev_init(struct platform_device *pdev)
+{
+	struct dfl_feature_platform_data *pdata = dev_get_platdata(&pdev->dev);
+	struct dfl_afu *afu;
+
+	afu = devm_kzalloc(&pdev->dev, sizeof(*afu), GFP_KERNEL);
+	if (!afu)
+		return -ENOMEM;
+
+	afu->pdata = pdata;
+
+	mutex_lock(&pdata->lock);
+	dfl_fpga_pdata_set_private(pdata, afu);
+	afu_mmio_region_init(pdata);
+	mutex_unlock(&pdata->lock);
+
+	return 0;
+}
+
+static int afu_dev_destroy(struct platform_device *pdev)
+{
+	struct dfl_feature_platform_data *pdata = dev_get_platdata(&pdev->dev);
+	struct dfl_afu *afu;
+
+	mutex_lock(&pdata->lock);
+	afu = dfl_fpga_pdata_get_private(pdata);
+	afu_mmio_region_destroy(pdata);
+	dfl_fpga_pdata_set_private(pdata, NULL);
+	mutex_unlock(&pdata->lock);
+
+	return 0;
+}
+
 static int port_enable_set(struct platform_device *pdev, bool enable)
 {
 	struct dfl_feature_platform_data *pdata = dev_get_platdata(&pdev->dev);
@@ -312,14 +507,25 @@ static int afu_probe(struct platform_device *pdev)
 
 	dev_dbg(&pdev->dev, "%s\n", __func__);
 
+	ret = afu_dev_init(pdev);
+	if (ret)
+		goto exit;
+
 	ret = dfl_fpga_dev_feature_init(pdev, port_feature_drvs);
 	if (ret)
-		return ret;
+		goto dev_destroy;
 
 	ret = dfl_fpga_register_dev_ops(pdev, &afu_fops, THIS_MODULE);
-	if (ret)
+	if (ret) {
 		dfl_fpga_dev_feature_uinit(pdev);
+		goto dev_destroy;
+	}
+
+	return 0;
 
+dev_destroy:
+	afu_dev_destroy(pdev);
+exit:
 	return ret;
 }
 
@@ -329,6 +535,7 @@ static int afu_remove(struct platform_device *pdev)
 
 	dfl_fpga_unregister_dev_ops(pdev);
 	dfl_fpga_dev_feature_uinit(pdev);
+	afu_dev_destroy(pdev);
 
 	return 0;
 }
diff --git a/drivers/fpga/dfl-afu-region.c b/drivers/fpga/dfl-afu-region.c
new file mode 100644
index 0000000..2f34f92
--- /dev/null
+++ b/drivers/fpga/dfl-afu-region.c
@@ -0,0 +1,166 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for FPGA Accelerated Function Unit (AFU) MMIO Region Management
+ *
+ * Copyright (C) 2017 Intel Corporation, Inc.
+ *
+ * Authors:
+ *   Wu Hao <hao.wu@intel.com>
+ *   Xiao Guangrong <guangrong.xiao@linux.intel.com>
+ */
+#include "dfl-afu.h"
+
+/**
+ * afu_mmio_region_init - init function for afu mmio region support
+ * @pdata: afu platform device's pdata.
+ */
+void afu_mmio_region_init(struct dfl_feature_platform_data *pdata)
+{
+	struct dfl_afu *afu = dfl_fpga_pdata_get_private(pdata);
+
+	INIT_LIST_HEAD(&afu->regions);
+}
+
+#define for_each_region(region, afu)	\
+	list_for_each_entry((region), &(afu)->regions, node)
+
+static struct dfl_afu_mmio_region *get_region_by_index(struct dfl_afu *afu,
+						       u32 region_index)
+{
+	struct dfl_afu_mmio_region *region;
+
+	for_each_region(region, afu)
+		if (region->index == region_index)
+			return region;
+
+	return NULL;
+}
+
+/**
+ * afu_mmio_region_add - add a mmio region to given feature dev.
+ *
+ * @region_index: region index.
+ * @region_size: region size.
+ * @phys: region's physical address of this region.
+ * @flags: region flags (access permission).
+ *
+ * Return: 0 on success, negative error code otherwise.
+ */
+int afu_mmio_region_add(struct dfl_feature_platform_data *pdata,
+			u32 region_index, u64 region_size, u64 phys, u32 flags)
+{
+	struct dfl_afu_mmio_region *region;
+	struct dfl_afu *afu;
+	int ret = 0;
+
+	region = devm_kzalloc(&pdata->dev->dev, sizeof(*region), GFP_KERNEL);
+	if (!region)
+		return -ENOMEM;
+
+	region->index = region_index;
+	region->size = region_size;
+	region->phys = phys;
+	region->flags = flags;
+
+	mutex_lock(&pdata->lock);
+
+	afu = dfl_fpga_pdata_get_private(pdata);
+
+	/* check if @index already exists */
+	if (get_region_by_index(afu, region_index)) {
+		mutex_unlock(&pdata->lock);
+		ret = -EEXIST;
+		goto exit;
+	}
+
+	region_size = PAGE_ALIGN(region_size);
+	region->offset = afu->region_cur_offset;
+	list_add(&region->node, &afu->regions);
+
+	afu->region_cur_offset += region_size;
+	afu->num_regions++;
+	mutex_unlock(&pdata->lock);
+
+	return 0;
+
+exit:
+	devm_kfree(&pdata->dev->dev, region);
+	return ret;
+}
+
+/**
+ * afu_mmio_region_destroy - destroy all mmio regions under given feature dev.
+ * @pdata: afu platform device's pdata.
+ */
+void afu_mmio_region_destroy(struct dfl_feature_platform_data *pdata)
+{
+	struct dfl_afu *afu = dfl_fpga_pdata_get_private(pdata);
+	struct dfl_afu_mmio_region *tmp, *region;
+
+	list_for_each_entry_safe(region, tmp, &afu->regions, node)
+		devm_kfree(&pdata->dev->dev, region);
+}
+
+/**
+ * afu_mmio_region_get_by_index - find an afu region by index.
+ * @pdata: afu platform device's pdata.
+ * @region_index: region index.
+ * @pregion: ptr to region for result.
+ *
+ * Return: 0 on success, negative error code otherwise.
+ */
+int afu_mmio_region_get_by_index(struct dfl_feature_platform_data *pdata,
+				 u32 region_index,
+				 struct dfl_afu_mmio_region *pregion)
+{
+	struct dfl_afu_mmio_region *region;
+	struct dfl_afu *afu;
+	int ret = 0;
+
+	mutex_lock(&pdata->lock);
+	afu = dfl_fpga_pdata_get_private(pdata);
+	region = get_region_by_index(afu, region_index);
+	if (!region) {
+		ret = -EINVAL;
+		goto exit;
+	}
+	*pregion = *region;
+exit:
+	mutex_unlock(&pdata->lock);
+	return ret;
+}
+
+/**
+ * afu_mmio_region_get_by_offset - find an afu mmio region by offset and size
+ *
+ * @pdata: afu platform device's pdata.
+ * @offset: region offset from start of the device fd.
+ * @size: region size.
+ * @pregion: ptr to region for result.
+ *
+ * Find the region which fully contains the region described by input
+ * parameters (offset and size) from the feature dev's region link list.
+ *
+ * Return: 0 on success, negative error code otherwise.
+ */
+int afu_mmio_region_get_by_offset(struct dfl_feature_platform_data *pdata,
+				  u64 offset, u64 size,
+				  struct dfl_afu_mmio_region *pregion)
+{
+	struct dfl_afu_mmio_region *region;
+	struct dfl_afu *afu;
+	int ret = 0;
+
+	mutex_lock(&pdata->lock);
+	afu = dfl_fpga_pdata_get_private(pdata);
+	for_each_region(region, afu)
+		if (region->offset <= offset &&
+		    region->offset + region->size >= offset + size) {
+			*pregion = *region;
+			goto exit;
+		}
+	ret = -EINVAL;
+exit:
+	mutex_unlock(&pdata->lock);
+	return ret;
+}
diff --git a/drivers/fpga/dfl-afu.h b/drivers/fpga/dfl-afu.h
new file mode 100644
index 0000000..7376af9
--- /dev/null
+++ b/drivers/fpga/dfl-afu.h
@@ -0,0 +1,71 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Header file for FPGA Accelerated Function Unit (AFU) Driver
+ *
+ * Copyright (C) 2017 Intel Corporation, Inc.
+ *
+ * Authors:
+ *     Wu Hao <hao.wu@intel.com>
+ *     Xiao Guangrong <guangrong.xiao@linux.intel.com>
+ *     Joseph Grecco <joe.grecco@intel.com>
+ *     Enno Luebbers <enno.luebbers@intel.com>
+ *     Tim Whisonant <tim.whisonant@intel.com>
+ *     Ananda Ravuri <ananda.ravuri@intel.com>
+ *     Henry Mitchel <henry.mitchel@intel.com>
+ */
+
+#ifndef __DFL_AFU_H
+#define __DFL_AFU_H
+
+#include <linux/mm.h>
+
+#include "dfl.h"
+
+/**
+ * struct dfl_afu_mmio_region - afu mmio region data structure
+ *
+ * @index: region index.
+ * @flags: region flags (access permission).
+ * @size: region size.
+ * @offset: region offset from start of the device fd.
+ * @phys: region's physical address.
+ * @node: node to add to afu feature dev's region list.
+ */
+struct dfl_afu_mmio_region {
+	u32 index;
+	u32 flags;
+	u64 size;
+	u64 offset;
+	u64 phys;
+	struct list_head node;
+};
+
+/**
+ * struct dfl_afu - afu device data structure
+ *
+ * @region_cur_offset: current region offset from start to the device fd.
+ * @num_regions: num of mmio regions.
+ * @regions: the mmio region link list of this afu feature device.
+ * @num_umsgs: num of umsgs.
+ * @pdata: afu platform device's pdata.
+ */
+struct dfl_afu {
+	u64 region_cur_offset;
+	int num_regions;
+	u8 num_umsgs;
+	struct list_head regions;
+
+	struct dfl_feature_platform_data *pdata;
+};
+
+void afu_mmio_region_init(struct dfl_feature_platform_data *pdata);
+int afu_mmio_region_add(struct dfl_feature_platform_data *pdata,
+			u32 region_index, u64 region_size, u64 phys, u32 flags);
+void afu_mmio_region_destroy(struct dfl_feature_platform_data *pdata);
+int afu_mmio_region_get_by_index(struct dfl_feature_platform_data *pdata,
+				 u32 region_index,
+				 struct dfl_afu_mmio_region *pregion);
+int afu_mmio_region_get_by_offset(struct dfl_feature_platform_data *pdata,
+				  u64 offset, u64 size,
+				  struct dfl_afu_mmio_region *pregion);
+#endif
diff --git a/include/uapi/linux/fpga-dfl.h b/include/uapi/linux/fpga-dfl.h
index 777d6ac..8a7dd5e 100644
--- a/include/uapi/linux/fpga-dfl.h
+++ b/include/uapi/linux/fpga-dfl.h
@@ -65,6 +65,54 @@
 
 #define DFL_FPGA_PORT_RESET		_IO(DFL_FPGA_MAGIC, DFL_PORT_BASE + 0)
 
+/**
+ * DFL_FPGA_PORT_GET_INFO - _IOR(DFL_FPGA_MAGIC, DFL_PORT_BASE + 1,
+ *						struct dfl_fpga_port_info)
+ *
+ * Retrieve information about the fpga port.
+ * Driver fills the info in provided struct dfl_fpga_port_info.
+ * Return: 0 on success, -errno on failure.
+ */
+struct dfl_fpga_port_info {
+	/* Input */
+	__u32 argsz;		/* Structure length */
+	/* Output */
+	__u32 flags;		/* Zero for now */
+	__u32 num_regions;	/* The number of supported regions */
+	__u32 num_umsgs;	/* The number of allocated umsgs */
+};
+
+#define DFL_FPGA_PORT_GET_INFO		_IO(DFL_FPGA_MAGIC, DFL_PORT_BASE + 1)
+
+/**
+ * FPGA_PORT_GET_REGION_INFO - _IOWR(FPGA_MAGIC, PORT_BASE + 2,
+ *					struct dfl_fpga_port_region_info)
+ *
+ * Retrieve information about a device memory region.
+ * Caller provides struct dfl_fpga_port_region_info with index value set.
+ * Driver returns the region info in other fields.
+ * Return: 0 on success, -errno on failure.
+ */
+struct dfl_fpga_port_region_info {
+	/* input */
+	__u32 argsz;		/* Structure length */
+	/* Output */
+	__u32 flags;		/* Access permission */
+#define DFL_PORT_REGION_READ	(1 << 0)	/* Region is readable */
+#define DFL_PORT_REGION_WRITE	(1 << 1)	/* Region is writable */
+#define DFL_PORT_REGION_MMAP	(1 << 2)	/* Can be mmaped to userspace */
+	/* Input */
+	__u32 index;		/* Region index */
+#define DFL_PORT_REGION_INDEX_AFU	0	/* AFU */
+#define DFL_PORT_REGION_INDEX_STP	1	/* Signal Tap */
+	__u32 padding;
+	/* Output */
+	__u64 size;		/* Region size (bytes) */
+	__u64 offset;		/* Region offset from start of device fd */
+};
+
+#define DFL_FPGA_PORT_GET_REGION_INFO	_IO(DFL_FPGA_MAGIC, DFL_PORT_BASE + 2)
+
 /* IOCTLs for FME file descriptor */
 
 /**
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH v5 26/28] fpga: dfl: afu: add DFL_FPGA_GET_API_VERSION/CHECK_EXTENSION ioctls support
From: Wu Hao @ 2018-05-02  2:50 UTC (permalink / raw)
  To: atull, mdf, linux-fpga, linux-kernel
  Cc: linux-api, luwei.kang, yi.z.zhang, hao.wu, Tim Whisonant,
	Enno Luebbers, Shiva Rao, Christopher Rauer, Xiao Guangrong
In-Reply-To: <1525229431-3087-1-git-send-email-hao.wu@intel.com>

DFL_FPGA_GET_API_VERSION and DFL_FPGA_CHECK_EXTENSION ioctls are common
ones which need to be supported by all feature devices drivers including
FME and AFU. This patch implements above 2 ioctls in FPGA Accelerated
Function Unit (AFU) driver.

Signed-off-by: Tim Whisonant <tim.whisonant@intel.com>
Signed-off-by: Enno Luebbers <enno.luebbers@intel.com>
Signed-off-by: Shiva Rao <shiva.rao@intel.com>
Signed-off-by: Christopher Rauer <christopher.rauer@intel.com>
Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Signed-off-by: Wu Hao <hao.wu@intel.com>
Acked-by: Alan Tull <atull@kernel.org>
Acked-by: Moritz Fischer <mdf@kernel.org>
---
v2: rebased
v3: rebased as driver renamed to fpga-dfl-afu
    fix one checkpatch issue
v4: add Acked-by from Alan and Moritz.
v5: rebase, and add DFL_ prefix to APIs.
---
 drivers/fpga/dfl-afu-main.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
index 6f846b7..57bb726 100644
--- a/drivers/fpga/dfl-afu-main.c
+++ b/drivers/fpga/dfl-afu-main.c
@@ -236,6 +236,13 @@ static int afu_release(struct inode *inode, struct file *filp)
 	return 0;
 }
 
+static long afu_ioctl_check_extension(struct dfl_feature_platform_data *pdata,
+				      unsigned long arg)
+{
+	/* No extension support for now */
+	return 0;
+}
+
 static long afu_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 {
 	struct platform_device *pdev = filp->private_data;
@@ -248,6 +255,10 @@ static long afu_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 	pdata = dev_get_platdata(&pdev->dev);
 
 	switch (cmd) {
+	case DFL_FPGA_GET_API_VERSION:
+		return DFL_FPGA_API_VERSION;
+	case DFL_FPGA_CHECK_EXTENSION:
+		return afu_ioctl_check_extension(pdata, arg);
 	default:
 		/*
 		 * Let sub-feature's ioctl function to handle the cmd
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH v5 25/28] fpga: dfl: afu: add header sub feature support
From: Wu Hao @ 2018-05-02  2:50 UTC (permalink / raw)
  To: atull, mdf, linux-fpga, linux-kernel
  Cc: linux-api, luwei.kang, yi.z.zhang, hao.wu, Tim Whisonant,
	Enno Luebbers, Shiva Rao, Christopher Rauer, Xiao Guangrong
In-Reply-To: <1525229431-3087-1-git-send-email-hao.wu@intel.com>

The port header register set is always present for port, it is mainly
for capability, control and status of the ports that AFU connected to.

This patch implements header sub feature support. Below user interfaces
are created by this patch.

Sysfs interface:
* /sys/class/fpga_region/<regionX>/<dfl-port.x>/id
  Read-only. Port ID.

Ioctl interface:
* DFL_FPGA_PORT_RESET
  Reset the FPGA Port and its AFU.

Signed-off-by: Tim Whisonant <tim.whisonant@intel.com>
Signed-off-by: Enno Luebbers <enno.luebbers@intel.com>
Signed-off-by: Shiva Rao <shiva.rao@intel.com>
Signed-off-by: Christopher Rauer <christopher.rauer@intel.com>
Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Signed-off-by: Wu Hao <hao.wu@intel.com>
Acked-by: Alan Tull <atull@kernel.org>
---
v3: rename driver name to fpga-dfl-afu
    add more description for reset ioctl.
    fix some checkpatch issues.
v4: rebase.
    add Acked-by from Alan.
v5: rebase, and merge port functions from DFL framework code.
    add DFL_ prefix to APIs.
---
 Documentation/ABI/testing/sysfs-platform-dfl-port |  7 ++
 drivers/fpga/dfl-afu-main.c                       | 79 ++++++++++++++++++++++-
 include/uapi/linux/fpga-dfl.h                     | 17 +++++
 3 files changed, 102 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-platform-dfl-port

diff --git a/Documentation/ABI/testing/sysfs-platform-dfl-port b/Documentation/ABI/testing/sysfs-platform-dfl-port
new file mode 100644
index 0000000..eb1f02d
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-platform-dfl-port
@@ -0,0 +1,7 @@
+What:		/sys/bus/platform/devices/dfl-port.0/id
+Date:		May 2018
+KernelVersion:	4.17
+Contact:	Wu Hao <hao.wu@intel.com>
+Description:	Read-only. It returns id of this port. One DFL FPGA device
+		may have more than one port. Userspace could use this id to
+		distinguish different ports under same FPGA device.
diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
index 4bdbe3d..6f846b7 100644
--- a/drivers/fpga/dfl-afu-main.c
+++ b/drivers/fpga/dfl-afu-main.c
@@ -16,6 +16,7 @@
 
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/fpga-dfl.h>
 
 #include "dfl.h"
 
@@ -87,6 +88,41 @@ static int port_disable(struct platform_device *pdev)
 	return 0;
 }
 
+/*
+ * This function resets the FPGA Port and its accelerator (AFU) by function
+ * __port_disable and __port_enable (set port soft reset bit and then clear
+ * it). Userspace can do Port reset at any time, e.g during DMA or Partial
+ * Reconfiguration. But it should never cause any system level issue, only
+ * functional failure (e.g DMA or PR operation failure) and be recoverable
+ * from the failure.
+ *
+ * Note: the accelerator (AFU) is not accessible when its port is in reset
+ * (disabled). Any attempts on MMIO access to AFU while in reset, will
+ * result errors reported via port error reporting sub feature (if present).
+ */
+static int __port_reset(struct platform_device *pdev)
+{
+	int ret;
+
+	ret = port_disable(pdev);
+	if (!ret)
+		port_enable(pdev);
+
+	return ret;
+}
+
+static int port_reset(struct platform_device *pdev)
+{
+	struct dfl_feature_platform_data *pdata = dev_get_platdata(&pdev->dev);
+	int ret;
+
+	mutex_lock(&pdata->lock);
+	ret = __port_reset(pdev);
+	mutex_unlock(&pdata->lock);
+
+	return ret;
+}
+
 static int port_get_id(struct platform_device *pdev)
 {
 	void __iomem *base;
@@ -96,23 +132,63 @@ static int port_get_id(struct platform_device *pdev)
 	return FIELD_GET(PORT_CAP_PORT_NUM, readq(base + PORT_HDR_CAP));
 }
 
+static ssize_t
+id_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	int id = port_get_id(to_platform_device(dev));
+
+	return scnprintf(buf, PAGE_SIZE, "%d\n", id);
+}
+static DEVICE_ATTR_RO(id);
+
+static const struct attribute *port_hdr_attrs[] = {
+	&dev_attr_id.attr,
+	NULL,
+};
+
 static int port_hdr_init(struct platform_device *pdev,
 			 struct dfl_feature *feature)
 {
 	dev_dbg(&pdev->dev, "PORT HDR Init.\n");
 
-	return 0;
+	port_reset(pdev);
+
+	return sysfs_create_files(&pdev->dev.kobj, port_hdr_attrs);
 }
 
 static void port_hdr_uinit(struct platform_device *pdev,
 			   struct dfl_feature *feature)
 {
 	dev_dbg(&pdev->dev, "PORT HDR UInit.\n");
+
+	sysfs_remove_files(&pdev->dev.kobj, port_hdr_attrs);
+}
+
+static long
+port_hdr_ioctl(struct platform_device *pdev, struct dfl_feature *feature,
+	       unsigned int cmd, unsigned long arg)
+{
+	long ret;
+
+	switch (cmd) {
+	case DFL_FPGA_PORT_RESET:
+		if (!arg)
+			ret = port_reset(pdev);
+		else
+			ret = -EINVAL;
+		break;
+	default:
+		dev_dbg(&pdev->dev, "%x cmd not handled", cmd);
+		ret = -ENODEV;
+	}
+
+	return ret;
 }
 
 static const struct dfl_feature_ops port_hdr_ops = {
 	.init = port_hdr_init,
 	.uinit = port_hdr_uinit,
+	.ioctl = port_hdr_ioctl,
 };
 
 static struct dfl_feature_driver port_feature_drvs[] = {
@@ -154,6 +230,7 @@ static int afu_release(struct inode *inode, struct file *filp)
 
 	pdata = dev_get_platdata(&pdev->dev);
 
+	port_reset(pdev);
 	dfl_feature_dev_use_end(pdata);
 
 	return 0;
diff --git a/include/uapi/linux/fpga-dfl.h b/include/uapi/linux/fpga-dfl.h
index 1c8bb2d..777d6ac 100644
--- a/include/uapi/linux/fpga-dfl.h
+++ b/include/uapi/linux/fpga-dfl.h
@@ -28,8 +28,11 @@
 #define DFL_FPGA_MAGIC 0xB6
 
 #define DFL_FPGA_BASE 0
+#define DFL_PORT_BASE 0x40
 #define DFL_FME_BASE 0x80
 
+/* Common IOCTLs for both FME and AFU file descriptor */
+
 /**
  * DFL_FPGA_GET_API_VERSION - _IO(DFL_FPGA_MAGIC, DFL_FPGA_BASE + 0)
  *
@@ -48,6 +51,20 @@
 
 #define DFL_FPGA_CHECK_EXTENSION	_IO(DFL_FPGA_MAGIC, DFL_FPGA_BASE + 1)
 
+/* IOCTLs for AFU file descriptor */
+
+/**
+ * DFL_FPGA_PORT_RESET - _IO(DFL_FPGA_MAGIC, DFL_PORT_BASE + 0)
+ *
+ * Reset the FPGA Port and its AFU. No parameters are supported.
+ * Userspace can do Port reset at any time, e.g during DMA or PR. But
+ * it should never cause any system level issue, only functional failure
+ * (e.g DMA or PR operation failure) and be recoverable from the failure.
+ * Return: 0 on success, -errno of failure
+ */
+
+#define DFL_FPGA_PORT_RESET		_IO(DFL_FPGA_MAGIC, DFL_PORT_BASE + 0)
+
 /* IOCTLs for FME file descriptor */
 
 /**
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH v5 24/28] fpga: dfl: afu: add port ops support
From: Wu Hao @ 2018-05-02  2:50 UTC (permalink / raw)
  To: atull, mdf, linux-fpga, linux-kernel
  Cc: linux-api, luwei.kang, yi.z.zhang, hao.wu
In-Reply-To: <1525229431-3087-1-git-send-email-hao.wu@intel.com>

This patch registers the port ops into the global list in the DFL
framework, and it allows other modules to use the port ops. And
This patch includes the implementation of the get_id and enable_set
ops too.

Signed-off-by: Wu Hao <hao.wu@intel.com>
---
 drivers/fpga/dfl-afu-main.c | 122 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 121 insertions(+), 1 deletion(-)

diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
index 35bec47..4bdbe3d 100644
--- a/drivers/fpga/dfl-afu-main.c
+++ b/drivers/fpga/dfl-afu-main.c
@@ -19,6 +19,83 @@
 
 #include "dfl.h"
 
+/**
+ * port_enable - enable a port
+ * @pdev: port platform device.
+ *
+ * Enable Port by clear the port soft reset bit, which is set by default.
+ * The User AFU is unable to respond to any MMIO access while in reset.
+ * port_enable function should only be used after port_disable
+ * function.
+ */
+static void port_enable(struct platform_device *pdev)
+{
+	struct dfl_feature_platform_data *pdata = dev_get_platdata(&pdev->dev);
+	void __iomem *base;
+	u64 v;
+
+	WARN_ON(!pdata->disable_count);
+
+	if (--pdata->disable_count != 0)
+		return;
+
+	base = dfl_get_feature_ioaddr_by_id(&pdev->dev, PORT_FEATURE_ID_HEADER);
+
+	/* Clear port soft reset */
+	v = readq(base + PORT_HDR_CTRL);
+	v &= ~PORT_CTRL_SFTRST;
+	writeq(v, base + PORT_HDR_CTRL);
+}
+
+#define RST_POLL_INVL 10 /* us */
+#define RST_POLL_TIMEOUT 1000 /* us */
+
+/**
+ * port_disable - disable a port
+ * @pdev: port platform device.
+ *
+ * Disable Port by setting the port soft reset bit, it puts the port into
+ * reset.
+ */
+static int port_disable(struct platform_device *pdev)
+{
+	struct dfl_feature_platform_data *pdata = dev_get_platdata(&pdev->dev);
+	void __iomem *base;
+	u64 v;
+
+	if (pdata->disable_count++ != 0)
+		return 0;
+
+	base = dfl_get_feature_ioaddr_by_id(&pdev->dev, PORT_FEATURE_ID_HEADER);
+
+	/* Set port soft reset */
+	v = readq(base + PORT_HDR_CTRL);
+	v |= PORT_CTRL_SFTRST;
+	writeq(v, base + PORT_HDR_CTRL);
+
+	/*
+	 * HW sets ack bit to 1 when all outstanding requests have been drained
+	 * on this port and minimum soft reset pulse width has elapsed.
+	 * Driver polls port_soft_reset_ack to determine if reset done by HW.
+	 */
+	if (readq_poll_timeout(base + PORT_HDR_CTRL, v, v & PORT_CTRL_SFTRST,
+			       RST_POLL_INVL, RST_POLL_TIMEOUT)) {
+		dev_err(&pdev->dev, "timeout, fail to reset device\n");
+		return -ETIMEDOUT;
+	}
+
+	return 0;
+}
+
+static int port_get_id(struct platform_device *pdev)
+{
+	void __iomem *base;
+
+	base = dfl_get_feature_ioaddr_by_id(&pdev->dev, PORT_FEATURE_ID_HEADER);
+
+	return FIELD_GET(PORT_CAP_PORT_NUM, readq(base + PORT_HDR_CAP));
+}
+
 static int port_hdr_init(struct platform_device *pdev,
 			 struct dfl_feature *feature)
 {
@@ -119,6 +196,28 @@ static long afu_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 	.unlocked_ioctl = afu_ioctl,
 };
 
+static int port_enable_set(struct platform_device *pdev, bool enable)
+{
+	struct dfl_feature_platform_data *pdata = dev_get_platdata(&pdev->dev);
+	int ret = 0;
+
+	mutex_lock(&pdata->lock);
+	if (enable)
+		port_enable(pdev);
+	else
+		ret = port_disable(pdev);
+	mutex_unlock(&pdata->lock);
+
+	return ret;
+}
+
+static struct dfl_fpga_port_ops afu_port_ops = {
+	.name = DFL_FPGA_FEATURE_DEV_PORT,
+	.owner = THIS_MODULE,
+	.get_id = port_get_id,
+	.enable_set = port_enable_set,
+};
+
 static int afu_probe(struct platform_device *pdev)
 {
 	int ret;
@@ -154,7 +253,28 @@ static int afu_remove(struct platform_device *pdev)
 	.remove  = afu_remove,
 };
 
-module_platform_driver(afu_driver);
+static int __init afu_init(void)
+{
+	int ret;
+
+	dfl_fpga_add_port_ops(&afu_port_ops);
+
+	ret = platform_driver_register(&afu_driver);
+	if (ret)
+		dfl_fpga_del_port_ops(&afu_port_ops);
+
+	return ret;
+}
+
+static void __exit afu_exit(void)
+{
+	platform_driver_unregister(&afu_driver);
+
+	dfl_fpga_del_port_ops(&afu_port_ops);
+}
+
+module_init(afu_init);
+module_exit(afu_exit);
 
 MODULE_DESCRIPTION("FPGA Accelerated Function Unit driver");
 MODULE_AUTHOR("Intel Corporation");
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH v5 23/28] fpga: dfl: add FPGA Accelerated Function Unit driver basic framework
From: Wu Hao @ 2018-05-02  2:50 UTC (permalink / raw)
  To: atull, mdf, linux-fpga, linux-kernel
  Cc: linux-api, luwei.kang, yi.z.zhang, hao.wu, Tim Whisonant,
	Enno Luebbers, Shiva Rao, Christopher Rauer, Xiao Guangrong
In-Reply-To: <1525229431-3087-1-git-send-email-hao.wu@intel.com>

On DFL FPGA devices, the Accelerated Function Unit (AFU), can be
reprogrammed for different functions. It connects to the FPGA
infrastructure (static FPGA region) via a Port. Port CSRs are
implemented separately from the AFU CSRs to provide control and
status of the Port. Once valid PR bitstream is programmed into
the AFU, it allows access to the AFU CSRs in the AFU MMIO space.

This patch only implements basic driver framework for AFU, including
device file operation framework.

Signed-off-by: Tim Whisonant <tim.whisonant@intel.com>
Signed-off-by: Enno Luebbers <enno.luebbers@intel.com>
Signed-off-by: Shiva Rao <shiva.rao@intel.com>
Signed-off-by: Christopher Rauer <christopher.rauer@intel.com>
Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Signed-off-by: Wu Hao <hao.wu@intel.com>
Acked-by: Alan Tull <atull@kernel.org>
---
v3: rename driver to dfl-afu-main.c
v4: rename to dfl-port and fix SPDX license issue.
v5: rebase, and add Acked-by from Alan.
    fix uinit order in remove function.
---
 drivers/fpga/Kconfig        |   9 +++
 drivers/fpga/Makefile       |   2 +
 drivers/fpga/dfl-afu-main.c | 162 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 173 insertions(+)
 create mode 100644 drivers/fpga/dfl-afu-main.c

diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index 65d54a4..4c6b45f 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -168,6 +168,15 @@ config FPGA_DFL_FME_REGION
 	help
 	  Say Y to enable FPGA Region driver for FPGA Management Engine.
 
+config FPGA_DFL_AFU
+	tristate "FPGA DFL AFU Driver"
+	depends on FPGA_DFL
+	help
+	  This is the driver for FPGA Accelerated Function Unit (AFU) which
+	  implements AFU and Port management features. A User AFU connects
+	  to the FPGA infrastructure via a Port. There may be more than 1
+	  Port/AFU per DFL based FPGA device.
+
 config FPGA_DFL_PCI
 	tristate "FPGA Device Feature List (DFL) PCIe Device Driver"
 	depends on PCI && FPGA_DFL
diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
index 163894e..5c9607b 100644
--- a/drivers/fpga/Makefile
+++ b/drivers/fpga/Makefile
@@ -34,8 +34,10 @@ obj-$(CONFIG_FPGA_DFL_FME)		+= dfl-fme.o
 obj-$(CONFIG_FPGA_DFL_FME_MGR)		+= dfl-fme-mgr.o
 obj-$(CONFIG_FPGA_DFL_FME_BRIDGE)	+= dfl-fme-br.o
 obj-$(CONFIG_FPGA_DFL_FME_REGION)	+= dfl-fme-region.o
+obj-$(CONFIG_FPGA_DFL_AFU)		+= dfl-afu.o
 
 dfl-fme-objs := dfl-fme-main.o dfl-fme-pr.o
+dfl-afu-objs := dfl-afu-main.o
 
 # Drivers for FPGAs which implement DFL
 obj-$(CONFIG_FPGA_DFL_PCI)		+= dfl-pci.o
diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
new file mode 100644
index 0000000..35bec47
--- /dev/null
+++ b/drivers/fpga/dfl-afu-main.c
@@ -0,0 +1,162 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for FPGA Accelerated Function Unit (AFU)
+ *
+ * Copyright (C) 2017 Intel Corporation, Inc.
+ *
+ * Authors:
+ *   Wu Hao <hao.wu@intel.com>
+ *   Xiao Guangrong <guangrong.xiao@linux.intel.com>
+ *   Joseph Grecco <joe.grecco@intel.com>
+ *   Enno Luebbers <enno.luebbers@intel.com>
+ *   Tim Whisonant <tim.whisonant@intel.com>
+ *   Ananda Ravuri <ananda.ravuri@intel.com>
+ *   Henry Mitchel <henry.mitchel@intel.com>
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+
+#include "dfl.h"
+
+static int port_hdr_init(struct platform_device *pdev,
+			 struct dfl_feature *feature)
+{
+	dev_dbg(&pdev->dev, "PORT HDR Init.\n");
+
+	return 0;
+}
+
+static void port_hdr_uinit(struct platform_device *pdev,
+			   struct dfl_feature *feature)
+{
+	dev_dbg(&pdev->dev, "PORT HDR UInit.\n");
+}
+
+static const struct dfl_feature_ops port_hdr_ops = {
+	.init = port_hdr_init,
+	.uinit = port_hdr_uinit,
+};
+
+static struct dfl_feature_driver port_feature_drvs[] = {
+	{
+		.id = PORT_FEATURE_ID_HEADER,
+		.ops = &port_hdr_ops,
+	},
+	{
+		.ops = NULL,
+	}
+};
+
+static int afu_open(struct inode *inode, struct file *filp)
+{
+	struct platform_device *fdev = dfl_fpga_inode_to_feature_dev(inode);
+	struct dfl_feature_platform_data *pdata;
+	int ret;
+
+	pdata = dev_get_platdata(&fdev->dev);
+	if (WARN_ON(!pdata))
+		return -ENODEV;
+
+	ret = dfl_feature_dev_use_begin(pdata);
+	if (ret)
+		return ret;
+
+	dev_dbg(&fdev->dev, "Device File Open\n");
+	filp->private_data = fdev;
+
+	return 0;
+}
+
+static int afu_release(struct inode *inode, struct file *filp)
+{
+	struct platform_device *pdev = filp->private_data;
+	struct dfl_feature_platform_data *pdata;
+
+	dev_dbg(&pdev->dev, "Device File Release\n");
+
+	pdata = dev_get_platdata(&pdev->dev);
+
+	dfl_feature_dev_use_end(pdata);
+
+	return 0;
+}
+
+static long afu_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
+{
+	struct platform_device *pdev = filp->private_data;
+	struct dfl_feature_platform_data *pdata;
+	struct dfl_feature *f;
+	long ret;
+
+	dev_dbg(&pdev->dev, "%s cmd 0x%x\n", __func__, cmd);
+
+	pdata = dev_get_platdata(&pdev->dev);
+
+	switch (cmd) {
+	default:
+		/*
+		 * Let sub-feature's ioctl function to handle the cmd
+		 * Sub-feature's ioctl returns -ENODEV when cmd is not
+		 * handled in this sub feature, and returns 0 and other
+		 * error code if cmd is handled.
+		 */
+		dfl_fpga_dev_for_each_feature(pdata, f)
+			if (f->ops && f->ops->ioctl) {
+				ret = f->ops->ioctl(pdev, f, cmd, arg);
+				if (ret != -ENODEV)
+					return ret;
+			}
+	}
+
+	return -EINVAL;
+}
+
+static const struct file_operations afu_fops = {
+	.owner = THIS_MODULE,
+	.open = afu_open,
+	.release = afu_release,
+	.unlocked_ioctl = afu_ioctl,
+};
+
+static int afu_probe(struct platform_device *pdev)
+{
+	int ret;
+
+	dev_dbg(&pdev->dev, "%s\n", __func__);
+
+	ret = dfl_fpga_dev_feature_init(pdev, port_feature_drvs);
+	if (ret)
+		return ret;
+
+	ret = dfl_fpga_register_dev_ops(pdev, &afu_fops, THIS_MODULE);
+	if (ret)
+		dfl_fpga_dev_feature_uinit(pdev);
+
+	return ret;
+}
+
+static int afu_remove(struct platform_device *pdev)
+{
+	dev_dbg(&pdev->dev, "%s\n", __func__);
+
+	dfl_fpga_unregister_dev_ops(pdev);
+	dfl_fpga_dev_feature_uinit(pdev);
+
+	return 0;
+}
+
+static struct platform_driver afu_driver = {
+	.driver	= {
+		.name    = DFL_FPGA_FEATURE_DEV_PORT,
+	},
+	.probe   = afu_probe,
+	.remove  = afu_remove,
+};
+
+module_platform_driver(afu_driver);
+
+MODULE_DESCRIPTION("FPGA Accelerated Function Unit driver");
+MODULE_AUTHOR("Intel Corporation");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:dfl-port");
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH v5 22/28] fpga: dfl: fme-region: add support for compat_id
From: Wu Hao @ 2018-05-02  2:50 UTC (permalink / raw)
  To: atull, mdf, linux-fpga, linux-kernel
  Cc: linux-api, luwei.kang, yi.z.zhang, hao.wu
In-Reply-To: <1525229431-3087-1-git-send-email-hao.wu@intel.com>

This patch adds compat_id support, it reuses fme manager's
compat id, as the per region compat id is actually from the
fme manager's register.

Signed-off-by: Wu Hao <hao.wu@intel.com>
---
v5: reuse fme manager's compat_id as per region compat_id
---
 drivers/fpga/dfl-fme-region.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/fpga/dfl-fme-region.c b/drivers/fpga/dfl-fme-region.c
index 696b313..b82a381 100644
--- a/drivers/fpga/dfl-fme-region.c
+++ b/drivers/fpga/dfl-fme-region.c
@@ -45,6 +45,7 @@ static int fme_region_probe(struct platform_device *pdev)
 	}
 
 	region->priv = pdata;
+	region->compat_id = mgr->compat_id;
 	platform_set_drvdata(pdev, region);
 
 	ret = fpga_region_register(region);
-- 
1.8.3.1

^ permalink raw reply related


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