Re: [for-next][PATCH 3/6] tracing: More reverting of "tracing: Centralize preemptirq tracepoints and unify their usage"
On Thu, Aug 9, 2018 at 2:03 PM, Steven Rostedt wrote:
> From: "Steven Rostedt (VMware)"
>
> Joel Fernandes created a nice patch that cleaned up the duplicate hooks used
> by lockdep and irqsoff latency tracer. It made both use tracepoints. But the
> latency tracer is triggering warnings when using tracepoints to call into
> the latency tracer's routines. Mainly, they can be called from NMI context.
> If that happens, then the SRCU may not work properly because on some
> architectures, SRCU is not safe to be called in both NMI and non-NMI
> context.
>
> This is a partial revert of the clean up patch c3bc8fd637a9 ("tracing:
> Centralize preemptirq tracepoints and unify their usage") that adds back the
> direct calls into the latency tracer. It also only calls the trace events
> when not in NMI.
>
> Cc: Joel Fernandes
> Fixes: c3bc8fd637a9 ("tracing: Centralize preemptirq tracepoints and unify
> their usage")
> Signed-off-by: Steven Rostedt (VMware)
Reviewed-by: Joel Fernandes (Google)
thanks,
- Joel
Re: [for-next][PATCH 3/6] tracing: More reverting of "tracing: Centralize preemptirq tracepoints and unify their usage"
On Thu, Aug 9, 2018 at 2:03 PM, Steven Rostedt wrote:
> From: "Steven Rostedt (VMware)"
>
> Joel Fernandes created a nice patch that cleaned up the duplicate hooks used
> by lockdep and irqsoff latency tracer. It made both use tracepoints. But the
> latency tracer is triggering warnings when using tracepoints to call into
> the latency tracer's routines. Mainly, they can be called from NMI context.
> If that happens, then the SRCU may not work properly because on some
> architectures, SRCU is not safe to be called in both NMI and non-NMI
> context.
>
> This is a partial revert of the clean up patch c3bc8fd637a9 ("tracing:
> Centralize preemptirq tracepoints and unify their usage") that adds back the
> direct calls into the latency tracer. It also only calls the trace events
> when not in NMI.
>
> Cc: Joel Fernandes
> Fixes: c3bc8fd637a9 ("tracing: Centralize preemptirq tracepoints and unify
> their usage")
> Signed-off-by: Steven Rostedt (VMware)
Reviewed-by: Joel Fernandes (Google)
thanks,
- Joel
Re: [for-next][PATCH 1/6] tracing: Partial revert of "tracing: Centralize preemptirq tracepoints and unify their usage"
On Thu, Aug 9, 2018 at 2:03 PM, Steven Rostedt wrote:
> From: "Steven Rostedt (VMware)"
>
> Joel Fernandes created a nice patch that cleaned up the duplicate hooks used
> by lockdep and irqsoff latency tracer. It made both use tracepoints. But it
> caused lockdep to trigger several false positives. We have not figured out
> why yet, but removing lockdep from using the trace event hooks and just call
> its helper functions directly (like it use to), makes the problem go away.
>
> This is a partial revert of the clean up patch c3bc8fd637a9 ("tracing:
> Centralize preemptirq tracepoints and unify their usage") that adds direct
> calls for lockdep, but also keeps most of the clean up done to get rid of
> the horrible preprocessor if statements.
>
> Link: http://lkml.kernel.org/r/20180806155058.5ee87...@gandalf.local.home
>
> Cc: Joel Fernandes
> Cc: Peter Zijlstra
> Fixes: c3bc8fd637a9 ("tracing: Centralize preemptirq tracepoints and unify
> their usage")
Reviewed-by: Joel Fernandes (Google)
thanks,
- Joel
> Signed-off-by: Steven Rostedt (VMware)
> ---
> include/linux/irqflags.h| 8 ++--
> include/linux/lockdep.h | 2 --
> init/main.c | 2 --
> kernel/locking/lockdep.c| 14 ++---
> kernel/trace/trace_preemptirq.c | 36 ++---
> 5 files changed, 28 insertions(+), 34 deletions(-)
>
> diff --git a/include/linux/irqflags.h b/include/linux/irqflags.h
Re: [for-next][PATCH 1/6] tracing: Partial revert of "tracing: Centralize preemptirq tracepoints and unify their usage"
On Thu, Aug 9, 2018 at 2:03 PM, Steven Rostedt wrote:
> From: "Steven Rostedt (VMware)"
>
> Joel Fernandes created a nice patch that cleaned up the duplicate hooks used
> by lockdep and irqsoff latency tracer. It made both use tracepoints. But it
> caused lockdep to trigger several false positives. We have not figured out
> why yet, but removing lockdep from using the trace event hooks and just call
> its helper functions directly (like it use to), makes the problem go away.
>
> This is a partial revert of the clean up patch c3bc8fd637a9 ("tracing:
> Centralize preemptirq tracepoints and unify their usage") that adds direct
> calls for lockdep, but also keeps most of the clean up done to get rid of
> the horrible preprocessor if statements.
>
> Link: http://lkml.kernel.org/r/20180806155058.5ee87...@gandalf.local.home
>
> Cc: Joel Fernandes
> Cc: Peter Zijlstra
> Fixes: c3bc8fd637a9 ("tracing: Centralize preemptirq tracepoints and unify
> their usage")
Reviewed-by: Joel Fernandes (Google)
thanks,
- Joel
> Signed-off-by: Steven Rostedt (VMware)
> ---
> include/linux/irqflags.h| 8 ++--
> include/linux/lockdep.h | 2 --
> init/main.c | 2 --
> kernel/locking/lockdep.c| 14 ++---
> kernel/trace/trace_preemptirq.c | 36 ++---
> 5 files changed, 28 insertions(+), 34 deletions(-)
>
> diff --git a/include/linux/irqflags.h b/include/linux/irqflags.h
Re: [PATCH v6 6/6] signal: Don't restart fork when signals come in.
writes:
> ebied...@xmission.com writes
>> Subject: [PATCH v6 6/6] signal: Don't restart fork when signals come in.
>>
>> Wen Yang and majiang
>> report that a periodic signal received during fork can cause fork to
>> continually restart preventing an application from making progress.
>>
>> ...
>> diff --git a/kernel/signal.c b/kernel/signal.c
>> index 9f0eafb6d474..cfa9d10e731a 100644
>> --- a/kernel/signal.c
>> +++ b/kernel/signal.c
>> @@ -1121,6 +1121,21 @@ static int __send_signal(int sig, struct siginfo
>> *info, struct task_struct *t,
>> out_set:
>> signalfd_notify(t, sig);
>> sigaddset(>signal, sig);
>> +
>> + /* Let multiprocess signals appear after on-going forks */
>> + if (type > PIDTYPE_TGID) {
>> + struct multiprocess_signals *delayed;
>> + hlist_for_each_entry(delayed, >signal->multiprocess, node) {
>> + sigset_t *signal = >signal;
>> + /* Can't queue both a stop and a continue signal */
>> + if (sig == SIGCONT)
>> + sigdelsetmask(signal, SIG_KERNEL_STOP_MASK);
>> + else if (sig_kernel_stop(sig))
>> + sigdelset(signal, SIGCONT);
>> + sigaddset(signal, sig);
>> + }
>> + }
>> +
>> complete_signal(sig, t, type);
>> ret:
>> trace_signal_generate(sig, info, t, type != PIDTYPE_PID, result);
>
> I've migrated this patch and try to test it, but I ran into a compile error:
>
> kernel/signal.c: In function '__send_signal':
> kernel/signal.c:1192:9: error: 'type' undeclared (first use in this function)
> if (type > PIDTYPE_TGID) {
> ^
> We may also need some patches, which pass pid type into
> __send_signal.
We most definitely need the the first 15 patches from my last series of
patches that push the pid type down.
I did not repeat them here because there is consensus that they
are fine working fine.
I have a branch with everything at:
https://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace.git
siginfo-testing
Eric
Re: [PATCH v6 6/6] signal: Don't restart fork when signals come in.
writes:
> ebied...@xmission.com writes
>> Subject: [PATCH v6 6/6] signal: Don't restart fork when signals come in.
>>
>> Wen Yang and majiang
>> report that a periodic signal received during fork can cause fork to
>> continually restart preventing an application from making progress.
>>
>> ...
>> diff --git a/kernel/signal.c b/kernel/signal.c
>> index 9f0eafb6d474..cfa9d10e731a 100644
>> --- a/kernel/signal.c
>> +++ b/kernel/signal.c
>> @@ -1121,6 +1121,21 @@ static int __send_signal(int sig, struct siginfo
>> *info, struct task_struct *t,
>> out_set:
>> signalfd_notify(t, sig);
>> sigaddset(>signal, sig);
>> +
>> + /* Let multiprocess signals appear after on-going forks */
>> + if (type > PIDTYPE_TGID) {
>> + struct multiprocess_signals *delayed;
>> + hlist_for_each_entry(delayed, >signal->multiprocess, node) {
>> + sigset_t *signal = >signal;
>> + /* Can't queue both a stop and a continue signal */
>> + if (sig == SIGCONT)
>> + sigdelsetmask(signal, SIG_KERNEL_STOP_MASK);
>> + else if (sig_kernel_stop(sig))
>> + sigdelset(signal, SIGCONT);
>> + sigaddset(signal, sig);
>> + }
>> + }
>> +
>> complete_signal(sig, t, type);
>> ret:
>> trace_signal_generate(sig, info, t, type != PIDTYPE_PID, result);
>
> I've migrated this patch and try to test it, but I ran into a compile error:
>
> kernel/signal.c: In function '__send_signal':
> kernel/signal.c:1192:9: error: 'type' undeclared (first use in this function)
> if (type > PIDTYPE_TGID) {
> ^
> We may also need some patches, which pass pid type into
> __send_signal.
We most definitely need the the first 15 patches from my last series of
patches that push the pid type down.
I did not repeat them here because there is consensus that they
are fine working fine.
I have a branch with everything at:
https://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace.git
siginfo-testing
Eric
Our Company Beijing Shougang Company Ltd., based in China is in search of a competent individual or firm that will be responsible in handling funds as our ''Representative Manager'' in the United Stat
Note: It is a part time job that won't interrupt your present work or business. Looking forward to your response. Best Regards, Liu Nianzu HR(Representative Manager) Beijing Shougang Company Ltd. No 15,Pingguoyuan Road,Shijingshan District,Beijing Website: www.sggf.com.cn
Our Company Beijing Shougang Company Ltd., based in China is in search of a competent individual or firm that will be responsible in handling funds as our ''Representative Manager'' in the United Stat
Note: It is a part time job that won't interrupt your present work or business. Looking forward to your response. Best Regards, Liu Nianzu HR(Representative Manager) Beijing Shougang Company Ltd. No 15,Pingguoyuan Road,Shijingshan District,Beijing Website: www.sggf.com.cn
Re: [RFC v7 PATCH 4/4] mm: unmap special vmas with regular do_munmap()
On Fri 10-08-18 11:51:54, Vlastimil Babka wrote: > On 08/10/2018 01:36 AM, Yang Shi wrote: > > Unmapping vmas, which have VM_HUGETLB | VM_PFNMAP flag set or > > have uprobes set, need get done with write mmap_sem held since > > they may update vm_flags. > > > > So, it might be not safe enough to deal with these kind of special > > mappings with read mmap_sem. Deal with such mappings with regular > > do_munmap() call. > > > > Michal suggested to make this as a separate patch for safer and more > > bisectable sake. > > Hm I believe Michal meant the opposite "evolution" though. Patch 2/4 > should be done in a way that special mappings keep using the regular > path, and this patch would convert them to the new path. Possibly even > each special case separately. yes, that is what I meant. Each of the special case should have its own patch and changelog explaining why it is safe. -- Michal Hocko SUSE Labs
possible deadlock in shmem_fallocate (2)
Hello,
syzbot found the following crash on:
HEAD commit:4110b42356f3 Add linux-next specific files for 20180810
git tree: linux-next
console output: https://syzkaller.appspot.com/x/log.txt?x=1411d6e240
kernel config: https://syzkaller.appspot.com/x/.config?x=1d80606e3795a4f5
dashboard link: https://syzkaller.appspot.com/bug?extid=4b8b031b89e6b96c4b2e
compiler: gcc (GCC) 8.0.1 20180413 (experimental)
syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=175052f840
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1187362240
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+4b8b031b89e6b96c4...@syzkaller.appspotmail.com
random: sshd: uninitialized urandom read (32 bytes read)
random: sshd: uninitialized urandom read (32 bytes read)
random: sshd: uninitialized urandom read (32 bytes read)
==
WARNING: possible circular locking dependency detected
4.18.0-rc8-next-20180810+ #36 Not tainted
--
syz-executor900/4483 is trying to acquire lock:
d2bfc8fe (>s_type->i_mutex_key#9){}, at: inode_lock
include/linux/fs.h:765 [inline]
d2bfc8fe (>s_type->i_mutex_key#9){}, at:
shmem_fallocate+0x18b/0x12e0 mm/shmem.c:2602
but task is already holding lock:
25208078 (ashmem_mutex){+.+.}, at: ashmem_shrink_scan+0xb4/0x630
drivers/staging/android/ashmem.c:448
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #2 (ashmem_mutex){+.+.}:
__mutex_lock_common kernel/locking/mutex.c:925 [inline]
__mutex_lock+0x171/0x1700 kernel/locking/mutex.c:1073
mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:1088
ashmem_mmap+0x55/0x520 drivers/staging/android/ashmem.c:361
call_mmap include/linux/fs.h:1844 [inline]
mmap_region+0xf27/0x1c50 mm/mmap.c:1762
do_mmap+0xa10/0x1220 mm/mmap.c:1535
do_mmap_pgoff include/linux/mm.h:2298 [inline]
vm_mmap_pgoff+0x213/0x2c0 mm/util.c:357
ksys_mmap_pgoff+0x4da/0x660 mm/mmap.c:1585
__do_sys_mmap arch/x86/kernel/sys_x86_64.c:100 [inline]
__se_sys_mmap arch/x86/kernel/sys_x86_64.c:91 [inline]
__x64_sys_mmap+0xe9/0x1b0 arch/x86/kernel/sys_x86_64.c:91
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
-> #1 (>mmap_sem){}:
__might_fault+0x155/0x1e0 mm/memory.c:4568
_copy_to_user+0x30/0x110 lib/usercopy.c:25
copy_to_user include/linux/uaccess.h:155 [inline]
filldir+0x1ea/0x3a0 fs/readdir.c:196
dir_emit_dot include/linux/fs.h:3464 [inline]
dir_emit_dots include/linux/fs.h:3475 [inline]
dcache_readdir+0x13a/0x620 fs/libfs.c:193
iterate_dir+0x48b/0x5d0 fs/readdir.c:51
__do_sys_getdents fs/readdir.c:231 [inline]
__se_sys_getdents fs/readdir.c:212 [inline]
__x64_sys_getdents+0x29f/0x510 fs/readdir.c:212
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
-> #0 (>s_type->i_mutex_key#9){}:
lock_acquire+0x1e4/0x540 kernel/locking/lockdep.c:3924
down_write+0x8f/0x130 kernel/locking/rwsem.c:70
inode_lock include/linux/fs.h:765 [inline]
shmem_fallocate+0x18b/0x12e0 mm/shmem.c:2602
ashmem_shrink_scan+0x236/0x630 drivers/staging/android/ashmem.c:455
ashmem_ioctl+0x3ae/0x13a0 drivers/staging/android/ashmem.c:797
vfs_ioctl fs/ioctl.c:46 [inline]
file_ioctl fs/ioctl.c:501 [inline]
do_vfs_ioctl+0x1de/0x1720 fs/ioctl.c:685
ksys_ioctl+0xa9/0xd0 fs/ioctl.c:702
__do_sys_ioctl fs/ioctl.c:709 [inline]
__se_sys_ioctl fs/ioctl.c:707 [inline]
__x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:707
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
other info that might help us debug this:
Chain exists of:
>s_type->i_mutex_key#9 --> >mmap_sem --> ashmem_mutex
Possible unsafe locking scenario:
CPU0CPU1
lock(ashmem_mutex);
lock(>mmap_sem);
lock(ashmem_mutex);
lock(>s_type->i_mutex_key#9);
*** DEADLOCK ***
1 lock held by syz-executor900/4483:
#0: 25208078 (ashmem_mutex){+.+.}, at:
ashmem_shrink_scan+0xb4/0x630 drivers/staging/android/ashmem.c:448
stack backtrace:
CPU: 1 PID: 4483 Comm: syz-executor900 Not tainted
4.18.0-rc8-next-20180810+ #36
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x1c9/0x2b4 lib/dump_stack.c:113
print_circular_bug.isra.37.cold.58+0x1bd/0x27d
kernel/locking/lockdep.c:1227
check_prev_add kernel/l
Re: [RFC v7 PATCH 4/4] mm: unmap special vmas with regular do_munmap()
On Fri 10-08-18 11:51:54, Vlastimil Babka wrote: > On 08/10/2018 01:36 AM, Yang Shi wrote: > > Unmapping vmas, which have VM_HUGETLB | VM_PFNMAP flag set or > > have uprobes set, need get done with write mmap_sem held since > > they may update vm_flags. > > > > So, it might be not safe enough to deal with these kind of special > > mappings with read mmap_sem. Deal with such mappings with regular > > do_munmap() call. > > > > Michal suggested to make this as a separate patch for safer and more > > bisectable sake. > > Hm I believe Michal meant the opposite "evolution" though. Patch 2/4 > should be done in a way that special mappings keep using the regular > path, and this patch would convert them to the new path. Possibly even > each special case separately. yes, that is what I meant. Each of the special case should have its own patch and changelog explaining why it is safe. -- Michal Hocko SUSE Labs
possible deadlock in shmem_fallocate (2)
Hello,
syzbot found the following crash on:
HEAD commit:4110b42356f3 Add linux-next specific files for 20180810
git tree: linux-next
console output: https://syzkaller.appspot.com/x/log.txt?x=1411d6e240
kernel config: https://syzkaller.appspot.com/x/.config?x=1d80606e3795a4f5
dashboard link: https://syzkaller.appspot.com/bug?extid=4b8b031b89e6b96c4b2e
compiler: gcc (GCC) 8.0.1 20180413 (experimental)
syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=175052f840
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1187362240
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+4b8b031b89e6b96c4...@syzkaller.appspotmail.com
random: sshd: uninitialized urandom read (32 bytes read)
random: sshd: uninitialized urandom read (32 bytes read)
random: sshd: uninitialized urandom read (32 bytes read)
==
WARNING: possible circular locking dependency detected
4.18.0-rc8-next-20180810+ #36 Not tainted
--
syz-executor900/4483 is trying to acquire lock:
d2bfc8fe (>s_type->i_mutex_key#9){}, at: inode_lock
include/linux/fs.h:765 [inline]
d2bfc8fe (>s_type->i_mutex_key#9){}, at:
shmem_fallocate+0x18b/0x12e0 mm/shmem.c:2602
but task is already holding lock:
25208078 (ashmem_mutex){+.+.}, at: ashmem_shrink_scan+0xb4/0x630
drivers/staging/android/ashmem.c:448
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #2 (ashmem_mutex){+.+.}:
__mutex_lock_common kernel/locking/mutex.c:925 [inline]
__mutex_lock+0x171/0x1700 kernel/locking/mutex.c:1073
mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:1088
ashmem_mmap+0x55/0x520 drivers/staging/android/ashmem.c:361
call_mmap include/linux/fs.h:1844 [inline]
mmap_region+0xf27/0x1c50 mm/mmap.c:1762
do_mmap+0xa10/0x1220 mm/mmap.c:1535
do_mmap_pgoff include/linux/mm.h:2298 [inline]
vm_mmap_pgoff+0x213/0x2c0 mm/util.c:357
ksys_mmap_pgoff+0x4da/0x660 mm/mmap.c:1585
__do_sys_mmap arch/x86/kernel/sys_x86_64.c:100 [inline]
__se_sys_mmap arch/x86/kernel/sys_x86_64.c:91 [inline]
__x64_sys_mmap+0xe9/0x1b0 arch/x86/kernel/sys_x86_64.c:91
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
-> #1 (>mmap_sem){}:
__might_fault+0x155/0x1e0 mm/memory.c:4568
_copy_to_user+0x30/0x110 lib/usercopy.c:25
copy_to_user include/linux/uaccess.h:155 [inline]
filldir+0x1ea/0x3a0 fs/readdir.c:196
dir_emit_dot include/linux/fs.h:3464 [inline]
dir_emit_dots include/linux/fs.h:3475 [inline]
dcache_readdir+0x13a/0x620 fs/libfs.c:193
iterate_dir+0x48b/0x5d0 fs/readdir.c:51
__do_sys_getdents fs/readdir.c:231 [inline]
__se_sys_getdents fs/readdir.c:212 [inline]
__x64_sys_getdents+0x29f/0x510 fs/readdir.c:212
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
-> #0 (>s_type->i_mutex_key#9){}:
lock_acquire+0x1e4/0x540 kernel/locking/lockdep.c:3924
down_write+0x8f/0x130 kernel/locking/rwsem.c:70
inode_lock include/linux/fs.h:765 [inline]
shmem_fallocate+0x18b/0x12e0 mm/shmem.c:2602
ashmem_shrink_scan+0x236/0x630 drivers/staging/android/ashmem.c:455
ashmem_ioctl+0x3ae/0x13a0 drivers/staging/android/ashmem.c:797
vfs_ioctl fs/ioctl.c:46 [inline]
file_ioctl fs/ioctl.c:501 [inline]
do_vfs_ioctl+0x1de/0x1720 fs/ioctl.c:685
ksys_ioctl+0xa9/0xd0 fs/ioctl.c:702
__do_sys_ioctl fs/ioctl.c:709 [inline]
__se_sys_ioctl fs/ioctl.c:707 [inline]
__x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:707
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
other info that might help us debug this:
Chain exists of:
>s_type->i_mutex_key#9 --> >mmap_sem --> ashmem_mutex
Possible unsafe locking scenario:
CPU0CPU1
lock(ashmem_mutex);
lock(>mmap_sem);
lock(ashmem_mutex);
lock(>s_type->i_mutex_key#9);
*** DEADLOCK ***
1 lock held by syz-executor900/4483:
#0: 25208078 (ashmem_mutex){+.+.}, at:
ashmem_shrink_scan+0xb4/0x630 drivers/staging/android/ashmem.c:448
stack backtrace:
CPU: 1 PID: 4483 Comm: syz-executor900 Not tainted
4.18.0-rc8-next-20180810+ #36
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x1c9/0x2b4 lib/dump_stack.c:113
print_circular_bug.isra.37.cold.58+0x1bd/0x27d
kernel/locking/lockdep.c:1227
check_prev_add kernel/l
Re: [PATCH 2/2] arm64: dts: meson-g12a: add initial g12a s905d2 SoC DT support
On Thu, 2018-08-09 at 16:22 +0800, Jianxin Pan wrote:
> Try to add basic DT support for the Amlogic's Meson-G12A S905D2 SoC,
> which describe components as follows: Reserve Memory, CPU, GIC, IRQ,
> Timer, UART. It's capable of booting up into the serial console.
>
> Signed-off-by: Jianxin
Could please fix your signoff here ? Your last name went missing
> ---
> arch/arm64/boot/dts/amlogic/Makefile| 1 +
> arch/arm64/boot/dts/amlogic/meson-g12a-u200.dts | 22 +++
> arch/arm64/boot/dts/amlogic/meson-g12a.dtsi | 174
>
> 3 files changed, 197 insertions(+)
> create mode 100644 arch/arm64/boot/dts/amlogic/meson-g12a-u200.dts
> create mode 100644 arch/arm64/boot/dts/amlogic/meson-g12a.dtsi
>
> diff --git a/arch/arm64/boot/dts/amlogic/Makefile
> b/arch/arm64/boot/dts/amlogic/Makefile
> index a97c0e2..c31f29d6 100644
> --- a/arch/arm64/boot/dts/amlogic/Makefile
> +++ b/arch/arm64/boot/dts/amlogic/Makefile
> @@ -1,5 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0
> dtb-$(CONFIG_ARCH_MESON) += meson-axg-s400.dtb
> +dtb-$(CONFIG_ARCH_MESON) += meson-g12a-u200.dtb
> dtb-$(CONFIG_ARCH_MESON) += meson-gxbb-nanopi-k2.dtb
> dtb-$(CONFIG_ARCH_MESON) += meson-gxbb-nexbox-a95x.dtb
> dtb-$(CONFIG_ARCH_MESON) += meson-gxbb-odroidc2.dtb
> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a-u200.dts
> b/arch/arm64/boot/dts/amlogic/meson-g12a-u200.dts
> new file mode 100644
> index 000..d267a37
> --- /dev/null
> +++ b/arch/arm64/boot/dts/amlogic/meson-g12a-u200.dts
> @@ -0,0 +1,22 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2018 Amlogic, Inc. All rights reserved.
> + */
> +
> +/dts-v1/;
> +
> +#include "meson-g12a.dtsi"
> +
> +/ {
> + compatible = "amlogic,u200", "amlogic,g12a";
> + model = "Amlogic Meson G12A U200 Development Board";
> +
> + aliases {
> + serial0 = _AO;
> + };
> +};
> +
> +_AO {
> + status = "okay";
> +};
> +
> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi
> b/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi
> new file mode 100644
> index 000..64a0f2e
> --- /dev/null
> +++ b/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi
> @@ -0,0 +1,174 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2018 Amlogic, Inc. All rights reserved.
> + */
> +
> +#include
> +#include
> +#include
> +
> +/ {
Could you please order the subnodes alphabetically ?
In general, we should try to order nodes by addresses when there is one and
alphabetically when there is none. This is something we have to fix for the AXG
as well.
> + compatible = "amlogic,g12a";
> +
> + interrupt-parent = <>;
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + reserved-memory {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> +
> + /* Alternate 3 MiB reserved for ARM Trusted Firmware (BL31) */
It's the only one (for now at least) so it's not really an alternate, isn't it ?
> + secmon_reserved: secmon@500 {
> + reg = <0x0 0x0500 0x0 0x30>;
> + no-map;
> + };
> + };
> +
> + cpus {
> + #address-cells = <0x2>;
> + #size-cells = <0x0>;
> +
> + cpu0: cpu@0 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53", "arm,armv8";
> + reg = <0x0 0x0>;
> + enable-method = "psci";
> + next-level-cache = <>;
> + };
> +
> + cpu1: cpu@1 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53", "arm,armv8";
> + reg = <0x0 0x1>;
> + enable-method = "psci";
> + next-level-cache = <>;
> + };
> +
> + cpu2: cpu@2 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53", "arm,armv8";
> + reg = <0x0 0x2>;
> + enable-method = "psci";
> + next-level-cache = <>;
> + };
> +
> + cpu3: cpu@3 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53", "arm,armv8";
> + reg = <0x0 0x3>;
> + enable-method = "psci";
> + next-level-cache = <>;
> + };
> +
> + l2: l2-cache0 {
> + compatible = "cache";
> + };
> + };
> +
> + psci {
> + compatible = "arm,psci-1.0";
> + method = "smc";
> + };
> +
> + timer {
> + compatible = "arm,armv8-timer";
> + interrupts = + (GIC_CPU_MASK_RAW(0xff) | IRQ_TYPE_LEVEL_LOW)>,
> + + (GIC_CPU_MASK_RAW(0xff) | IRQ_TYPE_LEVEL_LOW)>,
> +
Re: [PATCH 2/2] arm64: dts: meson-g12a: add initial g12a s905d2 SoC DT support
On Thu, 2018-08-09 at 16:22 +0800, Jianxin Pan wrote:
> Try to add basic DT support for the Amlogic's Meson-G12A S905D2 SoC,
> which describe components as follows: Reserve Memory, CPU, GIC, IRQ,
> Timer, UART. It's capable of booting up into the serial console.
>
> Signed-off-by: Jianxin
Could please fix your signoff here ? Your last name went missing
> ---
> arch/arm64/boot/dts/amlogic/Makefile| 1 +
> arch/arm64/boot/dts/amlogic/meson-g12a-u200.dts | 22 +++
> arch/arm64/boot/dts/amlogic/meson-g12a.dtsi | 174
>
> 3 files changed, 197 insertions(+)
> create mode 100644 arch/arm64/boot/dts/amlogic/meson-g12a-u200.dts
> create mode 100644 arch/arm64/boot/dts/amlogic/meson-g12a.dtsi
>
> diff --git a/arch/arm64/boot/dts/amlogic/Makefile
> b/arch/arm64/boot/dts/amlogic/Makefile
> index a97c0e2..c31f29d6 100644
> --- a/arch/arm64/boot/dts/amlogic/Makefile
> +++ b/arch/arm64/boot/dts/amlogic/Makefile
> @@ -1,5 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0
> dtb-$(CONFIG_ARCH_MESON) += meson-axg-s400.dtb
> +dtb-$(CONFIG_ARCH_MESON) += meson-g12a-u200.dtb
> dtb-$(CONFIG_ARCH_MESON) += meson-gxbb-nanopi-k2.dtb
> dtb-$(CONFIG_ARCH_MESON) += meson-gxbb-nexbox-a95x.dtb
> dtb-$(CONFIG_ARCH_MESON) += meson-gxbb-odroidc2.dtb
> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a-u200.dts
> b/arch/arm64/boot/dts/amlogic/meson-g12a-u200.dts
> new file mode 100644
> index 000..d267a37
> --- /dev/null
> +++ b/arch/arm64/boot/dts/amlogic/meson-g12a-u200.dts
> @@ -0,0 +1,22 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2018 Amlogic, Inc. All rights reserved.
> + */
> +
> +/dts-v1/;
> +
> +#include "meson-g12a.dtsi"
> +
> +/ {
> + compatible = "amlogic,u200", "amlogic,g12a";
> + model = "Amlogic Meson G12A U200 Development Board";
> +
> + aliases {
> + serial0 = _AO;
> + };
> +};
> +
> +_AO {
> + status = "okay";
> +};
> +
> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi
> b/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi
> new file mode 100644
> index 000..64a0f2e
> --- /dev/null
> +++ b/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi
> @@ -0,0 +1,174 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2018 Amlogic, Inc. All rights reserved.
> + */
> +
> +#include
> +#include
> +#include
> +
> +/ {
Could you please order the subnodes alphabetically ?
In general, we should try to order nodes by addresses when there is one and
alphabetically when there is none. This is something we have to fix for the AXG
as well.
> + compatible = "amlogic,g12a";
> +
> + interrupt-parent = <>;
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + reserved-memory {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> +
> + /* Alternate 3 MiB reserved for ARM Trusted Firmware (BL31) */
It's the only one (for now at least) so it's not really an alternate, isn't it ?
> + secmon_reserved: secmon@500 {
> + reg = <0x0 0x0500 0x0 0x30>;
> + no-map;
> + };
> + };
> +
> + cpus {
> + #address-cells = <0x2>;
> + #size-cells = <0x0>;
> +
> + cpu0: cpu@0 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53", "arm,armv8";
> + reg = <0x0 0x0>;
> + enable-method = "psci";
> + next-level-cache = <>;
> + };
> +
> + cpu1: cpu@1 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53", "arm,armv8";
> + reg = <0x0 0x1>;
> + enable-method = "psci";
> + next-level-cache = <>;
> + };
> +
> + cpu2: cpu@2 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53", "arm,armv8";
> + reg = <0x0 0x2>;
> + enable-method = "psci";
> + next-level-cache = <>;
> + };
> +
> + cpu3: cpu@3 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53", "arm,armv8";
> + reg = <0x0 0x3>;
> + enable-method = "psci";
> + next-level-cache = <>;
> + };
> +
> + l2: l2-cache0 {
> + compatible = "cache";
> + };
> + };
> +
> + psci {
> + compatible = "arm,psci-1.0";
> + method = "smc";
> + };
> +
> + timer {
> + compatible = "arm,armv8-timer";
> + interrupts = + (GIC_CPU_MASK_RAW(0xff) | IRQ_TYPE_LEVEL_LOW)>,
> + + (GIC_CPU_MASK_RAW(0xff) | IRQ_TYPE_LEVEL_LOW)>,
> +
Re: [PATCH v2] perf ordered_events: fix crash in free_dup_event()
On Fri, Aug 10, 2018 at 01:21:18AM -0700, Stephane Eranian wrote:
> On Thu, Aug 9, 2018 at 1:07 AM Jiri Olsa wrote:
> >
> > On Wed, Aug 08, 2018 at 03:33:20PM -0700, Stephane Eranian wrote:
> > > This patch fixes a bug in ordered_event.c:alloc_event().
> > > An ordered_event struct was not initialized properly potentially
> > > causing crashes later on in free_dup_event() depending on the
> > > content of the memory. If it was NULL, then it would work fine,
> > > otherwise, it could cause crashes such as:
> >
> > I'm now little puzzled what do we use this first event for..
> > I can't see anything special about it, other than it's added
> > on the list uninitialized ;-)
> >
> > it seems to work properly when we ditch it.. might be some
> > prehistoric leftover or I'm terribly missing something
> >
> You need to keep track of the buffers to free. You do not free the
> ordered_event structs
> individually. For each oe->buffer, you need one free(). Each buffer is
> put in the to_free
> list. But to link it into the list it needs a list_head. This is what
> buffer[0] is used for.
> But the logic is broken in ordered_events__free(). It does not free
> individual
> ordered_event structs, but a buffer with many. Yet, it is missing
> freeing all of the duped
> events.
>
> void ordered_events__free(struct ordered_events *oe)
> {
> while (!list_empty(>to_free)) {
> struct ordered_event *buffer;
>
> buffer = list_entry(oe->to_free.next, struct
> ordered_event, list);
> list_del(>list);
> > free_dup_event(oe, event->event);
> free(buffer);
> }
> }
> This only frees the dup_event of buffer[0] which we know is NULL (well, now).
> It needs to walk all the entries in buffer[] to free buffer[x].event.
yes.. if there's copy_on_queue set, we need to do that,
otherwise we're leaking all the events
>
> I think the goal was likely to avoid adding another list_head field to
> each ordered_event
> and instead use one per allocated buffer.
> This is very convoluted and prone to errors and we are seeing right
> now. This should
> be cleaned. So either you add a list_head to ordered_event or you
> would buffer[x] in
> ordered_events_free().
>
> At this point, this is my understanding.
> Do you agree?
yea, I see it now.. thanks for pointing this out
how about something like below? haven't tested properly yet
jirka
---
diff --git a/tools/perf/util/ordered-events.c b/tools/perf/util/ordered-events.c
index bad9e0296e9a..5c0d85e90a18 100644
--- a/tools/perf/util/ordered-events.c
+++ b/tools/perf/util/ordered-events.c
@@ -80,14 +80,20 @@ static union perf_event *dup_event(struct ordered_events
*oe,
return oe->copy_on_queue ? __dup_event(oe, event) : event;
}
-static void free_dup_event(struct ordered_events *oe, union perf_event *event)
+static void __free_dup_event(struct ordered_events *oe, union perf_event
*event)
{
- if (event && oe->copy_on_queue) {
+ if (event) {
oe->cur_alloc_size -= event->header.size;
free(event);
}
}
+static void free_dup_event(struct ordered_events *oe, union perf_event *event)
+{
+ if (oe->copy_on_queue)
+ __free_dup_event(oe, event);
+}
+
#define MAX_SAMPLE_BUFFER (64 * 1024 / sizeof(struct ordered_event))
static struct ordered_event *alloc_event(struct ordered_events *oe,
union perf_event *event)
@@ -104,11 +110,11 @@ static struct ordered_event *alloc_event(struct
ordered_events *oe,
new = list_entry(cache->next, struct ordered_event, list);
list_del(>list);
} else if (oe->buffer) {
- new = oe->buffer + oe->buffer_idx;
+ new = >buffer->event[oe->buffer_idx];
if (++oe->buffer_idx == MAX_SAMPLE_BUFFER)
oe->buffer = NULL;
} else if (oe->cur_alloc_size < oe->max_alloc_size) {
- size_t size = MAX_SAMPLE_BUFFER * sizeof(*new);
+ size_t size = sizeof(*oe->buffer) + MAX_SAMPLE_BUFFER *
sizeof(*new);
oe->buffer = malloc(size);
if (!oe->buffer) {
@@ -122,9 +128,8 @@ static struct ordered_event *alloc_event(struct
ordered_events *oe,
oe->cur_alloc_size += size;
list_add(>buffer->list, >to_free);
- /* First entry is abused to maintain the to_free list. */
- oe->buffer_idx = 2;
- new = oe->buffer + 1;
+ oe->buffer_idx = 1;
+ new = >buffer->event[0];
} else {
pr("allocation limit reached %" PRIu64 "B\n",
oe->max_alloc_size);
}
@@ -300,15 +305,27 @@ void ordered_events__init(struct ordered_events *oe,
ordered_events__deliver_t d
oe->deliver= deliver;
}
+static void
+ordered_events_buffer__free(struct ordered_events_buffer *buffer,
+
Re: [PATCH v2] perf ordered_events: fix crash in free_dup_event()
On Fri, Aug 10, 2018 at 01:21:18AM -0700, Stephane Eranian wrote:
> On Thu, Aug 9, 2018 at 1:07 AM Jiri Olsa wrote:
> >
> > On Wed, Aug 08, 2018 at 03:33:20PM -0700, Stephane Eranian wrote:
> > > This patch fixes a bug in ordered_event.c:alloc_event().
> > > An ordered_event struct was not initialized properly potentially
> > > causing crashes later on in free_dup_event() depending on the
> > > content of the memory. If it was NULL, then it would work fine,
> > > otherwise, it could cause crashes such as:
> >
> > I'm now little puzzled what do we use this first event for..
> > I can't see anything special about it, other than it's added
> > on the list uninitialized ;-)
> >
> > it seems to work properly when we ditch it.. might be some
> > prehistoric leftover or I'm terribly missing something
> >
> You need to keep track of the buffers to free. You do not free the
> ordered_event structs
> individually. For each oe->buffer, you need one free(). Each buffer is
> put in the to_free
> list. But to link it into the list it needs a list_head. This is what
> buffer[0] is used for.
> But the logic is broken in ordered_events__free(). It does not free
> individual
> ordered_event structs, but a buffer with many. Yet, it is missing
> freeing all of the duped
> events.
>
> void ordered_events__free(struct ordered_events *oe)
> {
> while (!list_empty(>to_free)) {
> struct ordered_event *buffer;
>
> buffer = list_entry(oe->to_free.next, struct
> ordered_event, list);
> list_del(>list);
> > free_dup_event(oe, event->event);
> free(buffer);
> }
> }
> This only frees the dup_event of buffer[0] which we know is NULL (well, now).
> It needs to walk all the entries in buffer[] to free buffer[x].event.
yes.. if there's copy_on_queue set, we need to do that,
otherwise we're leaking all the events
>
> I think the goal was likely to avoid adding another list_head field to
> each ordered_event
> and instead use one per allocated buffer.
> This is very convoluted and prone to errors and we are seeing right
> now. This should
> be cleaned. So either you add a list_head to ordered_event or you
> would buffer[x] in
> ordered_events_free().
>
> At this point, this is my understanding.
> Do you agree?
yea, I see it now.. thanks for pointing this out
how about something like below? haven't tested properly yet
jirka
---
diff --git a/tools/perf/util/ordered-events.c b/tools/perf/util/ordered-events.c
index bad9e0296e9a..5c0d85e90a18 100644
--- a/tools/perf/util/ordered-events.c
+++ b/tools/perf/util/ordered-events.c
@@ -80,14 +80,20 @@ static union perf_event *dup_event(struct ordered_events
*oe,
return oe->copy_on_queue ? __dup_event(oe, event) : event;
}
-static void free_dup_event(struct ordered_events *oe, union perf_event *event)
+static void __free_dup_event(struct ordered_events *oe, union perf_event
*event)
{
- if (event && oe->copy_on_queue) {
+ if (event) {
oe->cur_alloc_size -= event->header.size;
free(event);
}
}
+static void free_dup_event(struct ordered_events *oe, union perf_event *event)
+{
+ if (oe->copy_on_queue)
+ __free_dup_event(oe, event);
+}
+
#define MAX_SAMPLE_BUFFER (64 * 1024 / sizeof(struct ordered_event))
static struct ordered_event *alloc_event(struct ordered_events *oe,
union perf_event *event)
@@ -104,11 +110,11 @@ static struct ordered_event *alloc_event(struct
ordered_events *oe,
new = list_entry(cache->next, struct ordered_event, list);
list_del(>list);
} else if (oe->buffer) {
- new = oe->buffer + oe->buffer_idx;
+ new = >buffer->event[oe->buffer_idx];
if (++oe->buffer_idx == MAX_SAMPLE_BUFFER)
oe->buffer = NULL;
} else if (oe->cur_alloc_size < oe->max_alloc_size) {
- size_t size = MAX_SAMPLE_BUFFER * sizeof(*new);
+ size_t size = sizeof(*oe->buffer) + MAX_SAMPLE_BUFFER *
sizeof(*new);
oe->buffer = malloc(size);
if (!oe->buffer) {
@@ -122,9 +128,8 @@ static struct ordered_event *alloc_event(struct
ordered_events *oe,
oe->cur_alloc_size += size;
list_add(>buffer->list, >to_free);
- /* First entry is abused to maintain the to_free list. */
- oe->buffer_idx = 2;
- new = oe->buffer + 1;
+ oe->buffer_idx = 1;
+ new = >buffer->event[0];
} else {
pr("allocation limit reached %" PRIu64 "B\n",
oe->max_alloc_size);
}
@@ -300,15 +305,27 @@ void ordered_events__init(struct ordered_events *oe,
ordered_events__deliver_t d
oe->deliver= deliver;
}
+static void
+ordered_events_buffer__free(struct ordered_events_buffer *buffer,
+
Re: [PATCH] uprobes: Use synchronize_rcu() not synchronize_sched()
On 08/10, Oleg Nesterov wrote: > > @@ -920,7 +918,8 @@ probe_event_enable(struct trace_uprobe * > ret = uprobe_register(tu->inode, tu->offset, >consumer); > if (ret) > goto err_buffer; > - > + add: > + list_add_tail_rcu(>list, >tp.files); if (link) list_add_tail_rcu(>list, >tp.files); Oleg.
Re: [PATCH] uprobes: Use synchronize_rcu() not synchronize_sched()
On 08/10, Oleg Nesterov wrote: > > @@ -920,7 +918,8 @@ probe_event_enable(struct trace_uprobe * > ret = uprobe_register(tu->inode, tu->offset, >consumer); > if (ret) > goto err_buffer; > - > + add: > + list_add_tail_rcu(>list, >tp.files); if (link) list_add_tail_rcu(>list, >tp.files); Oleg.
Re: [PATCH] hid: microsoft: Add rumble support for Xbox One S controller
On Thu, 2018-08-09 at 17:17 -0700, Andrey Smirnov wrote: > Add HID quirk driver for Xbox One S controller over bluetooth. > > This driver only adds support for rumble. Standard controller > functionality is exposed by default HID driver. Did you manage to make the joypad work without hacks in the Bluetooth stack[1]? [1]: https://github.com/ValveSoftware/steamos_kernel/commit/549c3dc10fa3749b3999549a2672b14bf0e9786d
Re: [PATCH] hid: microsoft: Add rumble support for Xbox One S controller
On Thu, 2018-08-09 at 17:17 -0700, Andrey Smirnov wrote: > Add HID quirk driver for Xbox One S controller over bluetooth. > > This driver only adds support for rumble. Standard controller > functionality is exposed by default HID driver. Did you manage to make the joypad work without hacks in the Bluetooth stack[1]? [1]: https://github.com/ValveSoftware/steamos_kernel/commit/549c3dc10fa3749b3999549a2672b14bf0e9786d
Re: [PATCH] uprobes: Use synchronize_rcu() not synchronize_sched()
On 08/09, Steven Rostedt wrote:
>
> --- a/kernel/trace/trace_uprobe.c
> +++ b/kernel/trace/trace_uprobe.c
> @@ -952,7 +952,7 @@ probe_event_disable(struct trace_uprobe *tu, struct
> trace_event_file *file)
>
> list_del_rcu(>list);
> /* synchronize with u{,ret}probe_trace_func */
> - synchronize_sched();
> + synchronize_rcu();
Can't we change uprobe_trace_func() and uretprobe_trace_func() to use
rcu_read_lock_sched() instead? It is more cheap.
Hmm. probe_event_enable() does list_del + kfree on failure, this doesn't
look right... Not only because kfree() can race with list_for_each_entry_rcu(),
we should not put the 1st link on list until uprobe_buffer_enable().
Does the patch below make sense or I am confused?
Oleg.
--- x/kernel/trace/trace_uprobe.c
+++ x/kernel/trace/trace_uprobe.c
@@ -896,8 +896,6 @@ probe_event_enable(struct trace_uprobe *
return -ENOMEM;
link->file = file;
- list_add_tail_rcu(>list, >tp.files);
-
tu->tp.flags |= TP_FLAG_TRACE;
} else {
if (tu->tp.flags & TP_FLAG_TRACE)
@@ -909,7 +907,7 @@ probe_event_enable(struct trace_uprobe *
WARN_ON(!uprobe_filter_is_empty(>filter));
if (enabled)
- return 0;
+ goto add;
ret = uprobe_buffer_enable();
if (ret)
@@ -920,7 +918,8 @@ probe_event_enable(struct trace_uprobe *
ret = uprobe_register(tu->inode, tu->offset, >consumer);
if (ret)
goto err_buffer;
-
+ add:
+ list_add_tail_rcu(>list, >tp.files);
return 0;
err_buffer:
@@ -928,7 +927,6 @@ probe_event_enable(struct trace_uprobe *
err_flags:
if (file) {
- list_del(>list);
kfree(link);
tu->tp.flags &= ~TP_FLAG_TRACE;
} else {
Re: [PATCH] uprobes: Use synchronize_rcu() not synchronize_sched()
On 08/09, Steven Rostedt wrote:
>
> --- a/kernel/trace/trace_uprobe.c
> +++ b/kernel/trace/trace_uprobe.c
> @@ -952,7 +952,7 @@ probe_event_disable(struct trace_uprobe *tu, struct
> trace_event_file *file)
>
> list_del_rcu(>list);
> /* synchronize with u{,ret}probe_trace_func */
> - synchronize_sched();
> + synchronize_rcu();
Can't we change uprobe_trace_func() and uretprobe_trace_func() to use
rcu_read_lock_sched() instead? It is more cheap.
Hmm. probe_event_enable() does list_del + kfree on failure, this doesn't
look right... Not only because kfree() can race with list_for_each_entry_rcu(),
we should not put the 1st link on list until uprobe_buffer_enable().
Does the patch below make sense or I am confused?
Oleg.
--- x/kernel/trace/trace_uprobe.c
+++ x/kernel/trace/trace_uprobe.c
@@ -896,8 +896,6 @@ probe_event_enable(struct trace_uprobe *
return -ENOMEM;
link->file = file;
- list_add_tail_rcu(>list, >tp.files);
-
tu->tp.flags |= TP_FLAG_TRACE;
} else {
if (tu->tp.flags & TP_FLAG_TRACE)
@@ -909,7 +907,7 @@ probe_event_enable(struct trace_uprobe *
WARN_ON(!uprobe_filter_is_empty(>filter));
if (enabled)
- return 0;
+ goto add;
ret = uprobe_buffer_enable();
if (ret)
@@ -920,7 +918,8 @@ probe_event_enable(struct trace_uprobe *
ret = uprobe_register(tu->inode, tu->offset, >consumer);
if (ret)
goto err_buffer;
-
+ add:
+ list_add_tail_rcu(>list, >tp.files);
return 0;
err_buffer:
@@ -928,7 +927,6 @@ probe_event_enable(struct trace_uprobe *
err_flags:
if (file) {
- list_del(>list);
kfree(link);
tu->tp.flags &= ~TP_FLAG_TRACE;
} else {
Re: [PATCH v3 1/5] staging: rtl8188eu: use is_multicast_ether_addr in rtw_security.c
No no... I only gave it a Reviewed-by tag because I didn't want you to resend again... :P regards, dan carpenter
Re: [PATCH v3 1/5] staging: rtl8188eu: use is_multicast_ether_addr in rtw_security.c
No no... I only gave it a Reviewed-by tag because I didn't want you to resend again... :P regards, dan carpenter
Re: [PATCH 1/2] staging: fbtft: Moves ";" from macro definition to macro usage.
Hi Leonardo, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on staging/staging-testing] [also build test WARNING on v4.18-rc8 next-20180809] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Leonardo-Br-s/staging-fbtft-Moves-from-macro-definition-to-macro-usage/20180810-154004 reproduce: # apt-get install sparse make ARCH=x86_64 allmodconfig make C=1 CF=-D__CHECK_ENDIAN__ sparse warnings: (new ones prefixed by >>) >> drivers/staging/fbtft/fbtft-bus.c:66:1: sparse: incorrect type in assignment >> (different base types) @@expected restricted __be16 [usertype] >> @@got unsignedrestricted __be16 [usertype] @@ drivers/staging/fbtft/fbtft-bus.c:66:1:expected restricted __be16 [usertype] drivers/staging/fbtft/fbtft-bus.c:66:1:got unsigned short [unsigned] [usertype] >> drivers/staging/fbtft/fbtft-bus.c:66:1: sparse: incorrect type in assignment >> (different base types) @@expected restricted __be16 [usertype] >> @@got unsignedrestricted __be16 [usertype] @@ drivers/staging/fbtft/fbtft-bus.c:66:1:expected restricted __be16 [usertype] drivers/staging/fbtft/fbtft-bus.c:66:1:got unsigned short [unsigned] [usertype] >> drivers/staging/fbtft/fbtft-bus.c:66:1: sparse: incorrect type in assignment >> (different base types) @@expected restricted __be16 [usertype] >> @@got unsignedrestricted __be16 [usertype] @@ drivers/staging/fbtft/fbtft-bus.c:66:1:expected restricted __be16 [usertype] drivers/staging/fbtft/fbtft-bus.c:66:1:got unsigned short [unsigned] [usertype] drivers/staging/fbtft/fbtft-bus.c:156:27: sparse: expression using sizeof(void) drivers/staging/fbtft/fbtft-bus.c:156:27: sparse: expression using sizeof(void) drivers/staging/fbtft/fbtft-bus.c:200:27: sparse: expression using sizeof(void) drivers/staging/fbtft/fbtft-bus.c:200:27: sparse: expression using sizeof(void) vim +66 drivers/staging/fbtft/fbtft-bus.c 64 65 define_fbtft_write_reg(fbtft_write_reg8_bus8, u8, u8, ); > 66 define_fbtft_write_reg(fbtft_write_reg16_bus8, __be16, u16, ); 67 define_fbtft_write_reg(fbtft_write_reg16_bus16, u16, u16, ); 68 --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
Re: [PATCH 1/2] staging: fbtft: Moves ";" from macro definition to macro usage.
Hi Leonardo, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on staging/staging-testing] [also build test WARNING on v4.18-rc8 next-20180809] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Leonardo-Br-s/staging-fbtft-Moves-from-macro-definition-to-macro-usage/20180810-154004 reproduce: # apt-get install sparse make ARCH=x86_64 allmodconfig make C=1 CF=-D__CHECK_ENDIAN__ sparse warnings: (new ones prefixed by >>) >> drivers/staging/fbtft/fbtft-bus.c:66:1: sparse: incorrect type in assignment >> (different base types) @@expected restricted __be16 [usertype] >> @@got unsignedrestricted __be16 [usertype] @@ drivers/staging/fbtft/fbtft-bus.c:66:1:expected restricted __be16 [usertype] drivers/staging/fbtft/fbtft-bus.c:66:1:got unsigned short [unsigned] [usertype] >> drivers/staging/fbtft/fbtft-bus.c:66:1: sparse: incorrect type in assignment >> (different base types) @@expected restricted __be16 [usertype] >> @@got unsignedrestricted __be16 [usertype] @@ drivers/staging/fbtft/fbtft-bus.c:66:1:expected restricted __be16 [usertype] drivers/staging/fbtft/fbtft-bus.c:66:1:got unsigned short [unsigned] [usertype] >> drivers/staging/fbtft/fbtft-bus.c:66:1: sparse: incorrect type in assignment >> (different base types) @@expected restricted __be16 [usertype] >> @@got unsignedrestricted __be16 [usertype] @@ drivers/staging/fbtft/fbtft-bus.c:66:1:expected restricted __be16 [usertype] drivers/staging/fbtft/fbtft-bus.c:66:1:got unsigned short [unsigned] [usertype] drivers/staging/fbtft/fbtft-bus.c:156:27: sparse: expression using sizeof(void) drivers/staging/fbtft/fbtft-bus.c:156:27: sparse: expression using sizeof(void) drivers/staging/fbtft/fbtft-bus.c:200:27: sparse: expression using sizeof(void) drivers/staging/fbtft/fbtft-bus.c:200:27: sparse: expression using sizeof(void) vim +66 drivers/staging/fbtft/fbtft-bus.c 64 65 define_fbtft_write_reg(fbtft_write_reg8_bus8, u8, u8, ); > 66 define_fbtft_write_reg(fbtft_write_reg16_bus8, __be16, u16, ); 67 define_fbtft_write_reg(fbtft_write_reg16_bus16, u16, u16, ); 68 --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
[PATCH v3] cpuidle: menu: Handle stopped tick more aggressively
From: Rafael J. Wysocki
Commit 87c9fe6ee495 (cpuidle: menu: Avoid selecting shallow states
with stopped tick) missed the case when the target residencies of
deep idle states of CPUs are above the tick boundary which may cause
the CPU to get stuck in a shallow idle state for a long time.
Say there are two CPU idle states available: one shallow, with the
target residency much below the tick boundary and one deep, with
the target residency significantly above the tick boundary. In
that case, if the tick has been stopped already and the expected
next timer event is relatively far in the future, the governor will
assume the idle duration to be equal to TICK_USEC and it will select
the idle state for the CPU accordingly. However, that will cause the
shallow state to be selected even though it would have been more
energy-efficient to select the deep one.
To address this issue, modify the governor to always assume idle
duration to be equal to the time till the closest timer event if
the tick is not running which will cause the selected idle states
to always match the known CPU wakeup time.
Also make it always indicate that the tick should be stopped in
that case for consistency.
Fixes: 87c9fe6ee495 (cpuidle: menu: Avoid selecting shallow states with stopped
tick)
Reported-by: Leo Yan
Signed-off-by: Rafael J. Wysocki
---
-> v2: Initialize first_idx properly in the stopped tick case.
-> v3: Compute data->bucket before checking whether or not the tick has been
stopped already to prevent it from becoming stale.
---
drivers/cpuidle/governors/menu.c | 55 +--
1 file changed, 25 insertions(+), 30 deletions(-)
Index: linux-pm/drivers/cpuidle/governors/menu.c
===
--- linux-pm.orig/drivers/cpuidle/governors/menu.c
+++ linux-pm/drivers/cpuidle/governors/menu.c
@@ -285,9 +285,8 @@ static int menu_select(struct cpuidle_dr
{
struct menu_device *data = this_cpu_ptr(_devices);
int latency_req = cpuidle_governor_latency_req(dev->cpu);
- int i;
- int first_idx;
- int idx;
+ int first_idx = 0;
+ int idx, i;
unsigned int interactivity_req;
unsigned int expected_interval;
unsigned long nr_iowaiters, cpu_load;
@@ -311,6 +310,18 @@ static int menu_select(struct cpuidle_dr
data->bucket = which_bucket(data->next_timer_us, nr_iowaiters);
/*
+* If the tick is already stopped, the cost of possible short idle
+* duration misprediction is much higher, because the CPU may be stuck
+* in a shallow idle state for a long time as a result of it. In that
+* case say we might mispredict and use the known time till the closest
+* timer event for the idle state selection.
+*/
+ if (tick_nohz_tick_stopped()) {
+ data->predicted_us = ktime_to_us(delta_next);
+ goto select;
+ }
+
+ /*
* Force the result of multiplication to be 64 bits even if both
* operands are 32 bits.
* Make sure to round up for half microseconds.
@@ -322,7 +333,6 @@ static int menu_select(struct cpuidle_dr
expected_interval = get_typical_interval(data);
expected_interval = min(expected_interval, data->next_timer_us);
- first_idx = 0;
if (drv->states[0].flags & CPUIDLE_FLAG_POLLING) {
struct cpuidle_state *s = >states[1];
unsigned int polling_threshold;
@@ -344,29 +354,15 @@ static int menu_select(struct cpuidle_dr
*/
data->predicted_us = min(data->predicted_us, expected_interval);
- if (tick_nohz_tick_stopped()) {
- /*
-* If the tick is already stopped, the cost of possible short
-* idle duration misprediction is much higher, because the CPU
-* may be stuck in a shallow idle state for a long time as a
-* result of it. In that case say we might mispredict and try
-* to force the CPU into a state for which we would have stopped
-* the tick, unless a timer is going to expire really soon
-* anyway.
-*/
- if (data->predicted_us < TICK_USEC)
- data->predicted_us = min_t(unsigned int, TICK_USEC,
- ktime_to_us(delta_next));
- } else {
- /*
-* Use the performance multiplier and the user-configurable
-* latency_req to determine the maximum exit latency.
-*/
- interactivity_req = data->predicted_us /
performance_multiplier(nr_iowaiters, cpu_load);
- if (latency_req > interactivity_req)
- latency_req = interactivity_req;
- }
+ /*
+* Use the performance multiplier and the user-configurable latency_req
+* to
[PATCH v3] cpuidle: menu: Handle stopped tick more aggressively
From: Rafael J. Wysocki
Commit 87c9fe6ee495 (cpuidle: menu: Avoid selecting shallow states
with stopped tick) missed the case when the target residencies of
deep idle states of CPUs are above the tick boundary which may cause
the CPU to get stuck in a shallow idle state for a long time.
Say there are two CPU idle states available: one shallow, with the
target residency much below the tick boundary and one deep, with
the target residency significantly above the tick boundary. In
that case, if the tick has been stopped already and the expected
next timer event is relatively far in the future, the governor will
assume the idle duration to be equal to TICK_USEC and it will select
the idle state for the CPU accordingly. However, that will cause the
shallow state to be selected even though it would have been more
energy-efficient to select the deep one.
To address this issue, modify the governor to always assume idle
duration to be equal to the time till the closest timer event if
the tick is not running which will cause the selected idle states
to always match the known CPU wakeup time.
Also make it always indicate that the tick should be stopped in
that case for consistency.
Fixes: 87c9fe6ee495 (cpuidle: menu: Avoid selecting shallow states with stopped
tick)
Reported-by: Leo Yan
Signed-off-by: Rafael J. Wysocki
---
-> v2: Initialize first_idx properly in the stopped tick case.
-> v3: Compute data->bucket before checking whether or not the tick has been
stopped already to prevent it from becoming stale.
---
drivers/cpuidle/governors/menu.c | 55 +--
1 file changed, 25 insertions(+), 30 deletions(-)
Index: linux-pm/drivers/cpuidle/governors/menu.c
===
--- linux-pm.orig/drivers/cpuidle/governors/menu.c
+++ linux-pm/drivers/cpuidle/governors/menu.c
@@ -285,9 +285,8 @@ static int menu_select(struct cpuidle_dr
{
struct menu_device *data = this_cpu_ptr(_devices);
int latency_req = cpuidle_governor_latency_req(dev->cpu);
- int i;
- int first_idx;
- int idx;
+ int first_idx = 0;
+ int idx, i;
unsigned int interactivity_req;
unsigned int expected_interval;
unsigned long nr_iowaiters, cpu_load;
@@ -311,6 +310,18 @@ static int menu_select(struct cpuidle_dr
data->bucket = which_bucket(data->next_timer_us, nr_iowaiters);
/*
+* If the tick is already stopped, the cost of possible short idle
+* duration misprediction is much higher, because the CPU may be stuck
+* in a shallow idle state for a long time as a result of it. In that
+* case say we might mispredict and use the known time till the closest
+* timer event for the idle state selection.
+*/
+ if (tick_nohz_tick_stopped()) {
+ data->predicted_us = ktime_to_us(delta_next);
+ goto select;
+ }
+
+ /*
* Force the result of multiplication to be 64 bits even if both
* operands are 32 bits.
* Make sure to round up for half microseconds.
@@ -322,7 +333,6 @@ static int menu_select(struct cpuidle_dr
expected_interval = get_typical_interval(data);
expected_interval = min(expected_interval, data->next_timer_us);
- first_idx = 0;
if (drv->states[0].flags & CPUIDLE_FLAG_POLLING) {
struct cpuidle_state *s = >states[1];
unsigned int polling_threshold;
@@ -344,29 +354,15 @@ static int menu_select(struct cpuidle_dr
*/
data->predicted_us = min(data->predicted_us, expected_interval);
- if (tick_nohz_tick_stopped()) {
- /*
-* If the tick is already stopped, the cost of possible short
-* idle duration misprediction is much higher, because the CPU
-* may be stuck in a shallow idle state for a long time as a
-* result of it. In that case say we might mispredict and try
-* to force the CPU into a state for which we would have stopped
-* the tick, unless a timer is going to expire really soon
-* anyway.
-*/
- if (data->predicted_us < TICK_USEC)
- data->predicted_us = min_t(unsigned int, TICK_USEC,
- ktime_to_us(delta_next));
- } else {
- /*
-* Use the performance multiplier and the user-configurable
-* latency_req to determine the maximum exit latency.
-*/
- interactivity_req = data->predicted_us /
performance_multiplier(nr_iowaiters, cpu_load);
- if (latency_req > interactivity_req)
- latency_req = interactivity_req;
- }
+ /*
+* Use the performance multiplier and the user-configurable latency_req
+* to
Re: [PATCH v8 17/22] s390: vfio-ap: zeroize the AP queues.
On Fri, 10 Aug 2018 12:49:08 +0200
Pierre Morel wrote:
> On 10/08/2018 11:14, Cornelia Huck wrote:
> > On Wed, 8 Aug 2018 10:44:27 -0400
> > Tony Krowiak wrote:
> >
> >> From: Tony Krowiak
> >>
> >> Let's call PAPQ(ZAPQ) to zeroize a queue:
> >>
> >> * For each queue configured for a mediated matrix device
> >>when it is released.
> >>
> >> Zeroizing a queue resets the queue, clears all pending
> >> messages for the queue entries and disables adapter interruptions
> >> associated with the queue.
> >>
> >> Signed-off-by: Tony Krowiak
> >> Reviewed-by: Halil Pasic
> >> Tested-by: Michael Mueller
> >> Tested-by: Farhan Ali
> >> Signed-off-by: Christian Borntraeger
> >> ---
> >> drivers/s390/crypto/vfio_ap_ops.c | 29
> >> -
> >> drivers/s390/crypto/vfio_ap_private.h | 25 +
> >> 2 files changed, 53 insertions(+), 1 deletions(-)
> >>
> >> @@ -788,7 +812,10 @@ static void vfio_ap_mdev_release(struct mdev_device
> >> *mdev)
> >> {
> >>struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
> >>
> >> - kvm_arch_crypto_clear_masks(matrix_mdev->kvm);
> >> + if (matrix_mdev->kvm)
> >> + kvm_arch_crypto_clear_masks(matrix_mdev->kvm);
> > Confused. Why is the check for matrix_mdev->kvm added here?
>
> When using the KVM notifier we can get two notifications:
> -> KVM is here / is comming
> -> KVM is not here / disappearing
>
> In the first case we initialize matrix_mdev->kvm with a pointer to KVM
> In the second case we nullify the pointer.
>
> During the open of the mediated device, the guest should have been started
> or we refuse to start.
>
> During the close of the mediated device, the guest should be there, but
> we have no certitude that the guest did not disappear before the VFIO
> file being closed.
> Since we do not allow multiple guests using the same mediated device
> this case should not happen with QEMU. But I am not sure that
> a rogue user program could not stop KVM before closing the VFIO
> mediated device.
I'm not sure why the check is introduced in this patch, though. But
maybe I just need weekend :)
>
> Maybe Alex can confirm this point, if not we can remove the test.
Re: [PATCH v8 17/22] s390: vfio-ap: zeroize the AP queues.
On Fri, 10 Aug 2018 12:49:08 +0200
Pierre Morel wrote:
> On 10/08/2018 11:14, Cornelia Huck wrote:
> > On Wed, 8 Aug 2018 10:44:27 -0400
> > Tony Krowiak wrote:
> >
> >> From: Tony Krowiak
> >>
> >> Let's call PAPQ(ZAPQ) to zeroize a queue:
> >>
> >> * For each queue configured for a mediated matrix device
> >>when it is released.
> >>
> >> Zeroizing a queue resets the queue, clears all pending
> >> messages for the queue entries and disables adapter interruptions
> >> associated with the queue.
> >>
> >> Signed-off-by: Tony Krowiak
> >> Reviewed-by: Halil Pasic
> >> Tested-by: Michael Mueller
> >> Tested-by: Farhan Ali
> >> Signed-off-by: Christian Borntraeger
> >> ---
> >> drivers/s390/crypto/vfio_ap_ops.c | 29
> >> -
> >> drivers/s390/crypto/vfio_ap_private.h | 25 +
> >> 2 files changed, 53 insertions(+), 1 deletions(-)
> >>
> >> @@ -788,7 +812,10 @@ static void vfio_ap_mdev_release(struct mdev_device
> >> *mdev)
> >> {
> >>struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
> >>
> >> - kvm_arch_crypto_clear_masks(matrix_mdev->kvm);
> >> + if (matrix_mdev->kvm)
> >> + kvm_arch_crypto_clear_masks(matrix_mdev->kvm);
> > Confused. Why is the check for matrix_mdev->kvm added here?
>
> When using the KVM notifier we can get two notifications:
> -> KVM is here / is comming
> -> KVM is not here / disappearing
>
> In the first case we initialize matrix_mdev->kvm with a pointer to KVM
> In the second case we nullify the pointer.
>
> During the open of the mediated device, the guest should have been started
> or we refuse to start.
>
> During the close of the mediated device, the guest should be there, but
> we have no certitude that the guest did not disappear before the VFIO
> file being closed.
> Since we do not allow multiple guests using the same mediated device
> this case should not happen with QEMU. But I am not sure that
> a rogue user program could not stop KVM before closing the VFIO
> mediated device.
I'm not sure why the check is introduced in this patch, though. But
maybe I just need weekend :)
>
> Maybe Alex can confirm this point, if not we can remove the test.
Re: [PATCH v5 03/14] PM: Introduce an Energy Model management framework
On Friday, August 10, 2018 11:12:18 AM CEST Quentin Perret wrote: > On Friday 10 Aug 2018 at 10:41:56 (+0200), Rafael J. Wysocki wrote: > > On Friday, August 10, 2018 10:15:39 AM CEST Quentin Perret wrote: [cut] > Agreed. EAS and IPA don't care about the absolute real power values, all > they care about is relative correctness. But what I really want to avoid > is having IPA getting the power of the GPUs in mW, and the power of CPUs > in an abstract scale without unit. That _will_ create problems eventually > IMO, because the behaviour is undefined. Specifying a unit everywhere is > an easy way to enforce a consistent design across sub-systems, that's > all. OK > > > What I am currently proposing is to keep the unit (mW) in the EM > > > framework so that migrating IPA to using it can be done in a (relatively) > > > painless way. On a system where drivers don't know the exact wattage, > > > then they should just 'lie' to the EM framework, but it's their job to > > > lie coherently to all subsystems and keep things consistent, because all > > > subsystems have specified power in comparable units. > > > > Alternatively, there could be a translation layer between EM and IPA. > > Hmm, interesting... What do you have in mind exactly ? What would you > put in that layer ? Something able to say how the numbers used by EM and IPA are related. :-) Do you think that IPA and EM will always need to use the same set of data for the CPU? > > From my experience, if you want people to come up with some numbers, > > they will just choose them to game the system this way or another > > unless those numbers can be measured directly or are clearly documented. > > > > And if that happens and then you want to make any significant changes, > > you'll need to deal with "regressions" occuring because someone chose > > the numbers to make the system behave in a specific way and your changes > > break that. > > > > As a rule, I rather avoid requesting unknown numbers from people. :-) > > > > > Another solution to solve this problem could be to extend the EM > > > framework introduced by this patch and make it manage the EM of any > > > device, not just CPUs. Then we could just specify that all power costs > > > must be in the same scale, regardless of the actual unit, and register > > > the EM of CPUs, GPUs, ... > > > However, I was hoping that this patch as-is was enough for a first step, > > > and that this extension of the framework could be done in a second step ? > > > Thoughts ? > > > > > > In any case, if we decide to keep the mW unit for now, I should at least > > > explain clearly why in the commit message. > > > > Right. > > > > Actually, the unit is as good as any other, but you need to bear in mind > > that > > the numbers provided may not be realistic. > > As long as they're all correct in a relative way, that's fine by me :-) OK Thanks, Rafael
Re: [PATCH v5 03/14] PM: Introduce an Energy Model management framework
On Friday, August 10, 2018 11:12:18 AM CEST Quentin Perret wrote: > On Friday 10 Aug 2018 at 10:41:56 (+0200), Rafael J. Wysocki wrote: > > On Friday, August 10, 2018 10:15:39 AM CEST Quentin Perret wrote: [cut] > Agreed. EAS and IPA don't care about the absolute real power values, all > they care about is relative correctness. But what I really want to avoid > is having IPA getting the power of the GPUs in mW, and the power of CPUs > in an abstract scale without unit. That _will_ create problems eventually > IMO, because the behaviour is undefined. Specifying a unit everywhere is > an easy way to enforce a consistent design across sub-systems, that's > all. OK > > > What I am currently proposing is to keep the unit (mW) in the EM > > > framework so that migrating IPA to using it can be done in a (relatively) > > > painless way. On a system where drivers don't know the exact wattage, > > > then they should just 'lie' to the EM framework, but it's their job to > > > lie coherently to all subsystems and keep things consistent, because all > > > subsystems have specified power in comparable units. > > > > Alternatively, there could be a translation layer between EM and IPA. > > Hmm, interesting... What do you have in mind exactly ? What would you > put in that layer ? Something able to say how the numbers used by EM and IPA are related. :-) Do you think that IPA and EM will always need to use the same set of data for the CPU? > > From my experience, if you want people to come up with some numbers, > > they will just choose them to game the system this way or another > > unless those numbers can be measured directly or are clearly documented. > > > > And if that happens and then you want to make any significant changes, > > you'll need to deal with "regressions" occuring because someone chose > > the numbers to make the system behave in a specific way and your changes > > break that. > > > > As a rule, I rather avoid requesting unknown numbers from people. :-) > > > > > Another solution to solve this problem could be to extend the EM > > > framework introduced by this patch and make it manage the EM of any > > > device, not just CPUs. Then we could just specify that all power costs > > > must be in the same scale, regardless of the actual unit, and register > > > the EM of CPUs, GPUs, ... > > > However, I was hoping that this patch as-is was enough for a first step, > > > and that this extension of the framework could be done in a second step ? > > > Thoughts ? > > > > > > In any case, if we decide to keep the mW unit for now, I should at least > > > explain clearly why in the commit message. > > > > Right. > > > > Actually, the unit is as good as any other, but you need to bear in mind > > that > > the numbers provided may not be realistic. > > As long as they're all correct in a relative way, that's fine by me :-) OK Thanks, Rafael
Re: [PATCH v3 0/2] PCI: imx: Initial imx7d pm support
On Wed, 2018-08-08 at 16:27 +0100, Lorenzo Pieralisi wrote: > On Wed, Aug 08, 2018 at 02:58:15PM +, Leonard Crestez wrote: > > On Wed, 2018-08-08 at 15:19 +0100, Lorenzo Pieralisi wrote: > > > On Wed, Aug 08, 2018 at 11:37:14AM +, Leonard Crestez wrote: > > > > On Wed, 2018-08-08 at 12:14 +0100, Lorenzo Pieralisi wrote: > > > > > On Wed, Aug 08, 2018 at 10:53:52AM +, Leonard Crestez wrote: > > > > > > On Tue, 2018-07-24 at 19:14 +0300, Leonard Crestez wrote: > > > > > > > Changes since v2: > > > > > > > * Print with dev_info if link fails on resume (Lucas) > > > > > > > * Add a comment on imx7d pci irq mappings (Andrey) > > > > > > > * Make imx6_pcie_ltssm_disable print an error on IMX6Q (Lucas) > > > > > > > > > > > > > > The ltssm_disable does not return an error because it can't be > > > > > > > usefully > > > > > > > handled, reversing partial suspend is a nightmare and unlikely to > > > > > > > work. > > > > > > > > > > > > > > * Drop "reset: imx7: Fix always writing bits as 0 (accepted by > > > > > > > Philipp) > > > > > > > > > > > > > > Series is against linux-next tag next-20180724 where the reset > > > > > > > patch was > > > > > > > already accepted. The imx7d.dtsi patch is also useful standalone. > > > > > > > > > > > > This is a gentle reminder that this series was reviewed by Lucas two > > > > > > weeks ago but not yet included. > > > > > > > > > > Does this series have a functional dependency on the reset fix ? If > > > > > yes > > > > > we can have a bisection proplem depending on which tree gets merged > > > > > first. > > > > > > > > Yes, without the reset fix I expect hangs. This needs further clarification: without the reset patch this will hang on imx7 suspend/resume but this is the current behavior anyway. Both patches are required for imx7 pci suspend and including them out of order shouldn't break unrelated functionality. So it should actually be fine to just include the pci changes now, right?
Re: [PATCH v3 0/2] PCI: imx: Initial imx7d pm support
On Wed, 2018-08-08 at 16:27 +0100, Lorenzo Pieralisi wrote: > On Wed, Aug 08, 2018 at 02:58:15PM +, Leonard Crestez wrote: > > On Wed, 2018-08-08 at 15:19 +0100, Lorenzo Pieralisi wrote: > > > On Wed, Aug 08, 2018 at 11:37:14AM +, Leonard Crestez wrote: > > > > On Wed, 2018-08-08 at 12:14 +0100, Lorenzo Pieralisi wrote: > > > > > On Wed, Aug 08, 2018 at 10:53:52AM +, Leonard Crestez wrote: > > > > > > On Tue, 2018-07-24 at 19:14 +0300, Leonard Crestez wrote: > > > > > > > Changes since v2: > > > > > > > * Print with dev_info if link fails on resume (Lucas) > > > > > > > * Add a comment on imx7d pci irq mappings (Andrey) > > > > > > > * Make imx6_pcie_ltssm_disable print an error on IMX6Q (Lucas) > > > > > > > > > > > > > > The ltssm_disable does not return an error because it can't be > > > > > > > usefully > > > > > > > handled, reversing partial suspend is a nightmare and unlikely to > > > > > > > work. > > > > > > > > > > > > > > * Drop "reset: imx7: Fix always writing bits as 0 (accepted by > > > > > > > Philipp) > > > > > > > > > > > > > > Series is against linux-next tag next-20180724 where the reset > > > > > > > patch was > > > > > > > already accepted. The imx7d.dtsi patch is also useful standalone. > > > > > > > > > > > > This is a gentle reminder that this series was reviewed by Lucas two > > > > > > weeks ago but not yet included. > > > > > > > > > > Does this series have a functional dependency on the reset fix ? If > > > > > yes > > > > > we can have a bisection proplem depending on which tree gets merged > > > > > first. > > > > > > > > Yes, without the reset fix I expect hangs. This needs further clarification: without the reset patch this will hang on imx7 suspend/resume but this is the current behavior anyway. Both patches are required for imx7 pci suspend and including them out of order shouldn't break unrelated functionality. So it should actually be fine to just include the pci changes now, right?
Re: [PATCH v2] cpuidle: menu: Handle stopped tick more aggressively
On Fri, Aug 10, 2018 at 11:20 AM wrote:
>
> On Fri, Aug 10, 2018 at 09:57:18AM +0200, Rafael J . Wysocki wrote:
> > From: Rafael J. Wysocki
> > Subject: [PATCH] cpuidle: menu: Handle stopped tick more aggressively
> >
> > Commit 87c9fe6ee495 (cpuidle: menu: Avoid selecting shallow states
> > with stopped tick) missed the case when the target residencies of
> > deep idle states of CPUs are above the tick boundary which may cause
> > the CPU to get stuck in a shallow idle state for a long time.
> >
> > Say there are two CPU idle states available: one shallow, with the
> > target residency much below the tick boundary and one deep, with
> > the target residency significantly above the tick boundary. In
> > that case, if the tick has been stopped already and the expected
> > next timer event is relatively far in the future, the governor will
> > assume the idle duration to be equal to TICK_USEC and it will select
> > the idle state for the CPU accordingly. However, that will cause the
> > shallow state to be selected even though it would have been more
> > energy-efficient to select the deep one.
> >
> > To address this issue, modify the governor to always assume idle
> > duration to be equal to the time till the closest timer event if
> > the tick is not running which will cause the selected idle states
> > to always match the known CPU wakeup time.
> >
> > Also make it always indicate that the tick should be stopped in
> > that case for consistency.
> >
> > Fixes: 87c9fe6ee495 (cpuidle: menu: Avoid selecting shallow states with
> > stopped tick)
> > Reported-by: Leo Yan
> > Signed-off-by: Rafael J. Wysocki
> > ---
> >
> > -> v2: Initialize first_idx properly in the stopped tick case.
> >
> > ---
> > drivers/cpuidle/governors/menu.c | 55
> > +--
> > 1 file changed, 25 insertions(+), 30 deletions(-)
> >
> > Index: linux-pm/drivers/cpuidle/governors/menu.c
> > ===
> > --- linux-pm.orig/drivers/cpuidle/governors/menu.c
> > +++ linux-pm/drivers/cpuidle/governors/menu.c
> > @@ -285,9 +285,8 @@ static int menu_select(struct cpuidle_dr
> > {
> > struct menu_device *data = this_cpu_ptr(_devices);
> > int latency_req = cpuidle_governor_latency_req(dev->cpu);
> > - int i;
> > - int first_idx;
> > - int idx;
> > + int first_idx = 0;
> > + int idx, i;
> > unsigned int interactivity_req;
> > unsigned int expected_interval;
> > unsigned long nr_iowaiters, cpu_load;
> > @@ -307,6 +306,18 @@ static int menu_select(struct cpuidle_dr
> > /* determine the expected residency time, round up */
> > data->next_timer_us =
> > ktime_to_us(tick_nohz_get_sleep_length(_next));
> >
> > + /*
> > + * If the tick is already stopped, the cost of possible short idle
> > + * duration misprediction is much higher, because the CPU may be stuck
> > + * in a shallow idle state for a long time as a result of it. In that
> > + * case say we might mispredict and use the known time till the
> > closest
> > + * timer event for the idle state selection.
> > + */
> > + if (tick_nohz_tick_stopped()) {
> > + data->predicted_us = ktime_to_us(delta_next);
> > + goto select;
> > + }
> > +
>
> This introduce two potential issues:
>
> - This will totally ignore the typical pattern in idle loop; I
> observed on the mmc driver can trigger multiple times (> 10 times)
> with consistent interval;
I'm not sure what you mean by "ignore".
> but I have no strong opinion to not use next timer event for this case.
OK
> - Will this break correction factors when the CPU exit from idle?
> data->bucket is stale value
Good point.
I'll send a v3 with this addressed.
>
> > get_iowait_load(_iowaiters, _load);
> > data->bucket = which_bucket(data->next_timer_us, nr_iowaiters);
> >
> > @@ -322,7 +333,6 @@ static int menu_select(struct cpuidle_dr
> > expected_interval = get_typical_interval(data);
> > expected_interval = min(expected_interval, data->next_timer_us);
> >
> > - first_idx = 0;
> > if (drv->states[0].flags & CPUIDLE_FLAG_POLLING) {
> > struct cpuidle_state *s = >states[1];
> > unsigned int polling_threshold;
> > @@ -344,29 +354,15 @@ static int menu_select(struct cpuidle_dr
> >*/
> > data->predicted_us = min(data->predicted_us, expected_interval);
> >
> > - if (tick_nohz_tick_stopped()) {
> > - /*
> > - * If the tick is already stopped, the cost of possible short
> > - * idle duration misprediction is much higher, because the CPU
> > - * may be stuck in a shallow idle state for a long time as a
> > - * result of it. In that case say we might mispredict and try
> > - * to force the CPU into a state for which we would have
> > stopped
> > - *
Re: [PATCH v2] cpuidle: menu: Handle stopped tick more aggressively
On Fri, Aug 10, 2018 at 11:20 AM wrote:
>
> On Fri, Aug 10, 2018 at 09:57:18AM +0200, Rafael J . Wysocki wrote:
> > From: Rafael J. Wysocki
> > Subject: [PATCH] cpuidle: menu: Handle stopped tick more aggressively
> >
> > Commit 87c9fe6ee495 (cpuidle: menu: Avoid selecting shallow states
> > with stopped tick) missed the case when the target residencies of
> > deep idle states of CPUs are above the tick boundary which may cause
> > the CPU to get stuck in a shallow idle state for a long time.
> >
> > Say there are two CPU idle states available: one shallow, with the
> > target residency much below the tick boundary and one deep, with
> > the target residency significantly above the tick boundary. In
> > that case, if the tick has been stopped already and the expected
> > next timer event is relatively far in the future, the governor will
> > assume the idle duration to be equal to TICK_USEC and it will select
> > the idle state for the CPU accordingly. However, that will cause the
> > shallow state to be selected even though it would have been more
> > energy-efficient to select the deep one.
> >
> > To address this issue, modify the governor to always assume idle
> > duration to be equal to the time till the closest timer event if
> > the tick is not running which will cause the selected idle states
> > to always match the known CPU wakeup time.
> >
> > Also make it always indicate that the tick should be stopped in
> > that case for consistency.
> >
> > Fixes: 87c9fe6ee495 (cpuidle: menu: Avoid selecting shallow states with
> > stopped tick)
> > Reported-by: Leo Yan
> > Signed-off-by: Rafael J. Wysocki
> > ---
> >
> > -> v2: Initialize first_idx properly in the stopped tick case.
> >
> > ---
> > drivers/cpuidle/governors/menu.c | 55
> > +--
> > 1 file changed, 25 insertions(+), 30 deletions(-)
> >
> > Index: linux-pm/drivers/cpuidle/governors/menu.c
> > ===
> > --- linux-pm.orig/drivers/cpuidle/governors/menu.c
> > +++ linux-pm/drivers/cpuidle/governors/menu.c
> > @@ -285,9 +285,8 @@ static int menu_select(struct cpuidle_dr
> > {
> > struct menu_device *data = this_cpu_ptr(_devices);
> > int latency_req = cpuidle_governor_latency_req(dev->cpu);
> > - int i;
> > - int first_idx;
> > - int idx;
> > + int first_idx = 0;
> > + int idx, i;
> > unsigned int interactivity_req;
> > unsigned int expected_interval;
> > unsigned long nr_iowaiters, cpu_load;
> > @@ -307,6 +306,18 @@ static int menu_select(struct cpuidle_dr
> > /* determine the expected residency time, round up */
> > data->next_timer_us =
> > ktime_to_us(tick_nohz_get_sleep_length(_next));
> >
> > + /*
> > + * If the tick is already stopped, the cost of possible short idle
> > + * duration misprediction is much higher, because the CPU may be stuck
> > + * in a shallow idle state for a long time as a result of it. In that
> > + * case say we might mispredict and use the known time till the
> > closest
> > + * timer event for the idle state selection.
> > + */
> > + if (tick_nohz_tick_stopped()) {
> > + data->predicted_us = ktime_to_us(delta_next);
> > + goto select;
> > + }
> > +
>
> This introduce two potential issues:
>
> - This will totally ignore the typical pattern in idle loop; I
> observed on the mmc driver can trigger multiple times (> 10 times)
> with consistent interval;
I'm not sure what you mean by "ignore".
> but I have no strong opinion to not use next timer event for this case.
OK
> - Will this break correction factors when the CPU exit from idle?
> data->bucket is stale value
Good point.
I'll send a v3 with this addressed.
>
> > get_iowait_load(_iowaiters, _load);
> > data->bucket = which_bucket(data->next_timer_us, nr_iowaiters);
> >
> > @@ -322,7 +333,6 @@ static int menu_select(struct cpuidle_dr
> > expected_interval = get_typical_interval(data);
> > expected_interval = min(expected_interval, data->next_timer_us);
> >
> > - first_idx = 0;
> > if (drv->states[0].flags & CPUIDLE_FLAG_POLLING) {
> > struct cpuidle_state *s = >states[1];
> > unsigned int polling_threshold;
> > @@ -344,29 +354,15 @@ static int menu_select(struct cpuidle_dr
> >*/
> > data->predicted_us = min(data->predicted_us, expected_interval);
> >
> > - if (tick_nohz_tick_stopped()) {
> > - /*
> > - * If the tick is already stopped, the cost of possible short
> > - * idle duration misprediction is much higher, because the CPU
> > - * may be stuck in a shallow idle state for a long time as a
> > - * result of it. In that case say we might mispredict and try
> > - * to force the CPU into a state for which we would have
> > stopped
> > - *
Re: Droid 4: suspend to RAM?
Hi! > * Pavel Machek [180727 11:35]: > > Hi! > > > > > high even before modem (and thus USB) is enabled. > > > > > > > > > > Interestingly, CyanogenMod and Jolla seem to have higher power > > > > > consumption than stock operating system. > > > > > > > > > > (My Linux can survive for 10 hours, stock system could survive for 4 > > > > > days if I'm not mistaken). > > > > > > > > > > I thought I would experiment with suspend to RAM.. and it indeed > > > > > seemed to suspend ok, but I could not wake it up. Do I need to set up > > > > > wakeup with button somehow? Is suspend to RAM required for good power > > > > > consumption? > > > > > > > > Sorry but pm subsystem has debug mode that you can test in a easy way. > > > > You can even wakeup by any rtc alarm easily. > > > > > > Yes, that is how it works on PC (but there power button works, > > > too). Is it expected to work on Droid in v4.18? > > > > I tried setting up wakeup using RTC, but no, it does not seem to work: > > > > root@devuan:/my/tui/d4# rtcwake -m no -s 5 > > rtcwake: wakeup using /dev/rtc0 at Fri Jul 27 11:28:44 2018 > > root@devuan:/my/tui/d4# echo mem > /sys/power/state > > Works for me here as tested on next-20180808, maybe you don't have > CONFIG_RTC_DRV_CPCAP? Maybe you are trying to use CONFIG_RTC_DRV_OMAP? Mainline seems to fail suspend, with CONFIG_DRM turned off. Aha, and same behaviour with CONFIG_DRM on. Why is it different today? [ 334.933532] Charging, 435 uV, 532000 uA [ 338.093109] PM: suspend entry (deep) [ 338.096710] PM: Syncing filesystems ... done. [ 338.138977] Freezing user space processes ... (elapsed 0.001 seconds) done. [ 338.147338] OOM killer disabled. [ 338.150604] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done. [ 338.159240] Suspending console(s) (use no_console_suspend to debug) [ 338.168212] phy phy-usb-phy@1.1: phy poweroff failed --> -19 [ 338.181518] l4_wkup_cm:clk:0018:0: failed to disable [ 338.183227] PM: noirq suspend of devices failed [ 338.187377] g_ether gadget: reset config [ 338.187408] g_ether gadget: ecm deactivated [ 338.187408] usb0: gether_disconnect [ 338.513092] OOM killer enabled. [ 338.516296] Restarting tasks ... done. [ 338.529266] PM: suspend exit [ 338.698577] g_ether gadget: high-speed config #1: CDC Ethernet (ECM) [ 338.704986] g_ether gadget: init ecm [ 338.708587] g_ether gadget: notify connect true [ 338.713958] g_ether gadget: activate ecm [ 338.717895] usb0: qlen 10 [ 338.720550] g_ether gadget: ecm_open [ 338.724151] usb0: eth_start [ 338.727111] g_ether gadget: packet filter 0c [ 338.731414] g_ether gadget: ecm req21.43 v000c i l0 [ 338.993835] cpcap_usb_detect: 27 callbacks suppressed With USB unplugged, suspend seems to behave as expected, even on v4.18-rc8, and it seems to work even with X running and modem online. Good. > Then for deeper idle modes, you need to also idle UARTs, and unbind or > unload USB related modules. You should get to something like 160mW > power consumption with mdm6600 enabled and SoC suspended that way. > > Then again system running idle is about the same with timers and > interrupts working so I'd just idle UARTs and unload USB modules :) Ok, let me play with it some more. Thanks for help, 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: Droid 4: suspend to RAM?
Hi! > * Pavel Machek [180727 11:35]: > > Hi! > > > > > high even before modem (and thus USB) is enabled. > > > > > > > > > > Interestingly, CyanogenMod and Jolla seem to have higher power > > > > > consumption than stock operating system. > > > > > > > > > > (My Linux can survive for 10 hours, stock system could survive for 4 > > > > > days if I'm not mistaken). > > > > > > > > > > I thought I would experiment with suspend to RAM.. and it indeed > > > > > seemed to suspend ok, but I could not wake it up. Do I need to set up > > > > > wakeup with button somehow? Is suspend to RAM required for good power > > > > > consumption? > > > > > > > > Sorry but pm subsystem has debug mode that you can test in a easy way. > > > > You can even wakeup by any rtc alarm easily. > > > > > > Yes, that is how it works on PC (but there power button works, > > > too). Is it expected to work on Droid in v4.18? > > > > I tried setting up wakeup using RTC, but no, it does not seem to work: > > > > root@devuan:/my/tui/d4# rtcwake -m no -s 5 > > rtcwake: wakeup using /dev/rtc0 at Fri Jul 27 11:28:44 2018 > > root@devuan:/my/tui/d4# echo mem > /sys/power/state > > Works for me here as tested on next-20180808, maybe you don't have > CONFIG_RTC_DRV_CPCAP? Maybe you are trying to use CONFIG_RTC_DRV_OMAP? Mainline seems to fail suspend, with CONFIG_DRM turned off. Aha, and same behaviour with CONFIG_DRM on. Why is it different today? [ 334.933532] Charging, 435 uV, 532000 uA [ 338.093109] PM: suspend entry (deep) [ 338.096710] PM: Syncing filesystems ... done. [ 338.138977] Freezing user space processes ... (elapsed 0.001 seconds) done. [ 338.147338] OOM killer disabled. [ 338.150604] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done. [ 338.159240] Suspending console(s) (use no_console_suspend to debug) [ 338.168212] phy phy-usb-phy@1.1: phy poweroff failed --> -19 [ 338.181518] l4_wkup_cm:clk:0018:0: failed to disable [ 338.183227] PM: noirq suspend of devices failed [ 338.187377] g_ether gadget: reset config [ 338.187408] g_ether gadget: ecm deactivated [ 338.187408] usb0: gether_disconnect [ 338.513092] OOM killer enabled. [ 338.516296] Restarting tasks ... done. [ 338.529266] PM: suspend exit [ 338.698577] g_ether gadget: high-speed config #1: CDC Ethernet (ECM) [ 338.704986] g_ether gadget: init ecm [ 338.708587] g_ether gadget: notify connect true [ 338.713958] g_ether gadget: activate ecm [ 338.717895] usb0: qlen 10 [ 338.720550] g_ether gadget: ecm_open [ 338.724151] usb0: eth_start [ 338.727111] g_ether gadget: packet filter 0c [ 338.731414] g_ether gadget: ecm req21.43 v000c i l0 [ 338.993835] cpcap_usb_detect: 27 callbacks suppressed With USB unplugged, suspend seems to behave as expected, even on v4.18-rc8, and it seems to work even with X running and modem online. Good. > Then for deeper idle modes, you need to also idle UARTs, and unbind or > unload USB related modules. You should get to something like 160mW > power consumption with mdm6600 enabled and SoC suspended that way. > > Then again system running idle is about the same with timers and > interrupts working so I'd just idle UARTs and unload USB modules :) Ok, let me play with it some more. Thanks for help, 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
[PULL REQUEST] i2c for 4.18
Linus, here is a driver bugfix for I2C. The bug was found by systematically stress testing the driver, so I am confident to merge it that late in the cycle although it is probably unusually large. Please pull. Thanks, Wolfram The following changes since commit 1ffaddd029c867d134a1dde39f540dcc8c52e274: Linux 4.18-rc8 (2018-08-05 12:37:41 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-current for you to fetch changes up to 5eb173f5c8f3a3cdc47b3952c368f10a28c81ab8: i2c: xlp9xx: Fix case where SSIF read transaction completes early (2018-08-09 17:41:13 +0200) George Cherian (1): i2c: xlp9xx: Fix case where SSIF read transaction completes early drivers/i2c/busses/i2c-xlp9xx.c | 41 - 1 file changed, 28 insertions(+), 13 deletions(-) signature.asc Description: PGP signature
[PULL REQUEST] i2c for 4.18
Linus, here is a driver bugfix for I2C. The bug was found by systematically stress testing the driver, so I am confident to merge it that late in the cycle although it is probably unusually large. Please pull. Thanks, Wolfram The following changes since commit 1ffaddd029c867d134a1dde39f540dcc8c52e274: Linux 4.18-rc8 (2018-08-05 12:37:41 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-current for you to fetch changes up to 5eb173f5c8f3a3cdc47b3952c368f10a28c81ab8: i2c: xlp9xx: Fix case where SSIF read transaction completes early (2018-08-09 17:41:13 +0200) George Cherian (1): i2c: xlp9xx: Fix case where SSIF read transaction completes early drivers/i2c/busses/i2c-xlp9xx.c | 41 - 1 file changed, 28 insertions(+), 13 deletions(-) signature.asc Description: PGP signature
Re: [PATCH] spi: spi-geni-qcom: Add SPI driver support for GENI based QUP
On Thu, Aug 09, 2018 at 11:03:55AM -0700, Doug Anderson wrote: > On Fri, Aug 3, 2018 at 5:18 AM, wrote: > > Also, spi core framework will set the transfer speed to controller max > > frequency > > if transfer frequency is greater than controller max frequency. > > Please mention if you have a other opinion. > 1. It sure seems like the clock framework could be enforcing the max > speed here. SPI can just ask for the speed and the clock framework > will pick the highest speed it can if you ask for one too high. Isn't > that the whole point of the "struct freq_tbl" in the clock driver? This is more about matching the data rate between the two drivers - the clock framework could (and possibly should) reasonably return an error here, we're trying to ensure that drivers and controllers work well together here. > 2. The device tree writer already provides a max clock speed for each > SPI slave in the device tree. ...shouldn't the device tree writer > already be taking into account the max of the SPI port when setting > this value? Yes. We're overriding this because drivers can set a speed from code (this is especially common when devices have variable maximum speeds for different operations). > 3. If you really truly need code in the SPI driver then make sure you > include a compatible string for the SoC and have a table in the driver > that's found with of_device_get_match_data(). AKA: > compatible = "qcom,geni-spi-sdm845", "qcom,geni-spi"; A controller driver really shouldn't need to be open coding anything. signature.asc Description: PGP signature
Re: [PATCH] spi: spi-geni-qcom: Add SPI driver support for GENI based QUP
On Thu, Aug 09, 2018 at 11:03:55AM -0700, Doug Anderson wrote: > On Fri, Aug 3, 2018 at 5:18 AM, wrote: > > Also, spi core framework will set the transfer speed to controller max > > frequency > > if transfer frequency is greater than controller max frequency. > > Please mention if you have a other opinion. > 1. It sure seems like the clock framework could be enforcing the max > speed here. SPI can just ask for the speed and the clock framework > will pick the highest speed it can if you ask for one too high. Isn't > that the whole point of the "struct freq_tbl" in the clock driver? This is more about matching the data rate between the two drivers - the clock framework could (and possibly should) reasonably return an error here, we're trying to ensure that drivers and controllers work well together here. > 2. The device tree writer already provides a max clock speed for each > SPI slave in the device tree. ...shouldn't the device tree writer > already be taking into account the max of the SPI port when setting > this value? Yes. We're overriding this because drivers can set a speed from code (this is especially common when devices have variable maximum speeds for different operations). > 3. If you really truly need code in the SPI driver then make sure you > include a compatible string for the SoC and have a table in the driver > that's found with of_device_get_match_data(). AKA: > compatible = "qcom,geni-spi-sdm845", "qcom,geni-spi"; A controller driver really shouldn't need to be open coding anything. signature.asc Description: PGP signature
[PATCH v3 1/5] staging: rtl8188eu: use is_multicast_ether_addr in rtw_security.c
Use is_multicast_ether_addr instead of custom IS_MCAST in
core/rtw_security.c. In all uses the address array is properly
aligned.
Signed-off-by: Michael Straube
Reviewed-by: Dan Carpenter
---
v2: checked that in all uses of is_multicast_ether_addr
the address array/memory is properly aligned and
updated the commit messages
v3: added "Reviewed-by: Dan Carpenter "
drivers/staging/rtl8188eu/core/rtw_security.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_security.c
b/drivers/staging/rtl8188eu/core/rtw_security.c
index 2a48b09ea9ae..a2ec0e403718 100644
--- a/drivers/staging/rtl8188eu/core/rtw_security.c
+++ b/drivers/staging/rtl8188eu/core/rtw_security.c
@@ -608,7 +608,7 @@ u32 rtw_tkip_encrypt(struct adapter *padapter, u8
*pxmitframe)
if (stainfo != NULL) {
RT_TRACE(_module_rtl871x_security_c_, _drv_err_, ("%s:
stainfo!= NULL!!!\n", __func__));
- if (IS_MCAST(pattrib->ra))
+ if (is_multicast_ether_addr(pattrib->ra))
prwskey =
psecuritypriv->dot118021XGrpKey[psecuritypriv->dot118021XGrpKeyid].skey;
else
prwskey = >dot118021x_UncstKey.skey[0];
@@ -678,7 +678,7 @@ u32 rtw_tkip_decrypt(struct adapter *padapter, u8
*precvframe)
if (prxattrib->encrypt == _TKIP_) {
stainfo = rtw_get_stainfo(>stapriv,
>ta[0]);
if (stainfo) {
- if (IS_MCAST(prxattrib->ra)) {
+ if (is_multicast_ether_addr(prxattrib->ra)) {
if (!psecuritypriv->binstallGrpkey) {
res = _FAIL;
DBG_88E("%s:rx bc/mc packets, but
didn't install group key!!\n", __func__);
@@ -1250,7 +1250,7 @@ u32 rtw_aes_encrypt(struct adapter *padapter, u8
*pxmitframe)
if (stainfo) {
RT_TRACE(_module_rtl871x_security_c_, _drv_err_, ("%s:
stainfo!= NULL!!!\n", __func__));
- if (IS_MCAST(pattrib->ra))
+ if (is_multicast_ether_addr(pattrib->ra))
prwskey =
psecuritypriv->dot118021XGrpKey[psecuritypriv->dot118021XGrpKeyid].skey;
else
prwskey = >dot118021x_UncstKey.skey[0];
@@ -1296,7 +1296,7 @@ u32 rtw_aes_decrypt(struct adapter *padapter, u8
*precvframe)
struct security_priv *psecuritypriv =
>securitypriv;
char iv[8], icv[8];
- if (IS_MCAST(prxattrib->ra)) {
+ if (is_multicast_ether_addr(prxattrib->ra)) {
/* in concurrent we should use sw descrypt in
group key, so we remove this message */
if (!psecuritypriv->binstallGrpkey) {
res = _FAIL;
--
2.18.0
[PATCH v3 4/5] staging: rtl8188eu: remove unused IS_MCAST
Remove the now unused IS_MCAST.
Signed-off-by: Michael Straube
Reviewed-by: Dan Carpenter
---
drivers/staging/rtl8188eu/include/wifi.h | 8
1 file changed, 8 deletions(-)
diff --git a/drivers/staging/rtl8188eu/include/wifi.h
b/drivers/staging/rtl8188eu/include/wifi.h
index 4a56e54e38f6..0a952edf8a81 100644
--- a/drivers/staging/rtl8188eu/include/wifi.h
+++ b/drivers/staging/rtl8188eu/include/wifi.h
@@ -257,14 +257,6 @@ enum WIFI_REG_DOMAIN {
#define GetAddr4Ptr(pbuf) ((unsigned char *)((size_t)(pbuf) + 24))
-static inline int IS_MCAST(unsigned char *da)
-{
- if ((*da) & 0x01)
- return true;
- else
- return false;
-}
-
static inline unsigned char *get_da(unsigned char *pframe)
{
unsigned char *da;
--
2.18.0
[PATCH v3 2/5] staging: rtl8188eu: use is_multicast_ether_addr in rtw_recv.c
Use is_multicast_ether_addr instead of custom IS_MCAST in
core/rtw_recv.c. In all uses the address array/memory is
properly aligned.
Signed-off-by: Michael Straube
Reviewed-by: Dan Carpenter
---
drivers/staging/rtl8188eu/core/rtw_recv.c | 35 ---
1 file changed, 18 insertions(+), 17 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_recv.c
b/drivers/staging/rtl8188eu/core/rtw_recv.c
index 17b4b9257b49..ab9638d618a9 100644
--- a/drivers/staging/rtl8188eu/core/rtw_recv.c
+++ b/drivers/staging/rtl8188eu/core/rtw_recv.c
@@ -233,7 +233,7 @@ static int recvframe_chkmic(struct adapter *adapter,
/* calculate mic code */
if (stainfo) {
- if (IS_MCAST(prxattrib->ra)) {
+ if (is_multicast_ether_addr(prxattrib->ra)) {
if (!psecuritypriv) {
res = _FAIL;
RT_TRACE(_module_rtl871x_recv_c_,
_drv_err_,
@@ -321,11 +321,11 @@ static int recvframe_chkmic(struct adapter *adapter,
/* double check key_index for some timing
issue , */
/* cannot compare with
psecuritypriv->dot118021XGrpKeyid also cause timing issue */
- if ((IS_MCAST(prxattrib->ra) == true) &&
(prxattrib->key_index != pmlmeinfo->key_index))
+ if (is_multicast_ether_addr(prxattrib->ra) &&
prxattrib->key_index != pmlmeinfo->key_index)
brpt_micerror = false;
if ((prxattrib->bdecrypted) && (brpt_micerror))
{
- rtw_handle_tkip_mic_err(adapter,
(u8)IS_MCAST(prxattrib->ra));
+ rtw_handle_tkip_mic_err(adapter,
(u8)is_multicast_ether_addr(prxattrib->ra));
RT_TRACE(_module_rtl871x_recv_c_,
_drv_err_, (" mic error :prxattrib->bdecrypted=%d ", prxattrib->bdecrypted));
DBG_88E(" mic error
:prxattrib->bdecrypted=%d\n", prxattrib->bdecrypted);
} else {
@@ -335,7 +335,7 @@ static int recvframe_chkmic(struct adapter *adapter,
res = _FAIL;
} else {
/* mic checked ok */
- if ((!psecuritypriv->bcheck_grpkey) &&
(IS_MCAST(prxattrib->ra))) {
+ if (!psecuritypriv->bcheck_grpkey &&
is_multicast_ether_addr(prxattrib->ra)) {
psecuritypriv->bcheck_grpkey = true;
RT_TRACE(_module_rtl871x_recv_c_,
_drv_err_, ("psecuritypriv->bcheck_grpkey = true"));
}
@@ -648,7 +648,7 @@ int sta2sta_data_frame(struct adapter *adapter, struct
recv_frame *precv_frame,
u8 *mybssid = get_bssid(pmlmepriv);
u8 *myhwaddr = myid(>eeprompriv);
u8 *sta_addr = NULL;
- int bmcast = IS_MCAST(pattrib->dst);
+ bool mcast = is_multicast_ether_addr(pattrib->dst);
if ((check_fwstate(pmlmepriv, WIFI_ADHOC_STATE) == true) ||
(check_fwstate(pmlmepriv, WIFI_ADHOC_MASTER_STATE) == true)) {
@@ -659,7 +659,7 @@ int sta2sta_data_frame(struct adapter *adapter, struct
recv_frame *precv_frame,
goto exit;
}
- if ((memcmp(myhwaddr, pattrib->dst, ETH_ALEN)) && (!bmcast)) {
+ if (memcmp(myhwaddr, pattrib->dst, ETH_ALEN) && !mcast) {
ret = _FAIL;
goto exit;
}
@@ -681,9 +681,9 @@ int sta2sta_data_frame(struct adapter *adapter, struct
recv_frame *precv_frame,
}
sta_addr = pattrib->bssid;
} else if (check_fwstate(pmlmepriv, WIFI_AP_STATE)) {
- if (bmcast) {
+ if (mcast) {
/* For AP mode, if DA == MCAST, then BSSID should be
also MCAST */
- if (!IS_MCAST(pattrib->bssid)) {
+ if (!is_multicast_ether_addr(pattrib->bssid)) {
ret = _FAIL;
goto exit;
}
@@ -700,7 +700,7 @@ int sta2sta_data_frame(struct adapter *adapter, struct
recv_frame *precv_frame,
ret = _FAIL;
}
- if (bmcast)
+ if (mcast)
*psta = rtw_get_bcmc_stainfo(adapter);
else
*psta = rtw_get_stainfo(pstapriv, sta_addr); /* get ap_info */
@@ -727,7 +727,7 @@ static int ap2sta_data_frame(
struct mlme_priv *pmlmepriv = >mlmepriv;
u8 *mybssid = get_bssid(pmlmepriv);
u8 *myhwaddr = myid(>eeprompriv);
- int bmcast = IS_MCAST(pattrib->dst);
+ bool mcast =
[PATCH v3 1/5] staging: rtl8188eu: use is_multicast_ether_addr in rtw_security.c
Use is_multicast_ether_addr instead of custom IS_MCAST in
core/rtw_security.c. In all uses the address array is properly
aligned.
Signed-off-by: Michael Straube
Reviewed-by: Dan Carpenter
---
v2: checked that in all uses of is_multicast_ether_addr
the address array/memory is properly aligned and
updated the commit messages
v3: added "Reviewed-by: Dan Carpenter "
drivers/staging/rtl8188eu/core/rtw_security.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_security.c
b/drivers/staging/rtl8188eu/core/rtw_security.c
index 2a48b09ea9ae..a2ec0e403718 100644
--- a/drivers/staging/rtl8188eu/core/rtw_security.c
+++ b/drivers/staging/rtl8188eu/core/rtw_security.c
@@ -608,7 +608,7 @@ u32 rtw_tkip_encrypt(struct adapter *padapter, u8
*pxmitframe)
if (stainfo != NULL) {
RT_TRACE(_module_rtl871x_security_c_, _drv_err_, ("%s:
stainfo!= NULL!!!\n", __func__));
- if (IS_MCAST(pattrib->ra))
+ if (is_multicast_ether_addr(pattrib->ra))
prwskey =
psecuritypriv->dot118021XGrpKey[psecuritypriv->dot118021XGrpKeyid].skey;
else
prwskey = >dot118021x_UncstKey.skey[0];
@@ -678,7 +678,7 @@ u32 rtw_tkip_decrypt(struct adapter *padapter, u8
*precvframe)
if (prxattrib->encrypt == _TKIP_) {
stainfo = rtw_get_stainfo(>stapriv,
>ta[0]);
if (stainfo) {
- if (IS_MCAST(prxattrib->ra)) {
+ if (is_multicast_ether_addr(prxattrib->ra)) {
if (!psecuritypriv->binstallGrpkey) {
res = _FAIL;
DBG_88E("%s:rx bc/mc packets, but
didn't install group key!!\n", __func__);
@@ -1250,7 +1250,7 @@ u32 rtw_aes_encrypt(struct adapter *padapter, u8
*pxmitframe)
if (stainfo) {
RT_TRACE(_module_rtl871x_security_c_, _drv_err_, ("%s:
stainfo!= NULL!!!\n", __func__));
- if (IS_MCAST(pattrib->ra))
+ if (is_multicast_ether_addr(pattrib->ra))
prwskey =
psecuritypriv->dot118021XGrpKey[psecuritypriv->dot118021XGrpKeyid].skey;
else
prwskey = >dot118021x_UncstKey.skey[0];
@@ -1296,7 +1296,7 @@ u32 rtw_aes_decrypt(struct adapter *padapter, u8
*precvframe)
struct security_priv *psecuritypriv =
>securitypriv;
char iv[8], icv[8];
- if (IS_MCAST(prxattrib->ra)) {
+ if (is_multicast_ether_addr(prxattrib->ra)) {
/* in concurrent we should use sw descrypt in
group key, so we remove this message */
if (!psecuritypriv->binstallGrpkey) {
res = _FAIL;
--
2.18.0
[PATCH v3 4/5] staging: rtl8188eu: remove unused IS_MCAST
Remove the now unused IS_MCAST.
Signed-off-by: Michael Straube
Reviewed-by: Dan Carpenter
---
drivers/staging/rtl8188eu/include/wifi.h | 8
1 file changed, 8 deletions(-)
diff --git a/drivers/staging/rtl8188eu/include/wifi.h
b/drivers/staging/rtl8188eu/include/wifi.h
index 4a56e54e38f6..0a952edf8a81 100644
--- a/drivers/staging/rtl8188eu/include/wifi.h
+++ b/drivers/staging/rtl8188eu/include/wifi.h
@@ -257,14 +257,6 @@ enum WIFI_REG_DOMAIN {
#define GetAddr4Ptr(pbuf) ((unsigned char *)((size_t)(pbuf) + 24))
-static inline int IS_MCAST(unsigned char *da)
-{
- if ((*da) & 0x01)
- return true;
- else
- return false;
-}
-
static inline unsigned char *get_da(unsigned char *pframe)
{
unsigned char *da;
--
2.18.0
[PATCH v3 2/5] staging: rtl8188eu: use is_multicast_ether_addr in rtw_recv.c
Use is_multicast_ether_addr instead of custom IS_MCAST in
core/rtw_recv.c. In all uses the address array/memory is
properly aligned.
Signed-off-by: Michael Straube
Reviewed-by: Dan Carpenter
---
drivers/staging/rtl8188eu/core/rtw_recv.c | 35 ---
1 file changed, 18 insertions(+), 17 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_recv.c
b/drivers/staging/rtl8188eu/core/rtw_recv.c
index 17b4b9257b49..ab9638d618a9 100644
--- a/drivers/staging/rtl8188eu/core/rtw_recv.c
+++ b/drivers/staging/rtl8188eu/core/rtw_recv.c
@@ -233,7 +233,7 @@ static int recvframe_chkmic(struct adapter *adapter,
/* calculate mic code */
if (stainfo) {
- if (IS_MCAST(prxattrib->ra)) {
+ if (is_multicast_ether_addr(prxattrib->ra)) {
if (!psecuritypriv) {
res = _FAIL;
RT_TRACE(_module_rtl871x_recv_c_,
_drv_err_,
@@ -321,11 +321,11 @@ static int recvframe_chkmic(struct adapter *adapter,
/* double check key_index for some timing
issue , */
/* cannot compare with
psecuritypriv->dot118021XGrpKeyid also cause timing issue */
- if ((IS_MCAST(prxattrib->ra) == true) &&
(prxattrib->key_index != pmlmeinfo->key_index))
+ if (is_multicast_ether_addr(prxattrib->ra) &&
prxattrib->key_index != pmlmeinfo->key_index)
brpt_micerror = false;
if ((prxattrib->bdecrypted) && (brpt_micerror))
{
- rtw_handle_tkip_mic_err(adapter,
(u8)IS_MCAST(prxattrib->ra));
+ rtw_handle_tkip_mic_err(adapter,
(u8)is_multicast_ether_addr(prxattrib->ra));
RT_TRACE(_module_rtl871x_recv_c_,
_drv_err_, (" mic error :prxattrib->bdecrypted=%d ", prxattrib->bdecrypted));
DBG_88E(" mic error
:prxattrib->bdecrypted=%d\n", prxattrib->bdecrypted);
} else {
@@ -335,7 +335,7 @@ static int recvframe_chkmic(struct adapter *adapter,
res = _FAIL;
} else {
/* mic checked ok */
- if ((!psecuritypriv->bcheck_grpkey) &&
(IS_MCAST(prxattrib->ra))) {
+ if (!psecuritypriv->bcheck_grpkey &&
is_multicast_ether_addr(prxattrib->ra)) {
psecuritypriv->bcheck_grpkey = true;
RT_TRACE(_module_rtl871x_recv_c_,
_drv_err_, ("psecuritypriv->bcheck_grpkey = true"));
}
@@ -648,7 +648,7 @@ int sta2sta_data_frame(struct adapter *adapter, struct
recv_frame *precv_frame,
u8 *mybssid = get_bssid(pmlmepriv);
u8 *myhwaddr = myid(>eeprompriv);
u8 *sta_addr = NULL;
- int bmcast = IS_MCAST(pattrib->dst);
+ bool mcast = is_multicast_ether_addr(pattrib->dst);
if ((check_fwstate(pmlmepriv, WIFI_ADHOC_STATE) == true) ||
(check_fwstate(pmlmepriv, WIFI_ADHOC_MASTER_STATE) == true)) {
@@ -659,7 +659,7 @@ int sta2sta_data_frame(struct adapter *adapter, struct
recv_frame *precv_frame,
goto exit;
}
- if ((memcmp(myhwaddr, pattrib->dst, ETH_ALEN)) && (!bmcast)) {
+ if (memcmp(myhwaddr, pattrib->dst, ETH_ALEN) && !mcast) {
ret = _FAIL;
goto exit;
}
@@ -681,9 +681,9 @@ int sta2sta_data_frame(struct adapter *adapter, struct
recv_frame *precv_frame,
}
sta_addr = pattrib->bssid;
} else if (check_fwstate(pmlmepriv, WIFI_AP_STATE)) {
- if (bmcast) {
+ if (mcast) {
/* For AP mode, if DA == MCAST, then BSSID should be
also MCAST */
- if (!IS_MCAST(pattrib->bssid)) {
+ if (!is_multicast_ether_addr(pattrib->bssid)) {
ret = _FAIL;
goto exit;
}
@@ -700,7 +700,7 @@ int sta2sta_data_frame(struct adapter *adapter, struct
recv_frame *precv_frame,
ret = _FAIL;
}
- if (bmcast)
+ if (mcast)
*psta = rtw_get_bcmc_stainfo(adapter);
else
*psta = rtw_get_stainfo(pstapriv, sta_addr); /* get ap_info */
@@ -727,7 +727,7 @@ static int ap2sta_data_frame(
struct mlme_priv *pmlmepriv = >mlmepriv;
u8 *mybssid = get_bssid(pmlmepriv);
u8 *myhwaddr = myid(>eeprompriv);
- int bmcast = IS_MCAST(pattrib->dst);
+ bool mcast =
[PATCH v3 5/5] staging: rtl8188eu: use phydm_reg.h from rtlwifi
Use rtlwifi/phydm/phydm_reg.h instead of odm_reg.h and remove the now unused odm_reg.h. All defines from odm_reg.h are defined with the same values in rtlwifi/phydm/phydm_reg.h. Signed-off-by: Michael Straube Reviewed-by: Dan Carpenter --- .../staging/rtl8188eu/include/odm_precomp.h | 2 +- drivers/staging/rtl8188eu/include/odm_reg.h | 106 -- 2 files changed, 1 insertion(+), 107 deletions(-) delete mode 100644 drivers/staging/rtl8188eu/include/odm_reg.h diff --git a/drivers/staging/rtl8188eu/include/odm_precomp.h b/drivers/staging/rtl8188eu/include/odm_precomp.h index 658a938df4c1..0cf7f82b805f 100644 --- a/drivers/staging/rtl8188eu/include/odm_precomp.h +++ b/drivers/staging/rtl8188eu/include/odm_precomp.h @@ -29,7 +29,7 @@ #include "hal8188e_rate_adaptive.h" /* for RA,Power training */ #include "rtl8188e_hal.h" -#include "odm_reg.h" +#include "../../rtlwifi/phydm/phydm_reg.h" #include "odm_rtl8188e.h" diff --git a/drivers/staging/rtl8188eu/include/odm_reg.h b/drivers/staging/rtl8188eu/include/odm_reg.h deleted file mode 100644 index b56549ba1256.. --- a/drivers/staging/rtl8188eu/include/odm_reg.h +++ /dev/null @@ -1,106 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0 */ -/** - * - * Copyright(c) 2007 - 2011 Realtek Corporation. All rights reserved. - * - **/ -/* */ -/* File Name: odm_reg.h */ -/* */ -/* Description: */ -/* */ -/* This file is for general register definition. */ -/* */ -/* */ -/* */ -#ifndef__HAL_ODM_REG_H__ -#define __HAL_ODM_REG_H__ - -/* */ -/* Register Definition */ -/* */ - -/* MAC REG */ -#defineODM_BB_RESET0x002 -#defineODM_DUMMY 0x4fe -#defineODM_EDCA_VO_PARAM 0x500 -#defineODM_EDCA_VI_PARAM 0x504 -#defineODM_EDCA_BE_PARAM 0x508 -#defineODM_EDCA_BK_PARAM 0x50C -#defineODM_TXPAUSE 0x522 - -/* BB REG */ -#defineODM_FPGA_PHY0_PAGE8 0x800 -#defineODM_PSD_SETTING 0x808 -#defineODM_AFE_SETTING 0x818 -#defineODM_TXAGC_B_6_180x830 -#defineODM_TXAGC_B_24_54 0x834 -#defineODM_TXAGC_B_MCS32_5 0x838 -#defineODM_TXAGC_B_MCS0_MCS3 0x83c -#defineODM_TXAGC_B_MCS4_MCS7 0x848 -#defineODM_TXAGC_B_MCS8_MCS11 0x84c -#defineODM_ANALOG_REGISTER 0x85c -#defineODM_RF_INTERFACE_OUTPUT 0x860 -#defineODM_TXAGC_B_MCS12_MCS15 0x868 -#defineODM_TXAGC_B_11_A_2_11 0x86c -#defineODM_AD_DA_LSB_MASK 0x874 -#defineODM_ENABLE_3_WIRE 0x88c -#defineODM_PSD_REPORT 0x8b4 -#defineODM_R_ANT_SELECT0x90c -#defineODM_CCK_ANT_SELECT 0xa07 -#defineODM_CCK_PD_THRESH 0xa0a -#defineODM_CCK_RF_REG1 0xa11 -#defineODM_CCK_MATCH_FILTER0xa20 -#defineODM_CCK_RAKE_MAC0xa2e -#defineODM_CCK_CNT_RESET 0xa2d -#defineODM_CCK_TX_DIVERSITY0xa2f -#defineODM_CCK_FA_CNT_MSB 0xa5b -#defineODM_CCK_FA_CNT_LSB 0xa5c -#defineODM_CCK_NEW_FUNCTION0xa75 -#defineODM_OFDM_PHY0_PAGE_C0xc00 -#defineODM_OFDM_RX_ANT 0xc04 -#defineODM_R_A_RXIQI 0xc14 -#defineODM_R_A_AGC_CORE1 0xc50 -#defineODM_R_A_AGC_CORE2 0xc54 -#defineODM_R_B_AGC_CORE1 0xc58 -#defineODM_R_AGC_PAR 0xc70 -#defineODM_R_HTSTF_AGC_PAR 0xc7c -#defineODM_TX_PWR_TRAINING_A 0xc90 -#defineODM_TX_PWR_TRAINING_B 0xc98 -#defineODM_OFDM_FA_CNT10xcf0 -#defineODM_OFDM_PHY0_PAGE_D0xd00 -#defineODM_OFDM_FA_CNT20xda0 -#defineODM_OFDM_FA_CNT30xda4 -#defineODM_OFDM_FA_CNT40xda8 -#defineODM_TXAGC_A_6_180xe00 -#defineODM_TXAGC_A_24_54 0xe04 -#defineODM_TXAGC_A_1_MCS32
[PATCH v3 5/5] staging: rtl8188eu: use phydm_reg.h from rtlwifi
Use rtlwifi/phydm/phydm_reg.h instead of odm_reg.h and remove the now unused odm_reg.h. All defines from odm_reg.h are defined with the same values in rtlwifi/phydm/phydm_reg.h. Signed-off-by: Michael Straube Reviewed-by: Dan Carpenter --- .../staging/rtl8188eu/include/odm_precomp.h | 2 +- drivers/staging/rtl8188eu/include/odm_reg.h | 106 -- 2 files changed, 1 insertion(+), 107 deletions(-) delete mode 100644 drivers/staging/rtl8188eu/include/odm_reg.h diff --git a/drivers/staging/rtl8188eu/include/odm_precomp.h b/drivers/staging/rtl8188eu/include/odm_precomp.h index 658a938df4c1..0cf7f82b805f 100644 --- a/drivers/staging/rtl8188eu/include/odm_precomp.h +++ b/drivers/staging/rtl8188eu/include/odm_precomp.h @@ -29,7 +29,7 @@ #include "hal8188e_rate_adaptive.h" /* for RA,Power training */ #include "rtl8188e_hal.h" -#include "odm_reg.h" +#include "../../rtlwifi/phydm/phydm_reg.h" #include "odm_rtl8188e.h" diff --git a/drivers/staging/rtl8188eu/include/odm_reg.h b/drivers/staging/rtl8188eu/include/odm_reg.h deleted file mode 100644 index b56549ba1256.. --- a/drivers/staging/rtl8188eu/include/odm_reg.h +++ /dev/null @@ -1,106 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0 */ -/** - * - * Copyright(c) 2007 - 2011 Realtek Corporation. All rights reserved. - * - **/ -/* */ -/* File Name: odm_reg.h */ -/* */ -/* Description: */ -/* */ -/* This file is for general register definition. */ -/* */ -/* */ -/* */ -#ifndef__HAL_ODM_REG_H__ -#define __HAL_ODM_REG_H__ - -/* */ -/* Register Definition */ -/* */ - -/* MAC REG */ -#defineODM_BB_RESET0x002 -#defineODM_DUMMY 0x4fe -#defineODM_EDCA_VO_PARAM 0x500 -#defineODM_EDCA_VI_PARAM 0x504 -#defineODM_EDCA_BE_PARAM 0x508 -#defineODM_EDCA_BK_PARAM 0x50C -#defineODM_TXPAUSE 0x522 - -/* BB REG */ -#defineODM_FPGA_PHY0_PAGE8 0x800 -#defineODM_PSD_SETTING 0x808 -#defineODM_AFE_SETTING 0x818 -#defineODM_TXAGC_B_6_180x830 -#defineODM_TXAGC_B_24_54 0x834 -#defineODM_TXAGC_B_MCS32_5 0x838 -#defineODM_TXAGC_B_MCS0_MCS3 0x83c -#defineODM_TXAGC_B_MCS4_MCS7 0x848 -#defineODM_TXAGC_B_MCS8_MCS11 0x84c -#defineODM_ANALOG_REGISTER 0x85c -#defineODM_RF_INTERFACE_OUTPUT 0x860 -#defineODM_TXAGC_B_MCS12_MCS15 0x868 -#defineODM_TXAGC_B_11_A_2_11 0x86c -#defineODM_AD_DA_LSB_MASK 0x874 -#defineODM_ENABLE_3_WIRE 0x88c -#defineODM_PSD_REPORT 0x8b4 -#defineODM_R_ANT_SELECT0x90c -#defineODM_CCK_ANT_SELECT 0xa07 -#defineODM_CCK_PD_THRESH 0xa0a -#defineODM_CCK_RF_REG1 0xa11 -#defineODM_CCK_MATCH_FILTER0xa20 -#defineODM_CCK_RAKE_MAC0xa2e -#defineODM_CCK_CNT_RESET 0xa2d -#defineODM_CCK_TX_DIVERSITY0xa2f -#defineODM_CCK_FA_CNT_MSB 0xa5b -#defineODM_CCK_FA_CNT_LSB 0xa5c -#defineODM_CCK_NEW_FUNCTION0xa75 -#defineODM_OFDM_PHY0_PAGE_C0xc00 -#defineODM_OFDM_RX_ANT 0xc04 -#defineODM_R_A_RXIQI 0xc14 -#defineODM_R_A_AGC_CORE1 0xc50 -#defineODM_R_A_AGC_CORE2 0xc54 -#defineODM_R_B_AGC_CORE1 0xc58 -#defineODM_R_AGC_PAR 0xc70 -#defineODM_R_HTSTF_AGC_PAR 0xc7c -#defineODM_TX_PWR_TRAINING_A 0xc90 -#defineODM_TX_PWR_TRAINING_B 0xc98 -#defineODM_OFDM_FA_CNT10xcf0 -#defineODM_OFDM_PHY0_PAGE_D0xd00 -#defineODM_OFDM_FA_CNT20xda0 -#defineODM_OFDM_FA_CNT30xda4 -#defineODM_OFDM_FA_CNT40xda8 -#defineODM_TXAGC_A_6_180xe00 -#defineODM_TXAGC_A_24_54 0xe04 -#defineODM_TXAGC_A_1_MCS32
[PATCH v3 3/5] staging: rtl8188eu: use is_multicast_ether_addr in rtw_xmit.c
Use is_multicast_ether_addr instead of custom IS_MCAST in
core/rtw_xmit.c. In all uses the address array is properly
aligned.
Signed-off-by: Michael Straube
Reviewed-by: Dan Carpenter
---
drivers/staging/rtl8188eu/core/rtw_xmit.c | 35 +++
1 file changed, 16 insertions(+), 19 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_xmit.c
b/drivers/staging/rtl8188eu/core/rtw_xmit.c
index 2130d78e0d9f..fc06a13a6ea1 100644
--- a/drivers/staging/rtl8188eu/core/rtw_xmit.c
+++ b/drivers/staging/rtl8188eu/core/rtw_xmit.c
@@ -399,7 +399,7 @@ static s32 update_attrib(struct adapter *padapter, struct
sk_buff *pkt, struct p
struct sta_info *psta = NULL;
struct ethhdr etherhdr;
- int bmcast;
+ bool mcast;
struct sta_priv *pstapriv = >stapriv;
struct security_priv*psecuritypriv = >securitypriv;
struct mlme_priv*pmlmepriv = >mlmepriv;
@@ -460,10 +460,10 @@ static s32 update_attrib(struct adapter *padapter, struct
sk_buff *pkt, struct p
if ((pattrib->ether_type == ETH_P_ARP) || (pattrib->ether_type ==
ETH_P_PAE) || (pattrib->dhcp_pkt == 1))
rtw_lps_ctrl_wk_cmd(padapter, LPS_CTRL_SPECIAL_PACKET, 1);
- bmcast = IS_MCAST(pattrib->ra);
+ mcast = is_multicast_ether_addr(pattrib->ra);
/* get sta_info */
- if (bmcast) {
+ if (mcast) {
psta = rtw_get_bcmc_stainfo(padapter);
} else {
psta = rtw_get_stainfo(pstapriv, pattrib->ra);
@@ -517,7 +517,7 @@ static s32 update_attrib(struct adapter *padapter, struct
sk_buff *pkt, struct p
goto exit;
}
} else {
- GET_ENCRY_ALGO(psecuritypriv, psta, pattrib->encrypt, bmcast);
+ GET_ENCRY_ALGO(psecuritypriv, psta, pattrib->encrypt, mcast);
switch (psecuritypriv->dot11AuthAlgrthm) {
case dot11AuthAlgrthm_Open:
@@ -526,7 +526,7 @@ static s32 update_attrib(struct adapter *padapter, struct
sk_buff *pkt, struct p
pattrib->key_idx =
(u8)psecuritypriv->dot11PrivacyKeyIndex;
break;
case dot11AuthAlgrthm_8021X:
- if (bmcast)
+ if (mcast)
pattrib->key_idx =
(u8)psecuritypriv->dot118021XGrpKeyid;
else
pattrib->key_idx = 0;
@@ -596,7 +596,6 @@ static s32 xmitframe_addmic(struct adapter *padapter,
struct xmit_frame *pxmitfr
struct xmit_priv *pxmitpriv = >xmitpriv;
u8 priority[4] = {0x0, 0x0, 0x0, 0x0};
u8 hw_hdr_offset = 0;
- int bmcst = IS_MCAST(pattrib->ra);
if (pattrib->psta)
stainfo = pattrib->psta;
@@ -614,7 +613,7 @@ static s32 xmitframe_addmic(struct adapter *padapter,
struct xmit_frame *pxmitfr
pframe = pxmitframe->buf_addr + hw_hdr_offset;
- if (bmcst) {
+ if (is_multicast_ether_addr(pattrib->ra)) {
if
(!memcmp(psecuritypriv->dot118021XGrptxmickey[psecuritypriv->dot118021XGrpKeyid].skey,
null_key, 16))
return _FAIL;
/* start to calculate the mic code */
@@ -743,12 +742,10 @@ s32 rtw_make_wlanhdr(struct adapter *padapter, u8 *hdr,
struct pkt_attrib *pattr
struct sta_info *psta;
- int bmcst = IS_MCAST(pattrib->ra);
-
if (pattrib->psta) {
psta = pattrib->psta;
} else {
- if (bmcst)
+ if (is_multicast_ether_addr(pattrib->ra))
psta = rtw_get_bcmc_stainfo(padapter);
else
psta = rtw_get_stainfo(>stapriv, pattrib->ra);
@@ -914,7 +911,7 @@ s32 rtw_xmitframe_coalesce(struct adapter *padapter, struct
sk_buff *pkt, struct
struct xmit_priv*pxmitpriv = >xmitpriv;
struct pkt_attrib *pattrib = >attrib;
u8 *pbuf_start;
- s32 bmcst = IS_MCAST(pattrib->ra);
+ bool mcast = is_multicast_ether_addr(pattrib->ra);
s32 res = _SUCCESS;
size_t remainder = pkt->len - ETH_HLEN;
@@ -964,13 +961,13 @@ s32 rtw_xmitframe_coalesce(struct adapter *padapter,
struct sk_buff *pkt, struct
WEP_IV(pattrib->iv, psta->dot11txpn,
pattrib->key_idx);
break;
case _TKIP_:
- if (bmcst)
+ if (mcast)
TKIP_IV(pattrib->iv, psta->dot11txpn,
pattrib->key_idx);
else
TKIP_IV(pattrib->iv, psta->dot11txpn,
0);
break;
case _AES_:
- if (bmcst)
+
[PATCH v3 3/5] staging: rtl8188eu: use is_multicast_ether_addr in rtw_xmit.c
Use is_multicast_ether_addr instead of custom IS_MCAST in
core/rtw_xmit.c. In all uses the address array is properly
aligned.
Signed-off-by: Michael Straube
Reviewed-by: Dan Carpenter
---
drivers/staging/rtl8188eu/core/rtw_xmit.c | 35 +++
1 file changed, 16 insertions(+), 19 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_xmit.c
b/drivers/staging/rtl8188eu/core/rtw_xmit.c
index 2130d78e0d9f..fc06a13a6ea1 100644
--- a/drivers/staging/rtl8188eu/core/rtw_xmit.c
+++ b/drivers/staging/rtl8188eu/core/rtw_xmit.c
@@ -399,7 +399,7 @@ static s32 update_attrib(struct adapter *padapter, struct
sk_buff *pkt, struct p
struct sta_info *psta = NULL;
struct ethhdr etherhdr;
- int bmcast;
+ bool mcast;
struct sta_priv *pstapriv = >stapriv;
struct security_priv*psecuritypriv = >securitypriv;
struct mlme_priv*pmlmepriv = >mlmepriv;
@@ -460,10 +460,10 @@ static s32 update_attrib(struct adapter *padapter, struct
sk_buff *pkt, struct p
if ((pattrib->ether_type == ETH_P_ARP) || (pattrib->ether_type ==
ETH_P_PAE) || (pattrib->dhcp_pkt == 1))
rtw_lps_ctrl_wk_cmd(padapter, LPS_CTRL_SPECIAL_PACKET, 1);
- bmcast = IS_MCAST(pattrib->ra);
+ mcast = is_multicast_ether_addr(pattrib->ra);
/* get sta_info */
- if (bmcast) {
+ if (mcast) {
psta = rtw_get_bcmc_stainfo(padapter);
} else {
psta = rtw_get_stainfo(pstapriv, pattrib->ra);
@@ -517,7 +517,7 @@ static s32 update_attrib(struct adapter *padapter, struct
sk_buff *pkt, struct p
goto exit;
}
} else {
- GET_ENCRY_ALGO(psecuritypriv, psta, pattrib->encrypt, bmcast);
+ GET_ENCRY_ALGO(psecuritypriv, psta, pattrib->encrypt, mcast);
switch (psecuritypriv->dot11AuthAlgrthm) {
case dot11AuthAlgrthm_Open:
@@ -526,7 +526,7 @@ static s32 update_attrib(struct adapter *padapter, struct
sk_buff *pkt, struct p
pattrib->key_idx =
(u8)psecuritypriv->dot11PrivacyKeyIndex;
break;
case dot11AuthAlgrthm_8021X:
- if (bmcast)
+ if (mcast)
pattrib->key_idx =
(u8)psecuritypriv->dot118021XGrpKeyid;
else
pattrib->key_idx = 0;
@@ -596,7 +596,6 @@ static s32 xmitframe_addmic(struct adapter *padapter,
struct xmit_frame *pxmitfr
struct xmit_priv *pxmitpriv = >xmitpriv;
u8 priority[4] = {0x0, 0x0, 0x0, 0x0};
u8 hw_hdr_offset = 0;
- int bmcst = IS_MCAST(pattrib->ra);
if (pattrib->psta)
stainfo = pattrib->psta;
@@ -614,7 +613,7 @@ static s32 xmitframe_addmic(struct adapter *padapter,
struct xmit_frame *pxmitfr
pframe = pxmitframe->buf_addr + hw_hdr_offset;
- if (bmcst) {
+ if (is_multicast_ether_addr(pattrib->ra)) {
if
(!memcmp(psecuritypriv->dot118021XGrptxmickey[psecuritypriv->dot118021XGrpKeyid].skey,
null_key, 16))
return _FAIL;
/* start to calculate the mic code */
@@ -743,12 +742,10 @@ s32 rtw_make_wlanhdr(struct adapter *padapter, u8 *hdr,
struct pkt_attrib *pattr
struct sta_info *psta;
- int bmcst = IS_MCAST(pattrib->ra);
-
if (pattrib->psta) {
psta = pattrib->psta;
} else {
- if (bmcst)
+ if (is_multicast_ether_addr(pattrib->ra))
psta = rtw_get_bcmc_stainfo(padapter);
else
psta = rtw_get_stainfo(>stapriv, pattrib->ra);
@@ -914,7 +911,7 @@ s32 rtw_xmitframe_coalesce(struct adapter *padapter, struct
sk_buff *pkt, struct
struct xmit_priv*pxmitpriv = >xmitpriv;
struct pkt_attrib *pattrib = >attrib;
u8 *pbuf_start;
- s32 bmcst = IS_MCAST(pattrib->ra);
+ bool mcast = is_multicast_ether_addr(pattrib->ra);
s32 res = _SUCCESS;
size_t remainder = pkt->len - ETH_HLEN;
@@ -964,13 +961,13 @@ s32 rtw_xmitframe_coalesce(struct adapter *padapter,
struct sk_buff *pkt, struct
WEP_IV(pattrib->iv, psta->dot11txpn,
pattrib->key_idx);
break;
case _TKIP_:
- if (bmcst)
+ if (mcast)
TKIP_IV(pattrib->iv, psta->dot11txpn,
pattrib->key_idx);
else
TKIP_IV(pattrib->iv, psta->dot11txpn,
0);
break;
case _AES_:
- if (bmcst)
+
Re: [PATCH v8 17/22] s390: vfio-ap: zeroize the AP queues.
On 10/08/2018 11:14, Cornelia Huck wrote:
On Wed, 8 Aug 2018 10:44:27 -0400
Tony Krowiak wrote:
From: Tony Krowiak
Let's call PAPQ(ZAPQ) to zeroize a queue:
* For each queue configured for a mediated matrix device
when it is released.
Zeroizing a queue resets the queue, clears all pending
messages for the queue entries and disables adapter interruptions
associated with the queue.
Signed-off-by: Tony Krowiak
Reviewed-by: Halil Pasic
Tested-by: Michael Mueller
Tested-by: Farhan Ali
Signed-off-by: Christian Borntraeger
---
drivers/s390/crypto/vfio_ap_ops.c | 29 -
drivers/s390/crypto/vfio_ap_private.h | 25 +
2 files changed, 53 insertions(+), 1 deletions(-)
@@ -788,7 +812,10 @@ static void vfio_ap_mdev_release(struct mdev_device *mdev)
{
struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
- kvm_arch_crypto_clear_masks(matrix_mdev->kvm);
+ if (matrix_mdev->kvm)
+ kvm_arch_crypto_clear_masks(matrix_mdev->kvm);
Confused. Why is the check for matrix_mdev->kvm added here?
When using the KVM notifier we can get two notifications:
-> KVM is here / is comming
-> KVM is not here / disappearing
In the first case we initialize matrix_mdev->kvm with a pointer to KVM
In the second case we nullify the pointer.
During the open of the mediated device, the guest should have been started
or we refuse to start.
During the close of the mediated device, the guest should be there, but
we have no certitude that the guest did not disappear before the VFIO
file being closed.
Since we do not allow multiple guests using the same mediated device
this case should not happen with QEMU. But I am not sure that
a rogue user program could not stop KVM before closing the VFIO
mediated device.
Maybe Alex can confirm this point, if not we can remove the test.
Thanks
Pierre
+
+ vfio_ap_mdev_reset_queues(mdev, true);
vfio_unregister_notifier(mdev_dev(mdev), VFIO_GROUP_NOTIFY,
_mdev->group_notifier);
matrix_mdev->kvm = NULL;
--
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany
Re: [PATCH v8 17/22] s390: vfio-ap: zeroize the AP queues.
On 10/08/2018 11:14, Cornelia Huck wrote:
On Wed, 8 Aug 2018 10:44:27 -0400
Tony Krowiak wrote:
From: Tony Krowiak
Let's call PAPQ(ZAPQ) to zeroize a queue:
* For each queue configured for a mediated matrix device
when it is released.
Zeroizing a queue resets the queue, clears all pending
messages for the queue entries and disables adapter interruptions
associated with the queue.
Signed-off-by: Tony Krowiak
Reviewed-by: Halil Pasic
Tested-by: Michael Mueller
Tested-by: Farhan Ali
Signed-off-by: Christian Borntraeger
---
drivers/s390/crypto/vfio_ap_ops.c | 29 -
drivers/s390/crypto/vfio_ap_private.h | 25 +
2 files changed, 53 insertions(+), 1 deletions(-)
@@ -788,7 +812,10 @@ static void vfio_ap_mdev_release(struct mdev_device *mdev)
{
struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
- kvm_arch_crypto_clear_masks(matrix_mdev->kvm);
+ if (matrix_mdev->kvm)
+ kvm_arch_crypto_clear_masks(matrix_mdev->kvm);
Confused. Why is the check for matrix_mdev->kvm added here?
When using the KVM notifier we can get two notifications:
-> KVM is here / is comming
-> KVM is not here / disappearing
In the first case we initialize matrix_mdev->kvm with a pointer to KVM
In the second case we nullify the pointer.
During the open of the mediated device, the guest should have been started
or we refuse to start.
During the close of the mediated device, the guest should be there, but
we have no certitude that the guest did not disappear before the VFIO
file being closed.
Since we do not allow multiple guests using the same mediated device
this case should not happen with QEMU. But I am not sure that
a rogue user program could not stop KVM before closing the VFIO
mediated device.
Maybe Alex can confirm this point, if not we can remove the test.
Thanks
Pierre
+
+ vfio_ap_mdev_reset_queues(mdev, true);
vfio_unregister_notifier(mdev_dev(mdev), VFIO_GROUP_NOTIFY,
_mdev->group_notifier);
matrix_mdev->kvm = NULL;
--
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany
Re: [RFC v7 PATCH 4/4] mm: unmap special vmas with regular do_munmap()
On 08/10/2018 01:36 AM, Yang Shi wrote:
> Unmapping vmas, which have VM_HUGETLB | VM_PFNMAP flag set or
> have uprobes set, need get done with write mmap_sem held since
> they may update vm_flags.
>
> So, it might be not safe enough to deal with these kind of special
> mappings with read mmap_sem. Deal with such mappings with regular
> do_munmap() call.
>
> Michal suggested to make this as a separate patch for safer and more
> bisectable sake.
>
> Cc: Michal Hocko
> Signed-off-by: Yang Shi
> ---
> mm/mmap.c | 24
> 1 file changed, 24 insertions(+)
>
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 2234d5a..06cb83c 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -2766,6 +2766,16 @@ static inline void munlock_vmas(struct vm_area_struct
> *vma,
> }
> }
>
> +static inline bool can_zap_with_rlock(struct vm_area_struct *vma)
> +{
> + if ((vma->vm_file &&
> + vma_has_uprobes(vma, vma->vm_start, vma->vm_end)) |
vma_has_uprobes() seems to be rather expensive check with e.g.
unconditional spinlock. uprobe_munmap() seems to have some precondition
cheaper checks for e.g. cases when there's no uprobes in the system
(should be common?).
BTW, uprobe_munmap() touches mm->flags, not vma->flags, so it should be
evaluated more carefully for being called under mmap sem for reading, as
having vmas already detached is no guarantee.
> + (vma->vm_flags | (VM_HUGETLB | VM_PFNMAP)))
^ I think replace '|' with '&' here?
> + return false;
> +
> + return true;
> +}
> +
> /*
> * Zap pages with read mmap_sem held
> *
> @@ -2808,6 +2818,17 @@ static int do_munmap_zap_rlock(struct mm_struct *mm,
> unsigned long start,
> goto out;
> }
>
> + /*
> + * Unmapping vmas, which have VM_HUGETLB | VM_PFNMAP flag set or
> + * have uprobes set, need get done with write mmap_sem held since
> + * they may update vm_flags. Deal with such mappings with regular
> + * do_munmap() call.
> + */
> + for (vma = start_vma; vma && vma->vm_start < end; vma = vma->vm_next) {
> + if (!can_zap_with_rlock(vma))
> + goto regular_path;
> + }
> +
> /* Handle mlocked vmas */
> if (mm->locked_vm) {
> vma = start_vma;
> @@ -2828,6 +2849,9 @@ static int do_munmap_zap_rlock(struct mm_struct *mm,
> unsigned long start,
>
> return 0;
>
> +regular_path:
I think it's missing a down_write_* here.
> + ret = do_munmap(mm, start, len, uf);
> +
> out:
> up_write(>mmap_sem);
> return ret;
>
Re: [RFC v7 PATCH 4/4] mm: unmap special vmas with regular do_munmap()
On 08/10/2018 01:36 AM, Yang Shi wrote:
> Unmapping vmas, which have VM_HUGETLB | VM_PFNMAP flag set or
> have uprobes set, need get done with write mmap_sem held since
> they may update vm_flags.
>
> So, it might be not safe enough to deal with these kind of special
> mappings with read mmap_sem. Deal with such mappings with regular
> do_munmap() call.
>
> Michal suggested to make this as a separate patch for safer and more
> bisectable sake.
>
> Cc: Michal Hocko
> Signed-off-by: Yang Shi
> ---
> mm/mmap.c | 24
> 1 file changed, 24 insertions(+)
>
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 2234d5a..06cb83c 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -2766,6 +2766,16 @@ static inline void munlock_vmas(struct vm_area_struct
> *vma,
> }
> }
>
> +static inline bool can_zap_with_rlock(struct vm_area_struct *vma)
> +{
> + if ((vma->vm_file &&
> + vma_has_uprobes(vma, vma->vm_start, vma->vm_end)) |
vma_has_uprobes() seems to be rather expensive check with e.g.
unconditional spinlock. uprobe_munmap() seems to have some precondition
cheaper checks for e.g. cases when there's no uprobes in the system
(should be common?).
BTW, uprobe_munmap() touches mm->flags, not vma->flags, so it should be
evaluated more carefully for being called under mmap sem for reading, as
having vmas already detached is no guarantee.
> + (vma->vm_flags | (VM_HUGETLB | VM_PFNMAP)))
^ I think replace '|' with '&' here?
> + return false;
> +
> + return true;
> +}
> +
> /*
> * Zap pages with read mmap_sem held
> *
> @@ -2808,6 +2818,17 @@ static int do_munmap_zap_rlock(struct mm_struct *mm,
> unsigned long start,
> goto out;
> }
>
> + /*
> + * Unmapping vmas, which have VM_HUGETLB | VM_PFNMAP flag set or
> + * have uprobes set, need get done with write mmap_sem held since
> + * they may update vm_flags. Deal with such mappings with regular
> + * do_munmap() call.
> + */
> + for (vma = start_vma; vma && vma->vm_start < end; vma = vma->vm_next) {
> + if (!can_zap_with_rlock(vma))
> + goto regular_path;
> + }
> +
> /* Handle mlocked vmas */
> if (mm->locked_vm) {
> vma = start_vma;
> @@ -2828,6 +2849,9 @@ static int do_munmap_zap_rlock(struct mm_struct *mm,
> unsigned long start,
>
> return 0;
>
> +regular_path:
I think it's missing a down_write_* here.
> + ret = do_munmap(mm, start, len, uf);
> +
> out:
> up_write(>mmap_sem);
> return ret;
>
[PATCH 5/5] perf/hw_breakpoint: Simplify breakpoint enable in perf_event_modify_breakpoint
We can safely enable the breakpoint back for both the fail
and success paths by checking only the bp->attr.disabled,
which either holds the new 'requested' disabled state or
the original breakpoint state.
Suggested-by: Oleg Nesterov
Link: http://lkml.kernel.org/n/tip-vo1jm1u2nar6lj6hnd9g7...@git.kernel.org
Signed-off-by: Jiri Olsa
---
kernel/events/core.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index f6ea33a9f904..22ede28ec07d 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -2867,16 +2867,11 @@ static int perf_event_modify_breakpoint(struct
perf_event *bp,
_perf_event_disable(bp);
err = modify_user_hw_breakpoint_check(bp, attr, true);
- if (err) {
- if (!bp->attr.disabled)
- _perf_event_enable(bp);
- return err;
- }
-
- if (!attr->disabled)
+ if (!bp->attr.disabled)
_perf_event_enable(bp);
- return 0;
+
+ return err;
}
static int perf_event_modify_attr(struct perf_event *event,
--
2.17.1
[PATCH 5/5] perf/hw_breakpoint: Simplify breakpoint enable in perf_event_modify_breakpoint
We can safely enable the breakpoint back for both the fail
and success paths by checking only the bp->attr.disabled,
which either holds the new 'requested' disabled state or
the original breakpoint state.
Suggested-by: Oleg Nesterov
Link: http://lkml.kernel.org/n/tip-vo1jm1u2nar6lj6hnd9g7...@git.kernel.org
Signed-off-by: Jiri Olsa
---
kernel/events/core.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index f6ea33a9f904..22ede28ec07d 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -2867,16 +2867,11 @@ static int perf_event_modify_breakpoint(struct
perf_event *bp,
_perf_event_disable(bp);
err = modify_user_hw_breakpoint_check(bp, attr, true);
- if (err) {
- if (!bp->attr.disabled)
- _perf_event_enable(bp);
- return err;
- }
-
- if (!attr->disabled)
+ if (!bp->attr.disabled)
_perf_event_enable(bp);
- return 0;
+
+ return err;
}
static int perf_event_modify_attr(struct perf_event *event,
--
2.17.1
[PATCH 3/5] perf/hw_breakpoint: Remove superfluous bp->attr.disabled = 0
Once the breakpoint was succesfully modified, the attr->disabled
value is in bp->attr.disabled. So there's no reason to set it
again, removing that.
Acked-by: Oleg Nesterov
Link: http://lkml.kernel.org/n/tip-v5oaellzsmyszv3rfucux...@git.kernel.org
Signed-off-by: Jiri Olsa
---
kernel/events/hw_breakpoint.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c
index fb229d9c7f3c..3e560d7609fd 100644
--- a/kernel/events/hw_breakpoint.c
+++ b/kernel/events/hw_breakpoint.c
@@ -526,10 +526,9 @@ int modify_user_hw_breakpoint(struct perf_event *bp,
struct perf_event_attr *att
if (err)
return err;
- if (!attr->disabled) {
+ if (!attr->disabled)
perf_event_enable(bp);
- bp->attr.disabled = 0;
- }
+
return 0;
}
EXPORT_SYMBOL_GPL(modify_user_hw_breakpoint);
--
2.17.1
[PATCH 4/5] perf/hw_breakpoint: Enable breakpoint in modify_user_hw_breakpoint
Currently we enable the breakpoint back only if the breakpoint modification was successful. If it fails we can leave the breakpoint in disabled state with attr->disabled == 0. We can safely enable the breakpoint back for both the fail and success paths by checking the bp->attr.disabled, which either holds the new 'requested' disabled state or the original breakpoint state. Suggested-by: Oleg Nesterov Link: http://lkml.kernel.org/n/tip-79p9ttocwy2ju2s6qh25d...@git.kernel.org Signed-off-by: Jiri Olsa --- kernel/events/hw_breakpoint.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c index 3e560d7609fd..d6b56180827c 100644 --- a/kernel/events/hw_breakpoint.c +++ b/kernel/events/hw_breakpoint.c @@ -523,13 +523,11 @@ int modify_user_hw_breakpoint(struct perf_event *bp, struct perf_event_attr *att perf_event_disable(bp); err = modify_user_hw_breakpoint_check(bp, attr, false); - if (err) - return err; - if (!attr->disabled) + if (!bp->attr.disabled) perf_event_enable(bp); - return 0; + return err; } EXPORT_SYMBOL_GPL(modify_user_hw_breakpoint); -- 2.17.1
[PATCH 1/5] perf tests: Add breakpoint modify tests
Adding to tests that aims on kernel breakpoint
modification bugs.
First test creates HW breakpoint, tries to change
it and checks it was properly changed. It aims
on kernel issue that prevents HW breakpoint to
be changed via ptrace interface.
The first test forks, the child sets itself as ptrace
tracee and waits in signal for parent to trace it,
then it calls bp_1 and quits.
The parent does following steps:
- creates a new breakpoint (id 0) for bp_2 function
- changes that breakpoint to bp_1 function
- waits for the breakpoint to hit and checks
it has proper rip of bp_1 function
This test aims on an issue in kernel preventing to change
disabled breakpoints
Second test mimics the first one except for few steps
in the parent:
- creates a new breakpoint (id 0) for bp_1 function
- changes that breakpoint to bogus (-1) address
- waits for the breakpoint to hit and checks
it has proper rip of bp_1 function
This test aims on an issue in kernel disabling enabled
breakpoint after unsuccesful change.
Link: http://lkml.kernel.org/n/tip-8a3x8utnljw8xtumkxkxk...@git.kernel.org
Signed-off-by: Jiri Olsa
---
tools/perf/arch/x86/include/arch-tests.h | 1 +
tools/perf/arch/x86/tests/Build | 1 +
tools/perf/arch/x86/tests/arch-tests.c | 6 +
tools/perf/arch/x86/tests/bp-modify.c| 213 +++
4 files changed, 221 insertions(+)
create mode 100644 tools/perf/arch/x86/tests/bp-modify.c
diff --git a/tools/perf/arch/x86/include/arch-tests.h
b/tools/perf/arch/x86/include/arch-tests.h
index c1bd979b957b..613709cfbbd0 100644
--- a/tools/perf/arch/x86/include/arch-tests.h
+++ b/tools/perf/arch/x86/include/arch-tests.h
@@ -9,6 +9,7 @@ struct test;
int test__rdpmc(struct test *test __maybe_unused, int subtest);
int test__perf_time_to_tsc(struct test *test __maybe_unused, int subtest);
int test__insn_x86(struct test *test __maybe_unused, int subtest);
+int test__bp_modify(struct test *test, int subtest);
#ifdef HAVE_DWARF_UNWIND_SUPPORT
struct thread;
diff --git a/tools/perf/arch/x86/tests/Build b/tools/perf/arch/x86/tests/Build
index 8e2c5a38c3b9..586849ff83a0 100644
--- a/tools/perf/arch/x86/tests/Build
+++ b/tools/perf/arch/x86/tests/Build
@@ -5,3 +5,4 @@ libperf-y += arch-tests.o
libperf-y += rdpmc.o
libperf-y += perf-time-to-tsc.o
libperf-$(CONFIG_AUXTRACE) += insn-x86.o
+libperf-$(CONFIG_X86_64) += bp-modify.o
diff --git a/tools/perf/arch/x86/tests/arch-tests.c
b/tools/perf/arch/x86/tests/arch-tests.c
index cc1802ff5410..d47d3f8e3c8e 100644
--- a/tools/perf/arch/x86/tests/arch-tests.c
+++ b/tools/perf/arch/x86/tests/arch-tests.c
@@ -23,6 +23,12 @@ struct test arch_tests[] = {
.desc = "x86 instruction decoder - new instructions",
.func = test__insn_x86,
},
+#endif
+#if defined(__x86_64__)
+ {
+ .desc = "x86 bp modify",
+ .func = test__bp_modify,
+ },
#endif
{
.func = NULL,
diff --git a/tools/perf/arch/x86/tests/bp-modify.c
b/tools/perf/arch/x86/tests/bp-modify.c
new file mode 100644
index ..f53e4406709f
--- /dev/null
+++ b/tools/perf/arch/x86/tests/bp-modify.c
@@ -0,0 +1,213 @@
+// SPDX-License-Identifier: GPL-2.0
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include "debug.h"
+#include "tests/tests.h"
+#include "arch-tests.h"
+
+static noinline int bp_1(void)
+{
+ pr_debug("in %s\n", __func__);
+ return 0;
+}
+
+static noinline int bp_2(void)
+{
+ pr_debug("in %s\n", __func__);
+ return 0;
+}
+
+static int spawn_child(void)
+{
+ int child = fork();
+
+ if (child == 0) {
+ /*
+* The child sets itself for as tracee and
+* waits in signal for parent to trace it,
+* then it calls bp_1 and quits.
+*/
+ int err = ptrace(PTRACE_TRACEME, 0, NULL, NULL);
+
+ if (err) {
+ pr_debug("failed to PTRACE_TRACEME\n");
+ exit(1);
+ }
+
+ raise(SIGCONT);
+ bp_1();
+ exit(0);
+ }
+
+ return child;
+}
+
+/*
+ * This tests creates HW breakpoint, tries to
+ * change it and checks it was properly changed.
+ */
+static int bp_modify1(void)
+{
+ pid_t child;
+ int status;
+ unsigned long rip = 0, dr7 = 1;
+
+ child = spawn_child();
+
+ waitpid(child, , 0);
+ if (WIFEXITED(status)) {
+ pr_debug("tracee exited prematurely 1\n");
+ return TEST_FAIL;
+ }
+
+ /*
+* The parent does following steps:
+* - creates a new breakpoint (id 0) for bp_2 function
+* - changes that breakponit to bp_1 function
+* - waits for the breakpoint to hit and checks
+*it has proper rip of bp_1 function
+* - detaches the child
+*/
+
[PATCH 2/5] perf/hw_breakpoint: Modify breakpoint even if the new attr has disabled set
We need to change the breakpoint even if the attr with
new fields has disabled set to true.
Current code prevents following user code to change
the breakpoint address:
ptrace(PTRACE_POKEUSER, child, offsetof(struct user, u_debugreg[0]), addr_1)
ptrace(PTRACE_POKEUSER, child, offsetof(struct user, u_debugreg[0]), addr_2)
ptrace(PTRACE_POKEUSER, child, offsetof(struct user, u_debugreg[7]), dr7)
The first PTRACE_POKEUSER creates the breakpoint with
attr.disabled set to true:
ptrace_set_breakpoint_addr(nr = 0)
struct perf_event *bp = t->ptrace_bps[nr];
ptrace_register_breakpoint(..., disabled = true)
ptrace_fill_bp_fields(..., disabled)
register_user_hw_breakpoint
So the second PTRACE_POKEUSER will be omitted:
ptrace_set_breakpoint_addr(nr = 0)
struct perf_event *bp = t->ptrace_bps[nr];
struct perf_event_attr attr = bp->attr;
modify_user_hw_breakpoint(bp, )
if (!attr->disabled)
modify_user_hw_breakpoint_check
Acked-by: Oleg Nesterov
Reported-by: Milind Chabbi
Link: http://lkml.kernel.org/n/tip-yjhgplc28gk5gfzt7ceoo...@git.kernel.org
Signed-off-by: Jiri Olsa
---
kernel/events/hw_breakpoint.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c
index b3814fce5ecb..fb229d9c7f3c 100644
--- a/kernel/events/hw_breakpoint.c
+++ b/kernel/events/hw_breakpoint.c
@@ -509,6 +509,8 @@ modify_user_hw_breakpoint_check(struct perf_event *bp,
struct perf_event_attr *a
*/
int modify_user_hw_breakpoint(struct perf_event *bp, struct perf_event_attr
*attr)
{
+ int err;
+
/*
* modify_user_hw_breakpoint can be invoked with IRQs disabled and
hence it
* will not be possible to raise IPIs that invoke __perf_event_disable.
@@ -520,11 +522,11 @@ int modify_user_hw_breakpoint(struct perf_event *bp,
struct perf_event_attr *att
else
perf_event_disable(bp);
- if (!attr->disabled) {
- int err = modify_user_hw_breakpoint_check(bp, attr, false);
+ err = modify_user_hw_breakpoint_check(bp, attr, false);
+ if (err)
+ return err;
- if (err)
- return err;
+ if (!attr->disabled) {
perf_event_enable(bp);
bp->attr.disabled = 0;
}
--
2.17.1
[PATCHv3 0/5] perf/hw_breakpoint: Fix breakpoint modify
hi, Milind reported that modify_user_hw_breakpoint wouldn't allow the breakpoint changing if the new attr had 'disabled' set to true. I found a case where it actualy prevents ptrace user interface to change the breakpoint. It's described in patch 1 as perf test, patch 2 is the breakpoint code fix. I ran strace tests, nothing (new) broken there.. v3 changes: - added Oleg's ack for patch 3 - new patches 4,5 based on Oleg's suggestions replacing the v2 fallback approach by enabling the event directly after failed modification v2 changes: - added Oleg's ack for patch 2 - added new changes based on Oleg's questions plus new test code thanks, jirka --- Jiri Olsa (5): perf tests: Add breakpoint modify tests perf/hw_breakpoint: Modify breakpoint even if the new attr has disabled set perf/hw_breakpoint: Remove superfluous bp->attr.disabled = 0 perf/hw_breakpoint: Enable breakpoint in modify_user_hw_breakpoint perf/hw_breakpoint: Simplify breakpoint enable in perf_event_modify_breakpoint kernel/events/core.c | 11 ++- kernel/events/hw_breakpoint.c| 13 tools/perf/arch/x86/include/arch-tests.h | 1 + tools/perf/arch/x86/tests/Build | 1 + tools/perf/arch/x86/tests/arch-tests.c | 6 tools/perf/arch/x86/tests/bp-modify.c| 213 6 files changed, 230 insertions(+), 15 deletions(-) create mode 100644 tools/perf/arch/x86/tests/bp-modify.c
[PATCH 3/5] perf/hw_breakpoint: Remove superfluous bp->attr.disabled = 0
Once the breakpoint was succesfully modified, the attr->disabled
value is in bp->attr.disabled. So there's no reason to set it
again, removing that.
Acked-by: Oleg Nesterov
Link: http://lkml.kernel.org/n/tip-v5oaellzsmyszv3rfucux...@git.kernel.org
Signed-off-by: Jiri Olsa
---
kernel/events/hw_breakpoint.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c
index fb229d9c7f3c..3e560d7609fd 100644
--- a/kernel/events/hw_breakpoint.c
+++ b/kernel/events/hw_breakpoint.c
@@ -526,10 +526,9 @@ int modify_user_hw_breakpoint(struct perf_event *bp,
struct perf_event_attr *att
if (err)
return err;
- if (!attr->disabled) {
+ if (!attr->disabled)
perf_event_enable(bp);
- bp->attr.disabled = 0;
- }
+
return 0;
}
EXPORT_SYMBOL_GPL(modify_user_hw_breakpoint);
--
2.17.1
[PATCH 4/5] perf/hw_breakpoint: Enable breakpoint in modify_user_hw_breakpoint
Currently we enable the breakpoint back only if the breakpoint modification was successful. If it fails we can leave the breakpoint in disabled state with attr->disabled == 0. We can safely enable the breakpoint back for both the fail and success paths by checking the bp->attr.disabled, which either holds the new 'requested' disabled state or the original breakpoint state. Suggested-by: Oleg Nesterov Link: http://lkml.kernel.org/n/tip-79p9ttocwy2ju2s6qh25d...@git.kernel.org Signed-off-by: Jiri Olsa --- kernel/events/hw_breakpoint.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c index 3e560d7609fd..d6b56180827c 100644 --- a/kernel/events/hw_breakpoint.c +++ b/kernel/events/hw_breakpoint.c @@ -523,13 +523,11 @@ int modify_user_hw_breakpoint(struct perf_event *bp, struct perf_event_attr *att perf_event_disable(bp); err = modify_user_hw_breakpoint_check(bp, attr, false); - if (err) - return err; - if (!attr->disabled) + if (!bp->attr.disabled) perf_event_enable(bp); - return 0; + return err; } EXPORT_SYMBOL_GPL(modify_user_hw_breakpoint); -- 2.17.1
[PATCH 1/5] perf tests: Add breakpoint modify tests
Adding to tests that aims on kernel breakpoint
modification bugs.
First test creates HW breakpoint, tries to change
it and checks it was properly changed. It aims
on kernel issue that prevents HW breakpoint to
be changed via ptrace interface.
The first test forks, the child sets itself as ptrace
tracee and waits in signal for parent to trace it,
then it calls bp_1 and quits.
The parent does following steps:
- creates a new breakpoint (id 0) for bp_2 function
- changes that breakpoint to bp_1 function
- waits for the breakpoint to hit and checks
it has proper rip of bp_1 function
This test aims on an issue in kernel preventing to change
disabled breakpoints
Second test mimics the first one except for few steps
in the parent:
- creates a new breakpoint (id 0) for bp_1 function
- changes that breakpoint to bogus (-1) address
- waits for the breakpoint to hit and checks
it has proper rip of bp_1 function
This test aims on an issue in kernel disabling enabled
breakpoint after unsuccesful change.
Link: http://lkml.kernel.org/n/tip-8a3x8utnljw8xtumkxkxk...@git.kernel.org
Signed-off-by: Jiri Olsa
---
tools/perf/arch/x86/include/arch-tests.h | 1 +
tools/perf/arch/x86/tests/Build | 1 +
tools/perf/arch/x86/tests/arch-tests.c | 6 +
tools/perf/arch/x86/tests/bp-modify.c| 213 +++
4 files changed, 221 insertions(+)
create mode 100644 tools/perf/arch/x86/tests/bp-modify.c
diff --git a/tools/perf/arch/x86/include/arch-tests.h
b/tools/perf/arch/x86/include/arch-tests.h
index c1bd979b957b..613709cfbbd0 100644
--- a/tools/perf/arch/x86/include/arch-tests.h
+++ b/tools/perf/arch/x86/include/arch-tests.h
@@ -9,6 +9,7 @@ struct test;
int test__rdpmc(struct test *test __maybe_unused, int subtest);
int test__perf_time_to_tsc(struct test *test __maybe_unused, int subtest);
int test__insn_x86(struct test *test __maybe_unused, int subtest);
+int test__bp_modify(struct test *test, int subtest);
#ifdef HAVE_DWARF_UNWIND_SUPPORT
struct thread;
diff --git a/tools/perf/arch/x86/tests/Build b/tools/perf/arch/x86/tests/Build
index 8e2c5a38c3b9..586849ff83a0 100644
--- a/tools/perf/arch/x86/tests/Build
+++ b/tools/perf/arch/x86/tests/Build
@@ -5,3 +5,4 @@ libperf-y += arch-tests.o
libperf-y += rdpmc.o
libperf-y += perf-time-to-tsc.o
libperf-$(CONFIG_AUXTRACE) += insn-x86.o
+libperf-$(CONFIG_X86_64) += bp-modify.o
diff --git a/tools/perf/arch/x86/tests/arch-tests.c
b/tools/perf/arch/x86/tests/arch-tests.c
index cc1802ff5410..d47d3f8e3c8e 100644
--- a/tools/perf/arch/x86/tests/arch-tests.c
+++ b/tools/perf/arch/x86/tests/arch-tests.c
@@ -23,6 +23,12 @@ struct test arch_tests[] = {
.desc = "x86 instruction decoder - new instructions",
.func = test__insn_x86,
},
+#endif
+#if defined(__x86_64__)
+ {
+ .desc = "x86 bp modify",
+ .func = test__bp_modify,
+ },
#endif
{
.func = NULL,
diff --git a/tools/perf/arch/x86/tests/bp-modify.c
b/tools/perf/arch/x86/tests/bp-modify.c
new file mode 100644
index ..f53e4406709f
--- /dev/null
+++ b/tools/perf/arch/x86/tests/bp-modify.c
@@ -0,0 +1,213 @@
+// SPDX-License-Identifier: GPL-2.0
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include "debug.h"
+#include "tests/tests.h"
+#include "arch-tests.h"
+
+static noinline int bp_1(void)
+{
+ pr_debug("in %s\n", __func__);
+ return 0;
+}
+
+static noinline int bp_2(void)
+{
+ pr_debug("in %s\n", __func__);
+ return 0;
+}
+
+static int spawn_child(void)
+{
+ int child = fork();
+
+ if (child == 0) {
+ /*
+* The child sets itself for as tracee and
+* waits in signal for parent to trace it,
+* then it calls bp_1 and quits.
+*/
+ int err = ptrace(PTRACE_TRACEME, 0, NULL, NULL);
+
+ if (err) {
+ pr_debug("failed to PTRACE_TRACEME\n");
+ exit(1);
+ }
+
+ raise(SIGCONT);
+ bp_1();
+ exit(0);
+ }
+
+ return child;
+}
+
+/*
+ * This tests creates HW breakpoint, tries to
+ * change it and checks it was properly changed.
+ */
+static int bp_modify1(void)
+{
+ pid_t child;
+ int status;
+ unsigned long rip = 0, dr7 = 1;
+
+ child = spawn_child();
+
+ waitpid(child, , 0);
+ if (WIFEXITED(status)) {
+ pr_debug("tracee exited prematurely 1\n");
+ return TEST_FAIL;
+ }
+
+ /*
+* The parent does following steps:
+* - creates a new breakpoint (id 0) for bp_2 function
+* - changes that breakponit to bp_1 function
+* - waits for the breakpoint to hit and checks
+*it has proper rip of bp_1 function
+* - detaches the child
+*/
+
[PATCH 2/5] perf/hw_breakpoint: Modify breakpoint even if the new attr has disabled set
We need to change the breakpoint even if the attr with
new fields has disabled set to true.
Current code prevents following user code to change
the breakpoint address:
ptrace(PTRACE_POKEUSER, child, offsetof(struct user, u_debugreg[0]), addr_1)
ptrace(PTRACE_POKEUSER, child, offsetof(struct user, u_debugreg[0]), addr_2)
ptrace(PTRACE_POKEUSER, child, offsetof(struct user, u_debugreg[7]), dr7)
The first PTRACE_POKEUSER creates the breakpoint with
attr.disabled set to true:
ptrace_set_breakpoint_addr(nr = 0)
struct perf_event *bp = t->ptrace_bps[nr];
ptrace_register_breakpoint(..., disabled = true)
ptrace_fill_bp_fields(..., disabled)
register_user_hw_breakpoint
So the second PTRACE_POKEUSER will be omitted:
ptrace_set_breakpoint_addr(nr = 0)
struct perf_event *bp = t->ptrace_bps[nr];
struct perf_event_attr attr = bp->attr;
modify_user_hw_breakpoint(bp, )
if (!attr->disabled)
modify_user_hw_breakpoint_check
Acked-by: Oleg Nesterov
Reported-by: Milind Chabbi
Link: http://lkml.kernel.org/n/tip-yjhgplc28gk5gfzt7ceoo...@git.kernel.org
Signed-off-by: Jiri Olsa
---
kernel/events/hw_breakpoint.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c
index b3814fce5ecb..fb229d9c7f3c 100644
--- a/kernel/events/hw_breakpoint.c
+++ b/kernel/events/hw_breakpoint.c
@@ -509,6 +509,8 @@ modify_user_hw_breakpoint_check(struct perf_event *bp,
struct perf_event_attr *a
*/
int modify_user_hw_breakpoint(struct perf_event *bp, struct perf_event_attr
*attr)
{
+ int err;
+
/*
* modify_user_hw_breakpoint can be invoked with IRQs disabled and
hence it
* will not be possible to raise IPIs that invoke __perf_event_disable.
@@ -520,11 +522,11 @@ int modify_user_hw_breakpoint(struct perf_event *bp,
struct perf_event_attr *att
else
perf_event_disable(bp);
- if (!attr->disabled) {
- int err = modify_user_hw_breakpoint_check(bp, attr, false);
+ err = modify_user_hw_breakpoint_check(bp, attr, false);
+ if (err)
+ return err;
- if (err)
- return err;
+ if (!attr->disabled) {
perf_event_enable(bp);
bp->attr.disabled = 0;
}
--
2.17.1
[PATCHv3 0/5] perf/hw_breakpoint: Fix breakpoint modify
hi, Milind reported that modify_user_hw_breakpoint wouldn't allow the breakpoint changing if the new attr had 'disabled' set to true. I found a case where it actualy prevents ptrace user interface to change the breakpoint. It's described in patch 1 as perf test, patch 2 is the breakpoint code fix. I ran strace tests, nothing (new) broken there.. v3 changes: - added Oleg's ack for patch 3 - new patches 4,5 based on Oleg's suggestions replacing the v2 fallback approach by enabling the event directly after failed modification v2 changes: - added Oleg's ack for patch 2 - added new changes based on Oleg's questions plus new test code thanks, jirka --- Jiri Olsa (5): perf tests: Add breakpoint modify tests perf/hw_breakpoint: Modify breakpoint even if the new attr has disabled set perf/hw_breakpoint: Remove superfluous bp->attr.disabled = 0 perf/hw_breakpoint: Enable breakpoint in modify_user_hw_breakpoint perf/hw_breakpoint: Simplify breakpoint enable in perf_event_modify_breakpoint kernel/events/core.c | 11 ++- kernel/events/hw_breakpoint.c| 13 tools/perf/arch/x86/include/arch-tests.h | 1 + tools/perf/arch/x86/tests/Build | 1 + tools/perf/arch/x86/tests/arch-tests.c | 6 tools/perf/arch/x86/tests/bp-modify.c| 213 6 files changed, 230 insertions(+), 15 deletions(-) create mode 100644 tools/perf/arch/x86/tests/bp-modify.c
Droid 4: poweroff reboots the machine
Hi! On droid4, "sudo poweroff" reboots the system... which is not exactly welcome, because mainline kernel will discharge the battery in 10 hours. I learned to reboot into stock Motorola Android (lasts few days), but that takes some clicking; easier solution is powering it off in SafeStrap ... which thankfully works. But those are workarounds, if someone knows how to debug/fix it, let me know. 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
Droid 4: poweroff reboots the machine
Hi! On droid4, "sudo poweroff" reboots the system... which is not exactly welcome, because mainline kernel will discharge the battery in 10 hours. I learned to reboot into stock Motorola Android (lasts few days), but that takes some clicking; easier solution is powering it off in SafeStrap ... which thankfully works. But those are workarounds, if someone knows how to debug/fix it, let me know. 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 v2 5/5] staging: rtl8188eu: use phydm_reg.h from rtlwifi
On 10.08.18 11:44, Dan Carpenter wrote: Looks good. Thanks! I reviewed the series. Reviewed-by: Dan Carpenter Thanks, Michael
Re: [PATCH v2 5/5] staging: rtl8188eu: use phydm_reg.h from rtlwifi
On 10.08.18 11:44, Dan Carpenter wrote: Looks good. Thanks! I reviewed the series. Reviewed-by: Dan Carpenter Thanks, Michael
Re: Droid 4: suspend to RAM?
On Wed 2018-08-08 02:05:12, Tony Lindgren wrote: > * Pavel Machek [180727 11:35]: > > Hi! > > > > > high even before modem (and thus USB) is enabled. > > > > > > > > > > Interestingly, CyanogenMod and Jolla seem to have higher power > > > > > consumption than stock operating system. > > > > > > > > > > (My Linux can survive for 10 hours, stock system could survive for 4 > > > > > days if I'm not mistaken). > > > > > > > > > > I thought I would experiment with suspend to RAM.. and it indeed > > > > > seemed to suspend ok, but I could not wake it up. Do I need to set up > > > > > wakeup with button somehow? Is suspend to RAM required for good power > > > > > consumption? > > > > > > > > Sorry but pm subsystem has debug mode that you can test in a easy way. > > > > You can even wakeup by any rtc alarm easily. > > > > > > Yes, that is how it works on PC (but there power button works, > > > too). Is it expected to work on Droid in v4.18? > > > > I tried setting up wakeup using RTC, but no, it does not seem to work: > > > > root@devuan:/my/tui/d4# rtcwake -m no -s 5 > > rtcwake: wakeup using /dev/rtc0 at Fri Jul 27 11:28:44 2018 > > root@devuan:/my/tui/d4# echo mem > /sys/power/state > > Works for me here as tested on next-20180808, maybe you don't have > CONFIG_RTC_DRV_CPCAP? Maybe you are trying to use CONFIG_RTC_DRV_OMAP? I have right config, and it works for me on next-20180808. Power button wakes it up, as expected, and even usbnet connections survive, which is good. Unfortunately, next-20180808 has no video support, which may also be reason why it works. I guess I'll need to dust off serial cable and look for logs... 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: Droid 4: suspend to RAM?
On Wed 2018-08-08 02:05:12, Tony Lindgren wrote: > * Pavel Machek [180727 11:35]: > > Hi! > > > > > high even before modem (and thus USB) is enabled. > > > > > > > > > > Interestingly, CyanogenMod and Jolla seem to have higher power > > > > > consumption than stock operating system. > > > > > > > > > > (My Linux can survive for 10 hours, stock system could survive for 4 > > > > > days if I'm not mistaken). > > > > > > > > > > I thought I would experiment with suspend to RAM.. and it indeed > > > > > seemed to suspend ok, but I could not wake it up. Do I need to set up > > > > > wakeup with button somehow? Is suspend to RAM required for good power > > > > > consumption? > > > > > > > > Sorry but pm subsystem has debug mode that you can test in a easy way. > > > > You can even wakeup by any rtc alarm easily. > > > > > > Yes, that is how it works on PC (but there power button works, > > > too). Is it expected to work on Droid in v4.18? > > > > I tried setting up wakeup using RTC, but no, it does not seem to work: > > > > root@devuan:/my/tui/d4# rtcwake -m no -s 5 > > rtcwake: wakeup using /dev/rtc0 at Fri Jul 27 11:28:44 2018 > > root@devuan:/my/tui/d4# echo mem > /sys/power/state > > Works for me here as tested on next-20180808, maybe you don't have > CONFIG_RTC_DRV_CPCAP? Maybe you are trying to use CONFIG_RTC_DRV_OMAP? I have right config, and it works for me on next-20180808. Power button wakes it up, as expected, and even usbnet connections survive, which is good. Unfortunately, next-20180808 has no video support, which may also be reason why it works. I guess I'll need to dust off serial cable and look for logs... 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 v6 3/6] regmap: Add regmap_noinc_read API
On Fri, Aug 10, 2018 at 11:46:20AM +0300, Stefan Popa wrote: > From: Crestez Dan Leonard > > The regmap API usually assumes that bulk read operations will read a > range of registers but some I2C/SPI devices have certain registers for > which a such a read operation will return data from an internal FIFO > instead. Add an explicit API to support bulk read without range semantics. Please don't resubmit patches that have already been applied, you should submit patches against current code in the tree you're expecting things to be applied to. If any updates are needed to a patch that's already been applied you should submit incremental patches which make those updates. This avoids having to change published git commits which could cause problems for people working against git. signature.asc Description: PGP signature
Re: [PATCH v6 3/6] regmap: Add regmap_noinc_read API
On Fri, Aug 10, 2018 at 11:46:20AM +0300, Stefan Popa wrote: > From: Crestez Dan Leonard > > The regmap API usually assumes that bulk read operations will read a > range of registers but some I2C/SPI devices have certain registers for > which a such a read operation will return data from an internal FIFO > instead. Add an explicit API to support bulk read without range semantics. Please don't resubmit patches that have already been applied, you should submit patches against current code in the tree you're expecting things to be applied to. If any updates are needed to a patch that's already been applied you should submit incremental patches which make those updates. This avoids having to change published git commits which could cause problems for people working against git. signature.asc Description: PGP signature
Re: [PATCH] x86, kdump: Fix efi=noruntime NULL pointer dereference
On 08/10/18 at 04:45pm, Dave Young wrote:
> On 08/08/18 at 04:03pm, Mike Galbraith wrote:
> > When booting with efi=noruntime, we call efi_runtime_map_copy() while
> > loading the kdump kernel, and trip over a NULL efi.memmap.map. Avoid
> > that and a useless allocation when the only mapping we can use (1:1)
> > is not available.
> >
> > Signed-off-by: Mike Galbraith
> > ---
> > arch/x86/kernel/kexec-bzimage64.c | 22 +++---
> > 1 file changed, 11 insertions(+), 11 deletions(-)
> >
> > --- a/arch/x86/kernel/kexec-bzimage64.c
> > +++ b/arch/x86/kernel/kexec-bzimage64.c
> > @@ -122,9 +122,6 @@ static int setup_efi_info_memmap(struct
> > unsigned long efi_map_phys_addr = params_load_addr + efi_map_offset;
> > struct efi_info *ei = >efi_info;
> >
> > - if (!efi_map_sz)
> > - return 0;
> > -
> > efi_runtime_map_copy(efi_map, efi_map_sz);
> >
> > ei->efi_memmap = efi_map_phys_addr & 0x;
> > @@ -176,7 +173,7 @@ setup_efi_state(struct boot_params *para
> > * acpi_rsdp= on kernel command line to make second kernel boot
> > * without efi.
> > */
> > - if (efi_enabled(EFI_OLD_MEMMAP))
> > + if (efi_enabled(EFI_OLD_MEMMAP) || !efi_enabled(EFI_MEMMAP))
> > return 0;
> >
> > ei->efi_loader_signature = current_ei->efi_loader_signature;
> > @@ -338,7 +335,7 @@ static void *bzImage64_load(struct kimag
> > struct kexec_entry64_regs regs64;
> > void *stack;
> > unsigned int setup_hdr_offset = offsetof(struct boot_params, hdr);
> > - unsigned int efi_map_offset, efi_map_sz, efi_setup_data_offset;
> > + unsigned int efi_map_offset = 0, efi_map_sz = 0, efi_setup_data_offset
> > = 0;
> > struct kexec_buf kbuf = { .image = image, .buf_max = ULONG_MAX,
> > .top_down = true };
> > struct kexec_buf pbuf = { .image = image, .buf_min = MIN_PURGATORY_ADDR,
> > @@ -397,19 +394,22 @@ static void *bzImage64_load(struct kimag
> > * have to create separate segment for each. Keeps things
> > * little bit simple
> > */
> > - efi_map_sz = efi_get_runtime_map_size();
> > params_cmdline_sz = sizeof(struct boot_params) + cmdline_len +
> > MAX_ELFCOREHDR_STR_LEN;
> > params_cmdline_sz = ALIGN(params_cmdline_sz, 16);
> > - kbuf.bufsz = params_cmdline_sz + ALIGN(efi_map_sz, 16) +
> > - sizeof(struct setup_data) +
> > - sizeof(struct efi_setup_data);
> > + kbuf.bufsz = params_cmdline_sz + sizeof(struct setup_data);
> > +
> > + /* Now add space for the efi stuff if we have a useable 1:1 mapping. */
> > + if (!efi_enabled(EFI_OLD_MEMMAP) && efi_enabled(EFI_MEMMAP)) {
> > + efi_map_sz = efi_get_runtime_map_size();
> > + kbuf.bufsz += ALIGN(efi_map_sz, 16) + sizeof(struct
> > efi_setup_data);
> > + efi_map_offset = params_cmdline_sz;
> > + efi_setup_data_offset = efi_map_offset + ALIGN(efi_map_sz, 16);
> > + }
> >
> > params = kzalloc(kbuf.bufsz, GFP_KERNEL);
> > if (!params)
> > return ERR_PTR(-ENOMEM);
> > - efi_map_offset = params_cmdline_sz;
> > - efi_setup_data_offset = efi_map_offset + ALIGN(efi_map_sz, 16);
> >
> > /* Copy setup header onto bootparams. Documentation/x86/boot.txt */
> > setup_header_size = 0x0202 + kernel[0x0201] - setup_hdr_offset;
>
> BTW, this patch only fix the kexec load phase problem, even if kexec
> load successfully with the fix, the 2nd kernel can not boot because efi
> memmap info is not correct and usable.
>
> So we should go with some fix similar to below, and do the cleanup we
> mentioned with a separate patch later.
>
> Also user space kexec-tools need a similar patch to error out in case
> no runtime maps. It would be good to fix both userspace and kernel
> load.
>
> diff --git a/arch/x86/kernel/kexec-bzimage64.c
> b/arch/x86/kernel/kexec-bzimage64.c
> index 7326078eaa7a..e34ba2f53cfb 100644
> --- a/arch/x86/kernel/kexec-bzimage64.c
> +++ b/arch/x86/kernel/kexec-bzimage64.c
> @@ -123,7 +123,7 @@ static int setup_efi_info_memmap(struct boot_params
> *params,
> struct efi_info *ei = >efi_info;
>
> if (!efi_map_sz)
> - return 0;
> + return -EINVAL;
>
> efi_runtime_map_copy(efi_map, efi_map_sz);
>
> @@ -166,9 +166,10 @@ setup_efi_state(struct boot_params *params, unsigned
> long params_load_addr,
> {
> struct efi_info *current_ei = _params.efi_info;
> struct efi_info *ei = >efi_info;
> + int ret;
>
> if (!current_ei->efi_memmap_size)
> - return 0;
> + return -EINVAL;
>
> /*
>* If 1:1 mapping is not enabled, second kernel can not setup EFI
> @@ -176,8 +177,8 @@ setup_efi_state(struct boot_params *params, unsigned long
> params_load_addr,
>* acpi_rsdp= on kernel command line to make second kernel boot
>* without efi.
>*/
> - if
Re: [PATCH] x86, kdump: Fix efi=noruntime NULL pointer dereference
On 08/10/18 at 04:45pm, Dave Young wrote:
> On 08/08/18 at 04:03pm, Mike Galbraith wrote:
> > When booting with efi=noruntime, we call efi_runtime_map_copy() while
> > loading the kdump kernel, and trip over a NULL efi.memmap.map. Avoid
> > that and a useless allocation when the only mapping we can use (1:1)
> > is not available.
> >
> > Signed-off-by: Mike Galbraith
> > ---
> > arch/x86/kernel/kexec-bzimage64.c | 22 +++---
> > 1 file changed, 11 insertions(+), 11 deletions(-)
> >
> > --- a/arch/x86/kernel/kexec-bzimage64.c
> > +++ b/arch/x86/kernel/kexec-bzimage64.c
> > @@ -122,9 +122,6 @@ static int setup_efi_info_memmap(struct
> > unsigned long efi_map_phys_addr = params_load_addr + efi_map_offset;
> > struct efi_info *ei = >efi_info;
> >
> > - if (!efi_map_sz)
> > - return 0;
> > -
> > efi_runtime_map_copy(efi_map, efi_map_sz);
> >
> > ei->efi_memmap = efi_map_phys_addr & 0x;
> > @@ -176,7 +173,7 @@ setup_efi_state(struct boot_params *para
> > * acpi_rsdp= on kernel command line to make second kernel boot
> > * without efi.
> > */
> > - if (efi_enabled(EFI_OLD_MEMMAP))
> > + if (efi_enabled(EFI_OLD_MEMMAP) || !efi_enabled(EFI_MEMMAP))
> > return 0;
> >
> > ei->efi_loader_signature = current_ei->efi_loader_signature;
> > @@ -338,7 +335,7 @@ static void *bzImage64_load(struct kimag
> > struct kexec_entry64_regs regs64;
> > void *stack;
> > unsigned int setup_hdr_offset = offsetof(struct boot_params, hdr);
> > - unsigned int efi_map_offset, efi_map_sz, efi_setup_data_offset;
> > + unsigned int efi_map_offset = 0, efi_map_sz = 0, efi_setup_data_offset
> > = 0;
> > struct kexec_buf kbuf = { .image = image, .buf_max = ULONG_MAX,
> > .top_down = true };
> > struct kexec_buf pbuf = { .image = image, .buf_min = MIN_PURGATORY_ADDR,
> > @@ -397,19 +394,22 @@ static void *bzImage64_load(struct kimag
> > * have to create separate segment for each. Keeps things
> > * little bit simple
> > */
> > - efi_map_sz = efi_get_runtime_map_size();
> > params_cmdline_sz = sizeof(struct boot_params) + cmdline_len +
> > MAX_ELFCOREHDR_STR_LEN;
> > params_cmdline_sz = ALIGN(params_cmdline_sz, 16);
> > - kbuf.bufsz = params_cmdline_sz + ALIGN(efi_map_sz, 16) +
> > - sizeof(struct setup_data) +
> > - sizeof(struct efi_setup_data);
> > + kbuf.bufsz = params_cmdline_sz + sizeof(struct setup_data);
> > +
> > + /* Now add space for the efi stuff if we have a useable 1:1 mapping. */
> > + if (!efi_enabled(EFI_OLD_MEMMAP) && efi_enabled(EFI_MEMMAP)) {
> > + efi_map_sz = efi_get_runtime_map_size();
> > + kbuf.bufsz += ALIGN(efi_map_sz, 16) + sizeof(struct
> > efi_setup_data);
> > + efi_map_offset = params_cmdline_sz;
> > + efi_setup_data_offset = efi_map_offset + ALIGN(efi_map_sz, 16);
> > + }
> >
> > params = kzalloc(kbuf.bufsz, GFP_KERNEL);
> > if (!params)
> > return ERR_PTR(-ENOMEM);
> > - efi_map_offset = params_cmdline_sz;
> > - efi_setup_data_offset = efi_map_offset + ALIGN(efi_map_sz, 16);
> >
> > /* Copy setup header onto bootparams. Documentation/x86/boot.txt */
> > setup_header_size = 0x0202 + kernel[0x0201] - setup_hdr_offset;
>
> BTW, this patch only fix the kexec load phase problem, even if kexec
> load successfully with the fix, the 2nd kernel can not boot because efi
> memmap info is not correct and usable.
>
> So we should go with some fix similar to below, and do the cleanup we
> mentioned with a separate patch later.
>
> Also user space kexec-tools need a similar patch to error out in case
> no runtime maps. It would be good to fix both userspace and kernel
> load.
>
> diff --git a/arch/x86/kernel/kexec-bzimage64.c
> b/arch/x86/kernel/kexec-bzimage64.c
> index 7326078eaa7a..e34ba2f53cfb 100644
> --- a/arch/x86/kernel/kexec-bzimage64.c
> +++ b/arch/x86/kernel/kexec-bzimage64.c
> @@ -123,7 +123,7 @@ static int setup_efi_info_memmap(struct boot_params
> *params,
> struct efi_info *ei = >efi_info;
>
> if (!efi_map_sz)
> - return 0;
> + return -EINVAL;
>
> efi_runtime_map_copy(efi_map, efi_map_sz);
>
> @@ -166,9 +166,10 @@ setup_efi_state(struct boot_params *params, unsigned
> long params_load_addr,
> {
> struct efi_info *current_ei = _params.efi_info;
> struct efi_info *ei = >efi_info;
> + int ret;
>
> if (!current_ei->efi_memmap_size)
> - return 0;
> + return -EINVAL;
>
> /*
>* If 1:1 mapping is not enabled, second kernel can not setup EFI
> @@ -176,8 +177,8 @@ setup_efi_state(struct boot_params *params, unsigned long
> params_load_addr,
>* acpi_rsdp= on kernel command line to make second kernel boot
>* without efi.
>*/
> - if
Re: [PATCH 01/16] MAINTAINERS: Switch a maintainer for drivers/staging/gasket
On Thu, Aug 9, 2018 at 11:14 PM Greg Kroah-Hartman wrote: > > On Thu, Aug 09, 2018 at 08:40:06PM -0700, John Joseph wrote: > > Acked-by: John Joseph > > Why are you acking something you supposidly already signed-off on? Sorry, my fault, wasn't sure what the protocol was for these so I suggested he send an Acked-by as well, can drop that. > > > On Thu, Aug 9, 2018 at 8:20 PM, Todd Poynor wrote: > > > From: Todd Poynor > > > > > > Todd Poynor takes over for John Joseph. > > > > > > Signed-off-by: John Joseph > > > Signed-off-by: Todd Poynor > > Did you not really provide your signed-off-by here? We generated this together, the S-o-b is valid, > > totally confused, > > greg k-h
Re: [PATCH 01/16] MAINTAINERS: Switch a maintainer for drivers/staging/gasket
On Thu, Aug 9, 2018 at 11:14 PM Greg Kroah-Hartman wrote: > > On Thu, Aug 09, 2018 at 08:40:06PM -0700, John Joseph wrote: > > Acked-by: John Joseph > > Why are you acking something you supposidly already signed-off on? Sorry, my fault, wasn't sure what the protocol was for these so I suggested he send an Acked-by as well, can drop that. > > > On Thu, Aug 9, 2018 at 8:20 PM, Todd Poynor wrote: > > > From: Todd Poynor > > > > > > Todd Poynor takes over for John Joseph. > > > > > > Signed-off-by: John Joseph > > > Signed-off-by: Todd Poynor > > Did you not really provide your signed-off-by here? We generated this together, the S-o-b is valid, > > totally confused, > > greg k-h
Re: [RFC v7 PATCH 1/4] mm: refactor do_munmap() to extract the common part
On 08/10/2018 01:36 AM, Yang Shi wrote:
> Introduces three new helper functions:
> * addr_ok()
> * munmap_lookup_vma()
> * munlock_vmas()
>
> They will be used by do_munmap() and the new do_munmap with zapping
> large mapping early in the later patch.
>
> There is no functional change, just code refactor.
>
> Reviewed-by: Laurent Dufour
> Signed-off-by: Yang Shi
Acked-by: Vlastimil Babka
Small nit below.
> @@ -2764,13 +2812,7 @@ int do_munmap(struct mm_struct *mm, unsigned long
> start, size_t len,
>*/
> if (mm->locked_vm) {
> struct vm_area_struct *tmp = vma;
> - while (tmp && tmp->vm_start < end) {
> - if (tmp->vm_flags & VM_LOCKED) {
> - mm->locked_vm -= vma_pages(tmp);
> - munlock_vma_pages_all(tmp);
> - }
> - tmp = tmp->vm_next;
> - }
> + munlock_vmas(tmp, end);
No need for 'tmp' here.
> }
>
> /*
>
Re: [RFC v7 PATCH 1/4] mm: refactor do_munmap() to extract the common part
On 08/10/2018 01:36 AM, Yang Shi wrote:
> Introduces three new helper functions:
> * addr_ok()
> * munmap_lookup_vma()
> * munlock_vmas()
>
> They will be used by do_munmap() and the new do_munmap with zapping
> large mapping early in the later patch.
>
> There is no functional change, just code refactor.
>
> Reviewed-by: Laurent Dufour
> Signed-off-by: Yang Shi
Acked-by: Vlastimil Babka
Small nit below.
> @@ -2764,13 +2812,7 @@ int do_munmap(struct mm_struct *mm, unsigned long
> start, size_t len,
>*/
> if (mm->locked_vm) {
> struct vm_area_struct *tmp = vma;
> - while (tmp && tmp->vm_start < end) {
> - if (tmp->vm_flags & VM_LOCKED) {
> - mm->locked_vm -= vma_pages(tmp);
> - munlock_vma_pages_all(tmp);
> - }
> - tmp = tmp->vm_next;
> - }
> + munlock_vmas(tmp, end);
No need for 'tmp' here.
> }
>
> /*
>
Re: [PATCH] x86, kdump: Fix efi=noruntime NULL pointer dereference
On Fri, 2018-08-10 at 16:45 +0800, Dave Young wrote:
>
> BTW, this patch only fix the kexec load phase problem, even if kexec
> load successfully with the fix, the 2nd kernel can not boot because efi
> memmap info is not correct and usable.
Hm. I didn't do anything else with kexec, but did crashdump my box
both w/wo efi=noruntime.
> So we should go with some fix similar to below, and do the cleanup we
> mentioned with a separate patch later.
Ah, you mean the one I had _just_ built when I saw this :)
> Also user space kexec-tools need a similar patch to error out in case
> no runtime maps. It would be good to fix both userspace and kernel
> load.
>
> diff --git a/arch/x86/kernel/kexec-bzimage64.c
> b/arch/x86/kernel/kexec-bzimage64.c
> index 7326078eaa7a..e34ba2f53cfb 100644
> --- a/arch/x86/kernel/kexec-bzimage64.c
> +++ b/arch/x86/kernel/kexec-bzimage64.c
> @@ -123,7 +123,7 @@ static int setup_efi_info_memmap(struct boot_params
> *params,
> struct efi_info *ei = >efi_info;
>
> if (!efi_map_sz)
> - return 0;
> + return -EINVAL;
>
> efi_runtime_map_copy(efi_map, efi_map_sz);
>
> @@ -166,9 +166,10 @@ setup_efi_state(struct boot_params *params, unsigned
> long params_load_addr,
> {
> struct efi_info *current_ei = _params.efi_info;
> struct efi_info *ei = >efi_info;
> + int ret;
>
> if (!current_ei->efi_memmap_size)
> - return 0;
> + return -EINVAL;
>
> /*
>* If 1:1 mapping is not enabled, second kernel can not setup EFI
> @@ -176,8 +177,8 @@ setup_efi_state(struct boot_params *params, unsigned long
> params_load_addr,
>* acpi_rsdp= on kernel command line to make second kernel boot
>* without efi.
>*/
> - if (efi_enabled(EFI_OLD_MEMMAP))
> - return 0;
> + if (efi_enabled(EFI_OLD_MEMMAP) || !efi_enabled(EFI_RUNTIME_SERVICES))
> + return -ENODEV;
>
> ei->efi_loader_signature = current_ei->efi_loader_signature;
> ei->efi_systab = current_ei->efi_systab;
> @@ -186,8 +187,10 @@ setup_efi_state(struct boot_params *params, unsigned
> long params_load_addr,
> ei->efi_memdesc_version = current_ei->efi_memdesc_version;
> ei->efi_memdesc_size = efi_get_runtime_map_desc_size();
>
> - setup_efi_info_memmap(params, params_load_addr, efi_map_offset,
> + ret = setup_efi_info_memmap(params, params_load_addr, efi_map_offset,
> efi_map_sz);
> + if (ret)
> + return ret;
> prepare_add_efi_setup_data(params, params_load_addr,
> efi_setup_data_offset);
> return 0;
> @@ -250,8 +253,10 @@ setup_boot_parameters(struct kimage *image, struct
> boot_params *params,
>
> #ifdef CONFIG_EFI
> /* Setup EFI state */
> - setup_efi_state(params, params_load_addr, efi_map_offset, efi_map_sz,
> + ret = setup_efi_state(params, params_load_addr, efi_map_offset,
> efi_map_sz,
> efi_setup_data_offset);
> + if (ret)
> + return ret;
> #endif
>
> /* Setup EDD info */
Re: [PATCH] x86, kdump: Fix efi=noruntime NULL pointer dereference
On Fri, 2018-08-10 at 16:45 +0800, Dave Young wrote:
>
> BTW, this patch only fix the kexec load phase problem, even if kexec
> load successfully with the fix, the 2nd kernel can not boot because efi
> memmap info is not correct and usable.
Hm. I didn't do anything else with kexec, but did crashdump my box
both w/wo efi=noruntime.
> So we should go with some fix similar to below, and do the cleanup we
> mentioned with a separate patch later.
Ah, you mean the one I had _just_ built when I saw this :)
> Also user space kexec-tools need a similar patch to error out in case
> no runtime maps. It would be good to fix both userspace and kernel
> load.
>
> diff --git a/arch/x86/kernel/kexec-bzimage64.c
> b/arch/x86/kernel/kexec-bzimage64.c
> index 7326078eaa7a..e34ba2f53cfb 100644
> --- a/arch/x86/kernel/kexec-bzimage64.c
> +++ b/arch/x86/kernel/kexec-bzimage64.c
> @@ -123,7 +123,7 @@ static int setup_efi_info_memmap(struct boot_params
> *params,
> struct efi_info *ei = >efi_info;
>
> if (!efi_map_sz)
> - return 0;
> + return -EINVAL;
>
> efi_runtime_map_copy(efi_map, efi_map_sz);
>
> @@ -166,9 +166,10 @@ setup_efi_state(struct boot_params *params, unsigned
> long params_load_addr,
> {
> struct efi_info *current_ei = _params.efi_info;
> struct efi_info *ei = >efi_info;
> + int ret;
>
> if (!current_ei->efi_memmap_size)
> - return 0;
> + return -EINVAL;
>
> /*
>* If 1:1 mapping is not enabled, second kernel can not setup EFI
> @@ -176,8 +177,8 @@ setup_efi_state(struct boot_params *params, unsigned long
> params_load_addr,
>* acpi_rsdp= on kernel command line to make second kernel boot
>* without efi.
>*/
> - if (efi_enabled(EFI_OLD_MEMMAP))
> - return 0;
> + if (efi_enabled(EFI_OLD_MEMMAP) || !efi_enabled(EFI_RUNTIME_SERVICES))
> + return -ENODEV;
>
> ei->efi_loader_signature = current_ei->efi_loader_signature;
> ei->efi_systab = current_ei->efi_systab;
> @@ -186,8 +187,10 @@ setup_efi_state(struct boot_params *params, unsigned
> long params_load_addr,
> ei->efi_memdesc_version = current_ei->efi_memdesc_version;
> ei->efi_memdesc_size = efi_get_runtime_map_desc_size();
>
> - setup_efi_info_memmap(params, params_load_addr, efi_map_offset,
> + ret = setup_efi_info_memmap(params, params_load_addr, efi_map_offset,
> efi_map_sz);
> + if (ret)
> + return ret;
> prepare_add_efi_setup_data(params, params_load_addr,
> efi_setup_data_offset);
> return 0;
> @@ -250,8 +253,10 @@ setup_boot_parameters(struct kimage *image, struct
> boot_params *params,
>
> #ifdef CONFIG_EFI
> /* Setup EFI state */
> - setup_efi_state(params, params_load_addr, efi_map_offset, efi_map_sz,
> + ret = setup_efi_state(params, params_load_addr, efi_map_offset,
> efi_map_sz,
> efi_setup_data_offset);
> + if (ret)
> + return ret;
> #endif
>
> /* Setup EDD info */
Re: Linux 3.18.111
On Fri, Aug 10, 2018 at 03:43:02PM +0900, Seung-Woo Kim wrote:
> On 2018년 08월 08일 19:06, Seung-Woo Kim wrote:
> > On 2018년 07월 05일 09:52, Al Viro wrote:
> >> On Mon, Jul 02, 2018 at 10:01:25PM -0700, Linus Torvalds wrote:
> >>> On Mon, Jul 2, 2018 at 9:43 PM Seung-Woo Kim
> >>> wrote:
>
> I think the commit itself is required. Simple, but not reliable,
> workaround fix is like below:
>
> diff --git a/fs/dcache.c b/fs/dcache.c
> index a34d401..7c751f2 100644
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -1879,6 +1879,8 @@ void d_instantiate_new(struct dentry *entry,
> struct inode *inode)
> BUG_ON(!hlist_unhashed(>d_u.d_alias));
> BUG_ON(!inode);
> lockdep_annotate_inode_mutex_key(inode);
> + /* WORKAROUND for calling security_d_instantiate() */
> + entry->d_inode = inode;
> security_d_instantiate(entry, inode);
> spin_lock(>i_lock);
> __d_instantiate(entry, inode);
> >>>
> >>> Ugh. That looks horrible even if it might avoid the oops.
> >>>
> >>> I think a much better solution is to back-port commit b296821a7c42
> >>> ("xattr_handler: pass dentry and inode as separate arguments of
> >>> ->get()") to older kernels. Then the inode is passed down all the way,
> >>> and you don't have people try to get it from the (not yet initialized)
> >>> dentry.
> >>>
> >>> But there might be other parts missing too, and I didn't look at how
> >>> easy/painful that backport would be.
> >>>
> >>> Al - comments? This is all because of commit 1e2e547a93a0 ("do
> >>> d_instantiate/unlock_new_inode combinations safely") being marked for
> >>> stable, and various cases of security_d_instantiate() calling down to
> >>> getxattr. Which used to not get the inode at all, so those older
> >>> kernels use d_inode(dentry), which doesn't work in this path since
> >>> dentry->d_inode hasn't been instantiated yet..
> >>
> >> You also want b96809173e94 and ce23e6401334 there...
> >
> > For above two commits, also b296821a7c42 is required. And after
> > backport, smack still crashed because setxattr. To fix it, 5930122683df
> > and 3767e255b390 are also required.
> >
> > By the way, does no one have met this kind getxattr crash issue with
> > selinux from 3.18.y?
> >
>
> I have checked with selinux, and it is confirmed that there is no crash
> because selinux_d_instantiate() has null check for inode. So, it is only
> security smack issue.
So are the 5 patches you sent ok to apply to the 3.18-stable tree? Or
do we need to do something else?
thanks,
greg k-h
Re: Linux 3.18.111
On Fri, Aug 10, 2018 at 03:43:02PM +0900, Seung-Woo Kim wrote:
> On 2018년 08월 08일 19:06, Seung-Woo Kim wrote:
> > On 2018년 07월 05일 09:52, Al Viro wrote:
> >> On Mon, Jul 02, 2018 at 10:01:25PM -0700, Linus Torvalds wrote:
> >>> On Mon, Jul 2, 2018 at 9:43 PM Seung-Woo Kim
> >>> wrote:
>
> I think the commit itself is required. Simple, but not reliable,
> workaround fix is like below:
>
> diff --git a/fs/dcache.c b/fs/dcache.c
> index a34d401..7c751f2 100644
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -1879,6 +1879,8 @@ void d_instantiate_new(struct dentry *entry,
> struct inode *inode)
> BUG_ON(!hlist_unhashed(>d_u.d_alias));
> BUG_ON(!inode);
> lockdep_annotate_inode_mutex_key(inode);
> + /* WORKAROUND for calling security_d_instantiate() */
> + entry->d_inode = inode;
> security_d_instantiate(entry, inode);
> spin_lock(>i_lock);
> __d_instantiate(entry, inode);
> >>>
> >>> Ugh. That looks horrible even if it might avoid the oops.
> >>>
> >>> I think a much better solution is to back-port commit b296821a7c42
> >>> ("xattr_handler: pass dentry and inode as separate arguments of
> >>> ->get()") to older kernels. Then the inode is passed down all the way,
> >>> and you don't have people try to get it from the (not yet initialized)
> >>> dentry.
> >>>
> >>> But there might be other parts missing too, and I didn't look at how
> >>> easy/painful that backport would be.
> >>>
> >>> Al - comments? This is all because of commit 1e2e547a93a0 ("do
> >>> d_instantiate/unlock_new_inode combinations safely") being marked for
> >>> stable, and various cases of security_d_instantiate() calling down to
> >>> getxattr. Which used to not get the inode at all, so those older
> >>> kernels use d_inode(dentry), which doesn't work in this path since
> >>> dentry->d_inode hasn't been instantiated yet..
> >>
> >> You also want b96809173e94 and ce23e6401334 there...
> >
> > For above two commits, also b296821a7c42 is required. And after
> > backport, smack still crashed because setxattr. To fix it, 5930122683df
> > and 3767e255b390 are also required.
> >
> > By the way, does no one have met this kind getxattr crash issue with
> > selinux from 3.18.y?
> >
>
> I have checked with selinux, and it is confirmed that there is no crash
> because selinux_d_instantiate() has null check for inode. So, it is only
> security smack issue.
So are the 5 patches you sent ok to apply to the 3.18-stable tree? Or
do we need to do something else?
thanks,
greg k-h
Re: [PATCH v3 0/4] pci-dra7xx: Enable errata i870 workaround for RC mode
On Wednesday 08 August 2018 10:27 PM, Lorenzo Pieralisi wrote: > On Tue, Jul 24, 2018 at 11:01:46PM +0530, Vignesh R wrote: >> Make workaround for errata i870 applicable in Host mode as >> well(previously it was enabled only for EP mode) as per errata >> documentation: http://www.ti.com/lit/er/sprz450/sprz450.pdf >> >> Tested on DRA72 EVM >> >> Tony, >> >> If you are okay with the series, could you pick this via omap tree? >> All ACKs are in place and Lorenzo is okay with PCIe bits to go along with >> rest of DTS changes. > > I think we have missed the v4.19 merge window by now - Right. I didn't get any response from Tony. > please let me know if I can drop this series from the PCI patch queue. > Ok, I will resend the patch after 4.19-rc. Thanks! Regards Vignesh > Thanks, > Lorenzo > >> Regards >> Vignesh >> >> >> Changes since v2: >> Reorder patch 2 to appear at the last. >> Collect all the ACKs >> >> Changes since v1: >> Drop IRQ handling rework (will be sent out separately) >> >> v2: https://patchwork.ozlabs.org/cover/935454/ >> v1: https://lkml.org/lkml/2017/12/1/59 >> >> >> Vignesh R (4): >> dt-bindings: PCI: dra7xx: Add bindings for unaligned access in host >> mode >> ARM: dts: dra7: Enable workaround for errata i870 in PCIe host mode >> ARM: dts: dra7: Fix up unaligned access setting for PCIe EP >> pci: dwc: pci-dra7xx: Enable errata i870 for both EP and RC mode >> >> Documentation/devicetree/bindings/pci/ti-pci.txt | 5 + >> arch/arm/boot/dts/dra7.dtsi | 4 +++- >> drivers/pci/controller/dwc/pci-dra7xx.c | 12 ++-- >> 3 files changed, 14 insertions(+), 7 deletions(-) >> >> -- >> 2.18.0 >>
Re: [PATCH v3 0/4] pci-dra7xx: Enable errata i870 workaround for RC mode
On Wednesday 08 August 2018 10:27 PM, Lorenzo Pieralisi wrote: > On Tue, Jul 24, 2018 at 11:01:46PM +0530, Vignesh R wrote: >> Make workaround for errata i870 applicable in Host mode as >> well(previously it was enabled only for EP mode) as per errata >> documentation: http://www.ti.com/lit/er/sprz450/sprz450.pdf >> >> Tested on DRA72 EVM >> >> Tony, >> >> If you are okay with the series, could you pick this via omap tree? >> All ACKs are in place and Lorenzo is okay with PCIe bits to go along with >> rest of DTS changes. > > I think we have missed the v4.19 merge window by now - Right. I didn't get any response from Tony. > please let me know if I can drop this series from the PCI patch queue. > Ok, I will resend the patch after 4.19-rc. Thanks! Regards Vignesh > Thanks, > Lorenzo > >> Regards >> Vignesh >> >> >> Changes since v2: >> Reorder patch 2 to appear at the last. >> Collect all the ACKs >> >> Changes since v1: >> Drop IRQ handling rework (will be sent out separately) >> >> v2: https://patchwork.ozlabs.org/cover/935454/ >> v1: https://lkml.org/lkml/2017/12/1/59 >> >> >> Vignesh R (4): >> dt-bindings: PCI: dra7xx: Add bindings for unaligned access in host >> mode >> ARM: dts: dra7: Enable workaround for errata i870 in PCIe host mode >> ARM: dts: dra7: Fix up unaligned access setting for PCIe EP >> pci: dwc: pci-dra7xx: Enable errata i870 for both EP and RC mode >> >> Documentation/devicetree/bindings/pci/ti-pci.txt | 5 + >> arch/arm/boot/dts/dra7.dtsi | 4 +++- >> drivers/pci/controller/dwc/pci-dra7xx.c | 12 ++-- >> 3 files changed, 14 insertions(+), 7 deletions(-) >> >> -- >> 2.18.0 >>
[PATCH 0/2] clk: meson-g12a: Add AO clock controller driver
Add a Clock driver for Always-On part of the Amlogic Meson-G12A SoC. -The patch depends on "clk: meson-g12a: Add EE clock controller driver" patch [1] the parent clock of ao_clk81 is clk81 clock which provided in EE controller driver; the file of Makefile modify based on EE controller driver patch. [1]https://lkml.kernel.org/r/1533890858-113020-3-git-send-email-jian...@amlogic.com Jian Hu (2): dt-bindings: clk: meson-g12a: Add G12A AO Clock Bindings clk: meson-g12a: Add AO Clock controller driver .../bindings/clock/amlogic,gxbb-aoclkc.txt | 1 + drivers/clk/meson/Makefile | 2 +- drivers/clk/meson/g12a-aoclk.c | 170 + drivers/clk/meson/g12a-aoclk.h | 36 + include/dt-bindings/clock/g12a-aoclkc.h| 28 5 files changed, 236 insertions(+), 1 deletion(-) create mode 100644 drivers/clk/meson/g12a-aoclk.c create mode 100644 drivers/clk/meson/g12a-aoclk.h create mode 100755 include/dt-bindings/clock/g12a-aoclkc.h -- 1.9.1
[PATCH 0/2] clk: meson-g12a: Add AO clock controller driver
Add a Clock driver for Always-On part of the Amlogic Meson-G12A SoC. -The patch depends on "clk: meson-g12a: Add EE clock controller driver" patch [1] the parent clock of ao_clk81 is clk81 clock which provided in EE controller driver; the file of Makefile modify based on EE controller driver patch. [1]https://lkml.kernel.org/r/1533890858-113020-3-git-send-email-jian...@amlogic.com Jian Hu (2): dt-bindings: clk: meson-g12a: Add G12A AO Clock Bindings clk: meson-g12a: Add AO Clock controller driver .../bindings/clock/amlogic,gxbb-aoclkc.txt | 1 + drivers/clk/meson/Makefile | 2 +- drivers/clk/meson/g12a-aoclk.c | 170 + drivers/clk/meson/g12a-aoclk.h | 36 + include/dt-bindings/clock/g12a-aoclkc.h| 28 5 files changed, 236 insertions(+), 1 deletion(-) create mode 100644 drivers/clk/meson/g12a-aoclk.c create mode 100644 drivers/clk/meson/g12a-aoclk.h create mode 100755 include/dt-bindings/clock/g12a-aoclkc.h -- 1.9.1
[PATCH 2/2] clk: meson-g12a: Add AO Clock controller driver
Add a Clock driver for the ALways-On part
of the Amlogic Meson-G12A SoC.
Signed-off-by: Jian Hu
---
drivers/clk/meson/Makefile | 2 +-
drivers/clk/meson/g12a-aoclk.c | 170 +
drivers/clk/meson/g12a-aoclk.h | 36 +
3 files changed, 207 insertions(+), 1 deletion(-)
create mode 100644 drivers/clk/meson/g12a-aoclk.c
create mode 100644 drivers/clk/meson/g12a-aoclk.h
diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
index 2b1a562..d5c2dcd 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -9,5 +9,5 @@ obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o
obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o gxbb-aoclk-32k.o
obj-$(CONFIG_COMMON_CLK_AXG)+= axg.o axg-aoclk.o
obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o
-obj-$(CONFIG_COMMON_CLK_G12A) += g12a.o
+obj-$(CONFIG_COMMON_CLK_G12A) += g12a.o g12a-aoclk.c
obj-$(CONFIG_COMMON_CLK_REGMAP_MESON) += clk-regmap.o
diff --git a/drivers/clk/meson/g12a-aoclk.c b/drivers/clk/meson/g12a-aoclk.c
new file mode 100644
index 000..a5cd95c
--- /dev/null
+++ b/drivers/clk/meson/g12a-aoclk.c
@@ -0,0 +1,170 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Amlogic Meson-G12A Clock Controller Driver
+ *
+ * Copyright (c) 2016 Baylibre SAS.
+ * Author: Michael Turquette
+ *
+ * Copyright (c) 2018 Amlogic, inc.
+ * Author: Jian Hu
+ */
+#include
+#include
+#include
+#include
+#include
+#include "clkc.h"
+#include "g12a-aoclk.h"
+
+#define G12A_AO_GATE0(_name, _bit) \
+static struct clk_regmap _name##_ao = {
\
+ .data = &(struct clk_regmap_gate_data) {\
+ .offset = (AO_CLK_GATE0), \
+ .bit_idx = (_bit), \
+ }, \
+ .hw.init = &(struct clk_init_data) {\
+ .name = #_name "_ao", \
+ .ops = _regmap_gate_ops,\
+ .parent_names = (const char *[]){ "clk81" },\
+ .num_parents = 1, \
+ }, \
+}
+
+G12A_AO_GATE0(ahb_bus, 0);
+G12A_AO_GATE0(remote, 1);
+G12A_AO_GATE0(i2c_master, 2);
+G12A_AO_GATE0(i2c_slave, 3);
+G12A_AO_GATE0(uart1, 4);
+G12A_AO_GATE0(prod_i2c,5);
+G12A_AO_GATE0(uart2, 6);
+G12A_AO_GATE0(ir_blaster, 7);
+G12A_AO_GATE0(saradc, 8);
+
+static struct clk_regmap ao_clk81 = {
+ .data = &(struct clk_regmap_mux_data) {
+ .offset = AO_RTI_PWR_CNTL_REG0,
+ .mask = 0x1,
+ .shift = 8,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "ao_clk81",
+ .ops = _regmap_mux_ro_ops,
+ .parent_names = (const char *[]){ "clk81", "ao_alt_xtal"},
+ .num_parents = 2,
+ },
+};
+
+static struct clk_regmap g12a_saradc_mux = {
+ .data = &(struct clk_regmap_mux_data) {
+ .offset = AO_SAR_CLK,
+ .mask = 0x3,
+ .shift = 9,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "g12a_saradc_mux",
+ .ops = _regmap_mux_ops,
+ .parent_names = (const char *[]){ "xtal", "ao_clk81" },
+ .num_parents = 2,
+ },
+};
+
+static struct clk_regmap g12a_saradc_div = {
+ .data = &(struct clk_regmap_div_data) {
+ .offset = AO_SAR_CLK,
+ .shift = 0,
+ .width = 8,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "g12a_saradc_div",
+ .ops = _regmap_divider_ops,
+ .parent_names = (const char *[]){ "g12a_saradc_mux" },
+ .num_parents = 1,
+ .flags = CLK_SET_RATE_PARENT,
+ },
+};
+
+static struct clk_regmap g12a_saradc_gate = {
+ .data = &(struct clk_regmap_gate_data) {
+ .offset = AO_SAR_CLK,
+ .bit_idx = 8,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "g12a_saradc_gate",
+ .ops = _regmap_gate_ops,
+ .parent_names = (const char *[]){ "g12a_saradc_div" },
+ .num_parents = 1,
+ .flags = CLK_SET_RATE_PARENT,
+ },
+};
+
+static unsigned int g12a_aoclk_reset[] = {
+ [RESET_AO_REMOTE] = 16,
+ [RESET_AO_UART1] = 17,
+ [RESET_AO_I2C_MASTER] = 18,
+ [RESET_AO_I2C_SLAVE] = 19,
+ [RESET_AO_SARADC] = 20,
+ [RESET_AO_UART2] = 22,
+ [RESET_AO_IR_BLASTER] = 23,
+};
+
+static struct clk_regmap *g12a_aoclk_regmap[] = {
+ [CLKID_AO_AHB_BUS] =
[PATCH 2/2] clk: meson-g12a: Add AO Clock controller driver
Add a Clock driver for the ALways-On part
of the Amlogic Meson-G12A SoC.
Signed-off-by: Jian Hu
---
drivers/clk/meson/Makefile | 2 +-
drivers/clk/meson/g12a-aoclk.c | 170 +
drivers/clk/meson/g12a-aoclk.h | 36 +
3 files changed, 207 insertions(+), 1 deletion(-)
create mode 100644 drivers/clk/meson/g12a-aoclk.c
create mode 100644 drivers/clk/meson/g12a-aoclk.h
diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
index 2b1a562..d5c2dcd 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -9,5 +9,5 @@ obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o
obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o gxbb-aoclk-32k.o
obj-$(CONFIG_COMMON_CLK_AXG)+= axg.o axg-aoclk.o
obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o
-obj-$(CONFIG_COMMON_CLK_G12A) += g12a.o
+obj-$(CONFIG_COMMON_CLK_G12A) += g12a.o g12a-aoclk.c
obj-$(CONFIG_COMMON_CLK_REGMAP_MESON) += clk-regmap.o
diff --git a/drivers/clk/meson/g12a-aoclk.c b/drivers/clk/meson/g12a-aoclk.c
new file mode 100644
index 000..a5cd95c
--- /dev/null
+++ b/drivers/clk/meson/g12a-aoclk.c
@@ -0,0 +1,170 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Amlogic Meson-G12A Clock Controller Driver
+ *
+ * Copyright (c) 2016 Baylibre SAS.
+ * Author: Michael Turquette
+ *
+ * Copyright (c) 2018 Amlogic, inc.
+ * Author: Jian Hu
+ */
+#include
+#include
+#include
+#include
+#include
+#include "clkc.h"
+#include "g12a-aoclk.h"
+
+#define G12A_AO_GATE0(_name, _bit) \
+static struct clk_regmap _name##_ao = {
\
+ .data = &(struct clk_regmap_gate_data) {\
+ .offset = (AO_CLK_GATE0), \
+ .bit_idx = (_bit), \
+ }, \
+ .hw.init = &(struct clk_init_data) {\
+ .name = #_name "_ao", \
+ .ops = _regmap_gate_ops,\
+ .parent_names = (const char *[]){ "clk81" },\
+ .num_parents = 1, \
+ }, \
+}
+
+G12A_AO_GATE0(ahb_bus, 0);
+G12A_AO_GATE0(remote, 1);
+G12A_AO_GATE0(i2c_master, 2);
+G12A_AO_GATE0(i2c_slave, 3);
+G12A_AO_GATE0(uart1, 4);
+G12A_AO_GATE0(prod_i2c,5);
+G12A_AO_GATE0(uart2, 6);
+G12A_AO_GATE0(ir_blaster, 7);
+G12A_AO_GATE0(saradc, 8);
+
+static struct clk_regmap ao_clk81 = {
+ .data = &(struct clk_regmap_mux_data) {
+ .offset = AO_RTI_PWR_CNTL_REG0,
+ .mask = 0x1,
+ .shift = 8,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "ao_clk81",
+ .ops = _regmap_mux_ro_ops,
+ .parent_names = (const char *[]){ "clk81", "ao_alt_xtal"},
+ .num_parents = 2,
+ },
+};
+
+static struct clk_regmap g12a_saradc_mux = {
+ .data = &(struct clk_regmap_mux_data) {
+ .offset = AO_SAR_CLK,
+ .mask = 0x3,
+ .shift = 9,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "g12a_saradc_mux",
+ .ops = _regmap_mux_ops,
+ .parent_names = (const char *[]){ "xtal", "ao_clk81" },
+ .num_parents = 2,
+ },
+};
+
+static struct clk_regmap g12a_saradc_div = {
+ .data = &(struct clk_regmap_div_data) {
+ .offset = AO_SAR_CLK,
+ .shift = 0,
+ .width = 8,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "g12a_saradc_div",
+ .ops = _regmap_divider_ops,
+ .parent_names = (const char *[]){ "g12a_saradc_mux" },
+ .num_parents = 1,
+ .flags = CLK_SET_RATE_PARENT,
+ },
+};
+
+static struct clk_regmap g12a_saradc_gate = {
+ .data = &(struct clk_regmap_gate_data) {
+ .offset = AO_SAR_CLK,
+ .bit_idx = 8,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "g12a_saradc_gate",
+ .ops = _regmap_gate_ops,
+ .parent_names = (const char *[]){ "g12a_saradc_div" },
+ .num_parents = 1,
+ .flags = CLK_SET_RATE_PARENT,
+ },
+};
+
+static unsigned int g12a_aoclk_reset[] = {
+ [RESET_AO_REMOTE] = 16,
+ [RESET_AO_UART1] = 17,
+ [RESET_AO_I2C_MASTER] = 18,
+ [RESET_AO_I2C_SLAVE] = 19,
+ [RESET_AO_SARADC] = 20,
+ [RESET_AO_UART2] = 22,
+ [RESET_AO_IR_BLASTER] = 23,
+};
+
+static struct clk_regmap *g12a_aoclk_regmap[] = {
+ [CLKID_AO_AHB_BUS] =
[PATCH 1/2] dt-bindings: clk: meson-g12a: Add G12A AO Clock Bindings
Add new clock controller compatible and dt-bingdings headers for the Always-On domain of the g12a SoC Signed-off-by: Jian Hu --- .../bindings/clock/amlogic,gxbb-aoclkc.txt | 1 + include/dt-bindings/clock/g12a-aoclkc.h| 28 ++ 2 files changed, 29 insertions(+) create mode 100755 include/dt-bindings/clock/g12a-aoclkc.h diff --git a/Documentation/devicetree/bindings/clock/amlogic,gxbb-aoclkc.txt b/Documentation/devicetree/bindings/clock/amlogic,gxbb-aoclkc.txt index 3a88052..6f02288 100644 --- a/Documentation/devicetree/bindings/clock/amlogic,gxbb-aoclkc.txt +++ b/Documentation/devicetree/bindings/clock/amlogic,gxbb-aoclkc.txt @@ -10,6 +10,7 @@ Required Properties: - GXL (S905X, S905D) : "amlogic,meson-gxl-aoclkc" - GXM (S912) : "amlogic,meson-gxm-aoclkc" - AXG (A113D, A113X) : "amlogic,meson-axg-aoclkc" + - G12A (S905D2, S905X2) : "amlogic,g12a-aoclkc" followed by the common "amlogic,meson-gx-aoclkc" - #clock-cells: should be 1. diff --git a/include/dt-bindings/clock/g12a-aoclkc.h b/include/dt-bindings/clock/g12a-aoclkc.h new file mode 100755 index 000..6b3f921 --- /dev/null +++ b/include/dt-bindings/clock/g12a-aoclkc.h @@ -0,0 +1,28 @@ +/* SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause) */ +/* + * Copyright (c) 2016 BayLibre, SAS + * Author: Neil Armstrong + * + * Copyright (c) 2018 Amlogic, inc. + * Author: Jian Hu + */ + +#ifndef DT_BINDINGS_CLOCK_AMLOGIC_MESON_G12A_AOCLK +#define DT_BINDINGS_CLOCK_AMLOGIC_MESON_G12A_AOCLK + +#define CLKID_AO_AHB_BUS 0 +#define CLKID_AO_REMOTE1 +#define CLKID_AO_I2C_MASTER2 +#define CLKID_AO_I2C_SLAVE 3 +#define CLKID_AO_UART1 4 +#define CLKID_AO_PROD_I2C 5 +#define CLKID_AO_UART2 6 +#define CLKID_AO_IR_BLASTER7 +#define CLKID_AO_SAR_ADC 8 +#define CLKID_AO_CLK81 9 +#define CLKID_AO_SAR_ADC_SEL 10 +#define CLKID_AO_SAR_ADC_DIV 11 +#define CLKID_AO_SAR_ADC_CLK 12 +#define CLKID_AO_ALT_XTAL 13 + +#endif -- 1.9.1
[PATCH 1/2] dt-bindings: clk: meson-g12a: Add G12A AO Clock Bindings
Add new clock controller compatible and dt-bingdings headers for the Always-On domain of the g12a SoC Signed-off-by: Jian Hu --- .../bindings/clock/amlogic,gxbb-aoclkc.txt | 1 + include/dt-bindings/clock/g12a-aoclkc.h| 28 ++ 2 files changed, 29 insertions(+) create mode 100755 include/dt-bindings/clock/g12a-aoclkc.h diff --git a/Documentation/devicetree/bindings/clock/amlogic,gxbb-aoclkc.txt b/Documentation/devicetree/bindings/clock/amlogic,gxbb-aoclkc.txt index 3a88052..6f02288 100644 --- a/Documentation/devicetree/bindings/clock/amlogic,gxbb-aoclkc.txt +++ b/Documentation/devicetree/bindings/clock/amlogic,gxbb-aoclkc.txt @@ -10,6 +10,7 @@ Required Properties: - GXL (S905X, S905D) : "amlogic,meson-gxl-aoclkc" - GXM (S912) : "amlogic,meson-gxm-aoclkc" - AXG (A113D, A113X) : "amlogic,meson-axg-aoclkc" + - G12A (S905D2, S905X2) : "amlogic,g12a-aoclkc" followed by the common "amlogic,meson-gx-aoclkc" - #clock-cells: should be 1. diff --git a/include/dt-bindings/clock/g12a-aoclkc.h b/include/dt-bindings/clock/g12a-aoclkc.h new file mode 100755 index 000..6b3f921 --- /dev/null +++ b/include/dt-bindings/clock/g12a-aoclkc.h @@ -0,0 +1,28 @@ +/* SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause) */ +/* + * Copyright (c) 2016 BayLibre, SAS + * Author: Neil Armstrong + * + * Copyright (c) 2018 Amlogic, inc. + * Author: Jian Hu + */ + +#ifndef DT_BINDINGS_CLOCK_AMLOGIC_MESON_G12A_AOCLK +#define DT_BINDINGS_CLOCK_AMLOGIC_MESON_G12A_AOCLK + +#define CLKID_AO_AHB_BUS 0 +#define CLKID_AO_REMOTE1 +#define CLKID_AO_I2C_MASTER2 +#define CLKID_AO_I2C_SLAVE 3 +#define CLKID_AO_UART1 4 +#define CLKID_AO_PROD_I2C 5 +#define CLKID_AO_UART2 6 +#define CLKID_AO_IR_BLASTER7 +#define CLKID_AO_SAR_ADC 8 +#define CLKID_AO_CLK81 9 +#define CLKID_AO_SAR_ADC_SEL 10 +#define CLKID_AO_SAR_ADC_DIV 11 +#define CLKID_AO_SAR_ADC_CLK 12 +#define CLKID_AO_ALT_XTAL 13 + +#endif -- 1.9.1
Re: [RFC v7 PATCH 4/4] mm: unmap special vmas with regular do_munmap()
On 08/10/2018 01:36 AM, Yang Shi wrote:
> Unmapping vmas, which have VM_HUGETLB | VM_PFNMAP flag set or
> have uprobes set, need get done with write mmap_sem held since
> they may update vm_flags.
>
> So, it might be not safe enough to deal with these kind of special
> mappings with read mmap_sem. Deal with such mappings with regular
> do_munmap() call.
>
> Michal suggested to make this as a separate patch for safer and more
> bisectable sake.
Hm I believe Michal meant the opposite "evolution" though. Patch 2/4
should be done in a way that special mappings keep using the regular
path, and this patch would convert them to the new path. Possibly even
each special case separately.
> Cc: Michal Hocko
> Signed-off-by: Yang Shi
> ---
> mm/mmap.c | 24
> 1 file changed, 24 insertions(+)
>
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 2234d5a..06cb83c 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -2766,6 +2766,16 @@ static inline void munlock_vmas(struct vm_area_struct
> *vma,
> }
> }
>
> +static inline bool can_zap_with_rlock(struct vm_area_struct *vma)
> +{
> + if ((vma->vm_file &&
> + vma_has_uprobes(vma, vma->vm_start, vma->vm_end)) ||
> + (vma->vm_flags | (VM_HUGETLB | VM_PFNMAP)))
> + return false;
> +
> + return true;
> +}
> +
> /*
> * Zap pages with read mmap_sem held
> *
> @@ -2808,6 +2818,17 @@ static int do_munmap_zap_rlock(struct mm_struct *mm,
> unsigned long start,
> goto out;
> }
>
> + /*
> + * Unmapping vmas, which have VM_HUGETLB | VM_PFNMAP flag set or
> + * have uprobes set, need get done with write mmap_sem held since
> + * they may update vm_flags. Deal with such mappings with regular
> + * do_munmap() call.
> + */
> + for (vma = start_vma; vma && vma->vm_start < end; vma = vma->vm_next) {
> + if (!can_zap_with_rlock(vma))
> + goto regular_path;
> + }
> +
> /* Handle mlocked vmas */
> if (mm->locked_vm) {
> vma = start_vma;
> @@ -2828,6 +2849,9 @@ static int do_munmap_zap_rlock(struct mm_struct *mm,
> unsigned long start,
>
> return 0;
>
> +regular_path:
> + ret = do_munmap(mm, start, len, uf);
> +
> out:
> up_write(>mmap_sem);
> return ret;
>
Re: [RFC v7 PATCH 4/4] mm: unmap special vmas with regular do_munmap()
On 08/10/2018 01:36 AM, Yang Shi wrote:
> Unmapping vmas, which have VM_HUGETLB | VM_PFNMAP flag set or
> have uprobes set, need get done with write mmap_sem held since
> they may update vm_flags.
>
> So, it might be not safe enough to deal with these kind of special
> mappings with read mmap_sem. Deal with such mappings with regular
> do_munmap() call.
>
> Michal suggested to make this as a separate patch for safer and more
> bisectable sake.
Hm I believe Michal meant the opposite "evolution" though. Patch 2/4
should be done in a way that special mappings keep using the regular
path, and this patch would convert them to the new path. Possibly even
each special case separately.
> Cc: Michal Hocko
> Signed-off-by: Yang Shi
> ---
> mm/mmap.c | 24
> 1 file changed, 24 insertions(+)
>
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 2234d5a..06cb83c 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -2766,6 +2766,16 @@ static inline void munlock_vmas(struct vm_area_struct
> *vma,
> }
> }
>
> +static inline bool can_zap_with_rlock(struct vm_area_struct *vma)
> +{
> + if ((vma->vm_file &&
> + vma_has_uprobes(vma, vma->vm_start, vma->vm_end)) ||
> + (vma->vm_flags | (VM_HUGETLB | VM_PFNMAP)))
> + return false;
> +
> + return true;
> +}
> +
> /*
> * Zap pages with read mmap_sem held
> *
> @@ -2808,6 +2818,17 @@ static int do_munmap_zap_rlock(struct mm_struct *mm,
> unsigned long start,
> goto out;
> }
>
> + /*
> + * Unmapping vmas, which have VM_HUGETLB | VM_PFNMAP flag set or
> + * have uprobes set, need get done with write mmap_sem held since
> + * they may update vm_flags. Deal with such mappings with regular
> + * do_munmap() call.
> + */
> + for (vma = start_vma; vma && vma->vm_start < end; vma = vma->vm_next) {
> + if (!can_zap_with_rlock(vma))
> + goto regular_path;
> + }
> +
> /* Handle mlocked vmas */
> if (mm->locked_vm) {
> vma = start_vma;
> @@ -2828,6 +2849,9 @@ static int do_munmap_zap_rlock(struct mm_struct *mm,
> unsigned long start,
>
> return 0;
>
> +regular_path:
> + ret = do_munmap(mm, start, len, uf);
> +
> out:
> up_write(>mmap_sem);
> return ret;
>
[PATCH v3 9/9] clk: actions: Add Actions Semi S900 SoC Reset Management Unit support
Add Reset Management Unit (RMU) support for Actions Semi S900 SoC.
Signed-off-by: Manivannan Sadhasivam
---
drivers/clk/actions/owl-s900.c | 82 ++
1 file changed, 82 insertions(+)
diff --git a/drivers/clk/actions/owl-s900.c b/drivers/clk/actions/owl-s900.c
index bb7ee872d316..790890978424 100644
--- a/drivers/clk/actions/owl-s900.c
+++ b/drivers/clk/actions/owl-s900.c
@@ -19,8 +19,10 @@
#include "owl-gate.h"
#include "owl-mux.h"
#include "owl-pll.h"
+#include "owl-reset.h"
#include
+#include
#define CMU_COREPLL(0x)
#define CMU_DEVPLL (0x0004)
@@ -684,20 +686,100 @@ static struct clk_hw_onecell_data s900_hw_clks = {
.num= CLK_NR_CLKS,
};
+static const struct owl_reset_map s900_resets[] = {
+ [RESET_DMAC]= { CMU_DEVRST0, BIT(0) },
+ [RESET_SRAMI] = { CMU_DEVRST0, BIT(1) },
+ [RESET_DDR_CTL_PHY] = { CMU_DEVRST0, BIT(2) },
+ [RESET_NANDC0] = { CMU_DEVRST0, BIT(3) },
+ [RESET_SD0] = { CMU_DEVRST0, BIT(4) },
+ [RESET_SD1] = { CMU_DEVRST0, BIT(5) },
+ [RESET_PCM1]= { CMU_DEVRST0, BIT(6) },
+ [RESET_DE] = { CMU_DEVRST0, BIT(7) },
+ [RESET_LVDS]= { CMU_DEVRST0, BIT(8) },
+ [RESET_SD2] = { CMU_DEVRST0, BIT(9) },
+ [RESET_DSI] = { CMU_DEVRST0, BIT(10) },
+ [RESET_CSI0]= { CMU_DEVRST0, BIT(11) },
+ [RESET_BISP_AXI]= { CMU_DEVRST0, BIT(12) },
+ [RESET_CSI1]= { CMU_DEVRST0, BIT(13) },
+ [RESET_GPIO]= { CMU_DEVRST0, BIT(15) },
+ [RESET_EDP] = { CMU_DEVRST0, BIT(16) },
+ [RESET_AUDIO] = { CMU_DEVRST0, BIT(17) },
+ [RESET_PCM0]= { CMU_DEVRST0, BIT(18) },
+ [RESET_HDE] = { CMU_DEVRST0, BIT(21) },
+ [RESET_GPU3D_PA]= { CMU_DEVRST0, BIT(22) },
+ [RESET_IMX] = { CMU_DEVRST0, BIT(23) },
+ [RESET_SE] = { CMU_DEVRST0, BIT(24) },
+ [RESET_NANDC1] = { CMU_DEVRST0, BIT(25) },
+ [RESET_SD3] = { CMU_DEVRST0, BIT(26) },
+ [RESET_GIC] = { CMU_DEVRST0, BIT(27) },
+ [RESET_GPU3D_PB]= { CMU_DEVRST0, BIT(28) },
+ [RESET_DDR_CTL_PHY_AXI] = { CMU_DEVRST0, BIT(29) },
+ [RESET_CMU_DDR] = { CMU_DEVRST0, BIT(30) },
+ [RESET_DMM] = { CMU_DEVRST0, BIT(31) },
+ [RESET_USB2HUB] = { CMU_DEVRST1, BIT(0) },
+ [RESET_USB2HSIC]= { CMU_DEVRST1, BIT(1) },
+ [RESET_HDMI]= { CMU_DEVRST1, BIT(2) },
+ [RESET_HDCP2TX] = { CMU_DEVRST1, BIT(3) },
+ [RESET_UART6] = { CMU_DEVRST1, BIT(4) },
+ [RESET_UART0] = { CMU_DEVRST1, BIT(5) },
+ [RESET_UART1] = { CMU_DEVRST1, BIT(6) },
+ [RESET_UART2] = { CMU_DEVRST1, BIT(7) },
+ [RESET_SPI0]= { CMU_DEVRST1, BIT(8) },
+ [RESET_SPI1]= { CMU_DEVRST1, BIT(9) },
+ [RESET_SPI2]= { CMU_DEVRST1, BIT(10) },
+ [RESET_SPI3]= { CMU_DEVRST1, BIT(11) },
+ [RESET_I2C0]= { CMU_DEVRST1, BIT(12) },
+ [RESET_I2C1]= { CMU_DEVRST1, BIT(13) },
+ [RESET_USB3]= { CMU_DEVRST1, BIT(14) },
+ [RESET_UART3] = { CMU_DEVRST1, BIT(15) },
+ [RESET_UART4] = { CMU_DEVRST1, BIT(16) },
+ [RESET_UART5] = { CMU_DEVRST1, BIT(17) },
+ [RESET_I2C2]= { CMU_DEVRST1, BIT(18) },
+ [RESET_I2C3]= { CMU_DEVRST1, BIT(19) },
+ [RESET_ETHERNET]= { CMU_DEVRST1, BIT(20) },
+ [RESET_CHIPID] = { CMU_DEVRST1, BIT(21) },
+ [RESET_I2C4]= { CMU_DEVRST1, BIT(22) },
+ [RESET_I2C5]= { CMU_DEVRST1, BIT(23) },
+ [RESET_CPU_SCNT]= { CMU_DEVRST1, BIT(30) }
+};
+
static struct owl_clk_desc s900_clk_desc = {
.clks = s900_clks,
.num_clks = ARRAY_SIZE(s900_clks),
.hw_clks= _hw_clks,
+
+ .resets = s900_resets,
+ .num_resets = ARRAY_SIZE(s900_resets),
};
static int s900_clk_probe(struct platform_device *pdev)
{
struct owl_clk_desc *desc;
+ struct owl_reset *reset;
+ int ret;
desc = _clk_desc;
owl_clk_regmap_init(pdev, desc);
+ /*
+* FIXME: Reset controller registration should be moved to
+* common code, once all SoCs of Owl family supports it.
+*/
+ reset = devm_kzalloc(>dev, sizeof(*reset), GFP_KERNEL);
+ if (!reset)
+ return -ENOMEM;
+
+ reset->rcdev.of_node = pdev->dev.of_node;
+ reset->rcdev.ops = _reset_ops;
+ reset->rcdev.nr_resets = desc->num_resets;
+ reset->reset_map = desc->resets;
+ reset->regmap = desc->regmap;
+
+ ret =
[PATCH v3 9/9] clk: actions: Add Actions Semi S900 SoC Reset Management Unit support
Add Reset Management Unit (RMU) support for Actions Semi S900 SoC.
Signed-off-by: Manivannan Sadhasivam
---
drivers/clk/actions/owl-s900.c | 82 ++
1 file changed, 82 insertions(+)
diff --git a/drivers/clk/actions/owl-s900.c b/drivers/clk/actions/owl-s900.c
index bb7ee872d316..790890978424 100644
--- a/drivers/clk/actions/owl-s900.c
+++ b/drivers/clk/actions/owl-s900.c
@@ -19,8 +19,10 @@
#include "owl-gate.h"
#include "owl-mux.h"
#include "owl-pll.h"
+#include "owl-reset.h"
#include
+#include
#define CMU_COREPLL(0x)
#define CMU_DEVPLL (0x0004)
@@ -684,20 +686,100 @@ static struct clk_hw_onecell_data s900_hw_clks = {
.num= CLK_NR_CLKS,
};
+static const struct owl_reset_map s900_resets[] = {
+ [RESET_DMAC]= { CMU_DEVRST0, BIT(0) },
+ [RESET_SRAMI] = { CMU_DEVRST0, BIT(1) },
+ [RESET_DDR_CTL_PHY] = { CMU_DEVRST0, BIT(2) },
+ [RESET_NANDC0] = { CMU_DEVRST0, BIT(3) },
+ [RESET_SD0] = { CMU_DEVRST0, BIT(4) },
+ [RESET_SD1] = { CMU_DEVRST0, BIT(5) },
+ [RESET_PCM1]= { CMU_DEVRST0, BIT(6) },
+ [RESET_DE] = { CMU_DEVRST0, BIT(7) },
+ [RESET_LVDS]= { CMU_DEVRST0, BIT(8) },
+ [RESET_SD2] = { CMU_DEVRST0, BIT(9) },
+ [RESET_DSI] = { CMU_DEVRST0, BIT(10) },
+ [RESET_CSI0]= { CMU_DEVRST0, BIT(11) },
+ [RESET_BISP_AXI]= { CMU_DEVRST0, BIT(12) },
+ [RESET_CSI1]= { CMU_DEVRST0, BIT(13) },
+ [RESET_GPIO]= { CMU_DEVRST0, BIT(15) },
+ [RESET_EDP] = { CMU_DEVRST0, BIT(16) },
+ [RESET_AUDIO] = { CMU_DEVRST0, BIT(17) },
+ [RESET_PCM0]= { CMU_DEVRST0, BIT(18) },
+ [RESET_HDE] = { CMU_DEVRST0, BIT(21) },
+ [RESET_GPU3D_PA]= { CMU_DEVRST0, BIT(22) },
+ [RESET_IMX] = { CMU_DEVRST0, BIT(23) },
+ [RESET_SE] = { CMU_DEVRST0, BIT(24) },
+ [RESET_NANDC1] = { CMU_DEVRST0, BIT(25) },
+ [RESET_SD3] = { CMU_DEVRST0, BIT(26) },
+ [RESET_GIC] = { CMU_DEVRST0, BIT(27) },
+ [RESET_GPU3D_PB]= { CMU_DEVRST0, BIT(28) },
+ [RESET_DDR_CTL_PHY_AXI] = { CMU_DEVRST0, BIT(29) },
+ [RESET_CMU_DDR] = { CMU_DEVRST0, BIT(30) },
+ [RESET_DMM] = { CMU_DEVRST0, BIT(31) },
+ [RESET_USB2HUB] = { CMU_DEVRST1, BIT(0) },
+ [RESET_USB2HSIC]= { CMU_DEVRST1, BIT(1) },
+ [RESET_HDMI]= { CMU_DEVRST1, BIT(2) },
+ [RESET_HDCP2TX] = { CMU_DEVRST1, BIT(3) },
+ [RESET_UART6] = { CMU_DEVRST1, BIT(4) },
+ [RESET_UART0] = { CMU_DEVRST1, BIT(5) },
+ [RESET_UART1] = { CMU_DEVRST1, BIT(6) },
+ [RESET_UART2] = { CMU_DEVRST1, BIT(7) },
+ [RESET_SPI0]= { CMU_DEVRST1, BIT(8) },
+ [RESET_SPI1]= { CMU_DEVRST1, BIT(9) },
+ [RESET_SPI2]= { CMU_DEVRST1, BIT(10) },
+ [RESET_SPI3]= { CMU_DEVRST1, BIT(11) },
+ [RESET_I2C0]= { CMU_DEVRST1, BIT(12) },
+ [RESET_I2C1]= { CMU_DEVRST1, BIT(13) },
+ [RESET_USB3]= { CMU_DEVRST1, BIT(14) },
+ [RESET_UART3] = { CMU_DEVRST1, BIT(15) },
+ [RESET_UART4] = { CMU_DEVRST1, BIT(16) },
+ [RESET_UART5] = { CMU_DEVRST1, BIT(17) },
+ [RESET_I2C2]= { CMU_DEVRST1, BIT(18) },
+ [RESET_I2C3]= { CMU_DEVRST1, BIT(19) },
+ [RESET_ETHERNET]= { CMU_DEVRST1, BIT(20) },
+ [RESET_CHIPID] = { CMU_DEVRST1, BIT(21) },
+ [RESET_I2C4]= { CMU_DEVRST1, BIT(22) },
+ [RESET_I2C5]= { CMU_DEVRST1, BIT(23) },
+ [RESET_CPU_SCNT]= { CMU_DEVRST1, BIT(30) }
+};
+
static struct owl_clk_desc s900_clk_desc = {
.clks = s900_clks,
.num_clks = ARRAY_SIZE(s900_clks),
.hw_clks= _hw_clks,
+
+ .resets = s900_resets,
+ .num_resets = ARRAY_SIZE(s900_resets),
};
static int s900_clk_probe(struct platform_device *pdev)
{
struct owl_clk_desc *desc;
+ struct owl_reset *reset;
+ int ret;
desc = _clk_desc;
owl_clk_regmap_init(pdev, desc);
+ /*
+* FIXME: Reset controller registration should be moved to
+* common code, once all SoCs of Owl family supports it.
+*/
+ reset = devm_kzalloc(>dev, sizeof(*reset), GFP_KERNEL);
+ if (!reset)
+ return -ENOMEM;
+
+ reset->rcdev.of_node = pdev->dev.of_node;
+ reset->rcdev.ops = _reset_ops;
+ reset->rcdev.nr_resets = desc->num_resets;
+ reset->reset_map = desc->resets;
+ reset->regmap = desc->regmap;
+
+ ret =
[PATCH v3 8/9] clk: actions: Add Actions Semi S700 SoC Reset Management Unit support
Add Reset Management Unit (RMU) support for Actions Semi S700 SoC.
Signed-off-by: Manivannan Sadhasivam
---
drivers/clk/actions/owl-s700.c | 51 ++
1 file changed, 51 insertions(+)
diff --git a/drivers/clk/actions/owl-s700.c b/drivers/clk/actions/owl-s700.c
index e7cacd677275..a2f34d13fb54 100644
--- a/drivers/clk/actions/owl-s700.c
+++ b/drivers/clk/actions/owl-s700.c
@@ -20,8 +20,10 @@
#include "owl-gate.h"
#include "owl-mux.h"
#include "owl-pll.h"
+#include "owl-reset.h"
#include
+#include
#define CMU_COREPLL(0x)
#define CMU_DEVPLL (0x0004)
@@ -569,20 +571,69 @@ static struct clk_hw_onecell_data s700_hw_clks = {
.num= CLK_NR_CLKS,
};
+static const struct owl_reset_map s700_resets[] = {
+ [RESET_DE] = { CMU_DEVRST0, BIT(0) },
+ [RESET_LCD0]= { CMU_DEVRST0, BIT(1) },
+ [RESET_DSI] = { CMU_DEVRST0, BIT(2) },
+ [RESET_CSI] = { CMU_DEVRST0, BIT(13) },
+ [RESET_SI] = { CMU_DEVRST0, BIT(14) },
+ [RESET_I2C0]= { CMU_DEVRST1, BIT(0) },
+ [RESET_I2C1]= { CMU_DEVRST1, BIT(1) },
+ [RESET_I2C2]= { CMU_DEVRST1, BIT(2) },
+ [RESET_I2C3]= { CMU_DEVRST1, BIT(3) },
+ [RESET_SPI0]= { CMU_DEVRST1, BIT(4) },
+ [RESET_SPI1]= { CMU_DEVRST1, BIT(5) },
+ [RESET_SPI2]= { CMU_DEVRST1, BIT(6) },
+ [RESET_SPI3]= { CMU_DEVRST1, BIT(7) },
+ [RESET_UART0] = { CMU_DEVRST1, BIT(8) },
+ [RESET_UART1] = { CMU_DEVRST1, BIT(9) },
+ [RESET_UART2] = { CMU_DEVRST1, BIT(10) },
+ [RESET_UART3] = { CMU_DEVRST1, BIT(11) },
+ [RESET_UART4] = { CMU_DEVRST1, BIT(12) },
+ [RESET_UART5] = { CMU_DEVRST1, BIT(13) },
+ [RESET_UART6] = { CMU_DEVRST1, BIT(14) },
+ [RESET_KEY] = { CMU_DEVRST1, BIT(24) },
+ [RESET_GPIO]= { CMU_DEVRST1, BIT(25) },
+ [RESET_AUDIO] = { CMU_DEVRST1, BIT(29) },
+};
+
static struct owl_clk_desc s700_clk_desc = {
.clks = s700_clks,
.num_clks = ARRAY_SIZE(s700_clks),
.hw_clks= _hw_clks,
+
+ .resets = s700_resets,
+ .num_resets = ARRAY_SIZE(s700_resets),
};
static int s700_clk_probe(struct platform_device *pdev)
{
struct owl_clk_desc *desc;
+ struct owl_reset *reset;
+ int ret;
desc = _clk_desc;
owl_clk_regmap_init(pdev, desc);
+ /*
+* FIXME: Reset controller registration should be moved to
+* common code, once all SoCs of Owl family supports it.
+*/
+ reset = devm_kzalloc(>dev, sizeof(*reset), GFP_KERNEL);
+ if (!reset)
+ return -ENOMEM;
+
+ reset->rcdev.of_node = pdev->dev.of_node;
+ reset->rcdev.ops = _reset_ops;
+ reset->rcdev.nr_resets = desc->num_resets;
+ reset->reset_map = desc->resets;
+ reset->regmap = desc->regmap;
+
+ ret = devm_reset_controller_register(>dev, >rcdev);
+ if (ret)
+ dev_err(>dev, "Failed to register reset controller\n");
+
return owl_clk_probe(>dev, desc->hw_clks);
}
--
2.17.1
[PATCH v3 8/9] clk: actions: Add Actions Semi S700 SoC Reset Management Unit support
Add Reset Management Unit (RMU) support for Actions Semi S700 SoC.
Signed-off-by: Manivannan Sadhasivam
---
drivers/clk/actions/owl-s700.c | 51 ++
1 file changed, 51 insertions(+)
diff --git a/drivers/clk/actions/owl-s700.c b/drivers/clk/actions/owl-s700.c
index e7cacd677275..a2f34d13fb54 100644
--- a/drivers/clk/actions/owl-s700.c
+++ b/drivers/clk/actions/owl-s700.c
@@ -20,8 +20,10 @@
#include "owl-gate.h"
#include "owl-mux.h"
#include "owl-pll.h"
+#include "owl-reset.h"
#include
+#include
#define CMU_COREPLL(0x)
#define CMU_DEVPLL (0x0004)
@@ -569,20 +571,69 @@ static struct clk_hw_onecell_data s700_hw_clks = {
.num= CLK_NR_CLKS,
};
+static const struct owl_reset_map s700_resets[] = {
+ [RESET_DE] = { CMU_DEVRST0, BIT(0) },
+ [RESET_LCD0]= { CMU_DEVRST0, BIT(1) },
+ [RESET_DSI] = { CMU_DEVRST0, BIT(2) },
+ [RESET_CSI] = { CMU_DEVRST0, BIT(13) },
+ [RESET_SI] = { CMU_DEVRST0, BIT(14) },
+ [RESET_I2C0]= { CMU_DEVRST1, BIT(0) },
+ [RESET_I2C1]= { CMU_DEVRST1, BIT(1) },
+ [RESET_I2C2]= { CMU_DEVRST1, BIT(2) },
+ [RESET_I2C3]= { CMU_DEVRST1, BIT(3) },
+ [RESET_SPI0]= { CMU_DEVRST1, BIT(4) },
+ [RESET_SPI1]= { CMU_DEVRST1, BIT(5) },
+ [RESET_SPI2]= { CMU_DEVRST1, BIT(6) },
+ [RESET_SPI3]= { CMU_DEVRST1, BIT(7) },
+ [RESET_UART0] = { CMU_DEVRST1, BIT(8) },
+ [RESET_UART1] = { CMU_DEVRST1, BIT(9) },
+ [RESET_UART2] = { CMU_DEVRST1, BIT(10) },
+ [RESET_UART3] = { CMU_DEVRST1, BIT(11) },
+ [RESET_UART4] = { CMU_DEVRST1, BIT(12) },
+ [RESET_UART5] = { CMU_DEVRST1, BIT(13) },
+ [RESET_UART6] = { CMU_DEVRST1, BIT(14) },
+ [RESET_KEY] = { CMU_DEVRST1, BIT(24) },
+ [RESET_GPIO]= { CMU_DEVRST1, BIT(25) },
+ [RESET_AUDIO] = { CMU_DEVRST1, BIT(29) },
+};
+
static struct owl_clk_desc s700_clk_desc = {
.clks = s700_clks,
.num_clks = ARRAY_SIZE(s700_clks),
.hw_clks= _hw_clks,
+
+ .resets = s700_resets,
+ .num_resets = ARRAY_SIZE(s700_resets),
};
static int s700_clk_probe(struct platform_device *pdev)
{
struct owl_clk_desc *desc;
+ struct owl_reset *reset;
+ int ret;
desc = _clk_desc;
owl_clk_regmap_init(pdev, desc);
+ /*
+* FIXME: Reset controller registration should be moved to
+* common code, once all SoCs of Owl family supports it.
+*/
+ reset = devm_kzalloc(>dev, sizeof(*reset), GFP_KERNEL);
+ if (!reset)
+ return -ENOMEM;
+
+ reset->rcdev.of_node = pdev->dev.of_node;
+ reset->rcdev.ops = _reset_ops;
+ reset->rcdev.nr_resets = desc->num_resets;
+ reset->reset_map = desc->resets;
+ reset->regmap = desc->regmap;
+
+ ret = devm_reset_controller_register(>dev, >rcdev);
+ if (ret)
+ dev_err(>dev, "Failed to register reset controller\n");
+
return owl_clk_probe(>dev, desc->hw_clks);
}
--
2.17.1
