[PATCH v5 02/16] mm/gup: Fix __get_user_pages() on fault retry of hugetlb
When follow_hugetlb_page() returns with *locked==0, it means we've got
a VM_FAULT_RETRY within the fauling process and we've released the
mmap_sem. When that happens, we should stop and bail out.
Signed-off-by: Peter Xu
---
mm/gup.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/mm/gup.c b/mm/gup.c
index eddbb95dcb8f..e60d32f1674d 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -833,6 +833,16 @@ static long __get_user_pages(struct task_struct *tsk,
struct mm_struct *mm,
i = follow_hugetlb_page(mm, vma, pages, vmas,
&start, &nr_pages, i,
gup_flags, locked);
+ if (locked && *locked == 0) {
+ /*
+* We've got a VM_FAULT_RETRY
+* and we've lost mmap_sem.
+* We must stop here.
+*/
+ BUG_ON(gup_flags & FOLL_NOWAIT);
+ BUG_ON(ret != 0);
+ goto out;
+ }
continue;
}
}
--
2.21.0
[PATCH v5 01/16] mm/gup: Rename "nonblocking" to "locked" where proper
There's plenty of places around __get_user_pages() that has a parameter
"nonblocking" which does not really mean that "it won't block" (because
it can really block) but instead it shows whether the mmap_sem is
released by up_read() during the page fault handling mostly when
VM_FAULT_RETRY is returned.
We have the correct naming in e.g. get_user_pages_locked() or
get_user_pages_remote() as "locked", however there're still many places
that are using the "nonblocking" as name.
Renaming the places to "locked" where proper to better suite the
functionality of the variable. While at it, fixing up some of the
comments accordingly.
Reviewed-by: Mike Rapoport
Reviewed-by: Jerome Glisse
Reviewed-by: David Hildenbrand
Signed-off-by: Peter Xu
---
mm/gup.c | 44 +---
mm/hugetlb.c | 8
2 files changed, 25 insertions(+), 27 deletions(-)
diff --git a/mm/gup.c b/mm/gup.c
index 98f13ab37bac..eddbb95dcb8f 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -622,12 +622,12 @@ static int get_gate_page(struct mm_struct *mm, unsigned
long address,
}
/*
- * mmap_sem must be held on entry. If @nonblocking != NULL and
- * *@flags does not include FOLL_NOWAIT, the mmap_sem may be released.
- * If it is, *@nonblocking will be set to 0 and -EBUSY returned.
+ * mmap_sem must be held on entry. If @locked != NULL and *@flags
+ * does not include FOLL_NOWAIT, the mmap_sem may be released. If it
+ * is, *@locked will be set to 0 and -EBUSY returned.
*/
static int faultin_page(struct task_struct *tsk, struct vm_area_struct *vma,
- unsigned long address, unsigned int *flags, int *nonblocking)
+ unsigned long address, unsigned int *flags, int *locked)
{
unsigned int fault_flags = 0;
vm_fault_t ret;
@@ -639,7 +639,7 @@ static int faultin_page(struct task_struct *tsk, struct
vm_area_struct *vma,
fault_flags |= FAULT_FLAG_WRITE;
if (*flags & FOLL_REMOTE)
fault_flags |= FAULT_FLAG_REMOTE;
- if (nonblocking)
+ if (locked)
fault_flags |= FAULT_FLAG_ALLOW_RETRY;
if (*flags & FOLL_NOWAIT)
fault_flags |= FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_RETRY_NOWAIT;
@@ -665,8 +665,8 @@ static int faultin_page(struct task_struct *tsk, struct
vm_area_struct *vma,
}
if (ret & VM_FAULT_RETRY) {
- if (nonblocking && !(fault_flags & FAULT_FLAG_RETRY_NOWAIT))
- *nonblocking = 0;
+ if (locked && !(fault_flags & FAULT_FLAG_RETRY_NOWAIT))
+ *locked = 0;
return -EBUSY;
}
@@ -743,7 +743,7 @@ static int check_vma_flags(struct vm_area_struct *vma,
unsigned long gup_flags)
* only intends to ensure the pages are faulted in.
* @vmas: array of pointers to vmas corresponding to each page.
* Or NULL if the caller does not require them.
- * @nonblocking: whether waiting for disk IO or mmap_sem contention
+ * @locked: whether we're still with the mmap_sem held
*
* Returns number of pages pinned. This may be fewer than the number
* requested. If nr_pages is 0 or negative, returns 0. If no pages
@@ -772,13 +772,11 @@ static int check_vma_flags(struct vm_area_struct *vma,
unsigned long gup_flags)
* appropriate) must be called after the page is finished with, and
* before put_page is called.
*
- * If @nonblocking != NULL, __get_user_pages will not wait for disk IO
- * or mmap_sem contention, and if waiting is needed to pin all pages,
- * *@nonblocking will be set to 0. Further, if @gup_flags does not
- * include FOLL_NOWAIT, the mmap_sem will be released via up_read() in
- * this case.
+ * If @locked != NULL, *@locked will be set to 0 when mmap_sem is
+ * released by an up_read(). That can happen if @gup_flags does not
+ * have FOLL_NOWAIT.
*
- * A caller using such a combination of @nonblocking and @gup_flags
+ * A caller using such a combination of @locked and @gup_flags
* must therefore hold the mmap_sem for reading only, and recognize
* when it's been released. Otherwise, it must be held for either
* reading or writing and will not be released.
@@ -790,7 +788,7 @@ static int check_vma_flags(struct vm_area_struct *vma,
unsigned long gup_flags)
static long __get_user_pages(struct task_struct *tsk, struct mm_struct *mm,
unsigned long start, unsigned long nr_pages,
unsigned int gup_flags, struct page **pages,
- struct vm_area_struct **vmas, int *nonblocking)
+ struct vm_area_struct **vmas, int *locked)
{
long ret = 0, i = 0;
struct vm_area_struct *vma = NULL;
@@ -834,7 +832,7 @@ static long __get_user_pages(struct task_struct *tsk,
struct mm_struct *mm,
if (is_vm_hugetlb_page(vma)) {
i = follow_hugetlb_page(mm, vma, pages, vmas,
&star
[PATCH v5 03/16] mm: Introduce fault_signal_pending()
For most architectures, we've got a quick path to detect fatal signal
after a handle_mm_fault(). Introduce a helper for that quick path.
It cleans the current codes a bit so we don't need to duplicate the
same check across archs. More importantly, this will be an unified
place that we handle the signal immediately right after an interrupted
page fault, so it'll be much easier for us if we want to change the
behavior of handling signals later on for all the archs.
Note that currently only part of the archs are using this new helper,
because some archs have their own way to handle signals. In the
follow up patches, we'll try to apply this helper to all the rest of
archs.
Another note is that the "regs" parameter in the new helper is not
used yet. It'll be used very soon. Now we kept it in this patch only
to avoid touching all the archs again in the follow up patches.
Signed-off-by: Peter Xu
---
arch/alpha/mm/fault.c| 2 +-
arch/arm/mm/fault.c | 2 +-
arch/hexagon/mm/vm_fault.c | 2 +-
arch/ia64/mm/fault.c | 2 +-
arch/m68k/mm/fault.c | 2 +-
arch/microblaze/mm/fault.c | 2 +-
arch/mips/mm/fault.c | 2 +-
arch/nds32/mm/fault.c| 2 +-
arch/nios2/mm/fault.c| 2 +-
arch/openrisc/mm/fault.c | 2 +-
arch/parisc/mm/fault.c | 2 +-
arch/riscv/mm/fault.c| 2 +-
arch/s390/mm/fault.c | 3 +--
arch/sparc/mm/fault_32.c | 2 +-
arch/sparc/mm/fault_64.c | 2 +-
arch/unicore32/mm/fault.c| 2 +-
arch/xtensa/mm/fault.c | 2 +-
include/linux/sched/signal.h | 13 +
18 files changed, 30 insertions(+), 18 deletions(-)
diff --git a/arch/alpha/mm/fault.c b/arch/alpha/mm/fault.c
index 741e61ef9d3f..aea33b599037 100644
--- a/arch/alpha/mm/fault.c
+++ b/arch/alpha/mm/fault.c
@@ -150,7 +150,7 @@ do_page_fault(unsigned long address, unsigned long mmcsr,
the fault. */
fault = handle_mm_fault(vma, address, flags);
- if ((fault & VM_FAULT_RETRY) && fatal_signal_pending(current))
+ if (fault_signal_pending(fault, regs))
return;
if (unlikely(fault & VM_FAULT_ERROR)) {
diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index 890eeaac3cbb..c8f2950fa733 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -295,7 +295,7 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct
pt_regs *regs)
* signal first. We do not need to release the mmap_sem because
* it would already be released in __lock_page_or_retry in
* mm/filemap.c. */
- if ((fault & VM_FAULT_RETRY) && fatal_signal_pending(current)) {
+ if (fault_signal_pending(fault, regs)) {
if (!user_mode(regs))
goto no_context;
return 0;
diff --git a/arch/hexagon/mm/vm_fault.c b/arch/hexagon/mm/vm_fault.c
index b3bc71680ae4..d19beaf11b4c 100644
--- a/arch/hexagon/mm/vm_fault.c
+++ b/arch/hexagon/mm/vm_fault.c
@@ -91,7 +91,7 @@ void do_page_fault(unsigned long address, long cause, struct
pt_regs *regs)
fault = handle_mm_fault(vma, address, flags);
- if ((fault & VM_FAULT_RETRY) && fatal_signal_pending(current))
+ if (fault_signal_pending(fault, regs))
return;
/* The most common case -- we are done. */
diff --git a/arch/ia64/mm/fault.c b/arch/ia64/mm/fault.c
index c2f299fe9e04..211b4f439384 100644
--- a/arch/ia64/mm/fault.c
+++ b/arch/ia64/mm/fault.c
@@ -141,7 +141,7 @@ ia64_do_page_fault (unsigned long address, unsigned long
isr, struct pt_regs *re
*/
fault = handle_mm_fault(vma, address, flags);
- if ((fault & VM_FAULT_RETRY) && fatal_signal_pending(current))
+ if (fault_signal_pending(fault, regs))
return;
if (unlikely(fault & VM_FAULT_ERROR)) {
diff --git a/arch/m68k/mm/fault.c b/arch/m68k/mm/fault.c
index e9b1d7585b43..a455e202691b 100644
--- a/arch/m68k/mm/fault.c
+++ b/arch/m68k/mm/fault.c
@@ -138,7 +138,7 @@ int do_page_fault(struct pt_regs *regs, unsigned long
address,
fault = handle_mm_fault(vma, address, flags);
pr_debug("handle_mm_fault returns %x\n", fault);
- if ((fault & VM_FAULT_RETRY) && fatal_signal_pending(current))
+ if (fault_signal_pending(fault, regs))
return 0;
if (unlikely(fault & VM_FAULT_ERROR)) {
diff --git a/arch/microblaze/mm/fault.c b/arch/microblaze/mm/fault.c
index e6a810b0c7ad..cdde01dcdfc3 100644
--- a/arch/microblaze/mm/fault.c
+++ b/arch/microblaze/mm/fault.c
@@ -217,7 +217,7 @@ void do_page_fault(struct pt_regs *regs, unsigned long
address,
*/
fault = handle_mm_fault(vma, address, flags);
- if ((fault & VM_FAULT_RETRY) && fatal_signal_pending(current))
+ if (fault_signal_pending(fault, regs))
return;
if (unlikely(fault & VM_FAULT_ERROR)) {
diff --git a/arch/mips/mm/fault.c b/arch/mips/mm/fault.c
index f589aa8f47d9..31fda59199eb 10
[PATCH v5 06/16] arm64/mm: Use helper fault_signal_pending()
Let the arm64 fault handling to use the new fault_signal_pending()
helper, by moving the signal handling out of the retry logic.
Signed-off-by: Peter Xu
---
arch/arm64/mm/fault.c | 19 +++
1 file changed, 7 insertions(+), 12 deletions(-)
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index cfd65b63f36f..4a695a44fb05 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -477,19 +477,14 @@ static int __kprobes do_page_fault(unsigned long addr,
unsigned int esr,
fault = __do_page_fault(mm, addr, mm_flags, vm_flags);
major |= fault & VM_FAULT_MAJOR;
- if (fault & VM_FAULT_RETRY) {
- /*
-* If we need to retry but a fatal signal is pending,
-* handle the signal first. We do not need to release
-* the mmap_sem because it would already be released
-* in __lock_page_or_retry in mm/filemap.c.
-*/
- if (fatal_signal_pending(current)) {
- if (!user_mode(regs))
- goto no_context;
- return 0;
- }
+ /* Quick path to respond to signals */
+ if (fault_signal_pending(fault, regs)) {
+ if (!user_mode(regs))
+ goto no_context;
+ return 0;
+ }
+ if (fault & VM_FAULT_RETRY) {
/*
* Clear FAULT_FLAG_ALLOW_RETRY to avoid any risk of
* starvation.
--
2.21.0
[PATCH v5 05/16] arc/mm: Use helper fault_signal_pending()
Let ARC to use the new helper fault_signal_pending() by moving the
signal check out of the retry logic as standalone. This should also
helps to simplify the code a bit.
Signed-off-by: Peter Xu
---
arch/arc/mm/fault.c | 34 +-
1 file changed, 13 insertions(+), 21 deletions(-)
diff --git a/arch/arc/mm/fault.c b/arch/arc/mm/fault.c
index 3861543b66a0..ee3ba7c7b891 100644
--- a/arch/arc/mm/fault.c
+++ b/arch/arc/mm/fault.c
@@ -127,29 +127,21 @@ void do_page_fault(unsigned long address, struct pt_regs
*regs)
fault = handle_mm_fault(vma, address, flags);
+ /* Quick path to respond to signals */
+ if (fault_signal_pending(fault, regs)) {
+ if (!user_mode(regs))
+ goto no_context;
+ return;
+ }
+
/*
-* Fault retry nuances
+* Fault retry nuances, mmap_sem already relinquished by core mm
*/
- if (unlikely(fault & VM_FAULT_RETRY)) {
-
- /*
-* If fault needs to be retried, handle any pending signals
-* first (by returning to user mode).
-* mmap_sem already relinquished by core mm for RETRY case
-*/
- if (fatal_signal_pending(current)) {
- if (!user_mode(regs))
- goto no_context;
- return;
- }
- /*
-* retry state machine
-*/
- if (flags & FAULT_FLAG_ALLOW_RETRY) {
- flags &= ~FAULT_FLAG_ALLOW_RETRY;
- flags |= FAULT_FLAG_TRIED;
- goto retry;
- }
+ if (unlikely((fault & VM_FAULT_RETRY) &&
+(flags & FAULT_FLAG_ALLOW_RETRY))) {
+ flags &= ~FAULT_FLAG_ALLOW_RETRY;
+ flags |= FAULT_FLAG_TRIED;
+ goto retry;
}
bad_area:
--
2.21.0
[PATCH v5 04/16] x86/mm: Use helper fault_signal_pending()
Let's move the fatal signal check even earlier so that we can directly
use the new fault_signal_pending() in x86 mm code.
Signed-off-by: Peter Xu
---
arch/x86/mm/fault.c | 28 +---
1 file changed, 13 insertions(+), 15 deletions(-)
diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index 9ceacd1156db..059331797827 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -1441,27 +1441,25 @@ void do_user_addr_fault(struct pt_regs *regs,
fault = handle_mm_fault(vma, address, flags);
major |= fault & VM_FAULT_MAJOR;
+ /* Quick path to respond to signals */
+ if (fault_signal_pending(fault, regs)) {
+ if (!user_mode(regs))
+ no_context(regs, hw_error_code, address, SIGBUS,
+ BUS_ADRERR);
+ return;
+ }
+
/*
* If we need to retry the mmap_sem has already been released,
* and if there is a fatal signal pending there is no guarantee
* that we made any progress. Handle this case first.
*/
- if (unlikely(fault & VM_FAULT_RETRY)) {
+ if (unlikely((fault & VM_FAULT_RETRY) &&
+(flags & FAULT_FLAG_ALLOW_RETRY))) {
/* Retry at most once */
- if (flags & FAULT_FLAG_ALLOW_RETRY) {
- flags &= ~FAULT_FLAG_ALLOW_RETRY;
- flags |= FAULT_FLAG_TRIED;
- if (!fatal_signal_pending(tsk))
- goto retry;
- }
-
- /* User mode? Just return to handle the fatal exception */
- if (flags & FAULT_FLAG_USER)
- return;
-
- /* Not returning to user mode? Handle exceptions or die: */
- no_context(regs, hw_error_code, address, SIGBUS, BUS_ADRERR);
- return;
+ flags &= ~FAULT_FLAG_ALLOW_RETRY;
+ flags |= FAULT_FLAG_TRIED;
+ goto retry;
}
up_read(&mm->mmap_sem);
--
2.21.0
[PATCH v5 07/16] powerpc/mm: Use helper fault_signal_pending()
Let powerpc code to use the new helper, by moving the signal handling earlier before the retry logic. Signed-off-by: Peter Xu --- arch/powerpc/mm/fault.c | 12 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c index 8432c281de92..7dabd1243f1b 100644 --- a/arch/powerpc/mm/fault.c +++ b/arch/powerpc/mm/fault.c @@ -583,6 +583,9 @@ static int __do_page_fault(struct pt_regs *regs, unsigned long address, major |= fault & VM_FAULT_MAJOR; + if (fault_signal_pending(fault, regs)) + return user_mode(regs) ? 0 : SIGBUS; + /* * Handle the retry right now, the mmap_sem has been released in that * case. @@ -596,15 +599,8 @@ static int __do_page_fault(struct pt_regs *regs, unsigned long address, */ flags &= ~FAULT_FLAG_ALLOW_RETRY; flags |= FAULT_FLAG_TRIED; - if (!fatal_signal_pending(current)) - goto retry; + goto retry; } - - /* -* User mode? Just return to handle the fatal exception otherwise -* return to bad_page_fault -*/ - return is_user ? 0 : SIGBUS; } up_read(¤t->mm->mmap_sem); -- 2.21.0
[PATCH v5 08/16] sh/mm: Use helper fault_signal_pending()
Let SH to use the new fault_signal_pending() helper. Here we'll need
to move the up_read() out because that's actually needed as long as
!RETRY cases. At the meantime we can drop all the rest of up_read()s
now (which seems to be cleaner).
Signed-off-by: Peter Xu
---
arch/sh/mm/fault.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/sh/mm/fault.c b/arch/sh/mm/fault.c
index 5f51456f4fc7..eb4048ad0b38 100644
--- a/arch/sh/mm/fault.c
+++ b/arch/sh/mm/fault.c
@@ -302,25 +302,25 @@ mm_fault_error(struct pt_regs *regs, unsigned long
error_code,
* Pagefault was interrupted by SIGKILL. We have no reason to
* continue pagefault.
*/
- if (fatal_signal_pending(current)) {
- if (!(fault & VM_FAULT_RETRY))
- up_read(¤t->mm->mmap_sem);
+ if (fault_signal_pending(fault, regs)) {
if (!user_mode(regs))
no_context(regs, error_code, address);
return 1;
}
+ /* Release mmap_sem first if necessary */
+ if (!(fault & VM_FAULT_RETRY))
+ up_read(¤t->mm->mmap_sem);
+
if (!(fault & VM_FAULT_ERROR))
return 0;
if (fault & VM_FAULT_OOM) {
/* Kernel mode? Handle exceptions or die: */
if (!user_mode(regs)) {
- up_read(¤t->mm->mmap_sem);
no_context(regs, error_code, address);
return 1;
}
- up_read(¤t->mm->mmap_sem);
/*
* We ran out of memory, call the OOM killer, and return the
--
2.21.0
[PATCH v5 09/16] mm: Return faster for non-fatal signals in user mode faults
The idea comes from the upstream discussion between Linus and Andrea:
https://lore.kernel.org/lkml/20171102193644.gb22...@redhat.com/
A summary to the issue: there was a special path in handle_userfault()
in the past that we'll return a VM_FAULT_NOPAGE when we detected
non-fatal signals when waiting for userfault handling. We did that by
reacquiring the mmap_sem before returning. However that brings a risk
in that the vmas might have changed when we retake the mmap_sem and
even we could be holding an invalid vma structure.
This patch is a preparation of removing that special path by allowing
the page fault to return even faster if we were interrupted by a
non-fatal signal during a user-mode page fault handling routine.
Suggested-by: Linus Torvalds
Suggested-by: Andrea Arcangeli
Signed-off-by: Peter Xu
---
include/linux/sched/signal.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
index 46429192733b..031af0a6505a 100644
--- a/include/linux/sched/signal.h
+++ b/include/linux/sched/signal.h
@@ -387,7 +387,8 @@ static inline bool fault_signal_pending(unsigned int
fault_flags,
struct pt_regs *regs)
{
return unlikely((fault_flags & VM_FAULT_RETRY) &&
- fatal_signal_pending(current));
+ (fatal_signal_pending(current) ||
+(user_mode(regs) && signal_pending(current;
}
/*
--
2.21.0
[PATCH v5 10/16] userfaultfd: Don't retake mmap_sem to emulate NOPAGE
This patch removes the risk path in handle_userfault() then we will be
sure that the callers of handle_mm_fault() will know that the VMAs
might have changed. Meanwhile with previous patch we don't lose
responsiveness as well since the core mm code now can handle the
nonfatal userspace signals even if we return VM_FAULT_RETRY.
Suggested-by: Andrea Arcangeli
Suggested-by: Linus Torvalds
Reviewed-by: Jerome Glisse
Signed-off-by: Peter Xu
---
fs/userfaultfd.c | 24
1 file changed, 24 deletions(-)
diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
index fe6d804a38dc..d8777146bae7 100644
--- a/fs/userfaultfd.c
+++ b/fs/userfaultfd.c
@@ -524,30 +524,6 @@ vm_fault_t handle_userfault(struct vm_fault *vmf, unsigned
long reason)
__set_current_state(TASK_RUNNING);
- if (return_to_userland) {
- if (signal_pending(current) &&
- !fatal_signal_pending(current)) {
- /*
-* If we got a SIGSTOP or SIGCONT and this is
-* a normal userland page fault, just let
-* userland return so the signal will be
-* handled and gdb debugging works. The page
-* fault code immediately after we return from
-* this function is going to release the
-* mmap_sem and it's not depending on it
-* (unlike gup would if we were not to return
-* VM_FAULT_RETRY).
-*
-* If a fatal signal is pending we still take
-* the streamlined VM_FAULT_RETRY failure path
-* and there's no need to retake the mmap_sem
-* in such case.
-*/
- down_read(&mm->mmap_sem);
- ret = VM_FAULT_NOPAGE;
- }
- }
-
/*
* Here we race with the list_del; list_add in
* userfaultfd_ctx_read(), however because we don't ever run
--
2.21.0
[PATCH v5 13/16] mm: Allow VM_FAULT_RETRY for multiple times
The idea comes from a discussion between Linus and Andrea [1].
Before this patch we only allow a page fault to retry once. We
achieved this by clearing the FAULT_FLAG_ALLOW_RETRY flag when doing
handle_mm_fault() the second time. This was majorly used to avoid
unexpected starvation of the system by looping over forever to handle
the page fault on a single page. However that should hardly happen,
and after all for each code path to return a VM_FAULT_RETRY we'll
first wait for a condition (during which time we should possibly yield
the cpu) to happen before VM_FAULT_RETRY is really returned.
This patch removes the restriction by keeping the
FAULT_FLAG_ALLOW_RETRY flag when we receive VM_FAULT_RETRY. It means
that the page fault handler now can retry the page fault for multiple
times if necessary without the need to generate another page fault
event. Meanwhile we still keep the FAULT_FLAG_TRIED flag so page
fault handler can still identify whether a page fault is the first
attempt or not.
Then we'll have these combinations of fault flags (only considering
ALLOW_RETRY flag and TRIED flag):
- ALLOW_RETRY and !TRIED: this means the page fault allows to
retry, and this is the first try
- ALLOW_RETRY and TRIED: this means the page fault allows to
retry, and this is not the first try
- !ALLOW_RETRY and !TRIED: this means the page fault does not allow
to retry at all
- !ALLOW_RETRY and TRIED: this is forbidden and should never be used
In existing code we have multiple places that has taken special care
of the first condition above by checking against (fault_flags &
FAULT_FLAG_ALLOW_RETRY). This patch introduces a simple helper to
detect the first retry of a page fault by checking against
both (fault_flags & FAULT_FLAG_ALLOW_RETRY) and !(fault_flag &
FAULT_FLAG_TRIED) because now even the 2nd try will have the
ALLOW_RETRY set, then use that helper in all existing special paths.
One example is in __lock_page_or_retry(), now we'll drop the mmap_sem
only in the first attempt of page fault and we'll keep it in follow up
retries, so old locking behavior will be retained.
This will be a nice enhancement for current code [2] at the same time
a supporting material for the future userfaultfd-writeprotect work,
since in that work there will always be an explicit userfault
writeprotect retry for protected pages, and if that cannot resolve the
page fault (e.g., when userfaultfd-writeprotect is used in conjunction
with swapped pages) then we'll possibly need a 3rd retry of the page
fault. It might also benefit other potential users who will have
similar requirement like userfault write-protection.
GUP code is not touched yet and will be covered in follow up patch.
Please read the thread below for more information.
[1] https://lore.kernel.org/lkml/20171102193644.gb22...@redhat.com/
[2] https://lore.kernel.org/lkml/20181230154648.gb9...@redhat.com/
Suggested-by: Linus Torvalds
Suggested-by: Andrea Arcangeli
Reviewed-by: Jerome Glisse
Signed-off-by: Peter Xu
---
arch/alpha/mm/fault.c | 2 +-
arch/arc/mm/fault.c | 1 -
arch/arm/mm/fault.c | 3 ---
arch/arm64/mm/fault.c | 5 -
arch/hexagon/mm/vm_fault.c | 1 -
arch/ia64/mm/fault.c| 1 -
arch/m68k/mm/fault.c| 3 ---
arch/microblaze/mm/fault.c | 1 -
arch/mips/mm/fault.c| 1 -
arch/nds32/mm/fault.c | 1 -
arch/nios2/mm/fault.c | 3 ---
arch/openrisc/mm/fault.c| 1 -
arch/parisc/mm/fault.c | 4 +---
arch/powerpc/mm/fault.c | 6 --
arch/riscv/mm/fault.c | 5 -
arch/s390/mm/fault.c| 5 +
arch/sh/mm/fault.c | 1 -
arch/sparc/mm/fault_32.c| 1 -
arch/sparc/mm/fault_64.c| 1 -
arch/um/kernel/trap.c | 1 -
arch/unicore32/mm/fault.c | 4 +---
arch/x86/mm/fault.c | 2 --
arch/xtensa/mm/fault.c | 1 -
drivers/gpu/drm/ttm/ttm_bo_vm.c | 12 ---
include/linux/mm.h | 37 +
mm/filemap.c| 2 +-
mm/shmem.c | 2 +-
27 files changed, 52 insertions(+), 55 deletions(-)
diff --git a/arch/alpha/mm/fault.c b/arch/alpha/mm/fault.c
index fcfa229cc1e7..c2d7b6d7bac7 100644
--- a/arch/alpha/mm/fault.c
+++ b/arch/alpha/mm/fault.c
@@ -169,7 +169,7 @@ do_page_fault(unsigned long address, unsigned long mmcsr,
else
current->min_flt++;
if (fault & VM_FAULT_RETRY) {
- flags &= ~FAULT_FLAG_ALLOW_RETRY;
+ flags |= FAULT_FLAG_TRIED;
/* No need to up_read(&mm->mmap_sem) as we would
* have already released it in __lock_page_or_retry
diff --git a/arch/arc/mm/fault.c b/arch/arc/mm/fault.c
index 33a609f1c43
[PATCH v5 15/16] mm/gup: Allow to react to fatal signals
The existing gup code does not react to the fatal signals in many code
paths. For example, in one retry path of gup we're still using
down_read() rather than down_read_killable(). Also, when doing page
faults we don't pass in FAULT_FLAG_KILLABLE as well, which means that
within the faulting process we'll wait in non-killable way as well.
These were spotted by Linus during the code review of some other
patches.
Let's allow the gup code to react to fatal signals to improve the
responsiveness of threads when during gup and being killed.
Signed-off-by: Peter Xu
---
mm/gup.c | 12 +---
mm/hugetlb.c | 3 ++-
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/mm/gup.c b/mm/gup.c
index d2811bb15a25..4c638473db83 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -640,7 +640,7 @@ static int faultin_page(struct task_struct *tsk, struct
vm_area_struct *vma,
if (*flags & FOLL_REMOTE)
fault_flags |= FAULT_FLAG_REMOTE;
if (locked)
- fault_flags |= FAULT_FLAG_ALLOW_RETRY;
+ fault_flags |= FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE;
if (*flags & FOLL_NOWAIT)
fault_flags |= FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_RETRY_NOWAIT;
if (*flags & FOLL_TRIED) {
@@ -973,7 +973,7 @@ int fixup_user_fault(struct task_struct *tsk, struct
mm_struct *mm,
vm_fault_t ret, major = 0;
if (unlocked)
- fault_flags |= FAULT_FLAG_ALLOW_RETRY;
+ fault_flags |= FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE;
retry:
vma = find_extend_vma(mm, address);
@@ -1086,7 +1086,13 @@ static __always_inline long
__get_user_pages_locked(struct task_struct *tsk,
break;
*locked = 1;
- down_read(&mm->mmap_sem);
+ ret = down_read_killable(&mm->mmap_sem);
+ if (ret) {
+ BUG_ON(ret > 0);
+ if (!pages_done)
+ pages_done = ret;
+ break;
+ }
ret = __get_user_pages(tsk, mm, start, 1, flags | FOLL_TRIED,
pages, NULL, locked);
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index d0c98cff5b0f..84034154d50e 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -4342,7 +4342,8 @@ long follow_hugetlb_page(struct mm_struct *mm, struct
vm_area_struct *vma,
if (flags & FOLL_WRITE)
fault_flags |= FAULT_FLAG_WRITE;
if (locked)
- fault_flags |= FAULT_FLAG_ALLOW_RETRY;
+ fault_flags |= FAULT_FLAG_ALLOW_RETRY |
+ FAULT_FLAG_KILLABLE;
if (flags & FOLL_NOWAIT)
fault_flags |= FAULT_FLAG_ALLOW_RETRY |
FAULT_FLAG_RETRY_NOWAIT;
--
2.21.0
[PATCH v5 16/16] mm/userfaultfd: Honor FAULT_FLAG_KILLABLE in fault path
Userfaultfd fault path was by default killable even if the caller does
not have FAULT_FLAG_KILLABLE. That makes sense before in that when
with gup we don't have FAULT_FLAG_KILLABLE properly set before. Now
after previous patch we've got FAULT_FLAG_KILLABLE applied even for
gup code so it should also make sense to let userfaultfd to honor the
FAULT_FLAG_KILLABLE.
Because we're unconditionally setting FAULT_FLAG_KILLABLE in gup code
right now, this patch should have no functional change. It also
cleaned the code a little bit by introducing some helpers.
Signed-off-by: Peter Xu
---
fs/userfaultfd.c | 36
1 file changed, 28 insertions(+), 8 deletions(-)
diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
index 2b3b48e94ae4..8c5863ccbf0e 100644
--- a/fs/userfaultfd.c
+++ b/fs/userfaultfd.c
@@ -334,6 +334,30 @@ static inline bool userfaultfd_must_wait(struct
userfaultfd_ctx *ctx,
return ret;
}
+/* Should pair with userfaultfd_signal_pending() */
+static inline long userfaultfd_get_blocking_state(unsigned int flags)
+{
+ if (flags & FAULT_FLAG_INTERRUPTIBLE)
+ return TASK_INTERRUPTIBLE;
+
+ if (flags & FAULT_FLAG_KILLABLE)
+ return TASK_KILLABLE;
+
+ return TASK_UNINTERRUPTIBLE;
+}
+
+/* Should pair with userfaultfd_get_blocking_state() */
+static inline bool userfaultfd_signal_pending(unsigned int flags)
+{
+ if (flags & FAULT_FLAG_INTERRUPTIBLE)
+ return signal_pending(current);
+
+ if (flags & FAULT_FLAG_KILLABLE)
+ return fatal_signal_pending(current);
+
+ return false;
+}
+
/*
* The locking rules involved in returning VM_FAULT_RETRY depending on
* FAULT_FLAG_ALLOW_RETRY, FAULT_FLAG_RETRY_NOWAIT and
@@ -355,7 +379,7 @@ vm_fault_t handle_userfault(struct vm_fault *vmf, unsigned
long reason)
struct userfaultfd_ctx *ctx;
struct userfaultfd_wait_queue uwq;
vm_fault_t ret = VM_FAULT_SIGBUS;
- bool must_wait, return_to_userland;
+ bool must_wait;
long blocking_state;
/*
@@ -462,9 +486,7 @@ vm_fault_t handle_userfault(struct vm_fault *vmf, unsigned
long reason)
uwq.ctx = ctx;
uwq.waken = false;
- return_to_userland = vmf->flags & FAULT_FLAG_INTERRUPTIBLE;
- blocking_state = return_to_userland ? TASK_INTERRUPTIBLE :
-TASK_KILLABLE;
+ blocking_state = userfaultfd_get_blocking_state(vmf->flags);
spin_lock_irq(&ctx->fault_pending_wqh.lock);
/*
@@ -490,8 +512,7 @@ vm_fault_t handle_userfault(struct vm_fault *vmf, unsigned
long reason)
up_read(&mm->mmap_sem);
if (likely(must_wait && !READ_ONCE(ctx->released) &&
- (return_to_userland ? !signal_pending(current) :
- !fatal_signal_pending(current {
+ userfaultfd_signal_pending(vmf->flags))) {
wake_up_poll(&ctx->fd_wqh, EPOLLIN);
schedule();
ret |= VM_FAULT_MAJOR;
@@ -513,8 +534,7 @@ vm_fault_t handle_userfault(struct vm_fault *vmf, unsigned
long reason)
set_current_state(blocking_state);
if (READ_ONCE(uwq.waken) ||
READ_ONCE(ctx->released) ||
- (return_to_userland ? signal_pending(current) :
-fatal_signal_pending(current)))
+ userfaultfd_signal_pending(vmf->flags))
break;
schedule();
}
--
2.21.0
[PATCH v5 14/16] mm/gup: Allow VM_FAULT_RETRY for multiple times
This is the gup counterpart of the change that allows the
VM_FAULT_RETRY to happen for more than once. One thing to mention is
that we must check the fatal signal here before retry because the GUP
can be interrupted by that, otherwise we can loop forever.
Signed-off-by: Peter Xu
---
mm/gup.c | 27 +--
mm/hugetlb.c | 6 --
2 files changed, 25 insertions(+), 8 deletions(-)
diff --git a/mm/gup.c b/mm/gup.c
index e60d32f1674d..d2811bb15a25 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -644,7 +644,10 @@ static int faultin_page(struct task_struct *tsk, struct
vm_area_struct *vma,
if (*flags & FOLL_NOWAIT)
fault_flags |= FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_RETRY_NOWAIT;
if (*flags & FOLL_TRIED) {
- VM_WARN_ON_ONCE(fault_flags & FAULT_FLAG_ALLOW_RETRY);
+ /*
+* Note: FAULT_FLAG_ALLOW_RETRY and FAULT_FLAG_TRIED
+* can co-exist
+*/
fault_flags |= FAULT_FLAG_TRIED;
}
@@ -994,7 +997,6 @@ int fixup_user_fault(struct task_struct *tsk, struct
mm_struct *mm,
down_read(&mm->mmap_sem);
if (!(fault_flags & FAULT_FLAG_TRIED)) {
*unlocked = true;
- fault_flags &= ~FAULT_FLAG_ALLOW_RETRY;
fault_flags |= FAULT_FLAG_TRIED;
goto retry;
}
@@ -1069,17 +1071,30 @@ static __always_inline long
__get_user_pages_locked(struct task_struct *tsk,
if (likely(pages))
pages += ret;
start += ret << PAGE_SHIFT;
+ lock_dropped = true;
+retry:
/*
* Repeat on the address that fired VM_FAULT_RETRY
-* without FAULT_FLAG_ALLOW_RETRY but with
-* FAULT_FLAG_TRIED.
+* with both FAULT_FLAG_ALLOW_RETRY and
+* FAULT_FLAG_TRIED. Note that GUP can be interrupted
+* by fatal signals, so we need to check it before we
+* start trying again otherwise it can loop forever.
*/
+
+ if (fatal_signal_pending(current))
+ break;
+
*locked = 1;
- lock_dropped = true;
down_read(&mm->mmap_sem);
+
ret = __get_user_pages(tsk, mm, start, 1, flags | FOLL_TRIED,
- pages, NULL, NULL);
+ pages, NULL, locked);
+ if (!*locked) {
+ /* Continue to retry until we succeeded */
+ BUG_ON(ret != 0);
+ goto retry;
+ }
if (ret != 1) {
BUG_ON(ret > 1);
if (!pages_done)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 31c2a6275023..d0c98cff5b0f 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -4347,8 +4347,10 @@ long follow_hugetlb_page(struct mm_struct *mm, struct
vm_area_struct *vma,
fault_flags |= FAULT_FLAG_ALLOW_RETRY |
FAULT_FLAG_RETRY_NOWAIT;
if (flags & FOLL_TRIED) {
- VM_WARN_ON_ONCE(fault_flags &
- FAULT_FLAG_ALLOW_RETRY);
+ /*
+* Note: FAULT_FLAG_ALLOW_RETRY and
+* FAULT_FLAG_TRIED can co-exist
+*/
fault_flags |= FAULT_FLAG_TRIED;
}
ret = hugetlb_fault(mm, vma, vaddr, fault_flags);
--
2.21.0
[PATCH v5 12/16] mm: Introduce FAULT_FLAG_INTERRUPTIBLE
handle_userfaultfd() is currently the only one place in the kernel
page fault procedures that can respond to non-fatal userspace signals.
It was trying to detect such an allowance by checking against USER &
KILLABLE flags, which was "un-official".
In this patch, we introduced a new flag (FAULT_FLAG_INTERRUPTIBLE) to
show that the fault handler allows the fault procedure to respond even
to non-fatal signals. Meanwhile, add this new flag to the default
fault flags so that all the page fault handlers can benefit from the
new flag. With that, replacing the userfault check to this one.
Since the line is getting even longer, clean up the fault flags a bit
too to ease TTY users.
Although we've got a new flag and applied it, we shouldn't have any
functional change with this patch so far.
Suggested-by: Linus Torvalds
Reviewed-by: David Hildenbrand
Signed-off-by: Peter Xu
---
fs/userfaultfd.c | 4 +---
include/linux/mm.h | 39 ---
2 files changed, 29 insertions(+), 14 deletions(-)
diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
index d8777146bae7..2b3b48e94ae4 100644
--- a/fs/userfaultfd.c
+++ b/fs/userfaultfd.c
@@ -462,9 +462,7 @@ vm_fault_t handle_userfault(struct vm_fault *vmf, unsigned
long reason)
uwq.ctx = ctx;
uwq.waken = false;
- return_to_userland =
- (vmf->flags & (FAULT_FLAG_USER|FAULT_FLAG_KILLABLE)) ==
- (FAULT_FLAG_USER|FAULT_FLAG_KILLABLE);
+ return_to_userland = vmf->flags & FAULT_FLAG_INTERRUPTIBLE;
blocking_state = return_to_userland ? TASK_INTERRUPTIBLE :
TASK_KILLABLE;
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 57fb5c535f8e..53ec7abb8472 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -383,22 +383,38 @@ extern unsigned int kobjsize(const void *objp);
*/
extern pgprot_t protection_map[16];
-#define FAULT_FLAG_WRITE 0x01/* Fault was a write access */
-#define FAULT_FLAG_MKWRITE 0x02/* Fault was mkwrite of existing pte */
-#define FAULT_FLAG_ALLOW_RETRY 0x04/* Retry fault if blocking */
-#define FAULT_FLAG_RETRY_NOWAIT0x08/* Don't drop mmap_sem and wait
when retrying */
-#define FAULT_FLAG_KILLABLE0x10/* The fault task is in SIGKILL
killable region */
-#define FAULT_FLAG_TRIED 0x20/* Second try */
-#define FAULT_FLAG_USER0x40/* The fault originated in
userspace */
-#define FAULT_FLAG_REMOTE 0x80/* faulting for non current tsk/mm */
-#define FAULT_FLAG_INSTRUCTION 0x100 /* The fault was during an instruction
fetch */
+/**
+ * Fault flag definitions.
+ *
+ * @FAULT_FLAG_WRITE: Fault was a write fault.
+ * @FAULT_FLAG_MKWRITE: Fault was mkwrite of existing PTE.
+ * @FAULT_FLAG_ALLOW_RETRY: Allow to retry the fault if blocked.
+ * @FAULT_FLAG_RETRY_NOWAIT: Don't drop mmap_sem and wait when retrying.
+ * @FAULT_FLAG_KILLABLE: The fault task is in SIGKILL killable region.
+ * @FAULT_FLAG_TRIED: The fault has been tried once.
+ * @FAULT_FLAG_USER: The fault originated in userspace.
+ * @FAULT_FLAG_REMOTE: The fault is not for current task/mm.
+ * @FAULT_FLAG_INSTRUCTION: The fault was during an instruction fetch.
+ * @FAULT_FLAG_INTERRUPTIBLE: The fault can be interrupted by non-fatal
signals.
+ */
+#define FAULT_FLAG_WRITE 0x01
+#define FAULT_FLAG_MKWRITE 0x02
+#define FAULT_FLAG_ALLOW_RETRY 0x04
+#define FAULT_FLAG_RETRY_NOWAIT0x08
+#define FAULT_FLAG_KILLABLE0x10
+#define FAULT_FLAG_TRIED 0x20
+#define FAULT_FLAG_USER0x40
+#define FAULT_FLAG_REMOTE 0x80
+#define FAULT_FLAG_INSTRUCTION 0x100
+#define FAULT_FLAG_INTERRUPTIBLE 0x200
/*
* The default fault flags that should be used by most of the
* arch-specific page fault handlers.
*/
#define FAULT_FLAG_DEFAULT (FAULT_FLAG_ALLOW_RETRY | \
-FAULT_FLAG_KILLABLE)
+FAULT_FLAG_KILLABLE | \
+FAULT_FLAG_INTERRUPTIBLE)
#define FAULT_FLAG_TRACE \
{ FAULT_FLAG_WRITE, "WRITE" }, \
@@ -409,7 +425,8 @@ extern pgprot_t protection_map[16];
{ FAULT_FLAG_TRIED, "TRIED" }, \
{ FAULT_FLAG_USER, "USER" }, \
{ FAULT_FLAG_REMOTE,"REMOTE" }, \
- { FAULT_FLAG_INSTRUCTION, "INSTRUCTION" }
+ { FAULT_FLAG_INSTRUCTION, "INSTRUCTION" }, \
+ { FAULT_FLAG_INTERRUPTIBLE, "INTERRUPTIBLE" }
/*
* vm_fault is filled by the the pagefault handler and passed to the vma's
--
2.21.0
[PATCH v5 11/16] mm: Introduce FAULT_FLAG_DEFAULT
Although there're tons of arch-specific page fault handlers, most of them are still sharing the same initial value of the page fault flags. Say, merely all of the page fault handlers would allow the fault to be retried, and they also allow the fault to respond to SIGKILL. Let's define a default value for the fault flags to replace those initial page fault flags that were copied over. With this, it'll be far easier to introduce new fault flag that can be used by all the architectures instead of touching all the archs. Reviewed-by: David Hildenbrand Signed-off-by: Peter Xu --- arch/alpha/mm/fault.c | 2 +- arch/arc/mm/fault.c| 2 +- arch/arm/mm/fault.c| 2 +- arch/arm64/mm/fault.c | 2 +- arch/hexagon/mm/vm_fault.c | 2 +- arch/ia64/mm/fault.c | 2 +- arch/m68k/mm/fault.c | 2 +- arch/microblaze/mm/fault.c | 2 +- arch/mips/mm/fault.c | 2 +- arch/nds32/mm/fault.c | 2 +- arch/nios2/mm/fault.c | 2 +- arch/openrisc/mm/fault.c | 2 +- arch/parisc/mm/fault.c | 2 +- arch/powerpc/mm/fault.c| 2 +- arch/riscv/mm/fault.c | 2 +- arch/s390/mm/fault.c | 2 +- arch/sh/mm/fault.c | 2 +- arch/sparc/mm/fault_32.c | 2 +- arch/sparc/mm/fault_64.c | 2 +- arch/um/kernel/trap.c | 2 +- arch/unicore32/mm/fault.c | 2 +- arch/x86/mm/fault.c| 2 +- arch/xtensa/mm/fault.c | 2 +- include/linux/mm.h | 7 +++ 24 files changed, 30 insertions(+), 23 deletions(-) diff --git a/arch/alpha/mm/fault.c b/arch/alpha/mm/fault.c index aea33b599037..fcfa229cc1e7 100644 --- a/arch/alpha/mm/fault.c +++ b/arch/alpha/mm/fault.c @@ -89,7 +89,7 @@ do_page_fault(unsigned long address, unsigned long mmcsr, const struct exception_table_entry *fixup; int si_code = SEGV_MAPERR; vm_fault_t fault; - unsigned int flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE; + unsigned int flags = FAULT_FLAG_DEFAULT; /* As of EV6, a load into $31/$f31 is a prefetch, and never faults (or is suppressed by the PALcode). Support that for older CPUs diff --git a/arch/arc/mm/fault.c b/arch/arc/mm/fault.c index ee3ba7c7b891..33a609f1c43c 100644 --- a/arch/arc/mm/fault.c +++ b/arch/arc/mm/fault.c @@ -94,7 +94,7 @@ void do_page_fault(unsigned long address, struct pt_regs *regs) (regs->ecr_cause == ECR_C_PROTV_INST_FETCH)) exec = 1; - flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE; + flags = FAULT_FLAG_DEFAULT; if (user_mode(regs)) flags |= FAULT_FLAG_USER; if (write) diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c index c8f2950fa733..101ae6698637 100644 --- a/arch/arm/mm/fault.c +++ b/arch/arm/mm/fault.c @@ -241,7 +241,7 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs) struct mm_struct *mm; int sig, code; vm_fault_t fault; - unsigned int flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE; + unsigned int flags = FAULT_FLAG_DEFAULT; if (kprobe_page_fault(regs, fsr)) return 0; diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c index 4a695a44fb05..23e3feca0f02 100644 --- a/arch/arm64/mm/fault.c +++ b/arch/arm64/mm/fault.c @@ -410,7 +410,7 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr, struct mm_struct *mm = current->mm; vm_fault_t fault, major = 0; unsigned long vm_flags = VM_READ | VM_WRITE; - unsigned int mm_flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE; + unsigned int mm_flags = FAULT_FLAG_DEFAULT; if (kprobe_page_fault(regs, esr)) return 0; diff --git a/arch/hexagon/mm/vm_fault.c b/arch/hexagon/mm/vm_fault.c index d19beaf11b4c..d9e15d941bdb 100644 --- a/arch/hexagon/mm/vm_fault.c +++ b/arch/hexagon/mm/vm_fault.c @@ -41,7 +41,7 @@ void do_page_fault(unsigned long address, long cause, struct pt_regs *regs) int si_code = SEGV_MAPERR; vm_fault_t fault; const struct exception_table_entry *fixup; - unsigned int flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE; + unsigned int flags = FAULT_FLAG_DEFAULT; /* * If we're in an interrupt or have no user context, diff --git a/arch/ia64/mm/fault.c b/arch/ia64/mm/fault.c index 211b4f439384..b5aa4e80c762 100644 --- a/arch/ia64/mm/fault.c +++ b/arch/ia64/mm/fault.c @@ -65,7 +65,7 @@ ia64_do_page_fault (unsigned long address, unsigned long isr, struct pt_regs *re struct mm_struct *mm = current->mm; unsigned long mask; vm_fault_t fault; - unsigned int flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE; + unsigned int flags = FAULT_FLAG_DEFAULT; mask = isr >> IA64_ISR_X_BIT) & 1UL) << VM_EXEC_BIT) | (((isr >> IA64_ISR_W_BIT) & 1UL) << VM_WRITE_BIT)); diff --git a/arch/m68k/mm/fault.c b/arch/m68k/mm/fault.c index a455e202691b..182799fd9987 100644 -
Re: [PATCH] scsi: qla2xxx: Remove WARN_ON_ONCE in qla2x00_status_cont_entry()
On 9/26/19 9:46 AM, Daniel Wagner wrote:
> Commit 88263208dd23 ("scsi: qla2xxx: Complain if sp->done() is not
> called from the completion path") introduced the WARN_ON_ONCE in
> qla2x00_status_cont_entry(). The assumption was that there is only one
> status continuations element. According to the firmware documentation
> it is possible that multiple status continuations are emitted by the
> firmware.
>
> Cc: Bart Van Assche
> Signed-off-by: Daniel Wagner
> ---
> drivers/scsi/qla2xxx/qla_isr.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
> index 4c26630c1c3e..009fd5a33fcd 100644
> --- a/drivers/scsi/qla2xxx/qla_isr.c
> +++ b/drivers/scsi/qla2xxx/qla_isr.c
> @@ -2837,8 +2837,6 @@ qla2x00_status_cont_entry(struct rsp_que *rsp,
> sts_cont_entry_t *pkt)
> if (sense_len == 0) {
> rsp->status_srb = NULL;
> sp->done(sp, cp->result);
> - } else {
> - WARN_ON_ONCE(true);
> }
> }
>
>
Not that I can speak for firmware documentation, but:
Reviewed-by: Hannes Reinecke
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
h...@suse.de +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 247165 (AG München), GF: Felix Imendörffer
Re: [PATCH v3 0/3] Add MMC software queue support
Hi Adrian and Ulf, On Thu, 19 Sep 2019 at 13:59, Baolin Wang wrote: > > Hi All, > > Now the MMC read/write stack will always wait for previous request is > completed by mmc_blk_rw_wait(), before sending a new request to hardware, > or queue a work to complete request, that will bring context switching > overhead, especially for high I/O per second rates, to affect the IO > performance. > > Thus this patch set will introduce the MMC software command queue support > based on command queue engine's interfaces, and set the queue depth as 2, > that means we do not need wait for previous request is completed and can > queue 2 requests in flight. It is enough to let the irq handler always > trigger the next request without a context switch and then ask the blk_mq > layer for the next one to get queued, as well as avoiding a long latency. > > Moreover we can expand the MMC software queue interface to support > MMC packed request or packed command instead of adding new interfaces, > according to previosus discussion. > > Below are some comparison data with fio tool. The fio command I used > is like below with changing the '--rw' parameter and enabling the direct > IO flag to measure the actual hardware transfer speed in 4K block size. > > ./fio --filename=/dev/mmcblk0p30 --direct=1 --iodepth=20 --rw=read --bs=4K > --size=512M --group_reporting --numjobs=20 --name=test_read > > My eMMC card working at HS400 Enhanced strobe mode: > [2.229856] mmc0: new HS400 Enhanced strobe MMC card at address 0001 > [2.237566] mmcblk0: mmc0:0001 HBG4a2 29.1 GiB > [2.242621] mmcblk0boot0: mmc0:0001 HBG4a2 partition 1 4.00 MiB > [2.249110] mmcblk0boot1: mmc0:0001 HBG4a2 partition 2 4.00 MiB > [2.255307] mmcblk0rpmb: mmc0:0001 HBG4a2 partition 3 4.00 MiB, chardev > (248:0) > > 1. Without MMC software queue > I tested 3 times for each case and output a average speed. > > 1) Sequential read: > Speed: 28.9MiB/s, 26.4MiB/s, 30.9MiB/s > Average speed: 28.7MiB/s > > 2) Random read: > Speed: 18.2MiB/s, 8.9MiB/s, 15.8MiB/s > Average speed: 14.3MiB/s > > 3) Sequential write: > Speed: 21.1MiB/s, 27.9MiB/s, 25MiB/s > Average speed: 24.7MiB/s > > 4) Random write: > Speed: 21.5MiB/s, 18.1MiB/s, 18.1MiB/s > Average speed: 19.2MiB/s > > 2. With MMC software queue > I tested 3 times for each case and output a average speed. > > 1) Sequential read: > Speed: 44.1MiB/s, 42.3MiB/s, 44.4MiB/s > Average speed: 43.6MiB/s > > 2) Random read: > Speed: 30.6MiB/s, 30.9MiB/s, 30.5MiB/s > Average speed: 30.6MiB/s > > 3) Sequential write: > Speed: 44.1MiB/s, 45.9MiB/s, 44.2MiB/s > Average speed: 44.7MiB/s > > 4) Random write: > Speed: 45.1MiB/s, 43.3MiB/s, 42.4MiB/s > Average speed: 43.6MiB/s > > Form above data, we can see the MMC software queue can help to improve the > performance obviously. > > Any comments are welcome. Thanks a lot. > > Changes from v2: > - Remove reference to 'struct cqhci_host' and 'struct cqhci_slot', > instead adding 'struct sqhci_host', which is only used by software queue. > > Changes from v1: > - Add request_done ops for sdhci_ops. > - Replace virtual command queue with software queue for functions and > variables. > - Rename the software queue file and add sqhci.h header file. Do you have any comments for this patch set except the random config building issue that will be fixed in the next version? Thanks. > > Baolin Wang (3): > mmc: Add MMC software queue support > mmc: host: sdhci: Add request_done ops for struct sdhci_ops > mmc: host: sdhci-sprd: Add software queue support > > drivers/mmc/core/block.c | 61 > drivers/mmc/core/mmc.c| 13 +- > drivers/mmc/core/queue.c | 25 ++- > drivers/mmc/host/Kconfig |9 ++ > drivers/mmc/host/Makefile |1 + > drivers/mmc/host/sdhci-sprd.c | 26 > drivers/mmc/host/sdhci.c | 12 +- > drivers/mmc/host/sdhci.h |2 + > drivers/mmc/host/sqhci.c | 344 > + > drivers/mmc/host/sqhci.h | 53 +++ > include/linux/mmc/host.h |3 + > 11 files changed, 537 insertions(+), 12 deletions(-) > create mode 100644 drivers/mmc/host/sqhci.c > create mode 100644 drivers/mmc/host/sqhci.h > > -- > 1.7.9.5 > -- Baolin Wang Best Regards
Re: [PATCH] compiler: enable CONFIG_OPTIMIZE_INLINING forcibly
Hi Geert,
On Thu, Sep 26, 2019 at 6:26 PM Geert Uytterhoeven wrote:
>
> Hi Yamada-san,
>
> On Thu, Sep 26, 2019 at 11:03 AM Masahiro Yamada
> wrote:
> > On Thu, Sep 26, 2019 at 5:54 PM Geert Uytterhoeven
> > wrote:
> > > On Fri, Aug 30, 2019 at 5:44 AM Masahiro Yamada
> > > wrote:
> > > > Commit 9012d011660e ("compiler: allow all arches to enable
> > > > CONFIG_OPTIMIZE_INLINING") allowed all architectures to enable
> > > > this option. A couple of build errors were reported by randconfig,
> > > > but all of them have been ironed out.
> > > >
> > > > Towards the goal of removing CONFIG_OPTIMIZE_INLINING entirely
> > > > (and it will simplify the 'inline' macro in compiler_types.h),
> > > > this commit changes it to always-on option. Going forward, the
> > > > compiler will always be allowed to not inline functions marked
> > > > 'inline'.
> > > >
> > > > This is not a problem for x86 since it has been long used by
> > > > arch/x86/configs/{x86_64,i386}_defconfig.
> > > >
> > > > I am keeping the config option just in case any problem crops up for
> > > > other architectures.
> > > >
> > > > The code clean-up will be done after confirming this is solid.
> > > >
> > > > Signed-off-by: Masahiro Yamada
> > >
> > > This breaks compiling drivers/video/fbdev/c2p*, as the functions in
> > > drivers/video/fbdev/c2p_core.h are no longer inlined, leading to calls
> > > to the non-existent function c2p_unsupported(), as reported by KISSKB:
> > >
> > > On Thu, Sep 26, 2019 at 5:02 AM wrote:
> > > > FAILED linux-next/m68k-defconfig/m68k Thu Sep 26, 12:58
> > > >
> > > > http://kisskb.ellerman.id.au/kisskb/buildresult/13973194/
> > > >
> > > > Commit: Add linux-next specific files for 20190925
> > > > d47175169c28eedd2cc2ab8c01f38764cb0269cc
> > > > Compiler: m68k-linux-gcc (GCC) 4.6.3 / GNU ld (GNU Binutils) 2.22
> > > >
> > > > Possible errors
> > > > ---
> > > >
> > > > c2p_planar.c:(.text+0xd6): undefined reference to `c2p_unsupported'
> > > > c2p_planar.c:(.text+0x1dc): undefined reference to `c2p_unsupported'
> > > > c2p_iplan2.c:(.text+0xc4): undefined reference to `c2p_unsupported'
> > > > c2p_iplan2.c:(.text+0x150): undefined reference to `c2p_unsupported'
> > > > make[1]: *** [Makefile:1074: vmlinux] Error 1
> > > > make: *** [Makefile:179: sub-make] Error 2
> > >
> > > I managed to reproduce this with gcc version 8.3.0 (Ubuntu
> > > 8.3.0-6ubuntu1~18.04.1) , and bisected the failure to commit
> > > 025f072e5823947c ("compiler: enable CONFIG_OPTIMIZE_INLINING forcibly") .
> > >
> > > Marking the functions __always_inline instead of inline fixes that.
> > > Shall I send a patch to do that?
> >
> >
> > Yes, please.
>
> OK, will do.
>
> > But you do not need to touch _transp() or comp().
> > Only functions that call c2p_unsupported().
>
> However, the inlining of these functions is very performance-sensitive too.
> So perhaps I should mark all of them __always_inline?
I want to leave as much compiler's freedom as possible,
and the optimization criteria depends on CONFIG option.
When CONFIG_CC_OPTIMIZE_FOR_SIZE is chosen,
the optimization should respect the code size over performance,
and the compiler may prefer not inlining it.
So, the basic idea is, use __always_inline
only when otherwise the builds would be broken.
Masahiro Yamada
> > BTW, c2p_unsupported() can be replaced with BUILD_BUG().
> > This will break the build earlier in case
> > it cannot be optimized out.
>
> Right. Will do, too.
>
> Thanks!
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> ge...@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like
> that.
> -- Linus Torvalds
--
Best Regards
Masahiro Yamada
Re: [PATCH] drm: Don't free jobs in wait_event_interruptible()
On 26/09/2019 08:07, Koenig, Christian wrote:
> Am 25.09.19 um 17:14 schrieb Steven Price:
>> drm_sched_cleanup_jobs() attempts to free finished jobs, however because
>> it is called as the condition of wait_event_interruptible() it must not
>> sleep. Unfortunately some free callbacks (notably for Panfrost) do sleep.
>>
>> Instead let's rename drm_sched_cleanup_jobs() to
>> drm_sched_get_cleanup_job() and simply return a job for processing if
>> there is one. The caller can then call the free_job() callback outside
>> the wait_event_interruptible() where sleeping is possible before
>> re-checking and returning to sleep if necessary.
>>
>> Signed-off-by: Steven Price
>> ---
>> drivers/gpu/drm/scheduler/sched_main.c | 44 ++
>> 1 file changed, 24 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/scheduler/sched_main.c
>> b/drivers/gpu/drm/scheduler/sched_main.c
>> index 9a0ee74d82dc..0ed4aaa4e6d1 100644
>> --- a/drivers/gpu/drm/scheduler/sched_main.c
>> +++ b/drivers/gpu/drm/scheduler/sched_main.c
>> @@ -622,43 +622,41 @@ static void drm_sched_process_job(struct dma_fence *f,
>> struct dma_fence_cb *cb)
>> }
>>
>> /**
>> - * drm_sched_cleanup_jobs - destroy finished jobs
>> + * drm_sched_get_cleanup_job - fetch the next finished job to be destroyed
>>*
>>* @sched: scheduler instance
>>*
>> - * Remove all finished jobs from the mirror list and destroy them.
>> + * Returns the next finished job from the mirror list (if there is one)
>> + * ready for it to be destroyed.
>>*/
>> -static void drm_sched_cleanup_jobs(struct drm_gpu_scheduler *sched)
>> +static struct drm_sched_job *
>> +drm_sched_get_cleanup_job(struct drm_gpu_scheduler *sched)
>> {
>> +struct drm_sched_job *job = NULL;
>> unsigned long flags;
>>
>> /* Don't destroy jobs while the timeout worker is running */
>> if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
>> !cancel_delayed_work(&sched->work_tdr))
>> -return;
>> -
>> -
>> -while (!list_empty(&sched->ring_mirror_list)) {
>> -struct drm_sched_job *job;
>> +return NULL;
>>
>> -job = list_first_entry(&sched->ring_mirror_list,
>> +job = list_first_entry_or_null(&sched->ring_mirror_list,
>> struct drm_sched_job, node);
>
> This is probably better done after taking the lock, apart from that
> looks good to me.
It wasn't previously protected by the lock - but I don't see any harm so
I'll post a v2.
Steve
Re: [PATCH v3 5/5] media: platform: Add jpeg dec/enc feature
On 9/24/19 9:43 AM, Xia Jiang wrote:
> Add mtk jpeg encode v4l2 driver based on jpeg decode, because that jpeg
> decode and encode have great similarities with function operation.
>
> Signed-off-by: Xia Jiang
> ---
> v3: delete Change-Id
> only test once handler->error after the last v4l2_ctrl_new_std()
> seperate changes of v4l2-ctrls.c and v4l2-controls.h to new patch
> v2: fix compliance test fail, check created buffer size in driver
> ---
> drivers/media/platform/mtk-jpeg/Makefile | 5 +-
> .../media/platform/mtk-jpeg/mtk_jpeg_core.c | 735 ++
> .../media/platform/mtk-jpeg/mtk_jpeg_core.h | 114 ++-
> .../media/platform/mtk-jpeg/mtk_jpeg_dec_hw.h | 7 +-
> .../media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c | 175 +
> .../media/platform/mtk-jpeg/mtk_jpeg_enc_hw.h | 60 ++
> .../platform/mtk-jpeg/mtk_jpeg_enc_reg.h | 49 ++
> 7 files changed, 975 insertions(+), 170 deletions(-)
> create mode 100644 drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c
> create mode 100644 drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.h
> create mode 100644 drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_reg.h
>
> diff --git a/drivers/media/platform/mtk-jpeg/Makefile
> b/drivers/media/platform/mtk-jpeg/Makefile
> index 48516dcf96e6..76c33aad0f3f 100644
> --- a/drivers/media/platform/mtk-jpeg/Makefile
> +++ b/drivers/media/platform/mtk-jpeg/Makefile
> @@ -1,3 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0-only
> -mtk_jpeg-objs := mtk_jpeg_core.o mtk_jpeg_dec_hw.o mtk_jpeg_dec_parse.o
> +mtk_jpeg-objs := mtk_jpeg_core.o \
> + mtk_jpeg_dec_hw.o \
> + mtk_jpeg_dec_parse.o \
> + mtk_jpeg_enc_hw.o
> obj-$(CONFIG_VIDEO_MEDIATEK_JPEG) += mtk_jpeg.o
> diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> index 5f0990fce8ef..aa18b01802ed 100644
> --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> @@ -3,6 +3,7 @@
> * Copyright (c) 2016 MediaTek Inc.
> * Author: Ming Hsiu Tsai
> * Rick Chang
> + * Xia Jiang
> */
>
> #include
> @@ -23,6 +24,7 @@
> #include
> #include
>
> +#include "mtk_jpeg_enc_hw.h"
> #include "mtk_jpeg_dec_hw.h"
> #include "mtk_jpeg_core.h"
> #include "mtk_jpeg_dec_parse.h"
> @@ -31,7 +33,8 @@ static struct mtk_jpeg_fmt mtk_jpeg_formats[] = {
> {
> .fourcc = V4L2_PIX_FMT_JPEG,
> .colplanes = 1,
> - .flags = MTK_JPEG_FMT_FLAG_DEC_OUTPUT,
> + .flags = MTK_JPEG_FMT_FLAG_DEC_OUTPUT |
> + MTK_JPEG_FMT_FLAG_ENC_CAPTURE,
> },
> {
> .fourcc = V4L2_PIX_FMT_YUV420M,
> @@ -51,6 +54,42 @@ static struct mtk_jpeg_fmt mtk_jpeg_formats[] = {
> .v_align= 3,
> .flags = MTK_JPEG_FMT_FLAG_DEC_CAPTURE,
> },
> + {
> + .fourcc = V4L2_PIX_FMT_NV12M,
> + .h_sample = {4, 2, 2},
> + .v_sample = {4, 2, 2},
> + .colplanes = 2,
> + .h_align= 4,
> + .v_align= 4,
> + .flags = MTK_JPEG_FMT_FLAG_ENC_OUTPUT,
> + },
> + {
> + .fourcc = V4L2_PIX_FMT_NV21M,
> + .h_sample = {4, 2, 2},
> + .v_sample = {4, 2, 2},
> + .colplanes = 2,
> + .h_align= 4,
> + .v_align= 4,
> + .flags = MTK_JPEG_FMT_FLAG_ENC_OUTPUT,
> + },
> + {
> + .fourcc = V4L2_PIX_FMT_YUYV,
> + .h_sample = {4, 2, 2},
> + .v_sample = {4, 4, 4},
> + .colplanes = 1,
> + .h_align= 4,
> + .v_align= 3,
> + .flags = MTK_JPEG_FMT_FLAG_ENC_OUTPUT,
> + },
> + {
> + .fourcc = V4L2_PIX_FMT_YVYU,
> + .h_sample = {4, 2, 2},
> + .v_sample = {4, 4, 4},
> + .colplanes = 1,
> + .h_align= 4,
> + .v_align= 3,
> + .flags = MTK_JPEG_FMT_FLAG_ENC_OUTPUT,
> + },
> };
>
> #define MTK_JPEG_NUM_FORMATS ARRAY_SIZE(mtk_jpeg_formats)
> @@ -65,11 +104,19 @@ struct mtk_jpeg_src_buf {
> struct list_head list;
> int flags;
> struct mtk_jpeg_dec_param dec_param;
> + struct mtk_jpeg_enc_param enc_param;
> };
>
> +#define MTK_MAX_CTRLS_HINT 20
> +
> static int debug;
> module_param(debug, int, 0644);
>
> +static inline struct mtk_jpeg_ctx *ctrl_to_ctx(struct v4l2_ctrl *ctrl)
> +{
> + return container_of(ctrl->handler, struct mtk_jpeg_ctx, ctrl_hdl);
> +}
> +
> static inline struct mtk_jpeg_ctx *mtk_jpeg_fh_to_ctx(struct v4l2_fh *fh)
> {
> return container_of(fh, struct mtk_jpeg
Re: [RFC PATCH] xen/gntdev: Stop abusing DT of_dma_configure API
Hi Rob, On 9/25/19 10:50 PM, Rob Herring wrote: As the comment says, this isn't a DT based device. of_dma_configure() is going to stop allowing a NULL DT node, so this needs to be fixed. And this can't work on arch not selecting CONFIG_OF and can select CONFIG_XEN_GRANT_DMA_ALLOC. We are lucky enough on x86 because, AFAICT, arch_setup_dma_ops is just a nop. Not sure exactly what setup besides arch_setup_dma_ops is needed... We probably want to update dma_mask, coherent_dma_mask and dma_pfn_offset. Also, while look at of_configure_dma, I noticed that we consider the DMA will not be coherent for the grant-table. Oleksandr, do you know why they can't be coherent? Cheers, -- Julien Grall
RE: [v4,1/3] soc: fsl: handle RCPM errata A-008646 on SoC LS1021A
Hi all,
the linux patch depended by RCPM driver,FlexTimer driver and FlexTimer dts,
need apply these patches as follows:
1. RCPM driver:
https://patchwork.kernel.org/series/162731/mbox/
(https://patchwork.kernel.org/patch/11105279/)
2. FlexTimer dts:
https://lore.kernel.org/patchwork/series/405653/mbox/
(https://lore.kernel.org/patchwork/patch/1112493/)
3. FlexTimer driver:
https://patchwork.ozlabs.org/series/124718/mbox/
(https://patchwork.ozlabs.org/patch/1145999/)
https://patchwork.ozlabs.org/series/126942/mbox/
(https://patchwork.ozlabs.org/patch/1152085/)
4. Adjust drivers/soc/fsl/Makefile:
remove the line 'obj-y += ftm_alarm.o' in drivers/soc/fsl/Makefile to
resolve a compilation error
> Description:
> - Reading configuration register RCPM_IPPDEXPCR1
> always return zero
>
> Workaround:
> - Save register RCPM_IPPDEXPCR1's value to
> register SCFG_SPARECR8.(uboot's psci also
> need reading value from the register SCFG_SPARECR8
> to set register RCPM_IPPDEXPCR1)
>
> Impact:
> - FlexTimer module will cannot wakeup system in
> deep sleep on SoC LS1021A
>
> Signed-off-by: Biwen Li
> ---
> Change in v4:
> - rename property name
> fsl,ippdexpcr-alt-addr -> fsl,ippdexpcr1-alt-addr
>
> Change in v3:
> - update commit message
> - rename property name
> fsl,rcpm-scfg -> fsl,ippdexpcr-alt-addr
>
> Change in v2:
> - fix stype problems
>
> drivers/soc/fsl/rcpm.c | 33 +
> 1 file changed, 33 insertions(+)
>
> diff --git a/drivers/soc/fsl/rcpm.c b/drivers/soc/fsl/rcpm.c index
> 82c0ad5e663e..9a29c482fc2e 100644
> --- a/drivers/soc/fsl/rcpm.c
> +++ b/drivers/soc/fsl/rcpm.c
> @@ -13,6 +13,8 @@
> #include
> #include
> #include
> +#include
> +#include
>
> #define RCPM_WAKEUP_CELL_MAX_SIZE7
>
> @@ -29,6 +31,9 @@ static int rcpm_pm_prepare(struct device *dev)
> struct rcpm *rcpm;
> u32 value[RCPM_WAKEUP_CELL_MAX_SIZE + 1], tmp;
> int i, ret, idx;
> + struct regmap *scfg_addr_regmap = NULL;
> + u32 reg_offset[RCPM_WAKEUP_CELL_MAX_SIZE + 1];
> + u32 reg_value = 0;
>
> rcpm = dev_get_drvdata(dev);
> if (!rcpm)
> @@ -63,6 +68,34 @@ static int rcpm_pm_prepare(struct device *dev)
> tmp |= value[i + 1];
> iowrite32be(tmp, rcpm->ippdexpcr_base +
> i * 4);
> }
> + /* Workaround of errata A-008646 on SoC LS1021A:
> + * There is a bug of register ippdexpcr1.
> + * Reading configuration register
> RCPM_IPPDEXPCR1
> + * always return zero. So save ippdexpcr1's
> value
> + * to register SCFG_SPARECR8.And the value of
> + * ippdexpcr1 will be read from SCFG_SPARECR8.
> + */
> + scfg_addr_regmap =
> syscon_regmap_lookup_by_phandle(np,
> +
> "fsl,ippdexpcr1-alt-addr");
> + if (scfg_addr_regmap && (1 == i)) {
> + if
> (of_property_read_u32_array(dev->of_node,
> + "fsl,ippdexpcr1-alt-addr",
> + reg_offset,
> + 1 + sizeof(u64)/sizeof(u32))) {
> + scfg_addr_regmap = NULL;
> + continue;
> + }
> + /* Read value from register
> SCFG_SPARECR8 */
> + regmap_read(scfg_addr_regmap,
> + (u32)(((u64)(reg_offset[1]
> << (sizeof(u32) *
> 8) |
> + reg_offset[2])) &
> 0x),
> + ®_value);
> + /* Write value to register
> SCFG_SPARECR8 */
> + regmap_write(scfg_addr_regmap,
> + (u32)(((u64)(reg_offset[1]
> << (sizeof(u32) *
> 8) |
> + reg_offset[2])) &
> 0x),
> + tmp | reg_value);
> + }
> }
> }
> } while (ws = wakeup_source_get_next(ws));
> --
> 2.17.1
re: spi: Add call to spi_slave_abort() function when spidev driver is released
Hi,
Static analysis with Coverity has detected an potential dereference of a
free'd object with commit:
commit 9f918a728cf86b2757b6a7025e1f46824bfe3155
Author: Lukasz Majewski
Date: Wed Sep 25 11:11:42 2019 +0200
spi: Add call to spi_slave_abort() function when spidev driver is
released
In spidev_release() in drivers/spi/spidev.c the analysis is as follows:
600static int spidev_release(struct inode *inode, struct file *filp)
601{
602struct spidev_data *spidev;
603
604mutex_lock(&device_list_lock);
1. alias: Assigning: spidev = filp->private_data. Now both point to
the same storage.
605spidev = filp->private_data;
606filp->private_data = NULL;
607
608/* last close? */
609spidev->users--;
2. Condition !spidev->users, taking true branch.
610if (!spidev->users) {
611int dofree;
612
613kfree(spidev->tx_buffer);
614spidev->tx_buffer = NULL;
615
616kfree(spidev->rx_buffer);
617spidev->rx_buffer = NULL;
618
619spin_lock_irq(&spidev->spi_lock);
3. Condition spidev->spi, taking false branch.
620if (spidev->spi)
621spidev->speed_hz = spidev->spi->max_speed_hz;
622
623/* ... after we unbound from the underlying device? */
4. Condition spidev->spi == NULL, taking true branch.
624dofree = (spidev->spi == NULL);
625spin_unlock_irq(&spidev->spi_lock);
626
5. Condition dofree, taking true branch.
627if (dofree)
6. freed_arg: kfree frees spidev.
628kfree(spidev);
629}
630#ifdef CONFIG_SPI_SLAVE
CID 89726 (#1 of 1): Read from pointer after free (USE_AFTER_FREE)
7. deref_after_free: Dereferencing freed pointer spidev.
631spi_slave_abort(spidev->spi);
632#endif
633mutex_unlock(&device_list_lock);
634
635return 0;
636}
The call to spi_slave_abort() on spidev is reading an earlier kfree'd
spidev.
Colin
Re: [PATCH v2 1/2] drm/komeda: Add line size support
Hi Lowry,
On Wed, Sep 25, 2019 at 10:24:58AM +, Liviu Dudau wrote:
> Hi Lowry,
>
> On Tue, Sep 24, 2019 at 08:00:44AM +, Lowry Li (Arm Technology China)
> wrote:
> > From: "Lowry Li (Arm Technology China)"
> >
> > On D71, we are using the global line size. From D32, every
> > component have a line size register to indicate the fifo size.
> >
> > So this patch is to set line size support and do the line size
> > check.
> >
> > Signed-off-by: Lowry Li (Arm Technology China)
> > ---
> > .../arm/display/komeda/d71/d71_component.c| 57 ---
> > .../gpu/drm/arm/display/komeda/d71/d71_regs.h | 9 +--
> > .../drm/arm/display/komeda/komeda_pipeline.h | 2 +
> > .../display/komeda/komeda_pipeline_state.c| 17 ++
> > 4 files changed, 70 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > index 7b374a3b911e..357837b9d6ed 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > @@ -106,6 +106,23 @@ static void dump_block_header(struct seq_file *sf,
> > void __iomem *reg)
> >i, hdr.output_ids[i]);
> > }
> >
> > +/* On D71, we are using the global line size. From D32, every component
> > have
> > + * a line size register to indicate the fifo size.
> > + */
> > +static u32 __get_blk_line_size(struct d71_dev *d71, u32 __iomem *reg,
> > + u32 max_default)
> > +{
> > + if (!d71->periph_addr)
> > + max_default = malidp_read32(reg, BLK_MAX_LINE_SIZE);
> > +
> > + return max_default;
> > +}
> > +
> > +static u32 get_blk_line_size(struct d71_dev *d71, u32 __iomem *reg)
> > +{
> > + return __get_blk_line_size(d71, reg, d71->max_line_size);
> > +}
>
> I know you're trying to save typing the extra parameter, but looking at the
> rest of
> the diff I think it would look better if you get rid of get_blk_line_size()
> function
> and use the name for the function with 3 parameters.
>
> Otherwise, patch looks good to me.
>
> Reviewed-by: Liviu Dudau
>
> Best regards,
> Liviu
Thanks for the comments.
But considering from D32 every component have a line size register
and no need default value, so we have get_blk_line_size() without
default argument and also can save some typing and lines. That's
why we want to keep __get_blk_line_size().
> > +
> > static u32 to_rot_ctrl(u32 rot)
> > {
> > u32 lr_ctrl = 0;
> > @@ -365,7 +382,28 @@ static int d71_layer_init(struct d71_dev *d71,
> > else
> > layer->layer_type = KOMEDA_FMT_SIMPLE_LAYER;
> >
> > - set_range(&layer->hsize_in, 4, d71->max_line_size);
> > + if (!d71->periph_addr) {
> > + /* D32 or newer product */
> > + layer->line_sz = malidp_read32(reg, BLK_MAX_LINE_SIZE);
> > + layer->yuv_line_sz = L_INFO_YUV_MAX_LINESZ(layer_info);
> > + } else if (d71->max_line_size > 2048) {
> > + /* D71 4K */
> > + layer->line_sz = d71->max_line_size;
> > + layer->yuv_line_sz = layer->line_sz / 2;
> > + } else {
> > + /* D71 2K */
> > + if (layer->layer_type == KOMEDA_FMT_RICH_LAYER) {
> > + /* rich layer is 4K configuration */
> > + layer->line_sz = d71->max_line_size * 2;
> > + layer->yuv_line_sz = layer->line_sz / 2;
> > + } else {
> > + layer->line_sz = d71->max_line_size;
> > + layer->yuv_line_sz = 0;
> > + }
> > + }
> > +
> > + set_range(&layer->hsize_in, 4, layer->line_sz);
> > +
> > set_range(&layer->vsize_in, 4, d71->max_vsize);
> >
> > malidp_write32(reg, LAYER_PALPHA, D71_PALPHA_DEF_MAP);
> > @@ -456,9 +494,11 @@ static int d71_wb_layer_init(struct d71_dev *d71,
> >
> > wb_layer = to_layer(c);
> > wb_layer->layer_type = KOMEDA_FMT_WB_LAYER;
> > + wb_layer->line_sz = get_blk_line_size(d71, reg);
> > + wb_layer->yuv_line_sz = wb_layer->line_sz;
> >
> > - set_range(&wb_layer->hsize_in, D71_MIN_LINE_SIZE, d71->max_line_size);
> > - set_range(&wb_layer->vsize_in, D71_MIN_VERTICAL_SIZE, d71->max_vsize);
> > + set_range(&wb_layer->hsize_in, 64, wb_layer->line_sz);
> > + set_range(&wb_layer->vsize_in, 64, d71->max_vsize);
> >
> > return 0;
> > }
> > @@ -595,8 +635,8 @@ static int d71_compiz_init(struct d71_dev *d71,
> >
> > compiz = to_compiz(c);
> >
> > - set_range(&compiz->hsize, D71_MIN_LINE_SIZE, d71->max_line_size);
> > - set_range(&compiz->vsize, D71_MIN_VERTICAL_SIZE, d71->max_vsize);
> > + set_range(&compiz->hsize, 64, get_blk_line_size(d71, reg));
> > + set_range(&compiz->vsize, 64, d71->max_vsize);
> >
> > return 0;
> > }
> > @@ -753,7 +793,7 @@ static int d71_scaler_init(struct d71_dev *d71,
> > }
> >
> > scaler = to_scaler(c);
> > - set_range(&scaler->hsize, 4, 2048);
>
Re: [PATCH V2 1/1] tick: broadcast-hrtimer: Fix a race in bc_set_next
On Wed, 25 Sep 2019, Balasubramani Vivekanandan wrote: > > Since it is now allowed to start the hrtimer from the callback, there is Is now allowed? > no need for the try to cancel logic. All that is now removed. Sure I can see that it is removed from the patch, but why and why is it correct? > [1]: rcu stall warnings noticed before this patch > > [ 26.477514] INFO: rcu_preempt detected stalls on CPUs/tasks: I which way is this backtrace giving any useful information about the problem? > > Signed-off-by: Balasubramani Vivekanandan > > Signed-off-by: Thomas Gleixner Interesting. You claim authorship on that patch and then you put my SOB after yours just because you feel entitled to do so? I surely appreciate the time you spent to analyze the problem and I wouldn't have said anything if you just would have done the right thing: 1) Write a changelog which explains why the change is actually correct 2) Not wreckage the formatting which I intentionally did for readability sake 3) Add 'Originally-by: Thomas Gleixner' or at least having the courtesy to mention that this is not your work. Thanks, tglx
Re: [PATCH] firmware: imx: Skip return value check for some special SCU firmware APIs
Hi Anson,
On 19-09-26 08:03, Anson Huang wrote:
> Hi, Marco
>
> > On 19-09-25 18:07, Anson Huang wrote:
> > > The SCU firmware does NOT always have return value stored in message
> > > header's function element even the API has response data, those
> > > special APIs are defined as void function in SCU firmware, so they
> > > should be treated as return success always.
> > >
> > > Signed-off-by: Anson Huang
> > > ---
> > > - This patch is based on the patch of
> > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc
> > >
> > hwork.kernel.org%2Fpatch%2F11129553%2F&data=02%7C01%7Canson.
> > huang%
> > >
> > 40nxp.com%7C1f4108cc25eb4618f43c08d742576fa3%7C686ea1d3bc2b4c6fa
> > 92cd99
> > >
> > c5c301635%7C0%7C0%7C637050815608963707&sdata=BZBg4cOR2rP%2
> > BRBNn15i
> > > Qq3%2FXBYwhuCLkgYzFRbfEgVU%3D&reserved=0
> > > ---
> > > drivers/firmware/imx/imx-scu.c | 34
> > > --
> > > 1 file changed, 32 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/firmware/imx/imx-scu.c
> > > b/drivers/firmware/imx/imx-scu.c index 869be7a..ced5b12 100644
> > > --- a/drivers/firmware/imx/imx-scu.c
> > > +++ b/drivers/firmware/imx/imx-scu.c
> > > @@ -78,6 +78,11 @@ static int imx_sc_linux_errmap[IMX_SC_ERR_LAST] =
> > {
> > > -EIO,/* IMX_SC_ERR_FAIL */
> > > };
> > >
> > > +static const struct imx_sc_rpc_msg whitelist[] = {
> > > + { .svc = IMX_SC_RPC_SVC_MISC, .func =
> > IMX_SC_MISC_FUNC_UNIQUE_ID },
> > > + { .svc = IMX_SC_RPC_SVC_MISC, .func =
> > > +IMX_SC_MISC_FUNC_GET_BUTTON_STATUS }, };
> >
> > Is this going to be extended in the near future? I see some upcoming
> > problems here if someone uses a different scu-fw<->kernel combination as
> > nxp would suggest.
>
> Could be, but I checked the current APIs, ONLY these 2 will be used in Linux
> kernel, so
> I ONLY add these 2 APIs for now.
Okay.
> However, after rethink, maybe we should add another imx_sc_rpc API for those
> special
> APIs? To avoid checking it for all the APIs called which may impact some
> performance.
> Still under discussion, if you have better idea, please advise, thanks!
Adding a special api shouldn't be the right fix. Imagine if someone (not
a nxp-developer) wants to add a new driver. How could he be expected to
know which api he should use. The better abbroach would be to fix the
scu-fw instead of adding quirks..
Regards,
Marco
>
> Anson
Re: [PATCH] x86: uaccess: fix regression in unsafe_get_user
On Mon, Feb 18, 2019 at 09:51:50PM +, Arthur Gautier wrote: > On Mon, Feb 18, 2019 at 11:15:44AM -0800, Andy Lutomirski wrote: > > This seems like it's just papering over the underlying problem: with > > Jann's new checks in place, strncpy_from_user() is simply buggy. Does > > the patch below look decent? It's only compile-tested, but it's > > conceptually straightforward. I was hoping I could get rid of the > > check-maximum-address stuff, but it's needed for architectures where > > the user range is adjacent to the kernel range (i.e. not x86_64). > > I'm unable to trigger the BUG I had with my initramfs with this patch > applied. Thanks! > Hello All, Just a followup on this issue, I'm still able to reproduce the original issue with: truncate -s 8388313 a SECONDFILENAME= truncate -s 10 $SECONDFILENAME echo "a\n$SECONDFILENAME" | cpio -o --format=newc | lz4 -l > initrd.img.lz4 I think Andy submitted a patch Feb 25 2019, but I was not copied on it (I believe it was sent to x...@kernel.org) and I don't know which fate it had. Any chance we could have a look again? Thanks a lot! -- \o/ Arthur G Gandi.net
Re: [PATCH v2] soc: xilinx: Set CAP_UNUSABLE requirement for versal while powering down domain
On 26. 08. 19 22:30, Jolly Shah wrote: > From: Tejas Patel > > For "0" requirement which is used to inform firmware that device is > not required currently by master, Versal PLM (Platform Loader and > Manager) which runs on Platform Management Controller and is responsible > platform management of devices that disables clock, power it down > and reset the device. genpd_power_off() is being called during runtime > suspend also. So, if any device goes to runtime suspend state during > resumes it needs to be re-initialized again. It is possible that > drivers do not reinitialize device upon resume from runtime suspend > every time ans so dont want it to be powered down or get reset > during runtime suspend. > > In Versal PLM new PM_CAP_UNUSABLE capability is added, which disables > clock only and avoids power down and reset during runtime suspend. Power > and reset will be gated with core suspend.So, this patch sets > CAPABILITY_UNUSABLE requirement during gpd_power_off() > if platform is other than zynqmp. > > Signed-off-by: Tejas Patel > Signed-off-by: Jolly Shah > --- Where is the version log? > drivers/soc/xilinx/zynqmp_pm_domains.c | 10 -- > include/linux/firmware/xlnx-zynqmp.h | 3 ++- > 2 files changed, 10 insertions(+), 3 deletions(-) > > diff --git a/drivers/soc/xilinx/zynqmp_pm_domains.c > b/drivers/soc/xilinx/zynqmp_pm_domains.c > index 600f57c..23d90cb 100644 > --- a/drivers/soc/xilinx/zynqmp_pm_domains.c > +++ b/drivers/soc/xilinx/zynqmp_pm_domains.c > @@ -2,7 +2,7 @@ > /* > * ZynqMP Generic PM domain support > * > - * Copyright (C) 2015-2018 Xilinx, Inc. > + * Copyright (C) 2015-2019 Xilinx, Inc. > * > * Davorin Mista > * Jolly Shah > @@ -25,6 +25,8 @@ > > static const struct zynqmp_eemi_ops *eemi_ops; > > +static int min_capability; > + > /** > * struct zynqmp_pm_domain - Wrapper around struct generic_pm_domain > * @gpd: Generic power domain > @@ -106,7 +108,7 @@ static int zynqmp_gpd_power_off(struct generic_pm_domain > *domain) > int ret; > struct pm_domain_data *pdd, *tmp; > struct zynqmp_pm_domain *pd; > - u32 capabilities = 0; > + u32 capabilities = min_capability; > bool may_wakeup; > > if (!eemi_ops->set_requirement) > @@ -283,6 +285,10 @@ static int zynqmp_gpd_probe(struct platform_device *pdev) > if (!domains) > return -ENOMEM; > > + if (!of_device_is_compatible(dev->parent->of_node, > + "xlnx,zynqmp-firmware")) > + min_capability = ZYNQMP_PM_CAPABILITY_UNUSABLE; I have not a problem with this patch but versal firmware is not wired in firmware yet that's why this code shouldn't be called at all. Can you please wire it? Thanks, Michal
Re: [PATCH] arm64: zynqmp: Add dr_mode property to usb node
On 10. 09. 19 17:17, Anurag Kumar Vulisha wrote:
> This patch adds dr_mode property to the usb node for
> zynqmp boards.
>
> Signed-off-by: Anurag Kumar Vulisha
> ---
> arch/arm64/boot/dts/xilinx/zynqmp-zc1751-xm015-dc1.dts | 1 +
> arch/arm64/boot/dts/xilinx/zynqmp-zc1751-xm016-dc2.dts | 1 +
> arch/arm64/boot/dts/xilinx/zynqmp-zcu100-revC.dts | 2 ++
> arch/arm64/boot/dts/xilinx/zynqmp-zcu102-revA.dts | 1 +
> arch/arm64/boot/dts/xilinx/zynqmp-zcu104-revA.dts | 1 +
> arch/arm64/boot/dts/xilinx/zynqmp-zcu106-revA.dts | 1 +
> arch/arm64/boot/dts/xilinx/zynqmp-zcu111-revA.dts | 1 +
> 7 files changed, 8 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp-zc1751-xm015-dc1.dts
> b/arch/arm64/boot/dts/xilinx/zynqmp-zc1751-xm015-dc1.dts
> index 9a3e39d..386f701 100644
> --- a/arch/arm64/boot/dts/xilinx/zynqmp-zc1751-xm015-dc1.dts
> +++ b/arch/arm64/boot/dts/xilinx/zynqmp-zc1751-xm015-dc1.dts
> @@ -128,4 +128,5 @@
> /* ULPI SMSC USB3320 */
> &usb0 {
> status = "okay";
> + dr_mode = "host";
> };
> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp-zc1751-xm016-dc2.dts
> b/arch/arm64/boot/dts/xilinx/zynqmp-zc1751-xm016-dc2.dts
> index 2421ec7..c6c8e2c 100644
> --- a/arch/arm64/boot/dts/xilinx/zynqmp-zc1751-xm016-dc2.dts
> +++ b/arch/arm64/boot/dts/xilinx/zynqmp-zc1751-xm016-dc2.dts
> @@ -158,6 +158,7 @@
> /* ULPI SMSC USB3320 */
> &usb1 {
> status = "okay";
> + dr_mode = "host";
> };
>
> &uart0 {
> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp-zcu100-revC.dts
> b/arch/arm64/boot/dts/xilinx/zynqmp-zcu100-revC.dts
> index e5699d0..c64c7b8 100644
> --- a/arch/arm64/boot/dts/xilinx/zynqmp-zcu100-revC.dts
> +++ b/arch/arm64/boot/dts/xilinx/zynqmp-zcu100-revC.dts
> @@ -274,11 +274,13 @@
> /* ULPI SMSC USB3320 */
> &usb0 {
> status = "okay";
> + dr_mode = "peripheral";
> };
>
> /* ULPI SMSC USB3320 */
> &usb1 {
> status = "okay";
> + dr_mode = "host";
> };
>
> &watchdog0 {
> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp-zcu102-revA.dts
> b/arch/arm64/boot/dts/xilinx/zynqmp-zcu102-revA.dts
> index 2a3b665..011f138 100644
> --- a/arch/arm64/boot/dts/xilinx/zynqmp-zcu102-revA.dts
> +++ b/arch/arm64/boot/dts/xilinx/zynqmp-zcu102-revA.dts
> @@ -540,6 +540,7 @@
> /* ULPI SMSC USB3320 */
> &usb0 {
> status = "okay";
> + dr_mode = "host";
> };
>
> &watchdog0 {
> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp-zcu104-revA.dts
> b/arch/arm64/boot/dts/xilinx/zynqmp-zcu104-revA.dts
> index 8f45614..7e372b1 100644
> --- a/arch/arm64/boot/dts/xilinx/zynqmp-zcu104-revA.dts
> +++ b/arch/arm64/boot/dts/xilinx/zynqmp-zcu104-revA.dts
> @@ -189,6 +189,7 @@
> /* ULPI SMSC USB3320 */
> &usb0 {
> status = "okay";
> + dr_mode = "host";
> };
>
> &watchdog0 {
> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp-zcu106-revA.dts
> b/arch/arm64/boot/dts/xilinx/zynqmp-zcu106-revA.dts
> index 93ce7eb..c4aac7a 100644
> --- a/arch/arm64/boot/dts/xilinx/zynqmp-zcu106-revA.dts
> +++ b/arch/arm64/boot/dts/xilinx/zynqmp-zcu106-revA.dts
> @@ -514,6 +514,7 @@
> /* ULPI SMSC USB3320 */
> &usb0 {
> status = "okay";
> + dr_mode = "host";
> };
>
> &watchdog0 {
> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp-zcu111-revA.dts
> b/arch/arm64/boot/dts/xilinx/zynqmp-zcu111-revA.dts
> index 8bb0001..4909533 100644
> --- a/arch/arm64/boot/dts/xilinx/zynqmp-zcu111-revA.dts
> +++ b/arch/arm64/boot/dts/xilinx/zynqmp-zcu111-revA.dts
> @@ -440,4 +440,5 @@
> /* ULPI SMSC USB3320 */
> &usb0 {
> status = "okay";
> + dr_mode = "host";
> };
>
Applied.
M
Re: [Xen-devel] [PATCH] xen: xen-pciback: Reset MSI-X state when exposing a device
Hello Stanislav,
On Fri, Sep 13, 2019 at 11:28:20PM +0800, Chao Gao wrote:
> On Fri, Sep 13, 2019 at 10:02:24AM +, Spassov, Stanislav wrote:
> >On Thu, Dec 13, 2018 at 07:54, Chao Gao wrote:
> >>On Thu, Dec 13, 2018 at 12:54:52AM -0700, Jan Beulich wrote:
> >> On 13.12.18 at 04:46, wrote:
> On Wed, Dec 12, 2018 at 08:21:39AM -0700, Jan Beulich wrote:
> On 12.12.18 at 16:18, wrote:
> >> On Wed, Dec 12, 2018 at 01:51:01AM -0700, Jan Beulich wrote:
> >> On 12.12.18 at 08:06, wrote:
> On Wed, Dec 05, 2018 at 09:01:33AM -0500, Boris Ostrovsky wrote:
> >On 12/5/18 4:32 AM, Roger Pau Monné wrote:
> >> On Wed, Dec 05, 2018 at 10:19:17AM +0800, Chao Gao wrote:
> >>> I find some pass-thru devices don't work any more across guest
> >>> reboot.
> >>> Assigning it to another guest also meets the same issue. And the
> >>> only
> >>> way to make it work again is un-binding and binding it to pciback.
> >>> Someone reported this issue one year ago [1]. More detail also
> >>> can be
> >>> found in [2].
> >>>
> >>> The root-cause is Xen's internal MSI-X state isn't reset properly
> >>> during reboot or re-assignment. In the above case, Xen set
> >>> maskall bit
> >>> to mask all MSI interrupts after it detected a potential security
> >>> issue. Even after device reset, Xen didn't reset its internal
> >>> maskall
> >>> bit. As a result, maskall bit would be set again in next write to
> >>> MSI-X message control register.
> >>>
> >>> Given that PHYSDEVOPS_prepare_msix() also triggers Xen resetting
> >>> MSI-X
> >>> internal state of a device, we employ it to fix this issue rather
> >>> than
> >>> introducing another dedicated sub-hypercall.
> >>>
> >>> Note that PHYSDEVOPS_release_msix() will fail if the mapping
> >>> between
> >>> the device's msix and pirq has been created. This limitation
> >>> prevents
> >>> us calling this function when detaching a device from a guest
> >>> during
> >>> guest shutdown. Thus it is called right before calling
> >>> PHYSDEVOPS_prepare_msix().
> >> s/PHYSDEVOPS/PHYSDEVOP/ (no final S). And then I would also drop
> >> the
> >> () at the end of the hypercall name since it's not a function.
> >>
> >> I'm also wondering why the release can't be done when the device is
> >> detached from the guest (or the guest has been shut down). This
> >> makes
> >> me worry about the raciness of the attach/detach procedure: if
> >> there's
> >> a state where pciback assumes the device has been detached from the
> >> guest, but there are still pirqs bound, an attempt to attach to
> >> another guest in such state will fail.
> >
> >I wonder whether this additional reset functionality could be done
> >out
> >of xen_pcibk_xenbus_remove(). We first do a (best effort) device
> >reset
> >and then do the extra things that are not properly done there.
>
> No. It cannot be done in xen_pcibk_xenbus_remove() without modifying
> the handler of PHYSDEVOP_release_msix. To do a successful Xen
> internal
> MSI-X state reset, PHYSDEVOP_{release, prepare}_msix should be
> finished
> without error. But ATM, xen expects that no msi is bound to pirq when
> doing PHYSDEVOP_release_msix. Otherwise it fails with error code
> -EBUSY.
> However, the expectation isn't guaranteed in
> xen_pcibk_xenbus_remove().
> In some cases, if qemu fails to unmap MSIs, MSIs are unmapped by Xen
> at last minute, which happens after device reset in
> xen_pcibk_xenbus_remove().
> >>>
> >>>But that may need taking care of: I don't think it is a good idea to
> >>>have
> >>>anything left from the prior owning domain when the device gets reset.
> >>>I.e. left over IRQ bindings should perhaps be forcibly cleared before
> >>>invoking the reset;
> >>
> >> Agree. How about pciback to track the established IRQ bindings? Then
> >> pciback can clear irq binding before invoking the reset.
> >
> >How would pciback even know of those mappings, when it's qemu
> >who establishes (and manages) them?
>
> I meant to expose some interfaces from pciback. And pciback serves
> as the proxy of IRQ (un)binding APIs.
> >>>
> >>>If at all possible we should avoid having to change more parties (qemu,
> >>>libxc, kernel, hypervisor) than really necessary. Remember that such
> >>>a bug fix may want backporting, and making sure affected people have
> >>>all relevant components updated is increasingly difficult with their
>
Re: spi: Add call to spi_slave_abort() function when spidev driver is released
Hi Colin,
> Hi,
>
> Static analysis with Coverity has detected an potential dereference
> of a free'd object with commit:
>
> commit 9f918a728cf86b2757b6a7025e1f46824bfe3155
> Author: Lukasz Majewski
> Date: Wed Sep 25 11:11:42 2019 +0200
>
> spi: Add call to spi_slave_abort() function when spidev driver is
> released
>
> In spidev_release() in drivers/spi/spidev.c the analysis is as
> follows:
>
> 600static int spidev_release(struct inode *inode, struct file *filp)
> 601{
> 602struct spidev_data *spidev;
> 603
> 604mutex_lock(&device_list_lock);
>
>1. alias: Assigning: spidev = filp->private_data. Now both point to
> the same storage.
>
> 605spidev = filp->private_data;
> 606filp->private_data = NULL;
> 607
> 608/* last close? */
> 609spidev->users--;
>
>2. Condition !spidev->users, taking true branch.
>
> 610if (!spidev->users) {
> 611int dofree;
> 612
> 613kfree(spidev->tx_buffer);
> 614spidev->tx_buffer = NULL;
> 615
> 616kfree(spidev->rx_buffer);
> 617spidev->rx_buffer = NULL;
> 618
> 619spin_lock_irq(&spidev->spi_lock);
>
>3. Condition spidev->spi, taking false branch.
>
> 620if (spidev->spi)
> 621spidev->speed_hz =
> spidev->spi->max_speed_hz; 622
> 623/* ... after we unbound from the underlying
> device? */
>
>4. Condition spidev->spi == NULL, taking true branch.
>
> 624dofree = (spidev->spi == NULL);
> 625spin_unlock_irq(&spidev->spi_lock);
> 626
>
>5. Condition dofree, taking true branch.
>
> 627if (dofree)
>
>6. freed_arg: kfree frees spidev.
>
> 628kfree(spidev);
> 629}
> 630#ifdef CONFIG_SPI_SLAVE
>
>CID 89726 (#1 of 1): Read from pointer after free (USE_AFTER_FREE)
> 7. deref_after_free: Dereferencing freed pointer spidev.
>
> 631spi_slave_abort(spidev->spi);
> 632#endif
> 633mutex_unlock(&device_list_lock);
> 634
> 635return 0;
> 636}
>
> The call to spi_slave_abort() on spidev is reading an earlier kfree'd
> spidev.
Thanks for spotting this issue - indeed there is a possibility to use
spidev after being kfree'd.
However, Geert (CC'ed) had some questions about placement of this
function call, so I will wait with providing fix until he replies.
>
> Colin
>
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lu...@denx.de
pgpw_5wPFj_eM.pgp
Description: OpenPGP digital signature
Re: spi: Add call to spi_slave_abort() function when spidev driver is released
On 26/09/2019 11:14, Lukasz Majewski wrote:
> Hi Colin,
>
>> Hi,
>>
>> Static analysis with Coverity has detected an potential dereference
>> of a free'd object with commit:
>>
>> commit 9f918a728cf86b2757b6a7025e1f46824bfe3155
>> Author: Lukasz Majewski
>> Date: Wed Sep 25 11:11:42 2019 +0200
>>
>> spi: Add call to spi_slave_abort() function when spidev driver is
>> released
>>
>> In spidev_release() in drivers/spi/spidev.c the analysis is as
>> follows:
>>
>> 600static int spidev_release(struct inode *inode, struct file *filp)
>> 601{
>> 602struct spidev_data *spidev;
>> 603
>> 604mutex_lock(&device_list_lock);
>>
>>1. alias: Assigning: spidev = filp->private_data. Now both point to
>> the same storage.
>>
>> 605spidev = filp->private_data;
>> 606filp->private_data = NULL;
>> 607
>> 608/* last close? */
>> 609spidev->users--;
>>
>>2. Condition !spidev->users, taking true branch.
>>
>> 610if (!spidev->users) {
>> 611int dofree;
>> 612
>> 613kfree(spidev->tx_buffer);
>> 614spidev->tx_buffer = NULL;
>> 615
>> 616kfree(spidev->rx_buffer);
>> 617spidev->rx_buffer = NULL;
>> 618
>> 619spin_lock_irq(&spidev->spi_lock);
>>
>>3. Condition spidev->spi, taking false branch.
>>
>> 620if (spidev->spi)
>> 621spidev->speed_hz =
>> spidev->spi->max_speed_hz; 622
>> 623/* ... after we unbound from the underlying
>> device? */
>>
>>4. Condition spidev->spi == NULL, taking true branch.
>>
>> 624dofree = (spidev->spi == NULL);
>> 625spin_unlock_irq(&spidev->spi_lock);
>> 626
>>
>>5. Condition dofree, taking true branch.
>>
>> 627if (dofree)
>>
>>6. freed_arg: kfree frees spidev.
>>
>> 628kfree(spidev);
>> 629}
>> 630#ifdef CONFIG_SPI_SLAVE
>>
>>CID 89726 (#1 of 1): Read from pointer after free (USE_AFTER_FREE)
>> 7. deref_after_free: Dereferencing freed pointer spidev.
>>
>> 631spi_slave_abort(spidev->spi);
>> 632#endif
>> 633mutex_unlock(&device_list_lock);
>> 634
>> 635return 0;
>> 636}
>>
>> The call to spi_slave_abort() on spidev is reading an earlier kfree'd
>> spidev.
>
> Thanks for spotting this issue - indeed there is a possibility to use
> spidev after being kfree'd.
>
> However, Geert (CC'ed) had some questions about placement of this
> function call, so I will wait with providing fix until he replies.
Cool, thanks for the update.
Colin
>
>>
>> Colin
>>
>
>
> Best regards,
>
> Lukasz Majewski
>
> --
>
> DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lu...@denx.de
>
signature.asc
Description: OpenPGP digital signature
RE: [PATCH] sample/hw_breakpoint: avoid sample hw_breakpoint recursion for arm/arm64
-Original Message- From: Peter Zijlstra [mailto:pet...@infradead.org] Sent: Thursday, September 26, 2019 5:14 PM To: wangxu (AE) Cc: mi...@redhat.com; a...@kernel.org; mark.rutl...@arm.com; alexander.shish...@linux.intel.com; namhy...@kernel.org; gre...@linuxfoundation.org; t...@linutronix.de; rfont...@redhat.com; alli...@lohutok.net; linux-kernel@vger.kernel.org Subject: Re: [PATCH] sample/hw_breakpoint: avoid sample hw_breakpoint recursion for arm/arm64 On Mon, Sep 23, 2019 at 04:09:35PM +0800, wangxu wrote: > From: Wang Xu > > For x86/ppc, hw_breakpoint is triggered after the instruction is > executed. > > For arm/arm64, which is triggered before the instruction executed. > Arm/arm64 skips the instruction by using single step. But it only > supports default overflow_handler. Where is the recusion.. ? For arm/arm64, hw_breakpoint is triggered before the instruction executed. When instruction_A is triggered, watchpoint_handler() will deal with this exception, and after return instruction_A will be triggerd ... One using samples/hw_breakpoint/data_breakpoint.c in arm/arm64 will meet this problem. > This patch provides a chance to avoid sample hw_breakpoint recursion > for arm/arm64 by adding 'struct perf_event_attr.bp_step'. This patch also lacks justification for why this needs to come with ABI changes. There is also a distinct lack of comments. I agree too. but have no better idea... This problem is really a big pit, especially for one not familiar with implementation differences in hw breakpoint for different architectures. > Signed-off-by: Wang Xu > --- > include/linux/perf_event.h | 3 +++ > include/uapi/linux/perf_event.h | 3 ++- > samples/hw_breakpoint/data_breakpoint.c | 1 + > 3 files changed, 6 insertions(+), 1 deletion(-) > > diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h > index 61448c1..f270eb7 100644 > --- a/include/linux/perf_event.h > +++ b/include/linux/perf_event.h > @@ -1024,6 +1024,9 @@ extern int perf_event_output(struct perf_event *event, > return true; > if (unlikely(event->overflow_handler == perf_event_output_backward)) > return true; > + /* avoid sample hw_breakpoint recursion */ > + if (unlikely(event->attr.bp_step)) > + return true; This is just _wrong_.. it says that every event with bp_step set always is a 'default overflow handler', irrespective of what the overflow handler actually is. Thanks for comments. Function is_default_overflow_handler() was introduced in 1879445dfa7bbd6fe21b09c5cc72f4934798afed , which is only be called in arch/arm[64]/kernel/hw_breakpoint.c, and will never be used in other arch/ (I think). But keeping is_default_overflow_handler() unchanged, changing ' if (is_default_overflow_handler(bp)) ' to ' if (is_default_overflow_handler(bp) || unlikely(event->attr.bp_step) ) ' will be better in arch/arm[64]/kernel/hw_breakpoint.c. > return false; > } >
Re: [RFC PATCH] xen/gntdev: Stop abusing DT of_dma_configure API
On 9/26/19 12:49 PM, Julien Grall wrote: > Hi Rob, > > > On 9/25/19 10:50 PM, Rob Herring wrote: >> As the comment says, this isn't a DT based device. of_dma_configure() >> is going to stop allowing a NULL DT node, so this needs to be fixed. > > And this can't work on arch not selecting CONFIG_OF and can select > CONFIG_XEN_GRANT_DMA_ALLOC. > > We are lucky enough on x86 because, AFAICT, arch_setup_dma_ops is just > a nop. > No luck is needed as [1] does nothing for those platforms not using CONFIG_OF >> >> Not sure exactly what setup besides arch_setup_dma_ops is needed... > > We probably want to update dma_mask, coherent_dma_mask and > dma_pfn_offset. > > Also, while look at of_configure_dma, I noticed that we consider the > DMA will not be coherent for the grant-table. Oleksandr, do you know > why they can't be coherent? The main and the only reason to use of_configure_dma is that if we don't then we are about to stay with dma_dummy_ops [2]. It effectively means that operations on dma-bufs will end up returning errors, like [3], [4], thus not making it possible for Xen PV DRM and DMA part of gntdev driver to do what we need (dma-bufs in our use-cases allow zero-copying while using graphics buffers and many more). I didn't find any better way of achieving that, but of_configure_dma... If there is any better solution which will not break the existing functionality then I will definitely change the drivers so we do not abuse DT ) Before that, please keep in mind that merging this RFC will break Xen PV DRM + DMA buf support in gntdev... Hope we can work out some acceptable solution, so everyone is happy > > Cheers, > Thank you, Oleksandr [1] https://elixir.bootlin.com/linux/v5.3.1/source/include/linux/of_device.h#L109 [2] https://elixir.bootlin.com/linux/v5.3.1/source/kernel/dma/dummy.c#L33 [3] https://elixir.bootlin.com/linux/v5.3.1/source/kernel/dma/dummy.c#L11 [4] https://elixir.bootlin.com/linux/v5.3.1/source/kernel/dma/dummy.c#L18
RE: [PATCH 2/5] dt-bindings: mfd: da9062: add regulator voltage selection documentation
On 26 September 2019 09:10, Marco Felsch wrote: > On 19-09-25 16:18, Adam Thomson wrote: > > On 25 September 2019 16:52, Marco Felsch wrote: > > > > > Hi Adam, > > > > > > On 19-09-24 09:23, Adam Thomson wrote: > > > > On 17 September 2019 13:43, Marco Felsch wrote: > > > > > > > > > Add the documentation which describe the voltage selection gpio > support. > > > > > This property can be applied to each subnode within the 'regulators' > > > > > node so each regulator can be configured differently. > > > > > > > > > > Signed-off-by: Marco Felsch > > > > > --- > > > > > Documentation/devicetree/bindings/mfd/da9062.txt | 9 + > > > > > 1 file changed, 9 insertions(+) > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/mfd/da9062.txt > > > > > b/Documentation/devicetree/bindings/mfd/da9062.txt > > > > > index edca653a5777..9d9820d8177d 100644 > > > > > --- a/Documentation/devicetree/bindings/mfd/da9062.txt > > > > > +++ b/Documentation/devicetree/bindings/mfd/da9062.txt > > > > > @@ -66,6 +66,15 @@ Sub-nodes: > > > > >details of individual regulator device can be found in: > > > > >Documentation/devicetree/bindings/regulator/regulator.txt > > > > > > > > > > + Optional regulator device-specific properties: > > > > > + - dlg,vsel-sense-gpios : The GPIO reference which should be used by > the > > > > > +regulator to switch the voltage between active/suspend voltage > settings. > > > If > > > > > +the signal is active the active-settings are applied else the > > > > > suspend > > > > > +settings are applied. Attention: Sharing the same gpio for other > purposes > > > > > +or across multiple regulators is possible but the gpio settings > > > > > must be > the > > > > > +same. Also the gpio phandle must refer to to the dlg,da9062-gpio > device > > > > > +other gpios are not allowed and make no sense. > > > > > + > > > > > > > > Should we not use the binding names that are defined in 'gpio- > regulator.yaml' > > > as > > > > these seem to be generic and would probably serve the purpose here? > > > > > > Hm.. as the description says: > > > > > > 8<-- > > > gpios: > > >description: Array of one or more GPIO pins used to select the > > >regulator voltage/current listed in "states". > > > 8<-- > > > > > > But we don't have a "states" property and we can't select between > > > voltage or current. > > > > Yes I think I was at cross purposes when I made this remark. The bindings > > there > > describe the GPOs that are used to enable/disable and set voltage/current > > for > > regulators and the supported voltage/current levels that can be configured > > in > > this manner. What you're describing is the GPI for DA9061/2. If you look at > > GPIO handling in existing regulator drivers I believe they all deal with > > external > > GPOs that are configured to enable/disable and set voltage/current limits > rather > > than the GPI on the PMIC itself. That's why I'm thinking that the > > configurations > > you're doing here should actually be in a pinctrl or GPIO driver. > > That's true, the common gpio bindings are from the view of the > processor, e.g. which gpio must the processor drive to enable/switch the > regualtor. So one reasone more to use a non-common binding. > > Please take a look on my other comment I made :) I don't use the > gpio-alternative function. I use it as an input. I know in the datasheet this isn't marked as an alternate function specifically but to me having regulator control by the chip's own GPI is an alternative function for that GPIO pin, in the same way a specific pin can be used for SYS_EN or Watchdog control. It's a dedicated purpose rather than being a normal GPI. See the following as an example of what I'm suggesting: https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/pinctrl/pinctrl-palmas.txt You could then pass the pinctrl information to the regulator driver and use that rather than having device specific bindings for this. That's at least my current interpretation of this anyway. > > Regards, > Marco > > > > I'd be interested in hearing Mark's view on this though as he has far more > > experience in this area than I do. > > > > > > > > Regards, > > > Marco > > > > > > > > - rtc : This node defines settings required for the Real-Time Clock > associated > > > > >with the DA9062. There are currently no entries in this binding, > > > > > however > > > > >compatible = "dlg,da9062-rtc" should be added if a node is created. > > > > > -- > > > > > 2.20.1 > > > > > > > > > > > > > > -- > > > Pengutronix e.K. | | > > > Industrial Linux Solutions | http://www.pengutronix.de/ | > > > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| > > > Amtsgericht Hildesheim, HRA 2686 | Fax:
[PATCH] [media] MAINTAINERS: Update MAX2175 & R-Car DRIF driver maintainer email
Ramesh is now using a new email address. Update the maintainer entry for the MAX2175 SDR tuner and the Renesas R-Car DRIF drivers. Signed-off-by: Chris Paterson --- MAINTAINERS | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 37dca6d70d87..948612fcf60b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -9873,7 +9873,7 @@ F:Documentation/hwmon/max16065.rst F: drivers/hwmon/max16065.c MAX2175 SDR TUNER DRIVER -M: Ramesh Shanmugasundaram +M: Ramesh Shanmugasundaram L: linux-me...@vger.kernel.org T: git git://linuxtv.org/media_tree.git S: Maintained @@ -10135,7 +10135,7 @@ F: drivers/media/platform/renesas-ceu.c F: include/media/drv-intf/renesas-ceu.h MEDIA DRIVERS FOR RENESAS - DRIF -M: Ramesh Shanmugasundaram +M: Ramesh Shanmugasundaram L: linux-me...@vger.kernel.org L: linux-renesas-...@vger.kernel.org T: git git://linuxtv.org/media_tree.git -- 2.17.1
Re: [PATCH v3 3/4] mm: don't expose non-hugetlb page to fast gup prematurely
On Wed, Sep 25, 2019 at 04:26:54PM -0600, Yu Zhao wrote: > On Wed, Sep 25, 2019 at 10:25:30AM +0200, Peter Zijlstra wrote: > > On Tue, Sep 24, 2019 at 05:24:58PM -0600, Yu Zhao wrote: > > > We don't want to expose a non-hugetlb page to the fast gup running > > > on a remote CPU before all local non-atomic ops on the page flags > > > are visible first. > > > > > > For an anon page that isn't in swap cache, we need to make sure all > > > prior non-atomic ops, especially __SetPageSwapBacked() in > > > page_add_new_anon_rmap(), are ordered before set_pte_at() to prevent > > > the following race: > > > > > > CPU 1 CPU1 > > > set_pte_at()get_user_pages_fast() > > > page_add_new_anon_rmap()gup_pte_range() > > > __SetPageSwapBacked() SetPageReferenced() > > > > > > This demonstrates a non-fatal scenario. Though haven't been directly > > > observed, the fatal ones can exist, e.g., PG_lock set by fast gup > > > caller and then overwritten by __SetPageSwapBacked(). > > > > > > For an anon page that is already in swap cache or a file page, we > > > don't need smp_wmb() before set_pte_at() because adding to swap or > > > file cach serves as a valid write barrier. Using non-atomic ops > > > thereafter is a bug, obviously. > > > > > > smp_wmb() is added following 11 of total 12 page_add_new_anon_rmap() > > > call sites, with the only exception being > > > do_huge_pmd_wp_page_fallback() because of an existing smp_wmb(). > > > > > > > I'm thinking this patch make stuff rather fragile.. Should we instead > > stick the barrier in set_p*d_at() instead? Or rather, make that store a > > store-release? > > I prefer it this way too, but I suspected the majority would be > concerned with the performance implications, especially those > looping set_pte_at()s in mm/huge_memory.c. We can rename current set_pte_at() to __set_pte_at() or something and leave it in places where barrier is not needed. The new set_pte_at()( will be used in the rest of the places with the barrier inside. BTW, have you looked at other levels of page table hierarchy. Do we have the same issue for PMD/PUD/... pages? -- Kirill A. Shutemov
Re: [PATCH v3 2/2] x86/boot/64: round memory hole size up to next PMD page.
On Tue, Sep 24, 2019 at 04:04:31PM -0500, Steve Wahl wrote: > The kernel image map is created using PMD pages, which can include > some extra space beyond what's actually needed. Round the size of the > memory hole we search for up to the next PMD boundary, to be certain > all of the space to be mapped is usable RAM and includes no reserved > areas. > > Signed-off-by: Steve Wahl > Cc: sta...@vger.kernel.org Acked-by: Kirill A. Shutemov -- Kirill A. Shutemov
Re: [PATCH v3 1/2] x86/boot/64: Make level2_kernel_pgt pages invalid outside kernel area.
On Tue, Sep 24, 2019 at 04:03:55PM -0500, Steve Wahl wrote: > Our hardware (UV aka Superdome Flex) has address ranges marked > reserved by the BIOS. Access to these ranges is caught as an error, > causing the BIOS to halt the system. > > Initial page tables mapped a large range of physical addresses that > were not checked against the list of BIOS reserved addresses, and > sometimes included reserved addresses in part of the mapped range. > Including the reserved range in the map allowed processor speculative > accesses to the reserved range, triggering a BIOS halt. > > Used early in booting, the page table level2_kernel_pgt addresses 1 > GiB divided into 2 MiB pages, and it was set up to linearly map a full > 1 GiB of physical addresses that included the physical address range > of the kernel image, as chosen by KASLR. But this also included a > large range of unused addresses on either side of the kernel image. > And unlike the kernel image's physical address range, this extra > mapped space was not checked against the BIOS tables of usable RAM > addresses. So there were times when the addresses chosen by KASLR > would result in processor accessible mappings of BIOS reserved > physical addresses. > > The kernel code did not directly access any of this extra mapped > space, but having it mapped allowed the processor to issue speculative > accesses into reserved memory, causing system halts. > > This was encountered somewhat rarely on a normal system boot, and much > more often when starting the crash kernel if "crashkernel=512M,high" > was specified on the command line (this heavily restricts the physical > address of the crash kernel, in our case usually within 1 GiB of > reserved space). > > The solution is to invalidate the pages of this table outside the > kernel image's space before the page table is activated. This patch > has been validated to fix this problem on our hardware. > > Signed-off-by: Steve Wahl > Cc: sta...@vger.kernel.org Acked-by: Kirill A. Shutemov -- Kirill A. Shutemov
Re: [PATCH v9 3/3] mm: fix double page fault on arm64 if PTE_AF is cleared
On Wed, Sep 25, 2019 at 10:59:22AM +0800, Jia He wrote: > When we tested pmdk unit test [1] vmmalloc_fork TEST1 in arm64 guest, there > will be a double page fault in __copy_from_user_inatomic of cow_user_page. > > Below call trace is from arm64 do_page_fault for debugging purpose > [ 110.016195] Call trace: > [ 110.016826] do_page_fault+0x5a4/0x690 > [ 110.017812] do_mem_abort+0x50/0xb0 > [ 110.018726] el1_da+0x20/0xc4 > [ 110.019492] __arch_copy_from_user+0x180/0x280 > [ 110.020646] do_wp_page+0xb0/0x860 > [ 110.021517] __handle_mm_fault+0x994/0x1338 > [ 110.022606] handle_mm_fault+0xe8/0x180 > [ 110.023584] do_page_fault+0x240/0x690 > [ 110.024535] do_mem_abort+0x50/0xb0 > [ 110.025423] el0_da+0x20/0x24 > > The pte info before __copy_from_user_inatomic is (PTE_AF is cleared): > [9b007000] pgd=00023d4f8003, pud=00023da9b003, > pmd=00023d4b3003, pte=36298607bd3 > > As told by Catalin: "On arm64 without hardware Access Flag, copying from > user will fail because the pte is old and cannot be marked young. So we > always end up with zeroed page after fork() + CoW for pfn mappings. we > don't always have a hardware-managed access flag on arm64." > > This patch fix it by calling pte_mkyoung. Also, the parameter is > changed because vmf should be passed to cow_user_page() > > Add a WARN_ON_ONCE when __copy_from_user_inatomic() returns error > in case there can be some obscure use-case.(by Kirill) > > [1] https://github.com/pmem/pmdk/tree/master/src/test/vmmalloc_fork > > Signed-off-by: Jia He > Reported-by: Yibo Cai Acked-by: Kirill A. Shutemov -- Kirill A. Shutemov
Re: [PATCH v2] KVM: LAPIC: Loose fluctuation filter for auto tune lapic_timer_advance_ns
On 26/09/19 02:54, Wanpeng Li wrote: > -#define LAPIC_TIMER_ADVANCE_ADJUST_MIN 100 > -#define LAPIC_TIMER_ADVANCE_ADJUST_MAX 5000 > -#define LAPIC_TIMER_ADVANCE_ADJUST_INIT 1000 > +#define LAPIC_TIMER_ADVANCE_EXPIRE_MIN 100 > +#define LAPIC_TIMER_ADVANCE_EXPIRE_MAX 1 > +#define LAPIC_TIMER_ADVANCE_NS_INIT 1000 > +#define LAPIC_TIMER_ADVANCE_NS_MAX 5000 I think the old #define value is good. What about: -#define LAPIC_TIMER_ADVANCE_ADJUST_MIN 100 -#define LAPIC_TIMER_ADVANCE_ADJUST_MAX 5000 -#define LAPIC_TIMER_ADVANCE_ADJUST_INIT 1000 +#define LAPIC_TIMER_ADVANCE_ADJUST_MIN 100 /* clock cycles */ +#define LAPIC_TIMER_ADVANCE_ADJUST_MAX 1 /* clock cycles */ +#define LAPIC_TIMER_ADVANCE_NS_INIT1000 +#define LAPIC_TIMER_ADVANCE_NS_MAX 5000 ? Thanks, Paolo
[PATCH] power: reset: at91-poweroff: lookup for proper PMC DT node
Driver has been enabled also for SAM9X60. At the moment the patch which did
this has been sent to mainline the PMC for SAM9X60 wasn't integrated.
SAM9X60 has a new PMC compatible (see commit 01e2113de9a5
("clk: at91: add sam9x60 pmc driver")). Do to this we have to
look for proper PMC compatible here, in SHDWC driver.
Signed-off-by: Claudiu Beznea
---
drivers/power/reset/at91-sama5d2_shdwc.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/power/reset/at91-sama5d2_shdwc.c
b/drivers/power/reset/at91-sama5d2_shdwc.c
index e341cc5c0ea6..1c18f465a245 100644
--- a/drivers/power/reset/at91-sama5d2_shdwc.c
+++ b/drivers/power/reset/at91-sama5d2_shdwc.c
@@ -269,6 +269,12 @@ static const struct of_device_id at91_shdwc_of_match[] = {
};
MODULE_DEVICE_TABLE(of, at91_shdwc_of_match);
+static const struct of_device_id at91_pmc_ids[] = {
+ { .compatible = "atmel,sama5d2-pmc" },
+ { .compatible = "microchip,sam9x60-pmc" },
+ { /* Sentinel. */ }
+};
+
static int __init at91_shdwc_probe(struct platform_device *pdev)
{
struct resource *res;
@@ -313,7 +319,7 @@ static int __init at91_shdwc_probe(struct platform_device
*pdev)
at91_shdwc_dt_configure(pdev);
- np = of_find_compatible_node(NULL, NULL, "atmel,sama5d2-pmc");
+ np = of_find_matching_node(NULL, at91_pmc_ids);
if (!np) {
ret = -ENODEV;
goto clk_disable;
--
2.7.4
Re: [PATCH 1/2] HID: i2c-hid: allow delay after SET_POWER
Hi,
On Wed, Sep 25, 2019 at 11:43 AM You-Sheng Yang
wrote:
>
> According to HID over I2C specification v1.0 section 7.2.8, a device is
> allowed to take at most 1 second to make the transition to the specified
> power state. On some touchpad devices implements Microsoft Precision
> Touchpad, it may fail to execute following set PTP mode command without
> the delay and leaves the device in an unsupported Mouse mode.
>
> This change adds a post-setpower-delay-ms device property that allows
> specifying the delay after a SET_POWER command is issued.
I must confess I have at least 2 problems with your series:
- this patch is quite hard to review. There are unrelated changes and
it should be split in at least 2 (I'll detail this in the code review
below)
- you are basically adding a new quirk when Windows doesn't need it.
So before we merge this, I'd like to actually know if Windows is doing
the same and if we could not mimic what Windows is doing to prevent
further similar quirks in the future.
>
> References: https://bugzilla.kernel.org/show_bug.cgi?id=204991
> Signed-off-by: You-Sheng Yang
> ---
> .../bindings/input/hid-over-i2c.txt | 2 +
> drivers/hid/i2c-hid/i2c-hid-core.c| 46 +++
> include/linux/platform_data/i2c-hid.h | 3 ++
> 3 files changed, 32 insertions(+), 19 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/input/hid-over-i2c.txt
> b/Documentation/devicetree/bindings/input/hid-over-i2c.txt
> index c76bafaf98d2f..d82faae335da0 100644
> --- a/Documentation/devicetree/bindings/input/hid-over-i2c.txt
> +++ b/Documentation/devicetree/bindings/input/hid-over-i2c.txt
> @@ -32,6 +32,8 @@ device-specific compatible properties, which should be used
> in addition to the
> - vdd-supply: phandle of the regulator that provides the supply voltage.
> - post-power-on-delay-ms: time required by the device after enabling its
> regulators
Why are you removing those 2 properties?
>or powering it on, before it is ready for communication.
> +- post-setpower-delay-ms: time required by the device after a SET_POWER
> command
> + before it finished the state transition.
couple of issues:
- the name might not be the best. It is similar to the
post-power-on-delay while having a complete different impact. (note: I
don't have a better name at hand)
- checkpatch complains that device tree changes should be in a
separate patch, and I tend to agree.
>
> Example:
>
> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c
> b/drivers/hid/i2c-hid/i2c-hid-core.c
> index 2a7c6e33bb1c4..a5bc2786dc440 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
> @@ -168,6 +168,7 @@ static const struct i2c_hid_quirks {
> __u16 idVendor;
> __u16 idProduct;
> __u32 quirks;
> + __u32 post_setpower_delay_ms;
> } i2c_hid_quirks[] = {
> { USB_VENDOR_ID_WEIDA, HID_ANY_ID,
> I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV },
> @@ -189,21 +190,20 @@ static const struct i2c_hid_quirks {
> * i2c_hid_lookup_quirk: return any quirks associated with a I2C HID device
> * @idVendor: the 16-bit vendor ID
> * @idProduct: the 16-bit product ID
> - *
> - * Returns: a u32 quirks value.
> */
> -static u32 i2c_hid_lookup_quirk(const u16 idVendor, const u16 idProduct)
> +static void i2c_hid_set_quirk(struct i2c_hid *ihid,
> + const u16 idVendor, const u16 idProduct)
> {
> - u32 quirks = 0;
> int n;
>
> for (n = 0; i2c_hid_quirks[n].idVendor; n++)
> if (i2c_hid_quirks[n].idVendor == idVendor &&
> (i2c_hid_quirks[n].idProduct == (__u16)HID_ANY_ID ||
> -i2c_hid_quirks[n].idProduct == idProduct))
> - quirks = i2c_hid_quirks[n].quirks;
> -
> - return quirks;
> +i2c_hid_quirks[n].idProduct == idProduct)) {
> + ihid->quirks = i2c_hid_quirks[n].quirks;
> + ihid->pdata.post_setpower_delay_ms =
> + i2c_hid_quirks[n].post_setpower_delay_ms;
> + }
Why are you changing the signature of i2c_hid_set_quirk? If this is
really a good thing to do, it should go in a separate patch.
> }
>
> static int __i2c_hid_command(struct i2c_client *client,
> @@ -431,8 +431,22 @@ static int i2c_hid_set_power(struct i2c_client *client,
> int power_state)
> power_state == I2C_HID_PWR_SLEEP)
> ihid->sleep_delay = jiffies + msecs_to_jiffies(20);
>
> - if (ret)
> + if (ret) {
> dev_err(&client->dev, "failed to change power setting.\n");
> + goto set_pwr_exit;
> + }
> +
> + /*
> +* The HID over I2C specification states that if a DEVICE needs time
> +* after the PWR_ON request, it should utilise CLOCK stretching.
> +* However, it has been observered that the Windows driver provides a
> +* 1ms sleep
Re: [PATCH v3] KVM: X86: Fix userspace set invalid CR4
On 18/09/19 11:50, Wanpeng Li wrote:
> From: Wanpeng Li
>
> Reported by syzkaller:
>
> WARNING: CPU: 0 PID: 6544 at
> /home/kernel/data/kvm/arch/x86/kvm//vmx/vmx.c:4689 handle_desc+0x37/0x40
> [kvm_intel]
> CPU: 0 PID: 6544 Comm: a.out Tainted: G OE 5.3.0-rc4+ #4
> RIP: 0010:handle_desc+0x37/0x40 [kvm_intel]
> Call Trace:
>vmx_handle_exit+0xbe/0x6b0 [kvm_intel]
>vcpu_enter_guest+0x4dc/0x18d0 [kvm]
>kvm_arch_vcpu_ioctl_run+0x407/0x660 [kvm]
>kvm_vcpu_ioctl+0x3ad/0x690 [kvm]
>do_vfs_ioctl+0xa2/0x690
>ksys_ioctl+0x6d/0x80
>__x64_sys_ioctl+0x1a/0x20
>do_syscall_64+0x74/0x720
>entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
> When CR4.UMIP is set, guest should have UMIP cpuid flag. Current
> kvm set_sregs function doesn't have such check when userspace inputs
> sregs values. SECONDARY_EXEC_DESC is enabled on writes to CR4.UMIP
> in vmx_set_cr4 though guest doesn't have UMIP cpuid flag. The testcast
> triggers handle_desc warning when executing ltr instruction since
> guest architectural CR4 doesn't set UMIP. This patch fixes it by
> adding valid CR4 and CPUID combination checking in __set_sregs.
>
> syzkaller source: https://syzkaller.appspot.com/x/repro.c?x=138efb9960
>
> Reported-by: syzbot+0f1819555fbdce992...@syzkaller.appspotmail.com
> Cc: sta...@vger.kernel.org
> Signed-off-by: Wanpeng Li
> ---
> arch/x86/kvm/x86.c | 38 +-
> 1 file changed, 21 insertions(+), 17 deletions(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index f7cfd8e..d23cf0d 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -884,34 +884,42 @@ int kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64
> xcr)
> }
> EXPORT_SYMBOL_GPL(kvm_set_xcr);
>
> -int kvm_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
> +static int kvm_valid_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
> {
> - unsigned long old_cr4 = kvm_read_cr4(vcpu);
> - unsigned long pdptr_bits = X86_CR4_PGE | X86_CR4_PSE | X86_CR4_PAE |
> -X86_CR4_SMEP | X86_CR4_SMAP | X86_CR4_PKE;
> -
> if (cr4 & CR4_RESERVED_BITS)
> - return 1;
> + return -EINVAL;
>
> if (!guest_cpuid_has(vcpu, X86_FEATURE_XSAVE) && (cr4 &
> X86_CR4_OSXSAVE))
> - return 1;
> + return -EINVAL;
>
> if (!guest_cpuid_has(vcpu, X86_FEATURE_SMEP) && (cr4 & X86_CR4_SMEP))
> - return 1;
> + return -EINVAL;
>
> if (!guest_cpuid_has(vcpu, X86_FEATURE_SMAP) && (cr4 & X86_CR4_SMAP))
> - return 1;
> + return -EINVAL;
>
> if (!guest_cpuid_has(vcpu, X86_FEATURE_FSGSBASE) && (cr4 &
> X86_CR4_FSGSBASE))
> - return 1;
> + return -EINVAL;
>
> if (!guest_cpuid_has(vcpu, X86_FEATURE_PKU) && (cr4 & X86_CR4_PKE))
> - return 1;
> + return -EINVAL;
>
> if (!guest_cpuid_has(vcpu, X86_FEATURE_LA57) && (cr4 & X86_CR4_LA57))
> - return 1;
> + return -EINVAL;
>
> if (!guest_cpuid_has(vcpu, X86_FEATURE_UMIP) && (cr4 & X86_CR4_UMIP))
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +int kvm_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
> +{
> + unsigned long old_cr4 = kvm_read_cr4(vcpu);
> + unsigned long pdptr_bits = X86_CR4_PGE | X86_CR4_PSE | X86_CR4_PAE |
> +X86_CR4_SMEP | X86_CR4_SMAP | X86_CR4_PKE;
> +
> + if (kvm_valid_cr4(vcpu, cr4))
> return 1;
>
> if (is_long_mode(vcpu)) {
> @@ -8641,10 +8649,6 @@ EXPORT_SYMBOL_GPL(kvm_task_switch);
>
> static int kvm_valid_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs)
> {
> - if (!guest_cpuid_has(vcpu, X86_FEATURE_XSAVE) &&
> - (sregs->cr4 & X86_CR4_OSXSAVE))
> - return -EINVAL;
> -
> if ((sregs->efer & EFER_LME) && (sregs->cr0 & X86_CR0_PG)) {
> /*
>* When EFER.LME and CR0.PG are set, the processor is in
> @@ -8663,7 +8667,7 @@ static int kvm_valid_sregs(struct kvm_vcpu *vcpu,
> struct kvm_sregs *sregs)
> return -EINVAL;
> }
>
> - return 0;
> + return kvm_valid_cr4(vcpu, sregs->cr4);
> }
>
> static int __set_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs)
>
Queued, thanks.
Paolo
Re: [PATCH v3 01/11] PCI: designware-ep: Add multiple PFs support for DWC
On Tue, Sep 03, 2019 at 03:43:15AM +, Xiaowei Bao wrote:
>
>
> > -Original Message-
> > From: Andrew Murray
> > Sent: 2019年9月3日 0:26
> > To: Xiaowei Bao
> > Cc: robh...@kernel.org; mark.rutl...@arm.com; shawn...@kernel.org; Leo
> > Li ; kis...@ti.com; lorenzo.pieral...@arm.com; M.h.
> > Lian ; Mingkai Hu ; Roy
> > Zang ; jingooh...@gmail.com;
> > gustavo.pimen...@synopsys.com; linux-...@vger.kernel.org;
> > devicet...@vger.kernel.org; linux-kernel@vger.kernel.org;
> > linux-arm-ker...@lists.infradead.org; linuxppc-...@lists.ozlabs.org;
> > gre...@linuxfoundation.org; Z.q. Hou ;
> > a...@arndb.de
> > Subject: Re: [PATCH v3 01/11] PCI: designware-ep: Add multiple PFs support
> > for DWC
> >
> > On Mon, Sep 02, 2019 at 11:17:06AM +0800, Xiaowei Bao wrote:
> > > Add multiple PFs support for DWC, different PF have different config
> > > space we use pf-offset property which get from the DTS to access the
> > > different pF
> >
> > This needs to be updated as this no longer comes from the DT.
>
> Yes, thanks
>
> Thanks
> Xiaowei
>
> >
> > > config space.
> > >
> > > Signed-off-by: Xiaowei Bao
> >
> >
> > We're assuming:
> >
> > - The offset address (func_offset) between PF's in the memory map can be
> >different between different DWC implementations. And also that it's
> >possible for DWC implementations to address PFs without using an offset.
> >
> > - The current approach is preferable to adding DWC EP driver callbacks
> >for writing to the EP config space (e.g. a variant of dw_pcie_writew_dbi
> >that takes a func number).
>
> Even if use the a variant of dw_pcie_writew_dbi, we also need a offset value
> form
> different platform, due to the different platform may be have different
> implement
> about this, so I am not sure how to implement the variant of
> dw_pcie_writew_dbi?
>
> >
> > I'm keen to hear feedback from Jingoo/Gustavo on this.
>
> OK, expect the feedback.
Hi Jingoo/Gustavo,
I'm keen for your review/feedback on this.
Thanks,
Andrew Murray
>
> Thanks
> Xiaowei
>
> >
> > Thanks,
> >
> > Andrew Murray
> >
> > > ---
> > > v2:
> > > - Remove duplicate redundant code.
> > > - Reimplement the PF config space access way.
> > > v3:
> > > - Integrate duplicate code for func_select.
> > > - Move PCIE_ATU_FUNC_NUM(pf) (pf << 20) to ((pf) << 20).
> > > - Add the comments for func_conf_select function.
> > >
> > > drivers/pci/controller/dwc/pcie-designware-ep.c | 123
> >
> > > drivers/pci/controller/dwc/pcie-designware.c| 59
> > > drivers/pci/controller/dwc/pcie-designware.h| 18 +++-
> > > 3 files changed, 142 insertions(+), 58 deletions(-)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > index 65f4792..eb851c2 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > @@ -19,12 +19,26 @@ void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
> > > pci_epc_linkup(epc);
> > > }
> > >
> > > -static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno
> > bar,
> > > -int flags)
> > > +static unsigned int dw_pcie_ep_func_select(struct dw_pcie_ep *ep, u8
> > > +func_no) {
> > > + unsigned int func_offset = 0;
> > > +
> > > + if (ep->ops->func_conf_select)
> > > + func_offset = ep->ops->func_conf_select(ep, func_no);
> > > +
> > > + return func_offset;
> > > +}
> > > +
> > > +static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, u8 func_no,
> > > +enum pci_barno bar, int flags)
> > > {
> > > u32 reg;
> > > + unsigned int func_offset = 0;
> > > + struct dw_pcie_ep *ep = &pci->ep;
> > > +
> > > + func_offset = dw_pcie_ep_func_select(ep, func_no);
> > >
> > > - reg = PCI_BASE_ADDRESS_0 + (4 * bar);
> > > + reg = func_offset + PCI_BASE_ADDRESS_0 + (4 * bar);
> > > dw_pcie_dbi_ro_wr_en(pci);
> > > dw_pcie_writel_dbi2(pci, reg, 0x0);
> > > dw_pcie_writel_dbi(pci, reg, 0x0);
> > > @@ -37,7 +51,12 @@ static void __dw_pcie_ep_reset_bar(struct dw_pcie
> > > *pci, enum pci_barno bar,
> > >
> > > void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar) {
> > > - __dw_pcie_ep_reset_bar(pci, bar, 0);
> > > + u8 func_no, funcs;
> > > +
> > > + funcs = pci->ep.epc->max_functions;
> > > +
> > > + for (func_no = 0; func_no < funcs; func_no++)
> > > + __dw_pcie_ep_reset_bar(pci, func_no, bar, 0);
> > > }
> > >
> > > static int dw_pcie_ep_write_header(struct pci_epc *epc, u8 func_no,
> > > @@ -45,28 +64,31 @@ static int dw_pcie_ep_write_header(struct pci_epc
> > > *epc, u8 func_no, {
> > > struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> > > struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > + unsigned int func_offset = 0;
> > > +
> > > + func_offset = dw_pcie_ep_func_select(ep, func_no);
> > >
> > > dw_pcie_dbi_ro_wr_en(pci);
> > > - dw_pcie_writew_dbi
Re: spi: Add call to spi_slave_abort() function when spidev driver is released
Hi Lukasz,
On Thu, Sep 26, 2019 at 12:14 PM Lukasz Majewski wrote:
> > Static analysis with Coverity has detected an potential dereference
> > of a free'd object with commit:
> >
> > commit 9f918a728cf86b2757b6a7025e1f46824bfe3155
> > Author: Lukasz Majewski
> > Date: Wed Sep 25 11:11:42 2019 +0200
> >
> > spi: Add call to spi_slave_abort() function when spidev driver is
> > released
> >
> > In spidev_release() in drivers/spi/spidev.c the analysis is as
> > follows:
> >
> > 600static int spidev_release(struct inode *inode, struct file *filp)
> > 601{
> > 602struct spidev_data *spidev;
> > 603
> > 604mutex_lock(&device_list_lock);
> >
> >1. alias: Assigning: spidev = filp->private_data. Now both point to
> > the same storage.
> >
> > 605spidev = filp->private_data;
> > 606filp->private_data = NULL;
> > 607
> > 608/* last close? */
> > 609spidev->users--;
> >
> >2. Condition !spidev->users, taking true branch.
> >
> > 610if (!spidev->users) {
> > 611int dofree;
> > 612
> > 613kfree(spidev->tx_buffer);
> > 614spidev->tx_buffer = NULL;
> > 615
> > 616kfree(spidev->rx_buffer);
> > 617spidev->rx_buffer = NULL;
> > 618
> > 619spin_lock_irq(&spidev->spi_lock);
> >
> >3. Condition spidev->spi, taking false branch.
> >
> > 620if (spidev->spi)
> > 621spidev->speed_hz =
> > spidev->spi->max_speed_hz; 622
> > 623/* ... after we unbound from the underlying
> > device? */
> >
> >4. Condition spidev->spi == NULL, taking true branch.
> >
> > 624dofree = (spidev->spi == NULL);
> > 625spin_unlock_irq(&spidev->spi_lock);
> > 626
> >
> >5. Condition dofree, taking true branch.
> >
> > 627if (dofree)
> >
> >6. freed_arg: kfree frees spidev.
> >
> > 628kfree(spidev);
> > 629}
> > 630#ifdef CONFIG_SPI_SLAVE
> >
> >CID 89726 (#1 of 1): Read from pointer after free (USE_AFTER_FREE)
> > 7. deref_after_free: Dereferencing freed pointer spidev.
> >
> > 631spi_slave_abort(spidev->spi);
> > 632#endif
> > 633mutex_unlock(&device_list_lock);
> > 634
> > 635return 0;
> > 636}
> >
> > The call to spi_slave_abort() on spidev is reading an earlier kfree'd
> > spidev.
>
> Thanks for spotting this issue - indeed there is a possibility to use
> spidev after being kfree'd.
Worse, this makes me realize spidev->spi may be a NULL pointer, which
will be dereferenced by spi_slave_abort(), so caching it before the
call to kfree() won't work.
> However, Geert (CC'ed) had some questions about placement of this
> function call, so I will wait with providing fix until he replies.
Seems like this needs more thought...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v2] KVM: LAPIC: Loose fluctuation filter for auto tune lapic_timer_advance_ns
On Thu, 26 Sep 2019 at 18:25, Paolo Bonzini wrote: > > On 26/09/19 02:54, Wanpeng Li wrote: > > -#define LAPIC_TIMER_ADVANCE_ADJUST_MIN 100 > > -#define LAPIC_TIMER_ADVANCE_ADJUST_MAX 5000 > > -#define LAPIC_TIMER_ADVANCE_ADJUST_INIT 1000 > > +#define LAPIC_TIMER_ADVANCE_EXPIRE_MIN 100 > > +#define LAPIC_TIMER_ADVANCE_EXPIRE_MAX 1 > > +#define LAPIC_TIMER_ADVANCE_NS_INIT 1000 > > +#define LAPIC_TIMER_ADVANCE_NS_MAX 5000 > > I think the old #define value is good. What about: > > -#define LAPIC_TIMER_ADVANCE_ADJUST_MIN 100 > -#define LAPIC_TIMER_ADVANCE_ADJUST_MAX 5000 > -#define LAPIC_TIMER_ADVANCE_ADJUST_INIT 1000 > +#define LAPIC_TIMER_ADVANCE_ADJUST_MIN 100 /* clock cycles */ > +#define LAPIC_TIMER_ADVANCE_ADJUST_MAX 1 /* clock cycles */ > +#define LAPIC_TIMER_ADVANCE_NS_INIT1000 > +#define LAPIC_TIMER_ADVANCE_NS_MAX 5000 Looks good, I guess you can update the patch during apply. :) Wanpeng
Re: [PATCH v5 1/3] x86/hyper-v: Suspend/resume the hypercall page for hibernation
Dexuan Cui writes:
> This is needed for hibernation, e.g. when we resume the old kernel, we need
> to disable the "current" kernel's hypercall page and then resume the old
> kernel's.
>
> Signed-off-by: Dexuan Cui
> Reviewed-by: Michael Kelley
> ---
> arch/x86/hyperv/hv_init.c | 33 +
> 1 file changed, 33 insertions(+)
>
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index 866dfb3..037b0f3 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -20,6 +20,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> void *hv_hypercall_pg;
> @@ -223,6 +224,34 @@ static int __init hv_pci_init(void)
> return 1;
> }
>
> +static int hv_suspend(void)
> +{
> + union hv_x64_msr_hypercall_contents hypercall_msr;
> +
> + /* Reset the hypercall page */
> + rdmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> + hypercall_msr.enable = 0;
> + wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> +
(trying to think out loud, not sure there's a real issue):
When PV IPIs (or PV TLB flush) are enabled we do the following checks:
if (!hv_hypercall_pg)
return false;
or
if (!hv_hypercall_pg)
goto do_native;
which will pass as we're not invalidating the pointer. Can we actually
be sure that the kernel will never try to send an IPI/do TLB flush
before we resume?
--
Vitaly
[PATCH] clk: ti: dra7-atl-clock: Remove ti_clk_add_alias call
ti_clk_register() calls it already so the driver should not create
duplicated alias.
Signed-off-by: Peter Ujfalusi
---
drivers/clk/ti/clk-dra7-atl.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/drivers/clk/ti/clk-dra7-atl.c b/drivers/clk/ti/clk-dra7-atl.c
index a01ca9395179..c0e0ee974151 100644
--- a/drivers/clk/ti/clk-dra7-atl.c
+++ b/drivers/clk/ti/clk-dra7-atl.c
@@ -207,11 +207,6 @@ static void __init of_dra7_atl_clock_setup(struct
device_node *node)
clk = ti_clk_register(NULL, &clk_hw->hw, node->name);
if (!IS_ERR(clk)) {
- ret = ti_clk_add_alias(NULL, clk, node->name);
- if (ret) {
- clk_unregister(clk);
- goto cleanup;
- }
of_clk_add_provider(node, of_clk_src_simple_get, clk);
kfree(parent_names);
return;
--
Peter
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Re: [RFC PATCH] xen/gntdev: Stop abusing DT of_dma_configure API
On 2019-09-26 11:17 am, Oleksandr Andrushchenko wrote: On 9/26/19 12:49 PM, Julien Grall wrote: Hi Rob, On 9/25/19 10:50 PM, Rob Herring wrote: As the comment says, this isn't a DT based device. of_dma_configure() is going to stop allowing a NULL DT node, so this needs to be fixed. And this can't work on arch not selecting CONFIG_OF and can select CONFIG_XEN_GRANT_DMA_ALLOC. We are lucky enough on x86 because, AFAICT, arch_setup_dma_ops is just a nop. No luck is needed as [1] does nothing for those platforms not using CONFIG_OF Not sure exactly what setup besides arch_setup_dma_ops is needed... We probably want to update dma_mask, coherent_dma_mask and dma_pfn_offset. Also, while look at of_configure_dma, I noticed that we consider the DMA will not be coherent for the grant-table. Oleksandr, do you know why they can't be coherent? The main and the only reason to use of_configure_dma is that if we don't then we are about to stay with dma_dummy_ops [2]. It effectively means that operations on dma-bufs will end up returning errors, like [3], [4], thus not making it possible for Xen PV DRM and DMA part of gntdev driver to do what we need (dma-bufs in our use-cases allow zero-copying while using graphics buffers and many more). I didn't find any better way of achieving that, but of_configure_dma... If there is any better solution which will not break the existing functionality then I will definitely change the drivers so we do not abuse DT ) Before that, please keep in mind that merging this RFC will break Xen PV DRM + DMA buf support in gntdev... Hope we can work out some acceptable solution, so everyone is happy As I mentioned elsewhere, the recent dma-direct rework means that dma_dummy_ops are now only explicitly installed for the ACPI error case, so - much as I may dislike it - you should get regular (direct/SWIOTLB) ops by default again. Coherency is trickier - if the guest is allocating buffers for the PV device, which may be shared directly with hardware by the host driver, then the coherency of the PV device should really reflect that of the underlying hardware to avoid potential problems. There are some cases where the stage 2 attributes alone wouldn't be enough to correct a mismatch. Robin.
SUPPLY CONTRACT.
Sir,
I got your email information through a search in the internet. I am an
authorized agent registered with the Department for Business, Energy &
Industrial Strategy{Project Procurement Division} as a sourcing agent
for sundry products. I search for companies to bid for supply
contracts on commission basis. I receive 3% commission from the seller
in any successful contract done through my recommendation.
At the moment, the Ministry is interested in your products and I will
like to make a preliminary introduction of you/your company to the
Ministry for their consideration. Please furnish me with the following
information to enable me introduce you/your company to the
Tender/procurement department of the ministry:
Payment of every government contract in U.K is by 100% bank swift
transfer (TT) in advance immediately every contract formality is
completed.
1) Your price list
2) Comprehensive details/information of your product
3) Your Company profile
I am looking forward to working with you.
Best regards,
Benjamin Hulk
[PATCH -next 0/4] dmaengine: xilinx_dma: Minor functional fixes
This patchset fixes axidma simple mode 64-bit transfer. It clears vdma control registers before update, in probe use devm_platform API and remove clk_get error in case of EPROBE_DEFER. Radhey Shyam Pandey (4): dmaengine: xilinx_dma: Fix 64-bit simple AXIDMA transfer dmaengine: xilinx_dma: Fix control reg update in vdma_channel_set_config dmaengine: xilinx_dma: use devm_platform_ioremap_resource() dmaengine: xilinx_dma: Remove clk_get error message for probe defer drivers/dma/xilinx/xilinx_dma.c | 33 - 1 files changed, 24 insertions(+), 9 deletions(-)
[PATCH -next 2/4] dmaengine: xilinx_dma: Fix control reg update in vdma_channel_set_config
In vdma_channel_set_config clear the delay, frame count and master mask
before updating their new values. It avoids programming incorrect state
when input parameters are different from default.
Signed-off-by: Radhey Shyam Pandey
Acked-by: Appana Durga Kedareswara rao
Signed-off-by: Michal Simek
---
drivers/dma/xilinx/xilinx_dma.c |7 +++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index 1fbe025..5d56f1e 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -68,6 +68,9 @@
#define XILINX_DMA_DMACR_CIRC_EN BIT(1)
#define XILINX_DMA_DMACR_RUNSTOP BIT(0)
#define XILINX_DMA_DMACR_FSYNCSRC_MASK GENMASK(6, 5)
+#define XILINX_DMA_DMACR_DELAY_MASKGENMASK(31, 24)
+#define XILINX_DMA_DMACR_FRAME_COUNT_MASK GENMASK(23, 16)
+#define XILINX_DMA_DMACR_MASTER_MASK GENMASK(11, 8)
#define XILINX_DMA_REG_DMASR 0x0004
#define XILINX_DMA_DMASR_EOL_LATE_ERR BIT(15)
@@ -2118,8 +2121,10 @@ int xilinx_vdma_channel_set_config(struct dma_chan
*dchan,
chan->config.gen_lock = cfg->gen_lock;
chan->config.master = cfg->master;
+ dmacr &= ~XILINX_DMA_DMACR_GENLOCK_EN;
if (cfg->gen_lock && chan->genlock) {
dmacr |= XILINX_DMA_DMACR_GENLOCK_EN;
+ dmacr &= ~XILINX_DMA_DMACR_MASTER_MASK;
dmacr |= cfg->master << XILINX_DMA_DMACR_MASTER_SHIFT;
}
@@ -2135,11 +2140,13 @@ int xilinx_vdma_channel_set_config(struct dma_chan
*dchan,
chan->config.delay = cfg->delay;
if (cfg->coalesc <= XILINX_DMA_DMACR_FRAME_COUNT_MAX) {
+ dmacr &= ~XILINX_DMA_DMACR_FRAME_COUNT_MASK;
dmacr |= cfg->coalesc << XILINX_DMA_DMACR_FRAME_COUNT_SHIFT;
chan->config.coalesc = cfg->coalesc;
}
if (cfg->delay <= XILINX_DMA_DMACR_DELAY_MAX) {
+ dmacr &= ~XILINX_DMA_DMACR_DELAY_MASK;
dmacr |= cfg->delay << XILINX_DMA_DMACR_DELAY_SHIFT;
chan->config.delay = cfg->delay;
}
--
1.7.1
[PATCH -next 1/4] dmaengine: xilinx_dma: Fix 64-bit simple AXIDMA transfer
In AXI DMA simple mode also pass MSB bits of source and destination address to xilinx_write function. It fixes simple AXI DMA operation mode using 64-bit addressing. Signed-off-by: Radhey Shyam Pandey --- drivers/dma/xilinx/xilinx_dma.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c index e7dc3c4..1fbe025 100644 --- a/drivers/dma/xilinx/xilinx_dma.c +++ b/drivers/dma/xilinx/xilinx_dma.c @@ -1354,7 +1354,8 @@ static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan) node); hw = &segment->hw; - xilinx_write(chan, XILINX_DMA_REG_SRCDSTADDR, hw->buf_addr); + xilinx_write(chan, XILINX_DMA_REG_SRCDSTADDR, +xilinx_prep_dma_addr_t(hw->buf_addr)); /* Start the transfer */ dma_ctrl_write(chan, XILINX_DMA_REG_BTT, -- 1.7.1
[PATCH -next 4/4] dmaengine: xilinx_dma: Remove clk_get error message for probe defer
In dma probe, the driver checks for devm_clk_get return and print error
message in the failing case. However for -EPROBE_DEFER this message is
confusing so avoid it.
Signed-off-by: Radhey Shyam Pandey
Signed-off-by: Michal Simek
---
drivers/dma/xilinx/xilinx_dma.c | 19 ++-
1 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index bc9979d..440f2ce 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -2194,7 +2194,9 @@ static int axidma_clk_init(struct platform_device *pdev,
struct clk **axi_clk,
*axi_clk = devm_clk_get(&pdev->dev, "s_axi_lite_aclk");
if (IS_ERR(*axi_clk)) {
err = PTR_ERR(*axi_clk);
- dev_err(&pdev->dev, "failed to get axi_aclk (%d)\n", err);
+ if (err != -EPROBE_DEFER)
+ dev_err(&pdev->dev, "failed to get axi_aclk (%d)\n",
+ err);
return err;
}
@@ -2259,14 +2261,18 @@ static int axicdma_clk_init(struct platform_device
*pdev, struct clk **axi_clk,
*axi_clk = devm_clk_get(&pdev->dev, "s_axi_lite_aclk");
if (IS_ERR(*axi_clk)) {
err = PTR_ERR(*axi_clk);
- dev_err(&pdev->dev, "failed to get axi_clk (%d)\n", err);
+ if (err != -EPROBE_DEFER)
+ dev_err(&pdev->dev, "failed to get axi_clk (%d)\n",
+ err);
return err;
}
*dev_clk = devm_clk_get(&pdev->dev, "m_axi_aclk");
if (IS_ERR(*dev_clk)) {
err = PTR_ERR(*dev_clk);
- dev_err(&pdev->dev, "failed to get dev_clk (%d)\n", err);
+ if (err != -EPROBE_DEFER)
+ dev_err(&pdev->dev, "failed to get dev_clk (%d)\n",
+ err);
return err;
}
@@ -2299,7 +2305,9 @@ static int axivdma_clk_init(struct platform_device *pdev,
struct clk **axi_clk,
*axi_clk = devm_clk_get(&pdev->dev, "s_axi_lite_aclk");
if (IS_ERR(*axi_clk)) {
err = PTR_ERR(*axi_clk);
- dev_err(&pdev->dev, "failed to get axi_aclk (%d)\n", err);
+ if (err != -EPROBE_DEFER)
+ dev_err(&pdev->dev, "failed to get axi_aclk (%d)\n",
+ err);
return err;
}
@@ -2321,7 +2329,8 @@ static int axivdma_clk_init(struct platform_device *pdev,
struct clk **axi_clk,
err = clk_prepare_enable(*axi_clk);
if (err) {
- dev_err(&pdev->dev, "failed to enable axi_clk (%d)\n", err);
+ dev_err(&pdev->dev, "failed to enable axi_clk (%d)\n",
+ err);
return err;
}
--
1.7.1
[PATCH -next 3/4] dmaengine: xilinx_dma: use devm_platform_ioremap_resource()
Replace the chain of platform_get_resource() and devm_ioremap_resource() with devm_platform_ioremap_resource(). It simplifies the flow and there is no functional change. Fixes below cocinelle warning- WARNING: Use devm_platform_ioremap_resource for xdev -> regs Signed-off-by: Radhey Shyam Pandey --- drivers/dma/xilinx/xilinx_dma.c |4 +--- 1 files changed, 1 insertions(+), 3 deletions(-) diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c index 5d56f1e..bc9979d 100644 --- a/drivers/dma/xilinx/xilinx_dma.c +++ b/drivers/dma/xilinx/xilinx_dma.c @@ -2612,7 +2612,6 @@ static int xilinx_dma_probe(struct platform_device *pdev) struct device_node *node = pdev->dev.of_node; struct xilinx_dma_device *xdev; struct device_node *child, *np = pdev->dev.of_node; - struct resource *io; u32 num_frames, addr_width, len_width; int i, err; @@ -2638,8 +2637,7 @@ static int xilinx_dma_probe(struct platform_device *pdev) return err; /* Request and map I/O memory */ - io = platform_get_resource(pdev, IORESOURCE_MEM, 0); - xdev->regs = devm_ioremap_resource(&pdev->dev, io); + xdev->regs = devm_platform_ioremap_resource(pdev, 0); if (IS_ERR(xdev->regs)) return PTR_ERR(xdev->regs); -- 1.7.1
[PATCH v4 07/19] spi: core,atmel: convert `word_delay_usecs` -> `word_delay` for spi_device
This change does a conversion from the `word_delay_usecs` -> `word_delay`
for the `spi_device` struct.
This allows users to specify inter-word delays in other unit types
(nano-seconds or clock cycles), depending on how users want.
The Atmel SPI driver is the only current user of the `word_delay_usecs`
field (from the `spi_device` struct).
So, it needed a slight conversion to use the `word_delay` as an `spi_delay`
struct.
In SPI core, the only required mechanism is to update the `word_delay`
information per `spi_transfer`. This requires a bit more logic than before,
because it needs that both delays be converted to a common unit
(nano-seconds) for comparison.
Signed-off-by: Alexandru Ardelean
---
drivers/spi/spi-atmel.c | 26 --
drivers/spi/spi.c | 24 ++--
include/linux/spi/spi.h | 7 ++-
3 files changed, 48 insertions(+), 9 deletions(-)
diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
index f00b367523cd..f04b92791515 100644
--- a/drivers/spi/spi-atmel.c
+++ b/drivers/spi/spi-atmel.c
@@ -1172,12 +1172,31 @@ atmel_spi_pdc_interrupt(int irq, void *dev_id)
return ret;
}
+static int atmel_word_delay_csr(struct spi_device *spi, struct atmel_spi *as)
+{
+ struct spi_delay *delay = &spi->word_delay;
+ u32 value = delay->value;
+
+ switch (delay->unit) {
+ case SPI_DELAY_UNIT_NSECS:
+ value /= 1000;
+ break;
+ case SPI_DELAY_UNIT_USECS:
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return (as->spi_clk / 100 * value) >> 5;
+}
+
static int atmel_spi_setup(struct spi_device *spi)
{
struct atmel_spi*as;
struct atmel_spi_device *asd;
u32 csr;
unsigned intbits = spi->bits_per_word;
+ int word_delay_csr;
as = spi_master_get_devdata(spi->master);
@@ -1201,11 +1220,14 @@ static int atmel_spi_setup(struct spi_device *spi)
*/
csr |= SPI_BF(DLYBS, 0);
+ word_delay_csr = atmel_word_delay_csr(spi, as);
+ if (word_delay_csr < 0)
+ return word_delay_csr;
+
/* DLYBCT adds delays between words. This is useful for slow devices
* that need a bit of time to setup the next transfer.
*/
- csr |= SPI_BF(DLYBCT,
- (as->spi_clk / 100 * spi->word_delay_usecs) >> 5);
+ csr |= SPI_BF(DLYBCT, word_delay_csr);
asd = spi->controller_state;
if (!asd) {
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index d0bf0ffca042..ba8731b77753 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -3159,6 +3159,26 @@ void spi_set_cs_timing(struct spi_device *spi, u8 setup,
u8 hold,
}
EXPORT_SYMBOL_GPL(spi_set_cs_timing);
+static int _spi_xfer_word_delay_update(struct spi_transfer *xfer,
+ struct spi_device *spi)
+{
+ int delay1, delay2;
+
+ delay1 = _spi_delay_to_ns(&xfer->word_delay, xfer);
+ if (delay1 < 0)
+ return delay1;
+
+ delay2 = _spi_delay_to_ns(&spi->word_delay, xfer);
+ if (delay2 < 0)
+ return delay2;
+
+ if (delay1 < delay2)
+ memcpy(&xfer->word_delay, &spi->word_delay,
+ sizeof(xfer->word_delay));
+
+ return 0;
+}
+
static int __spi_validate(struct spi_device *spi, struct spi_message *message)
{
struct spi_controller *ctlr = spi->controller;
@@ -3294,8 +3314,8 @@ static int __spi_validate(struct spi_device *spi, struct
spi_message *message)
return -EINVAL;
}
- if (xfer->word_delay_usecs < spi->word_delay_usecs)
- xfer->word_delay_usecs = spi->word_delay_usecs;
+ if (_spi_xfer_word_delay_update(xfer, spi))
+ return -EINVAL;
}
message->status = -EINPROGRESS;
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 16397b210e0c..e7ec0be16a82 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -138,7 +138,7 @@ extern int spi_delay_exec(struct spi_delay *_delay, struct
spi_transfer *xfer);
* the spi_master.
* @cs_gpiod: gpio descriptor of the chipselect line (optional, NULL when
* not using a GPIO line)
- * @word_delay_usecs: microsecond delay to be inserted between consecutive
+ * @word_delay: delay to be inserted between consecutive
* words of a transfer
*
* @statistics: statistics for the spi_device
@@ -188,7 +188,7 @@ struct spi_device {
const char *driver_override;
int cs_gpio;/* LEGACY: chip select gpio */
struct gpio_desc*cs_gpiod; /* chip select gpio desc */
- uint8_t word_delay_usecs; /* inter-word delay */
+ struct spi_delayword_delay; /* inter-wor
[PATCH v4 03/19] spi: make `cs_change_delay` the first user of the `spi_delay` logic
Since the logic for `spi_delay` struct + `spi_delay_exec()` has been copied
from the `cs_change_delay` logic, it's natural to make this delay, the
first user.
The `cs_change_delay` logic requires that the default remain 10 uS, in case
it is unspecified/unconfigured. So, there is some special handling needed
to do that.
The ADIS library is one of the few users of the new `cs_change_delay`
parameter for an spi_transfer.
The introduction of the `spi_delay` struct, requires that the users of of
`cs_change_delay` get an update. This change also updates the ADIS library.
Signed-off-by: Alexandru Ardelean
---
drivers/iio/imu/adis.c | 24
drivers/spi/spi.c | 28 +++-
include/linux/spi/spi.h | 4 +---
3 files changed, 20 insertions(+), 36 deletions(-)
diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
index 1631c255deab..2cd2cc2316c6 100644
--- a/drivers/iio/imu/adis.c
+++ b/drivers/iio/imu/adis.c
@@ -39,24 +39,24 @@ int adis_write_reg(struct adis *adis, unsigned int reg,
.len = 2,
.cs_change = 1,
.delay_usecs = adis->data->write_delay,
- .cs_change_delay = adis->data->cs_change_delay,
- .cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
+ .cs_change_delay.value = adis->data->cs_change_delay,
+ .cs_change_delay.unit = SPI_DELAY_UNIT_USECS,
}, {
.tx_buf = adis->tx + 2,
.bits_per_word = 8,
.len = 2,
.cs_change = 1,
.delay_usecs = adis->data->write_delay,
- .cs_change_delay = adis->data->cs_change_delay,
- .cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
+ .cs_change_delay.value = adis->data->cs_change_delay,
+ .cs_change_delay.unit = SPI_DELAY_UNIT_USECS,
}, {
.tx_buf = adis->tx + 4,
.bits_per_word = 8,
.len = 2,
.cs_change = 1,
.delay_usecs = adis->data->write_delay,
- .cs_change_delay = adis->data->cs_change_delay,
- .cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
+ .cs_change_delay.value = adis->data->cs_change_delay,
+ .cs_change_delay.unit = SPI_DELAY_UNIT_USECS,
}, {
.tx_buf = adis->tx + 6,
.bits_per_word = 8,
@@ -139,16 +139,16 @@ int adis_read_reg(struct adis *adis, unsigned int reg,
.len = 2,
.cs_change = 1,
.delay_usecs = adis->data->write_delay,
- .cs_change_delay = adis->data->cs_change_delay,
- .cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
+ .cs_change_delay.value = adis->data->cs_change_delay,
+ .cs_change_delay.unit = SPI_DELAY_UNIT_USECS,
}, {
.tx_buf = adis->tx + 2,
.bits_per_word = 8,
.len = 2,
.cs_change = 1,
.delay_usecs = adis->data->read_delay,
- .cs_change_delay = adis->data->cs_change_delay,
- .cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
+ .cs_change_delay.value = adis->data->cs_change_delay,
+ .cs_change_delay.unit = SPI_DELAY_UNIT_USECS,
}, {
.tx_buf = adis->tx + 4,
.rx_buf = adis->rx,
@@ -156,8 +156,8 @@ int adis_read_reg(struct adis *adis, unsigned int reg,
.len = 2,
.cs_change = 1,
.delay_usecs = adis->data->read_delay,
- .cs_change_delay = adis->data->cs_change_delay,
- .cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
+ .cs_change_delay.value = adis->data->cs_change_delay,
+ .cs_change_delay.unit = SPI_DELAY_UNIT_USECS,
}, {
.rx_buf = adis->rx + 2,
.bits_per_word = 8,
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 1883de8ffa82..d0bf0ffca042 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1160,9 +1160,9 @@ EXPORT_SYMBOL_GPL(spi_delay_exec);
static void _spi_transfer_cs_change_delay(struct spi_message *msg,
struct spi_transfer *xfer)
{
- u32 delay = xfer->cs_change_delay;
- u32 unit = xfer->cs_change_delay_unit;
- u32 hz;
+ u32 delay = xfer->cs_change_delay.value;
+ u32 unit = xfer->cs_change_delay.unit;
+ int r
[PATCH v4 01/19] spi: move `cs_change_delay` backwards compat logic outside switch
The `cs_change_delay` backwards compatibility value could be moved outside
of the switch statement.
The only reason to do it, is to make the next patches easier to diff.
Signed-off-by: Alexandru Ardelean
---
drivers/spi/spi.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 75ac046cae52..c90e02e6d62f 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1114,16 +1114,15 @@ static void _spi_transfer_cs_change_delay(struct
spi_message *msg,
u32 hz;
/* return early on "fast" mode - for everything but USECS */
- if (!delay && unit != SPI_DELAY_UNIT_USECS)
+ if (!delay) {
+ if (unit == SPI_DELAY_UNIT_USECS)
+ _spi_transfer_delay_ns(1);
return;
+ }
switch (unit) {
case SPI_DELAY_UNIT_USECS:
- /* for compatibility use default of 10us */
- if (!delay)
- delay = 1;
- else
- delay *= 1000;
+ delay *= 1000;
break;
case SPI_DELAY_UNIT_NSECS: /* nothing to do here */
break;
--
2.20.1
[PATCH v4 08/19] spi: introduce `delay` field for `spi_transfer` + spi_transfer_delay_exec()
The change introduces the `delay` field to the `spi_transfer` struct as an
`struct spi_delay` type.
This intends to eventually replace `delay_usecs`.
But, since there are many users of `delay_usecs`, this needs some
intermediate work.
A helper called `spi_transfer_delay_exec()` is also added, which maintains
backwards compatibility with `delay_usecs`, by assigning the value to
`delay` if non-zero.
This should maintain backwards compatibility with current users of
`udelay_usecs`.
Signed-off-by: Alexandru Ardelean
---
drivers/spi/spi.c | 6 +++---
include/linux/spi/spi.h | 18 ++
2 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index ba8731b77753..489eb64d9ee0 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1238,8 +1238,7 @@ static int spi_transfer_one_message(struct spi_controller
*ctlr,
if (msg->status != -EINPROGRESS)
goto out;
- if (xfer->delay_usecs)
- _spi_transfer_delay_ns(xfer->delay_usecs * 1000);
+ spi_transfer_delay_exec(xfer);
if (xfer->cs_change) {
if (list_is_last(&xfer->transfer_list,
@@ -2904,10 +2903,11 @@ struct spi_replaced_transfers *spi_replace_transfers(
/* add to list */
list_add(&xfer->transfer_list, rxfer->replaced_after);
- /* clear cs_change and delay_usecs for all but the last */
+ /* clear cs_change and delay for all but the last */
if (i) {
xfer->cs_change = false;
xfer->delay_usecs = 0;
+ xfer->delay.value = 0;
}
}
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index e7ec0be16a82..2cee9ac35737 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -754,6 +754,9 @@ extern void spi_res_release(struct spi_controller *ctlr,
* @cs_change: affects chipselect after this transfer completes
* @cs_change_delay: delay between cs deassert and assert when
* @cs_change is set and @spi_transfer is not the last in @spi_message
+ * @delay: delay to be introduced after this transfer before
+ * (optionally) changing the chipselect status, then starting
+ * the next transfer or completing this @spi_message.
* @delay_usecs: microseconds to delay after this transfer before
* (optionally) changing the chipselect status, then starting
* the next transfer or completing this @spi_message.
@@ -843,6 +846,7 @@ struct spi_transfer {
#defineSPI_NBITS_QUAD 0x04 /* 4bits transfer */
u8 bits_per_word;
u16 delay_usecs;
+ struct spi_delaydelay;
struct spi_delaycs_change_delay;
struct spi_delayword_delay;
u32 speed_hz;
@@ -942,6 +946,20 @@ spi_transfer_del(struct spi_transfer *t)
list_del(&t->transfer_list);
}
+static inline int
+spi_transfer_delay_exec(struct spi_transfer *t)
+{
+ struct spi_delay d;
+
+ if (t->delay_usecs) {
+ d.value = t->delay_usecs;
+ d.unit = SPI_DELAY_UNIT_USECS;
+ return spi_delay_exec(&d, NULL);
+ }
+
+ return spi_delay_exec(&t->delay, t);
+}
+
/**
* spi_message_init_with_transfers - Initialize spi_message and append
transfers
* @m: spi_message to be initialized
--
2.20.1
[PATCH v4 00/19] spi: introduce `struct spi_delay` data-type
Initially, I started this patchset thinking: "we need a new delay for something-something" (in case someone is curios, we needed a CS-hold-time when CS is asserted, because the CS wakes a chip from sleep-mode). Then I added the delay, and felt a bit dirty-inside about adding a new one (just like that), and decided to look at maybe cleaning things up a bit, and a few days/weeks later, I got here. It's based on top of Jonathan's `iio/togreg` branch which also includes the ADIS driver library changes for `cs_change_delay`. The current v4 tries to be a complete change as possible about converting all delays to the `struct delay` data-type; at least in the scope of the SPI subsystem. Many external users of the SPI subsystem use the `delay_usecs` field to specify delays for transfers; which is why this field is still kept temporarily for backwards compatibility until all other delays are switched to `delay`. Thanks Alex Changelog v3 -> v4: * re-unify all `spi_delay` changes into a single big series spanning SPI & IIO subsystems; patches are based on top of IIO, so patch `drivers/iio/imu/adis.c` does not apply on top of SPI/for-next tree Changelog v2 -> v3: * squash patches 3 & 4 into a single patch; otherwise the kernel is in broken state between those 2 patches Changelog v1 -> v2: * split away from the RFC patchset, which aims to be a broader explanation for this changeset; parts of v1 are not 100% defined yet, and may require some discussion and refinement. Alexandru Ardelean (19): spi: move `cs_change_delay` backwards compat logic outside switch spi: introduce spi_delay struct as "value + unit" & spi_delay_exec() spi: make `cs_change_delay` the first user of the `spi_delay` logic spi: sprd: convert transfer word delay to spi_delay struct spi: orion: use new `word_delay` field for SPI transfers spi: spidev: use new `word_delay` field for spi transfers spi: core,atmel: convert `word_delay_usecs` -> `word_delay` for spi_device spi: introduce `delay` field for `spi_transfer` + spi_transfer_delay_exec() spi: use new `spi_transfer_delay_exec` helper where straightforward spi: tegra114: use `spi_transfer_delay_exec` helper spi: tegra20-sflash: use to new `spi_transfer_delay_exec` spi: spi-loopback-test: use new `delay` field spi: spidev: use new `delay` field for spi transfers spi: tegra114: change format for `spi_set_cs_timing()` function spi: implement SW control for CS times spi: spi-fsl-espi: convert transfer delay to `spi_delay` format spi: spi-falcon: extend warning to `delay` as well spi: bcm63xx: extend error condition to `delay` as well spi: spi-axi: extend support for the `delay` field drivers/iio/imu/adis.c | 24 ++--- drivers/spi/spi-atmel.c | 29 +- drivers/spi/spi-axi-spi-engine.c | 16 +++- drivers/spi/spi-bcm63xx-hsspi.c | 3 +- drivers/spi/spi-bcm63xx.c| 2 +- drivers/spi/spi-cavium.c | 3 +- drivers/spi/spi-falcon.c | 2 +- drivers/spi/spi-fsl-dspi.c | 3 +- drivers/spi/spi-fsl-espi.c | 19 +++- drivers/spi/spi-fsl-spi.c| 3 +- drivers/spi/spi-loopback-test.c | 12 ++- drivers/spi/spi-mpc512x-psc.c| 3 +- drivers/spi/spi-mpc52xx-psc.c| 3 +- drivers/spi/spi-omap-100k.c | 3 +- drivers/spi/spi-orion.c | 6 +- drivers/spi/spi-pl022.c | 25 +++-- drivers/spi/spi-sc18is602.c | 3 +- drivers/spi/spi-sh-hspi.c| 3 +- drivers/spi/spi-sprd.c | 15 ++- drivers/spi/spi-tegra114.c | 39 drivers/spi/spi-tegra20-sflash.c | 5 +- drivers/spi/spi-topcliff-pch.c | 7 +- drivers/spi/spi-txx9.c | 3 +- drivers/spi/spi-xcomm.c | 3 +- drivers/spi/spi.c| 158 +-- drivers/spi/spidev.c | 6 +- include/linux/spi/spi.h | 66 + 27 files changed, 323 insertions(+), 141 deletions(-) -- 2.20.1
Re: [RFC v3] zswap: Add CONFIG_ZSWAP_IO_SWITCH to handle swap IO issue
On Mon, Sep 23, 2019 at 4:14 PM Dan Streetman wrote: > > On Sun, Sep 22, 2019 at 11:32 PM Hui Zhu wrote: > > > > This is the third version of this patch. The first and second version > > is in [1] and [2]. > > This verion is updated according to the comments from Randy Dunlap > > in [3]. > > > > Currently, I use a VM that has 2 CPUs, 4G memory and 4G swap file. > > I found that swap will affect the IO performance when it is running. > > So I open zswap to handle it because it just use CPU cycles but not > > disk IO. > > > > It work OK but I found that zswap is slower than normal swap in this > > VM. zswap is about 300M/s and normal swap is about 500M/s. (The reason > > is disk inside VM has fscache in host machine.) > > I must be missing something here - if zswap in the guest is *slower* > than real swap, why are you using zswap? > > Also, I don't see why zswap is slower than normal swap, unless you > mean that your zswap is full, since once zswap fills up any additional > swap will absolutely be slower than not having zswap at all. > > > So open zswap is make memory shrinker slower but good for IO performance > > in this VM. > > So I just want zswap work when the disk of the swap file is under high > > IO load. > > > > This commit is designed for this idea. > > It add two parameters read_in_flight_limit and write_in_flight_limit to > > zswap. > > In zswap_frontswap_store, pages will be stored to zswap only when > > the IO in flight number of swap device is bigger than > > zswap_read_in_flight_limit or zswap_write_in_flight_limit > > when zswap is enabled. > > Then the zswap just work when the IO in flight number of swap device > > is low. > > Ok, so maybe I understand what you mean, your disk I/O is normally > very fast, but once your host-side cache is full it starts actually > writing to your host physical disk, and your guest swap I/O drops way > down (since caching pages in host memory is much faster than writing > to a host physical disk). Is that what's going on? That was not > clear at all to me from the commit description... > > In general I think the description of this commit, as well as the docs > and even user interface of how to use it, is very confusing. I can > see how it would be beneficial in this specific situation, but I'm not > a fan of the implementation, and I'm very concerned that nobody will > be able to understand how to use it properly - when should they enable > it? What limit values should they use? Why are there separate read > and write limits? None of that is clear to me, and I'm fairly > certainly it would not be clear to other normal users. > > Is there a better way this can be done? > > > > > [1] https://lkml.org/lkml/2019/9/11/935 > > [2] https://lkml.org/lkml/2019/9/20/90 > > [3] https://lkml.org/lkml/2019/9/20/1076 > > > > Signed-off-by: Hui Zhu Nacked-by: Dan Streetman due to my concerns that I emailed before > > --- > > include/linux/swap.h | 3 +++ > > mm/Kconfig | 18 > > mm/page_io.c | 16 +++ > > mm/zswap.c | 58 > > > > 4 files changed, 95 insertions(+) > > > > diff --git a/include/linux/swap.h b/include/linux/swap.h > > index de2c67a..82b621f 100644 > > --- a/include/linux/swap.h > > +++ b/include/linux/swap.h > > @@ -389,6 +389,9 @@ extern void end_swap_bio_write(struct bio *bio); > > extern int __swap_writepage(struct page *page, struct writeback_control > > *wbc, > > bio_end_io_t end_write_func); > > extern int swap_set_page_dirty(struct page *page); > > +#ifdef CONFIG_ZSWAP_IO_SWITCH > > +extern void swap_io_in_flight(struct page *page, unsigned int inflight[2]); > > +#endif > > > > int add_swap_extent(struct swap_info_struct *sis, unsigned long start_page, > > unsigned long nr_pages, sector_t start_block); > > diff --git a/mm/Kconfig b/mm/Kconfig > > index 56cec63..387c3b5 100644 > > --- a/mm/Kconfig > > +++ b/mm/Kconfig > > @@ -546,6 +546,24 @@ config ZSWAP > > they have not be fully explored on the large set of potential > > configurations and workloads that exist. > > > > +config ZSWAP_IO_SWITCH > > + bool "Compressed cache for swap pages according to the IO status" > > + depends on ZSWAP > > + help > > + This function helps the system that normal swap speed is higher > > + than zswap speed to handle the swap IO issue. > > + For example, a VM where the disk device is not set cache config or > > + set cache=writeback. > > + > > + This function makes zswap just work when the disk of the swap file > > + is under high IO load. > > + It add two parameters (read_in_flight_limit and > > + write_in_flight_limit) to zswap. When zswap is enabled, pages > > will > > + be stored to zswap only when the IO in flight number of swap > > device > > + is bigger than zswap_read_in_flight_limit or >
[PATCH v4 15/19] spi: implement SW control for CS times
This change implements CS control for setup, hold & inactive delays.
The `cs_setup` delay is completely new, and can help with cases where
asserting the CS, also brings the device out of power-sleep, where there
needs to be a longer (than usual), before transferring data.
The `cs_hold` time can overlap with the `delay` (or `delay_usecs`) from an
SPI transfer. The main difference is that `cs_hold` implies that CS will be
de-asserted.
The `cs_inactive` delay does not have a clear use-case yet. It has been
implemented mostly because the `spi_set_cs_timing()` function implements
it. To some degree, this could overlap or replace `cs_change_delay`, but
this will require more consideration/investigation in the future.
All these delays have been added to the `spi_controller` struct, as they
would typically be configured by calling `spi_set_cs_timing()` after an
`spi_setup()` call.
Software-mode for CS control, implies that the `set_cs_timing()` hook has
not been provided for the `spi_controller` object.
Signed-off-by: Alexandru Ardelean
---
drivers/spi/spi.c | 45 -
include/linux/spi/spi.h | 5 +
2 files changed, 49 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 122af2264bfe..61ef286f954a 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -775,6 +775,15 @@ int spi_register_board_info(struct spi_board_info const
*info, unsigned n)
static void spi_set_cs(struct spi_device *spi, bool enable)
{
+ bool enable1 = enable;
+
+ if (!spi->controller->set_cs_timing) {
+ if (enable1)
+ spi_delay_exec(&spi->controller->cs_setup, NULL);
+ else
+ spi_delay_exec(&spi->controller->cs_hold, NULL);
+ }
+
if (spi->mode & SPI_CS_HIGH)
enable = !enable;
@@ -800,6 +809,11 @@ static void spi_set_cs(struct spi_device *spi, bool enable)
} else if (spi->controller->set_cs) {
spi->controller->set_cs(spi, !enable);
}
+
+ if (!spi->controller->set_cs_timing) {
+ if (!enable1)
+ spi_delay_exec(&spi->controller->cs_inactive, NULL);
+ }
}
#ifdef CONFIG_HAS_DMA
@@ -3156,10 +3170,39 @@ EXPORT_SYMBOL_GPL(spi_setup);
int spi_set_cs_timing(struct spi_device *spi, struct spi_delay *setup,
struct spi_delay *hold, struct spi_delay *inactive)
{
+ size_t len;
+
if (spi->controller->set_cs_timing)
return spi->controller->set_cs_timing(spi, setup, hold,
inactive);
- return -ENOTSUPP;
+
+ if ((setup && setup->unit == SPI_DELAY_UNIT_SCK) ||
+ (hold && hold->unit == SPI_DELAY_UNIT_SCK) ||
+ (inactive && inactive->unit == SPI_DELAY_UNIT_SCK)) {
+ dev_err(&spi->dev,
+ "Clock-cycle delays for CS not supported in SW mode\n");
+ return -ENOTSUPP;
+ }
+
+ len = sizeof(struct spi_delay);
+
+ /* copy delays to controller */
+ if (setup)
+ memcpy(&spi->controller->cs_setup, setup, len);
+ else
+ memset(&spi->controller->cs_setup, 0, len);
+
+ if (hold)
+ memcpy(&spi->controller->cs_hold, hold, len);
+ else
+ memset(&spi->controller->cs_hold, 0, len);
+
+ if (inactive)
+ memcpy(&spi->controller->cs_inactive, inactive, len);
+ else
+ memset(&spi->controller->cs_inactive, 0, len);
+
+ return 0;
}
EXPORT_SYMBOL_GPL(spi_set_cs_timing);
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 592e50aae998..66031c8d0093 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -602,6 +602,11 @@ struct spi_controller {
/* Optimized handlers for SPI memory-like operations. */
const struct spi_controller_mem_ops *mem_ops;
+ /* CS delays */
+ struct spi_delaycs_setup;
+ struct spi_delaycs_hold;
+ struct spi_delaycs_inactive;
+
/* gpio chip select */
int *cs_gpios;
struct gpio_desc**cs_gpiods;
--
2.20.1
[PATCH v4 09/19] spi: use new `spi_transfer_delay_exec` helper where straightforward
For many places in the spi drivers, using the new `spi_transfer_delay`
helper is straightforward.
It's just replacing:
```
if (t->delay_usecs)
udelay(t->delay_usecs);
```
with `spi_transfer_delay(t)` which handles both `delay_usecs` and the new
`delay` field.
This change replaces in all places (in the spi drivers) where this change
is simple.
Signed-off-by: Alexandru Ardelean
---
drivers/spi/spi-atmel.c | 3 +--
drivers/spi/spi-bcm63xx-hsspi.c | 3 +--
drivers/spi/spi-cavium.c| 3 +--
drivers/spi/spi-fsl-dspi.c | 3 +--
drivers/spi/spi-fsl-espi.c | 3 +--
drivers/spi/spi-fsl-spi.c | 3 +--
drivers/spi/spi-mpc512x-psc.c | 3 +--
drivers/spi/spi-mpc52xx-psc.c | 3 +--
drivers/spi/spi-omap-100k.c | 3 +--
drivers/spi/spi-pl022.c | 25 +++--
drivers/spi/spi-sc18is602.c | 3 +--
drivers/spi/spi-sh-hspi.c | 3 +--
drivers/spi/spi-topcliff-pch.c | 7 +--
drivers/spi/spi-txx9.c | 3 +--
drivers/spi/spi-xcomm.c | 3 +--
15 files changed, 25 insertions(+), 46 deletions(-)
diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
index f04b92791515..2f7388e53b82 100644
--- a/drivers/spi/spi-atmel.c
+++ b/drivers/spi/spi-atmel.c
@@ -1389,8 +1389,7 @@ static int atmel_spi_one_transfer(struct spi_master
*master,
&& as->use_pdc)
atmel_spi_dma_unmap_xfer(master, xfer);
- if (xfer->delay_usecs)
- udelay(xfer->delay_usecs);
+ spi_transfer_delay_exec(xfer);
if (xfer->cs_change) {
if (list_is_last(&xfer->transfer_list,
diff --git a/drivers/spi/spi-bcm63xx-hsspi.c b/drivers/spi/spi-bcm63xx-hsspi.c
index 9a06ffdb73b8..949b59b6449f 100644
--- a/drivers/spi/spi-bcm63xx-hsspi.c
+++ b/drivers/spi/spi-bcm63xx-hsspi.c
@@ -291,8 +291,7 @@ static int bcm63xx_hsspi_transfer_one(struct spi_master
*master,
msg->actual_length += t->len;
- if (t->delay_usecs)
- udelay(t->delay_usecs);
+ spi_transfer_delay_exec(t);
if (t->cs_change)
bcm63xx_hsspi_set_cs(bs, spi->chip_select, false);
diff --git a/drivers/spi/spi-cavium.c b/drivers/spi/spi-cavium.c
index 5aaf21582cb5..6854c3ce423b 100644
--- a/drivers/spi/spi-cavium.c
+++ b/drivers/spi/spi-cavium.c
@@ -119,8 +119,7 @@ static int octeon_spi_do_transfer(struct octeon_spi *p,
*rx_buf++ = (u8)v;
}
- if (xfer->delay_usecs)
- udelay(xfer->delay_usecs);
+ spi_transfer_delay_exec(xfer);
return xfer->len;
}
diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index 53335ccc98f6..736071e2b9d8 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -752,8 +752,7 @@ static int dspi_transfer_one_message(struct spi_master
*master,
dspi->waitflags = 0;
}
- if (transfer->delay_usecs)
- udelay(transfer->delay_usecs);
+ spi_transfer_delay_exec(transfer);
}
out:
diff --git a/drivers/spi/spi-fsl-espi.c b/drivers/spi/spi-fsl-espi.c
index f20326714b9d..732bf3195917 100644
--- a/drivers/spi/spi-fsl-espi.c
+++ b/drivers/spi/spi-fsl-espi.c
@@ -427,8 +427,7 @@ static int fsl_espi_trans(struct spi_message *m, struct
spi_transfer *trans)
ret = fsl_espi_bufs(spi, trans);
- if (trans->delay_usecs)
- udelay(trans->delay_usecs);
+ spi_transfer_delay_exec(trans);
return ret;
}
diff --git a/drivers/spi/spi-fsl-spi.c b/drivers/spi/spi-fsl-spi.c
index 1d9b33aa1a3b..139ab57aec99 100644
--- a/drivers/spi/spi-fsl-spi.c
+++ b/drivers/spi/spi-fsl-spi.c
@@ -417,8 +417,7 @@ static int fsl_spi_do_one_msg(struct spi_master *master,
}
m->actual_length += t->len;
- if (t->delay_usecs)
- udelay(t->delay_usecs);
+ spi_transfer_delay_exec(t);
if (cs_change) {
ndelay(nsecs);
diff --git a/drivers/spi/spi-mpc512x-psc.c b/drivers/spi/spi-mpc512x-psc.c
index a337b842ae8c..ea1b07953d38 100644
--- a/drivers/spi/spi-mpc512x-psc.c
+++ b/drivers/spi/spi-mpc512x-psc.c
@@ -311,8 +311,7 @@ static int mpc512x_psc_spi_msg_xfer(struct spi_master
*master,
break;
m->actual_length += t->len;
- if (t->delay_usecs)
- udelay(t->delay_usecs);
+ spi_transfer_delay_exec(t);
if (cs_change)
mpc512x_psc_spi_deactivate_cs(spi);
diff --git a/drivers/spi/spi-mpc52xx-psc.c b/drivers/spi/spi-mpc52xx-psc.c
index c7e478b9b586..17935e71b02f 100644
--- a/drivers/spi/spi-mpc52xx-psc.c
+++ b/drivers/spi/spi-mpc52xx-psc.c
@@ -234,8 +234,7 @@ static void mpc52xx_psc_spi_work(struct work_struct *work)
break
[PATCH v4 11/19] spi: tegra20-sflash: use to new `spi_transfer_delay_exec`
This conversion to the spi_transfer_delay_exec() helper is not
straightforward, as it seems that when a delay is present, the controller
issues a command, and then a delay is followed.
This change adds support for the new `delay` field which is of type
`spi_delay` and keeps backwards compatibility with the old `delay_usecs`
field.
Signed-off-by: Alexandru Ardelean
---
drivers/spi/spi-tegra20-sflash.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/spi/spi-tegra20-sflash.c b/drivers/spi/spi-tegra20-sflash.c
index cd714a4f52c6..ff1933e55830 100644
--- a/drivers/spi/spi-tegra20-sflash.c
+++ b/drivers/spi/spi-tegra20-sflash.c
@@ -341,10 +341,11 @@ static int tegra_sflash_transfer_one_message(struct
spi_master *master,
goto exit;
}
msg->actual_length += xfer->len;
- if (xfer->cs_change && xfer->delay_usecs) {
+ if (xfer->cs_change &&
+ (xfer->delay_usecs || xfer->delay.value)) {
tegra_sflash_writel(tsd, tsd->def_command_reg,
SPI_COMMAND);
- udelay(xfer->delay_usecs);
+ spi_transfer_delay_exec(xfer);
}
}
ret = 0;
--
2.20.1
[PATCH v4 10/19] spi: tegra114: use `spi_transfer_delay_exec` helper
The tegra114 driver has a weird/separate `tegra_spi_transfer_delay()`
function that does 2 delays: one mdelay() and one udelay().
This was introduced via commit f4fade12d506e14867a2b0a5e2f7aaf227297d8b
("spi/tegra114: Correct support for cs_change").
There doesn't seem to be a mention in that commit message to suggest a
specific need/use-case for having the 2 delay calls.
For the most part, udelay() should be sufficient.
This change replaces it with the new `spi_transfer_delay_exec()`, which
should do the same thing.
Signed-off-by: Alexandru Ardelean
---
drivers/spi/spi-tegra114.c | 17 +++--
1 file changed, 3 insertions(+), 14 deletions(-)
diff --git a/drivers/spi/spi-tegra114.c b/drivers/spi/spi-tegra114.c
index 39374c2edcf3..8133dc49d34f 100644
--- a/drivers/spi/spi-tegra114.c
+++ b/drivers/spi/spi-tegra114.c
@@ -984,17 +984,6 @@ static int tegra_spi_setup(struct spi_device *spi)
return 0;
}
-static void tegra_spi_transfer_delay(int delay)
-{
- if (!delay)
- return;
-
- if (delay >= 1000)
- mdelay(delay / 1000);
-
- udelay(delay % 1000);
-}
-
static void tegra_spi_transfer_end(struct spi_device *spi)
{
struct tegra_spi_data *tspi = spi_master_get_devdata(spi->master);
@@ -1098,7 +1087,7 @@ static int tegra_spi_transfer_one_message(struct
spi_master *master,
complete_xfer:
if (ret < 0 || skip) {
tegra_spi_transfer_end(spi);
- tegra_spi_transfer_delay(xfer->delay_usecs);
+ spi_transfer_delay_exec(xfer);
goto exit;
} else if (list_is_last(&xfer->transfer_list,
&msg->transfers)) {
@@ -1106,11 +1095,11 @@ static int tegra_spi_transfer_one_message(struct
spi_master *master,
tspi->cs_control = spi;
else {
tegra_spi_transfer_end(spi);
- tegra_spi_transfer_delay(xfer->delay_usecs);
+ spi_transfer_delay_exec(xfer);
}
} else if (xfer->cs_change) {
tegra_spi_transfer_end(spi);
- tegra_spi_transfer_delay(xfer->delay_usecs);
+ spi_transfer_delay_exec(xfer);
}
}
--
2.20.1
[PATCH v4 12/19] spi: spi-loopback-test: use new `delay` field
This change replaces the use of the `delay_usecs` field with the new
`delay` field. The code/test still uses micro-seconds, but they are now
configured and used via the `struct spi_delay` format of the `delay` field.
Signed-off-by: Alexandru Ardelean
---
drivers/spi/spi-loopback-test.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/spi/spi-loopback-test.c b/drivers/spi/spi-loopback-test.c
index 6f18d4952767..b6d79cd156fb 100644
--- a/drivers/spi/spi-loopback-test.c
+++ b/drivers/spi/spi-loopback-test.c
@@ -298,12 +298,18 @@ static struct spi_test spi_tests[] = {
{
.tx_buf = TX(0),
.rx_buf = RX(0),
- .delay_usecs = 1000,
+ .delay = {
+ .value = 1000,
+ .unit = SPI_DELAY_UNIT_USECS,
+ },
},
{
.tx_buf = TX(0),
.rx_buf = RX(0),
- .delay_usecs = 1000,
+ .delay = {
+ .value = 1000,
+ .unit = SPI_DELAY_UNIT_USECS,
+ },
},
},
},
@@ -537,7 +543,7 @@ static int spi_test_check_elapsed_time(struct spi_device
*spi,
unsigned long long nbits = (unsigned long long)BITS_PER_BYTE *
xfer->len;
- delay_usecs += xfer->delay_usecs;
+ delay_usecs += xfer->delay.value;
if (!xfer->speed_hz)
continue;
estimated_time += div_u64(nbits * NSEC_PER_SEC, xfer->speed_hz);
--
2.20.1
[PATCH v4 14/19] spi: tegra114: change format for `spi_set_cs_timing()` function
The initial version of `spi_set_cs_timing()` was implemented with
consideration only for clock-cycles as delay.
For cases like `CS setup` time, it's sometimes needed that micro-seconds
(or nano-seconds) are required, or sometimes even longer delays, for cases
where the device needs a little longer to start transferring that after CS
is asserted.
Signed-off-by: Alexandru Ardelean
---
drivers/spi/spi-tegra114.c | 22 --
drivers/spi/spi.c | 16 ++--
include/linux/spi/spi.h| 9 ++---
3 files changed, 36 insertions(+), 11 deletions(-)
diff --git a/drivers/spi/spi-tegra114.c b/drivers/spi/spi-tegra114.c
index 8133dc49d34f..e6a450d9b4f0 100644
--- a/drivers/spi/spi-tegra114.c
+++ b/drivers/spi/spi-tegra114.c
@@ -723,15 +723,31 @@ static void tegra_spi_deinit_dma_param(struct
tegra_spi_data *tspi,
dma_release_channel(dma_chan);
}
-static void tegra_spi_set_hw_cs_timing(struct spi_device *spi, u8 setup_dly,
- u8 hold_dly, u8 inactive_dly)
+static int tegra_spi_set_hw_cs_timing(struct spi_device *spi,
+ struct spi_delay *setup,
+ struct spi_delay *hold,
+ struct spi_delay *inactive)
{
struct tegra_spi_data *tspi = spi_master_get_devdata(spi->master);
+ u8 setup_dly, hold_dly, inactive_dly;
u32 setup_hold;
u32 spi_cs_timing;
u32 inactive_cycles;
u8 cs_state;
+ if ((setup && setup->unit != SPI_DELAY_UNIT_SCK) ||
+ (hold && hold->unit != SPI_DELAY_UNIT_SCK) ||
+ (inactive && inactive->unit != SPI_DELAY_UNIT_SCK)) {
+ dev_err(&spi->dev,
+ "Invalid delay unit %d, should be SPI_DELAY_UNIT_SCK\n",
+ SPI_DELAY_UNIT_SCK);
+ return -EINVAL;
+ }
+
+ setup_dly = setup ? setup->value : 0;
+ hold_dly = hold ? hold->value : 0;
+ inactive_dly = inactive ? inactive->value : 0;
+
setup_dly = min_t(u8, setup_dly, MAX_SETUP_HOLD_CYCLES);
hold_dly = min_t(u8, hold_dly, MAX_SETUP_HOLD_CYCLES);
if (setup_dly && hold_dly) {
@@ -758,6 +774,8 @@ static void tegra_spi_set_hw_cs_timing(struct spi_device
*spi, u8 setup_dly,
tspi->spi_cs_timing2 = spi_cs_timing;
tegra_spi_writel(tspi, spi_cs_timing, SPI_CS_TIMING2);
}
+
+ return 0;
}
static u32 tegra_spi_setup_transfer_one(struct spi_device *spi,
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 489eb64d9ee0..122af2264bfe 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -3147,15 +3147,19 @@ EXPORT_SYMBOL_GPL(spi_setup);
/**
* spi_set_cs_timing - configure CS setup, hold, and inactive delays
* @spi: the device that requires specific CS timing configuration
- * @setup: CS setup time in terms of clock count
- * @hold: CS hold time in terms of clock count
- * @inactive_dly: CS inactive delay between transfers in terms of clock count
+ * @setup: CS setup time specified via @spi_delay
+ * @hold: CS hold time specified via @spi_delay
+ * @inactive: CS inactive delay between transfers specified via @spi_delay
+ *
+ * Return: zero on success, else a negative error code.
*/
-void spi_set_cs_timing(struct spi_device *spi, u8 setup, u8 hold,
- u8 inactive_dly)
+int spi_set_cs_timing(struct spi_device *spi, struct spi_delay *setup,
+ struct spi_delay *hold, struct spi_delay *inactive)
{
if (spi->controller->set_cs_timing)
- spi->controller->set_cs_timing(spi, setup, hold, inactive_dly);
+ return spi->controller->set_cs_timing(spi, setup, hold,
+ inactive);
+ return -ENOTSUPP;
}
EXPORT_SYMBOL_GPL(spi_set_cs_timing);
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 2cee9ac35737..592e50aae998 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -517,8 +517,8 @@ struct spi_controller {
* to configure specific CS timing through spi_set_cs_timing() after
* spi_setup().
*/
- void (*set_cs_timing)(struct spi_device *spi, u8 setup_clk_cycles,
- u8 hold_clk_cycles, u8 inactive_clk_cycles);
+ int (*set_cs_timing)(struct spi_device *spi, struct spi_delay *setup,
+struct spi_delay *hold, struct spi_delay
*inactive);
/* bidirectional bulk transfers
*
@@ -1007,7 +1007,10 @@ static inline void spi_message_free(struct spi_message
*m)
kfree(m);
}
-extern void spi_set_cs_timing(struct spi_device *spi, u8 setup, u8 hold, u8
inactive_dly);
+extern int spi_set_cs_timing(struct spi_device *spi,
+struct spi_delay *setup,
+struct spi_delay *hold,
+struct spi_delay *inactive);
[PATCH v4 05/19] spi: orion: use new `word_delay` field for SPI transfers
The `word_delay` field had it's type changed to `struct spi_delay`.
This allows users to specify nano-second or clock-cycle delays (if needed).
Converting to use `word_delay` is straightforward: it just uses the new
`spi_delay_exec()` routine, that handles the `unit` part.
Signed-off-by: Alexandru Ardelean
---
drivers/spi/spi-orion.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/spi-orion.c b/drivers/spi/spi-orion.c
index 6643ccdc2508..756c37e32dfe 100644
--- a/drivers/spi/spi-orion.c
+++ b/drivers/spi/spi-orion.c
@@ -467,8 +467,7 @@ orion_spi_write_read(struct spi_device *spi, struct
spi_transfer *xfer)
if (orion_spi_write_read_8bit(spi, &tx, &rx) < 0)
goto out;
count--;
- if (xfer->word_delay_usecs)
- udelay(xfer->word_delay_usecs);
+ spi_delay_exec(&xfer->word_delay, xfer);
} while (count);
} else if (word_len == 16) {
const u16 *tx = xfer->tx_buf;
@@ -478,8 +477,7 @@ orion_spi_write_read(struct spi_device *spi, struct
spi_transfer *xfer)
if (orion_spi_write_read_16bit(spi, &tx, &rx) < 0)
goto out;
count -= 2;
- if (xfer->word_delay_usecs)
- udelay(xfer->word_delay_usecs);
+ spi_delay_exec(&xfer->word_delay, xfer);
} while (count);
}
--
2.20.1
[PATCH v4 16/19] spi: spi-fsl-espi: convert transfer delay to `spi_delay` format
The way the max delay is computed for this controller, it looks like it is
searching for the max delay from an SPI message a using that.
No idea if this is valid. But this change should support both `delay_usecs`
and the new `delay` data which is of `spi_delay` type.
Signed-off-by: Alexandru Ardelean
---
drivers/spi/spi-fsl-espi.c | 16 +---
drivers/spi/spi.c | 9 +
include/linux/spi/spi.h| 1 +
3 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/spi/spi-fsl-espi.c b/drivers/spi/spi-fsl-espi.c
index 732bf3195917..e60581283a24 100644
--- a/drivers/spi/spi-fsl-espi.c
+++ b/drivers/spi/spi-fsl-espi.c
@@ -436,6 +436,7 @@ static int fsl_espi_do_one_msg(struct spi_master *master,
struct spi_message *m)
{
unsigned int delay_usecs = 0, rx_nbits = 0;
+ unsigned int delay_nsecs = 0, delay_nsecs1 = 0;
struct spi_transfer *t, trans = {};
int ret;
@@ -444,8 +445,16 @@ static int fsl_espi_do_one_msg(struct spi_master *master,
goto out;
list_for_each_entry(t, &m->transfers, transfer_list) {
- if (t->delay_usecs > delay_usecs)
- delay_usecs = t->delay_usecs;
+ if (t->delay_usecs) {
+ if (t->delay_usecs > delay_usecs) {
+ delay_usecs = t->delay_usecs;
+ delay_nsecs = delay_usecs * 1000;
+ }
+ } else {
+ delay_nsecs1 = spi_delay_to_ns(&t->delay, t);
+ if (delay_nsecs1 > delay_nsecs)
+ delay_nsecs = delay_nsecs1;
+ }
if (t->rx_nbits > rx_nbits)
rx_nbits = t->rx_nbits;
}
@@ -456,7 +465,8 @@ static int fsl_espi_do_one_msg(struct spi_master *master,
trans.len = m->frame_length;
trans.speed_hz = t->speed_hz;
trans.bits_per_word = t->bits_per_word;
- trans.delay_usecs = delay_usecs;
+ trans.delay.value = delay_nsecs;
+ trans.delay.unit = SPI_DELAY_UNIT_NSECS;
trans.rx_nbits = rx_nbits;
if (trans.len)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 61ef286f954a..1f0b55c9e2e8 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1120,7 +1120,7 @@ static void _spi_transfer_delay_ns(u32 ns)
}
}
-static int _spi_delay_to_ns(struct spi_delay *_delay, struct spi_transfer
*xfer)
+int spi_delay_to_ns(struct spi_delay *_delay, struct spi_transfer *xfer)
{
u32 delay = _delay->value;
u32 unit = _delay->unit;
@@ -1153,6 +1153,7 @@ static int _spi_delay_to_ns(struct spi_delay *_delay,
struct spi_transfer *xfer)
return delay;
}
+EXPORT_SYMBOL_GPL(spi_delay_to_ns);
int spi_delay_exec(struct spi_delay *_delay, struct spi_transfer *xfer)
{
@@ -1161,7 +1162,7 @@ int spi_delay_exec(struct spi_delay *_delay, struct
spi_transfer *xfer)
if (!_delay)
return -EINVAL;
- delay = _spi_delay_to_ns(_delay, xfer);
+ delay = spi_delay_to_ns(_delay, xfer);
if (delay < 0)
return delay;
@@ -3211,11 +3212,11 @@ static int _spi_xfer_word_delay_update(struct
spi_transfer *xfer,
{
int delay1, delay2;
- delay1 = _spi_delay_to_ns(&xfer->word_delay, xfer);
+ delay1 = spi_delay_to_ns(&xfer->word_delay, xfer);
if (delay1 < 0)
return delay1;
- delay2 = _spi_delay_to_ns(&spi->word_delay, xfer);
+ delay2 = spi_delay_to_ns(&spi->word_delay, xfer);
if (delay2 < 0)
return delay2;
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 66031c8d0093..3bc5912fca36 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -102,6 +102,7 @@ struct spi_delay {
u8 unit;
};
+extern int spi_delay_to_ns(struct spi_delay *_delay, struct spi_transfer
*xfer);
extern int spi_delay_exec(struct spi_delay *_delay, struct spi_transfer *xfer);
/**
--
2.20.1
[PATCH v4 13/19] spi: spidev: use new `delay` field for spi transfers
The `delay` field has type `struct spi_delay`. This allows users to specify nano-second or clock-cycle delays (if needed). Converting to use `delay` is straightforward: it's just assigning the value to `delay.value` and hard-coding the `delay.unit` to `SPI_DELAY_UNIT_USECS`. This keeps the uapi for spidev un-changed. Changing it can be part of another changeset and discussion. Signed-off-by: Alexandru Ardelean --- drivers/spi/spidev.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c index 495319c2fa11..6b6afd27f4a1 100644 --- a/drivers/spi/spidev.c +++ b/drivers/spi/spidev.c @@ -265,7 +265,8 @@ static int spidev_message(struct spidev_data *spidev, k_tmp->tx_nbits = u_tmp->tx_nbits; k_tmp->rx_nbits = u_tmp->rx_nbits; k_tmp->bits_per_word = u_tmp->bits_per_word; - k_tmp->delay_usecs = u_tmp->delay_usecs; + k_tmp->delay.value = u_tmp->delay_usecs; + k_tmp->delay.unit = SPI_DELAY_UNIT_USECS; k_tmp->speed_hz = u_tmp->speed_hz; k_tmp->word_delay.value = u_tmp->word_delay_usecs; k_tmp->word_delay.unit = SPI_DELAY_UNIT_USECS; -- 2.20.1
Re: [PATCHv4-next 0/3] Odroid c2 usb fixs rebase on linux-next
Hi Kevin, On Thu, 26 Sep 2019 at 03:34, Kevin Hilman wrote: > > Anand Moon writes: > > > Some time ago I had tired to enable usb bus 1 for Odroid C2/C1 > > but it's look like some more work is needed to u-boot and > > usb_phy driver to initialize this port. > > > > Below patches tries to address the issue regarding usb bus 2 (4 port) > > while disable the usb bus 1 on this board. > > > > Previous patch > > [0] https://lkml.org/lkml/2019/1/29/325 > > > > Re send below series based re based on linux-next-20190830. > > For review and testing. > > > > [1] https://patchwork.kernel.org/cover/3091/ > > > > Small changes from previous series. > > Fix the commit message for patch 1 > > Queued for v5.5. > > I fixed up the typo in patch 2/3 when applying as suggested by Martin. > > Kevin Thanks, Best Regards -Anand
[PATCH v4 18/19] spi: bcm63xx: extend error condition to `delay` as well
The driver errors out if `delay_usecs` is non-zero. This error condition
should be extended to the new `delay` field, to account for when it will be
used.
Signed-off-by: Alexandru Ardelean
---
drivers/spi/spi-bcm63xx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/spi/spi-bcm63xx.c b/drivers/spi/spi-bcm63xx.c
index df1c94a131e6..4e582acda709 100644
--- a/drivers/spi/spi-bcm63xx.c
+++ b/drivers/spi/spi-bcm63xx.c
@@ -368,7 +368,7 @@ static int bcm63xx_spi_transfer_one(struct spi_master
*master,
}
/* CS will be deasserted directly after transfer */
- if (t->delay_usecs) {
+ if (t->delay_usecs || t->delay.value) {
dev_err(&spi->dev, "unable to keep CS asserted after
transfer\n");
status = -EINVAL;
goto exit;
--
2.20.1
[PATCH v4 17/19] spi: spi-falcon: extend warning to `delay` as well
The WARN_ON macro prints a warning in syslog if `delay_usecs` is non-zero. However, with the new intermediate `delay`, the warning should also be printed. Signed-off-by: Alexandru Ardelean --- drivers/spi/spi-falcon.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/spi/spi-falcon.c b/drivers/spi/spi-falcon.c index 00f46c816a56..d3336a63f462 100644 --- a/drivers/spi/spi-falcon.c +++ b/drivers/spi/spi-falcon.c @@ -377,7 +377,7 @@ static int falcon_sflash_xfer_one(struct spi_master *master, m->actual_length += t->len; - WARN_ON(t->delay_usecs || t->cs_change); + WARN_ON(t->delay_usecs || t->delay.value || t->cs_change); spi_flags = 0; } -- 2.20.1
[PATCH v4 19/19] spi: spi-axi: extend support for the `delay` field
The AXI SPI engine driver uses the `delay_usecs` field from `spi_transfer`
to configure delays, which the controller will execute.
This change extends the logic to also include the `delay` value, in case it
is used (instead if `delay_usecs`).
Signed-off-by: Alexandru Ardelean
---
drivers/spi/spi-axi-spi-engine.c | 16 +---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/spi/spi-axi-spi-engine.c b/drivers/spi/spi-axi-spi-engine.c
index 3b1833e6c7ad..111d3b83285f 100644
--- a/drivers/spi/spi-axi-spi-engine.c
+++ b/drivers/spi/spi-axi-spi-engine.c
@@ -163,10 +163,21 @@ static void spi_engine_gen_xfer(struct spi_engine_program
*p, bool dry,
}
static void spi_engine_gen_sleep(struct spi_engine_program *p, bool dry,
- struct spi_engine *spi_engine, unsigned int clk_div, unsigned int delay)
+ struct spi_engine *spi_engine, unsigned int clk_div,
+ struct spi_transfer *xfer)
{
unsigned int spi_clk = clk_get_rate(spi_engine->ref_clk);
unsigned int t;
+ int delay;
+
+ if (xfer->delay_usecs) {
+ delay = xfer->delay_usecs;
+ } else {
+ delay = spi_delay_to_ns(&xfer->delay, xfer);
+ if (delay < 0)
+ return;
+ delay /= 1000;
+ }
if (delay == 0)
return;
@@ -218,8 +229,7 @@ static int spi_engine_compile_message(struct spi_engine
*spi_engine,
spi_engine_gen_cs(p, dry, spi, true);
spi_engine_gen_xfer(p, dry, xfer);
- spi_engine_gen_sleep(p, dry, spi_engine, clk_div,
- xfer->delay_usecs);
+ spi_engine_gen_sleep(p, dry, spi_engine, clk_div, xfer);
cs_change = xfer->cs_change;
if (list_is_last(&xfer->transfer_list, &msg->transfers))
--
2.20.1
[PATCH v4 06/19] spi: spidev: use new `word_delay` field for spi transfers
The `word_delay` field had it's type changed to `struct spi_delay`. This allows users to specify nano-second or clock-cycle delays (if needed). Converting to use `word_delay` is straightforward: it's just assigning the value to `word_delay.value` and hard-coding the `word_delay.unit` to `SPI_DELAY_UNIT_USECS` This keeps the uapi for spidev un-changed. Changing it can be part of another changeset and discussion. Signed-off-by: Alexandru Ardelean --- drivers/spi/spidev.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c index 255786f2e844..495319c2fa11 100644 --- a/drivers/spi/spidev.c +++ b/drivers/spi/spidev.c @@ -267,7 +267,8 @@ static int spidev_message(struct spidev_data *spidev, k_tmp->bits_per_word = u_tmp->bits_per_word; k_tmp->delay_usecs = u_tmp->delay_usecs; k_tmp->speed_hz = u_tmp->speed_hz; - k_tmp->word_delay_usecs = u_tmp->word_delay_usecs; + k_tmp->word_delay.value = u_tmp->word_delay_usecs; + k_tmp->word_delay.unit = SPI_DELAY_UNIT_USECS; if (!k_tmp->speed_hz) k_tmp->speed_hz = spidev->speed_hz; #ifdef VERBOSE -- 2.20.1
[PATCH v4 02/19] spi: introduce spi_delay struct as "value + unit" & spi_delay_exec()
There are plenty of delays that have been introduced in SPI core. Most of
them are in micro-seconds, some need to be in nano-seconds, and some in
clock-cycles.
For some of these delays (related to transfers & CS timing) it may make
sense to have a `spi_delay` struct that abstracts these a bit.
The important element of these delays [for unification] seems to be the
`unit` of the delay.
It looks like micro-seconds is good enough for most people, but every-once
in a while, some delays seem to require other units of measurement.
This change adds the `spi_delay` struct & a `spi_delay_exec()` function
that processes a `spi_delay` object/struct to execute the delay.
It's a copy of the `cs_change_delay` mechanism, but without the default
for 10 uS.
The clock-cycle delay unit is a bit special, as it needs to be bound to an
`spi_transfer` object to execute.
Signed-off-by: Alexandru Ardelean
---
drivers/spi/spi.c | 51 +
include/linux/spi/spi.h | 18 ---
2 files changed, 66 insertions(+), 3 deletions(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index c90e02e6d62f..1883de8ffa82 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1106,6 +1106,57 @@ static void _spi_transfer_delay_ns(u32 ns)
}
}
+static int _spi_delay_to_ns(struct spi_delay *_delay, struct spi_transfer
*xfer)
+{
+ u32 delay = _delay->value;
+ u32 unit = _delay->unit;
+ u32 hz;
+
+ if (!delay)
+ return 0;
+
+ switch (unit) {
+ case SPI_DELAY_UNIT_USECS:
+ delay *= 1000;
+ break;
+ case SPI_DELAY_UNIT_NSECS: /* nothing to do here */
+ break;
+ case SPI_DELAY_UNIT_SCK:
+ /* clock cycles need to be obtained from spi_transfer */
+ if (!xfer)
+ return -EINVAL;
+ /* if there is no effective speed know, then approximate
+* by underestimating with half the requested hz
+*/
+ hz = xfer->effective_speed_hz ?: xfer->speed_hz / 2;
+ if (!hz)
+ return -EINVAL;
+ delay *= DIV_ROUND_UP(10, hz);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return delay;
+}
+
+int spi_delay_exec(struct spi_delay *_delay, struct spi_transfer *xfer)
+{
+ int delay;
+
+ if (!_delay)
+ return -EINVAL;
+
+ delay = _spi_delay_to_ns(_delay, xfer);
+ if (delay < 0)
+ return delay;
+
+ _spi_transfer_delay_ns(delay);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(spi_delay_exec);
+
static void _spi_transfer_cs_change_delay(struct spi_message *msg,
struct spi_transfer *xfer)
{
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index af4f265d0f67..c18cfa7cda35 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -89,6 +89,21 @@ void spi_statistics_add_transfer_stats(struct spi_statistics
*stats,
#define SPI_STATISTICS_INCREMENT_FIELD(stats, field) \
SPI_STATISTICS_ADD_TO_FIELD(stats, field, 1)
+/**
+ * struct spi_delay - SPI delay information
+ * @value: Value for the delay
+ * @unit: Unit for the delay
+ */
+struct spi_delay {
+#define SPI_DELAY_UNIT_USECS 0
+#define SPI_DELAY_UNIT_NSECS 1
+#define SPI_DELAY_UNIT_SCK 2
+ u16 value;
+ u8 unit;
+};
+
+extern int spi_delay_exec(struct spi_delay *_delay, struct spi_transfer *xfer);
+
/**
* struct spi_device - Controller side proxy for an SPI slave device
* @dev: Driver model representation of the device.
@@ -834,9 +849,6 @@ struct spi_transfer {
u16 delay_usecs;
u16 cs_change_delay;
u8 cs_change_delay_unit;
-#define SPI_DELAY_UNIT_USECS 0
-#define SPI_DELAY_UNIT_NSECS 1
-#define SPI_DELAY_UNIT_SCK 2
u32 speed_hz;
u16 word_delay;
--
2.20.1
Re: [Xen-devel] [PATCH] xen: xen-pciback: Reset MSI-X state when exposing a device
Hello Pasi, Unfortunately, I am not able to continue the work on the Xen patches in the foreseeable future. For what it's worth: the xen-pciback workaround from this thread solves my current issue as confirmed by internal testing. -- Stanislav (apologies for ugly footer injected below by company SMTP server due to local laws) On Thu, 2019-09-26 at 13:13 +0300, Pasi Kärkkäinen wrote: > Hello Stanislav, > > On Fri, Sep 13, 2019 at 11:28:20PM +0800, Chao Gao wrote: > > On Fri, Sep 13, 2019 at 10:02:24AM +, Spassov, Stanislav wrote: > > > On Thu, Dec 13, 2018 at 07:54, Chao Gao wrote: > > > > On Thu, Dec 13, 2018 at 12:54:52AM -0700, Jan Beulich wrote: > > > > > > > > On 13.12.18 at 04:46, wrote: > > > > > > > > > > > > On Wed, Dec 12, 2018 at 08:21:39AM -0700, Jan Beulich > > > > > > wrote: > > > > > > > > > > On 12.12.18 at 16:18, wrote: > > > > > > > > > > > > > > > > On Wed, Dec 12, 2018 at 01:51:01AM -0700, Jan Beulich > > > > > > > > wrote: > > > > > > > > > > > > On 12.12.18 at 08:06, > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > On Wed, Dec 05, 2018 at 09:01:33AM -0500, Boris > > > > > > > > > > Ostrovsky wrote: > > > > > > > > > > > On 12/5/18 4:32 AM, Roger Pau Monné wrote: > > > > > > > > > > > > On Wed, Dec 05, 2018 at 10:19:17AM +0800, Chao > > > > > > > > > > > > Gao wrote: > > > > > > > > > > > > > I find some pass-thru devices don't work any > > > > > > > > > > > > > more across guest reboot. > > > > > > > > > > > > > Assigning it to another guest also meets the > > > > > > > > > > > > > same issue. And the only > > > > > > > > > > > > > way to make it work again is un-binding and > > > > > > > > > > > > > binding it to pciback. > > > > > > > > > > > > > Someone reported this issue one year ago [1]. > > > > > > > > > > > > > More detail also can be > > > > > > > > > > > > > found in [2]. > > > > > > > > > > > > > > > > > > > > > > > > > > The root-cause is Xen's internal MSI-X state > > > > > > > > > > > > > isn't reset properly > > > > > > > > > > > > > during reboot or re-assignment. In the above > > > > > > > > > > > > > case, Xen set maskall bit > > > > > > > > > > > > > to mask all MSI interrupts after it detected > > > > > > > > > > > > > a potential security > > > > > > > > > > > > > issue. Even after device reset, Xen didn't > > > > > > > > > > > > > reset its internal maskall > > > > > > > > > > > > > bit. As a result, maskall bit would be set > > > > > > > > > > > > > again in next write to > > > > > > > > > > > > > MSI-X message control register. > > > > > > > > > > > > > > > > > > > > > > > > > > Given that PHYSDEVOPS_prepare_msix() also > > > > > > > > > > > > > triggers Xen resetting MSI-X > > > > > > > > > > > > > internal state of a device, we employ it to > > > > > > > > > > > > > fix this issue rather than > > > > > > > > > > > > > introducing another dedicated sub-hypercall. > > > > > > > > > > > > > > > > > > > > > > > > > > Note that PHYSDEVOPS_release_msix() will fail > > > > > > > > > > > > > if the mapping between > > > > > > > > > > > > > the device's msix and pirq has been created. > > > > > > > > > > > > > This limitation prevents > > > > > > > > > > > > > us calling this function when detaching a > > > > > > > > > > > > > device from a guest during > > > > > > > > > > > > > guest shutdown. Thus it is called right > > > > > > > > > > > > > before calling > > > > > > > > > > > > > PHYSDEVOPS_prepare_msix(). > > > > > > > > > > > > > > > > > > > > > > > > s/PHYSDEVOPS/PHYSDEVOP/ (no final S). And then > > > > > > > > > > > > I would also drop the > > > > > > > > > > > > () at the end of the hypercall name since it's > > > > > > > > > > > > not a function. > > > > > > > > > > > > > > > > > > > > > > > > I'm also wondering why the release can't be > > > > > > > > > > > > done when the device is > > > > > > > > > > > > detached from the guest (or the guest has been > > > > > > > > > > > > shut down). This makes > > > > > > > > > > > > me worry about the raciness of the > > > > > > > > > > > > attach/detach procedure: if there's > > > > > > > > > > > > a state where pciback assumes the device has > > > > > > > > > > > > been detached from the > > > > > > > > > > > > guest, but there are still pirqs bound, an > > > > > > > > > > > > attempt to attach to > > > > > > > > > > > > another guest in such state will fail. > > > > > > > > > > > > > > > > > > > > > > I wonder whether this additional reset > > > > > > > > > > > functionality could be done out > > > > > > > > > > > of xen_pcibk_xenbus_remove(). We first do a (best > > > > > > > > > > > effort) device reset > > > > > > > > > > > and then do the extra things that are not > > > > > > > > > > > properly done there. > > > > > > > > > > > > > > > > > > > > No. It cannot be done in xen_pcibk_xenbus_remove() > > > > > > > > > > without modifying > > > > > > > > > > the handler of PHYSDEVOP_release_msix. To do a > > > > > > > > > > successful Xen internal > > > > > > > > > > MSI-X
Re: [PATCH 1/4] arm64: vdso32: Introduce COMPAT_CC_IS_GCC
On 9/26/19 9:06 AM, Catalin Marinas wrote: > On Thu, Sep 26, 2019 at 07:03:50AM +0100, Vincenzo Frascino wrote: >> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig >> index 37c610963eee..afe8c948b493 100644 >> --- a/arch/arm64/Kconfig >> +++ b/arch/arm64/Kconfig >> @@ -110,7 +110,7 @@ config ARM64 >> select GENERIC_STRNLEN_USER >> select GENERIC_TIME_VSYSCALL >> select GENERIC_GETTIMEOFDAY >> -select GENERIC_COMPAT_VDSO if (!CPU_BIG_ENDIAN && COMPAT) >> +select GENERIC_COMPAT_VDSO if (!CPU_BIG_ENDIAN && COMPAT && >> COMPAT_CC_IS_GCC) >> select HANDLE_DOMAIN_IRQ >> select HARDIRQS_SW_RESEND >> select HAVE_PCI >> @@ -313,6 +313,9 @@ config KASAN_SHADOW_OFFSET >> default 0xeff9 if ARM64_VA_BITS_36 && KASAN_SW_TAGS >> default 0x >> >> +config COMPAT_CC_IS_GCC >> +def_bool $(success,$(CROSS_COMPILE_COMPAT)gcc --version | head -n 1 | >> grep -q arm) > > Nitpick: I prefer COMPATCC instead of COMPAT_CC for consistency with > HOSTCC. > Ok, will change this in v2. > Now, could we not generate a COMPATCC in the Makefile and use > $(COMPATCC) here instead of $(CROSS_COMPILE_COMPAT)gcc? It really > doesn't make sense to check that gcc is gcc. > All right, COMPATCC is already in the makefile, I will use it in here. > A next step would be to check that COMPATCC can actually generate 32-bit > objects. But it's not essential at this stage. > We are already checking this making sure that arm is present in the triple (grep -q arm). >> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile >> index 84a3d502c5a5..34f53eb11878 100644 >> --- a/arch/arm64/Makefile >> +++ b/arch/arm64/Makefile >> @@ -54,19 +54,8 @@ $(warning Detected assembler with broken .inst; >> disassembly will be unreliable) >> endif >> >> ifeq ($(CONFIG_GENERIC_COMPAT_VDSO), y) >> - CROSS_COMPILE_COMPAT ?= $(CONFIG_CROSS_COMPILE_COMPAT_VDSO:"%"=%) >> - >> - ifeq ($(CONFIG_CC_IS_CLANG), y) >> -$(warning CROSS_COMPILE_COMPAT is clang, the compat vDSO will not be >> built) >> - else ifeq ($(strip $(CROSS_COMPILE_COMPAT)),) >> -$(warning CROSS_COMPILE_COMPAT not defined or empty, the compat vDSO >> will not be built) >> - else ifeq ($(shell which $(CROSS_COMPILE_COMPAT)gcc 2> /dev/null),) >> -$(error $(CROSS_COMPILE_COMPAT)gcc not found, check >> CROSS_COMPILE_COMPAT) >> - else >> -export CROSS_COMPILE_COMPAT >> -export CONFIG_COMPAT_VDSO := y >> -compat_vdso := -DCONFIG_COMPAT_VDSO=1 >> - endif >> + export CONFIG_COMPAT_VDSO := y >> + compat_vdso := -DCONFIG_COMPAT_VDSO=1 >> endif > > Has CONFIG_CROSS_COMPILE_COMPAT_VDSO actually been removed from > lib/vdso/Kconfig? (I haven't checked the subsequent patches). > -- Regards, Vincenzo
[PATCH v4 04/19] spi: sprd: convert transfer word delay to spi_delay struct
The Spreadtrum SPI driver is the only user of the `word_delay` field in
the `spi_transfer` struct.
This change converts the field to use the `spi_delay` struct. This also
enforces the users to specify the delay unit to be `SPI_DELAY_UNIT_SCK`.
Signed-off-by: Alexandru Ardelean
---
drivers/spi/spi-sprd.c | 15 ---
include/linux/spi/spi.h | 4 ++--
2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/spi/spi-sprd.c b/drivers/spi/spi-sprd.c
index 1b7eebb72c07..7e72e8facff7 100644
--- a/drivers/spi/spi-sprd.c
+++ b/drivers/spi/spi-sprd.c
@@ -669,11 +669,15 @@ static void sprd_spi_set_speed(struct sprd_spi *ss, u32
speed_hz)
writel_relaxed(clk_div, ss->base + SPRD_SPI_CLKD);
}
-static void sprd_spi_init_hw(struct sprd_spi *ss, struct spi_transfer *t)
+static int sprd_spi_init_hw(struct sprd_spi *ss, struct spi_transfer *t)
{
+ struct spi_delay *d = &t->word_delay;
u16 word_delay, interval;
u32 val;
+ if (d->unit != SPI_DELAY_UNIT_SCK)
+ return -EINVAL;
+
val = readl_relaxed(ss->base + SPRD_SPI_CTL7);
val &= ~(SPRD_SPI_SCK_REV | SPRD_SPI_NG_TX | SPRD_SPI_NG_RX);
/* Set default chip selection, clock phase and clock polarity */
@@ -686,7 +690,7 @@ static void sprd_spi_init_hw(struct sprd_spi *ss, struct
spi_transfer *t)
* formula as below per datasheet:
* interval time (source clock cycles) = interval * 4 + 10.
*/
- word_delay = clamp_t(u16, t->word_delay, SPRD_SPI_MIN_DELAY_CYCLE,
+ word_delay = clamp_t(u16, d->value, SPRD_SPI_MIN_DELAY_CYCLE,
SPRD_SPI_MAX_DELAY_CYCLE);
interval = DIV_ROUND_UP(word_delay - 10, 4);
ss->word_delay = interval * 4 + 10;
@@ -711,6 +715,8 @@ static void sprd_spi_init_hw(struct sprd_spi *ss, struct
spi_transfer *t)
val &= ~SPRD_SPI_DATA_LINE2_EN;
writel_relaxed(val, ss->base + SPRD_SPI_CTL7);
+
+ return 0;
}
static int sprd_spi_setup_transfer(struct spi_device *sdev,
@@ -719,13 +725,16 @@ static int sprd_spi_setup_transfer(struct spi_device
*sdev,
struct sprd_spi *ss = spi_controller_get_devdata(sdev->controller);
u8 bits_per_word = t->bits_per_word;
u32 val, mode = 0;
+ int ret;
ss->len = t->len;
ss->tx_buf = t->tx_buf;
ss->rx_buf = t->rx_buf;
ss->hw_mode = sdev->mode;
- sprd_spi_init_hw(ss, t);
+ ret = sprd_spi_init_hw(ss, t);
+ if (ret)
+ return ret;
/* Set tansfer speed and valid bits */
sprd_spi_set_speed(ss, t->speed_hz);
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 9ded3f44d58e..16397b210e0c 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -759,7 +759,7 @@ extern void spi_res_release(struct spi_controller *ctlr,
* the next transfer or completing this @spi_message.
* @word_delay_usecs: microseconds to inter word delay after each word size
* (set by bits_per_word) transmission.
- * @word_delay: clock cycles to inter word delay after each word size
+ * @word_delay: inter word delay to be introduced after each word size
* (set by bits_per_word) transmission.
* @effective_speed_hz: the effective SCK-speed that was used to
* transfer this transfer. Set to 0 if the spi bus driver does
@@ -847,8 +847,8 @@ struct spi_transfer {
u8 word_delay_usecs;
u16 delay_usecs;
struct spi_delaycs_change_delay;
+ struct spi_delayword_delay;
u32 speed_hz;
- u16 word_delay;
u32 effective_speed_hz;
--
2.20.1
Re: [PATCH 2/4] arm64: vdso32: Detect binutils support for dmb ishld
On 9/26/19 9:30 AM, Catalin Marinas wrote:
> On Thu, Sep 26, 2019 at 07:03:51AM +0100, Vincenzo Frascino wrote:
>> diff --git a/arch/arm64/include/asm/vdso/compat_barrier.h
>> b/arch/arm64/include/asm/vdso/compat_barrier.h
>> index fb60a88b5ed4..3fd8fd6d8fc2 100644
>> --- a/arch/arm64/include/asm/vdso/compat_barrier.h
>> +++ b/arch/arm64/include/asm/vdso/compat_barrier.h
>> @@ -20,7 +20,7 @@
>>
>> #define dmb(option) __asm__ __volatile__ ("dmb " #option : : : "memory")
>>
>> -#if __LINUX_ARM_ARCH__ >= 8
>> +#if __LINUX_ARM_ARCH__ >= 8 && defined(CONFIG_AS_DMB_ISHLD)
>> #define aarch32_smp_mb()dmb(ish)
>> #define aarch32_smp_rmb() dmb(ishld)
>> #define aarch32_smp_wmb() dmb(ishst)
>> diff --git a/arch/arm64/kernel/vdso32/Makefile
>> b/arch/arm64/kernel/vdso32/Makefile
>> index 1fba0776ed40..1a3299d901b1 100644
>> --- a/arch/arm64/kernel/vdso32/Makefile
>> +++ b/arch/arm64/kernel/vdso32/Makefile
>> @@ -55,6 +55,9 @@ endif
>> VDSO_CAFLAGS += -fPIC -fno-builtin -fno-stack-protector
>> VDSO_CAFLAGS += -DDISABLE_BRANCH_PROFILING
>>
>> +# Check for binutils support for dmb ishld
>> +dmbinstr := $(call as-instr,dmb ishld,-DCONFIG_AS_DMB_ISHLD=1)
>
> Is this check for the compat gas or the native one? Looking at
> scripts/Kbuild.include, as-instr uses CC but you'd need a COMPATCC here
> instead.
>
> I may have missed something but I don't think this fixes the issue
> Will's issue since he reported the problem with an old compat binutils.
>
Good catch I thought that it was sufficient to put it in the correct makefile,
but I missed that COMPATCC != CC. This selects the wrong binutils.
Need to add something in Kbuild for that.
--
Regards,
Vincenzo
Re: [PATCH 1/4] arm64: vdso32: Introduce COMPAT_CC_IS_GCC
On 9/26/19 11:56 AM, Vincenzo Frascino wrote: > On 9/26/19 9:06 AM, Catalin Marinas wrote: >> On Thu, Sep 26, 2019 at 07:03:50AM +0100, Vincenzo Frascino wrote: >>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig >>> index 37c610963eee..afe8c948b493 100644 >>> --- a/arch/arm64/Kconfig >>> +++ b/arch/arm64/Kconfig >>> @@ -110,7 +110,7 @@ config ARM64 >>> select GENERIC_STRNLEN_USER >>> select GENERIC_TIME_VSYSCALL >>> select GENERIC_GETTIMEOFDAY >>> - select GENERIC_COMPAT_VDSO if (!CPU_BIG_ENDIAN && COMPAT) >>> + select GENERIC_COMPAT_VDSO if (!CPU_BIG_ENDIAN && COMPAT && >>> COMPAT_CC_IS_GCC) >>> select HANDLE_DOMAIN_IRQ >>> select HARDIRQS_SW_RESEND >>> select HAVE_PCI >>> @@ -313,6 +313,9 @@ config KASAN_SHADOW_OFFSET >>> default 0xeff9 if ARM64_VA_BITS_36 && KASAN_SW_TAGS >>> default 0x >>> >>> +config COMPAT_CC_IS_GCC >>> + def_bool $(success,$(CROSS_COMPILE_COMPAT)gcc --version | head -n 1 | >>> grep -q arm) >> >> Nitpick: I prefer COMPATCC instead of COMPAT_CC for consistency with >> HOSTCC. >> > > Ok, will change this in v2. > >> Now, could we not generate a COMPATCC in the Makefile and use >> $(COMPATCC) here instead of $(CROSS_COMPILE_COMPAT)gcc? It really >> doesn't make sense to check that gcc is gcc. >> > > All right, COMPATCC is already in the makefile, I will use it in here. > >> A next step would be to check that COMPATCC can actually generate 32-bit >> objects. But it's not essential at this stage. >> > > We are already checking this making sure that arm is present in the triple > (grep > -q arm). > >>> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile >>> index 84a3d502c5a5..34f53eb11878 100644 >>> --- a/arch/arm64/Makefile >>> +++ b/arch/arm64/Makefile >>> @@ -54,19 +54,8 @@ $(warning Detected assembler with broken .inst; >>> disassembly will be unreliable) >>> endif >>> >>> ifeq ($(CONFIG_GENERIC_COMPAT_VDSO), y) >>> - CROSS_COMPILE_COMPAT ?= $(CONFIG_CROSS_COMPILE_COMPAT_VDSO:"%"=%) >>> - >>> - ifeq ($(CONFIG_CC_IS_CLANG), y) >>> -$(warning CROSS_COMPILE_COMPAT is clang, the compat vDSO will not be >>> built) >>> - else ifeq ($(strip $(CROSS_COMPILE_COMPAT)),) >>> -$(warning CROSS_COMPILE_COMPAT not defined or empty, the compat vDSO >>> will not be built) >>> - else ifeq ($(shell which $(CROSS_COMPILE_COMPAT)gcc 2> /dev/null),) >>> -$(error $(CROSS_COMPILE_COMPAT)gcc not found, check >>> CROSS_COMPILE_COMPAT) >>> - else >>> -export CROSS_COMPILE_COMPAT >>> -export CONFIG_COMPAT_VDSO := y >>> -compat_vdso := -DCONFIG_COMPAT_VDSO=1 >>> - endif >>> + export CONFIG_COMPAT_VDSO := y >>> + compat_vdso := -DCONFIG_COMPAT_VDSO=1 >>> endif >> >> Has CONFIG_CROSS_COMPILE_COMPAT_VDSO actually been removed from >> lib/vdso/Kconfig? (I haven't checked the subsequent patches). >> > Missed this, I have the patch ready for that. When this series will be merged, no more architectures will use the macro hence I will send a separate patch to remove it from the common code. -- Regards, Vincenzo
[PATCH v2] ASoC: samsung: i2s: Document clocks macros
From: Maciej Falkowski Document clocks macros with their description from 'Documentation/devicetree/bindings/sound/samsung-i2s.txt' Signed-off-by: Maciej Falkowski Signed-off-by: Marek Szyprowski --- v2: - Added proper commit message and description - Moved comments to the right side Best regards, Maciej Falkowski --- include/dt-bindings/sound/samsung-i2s.h | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/include/dt-bindings/sound/samsung-i2s.h b/include/dt-bindings/sound/samsung-i2s.h index 77545f14c379..250de0d6c734 100644 --- a/include/dt-bindings/sound/samsung-i2s.h +++ b/include/dt-bindings/sound/samsung-i2s.h @@ -2,8 +2,14 @@ #ifndef _DT_BINDINGS_SAMSUNG_I2S_H #define _DT_BINDINGS_SAMSUNG_I2S_H -#define CLK_I2S_CDCLK 0 -#define CLK_I2S_RCLK_SRC 1 -#define CLK_I2S_RCLK_PSR 2 +#define CLK_I2S_CDCLK 0 /* the CDCLK (CODECLKO) gate clock */ + +#define CLK_I2S_RCLK_SRC 1 /* the RCLKSRC mux clock (corresponding to + * RCLKSRC bit in IISMOD register) + */ + +#define CLK_I2S_RCLK_PSR 2 /* the RCLK prescaler divider clock + * (corresponding to the IISPSR register) + */ #endif /* _DT_BINDINGS_SAMSUNG_I2S_H */ -- 2.17.1
Re: [PATCH 3/4] arm64: vdso32: Fix compilation warning
On 9/26/19 9:32 AM, Catalin Marinas wrote:
> On Thu, Sep 26, 2019 at 07:03:52AM +0100, Vincenzo Frascino wrote:
>> diff --git a/arch/arm64/include/asm/memory.h
>> b/arch/arm64/include/asm/memory.h
>> index b61b50bf68b1..b1c8c43234c5 100644
>> --- a/arch/arm64/include/asm/memory.h
>> +++ b/arch/arm64/include/asm/memory.h
>> @@ -228,11 +228,16 @@ static inline unsigned long kaslr_offset(void)
>> #define __tag_get(addr) 0
>> #endif /* CONFIG_KASAN_SW_TAGS */
>>
>> +#ifdef __aarch64__
>> static inline const void *__tag_set(const void *addr, u8 tag)
>> {
>> u64 __addr = (u64)addr & ~__tag_shifted(0xff);
>> return (const void *)(__addr | __tag_shifted(tag));
>> }
>> +#else
>> +/* Unused in 32 bit mode */
>> +#define __tag_set(addr, tag) 0
>> +#endif
>
> I'm fine with this as a temporary workaround (or hack). But please add a
> better comment on what the 32-bit mode is about - the compat vDSO.
>
Ok, I will do in v2, to avoid confusion.
--
Regards,
Vincenzo
Re: [PATCH] async: Let kfree() out of the critical area of the lock
On Thu, Sep 26, 2019 at 03:58:36PM +0800, Yunfeng Ye wrote: > On 2019/9/25 23:20, Alexander Duyck wrote: > > On Wed, 2019-09-25 at 20:52 +0800, Yunfeng Ye wrote: > > It probably wouldn't hurt to update the patch description to mention that > > async_schedule_node_domain does the allocation outside of the lock, then > > takes the lock and does the list addition and entry_count increment inside > > the critical section so this is just updating the code to match that it > > seems. > > > > Otherwise the change itself looks safe to me, though I am not sure there > > is a performance gain to be had so this is mostly just a cosmetic patch. > > > The async_lock is big global lock, I think it's good to put kfree() outside > to keep the critical area as short as possible. Agreed, kfree is not always cheap. We had patches in btrfs moving kfree out of critical section(s) after causing softlockups due to increased lock contention.
Re: [PATCH] iio: imu: adis16480: clean up a condition
On Thu, 2019-09-26 at 11:10 +0300, Dan Carpenter wrote: > [External] > > The "t" variable is unsigned so it can't be less than zero. We really > are just trying to prevent divide by zero bugs so just checking against > zero is sufficient. > > Signed-off-by: Dan Carpenter > --- > drivers/iio/imu/adis16480.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c > index b99d73887c9f..e144e567675d 100644 > --- a/drivers/iio/imu/adis16480.c > +++ b/drivers/iio/imu/adis16480.c > @@ -318,7 +318,7 @@ static int adis16480_set_freq(struct iio_dev > *indio_dev, int val, int val2) > unsigned int t, reg; I would just change the type of "t" to "int". Especially, since "val" & "val2" are "int". Thanks for the catch :) Alex > > t = val * 1000 + val2 / 1000; > - if (t <= 0) > + if (t == 0) > return -EINVAL; > > /*
Re: [alsa-devel] [PATCH v5] dt-bindings: sound: Convert Samsung I2S controller to dt-schema
On 9/25/19 15:26, Marek Szyprowski wrote: > From: Maciej Falkowski > > Convert Samsung I2S controller to newer dt-schema format. > > Signed-off-by: Maciej Falkowski > Signed-off-by: Marek Szyprowski > --- Thanks Maciej, it looks good to me, I just think it might make sense to improve the comments a little while we are doing such a conversion. Please see my comments below. With those corrections made: Reviewed-by: Sylwester Nawrocki > .../devicetree/bindings/sound/samsung-i2s.txt | 84 --- > .../bindings/sound/samsung-i2s.yaml | 136 ++ > 2 files changed, 136 insertions(+), 84 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/sound/samsung-i2s.txt > create mode 100644 Documentation/devicetree/bindings/sound/samsung-i2s.yaml > +++ b/Documentation/devicetree/bindings/sound/samsung-i2s.yaml > @@ -0,0 +1,136 @@ > +# SPDX-License-Identifier: GPL-2.0 > +%YAML 1.2 > +--- > +$id: > https://protect2.fireeye.com/url?k=9b0307ba8b0d1f39.9b028cf5-9870da798974f201&u=http://devicetree.org/schemas/sound/samsung-i2s.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Samsung SoC I2S controller > + > +maintainers: > + - Krzysztof Kozlowski > + - Sylwester Nawrocki > + > +properties: > + compatible: > +description: | > + samsung,s3c6410-i2s: for 8/16/24bit stereo I2S. > + > + samsung,s5pv210-i2s: for 8/16/24bit multichannel(5.1) I2S with space before "(5.1)" ? > + secondary fifo, s/w reset control and internal mux for root clk src. s/fifo/FIFO ? s/clk src/clock source ? > + samsung,exynos5420-i2s: for 8/16/24bit multichannel(5.1) I2S for space before "(5.1)" ? > + playback, stereo channel capture, secondary fifo using internal s/fifo/FIFO > + or external dma, s/w reset control, internal mux for root clk src s/dma/DMA ? s/clk src/clock source ? > + and 7.1 channel TDM support for playback. TDM (Time division > multiplexing) > + is to allow transfer of multiple channel audio data on single data > line. > + > + samsung,exynos7-i2s: with all the available features of exynos5 i2s. s/exynos5 i2s/ exynos5 I2S ? > + exynos7 I2S has 7.1 channel TDM support for capture, secondary fifo s/fifo/FIFO ? > + with only external dma and more no.of root clk sampling frequencies. s/dma/DMA ? s/no.of/number of ? > + samsung,exynos7-i2s1: I2S1 on previous samsung platforms supports > + stereo channels. exynos7 i2s1 upgraded to 5.1 multichannel with s/i2s1/I2S1 ? It would be good to convert all i2s0, i2s1, i2s2 occurrences in comments/descriptions into either upper or lower case for consistency. > + clock-names: > +oneOf: > + - items: > + - const: iis > + - items: # for i2s0 > + - const: iis > + - const: i2s_opclk0 > + - const: i2s_opclk1 > + - items: # for i2s1 and i2s2 > + - const: iis > + - const: i2s_opclk0 > +description: | > + "iis" is the i2s bus clock and i2s_opclk0, i2s_opclk1 are sources > + of the root clk. i2s0 has internal mux to select the source > + of root clk and i2s1 and i2s2 doesn't have any such mux. > + clock-output-names: > +deprecated: true > +oneOf: > + - items: # for i2s0 > + - const: i2s_cdclk0 > + - items: # for i2s1> + - const: i2s_cdclk1 > + - items: # for i2s2 > + - const: i2s_cdclk2 > +description: Names of the CDCLK I2S output clocks. > + samsung,idma-addr: > +$ref: /schemas/types.yaml#/definitions/uint32 > +description: | > + Internal DMA register base address of the audio > + sub system(used in secondary sound source). s/sub system(used/subsystem (used ? -- Thanks, Sylwester
Re: [PATCH v9 01/15] leds: multicolor: Add sysfs interface definition
Hi! > diff --git a/Documentation/leds/leds-class-multicolor.rst > b/Documentation/leds/leds-class-multicolor.rst > new file mode 100644 > index ..87a1588d7619 > --- /dev/null > +++ b/Documentation/leds/leds-class-multicolor.rst > @@ -0,0 +1,96 @@ > + > +Multi Color LED handling under Linux > + > + > +Description > +=== > +The multi color class groups monochrome LEDs and allows controlling two > +aspects of the final combined color: hue and lightness. The former is > +controlled via _intensity files and the latter is controlled > +via brightness file. > + > +For more details on hue and lightness notions please refer to > +https://en.wikipedia.org/wiki/CIECAM02. > + > +Note that intensity files only cache the written value and the actual > +change of hardware state occurs upon writing brightness file. This > +allows for changing many factors of the perceived color in a virtually > +unnoticeable way for the human observer. So unlike previous versions, userspace will need to write 4 files instead of one in the common case. > +Directory Layout Example > + > +root:/sys/class/leds/rgb:grouped_leds# ls -lR colors/ > +-rw-rwxr-- 1 root root 4096 Jul 7 03:10 red_max_intensity > +--w--wx-w- 1 root root 4096 Jul 7 03:10 red_intensity > +-rw-rwxr-- 1 root root 4096 Jul 7 03:10 green_max_intensity > +--w--wx-w- 1 root root 4096 Jul 7 03:10 green_intensity > +-rw-rwxr-- 1 root root 4096 Jul 7 03:10 blue_max_intensity > +--w--wx-w- 1 root root 4096 Jul 7 03:10 blue_intensity Permissions are way off here. > +A user first writes the color LED brightness file with the brightness level > that > +is necessary to achieve a blueish violet output from the RGB LED group. > + > +echo 138 > /sys/class/leds/rgb:grouped_leds/red_intensity > +echo 43 > /sys/class/leds/rgb:grouped_leds/green_intensity > +echo 226 > /sys/class/leds/rgb:grouped_leds/blue_intensity No, you can't tell what kind of color this will result in. Will you be on ELCE/OSS in Lyon? Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: [PATCH 2/2] KVM: x86: Expose CLZERO and XSAVEERPTR to the guest
On 25/09/19 23:37, Sebastian Andrzej Siewior wrote: > I was surprised to see that the guest reported `fxsave_leak' while the > host did not. After digging deeper I noticed that the bits are simply > masked out during enumeration. > The XSAVEERPTR feature is actually a bug fix on AMD which means the > kernel can disable a workaround. > While here, I've seen that CLZERO is also masked out. This opcode is > unprivilged so exposing it to the guest should not make any difference. > > Pass CLZERO and XSAVEERPTR to the guest if available on the host. > > Signed-off-by: Sebastian Andrzej Siewior > --- > arch/x86/kvm/cpuid.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c > index 22c2720cd948e..0ae9194d0f4d2 100644 > --- a/arch/x86/kvm/cpuid.c > +++ b/arch/x86/kvm/cpuid.c > @@ -473,6 +473,7 @@ static inline int __do_cpuid_func(struct kvm_cpuid_entry2 > *entry, u32 function, > > /* cpuid 0x8008.ebx */ > const u32 kvm_cpuid_8000_0008_ebx_x86_features = > + F(CLZERO) | F(XSAVEERPTR) | > F(WBNOINVD) | F(AMD_IBPB) | F(AMD_IBRS) | F(AMD_SSBD) | > F(VIRT_SSBD) | > F(AMD_SSB_NO) | F(AMD_STIBP) | F(AMD_STIBP_ALWAYS_ON); > > Applied (on top of Jim's CLZERO patch, so removing the CLZERO reference in the commit message). Paolo
[PATCH] NFC: st95hf: clean up indentation issue
From: Colin Ian King The return statement is indented incorrectly, add in a missing tab and remove an extraneous space after the return Signed-off-by: Colin Ian King --- drivers/nfc/st95hf/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/nfc/st95hf/core.c b/drivers/nfc/st95hf/core.c index ce38782ebf80..291efff048af 100644 --- a/drivers/nfc/st95hf/core.c +++ b/drivers/nfc/st95hf/core.c @@ -661,7 +661,7 @@ static int st95hf_error_handling(struct st95hf_context *stcontext, result = -ETIMEDOUT; else result = -EIO; - return result; + return result; } /* Check for CRC err only if CRC is present in the tag response */ -- 2.20.1
Re: [PATCH 57/66] perf evsel: Introduce evsel_fprintf.h
On Wed, Sep 25, 2019 at 09:32:35PM -0300, Arnaldo Carvalho de Melo wrote: > From: Arnaldo Carvalho de Melo > > We already had evsel_fprintf.c, add its counterpart, so that we can > reduce evsel.h a bit more. > > We needed a new perf_event_attr_fprintf.c file so as to have a separate > object to link with the python binding in tools/perf/util/python-ext-sources > and not drag symbol_conf, etc into the python binding. > > Cc: Adrian Hunter > Cc: Jiri Olsa > Cc: Namhyung Kim > Link: https://lkml.kernel.org/n/tip-06bdmt1062d9unzgqmxwl...@git.kernel.org > Signed-off-by: Arnaldo Carvalho de Melo > --- > tools/perf/builtin-evlist.c | 1 + > tools/perf/builtin-sched.c| 1 + > tools/perf/builtin-script.c | 1 + > tools/perf/builtin-trace.c| 2 + > tools/perf/util/Build | 1 + > tools/perf/util/evsel.c | 153 +- > tools/perf/util/evsel.h | 51 +--- > tools/perf/util/evsel_fprintf.c | 1 + > tools/perf/util/evsel_fprintf.h | 50 +++ > tools/perf/util/header.c | 1 + > tools/perf/util/perf_event_attr_fprintf.c | 148 + > tools/perf/util/python-ext-sources| 1 + > 12 files changed, 218 insertions(+), 193 deletions(-) > create mode 100644 tools/perf/util/evsel_fprintf.h > create mode 100644 tools/perf/util/perf_event_attr_fprintf.c hum, I see this file in this patch, but not when I checkout yours perf/core jirka
Re: [RFC PATCH] xen/gntdev: Stop abusing DT of_dma_configure API
On 9/26/19 1:46 PM, Robin Murphy wrote: > On 2019-09-26 11:17 am, Oleksandr Andrushchenko wrote: >> >> On 9/26/19 12:49 PM, Julien Grall wrote: >>> Hi Rob, >>> >>> >>> On 9/25/19 10:50 PM, Rob Herring wrote: As the comment says, this isn't a DT based device. of_dma_configure() is going to stop allowing a NULL DT node, so this needs to be fixed. >>> >>> And this can't work on arch not selecting CONFIG_OF and can select >>> CONFIG_XEN_GRANT_DMA_ALLOC. >>> >>> We are lucky enough on x86 because, AFAICT, arch_setup_dma_ops is just >>> a nop. >>> >> No luck is needed as [1] does nothing for those platforms not using >> CONFIG_OF Not sure exactly what setup besides arch_setup_dma_ops is needed... >>> >>> We probably want to update dma_mask, coherent_dma_mask and >>> dma_pfn_offset. >>> >>> Also, while look at of_configure_dma, I noticed that we consider the >>> DMA will not be coherent for the grant-table. Oleksandr, do you know >>> why they can't be coherent? >> The main and the only reason to use of_configure_dma is that if we don't >> then we >> are about to stay with dma_dummy_ops [2]. It effectively means that >> operations on dma-bufs >> will end up returning errors, like [3], [4], thus not making it possible >> for Xen PV DRM and DMA >> part of gntdev driver to do what we need (dma-bufs in our use-cases >> allow zero-copying >> while using graphics buffers and many more). >> >> I didn't find any better way of achieving that, but of_configure_dma... >> If there is any better solution which will not break the existing >> functionality then >> I will definitely change the drivers so we do not abuse DT ) >> Before that, please keep in mind that merging this RFC will break Xen PV >> DRM + >> DMA buf support in gntdev... >> Hope we can work out some acceptable solution, so everyone is happy > > As I mentioned elsewhere, the recent dma-direct rework means that > dma_dummy_ops are now only explicitly installed for the ACPI error > case, so - much as I may dislike it - you should get regular > (direct/SWIOTLB) ops by default again. Ah, my bad, I missed that change. So, if no dummy dma ops are to be used then I believe we can apply both changes, e.g. remove of_dma_configure from both of the drivers. > > Coherency is trickier - if the guest is allocating buffers for the PV > device, which may be shared directly with hardware by the host driver, > then the coherency of the PV device should really reflect that of the > underlying hardware to avoid potential problems. There are some cases > where the stage 2 attributes alone wouldn't be enough to correct a > mismatch. > > Robin. Thank you, Oleksandr
[PATCH 01/10] iio: imu: adis: rename txrx_lock -> state_lock
The lock can be extended a bit to protect other elements that are not
particular to just TX/RX. Another idea would have been to just add a new
`state_lock`, but that would mean 2 locks which would be redundant, and
probably cause more potential for dead-locks.
What will be done in the next patches, will be to add some unlocked
versions for read/write_reg functions.
Signed-off-by: Alexandru Ardelean
---
drivers/iio/imu/adis.c| 10 +-
drivers/iio/imu/adis_buffer.c | 4 ++--
include/linux/iio/imu/adis.h | 2 +-
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
index 1631c255deab..3c2d896e3a96 100644
--- a/drivers/iio/imu/adis.c
+++ b/drivers/iio/imu/adis.c
@@ -70,7 +70,7 @@ int adis_write_reg(struct adis *adis, unsigned int reg,
},
};
- mutex_lock(&adis->txrx_lock);
+ mutex_lock(&adis->state_lock);
spi_message_init(&msg);
@@ -114,7 +114,7 @@ int adis_write_reg(struct adis *adis, unsigned int reg,
}
out_unlock:
- mutex_unlock(&adis->txrx_lock);
+ mutex_unlock(&adis->state_lock);
return ret;
}
@@ -166,7 +166,7 @@ int adis_read_reg(struct adis *adis, unsigned int reg,
},
};
- mutex_lock(&adis->txrx_lock);
+ mutex_lock(&adis->state_lock);
spi_message_init(&msg);
if (adis->current_page != page) {
@@ -211,7 +211,7 @@ int adis_read_reg(struct adis *adis, unsigned int reg,
}
out_unlock:
- mutex_unlock(&adis->txrx_lock);
+ mutex_unlock(&adis->state_lock);
return ret;
}
@@ -437,7 +437,7 @@ EXPORT_SYMBOL_GPL(adis_single_conversion);
int adis_init(struct adis *adis, struct iio_dev *indio_dev,
struct spi_device *spi, const struct adis_data *data)
{
- mutex_init(&adis->txrx_lock);
+ mutex_init(&adis->state_lock);
adis->spi = spi;
adis->data = data;
iio_device_set_drvdata(indio_dev, adis);
diff --git a/drivers/iio/imu/adis_buffer.c b/drivers/iio/imu/adis_buffer.c
index 9ac8356d9a95..bf581a2c321d 100644
--- a/drivers/iio/imu/adis_buffer.c
+++ b/drivers/iio/imu/adis_buffer.c
@@ -123,7 +123,7 @@ static irqreturn_t adis_trigger_handler(int irq, void *p)
return -ENOMEM;
if (adis->data->has_paging) {
- mutex_lock(&adis->txrx_lock);
+ mutex_lock(&adis->state_lock);
if (adis->current_page != 0) {
adis->tx[0] = ADIS_WRITE_REG(ADIS_REG_PAGE_ID);
adis->tx[1] = 0;
@@ -138,7 +138,7 @@ static irqreturn_t adis_trigger_handler(int irq, void *p)
if (adis->data->has_paging) {
adis->current_page = 0;
- mutex_unlock(&adis->txrx_lock);
+ mutex_unlock(&adis->state_lock);
}
iio_push_to_buffers_with_timestamp(indio_dev, adis->buffer,
diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
index 4c53815bb729..3ed5eceaac2d 100644
--- a/include/linux/iio/imu/adis.h
+++ b/include/linux/iio/imu/adis.h
@@ -61,7 +61,7 @@ struct adis {
const struct adis_data *data;
struct adis_burst *burst;
- struct mutextxrx_lock;
+ struct mutexstate_lock;
struct spi_message msg;
struct spi_transfer *xfer;
unsigned intcurrent_page;
--
2.20.1
[PATCH 02/10] iio: imu: adis: add unlocked read/write function versions
This will allow more flexible control to group reads & writes into a single
lock (particularly the state_lock).
The end-goal is to remove the indio_dev->mlock usage, and the simplest fix
would have been to just add another lock, which would not be a good idea on
the long-run.
Signed-off-by: Alexandru Ardelean
---
drivers/iio/imu/adis.c | 34 +--
include/linux/iio/imu/adis.h | 114 ++-
2 files changed, 128 insertions(+), 20 deletions(-)
diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
index 3c2d896e3a96..4f3be011c898 100644
--- a/drivers/iio/imu/adis.c
+++ b/drivers/iio/imu/adis.c
@@ -26,7 +26,14 @@
#define ADIS_MSC_CTRL_DATA_RDY_DIO2BIT(0)
#define ADIS_GLOB_CMD_SW_RESET BIT(7)
-int adis_write_reg(struct adis *adis, unsigned int reg,
+/**
+ * __adis_write_reg() - write N bytes to register (unlocked version)
+ * @adis: The adis device
+ * @reg: The address of the lower of the two registers
+ * @value: The value to write to device (up to 4 bytes)
+ * @size: The size of the @value (in bytes)
+ */
+int __adis_write_reg(struct adis *adis, unsigned int reg,
unsigned int value, unsigned int size)
{
unsigned int page = reg / ADIS_PAGE_SIZE;
@@ -70,8 +77,6 @@ int adis_write_reg(struct adis *adis, unsigned int reg,
},
};
- mutex_lock(&adis->state_lock);
-
spi_message_init(&msg);
if (adis->current_page != page) {
@@ -96,8 +101,7 @@ int adis_write_reg(struct adis *adis, unsigned int reg,
adis->tx[3] = value & 0xff;
break;
default:
- ret = -EINVAL;
- goto out_unlock;
+ return -EINVAL;
}
xfers[size].cs_change = 0;
@@ -113,20 +117,18 @@ int adis_write_reg(struct adis *adis, unsigned int reg,
adis->current_page = page;
}
-out_unlock:
- mutex_unlock(&adis->state_lock);
-
return ret;
}
-EXPORT_SYMBOL_GPL(adis_write_reg);
+EXPORT_SYMBOL_GPL(__adis_write_reg);
/**
- * adis_read_reg() - read 2 bytes from a 16-bit register
+ * __adis_read_reg() - read N bytes from register (unlocked version)
* @adis: The adis device
* @reg: The address of the lower of the two registers
* @val: The value read back from the device
+ * @size: The size of the @val buffer
*/
-int adis_read_reg(struct adis *adis, unsigned int reg,
+int __adis_read_reg(struct adis *adis, unsigned int reg,
unsigned int *val, unsigned int size)
{
unsigned int page = reg / ADIS_PAGE_SIZE;
@@ -188,15 +190,14 @@ int adis_read_reg(struct adis *adis, unsigned int reg,
spi_message_add_tail(&xfers[3], &msg);
break;
default:
- ret = -EINVAL;
- goto out_unlock;
+ return -EINVAL;
}
ret = spi_sync(adis->spi, &msg);
if (ret) {
dev_err(&adis->spi->dev, "Failed to read register 0x%02X: %d\n",
reg, ret);
- goto out_unlock;
+ return ret;
} else {
adis->current_page = page;
}
@@ -210,12 +211,9 @@ int adis_read_reg(struct adis *adis, unsigned int reg,
break;
}
-out_unlock:
- mutex_unlock(&adis->state_lock);
-
return ret;
}
-EXPORT_SYMBOL_GPL(adis_read_reg);
+EXPORT_SYMBOL_GPL(__adis_read_reg);
#ifdef CONFIG_DEBUG_FS
diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
index 3ed5eceaac2d..3a028c40e04e 100644
--- a/include/linux/iio/imu/adis.h
+++ b/include/linux/iio/imu/adis.h
@@ -75,11 +75,121 @@ int adis_init(struct adis *adis, struct iio_dev *indio_dev,
struct spi_device *spi, const struct adis_data *data);
int adis_reset(struct adis *adis);
-int adis_write_reg(struct adis *adis, unsigned int reg,
+int __adis_write_reg(struct adis *adis, unsigned int reg,
unsigned int val, unsigned int size);
-int adis_read_reg(struct adis *adis, unsigned int reg,
+int __adis_read_reg(struct adis *adis, unsigned int reg,
unsigned int *val, unsigned int size);
+/**
+ * __adis_write_reg_8() - Write single byte to a register (unlocked version)
+ * @adis: The adis device
+ * @reg: The address of the register to be written
+ * @value: The value to write
+ */
+static inline int __adis_write_reg_8(struct adis *adis, unsigned int reg,
+ uint8_t val)
+{
+ return __adis_write_reg(adis, reg, val, 1);
+}
+
+/**
+ * __adis_write_reg_16() - Write 2 bytes to a pair of registers (unlocked
version)
+ * @adis: The adis device
+ * @reg: The address of the lower of the two registers
+ * @value: Value to be written
+ */
+static inline int __adis_write_reg_16(struct adis *adis, unsigned int reg,
+ uint16_t val)
+{
+ return __adis_write_reg(adis, reg, val, 2);
+}
+
+/**
+ * __adis_write_reg_32() - write 4 bytes to four registers (unlocked version)
+ * @adis: The adis device
+ * @reg: Th
[PATCH 04/10] iio: imu: adis: create an unlocked version of adis_check_status()
This one also gets re-used in certain operations, so it makes sense to
have an unlocked version of this to group it with other
reads/writes/operations to have a single lock for the whole state change.
Signed-off-by: Alexandru Ardelean
---
drivers/iio/imu/adis.c | 8
include/linux/iio/imu/adis.h | 13 -
2 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
index dc30f70d36f3..e461b9ae22a5 100644
--- a/drivers/iio/imu/adis.c
+++ b/drivers/iio/imu/adis.c
@@ -277,18 +277,18 @@ int adis_enable_irq(struct adis *adis, bool enable)
EXPORT_SYMBOL(adis_enable_irq);
/**
- * adis_check_status() - Check the device for error conditions
+ * __adis_check_status() - Check the device for error conditions (unlocked)
* @adis: The adis device
*
* Returns 0 on success, a negative error code otherwise
*/
-int adis_check_status(struct adis *adis)
+int __adis_check_status(struct adis *adis)
{
uint16_t status;
int ret;
int i;
- ret = adis_read_reg_16(adis, adis->data->diag_stat_reg, &status);
+ ret = __adis_read_reg_16(adis, adis->data->diag_stat_reg, &status);
if (ret < 0)
return ret;
@@ -306,7 +306,7 @@ int adis_check_status(struct adis *adis)
return -EIO;
}
-EXPORT_SYMBOL_GPL(adis_check_status);
+EXPORT_SYMBOL_GPL(__adis_check_status);
/**
* adis_reset() - Reset the device
diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
index 3a028c40e04e..f4ffba0c36b1 100644
--- a/include/linux/iio/imu/adis.h
+++ b/include/linux/iio/imu/adis.h
@@ -263,7 +263,18 @@ static inline int adis_read_reg_32(struct adis *adis,
unsigned int reg,
}
int adis_enable_irq(struct adis *adis, bool enable);
-int adis_check_status(struct adis *adis);
+int __adis_check_status(struct adis *adis);
+
+static inline int adis_check_status(struct adis *adis)
+{
+ int ret;
+
+ mutex_lock(&adis->state_lock);
+ ret = __adis_check_status(adis);
+ mutex_unlock(&adis->state_lock);
+
+ return ret;
+}
int adis_initial_startup(struct adis *adis);
--
2.20.1
[PATCH 03/10] iio: imu: adis[16480]: group RW into a single lock in adis_enable_irq()
The adis_enable_irq() does a read & a write. This change keeps a lock for
the duration of both operations vs for each op.
The change is also needed in adis16480, since that has it's own
implementation for adis_enable_irq().
Signed-off-by: Alexandru Ardelean
---
drivers/iio/imu/adis.c | 17 +++--
drivers/iio/imu/adis16480.c | 4 ++--
2 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
index 4f3be011c898..dc30f70d36f3 100644
--- a/drivers/iio/imu/adis.c
+++ b/drivers/iio/imu/adis.c
@@ -250,12 +250,16 @@ int adis_enable_irq(struct adis *adis, bool enable)
int ret = 0;
uint16_t msc;
- if (adis->data->enable_irq)
- return adis->data->enable_irq(adis, enable);
+ mutex_lock(&adis->state_lock);
+
+ if (adis->data->enable_irq) {
+ ret = adis->data->enable_irq(adis, enable);
+ goto out_unlock;
+ }
- ret = adis_read_reg_16(adis, adis->data->msc_ctrl_reg, &msc);
+ ret = __adis_read_reg_16(adis, adis->data->msc_ctrl_reg, &msc);
if (ret)
- goto error_ret;
+ goto out_unlock;
msc |= ADIS_MSC_CTRL_DATA_RDY_POL_HIGH;
msc &= ~ADIS_MSC_CTRL_DATA_RDY_DIO2;
@@ -264,9 +268,10 @@ int adis_enable_irq(struct adis *adis, bool enable)
else
msc &= ~ADIS_MSC_CTRL_DATA_RDY_EN;
- ret = adis_write_reg_16(adis, adis->data->msc_ctrl_reg, msc);
+ ret = __adis_write_reg_16(adis, adis->data->msc_ctrl_reg, msc);
-error_ret:
+out_unlock:
+ mutex_unlock(&adis->state_lock);
return ret;
}
EXPORT_SYMBOL(adis_enable_irq);
diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c
index b99d73887c9f..dc13d8a33612 100644
--- a/drivers/iio/imu/adis16480.c
+++ b/drivers/iio/imu/adis16480.c
@@ -936,14 +936,14 @@ static int adis16480_enable_irq(struct adis *adis, bool
enable)
uint16_t val;
int ret;
- ret = adis_read_reg_16(adis, ADIS16480_REG_FNCTIO_CTRL, &val);
+ ret = __adis_read_reg_16(adis, ADIS16480_REG_FNCTIO_CTRL, &val);
if (ret < 0)
return ret;
val &= ~ADIS16480_DRDY_EN_MSK;
val |= ADIS16480_DRDY_EN(enable);
- return adis_write_reg_16(adis, ADIS16480_REG_FNCTIO_CTRL, val);
+ return __adis_write_reg_16(adis, ADIS16480_REG_FNCTIO_CTRL, val);
}
static int adis16480_initial_setup(struct iio_dev *indio_dev)
--
2.20.1
[PATCH 00/10] iio: imu: adis: cleanup lock usage
There is a general effort for cleaning up `indio_dev->mlock` usage in IIO drivers. As part of that some ADIS drivers also need to do that. This changeset does a little more, by reworking the `txrx_lock` from the ADIS library to be used as a general `state_lock` to protect state between consecutive read/write ops. As such, all users of the ADIS lib have been verified to have their usage of mlock checked. Some just needed the mlock usage removed (as done in other patches). Alexandru Ardelean (10): iio: imu: adis: rename txrx_lock -> state_lock iio: imu: adis: add unlocked read/write function versions iio: imu: adis[16480]: group RW into a single lock in adis_enable_irq() iio: imu: adis: create an unlocked version of adis_check_status() iio: imu: adis: create an unlocked version of adis_reset() iio: imu: adis: protect initial startup routine with state lock iio: imu: adis: group single conversion under a single state lock iio: imu: adis16400: rework locks using ADIS library's state lock iio: gyro: adis16136: rework locks using ADIS library's state lock iio: imu: adis16480: use state lock for filter freq set drivers/iio/gyro/adis16136.c | 31 --- drivers/iio/imu/adis.c| 95 -- drivers/iio/imu/adis16400.c | 51 ++-- drivers/iio/imu/adis16480.c | 17 ++-- drivers/iio/imu/adis_buffer.c | 4 +- include/linux/iio/imu/adis.h | 148 -- 6 files changed, 254 insertions(+), 92 deletions(-) -- 2.20.1
[PATCH v3 2/3] dt-bindings: dma: ti-edma: Document dma-channel-mask for EDMA
Similarly to paRAM slots, channels can be used by other cores.
The common dma-channel-mask property can be used for specifying the
available channels.
Signed-off-by: Peter Ujfalusi
---
Documentation/devicetree/bindings/dma/ti-edma.txt | 8
1 file changed, 8 insertions(+)
diff --git a/Documentation/devicetree/bindings/dma/ti-edma.txt
b/Documentation/devicetree/bindings/dma/ti-edma.txt
index 4bbc94d829c8..014187088020 100644
--- a/Documentation/devicetree/bindings/dma/ti-edma.txt
+++ b/Documentation/devicetree/bindings/dma/ti-edma.txt
@@ -42,6 +42,11 @@ Optional properties:
- ti,edma-reserved-slot-ranges: PaRAM slot ranges which should not be used by
the driver, they are allocated to be used by for example the
DSP. See example.
+- dma-channel-mask: Mask of usable channels.
+ Single uint32 for EDMA with 32 channels, array of two uint32 for
+ EDMA with 64 channels. See example and
+ Documentation/devicetree/bindings/dma/dma-common.yaml
+
--
eDMA3 Transfer Controller
@@ -91,6 +96,9 @@ edma: edma@4900 {
ti,edma-memcpy-channels = <20 21>;
/* The following PaRAM slots are reserved: 35-44 and 100-109 */
ti,edma-reserved-slot-ranges = <35 10>, <100 10>;
+ /* The following channels are reserved: 35-44 */
+ dma-channel-mask = <0x>, /* Channel 0-31 */
+ <0xe007>; /* Channel 32-63 */
};
edma_tptc0: tptc@4980 {
--
Peter
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
[PATCH 10/10] iio: imu: adis16480: use state lock for filter freq set
It's the only operation that does 2 operations (a read & a write), so the
unlocked functions can be used under a single state lock.
Signed-off-by: Alexandru Ardelean
---
drivers/iio/imu/adis16480.c | 13 ++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c
index dc13d8a33612..01dae50e985b 100644
--- a/drivers/iio/imu/adis16480.c
+++ b/drivers/iio/imu/adis16480.c
@@ -550,6 +550,7 @@ static int adis16480_set_filter_freq(struct iio_dev
*indio_dev,
const struct iio_chan_spec *chan, unsigned int freq)
{
struct adis16480 *st = iio_priv(indio_dev);
+ struct mutex *slock = &st->adis.state_lock;
unsigned int enable_mask, offset, reg;
unsigned int diff, best_diff;
unsigned int i, best_freq;
@@ -560,9 +561,11 @@ static int adis16480_set_filter_freq(struct iio_dev
*indio_dev,
offset = ad16480_filter_data[chan->scan_index][1];
enable_mask = BIT(offset + 2);
- ret = adis_read_reg_16(&st->adis, reg, &val);
+ mutex_lock(slock);
+
+ ret = __adis_read_reg_16(&st->adis, reg, &val);
if (ret < 0)
- return ret;
+ goto out_unlock;
if (freq == 0) {
val &= ~enable_mask;
@@ -584,7 +587,11 @@ static int adis16480_set_filter_freq(struct iio_dev
*indio_dev,
val |= enable_mask;
}
- return adis_write_reg_16(&st->adis, reg, val);
+ ret = __adis_write_reg_16(&st->adis, reg, val);
+out_unlock:
+ mutex_unlock(slock);
+
+ return ret;
}
static int adis16480_read_raw(struct iio_dev *indio_dev,
--
2.20.1
