Re: [Intel-wired-lan] [PATCH iwl-net v2 5/5] idpf: fix error handling in the init_task on load

["Chittim, Madhu" <madhu.chittim@intel.com>]

On 11/20/2025 4:12 PM, Tantilov, Emil S wrote:
> If the init_task fails during a driver load, we end up without vports and
> netdevs, effectively failing the entire process. In that state a
> subsequent reset will result in a crash as the service task attempts to
> access uninitialized resources. Following trace is from an error in the
> init_task where the CREATE_VPORT (op 501) is rejected by the FW:
> 
> [40922.763136] idpf 0000:83:00.0: Device HW Reset initiated
> [40924.449797] idpf 0000:83:00.0: Transaction failed (op 501)
> [40958.148190] idpf 0000:83:00.0: HW reset detected
> [40958.161202] BUG: kernel NULL pointer dereference, address: 00000000000000a8
> ...
> [40958.168094] Workqueue: idpf-0000:83:00.0-vc_event idpf_vc_event_task [idpf]
> [40958.168865] RIP: 0010:idpf_vc_event_task+0x9b/0x350 [idpf]
> ...
> [40958.177932] Call Trace:
> [40958.178491]  <TASK>
> [40958.179040]  process_one_work+0x226/0x6d0
> [40958.179609]  worker_thread+0x19e/0x340
> [40958.180158]  ? __pfx_worker_thread+0x10/0x10
> [40958.180702]  kthread+0x10f/0x250
> [40958.181238]  ? __pfx_kthread+0x10/0x10
> [40958.181774]  ret_from_fork+0x251/0x2b0
> [40958.182307]  ? __pfx_kthread+0x10/0x10
> [40958.182834]  ret_from_fork_asm+0x1a/0x30
> [40958.183370]  </TASK>
> 
> Fix the error handling in the init_task to make sure the service and
> mailbox tasks are disabled if the error happens during load. These are
> started in idpf_vc_core_init(), which spawns the init_task and has no way
> of knowing if it failed. If the error happens on reset, following
> successful driver load, the tasks can still run, as that will allow the
> netdevs to attempt recovery through another reset. Stop the PTP callbacks
> either way as those will be restarted by the call to idpf_vc_core_init()
> during a successful reset.
> 
> Fixes: 0fe45467a104 ("idpf: add create vport and netdev configuration")
> Reported-by: Vivek Kumar <iamvivekkumar@google.com>
> Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>

Reviewed-by: Madhu Chittim <madhu.chittim@intel.com>

> ---
>   drivers/net/ethernet/intel/idpf/idpf_lib.c | 16 ++++++++++++----
>   1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c b/drivers/net/ethernet/intel/idpf/idpf_lib.c
> index 5193968c9bb1..89f3b46378c4 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_lib.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c
> @@ -1716,10 +1716,9 @@ void idpf_init_task(struct work_struct *work)
>   		set_bit(IDPF_VPORT_REG_NETDEV, vport_config->flags);
>   	}
>   
> -	/* As all the required vports are created, clear the reset flag
> -	 * unconditionally here in case we were in reset and the link was down.
> -	 */
> +	/* Clear the reset and load bits as all vports are created */
>   	clear_bit(IDPF_HR_RESET_IN_PROG, adapter->flags);
> +	clear_bit(IDPF_HR_DRV_LOAD, adapter->flags);
>   	/* Start the statistics task now */
>   	queue_delayed_work(adapter->stats_wq, &adapter->stats_task,
>   			   msecs_to_jiffies(10 * (pdev->devfn & 0x07)));
> @@ -1733,6 +1732,15 @@ void idpf_init_task(struct work_struct *work)
>   				idpf_vport_dealloc(adapter->vports[index]);
>   		}
>   	}
> +	/* Cleanup after vc_core_init, which has no way of knowing the
> +	 * init task failed on driver load.
> +	 */
> +	if (test_and_clear_bit(IDPF_HR_DRV_LOAD, adapter->flags)) {
> +		cancel_delayed_work_sync(&adapter->serv_task);
> +		cancel_delayed_work_sync(&adapter->mbx_task);
> +	}
> +	idpf_ptp_release(adapter);
> +
>   	clear_bit(IDPF_HR_RESET_IN_PROG, adapter->flags);
>   }
>   
> @@ -1882,7 +1890,7 @@ static void idpf_init_hard_reset(struct idpf_adapter *adapter)
>   	dev_info(dev, "Device HW Reset initiated\n");
>   
>   	/* Prepare for reset */
> -	if (test_and_clear_bit(IDPF_HR_DRV_LOAD, adapter->flags)) {
> +	if (test_bit(IDPF_HR_DRV_LOAD, adapter->flags)) {
>   		reg_ops->trigger_reset(adapter, IDPF_HR_DRV_LOAD);
>   	} else if (test_and_clear_bit(IDPF_HR_FUNC_RESET, adapter->flags)) {
>   		bool is_reset = idpf_is_reset_detected(adapter);