diff mbox series

[v2] wifi: ath11k: Fix invalid ring usage in full monitor mode

Message ID 20240924194119.15942-1-repk@triplefau.lt
State New
Headers show
Series [v2] wifi: ath11k: Fix invalid ring usage in full monitor mode | expand

Commit Message

Remi Pommarel Sept. 24, 2024, 7:41 p.m. UTC 
On full monitor HW the monitor destination rxdma ring does not have the
same descriptor format as in the "classical" mode. The full monitor
destination entries are of hal_sw_monitor_ring type and fetched using
ath11k_dp_full_mon_process_rx while the classical ones are of type
hal_reo_entrance_ring and fetched with ath11k_dp_rx_mon_dest_process.

Although both hal_sw_monitor_ring and hal_reo_entrance_ring are of same
size, the offset to useful info (such as sw_cookie, paddr, etc) are
different. Thus if ath11k_dp_rx_mon_dest_process gets called on full
monitor destination ring, invalid skb buffer id will be fetched from DMA
ring causing issues such as the following rcu_sched stall:

 rcu: INFO: rcu_sched self-detected stall on CPU
 rcu:     0-....: (1 GPs behind) idle=c67/0/0x7 softirq=45768/45769 fqs=1012
  (t=2100 jiffies g=14817 q=8703)
 Task dump for CPU 0:
 task:swapper/0       state:R  running task     stack: 0 pid:    0 ppid:     0 flags:0x0000000a
 Call trace:
  dump_backtrace+0x0/0x160
  show_stack+0x14/0x20
  sched_show_task+0x158/0x184
  dump_cpu_task+0x40/0x4c
  rcu_dump_cpu_stacks+0xec/0x12c
  rcu_sched_clock_irq+0x6c8/0x8a0
  update_process_times+0x88/0xd0
  tick_sched_timer+0x74/0x1e0
  __hrtimer_run_queues+0x150/0x204
  hrtimer_interrupt+0xe4/0x240
  arch_timer_handler_phys+0x30/0x40
  handle_percpu_devid_irq+0x80/0x130
  handle_domain_irq+0x5c/0x90
  gic_handle_irq+0x8c/0xb4
  do_interrupt_handler+0x30/0x54
  el1_interrupt+0x2c/0x4c
  el1h_64_irq_handler+0x14/0x1c
  el1h_64_irq+0x74/0x78
  do_raw_spin_lock+0x60/0x100
  _raw_spin_lock_bh+0x1c/0x2c
  ath11k_dp_rx_mon_mpdu_pop.constprop.0+0x174/0x650
  ath11k_dp_rx_process_mon_status+0x8b4/0xa80
  ath11k_dp_rx_process_mon_rings+0x244/0x510
  ath11k_dp_service_srng+0x190/0x300
  ath11k_pcic_ext_grp_napi_poll+0x30/0xc0
  __napi_poll+0x34/0x174
  net_rx_action+0xf8/0x2a0
  _stext+0x12c/0x2ac
  irq_exit+0x94/0xc0
  handle_domain_irq+0x60/0x90
  gic_handle_irq+0x8c/0xb4
  call_on_irq_stack+0x28/0x44
  do_interrupt_handler+0x4c/0x54
  el1_interrupt+0x2c/0x4c
  el1h_64_irq_handler+0x14/0x1c
  el1h_64_irq+0x74/0x78
  arch_cpu_idle+0x14/0x20
  do_idle+0xf0/0x130
  cpu_startup_entry+0x24/0x50
  rest_init+0xf8/0x104
  arch_call_rest_init+0xc/0x14
  start_kernel+0x56c/0x58c
  __primary_switched+0xa0/0xa8

Thus ath11k_dp_rx_mon_dest_process(), which use classical destination
entry format, should no be called on full monitor capable HW.

Fixes: 67a9d399fcb0 ("ath11k: enable RX PPDU stats in monitor co-exist mode")
Signed-off-by: Remi Pommarel <repk@triplefau.lt>
---
v2: set ppdu_status to DP_PPDU_STATUS_DONE as suggested by
    https://lore.kernel.org/ath11k/d376023d-267a-4512-8749-f816fefeb842@quicinc.com/

 drivers/net/wireless/ath/ath11k/dp_rx.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

P Praneesh Oct. 15, 2024, 11:43 a.m. UTC | #1 
On 9/25/2024 1:11 AM, Remi Pommarel wrote:
> On full monitor HW the monitor destination rxdma ring does not have the
> same descriptor format as in the "classical" mode. The full monitor
> destination entries are of hal_sw_monitor_ring type and fetched using
> ath11k_dp_full_mon_process_rx while the classical ones are of type
> hal_reo_entrance_ring and fetched with ath11k_dp_rx_mon_dest_process.
> 
> Although both hal_sw_monitor_ring and hal_reo_entrance_ring are of same
> size, the offset to useful info (such as sw_cookie, paddr, etc) are
> different. Thus if ath11k_dp_rx_mon_dest_process gets called on full
> monitor destination ring, invalid skb buffer id will be fetched from DMA
> ring causing issues such as the following rcu_sched stall:
> 
>   rcu: INFO: rcu_sched self-detected stall on CPU
>   rcu:     0-....: (1 GPs behind) idle=c67/0/0x7 softirq=45768/45769 fqs=1012
>    (t=2100 jiffies g=14817 q=8703)
>   Task dump for CPU 0:
>   task:swapper/0       state:R  running task     stack: 0 pid:    0 ppid:     0 flags:0x0000000a
>   Call trace:
>    dump_backtrace+0x0/0x160
>    show_stack+0x14/0x20
>    sched_show_task+0x158/0x184
>    dump_cpu_task+0x40/0x4c
>    rcu_dump_cpu_stacks+0xec/0x12c
>    rcu_sched_clock_irq+0x6c8/0x8a0
>    update_process_times+0x88/0xd0
>    tick_sched_timer+0x74/0x1e0
>    __hrtimer_run_queues+0x150/0x204
>    hrtimer_interrupt+0xe4/0x240
>    arch_timer_handler_phys+0x30/0x40
>    handle_percpu_devid_irq+0x80/0x130
>    handle_domain_irq+0x5c/0x90
>    gic_handle_irq+0x8c/0xb4
>    do_interrupt_handler+0x30/0x54
>    el1_interrupt+0x2c/0x4c
>    el1h_64_irq_handler+0x14/0x1c
>    el1h_64_irq+0x74/0x78
>    do_raw_spin_lock+0x60/0x100
>    _raw_spin_lock_bh+0x1c/0x2c
>    ath11k_dp_rx_mon_mpdu_pop.constprop.0+0x174/0x650
>    ath11k_dp_rx_process_mon_status+0x8b4/0xa80
>    ath11k_dp_rx_process_mon_rings+0x244/0x510
>    ath11k_dp_service_srng+0x190/0x300
>    ath11k_pcic_ext_grp_napi_poll+0x30/0xc0
>    __napi_poll+0x34/0x174
>    net_rx_action+0xf8/0x2a0
>    _stext+0x12c/0x2ac
>    irq_exit+0x94/0xc0
>    handle_domain_irq+0x60/0x90
>    gic_handle_irq+0x8c/0xb4
>    call_on_irq_stack+0x28/0x44
>    do_interrupt_handler+0x4c/0x54
>    el1_interrupt+0x2c/0x4c
>    el1h_64_irq_handler+0x14/0x1c
>    el1h_64_irq+0x74/0x78
>    arch_cpu_idle+0x14/0x20
>    do_idle+0xf0/0x130
>    cpu_startup_entry+0x24/0x50
>    rest_init+0xf8/0x104
>    arch_call_rest_init+0xc/0x14
>    start_kernel+0x56c/0x58c
>    __primary_switched+0xa0/0xa8
> 
> Thus ath11k_dp_rx_mon_dest_process(), which use classical destination
> entry format, should no be called on full monitor capable HW.
>
Thanks, it looks good to me.
Reviewed-by: Praneesh P <quic_ppranees@quicinc.com>
> Fixes: 67a9d399fcb0 ("ath11k: enable RX PPDU stats in monitor co-exist mode")
> Signed-off-by: Remi Pommarel <repk@triplefau.lt>
> ---
> v2: set ppdu_status to DP_PPDU_STATUS_DONE as suggested by
>      https://lore.kernel.org/ath11k/d376023d-267a-4512-8749-f816fefeb842@quicinc.com/
> 
>   drivers/net/wireless/ath/ath11k/dp_rx.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath11k/dp_rx.c b/drivers/net/wireless/ath/ath11k/dp_rx.c
> index c087d8a0f5b2..40088e62572e 100644
> --- a/drivers/net/wireless/ath/ath11k/dp_rx.c
> +++ b/drivers/net/wireless/ath/ath11k/dp_rx.c
> @@ -5291,8 +5291,11 @@ int ath11k_dp_rx_process_mon_status(struct ath11k_base *ab, int mac_id,
>   		    hal_status == HAL_TLV_STATUS_PPDU_DONE) {
>   			rx_mon_stats->status_ppdu_done++;
>   			pmon->mon_ppdu_status = DP_PPDU_STATUS_DONE;
> -			ath11k_dp_rx_mon_dest_process(ar, mac_id, budget, napi);
> -			pmon->mon_ppdu_status = DP_PPDU_STATUS_START;
> +			if (!ab->hw_params.full_monitor_mode) {
> +				ath11k_dp_rx_mon_dest_process(ar, mac_id,
> +							      budget, napi);
> +				pmon->mon_ppdu_status = DP_PPDU_STATUS_START;
> +			}
>   		}
>   
>   		if (ppdu_info->peer_id == HAL_INVALID_PEERID ||
Jeff Johnson Oct. 16, 2024, 2:39 p.m. UTC | #2 
On Tue, 24 Sep 2024 21:41:19 +0200, Remi Pommarel wrote:
> On full monitor HW the monitor destination rxdma ring does not have the
> same descriptor format as in the "classical" mode. The full monitor
> destination entries are of hal_sw_monitor_ring type and fetched using
> ath11k_dp_full_mon_process_rx while the classical ones are of type
> hal_reo_entrance_ring and fetched with ath11k_dp_rx_mon_dest_process.
> 
> Although both hal_sw_monitor_ring and hal_reo_entrance_ring are of same
> size, the offset to useful info (such as sw_cookie, paddr, etc) are
> different. Thus if ath11k_dp_rx_mon_dest_process gets called on full
> monitor destination ring, invalid skb buffer id will be fetched from DMA
> ring causing issues such as the following rcu_sched stall:
> 
> [...]

Applied, thanks!

[1/1] wifi: ath11k: Fix invalid ring usage in full monitor mode
      commit: befd716ed429b26eca7abde95da6195c548470de

Best regards,
diff mbox series

Patch

diff --git a/drivers/net/wireless/ath/ath11k/dp_rx.c b/drivers/net/wireless/ath/ath11k/dp_rx.c
index c087d8a0f5b2..40088e62572e 100644
--- a/drivers/net/wireless/ath/ath11k/dp_rx.c
+++ b/drivers/net/wireless/ath/ath11k/dp_rx.c
@@ -5291,8 +5291,11 @@  int ath11k_dp_rx_process_mon_status(struct ath11k_base *ab, int mac_id,
 		    hal_status == HAL_TLV_STATUS_PPDU_DONE) {
 			rx_mon_stats->status_ppdu_done++;
 			pmon->mon_ppdu_status = DP_PPDU_STATUS_DONE;
-			ath11k_dp_rx_mon_dest_process(ar, mac_id, budget, napi);
-			pmon->mon_ppdu_status = DP_PPDU_STATUS_START;
+			if (!ab->hw_params.full_monitor_mode) {
+				ath11k_dp_rx_mon_dest_process(ar, mac_id,
+							      budget, napi);
+				pmon->mon_ppdu_status = DP_PPDU_STATUS_START;
+			}
 		}
 
 		if (ppdu_info->peer_id == HAL_INVALID_PEERID ||