[PATCH RESEND v2] tools/virtio: creating pipe assertion in vringh_test
From: Yunseong Kim
parallel_test() function in vringh_test needs to verify
the creation of the guest/host pipe.
Signed-off-by: Yunseong Kim
---
tools/virtio/vringh_test.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/tools/virtio/vringh_test.c b/tools/virtio/vringh_test.c
index 98ff808d6f0c..43d3a6aa1dcf 100644
--- a/tools/virtio/vringh_test.c
+++ b/tools/virtio/vringh_test.c
@@ -139,7 +139,7 @@ static int parallel_test(u64 features,
bool fast_vringh)
{
void *host_map, *guest_map;
- int fd, mapsize, to_guest[2], to_host[2];
+ int pipe_ret, fd, mapsize, to_guest[2], to_host[2];
unsigned long xfers = 0, notifies = 0, receives = 0;
unsigned int first_cpu, last_cpu;
cpu_set_t cpu_set;
@@ -161,8 +161,11 @@ static int parallel_test(u64 features,
host_map = mmap(NULL, mapsize, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
guest_map = mmap(NULL, mapsize, PROT_READ|PROT_WRITE, MAP_SHARED, fd,
0);
- pipe(to_guest);
- pipe(to_host);
+ pipe_ret = pipe(to_guest);
+ assert(!pipe_ret);
+
+ pipe_ret = pipe(to_host);
+ assert(!pipe_ret);
CPU_ZERO(_set);
find_cpus(_cpu, _cpu);
--
2.45.2
[PATCH v4] tracing/net_sched: NULL pointer dereference in perf_trace_qdisc_reset()
From: Yunseong Kim In the TRACE_EVENT(qdisc_reset) NULL dereference occurred from qdisc->dev_queue->dev ->name This situation simulated from bunch of veths and Bluetooth disconnection and reconnection. During qdisc initialization, qdisc was being set to noop_queue. In veth_init_queue, the initial tx_num was reduced back to one, causing the qdisc reset to be called with noop, which led to the kernel panic. I've attached the GitHub gist link that C converted syz-execprogram source code and 3 log of reproduced vmcore-dmesg. https://gist.github.com/yskelg/cc64562873ce249cdd0d5a358b77d740 Yeoreum and I use two fuzzing tool simultaneously. One process with syz-executor : https://github.com/google/syzkaller $ ./syz-execprog -executor=./syz-executor -repeat=1 -sandbox=setuid \ -enable=none -collide=false log1 The other process with perf fuzzer: https://github.com/deater/perf_event_tests/tree/master/fuzzer $ perf_event_tests/fuzzer/perf_fuzzer I think this will happen on the kernel version. Linux kernel version +v6.7.10, +v6.8, +v6.9 and it could happen in v6.10. This occurred from 51270d573a8d. I think this patch is absolutely necessary. Previously, It was showing not intended string value of name. I've reproduced 3 time from my fedora 40 Debug Kernel with any other module or patched. version: 6.10.0-0.rc2.20240608gitdc772f8237f9.29.fc41.aarch64+debug [ 5287.164555] veth0_vlan: left promiscuous mode [ 5287.164929] veth1_macvtap: left promiscuous mode [ 5287.164950] veth0_macvtap: left promiscuous mode [ 5287.164983] veth1_vlan: left promiscuous mode [ 5287.165008] veth0_vlan: left promiscuous mode [ 5287.165450] veth1_macvtap: left promiscuous mode [ 5287.165472] veth0_macvtap: left promiscuous mode [ 5287.165502] veth1_vlan: left promiscuous mode … [ 5297.598240] bridge0: port 2(bridge_slave_1) entered blocking state [ 5297.598262] bridge0: port 2(bridge_slave_1) entered forwarding state [ 5297.598296] bridge0: port 1(bridge_slave_0) entered blocking state [ 5297.598313] bridge0: port 1(bridge_slave_0) entered forwarding state [ 5297.616090] 8021q: adding VLAN 0 to HW filter on device bond0 [ 5297.620405] bridge0: port 1(bridge_slave_0) entered disabled state [ 5297.620730] bridge0: port 2(bridge_slave_1) entered disabled state [ 5297.627247] 8021q: adding VLAN 0 to HW filter on device team0 [ 5297.629636] bridge0: port 1(bridge_slave_0) entered blocking state … [ 5298.002798] bridge_slave_0: left promiscuous mode [ 5298.002869] bridge0: port 1(bridge_slave_0) entered disabled state [ 5298.309444] bond0 (unregistering): (slave bond_slave_0): Releasing backup interface [ 5298.315206] bond0 (unregistering): (slave bond_slave_1): Releasing backup interface [ 5298.320207] bond0 (unregistering): Released all slaves [ 5298.354296] hsr_slave_0: left promiscuous mode [ 5298.360750] hsr_slave_1: left promiscuous mode [ 5298.374889] veth1_macvtap: left promiscuous mode [ 5298.374931] veth0_macvtap: left promiscuous mode [ 5298.374988] veth1_vlan: left promiscuous mode [ 5298.375024] veth0_vlan: left promiscuous mode [ 5299.109741] team0 (unregistering): Port device team_slave_1 removed [ 5299.185870] team0 (unregistering): Port device team_slave_0 removed … [ 5300.155443] Bluetooth: hci3: unexpected cc 0x0c03 length: 249 > 1 [ 5300.155724] Bluetooth: hci3: unexpected cc 0x1003 length: 249 > 9 [ 5300.155988] Bluetooth: hci3: unexpected cc 0x1001 length: 249 > 9 …. [ 5301.075531] team0: Port device team_slave_1 added [ 5301.085515] bridge0: port 1(bridge_slave_0) entered blocking state [ 5301.085531] bridge0: port 1(bridge_slave_0) entered disabled state [ 5301.085588] bridge_slave_0: entered allmulticast mode [ 5301.085800] bridge_slave_0: entered promiscuous mode [ 5301.095617] bridge0: port 1(bridge_slave_0) entered blocking state [ 5301.095633] bridge0: port 1(bridge_slave_0) entered disabled state … [ 5301.149734] bond0: (slave bond_slave_0): Enslaving as an active interface with an up link [ 5301.173234] bond0: (slave bond_slave_0): Enslaving as an active interface with an up link [ 5301.180517] bond0: (slave bond_slave_1): Enslaving as an active interface with an up link [ 5301.193481] hsr_slave_0: entered promiscuous mode [ 5301.204425] hsr_slave_1: entered promiscuous mode [ 5301.210172] debugfs: Directory 'hsr0' with parent 'hsr' already present! [ 5301.210185] Cannot create hsr debugfs directory [ 5301.224061] bond0: (slave bond_slave_1): Enslaving as an active interface with an up link [ 5301.246901] bond0: (slave bond_slave_0): Enslaving as an active interface with an up link [ 5301.255934] team0: Port device team_slave_0 added [ 5301.256480] team0: Port device team_slave_1 added [ 5301.256948] team0: Port device team_slave_0 added … [ 5301.435928] hsr_slave_0: entered promiscuous mode [ 5301.446029] hsr_slave_1: entered promiscuous mode [ 5301.455872] debugfs: Directory 'hsr0' with parent 'hsr' already present! [ 5301.455884] Cannot create hsr debugfs directory [
Re: [PATCH v3] tracing/net_sched: NULL pointer dereference in perf_trace_qdisc_reset()
Hi Pedro,
On 6/25/24 12:55 오전, Pedro Tammela wrote:
> On 24/06/2024 12:43, Yunseong Kim wrote:
>> Hi Pedro,
>>
>> On 6/25/24 12:12 오전, Pedro Tammela wrote:
>>> On 22/06/2024 01:57, ysk...@gmail.com wrote:
From: Yunseong Kim
In the TRACE_EVENT(qdisc_reset) NULL dereference occurred from
qdisc->dev_queue->dev ->name
[ 5301.595872] KASAN: null-ptr-deref in range
[0x0130-0x0137]
[ 5301.595877] Mem abort info:
[ 5301.595881] ESR = 0x9606
[ 5301.595885] EC = 0x25: DABT (current EL), IL = 32 bits
[ 5301.595889] SET = 0, FnV = 0
[ 5301.595893] EA = 0, S1PTW = 0
[ 5301.595896] FSC = 0x06: level 2 translation fault
[ 5301.595900] Data abort info:
[ 5301.595903] ISV = 0, ISS = 0x0006, ISS2 = 0x
[ 5301.595907] CM = 0, WnR = 0, TnD = 0, TagAccess = 0
[ 5301.595911] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[ 5301.595915] [dfff8026] address between user and kernel
address ranges
[ 5301.595971] Internal error: Oops: 9606 [#1] SMP
Link:
https://lore.kernel.org/lkml/20240229143432.273b4...@gandalf.local.home/t/
Fixes: 51270d573a8d ("tracing/net_sched: Fix tracepoints that save
qdisc_dev() as a string")
Cc: net...@vger.kernel.org
Cc: sta...@vger.kernel.org # +v6.7.10, +v6.8
Signed-off-by: Yunseong Kim
Signed-off-by: Yeoreum Yun
---
include/trace/events/qdisc.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/trace/events/qdisc.h
b/include/trace/events/qdisc.h
index f1b5e816e7e5..170b51fbe47a 100644
--- a/include/trace/events/qdisc.h
+++ b/include/trace/events/qdisc.h
@@ -81,7 +81,7 @@ TRACE_EVENT(qdisc_reset,
TP_ARGS(q),
TP_STRUCT__entry(
- __string( dev, qdisc_dev(q)->name )
+ __string(dev, qdisc_dev(q) ? qdisc_dev(q)->name :
"noop_queue")
__string( kind, q->ops->id )
__field( u32, parent )
__field( u32, handle )
>>>
>>> You missed the __assign_str portion (see below). Also let's just say
>>> "(null)" as it's the correct device name. "noop_queue" could be
>>> misleading.
>>
>> Thanks for the code review Pedro, I agree your advice.
>>
>>> diff --git a/include/trace/events/qdisc.h b/include/trace/events/qdisc.h
>>> index 1f4258308b96..f54e0b4dbcf4 100644
>>> --- a/include/trace/events/qdisc.h
>>> +++ b/include/trace/events/qdisc.h
>>> @@ -81,14 +81,14 @@ TRACE_EVENT(qdisc_reset,
>>> TP_ARGS(q),
>>>
>>> TP_STRUCT__entry(
>>> - __string( dev,
>>> qdisc_dev(q)->name )
>>> + __string( dev, qdisc_dev(q) ?
>>> qdisc_dev(q)->name : "(null)" )
>>> __string( kind,
>>> q->ops->id )
>>> __field( u32,
>>> parent )
>>> __field( u32,
>>> handle )
>>> ),
>>
>> It looks better to align the name with the current convention.
>>
>> Link:
>> https://lore.kernel.org/linux-trace-kernel/2024011442.634192...@goodmis.org/
>>
>>> TP_fast_assign(
>>> - __assign_str(dev, qdisc_dev(q)->name);
>>> + __assign_str(dev, qdisc_dev(q) ? qdisc_dev(q)->name :
>>> "(null)");
>>> __assign_str(kind, q->ops->id);
>>> __entry->parent = q->parent;
>>> __entry->handle = q->handle;
>>>
>>>
>>
>> The second part you mentioned, Steve recently worked on it and changed
>> it.
>>
>> Link:
>> https://lore.kernel.org/linux-trace-kernel/20240516133454.681ba...@rorschach.local.home/
>
> Oh!
Thanks for the double check, Pedro.
>> If it hadn't, I don't think I would have been able to prevent the panic
>> by just applying my patch.
>
> But you must be careful with the backports.
>
> In any case, perhaps send another patch to net-next updating the new
> conventions there and use the 'old convention' for the bug fix?
Right, I agree, I'll send a patch for the next version.
Warm regards,
Yunseong Kim
Re: [PATCH v3] tracing/net_sched: NULL pointer dereference in perf_trace_qdisc_reset()
On 24/06/2024 12:43, Yunseong Kim wrote:
Hi Pedro,
On 6/25/24 12:12 오전, Pedro Tammela wrote:
On 22/06/2024 01:57, ysk...@gmail.com wrote:
From: Yunseong Kim
In the TRACE_EVENT(qdisc_reset) NULL dereference occurred from
qdisc->dev_queue->dev ->name
[ 5301.595872] KASAN: null-ptr-deref in range
[0x0130-0x0137]
[ 5301.595877] Mem abort info:
[ 5301.595881] ESR = 0x9606
[ 5301.595885] EC = 0x25: DABT (current EL), IL = 32 bits
[ 5301.595889] SET = 0, FnV = 0
[ 5301.595893] EA = 0, S1PTW = 0
[ 5301.595896] FSC = 0x06: level 2 translation fault
[ 5301.595900] Data abort info:
[ 5301.595903] ISV = 0, ISS = 0x0006, ISS2 = 0x
[ 5301.595907] CM = 0, WnR = 0, TnD = 0, TagAccess = 0
[ 5301.595911] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[ 5301.595915] [dfff8026] address between user and kernel
address ranges
[ 5301.595971] Internal error: Oops: 9606 [#1] SMP
Link:
https://lore.kernel.org/lkml/20240229143432.273b4...@gandalf.local.home/t/
Fixes: 51270d573a8d ("tracing/net_sched: Fix tracepoints that save
qdisc_dev() as a string")
Cc: net...@vger.kernel.org
Cc: sta...@vger.kernel.org # +v6.7.10, +v6.8
Signed-off-by: Yunseong Kim
Signed-off-by: Yeoreum Yun
---
include/trace/events/qdisc.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/trace/events/qdisc.h b/include/trace/events/qdisc.h
index f1b5e816e7e5..170b51fbe47a 100644
--- a/include/trace/events/qdisc.h
+++ b/include/trace/events/qdisc.h
@@ -81,7 +81,7 @@ TRACE_EVENT(qdisc_reset,
TP_ARGS(q),
TP_STRUCT__entry(
- __string( dev, qdisc_dev(q)->name )
+ __string(dev, qdisc_dev(q) ? qdisc_dev(q)->name : "noop_queue")
__string( kind, q->ops->id )
__field( u32, parent )
__field( u32, handle )
You missed the __assign_str portion (see below). Also let's just say
"(null)" as it's the correct device name. "noop_queue" could be misleading.
Thanks for the code review Pedro, I agree your advice.
diff --git a/include/trace/events/qdisc.h b/include/trace/events/qdisc.h
index 1f4258308b96..f54e0b4dbcf4 100644
--- a/include/trace/events/qdisc.h
+++ b/include/trace/events/qdisc.h
@@ -81,14 +81,14 @@ TRACE_EVENT(qdisc_reset,
TP_ARGS(q),
TP_STRUCT__entry(
- __string( dev, qdisc_dev(q)->name )
+ __string( dev, qdisc_dev(q) ?
qdisc_dev(q)->name : "(null)" )
__string( kind, q->ops->id )
__field( u32, parent )
__field( u32, handle )
),
It looks better to align the name with the current convention.
Link:
https://lore.kernel.org/linux-trace-kernel/2024011442.634192...@goodmis.org/
TP_fast_assign(
- __assign_str(dev, qdisc_dev(q)->name);
+ __assign_str(dev, qdisc_dev(q) ? qdisc_dev(q)->name :
"(null)");
__assign_str(kind, q->ops->id);
__entry->parent = q->parent;
__entry->handle = q->handle;
The second part you mentioned, Steve recently worked on it and changed it.
Link:
https://lore.kernel.org/linux-trace-kernel/20240516133454.681ba...@rorschach.local.home/
Oh!
If it hadn't, I don't think I would have been able to prevent the panic
by just applying my patch.
But you must be careful with the backports.
In any case, perhaps send another patch to net-next updating the new
conventions there and use the 'old convention' for the bug fix?
Re: [PATCH v3] tracing/net_sched: NULL pointer dereference in perf_trace_qdisc_reset()
Hi Pedro,
On 6/25/24 12:12 오전, Pedro Tammela wrote:
> On 22/06/2024 01:57, ysk...@gmail.com wrote:
>> From: Yunseong Kim
>>
>> In the TRACE_EVENT(qdisc_reset) NULL dereference occurred from
>>
>> qdisc->dev_queue->dev ->name
>>
>> [ 5301.595872] KASAN: null-ptr-deref in range
>> [0x0130-0x0137]
>> [ 5301.595877] Mem abort info:
>> [ 5301.595881] ESR = 0x9606
>> [ 5301.595885] EC = 0x25: DABT (current EL), IL = 32 bits
>> [ 5301.595889] SET = 0, FnV = 0
>> [ 5301.595893] EA = 0, S1PTW = 0
>> [ 5301.595896] FSC = 0x06: level 2 translation fault
>> [ 5301.595900] Data abort info:
>> [ 5301.595903] ISV = 0, ISS = 0x0006, ISS2 = 0x
>> [ 5301.595907] CM = 0, WnR = 0, TnD = 0, TagAccess = 0
>> [ 5301.595911] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
>> [ 5301.595915] [dfff8026] address between user and kernel
>> address ranges
>> [ 5301.595971] Internal error: Oops: 9606 [#1] SMP
>> Link:
>> https://lore.kernel.org/lkml/20240229143432.273b4...@gandalf.local.home/t/
>> Fixes: 51270d573a8d ("tracing/net_sched: Fix tracepoints that save
>> qdisc_dev() as a string")
>> Cc: net...@vger.kernel.org
>> Cc: sta...@vger.kernel.org # +v6.7.10, +v6.8
>> Signed-off-by: Yunseong Kim
>> Signed-off-by: Yeoreum Yun
>> ---
>> include/trace/events/qdisc.h | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/trace/events/qdisc.h b/include/trace/events/qdisc.h
>> index f1b5e816e7e5..170b51fbe47a 100644
>> --- a/include/trace/events/qdisc.h
>> +++ b/include/trace/events/qdisc.h
>> @@ -81,7 +81,7 @@ TRACE_EVENT(qdisc_reset,
>> TP_ARGS(q),
>> TP_STRUCT__entry(
>> - __string( dev, qdisc_dev(q)->name )
>> + __string(dev, qdisc_dev(q) ? qdisc_dev(q)->name : "noop_queue")
>> __string( kind, q->ops->id )
>> __field( u32, parent )
>> __field( u32, handle )
>
> You missed the __assign_str portion (see below). Also let's just say
> "(null)" as it's the correct device name. "noop_queue" could be misleading.
Thanks for the code review Pedro, I agree your advice.
> diff --git a/include/trace/events/qdisc.h b/include/trace/events/qdisc.h
> index 1f4258308b96..f54e0b4dbcf4 100644
> --- a/include/trace/events/qdisc.h
> +++ b/include/trace/events/qdisc.h
> @@ -81,14 +81,14 @@ TRACE_EVENT(qdisc_reset,
> TP_ARGS(q),
>
> TP_STRUCT__entry(
> - __string( dev, qdisc_dev(q)->name )
> + __string( dev, qdisc_dev(q) ?
> qdisc_dev(q)->name : "(null)" )
> __string( kind, q->ops->id )
> __field( u32, parent )
> __field( u32, handle )
> ),
It looks better to align the name with the current convention.
Link:
https://lore.kernel.org/linux-trace-kernel/2024011442.634192...@goodmis.org/
> TP_fast_assign(
> - __assign_str(dev, qdisc_dev(q)->name);
> + __assign_str(dev, qdisc_dev(q) ? qdisc_dev(q)->name :
> "(null)");
> __assign_str(kind, q->ops->id);
> __entry->parent = q->parent;
> __entry->handle = q->handle;
>
>
The second part you mentioned, Steve recently worked on it and changed it.
Link:
https://lore.kernel.org/linux-trace-kernel/20240516133454.681ba...@rorschach.local.home/
If it hadn't, I don't think I would have been able to prevent the panic
by just applying my patch.
Link:
https://lore.kernel.org/all/e2f9da4e-919d-4a98-919d-167726ef6...@gmail.com/
Warm Regards,
Yunseong Kim
[syzbot] [lvs?] possible deadlock in start_sync_thread
Hello,
syzbot found the following issue on:
HEAD commit:3226607302ca selftests: net: change shebang to bash in amt..
git tree: net-next
console output: https://syzkaller.appspot.com/x/log.txt?x=12a2683e98
kernel config: https://syzkaller.appspot.com/x/.config?x=e78fc116033e0ab7
dashboard link: https://syzkaller.appspot.com/bug?extid=e929093395ec65f969c7
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian)
2.40
Unfortunately, I don't have any reproducer for this issue yet.
Downloadable assets:
disk image:
https://storage.googleapis.com/syzbot-assets/125c85863435/disk-32266073.raw.xz
vmlinux:
https://storage.googleapis.com/syzbot-assets/4477ecab2e1f/vmlinux-32266073.xz
kernel image:
https://storage.googleapis.com/syzbot-assets/43e28f6ce879/bzImage-32266073.xz
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+e929093395ec65f96...@syzkaller.appspotmail.com
==
WARNING: possible circular locking dependency detected
6.10.0-rc4-syzkaller-00837-g3226607302ca #0 Not tainted
--
syz-executor.4/10811 is trying to acquire lock:
8f5e6f48 (rtnl_mutex){+.+.}-{3:3}, at: start_sync_thread+0xdc/0x2dc0
net/netfilter/ipvs/ip_vs_sync.c:1761
but task is already holding lock:
88805ba95750 (>clcsock_release_lock){+.+.}-{3:3}, at:
smc_setsockopt+0x1c3/0xe50 net/smc/af_smc.c:3064
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #2 (>clcsock_release_lock){+.+.}-{3:3}:
lock_acquire+0x1ed/0x550 kernel/locking/lockdep.c:5754
__mutex_lock_common kernel/locking/mutex.c:608 [inline]
__mutex_lock+0x136/0xd70 kernel/locking/mutex.c:752
smc_switch_to_fallback+0x35/0xd00 net/smc/af_smc.c:902
smc_sendmsg+0x11f/0x530 net/smc/af_smc.c:2779
sock_sendmsg_nosec net/socket.c:730 [inline]
__sock_sendmsg+0x221/0x270 net/socket.c:745
__sys_sendto+0x3a4/0x4f0 net/socket.c:2192
__do_sys_sendto net/socket.c:2204 [inline]
__se_sys_sendto net/socket.c:2200 [inline]
__x64_sys_sendto+0xde/0x100 net/socket.c:2200
do_syscall_x64 arch/x86/entry/common.c:52 [inline]
do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
entry_SYSCALL_64_after_hwframe+0x77/0x7f
-> #1 (sk_lock-AF_INET){+.+.}-{0:0}:
lock_acquire+0x1ed/0x550 kernel/locking/lockdep.c:5754
lock_sock_nested+0x48/0x100 net/core/sock.c:3543
do_ip_setsockopt+0x1a2d/0x3cd0 net/ipv4/ip_sockglue.c:1078
ip_setsockopt+0x63/0x100 net/ipv4/ip_sockglue.c:1417
do_sock_setsockopt+0x3af/0x720 net/socket.c:2312
__sys_setsockopt+0x1ae/0x250 net/socket.c:2335
__do_sys_setsockopt net/socket.c:2344 [inline]
__se_sys_setsockopt net/socket.c:2341 [inline]
__x64_sys_setsockopt+0xb5/0xd0 net/socket.c:2341
do_syscall_x64 arch/x86/entry/common.c:52 [inline]
do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
entry_SYSCALL_64_after_hwframe+0x77/0x7f
-> #0 (rtnl_mutex){+.+.}-{3:3}:
check_prev_add kernel/locking/lockdep.c:3134 [inline]
check_prevs_add kernel/locking/lockdep.c:3253 [inline]
validate_chain+0x18e0/0x5900 kernel/locking/lockdep.c:3869
__lock_acquire+0x1346/0x1fd0 kernel/locking/lockdep.c:5137
lock_acquire+0x1ed/0x550 kernel/locking/lockdep.c:5754
__mutex_lock_common kernel/locking/mutex.c:608 [inline]
__mutex_lock+0x136/0xd70 kernel/locking/mutex.c:752
start_sync_thread+0xdc/0x2dc0 net/netfilter/ipvs/ip_vs_sync.c:1761
do_ip_vs_set_ctl+0x442/0x13d0 net/netfilter/ipvs/ip_vs_ctl.c:2732
nf_setsockopt+0x295/0x2c0 net/netfilter/nf_sockopt.c:101
smc_setsockopt+0x275/0xe50 net/smc/af_smc.c:3072
do_sock_setsockopt+0x3af/0x720 net/socket.c:2312
__sys_setsockopt+0x1ae/0x250 net/socket.c:2335
__do_sys_setsockopt net/socket.c:2344 [inline]
__se_sys_setsockopt net/socket.c:2341 [inline]
__x64_sys_setsockopt+0xb5/0xd0 net/socket.c:2341
do_syscall_x64 arch/x86/entry/common.c:52 [inline]
do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
entry_SYSCALL_64_after_hwframe+0x77/0x7f
other info that might help us debug this:
Chain exists of:
rtnl_mutex --> sk_lock-AF_INET --> >clcsock_release_lock
Possible unsafe locking scenario:
CPU0CPU1
lock(>clcsock_release_lock);
lock(sk_lock-AF_INET);
lock(>clcsock_release_lock);
lock(rtnl_mutex);
*** DEADLOCK ***
1 lock held by syz-executor.4/10811:
#0: 88805ba95750 (>clcsock_release_lock){+.+.}-{3:3}, at:
smc_setsockopt+0x1c3/0xe50 net/smc/af_smc.c:3064
stack backtrace:
CPU: 0 PID: 10811 Comm: syz-executor.4 Not tainted
6.10.0-rc4-syzkaller-00837-g3226607302ca #0
Hardware name:
Re: [PATCH v3] tracing/net_sched: NULL pointer dereference in perf_trace_qdisc_reset()
On 22/06/2024 01:57, ysk...@gmail.com wrote: From: Yunseong Kim In the TRACE_EVENT(qdisc_reset) NULL dereference occurred from qdisc->dev_queue->dev ->name This situation simulated from bunch of veths and Bluetooth dis/reconnection. During qdisc initialization, qdisc was being set to noop_queue. In veth_init_queue, the initial tx_num was reduced back to one, causing the qdisc reset to be called with noop, which led to the kernel panic. I've attached the GitHub gist link that C converted syz-execprogram source code and 3 log of reproduced vmcore-dmesg. https://gist.github.com/yskelg/cc64562873ce249cdd0d5a358b77d740 Yeoreum and I use two fuzzing tool simultaneously. One process with syz-executor : https://github.com/google/syzkaller $ ./syz-execprog -executor=./syz-executor -repeat=1 -sandbox=setuid \ -enable=none -collide=false log1 The other process with perf fuzzer: https://github.com/deater/perf_event_tests/tree/master/fuzzer $ perf_event_tests/fuzzer/perf_fuzzer I think this will happen on the kernel version. Linux kernel version +v6.7.10, +v6.8, +v6.9 and it could happen in v6.10. This occurred from 51270d573a8d. I think this patch is absolutely necessary. Previously, It was showing not intended string value of name. I've reproduced 3 time from my fedora 40 Debug Kernel with any other module or patched. version: 6.10.0-0.rc2.20240608gitdc772f8237f9.29.fc41.aarch64+debug [ 5287.164555] veth0_vlan: left promiscuous mode [ 5287.164929] veth1_macvtap: left promiscuous mode [ 5287.164950] veth0_macvtap: left promiscuous mode [ 5287.164983] veth1_vlan: left promiscuous mode [ 5287.165008] veth0_vlan: left promiscuous mode [ 5287.165450] veth1_macvtap: left promiscuous mode [ 5287.165472] veth0_macvtap: left promiscuous mode [ 5287.165502] veth1_vlan: left promiscuous mode … [ 5297.598240] bridge0: port 2(bridge_slave_1) entered blocking state [ 5297.598262] bridge0: port 2(bridge_slave_1) entered forwarding state [ 5297.598296] bridge0: port 1(bridge_slave_0) entered blocking state [ 5297.598313] bridge0: port 1(bridge_slave_0) entered forwarding state [ 5297.616090] 8021q: adding VLAN 0 to HW filter on device bond0 [ 5297.620405] bridge0: port 1(bridge_slave_0) entered disabled state [ 5297.620730] bridge0: port 2(bridge_slave_1) entered disabled state [ 5297.627247] 8021q: adding VLAN 0 to HW filter on device team0 [ 5297.629636] bridge0: port 1(bridge_slave_0) entered blocking state … [ 5298.002798] bridge_slave_0: left promiscuous mode [ 5298.002869] bridge0: port 1(bridge_slave_0) entered disabled state [ 5298.309444] bond0 (unregistering): (slave bond_slave_0): Releasing backup interface [ 5298.315206] bond0 (unregistering): (slave bond_slave_1): Releasing backup interface [ 5298.320207] bond0 (unregistering): Released all slaves [ 5298.354296] hsr_slave_0: left promiscuous mode [ 5298.360750] hsr_slave_1: left promiscuous mode [ 5298.374889] veth1_macvtap: left promiscuous mode [ 5298.374931] veth0_macvtap: left promiscuous mode [ 5298.374988] veth1_vlan: left promiscuous mode [ 5298.375024] veth0_vlan: left promiscuous mode [ 5299.109741] team0 (unregistering): Port device team_slave_1 removed [ 5299.185870] team0 (unregistering): Port device team_slave_0 removed … [ 5300.155443] Bluetooth: hci3: unexpected cc 0x0c03 length: 249 > 1 [ 5300.155724] Bluetooth: hci3: unexpected cc 0x1003 length: 249 > 9 [ 5300.155988] Bluetooth: hci3: unexpected cc 0x1001 length: 249 > 9 …. [ 5301.075531] team0: Port device team_slave_1 added [ 5301.085515] bridge0: port 1(bridge_slave_0) entered blocking state [ 5301.085531] bridge0: port 1(bridge_slave_0) entered disabled state [ 5301.085588] bridge_slave_0: entered allmulticast mode [ 5301.085800] bridge_slave_0: entered promiscuous mode [ 5301.095617] bridge0: port 1(bridge_slave_0) entered blocking state [ 5301.095633] bridge0: port 1(bridge_slave_0) entered disabled state … [ 5301.149734] bond0: (slave bond_slave_0): Enslaving as an active interface with an up link [ 5301.173234] bond0: (slave bond_slave_0): Enslaving as an active interface with an up link [ 5301.180517] bond0: (slave bond_slave_1): Enslaving as an active interface with an up link [ 5301.193481] hsr_slave_0: entered promiscuous mode [ 5301.204425] hsr_slave_1: entered promiscuous mode [ 5301.210172] debugfs: Directory 'hsr0' with parent 'hsr' already present! [ 5301.210185] Cannot create hsr debugfs directory [ 5301.224061] bond0: (slave bond_slave_1): Enslaving as an active interface with an up link [ 5301.246901] bond0: (slave bond_slave_0): Enslaving as an active interface with an up link [ 5301.255934] team0: Port device team_slave_0 added [ 5301.256480] team0: Port device team_slave_1 added [ 5301.256948] team0: Port device team_slave_0 added … [ 5301.435928] hsr_slave_0: entered promiscuous mode [ 5301.446029] hsr_slave_1: entered promiscuous mode [ 5301.455872] debugfs: Directory 'hsr0' with parent 'hsr' already present! [ 5301.455884] Cannot
Re: [RFC PATCH 00/20] Introduce the famfs shared-memory file system
On 24/05/23 03:57PM, Miklos Szeredi wrote: > [trimming CC list] > > On Thu, 23 May 2024 at 04:49, John Groves wrote: > > > - memmap=! will reserve a pretend pmem device at > > > > - memmap=$ will reserve a pretend dax device at > > > > Doesn't get me a /dev/dax or /dev/pmem > > Complete qemu command line: > > qemu-kvm -s -serial none -parallel none -kernel > /home/mszeredi/git/linux/arch/x86/boot/bzImage -drive > format=raw,file=/home/mszeredi/root_fs,index=0,if=virtio -drive > format=raw,file=/home/mszeredi/images/ubd1,index=1,if=virtio -chardev > stdio,id=virtiocon0,signal=off -device virtio-serial -device > virtconsole,chardev=virtiocon0 -cpu host -m 8G -net user -net > nic,model=virtio -fsdev local,security_model=none,id=fsdev0,path=/home > -device virtio-9p-pci,fsdev=fsdev0,mount_tag=hostshare -device > virtio-rng-pci -smp 4 -append 'root=/dev/vda console=hvc0 > memmap=4G$4G' > > root@kvm:~/famfs# scripts/chk_efi.sh > This system is neither Ubuntu nor Fedora. It is identified as debian. > /sys/firmware/efi not found; probably not efi > not found; probably nof efi > /boot/efi/EFI not found; probably not efi > /boot/efi/EFI/BOOT not found; probably not efi > /boot/efi/EFI/ not found; probably not efi > /boot/efi/EFI//grub.cfg not found; probably nof efi > Probably not efi; errs=6 > > Thanks, > Miklos I'm baffled as to why the memmap thing is not working for you. I don't see anything amiss in your config file, but the actual plumbing of that kernel option isn't anything I've worked on. Out of curiosity, are you running on x86? Have you tried the 's/$/!/' method with memmap? That should give you a pmem device instead, which you will see with 'ndctl list', and can convert to devdax with ndctl (recipe above in this thread). Note that 4GiB is the minimum size that famfs supports. A quick status on where I am with famfs: I've made progress on my substantial learning curve with fuse, and have come up with a strategy for the famfs fuse daemon to access metadata in a way that leverages the current famfs user space without excessive re-writing (which is encouraging). I haven't started test-hacking dax_iomap_* enabled files into the fuse kmod yet; initial RFCs in that area are probably a few weeks out, but definitely coming - undoubtedly with a lot of questions. Regards, John
Re: [PATCH -fixes] riscv: patch: Flush the icache right after patching to avoid illegal insns
Hi Alex,
On Mon, Jun 24, 2024 at 4:21 PM Alexandre Ghiti wrote:
>
> We cannot delay the icache flush after patching some functions as we may
> have patched a function that will get called before the icache flush.
>
> The only way to completely avoid such scenario is by flushing the icache
> as soon as we patch a function. This will probably be costly as we don't
> batch the icache maintenance anymore.
>
> Fixes: 6ca445d8af0e ("riscv: Fix early ftrace nop patching")
> Reported-by: Conor Dooley
> Closes:
> https://lore.kernel.org/linux-riscv/20240613-lubricant-breath-061192a9489a@wendy/
> Signed-off-by: Alexandre Ghiti
> ---
> arch/riscv/kernel/ftrace.c | 7 ++-
> arch/riscv/kernel/patch.c | 26 ++
> 2 files changed, 20 insertions(+), 13 deletions(-)
>
> diff --git a/arch/riscv/kernel/ftrace.c b/arch/riscv/kernel/ftrace.c
> index 87cbd86576b2..4b95c574fd04 100644
> --- a/arch/riscv/kernel/ftrace.c
> +++ b/arch/riscv/kernel/ftrace.c
> @@ -120,9 +120,6 @@ int ftrace_init_nop(struct module *mod, struct dyn_ftrace
> *rec)
> out = ftrace_make_nop(mod, rec, MCOUNT_ADDR);
> mutex_unlock(_mutex);
>
> - if (!mod)
> - local_flush_icache_range(rec->ip, rec->ip + MCOUNT_INSN_SIZE);
> -
> return out;
> }
>
> @@ -156,9 +153,9 @@ static int __ftrace_modify_code(void *data)
> } else {
> while (atomic_read(>cpu_count) <= num_online_cpus())
> cpu_relax();
> - }
>
> - local_flush_icache_all();
> + local_flush_icache_all();
> + }
Sorry, maybe I should point it out directly earlier. But I don't think
this diff chunk is required. Threads running in the else clause from
stop_machine must not run into any patchable entry. If it runs into a
patchable entry, running the local fence.i is not going to fix the
problem.
>
> return 0;
> }
> diff --git a/arch/riscv/kernel/patch.c b/arch/riscv/kernel/patch.c
> index 4007563fb607..ab03732d06c4 100644
> --- a/arch/riscv/kernel/patch.c
> +++ b/arch/riscv/kernel/patch.c
> @@ -89,6 +89,14 @@ static int __patch_insn_set(void *addr, u8 c, size_t len)
>
> memset(waddr, c, len);
>
> + /*
> +* We could have just patched a function that is about to be
> +* called so make sure we don't execute partially patched
> +* instructions by flushing the icache as soon as possible.
> +*/
> + local_flush_icache_range((unsigned long)waddr,
> +(unsigned long)waddr + len);
> +
> patch_unmap(FIX_TEXT_POKE0);
>
> if (across_pages)
> @@ -135,6 +143,14 @@ static int __patch_insn_write(void *addr, const void
> *insn, size_t len)
>
> ret = copy_to_kernel_nofault(waddr, insn, len);
>
> + /*
> +* We could have just patched a function that is about to be
> +* called so make sure we don't execute partially patched
> +* instructions by flushing the icache as soon as possible.
> +*/
> + local_flush_icache_range((unsigned long)waddr,
> +(unsigned long)waddr + len);
> +
> patch_unmap(FIX_TEXT_POKE0);
>
> if (across_pages)
> @@ -189,9 +205,6 @@ int patch_text_set_nosync(void *addr, u8 c, size_t len)
>
> ret = patch_insn_set(tp, c, len);
>
> - if (!ret)
> - flush_icache_range((uintptr_t)tp, (uintptr_t)tp + len);
> -
> return ret;
> }
> NOKPROBE_SYMBOL(patch_text_set_nosync);
> @@ -224,9 +237,6 @@ int patch_text_nosync(void *addr, const void *insns,
> size_t len)
>
> ret = patch_insn_write(tp, insns, len);
>
> - if (!ret)
> - flush_icache_range((uintptr_t) tp, (uintptr_t) tp + len);
> -
> return ret;
> }
> NOKPROBE_SYMBOL(patch_text_nosync);
> @@ -253,9 +263,9 @@ static int patch_text_cb(void *data)
> } else {
> while (atomic_read(>cpu_count) <= num_online_cpus())
> cpu_relax();
> - }
>
> - local_flush_icache_all();
> + local_flush_icache_all();
> + }
>
> return ret;
> }
> --
> 2.39.2
>
Thanks,
Andy
Re: [PATCH v3 3/5] arm64: dts: qcom: sdx75: update reserved memory regions for mpss
On 6/18/2024 7:08 PM, Konrad Dybcio wrote: On 6/18/24 15:13, Naina Mehta wrote: Rename qdss@8880 memory region as qlink_logging memory region and add qdss_mem memory region at address of 0x8850. Split mpss_dsmharq_mem region into 2 separate regions and reduce the size of mpssadsp_mem region. Signed-off-by: Naina Mehta --- Alright, we're getting somewhere. The commit message should however motivate why such changes are necessary. For all we know, the splitting in two is currently done for no reason, as qdss_mem and qlink_logging_mem are contiguous - does the firmware have some expectations about them being separate? Since different DSM region size is required for different modem firmware, mpss_dsmharq_mem region being split into 2 separate regions. This would provide the flexibility to remove the region which is not required for a particular platform. qlink_logging is being added at the memory region at the address of 0x8880 as the region is being used by modem firmware. Regards, Naina Konrad
Re: [PATCH V2 3/3] virtio-net: synchronize operstate with admin state on up/down
On Mon, Jun 24, 2024 at 10:45:23AM +0800, Jason Wang wrote:
> This patch synchronize operstate with admin state per RFC2863.
>
> This is done by trying to toggle the carrier upon open/close and
> synchronize with the config change work. This allows propagate status
> correctly to stacked devices like:
>
> ip link add link enp0s3 macvlan0 type macvlan
> ip link set link enp0s3 down
> ip link show
>
> Before this patch:
>
> 3: enp0s3: mtu 1500 qdisc pfifo_fast state DOWN mode
> DEFAULT group default qlen 1000
> link/ether 00:00:05:00:00:09 brd ff:ff:ff:ff:ff:ff
> ..
> 5: macvlan0@enp0s3: mtu 1500 qdisc
> noqueue state UP mode DEFAULT group default qlen 1000
> link/ether b2:a9:c5:04:da:53 brd ff:ff:ff:ff:ff:ff
>
> After this patch:
>
> 3: enp0s3: mtu 1500 qdisc pfifo_fast state DOWN mode
> DEFAULT group default qlen 1000
> link/ether 00:00:05:00:00:09 brd ff:ff:ff:ff:ff:ff
> ...
> 5: macvlan0@enp0s3: mtu 1500 qdisc
> noqueue state LOWERLAYERDOWN mode DEFAULT group default qlen 1000
> link/ether b2:a9:c5:04:da:53 brd ff:ff:ff:ff:ff:ff
>
> Cc: Venkat Venkatsubra
> Cc: Gia-Khanh Nguyen
> Signed-off-by: Jason Wang
> ---
> drivers/net/virtio_net.c | 72 +++-
> 1 file changed, 42 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index b1f8b720733e..eff3ad3d6bcc 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -2468,6 +2468,25 @@ static int virtnet_enable_queue_pair(struct
> virtnet_info *vi, int qp_index)
> return err;
> }
>
> +static void virtnet_update_settings(struct virtnet_info *vi)
> +{
> + u32 speed;
> + u8 duplex;
> +
> + if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_SPEED_DUPLEX))
> + return;
> +
> + virtio_cread_le(vi->vdev, struct virtio_net_config, speed, );
> +
> + if (ethtool_validate_speed(speed))
> + vi->speed = speed;
> +
> + virtio_cread_le(vi->vdev, struct virtio_net_config, duplex, );
> +
> + if (ethtool_validate_duplex(duplex))
> + vi->duplex = duplex;
> +}
> +
> static int virtnet_open(struct net_device *dev)
> {
> struct virtnet_info *vi = netdev_priv(dev);
> @@ -2486,6 +2505,22 @@ static int virtnet_open(struct net_device *dev)
> goto err_enable_qp;
> }
>
> + /* Assume link up if device can't report link status,
> +otherwise get link status from config. */
> + netif_carrier_off(dev);
> + if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_STATUS)) {
> + virtio_config_enable(vi->vdev);
> + /* We are not sure if config interrupt is disabled by
> + * core or not, so we can't schedule config_work by
> + * ourselves.
> + */
This comment confuses more than it explains.
You seem to be arguing about some alternative design
you had in mind, but readers don't have it in mind.
Please just explain what this does and why.
For what: something like "Trigger re-read of config - same
as we'd do if config changed".
Now, please do what you don't do here: explain the why:
why do we want all these VM
exits on each open/close as opposed to once on probe and later on
config changed interrupt.
> + virtio_config_changed(vi->vdev);
> + } else {
> + vi->status = VIRTIO_NET_S_LINK_UP;
> + virtnet_update_settings(vi);
> + netif_carrier_on(dev);
> + }
> +
> return 0;
>
> err_enable_qp:
> @@ -2928,12 +2963,19 @@ static int virtnet_close(struct net_device *dev)
> disable_delayed_refill(vi);
> /* Make sure refill_work doesn't re-enable napi! */
> cancel_delayed_work_sync(>refill);
> + /* Make sure config notification doesn't schedule config work */
> + virtio_config_disable(vi->vdev);
> + /* Make sure status updating is cancelled */
> + cancel_work_sync(>config_work);
>
> for (i = 0; i < vi->max_queue_pairs; i++) {
> virtnet_disable_queue_pair(vi, i);
> cancel_work_sync(>rq[i].dim.work);
> }
>
> + vi->status &= ~VIRTIO_NET_S_LINK_UP;
> + netif_carrier_off(dev);
> +
> return 0;
> }
>
> @@ -4632,25 +4674,6 @@ static void virtnet_init_settings(struct net_device
> *dev)
> vi->duplex = DUPLEX_UNKNOWN;
> }
>
> -static void virtnet_update_settings(struct virtnet_info *vi)
> -{
> - u32 speed;
> - u8 duplex;
> -
> - if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_SPEED_DUPLEX))
> - return;
> -
> - virtio_cread_le(vi->vdev, struct virtio_net_config, speed, );
> -
> - if (ethtool_validate_speed(speed))
> - vi->speed = speed;
> -
> - virtio_cread_le(vi->vdev, struct virtio_net_config, duplex, );
> -
> - if (ethtool_validate_duplex(duplex))
> - vi->duplex = duplex;
> -}
> -
> static u32 virtnet_get_rxfh_key_size(struct net_device *dev)
> {
> return
Re: [PATCH V2 1/3] virtio: allow nested disabling of the configure interrupt
On Mon, Jun 24, 2024 at 10:45:21AM +0800, Jason Wang wrote:
> Somtime driver may want to enable or disable the config callback. This
> requires a synchronization with the core. So this patch change the
> config_enabled to be a integer counter. This allows the toggling of
> the config_enable to be synchronized between the virtio core and the
> virtio driver.
>
> The counter is not allowed to be increased greater than one, this
> simplifies the logic where the interrupt could be disabled immediately
> without extra synchronization between driver and core.
>
> Signed-off-by: Jason Wang
> ---
> drivers/virtio/virtio.c | 20 +---
> include/linux/virtio.h | 2 +-
> 2 files changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
> index b968b2aa5f4d..d3aa74b8ae5d 100644
> --- a/drivers/virtio/virtio.c
> +++ b/drivers/virtio/virtio.c
> @@ -127,7 +127,7 @@ static void __virtio_config_changed(struct virtio_device
> *dev)
> {
> struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
>
> - if (!dev->config_enabled)
> + if (dev->config_enabled < 1)
> dev->config_change_pending = true;
> else if (drv && drv->config_changed)
> drv->config_changed(dev);
> @@ -146,17 +146,23 @@ EXPORT_SYMBOL_GPL(virtio_config_changed);
> static void virtio_config_disable(struct virtio_device *dev)
> {
> spin_lock_irq(>config_lock);
> - dev->config_enabled = false;
> + --dev->config_enabled;
> spin_unlock_irq(>config_lock);
> }
>
> static void virtio_config_enable(struct virtio_device *dev)
> {
> spin_lock_irq(>config_lock);
> - dev->config_enabled = true;
> - if (dev->config_change_pending)
> - __virtio_config_changed(dev);
> - dev->config_change_pending = false;
> +
> + if (dev->config_enabled < 1) {
> + ++dev->config_enabled;
> + if (dev->config_enabled == 1 &&
> + dev->config_change_pending) {
> + __virtio_config_changed(dev);
> + dev->config_change_pending = false;
> + }
> + }
> +
> spin_unlock_irq(>config_lock);
> }
>
So every disable decrements the counter. Enable only increments it up to 1.
You seem to be making some very specific assumptions
about how this API will be used. Any misuse will lead to under/overflow
eventually ...
My suggestion would be to
1. rename config_enabled to config_core_enabled
2. rename virtio_config_enable/disable to virtio_config_core_enable/disable
3. add bool config_driver_disabled and make virtio_config_enable/disable
switch that.
4. Change logic from dev->config_enabled to
dev->config_core_enabled && !dev->config_driver_disabled
> @@ -455,7 +461,7 @@ int register_virtio_device(struct virtio_device *dev)
> goto out_ida_remove;
>
> spin_lock_init(>config_lock);
> - dev->config_enabled = false;
> + dev->config_enabled = 0;
> dev->config_change_pending = false;
>
> INIT_LIST_HEAD(>vqs);
> diff --git a/include/linux/virtio.h b/include/linux/virtio.h
> index 96fea920873b..4496f9ba5d82 100644
> --- a/include/linux/virtio.h
> +++ b/include/linux/virtio.h
> @@ -132,7 +132,7 @@ struct virtio_admin_cmd {
> struct virtio_device {
> int index;
> bool failed;
> - bool config_enabled;
> + int config_enabled;
> bool config_change_pending;
> spinlock_t config_lock;
> spinlock_t vqs_list_lock;
> --
> 2.31.1
Re: [PATCH v4 11/11] riscv: Enable DAX VMEMMAP optimization
On Wed, Jun 5, 2024 at 1:42 PM Björn Töpel wrote: > > From: Björn Töpel > > Now that DAX is usable, enable the DAX VMEMMAP optimization as well. > > Signed-off-by: Björn Töpel > --- > arch/riscv/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > index 8a49b5f4c017..1631bf568158 100644 > --- a/arch/riscv/Kconfig > +++ b/arch/riscv/Kconfig > @@ -73,6 +73,7 @@ config RISCV > select ARCH_WANT_GENERAL_HUGETLB if !RISCV_ISA_SVNAPOT > select ARCH_WANT_HUGE_PMD_SHARE if 64BIT > select ARCH_WANT_LD_ORPHAN_WARN if !XIP_KERNEL > + select ARCH_WANT_OPTIMIZE_DAX_VMEMMAP > select ARCH_WANT_OPTIMIZE_HUGETLB_VMEMMAP > select ARCH_WANTS_NO_INSTR > select ARCH_WANTS_THP_SWAP if HAVE_ARCH_TRANSPARENT_HUGEPAGE > -- > 2.43.0 > Reviewed-by: Alexandre Ghiti Thanks, Alex
Re: [PATCH v4 06/11] riscv: mm: Add memory hotplugging support
On Wed, Jun 5, 2024 at 1:41 PM Björn Töpel wrote:
>
> From: Björn Töpel
>
> For an architecture to support memory hotplugging, a couple of
> callbacks needs to be implemented:
>
> arch_add_memory()
> This callback is responsible for adding the physical memory into the
> direct map, and call into the memory hotplugging generic code via
> __add_pages() that adds the corresponding struct page entries, and
> updates the vmemmap mapping.
>
> arch_remove_memory()
> This is the inverse of the callback above.
>
> vmemmap_free()
> This function tears down the vmemmap mappings (if
> CONFIG_SPARSEMEM_VMEMMAP is enabled), and also deallocates the
> backing vmemmap pages. Note that for persistent memory, an
> alternative allocator for the backing pages can be used; The
> vmem_altmap. This means that when the backing pages are cleared,
> extra care is needed so that the correct deallocation method is
> used.
>
> arch_get_mappable_range()
> This functions returns the PA range that the direct map can map.
> Used by the MHP internals for sanity checks.
>
> The page table unmap/teardown functions are heavily based on code from
> the x86 tree. The same remove_pgd_mapping() function is used in both
> vmemmap_free() and arch_remove_memory(), but in the latter function
> the backing pages are not removed.
>
> Signed-off-by: Björn Töpel
> ---
> arch/riscv/mm/init.c | 267 +++
> 1 file changed, 267 insertions(+)
>
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index 1f7e7c223bec..bfa2dea95354 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -1534,3 +1534,270 @@ struct execmem_info __init *execmem_arch_setup(void)
> }
> #endif /* CONFIG_MMU */
> #endif /* CONFIG_EXECMEM */
> +
> +#ifdef CONFIG_MEMORY_HOTPLUG
> +static void __meminit free_pte_table(pte_t *pte_start, pmd_t *pmd)
> +{
> + struct page *page = pmd_page(*pmd);
> + struct ptdesc *ptdesc = page_ptdesc(page);
> + pte_t *pte;
> + int i;
> +
> + for (i = 0; i < PTRS_PER_PTE; i++) {
> + pte = pte_start + i;
> + if (!pte_none(*pte))
> + return;
> + }
> +
> + pagetable_pte_dtor(ptdesc);
> + if (PageReserved(page))
> + free_reserved_page(page);
> + else
> + pagetable_free(ptdesc);
> + pmd_clear(pmd);
> +}
> +
> +static void __meminit free_pmd_table(pmd_t *pmd_start, pud_t *pud)
> +{
> + struct page *page = pud_page(*pud);
> + struct ptdesc *ptdesc = page_ptdesc(page);
> + pmd_t *pmd;
> + int i;
> +
> + for (i = 0; i < PTRS_PER_PMD; i++) {
> + pmd = pmd_start + i;
> + if (!pmd_none(*pmd))
> + return;
> + }
> +
> + pagetable_pmd_dtor(ptdesc);
> + if (PageReserved(page))
> + free_reserved_page(page);
> + else
> + pagetable_free(ptdesc);
> + pud_clear(pud);
> +}
> +
> +static void __meminit free_pud_table(pud_t *pud_start, p4d_t *p4d)
> +{
> + struct page *page = p4d_page(*p4d);
> + pud_t *pud;
> + int i;
> +
> + for (i = 0; i < PTRS_PER_PUD; i++) {
> + pud = pud_start + i;
> + if (!pud_none(*pud))
> + return;
> + }
> +
> + if (PageReserved(page))
> + free_reserved_page(page);
> + else
> + free_pages((unsigned long)page_address(page), 0);
> + p4d_clear(p4d);
> +}
> +
> +static void __meminit free_vmemmap_storage(struct page *page, size_t size,
> + struct vmem_altmap *altmap)
> +{
> + int order = get_order(size);
> +
> + if (altmap) {
> + vmem_altmap_free(altmap, size >> PAGE_SHIFT);
> + return;
> + }
> +
> + if (PageReserved(page)) {
> + unsigned int nr_pages = 1 << order;
> +
> + while (nr_pages--)
> + free_reserved_page(page++);
> + return;
> + }
> +
> + free_pages((unsigned long)page_address(page), order);
> +}
> +
> +static void __meminit remove_pte_mapping(pte_t *pte_base, unsigned long
> addr, unsigned long end,
> +bool is_vmemmap, struct vmem_altmap
> *altmap)
> +{
> + unsigned long next;
> + pte_t *ptep, pte;
> +
> + for (; addr < end; addr = next) {
> + next = (addr + PAGE_SIZE) & PAGE_MASK;
> + if (next > end)
> + next = end;
> +
> + ptep = pte_base + pte_index(addr);
> + pte = ptep_get(ptep);
> + if (!pte_present(*ptep))
> + continue;
> +
> + pte_clear(_mm, addr, ptep);
> + if (is_vmemmap)
> + free_vmemmap_storage(pte_page(pte), PAGE_SIZE,
> altmap);
> + }
> +}
> +
>
Re: [PATCH v4 05/11] riscv: mm: Add pfn_to_kaddr() implementation
(resending as gmail client turned to html)
On Wed, Jun 5, 2024 at 1:41 PM Björn Töpel wrote:
>
> From: Björn Töpel
>
> The pfn_to_kaddr() function is used by KASAN's memory hotplugging
> path. Add the missing function to the RISC-V port, so that it can be
> built with MHP and CONFIG_KASAN.
>
> Signed-off-by: Björn Töpel
> ---
> arch/riscv/include/asm/page.h | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/arch/riscv/include/asm/page.h b/arch/riscv/include/asm/page.h
> index 115ac98b8d72..235fd45d998d 100644
> --- a/arch/riscv/include/asm/page.h
> +++ b/arch/riscv/include/asm/page.h
> @@ -188,6 +188,11 @@ extern phys_addr_t __phys_addr_symbol(unsigned long x);
>
> unsigned long kaslr_offset(void);
>
> +static __always_inline void *pfn_to_kaddr(unsigned long pfn)
> +{
> + return __va(pfn << PAGE_SHIFT);
> +}
> +
> #endif /* __ASSEMBLY__ */
>
> #define virt_addr_valid(vaddr) ({
> \
> --
> 2.43.0
>
Reviewed-by: Alexandre Ghiti
Thanks,
Alex
[PATCH -fixes] riscv: patch: Flush the icache right after patching to avoid illegal insns
We cannot delay the icache flush after patching some functions as we may
have patched a function that will get called before the icache flush.
The only way to completely avoid such scenario is by flushing the icache
as soon as we patch a function. This will probably be costly as we don't
batch the icache maintenance anymore.
Fixes: 6ca445d8af0e ("riscv: Fix early ftrace nop patching")
Reported-by: Conor Dooley
Closes:
https://lore.kernel.org/linux-riscv/20240613-lubricant-breath-061192a9489a@wendy/
Signed-off-by: Alexandre Ghiti
---
arch/riscv/kernel/ftrace.c | 7 ++-
arch/riscv/kernel/patch.c | 26 ++
2 files changed, 20 insertions(+), 13 deletions(-)
diff --git a/arch/riscv/kernel/ftrace.c b/arch/riscv/kernel/ftrace.c
index 87cbd86576b2..4b95c574fd04 100644
--- a/arch/riscv/kernel/ftrace.c
+++ b/arch/riscv/kernel/ftrace.c
@@ -120,9 +120,6 @@ int ftrace_init_nop(struct module *mod, struct dyn_ftrace
*rec)
out = ftrace_make_nop(mod, rec, MCOUNT_ADDR);
mutex_unlock(_mutex);
- if (!mod)
- local_flush_icache_range(rec->ip, rec->ip + MCOUNT_INSN_SIZE);
-
return out;
}
@@ -156,9 +153,9 @@ static int __ftrace_modify_code(void *data)
} else {
while (atomic_read(>cpu_count) <= num_online_cpus())
cpu_relax();
- }
- local_flush_icache_all();
+ local_flush_icache_all();
+ }
return 0;
}
diff --git a/arch/riscv/kernel/patch.c b/arch/riscv/kernel/patch.c
index 4007563fb607..ab03732d06c4 100644
--- a/arch/riscv/kernel/patch.c
+++ b/arch/riscv/kernel/patch.c
@@ -89,6 +89,14 @@ static int __patch_insn_set(void *addr, u8 c, size_t len)
memset(waddr, c, len);
+ /*
+* We could have just patched a function that is about to be
+* called so make sure we don't execute partially patched
+* instructions by flushing the icache as soon as possible.
+*/
+ local_flush_icache_range((unsigned long)waddr,
+(unsigned long)waddr + len);
+
patch_unmap(FIX_TEXT_POKE0);
if (across_pages)
@@ -135,6 +143,14 @@ static int __patch_insn_write(void *addr, const void
*insn, size_t len)
ret = copy_to_kernel_nofault(waddr, insn, len);
+ /*
+* We could have just patched a function that is about to be
+* called so make sure we don't execute partially patched
+* instructions by flushing the icache as soon as possible.
+*/
+ local_flush_icache_range((unsigned long)waddr,
+(unsigned long)waddr + len);
+
patch_unmap(FIX_TEXT_POKE0);
if (across_pages)
@@ -189,9 +205,6 @@ int patch_text_set_nosync(void *addr, u8 c, size_t len)
ret = patch_insn_set(tp, c, len);
- if (!ret)
- flush_icache_range((uintptr_t)tp, (uintptr_t)tp + len);
-
return ret;
}
NOKPROBE_SYMBOL(patch_text_set_nosync);
@@ -224,9 +237,6 @@ int patch_text_nosync(void *addr, const void *insns, size_t
len)
ret = patch_insn_write(tp, insns, len);
- if (!ret)
- flush_icache_range((uintptr_t) tp, (uintptr_t) tp + len);
-
return ret;
}
NOKPROBE_SYMBOL(patch_text_nosync);
@@ -253,9 +263,9 @@ static int patch_text_cb(void *data)
} else {
while (atomic_read(>cpu_count) <= num_online_cpus())
cpu_relax();
- }
- local_flush_icache_all();
+ local_flush_icache_all();
+ }
return ret;
}
--
2.39.2
Re: [PATCH] riscv: Fix early ftrace nop patching
On 20/06/2024 19:03, Alexandre Ghiti wrote:
On 19/06/2024 05:40, Andy Chiu wrote:
On Tue, Jun 18, 2024 at 9:40 PM Alexandre Ghiti
wrote:
Hi Andy,
On Tue, Jun 18, 2024 at 2:48 PM Andy Chiu wrote:
On Tue, Jun 18, 2024 at 8:02 PM Alexandre Ghiti wrote:
Hi Conor,
On 17/06/2024 15:23, Alexandre Ghiti wrote:
Hi Conor,
Sorry for the delay here.
On 13/06/2024 09:48, Conor Dooley wrote:
On Thu, May 23, 2024 at 01:51:34PM +0200, Alexandre Ghiti wrote:
Commit c97bf629963e ("riscv: Fix text patching when IPI are used")
converted ftrace_make_nop() to use patch_insn_write() which
does not
emit any icache flush relying entirely on
__ftrace_modify_code() to do
that.
But we missed that ftrace_make_nop() was called very early
directly
when
converting mcount calls into nops (actually on riscv it
converts 2B
nops
emitted by the compiler into 4B nops).
This caused crashes on multiple HW as reported by Conor and
Björn since
the booting core could have half-patched instructions in its
icache
which would trigger an illegal instruction trap: fix this by
emitting a
local flush icache when early patching nops.
Fixes: c97bf629963e ("riscv: Fix text patching when IPI are used")
Signed-off-by: Alexandre Ghiti
---
arch/riscv/include/asm/cacheflush.h | 6 ++
arch/riscv/kernel/ftrace.c | 3 +++
2 files changed, 9 insertions(+)
diff --git a/arch/riscv/include/asm/cacheflush.h
b/arch/riscv/include/asm/cacheflush.h
index dd8d07146116..ce79c558a4c8 100644
--- a/arch/riscv/include/asm/cacheflush.h
+++ b/arch/riscv/include/asm/cacheflush.h
@@ -13,6 +13,12 @@ static inline void local_flush_icache_all(void)
asm volatile ("fence.i" ::: "memory");
}
+static inline void local_flush_icache_range(unsigned long
start,
+ unsigned long end)
+{
+ local_flush_icache_all();
+}
+
#define PG_dcache_clean PG_arch_1
static inline void flush_dcache_folio(struct folio *folio)
diff --git a/arch/riscv/kernel/ftrace.c
b/arch/riscv/kernel/ftrace.c
index 4f4987a6d83d..32e7c401dfb4 100644
--- a/arch/riscv/kernel/ftrace.c
+++ b/arch/riscv/kernel/ftrace.c
@@ -120,6 +120,9 @@ int ftrace_init_nop(struct module *mod, struct
dyn_ftrace *rec)
out = ftrace_make_nop(mod, rec, MCOUNT_ADDR);
mutex_unlock(_mutex);
So, turns out that this patch is not sufficient. I've seen some
more
crashes, seemingly due to initialising nops on this mutex_unlock().
Palmer suggested moving the if (!mod) ... into the lock, which
fixed
the problem for me.
Ok, it makes sense, I completely missed that. I'll send a fix for
that
shortly so that it can be merged in rc5.
Thanks,
Alex
So I digged a bit more and I'm afraid that the only way to make sure
this issue does not happen elsewhere is to flush the icache right
after
the patching. We actually can't wait to batch the icache flush since
along the way, we may call a function that has just been patched (the
issue that you encountered here).
Trying to provide my thoughts, please let me know if I missed
anything. I think what Conor suggested is safe for init_nop, as it
would be called only when there is only one core (booting) and at the
loading stage of kernel modules. In the first case we just have to
make sure there is no patchable entry before the core executes
fence.i. The second case is unconditionally safe because there is no
read-side of the race.
It is a bit tricky for the generic (runtime) case of ftrace code
patching, but that is not because of the batch fence.i maintenance. As
long as there exists a patchable entry for the stopping thread, it is
possible for them to execute on a partially patched instruction. A
solution for this is again to prevent any patchable entry in the
stop_machine's stopping thread. Another solution is to apply the
atomic ftrace patching series which aims to get rid of the race.
Yeah but my worry is that we can't make sure that functions called
between the patching and the fence.i are not the ones that just get
patched. At least today, patch_insn_write() etc should be marked as
IIUC, the compiler does not generate a patchable entry for
patch_insn_write, and so do all functions in patch.c. The entire file
is not compiled with patchable entry because the flag is removed in
riscv's Makefile. Please let me know if I misunderstand it.
noinstr. But even then, we call core functions that could be patchable
(I went through all and it *seems* we are safe *for now*, but this is
very fragile).
Yes, functions in the "else" clause, atomic_read() etc, are safe for
now. However, it is impossible to fix as long as there exists a
patchable entry along the way. This is the problem that we cannot
patch 2 instructions with a concurrent read-side. On the other hand,
the path of ftrace_modify_all_code may experience the batch fence.i
maintenance issue, and can be fixed via a local fence.i. This is
because the path is executed by only one core. There does not exist a
concurrent write-side. As a result, we
Re: [PATCH v7 18/38] mm: slub: Disable KMSAN when checking the padding bytes
On 6/21/24 1:35 PM, Ilya Leoshkevich wrote:
> Even though the KMSAN warnings generated by memchr_inv() are suppressed
> by metadata_access_enable(), its return value may still be poisoned.
>
> The reason is that the last iteration of memchr_inv() returns
> `*start != value ? start : NULL`, where *start is poisoned. Because of
> this, somewhat counterintuitively, the shadow value computed by
> visitSelectInst() is equal to `(uintptr_t)start`.
>
> One possibility to fix this, since the intention behind guarding
> memchr_inv() behind metadata_access_enable() is to touch poisoned
> metadata without triggering KMSAN, is to unpoison its return value.
> However, this approach is too fragile. So simply disable the KMSAN
> checks in the respective functions.
>
> Reviewed-by: Alexander Potapenko
> Signed-off-by: Ilya Leoshkevich
Acked-by: Vlastimil Babka
> ---
> mm/slub.c | 16
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/mm/slub.c b/mm/slub.c
> index b050e528112c..fcd68fcea4ab 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -1176,9 +1176,16 @@ static void restore_bytes(struct kmem_cache *s, char
> *message, u8 data,
> memset(from, data, to - from);
> }
>
> -static int check_bytes_and_report(struct kmem_cache *s, struct slab *slab,
> - u8 *object, char *what,
> - u8 *start, unsigned int value, unsigned int bytes)
> +#ifdef CONFIG_KMSAN
> +#define pad_check_attributes noinline __no_kmsan_checks
> +#else
> +#define pad_check_attributes
> +#endif
> +
> +static pad_check_attributes int
> +check_bytes_and_report(struct kmem_cache *s, struct slab *slab,
> +u8 *object, char *what,
> +u8 *start, unsigned int value, unsigned int bytes)
> {
> u8 *fault;
> u8 *end;
> @@ -1270,7 +1277,8 @@ static int check_pad_bytes(struct kmem_cache *s, struct
> slab *slab, u8 *p)
> }
>
> /* Check the pad bytes at the end of a slab page */
> -static void slab_pad_check(struct kmem_cache *s, struct slab *slab)
> +static pad_check_attributes void
> +slab_pad_check(struct kmem_cache *s, struct slab *slab)
> {
> u8 *start;
> u8 *fault;
[PATCH V2 3/3] virtio-net: synchronize operstate with admin state on up/down
This patch synchronize operstate with admin state per RFC2863.
This is done by trying to toggle the carrier upon open/close and
synchronize with the config change work. This allows propagate status
correctly to stacked devices like:
ip link add link enp0s3 macvlan0 type macvlan
ip link set link enp0s3 down
ip link show
Before this patch:
3: enp0s3: mtu 1500 qdisc pfifo_fast state DOWN mode
DEFAULT group default qlen 1000
link/ether 00:00:05:00:00:09 brd ff:ff:ff:ff:ff:ff
..
5: macvlan0@enp0s3: mtu 1500 qdisc
noqueue state UP mode DEFAULT group default qlen 1000
link/ether b2:a9:c5:04:da:53 brd ff:ff:ff:ff:ff:ff
After this patch:
3: enp0s3: mtu 1500 qdisc pfifo_fast state DOWN mode
DEFAULT group default qlen 1000
link/ether 00:00:05:00:00:09 brd ff:ff:ff:ff:ff:ff
...
5: macvlan0@enp0s3: mtu 1500 qdisc
noqueue state LOWERLAYERDOWN mode DEFAULT group default qlen 1000
link/ether b2:a9:c5:04:da:53 brd ff:ff:ff:ff:ff:ff
Cc: Venkat Venkatsubra
Cc: Gia-Khanh Nguyen
Signed-off-by: Jason Wang
---
drivers/net/virtio_net.c | 72 +++-
1 file changed, 42 insertions(+), 30 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index b1f8b720733e..eff3ad3d6bcc 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2468,6 +2468,25 @@ static int virtnet_enable_queue_pair(struct virtnet_info
*vi, int qp_index)
return err;
}
+static void virtnet_update_settings(struct virtnet_info *vi)
+{
+ u32 speed;
+ u8 duplex;
+
+ if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_SPEED_DUPLEX))
+ return;
+
+ virtio_cread_le(vi->vdev, struct virtio_net_config, speed, );
+
+ if (ethtool_validate_speed(speed))
+ vi->speed = speed;
+
+ virtio_cread_le(vi->vdev, struct virtio_net_config, duplex, );
+
+ if (ethtool_validate_duplex(duplex))
+ vi->duplex = duplex;
+}
+
static int virtnet_open(struct net_device *dev)
{
struct virtnet_info *vi = netdev_priv(dev);
@@ -2486,6 +2505,22 @@ static int virtnet_open(struct net_device *dev)
goto err_enable_qp;
}
+ /* Assume link up if device can't report link status,
+ otherwise get link status from config. */
+ netif_carrier_off(dev);
+ if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_STATUS)) {
+ virtio_config_enable(vi->vdev);
+ /* We are not sure if config interrupt is disabled by
+* core or not, so we can't schedule config_work by
+* ourselves.
+*/
+ virtio_config_changed(vi->vdev);
+ } else {
+ vi->status = VIRTIO_NET_S_LINK_UP;
+ virtnet_update_settings(vi);
+ netif_carrier_on(dev);
+ }
+
return 0;
err_enable_qp:
@@ -2928,12 +2963,19 @@ static int virtnet_close(struct net_device *dev)
disable_delayed_refill(vi);
/* Make sure refill_work doesn't re-enable napi! */
cancel_delayed_work_sync(>refill);
+ /* Make sure config notification doesn't schedule config work */
+ virtio_config_disable(vi->vdev);
+ /* Make sure status updating is cancelled */
+ cancel_work_sync(>config_work);
for (i = 0; i < vi->max_queue_pairs; i++) {
virtnet_disable_queue_pair(vi, i);
cancel_work_sync(>rq[i].dim.work);
}
+ vi->status &= ~VIRTIO_NET_S_LINK_UP;
+ netif_carrier_off(dev);
+
return 0;
}
@@ -4632,25 +4674,6 @@ static void virtnet_init_settings(struct net_device *dev)
vi->duplex = DUPLEX_UNKNOWN;
}
-static void virtnet_update_settings(struct virtnet_info *vi)
-{
- u32 speed;
- u8 duplex;
-
- if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_SPEED_DUPLEX))
- return;
-
- virtio_cread_le(vi->vdev, struct virtio_net_config, speed, );
-
- if (ethtool_validate_speed(speed))
- vi->speed = speed;
-
- virtio_cread_le(vi->vdev, struct virtio_net_config, duplex, );
-
- if (ethtool_validate_duplex(duplex))
- vi->duplex = duplex;
-}
-
static u32 virtnet_get_rxfh_key_size(struct net_device *dev)
{
return ((struct virtnet_info *)netdev_priv(dev))->rss_key_size;
@@ -5958,17 +5981,6 @@ static int virtnet_probe(struct virtio_device *vdev)
goto free_unregister_netdev;
}
- /* Assume link up if device can't report link status,
- otherwise get link status from config. */
- netif_carrier_off(dev);
- if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_STATUS)) {
- schedule_work(>config_work);
- } else {
- vi->status = VIRTIO_NET_S_LINK_UP;
- virtnet_update_settings(vi);
- netif_carrier_on(dev);
- }
-
for (i = 0; i < ARRAY_SIZE(guest_offloads); i++)
if
[PATCH V2 2/3] virtio: export virtio_config_{enable|disable}()
This patch exports virtio_config_enable() and virtio_config_disable()
for drivers to disable config callbacks.
Signed-off-by: Jason Wang
---
drivers/virtio/virtio.c | 6 --
include/linux/virtio.h | 3 +++
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index d3aa74b8ae5d..a4622a62aac3 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -143,14 +143,15 @@ void virtio_config_changed(struct virtio_device *dev)
}
EXPORT_SYMBOL_GPL(virtio_config_changed);
-static void virtio_config_disable(struct virtio_device *dev)
+void virtio_config_disable(struct virtio_device *dev)
{
spin_lock_irq(>config_lock);
--dev->config_enabled;
spin_unlock_irq(>config_lock);
}
+EXPORT_SYMBOL_GPL(virtio_config_disable);
-static void virtio_config_enable(struct virtio_device *dev)
+void virtio_config_enable(struct virtio_device *dev)
{
spin_lock_irq(>config_lock);
@@ -165,6 +166,7 @@ static void virtio_config_enable(struct virtio_device *dev)
spin_unlock_irq(>config_lock);
}
+EXPORT_SYMBOL_GPL(virtio_config_enable);
void virtio_add_status(struct virtio_device *dev, unsigned int status)
{
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 4496f9ba5d82..bc5e408582c9 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -163,6 +163,9 @@ void __virtqueue_break(struct virtqueue *_vq);
void __virtqueue_unbreak(struct virtqueue *_vq);
void virtio_config_changed(struct virtio_device *dev);
+
+void virtio_config_disable(struct virtio_device *dev);
+void virtio_config_enable(struct virtio_device *dev);
#ifdef CONFIG_PM_SLEEP
int virtio_device_freeze(struct virtio_device *dev);
int virtio_device_restore(struct virtio_device *dev);
--
2.31.1
[PATCH V2 1/3] virtio: allow nested disabling of the configure interrupt
Somtime driver may want to enable or disable the config callback. This
requires a synchronization with the core. So this patch change the
config_enabled to be a integer counter. This allows the toggling of
the config_enable to be synchronized between the virtio core and the
virtio driver.
The counter is not allowed to be increased greater than one, this
simplifies the logic where the interrupt could be disabled immediately
without extra synchronization between driver and core.
Signed-off-by: Jason Wang
---
drivers/virtio/virtio.c | 20 +---
include/linux/virtio.h | 2 +-
2 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index b968b2aa5f4d..d3aa74b8ae5d 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -127,7 +127,7 @@ static void __virtio_config_changed(struct virtio_device
*dev)
{
struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
- if (!dev->config_enabled)
+ if (dev->config_enabled < 1)
dev->config_change_pending = true;
else if (drv && drv->config_changed)
drv->config_changed(dev);
@@ -146,17 +146,23 @@ EXPORT_SYMBOL_GPL(virtio_config_changed);
static void virtio_config_disable(struct virtio_device *dev)
{
spin_lock_irq(>config_lock);
- dev->config_enabled = false;
+ --dev->config_enabled;
spin_unlock_irq(>config_lock);
}
static void virtio_config_enable(struct virtio_device *dev)
{
spin_lock_irq(>config_lock);
- dev->config_enabled = true;
- if (dev->config_change_pending)
- __virtio_config_changed(dev);
- dev->config_change_pending = false;
+
+ if (dev->config_enabled < 1) {
+ ++dev->config_enabled;
+ if (dev->config_enabled == 1 &&
+ dev->config_change_pending) {
+ __virtio_config_changed(dev);
+ dev->config_change_pending = false;
+ }
+ }
+
spin_unlock_irq(>config_lock);
}
@@ -455,7 +461,7 @@ int register_virtio_device(struct virtio_device *dev)
goto out_ida_remove;
spin_lock_init(>config_lock);
- dev->config_enabled = false;
+ dev->config_enabled = 0;
dev->config_change_pending = false;
INIT_LIST_HEAD(>vqs);
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 96fea920873b..4496f9ba5d82 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -132,7 +132,7 @@ struct virtio_admin_cmd {
struct virtio_device {
int index;
bool failed;
- bool config_enabled;
+ int config_enabled;
bool config_change_pending;
spinlock_t config_lock;
spinlock_t vqs_list_lock;
--
2.31.1
[PATCH V2 0/3] virtio-net: synchronize operstate with admin state on up/down
Hi All:
Currently, we don't synchronize the opersteate with the admin state
currently when prevent the link status to be propagated to the stacked
device on top of the virtio-net.
This patch fixes this issue.
Changes since V1:
- Try to reuse the virtio core facility to enable and disable the
configure interrupt
Jason Wang (3):
virtio: allow nested disabling of the configure interrupt
virtio: export virtio_config_{enable|disable}()
virtio-net: synchronize operstate with admin state on up/down
drivers/net/virtio_net.c | 72 +++-
drivers/virtio/virtio.c | 26 ++-
include/linux/virtio.h | 5 ++-
3 files changed, 63 insertions(+), 40 deletions(-)
--
2.31.1
Re: (subset) [PATCH v2 0/3] Add Sony Xperia Z3 Compact smartphone
On Fri, 21 Jun 2024 17:26:41 +0300, Valeriy Klimin wrote: > This is almost the same as the dts of the Xperia Z3, except for the > battery charge limits. > > The current on the l21 regulator for shinano is also bumped up > to stop SD card errors. > > > [...] Applied, thanks! [1/3] dt-bindings: arm: qcom: Add Sony Xperia Z3 Compact commit: a69274e1c6f557c2fa7f35f194acb51d723adbc8 Best regards, -- Bjorn Andersson
Re: [PATCH 0/2] Add PM8008 regulator support for Fairphone 4 & 5
On Fri, 21 Jun 2024 10:42:29 +0200, Luca Weiss wrote: > With the PM8008 regulator driver scheduled for Linux v6.11[0] let's add > the dts bits for Fairphone 4 and Fairphone 5 which both use this PMIC > for powering the camera sensors - and the pull-up for the CCI (camera > I2C bus). > > [0] > https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git/log/?h=ib-mfd-regulator-pm8008-6.11 > > [...] Applied, thanks! [1/2] arm64: dts: qcom: sm7225-fairphone-fp4: Configure PM8008 regulators commit: d315b45ab8b312d6e74d85064ef916aafd1bbdef [2/2] arm64: dts: qcom: qcm6490-fairphone-fp5: Configure PM8008 regulators commit: 2cf5ec58e87bf4df1b360ab45c047d2b311930c8 Best regards, -- Bjorn Andersson
Re: (subset) [PATCH 0/5] Use mboxes in smsm node for all dtsi where possible
On Wed, 19 Jun 2024 18:42:26 +0200, Luca Weiss wrote: > With the binding and driver patches queued for 6.11[0][1], it's time to > update the dtsi files to use the new binding. > > [0] > https://lore.kernel.org/linux-arm-msm/171840533352.102487.9576671528001022676.b4...@kernel.org/ > [1] > https://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git/log/?h=drivers-for-6.11 > > @Bjorn: I think this makes sense to only apply these patches for 6.12 so > that also in the arm64 tree the driver will exist already, so git bisect > is not impeded by that. > > [...] Applied, thanks! [2/5] arm64: dts: qcom: msm8916: Use mboxes in smsm node commit: d605f9c75949997150dbb32bf082695326d3e110 [3/5] arm64: dts: qcom: msm8939: Use mboxes in smsm node commit: 9f8b7c4e3d8bbb6eb787752ad14a82e714d917ff [4/5] arm64: dts: qcom: msm8953: Use mboxes in smsm node commit: e36402b55684c64af23575f39e0a6ce27272b5f7 [5/5] arm64: dts: qcom: msm8976: Use mboxes in smsm node commit: 585141c57a49315f6522d5f7265a3f1aa05424c1 Best regards, -- Bjorn Andersson
Re: [PATCH] virtio: add missing MODULE_DESCRIPTION() macro
On 6/2/2024 1:25 PM, Jeff Johnson wrote:
> make allmodconfig && make W=1 C=1 reports:
> WARNING: modpost: missing MODULE_DESCRIPTION() in
> drivers/virtio/virtio_dma_buf.o
>
> Add the missing invocation of the MODULE_DESCRIPTION() macro.
>
> Signed-off-by: Jeff Johnson
> ---
> drivers/virtio/virtio_dma_buf.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/virtio/virtio_dma_buf.c b/drivers/virtio/virtio_dma_buf.c
> index 2521a75009c3..3034a2f605c8 100644
> --- a/drivers/virtio/virtio_dma_buf.c
> +++ b/drivers/virtio/virtio_dma_buf.c
> @@ -85,5 +85,6 @@ int virtio_dma_buf_get_uuid(struct dma_buf *dma_buf,
> }
> EXPORT_SYMBOL(virtio_dma_buf_get_uuid);
>
> +MODULE_DESCRIPTION("dma-bufs for virtio exported objects");
> MODULE_LICENSE("GPL");
> MODULE_IMPORT_NS(DMA_BUF);
>
> ---
> base-commit: 83814698cf48ce3aadc5d88a3f577f04482ff92a
> change-id: 20240602-md-virtio_dma_buf-b3552ca6c5d5
>
Following up to see if anything else is needed from me.
Hoping to see this in linux-next :)
Re: [PATCH] ring-buffer: Limit time with disabled interrupts in rb_check_pages()
On 6/21/24 17:09, Petr Pavlu wrote: > Function rb_check_pages() validates the integrity of a specified per-CPU > tracing ring buffer. It does so by walking the underlying linked > list and checking its next and prev links. > > To guarantee that the list doesn't get modified during the check, > a caller typically needs to take cpu_buffer->reader_lock. This prevents > the check from running concurrently with a potential reader which can > make the list temporarily inconsistent when swapping its old reader page > into the buffer. > > A problem is that the time when interrupts are disabled is > non-deterministic, dependent on the ring buffer size. This particularly > affects PREEMPT_RT because the reader_lock is a raw spinlock which > doesn't become sleepable on PREEMPT_RT kernels. > > Modify the check so it still tries to walk the whole list but gives up > the reader_lock between checking individual pages. Sorry, I'm afraid this patch is actually buggy. It considers only the case when rb_check_pages() is called by ring_buffer_resize() and the ring-buffer list is potentially modified by a reader swapping its page into the buffer from rb_get_reader_page(). However, I suspect the opposite can happen, rb_check_pages() called by a completing reader from ring_buffer_read_finish() and rb_remove_pages()/rb_insert_pages() concurrently modifying the ring-buffer pages. This needs to be handled as well. I'm at a conference next week, I'll have a closer look afterwards. -- Petr
Re: [PATCH] vfs: Add AT_EMPTY_PATH_NOCHECK as unchecked AT_EMPTY_PATH
On Sun, 2024-06-23 at 14:04 +0200, Mateusz Guzik wrote: > On Sun, Jun 23, 2024 at 3:22 AM Xi Ruoyao wrote: > > > > On Sun, 2024-06-23 at 03:07 +0200, Mateusz Guzik wrote: > > > On Sun, Jun 23, 2024 at 2:59 AM Xi Ruoyao > > > wrote: > > > > > > > > On Sat, 2024-06-22 at 15:41 -0700, Linus Torvalds wrote: > > > > > > > > > I do think that we should make AT_EMPTY_PATH with a NULL path > > > > > "JustWork(tm)", because the stupid "look if the pathname is > > > > > empty" is > > > > > horrible. > > > > > > > > > > But moving that check into getname() is *NOT* the right > > > > > answer, > > > > > because by the time you get to getname(), you have already > > > > > lost. > > > > > > > > Oops. I'll try to get around of getname() too... > > > > > > > > > So the short-cut in vfs_fstatat() to never get a pathname is > > > > > disgusting - people should have used 'fstat()' - but it's > > > > > _important_ > > > > > disgusting. > > > > > > > > The problem is we don't have fstat() for LoongArch, and it'll be > > > > unusable on all 32-bit arch after 2037. > > > > > > > > And Arnd hates the idea adding fstat() for LoongArch because > > > > there would > > > > be one more 32-bit arch broken in 2037. > > > > > > > > Or should we just add something like "fstat_2037()"? > > > > > > > > > > In that case fstat is out of the question, but no problem. > > > > > > It was suggested to make AT_EMPTY_PATH + NULL pathname do the > > > right > > > thing and have the syscalls short-circuit as needed. > > > > > > for statx it would look like this (except you are going to have > > > implement do_statx_by_fd): > > > > > > diff --git a/fs/stat.c b/fs/stat.c > > > index 16aa1f5ceec4..0afe72b320cc 100644 > > > --- a/fs/stat.c > > > +++ b/fs/stat.c > > > @@ -710,6 +710,9 @@ SYSCALL_DEFINE5(statx, > > > int ret; > > > struct filename *name; > > > > > > + if (flags == AT_EMPTY_PATH && filename == NULL) > > > + return do_statx_by_fd(...); > > > + > > > name = getname_flags(filename, > > > getname_statx_lookup_flags(flags)); > > > ret = do_statx(dfd, name, flags, mask, buffer); > > > putname(name); > > > > > > and so on > > > > > > Personally I would prefer if fstatx was added instead, fwiw most > > > massaging in the area will be the same regardless. > > > > I do agree. But if we do it *now* would it be "breaking the > > userspace" > > if some stupid program relies on fstatx() to return some error when > > the > > path is NULL? The "stupid programs" may just exist in the wild... > > > > You mean statx? fstatx would not accept a path to begin with. Yes I mean statx. > Worry about some code breaking is why I suggested a dedicated flag > (AT_NO_PATH) myself in case fstatx is a no-go. I agree a dedicated flag will be better but I only came up with nasty names like it AT_FORCE_EMPTY_PATH or AT_EMPTY_PATH_NOCHECK. I think your AT_NO_PATH is a better name. > I am not convinced messing with AT_* flags is justified to begin with. > Any syscall which does not have a fd-only variant and is found to be > routinely used with AT_EMPTY_PATH should get one instead. > > As far as I know that's only stat(due to a perf bug in glibc, now > fixed) and increasingly statx. And you mean fstatat instead of stat, I guess. > Suppose AT_EMPTY_PATH + NULL are to land and stat + statx get the > treatment. What about all the other syscalls? Sorting all that out is > quite a big of churn which is probably not worth it. But then there is > a feature gap in that they EFAULT for this pair while the stat* ones > don't and that's bound to raise confusion. Then one could add the > check in the bowels of path lookup in similar way you do did to > maintain the same behavior (but without per-syscall churn) and a big > fat warning that anyone getting there often needs to get patched with > short-circuiting the entire thing. So I think that's either a lot of > churn or nasty additions. > > Regardless, as noted above, either making fstatx a thing or > short-circuiting mostly the same patching has to be done for > statx-related stuff. > > However, this is not my call to make. -- Xi Ruoyao School of Aerospace Science and Technology, Xidian University
Re: [PATCH] vfs: Add AT_EMPTY_PATH_NOCHECK as unchecked AT_EMPTY_PATH
On Sun, Jun 23, 2024 at 3:22 AM Xi Ruoyao wrote: > > On Sun, 2024-06-23 at 03:07 +0200, Mateusz Guzik wrote: > > On Sun, Jun 23, 2024 at 2:59 AM Xi Ruoyao wrote: > > > > > > On Sat, 2024-06-22 at 15:41 -0700, Linus Torvalds wrote: > > > > > > > I do think that we should make AT_EMPTY_PATH with a NULL path > > > > "JustWork(tm)", because the stupid "look if the pathname is empty" is > > > > horrible. > > > > > > > > But moving that check into getname() is *NOT* the right answer, > > > > because by the time you get to getname(), you have already lost. > > > > > > Oops. I'll try to get around of getname() too... > > > > > > > So the short-cut in vfs_fstatat() to never get a pathname is > > > > disgusting - people should have used 'fstat()' - but it's _important_ > > > > disgusting. > > > > > > The problem is we don't have fstat() for LoongArch, and it'll be > > > unusable on all 32-bit arch after 2037. > > > > > > And Arnd hates the idea adding fstat() for LoongArch because there would > > > be one more 32-bit arch broken in 2037. > > > > > > Or should we just add something like "fstat_2037()"? > > > > > > > In that case fstat is out of the question, but no problem. > > > > It was suggested to make AT_EMPTY_PATH + NULL pathname do the right > > thing and have the syscalls short-circuit as needed. > > > > for statx it would look like this (except you are going to have > > implement do_statx_by_fd): > > > > diff --git a/fs/stat.c b/fs/stat.c > > index 16aa1f5ceec4..0afe72b320cc 100644 > > --- a/fs/stat.c > > +++ b/fs/stat.c > > @@ -710,6 +710,9 @@ SYSCALL_DEFINE5(statx, > > int ret; > > struct filename *name; > > > > + if (flags == AT_EMPTY_PATH && filename == NULL) > > + return do_statx_by_fd(...); > > + > > name = getname_flags(filename, getname_statx_lookup_flags(flags)); > > ret = do_statx(dfd, name, flags, mask, buffer); > > putname(name); > > > > and so on > > > > Personally I would prefer if fstatx was added instead, fwiw most > > massaging in the area will be the same regardless. > > I do agree. But if we do it *now* would it be "breaking the userspace" > if some stupid program relies on fstatx() to return some error when the > path is NULL? The "stupid programs" may just exist in the wild... > You mean statx? fstatx would not accept a path to begin with. Worry about some code breaking is why I suggested a dedicated flag (AT_NO_PATH) myself in case fstatx is a no-go. I am not convinced messing with AT_* flags is justified to begin with. Any syscall which does not have a fd-only variant and is found to be routinely used with AT_EMPTY_PATH should get one instead. As far as I know that's only stat(due to a perf bug in glibc, now fixed) and increasingly statx. Suppose AT_EMPTY_PATH + NULL are to land and stat + statx get the treatment. What about all the other syscalls? Sorting all that out is quite a big of churn which is probably not worth it. But then there is a feature gap in that they EFAULT for this pair while the stat* ones don't and that's bound to raise confusion. Then one could add the check in the bowels of path lookup in similar way you do did to maintain the same behavior (but without per-syscall churn) and a big fat warning that anyone getting there often needs to get patched with short-circuiting the entire thing. So I think that's either a lot of churn or nasty additions. Regardless, as noted above, either making fstatx a thing or short-circuiting mostly the same patching has to be done for statx-related stuff. However, this is not my call to make. -- Mateusz Guzik
Re: [PATCH vhost 18/23] vdpa/mlx5: Forward error in suspend/resume device
在 2024/6/17 23:07, Dragos Tatulea 写道:
Start using the suspend/resume_vq() error return codes previously added.
Signed-off-by: Dragos Tatulea
Reviewed-by: Cosmin Ratiu
---
drivers/vdpa/mlx5/net/mlx5_vnet.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index f5d5b25cdb01..0e1c1b7ff297 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -3436,22 +3436,25 @@ static int mlx5_vdpa_suspend(struct vdpa_device *vdev)
{
struct mlx5_vdpa_dev *mvdev = to_mvdev(vdev);
struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev);
+ int err;
Reverse Christmas Tree?
Reviewed-by: Zhu Yanjun
Zhu Yanjun
mlx5_vdpa_info(mvdev, "suspending device\n");
down_write(>reslock);
unregister_link_notifier(ndev);
- suspend_vqs(ndev);
+ err = suspend_vqs(ndev);
mlx5_vdpa_cvq_suspend(mvdev);
mvdev->suspended = true;
up_write(>reslock);
- return 0;
+
+ return err;
}
static int mlx5_vdpa_resume(struct vdpa_device *vdev)
{
struct mlx5_vdpa_dev *mvdev = to_mvdev(vdev);
struct mlx5_vdpa_net *ndev;
+ int err;
ndev = to_mlx5_vdpa_ndev(mvdev);
@@ -3459,10 +3462,11 @@ static int mlx5_vdpa_resume(struct vdpa_device *vdev)
down_write(>reslock);
mvdev->suspended = false;
- resume_vqs(ndev);
+ err = resume_vqs(ndev);
register_link_notifier(ndev);
up_write(>reslock);
- return 0;
+
+ return err;
}
static int mlx5_set_group_asid(struct vdpa_device *vdev, u32 group,
Re: [PATCH 1/3] dt-bindings: arm: qcom: Add msm8916 based LG devices
On 23/06/2024 11:26, Nikita Travkin wrote: > Add compatible values for the msm8916 based LG smartphones. > > Signed-off-by: Nikita Travkin > --- > Documentation/devicetree/bindings/arm/qcom.yaml | 2 + Acked-by: Krzysztof Kozlowski Best regards, Krzysztof
[PATCH 0/3] Introduce msm8916 based LG devices
This series introduces two msm8916-based LG devices: - LG Leon LTE (c50) - LG LG K10 (m216) The devices only have basic support for now. Signed-off-by: Nikita Travkin --- Anton Bambura (1): arm64: dts: qcom: msm8916-lg-c50: add initial dts for LG Leon LTE Cristian Cozzolino (1): arm64: dts: qcom: msm8916-lg-m216: Add initial device tree Nikita Travkin (1): dt-bindings: arm: qcom: Add msm8916 based LG devices Documentation/devicetree/bindings/arm/qcom.yaml | 2 + arch/arm64/boot/dts/qcom/Makefile | 2 + arch/arm64/boot/dts/qcom/msm8916-lg-c50.dts | 140 + arch/arm64/boot/dts/qcom/msm8916-lg-m216.dts| 251 4 files changed, 395 insertions(+) --- base-commit: f76698bd9a8ca01d3581236082d786e9a6b72bb7 change-id: 20240621-msm8916-lg-initial-8d4a399ec3c2 Best regards, -- Nikita Travkin
[PATCH 1/3] dt-bindings: arm: qcom: Add msm8916 based LG devices
Add compatible values for the msm8916 based LG smartphones. Signed-off-by: Nikita Travkin --- Documentation/devicetree/bindings/arm/qcom.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml index d839691a900c..09edce29ff3a 100644 --- a/Documentation/devicetree/bindings/arm/qcom.yaml +++ b/Documentation/devicetree/bindings/arm/qcom.yaml @@ -208,6 +208,8 @@ properties: - asus,z00l - gplus,fl8005a - huawei,g7 + - lg,c50 + - lg,m216 - longcheer,l8910 - longcheer,l8150 - motorola,harpia -- 2.45.2
[PATCH 2/3] arm64: dts: qcom: msm8916-lg-m216: Add initial device tree
From: Cristian Cozzolino
This commit adds initial support for the LG K10 smartphone.
Support for the following features is included:
- Serial
- Keys
- Battery and charger
- Accelerometer, magnetometer
- Touchscreen
- Sound and modem
- Haptic
Signed-off-by: Cristian Cozzolino
[Nikita: Minor cleanup]
Signed-off-by: Nikita Travkin
---
arch/arm64/boot/dts/qcom/Makefile| 1 +
arch/arm64/boot/dts/qcom/msm8916-lg-m216.dts | 251 +++
2 files changed, 252 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/Makefile
b/arch/arm64/boot/dts/qcom/Makefile
index 5576c7d6ea06..b35e46d75a1d 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -31,6 +31,7 @@ dtb-$(CONFIG_ARCH_QCOM) += msm8916-alcatel-idol347.dtb
dtb-$(CONFIG_ARCH_QCOM)+= msm8916-asus-z00l.dtb
dtb-$(CONFIG_ARCH_QCOM)+= msm8916-gplus-fl8005a.dtb
dtb-$(CONFIG_ARCH_QCOM)+= msm8916-huawei-g7.dtb
+dtb-$(CONFIG_ARCH_QCOM)+= msm8916-lg-m216.dtb
dtb-$(CONFIG_ARCH_QCOM)+= msm8916-longcheer-l8150.dtb
dtb-$(CONFIG_ARCH_QCOM)+= msm8916-longcheer-l8910.dtb
dtb-$(CONFIG_ARCH_QCOM)+= msm8916-motorola-harpia.dtb
diff --git a/arch/arm64/boot/dts/qcom/msm8916-lg-m216.dts
b/arch/arm64/boot/dts/qcom/msm8916-lg-m216.dts
new file mode 100644
index ..07345e694f6f
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/msm8916-lg-m216.dts
@@ -0,0 +1,251 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+/dts-v1/;
+
+#include "msm8916-pm8916.dtsi"
+#include "msm8916-modem-qdsp6.dtsi"
+
+#include
+
+/ {
+ model = "LG K10 (K420n)";
+ compatible = "lg,m216", "qcom,msm8916";
+ chassis-type = "handset";
+
+ aliases {
+ mmc0 = _1; /* eMMC */
+ mmc1 = _2; /* SD card */
+ serial0 = _uart2;
+ };
+
+ chosen {
+ stdout-path = "serial0";
+ };
+
+ battery: battery {
+ compatible = "simple-battery";
+ voltage-min-design-microvolt = <330>;
+ voltage-max-design-microvolt = <435>;
+ energy-full-design-microwatt-hours = <880>;
+ charge-full-design-microamp-hours = <230>;
+
+ ocv-capacity-celsius = <25>;
+ ocv-capacity-table-0 = <4342000 100>, <4266000 95>, <4206000
90>,
+ <4148000 85>, <4094000 80>, <4046000 75>, <3994000 70>,
+ <3956000 65>, <3916000 60>, <3866000 55>, <3831000 50>,
+ <3808000 45>, <3789000 40>, <3776000 35>, <3769000 30>,
+ <376 25>, <374 20>, <3712000 16>, <3684000 13>,
+ <3676000 11>, <3674000 10>, <3672000 9>, <3669000 8>,
+ <3665000 7>, <366 6>, <3643000 5>, <3602000 4>,
+ <3542000 3>, <3458000 2>, <3326000 1>, <300 0>;
+ };
+
+ gpio-keys {
+ compatible = "gpio-keys";
+
+ pinctrl-0 = <_keys_default>;
+ pinctrl-names = "default";
+
+ label = "GPIO Buttons";
+
+ volume-up-button {
+ label = "Volume Up";
+ gpios = < 107 GPIO_ACTIVE_LOW>;
+ linux,code = ;
+ };
+
+ volume-down-button {
+ label = "Volume Down";
+ gpios = < 108 GPIO_ACTIVE_LOW>;
+ linux,code = ;
+ };
+ };
+};
+
+_i2c2 {
+ status = "okay";
+
+ accelerometer@11 {
+ compatible = "bosch,bmc150_accel";
+ reg = <0x11>;
+
+ interrupts-extended = < 115 IRQ_TYPE_EDGE_RISING>;
+
+ mount-matrix = "0", "1", "0",
+ "-1", "0", "0",
+"0", "0", "1";
+
+ vdd-supply = <_l17>;
+ vddio-supply = <_l6>;
+
+ pinctrl-0 = <_int_default>;
+ pinctrl-names = "default";
+ };
+
+ magnetometer@13 {
+ compatible = "bosch,bmc150_magn";
+ reg = <0x13>;
+
+ interrupts-extended = < 69 IRQ_TYPE_EDGE_RISING>;
+
+ vdd-supply = <_l17>;
+ vddio-supply = <_l6>;
+
+ pinctrl-0 = <_int_default>;
+ pinctrl-names = "default";
+ };
+};
+
+_i2c5 {
+ status = "okay";
+
+ touchscreen@34 {
+ compatible = "melfas,mip4_ts";
+ reg = <0x34>;
+
+ interrupts-extended = < 13 IRQ_TYPE_EDGE_FALLING>;
+ ce-gpios = < 12 GPIO_ACTIVE_HIGH>;
+
+ pinctrl-0 = <_default>;
+ pinctrl-names = "default";
+ };
+};
+
+_uart2 {
+ status = "okay";
+};
+
+_mem {
+ reg = <0x0 0x8680 0x0 0x4a0>;
+};
+
+_bms {
+ monitored-battery = <>;
+ power-supplies = <_charger>;
+
+
[PATCH 3/3] arm64: dts: qcom: msm8916-lg-c50: add initial dts for LG Leon LTE
From: Anton Bambura
Add initial device-tree for LG Leon LTE (lg-c50), currently supported
features:
- eMMC;
- MicroSD;
- usb in peripheral mode;
- WiFi/BT;
- vibration;
- keys.
Signed-off-by: Anton Bambura
Signed-off-by: Nikita Travkin
---
arch/arm64/boot/dts/qcom/Makefile | 1 +
arch/arm64/boot/dts/qcom/msm8916-lg-c50.dts | 140
2 files changed, 141 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/Makefile
b/arch/arm64/boot/dts/qcom/Makefile
index b35e46d75a1d..919dfcb26d15 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -31,6 +31,7 @@ dtb-$(CONFIG_ARCH_QCOM) += msm8916-alcatel-idol347.dtb
dtb-$(CONFIG_ARCH_QCOM)+= msm8916-asus-z00l.dtb
dtb-$(CONFIG_ARCH_QCOM)+= msm8916-gplus-fl8005a.dtb
dtb-$(CONFIG_ARCH_QCOM)+= msm8916-huawei-g7.dtb
+dtb-$(CONFIG_ARCH_QCOM)+= msm8916-lg-c50.dtb
dtb-$(CONFIG_ARCH_QCOM)+= msm8916-lg-m216.dtb
dtb-$(CONFIG_ARCH_QCOM)+= msm8916-longcheer-l8150.dtb
dtb-$(CONFIG_ARCH_QCOM)+= msm8916-longcheer-l8910.dtb
diff --git a/arch/arm64/boot/dts/qcom/msm8916-lg-c50.dts
b/arch/arm64/boot/dts/qcom/msm8916-lg-c50.dts
new file mode 100644
index ..a823a1c40208
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/msm8916-lg-c50.dts
@@ -0,0 +1,140 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+/dts-v1/;
+
+#include "msm8916-pm8916.dtsi"
+
+#include
+
+/ {
+ model = "LG Leon LTE";
+ compatible = "lg,c50", "qcom,msm8916";
+ chassis-type = "handset";
+
+ aliases {
+ mmc0 = _1; /* eMMC */
+ mmc1 = _2; /* SD card */
+ serial0 = _uart2;
+ };
+
+ chosen {
+ stdout-path = "serial0";
+ };
+
+ gpio-keys {
+ compatible = "gpio-keys";
+
+ pinctrl-0 = <_keys_default>;
+ pinctrl-names = "default";
+
+ label = "GPIO Buttons";
+
+ volume-up-button {
+ label = "Volume Up";
+ gpios = < 108 GPIO_ACTIVE_LOW>;
+ linux,code = ;
+ };
+
+ volume-down-button {
+ label = "Volume Down";
+ gpios = < 107 GPIO_ACTIVE_LOW>;
+ linux,code = ;
+ };
+ };
+
+ reg_sd_vmmc: regulator-sdcard-vmmc {
+ compatible = "regulator-fixed";
+ regulator-name = "sdcard-vmmc";
+ regulator-min-microvolt = <295>;
+ regulator-max-microvolt = <295>;
+
+ gpio = < 60 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+
+ startup-delay-us = <5000>;
+
+ pinctrl-0 = <_vmmc_en_default>;
+ pinctrl-names = "default";
+ };
+};
+
+_uart2 {
+ status = "okay";
+};
+
+_usbin {
+ status = "okay";
+};
+
+_vib {
+ status = "okay";
+};
+
+_1 {
+ status = "okay";
+};
+
+_2 {
+ vmmc-supply = <_sd_vmmc>;
+
+ pinctrl-0 = <_default _cd_default>;
+ pinctrl-1 = <_sleep _cd_default>;
+ pinctrl-names = "default", "sleep";
+
+ cd-gpios = < 38 GPIO_ACTIVE_HIGH>;
+
+ status = "okay";
+};
+
+ {
+ dr_mode = "peripheral";
+ extcon = <_usbin>;
+ status = "okay";
+};
+
+_hs_phy {
+ extcon = <_usbin>;
+};
+
+ {
+ status = "okay";
+};
+
+_mem {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+_iris {
+ compatible = "qcom,wcn3620";
+};
+
+_mem {
+ status = "okay";
+};
+
+ {
+ gpio_keys_default: gpio-keys-default-state {
+ pins = "gpio107", "gpio108";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-pull-up;
+ };
+
+ sd_vmmc_en_default: sd-vmmc-en-default-state {
+ pins = "gpio60";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
+ sdc2_cd_default: sdc2-cd-default-state {
+ pins = "gpio38";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-pull-down;
+ };
+};
--
2.45.2
Re: [PATCH] vfs: Add AT_EMPTY_PATH_NOCHECK as unchecked AT_EMPTY_PATH
On Sun, 2024-06-23 at 03:07 +0200, Mateusz Guzik wrote: > On Sun, Jun 23, 2024 at 2:59 AM Xi Ruoyao wrote: > > > > On Sat, 2024-06-22 at 15:41 -0700, Linus Torvalds wrote: > > > > > I do think that we should make AT_EMPTY_PATH with a NULL path > > > "JustWork(tm)", because the stupid "look if the pathname is empty" is > > > horrible. > > > > > > But moving that check into getname() is *NOT* the right answer, > > > because by the time you get to getname(), you have already lost. > > > > Oops. I'll try to get around of getname() too... > > > > > So the short-cut in vfs_fstatat() to never get a pathname is > > > disgusting - people should have used 'fstat()' - but it's _important_ > > > disgusting. > > > > The problem is we don't have fstat() for LoongArch, and it'll be > > unusable on all 32-bit arch after 2037. > > > > And Arnd hates the idea adding fstat() for LoongArch because there would > > be one more 32-bit arch broken in 2037. > > > > Or should we just add something like "fstat_2037()"? > > > > In that case fstat is out of the question, but no problem. > > It was suggested to make AT_EMPTY_PATH + NULL pathname do the right > thing and have the syscalls short-circuit as needed. > > for statx it would look like this (except you are going to have > implement do_statx_by_fd): > > diff --git a/fs/stat.c b/fs/stat.c > index 16aa1f5ceec4..0afe72b320cc 100644 > --- a/fs/stat.c > +++ b/fs/stat.c > @@ -710,6 +710,9 @@ SYSCALL_DEFINE5(statx, > int ret; > struct filename *name; > > + if (flags == AT_EMPTY_PATH && filename == NULL) > + return do_statx_by_fd(...); > + > name = getname_flags(filename, getname_statx_lookup_flags(flags)); > ret = do_statx(dfd, name, flags, mask, buffer); > putname(name); > > and so on > > Personally I would prefer if fstatx was added instead, fwiw most > massaging in the area will be the same regardless. I do agree. But if we do it *now* would it be "breaking the userspace" if some stupid program relies on fstatx() to return some error when the path is NULL? The "stupid programs" may just exist in the wild... I remember recently we have to pretend pidfd is stupid to please some stupid programs... https://lore.kernel.org/all/20240521-girlanden-zehnfach-1bff7eb9218c@brauner/ -- Xi Ruoyao School of Aerospace Science and Technology, Xidian University
Re: [PATCH] vfs: Add AT_EMPTY_PATH_NOCHECK as unchecked AT_EMPTY_PATH
On Sun, Jun 23, 2024 at 2:59 AM Xi Ruoyao wrote: > > On Sat, 2024-06-22 at 15:41 -0700, Linus Torvalds wrote: > > > I do think that we should make AT_EMPTY_PATH with a NULL path > > "JustWork(tm)", because the stupid "look if the pathname is empty" is > > horrible. > > > > But moving that check into getname() is *NOT* the right answer, > > because by the time you get to getname(), you have already lost. > > Oops. I'll try to get around of getname() too... > > > So the short-cut in vfs_fstatat() to never get a pathname is > > disgusting - people should have used 'fstat()' - but it's _important_ > > disgusting. > > The problem is we don't have fstat() for LoongArch, and it'll be > unusable on all 32-bit arch after 2037. > > And Arnd hates the idea adding fstat() for LoongArch because there would > be one more 32-bit arch broken in 2037. > > Or should we just add something like "fstat_2037()"? > In that case fstat is out of the question, but no problem. It was suggested to make AT_EMPTY_PATH + NULL pathname do the right thing and have the syscalls short-circuit as needed. for statx it would look like this (except you are going to have implement do_statx_by_fd): diff --git a/fs/stat.c b/fs/stat.c index 16aa1f5ceec4..0afe72b320cc 100644 --- a/fs/stat.c +++ b/fs/stat.c @@ -710,6 +710,9 @@ SYSCALL_DEFINE5(statx, int ret; struct filename *name; + if (flags == AT_EMPTY_PATH && filename == NULL) + return do_statx_by_fd(...); + name = getname_flags(filename, getname_statx_lookup_flags(flags)); ret = do_statx(dfd, name, flags, mask, buffer); putname(name); and so on Personally I would prefer if fstatx was added instead, fwiw most massaging in the area will be the same regardless. -- Mateusz Guzik
Re: [PATCH] vfs: Add AT_EMPTY_PATH_NOCHECK as unchecked AT_EMPTY_PATH
On Sat, 2024-06-22 at 15:41 -0700, Linus Torvalds wrote: > I do think that we should make AT_EMPTY_PATH with a NULL path > "JustWork(tm)", because the stupid "look if the pathname is empty" is > horrible. > > But moving that check into getname() is *NOT* the right answer, > because by the time you get to getname(), you have already lost. Oops. I'll try to get around of getname() too... > So the short-cut in vfs_fstatat() to never get a pathname is > disgusting - people should have used 'fstat()' - but it's _important_ > disgusting. The problem is we don't have fstat() for LoongArch, and it'll be unusable on all 32-bit arch after 2037. And Arnd hates the idea adding fstat() for LoongArch because there would be one more 32-bit arch broken in 2037. Or should we just add something like "fstat_2037()"? -- Xi Ruoyao School of Aerospace Science and Technology, Xidian University
Re: [PATCH] vfs: Add AT_EMPTY_PATH_NOCHECK as unchecked AT_EMPTY_PATH
On Sat, 22 Jun 2024 at 14:25, Mateusz Guzik wrote: > > +cc Linus Thanks. > To sum up the problem: stat and statx met with "" + AT_EMPTY_PATH have > more work to do than fstat and its hypotethical statx counterpart: > - buf alloc/free for the path > - userspace access (very painful on x86_64 + SMAP) > - lockref acquire/release Yes. That LOOKUP_EMPTY_NOCHECK is *not* the fix. I do think that we should make AT_EMPTY_PATH with a NULL path "JustWork(tm)", because the stupid "look if the pathname is empty" is horrible. But moving that check into getname() is *NOT* the right answer, because by the time you get to getname(), you have already lost. There's a very real reason why vfs_fstatat() catches this empty case early, and never goes to filename lookup at all. You don't want to generate a 'struct path' from the 'int fd', because you want to never get anywhere close to that path, and instead only ever need a 'struct fd' that can be looked up much more cheaply (particularly if not in a threaded environment). So the short-cut in vfs_fstatat() to never get a pathname is disgusting - people should have used 'fstat()' - but it's _important_ disgusting. This thing that tries to short-circuit things at the path level is too late. Linus
Re: [PATCH] vfs: Add AT_EMPTY_PATH_NOCHECK as unchecked AT_EMPTY_PATH
+cc Linus
On Sat, Jun 22, 2024 at 06:56:08PM +0800, Xi Ruoyao wrote:
> It's cheap to check if the path is empty in the userspace, but expensive
> to check if a userspace string is empty from the kernel. So using statx
> and AT_EMPTY_PATH to implement fstat is slower than a "native" fstat
> call. But for arch/loongarch fstat does not exist so we have to use
> statx, and on all 32-bit architectures we must use statx after 2037.
> And seccomp also cannot audit AT_EMPTY_PATH properly because it cannot
> check if path is empty.
>
> To resolve these issues, add a relaxed version of AT_EMPTY_PATH: it does
> not check if the path is empty, but just assumes the path is empty and
> then behaves like AT_EMPTY_PATH.
>
> Link: https://sourceware.org/pipermail/libc-alpha/2023-September/151364.html
> Link:
> https://lore.kernel.org/loongarch/599df4a3-47a4-49be-9c81-8e21ea1f9...@xen0n.name/
imo the thing to do is to add fstat for your arch and add fstatx for
everyone.
My argument for fstatx specifically is that Rust uses statx instead of
fstat and is growing in popularity.
To sum up the problem: stat and statx met with "" + AT_EMPTY_PATH have
more work to do than fstat and its hypotethical statx counterpart:
- buf alloc/free for the path
- userspace access (very painful on x86_64 + SMAP)
- lockref acquire/release
(and other things concerning lookup itself which I'm going to ignore
here)
Your patch avoids the peek at userspace, but the other overhead remains.
In particular the lockref cycle, apart from adding work single-threaded,
adds avoidable serialization against other threads issuing stat(x) on
the same file. iow your patch still leaves performance on the table and
I don't think it is necessary.
If the flag is the way to go (I don't see why though), I would suggest
something like AT_NO_PATH and requring NULL as the path argument (or
some other predefined "there is nothing here" constant).
I wanted to type up a proposal for fstatx (+ patch) some time ago, but
some refactoring was needed to make it happen and put it on the back
burner.
Perhaps you would be willing to pick it up, assuming the vfs folk are
oke with it.
Regardless of what happens with statx or this patch you should probably
add fstat anyway.
If there are any other perf-sensitive syscalls which don't have their
fd-only variants they should be plugged to, but I can't think of
anything.
> ---
> fs/namei.c | 8 +++-
> fs/stat.c | 4 +++-
> include/linux/namei.h | 4
> include/trace/misc/fs.h| 1 +
> include/uapi/linux/fcntl.h | 3 +++
> 5 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/fs/namei.c b/fs/namei.c
> index 37fb0a8aa09a..0c44a7ea5961 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -147,7 +147,13 @@ getname_flags(const char __user *filename, int flags,
> int *empty)
> kname = (char *)result->iname;
> result->name = kname;
>
> - len = strncpy_from_user(kname, filename, EMBEDDED_NAME_MAX);
> + if (!(flags & LOOKUP_EMPTY_NOCHECK))
> + len = strncpy_from_user(kname, filename, EMBEDDED_NAME_MAX);
> + else {
> + len = 0;
> + kname[0] = '\0';
> + }
> +
> if (unlikely(len < 0)) {
> __putname(result);
> return ERR_PTR(len);
> diff --git a/fs/stat.c b/fs/stat.c
> index 70bd3e888cfa..53944d3287cd 100644
> --- a/fs/stat.c
> +++ b/fs/stat.c
> @@ -210,6 +210,8 @@ int getname_statx_lookup_flags(int flags)
> lookup_flags |= LOOKUP_AUTOMOUNT;
> if (flags & AT_EMPTY_PATH)
> lookup_flags |= LOOKUP_EMPTY;
> + if (flags & AT_EMPTY_PATH_NOCHECK)
> + lookup_flags |= LOOKUP_EMPTY | LOOKUP_EMPTY_NOCHECK;
>
> return lookup_flags;
> }
> @@ -237,7 +239,7 @@ static int vfs_statx(int dfd, struct filename *filename,
> int flags,
> int error;
>
> if (flags & ~(AT_SYMLINK_NOFOLLOW | AT_NO_AUTOMOUNT | AT_EMPTY_PATH |
> - AT_STATX_SYNC_TYPE))
> + AT_STATX_SYNC_TYPE | AT_EMPTY_PATH_NOCHECK))
> return -EINVAL;
>
> retry:
> diff --git a/include/linux/namei.h b/include/linux/namei.h
> index 967aa9ea9f96..def6a8a1b531 100644
> --- a/include/linux/namei.h
> +++ b/include/linux/namei.h
> @@ -45,9 +45,13 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT};
> #define LOOKUP_IN_ROOT 0x10 /* Treat dirfd as fs root. */
> #define LOOKUP_CACHED0x20 /* Only do cached lookup */
> #define LOOKUP_LINKAT_EMPTY 0x40 /* Linkat request with empty path. */
> +
> /* LOOKUP_* flags which do scope-related checks based on the dirfd. */
> #define LOOKUP_IS_SCOPED (LOOKUP_BENEATH | LOOKUP_IN_ROOT)
>
> +/* If this is set, LOOKUP_EMPTY must be set as well. */
> +#define LOOKUP_EMPTY_NOCHECK 0x80 /* Consider path empty. */
> +
> extern int path_pts(struct path *path);
>
> extern int user_path_at_empty(int, const char __user *, unsigned, struct
>
Re: [PATCH v2 1/3] dt-bindings: arm: qcom: Add Sony Xperia Z3 Compact
On Fri, Jun 21, 2024 at 05:26:42PM +0300, Valeriy Klimin wrote: > Add the compatible for this device. > > Signed-off-by: Valeriy Klimin Acked-by: Conor Dooley signature.asc Description: PGP signature
Re: [PATCH v3] tracing/net_sched: NULL pointer dereference in perf_trace_qdisc_reset()
Hi, On 6/22/24 3:12 오후, Yunseong Kim wrote: > Hi Taehee, > > On 6/22/24 2:50 오후, Taehee Yoo wrote: >> On Sat, Jun 22, 2024 at 1:58 PM wrote: >>> >>> From: Yunseong Kim >>> >> >> Hi Yunseong, >> Thanks a lot for this work! > > Thank you Taehee for reviewing our patch. It's greatly appreciated. > >>> During qdisc initialization, qdisc was being set to noop_queue. >>> In veth_init_queue, the initial tx_num was reduced back to one, >>> causing the qdisc reset to be called with noop, which led to the kernel >>> panic. >>> >>> I've attached the GitHub gist link that C converted syz-execprogram >>> source code and 3 log of reproduced vmcore-dmesg. >>> >>> https://gist.github.com/yskelg/cc64562873ce249cdd0d5a358b77d740 >>> >>> Yeoreum and I use two fuzzing tool simultaneously. >>> >>> One process with syz-executor : https://github.com/google/syzkaller >>> >>> $ ./syz-execprog -executor=./syz-executor -repeat=1 -sandbox=setuid \ >>> -enable=none -collide=false log1 >>> >>> The other process with perf fuzzer: >>> https://github.com/deater/perf_event_tests/tree/master/fuzzer >>> >>> $ perf_event_tests/fuzzer/perf_fuzzer >>> >>> I think this will happen on the kernel version. >>> >>> Linux kernel version +v6.7.10, +v6.8, +v6.9 and it could happen in v6.10. >>> >>> This occurred from 51270d573a8d. I think this patch is absolutely >>> necessary. Previously, It was showing not intended string value of name. > > >> I found a simple reproducer, please use the below command to test this patch. >> >> echo 1 > /sys/kernel/debug/tracing/events/enable >> ip link add veth0 type veth peer name veth1 After applying our patch, I didn't find any message or kernel panic errors. # echo 1 > /sys/kernel/debug/tracing/events/qdisc/qdisc_reset/enable # ip link add veth0 type veth peer name veth1 Error: Unknown device type. However, without our patch applied, I tested Tahee's command line on the upstream 6.10.0-rc3 kernel using the qdisc_reset event and the ip command. $ echo 1 > /sys/kernel/debug/tracing/events/qdisc/qdisc_reset/enable $ ip link add veth0 type veth peer name veth1 This make always kernel panic. Linux version: 6.10.0-rc3 [0.00] Linux version 6.10.0-rc3-00164-g44ef20baed8e-dirty (paran@fedora) (gcc (GCC) 14.1.1 20240522 (Red Hat 14.1.1-4), GNU ld version 2.41-34.fc40) #20 SMP PREEMPT Sat Jun 15 16:51:25 KST 2024 Kernel panic message: [ 615.236484] Internal error: Oops: 9605 [#1] PREEMPT SMP [ 615.237250] Dumping ftrace buffer: [ 615.237679](ftrace buffer empty) [ 615.238097] Modules linked in: veth crct10dif_ce virtio_gpu virtio_dma_buf drm_shmem_helper drm_kms_helper zynqmp_fpga xilinx_can xilinx_spi xilinx_selectmap xilinx_core xilinx_pr_decoupler versal_fpga uvcvideo uvc videobuf2_vmalloc videobuf2_memops videobuf2_v4l2 videodev videobuf2_common mc usbnet deflate zstd ubifs ubi rcar_canfd rcar_can omap_mailbox ntb_msi_test ntb_hw_epf lattice_sysconfig_spi lattice_sysconfig ice40_spi gpio_xilinx dwmac_altr_socfpga mdio_regmap stmmac_platform stmmac pcs_xpcs dfl_fme_region dfl_fme_mgr dfl_fme_br dfl_afu dfl fpga_region fpga_bridge can can_dev br_netfilter bridge stp llc atl1c ath11k_pci mhi ath11k_ahb ath11k qmi_helpers ath10k_sdio ath10k_pci ath10k_core ath mac80211 libarc4 cfg80211 drm fuse backlight ipv6 Jun 22 02:36:5[3 6k152.62-4sm98k4-0k]v kCePUr:n e1l :P IUDn:a b4le6 8t oC ohmma: nidpl eN oketr nteali nptaedg i6n.g1 0re.0q-urecs3t- 0at0 1v6i4r-tgu4a4le fa2d0dbraeeds0se-dir tyd f#f2f08 615.252376] Hardware name: linux,dummy-virt (DT) [ 615.253220] pstate: 8045 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 615.254433] pc : strnlen+0x6c/0xe0 [ 615.255096] lr : trace_event_get_offsets_qdisc_reset+0x94/0x3d0 [ 615.256088] sp : 800080b269a0 [ 615.256615] x29: 800080b269a0 x28: c070f3f98500 x27: 0001 [ 615.257831] x26: 0010 x25: c070f3f98540 x24: c070f619cf60 [ 615.259020] x23: 0128 x22: 0138 x21: dfff8000 [ 615.260241] x20: c070f631ad00 x19: 0128 x18: c070f448b800 [ 615.261454] x17: x16: 0001 x15: c070f4ba2a90 [ 615.262635] x14: 700010164d73 x13: 180e1e8d5eb3 x12: 100010164d72 [ 615.263877] x11: 700010164d72 x10: dfff8000 x9 : c070e85d6184 [ 615.265047] x8 : c070e4402070 x7 : f1f1 x6 : 1504a6d3 [ 615.266336] x5 : 28ca21122140 x4 : c070f5043ea8 x3 : [ 615.267528] x2 : 0025 x1 : x0 : [ 615.268747] Call trace: [ 615.269180] strnlen+0x6c/0xe0 [ 615.269767] trace_event_get_offsets_qdisc_reset+0x94/0x3d0 [ 615.270716] trace_event_raw_event_qdisc_reset+0xe8/0x4e8 [ 615.271667] __traceiter_qdisc_reset+0xa0/0x140 [ 615.272499] qdisc_reset+0x554/0x848 [ 615.273134] netif_set_real_num_tx_queues+0x360/0x9a8 [ 615.274050] veth_init_queues+0x110/0x220 [veth] [ 615.275110]
Re: [PATCH 7/7] ARM: dts: qcom: msm8226: Convert APCS usages to mbox interface
On 19.06.2024 11:02 PM, Luca Weiss wrote: > Since we now have the apcs set up as a mailbox provider, let's use the > interface for all drivers where possible. > > Signed-off-by: Luca Weiss > --- Reviewed-by: Konrad Dybcio Konrad
Re: [PATCH 6/7] ARM: dts: qcom: msm8226: Hook up CPU cooling
On 19.06.2024 11:02 PM, Luca Weiss wrote: > Add cooling-maps for the CPU thermal zones so the driver can actually do > something when the CPU temperature rises too much. > > Signed-off-by: Luca Weiss > --- Very cool, thanks Reviewed-by: Konrad Dybcio Konrad
Re: [PATCH 5/7] ARM: dts: qcom: msm8226: Add CPU frequency scaling support
On 19.06.2024 11:02 PM, Luca Weiss wrote:
> Add a node for the a7pll with its frequencies. With this we can use the
> apcs-kpss-global driver for the apcs node and use the apcs to scale the
> CPU frequency according to the opp-table.
>
> At the same time unfortunately we need to provide the gcc node xo_board
> instead of the XO via rpmcc since otherwise we'll have a circular
> dependency between apcs, gcc and the rpm.
Hm.. thinking of a solution to that, should we maybe split the mux/clk
part of APCS into a subnode and bind the clk device to that?
Dmitry, Bjorn, thoughts?
[...]
> +
> + opp-6 {
Can't find this one in the random msm-3.10 I have
> + opp-hz = /bits/ 64 <6>;
> + };
> +
> + opp-78720 {
> + opp-hz = /bits/ 64 <78720>;
> + };
> +
> + /* Higher CPU frequencies need speedbin support */
1190400 kHz seems to also be a supported-across-the-board one.. unless the
watch edition shuffled things around with a newer tree
> + };
> +
> pmu {
> compatible = "arm,cortex-a7-pmu";
> interrupts = @@ -231,9 +262,75 @@ intc: interrupt-controller@f900 {
> #interrupt-cells = <3>;
> };
>
> - apcs: syscon@f9011000 {
> - compatible = "syscon";
> + apcs: mailbox@f9011000 {
> + compatible = "qcom,msm8226-apcs-kpss-global",
> + "qcom,msm8916-apcs-kpss-global", "syscon";
> reg = <0xf9011000 0x1000>;
> + #mbox-cells = <1>;
> + clocks = <>, < GPLL0_VOTE>;
> + clock-names = "pll", "aux";
> + #clock-cells = <0>;
> + };
> +
> + a7pll: clock@f9016000 {
> + compatible = "qcom,msm8226-a7pll";
> + reg = <0xf9016000 0x40>;
> + #clock-cells = <0>;
> + clocks = <_board>;
> + clock-names = "xo";
> + operating-points-v2 = <_opp_table>;
> +
> + a7pll_opp_table: opp-table {
> + compatible = "operating-points-v2";
> +
> + opp-76800 {
> + opp-hz = /bits/ 64 <76800>;
> + };
Looks like scaling this PLL should also scale some voltage domains:
CPR (fed by pm8226_s2) and MX
Perhaps hook up MX to this one for now and add CPR to the CPU nodes( & OPP
table)
after that is brought up
Konrad
Re: [PATCH v2 3/3] ARM: dts: qcom: msm8974-sony-shinano: increase load on l21 for sdhc2
On 21.06.2024 4:26 PM, Valeriy Klimin wrote:
> SD cards would exhibit errors similar to ones described in commit
> 27fe0fc05f35 ("ARM: dts: msm8974-FP2: Increase load on l20 for sdhci")
>
> This patch applies the same change to the regulator for sdhc2.
>
> Signed-off-by: Valeriy Klimin
> ---
Reviewed-by: Konrad Dybcio
Konrad
Re: [PATCH v2 2/3] ARM: dts: qcom: Add Sony Xperia Z3 Compact smartphone
On 21.06.2024 4:26 PM, Valeriy Klimin wrote: > Add the dts for the Z3 Compact. This is currently almost the same > as the plain Z3 as they share almost the same hardware and > nothing device-specific is currently supported. > > Signed-off-by: Valeriy Klimin > --- Reviewed-by: Konrad Dybcio Konrad
[PATCH] vfs: Add AT_EMPTY_PATH_NOCHECK as unchecked AT_EMPTY_PATH
It's cheap to check if the path is empty in the userspace, but expensive
to check if a userspace string is empty from the kernel. So using statx
and AT_EMPTY_PATH to implement fstat is slower than a "native" fstat
call. But for arch/loongarch fstat does not exist so we have to use
statx, and on all 32-bit architectures we must use statx after 2037.
And seccomp also cannot audit AT_EMPTY_PATH properly because it cannot
check if path is empty.
To resolve these issues, add a relaxed version of AT_EMPTY_PATH: it does
not check if the path is empty, but just assumes the path is empty and
then behaves like AT_EMPTY_PATH.
Link: https://sourceware.org/pipermail/libc-alpha/2023-September/151364.html
Link:
https://lore.kernel.org/loongarch/599df4a3-47a4-49be-9c81-8e21ea1f9...@xen0n.name/
Cc: Christian Brauner
Cc: Alexander Viro
Cc: Jan Kara
Cc: Steven Rostedt
Cc: Masami Hiramatsu
Cc: Alejandro Colomar
Cc: Arnd Bergmann
Cc: Huacai Chen
Cc: Xuerui Wang
Cc: Jiaxun Yang
Cc: Icenowy Zheng
Cc: linux-fsde...@vger.kernel.org
Cc: linux-trace-ker...@vger.kernel.org
Cc: linux-a...@vger.kernel.org
Cc: loonga...@lists.linux.dev
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Xi Ruoyao
---
fs/namei.c | 8 +++-
fs/stat.c | 4 +++-
include/linux/namei.h | 4
include/trace/misc/fs.h| 1 +
include/uapi/linux/fcntl.h | 3 +++
5 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index 37fb0a8aa09a..0c44a7ea5961 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -147,7 +147,13 @@ getname_flags(const char __user *filename, int flags, int
*empty)
kname = (char *)result->iname;
result->name = kname;
- len = strncpy_from_user(kname, filename, EMBEDDED_NAME_MAX);
+ if (!(flags & LOOKUP_EMPTY_NOCHECK))
+ len = strncpy_from_user(kname, filename, EMBEDDED_NAME_MAX);
+ else {
+ len = 0;
+ kname[0] = '\0';
+ }
+
if (unlikely(len < 0)) {
__putname(result);
return ERR_PTR(len);
diff --git a/fs/stat.c b/fs/stat.c
index 70bd3e888cfa..53944d3287cd 100644
--- a/fs/stat.c
+++ b/fs/stat.c
@@ -210,6 +210,8 @@ int getname_statx_lookup_flags(int flags)
lookup_flags |= LOOKUP_AUTOMOUNT;
if (flags & AT_EMPTY_PATH)
lookup_flags |= LOOKUP_EMPTY;
+ if (flags & AT_EMPTY_PATH_NOCHECK)
+ lookup_flags |= LOOKUP_EMPTY | LOOKUP_EMPTY_NOCHECK;
return lookup_flags;
}
@@ -237,7 +239,7 @@ static int vfs_statx(int dfd, struct filename *filename,
int flags,
int error;
if (flags & ~(AT_SYMLINK_NOFOLLOW | AT_NO_AUTOMOUNT | AT_EMPTY_PATH |
- AT_STATX_SYNC_TYPE))
+ AT_STATX_SYNC_TYPE | AT_EMPTY_PATH_NOCHECK))
return -EINVAL;
retry:
diff --git a/include/linux/namei.h b/include/linux/namei.h
index 967aa9ea9f96..def6a8a1b531 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -45,9 +45,13 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT};
#define LOOKUP_IN_ROOT 0x10 /* Treat dirfd as fs root. */
#define LOOKUP_CACHED 0x20 /* Only do cached lookup */
#define LOOKUP_LINKAT_EMPTY0x40 /* Linkat request with empty path. */
+
/* LOOKUP_* flags which do scope-related checks based on the dirfd. */
#define LOOKUP_IS_SCOPED (LOOKUP_BENEATH | LOOKUP_IN_ROOT)
+/* If this is set, LOOKUP_EMPTY must be set as well. */
+#define LOOKUP_EMPTY_NOCHECK 0x80 /* Consider path empty. */
+
extern int path_pts(struct path *path);
extern int user_path_at_empty(int, const char __user *, unsigned, struct path
*, int *empty);
diff --git a/include/trace/misc/fs.h b/include/trace/misc/fs.h
index 738b97f22f36..24aec7ed6b0b 100644
--- a/include/trace/misc/fs.h
+++ b/include/trace/misc/fs.h
@@ -119,4 +119,5 @@
{ LOOKUP_NO_XDEV, "NO_XDEV" }, \
{ LOOKUP_BENEATH, "BENEATH" }, \
{ LOOKUP_IN_ROOT, "IN_ROOT" }, \
+ { LOOKUP_EMPTY_NOCHECK, "EMPTY_NOCHECK" }, \
{ LOOKUP_CACHED,"CACHED" })
diff --git a/include/uapi/linux/fcntl.h b/include/uapi/linux/fcntl.h
index c0bcc185fa48..aa2f68d80820 100644
--- a/include/uapi/linux/fcntl.h
+++ b/include/uapi/linux/fcntl.h
@@ -113,6 +113,9 @@
#define AT_STATX_DONT_SYNC 0x4000 /* - Don't sync attributes with the
server */
#define AT_RECURSIVE 0x8000 /* Apply to the entire subtree */
+#define AT_EMPTY_PATH_NOCHECK 0x1 /* Like AT_EMPTY_PATH, but the path
+ is not checked and it's just
+ assumed to be empty */
/* Flags for name_to_handle_at(2). We reuse AT_ flag space to save bits... */
#define AT_HANDLE_FID AT_REMOVEDIR/* file handle is needed to
--
2.45.2
Re: [PATCH v3] tracing/net_sched: NULL pointer dereference in perf_trace_qdisc_reset()
Hi Taehee, On 6/22/24 2:50 오후, Taehee Yoo wrote: > On Sat, Jun 22, 2024 at 1:58 PM wrote: >> >> From: Yunseong Kim >> > > Hi Yunseong, > Thanks a lot for this work! Thank you Taehee for reviewing our patch. It's greatly appreciated. >> During qdisc initialization, qdisc was being set to noop_queue. >> In veth_init_queue, the initial tx_num was reduced back to one, >> causing the qdisc reset to be called with noop, which led to the kernel >> panic. >> >> I've attached the GitHub gist link that C converted syz-execprogram >> source code and 3 log of reproduced vmcore-dmesg. >> >> https://gist.github.com/yskelg/cc64562873ce249cdd0d5a358b77d740 >> >> Yeoreum and I use two fuzzing tool simultaneously. >> >> One process with syz-executor : https://github.com/google/syzkaller >> >> $ ./syz-execprog -executor=./syz-executor -repeat=1 -sandbox=setuid \ >> -enable=none -collide=false log1 >> >> The other process with perf fuzzer: >> https://github.com/deater/perf_event_tests/tree/master/fuzzer >> >> $ perf_event_tests/fuzzer/perf_fuzzer >> >> I think this will happen on the kernel version. >> >> Linux kernel version +v6.7.10, +v6.8, +v6.9 and it could happen in v6.10. >> >> This occurred from 51270d573a8d. I think this patch is absolutely >> necessary. Previously, It was showing not intended string value of name. > I found a simple reproducer, please use the below command to test this patch. > > echo 1 > /sys/kernel/debug/tracing/events/enable > ip link add veth0 type veth peer name veth1 The perf event is activated by perf_fuzzer, and it's indeed a similar environment with veth. > In my machine, the splat looks like: > > BUG: kernel NULL pointer dereference, address: 0130 > #PF: supervisor read access in kernel mode > #PF: error_code(0x) - not-present page > PGD 0 P4D 0 > Oops: Oops: [#1] PREEMPT SMP NOPTI > CPU: 1 PID: 1207 Comm: ip Not tainted 6.10.0-rc4+ #25 > 362ec22a686962a9936425abea9a73f03b445c0c > Hardware name: ASUS System Product Name/PRIME Z690-P D4, BIOS 0603 11/01/2021 > RIP: 0010:strlen+0x0/0x20 > Code: f7 75 ec 31 c0 c3 cc cc cc cc 48 89 f8 c3 cc cc cc cc 0f 1f 84 > 00 00 00 00 00 90 90 90 90 9c > RSP: 0018:bed8435c7630 EFLAGS: 00010206 > RAX: 92d629c0 RBX: a14100185c60 RCX: > RDX: 0001 RSI: 92d62840 RDI: 0130 > RBP: 92dc4600 R08: 0fd0 R09: 0010 > R10: 92c66c98 R11: 0001 R12: 0001 > R13: R14: 0130 R15: 92d62840 > FS: 7f6a94e50b80() GS:a1485f68() knlGS: > CS: 0010 DS: ES: CR0: 80050033 > CR2: 0130 CR3: 000103414000 CR4: 007506f0 > PKRU: 5554 > Call Trace: > > ? __die+0x20/0x70 > ? page_fault_oops+0x15a/0x460 > ? trace_event_raw_event_x86_exceptions+0x5f/0xa0 > ? exc_page_fault+0x6e/0x180 > ? asm_exc_page_fault+0x22/0x30 > ? __pfx_strlen+0x10/0x10 > trace_event_raw_event_qdisc_reset+0x4d/0x180 > ? synchronize_rcu_expedited+0x215/0x240 > ? __pfx_autoremove_wake_function+0x10/0x10 > qdisc_reset+0x130/0x150 > netif_set_real_num_tx_queues+0xe3/0x1e0 > veth_init_queues+0x44/0x70 [veth 24a9dd1cd1b1b279e1b467ad46d47a753799b428] > veth_newlink+0x22b/0x440 [veth 24a9dd1cd1b1b279e1b467ad46d47a753799b428] > __rtnl_newlink+0x718/0x990 > rtnl_newlink+0x44/0x70 > rtnetlink_rcv_msg+0x159/0x410 > ? kmalloc_reserve+0x90/0xf0 > ? trace_event_raw_event_kmem_cache_alloc+0x87/0xe0 > ? __pfx_rtnetlink_rcv_msg+0x10/0x10 > netlink_rcv_skb+0x54/0x100 > netlink_unicast+0x243/0x370 > netlink_sendmsg+0x1bb/0x3e0 > sys_sendmsg+0x2bb/0x320 > ? copy_msghdr_from_user+0x6d/0xa0 > ___sys_sendmsg+0x88/0xd0 > > Thanks a lot! > Taehee Yoo I think this bug might cause inconvenience for developers working on net devices driver in a virtual machine when they use tracing. I'm appreciate your effort in reproducing it. Warm Regards, Yunseong Kim
Re: [PATCH v3] tracing/net_sched: NULL pointer dereference in perf_trace_qdisc_reset()
On Sat, Jun 22, 2024 at 1:58 PM wrote: > > From: Yunseong Kim > Hi Yunseong, Thanks a lot for this work! > In the TRACE_EVENT(qdisc_reset) NULL dereference occurred from > > qdisc->dev_queue->dev ->name > > This situation simulated from bunch of veths and Bluetooth dis/reconnection. > > During qdisc initialization, qdisc was being set to noop_queue. > In veth_init_queue, the initial tx_num was reduced back to one, > causing the qdisc reset to be called with noop, which led to the kernel panic. > > I've attached the GitHub gist link that C converted syz-execprogram > source code and 3 log of reproduced vmcore-dmesg. > > https://gist.github.com/yskelg/cc64562873ce249cdd0d5a358b77d740 > > Yeoreum and I use two fuzzing tool simultaneously. > > One process with syz-executor : https://github.com/google/syzkaller > > $ ./syz-execprog -executor=./syz-executor -repeat=1 -sandbox=setuid \ > -enable=none -collide=false log1 > > The other process with perf fuzzer: > https://github.com/deater/perf_event_tests/tree/master/fuzzer > > $ perf_event_tests/fuzzer/perf_fuzzer > > I think this will happen on the kernel version. > > Linux kernel version +v6.7.10, +v6.8, +v6.9 and it could happen in v6.10. > > This occurred from 51270d573a8d. I think this patch is absolutely > necessary. Previously, It was showing not intended string value of name. > > I've reproduced 3 time from my fedora 40 Debug Kernel with any other module > or patched. > > version: 6.10.0-0.rc2.20240608gitdc772f8237f9.29.fc41.aarch64+debug > > [ 5287.164555] veth0_vlan: left promiscuous mode > [ 5287.164929] veth1_macvtap: left promiscuous mode > [ 5287.164950] veth0_macvtap: left promiscuous mode > [ 5287.164983] veth1_vlan: left promiscuous mode > [ 5287.165008] veth0_vlan: left promiscuous mode > [ 5287.165450] veth1_macvtap: left promiscuous mode > [ 5287.165472] veth0_macvtap: left promiscuous mode > [ 5287.165502] veth1_vlan: left promiscuous mode > … > [ 5297.598240] bridge0: port 2(bridge_slave_1) entered blocking state > [ 5297.598262] bridge0: port 2(bridge_slave_1) entered forwarding state > [ 5297.598296] bridge0: port 1(bridge_slave_0) entered blocking state > [ 5297.598313] bridge0: port 1(bridge_slave_0) entered forwarding state > [ 5297.616090] 8021q: adding VLAN 0 to HW filter on device bond0 > [ 5297.620405] bridge0: port 1(bridge_slave_0) entered disabled state > [ 5297.620730] bridge0: port 2(bridge_slave_1) entered disabled state > [ 5297.627247] 8021q: adding VLAN 0 to HW filter on device team0 > [ 5297.629636] bridge0: port 1(bridge_slave_0) entered blocking state > … > [ 5298.002798] bridge_slave_0: left promiscuous mode > [ 5298.002869] bridge0: port 1(bridge_slave_0) entered disabled state > [ 5298.309444] bond0 (unregistering): (slave bond_slave_0): Releasing backup > interface > [ 5298.315206] bond0 (unregistering): (slave bond_slave_1): Releasing backup > interface > [ 5298.320207] bond0 (unregistering): Released all slaves > [ 5298.354296] hsr_slave_0: left promiscuous mode > [ 5298.360750] hsr_slave_1: left promiscuous mode > [ 5298.374889] veth1_macvtap: left promiscuous mode > [ 5298.374931] veth0_macvtap: left promiscuous mode > [ 5298.374988] veth1_vlan: left promiscuous mode > [ 5298.375024] veth0_vlan: left promiscuous mode > [ 5299.109741] team0 (unregistering): Port device team_slave_1 removed > [ 5299.185870] team0 (unregistering): Port device team_slave_0 removed > … > [ 5300.155443] Bluetooth: hci3: unexpected cc 0x0c03 length: 249 > 1 > [ 5300.155724] Bluetooth: hci3: unexpected cc 0x1003 length: 249 > 9 > [ 5300.155988] Bluetooth: hci3: unexpected cc 0x1001 length: 249 > 9 > …. > [ 5301.075531] team0: Port device team_slave_1 added > [ 5301.085515] bridge0: port 1(bridge_slave_0) entered blocking state > [ 5301.085531] bridge0: port 1(bridge_slave_0) entered disabled state > [ 5301.085588] bridge_slave_0: entered allmulticast mode > [ 5301.085800] bridge_slave_0: entered promiscuous mode > [ 5301.095617] bridge0: port 1(bridge_slave_0) entered blocking state > [ 5301.095633] bridge0: port 1(bridge_slave_0) entered disabled state > … > [ 5301.149734] bond0: (slave bond_slave_0): Enslaving as an active interface > with an up link > [ 5301.173234] bond0: (slave bond_slave_0): Enslaving as an active interface > with an up link > [ 5301.180517] bond0: (slave bond_slave_1): Enslaving as an active interface > with an up link > [ 5301.193481] hsr_slave_0: entered promiscuous mode > [ 5301.204425] hsr_slave_1: entered promiscuous mode > [ 5301.210172] debugfs: Directory 'hsr0' with parent 'hsr' already present! > [ 5301.210185] Cannot create hsr debugfs directory > [ 5301.224061] bond0: (slave bond_slave_1): Enslaving as an active interface > with an up link > [ 5301.246901] bond0: (slave bond_slave_0): Enslaving as an active interface > with an up link > [ 5301.255934] team0: Port device team_slave_0 added > [ 5301.256480] team0: Port device team_slave_1 added > [ 5301.256948] team0: Port device
[PATCH v3] tracing/net_sched: NULL pointer dereference in perf_trace_qdisc_reset()
From: Yunseong Kim In the TRACE_EVENT(qdisc_reset) NULL dereference occurred from qdisc->dev_queue->dev ->name This situation simulated from bunch of veths and Bluetooth dis/reconnection. During qdisc initialization, qdisc was being set to noop_queue. In veth_init_queue, the initial tx_num was reduced back to one, causing the qdisc reset to be called with noop, which led to the kernel panic. I've attached the GitHub gist link that C converted syz-execprogram source code and 3 log of reproduced vmcore-dmesg. https://gist.github.com/yskelg/cc64562873ce249cdd0d5a358b77d740 Yeoreum and I use two fuzzing tool simultaneously. One process with syz-executor : https://github.com/google/syzkaller $ ./syz-execprog -executor=./syz-executor -repeat=1 -sandbox=setuid \ -enable=none -collide=false log1 The other process with perf fuzzer: https://github.com/deater/perf_event_tests/tree/master/fuzzer $ perf_event_tests/fuzzer/perf_fuzzer I think this will happen on the kernel version. Linux kernel version +v6.7.10, +v6.8, +v6.9 and it could happen in v6.10. This occurred from 51270d573a8d. I think this patch is absolutely necessary. Previously, It was showing not intended string value of name. I've reproduced 3 time from my fedora 40 Debug Kernel with any other module or patched. version: 6.10.0-0.rc2.20240608gitdc772f8237f9.29.fc41.aarch64+debug [ 5287.164555] veth0_vlan: left promiscuous mode [ 5287.164929] veth1_macvtap: left promiscuous mode [ 5287.164950] veth0_macvtap: left promiscuous mode [ 5287.164983] veth1_vlan: left promiscuous mode [ 5287.165008] veth0_vlan: left promiscuous mode [ 5287.165450] veth1_macvtap: left promiscuous mode [ 5287.165472] veth0_macvtap: left promiscuous mode [ 5287.165502] veth1_vlan: left promiscuous mode … [ 5297.598240] bridge0: port 2(bridge_slave_1) entered blocking state [ 5297.598262] bridge0: port 2(bridge_slave_1) entered forwarding state [ 5297.598296] bridge0: port 1(bridge_slave_0) entered blocking state [ 5297.598313] bridge0: port 1(bridge_slave_0) entered forwarding state [ 5297.616090] 8021q: adding VLAN 0 to HW filter on device bond0 [ 5297.620405] bridge0: port 1(bridge_slave_0) entered disabled state [ 5297.620730] bridge0: port 2(bridge_slave_1) entered disabled state [ 5297.627247] 8021q: adding VLAN 0 to HW filter on device team0 [ 5297.629636] bridge0: port 1(bridge_slave_0) entered blocking state … [ 5298.002798] bridge_slave_0: left promiscuous mode [ 5298.002869] bridge0: port 1(bridge_slave_0) entered disabled state [ 5298.309444] bond0 (unregistering): (slave bond_slave_0): Releasing backup interface [ 5298.315206] bond0 (unregistering): (slave bond_slave_1): Releasing backup interface [ 5298.320207] bond0 (unregistering): Released all slaves [ 5298.354296] hsr_slave_0: left promiscuous mode [ 5298.360750] hsr_slave_1: left promiscuous mode [ 5298.374889] veth1_macvtap: left promiscuous mode [ 5298.374931] veth0_macvtap: left promiscuous mode [ 5298.374988] veth1_vlan: left promiscuous mode [ 5298.375024] veth0_vlan: left promiscuous mode [ 5299.109741] team0 (unregistering): Port device team_slave_1 removed [ 5299.185870] team0 (unregistering): Port device team_slave_0 removed … [ 5300.155443] Bluetooth: hci3: unexpected cc 0x0c03 length: 249 > 1 [ 5300.155724] Bluetooth: hci3: unexpected cc 0x1003 length: 249 > 9 [ 5300.155988] Bluetooth: hci3: unexpected cc 0x1001 length: 249 > 9 …. [ 5301.075531] team0: Port device team_slave_1 added [ 5301.085515] bridge0: port 1(bridge_slave_0) entered blocking state [ 5301.085531] bridge0: port 1(bridge_slave_0) entered disabled state [ 5301.085588] bridge_slave_0: entered allmulticast mode [ 5301.085800] bridge_slave_0: entered promiscuous mode [ 5301.095617] bridge0: port 1(bridge_slave_0) entered blocking state [ 5301.095633] bridge0: port 1(bridge_slave_0) entered disabled state … [ 5301.149734] bond0: (slave bond_slave_0): Enslaving as an active interface with an up link [ 5301.173234] bond0: (slave bond_slave_0): Enslaving as an active interface with an up link [ 5301.180517] bond0: (slave bond_slave_1): Enslaving as an active interface with an up link [ 5301.193481] hsr_slave_0: entered promiscuous mode [ 5301.204425] hsr_slave_1: entered promiscuous mode [ 5301.210172] debugfs: Directory 'hsr0' with parent 'hsr' already present! [ 5301.210185] Cannot create hsr debugfs directory [ 5301.224061] bond0: (slave bond_slave_1): Enslaving as an active interface with an up link [ 5301.246901] bond0: (slave bond_slave_0): Enslaving as an active interface with an up link [ 5301.255934] team0: Port device team_slave_0 added [ 5301.256480] team0: Port device team_slave_1 added [ 5301.256948] team0: Port device team_slave_0 added … [ 5301.435928] hsr_slave_0: entered promiscuous mode [ 5301.446029] hsr_slave_1: entered promiscuous mode [ 5301.455872] debugfs: Directory 'hsr0' with parent 'hsr' already present! [ 5301.455884] Cannot create hsr debugfs directory [ 5301.502664]
Re: [PATCH net v2] net/sched: Fixes: 51270d573a8d NULL ptr deref in perf_trace_qdisc_reset()
Hi Jakub, On 6/22/24 9:05 오전, Jakub Kicinski wrote: > On Sat, 22 Jun 2024 01:25:54 +0900 ysk...@gmail.com wrote: >> Subject: [PATCH net v2] net/sched: Fixes: 51270d573a8d NULL ptr deref in >> perf_trace_qdisc_reset() > > the Fixes tag goes before your signoff, rather than as title. > try > > git log --grep=Fixes Oh, I'm sorry. I completely misunderstood. Thank you for kindly explaining! I'll take your advice and send the next version of the patch! Warm Regards, Yunseong Kim
Re: [PATCH 0/4] MCE wrapper and support for new SMCA syndrome MSRs
On 6/21/2024 15:01, Yazen Ghannam wrote: > On Fri, Jun 21, 2024 at 06:58:23PM +0200, Borislav Petkov wrote: >> On Thu, May 30, 2024 at 04:16:16PM -0500, Avadhut Naik wrote: >>> arch/x86/include/asm/mce.h | 20 ++- >>> arch/x86/kernel/cpu/mce/apei.c | 111 ++ >>> arch/x86/kernel/cpu/mce/core.c | 191 ++-- >>> arch/x86/kernel/cpu/mce/dev-mcelog.c| 2 +- >>> arch/x86/kernel/cpu/mce/genpool.c | 20 +-- >>> arch/x86/kernel/cpu/mce/inject.c| 4 +- >>> arch/x86/kernel/cpu/mce/internal.h | 4 +- >>> drivers/acpi/acpi_extlog.c | 2 +- >>> drivers/acpi/nfit/mce.c | 2 +- >>> drivers/edac/i7core_edac.c | 2 +- >>> drivers/edac/igen6_edac.c | 2 +- >>> drivers/edac/mce_amd.c | 27 +++- >>> drivers/edac/pnd2_edac.c| 2 +- >>> drivers/edac/sb_edac.c | 2 +- >>> drivers/edac/skx_common.c | 2 +- >>> drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 2 +- >>> drivers/ras/amd/fmpm.c | 2 +- >>> drivers/ras/cec.c | 2 +- >>> include/trace/events/mce.h | 51 --- >>> 19 files changed, 286 insertions(+), 164 deletions(-) >> >> This doesn't apply anymore - please redo this ontop of the latest tip/master. >> > > Avadhut, > > You can drop the dependencies on other sets. We can sort out any > conflicts as needed. > Sounds good! Will redo on top of tip/master and resubmit. > Thanks, > Yazen -- Thanks, Avadhut Naik
Re: [PATCH net-next] net: virtio: unify code to init stats
On Thu, 20 Jun 2024 03:44:53 -0400 Michael S. Tsirkin wrote: > Moving initialization of stats structure into > __free_old_xmit reduces the code size slightly. > It also makes it clearer that this function shouldn't > be called multiple times on the same stats struct. > > Signed-off-by: Michael S. Tsirkin Hm, doesn't apply?
Re: [PATCH net v2] net/sched: Fixes: 51270d573a8d NULL ptr deref in perf_trace_qdisc_reset()
On Sat, 22 Jun 2024 01:25:54 +0900 ysk...@gmail.com wrote: > Subject: [PATCH net v2] net/sched: Fixes: 51270d573a8d NULL ptr deref in > perf_trace_qdisc_reset() the Fixes tag goes before your signoff, rather than as title. try git log --grep=Fixes -- pw-bot: cr
[PATCH v9 5/5] remoteproc: qcom: enable in-kernel PD mapper
Request in-kernel protection domain mapper to be started before starting
Qualcomm DSP and release it once DSP is stopped. Once all DSPs are
stopped, the PD mapper will be stopped too.
Reviewed-by: Chris Lew
Tested-by: Steev Klimaszewski
Tested-by: Neil Armstrong # on SM8550-QRD
Signed-off-by: Dmitry Baryshkov
---
drivers/remoteproc/qcom_common.c| 87 +
drivers/remoteproc/qcom_common.h| 10 +
drivers/remoteproc/qcom_q6v5_adsp.c | 3 ++
drivers/remoteproc/qcom_q6v5_mss.c | 3 ++
drivers/remoteproc/qcom_q6v5_pas.c | 3 ++
drivers/remoteproc/qcom_q6v5_wcss.c | 3 ++
6 files changed, 109 insertions(+)
diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
index 03e5f5d533eb..8c8688f99f0a 100644
--- a/drivers/remoteproc/qcom_common.c
+++ b/drivers/remoteproc/qcom_common.c
@@ -13,6 +13,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -25,6 +26,7 @@
#define to_glink_subdev(d) container_of(d, struct qcom_rproc_glink, subdev)
#define to_smd_subdev(d) container_of(d, struct qcom_rproc_subdev, subdev)
#define to_ssr_subdev(d) container_of(d, struct qcom_rproc_ssr, subdev)
+#define to_pdm_subdev(d) container_of(d, struct qcom_rproc_pdm, subdev)
#define MAX_NUM_OF_SS 10
#define MAX_REGION_NAME_LENGTH 16
@@ -519,5 +521,90 @@ void qcom_remove_ssr_subdev(struct rproc *rproc, struct
qcom_rproc_ssr *ssr)
}
EXPORT_SYMBOL_GPL(qcom_remove_ssr_subdev);
+static void pdm_dev_release(struct device *dev)
+{
+ struct auxiliary_device *adev = to_auxiliary_dev(dev);
+
+ kfree(adev);
+}
+
+static int pdm_notify_prepare(struct rproc_subdev *subdev)
+{
+ struct qcom_rproc_pdm *pdm = to_pdm_subdev(subdev);
+ struct auxiliary_device *adev;
+ int ret;
+
+ adev = kzalloc(sizeof(*adev), GFP_KERNEL);
+ if (!adev)
+ return -ENOMEM;
+
+ adev->dev.parent = pdm->dev;
+ adev->dev.release = pdm_dev_release;
+ adev->name = "pd-mapper";
+ adev->id = pdm->index;
+
+ ret = auxiliary_device_init(adev);
+ if (ret) {
+ kfree(adev);
+ return ret;
+ }
+
+ ret = auxiliary_device_add(adev);
+ if (ret) {
+ auxiliary_device_uninit(adev);
+ return ret;
+ }
+
+ pdm->adev = adev;
+
+ return 0;
+}
+
+
+static void pdm_notify_unprepare(struct rproc_subdev *subdev)
+{
+ struct qcom_rproc_pdm *pdm = to_pdm_subdev(subdev);
+
+ if (!pdm->adev)
+ return;
+
+ auxiliary_device_delete(pdm->adev);
+ auxiliary_device_uninit(pdm->adev);
+ pdm->adev = NULL;
+}
+
+/**
+ * qcom_add_pdm_subdev() - register PD Mapper subdevice
+ * @rproc: rproc handle
+ * @pdm: PDM subdevice handle
+ *
+ * Register @pdm so that Protection Device mapper service is started when the
+ * DSP is started too.
+ */
+void qcom_add_pdm_subdev(struct rproc *rproc, struct qcom_rproc_pdm *pdm)
+{
+ pdm->dev = >dev;
+ pdm->index = rproc->index;
+
+ pdm->subdev.prepare = pdm_notify_prepare;
+ pdm->subdev.unprepare = pdm_notify_unprepare;
+
+ rproc_add_subdev(rproc, >subdev);
+}
+EXPORT_SYMBOL_GPL(qcom_add_pdm_subdev);
+
+/**
+ * qcom_remove_pdm_subdev() - remove PD Mapper subdevice
+ * @rproc: rproc handle
+ * @pdm: PDM subdevice handle
+ *
+ * Remove the PD Mapper subdevice.
+ */
+void qcom_remove_pdm_subdev(struct rproc *rproc, struct qcom_rproc_pdm *pdm)
+{
+ rproc_remove_subdev(rproc, >subdev);
+}
+EXPORT_SYMBOL_GPL(qcom_remove_pdm_subdev);
+
MODULE_DESCRIPTION("Qualcomm Remoteproc helper driver");
MODULE_LICENSE("GPL v2");
diff --git a/drivers/remoteproc/qcom_common.h b/drivers/remoteproc/qcom_common.h
index 9ef4449052a9..b07fbaa091a0 100644
--- a/drivers/remoteproc/qcom_common.h
+++ b/drivers/remoteproc/qcom_common.h
@@ -34,6 +34,13 @@ struct qcom_rproc_ssr {
struct qcom_ssr_subsystem *info;
};
+struct qcom_rproc_pdm {
+ struct rproc_subdev subdev;
+ struct device *dev;
+ int index;
+ struct auxiliary_device *adev;
+};
+
void qcom_minidump(struct rproc *rproc, unsigned int minidump_id,
void (*rproc_dumpfn_t)(struct rproc *rproc,
struct rproc_dump_segment *segment, void *dest,
size_t offset,
@@ -52,6 +59,9 @@ void qcom_add_ssr_subdev(struct rproc *rproc, struct
qcom_rproc_ssr *ssr,
const char *ssr_name);
void qcom_remove_ssr_subdev(struct rproc *rproc, struct qcom_rproc_ssr *ssr);
+void qcom_add_pdm_subdev(struct rproc *rproc, struct qcom_rproc_pdm *pdm);
+void qcom_remove_pdm_subdev(struct rproc *rproc, struct qcom_rproc_pdm *pdm);
+
#if IS_ENABLED(CONFIG_QCOM_SYSMON)
struct qcom_sysmon *qcom_add_sysmon_subdev(struct rproc *rproc,
const char *name,
diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c
[PATCH v9 4/5] soc: qcom: add pd-mapper implementation
Existing userspace protection domain mapper implementation has several
issue. It doesn't play well with CONFIG_EXTRA_FIRMWARE, it doesn't
reread JSON files if firmware location is changed (or if firmware was
not available at the time pd-mapper was started but the corresponding
directory is mounted later), etc.
Provide in-kernel service implementing protection domain mapping
required to work with several services, which are provided by the DSP
firmware.
This module is loaded automatically by the remoteproc drivers when
necessary via the symbol dependency. It uses a root node to match a
protection domains map for a particular board. It is not possible to
implement it as a 'driver' as there is no corresponding device.
Tested-by: Steev Klimaszewski
Tested-by: Alexey Minnekhanov
Reviewed-by: Chris Lew
Tested-by: Neil Armstrong # on SM8550-QRD
Signed-off-by: Dmitry Baryshkov
---
drivers/soc/qcom/Kconfig | 11 +
drivers/soc/qcom/Makefile | 1 +
drivers/soc/qcom/pdr_internal.h | 14 +
drivers/soc/qcom/qcom_pd_mapper.c | 676 ++
drivers/soc/qcom/qcom_pdr_msg.c | 34 ++
5 files changed, 736 insertions(+)
diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index 95973c6b828f..0a2f2bfd7863 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -72,6 +72,17 @@ config QCOM_OCMEM
requirements. This is typically used by the GPU, camera/video, and
audio components on some Snapdragon SoCs.
+config QCOM_PD_MAPPER
+ tristate "Qualcomm Protection Domain Mapper"
+ select QCOM_QMI_HELPERS
+ depends on NET && QRTR
+ default QCOM_RPROC_COMMON
+ help
+ The Protection Domain Mapper maps registered services to the domains
+ and instances handled by the remote DSPs. This is a kernel-space
+ implementation of the service. It is a simpler alternative to the
+ userspace daemon.
+
config QCOM_PDR_HELPERS
tristate
select QCOM_QMI_HELPERS
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index 3110ac3288bc..d3560f861085 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_QCOM_COMMAND_DB) += cmd-db.o
obj-$(CONFIG_QCOM_GSBI)+= qcom_gsbi.o
obj-$(CONFIG_QCOM_MDT_LOADER) += mdt_loader.o
obj-$(CONFIG_QCOM_OCMEM) += ocmem.o
+obj-$(CONFIG_QCOM_PD_MAPPER) += qcom_pd_mapper.o
obj-$(CONFIG_QCOM_PDR_HELPERS) += pdr_interface.o
obj-$(CONFIG_QCOM_PDR_MSG) += qcom_pdr_msg.o
obj-$(CONFIG_QCOM_PMIC_GLINK) += pmic_glink.o
diff --git a/drivers/soc/qcom/pdr_internal.h b/drivers/soc/qcom/pdr_internal.h
index 7e5bb5a95275..8d17f7fb79e7 100644
--- a/drivers/soc/qcom/pdr_internal.h
+++ b/drivers/soc/qcom/pdr_internal.h
@@ -13,6 +13,8 @@
#define SERVREG_SET_ACK_REQ0x23
#define SERVREG_RESTART_PD_REQ 0x24
+#define SERVREG_LOC_PFR_REQ0x24
+
#define SERVREG_DOMAIN_LIST_LENGTH 32
#define SERVREG_RESTART_PD_REQ_MAX_LEN 67
#define SERVREG_REGISTER_LISTENER_REQ_LEN 71
@@ -20,6 +22,7 @@
#define SERVREG_GET_DOMAIN_LIST_REQ_MAX_LEN74
#define SERVREG_STATE_UPDATED_IND_MAX_LEN 79
#define SERVREG_GET_DOMAIN_LIST_RESP_MAX_LEN 2389
+#define SERVREG_LOC_PFR_RESP_MAX_LEN 10
struct servreg_location_entry {
char name[SERVREG_NAME_LENGTH + 1];
@@ -79,6 +82,15 @@ struct servreg_set_ack_resp {
struct qmi_response_type_v01 resp;
};
+struct servreg_loc_pfr_req {
+ char service[SERVREG_NAME_LENGTH + 1];
+ char reason[257];
+};
+
+struct servreg_loc_pfr_resp {
+ struct qmi_response_type_v01 rsp;
+};
+
extern const struct qmi_elem_info servreg_location_entry_ei[];
extern const struct qmi_elem_info servreg_get_domain_list_req_ei[];
extern const struct qmi_elem_info servreg_get_domain_list_resp_ei[];
@@ -89,5 +101,7 @@ extern const struct qmi_elem_info
servreg_restart_pd_resp_ei[];
extern const struct qmi_elem_info servreg_state_updated_ind_ei[];
extern const struct qmi_elem_info servreg_set_ack_req_ei[];
extern const struct qmi_elem_info servreg_set_ack_resp_ei[];
+extern const struct qmi_elem_info servreg_loc_pfr_req_ei[];
+extern const struct qmi_elem_info servreg_loc_pfr_resp_ei[];
#endif
diff --git a/drivers/soc/qcom/qcom_pd_mapper.c
b/drivers/soc/qcom/qcom_pd_mapper.c
new file mode 100644
index ..ecb64f06527f
--- /dev/null
+++ b/drivers/soc/qcom/qcom_pd_mapper.c
@@ -0,0 +1,676 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Qualcomm Protection Domain mapper
+ *
+ * Copyright (c) 2023 Linaro Ltd.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "pdr_internal.h"
+
+#define SERVREG_QMI_VERSION 0x101
+#define SERVREG_QMI_INSTANCE 0
+
+#define TMS_SERVREG_SERVICE "tms/servreg"
+
+struct
[PATCH v9 3/5] soc: qcom: pdr: extract PDR message marshalling data
The in-kernel PD mapper is going to use same message structures as the
QCOM_PDR_HELPERS module. Extract message marshalling data to separate
module that can be used by both PDR helpers and by PD mapper.
Reviewed-by: Bryan O'Donoghue
Tested-by: Steev Klimaszewski
Tested-by: Alexey Minnekhanov
Tested-by: Neil Armstrong # on SM8550-QRD
Signed-off-by: Dmitry Baryshkov
---
drivers/soc/qcom/Kconfig| 4 +
drivers/soc/qcom/Makefile | 1 +
drivers/soc/qcom/pdr_internal.h | 306 ++
drivers/soc/qcom/qcom_pdr_msg.c | 319
4 files changed, 334 insertions(+), 296 deletions(-)
diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index 5af33b0e3470..95973c6b828f 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -75,8 +75,12 @@ config QCOM_OCMEM
config QCOM_PDR_HELPERS
tristate
select QCOM_QMI_HELPERS
+ select QCOM_PDR_MSG
depends on NET
+config QCOM_PDR_MSG
+ tristate
+
config QCOM_PMIC_PDCHARGER_ULOG
tristate "Qualcomm PMIC PDCharger ULOG driver"
depends on RPMSG
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index ca0bece0dfff..3110ac3288bc 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_QCOM_GSBI) += qcom_gsbi.o
obj-$(CONFIG_QCOM_MDT_LOADER) += mdt_loader.o
obj-$(CONFIG_QCOM_OCMEM) += ocmem.o
obj-$(CONFIG_QCOM_PDR_HELPERS) += pdr_interface.o
+obj-$(CONFIG_QCOM_PDR_MSG) += qcom_pdr_msg.o
obj-$(CONFIG_QCOM_PMIC_GLINK) += pmic_glink.o
obj-$(CONFIG_QCOM_PMIC_GLINK) += pmic_glink_altmode.o
obj-$(CONFIG_QCOM_PMIC_PDCHARGER_ULOG) += pmic_pdcharger_ulog.o
diff --git a/drivers/soc/qcom/pdr_internal.h b/drivers/soc/qcom/pdr_internal.h
index 03c282b7f17e..7e5bb5a95275 100644
--- a/drivers/soc/qcom/pdr_internal.h
+++ b/drivers/soc/qcom/pdr_internal.h
@@ -28,83 +28,12 @@ struct servreg_location_entry {
u32 instance;
};
-static const struct qmi_elem_info servreg_location_entry_ei[] = {
- {
- .data_type = QMI_STRING,
- .elem_len = SERVREG_NAME_LENGTH + 1,
- .elem_size = sizeof(char),
- .array_type = NO_ARRAY,
- .tlv_type = 0,
- .offset = offsetof(struct servreg_location_entry,
- name),
- },
- {
- .data_type = QMI_UNSIGNED_4_BYTE,
- .elem_len = 1,
- .elem_size = sizeof(u32),
- .array_type = NO_ARRAY,
- .tlv_type = 0,
- .offset = offsetof(struct servreg_location_entry,
- instance),
- },
- {
- .data_type = QMI_UNSIGNED_1_BYTE,
- .elem_len = 1,
- .elem_size = sizeof(u8),
- .array_type = NO_ARRAY,
- .tlv_type = 0,
- .offset = offsetof(struct servreg_location_entry,
- service_data_valid),
- },
- {
- .data_type = QMI_UNSIGNED_4_BYTE,
- .elem_len = 1,
- .elem_size = sizeof(u32),
- .array_type = NO_ARRAY,
- .tlv_type = 0,
- .offset = offsetof(struct servreg_location_entry,
- service_data),
- },
- {}
-};
-
struct servreg_get_domain_list_req {
char service_name[SERVREG_NAME_LENGTH + 1];
u8 domain_offset_valid;
u32 domain_offset;
};
-static const struct qmi_elem_info servreg_get_domain_list_req_ei[] = {
- {
- .data_type = QMI_STRING,
- .elem_len = SERVREG_NAME_LENGTH + 1,
- .elem_size = sizeof(char),
- .array_type = NO_ARRAY,
- .tlv_type = 0x01,
- .offset = offsetof(struct servreg_get_domain_list_req,
- service_name),
- },
- {
- .data_type = QMI_OPT_FLAG,
- .elem_len = 1,
- .elem_size = sizeof(u8),
- .array_type = NO_ARRAY,
- .tlv_type = 0x10,
- .offset = offsetof(struct servreg_get_domain_list_req,
- domain_offset_valid),
- },
- {
- .data_type = QMI_UNSIGNED_4_BYTE,
- .elem_len = 1,
- .elem_size = sizeof(u32),
- .array_type = NO_ARRAY,
- .tlv_type = 0x10,
- .offset = offsetof(struct servreg_get_domain_list_req,
- domain_offset),
- },
-
[PATCH v9 2/5] soc: qcom: pdr: fix parsing of domains lists
While parsing the domains list, start offsets from 0 rather than from
domains_read. The domains_read is equal to the total count of the
domains we have seen, while the domains list in the message starts from
offset 0.
Fixes: fbe639b44a82 ("soc: qcom: Introduce Protection Domain Restart helpers")
Tested-by: Steev Klimaszewski
Tested-by: Alexey Minnekhanov
Reviewed-by: Chris Lew
Tested-by: Neil Armstrong # on SM8550-QRD
Signed-off-by: Dmitry Baryshkov
---
drivers/soc/qcom/pdr_interface.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/soc/qcom/pdr_interface.c b/drivers/soc/qcom/pdr_interface.c
index 76a62c2ecc58..216166e98fae 100644
--- a/drivers/soc/qcom/pdr_interface.c
+++ b/drivers/soc/qcom/pdr_interface.c
@@ -417,7 +417,7 @@ static int pdr_locate_service(struct pdr_handle *pdr,
struct pdr_service *pds)
if (ret < 0)
goto out;
- for (i = domains_read; i < resp->domain_list_len; i++) {
+ for (i = 0; i < resp->domain_list_len; i++) {
entry = >domain_list[i];
if (strnlen(entry->name, sizeof(entry->name)) ==
sizeof(entry->name))
--
2.39.2
[PATCH v9 1/5] soc: qcom: pdr: protect locator_addr with the main mutex
If the service locator server is restarted fast enough, the PDR can
rewrite locator_addr fields concurrently. Protect them by placing
modification of those fields under the main pdr->lock.
Fixes: fbe639b44a82 ("soc: qcom: Introduce Protection Domain Restart helpers")
Tested-by: Neil Armstrong # on SM8550-QRD
Tested-by: Steev Klimaszewski
Tested-by: Alexey Minnekhanov
Signed-off-by: Dmitry Baryshkov
---
drivers/soc/qcom/pdr_interface.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/soc/qcom/pdr_interface.c b/drivers/soc/qcom/pdr_interface.c
index a1b6a4081dea..76a62c2ecc58 100644
--- a/drivers/soc/qcom/pdr_interface.c
+++ b/drivers/soc/qcom/pdr_interface.c
@@ -76,12 +76,12 @@ static int pdr_locator_new_server(struct qmi_handle *qmi,
locator_hdl);
struct pdr_service *pds;
+ mutex_lock(>lock);
/* Create a local client port for QMI communication */
pdr->locator_addr.sq_family = AF_QIPCRTR;
pdr->locator_addr.sq_node = svc->node;
pdr->locator_addr.sq_port = svc->port;
- mutex_lock(>lock);
pdr->locator_init_complete = true;
mutex_unlock(>lock);
@@ -104,10 +104,10 @@ static void pdr_locator_del_server(struct qmi_handle *qmi,
mutex_lock(>lock);
pdr->locator_init_complete = false;
- mutex_unlock(>lock);
pdr->locator_addr.sq_node = 0;
pdr->locator_addr.sq_port = 0;
+ mutex_unlock(>lock);
}
static const struct qmi_ops pdr_locator_ops = {
@@ -365,12 +365,14 @@ static int pdr_get_domain_list(struct
servreg_get_domain_list_req *req,
if (ret < 0)
return ret;
+ mutex_lock(>lock);
ret = qmi_send_request(>locator_hdl,
>locator_addr,
, SERVREG_GET_DOMAIN_LIST_REQ,
SERVREG_GET_DOMAIN_LIST_REQ_MAX_LEN,
servreg_get_domain_list_req_ei,
req);
+ mutex_unlock(>lock);
if (ret < 0) {
qmi_txn_cancel();
return ret;
--
2.39.2
[PATCH v9 0/5] soc: qcom: add in-kernel pd-mapper implementation
Protection domain mapper is a QMI service providing mapping between 'protection domains' and services supported / allowed in these domains. For example such mapping is required for loading of the WiFi firmware or for properly starting up the UCSI / altmode / battery manager support. The existing userspace implementation has several issue. It doesn't play well with CONFIG_EXTRA_FIRMWARE, it doesn't reread the JSON files if the firmware location is changed (or if the firmware was not available at the time pd-mapper was started but the corresponding directory is mounted later), etc. However this configuration is largely static and common between different platforms. Provide in-kernel service implementing static per-platform data. --- Changes in v9: - Adjust locking in pdr_get_domain_list(), releasing the mutex right after qmi_send_request() (Chris Lew) - Link to v8: https://lore.kernel.org/r/20240512-qcom-pd-mapper-v8-0-5ecbb276f...@linaro.org Changes in v8: - Reworked pd-mapper to register as an rproc_subdev / auxdev - Dropped Tested-by from Steev and Alexey from the last patch since the implementation was changed significantly. - Add sensors, cdsp and mpss_root domains to 660 config (Alexey Minnekhanov) - Added platform entry for sm4250 (used for qrb4210 / RB2) - Added locking to the pdr_get_domain_list() (Chris Lew) - Remove the call to qmi_del_server() and corresponding API (Chris Lew) - In qmi_handle_init() changed 1024 to a defined constant (Chris Lew) - Link to v7: https://lore.kernel.org/r/20240424-qcom-pd-mapper-v7-0-05f7fc646...@linaro.org Changes in v7: - Fixed modular build (Steev) - Link to v6: https://lore.kernel.org/r/20240422-qcom-pd-mapper-v6-0-f96957d01...@linaro.org Changes in v6: - Reworked mutex to fix lockdep issue on deregistration - Fixed dependencies between PD-mapper and remoteproc to fix modular builds (Krzysztof) - Added EXPORT_SYMBOL_GPL to fix modular builds (Krzysztof) - Fixed kerneldocs (Krzysztof) - Removed extra pr_debug messages (Krzysztof) - Fixed wcss build (Krzysztof) - Added platforms which do not require protection domain mapping to silence the notice on those platforms - Link to v5: https://lore.kernel.org/r/20240419-qcom-pd-mapper-v5-0-e35b6f847...@linaro.org Changes in v5: - pdr: drop lock in pdr_register_listener, list_lock is already held (Chris Lew) - pd_mapper: reworked to provide static configuration per platform (Bjorn) - Link to v4: https://lore.kernel.org/r/20240311-qcom-pd-mapper-v4-0-24679cca5...@linaro.org Changes in v4: - Fixed missing chunk, reenabled kfree in qmi_del_server (Konrad) - Added configuration for sm6350 (Thanks to Luca) - Removed RFC tag (Konrad) - Link to v3: https://lore.kernel.org/r/20240304-qcom-pd-mapper-v3-0-6858fa1ac...@linaro.org Changes in RFC v3: - Send start / stop notifications when PD-mapper domain list is changed - Reworked the way PD-mapper treats protection domains, register all of them in a single batch - Added SC7180 domains configuration based on TCL Book 14 GO - Link to v2: https://lore.kernel.org/r/20240301-qcom-pd-mapper-v2-0-5d12a081d...@linaro.org Changes in RFC v2: - Swapped num_domains / domains (Konrad) - Fixed an issue with battery not working on sc8280xp - Added missing configuration for QCS404 To: Bjorn Andersson To: Konrad Dybcio To: Sibi Sankar To: Mathieu Poirier Cc: linux-arm-...@vger.kernel.org Cc: linux-kernel@vger.kernel.org Cc: linux-remotep...@vger.kernel.org --- Dmitry Baryshkov (5): soc: qcom: pdr: protect locator_addr with the main mutex soc: qcom: pdr: fix parsing of domains lists soc: qcom: pdr: extract PDR message marshalling data soc: qcom: add pd-mapper implementation remoteproc: qcom: enable in-kernel PD mapper drivers/remoteproc/qcom_common.c| 87 + drivers/remoteproc/qcom_common.h| 10 + drivers/remoteproc/qcom_q6v5_adsp.c | 3 + drivers/remoteproc/qcom_q6v5_mss.c | 3 + drivers/remoteproc/qcom_q6v5_pas.c | 3 + drivers/remoteproc/qcom_q6v5_wcss.c | 3 + drivers/soc/qcom/Kconfig| 15 + drivers/soc/qcom/Makefile | 2 + drivers/soc/qcom/pdr_interface.c| 8 +- drivers/soc/qcom/pdr_internal.h | 318 ++--- drivers/soc/qcom/qcom_pd_mapper.c | 676 drivers/soc/qcom/qcom_pdr_msg.c | 353 +++ 12 files changed, 1183 insertions(+), 298 deletions(-) --- base-commit: 2102cb0d050d34d50b9642a3a50861787527e922 change-id: 20240301-qcom-pd-mapper-e12d622d4ad0 Best regards, -- Dmitry Baryshkov
Re: [PATCH v2 0/3] Add Sony Xperia Z3 Compact smartphone
On Fri, Jun 21, 2024 at 05:26:41PM GMT, Valeriy Klimin wrote: > This is almost the same as the dts of the Xperia Z3, except for the > battery charge limits. > > The current on the l21 regulator for shinano is also bumped up > to stop SD card errors. > > Signed-off-by: Valeriy Klimin > --- > Changes in v2: > - Reordered dt-bindings and dts commits > - Link to v1: > https://lore.kernel.org/r/20240621-sony-aries-v1-0-bcf968769...@gmail.com Please let reviewers finish their job first. It's recommended to post once in 24 hours timeframe. Otherwise you end up getting feedback and tags for v1, while you have already posted v2. > > --- > Valeriy Klimin (3): > dt-bindings: arm: qcom: Add Sony Xperia Z3 Compact > ARM: dts: qcom: Add Sony Xperia Z3 Compact smartphone > ARM: dts: qcom: msm8974-sony-shinano: increase load on l21 for sdhc2 > > Documentation/devicetree/bindings/arm/qcom.yaml| 1 + > arch/arm/boot/dts/qcom/Makefile| 1 + > .../qcom-msm8974pro-sony-xperia-shinano-aries.dts | 44 > ++ > ...qcom-msm8974pro-sony-xperia-shinano-common.dtsi | 2 + > 4 files changed, 48 insertions(+) > --- > base-commit: cd214efd16e30bf1aa40ccfaaf9177f47dd21fd5 > change-id: 20240620-sony-aries-4a5bce06c91d > > Best regards, > -- > Valeriy Klimin > -- With best wishes Dmitry
Re: [PATCH v9 7/8] clk: qcom: Add WCSSAON reset
On Fri, Jun 21, 2024 at 05:16:58PM GMT, Gokul Sriram Palanisamy wrote:
> Add WCSSAON reset required for Q6v5 on IPQ8074 SoC.
>
> Signed-off-by: Nikhil Prakash V
> Signed-off-by: Sricharan R
> Signed-off-by: Gokul Sriram Palanisamy
Three authors for a single line?
> Acked-by: Stephen Boyd
> ---
> drivers/clk/qcom/gcc-ipq8074.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/clk/qcom/gcc-ipq8074.c b/drivers/clk/qcom/gcc-ipq8074.c
> index 32fd01ef469a..d382d16b9c10 100644
> --- a/drivers/clk/qcom/gcc-ipq8074.c
> +++ b/drivers/clk/qcom/gcc-ipq8074.c
> @@ -4712,6 +4712,7 @@ static const struct qcom_reset_map gcc_ipq8074_resets[]
> = {
> [GCC_NSSPORT4_RESET] = { .reg = 0x68014, .bitmask = BIT(27) |
> GENMASK(9, 8) },
> [GCC_NSSPORT5_RESET] = { .reg = 0x68014, .bitmask = BIT(28) |
> GENMASK(11, 10) },
> [GCC_NSSPORT6_RESET] = { .reg = 0x68014, .bitmask = BIT(29) |
> GENMASK(13, 12) },
> + [GCC_WCSSAON_RESET] = { 0x59010, 0 },
Do you notice that you line is pretty significantly different from the
previous lines? The time has passed since Stephen has acked this line in
2019.
> };
>
> static struct gdsc *gcc_ipq8074_gdscs[] = {
> --
> 2.34.1
>
--
With best wishes
Dmitry
Re: [PATCH v9 0/8] remoteproc: qcom: q6v5-wcss: Add support for secure pil
On Fri, Jun 21, 2024 at 05:16:51PM GMT, Gokul Sriram Palanisamy wrote: > IPQ8074 needs support for secure pil as well. > Also, currently only unified firmware is supported. > IPQ8074 supports split firmware for q6 and m3, so > adding support for that. > > changes since v8: > - Rebased on top of Linux 6.10-rc4 Why do you have so many dead email addresses in you To/Cc lists? > > Gokul Sriram Palanisamy (8): > remoteproc: qcom: Add PRNG proxy clock > remoteproc: qcom: Add secure PIL support > remoteproc: qcom: Add support for split q6 + m3 wlan firmware > remoteproc: qcom: Add ssr subdevice identifier > remoteproc: qcom: Update regmap offsets for halt register > dt-bindings: clock: qcom: Add reset for WCSSAON > clk: qcom: Add WCSSAON reset > arm64: dts: qcom: Enable Q6v5 WCSS for ipq8074 SoC > > arch/arm64/boot/dts/qcom/ipq8074.dtsi| 80 + > drivers/clk/qcom/gcc-ipq8074.c | 1 + > drivers/remoteproc/qcom_q6v5_wcss.c | 162 +++ > include/dt-bindings/clock/qcom,gcc-ipq8074.h | 1 + > 4 files changed, 212 insertions(+), 32 deletions(-) > > -- > 2.34.1 > -- With best wishes Dmitry
Re: [PATCH v9 4/8] remoteproc: qcom: Add ssr subdevice identifier
On Fri, Jun 21, 2024 at 05:16:55PM GMT, Gokul Sriram Palanisamy wrote:
> Add name for ssr subdevice on IPQ8074 SoC.
Is it SSR or ssr? Why is it necessary?
>
> Signed-off-by: Nikhil Prakash V
> Signed-off-by: Sricharan R
> Signed-off-by: Gokul Sriram Palanisamy
Three authors for a single-line patch?
> ---
> drivers/remoteproc/qcom_q6v5_wcss.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/remoteproc/qcom_q6v5_wcss.c
> b/drivers/remoteproc/qcom_q6v5_wcss.c
> index d8b79765d5c6..06936ca1d079 100644
> --- a/drivers/remoteproc/qcom_q6v5_wcss.c
> +++ b/drivers/remoteproc/qcom_q6v5_wcss.c
> @@ -1170,6 +1170,7 @@ static const struct wcss_data wcss_ipq8074_res_init = {
> .crash_reason_smem = WCSS_CRASH_REASON,
> .aon_reset_required = true,
> .wcss_q6_reset_required = true,
> + .ssr_name = "q6wcss",
> .ops = _wcss_ipq8074_ops,
> .requires_force_stop = true,
> .need_mem_protection = true,
> --
> 2.34.1
>
--
With best wishes
Dmitry
Re: [PATCH v9 0/8] remoteproc: qcom: q6v5-wcss: Add support for secure pil
On Fri, Jun 21, 2024 at 05:16:51PM GMT, Gokul Sriram Palanisamy wrote: > IPQ8074 needs support for secure pil as well. Could you please settle on 'pil' or 'PIL'. Just use one of them. Explain, what is secure PIL. > Also, currently only unified firmware is supported. What is unified firmware? What is split firmware? Why do I have so many questions after reading the cover letter for what claims to be v9? > IPQ8074 supports split firmware for q6 and m3, so > adding support for that. Ok. After reading through the first few patches. Is WCSS support on IPQ8074 broken? > > changes since v8: > - Rebased on top of Linux 6.10-rc4 Previous changelog has wanished? > > Gokul Sriram Palanisamy (8): > remoteproc: qcom: Add PRNG proxy clock > remoteproc: qcom: Add secure PIL support > remoteproc: qcom: Add support for split q6 + m3 wlan firmware > remoteproc: qcom: Add ssr subdevice identifier > remoteproc: qcom: Update regmap offsets for halt register > dt-bindings: clock: qcom: Add reset for WCSSAON > clk: qcom: Add WCSSAON reset > arm64: dts: qcom: Enable Q6v5 WCSS for ipq8074 SoC > > arch/arm64/boot/dts/qcom/ipq8074.dtsi| 80 + > drivers/clk/qcom/gcc-ipq8074.c | 1 + > drivers/remoteproc/qcom_q6v5_wcss.c | 162 +++ > include/dt-bindings/clock/qcom,gcc-ipq8074.h | 1 + > 4 files changed, 212 insertions(+), 32 deletions(-) > > -- > 2.34.1 > -- With best wishes Dmitry
Re: [PATCH v9 2/8] remoteproc: qcom: Add secure PIL support
On Fri, Jun 21, 2024 at 05:16:53PM GMT, Gokul Sriram Palanisamy wrote:
> IPQ8074 uses secure PIL. Hence, adding the support for the same.
See Documentation/process/submitting-patches.rst
>
> Signed-off-by: Nikhil Prakash V
> Signed-off-by: Sricharan R
> Signed-off-by: Gokul Sriram Palanisamy
> ---
> drivers/remoteproc/qcom_q6v5_wcss.c | 43 +++--
> 1 file changed, 40 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/remoteproc/qcom_q6v5_wcss.c
> b/drivers/remoteproc/qcom_q6v5_wcss.c
> index 366b19cbd994..e45e79d80238 100644
> --- a/drivers/remoteproc/qcom_q6v5_wcss.c
> +++ b/drivers/remoteproc/qcom_q6v5_wcss.c
> @@ -18,6 +18,7 @@
> #include
> #include
> #include
> +#include
> #include "qcom_common.h"
> #include "qcom_pil_info.h"
> #include "qcom_q6v5.h"
> @@ -86,6 +87,9 @@
> #define TCSR_WCSS_CLK_ENABLE 0x14
>
> #define MAX_HALT_REG 3
> +
> +#define WCNSS_PAS_ID 6
> +
> enum {
> WCSS_IPQ8074,
> WCSS_QCS404,
> @@ -134,6 +138,7 @@ struct q6v5_wcss {
> unsigned int crash_reason_smem;
> u32 version;
> bool requires_force_stop;
> + bool need_mem_protection;
needs
>
> struct qcom_rproc_glink glink_subdev;
> struct qcom_rproc_ssr ssr_subdev;
> @@ -152,6 +157,7 @@ struct wcss_data {
> int ssctl_id;
> const struct rproc_ops *ops;
> bool requires_force_stop;
> + bool need_mem_protection;
needs
> };
>
> static int q6v5_wcss_reset(struct q6v5_wcss *wcss)
> @@ -251,6 +257,15 @@ static int q6v5_wcss_start(struct rproc *rproc)
>
> qcom_q6v5_prepare(>q6v5);
>
> + if (wcss->need_mem_protection) {
> + ret = qcom_scm_pas_auth_and_reset(WCNSS_PAS_ID);
> + if (ret) {
> + dev_err(wcss->dev, "wcss_reset failed\n");
> + return ret;
> + }
> + goto wait_for_reset;
> + }
Use if/else instead of a goto.
> +
> /* Release Q6 and WCSS reset */
> ret = reset_control_deassert(wcss->wcss_reset);
> if (ret) {
> @@ -285,6 +300,7 @@ static int q6v5_wcss_start(struct rproc *rproc)
> if (ret)
> goto wcss_q6_reset;
>
> +wait_for_reset:
This is more like wait_for_start
> ret = qcom_q6v5_wait_for_start(>q6v5, 5 * HZ);
> if (ret == -ETIMEDOUT)
> dev_err(wcss->dev, "start timed out\n");
> @@ -718,6 +734,15 @@ static int q6v5_wcss_stop(struct rproc *rproc)
> struct q6v5_wcss *wcss = rproc->priv;
> int ret;
>
> + if (wcss->need_mem_protection) {
> + ret = qcom_scm_pas_shutdown(WCNSS_PAS_ID);
> + if (ret) {
> + dev_err(wcss->dev, "not able to shutdown\n");
> + return ret;
> + }
> + goto pas_done;
> + }
if/else. Or abstract this to functions.
> +
> /* WCSS powerdown */
> if (wcss->requires_force_stop) {
> ret = qcom_q6v5_request_stop(>q6v5, NULL);
> @@ -742,6 +767,7 @@ static int q6v5_wcss_stop(struct rproc *rproc)
> return ret;
> }
>
> +pas_done:
> clk_disable_unprepare(wcss->prng_clk);
> qcom_q6v5_unprepare(>q6v5);
>
> @@ -765,9 +791,15 @@ static int q6v5_wcss_load(struct rproc *rproc, const
> struct firmware *fw)
> struct q6v5_wcss *wcss = rproc->priv;
> int ret;
>
> - ret = qcom_mdt_load_no_init(wcss->dev, fw, rproc->firmware,
> - 0, wcss->mem_region, wcss->mem_phys,
> - wcss->mem_size, >mem_reloc);
> + if (wcss->need_mem_protection)
> + ret = qcom_mdt_load(wcss->dev, fw, rproc->firmware,
> + WCNSS_PAS_ID, wcss->mem_region,
> + wcss->mem_phys, wcss->mem_size,
> + >mem_reloc);
> + else
> + ret = qcom_mdt_load_no_init(wcss->dev, fw, rproc->firmware,
> + 0, wcss->mem_region, wcss->mem_phys,
> + wcss->mem_size, >mem_reloc);
> if (ret)
> return ret;
>
> @@ -1035,6 +1067,9 @@ static int q6v5_wcss_probe(struct platform_device *pdev)
> if (!desc)
> return -EINVAL;
>
> + if (desc->need_mem_protection && !qcom_scm_is_available())
> + return -EPROBE_DEFER;
> +
> rproc = devm_rproc_alloc(>dev, pdev->name, desc->ops,
>desc->firmware_name, sizeof(*wcss));
> if (!rproc) {
> @@ -1048,6 +1083,7 @@ static int q6v5_wcss_probe(struct platform_device *pdev)
>
> wcss->version = desc->version;
> wcss->requires_force_stop = desc->requires_force_stop;
> + wcss->need_mem_protection = desc->need_mem_protection;
>
> ret = q6v5_wcss_init_mmio(wcss, pdev);
> if (ret)
> @@ -,6 +1147,7 @@ static const struct wcss_data wcss_ipq8074_res_init = {
>
Re: [PATCH v9 1/8] remoteproc: qcom: Add PRNG proxy clock
On Fri, Jun 21, 2024 at 05:16:52PM GMT, Gokul Sriram Palanisamy wrote:
> PRNG clock is needed by the secure PIL, support for the same
> is added in subsequent patches.
Which 'same'?
What is 'secure PIL'?
>
> Signed-off-by: Nikhil Prakash V
> Signed-off-by: Sricharan R
> Signed-off-by: Gokul Sriram Palanisamy
> ---
> drivers/remoteproc/qcom_q6v5_wcss.c | 65 +
> 1 file changed, 47 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/remoteproc/qcom_q6v5_wcss.c
> b/drivers/remoteproc/qcom_q6v5_wcss.c
> index 94f68c919ee6..366b19cbd994 100644
> --- a/drivers/remoteproc/qcom_q6v5_wcss.c
> +++ b/drivers/remoteproc/qcom_q6v5_wcss.c
> @@ -91,19 +91,6 @@ enum {
> WCSS_QCS404,
> };
>
> -struct wcss_data {
> - const char *firmware_name;
> - unsigned int crash_reason_smem;
> - u32 version;
> - bool aon_reset_required;
> - bool wcss_q6_reset_required;
> - const char *ssr_name;
> - const char *sysmon_name;
> - int ssctl_id;
> - const struct rproc_ops *ops;
> - bool requires_force_stop;
> -};
> -
> struct q6v5_wcss {
> struct device *dev;
>
> @@ -128,6 +115,7 @@ struct q6v5_wcss {
> struct clk *qdsp6ss_xo_cbcr;
> struct clk *qdsp6ss_core_gfmux;
> struct clk *lcc_bcr_sleep;
> + struct clk *prng_clk;
> struct regulator *cx_supply;
> struct qcom_sysmon *sysmon;
>
> @@ -151,6 +139,21 @@ struct q6v5_wcss {
> struct qcom_rproc_ssr ssr_subdev;
> };
>
> +struct wcss_data {
> + int (*init_clock)(struct q6v5_wcss *wcss);
> + int (*init_regulator)(struct q6v5_wcss *wcss);
> + const char *firmware_name;
> + unsigned int crash_reason_smem;
> + u32 version;
> + bool aon_reset_required;
> + bool wcss_q6_reset_required;
> + const char *ssr_name;
> + const char *sysmon_name;
> + int ssctl_id;
> + const struct rproc_ops *ops;
> + bool requires_force_stop;
> +};
Move this back and use forward-declaration of struct q6v5_wcss.
> +
> static int q6v5_wcss_reset(struct q6v5_wcss *wcss)
> {
> int ret;
--
With best wishes
Dmitry
Re: [PATCH 2/2] arm64: dts: qcom: qcm6490-fairphone-fp5: Configure PM8008 regulators
On Fri, Jun 21, 2024 at 10:42:31AM GMT, Luca Weiss wrote: > PM8008 regulators are used for the cameras found on FP5. Configure the > chip and its voltages. > > Signed-off-by: Luca Weiss > --- > arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts | 105 > - > 1 file changed, 104 insertions(+), 1 deletion(-) > Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH 1/2] arm64: dts: qcom: sm7225-fairphone-fp4: Configure PM8008 regulators
On Fri, Jun 21, 2024 at 10:42:30AM GMT, Luca Weiss wrote: > PM8008 regulators are used for the cameras found on FP4. Configure the > chip and its voltages. > > Signed-off-by: Luca Weiss > --- > arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts | 109 > +- > 1 file changed, 108 insertions(+), 1 deletion(-) > Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH 1/3] ARM: dts: qcom: Add Sony Xperia Z3 Compact smartphone
On Fri, Jun 21, 2024 at 11:14:46AM GMT, Valeriy Klimin wrote: > Add the dts for the Z3 Compact. This is currently almost the same > as the plain Z3 as they share almost the same hardware and > nothing device-specific is currently supported. > > Signed-off-by: Valeriy Klimin > --- > arch/arm/boot/dts/qcom/Makefile| 1 + > .../qcom-msm8974pro-sony-xperia-shinano-aries.dts | 44 > ++ > 2 files changed, 45 insertions(+) > Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH 3/3] ARM: dts: qcom: msm8974-sony-shinano: increase load on l21 for sdhc2
On Fri, Jun 21, 2024 at 11:14:48AM GMT, Valeriy Klimin wrote:
> SD cards would exhibit errors similar to ones described in commit
> 27fe0fc05f35 ("ARM: dts: msm8974-FP2: Increase load on l20 for sdhci")
>
> This patch applies the same change to the regulator for sdhc2.
>
> Signed-off-by: Valeriy Klimin
> ---
> arch/arm/boot/dts/qcom/qcom-msm8974pro-sony-xperia-shinano-common.dtsi | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git
> a/arch/arm/boot/dts/qcom/qcom-msm8974pro-sony-xperia-shinano-common.dtsi
> b/arch/arm/boot/dts/qcom/qcom-msm8974pro-sony-xperia-shinano-common.dtsi
> index e129bb1bd6ec..6af7c71c7158 100644
> --- a/arch/arm/boot/dts/qcom/qcom-msm8974pro-sony-xperia-shinano-common.dtsi
> +++ b/arch/arm/boot/dts/qcom/qcom-msm8974pro-sony-xperia-shinano-common.dtsi
> @@ -380,6 +380,8 @@ pm8941_l20: l20 {
> pm8941_l21: l21 {
> regulator-min-microvolt = <295>;
> regulator-max-microvolt = <295>;
> + regulator-system-load = <50>;
> + regulator-allow-set-load;
> regulator-boot-on;
> };
>
Just out of pure interest, how the value was choosen? Do you have any
reference to the vendor kernel or DT?
--
With best wishes
Dmitry
Re: [PATCH 0/4] MCE wrapper and support for new SMCA syndrome MSRs
On Fri, Jun 21, 2024 at 06:58:23PM +0200, Borislav Petkov wrote: > On Thu, May 30, 2024 at 04:16:16PM -0500, Avadhut Naik wrote: > > arch/x86/include/asm/mce.h | 20 ++- > > arch/x86/kernel/cpu/mce/apei.c | 111 ++ > > arch/x86/kernel/cpu/mce/core.c | 191 ++-- > > arch/x86/kernel/cpu/mce/dev-mcelog.c| 2 +- > > arch/x86/kernel/cpu/mce/genpool.c | 20 +-- > > arch/x86/kernel/cpu/mce/inject.c| 4 +- > > arch/x86/kernel/cpu/mce/internal.h | 4 +- > > drivers/acpi/acpi_extlog.c | 2 +- > > drivers/acpi/nfit/mce.c | 2 +- > > drivers/edac/i7core_edac.c | 2 +- > > drivers/edac/igen6_edac.c | 2 +- > > drivers/edac/mce_amd.c | 27 +++- > > drivers/edac/pnd2_edac.c| 2 +- > > drivers/edac/sb_edac.c | 2 +- > > drivers/edac/skx_common.c | 2 +- > > drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 2 +- > > drivers/ras/amd/fmpm.c | 2 +- > > drivers/ras/cec.c | 2 +- > > include/trace/events/mce.h | 51 --- > > 19 files changed, 286 insertions(+), 164 deletions(-) > > This doesn't apply anymore - please redo this ontop of the latest tip/master. > Avadhut, You can drop the dependencies on other sets. We can sort out any conflicts as needed. Thanks, Yazen
[RFC PATCH v1 1/2] virtio/vsock: rework deferred credit update logic
Previous calculation of 'free_space' was wrong (but worked as expected
in most cases, see below), because it didn't account number of bytes in
rx queue. Let's rework 'free_space' calculation in the following way:
as this value is considered free space at rx side from tx point of view,
it must be equal to return value of 'virtio_transport_get_credit()' at
tx side. This function uses 'tx_cnt' counter and 'peer_fwd_cnt': first
is number of transmitted bytes (without wrap), second is last 'fwd_cnt'
value received from rx. So let's use same approach at rx side during
'free_space' calculation: add 'rx_cnt' counter which is number of
received bytes (also without wrap) and subtract 'last_fwd_cnt' from it.
Now we have:
1) 'rx_cnt' == 'tx_cnt' at both sides.
2) 'last_fwd_cnt' == 'peer_fwd_cnt' - because first is last 'fwd_cnt'
sent to tx, while second is last 'fwd_cnt' received from rx.
Now 'free_space' is handled correctly and also we don't need
'low_rx_bytes' flag - this was more like a hack.
Previous calculation of 'free_space' worked (in 99% cases), because if
we take a look on behaviour of both expressions (new and previous):
'(rx_cnt - last_fwd_cnt)' and '(fwd_cnt - last_fwd_cnt)'
Both of them always grows up, with almost same "speed": only difference
is that 'rx_cnt' is incremented earlier during packet is received,
while 'fwd_cnt' in incremented when packet is read by user. So if 'rx_cnt'
grows "faster", then resulting 'free_space' become smaller also, so we
send credit updates a little bit more, but:
* 'free_space' calculation based on 'rx_cnt' gives the same value,
which tx sees as free space at rx side, so original idea of
'free_space' is now implemented as planned.
* Hack with 'low_rx_bytes' now is not needed.
Also here is some performance comparison between both versions of
'free_space' calculation:
*--*--*--*
| | 'rx_cnt' | previous |
*--*--*--*
|H -> G| 8.42 | 7.82 |
*--*--*--*
|G -> H| 11.6 | 12.1 |
*--*--*--*
As benchmark 'vsock-iperf' with default arguments was used. There is no
significant performance difference before and after this patch.
Signed-off-by: Arseniy Krasnov
---
include/linux/virtio_vsock.h| 1 +
net/vmw_vsock/virtio_transport_common.c | 8 +++-
2 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
index c82089dee0c8..3579491c411e 100644
--- a/include/linux/virtio_vsock.h
+++ b/include/linux/virtio_vsock.h
@@ -135,6 +135,7 @@ struct virtio_vsock_sock {
u32 peer_buf_alloc;
/* Protected by rx_lock */
+ u32 rx_cnt;
u32 fwd_cnt;
u32 last_fwd_cnt;
u32 rx_bytes;
diff --git a/net/vmw_vsock/virtio_transport_common.c
b/net/vmw_vsock/virtio_transport_common.c
index 16ff976a86e3..1d4e2328e06e 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -441,6 +441,7 @@ static bool virtio_transport_inc_rx_pkt(struct
virtio_vsock_sock *vvs,
return false;
vvs->rx_bytes += len;
+ vvs->rx_cnt += len;
return true;
}
@@ -558,7 +559,6 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk,
size_t bytes, total = 0;
struct sk_buff *skb;
u32 fwd_cnt_delta;
- bool low_rx_bytes;
int err = -EFAULT;
u32 free_space;
@@ -603,9 +603,7 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk,
}
fwd_cnt_delta = vvs->fwd_cnt - vvs->last_fwd_cnt;
- free_space = vvs->buf_alloc - fwd_cnt_delta;
- low_rx_bytes = (vvs->rx_bytes <
- sock_rcvlowat(sk_vsock(vsk), 0, INT_MAX));
+ free_space = vvs->buf_alloc - (vvs->rx_cnt - vvs->last_fwd_cnt);
spin_unlock_bh(>rx_lock);
@@ -619,7 +617,7 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk,
* number of bytes in rx queue is not enough to wake up reader.
*/
if (fwd_cnt_delta &&
- (free_space < VIRTIO_VSOCK_MAX_PKT_BUF_SIZE || low_rx_bytes))
+ (free_space < VIRTIO_VSOCK_MAX_PKT_BUF_SIZE))
virtio_transport_send_credit_update(vsk);
return total;
--
2.25.1
[RFC PATCH v1 2/2] vsock/test: add test for deferred credit update
This test checks, that we send exactly expected number of credit
update packets during deferred credit update optimization. Test
work in client/server modes:
1) Client just connects to server and send 256Kb of data. 256Kb
is chosen because it is default space for vsock peer. After
transmission client waits until server performs checks of this
test.
2) Server waits for vsock connection and also open raw socket
binded to vsock monitor interface. Then server waits until
there will be 256Kb of data in its rx queue (by reading data
from stream socket with MSG_PEEK flag). Then server starts
reading data from stream socket - it reads entire socket,
also reading packets from raw vsock socket, to check that
there is OP_RW packets. After data read is done, it checks
raw socket again, by waiting exact amount of CREDIT_UPDATE
packets. Finally it checks that rx queue of raw socket is
empty using read with timeout.
Some notes about this test:
* It is only for virtio transport.
* It relies on sizes of virtio rx buffers for guest and host,
to handle raw packets properly (4Kb for guest and 64Kb for
host).
* It relies on free space limit for deferred credit update -
currently it is 64Kb.
* It needs permissions to open raw sockets.
Signed-off-by: Arseniy Krasnov
---
tools/testing/vsock/.gitignore | 1 +
tools/testing/vsock/Makefile| 2 +
tools/testing/vsock/virtio_vsock_test.c | 369
3 files changed, 372 insertions(+)
create mode 100644 tools/testing/vsock/virtio_vsock_test.c
diff --git a/tools/testing/vsock/.gitignore b/tools/testing/vsock/.gitignore
index d9f798713cd7..74d13a38cf43 100644
--- a/tools/testing/vsock/.gitignore
+++ b/tools/testing/vsock/.gitignore
@@ -4,3 +4,4 @@ vsock_test
vsock_diag_test
vsock_perf
vsock_uring_test
+virtio_vsock_test
diff --git a/tools/testing/vsock/Makefile b/tools/testing/vsock/Makefile
index a7f56a09ca9f..e04d69903687 100644
--- a/tools/testing/vsock/Makefile
+++ b/tools/testing/vsock/Makefile
@@ -8,6 +8,8 @@ vsock_perf: vsock_perf.o msg_zerocopy_common.o
vsock_uring_test: LDLIBS = -luring
vsock_uring_test: control.o util.o vsock_uring_test.o timeout.o
msg_zerocopy_common.o
+virtio_vsock_test: virtio_vsock_test.o util.o timeout.o control.o
+
CFLAGS += -g -O2 -Werror -Wall -I. -I../../include -I../../../usr/include
-Wno-pointer-sign -fno-strict-overflow -fno-strict-aliasing -fno-common -MMD
-U_FORTIFY_SOURCE -D_GNU_SOURCE
.PHONY: all test clean
clean:
diff --git a/tools/testing/vsock/virtio_vsock_test.c
b/tools/testing/vsock/virtio_vsock_test.c
new file mode 100644
index ..4320dbc31ecc
--- /dev/null
+++ b/tools/testing/vsock/virtio_vsock_test.c
@@ -0,0 +1,369 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * virtio_vsock_test - vsock.ko test suite for VirtIO transport.
+ *
+ * Copyright (C) 2024 SaluteDevices.
+ *
+ * Author: Arseniy Krasnov
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "control.h"
+#include "util.h"
+
+#define RAW_SOCK_RCV_BUF (1024 * 1024)
+
+#define TOTAL_TX_BYTES (1024 * 256)
+
+#define VIRTIO_VSOCK_MAX_PKT_BUF_SIZE (1024 * 64)
+
+#define VIRTIO_VSOCK_GUEST_RX_BUF_SIZE (4096)
+#define VIRTIO_VSOCK_HOST_RX_BUF_SIZE (1024 * 64)
+
+static const char *interface;
+
+static void test_stream_deferred_credit_update_client(const struct test_opts
*opts)
+{
+ unsigned char buf[VIRTIO_VSOCK_MAX_PKT_BUF_SIZE];
+ int fd;
+ int i;
+
+ fd = vsock_stream_connect(opts->peer_cid, opts->peer_port);
+ if (fd < 0) {
+ perror("connect");
+ exit(EXIT_FAILURE);
+ }
+
+ control_expectln("SERVERREADY");
+ memset(buf, 0, sizeof(buf));
+
+ for (i = 0; i < TOTAL_TX_BYTES / VIRTIO_VSOCK_MAX_PKT_BUF_SIZE; i++) {
+ if (write(fd, buf, sizeof(buf)) != sizeof(buf)) {
+ perror("write");
+ exit(EXIT_FAILURE);
+ }
+ }
+
+ control_writeln("CLIENTDONE");
+ control_expectln("SERVERDONE");
+
+ close(fd);
+}
+
+static int create_raw_sock(const char *ifname)
+{
+ struct sockaddr_ll addr_ll;
+ struct ifreq ifr;
+ size_t rcv_buf;
+ int fd;
+
+ fd = socket(AF_PACKET, SOCK_RAW, htons(ETH_P_ALL));
+ if (fd < 0) {
+ perror("socket");
+ exit(EXIT_FAILURE);
+ }
+
+ memset(, 0, sizeof(ifr));
+ snprintf(ifr.ifr_name, sizeof(ifr.ifr_name),
+"%s", ifname);
+
+ rcv_buf = RAW_SOCK_RCV_BUF;
+ if (setsockopt(fd, SOL_SOCKET, SO_RCVBUFFORCE, _buf,
+ sizeof(rcv_buf))) {
+ perror("setsockopt(SO_RCVBUFFORCE)");
+ exit(EXIT_FAILURE);
+ }
+
+ if (ioctl(fd,
[RFC PATCH v1 0/2] virtio/vsock: some updates for deferred credit update
This patchset contains: 0001 - patch which reworks deferred credit update. Pls see commit message, it contains full description of this problem. 0002 - test which uses vsockmon interface, and checks that deferred credit update works as expected by parsing raw packets. Arseniy Krasnov (2): virtio/vsock: rework deferred credit update logic vsock/test: add test for deferred credit update include/linux/virtio_vsock.h| 1 + net/vmw_vsock/virtio_transport_common.c | 8 +- tools/testing/vsock/.gitignore | 1 + tools/testing/vsock/Makefile| 2 + tools/testing/vsock/virtio_vsock_test.c | 369 5 files changed, 376 insertions(+), 5 deletions(-) create mode 100644 tools/testing/vsock/virtio_vsock_test.c -- 2.25.1
Re: [PATCH v7 00/38] kmsan: Enable on s390
On Fri, 21 Jun 2024 13:34:44 +0200 Ilya Leoshkevich wrote: > v6 -> v7: Drop the ptdump patch. > All patches are reviewed. I added v7 to mm.git (and hence linux-next).
Re: [PATCH 0/4] MCE wrapper and support for new SMCA syndrome MSRs
On Thu, May 30, 2024 at 04:16:16PM -0500, Avadhut Naik wrote: > arch/x86/include/asm/mce.h | 20 ++- > arch/x86/kernel/cpu/mce/apei.c | 111 ++ > arch/x86/kernel/cpu/mce/core.c | 191 ++-- > arch/x86/kernel/cpu/mce/dev-mcelog.c| 2 +- > arch/x86/kernel/cpu/mce/genpool.c | 20 +-- > arch/x86/kernel/cpu/mce/inject.c| 4 +- > arch/x86/kernel/cpu/mce/internal.h | 4 +- > drivers/acpi/acpi_extlog.c | 2 +- > drivers/acpi/nfit/mce.c | 2 +- > drivers/edac/i7core_edac.c | 2 +- > drivers/edac/igen6_edac.c | 2 +- > drivers/edac/mce_amd.c | 27 +++- > drivers/edac/pnd2_edac.c| 2 +- > drivers/edac/sb_edac.c | 2 +- > drivers/edac/skx_common.c | 2 +- > drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 2 +- > drivers/ras/amd/fmpm.c | 2 +- > drivers/ras/cec.c | 2 +- > include/trace/events/mce.h | 51 --- > 19 files changed, 286 insertions(+), 164 deletions(-) This doesn't apply anymore - please redo this ontop of the latest tip/master. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
[PATCH net v2] net/sched: Fixes: 51270d573a8d NULL ptr deref in perf_trace_qdisc_reset()
From: Yunseong Kim In the TRACE_EVENT(qdisc_reset) NULL dereference occurred from qdisc->dev_queue->dev ->name This situation simulated from bunch of veths and Bluetooth dis/reconnection. During qdisc initialization, qdisc was being set to noop_queue. In veth_init_queue, the initial tx_num was reduced back to one, causing the qdisc reset to be called with noop, which led to the kernel panic. I've attached the GitHub gist link that C converted syz-execprogram source code and 3 log of reproduced vmcore-dmesg. https://gist.github.com/yskelg/cc64562873ce249cdd0d5a358b77d740 Yeoreum and I use two fuzzing tool simultaneously. One process with syz-executor : https://github.com/google/syzkaller $ ./syz-execprog -executor=./syz-executor -repeat=1 -sandbox=setuid \ -enable=none -collide=false log1 The other process with perf fuzzer: https://github.com/deater/perf_event_tests/tree/master/fuzzer $ perf_event_tests/fuzzer/perf_fuzzer I think this will happen on the kernel version. Linux kernel version ≥ v6.7.10, ≥ v6.8 ≥ v6.9 and 6.10 This occurred from 51270d573a8d. I think this patch is absolutely necessary. Previously, It was showing not intended string value of name. I've reproduced 3 time from my fedora 40 Debug Kernel with any other module or patched. version: 6.10.0-0.rc2.20240608gitdc772f8237f9.29.fc41.aarch64+debug [ 5287.164555] veth0_vlan: left promiscuous mode [ 5287.164929] veth1_macvtap: left promiscuous mode [ 5287.164950] veth0_macvtap: left promiscuous mode [ 5287.164983] veth1_vlan: left promiscuous mode [ 5287.165008] veth0_vlan: left promiscuous mode [ 5287.165450] veth1_macvtap: left promiscuous mode [ 5287.165472] veth0_macvtap: left promiscuous mode [ 5287.165502] veth1_vlan: left promiscuous mode … [ 5297.598240] bridge0: port 2(bridge_slave_1) entered blocking state [ 5297.598262] bridge0: port 2(bridge_slave_1) entered forwarding state [ 5297.598296] bridge0: port 1(bridge_slave_0) entered blocking state [ 5297.598313] bridge0: port 1(bridge_slave_0) entered forwarding state [ 5297.616090] 8021q: adding VLAN 0 to HW filter on device bond0 [ 5297.620405] bridge0: port 1(bridge_slave_0) entered disabled state [ 5297.620730] bridge0: port 2(bridge_slave_1) entered disabled state [ 5297.627247] 8021q: adding VLAN 0 to HW filter on device team0 [ 5297.629636] bridge0: port 1(bridge_slave_0) entered blocking state … [ 5298.002798] bridge_slave_0: left promiscuous mode [ 5298.002869] bridge0: port 1(bridge_slave_0) entered disabled state [ 5298.309444] bond0 (unregistering): (slave bond_slave_0): Releasing backup interface [ 5298.315206] bond0 (unregistering): (slave bond_slave_1): Releasing backup interface [ 5298.320207] bond0 (unregistering): Released all slaves [ 5298.354296] hsr_slave_0: left promiscuous mode [ 5298.360750] hsr_slave_1: left promiscuous mode [ 5298.374889] veth1_macvtap: left promiscuous mode [ 5298.374931] veth0_macvtap: left promiscuous mode [ 5298.374988] veth1_vlan: left promiscuous mode [ 5298.375024] veth0_vlan: left promiscuous mode [ 5299.109741] team0 (unregistering): Port device team_slave_1 removed [ 5299.185870] team0 (unregistering): Port device team_slave_0 removed … [ 5300.155443] Bluetooth: hci3: unexpected cc 0x0c03 length: 249 > 1 [ 5300.155724] Bluetooth: hci3: unexpected cc 0x1003 length: 249 > 9 [ 5300.155988] Bluetooth: hci3: unexpected cc 0x1001 length: 249 > 9 …. [ 5301.075531] team0: Port device team_slave_1 added [ 5301.085515] bridge0: port 1(bridge_slave_0) entered blocking state [ 5301.085531] bridge0: port 1(bridge_slave_0) entered disabled state [ 5301.085588] bridge_slave_0: entered allmulticast mode [ 5301.085800] bridge_slave_0: entered promiscuous mode [ 5301.095617] bridge0: port 1(bridge_slave_0) entered blocking state [ 5301.095633] bridge0: port 1(bridge_slave_0) entered disabled state … [ 5301.149734] bond0: (slave bond_slave_0): Enslaving as an active interface with an up link [ 5301.173234] bond0: (slave bond_slave_0): Enslaving as an active interface with an up link [ 5301.180517] bond0: (slave bond_slave_1): Enslaving as an active interface with an up link [ 5301.193481] hsr_slave_0: entered promiscuous mode [ 5301.204425] hsr_slave_1: entered promiscuous mode [ 5301.210172] debugfs: Directory 'hsr0' with parent 'hsr' already present! [ 5301.210185] Cannot create hsr debugfs directory [ 5301.224061] bond0: (slave bond_slave_1): Enslaving as an active interface with an up link [ 5301.246901] bond0: (slave bond_slave_0): Enslaving as an active interface with an up link [ 5301.255934] team0: Port device team_slave_0 added [ 5301.256480] team0: Port device team_slave_1 added [ 5301.256948] team0: Port device team_slave_0 added … [ 5301.435928] hsr_slave_0: entered promiscuous mode [ 5301.446029] hsr_slave_1: entered promiscuous mode [ 5301.455872] debugfs: Directory 'hsr0' with parent 'hsr' already present! [ 5301.455884] Cannot create hsr debugfs directory [ 5301.502664] hsr_slave_0: entered
Re: [PATCH v9 6/8] dt-bindings: clock: qcom: Add reset for WCSSAON
On 6/21/2024 5:16 PM, Gokul Sriram Palanisamy wrote: Add binding for WCSSAON reset required for Q6v5 reset on IPQ8074 SoC. Can we include ipq8074 in the title? "dt-bindings: clock: qcom: ipq8074: Add reset for WCSSAON" Signed-off-by: Nikhil Prakash V Signed-off-by: Sricharan R Signed-off-by: Gokul Sriram Palanisamy Acked-by: Rob Herring Acked-by: Stephen Boyd --- include/dt-bindings/clock/qcom,gcc-ipq8074.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/dt-bindings/clock/qcom,gcc-ipq8074.h b/include/dt-bindings/clock/qcom,gcc-ipq8074.h index f9ea55811104..e47cbf7394aa 100644 --- a/include/dt-bindings/clock/qcom,gcc-ipq8074.h +++ b/include/dt-bindings/clock/qcom,gcc-ipq8074.h @@ -381,6 +381,7 @@ #define GCC_NSSPORT4_RESET143 #define GCC_NSSPORT5_RESET144 #define GCC_NSSPORT6_RESET145 +#define GCC_WCSSAON_RESET 146 #define USB0_GDSC0 #define USB1_GDSC 1
Re: [PATCH v9 7/8] clk: qcom: Add WCSSAON reset
On 6/21/2024 5:16 PM, Gokul Sriram Palanisamy wrote:
Add WCSSAON reset required for Q6v5 on IPQ8074 SoC.
Commit title can be written as "clk: qcom: ipq8074: Add WCSSAON reset" ?
With that,
Reviewed-by: Kathiravan Thirumoorthy
Signed-off-by: Nikhil Prakash V
Signed-off-by: Sricharan R
Signed-off-by: Gokul Sriram Palanisamy
Acked-by: Stephen Boyd
---
drivers/clk/qcom/gcc-ipq8074.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/clk/qcom/gcc-ipq8074.c b/drivers/clk/qcom/gcc-ipq8074.c
index 32fd01ef469a..d382d16b9c10 100644
--- a/drivers/clk/qcom/gcc-ipq8074.c
+++ b/drivers/clk/qcom/gcc-ipq8074.c
@@ -4712,6 +4712,7 @@ static const struct qcom_reset_map gcc_ipq8074_resets[] =
{
[GCC_NSSPORT4_RESET] = { .reg = 0x68014, .bitmask = BIT(27) |
GENMASK(9, 8) },
[GCC_NSSPORT5_RESET] = { .reg = 0x68014, .bitmask = BIT(28) |
GENMASK(11, 10) },
[GCC_NSSPORT6_RESET] = { .reg = 0x68014, .bitmask = BIT(29) |
GENMASK(13, 12) },
+ [GCC_WCSSAON_RESET] = { 0x59010, 0 },
};
static struct gdsc *gcc_ipq8074_gdscs[] = {
Re: [PATCH v9 8/8] arm64: dts: qcom: Enable Q6v5 WCSS for ipq8074 SoC
On 6/21/2024 5:16 PM, Gokul Sriram Palanisamy wrote:
Enable remoteproc WCSS PIL driver with glink. Also,
configure shared memory and enables smp2p required for IPC.
Signed-off-by: Nikhil Prakash V
Signed-off-by: Sricharan R
Signed-off-by: Gokul Sriram Palanisamy
---
arch/arm64/boot/dts/qcom/ipq8074.dtsi | 80 +++
1 file changed, 80 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/ipq8074.dtsi
b/arch/arm64/boot/dts/qcom/ipq8074.dtsi
index 92682d3c9478..b98766cce0d6 100644
--- a/arch/arm64/boot/dts/qcom/ipq8074.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq8074.dtsi
@@ -108,6 +108,12 @@ memory@4ac0 {
reg = <0x0 0x4ac0 0x0 0x40>;
no-map;
};
+
+ q6_region: memory@4b00 {
+ no-map;
move the no-map after reg, to align with other entries.
+ reg = <0x0 0x4b00 0x0 0x5f0>;
+ };
+
};
firmware {
@@ -117,6 +123,30 @@ scm {
};
};
...
+ q6v5_wcss: remoteproc@cd0 {
+ compatible = "qcom,ipq8074-wcss-pil";
+ reg = <0x0cd0 0x4040>,
+ <0x004ab000 0x20>;
+ reg-names = "qdsp6",
+ "rmb";
+ qca,auto-restart;
+ qca,extended-intc;
+ interrupts-extended = < 0 325 1>,
Use macros instead of open coding.
+ <_smp2p_in 0 0>,
+ <_smp2p_in 1 0>,
+ <_smp2p_in 2 0>,
+ <_smp2p_in 3 0>;
+ interrupt-names = "wdog",
+ "fatal",
+ "ready",
+ "handover",
+ "stop-ack";
+
+ resets = < GCC_WCSSAON_RESET>,
+< GCC_WCSS_BCR>,
+< GCC_WCSS_Q6_BCR>;
+
+ reset-names = "wcss_aon_reset",
+ "wcss_reset",
+ "wcss_q6_reset";
+
+ clocks = < GCC_PRNG_AHB_CLK>;
+ clock-names = "prng";
+
+ qcom,halt-regs = < 0xa000 0xd000 0xe000>;
+
+ qcom,smem-states = <_smp2p_out 0>,
+ <_smp2p_out 1>;
+ qcom,smem-state-names = "shutdown",
+ "stop";
+
+ memory-region = <_region>;
+
+ glink-edge {
+ interrupts = ;
+ qcom,remote-pid = <1>;
+ mboxes = <_glb 8>;
+
+ rpm_requests {
+ qcom,glink-channels = "rpm_requests";
+ };
+ };
+ };
+
pcie1: pcie@1000 {
compatible = "qcom,pcie-ipq8074";
reg = <0x1000 0xf1d>,
Re: [PATCH v9 4/8] remoteproc: qcom: Add ssr subdevice identifier
On 21/06/2024 13:46, Gokul Sriram Palanisamy wrote: > Add name for ssr subdevice on IPQ8074 SoC. Why? > > Signed-off-by: Nikhil Prakash V > Signed-off-by: Sricharan R > Signed-off-by: Gokul Sriram Palanisamy Three people developed that single line? Something is really odd with your DCO chain. Best regards, Krzysztof
Re: [PATCH v9 8/8] arm64: dts: qcom: Enable Q6v5 WCSS for ipq8074 SoC
On 21/06/2024 13:46, Gokul Sriram Palanisamy wrote:
> Enable remoteproc WCSS PIL driver with glink. Also,
> configure shared memory and enables smp2p required for IPC.
>
> Signed-off-by: Nikhil Prakash V
> Signed-off-by: Sricharan R
> Signed-off-by: Gokul Sriram Palanisamy
> ---
> arch/arm64/boot/dts/qcom/ipq8074.dtsi | 80 +++
> 1 file changed, 80 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/ipq8074.dtsi
> b/arch/arm64/boot/dts/qcom/ipq8074.dtsi
> index 92682d3c9478..b98766cce0d6 100644
> --- a/arch/arm64/boot/dts/qcom/ipq8074.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq8074.dtsi
> @@ -108,6 +108,12 @@ memory@4ac0 {
> reg = <0x0 0x4ac0 0x0 0x40>;
> no-map;
> };
> +
> + q6_region: memory@4b00 {
> + no-map;
> + reg = <0x0 0x4b00 0x0 0x5f0>;
> + };
> +
> };
>
> firmware {
> @@ -117,6 +123,30 @@ scm {
> };
> };
>
> + wcss: smp2p-wcss {
> + compatible = "qcom,smp2p";
> + qcom,smem = <435>, <428>;
> +
> + interrupt-parent = <>;
> + interrupts = ;
> +
> + mboxes = <_glb 9>;
> +
> + qcom,local-pid = <0>;
> + qcom,remote-pid = <1>;
> +
> + wcss_smp2p_out: master-kernel {
> + qcom,entry-name = "master-kernel";
> + #qcom,smem-state-cells = <1>;
> + };
> +
> + wcss_smp2p_in: slave-kernel {
> + qcom,entry-name = "slave-kernel";
> + interrupt-controller;
> + #interrupt-cells = <2>;
> + };
> + };
> +
> soc: soc@0 {
> #address-cells = <1>;
> #size-cells = <1>;
> @@ -824,6 +854,56 @@ frame@b128000 {
> };
> };
>
> + q6v5_wcss: remoteproc@cd0 {
> + compatible = "qcom,ipq8074-wcss-pil";
> + reg = <0x0cd0 0x4040>,
> + <0x004ab000 0x20>;
> + reg-names = "qdsp6",
> + "rmb";
> + qca,auto-restart;
> + qca,extended-intc;
> + interrupts-extended = < 0 325 1>,
> + <_smp2p_in 0 0>,
> + <_smp2p_in 1 0>,
> + <_smp2p_in 2 0>,
> + <_smp2p_in 3 0>;
> + interrupt-names = "wdog",
> + "fatal",
> + "ready",
> + "handover",
> + "stop-ack";
> +
> + resets = < GCC_WCSSAON_RESET>,
> + < GCC_WCSS_BCR>,
> + < GCC_WCSS_Q6_BCR>;
> +
> + reset-names = "wcss_aon_reset",
> + "wcss_reset",
> + "wcss_q6_reset";
> +
> + clocks = < GCC_PRNG_AHB_CLK>;
> + clock-names = "prng";
That's not what your binding is saying. Convert the binding to DT schema
and then validate this DTS.
Best regards,
Krzysztof
Re: [PATCH v9 6/8] dt-bindings: clock: qcom: Add reset for WCSSAON
On 21/06/2024 13:46, Gokul Sriram Palanisamy wrote: > Add binding for WCSSAON reset required for Q6v5 reset on IPQ8074 SoC. > > Signed-off-by: Nikhil Prakash V > Signed-off-by: Sricharan R > Signed-off-by: Gokul Sriram Palanisamy Again, three people contributed to this one define? Best regards, Krzysztof
Re: [PATCH v9 5/8] remoteproc: qcom: Update regmap offsets for halt register
On 21/06/2024 13:46, Gokul Sriram Palanisamy wrote: > Fixed issue in reading halt-regs parameter from device-tree. What issue? That's a terrible commit msg. Explain what is the problem, how can it be reproduced. > > Signed-off-by: Sricharan R > Signed-off-by: Gokul Sriram Palanisamy > --- > drivers/remoteproc/qcom_q6v5_wcss.c | 22 ++ > 1 file changed, 14 insertions(+), 8 deletions(-) > > diff --git a/drivers/remoteproc/qcom_q6v5_wcss.c > b/drivers/remoteproc/qcom_q6v5_wcss.c > index 06936ca1d079..87b78eb15b86 100644 > --- a/drivers/remoteproc/qcom_q6v5_wcss.c > +++ b/drivers/remoteproc/qcom_q6v5_wcss.c > @@ -86,7 +86,7 @@ > #define TCSR_WCSS_CLK_MASK 0x1F > #define TCSR_WCSS_CLK_ENABLE 0x14 > > -#define MAX_HALT_REG 3 > +#define MAX_HALT_REG 4 ? That's confusing and looks unrelated. Best regards, Krzysztof
[PATCH] ring-buffer: Limit time with disabled interrupts in rb_check_pages()
Function rb_check_pages() validates the integrity of a specified per-CPU
tracing ring buffer. It does so by walking the underlying linked
list and checking its next and prev links.
To guarantee that the list doesn't get modified during the check,
a caller typically needs to take cpu_buffer->reader_lock. This prevents
the check from running concurrently with a potential reader which can
make the list temporarily inconsistent when swapping its old reader page
into the buffer.
A problem is that the time when interrupts are disabled is
non-deterministic, dependent on the ring buffer size. This particularly
affects PREEMPT_RT because the reader_lock is a raw spinlock which
doesn't become sleepable on PREEMPT_RT kernels.
Modify the check so it still tries to walk the whole list but gives up
the reader_lock between checking individual pages.
Signed-off-by: Petr Pavlu
---
This is a follow-up to the discussion in
https://lore.kernel.org/linux-trace-kernel/20240517134008.24529-1-petr.pa...@suse.com/
kernel/trace/ring_buffer.c | 96 +++---
1 file changed, 70 insertions(+), 26 deletions(-)
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 28853966aa9a..3aefaf8a4d58 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -1454,40 +1454,91 @@ static void rb_check_bpage(struct ring_buffer_per_cpu
*cpu_buffer,
RB_WARN_ON(cpu_buffer, val & RB_FLAG_MASK);
}
+static bool rb_check_links(struct ring_buffer_per_cpu *cpu_buffer,
+ struct list_head *list)
+{
+ if (RB_WARN_ON(cpu_buffer,
+ rb_list_head(rb_list_head(list->next)->prev) != list))
+ return false;
+
+ if (RB_WARN_ON(cpu_buffer,
+ rb_list_head(rb_list_head(list->prev)->next) != list))
+ return false;
+
+ return true;
+}
+
/**
* rb_check_pages - integrity check of buffer pages
* @cpu_buffer: CPU buffer with pages to test
*
* As a safety measure we check to make sure the data pages have not
* been corrupted.
- *
- * Callers of this function need to guarantee that the list of pages doesn't
get
- * modified during the check. In particular, if it's possible that the function
- * is invoked with concurrent readers which can swap in a new reader page then
- * the caller should take cpu_buffer->reader_lock.
*/
static void rb_check_pages(struct ring_buffer_per_cpu *cpu_buffer)
{
- struct list_head *head = rb_list_head(cpu_buffer->pages);
- struct list_head *tmp;
+ struct list_head *head, *tmp;
+ unsigned long flags;
+ size_t pages_read;
+ int nr_loops = 0;
- if (RB_WARN_ON(cpu_buffer,
- rb_list_head(rb_list_head(head->next)->prev) != head))
+ /*
+* Walk the linked list underpinning the ring buffer and validate all
+* its next and prev links.
+*
+* The check acquires the reader_lock in order to avoid concurrent
+* processing with a potential reader which can make the list
+* temporarily inconsistent when swapping its old reader page into the
+* buffer and obtaining a new page. However, the lock cannot be held for
+* the whole duration of the walk, as this would make the time when
+* interrupts are disabled non-deterministic, dependent on the ring
+* buffer size. Therefore, the code releases and re-acquires the lock
+* after checking each page. The pages_read variable is then used to
+* detect if a reader changed the list while the lock was not held, in
+* which case the check needs to be restarted.
+*
+* The code attempts to perform the check at most three times before
+* giving up. That itself is ok because this is only a self-validation
+* to detect problems early on. In practice, if it even happens that
+* this code runs concurrently with a reader getting a new page from the
+* buffer, it should take the reader a bit to process the obtained page
+* before coming back for more data and so this check typically succeeds
+* at most on the second try.
+*/
+again:
+ if (++nr_loops > 3)
return;
- if (RB_WARN_ON(cpu_buffer,
- rb_list_head(rb_list_head(head->prev)->next) != head))
- return;
+ raw_spin_lock_irqsave(_buffer->reader_lock, flags);
+ head = rb_list_head(cpu_buffer->pages);
+ if (!rb_check_links(cpu_buffer, head))
+ goto out_locked;
+ pages_read = local_read(_buffer->pages_read);
+ tmp = head;
+ raw_spin_unlock_irqrestore(_buffer->reader_lock, flags);
- for (tmp = rb_list_head(head->next); tmp != head; tmp =
rb_list_head(tmp->next)) {
- if (RB_WARN_ON(cpu_buffer,
- rb_list_head(rb_list_head(tmp->next)->prev) !=
tmp))
-
Re: [PATCH v9 1/8] remoteproc: qcom: Add PRNG proxy clock
On 21/06/2024 13:46, Gokul Sriram Palanisamy wrote:
>
> -static int q6v5_wcss_init_clock(struct q6v5_wcss *wcss)
> +static int ipq8074_init_clock(struct q6v5_wcss *wcss)
> +{
> + int ret;
> +
> + wcss->prng_clk = devm_clk_get(wcss->dev, "prng");
Missing binding.
Best regards,
Krzysztof
Re: [PATCH] qdisc: fix NULL pointer dereference in perf_trace_qdisc_reset()
Hi Pedro,
On 6/21/24 11:24 오후, Pedro Tammela wrote:
> On 21/06/2024 08:45, ysk...@gmail.com wrote:
>> From: Yunseong Kim
>>
>> In the TRACE_EVENT(qdisc_reset) NULL dereference occurred from
>>
>> qdisc->dev_queue->dev ->name
>>
>> This situation simulated from bunch of veths and Bluetooth
>> dis/reconnection.
>>
>> During qdisc initialization, qdisc was being set to noop_queue.
>> In veth_init_queue, the initial tx_num was reduced back to one,
>> causing the qdisc reset to be called with noop, which led to the
>> kernel panic.
>>
>> I think this will happen on the kernel version.
>> Linux kernel version ≥ v6.7.10, ≥ v6.8 ≥ v6.9 and 6.10
>
> You should tag your patch for the net tree
Thank you for the code review, I will tag the next patch for the net tree.
>> This occurred from 51270d573a8d. I think this patch is absolutely
>> necessary. Previously, It was showing not intended string value of name.
> Add a 'Fixes:' tag with this commit
I will added 'Fixes: 51270d573a8d' Tag on patch v2 message.
>> I can attach a sys-execprog's executing program, kernel dump and dmesg
>> if someone need it, but I'm not sure how to safely attach large vmcore
>> with vmlinux.
>
> The syzkaller program + C reproducer is usually enough, please make it
> visible somewhere
I got it, I have a converted C syz program. So, I've attached the GitHub
gist link and C source code in this mail.
https://gist.github.com/yskelg/cc64562873ce249cdd0d5a358b77d740
>> Signed-off-by: Yunseong Kim , Yeoreum Yun
>>
>
> Should be two SoB tags
Oh, It's the first time we've sent together, I made a mistake.. Sorry.
Thank you Pedro for the advice!
>> ---
>> include/trace/events/qdisc.h | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/trace/events/qdisc.h b/include/trace/events/qdisc.h
>> index f1b5e816e7e5..170b51fbe47a 100644
>> --- a/include/trace/events/qdisc.h
>> +++ b/include/trace/events/qdisc.h
>> @@ -81,7 +81,7 @@ TRACE_EVENT(qdisc_reset,
>> TP_ARGS(q),
>> TP_STRUCT__entry(
>> - __string( dev, qdisc_dev(q)->name )
>> + __string(dev, qdisc_dev(q) ? qdisc_dev(q)->name : "noop_queue")
>> __string( kind, q->ops->id )
>> __field( u32, parent )
>> __field( u32, handle )
>
Warm Regards,
Yunseong Kim// autogenerated by syzkaller (https://github.com/google/syzkaller)
#define _GNU_SOURCE
#include
#include
#include
#include
#include
#include
#include
#include
#ifndef __NR_add_key
#define __NR_add_key 217
#endif
#ifndef __NR_bpf
#define __NR_bpf 280
#endif
#ifndef __NR_io_uring_register
#define __NR_io_uring_register 427
#endif
#ifndef __NR_io_uring_setup
#define __NR_io_uring_setup 425
#endif
#ifndef __NR_keyctl
#define __NR_keyctl 219
#endif
#ifndef __NR_mlockall
#define __NR_mlockall 230
#endif
#ifndef __NR_mmap
#define __NR_mmap 222
#endif
#ifndef __NR_mremap
#define __NR_mremap 216
#endif
#ifndef __NR_munmap
#define __NR_munmap 215
#endif
#ifndef __NR_openat
#define __NR_openat 56
#endif
#ifndef __NR_read
#define __NR_read 63
#endif
#ifndef __NR_shmctl
#define __NR_shmctl 195
#endif
#define BITMASK(bf_off, bf_len) (((1ull << (bf_len)) - 1) << (bf_off))
#define STORE_BY_BITMASK(type, htobe, addr, val, bf_off, bf_len) \
*(type*)(addr) = \
htobe((htobe(*(type*)(addr)) & ~BITMASK((bf_off), (bf_len))) | \
(((type)(val) << (bf_off)) & BITMASK((bf_off), (bf_len
uint64_t r[7] = {0x,
0x,
0x,
0x0,
0x,
0x,
0x0};
int main(void)
{
syscall(__NR_mmap, /*addr=*/0x1000ul, /*len=*/0x1000ul, /*prot=*/0ul,
/*flags=MAP_FIXED|MAP_ANONYMOUS|MAP_PRIVATE*/ 0x32ul, /*fd=*/-1,
/*offset=*/0ul);
syscall(__NR_mmap, /*addr=*/0x2000ul, /*len=*/0x100ul,
/*prot=PROT_WRITE|PROT_READ|PROT_EXEC*/ 7ul,
/*flags=MAP_FIXED|MAP_ANONYMOUS|MAP_PRIVATE*/ 0x32ul, /*fd=*/-1,
/*offset=*/0ul);
syscall(__NR_mmap, /*addr=*/0x2100ul, /*len=*/0x1000ul, /*prot=*/0ul,
/*flags=MAP_FIXED|MAP_ANONYMOUS|MAP_PRIVATE*/ 0x32ul, /*fd=*/-1,
/*offset=*/0ul);
const char* reason;
(void)reason;
intptr_t res = 0;
if (write(1, "executing program\n", sizeof("executing program\n") - 1)) {
}
*(uint32_t*)0x2004 = 0;
*(uint32_t*)0x2008 = 0;
*(uint32_t*)0x200c = 0;
*(uint32_t*)0x2010 = 0;
*(uint32_t*)0x2018 = -1;
memset((void*)0x201c, 0, 12);
res =
syscall(__NR_io_uring_setup, /*entries=*/0xe68, /*params=*/0x2000ul);
if (res != -1)
r[0] = res;
memset((void*)0x2080, 111, 1);
syscall(__NR_io_uring_register, /*fd=*/r[0], /*opcode=*/0xaul,
/*arg=*/0x2080ul,
[PATCH 1/4] remoteproc: k3-r5: Fix IPC-only mode detection
ret variable was used to test reset status, get from
reset_control_status() call. But this variable was overwritten by
ti_sci_proc_get_status() a few lines bellow.
And as ti_sci_proc_get_status() returns 0 or a negative value (in this
latter case, followed by a return), the expression !ret was always true,
Clearly, this was not what was intended:
In the comment above it's said that "requires both local and module
resets to be deasserted"; if reset_control_status() returns 0 it means
that the reset line is deasserted.
So, it's pretty clear that the return value of reset_control_status()
was intended to be used instead of ti_sci_proc_get_status() return
value.
This could lead in an incorrect IPC-only mode detection if reset line is
asserted (so reset_control_status() return > 0) and c_state != 0 and
halted == 0.
In this case, the old code would have detected an IPC-only mode instead
of a mismatched mode.
Fixes: 1168af40b1ad ("remoteproc: k3-r5: Add support for IPC-only mode for all
R5Fs")
Signed-off-by: Richard Genoud
---
drivers/remoteproc/ti_k3_r5_remoteproc.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c
b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index 50e486bcfa10..39a47540c590 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -1144,6 +1144,7 @@ static int k3_r5_rproc_configure_mode(struct k3_r5_rproc
*kproc)
u32 atcm_enable, btcm_enable, loczrama;
struct k3_r5_core *core0;
enum cluster_mode mode = cluster->mode;
+ int reset_ctrl_status;
int ret;
core0 = list_first_entry(>cores, struct k3_r5_core, elem);
@@ -1160,11 +1161,11 @@ static int k3_r5_rproc_configure_mode(struct
k3_r5_rproc *kproc)
r_state, c_state);
}
- ret = reset_control_status(core->reset);
- if (ret < 0) {
+ reset_ctrl_status = reset_control_status(core->reset);
+ if (reset_ctrl_status < 0) {
dev_err(cdev, "failed to get initial local reset status, ret =
%d\n",
- ret);
- return ret;
+ reset_ctrl_status);
+ return reset_ctrl_status;
}
/*
@@ -1199,7 +1200,7 @@ static int k3_r5_rproc_configure_mode(struct k3_r5_rproc
*kproc)
* irrelevant if module reset is asserted (POR value has local reset
* deasserted), and is deemed as remoteproc mode
*/
- if (c_state && !ret && !halted) {
+ if (c_state && !reset_ctrl_status && !halted) {
dev_info(cdev, "configured R5F for IPC-only mode\n");
kproc->rproc->state = RPROC_DETACHED;
ret = 1;
@@ -1217,7 +1218,7 @@ static int k3_r5_rproc_configure_mode(struct k3_r5_rproc
*kproc)
ret = 0;
} else {
dev_err(cdev, "mismatched mode: local_reset = %s, module_reset
= %s, core_state = %s\n",
- !ret ? "deasserted" : "asserted",
+ !reset_ctrl_status ? "deasserted" : "asserted",
c_state ? "deasserted" : "asserted",
halted ? "halted" : "unhalted");
ret = -EINVAL;
[PATCH 4/4] remoteproc: k3-r5: support for graceful stop of remote cores
Introduce software IPC handshake between the K3-R5 remote proc driver
and the R5 MCU to gracefully stop/reset the remote core.
Upon a stop request, K3-R5 remote proc driver sends a RP_MBOX_SHUTDOWN
mailbox message to the remote R5 core.
The remote core is expected to:
- relinquish all the resources acquired through Device Manager (DM)
- disable its interrupts
- send back a mailbox acknowledgment RP_MBOX_SHUDOWN_ACK
- enter WFI state.
Meanwhile, the K3-R5 remote proc driver does:
- wait for the RP_MBOX_SHUTDOWN_ACK from the remote core
- wait for the remote proc to enter WFI state
- reset the remote core through device manager
Based on work from: Hari Nagalla
Signed-off-by: Richard Genoud
---
drivers/remoteproc/omap_remoteproc.h | 9 +-
drivers/remoteproc/ti_k3_r5_remoteproc.c | 40
2 files changed, 48 insertions(+), 1 deletion(-)
diff --git a/drivers/remoteproc/omap_remoteproc.h
b/drivers/remoteproc/omap_remoteproc.h
index 828e13256c02..c008f11fa2a4 100644
--- a/drivers/remoteproc/omap_remoteproc.h
+++ b/drivers/remoteproc/omap_remoteproc.h
@@ -42,6 +42,11 @@
* @RP_MBOX_SUSPEND_CANCEL: a cancel suspend response from a remote processor
* on a suspend request
*
+ * @RP_MBOX_SHUTDOWN: shutdown request for the remote processor
+ *
+ * @RP_MBOX_SHUTDOWN_ACK: successful response from remote processor for a
+ * shutdown request. The remote processor should be in WFI state short after.
+ *
* Introduce new message definitions if any here.
*
* @RP_MBOX_END_MSG: Indicates end of known/defined messages from remote core
@@ -59,7 +64,9 @@ enum omap_rp_mbox_messages {
RP_MBOX_SUSPEND_SYSTEM = 0xFF11,
RP_MBOX_SUSPEND_ACK = 0xFF12,
RP_MBOX_SUSPEND_CANCEL = 0xFF13,
- RP_MBOX_END_MSG = 0xFF14,
+ RP_MBOX_SHUTDOWN= 0xFF14,
+ RP_MBOX_SHUTDOWN_ACK= 0xFF15,
+ RP_MBOX_END_MSG = 0xFF16,
};
#endif /* _OMAP_RPMSG_H */
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c
b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index a2ead87952c7..918a15e1dd9a 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -21,6 +21,7 @@
#include
#include
#include
+#include
#include
#include
@@ -172,8 +173,23 @@ struct k3_r5_rproc {
struct k3_r5_core *core;
struct k3_r5_mem *rmem;
int num_rmems;
+ struct completion shutdown_complete;
};
+/*
+ * This will return true if the remote core is in Wait For Interrupt state.
+ */
+static bool k3_r5_is_core_in_wfi(struct k3_r5_core *core)
+{
+ int ret;
+ u64 boot_vec;
+ u32 cfg, ctrl, stat;
+
+ ret = ti_sci_proc_get_status(core->tsp, _vec, , , );
+
+ return !ret ? !!(stat & PROC_BOOT_STATUS_FLAG_R5_WFI) : false;
+}
+
/**
* k3_r5_rproc_mbox_callback() - inbound mailbox message handler
* @client: mailbox client pointer used for requesting the mailbox channel
@@ -209,6 +225,10 @@ static void k3_r5_rproc_mbox_callback(struct mbox_client
*client, void *data)
case RP_MBOX_ECHO_REPLY:
dev_info(dev, "received echo reply from %s\n", name);
break;
+ case RP_MBOX_SHUTDOWN_ACK:
+ dev_dbg(dev, "received shutdown_ack from %s\n", name);
+ complete(>shutdown_complete);
+ break;
default:
/* silently handle all other valid messages */
if (msg >= RP_MBOX_READY && msg < RP_MBOX_END_MSG)
@@ -634,6 +654,7 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
struct k3_r5_cluster *cluster = kproc->cluster;
struct device *dev = kproc->dev;
struct k3_r5_core *core1, *core = kproc->core;
+ bool wfi;
int ret;
@@ -650,6 +671,24 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
}
}
+ /* Send SHUTDOWN message to remote proc */
+ reinit_completion(>shutdown_complete);
+ ret = mbox_send_message(kproc->mbox, (void *)RP_MBOX_SHUTDOWN);
+ if (ret < 0) {
+ dev_err(dev, "Sending SHUTDOWN message failed: %d. Halting core
anyway.\n", ret);
+ } else {
+ ret = wait_for_completion_timeout(>shutdown_complete,
+ msecs_to_jiffies(1000));
+ if (ret == 0) {
+ dev_err(dev, "Timeout waiting SHUTDOWN_ACK message.
Halting core anyway.\n");
+ } else {
+ ret = readx_poll_timeout(k3_r5_is_core_in_wfi, core,
+wfi, wfi, 200, 2000);
+ if (ret)
+ dev_err(dev, "Timeout waiting for remote proc
to be in WFI state. Halting core anyway.\n");
+ }
+ }
+
/* halt all applicable cores */
if (cluster->mode == CLUSTER_MODE_LOCKSTEP) {
list_for_each_entry(core, >cores,
[PATCH 3/4] remoteproc: k3-r5: k3_r5_rproc_stop: code reorder
In the next commit, a RP_MBOX_SHUTDOWN message will be sent in
k3_r5_rproc_stop() to the remote proc (in lockstep on not)
Thus, the sanity check "do not allow core 0 to stop before core 1"
should be moved at the beginning of the function so that the generic case
can be dealt with.
In order to have an easier patch to review, those actions are broke in
two patches:
- this patch: moving the sanity check at the beginning (No functional
change).
- next patch: doing the real job (sending shutdown messages to remote
procs before halting them).
Basically, we had:
- cluster_mode actions
- !cluster_mode sanity check
- !cluster_mode actions
And now:
- !cluster_mode sanity check
- cluster_mode actions
- !cluster_mode actions
Signed-off-by: Richard Genoud
---
drivers/remoteproc/ti_k3_r5_remoteproc.c | 24 ++--
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c
b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index 1f18b08618c8..a2ead87952c7 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -636,16 +636,8 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
struct k3_r5_core *core1, *core = kproc->core;
int ret;
- /* halt all applicable cores */
- if (cluster->mode == CLUSTER_MODE_LOCKSTEP) {
- list_for_each_entry(core, >cores, elem) {
- ret = k3_r5_core_halt(core);
- if (ret) {
- core = list_prev_entry(core, elem);
- goto unroll_core_halt;
- }
- }
- } else {
+
+ if (cluster->mode != CLUSTER_MODE_LOCKSTEP) {
/* do not allow core 0 to stop before core 1 */
core1 = list_last_entry(>cores, struct k3_r5_core,
elem);
@@ -656,6 +648,18 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
ret = -EPERM;
goto out;
}
+ }
+
+ /* halt all applicable cores */
+ if (cluster->mode == CLUSTER_MODE_LOCKSTEP) {
+ list_for_each_entry(core, >cores, elem) {
+ ret = k3_r5_core_halt(core);
+ if (ret) {
+ core = list_prev_entry(core, elem);
+ goto unroll_core_halt;
+ }
+ }
+ } else {
ret = k3_r5_core_halt(core);
if (ret)
[PATCH 2/4] remoteproc: k3-r5: Introduce PM suspend/resume handlers
This patch adds the support for system suspend/resume to the ti_k3_R5
remoteproc driver.
In order to save maximum power, the approach here is to shutdown
completely the cores that were started by the kernel (i.e. those in
RUNNING state).
Those which were started before the kernel (in attached mode) will be
detached.
The pm_notifier mechanism is used here because the remote procs firmwares
have to be reloaded at resume, and thus the driver must have access to
the file system were the firmware is stored.
On suspend, the running remote procs are stopped, the attached remote
procs are detached and processor control released.
On resume, the reverse operation is done.
Based on work from: Hari Nagalla
Signed-off-by: Richard Genoud
---
drivers/remoteproc/ti_k3_r5_remoteproc.c | 123 ++-
1 file changed, 121 insertions(+), 2 deletions(-)
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c
b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index 39a47540c590..1f18b08618c8 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#include
#include
@@ -112,6 +113,7 @@ struct k3_r5_cluster {
struct list_head cores;
wait_queue_head_t core_transition;
const struct k3_r5_soc_data *soc_data;
+ struct notifier_block pm_notifier;
};
/**
@@ -577,7 +579,8 @@ static int k3_r5_rproc_start(struct rproc *rproc)
/* do not allow core 1 to start before core 0 */
core0 = list_first_entry(>cores, struct k3_r5_core,
elem);
- if (core != core0 && core0->rproc->state == RPROC_OFFLINE) {
+ if (core != core0 && (core0->rproc->state == RPROC_OFFLINE ||
+ core0->rproc->state == RPROC_SUSPENDED)) {
dev_err(dev, "%s: can not start core 1 before core 0\n",
__func__);
ret = -EPERM;
@@ -646,7 +649,8 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
/* do not allow core 0 to stop before core 1 */
core1 = list_last_entry(>cores, struct k3_r5_core,
elem);
- if (core != core1 && core1->rproc->state != RPROC_OFFLINE) {
+ if (core != core1 && core1->rproc->state != RPROC_OFFLINE &&
+ core1->rproc->state != RPROC_SUSPENDED) {
dev_err(dev, "%s: can not stop core 0 before core 1\n",
__func__);
ret = -EPERM;
@@ -1238,6 +1242,117 @@ static int k3_r5_rproc_configure_mode(struct
k3_r5_rproc *kproc)
return ret;
}
+static int k3_r5_rproc_suspend(struct k3_r5_rproc *kproc)
+{
+ unsigned int rproc_state = kproc->rproc->state;
+ int ret;
+
+ if (rproc_state != RPROC_RUNNING && rproc_state != RPROC_ATTACHED)
+ return 0;
+
+ if (rproc_state == RPROC_RUNNING)
+ ret = rproc_shutdown(kproc->rproc);
+ else
+ ret = rproc_detach(kproc->rproc);
+
+ if (ret) {
+ dev_err(kproc->dev, "Failed to %s rproc (%d)\n",
+ (rproc_state == RPROC_RUNNING) ? "shutdown" : "detach",
+ ret);
+ return ret;
+ }
+
+ kproc->rproc->state = RPROC_SUSPENDED;
+
+ return ret;
+}
+
+static int k3_r5_rproc_resume(struct k3_r5_rproc *kproc)
+{
+ int ret;
+
+ if (kproc->rproc->state != RPROC_SUSPENDED)
+ return 0;
+
+ ret = k3_r5_rproc_configure_mode(kproc);
+ if (ret < 0)
+ return -EBUSY;
+
+ /*
+* ret > 0 for IPC-only mode
+* ret == 0 for remote proc mode
+*/
+ if (ret == 0) {
+ /*
+* remote proc looses its configuration when powered off.
+* So, we have to configure it again on resume.
+*/
+ ret = k3_r5_rproc_configure(kproc);
+ if (ret < 0) {
+ dev_err(kproc->dev, "k3_r5_rproc_configure failed
(%d)\n", ret);
+ return -EBUSY;
+ }
+ }
+
+ return rproc_boot(kproc->rproc);
+}
+
+static int k3_r5_cluster_pm_notifier_call(struct notifier_block *bl,
+ unsigned long state, void *unused)
+{
+ struct k3_r5_cluster *cluster = container_of(bl, struct k3_r5_cluster,
+pm_notifier);
+ struct k3_r5_core *core;
+ int ret;
+
+ switch (state) {
+ case PM_HIBERNATION_PREPARE:
+ case PM_RESTORE_PREPARE:
+ case PM_SUSPEND_PREPARE:
+ /* core1 should be suspended before core0 */
+ list_for_each_entry_reverse(core, >cores, elem) {
+ /*
+
[PATCH 0/4] remoteproc: k3-r5: Introduce suspend to ram support
This series enables the suspend to ram with R5F remote processors on TI K3 platform. The 1st patch is actually a fix, independent from the others The 2nd patch introduces the suspend/resume handlers. On suspend, the running rprocs will be stopped (or detached) and then re-loaded in resume. The logic behind this is: - shutdown the cores that Linux started to save power in suspend. - detach the cores that were started before Linux. Then, the 3rd and 4th patches introduce the graceful shutdown of remote procs. This will give them a chance to release resources and shut down in a civilized manner. Without this series, the suspend fails with: omap-mailbox 31f81000.mailbox: fifo 1 has unexpected unread messages omap-mailbox 31f81000.mailbox: PM: dpm_run_callback(): platform_pm_suspend returns -16 omap-mailbox 31f81000.mailbox: PM: platform_pm_suspend returned -16 after 16328 usecs omap-mailbox 31f81000.mailbox: PM: failed to suspend: error -16 Patches 2 and 4 are based on work from Hari Nagalla . @Hari, please feel free to add your Co-developed-by:/Signed-off-by: Tested on J7200X SoM Series is based on v6.10-rc4 Richard Genoud (4): remoteproc: k3-r5: Fix IPC-only mode detection remoteproc: k3-r5: Introduce PM suspend/resume handlers remoteproc: k3-r5: k3_r5_rproc_stop: code reorder remoteproc: k3-r5: support for graceful stop of remote cores drivers/remoteproc/omap_remoteproc.h | 9 +- drivers/remoteproc/ti_k3_r5_remoteproc.c | 196 +-- 2 files changed, 188 insertions(+), 17 deletions(-)
[PATCH v8 0/5] Introduction of a remoteproc tee to load signed firmware
Main updates from version V7[1]
Update the series based on Mathieu Poirier's comments.
Details of the updates are listed in the commit messages of the patches.
[1]
https://lore.kernel.org/linux-arm-kernel/20240611073904.475019-1-arnaud.pouliq...@foss.st.com/
base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0
Description of the feature:
--
This series proposes the implementation of a remoteproc tee driver to
communicate with a TEE trusted application responsible for authenticating
and loading the remoteproc firmware image in an Arm secure context.
1) Principle:
The remoteproc tee driver provides services to communicate with the OP-TEE
trusted application running on the Trusted Execution Context (TEE).
The trusted application in TEE manages the remote processor lifecycle:
- authenticating and loading firmware images,
- isolating and securing the remote processor memories,
- supporting multi-firmware (e.g., TF-M + Zephyr on a Cortex-M33),
- managing the start and stop of the firmware by the TEE.
2) Format of the signed image:
Refer to:
https://github.com/OP-TEE/optee_os/blob/master/ta/remoteproc/src/remoteproc_core.c#L18-L57
3) OP-TEE trusted application API:
Refer to:
https://github.com/OP-TEE/optee_os/blob/master/ta/remoteproc/include/ta_remoteproc.h
4) OP-TEE signature script
Refer to:
https://github.com/OP-TEE/optee_os/blob/master/scripts/sign_rproc_fw.py
Example of usage:
sign_rproc_fw.py --in --in --out --key
${OP-TEE_PATH}/keys/default.pem
5) Impact on User space Application
No sysfs impact.the user only needs to provide the signed firmware image
instead of the ELF image.
For more information about the implementation, a presentation is available here
(note that the format of the signed image has evolved between the presentation
and the integration in OP-TEE).
https://resources.linaro.org/en/resource/6c5bGvZwUAjX56fvxthxds
Arnaud Pouliquen (5):
remoteproc: core: Introduce rproc_pa_to_va helper
remoteproc: Add TEE support
dt-bindings: remoteproc: Add compatibility for TEE support
remoteproc: stm32: Create sub-functions to request shutdown and
release
remoteproc: stm32: Add support of an OP-TEE TA to load the firmware
.../bindings/remoteproc/st,stm32-rproc.yaml | 58 ++-
drivers/remoteproc/Kconfig| 10 +
drivers/remoteproc/Makefile | 1 +
drivers/remoteproc/remoteproc_core.c | 46 ++
drivers/remoteproc/remoteproc_tee.c | 446 ++
drivers/remoteproc/stm32_rproc.c | 147 --
include/linux/remoteproc.h| 5 +
include/linux/remoteproc_tee.h| 100
8 files changed, 769 insertions(+), 44 deletions(-)
create mode 100644 drivers/remoteproc/remoteproc_tee.c
create mode 100644 include/linux/remoteproc_tee.h
base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0
--
2.25.1
[PATCH v8 3/5] dt-bindings: remoteproc: Add compatibility for TEE support
The "st,stm32mp1-m4-tee" compatible is utilized in a system configuration
where the Cortex-M4 firmware is loaded by the Trusted Execution Environment
(TEE).
For instance, this compatible is used in both the Linux and OP-TEE device
trees:
- In OP-TEE, a node is defined in the device tree with the
"st,stm32mp1-m4-tee" compatible to support signed remoteproc firmware.
Based on DT properties, the OP-TEE remoteproc framework is initiated to
expose a trusted application service to authenticate and load the remote
processor firmware provided by the Linux remoteproc framework, as well
as to start and stop the remote processor.
- In Linux, when the compatibility is set, the Cortex-M resets should not
be declared in the device tree. In such a configuration, the reset is
managed by the OP-TEE remoteproc driver and is no longer accessible from
the Linux kernel.
Associated with this new compatible, add the "st,proc-id" property to
identify the remote processor. This ID is used to define a unique ID,
common between Linux, U-Boot, and OP-TEE, to identify a coprocessor.
This ID will be used in requests to the OP-TEE remoteproc Trusted
Application to specify the remote processor.
Signed-off-by: Arnaud Pouliquen
Reviewed-by: Rob Herring (Arm)
---
.../bindings/remoteproc/st,stm32-rproc.yaml | 58 ---
1 file changed, 50 insertions(+), 8 deletions(-)
diff --git a/Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.yaml
b/Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.yaml
index 370af61d8f28..409123cd4667 100644
--- a/Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.yaml
@@ -16,7 +16,12 @@ maintainers:
properties:
compatible:
-const: st,stm32mp1-m4
+enum:
+ - st,stm32mp1-m4
+ - st,stm32mp1-m4-tee
+description:
+ Use "st,stm32mp1-m4" for the Cortex-M4 coprocessor management by
non-secure context
+ Use "st,stm32mp1-m4-tee" for the Cortex-M4 coprocessor management by
secure context
reg:
description:
@@ -43,6 +48,10 @@ properties:
- description: The offset of the hold boot setting register
- description: The field mask of the hold boot
+ st,proc-id:
+description: remote processor identifier
+$ref: /schemas/types.yaml#/definitions/uint32
+
st,syscfg-tz:
deprecated: true
description:
@@ -142,21 +151,43 @@ properties:
required:
- compatible
- reg
- - resets
allOf:
- if:
properties:
-reset-names:
- not:
-contains:
- const: hold_boot
+compatible:
+ contains:
+const: st,stm32mp1-m4
then:
+ if:
+properties:
+ reset-names:
+not:
+ contains:
+const: hold_boot
+ then:
+required:
+ - st,syscfg-holdboot
+ else:
+properties:
+ st,syscfg-holdboot: false
+required:
+ - reset-names
required:
-- st,syscfg-holdboot
-else:
+- resets
+
+ - if:
+ properties:
+compatible:
+ contains:
+const: st,stm32mp1-m4-tee
+then:
properties:
st,syscfg-holdboot: false
+reset-names: false
+resets: false
+ required:
+- st,proc-id
additionalProperties: false
@@ -188,5 +219,16 @@ examples:
st,syscfg-rsc-tbl = < 0x144 0x>;
st,syscfg-m4-state = < 0x148 0x>;
};
+ - |
+#include
+m4@1000 {
+ compatible = "st,stm32mp1-m4-tee";
+ reg = <0x1000 0x4>,
+<0x3000 0x4>,
+<0x3800 0x1>;
+ st,proc-id = <0>;
+ st,syscfg-rsc-tbl = < 0x144 0x>;
+ st,syscfg-m4-state = < 0x148 0x>;
+};
...
--
2.25.1
[PATCH v8 4/5] remoteproc: stm32: Create sub-functions to request shutdown and release
To prepare for the support of TEE remoteproc, create sub-functions
that can be used in both cases, with and without remoteproc TEE support.
Signed-off-by: Arnaud Pouliquen
---
drivers/remoteproc/stm32_rproc.c | 84 +++-
1 file changed, 51 insertions(+), 33 deletions(-)
diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
index 88623df7d0c3..8cd838df4e92 100644
--- a/drivers/remoteproc/stm32_rproc.c
+++ b/drivers/remoteproc/stm32_rproc.c
@@ -209,6 +209,54 @@ static int stm32_rproc_mbox_idx(struct rproc *rproc, const
unsigned char *name)
return -EINVAL;
}
+static void stm32_rproc_request_shutdown(struct rproc *rproc)
+{
+ struct stm32_rproc *ddata = rproc->priv;
+ int err, dummy_data, idx;
+
+ /* Request shutdown of the remote processor */
+ if (rproc->state != RPROC_OFFLINE && rproc->state != RPROC_CRASHED) {
+ idx = stm32_rproc_mbox_idx(rproc, STM32_MBX_SHUTDOWN);
+ if (idx >= 0 && ddata->mb[idx].chan) {
+ /* A dummy data is sent to allow to block on transmit.
*/
+ err = mbox_send_message(ddata->mb[idx].chan,
+ _data);
+ if (err < 0)
+ dev_warn(>dev, "warning: remote FW
shutdown without ack\n");
+ }
+ }
+}
+
+static int stm32_rproc_release(struct rproc *rproc)
+{
+ struct stm32_rproc *ddata = rproc->priv;
+ unsigned int err = 0;
+
+ /* To allow platform Standby power mode, set remote proc Deep Sleep. */
+ if (ddata->pdds.map) {
+ err = regmap_update_bits(ddata->pdds.map, ddata->pdds.reg,
+ddata->pdds.mask, 1);
+ if (err) {
+ dev_err(>dev, "failed to set pdds\n");
+ return err;
+ }
+ }
+
+ /* Update coprocessor state to OFF if available. */
+ if (ddata->m4_state.map) {
+ err = regmap_update_bits(ddata->m4_state.map,
+ddata->m4_state.reg,
+ddata->m4_state.mask,
+M4_STATE_OFF);
+ if (err) {
+ dev_err(>dev, "failed to set copro state\n");
+ return err;
+ }
+ }
+
+ return 0;
+}
+
static int stm32_rproc_prepare(struct rproc *rproc)
{
struct device *dev = rproc->dev.parent;
@@ -519,17 +567,9 @@ static int stm32_rproc_detach(struct rproc *rproc)
static int stm32_rproc_stop(struct rproc *rproc)
{
struct stm32_rproc *ddata = rproc->priv;
- int err, idx;
+ int err;
- /* request shutdown of the remote processor */
- if (rproc->state != RPROC_OFFLINE && rproc->state != RPROC_CRASHED) {
- idx = stm32_rproc_mbox_idx(rproc, STM32_MBX_SHUTDOWN);
- if (idx >= 0 && ddata->mb[idx].chan) {
- err = mbox_send_message(ddata->mb[idx].chan, "detach");
- if (err < 0)
- dev_warn(>dev, "warning: remote FW
shutdown without ack\n");
- }
- }
+ stm32_rproc_request_shutdown(rproc);
err = stm32_rproc_set_hold_boot(rproc, true);
if (err)
@@ -541,29 +581,7 @@ static int stm32_rproc_stop(struct rproc *rproc)
return err;
}
- /* to allow platform Standby power mode, set remote proc Deep Sleep */
- if (ddata->pdds.map) {
- err = regmap_update_bits(ddata->pdds.map, ddata->pdds.reg,
-ddata->pdds.mask, 1);
- if (err) {
- dev_err(>dev, "failed to set pdds\n");
- return err;
- }
- }
-
- /* update coprocessor state to OFF if available */
- if (ddata->m4_state.map) {
- err = regmap_update_bits(ddata->m4_state.map,
-ddata->m4_state.reg,
-ddata->m4_state.mask,
-M4_STATE_OFF);
- if (err) {
- dev_err(>dev, "failed to set copro state\n");
- return err;
- }
- }
-
- return 0;
+ return stm32_rproc_release(rproc);
}
static void stm32_rproc_kick(struct rproc *rproc, int vqid)
--
2.25.1
[PATCH v8 2/5] remoteproc: Add TEE support
Add a remoteproc TEE (Trusted Execution Environment) driver
that will be probed by the TEE bus. If the associated Trusted
application is supported on secure part this driver offers a client
interface to load a firmware by the secure part.
This firmware could be authenticated by the secure trusted application.
Signed-off-by: Arnaud Pouliquen
---
Updates vs previous version:
- rename tee_remoteproc.* file to rmeoteproc_tee.*,
- rename TEE_REMOTEPROC config to REMOTEPROC_TEE,
- remove "stm32" in some variable declarations,
- remove useless "remoteproc_internal.h" include,
- fix tee_rproc_ctx devm_kzalloc.
- rework module description
---
drivers/remoteproc/Kconfig | 10 +
drivers/remoteproc/Makefile | 1 +
drivers/remoteproc/remoteproc_tee.c | 446
include/linux/remoteproc.h | 4 +
include/linux/remoteproc_tee.h | 100 +++
5 files changed, 561 insertions(+)
create mode 100644 drivers/remoteproc/remoteproc_tee.c
create mode 100644 include/linux/remoteproc_tee.h
diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 48845dc8fa85..278f197acb90 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -365,6 +365,16 @@ config XLNX_R5_REMOTEPROC
It's safe to say N if not interested in using RPU r5f cores.
+
+config REMOTEPROC_TEE
+ tristate "Remoteproc support by a TEE application"
+ depends on OPTEE
+ help
+ Support a remote processor with a TEE application. The Trusted
+ Execution Context is responsible for loading the trusted firmware
+ image and managing the remote processor's lifecycle.
+ This can be either built-in or a loadable module.
+
endif # REMOTEPROC
endmenu
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index 91314a9b43ce..b4eb37177005 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -36,6 +36,7 @@ obj-$(CONFIG_RCAR_REMOTEPROC) += rcar_rproc.o
obj-$(CONFIG_ST_REMOTEPROC)+= st_remoteproc.o
obj-$(CONFIG_ST_SLIM_REMOTEPROC) += st_slim_rproc.o
obj-$(CONFIG_STM32_RPROC) += stm32_rproc.o
+obj-$(CONFIG_REMOTEPROC_TEE) += remoteproc_tee.o
obj-$(CONFIG_TI_K3_DSP_REMOTEPROC) += ti_k3_dsp_remoteproc.o
obj-$(CONFIG_TI_K3_R5_REMOTEPROC) += ti_k3_r5_remoteproc.o
obj-$(CONFIG_XLNX_R5_REMOTEPROC) += xlnx_r5_remoteproc.o
diff --git a/drivers/remoteproc/remoteproc_tee.c
b/drivers/remoteproc/remoteproc_tee.c
new file mode 100644
index ..70cb67451767
--- /dev/null
+++ b/drivers/remoteproc/remoteproc_tee.c
@@ -0,0 +1,446 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) STMicroelectronics 2024
+ * Author: Arnaud Pouliquen
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define MAX_TEE_PARAM_ARRY_MEMBER 4
+
+/*
+ * Authentication of the firmware and load in the remote processor memory
+ *
+ * [in] params[0].value.a:unique 32bit identifier of the remote processor
+ * [in] params[1].memref: buffer containing the image of the
buffer
+ */
+#define TA_RPROC_FW_CMD_LOAD_FW1
+
+/*
+ * Start the remote processor
+ *
+ * [in] params[0].value.a:unique 32bit identifier of the remote processor
+ */
+#define TA_RPROC_FW_CMD_START_FW 2
+
+/*
+ * Stop the remote processor
+ *
+ * [in] params[0].value.a:unique 32bit identifier of the remote processor
+ */
+#define TA_RPROC_FW_CMD_STOP_FW3
+
+/*
+ * Return the address of the resource table, or 0 if not found
+ * No check is done to verify that the address returned is accessible by
+ * the non secure context. If the resource table is loaded in a protected
+ * memory the access by the non secure context will lead to a data abort.
+ *
+ * [in] params[0].value.a:unique 32bit identifier of the remote processor
+ * [out] params[1].value.a: 32bit LSB resource table memory address
+ * [out] params[1].value.b: 32bit MSB resource table memory address
+ * [out] params[2].value.a: 32bit LSB resource table memory size
+ * [out] params[2].value.b: 32bit MSB resource table memory size
+ */
+#define TA_RPROC_FW_CMD_GET_RSC_TABLE 4
+
+/*
+ * Return the address of the core dump
+ *
+ * [in] params[0].value.a:unique 32bit identifier of the remote processor
+ * [out] params[1].memref: address of the core dump image if exist,
+ * else return Null
+ */
+#define TA_RPROC_FW_CMD_GET_COREDUMP 5
+
+struct tee_rproc_context {
+ struct list_head sessions;
+ struct tee_context *tee_ctx;
+ struct device *dev;
+};
+
+static struct tee_rproc_context *tee_rproc_ctx;
+
+static void tee_rproc_prepare_args(struct tee_rproc *trproc, int cmd,
+ struct tee_ioctl_invoke_arg *arg,
+ struct tee_param *param,
+
[PATCH v8 1/5] remoteproc: core: Introduce rproc_pa_to_va helper
When a resource table is loaded by an external entity such as U-boot or
OP-TEE, we do not necessarily get the device address(da) but the physical
address(pa).
This helper performs similar translation than the rproc_da_to_va()
but based on a physical address.
Signed-off-by: Arnaud Pouliquen
---
updates vs previous version:
- remove pa_to_va in the rproc_ops structure as not used
- simplify the rproc_pa_to_va() documentation header
- fix typos
---
drivers/remoteproc/remoteproc_core.c | 46
include/linux/remoteproc.h | 1 +
2 files changed, 47 insertions(+)
diff --git a/drivers/remoteproc/remoteproc_core.c
b/drivers/remoteproc/remoteproc_core.c
index f276956f2c5c..ace11ea17097 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -230,6 +230,52 @@ void *rproc_da_to_va(struct rproc *rproc, u64 da, size_t
len, bool *is_iomem)
}
EXPORT_SYMBOL(rproc_da_to_va);
+/**
+ * rproc_pa_to_va() - lookup the kernel virtual address for a physical address
of a remoteproc
+ * memory
+ *
+ * @rproc: handle of a remote processor
+ * @pa: remoteproc physical address
+ * @len: length of the memory region @pa is pointing to
+ * @is_iomem: optional pointer filled in to indicate if @da is iomapped memory
+ *
+ * This function is a helper function similar to rproc_da_to_va() but it deals
with physical
+ * addresses instead of device addresses.
+ *
+ * Return: a valid kernel address on success or NULL on failure
+ */
+void *rproc_pa_to_va(struct rproc *rproc, phys_addr_t pa, size_t len, bool
*is_iomem)
+{
+ struct rproc_mem_entry *carveout;
+ void *ptr = NULL;
+
+ list_for_each_entry(carveout, >carveouts, node) {
+ int offset = pa - carveout->dma;
+
+ /* Verify that carveout is allocated */
+ if (!carveout->va)
+ continue;
+
+ /* try next carveout if da is too small */
+ if (offset < 0)
+ continue;
+
+ /* try next carveout if da is too large */
+ if (offset + len > carveout->len)
+ continue;
+
+ ptr = carveout->va + offset;
+
+ if (is_iomem)
+ *is_iomem = carveout->is_iomem;
+
+ break;
+ }
+
+ return ptr;
+}
+EXPORT_SYMBOL(rproc_pa_to_va);
+
/**
* rproc_find_carveout_by_name() - lookup the carveout region by a name
* @rproc: handle of a remote processor
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index b4795698d8c2..8fd0d7f63c8e 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -690,6 +690,7 @@ int rproc_detach(struct rproc *rproc);
int rproc_set_firmware(struct rproc *rproc, const char *fw_name);
void rproc_report_crash(struct rproc *rproc, enum rproc_crash_type type);
void *rproc_da_to_va(struct rproc *rproc, u64 da, size_t len, bool *is_iomem);
+void *rproc_pa_to_va(struct rproc *rproc, phys_addr_t pa, size_t len, bool
*is_iomem);
/* from remoteproc_coredump.c */
void rproc_coredump_cleanup(struct rproc *rproc);
--
2.25.1
[PATCH v8 5/5] remoteproc: stm32: Add support of an OP-TEE TA to load the firmware
The new TEE remoteproc device is used to manage remote firmware in a
secure, trusted context. The 'st,stm32mp1-m4-tee' compatibility is
introduced to delegate the loading of the firmware to the trusted
execution context. In such cases, the firmware should be signed and
adhere to the image format defined by the TEE.
Signed-off-by: Arnaud Pouliquen
---
drivers/remoteproc/stm32_rproc.c | 63 ++--
1 file changed, 60 insertions(+), 3 deletions(-)
diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
index 8cd838df4e92..fd905b3cf206 100644
--- a/drivers/remoteproc/stm32_rproc.c
+++ b/drivers/remoteproc/stm32_rproc.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -257,6 +258,19 @@ static int stm32_rproc_release(struct rproc *rproc)
return 0;
}
+static int stm32_rproc_tee_stop(struct rproc *rproc)
+{
+ int err;
+
+ stm32_rproc_request_shutdown(rproc);
+
+ err = tee_rproc_stop(rproc);
+ if (err)
+ return err;
+
+ return stm32_rproc_release(rproc);
+}
+
static int stm32_rproc_prepare(struct rproc *rproc)
{
struct device *dev = rproc->dev.parent;
@@ -693,8 +707,20 @@ static const struct rproc_ops st_rproc_ops = {
.get_boot_addr = rproc_elf_get_boot_addr,
};
+static const struct rproc_ops st_rproc_tee_ops = {
+ .prepare= stm32_rproc_prepare,
+ .start = tee_rproc_start,
+ .stop = stm32_rproc_tee_stop,
+ .kick = stm32_rproc_kick,
+ .load = tee_rproc_load_fw,
+ .parse_fw = tee_rproc_parse_fw,
+ .find_loaded_rsc_table = tee_rproc_find_loaded_rsc_table,
+
+};
+
static const struct of_device_id stm32_rproc_match[] = {
{ .compatible = "st,stm32mp1-m4" },
+ { .compatible = "st,stm32mp1-m4-tee" },
{},
};
MODULE_DEVICE_TABLE(of, stm32_rproc_match);
@@ -853,17 +879,42 @@ static int stm32_rproc_probe(struct platform_device *pdev)
struct device *dev = >dev;
struct stm32_rproc *ddata;
struct device_node *np = dev->of_node;
+ struct tee_rproc *trproc = NULL;
struct rproc *rproc;
unsigned int state;
+ u32 proc_id;
int ret;
ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32));
if (ret)
return ret;
- rproc = devm_rproc_alloc(dev, np->name, _rproc_ops, NULL,
sizeof(*ddata));
- if (!rproc)
- return -ENOMEM;
+ if (of_device_is_compatible(np, "st,stm32mp1-m4-tee")) {
+ /*
+* Delegate the firmware management to the secure context.
+* The firmware loaded has to be signed.
+*/
+ ret = of_property_read_u32(np, "st,proc-id", _id);
+ if (ret) {
+ dev_err(dev, "failed to read st,rproc-id property\n");
+ return ret;
+ }
+
+ rproc = devm_rproc_alloc(dev, np->name, _rproc_tee_ops,
NULL, sizeof(*ddata));
+ if (!rproc)
+ return -ENOMEM;
+
+ trproc = tee_rproc_register(dev, rproc, proc_id);
+ if (IS_ERR(trproc)) {
+ dev_err_probe(dev, PTR_ERR(trproc),
+ "signed firmware not supported by TEE\n");
+ return PTR_ERR(trproc);
+ }
+ } else {
+ rproc = devm_rproc_alloc(dev, np->name, _rproc_ops, NULL,
sizeof(*ddata));
+ if (!rproc)
+ return -ENOMEM;
+ }
ddata = rproc->priv;
@@ -915,6 +966,9 @@ static int stm32_rproc_probe(struct platform_device *pdev)
dev_pm_clear_wake_irq(dev);
device_init_wakeup(dev, false);
}
+ if (trproc)
+ tee_rproc_unregister(trproc);
+
return ret;
}
@@ -935,6 +989,9 @@ static void stm32_rproc_remove(struct platform_device *pdev)
dev_pm_clear_wake_irq(dev);
device_init_wakeup(dev, false);
}
+ if (rproc->tee_interface)
+ tee_rproc_unregister(rproc->tee_interface);
+
}
static int stm32_rproc_suspend(struct device *dev)
--
2.25.1
