* Re: [RFC PATCH for 4.18 1/2] rseq: use __u64 for rseq_cs fields, validate abort_ip < TASK_SIZE
From: Mathieu Desnoyers @ 2018-07-02 22:03 UTC (permalink / raw)
To: Linus Torvalds
Cc: Thomas Gleixner, linux-kernel, linux-api, Peter Zijlstra,
Paul E. McKenney, Boqun Feng, Andy Lutomirski, Dave Watson,
Paul Turner, Andrew Morton, Russell King, Ingo Molnar,
H. Peter Anvin, Andi Kleen, Chris Lameter, Ben Maurer, rostedt,
Josh Triplett, Catalin Marinas, Will Deacon,
Michael Kerrisk <mtk.ma>
In-Reply-To: <CA+55aFwuBef8ajmeE-mw=6YHwYOQdnCMir7h2ebWxOzNrSQQFw@mail.gmail.com>
----- On Jul 2, 2018, at 5:20 PM, Linus Torvalds torvalds@linux-foundation.org wrote:
> On Mon, Jul 2, 2018 at 2:03 PM Mathieu Desnoyers
> <mathieu.desnoyers@efficios.com> wrote:
>>
>> /* Ensure that abort_ip is not in the critical section. */
>> if (rseq_cs->abort_ip - rseq_cs->start_ip < rseq_cs->post_commit_offset)
>> return -EINVAL;
>> ...
>> What underflow issues are you concerned with ?
>
> That.
>
> Looking closer, it looks like what you want to do is
>
> if (rseq_cs->abort_ip >= rseq_cs->start_ip && rseq_cs->abort_ip <
> rseq_cs->start_ip + rseq_cs->post_commit_offset)
>
> but you're not actually verifying that the range you're testing is
> even vlid, because "rseq_cs->start_ip + rseq_cs->post_commit_offset"
> could be something invalid that overflowed (or, put another way, the
> subtraction you did on both sides to get the simplified version
> underflowed).
>
> So to actually get the range check you want, you should check the
> overflow/underflow condition. Maybe it ends up being
>
> if (rseq_cs->start_ip + rseq_cs->post_commit_offset < rseq_cs->start_ip)
> return -EINVAL;
>
> after which your simplified conditional looks fine.
>
> But I think you should also do
>
> if (rseq_cs->start_ip + rseq_cs->post_commit_offset > TASK_SIZE)
> return -EINVAL;
>
> to make sure the range is valid in the first place.
Taking into account your comments, and adding also an extra check for
rseq_cs->start_ip >= TASK_SIZE, and restricting the end of range
rseq_cs->start_ip + rseq_cs->post_commit_offset to exclude TASK_SIZE
(>= rather than >), the resulting function now looks like this:
static int rseq_get_rseq_cs(struct task_struct *t, struct rseq_cs *rseq_cs)
{
struct rseq_cs __user *urseq_cs;
unsigned long ptr;
u32 __user *usig;
u32 sig;
if (__get_user(ptr, &t->rseq->rseq_cs))
return -EINVAL;
if (check_rseq_cs_padding(t))
return -EINVAL;
if (!ptr) {
memset(rseq_cs, 0, sizeof(*rseq_cs));
return 0;
}
urseq_cs = (struct rseq_cs __user *)ptr;
if (copy_from_user(rseq_cs, urseq_cs, sizeof(*rseq_cs)) ||
rseq_cs->start_ip >= TASK_SIZE ||
rseq_cs->start_ip + rseq_cs->post_commit_offset >= TASK_SIZE ||
rseq_cs->abort_ip >= TASK_SIZE ||
rseq_cs->version > 0)
return -EINVAL;
/* Check for overflow. */
if (rseq_cs->start_ip + rseq_cs->post_commit_offset < rseq_cs->start_ip)
return -EINVAL;
/* Ensure that abort_ip is not in the critical section. */
if (rseq_cs->abort_ip - rseq_cs->start_ip < rseq_cs->post_commit_offset)
return -EINVAL;
usig = (u32 __user *)(unsigned long)(rseq_cs->abort_ip - sizeof(u32));
if (get_user(sig, usig))
return -EINVAL;
if (current->rseq_sig != sig) {
printk_ratelimited(KERN_WARNING
"Possible attack attempt. Unexpected rseq signature 0x%x, expecting 0x%x (pid=%d, addr=%p).\n",
sig, current->rseq_sig, current->pid, usig);
return -EINVAL;
}
return 0;
}
The end of range exclusion with (rseq_cs->start_ip + rseq_cs->post_commit_offset >= TASK_SIZE)
stems from the reasoning that we need a valid user-space instruction _after_ the range, so
having the range end exactly at the very last byte of TASK_SIZE would require to have a
user-space instruction at TASK_SIZE, which is not valid.
Does it capture your intent ?
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [RFC PATCH for 4.18 1/2] rseq: use __u64 for rseq_cs fields, validate abort_ip < TASK_SIZE
From: Linus Torvalds @ 2018-07-02 21:20 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Thomas Gleixner, Linux Kernel Mailing List, Linux API,
Peter Zijlstra, Paul McKenney, Boqun Feng, Andy Lutomirski,
Dave Watson, Paul Turner, Andrew Morton, Russell King - ARM Linux,
Ingo Molnar, Peter Anvin, Andi Kleen, Christoph Lameter,
Ben Maurer, Steven Rostedt, Josh Triplett, Catalin Marinas,
Will Deacon
In-Reply-To: <1307337131.10790.1530565424717.JavaMail.zimbra@efficios.com>
On Mon, Jul 2, 2018 at 2:03 PM Mathieu Desnoyers
<mathieu.desnoyers@efficios.com> wrote:
>
> /* Ensure that abort_ip is not in the critical section. */
> if (rseq_cs->abort_ip - rseq_cs->start_ip < rseq_cs->post_commit_offset)
> return -EINVAL;
> ...
> What underflow issues are you concerned with ?
That.
Looking closer, it looks like what you want to do is
if (rseq_cs->abort_ip >= rseq_cs->start_ip && rseq_cs->abort_ip <
rseq_cs->start_ip + rseq_cs->post_commit_offset)
but you're not actually verifying that the range you're testing is
even vlid, because "rseq_cs->start_ip + rseq_cs->post_commit_offset"
could be something invalid that overflowed (or, put another way, the
subtraction you did on both sides to get the simplified version
underflowed).
So to actually get the range check you want, you should check the
overflow/underflow condition. Maybe it ends up being
if (rseq_cs->start_ip + rseq_cs->post_commit_offset < rseq_cs->start_ip)
return -EINVAL;
after which your simplified conditional looks fine.
But I think you should also do
if (rseq_cs->start_ip + rseq_cs->post_commit_offset > TASK_SIZE)
return -EINVAL;
to make sure the range is valid in the first place.
Linus
^ permalink raw reply
* Re: [RFC PATCH for 4.18 1/2] rseq: use __u64 for rseq_cs fields, validate abort_ip < TASK_SIZE
From: Mathieu Desnoyers @ 2018-07-02 21:03 UTC (permalink / raw)
To: Linus Torvalds
Cc: Thomas Gleixner, linux-kernel, linux-api, Peter Zijlstra,
Paul E. McKenney, Boqun Feng, Andy Lutomirski, Dave Watson,
Paul Turner, Andrew Morton, Russell King, Ingo Molnar,
H. Peter Anvin, Andi Kleen, Chris Lameter, Ben Maurer, rostedt,
Josh Triplett, Catalin Marinas, Will Deacon,
Michael Kerrisk <mtk.ma>
In-Reply-To: <CA+55aFzH+4=Aeh_P1ptFn5nVm7yFuEGpxUJ0kNB-sUHVOv8rcw@mail.gmail.com>
----- On Jul 2, 2018, at 4:52 PM, Linus Torvalds torvalds@linux-foundation.org wrote:
> On Mon, Jul 2, 2018 at 1:41 PM Mathieu Desnoyers
> <mathieu.desnoyers@efficios.com> wrote:
>>
>> - if (copy_from_user(rseq_cs, urseq_cs, sizeof(*rseq_cs)))
>> + if (copy_from_user(rseq_cs, urseq_cs, sizeof(*rseq_cs)) ||
>> + rseq_cs->abort_ip >= TASK_SIZE)
>> return -EFAULT;
>
> I think the abort_ip check should have the same error value as the
> other sanity checks, ie just be of this format:
>
>> if (rseq_cs->version > 0)
>> return -EINVAL;
OK, so I'll go for -EINVAL.
>
> also, I think you should check start_ip to be consistent. You kind of
> accidentally do it with the check for
>
> if (rseq_cs->abort_ip - rseq_cs->start_ip - rseq_cs->post_commit_offset)
The check is actually:
/* Ensure that abort_ip is not in the critical section. */
if (rseq_cs->abort_ip - rseq_cs->start_ip < rseq_cs->post_commit_offset)
return -EINVAL;
>
> but honestly, that has underflow issues already, so I think you want
> to basically make the check be
>
> if (rseq_cs->abort_ip >= TASK_SIZE)
> return -EINVAL;
that works.
>
> if (rseq_cs->start_ip >= rseq_cs->abort_ip)
> return -EINVAL;
this one does not work. We need to ensure that abort_ip
is not between [ start_ip, start_ip + post_commit_offset ]. The
check you propose validates that start_ip is below abort_ip,
which is bogus. For instance, abort_ip can very well be in a
different section of the binary, at an address either below
or above start_ip.
>
> which takes care of checkint start_ip, and also the underflow for the
> post_commit_offset check.
What underflow issues are you concerned with ?
>
> If somebody is depending on negative offsets, then that
> post_commit_offset logic is already wrong.
>
>> + usig = (u32 __user *)(unsigned long)(rseq_cs->abort_ip - sizeof(u32));
>> ret = get_user(sig, usig);
>
> That can underflow too, but I guess we can just rely on get_user()
> getting it right.
Yes, get_user() should handle that one properly.
Thanks,
Mathieu
>
> Linus
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [RFC PATCH for 4.18 1/2] rseq: use __u64 for rseq_cs fields, validate abort_ip < TASK_SIZE
From: Linus Torvalds @ 2018-07-02 20:52 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Thomas Gleixner, Linux Kernel Mailing List, Linux API,
Peter Zijlstra, Paul McKenney, Boqun Feng, Andy Lutomirski,
Dave Watson, Paul Turner, Andrew Morton, Russell King - ARM Linux,
Ingo Molnar, Peter Anvin, Andi Kleen, Christoph Lameter,
Ben Maurer, Steven Rostedt, Josh Triplett, Catalin Marinas,
Will Deacon
In-Reply-To: <20180702204058.819-1-mathieu.desnoyers@efficios.com>
On Mon, Jul 2, 2018 at 1:41 PM Mathieu Desnoyers
<mathieu.desnoyers@efficios.com> wrote:
>
> - if (copy_from_user(rseq_cs, urseq_cs, sizeof(*rseq_cs)))
> + if (copy_from_user(rseq_cs, urseq_cs, sizeof(*rseq_cs)) ||
> + rseq_cs->abort_ip >= TASK_SIZE)
> return -EFAULT;
I think the abort_ip check should have the same error value as the
other sanity checks, ie just be of this format:
> if (rseq_cs->version > 0)
> return -EINVAL;
also, I think you should check start_ip to be consistent. You kind of
accidentally do it with the check for
if (rseq_cs->abort_ip - rseq_cs->start_ip - rseq_cs->post_commit_offset)
but honestly, that has underflow issues already, so I think you want
to basically make the check be
if (rseq_cs->abort_ip >= TASK_SIZE)
return -EINVAL;
if (rseq_cs->start_ip >= rseq_cs->abort_ip)
return -EINVAL;
which takes care of checkint start_ip, and also the underflow for the
post_commit_offset check.
If somebody is depending on negative offsets, then that
post_commit_offset logic is already wrong.
> + usig = (u32 __user *)(unsigned long)(rseq_cs->abort_ip - sizeof(u32));
> ret = get_user(sig, usig);
That can underflow too, but I guess we can just rely on get_user()
getting it right.
Linus
^ permalink raw reply
* [RFC PATCH for 4.18 2/2] rseq: validate rseq->rseq_cs padding to be zero
From: Mathieu Desnoyers @ 2018-07-02 20:40 UTC (permalink / raw)
To: Thomas Gleixner
Cc: linux-kernel, linux-api, Peter Zijlstra, Paul E . McKenney,
Boqun Feng, Andy Lutomirski, Dave Watson, Paul Turner,
Andrew Morton, Russell King, Ingo Molnar, H . Peter Anvin,
Andi Kleen, Chris Lameter, Ben Maurer, Steven Rostedt,
Josh Triplett, Linus Torvalds, Catalin Marinas, Will Deacon,
Michael Kerrisk
In-Reply-To: <20180702204058.819-1-mathieu.desnoyers@efficios.com>
On 32-bit kernels, the rseq->rseq_cs_padding field is never read by the
kernel. However, 64-bit kernels dealing with 32-bit compat tasks read the
full 64-bit in its entirety, and terminates the offending process with
a segmentation fault if the upper 32 bits are set due to failure of
copy_from_user().
Ensure that both 32-bit and 64-bit kernels dealing with 32-bit tasks end
up terminating offending tasks with a segmentation fault if the upper
32-bit padding bits (rseq->rseq_cs_padding) are set by explicitly ensuring
that padding is zero on 32-bit kernels.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Paul Turner <pjt@google.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Andi Kleen <andi@firstfloor.org>
CC: Dave Watson <davejwatson@fb.com>
CC: Chris Lameter <cl@linux.com>
CC: Ingo Molnar <mingo@redhat.com>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Ben Maurer <bmaurer@fb.com>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Josh Triplett <josh@joshtriplett.org>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: Russell King <linux@arm.linux.org.uk>
CC: Catalin Marinas <catalin.marinas@arm.com>
CC: Will Deacon <will.deacon@arm.com>
CC: Michael Kerrisk <mtk.manpages@gmail.com>
CC: Boqun Feng <boqun.feng@gmail.com>
CC: linux-api@vger.kernel.org
---
kernel/rseq.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/kernel/rseq.c b/kernel/rseq.c
index 2e5d88f09baf..c4c48157198f 100644
--- a/kernel/rseq.c
+++ b/kernel/rseq.c
@@ -112,6 +112,29 @@ static int rseq_reset_rseq_cpu_id(struct task_struct *t)
return 0;
}
+#ifndef __LP64__
+/*
+ * Ensure that padding is zero.
+ */
+static int check_rseq_cs_padding(struct task_struct *t)
+{
+ unsigned long pad;
+ int ret;
+
+ ret = __get_user(pad, &t->rseq->rseq_cs_padding);
+ if (ret)
+ return ret;
+ if (pad)
+ return -EFAULT;
+ return 0;
+}
+#else
+static int check_rseq_cs_padding(struct task_struct *t)
+{
+ return 0;
+}
+#endif
+
static int rseq_get_rseq_cs(struct task_struct *t, struct rseq_cs *rseq_cs)
{
struct rseq_cs __user *urseq_cs;
@@ -123,6 +146,9 @@ static int rseq_get_rseq_cs(struct task_struct *t, struct rseq_cs *rseq_cs)
ret = __get_user(ptr, &t->rseq->rseq_cs);
if (ret)
return ret;
+ ret = check_rseq_cs_padding(t);
+ if (ret)
+ return ret;
if (!ptr) {
memset(rseq_cs, 0, sizeof(*rseq_cs));
return 0;
--
2.11.0
^ permalink raw reply related
* [RFC PATCH for 4.18 1/2] rseq: use __u64 for rseq_cs fields, validate abort_ip < TASK_SIZE
From: Mathieu Desnoyers @ 2018-07-02 20:40 UTC (permalink / raw)
To: Thomas Gleixner
Cc: linux-kernel, linux-api, Peter Zijlstra, Paul E . McKenney,
Boqun Feng, Andy Lutomirski, Dave Watson, Paul Turner,
Andrew Morton, Russell King, Ingo Molnar, H . Peter Anvin,
Andi Kleen, Chris Lameter, Ben Maurer, Steven Rostedt,
Josh Triplett, Linus Torvalds, Catalin Marinas, Will Deacon,
Michael Kerrisk
Change the rseq ABI so rseq_cs start_ip, post_commit_offset and abort_ip
fields are seen as 64-bit fields by both 32-bit and 64-bit kernels rather
that ignoring the 32 upper bits on 32-bit kernels. This ensures we have a
consistent behavior for a 32-bit binary executed on 32-bit kernels and in
compat mode on 64-bit kernels.
Validating the value of abort_ip field to be below TASK_SIZE ensures the
kernel don't return to an invalid address when returning to userspace
after an abort. I don't fully trust each architecture code to consistently
deal with invalid return addresses.
If validation fails, the process is killed with a segmentation fault.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Paul Turner <pjt@google.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Andi Kleen <andi@firstfloor.org>
CC: Dave Watson <davejwatson@fb.com>
CC: Chris Lameter <cl@linux.com>
CC: Ingo Molnar <mingo@redhat.com>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Ben Maurer <bmaurer@fb.com>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Josh Triplett <josh@joshtriplett.org>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: Russell King <linux@arm.linux.org.uk>
CC: Catalin Marinas <catalin.marinas@arm.com>
CC: Will Deacon <will.deacon@arm.com>
CC: Michael Kerrisk <mtk.manpages@gmail.com>
CC: Boqun Feng <boqun.feng@gmail.com>
CC: linux-api@vger.kernel.org
---
include/uapi/linux/rseq.h | 6 +++---
kernel/rseq.c | 5 +++--
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/include/uapi/linux/rseq.h b/include/uapi/linux/rseq.h
index d620fa43756c..519ad6e176d1 100644
--- a/include/uapi/linux/rseq.h
+++ b/include/uapi/linux/rseq.h
@@ -52,10 +52,10 @@ struct rseq_cs {
__u32 version;
/* enum rseq_cs_flags */
__u32 flags;
- LINUX_FIELD_u32_u64(start_ip);
+ __u64 start_ip;
/* Offset from start_ip. */
- LINUX_FIELD_u32_u64(post_commit_offset);
- LINUX_FIELD_u32_u64(abort_ip);
+ __u64 post_commit_offset;
+ __u64 abort_ip;
} __attribute__((aligned(4 * sizeof(__u64))));
/*
diff --git a/kernel/rseq.c b/kernel/rseq.c
index 22b6acf1ad63..2e5d88f09baf 100644
--- a/kernel/rseq.c
+++ b/kernel/rseq.c
@@ -128,7 +128,8 @@ static int rseq_get_rseq_cs(struct task_struct *t, struct rseq_cs *rseq_cs)
return 0;
}
urseq_cs = (struct rseq_cs __user *)ptr;
- if (copy_from_user(rseq_cs, urseq_cs, sizeof(*rseq_cs)))
+ if (copy_from_user(rseq_cs, urseq_cs, sizeof(*rseq_cs)) ||
+ rseq_cs->abort_ip >= TASK_SIZE)
return -EFAULT;
if (rseq_cs->version > 0)
return -EINVAL;
@@ -137,7 +138,7 @@ static int rseq_get_rseq_cs(struct task_struct *t, struct rseq_cs *rseq_cs)
if (rseq_cs->abort_ip - rseq_cs->start_ip < rseq_cs->post_commit_offset)
return -EINVAL;
- usig = (u32 __user *)(rseq_cs->abort_ip - sizeof(u32));
+ usig = (u32 __user *)(unsigned long)(rseq_cs->abort_ip - sizeof(u32));
ret = get_user(sig, usig);
if (ret)
return ret;
--
2.11.0
^ permalink raw reply related
* Re: [RFC PATCH for 4.18 1/2] rseq: validate rseq_cs fields are < TASK_SIZE
From: Linus Torvalds @ 2018-07-02 20:22 UTC (permalink / raw)
To: Andrew Lutomirski
Cc: Mathieu Desnoyers, Thomas Gleixner, Linux Kernel Mailing List,
Linux API, Peter Zijlstra, Paul McKenney, Boqun Feng, Dave Watson,
Paul Turner, Andrew Morton, Russell King - ARM Linux, Ingo Molnar,
Peter Anvin, Andi Kleen, Christoph Lameter, Ben Maurer,
Steven Rostedt, Josh Triplett, Catalin Marinas, Will Deacon
In-Reply-To: <CALCETrWps4Bvd_hGviEZ+60nQFr5ovp2A0+afDWg-QOHB9x7cA@mail.gmail.com>
On Mon, Jul 2, 2018 at 1:13 PM Andy Lutomirski <luto@kernel.org> wrote:
>
> Like this:
>
> diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
> index 3b2490b81918..ec40223c8856 100644
> --- a/arch/x86/entry/common.c
> +++ b/arch/x86/entry/common.c
> @@ -170,6 +170,26 @@ static void exit_to_usermode_loop(struct pt_regs
> *regs, u32 cached_flags)
> if (cached_flags & _TIF_USER_RETURN_NOTIFY)
> fire_user_return_notifiers();
>
> + if (unlikely(!user_64bit_mode(regs) &&
> + (regs->ip & 0xffffffff00000000ull))) {
I'd be afraid that code generation is atrocious. So more something like this:
static noinline send_sigsegv(..) { }
...
if (unlikely(!user_64bit_mode(regs)) {
if (unlikely(*(1+(u32 *)®s->ip)))
send_sigsegv(tsk);
to make sure it doesn't do crazy big constants in the normal path, and
doesn't allocate silly stack frames.
But as mentioned, I'm not entirely convinced this is worth it. But I
wasn't sure it's worth it for rseq.
So basically, *if* we do these kinds of checks, I'd personally rather
do a *generic* "we don't return to garbage 64-bit values in compat
mode" than have special case code that is only for rseq and is truly
irrelevant to all normal cases.
The generic case might even be worth a test-case. And if we do that,
we should probably check that we don't do something odd like
sign-extending the %rip value we load from stack for signal return
etc, that just happens to work because nobody cared about the upper
bits.
So there's a lot of these kinds of small details that are of
questionable importance, but might in theory be worth it.
Linus
^ permalink raw reply
* Re: [RFC PATCH for 4.18 1/2] rseq: validate rseq_cs fields are < TASK_SIZE
From: Andy Lutomirski @ 2018-07-02 20:12 UTC (permalink / raw)
To: Linus Torvalds
Cc: Mathieu Desnoyers, Andrew Lutomirski, Thomas Gleixner,
Linux Kernel Mailing List, Linux API, Peter Zijlstra,
Paul McKenney, Boqun Feng, Dave Watson, Paul Turner,
Andrew Morton, Russell King - ARM Linux, Ingo Molnar, Peter Anvin,
Andi Kleen, Christoph Lameter, Ben Maurer, Steven Rostedt,
Josh Triplett, Catalin Marinas <catalin.marinas@
In-Reply-To: <CA+55aFy2rxnLhbqGtz0wxW_u=x_A1zHZBNP8ZC5P12_AHyb3PA@mail.gmail.com>
On Mon, Jul 2, 2018 at 12:31 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Mon, Jul 2, 2018 at 12:02 PM Andy Lutomirski <luto@amacapital.net> wrote:
>>
>> Works for me. Linus, any objection?
>
> I think the 4.19 stage may be overkill, but I don't hate it, so no
> real objections.
>
> If the main reason for this is that we silently clear the upper bits
> when returning to compat mode, I actually think that a better fix
> would be to just fix that. We shouldn't silently ignore bogus data in
> the return path.
>
> But I don't care enough, I think.
Like this:
diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
index 3b2490b81918..ec40223c8856 100644
--- a/arch/x86/entry/common.c
+++ b/arch/x86/entry/common.c
@@ -170,6 +170,26 @@ static void exit_to_usermode_loop(struct pt_regs
*regs, u32 cached_flags)
if (cached_flags & _TIF_USER_RETURN_NOTIFY)
fire_user_return_notifiers();
+ if (unlikely(!user_64bit_mode(regs) &&
+ (regs->ip & 0xffffffff00000000ull))) {
+ siginfo_t info;
+ struct task_struct *tsk = current;
+
+ /* I haven't thought about this *that* hard. */
+ clear_siginfo(&info);
+ tsk->thread.cr2 = regs->ip;
+ tsk->thread.trap_nr = X86_TRAP_PF;
+ tsk->thread.error_code = X86_PF_USER | X86_PF_INSTR;
+ info.si_signo = SIGSEGV;
+ info.si_errno = 0;
+ info.si_code = SEGV_MAPERR;
+ info.si_addr = (void __user *)regs->ip;
+ /* si_addr_lsb? */
+ force_sig_info(SIGSEGV, &info, tsk);
+
+ /* And we'll go through the loop again. */
+ }
+
/* Disable IRQs and retry */
local_irq_disable();
It's whitespace damaged and barely tested, but it seems to at least
not be completely busted. I don't really love doing this, though.
^ permalink raw reply related
* Re: [RFC PATCH for 4.18 1/2] rseq: validate rseq_cs fields are < TASK_SIZE
From: Linus Torvalds @ 2018-07-02 19:31 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Mathieu Desnoyers, Andrew Lutomirski, Thomas Gleixner,
Linux Kernel Mailing List, Linux API, Peter Zijlstra,
Paul McKenney, Boqun Feng, Dave Watson, Paul Turner,
Andrew Morton, Russell King - ARM Linux, Ingo Molnar, Peter Anvin,
Andi Kleen, Christoph Lameter, Ben Maurer, Steven Rostedt,
Josh Triplett, Catalin Marinas <catalin.marinas@
In-Reply-To: <CALCETrVMhU8SrZBXOXxJerrZpQ71A_f_cRBCU=Lmj_KPV0+rRA@mail.gmail.com>
On Mon, Jul 2, 2018 at 12:02 PM Andy Lutomirski <luto@amacapital.net> wrote:
>
> Works for me. Linus, any objection?
I think the 4.19 stage may be overkill, but I don't hate it, so no
real objections.
If the main reason for this is that we silently clear the upper bits
when returning to compat mode, I actually think that a better fix
would be to just fix that. We shouldn't silently ignore bogus data in
the return path.
But I don't care enough, I think.
Linus
^ permalink raw reply
* Re: [RFC PATCH for 4.18 1/2] rseq: validate rseq_cs fields are < TASK_SIZE
From: Andy Lutomirski @ 2018-07-02 19:02 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Linus Torvalds, Andy Lutomirski, Thomas Gleixner, linux-kernel,
linux-api, Peter Zijlstra, Paul E. McKenney, Boqun Feng,
Dave Watson, Paul Turner, Andrew Morton, Russell King,
Ingo Molnar, H. Peter Anvin, Andi Kleen, Chris Lameter,
Ben Maurer, rostedt, Josh Triplett, Catalin Marinas
In-Reply-To: <84003252.10754.1530558015813.JavaMail.zimbra@efficios.com>
On Mon, Jul 2, 2018 at 12:00 PM, Mathieu Desnoyers
<mathieu.desnoyers@efficios.com> wrote:
> ----- On Jul 2, 2018, at 1:11 PM, Andy Lutomirski luto@amacapital.net wrote:
>>
>> But I think that the limited solution of changing
>> instruction_pointer_set() really is a sufficient
>> architecture-dependent change to fully solve your problem.
>
> So let me recap with the changes I gather for 4.18 and 4.19:
>
> 4.18:
>
> * Change struct rseq_cs field types from LINUX_FIELD_u32_u64() to __u64 in
> uapi/linux/rseq.h,
> * Compare rseq->rseq_cs->abort_ip with TASK_SIZE before using it. Kill offending
> process if its value is over TASK_SIZE,
> * Explicitly check that padding of rseq->rseq_cs is zero on 32-bit kernels
> (#ifndef __LP64__).
>
> 4.19:
>
> * Introduce instruction_pointer_set() with input validation, use it when setting
> IP to abort_ip in rseq. This replaces the comparison of abort_ip with TASK_SIZE.
>
> Is that consistent with what you have in mind ?
>
Works for me. Linus, any objection?
^ permalink raw reply
* Re: [RFC PATCH for 4.18 1/2] rseq: validate rseq_cs fields are < TASK_SIZE
From: Mathieu Desnoyers @ 2018-07-02 19:00 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Linus Torvalds, Andy Lutomirski, Thomas Gleixner, linux-kernel,
linux-api, Peter Zijlstra, Paul E. McKenney, Boqun Feng,
Dave Watson, Paul Turner, Andrew Morton, Russell King,
Ingo Molnar, H. Peter Anvin, Andi Kleen, Chris Lameter,
Ben Maurer, rostedt, Josh Triplett, Catalin Marinas
In-Reply-To: <CALCETrVeu_V-3K8OfKv9QixrWBJs+q866yb=e96sC76SS4hxYg@mail.gmail.com>
----- On Jul 2, 2018, at 1:11 PM, Andy Lutomirski luto@amacapital.net wrote:
>
> But I think that the limited solution of changing
> instruction_pointer_set() really is a sufficient
> architecture-dependent change to fully solve your problem.
So let me recap with the changes I gather for 4.18 and 4.19:
4.18:
* Change struct rseq_cs field types from LINUX_FIELD_u32_u64() to __u64 in
uapi/linux/rseq.h,
* Compare rseq->rseq_cs->abort_ip with TASK_SIZE before using it. Kill offending
process if its value is over TASK_SIZE,
* Explicitly check that padding of rseq->rseq_cs is zero on 32-bit kernels
(#ifndef __LP64__).
4.19:
* Introduce instruction_pointer_set() with input validation, use it when setting
IP to abort_ip in rseq. This replaces the comparison of abort_ip with TASK_SIZE.
Is that consistent with what you have in mind ?
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [RFC PATCH for 4.18 1/2] rseq: validate rseq_cs fields are < TASK_SIZE
From: Andy Lutomirski @ 2018-07-02 17:11 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Linus Torvalds, Andrew Lutomirski, Thomas Gleixner, LKML,
Linux API, Peter Zijlstra, Paul E. McKenney, Boqun Feng,
Dave Watson, Paul Turner, Andrew Morton, Russell King,
Ingo Molnar, H. Peter Anvin, Andi Kleen, Christoph Lameter,
Ben Maurer, Steven Rostedt, Josh Triplett, Catalin Marinas, Will
In-Reply-To: <1527399163.10673.1530541966296.JavaMail.zimbra@efficios.com>
On Mon, Jul 2, 2018 at 7:32 AM Mathieu Desnoyers
<mathieu.desnoyers@efficios.com> wrote:
>
> ----- On Jun 29, 2018, at 4:39 PM, Andy Lutomirski luto@amacapital.net wrote:
>
> > On Fri, Jun 29, 2018 at 12:48 PM, Mathieu Desnoyers
> > <mathieu.desnoyers@efficios.com> wrote:
> >> There are two aspects I'm concerned about here:
> >>
> >> 1) security: we don't want 32-bit user-space to feed a 64-bit value over 4GB
> >> as abort_ip that may end up causing OOPSes on architectures that would
> >> lack proper validation of those values on return to userspace.
> >
> > I'm not too worried about this. As long as you're doing it from
> > signal-delivery context (which you are AFAICT) you're fine.
>
> No, it's not just signal-delivery context. It's _also_ called from
> return to usermode loop, which can by called on return from
> interrupt/trap/syscall.
>
TIF_NOTIFY_RESUME context in the exit slowpath is fine, too.
> >
> > But I re-read the code and I think I have a really straightforward
> > solution. Two choices:
> >
> > (1) Change instruction_pointer_set() to return an error code if the
> > address passed in is garbage in a way that could cause unexpected
> > behavior (like >=2^32 on x86_64 if regs->cs is 32-bit). It has very
> > very few callers.
>
> This would take care of my security concern wrt abort_ip, but would not
> provide consistent behavior for the other fields. Also, perhaps this
> kind of change should aim the next merge window ?
It's not about security. The idea is that instruction_pointer_set()
should return some indication of whether it actually set the
instruction pointer to the requested value. On x86, if you have
!user_64bit_mode(regs) and you call instruction_pointer_set() to set
ip to 0xbaadc0de12345678, then you end up with a state where we will
probably execute user code at the address 0x12345678. Conversely, if
you have user_64bit_mode(regs) == true and you set ip to
0xbaadc0de12345678, then you will end up sending a signal to the task
because 0xbaadc0de12345678 is not executable (and, in fact, is highly
likely to be noncanonical).
So I would argue that the semantics *should* be:
/*
* Attempts to modify @regs such that the next user instruction to be
executed is
* the instruction at @addr. instruction_pointer_set() may return
false to indicate
* that addr was invalid in the sense that the next user instruction executed
* might be some other address instead. The most likely cause is that
* regs refers to a 32-bit compat context, addr != (u32)addr, and the
architecture
* might silently truncate the address on the next return to user code.
*
* instruction_pointer_set() must only be called from a context in
which the architecture
* allows arbitrary modifications of @regs.
*
* Architecture implementations promise that calling
instruction_pointer_set() will not
* crash or otherwise corrupt the kernel when called from a valid
context, regardless
* of what value is passed in @addr.
*/
bool instruction_pointer_set(struct pt_regs *regs, unsigned long addr);
>
> >
> > (2) Add instruction_pointer_validate() to go along with
> > instruction_pointer_set().
> >
> > That should be enough to solve the problem, right?
>
> This would only handle the "security" part of the matter, which
> is specifically related to rseq->rseq_cs->abort_ip.
>
> What is left is ensuring that we have consistent behavior for
> other fields:
>
> [ Note: we have introduced this helper macro: LINUX_FIELD_u32_u64
> which defines a field which is 64-bit for 64-bit processes, and 32-bit
> with 32-bit of padding for 32-bit processes. ]
>
> * rseq->rseq_cs: (userspace pointer to user-space, updated by user-space
> with single-copy atomicity): current type: LINUX_FIELD_u32_u64,
> cannot be changed to __u64 due to single-copy atomicity requirement,
>
> * rseq->rseq_cs->start_ip: currently a LINUX_FIELD_u32_u64,
> could become a __u64,
>
> * rseq->rseq_cs->post_commit_ip: currently a LINUX_FIELD_u32_u64,
> could become a __u64,
>
> * rseq->rseq_cs->abort_ip: currently a LINUX_FIELD_u32_u64,
> could become a __u64,
>
> For abort_ip, changing the type to __u64 and using the
> instruction_pointer_validate() approach you propose would work.
>
> For start_ip and post_commit_ip, we need to decide whether we
> want to kill a 32-bit process setting the high bits or if we just
> accept and use the full __u64 content on both 32-bit and 64-bit
> kernels. Those two fields are only used for arithmetic comparison.
> Using the full __u64 content means using 64-bit arithmetic on
> 32-bit native kernels though.
Just use the 64-bit values, I think. I see no point in killing the task.
>
> For rseq->rseq_cs, we cannot use __u64 due to single-copy atomicity
> update requirement for 32-bit processes. However, we are using this
> field in a copy_from_user(), so it will EFAULT if the high-bits are
> set by a compat 32-bit task on a 64-bit kernel. We can therefore check
> that the padding is zeroed explicitly on a native 32-bit kernel to
> provide a consistent behavior. Specifically because rseq->rseq_cs is
> checked with access_ok(), it is therefore enough to check the padding
> when __LP64__ is not defined by the preprocessor.
Agreed.
>
> But rather than trying to play games with input validation, I would
> favor an approach that would allow rseq to validate all its inputs
> straightforwardly. Introducing user_64bit_mode(struct pt_regs *)
> across all architectures would allow doing just that.
I would be okay with that, too, but I think it would have to be
user_64bit_mode(task, regs), since
sane architectures would have the task bitness somewhere other than in
regs. x86 is IMO rather
weird in this regard. When I added user_64bit_mode(), I didn't
envision its use outside x86 arch code.
> AFAIU this could be achieved by re-introducing is_compat_task() on x86 as:
>
> #ifdef CONFIG_COMPAT
> static bool is_compat_task(void)
> {
> return user_64bit_mode(current_pt_regs()));
> }
> #else
> static bool is_compat_task(void) { return false; };
> #endif
>
> Or am I missing something ?
is_compat_task() historically literally meant "am I in a compat system
call". It never worked consistently on x86 outside of syscall
context. While I do have fundamental objections to having a generic
concept of "is this a compat task?" on Linux, that's not why I removed
is_compat_task(). I removed it because it didn't do what the name
suggested.
Unfortunately, while it's gone from generic code, it's still there on
non-x86 arches, and it probably still has inconsistent semantics. So
I don't want to re-add it.
But I think that the limited solution of changing
instruction_pointer_set() really is a sufficient
architecture-dependent change to fully solve your problem.
^ permalink raw reply
* Re: [PATCH v2 5/7] mm: rename and change semantics of nr_indirectly_reclaimable_bytes
From: Roman Gushchin @ 2018-07-02 16:52 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Andrew Morton, linux-mm, linux-kernel, Michal Hocko,
Johannes Weiner, linux-api, Christoph Lameter, David Rientjes,
Mel Gorman, Matthew Wilcox, Vijayanand Jitta, Laura Abbott,
Sumit Semwal
In-Reply-To: <ef2dea13-0102-c4bc-a28f-c1b2408f0753@suse.cz>
On Sat, Jun 30, 2018 at 12:09:27PM +0200, Vlastimil Babka wrote:
> On 06/29/2018 11:12 PM, Roman Gushchin wrote:
> >>
> >> The vmstat counter NR_INDIRECTLY_RECLAIMABLE_BYTES was introduced by commit
> >> eb59254608bc ("mm: introduce NR_INDIRECTLY_RECLAIMABLE_BYTES") with the goal of
> >> accounting objects that can be reclaimed, but cannot be allocated via a
> >> SLAB_RECLAIM_ACCOUNT cache. This is now possible via kmalloc() with
> >> __GFP_RECLAIMABLE flag, and the dcache external names user is converted.
> >>
> >> The counter is however still useful for accounting direct page allocations
> >> (i.e. not slab) with a shrinker, such as the ION page pool. So keep it, and:
> >
> > Btw, it looks like I've another example of usefulness of this counter:
> > dynamic per-cpu data.
>
> Hmm, but are those reclaimable? Most likely not in general? Do you have
> examples that are?
If these per-cpu data is something like per-cpu refcounters,
which are using to manage reclaimable objects (e.g. cgroup css objects).
Of course, they are not always reclaimable, but in certain states.
Thanks!
^ permalink raw reply
* Re: [RFC PATCH for 4.18 1/2] rseq: validate rseq_cs fields are < TASK_SIZE
From: Mathieu Desnoyers @ 2018-07-02 16:04 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Linus Torvalds, Andy Lutomirski, Thomas Gleixner, linux-kernel,
linux-api, Peter Zijlstra, Paul E. McKenney, Boqun Feng,
Dave Watson, Paul Turner, Andrew Morton, Russell King,
Ingo Molnar, H. Peter Anvin, Andi Kleen, Chris Lameter,
Ben Maurer, rostedt, Josh Triplett, Catalin Marinas
In-Reply-To: <1527399163.10673.1530541966296.JavaMail.zimbra@efficios.com>
----- On Jul 2, 2018, at 10:32 AM, Mathieu Desnoyers mathieu.desnoyers@efficios.com wrote:
[...]
>
> But rather than trying to play games with input validation, I would
> favor an approach that would allow rseq to validate all its inputs
> straightforwardly. Introducing user_64bit_mode(struct pt_regs *)
> across all architectures would allow doing just that. rseq signal
> delivery and return to usermode code could then ensure that high bits are
> cleared by 32-bit tasks for all fields and thus provide a consistent
> behavior for 32-bit tasks running on 32-bit and 64-bit kernels.
AFAIU this could be achieved by re-introducing is_compat_task() on x86 as:
#ifdef CONFIG_COMPAT
static bool is_compat_task(void)
{
return user_64bit_mode(current_pt_regs()));
}
#else
static bool is_compat_task(void) { return false; };
#endif
Or am I missing something ?
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [RFC PATCH for 4.18 1/2] rseq: validate rseq_cs fields are < TASK_SIZE
From: Mathieu Desnoyers @ 2018-07-02 14:32 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Linus Torvalds, Andy Lutomirski, Thomas Gleixner, linux-kernel,
linux-api, Peter Zijlstra, Paul E. McKenney, Boqun Feng,
Dave Watson, Paul Turner, Andrew Morton, Russell King,
Ingo Molnar, H. Peter Anvin, Andi Kleen, Chris Lameter,
Ben Maurer, rostedt, Josh Triplett, Catalin Marinas
In-Reply-To: <CALCETrXqjrCoHX1KyL0iKGBaAZSGJb1cVdNN1gNigJ8DW9LF8w@mail.gmail.com>
----- On Jun 29, 2018, at 4:39 PM, Andy Lutomirski luto@amacapital.net wrote:
> On Fri, Jun 29, 2018 at 12:48 PM, Mathieu Desnoyers
> <mathieu.desnoyers@efficios.com> wrote:
>> There are two aspects I'm concerned about here:
>>
>> 1) security: we don't want 32-bit user-space to feed a 64-bit value over 4GB
>> as abort_ip that may end up causing OOPSes on architectures that would
>> lack proper validation of those values on return to userspace.
>
> I'm not too worried about this. As long as you're doing it from
> signal-delivery context (which you are AFAICT) you're fine.
No, it's not just signal-delivery context. It's _also_ called from
return to usermode loop, which can by called on return from
interrupt/trap/syscall.
>
> But I re-read the code and I think I have a really straightforward
> solution. Two choices:
>
> (1) Change instruction_pointer_set() to return an error code if the
> address passed in is garbage in a way that could cause unexpected
> behavior (like >=2^32 on x86_64 if regs->cs is 32-bit). It has very
> very few callers.
This would take care of my security concern wrt abort_ip, but would not
provide consistent behavior for the other fields. Also, perhaps this
kind of change should aim the next merge window ?
>
> (2) Add instruction_pointer_validate() to go along with
> instruction_pointer_set().
>
> That should be enough to solve the problem, right?
This would only handle the "security" part of the matter, which
is specifically related to rseq->rseq_cs->abort_ip.
What is left is ensuring that we have consistent behavior for
other fields:
[ Note: we have introduced this helper macro: LINUX_FIELD_u32_u64
which defines a field which is 64-bit for 64-bit processes, and 32-bit
with 32-bit of padding for 32-bit processes. ]
* rseq->rseq_cs: (userspace pointer to user-space, updated by user-space
with single-copy atomicity): current type: LINUX_FIELD_u32_u64,
cannot be changed to __u64 due to single-copy atomicity requirement,
* rseq->rseq_cs->start_ip: currently a LINUX_FIELD_u32_u64,
could become a __u64,
* rseq->rseq_cs->post_commit_ip: currently a LINUX_FIELD_u32_u64,
could become a __u64,
* rseq->rseq_cs->abort_ip: currently a LINUX_FIELD_u32_u64,
could become a __u64,
For abort_ip, changing the type to __u64 and using the
instruction_pointer_validate() approach you propose would work.
For start_ip and post_commit_ip, we need to decide whether we
want to kill a 32-bit process setting the high bits or if we just
accept and use the full __u64 content on both 32-bit and 64-bit
kernels. Those two fields are only used for arithmetic comparison.
Using the full __u64 content means using 64-bit arithmetic on
32-bit native kernels though.
If we decide to kill the offending process (whether the field type is
__u64 or LINUX_FIELD_u32_u64), we need to be able to figure out if
the process is a compat task when called from signal delivery and from
return to usermode loop (return from irq/trap/syscall).
Comparison with TASK_SIZE solves the issue for abort_ip, post_commit_ip
and start_ip, although nobody seems to like that approach very much.
For rseq->rseq_cs, we cannot use __u64 due to single-copy atomicity
update requirement for 32-bit processes. However, we are using this
field in a copy_from_user(), so it will EFAULT if the high-bits are
set by a compat 32-bit task on a 64-bit kernel. We can therefore check
that the padding is zeroed explicitly on a native 32-bit kernel to
provide a consistent behavior. Specifically because rseq->rseq_cs is
checked with access_ok(), it is therefore enough to check the padding
when __LP64__ is not defined by the preprocessor.
But rather than trying to play games with input validation, I would
favor an approach that would allow rseq to validate all its inputs
straightforwardly. Introducing user_64bit_mode(struct pt_regs *)
across all architectures would allow doing just that. rseq signal
delivery and return to usermode code could then ensure that high bits are
cleared by 32-bit tasks for all fields and thus provide a consistent
behavior for 32-bit tasks running on 32-bit and 64-bit kernels.
Thoughts ?
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [PATCH v7 00/29] FPGA Device Feature List (DFL) Device Drivers
From: Alan Tull @ 2018-07-02 13:09 UTC (permalink / raw)
To: Wu Hao
Cc: Moritz Fischer, linux-fpga, linux-kernel, linux-api, Kang, Luwei,
Zhang, Yi Z
In-Reply-To: <1530320016-24712-1-git-send-email-hao.wu@intel.com>
On Fri, Jun 29, 2018 at 7:53 PM, Wu Hao <hao.wu@intel.com> wrote:
> Hi All,
>
> Here is v7 patch-series adding drivers for FPGA DFL devices.
I've pushed this to a branch on my linux-fpga kernel.org git repo for
robots and humans to look at. Branch name is
for-review-next-20180702-intel-fpga-v7
Alan
^ permalink raw reply
* Re: [PATCH] Add renameat2 function [BZ #17662]
From: Florian Weimer @ 2018-07-02 9:32 UTC (permalink / raw)
To: Yury Norov
Cc: libc-alpha, Alexander Viro, linux-kernel, linux-fsdevel,
linux-api
In-Reply-To: <20180702084622.GA15274@yury-thinkpad>
On 07/02/2018 10:46 AM, Yury Norov wrote:
> Is my understanding correct that glibc community finds <linux/fs.h>
> inappropriate for their use, and prefer to re-introduce (duplicate)
> its functionality locally? I think it's wrong. The right way to go
> is to make kernel headers comfortable for users instead of ignoring
> it.
In some cases, we already use UAPI headers (<linux/falloc.h> is an
example), but it is not always possible.
> Are you OK to switch to kernel RENAME_* definitions if they will be
> located in separated small file? Like in the patch below.
>
> Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>
> ---
> include/uapi/linux/fs.h | 4 +---
> include/uapi/linux/rename.h | 12 ++++++++++++
> 2 files changed, 13 insertions(+), 3 deletions(-)
> create mode 100644 include/uapi/linux/rename.h
>
> diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
> index c27576d471c2..46c03ea31a76 100644
> --- a/include/uapi/linux/fs.h
> +++ b/include/uapi/linux/fs.h
> @@ -44,9 +44,7 @@
> #define SEEK_HOLE 4 /* seek to the next hole */
> #define SEEK_MAX SEEK_HOLE
>
> -#define RENAME_NOREPLACE (1 << 0) /* Don't overwrite target */
> -#define RENAME_EXCHANGE (1 << 1) /* Exchange source and dest */
> -#define RENAME_WHITEOUT (1 << 2) /* Whiteout source */
> +#include <linux/rename.h>
>
> struct file_clone_range {
> __s64 src_fd;
> diff --git a/include/uapi/linux/rename.h b/include/uapi/linux/rename.h
> new file mode 100644
> index 000000000000..7178f0565657
> --- /dev/null
> +++ b/include/uapi/linux/rename.h
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +#ifndef _UAPI_LINUX_RENAME_H
> +#define _UAPI_LINUX_RENAME_H
> +
> +/*
> + * Definitions for rename syscall family.
> + */
> +#define RENAME_NOREPLACE (1 << 0) /* Don't overwrite target */
> +#define RENAME_EXCHANGE (1 << 1) /* Exchange source and dest */
> +#define RENAME_WHITEOUT (1 << 2) /* Whiteout source */
> +
> +#endif /* _UAPI_LINUX_RENAME_H */
This would help.
We would need to provide definitions for compatibility with older kernel
headers locally, but on newer kernels, we could use the UAPI header file.
Thanks,
Florian
^ permalink raw reply
* Re: [PATCH] Add renameat2 function [BZ #17662]
From: Yury Norov @ 2018-07-02 8:46 UTC (permalink / raw)
To: Florian Weimer
Cc: libc-alpha, Alexander Viro, linux-kernel, linux-fsdevel,
linux-api
In-Reply-To: <60505ccf-a399-6320-74f5-e2e17965d25c@redhat.com>
+ Alexander Viro <viro@zeniv.linux.org.uk>, kernel maillists.
On Mon, Jul 02, 2018 at 08:48:36AM +0200, Florian Weimer wrote:
> On 07/01/2018 11:49 PM, Yury Norov wrote:
>
> > > +#ifdef __USE_GNU
> > > +/* Flags for renameat. */
> >
> > Flags for renameat2, right?
>
> Thanks, fixed.
>
> > > +# define RENAME_NOREPLACE (1 << 0)
> > > +# define RENAME_EXCHANGE (1 << 1)
> > > +# define RENAME_WHITEOUT (1 << 2)
> >
> > I really don't understand how it works. Could you / somebody explain me?
> >
> > include/uapi/linux/fs.h in kernel sources already defines this flags,
> > and this file is usually available in Linux distribution. So I don't
> > understand what for it is duplicated here. If you keep in mind
> > old linux headers or non-linux systems, I think it should be protected
> > with #ifndef guards.
>
> <linux/fs.h> undefines and defines macros not mentioned in the standards
> (and it even contains a few unrelated structs), so we cannot include it
> without _GNU_SOURCE.
>
> It might be possible to include it only for _GNU_SOURCE, but there are a
> lot of things in <linux/fs.h>, so that does not seem to be particularly
> advisable.
>
> We still support building glibc with 3.2 kernel headers, and if the
> definitions you quoted above are not available, building the test case
> would fail.
Is my understanding correct that glibc community finds <linux/fs.h>
inappropriate for their use, and prefer to re-introduce (duplicate)
its functionality locally? I think it's wrong. The right way to go
is to make kernel headers comfortable for users instead of ignoring
it.
Are you OK to switch to kernel RENAME_* definitions if they will be
located in separated small file? Like in the patch below.
Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>
---
include/uapi/linux/fs.h | 4 +---
include/uapi/linux/rename.h | 12 ++++++++++++
2 files changed, 13 insertions(+), 3 deletions(-)
create mode 100644 include/uapi/linux/rename.h
diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
index c27576d471c2..46c03ea31a76 100644
--- a/include/uapi/linux/fs.h
+++ b/include/uapi/linux/fs.h
@@ -44,9 +44,7 @@
#define SEEK_HOLE 4 /* seek to the next hole */
#define SEEK_MAX SEEK_HOLE
-#define RENAME_NOREPLACE (1 << 0) /* Don't overwrite target */
-#define RENAME_EXCHANGE (1 << 1) /* Exchange source and dest */
-#define RENAME_WHITEOUT (1 << 2) /* Whiteout source */
+#include <linux/rename.h>
struct file_clone_range {
__s64 src_fd;
diff --git a/include/uapi/linux/rename.h b/include/uapi/linux/rename.h
new file mode 100644
index 000000000000..7178f0565657
--- /dev/null
+++ b/include/uapi/linux/rename.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _UAPI_LINUX_RENAME_H
+#define _UAPI_LINUX_RENAME_H
+
+/*
+ * Definitions for rename syscall family.
+ */
+#define RENAME_NOREPLACE (1 << 0) /* Don't overwrite target */
+#define RENAME_EXCHANGE (1 << 1) /* Exchange source and dest */
+#define RENAME_WHITEOUT (1 << 2) /* Whiteout source */
+
+#endif /* _UAPI_LINUX_RENAME_H */
--
2.17.1
^ permalink raw reply related
* [REGRESSION] "Locked" and "Pss" in /proc/*/smaps are the same
From: Thomas Lindroth @ 2018-07-01 18:31 UTC (permalink / raw)
To: dancol; +Cc: linux-api, linux-kernel
While looking around in /proc on my v4.14.52 system I noticed that
all processes got a lot of "Locked" memory in /proc/*/smaps. A lot
more memory than a regular user can usually lock with mlock().
commit 493b0e9d945fa9dfe96be93ae41b4ca4b6fdb317 (v4.14-rc1) seems
to have changed the behavior of "Locked".
commit 493b0e9d945fa9dfe96be93ae41b4ca4b6fdb317
Author: Daniel Colascione <dancol@google.com>
Date: Wed Sep 6 16:25:08 2017 -0700
mm: add /proc/pid/smaps_rollup
Before that commit the code was like this. Notice the VM_LOCKED
check.
seq_printf(m,
"Size: %8lu kB\n"
"Rss: %8lu kB\n"
"Pss: %8lu kB\n"
"Shared_Clean: %8lu kB\n"
"Shared_Dirty: %8lu kB\n"
"Private_Clean: %8lu kB\n"
"Private_Dirty: %8lu kB\n"
"Referenced: %8lu kB\n"
"Anonymous: %8lu kB\n"
"LazyFree: %8lu kB\n"
"AnonHugePages: %8lu kB\n"
"ShmemPmdMapped: %8lu kB\n"
"Shared_Hugetlb: %8lu kB\n"
"Private_Hugetlb: %7lu kB\n"
"Swap: %8lu kB\n"
"SwapPss: %8lu kB\n"
"KernelPageSize: %8lu kB\n"
"MMUPageSize: %8lu kB\n"
"Locked: %8lu kB\n",
(vma->vm_end - vma->vm_start) >> 10,
mss.resident >> 10,
(unsigned long)(mss.pss >> (10 + PSS_SHIFT)),
mss.shared_clean >> 10,
mss.shared_dirty >> 10,
mss.private_clean >> 10,
mss.private_dirty >> 10,
mss.referenced >> 10,
mss.anonymous >> 10,
mss.lazyfree >> 10,
mss.anonymous_thp >> 10,
mss.shmem_thp >> 10,
mss.shared_hugetlb >> 10,
mss.private_hugetlb >> 10,
mss.swap >> 10,
(unsigned long)(mss.swap_pss >> (10 + PSS_SHIFT)),
vma_kernel_pagesize(vma) >> 10,
vma_mmu_pagesize(vma) >> 10,
(vma->vm_flags & VM_LOCKED) ?
(unsigned long)(mss.pss >> (10 + PSS_SHIFT)) : 0);
After that commit Locked is now the same as Pss. This looks like a
mistake.
seq_printf(m,
"Rss: %8lu kB\n"
"Pss: %8lu kB\n"
"Shared_Clean: %8lu kB\n"
"Shared_Dirty: %8lu kB\n"
"Private_Clean: %8lu kB\n"
"Private_Dirty: %8lu kB\n"
"Referenced: %8lu kB\n"
"Anonymous: %8lu kB\n"
"LazyFree: %8lu kB\n"
"AnonHugePages: %8lu kB\n"
"ShmemPmdMapped: %8lu kB\n"
"Shared_Hugetlb: %8lu kB\n"
"Private_Hugetlb: %7lu kB\n"
"Swap: %8lu kB\n"
"SwapPss: %8lu kB\n"
"Locked: %8lu kB\n",
mss->resident >> 10,
(unsigned long)(mss->pss >> (10 + PSS_SHIFT)),
mss->shared_clean >> 10,
mss->shared_dirty >> 10,
mss->private_clean >> 10,
mss->private_dirty >> 10,
mss->referenced >> 10,
mss->anonymous >> 10,
mss->lazyfree >> 10,
mss->anonymous_thp >> 10,
mss->shmem_thp >> 10,
mss->shared_hugetlb >> 10,
mss->private_hugetlb >> 10,
mss->swap >> 10,
(unsigned long)(mss->swap_pss >> (10 + PSS_SHIFT)),
(unsigned long)(mss->pss >> (10 + PSS_SHIFT)));
The latest git has changed a bit but the functionality is the
same.
^ permalink raw reply
* Re: [PATCH v2 5/7] mm: rename and change semantics of nr_indirectly_reclaimable_bytes
From: Vlastimil Babka @ 2018-06-30 10:09 UTC (permalink / raw)
To: Roman Gushchin
Cc: Andrew Morton, linux-mm, linux-kernel, Michal Hocko,
Johannes Weiner, linux-api, Christoph Lameter, David Rientjes,
Mel Gorman, Matthew Wilcox, Vijayanand Jitta, Laura Abbott,
Sumit Semwal
In-Reply-To: <20180629211201.GA14897@castle.DHCP.thefacebook.com>
On 06/29/2018 11:12 PM, Roman Gushchin wrote:
>>
>> The vmstat counter NR_INDIRECTLY_RECLAIMABLE_BYTES was introduced by commit
>> eb59254608bc ("mm: introduce NR_INDIRECTLY_RECLAIMABLE_BYTES") with the goal of
>> accounting objects that can be reclaimed, but cannot be allocated via a
>> SLAB_RECLAIM_ACCOUNT cache. This is now possible via kmalloc() with
>> __GFP_RECLAIMABLE flag, and the dcache external names user is converted.
>>
>> The counter is however still useful for accounting direct page allocations
>> (i.e. not slab) with a shrinker, such as the ION page pool. So keep it, and:
>
> Btw, it looks like I've another example of usefulness of this counter:
> dynamic per-cpu data.
Hmm, but are those reclaimable? Most likely not in general? Do you have
examples that are?
^ permalink raw reply
* [PATCH v7 29/29] MAINTAINERS: add entry for FPGA DFL drivers
From: Wu Hao @ 2018-06-30 0:53 UTC (permalink / raw)
To: atull, mdf, linux-fpga, linux-kernel
Cc: linux-api, luwei.kang, yi.z.zhang, hao.wu
In-Reply-To: <1530320016-24712-1-git-send-email-hao.wu@intel.com>
Add entry for FPGA Device Feature List (DFL) drivers.
Signed-off-by: Wu Hao <hao.wu@intel.com>
Acked-by: Alan Tull <atull@kernel.org>
Acked-by: Moritz Fischer <mdf@kernel.org>
---
v7: add Acked-by from Alan and Moritz
---
MAINTAINERS | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 07d1576..b9bb042 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5638,6 +5638,14 @@ F: drivers/fpga/
F: include/linux/fpga/
W: http://www.rocketboards.org
+FPGA DFL DRIVERS
+M: Wu Hao <hao.wu@intel.com>
+L: linux-fpga@vger.kernel.org
+S: Maintained
+F: Documentation/fpga/dfl.txt
+F: include/uapi/linux/fpga-dfl.h
+F: drivers/fpga/dfl*
+
FPU EMULATOR
M: Bill Metzenthen <billm@melbpc.org.au>
W: http://floatingpoint.sourceforge.net/emulator/index.html
--
1.8.3.1
^ permalink raw reply related
* [PATCH v7 28/29] fpga: dfl: afu: add DFL_FPGA_PORT_DMA_MAP/UNMAP ioctls support
From: Wu Hao @ 2018-06-30 0:53 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: <1530320016-24712-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 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>
Acked-by: Alan Tull <atull@kernel.org>
---
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.
v6: rebase, fix copyright time and add Acked-by from Alan.
v7: fix typos.
---
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 a44d50d..7a2d73b 100644
--- a/drivers/fpga/Makefile
+++ b/drivers/fpga/Makefile
@@ -38,7 +38,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..0e81d33
--- /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-2018 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(¤t->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(¤t->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(®ion->node, parent, new);
+ rb_insert_color(®ion->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(®ion->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 f67a78d..02baa6a 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 11ce2cf..0c7630a 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 linked 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 a3ccdfb..2e324e5 100644
--- a/include/uapi/linux/fpga-dfl.h
+++ b/include/uapi/linux/fpga-dfl.h
@@ -114,6 +114,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 v7 27/29] fpga: dfl: afu: add afu sub feature support
From: Wu Hao @ 2018-06-30 0:53 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: <1530320016-24712-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>
Acked-by: Alan Tull <atull@kernel.org>
---
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.
v6: rebase, fix copyright time and add Acked-by Alan.
v7: update kernelversion in sysfs doc and fix typos.
---
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 cb91165..6a92dda 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: June 2018
+KernelVersion: 4.19
+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 1ac7749..a44d50d 100644
--- a/drivers/fpga/Makefile
+++ b/drivers/fpga/Makefile
@@ -38,7 +38,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 4074b97..f67a78d 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, ®ion);
+ 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, ®ion);
+ 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_dev_ops_register(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_dev_ops_unregister(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..0804b7a
--- /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-2018 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(®ion->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 linked 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..11ce2cf
--- /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-2018 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 linked 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 e6b4dd2..a3ccdfb 100644
--- a/include/uapi/linux/fpga-dfl.h
+++ b/include/uapi/linux/fpga-dfl.h
@@ -66,6 +66,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 v7 26/29] fpga: dfl: afu: add DFL_FPGA_GET_API_VERSION/CHECK_EXTENSION ioctls support
From: Wu Hao @ 2018-06-30 0:53 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: <1530320016-24712-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.
v6: rebase.
---
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 d36b3e9..4074b97 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 v7 25/29] fpga: dfl: afu: add header sub feature support
From: Wu Hao @ 2018-06-30 0:53 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: <1530320016-24712-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.
v6: rebase.
v7: update kernelversion in sysfs doc and fix typos.
---
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..cb91165
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-platform-dfl-port
@@ -0,0 +1,7 @@
+What: /sys/bus/platform/devices/dfl-port.0/id
+Date: June 2018
+KernelVersion: 4.19
+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 a38d6a8..d36b3e9 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 9666af8..e6b4dd2 100644
--- a/include/uapi/linux/fpga-dfl.h
+++ b/include/uapi/linux/fpga-dfl.h
@@ -29,8 +29,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)
*
@@ -49,6 +52,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
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox