[PATCH v3 1/2] ice: Fix NULL pointer dereference in ice_vsi_set_napi_queues

[PATCH v3 1/2] ice: Fix NULL pointer dereference in ice_vsi_set_napi_queues

[Aaron Ma]

Add NULL pointer checks in ice_vsi_set_napi_queues() to prevent crashes
during resume from suspend when rings[q_idx]->q_vector is NULL.

Tested adaptor:
60:00.0 Ethernet controller [0200]: Intel Corporation Ethernet Controller E810-XXV for SFP [8086:159b] (rev 02)
        Subsystem: Intel Corporation Ethernet Network Adapter E810-XXV-2 [8086:4003]

SR-IOV state: both disabled and enabled can reproduce this issue.

kernel version: v6.18

Reproduce steps:
Bootup and execute suspend like systemctl suspend or rtcwake.

Log:
<1>[  231.443607] BUG: kernel NULL pointer dereference, address: 0000000000000040
<1>[  231.444052] #PF: supervisor read access in kernel mode
<1>[  231.444484] #PF: error_code(0x0000) - not-present page
<6>[  231.444913] PGD 0 P4D 0
<4>[  231.445342] Oops: Oops: 0000 [#1] SMP NOPTI
<4>[  231.446635] RIP: 0010:netif_queue_set_napi+0xa/0x170
<4>[  231.447067] Code: 31 f6 31 ff c3 cc cc cc cc 0f 1f 80 00 00 00 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 0f 1f 44 00 00 48 85 c9 74 0b <48> 83 79 30 00 0f 84 39 01 00 00 55 41 89 d1 49 89 f8 89 f2 48 89
<4>[  231.447513] RSP: 0018:ffffcc780fc078c0 EFLAGS: 00010202
<4>[  231.447961] RAX: ffff8b848ca30400 RBX: ffff8b848caf2028 RCX: 0000000000000010
<4>[  231.448443] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff8b848dbd4000
<4>[  231.448896] RBP: ffffcc780fc078e8 R08: 0000000000000000 R09: 0000000000000000
<4>[  231.449345] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000001
<4>[  231.449817] R13: ffff8b848dbd4000 R14: ffff8b84833390c8 R15: 0000000000000000
<4>[  231.450265] FS:  00007c7b29e9d740(0000) GS:ffff8b8c068e2000(0000) knlGS:0000000000000000
<4>[  231.450715] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
<4>[  231.451179] CR2: 0000000000000040 CR3: 000000030626f004 CR4: 0000000000f72ef0
<4>[  231.451629] PKRU: 55555554
<4>[  231.452076] Call Trace:
<4>[  231.452549]  <TASK>
<4>[  231.452996]  ? ice_vsi_set_napi_queues+0x4d/0x110 [ice]
<4>[  231.453482]  ice_resume+0xfd/0x220 [ice]
<4>[  231.453977]  ? __pfx_pci_pm_resume+0x10/0x10
<4>[  231.454425]  pci_pm_resume+0x8c/0x140
<4>[  231.454872]  ? __pfx_pci_pm_resume+0x10/0x10
<4>[  231.455347]  dpm_run_callback+0x5f/0x160
<4>[  231.455796]  ? dpm_wait_for_superior+0x107/0x170
<4>[  231.456244]  device_resume+0x177/0x270
<4>[  231.456708]  dpm_resume+0x209/0x2f0
<4>[  231.457151]  dpm_resume_end+0x15/0x30
<4>[  231.457596]  suspend_devices_and_enter+0x1da/0x2b0
<4>[  231.458054]  enter_state+0x10e/0x570

Add defensive checks for both the ring pointer and its q_vector
before dereferencing, allowing the system to resume successfully even when
q_vectors are unmapped.

Fixes: 2a5dc090b92cf ("ice: move netif_queue_set_napi to rtnl-protected sections")
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
---
V1 -> V2: add test device info.
V2 -> V3: no changes.

 drivers/net/ethernet/intel/ice/ice_lib.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
index 15621707fbf81..9d1178bde4495 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -2779,11 +2779,13 @@ void ice_vsi_set_napi_queues(struct ice_vsi *vsi)
 
 	ASSERT_RTNL();
 	ice_for_each_rxq(vsi, q_idx)
-		netif_queue_set_napi(netdev, q_idx, NETDEV_QUEUE_TYPE_RX,
+		if (vsi->rx_rings[q_idx] && vsi->rx_rings[q_idx]->q_vector)
+			netif_queue_set_napi(netdev, q_idx, NETDEV_QUEUE_TYPE_RX,
 				     &vsi->rx_rings[q_idx]->q_vector->napi);
 
 	ice_for_each_txq(vsi, q_idx)
-		netif_queue_set_napi(netdev, q_idx, NETDEV_QUEUE_TYPE_TX,
+		if (vsi->tx_rings[q_idx] && vsi->tx_rings[q_idx]->q_vector)
+			netif_queue_set_napi(netdev, q_idx, NETDEV_QUEUE_TYPE_TX,
 				     &vsi->tx_rings[q_idx]->q_vector->napi);
 	/* Also set the interrupt number for the NAPI */
 	ice_for_each_q_vector(vsi, v_idx) {
-- 
2.43.0

[PATCH v3 2/2] ice: recap the VSI and QoS info after rebuild

[Aaron Ma]

Fix IRDMA hardware initialization timeout (-110) after resume by
separating VSI-dependent configuration from RDMA resource allocation,
ensuring VSI is rebuilt before IRDMA accesses it.

After resume from suspend, IRDMA hardware initialization fails:
  ice: IRDMA hardware initialization FAILED init_state=4 status=-110

Separate RDMA initialization into two phases:
1. ice_init_rdma() - Allocate resources only (no VSI/QoS access, no plug)
2. ice_rdma_finalize_setup() - Assign VSI/QoS info and plug device

This allows:
- ice_init_rdma() to stay in ice_resume() (mirrors ice_deinit_rdma()
  in ice_suspend()
- VSI assignment deferred until after ice_vsi_rebuild() completes
- QoS info updated after ice_dcb_rebuild() completes
- Device plugged only when control queues, VSI, and DCB are all ready

Fixes: bc69ad74867db ("ice: avoid IRQ collision to fix init failure on ACPI S3 resume")
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
---
V1 -> V2: no changes.
V2 -> V3:
- mirrors init_rdma in resume as Tony Nguyen suggested to fix
the memleak and move ice_plug_aux_dev/ice_unplug_aux_dev out of
init/deinit rdma.
- ensure the correct VSI/QoS info is loaded after rebuild.

 drivers/net/ethernet/intel/ice/ice.h      |  1 +
 drivers/net/ethernet/intel/ice/ice_idc.c  | 41 +++++++++++++++++------
 drivers/net/ethernet/intel/ice/ice_main.c |  7 +++-
 3 files changed, 38 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
index 147aaee192a79..6463c1fea7871 100644
--- a/drivers/net/ethernet/intel/ice/ice.h
+++ b/drivers/net/ethernet/intel/ice/ice.h
@@ -989,6 +989,7 @@ int ice_schedule_reset(struct ice_pf *pf, enum ice_reset_req reset);
 void ice_print_link_msg(struct ice_vsi *vsi, bool isup);
 int ice_plug_aux_dev(struct ice_pf *pf);
 void ice_unplug_aux_dev(struct ice_pf *pf);
+void ice_rdma_finalize_setup(struct ice_pf *pf);
 int ice_init_rdma(struct ice_pf *pf);
 void ice_deinit_rdma(struct ice_pf *pf);
 bool ice_is_wol_supported(struct ice_hw *hw);
diff --git a/drivers/net/ethernet/intel/ice/ice_idc.c b/drivers/net/ethernet/intel/ice/ice_idc.c
index 420d45c2558b6..b6079a6cb7736 100644
--- a/drivers/net/ethernet/intel/ice/ice_idc.c
+++ b/drivers/net/ethernet/intel/ice/ice_idc.c
@@ -360,6 +360,35 @@ void ice_unplug_aux_dev(struct ice_pf *pf)
 	auxiliary_device_uninit(adev);
 }
 
+/**
+ * ice_rdma_finalize_setup - Complete RDMA setup after VSI is ready
+ * @pf: ptr to ice_pf
+ *
+ * Sets VSI-dependent information and plugs aux device.
+ * Must be called after ice_init_rdma(), ice_vsi_rebuild(), and
+ * ice_dcb_rebuild() complete.
+ */
+void ice_rdma_finalize_setup(struct ice_pf *pf)
+{
+	struct iidc_rdma_priv_dev_info *privd;
+
+	if (!ice_is_rdma_ena(pf) || !pf->cdev_info)
+		return;
+
+	privd = pf->cdev_info->iidc_priv;
+	if (!privd || !pf->vsi[0] || !pf->vsi[0]->netdev)
+		return;
+
+	/* Assign VSI info now that VSI is valid */
+	privd->netdev = pf->vsi[0]->netdev;
+	privd->vport_id = pf->vsi[0]->vsi_num;
+
+	/* Update QoS info after DCB has been rebuilt */
+	ice_setup_dcb_qos_info(pf, &privd->qos_info);
+
+	ice_plug_aux_dev(pf);
+}
+
 /**
  * ice_init_rdma - initializes PF for RDMA use
  * @pf: ptr to ice_pf
@@ -398,23 +427,16 @@ int ice_init_rdma(struct ice_pf *pf)
 	}
 
 	cdev->iidc_priv = privd;
-	privd->netdev = pf->vsi[0]->netdev;
 
 	privd->hw_addr = (u8 __iomem *)pf->hw.hw_addr;
 	cdev->pdev = pf->pdev;
-	privd->vport_id = pf->vsi[0]->vsi_num;
 
 	pf->cdev_info->rdma_protocol |= IIDC_RDMA_PROTOCOL_ROCEV2;
-	ice_setup_dcb_qos_info(pf, &privd->qos_info);
-	ret = ice_plug_aux_dev(pf);
-	if (ret)
-		goto err_plug_aux_dev;
+
 	return 0;
 
-err_plug_aux_dev:
-	pf->cdev_info->adev = NULL;
-	xa_erase(&ice_aux_id, pf->aux_idx);
 err_alloc_xa:
+	xa_erase(&ice_aux_id, pf->aux_idx);
 	kfree(privd);
 err_privd_alloc:
 	kfree(cdev);
@@ -432,7 +454,6 @@ void ice_deinit_rdma(struct ice_pf *pf)
 	if (!ice_is_rdma_ena(pf))
 		return;
 
-	ice_unplug_aux_dev(pf);
 	xa_erase(&ice_aux_id, pf->aux_idx);
 	kfree(pf->cdev_info->iidc_priv);
 	kfree(pf->cdev_info);
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 4bb68e7a00f5f..1851e9932cefe 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -5135,6 +5135,9 @@ int ice_load(struct ice_pf *pf)
 	if (err)
 		goto err_init_rdma;
 
+	/* Finalize RDMA: VSI already created, assign info and plug device */
+	ice_rdma_finalize_setup(pf);
+
 	ice_service_task_restart(pf);
 
 	clear_bit(ICE_DOWN, pf->state);
@@ -5166,6 +5169,7 @@ void ice_unload(struct ice_pf *pf)
 
 	devl_assert_locked(priv_to_devlink(pf));
 
+	ice_unplug_aux_dev(pf);
 	ice_deinit_rdma(pf);
 	ice_deinit_features(pf);
 	ice_tc_indir_block_unregister(vsi);
@@ -5594,6 +5598,7 @@ static int ice_suspend(struct device *dev)
 	 */
 	disabled = ice_service_task_stop(pf);
 
+	ice_unplug_aux_dev(pf);
 	ice_deinit_rdma(pf);
 
 	/* Already suspended?, then there is nothing to do */
@@ -7803,7 +7808,7 @@ static void ice_rebuild(struct ice_pf *pf, enum ice_reset_req reset_type)
 
 	ice_health_clear(pf);
 
-	ice_plug_aux_dev(pf);
+	ice_rdma_finalize_setup(pf);
 	if (ice_is_feature_supported(pf, ICE_F_SRIOV_LAG))
 		ice_lag_rebuild(pf);
 
-- 
2.43.0

Re: [Intel-wired-lan] [PATCH v3 1/2] ice: Fix NULL pointer dereference in ice_vsi_set_napi_queues

[Paul Menzel]

Dear Aaron,


Thank you for the patch.

Am 25.12.25 um 11:21 schrieb Aaron Ma via Intel-wired-lan:
> Add NULL pointer checks in ice_vsi_set_napi_queues() to prevent crashes
> during resume from suspend when rings[q_idx]->q_vector is NULL.
> 
> Tested adaptor:
> 60:00.0 Ethernet controller [0200]: Intel Corporation Ethernet Controller E810-XXV for SFP [8086:159b] (rev 02)
>          Subsystem: Intel Corporation Ethernet Network Adapter E810-XXV-2 [8086:4003]
> 
> SR-IOV state: both disabled and enabled can reproduce this issue.
> 
> kernel version: v6.18
> 
> Reproduce steps:
> Bootup and execute suspend like systemctl suspend or rtcwake.
> 
> Log:
> <1>[  231.443607] BUG: kernel NULL pointer dereference, address: 0000000000000040
> <1>[  231.444052] #PF: supervisor read access in kernel mode
> <1>[  231.444484] #PF: error_code(0x0000) - not-present page
> <6>[  231.444913] PGD 0 P4D 0
> <4>[  231.445342] Oops: Oops: 0000 [#1] SMP NOPTI
> <4>[  231.446635] RIP: 0010:netif_queue_set_napi+0xa/0x170
> <4>[  231.447067] Code: 31 f6 31 ff c3 cc cc cc cc 0f 1f 80 00 00 00 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 0f 1f 44 00 00 48 85 c9 74 0b <48> 83 79 30 00 0f 84 39 01 00 00 55 41 89 d1 49 89 f8 89 f2 48 89
> <4>[  231.447513] RSP: 0018:ffffcc780fc078c0 EFLAGS: 00010202
> <4>[  231.447961] RAX: ffff8b848ca30400 RBX: ffff8b848caf2028 RCX: 0000000000000010
> <4>[  231.448443] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff8b848dbd4000
> <4>[  231.448896] RBP: ffffcc780fc078e8 R08: 0000000000000000 R09: 0000000000000000
> <4>[  231.449345] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000001
> <4>[  231.449817] R13: ffff8b848dbd4000 R14: ffff8b84833390c8 R15: 0000000000000000
> <4>[  231.450265] FS:  00007c7b29e9d740(0000) GS:ffff8b8c068e2000(0000) knlGS:0000000000000000
> <4>[  231.450715] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> <4>[  231.451179] CR2: 0000000000000040 CR3: 000000030626f004 CR4: 0000000000f72ef0
> <4>[  231.451629] PKRU: 55555554
> <4>[  231.452076] Call Trace:
> <4>[  231.452549]  <TASK>
> <4>[  231.452996]  ? ice_vsi_set_napi_queues+0x4d/0x110 [ice]
> <4>[  231.453482]  ice_resume+0xfd/0x220 [ice]
> <4>[  231.453977]  ? __pfx_pci_pm_resume+0x10/0x10
> <4>[  231.454425]  pci_pm_resume+0x8c/0x140
> <4>[  231.454872]  ? __pfx_pci_pm_resume+0x10/0x10
> <4>[  231.455347]  dpm_run_callback+0x5f/0x160
> <4>[  231.455796]  ? dpm_wait_for_superior+0x107/0x170
> <4>[  231.456244]  device_resume+0x177/0x270
> <4>[  231.456708]  dpm_resume+0x209/0x2f0
> <4>[  231.457151]  dpm_resume_end+0x15/0x30
> <4>[  231.457596]  suspend_devices_and_enter+0x1da/0x2b0
> <4>[  231.458054]  enter_state+0x10e/0x570
> 
> Add defensive checks for both the ring pointer and its q_vector
> before dereferencing, allowing the system to resume successfully even when
> q_vectors are unmapped.
> 
> Fixes: 2a5dc090b92cf ("ice: move netif_queue_set_napi to rtnl-protected sections")
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
> ---
> V1 -> V2: add test device info.
> V2 -> V3: no changes.
> 
>   drivers/net/ethernet/intel/ice/ice_lib.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
> index 15621707fbf81..9d1178bde4495 100644
> --- a/drivers/net/ethernet/intel/ice/ice_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
> @@ -2779,11 +2779,13 @@ void ice_vsi_set_napi_queues(struct ice_vsi *vsi)
>   
>   	ASSERT_RTNL();
>   	ice_for_each_rxq(vsi, q_idx)
> -		netif_queue_set_napi(netdev, q_idx, NETDEV_QUEUE_TYPE_RX,
> +		if (vsi->rx_rings[q_idx] && vsi->rx_rings[q_idx]->q_vector)
> +			netif_queue_set_napi(netdev, q_idx, NETDEV_QUEUE_TYPE_RX,
>   				     &vsi->rx_rings[q_idx]->q_vector->napi);
>   
>   	ice_for_each_txq(vsi, q_idx)
> -		netif_queue_set_napi(netdev, q_idx, NETDEV_QUEUE_TYPE_TX,
> +		if (vsi->tx_rings[q_idx] && vsi->tx_rings[q_idx]->q_vector)
> +			netif_queue_set_napi(netdev, q_idx, NETDEV_QUEUE_TYPE_TX,
>   				     &vsi->tx_rings[q_idx]->q_vector->napi);
>   	/* Also set the interrupt number for the NAPI */
>   	ice_for_each_q_vector(vsi, v_idx) {

Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>


Kind regards,

Pa

RE: [Intel-wired-lan] [PATCH v3 2/2] ice: recap the VSI and QoS info after rebuild

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Aaron Ma via Intel-wired-lan
> Sent: Thursday, December 25, 2025 7:21 AM
> To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel,
> Przemyslaw <przemyslaw.kitszel@intel.com>; andrew+netdev@lunn.ch;
> davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> pabeni@redhat.com; intel-wired-lan@lists.osuosl.org;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: [Intel-wired-lan] [PATCH v3 2/2] ice: recap the VSI and QoS
> info after rebuild
> 
> Fix IRDMA hardware initialization timeout (-110) after resume by
> separating VSI-dependent configuration from RDMA resource allocation,
> ensuring VSI is rebuilt before IRDMA accesses it.
> 
> After resume from suspend, IRDMA hardware initialization fails:
>   ice: IRDMA hardware initialization FAILED init_state=4 status=-110
> 
> Separate RDMA initialization into two phases:
> 1. ice_init_rdma() - Allocate resources only (no VSI/QoS access, no
> plug) 2. ice_rdma_finalize_setup() - Assign VSI/QoS info and plug
> device
> 
> This allows:
> - ice_init_rdma() to stay in ice_resume() (mirrors ice_deinit_rdma()
>   in ice_suspend()
> - VSI assignment deferred until after ice_vsi_rebuild() completes
> - QoS info updated after ice_dcb_rebuild() completes
> - Device plugged only when control queues, VSI, and DCB are all ready
> 
> Fixes: bc69ad74867db ("ice: avoid IRQ collision to fix init failure on
> ACPI S3 resume")
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
> ---
> V1 -> V2: no changes.
> V2 -> V3:
> - mirrors init_rdma in resume as Tony Nguyen suggested to fix the
> memleak and move ice_plug_aux_dev/ice_unplug_aux_dev out of
> init/deinit rdma.
> - ensure the correct VSI/QoS info is loaded after rebuild.
> 
>  drivers/net/ethernet/intel/ice/ice.h      |  1 +
>  drivers/net/ethernet/intel/ice/ice_idc.c  | 41 +++++++++++++++++-----
> -  drivers/net/ethernet/intel/ice/ice_main.c |  7 +++-
>  3 files changed, 38 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice.h
> b/drivers/net/ethernet/intel/ice/ice.h
> index 147aaee192a79..6463c1fea7871 100644
> --- a/drivers/net/ethernet/intel/ice/ice.h
> +++ b/drivers/net/ethernet/intel/ice/ice.h
> @@ -989,6 +989,7 @@ int ice_schedule_reset(struct ice_pf *pf, enum
> ice_reset_req reset);  void ice_print_link_msg(struct ice_vsi *vsi,
> bool isup);  int ice_plug_aux_dev(struct ice_pf *pf);  void
> ice_unplug_aux_dev(struct ice_pf *pf);
> +void ice_rdma_finalize_setup(struct ice_pf *pf);
>  int ice_init_rdma(struct ice_pf *pf);
>  void ice_deinit_rdma(struct ice_pf *pf);  bool
> ice_is_wol_supported(struct ice_hw *hw); diff --git
> a/drivers/net/ethernet/intel/ice/ice_idc.c
> b/drivers/net/ethernet/intel/ice/ice_idc.c
> index 420d45c2558b6..b6079a6cb7736 100644
> --- a/drivers/net/ethernet/intel/ice/ice_idc.c
> +++ b/drivers/net/ethernet/intel/ice/ice_idc.c
> @@ -360,6 +360,35 @@ void ice_unplug_aux_dev(struct ice_pf *pf)
>  	auxiliary_device_uninit(adev);
>  }
> 
> +/**
> + * ice_rdma_finalize_setup - Complete RDMA setup after VSI is ready
> + * @pf: ptr to ice_pf
> + *
> + * Sets VSI-dependent information and plugs aux device.
> + * Must be called after ice_init_rdma(), ice_vsi_rebuild(), and
> + * ice_dcb_rebuild() complete.
> + */
> +void ice_rdma_finalize_setup(struct ice_pf *pf) {
> +	struct iidc_rdma_priv_dev_info *privd;
> +
> +	if (!ice_is_rdma_ena(pf) || !pf->cdev_info)
> +		return;
> +
> +	privd = pf->cdev_info->iidc_priv;
> +	if (!privd || !pf->vsi[0] || !pf->vsi[0]->netdev)
Direct array index access assumes VSI 0 exists without verifying the array size.
What do you think about:

    if (!privd || !pf->vsi || !pf->vsi[0] || !pf->vsi[0]->netdev)
        return;

?

> +		return;
> +
> +	/* Assign VSI info now that VSI is valid */
> +	privd->netdev = pf->vsi[0]->netdev;
> +	privd->vport_id = pf->vsi[0]->vsi_num;
> +
> +	/* Update QoS info after DCB has been rebuilt */
> +	ice_setup_dcb_qos_info(pf, &privd->qos_info);
> +
> +	ice_plug_aux_dev(pf);
> +}
> +
>  /**
>   * ice_init_rdma - initializes PF for RDMA use
>   * @pf: ptr to ice_pf
> @@ -398,23 +427,16 @@ int ice_init_rdma(struct ice_pf *pf)
>  	}
> 
>  	cdev->iidc_priv = privd;
> -	privd->netdev = pf->vsi[0]->netdev;
> 
>  	privd->hw_addr = (u8 __iomem *)pf->hw.hw_addr;
>  	cdev->pdev = pf->pdev;
> -	privd->vport_id = pf->vsi[0]->vsi_num;
> 
>  	pf->cdev_info->rdma_protocol |= IIDC_RDMA_PROTOCOL_ROCEV2;
> -	ice_setup_dcb_qos_info(pf, &privd->qos_info);
> -	ret = ice_plug_aux_dev(pf);
> -	if (ret)
> -		goto err_plug_aux_dev;
> +
>  	return 0;
> 
> -err_plug_aux_dev:
> -	pf->cdev_info->adev = NULL;
> -	xa_erase(&ice_aux_id, pf->aux_idx);
>  err_alloc_xa:
> +	xa_erase(&ice_aux_id, pf->aux_idx);
>  	kfree(privd);
>  err_privd_alloc:
>  	kfree(cdev);
> @@ -432,7 +454,6 @@ void ice_deinit_rdma(struct ice_pf *pf)
>  	if (!ice_is_rdma_ena(pf))
>  		return;
> 
> -	ice_unplug_aux_dev(pf);
>  	xa_erase(&ice_aux_id, pf->aux_idx);
>  	kfree(pf->cdev_info->iidc_priv);
>  	kfree(pf->cdev_info);
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c
> b/drivers/net/ethernet/intel/ice/ice_main.c
> index 4bb68e7a00f5f..1851e9932cefe 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -5135,6 +5135,9 @@ int ice_load(struct ice_pf *pf)
>  	if (err)
>  		goto err_init_rdma;
> 
> +	/* Finalize RDMA: VSI already created, assign info and plug
> device */
> +	ice_rdma_finalize_setup(pf);
> +
>  	ice_service_task_restart(pf);
> 
>  	clear_bit(ICE_DOWN, pf->state);
> @@ -5166,6 +5169,7 @@ void ice_unload(struct ice_pf *pf)
> 
>  	devl_assert_locked(priv_to_devlink(pf));
> 
> +	ice_unplug_aux_dev(pf);
>  	ice_deinit_rdma(pf);
>  	ice_deinit_features(pf);
>  	ice_tc_indir_block_unregister(vsi);
> @@ -5594,6 +5598,7 @@ static int ice_suspend(struct device *dev)
>  	 */
>  	disabled = ice_service_task_stop(pf);
> 
> +	ice_unplug_aux_dev(pf);
>  	ice_deinit_rdma(pf);
> 
>  	/* Already suspended?, then there is nothing to do */ @@ -
> 7803,7 +7808,7 @@ static void ice_rebuild(struct ice_pf *pf, enum
> ice_reset_req reset_type)
> 
>  	ice_health_clear(pf);
> 
> -	ice_plug_aux_dev(pf);
> +	ice_rdma_finalize_setup(pf);
>  	if (ice_is_feature_supported(pf, ICE_F_SRIOV_LAG))
>  		ice_lag_rebuild(pf);
> 
> --
> 2.43.0

RE: [Intel-wired-lan] [PATCH v3 2/2] ice: recap the VSI and QoS info after rebuild

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Aaron Ma via Intel-wired-lan
> Sent: Thursday, December 25, 2025 7:21 AM
> To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel,
> Przemyslaw <przemyslaw.kitszel@intel.com>; andrew+netdev@lunn.ch;
> davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> pabeni@redhat.com; intel-wired-lan@lists.osuosl.org;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: [Intel-wired-lan] [PATCH v3 2/2] ice: recap the VSI and QoS
> info after rebuild
> 
> Fix IRDMA hardware initialization timeout (-110) after resume by
> separating VSI-dependent configuration from RDMA resource allocation,
> ensuring VSI is rebuilt before IRDMA accesses it.
> 
> After resume from suspend, IRDMA hardware initialization fails:
>   ice: IRDMA hardware initialization FAILED init_state=4 status=-110
> 
> Separate RDMA initialization into two phases:
> 1. ice_init_rdma() - Allocate resources only (no VSI/QoS access, no
> plug) 2. ice_rdma_finalize_setup() - Assign VSI/QoS info and plug
> device
> 
> This allows:
> - ice_init_rdma() to stay in ice_resume() (mirrors ice_deinit_rdma()
>   in ice_suspend()
> - VSI assignment deferred until after ice_vsi_rebuild() completes
> - QoS info updated after ice_dcb_rebuild() completes
> - Device plugged only when control queues, VSI, and DCB are all ready
> 
> Fixes: bc69ad74867db ("ice: avoid IRQ collision to fix init failure on
> ACPI S3 resume")
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
> ---
> V1 -> V2: no changes.
> V2 -> V3:
> - mirrors init_rdma in resume as Tony Nguyen suggested to fix the
> memleak and move ice_plug_aux_dev/ice_unplug_aux_dev out of
> init/deinit rdma.
> - ensure the correct VSI/QoS info is loaded after rebuild.
> 
>  drivers/net/ethernet/intel/ice/ice.h      |  1 +
>  drivers/net/ethernet/intel/ice/ice_idc.c  | 41 +++++++++++++++++-----
> -  drivers/net/ethernet/intel/ice/ice_main.c |  7 +++-
>  3 files changed, 38 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice.h
> b/drivers/net/ethernet/intel/ice/ice.h
> index 147aaee192a79..6463c1fea7871 100644
> --- a/drivers/net/ethernet/intel/ice/ice.h
> +++ b/drivers/net/ethernet/intel/ice/ice.h
> @@ -989,6 +989,7 @@ int ice_schedule_reset(struct ice_pf *pf, enum
> ice_reset_req reset);  void ice_print_link_msg(struct ice_vsi *vsi,
> bool isup);  int ice_plug_aux_dev(struct ice_pf *pf);  void
> ice_unplug_aux_dev(struct ice_pf *pf);
> +void ice_rdma_finalize_setup(struct ice_pf *pf);
>  int ice_init_rdma(struct ice_pf *pf);
>  void ice_deinit_rdma(struct ice_pf *pf);  bool
> ice_is_wol_supported(struct ice_hw *hw); diff --git
> a/drivers/net/ethernet/intel/ice/ice_idc.c
> b/drivers/net/ethernet/intel/ice/ice_idc.c
> index 420d45c2558b6..b6079a6cb7736 100644
> --- a/drivers/net/ethernet/intel/ice/ice_idc.c
> +++ b/drivers/net/ethernet/intel/ice/ice_idc.c
> @@ -360,6 +360,35 @@ void ice_unplug_aux_dev(struct ice_pf *pf)
>  	auxiliary_device_uninit(adev);
>  }
> 
> +/**
> + * ice_rdma_finalize_setup - Complete RDMA setup after VSI is ready
> + * @pf: ptr to ice_pf
> + *
> + * Sets VSI-dependent information and plugs aux device.
> + * Must be called after ice_init_rdma(), ice_vsi_rebuild(), and
> + * ice_dcb_rebuild() complete.
> + */
> +void ice_rdma_finalize_setup(struct ice_pf *pf) {
> +	struct iidc_rdma_priv_dev_info *privd;
> +
> +	if (!ice_is_rdma_ena(pf) || !pf->cdev_info)
> +		return;
> +
> +	privd = pf->cdev_info->iidc_priv;
> +	if (!privd || !pf->vsi[0] || !pf->vsi[0]->netdev)
> +		return;
> +
> +	/* Assign VSI info now that VSI is valid */
> +	privd->netdev = pf->vsi[0]->netdev;
> +	privd->vport_id = pf->vsi[0]->vsi_num;
> +
> +	/* Update QoS info after DCB has been rebuilt */
> +	ice_setup_dcb_qos_info(pf, &privd->qos_info);
> +
> +	ice_plug_aux_dev(pf);
The ice_plug_aux_dev()  returns error code, but it's  being ignored.
Can you add logging or comment the reason in the code why?
Thank you

> +}
> +
>  /**
>   * ice_init_rdma - initializes PF for RDMA use
>   * @pf: ptr to ice_pf
> @@ -398,23 +427,16 @@ int ice_init_rdma(struct ice_pf *pf)
>  	}
> 
>  	cdev->iidc_priv = privd;
> -	privd->netdev = pf->vsi[0]->netdev;
> 
>  	privd->hw_addr = (u8 __iomem *)pf->hw.hw_addr;
>  	cdev->pdev = pf->pdev;
> -	privd->vport_id = pf->vsi[0]->vsi_num;
> 
>  	pf->cdev_info->rdma_protocol |= IIDC_RDMA_PROTOCOL_ROCEV2;
> -	ice_setup_dcb_qos_info(pf, &privd->qos_info);
> -	ret = ice_plug_aux_dev(pf);
> -	if (ret)
> -		goto err_plug_aux_dev;
> +
>  	return 0;
> 
> -err_plug_aux_dev:
> -	pf->cdev_info->adev = NULL;
> -	xa_erase(&ice_aux_id, pf->aux_idx);
>  err_alloc_xa:
> +	xa_erase(&ice_aux_id, pf->aux_idx);
>  	kfree(privd);
>  err_privd_alloc:
>  	kfree(cdev);
> @@ -432,7 +454,6 @@ void ice_deinit_rdma(struct ice_pf *pf)
>  	if (!ice_is_rdma_ena(pf))
>  		return;
> 
> -	ice_unplug_aux_dev(pf);
>  	xa_erase(&ice_aux_id, pf->aux_idx);
>  	kfree(pf->cdev_info->iidc_priv);
>  	kfree(pf->cdev_info);
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c
> b/drivers/net/ethernet/intel/ice/ice_main.c
> index 4bb68e7a00f5f..1851e9932cefe 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -5135,6 +5135,9 @@ int ice_load(struct ice_pf *pf)
>  	if (err)
>  		goto err_init_rdma;
> 
> +	/* Finalize RDMA: VSI already created, assign info and plug
> device */
> +	ice_rdma_finalize_setup(pf);
> +
>  	ice_service_task_restart(pf);
> 
>  	clear_bit(ICE_DOWN, pf->state);
> @@ -5166,6 +5169,7 @@ void ice_unload(struct ice_pf *pf)
> 
>  	devl_assert_locked(priv_to_devlink(pf));
> 
> +	ice_unplug_aux_dev(pf);
>  	ice_deinit_rdma(pf);
>  	ice_deinit_features(pf);
>  	ice_tc_indir_block_unregister(vsi);
> @@ -5594,6 +5598,7 @@ static int ice_suspend(struct device *dev)
>  	 */
>  	disabled = ice_service_task_stop(pf);
> 
> +	ice_unplug_aux_dev(pf);
>  	ice_deinit_rdma(pf);
> 
>  	/* Already suspended?, then there is nothing to do */ @@ -
> 7803,7 +7808,7 @@ static void ice_rebuild(struct ice_pf *pf, enum
> ice_reset_req reset_type)
> 
>  	ice_health_clear(pf);
> 
> -	ice_plug_aux_dev(pf);
> +	ice_rdma_finalize_setup(pf);
>  	if (ice_is_feature_supported(pf, ICE_F_SRIOV_LAG))
>  		ice_lag_rebuild(pf);
> 
> --
> 2.43.0

Re: [PATCH v3 2/2] ice: recap the VSI and QoS info after rebuild

[Tony Nguyen]

On 12/24/2025 10:21 PM, Aaron Ma wrote:
> Fix IRDMA hardware initialization timeout (-110) after resume by
> separating VSI-dependent configuration from RDMA resource allocation,
> ensuring VSI is rebuilt before IRDMA accesses it.
> 
> After resume from suspend, IRDMA hardware initialization fails:
>    ice: IRDMA hardware initialization FAILED init_state=4 status=-110
> 
> Separate RDMA initialization into two phases:
> 1. ice_init_rdma() - Allocate resources only (no VSI/QoS access, no plug)
> 2. ice_rdma_finalize_setup() - Assign VSI/QoS info and plug device
> 
> This allows:
> - ice_init_rdma() to stay in ice_resume() (mirrors ice_deinit_rdma()
>    in ice_suspend()
> - VSI assignment deferred until after ice_vsi_rebuild() completes
> - QoS info updated after ice_dcb_rebuild() completes
> - Device plugged only when control queues, VSI, and DCB are all ready
Hi Aaron,

Sorry for the late feedback, but I'm working on getting AI Review in 
place and when I ran it against this path it flagged a couple of things...

> Fixes: bc69ad74867db ("ice: avoid IRQ collision to fix init failure on ACPI S3 resume")
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
> ---
> V1 -> V2: no changes.
> V2 -> V3:
> - mirrors init_rdma in resume as Tony Nguyen suggested to fix
> the memleak and move ice_plug_aux_dev/ice_unplug_aux_dev out of
> init/deinit rdma.
> - ensure the correct VSI/QoS info is loaded after rebuild.
> 
>   drivers/net/ethernet/intel/ice/ice.h      |  1 +
>   drivers/net/ethernet/intel/ice/ice_idc.c  | 41 +++++++++++++++++------
>   drivers/net/ethernet/intel/ice/ice_main.c |  7 +++-
>   3 files changed, 38 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
> index 147aaee192a79..6463c1fea7871 100644
> --- a/drivers/net/ethernet/intel/ice/ice.h
> +++ b/drivers/net/ethernet/intel/ice/ice.h
> @@ -989,6 +989,7 @@ int ice_schedule_reset(struct ice_pf *pf, enum ice_reset_req reset);
>   void ice_print_link_msg(struct ice_vsi *vsi, bool isup);
>   int ice_plug_aux_dev(struct ice_pf *pf);
>   void ice_unplug_aux_dev(struct ice_pf *pf);
> +void ice_rdma_finalize_setup(struct ice_pf *pf);
>   int ice_init_rdma(struct ice_pf *pf);
>   void ice_deinit_rdma(struct ice_pf *pf);
>   bool ice_is_wol_supported(struct ice_hw *hw);
> diff --git a/drivers/net/ethernet/intel/ice/ice_idc.c b/drivers/net/ethernet/intel/ice/ice_idc.c
> index 420d45c2558b6..b6079a6cb7736 100644
> --- a/drivers/net/ethernet/intel/ice/ice_idc.c
> +++ b/drivers/net/ethernet/intel/ice/ice_idc.c
> @@ -360,6 +360,35 @@ void ice_unplug_aux_dev(struct ice_pf *pf)
>   	auxiliary_device_uninit(adev);
>   }
>   
> +/**
> + * ice_rdma_finalize_setup - Complete RDMA setup after VSI is ready
> + * @pf: ptr to ice_pf
> + *
> + * Sets VSI-dependent information and plugs aux device.
> + * Must be called after ice_init_rdma(), ice_vsi_rebuild(), and
> + * ice_dcb_rebuild() complete.
> + */
> +void ice_rdma_finalize_setup(struct ice_pf *pf)
> +{
> +	struct iidc_rdma_priv_dev_info *privd;
> +
> +	if (!ice_is_rdma_ena(pf) || !pf->cdev_info)
> +		return;
> +
> +	privd = pf->cdev_info->iidc_priv;
> +	if (!privd || !pf->vsi[0] || !pf->vsi[0]->netdev)
> +		return;
> +
> +	/* Assign VSI info now that VSI is valid */
> +	privd->netdev = pf->vsi[0]->netdev;
> +	privd->vport_id = pf->vsi[0]->vsi_num;
> +
> +	/* Update QoS info after DCB has been rebuilt */
> +	ice_setup_dcb_qos_info(pf, &privd->qos_info);
> +
> +	ice_plug_aux_dev(pf);

"
Does making ice_rdma_finalize_setup() void lose error reporting? The
old ice_init_rdma() called ice_plug_aux_dev() and propagated errors to
callers with error cleanup. Now ice_plug_aux_dev() returns int but
ice_rdma_finalize_setup() is void, so the return value is ignored.

If ice_plug_aux_dev() fails (kzalloc failure, auxiliary_device_init
failure, or auxiliary_device_add failure), the error is silently
ignored and RDMA functionality won't be available. The old code would
return an error that ice_load() and ice_resume() would log, helping
users understand why RDMA isn't working.

Should ice_rdma_finalize_setup() return int and propagate the error, or
at least log a message when ice_plug_aux_dev() fails?
"

I agree that logging a message would be helpful to inform the user. 
Either here or propagating it and printing a differentiated messages 
would be nice.

> +}
> +
>   /**
>    * ice_init_rdma - initializes PF for RDMA use
>    * @pf: ptr to ice_pf
> @@ -398,23 +427,16 @@ int ice_init_rdma(struct ice_pf *pf)
>   	}
>   
>   	cdev->iidc_priv = privd;
> -	privd->netdev = pf->vsi[0]->netdev;
>   
>   	privd->hw_addr = (u8 __iomem *)pf->hw.hw_addr;
>   	cdev->pdev = pf->pdev;
> -	privd->vport_id = pf->vsi[0]->vsi_num;
>   
>   	pf->cdev_info->rdma_protocol |= IIDC_RDMA_PROTOCOL_ROCEV2;
> -	ice_setup_dcb_qos_info(pf, &privd->qos_info);
> -	ret = ice_plug_aux_dev(pf);
> -	if (ret)
> -		goto err_plug_aux_dev;
> +
>   	return 0;
>   
> -err_plug_aux_dev:
> -	pf->cdev_info->adev = NULL;
> -	xa_erase(&ice_aux_id, pf->aux_idx);
>   err_alloc_xa:
> +	xa_erase(&ice_aux_id, pf->aux_idx);

"
Does this error path call xa_erase() with an uninitialized pf->aux_idx?

When xa_alloc() fails at line 423, the code jumps to err_alloc_xa which
now calls xa_erase(). But if xa_alloc() failed, nothing was allocated
into the xarray, and pf->aux_idx contains an undefined value from the
failed xa_alloc() call. Calling xa_erase() with this undefined index
seems incorrect.

In the original code, the err_plug_aux_dev label came after successful
xa_alloc(), so xa_erase() was only called when something was actually
allocated. The err_alloc_xa label came after err_plug_aux_dev and fell
through without calling xa_erase().
"

Semi-related. I sent patch 1 of this series on since it seems like it 
can be independent of this one so no need to carry that one with this.

Thanks,
Tony

>   	kfree(privd);
>   err_privd_alloc:
>   	kfree(cdev);

Re: [PATCH v3 2/2] ice: recap the VSI and QoS info after rebuild

[Aaron Ma]

On Wed, Jan 28, 2026 at 6:32 AM Tony Nguyen <anthony.l.nguyen@intel.com> wrote:
>
>
>
> On 12/24/2025 10:21 PM, Aaron Ma wrote:
> > Fix IRDMA hardware initialization timeout (-110) after resume by
> > separating VSI-dependent configuration from RDMA resource allocation,
> > ensuring VSI is rebuilt before IRDMA accesses it.
> >
> > After resume from suspend, IRDMA hardware initialization fails:
> >    ice: IRDMA hardware initialization FAILED init_state=4 status=-110
> >
> > Separate RDMA initialization into two phases:
> > 1. ice_init_rdma() - Allocate resources only (no VSI/QoS access, no plug)
> > 2. ice_rdma_finalize_setup() - Assign VSI/QoS info and plug device
> >
> > This allows:
> > - ice_init_rdma() to stay in ice_resume() (mirrors ice_deinit_rdma()
> >    in ice_suspend()
> > - VSI assignment deferred until after ice_vsi_rebuild() completes
> > - QoS info updated after ice_dcb_rebuild() completes
> > - Device plugged only when control queues, VSI, and DCB are all ready
> Hi Aaron,
>
> Sorry for the late feedback, but I'm working on getting AI Review in
> place and when I ran it against this path it flagged a couple of things...
>
> > Fixes: bc69ad74867db ("ice: avoid IRQ collision to fix init failure on ACPI S3 resume")
> > Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> > Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
> > ---
> > V1 -> V2: no changes.
> > V2 -> V3:
> > - mirrors init_rdma in resume as Tony Nguyen suggested to fix
> > the memleak and move ice_plug_aux_dev/ice_unplug_aux_dev out of
> > init/deinit rdma.
> > - ensure the correct VSI/QoS info is loaded after rebuild.
> >
> >   drivers/net/ethernet/intel/ice/ice.h      |  1 +
> >   drivers/net/ethernet/intel/ice/ice_idc.c  | 41 +++++++++++++++++------
> >   drivers/net/ethernet/intel/ice/ice_main.c |  7 +++-
> >   3 files changed, 38 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
> > index 147aaee192a79..6463c1fea7871 100644
> > --- a/drivers/net/ethernet/intel/ice/ice.h
> > +++ b/drivers/net/ethernet/intel/ice/ice.h
> > @@ -989,6 +989,7 @@ int ice_schedule_reset(struct ice_pf *pf, enum ice_reset_req reset);
> >   void ice_print_link_msg(struct ice_vsi *vsi, bool isup);
> >   int ice_plug_aux_dev(struct ice_pf *pf);
> >   void ice_unplug_aux_dev(struct ice_pf *pf);
> > +void ice_rdma_finalize_setup(struct ice_pf *pf);
> >   int ice_init_rdma(struct ice_pf *pf);
> >   void ice_deinit_rdma(struct ice_pf *pf);
> >   bool ice_is_wol_supported(struct ice_hw *hw);
> > diff --git a/drivers/net/ethernet/intel/ice/ice_idc.c b/drivers/net/ethernet/intel/ice/ice_idc.c
> > index 420d45c2558b6..b6079a6cb7736 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_idc.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_idc.c
> > @@ -360,6 +360,35 @@ void ice_unplug_aux_dev(struct ice_pf *pf)
> >       auxiliary_device_uninit(adev);
> >   }
> >
> > +/**
> > + * ice_rdma_finalize_setup - Complete RDMA setup after VSI is ready
> > + * @pf: ptr to ice_pf
> > + *
> > + * Sets VSI-dependent information and plugs aux device.
> > + * Must be called after ice_init_rdma(), ice_vsi_rebuild(), and
> > + * ice_dcb_rebuild() complete.
> > + */
> > +void ice_rdma_finalize_setup(struct ice_pf *pf)
> > +{
> > +     struct iidc_rdma_priv_dev_info *privd;
> > +
> > +     if (!ice_is_rdma_ena(pf) || !pf->cdev_info)
> > +             return;
> > +
> > +     privd = pf->cdev_info->iidc_priv;
> > +     if (!privd || !pf->vsi[0] || !pf->vsi[0]->netdev)

Aleksandr raised this change to:
  if (!privd || !pf->vsi || !pf->vsi[0] || !pf->vsi[0]->netdev)

I will add this in v4.

> > +             return;
> > +
> > +     /* Assign VSI info now that VSI is valid */
> > +     privd->netdev = pf->vsi[0]->netdev;
> > +     privd->vport_id = pf->vsi[0]->vsi_num;
> > +
> > +     /* Update QoS info after DCB has been rebuilt */
> > +     ice_setup_dcb_qos_info(pf, &privd->qos_info);
> > +
> > +     ice_plug_aux_dev(pf);
>
> "
> Does making ice_rdma_finalize_setup() void lose error reporting? The
> old ice_init_rdma() called ice_plug_aux_dev() and propagated errors to
> callers with error cleanup. Now ice_plug_aux_dev() returns int but
> ice_rdma_finalize_setup() is void, so the return value is ignored.
>
> If ice_plug_aux_dev() fails (kzalloc failure, auxiliary_device_init
> failure, or auxiliary_device_add failure), the error is silently
> ignored and RDMA functionality won't be available. The old code would
> return an error that ice_load() and ice_resume() would log, helping
> users understand why RDMA isn't working.
>
> Should ice_rdma_finalize_setup() return int and propagate the error, or
> at least log a message when ice_plug_aux_dev() fails?
> "
>
> I agree that logging a message would be helpful to inform the user.
> Either here or propagating it and printing a differentiated messages
> would be nice.

Aleksandr also raised this.
Since ice_rebuild() which called this function can't propagate errors,
so I think logging a dev_warn() when ice_plug_aux_dev() fails is more
appropriate.

>
> > +}
> > +
> >   /**
> >    * ice_init_rdma - initializes PF for RDMA use
> >    * @pf: ptr to ice_pf
> > @@ -398,23 +427,16 @@ int ice_init_rdma(struct ice_pf *pf)
> >       }
> >
> >       cdev->iidc_priv = privd;
> > -     privd->netdev = pf->vsi[0]->netdev;
> >
> >       privd->hw_addr = (u8 __iomem *)pf->hw.hw_addr;
> >       cdev->pdev = pf->pdev;
> > -     privd->vport_id = pf->vsi[0]->vsi_num;
> >
> >       pf->cdev_info->rdma_protocol |= IIDC_RDMA_PROTOCOL_ROCEV2;
> > -     ice_setup_dcb_qos_info(pf, &privd->qos_info);
> > -     ret = ice_plug_aux_dev(pf);
> > -     if (ret)
> > -             goto err_plug_aux_dev;
> > +
> >       return 0;
> >
> > -err_plug_aux_dev:
> > -     pf->cdev_info->adev = NULL;
> > -     xa_erase(&ice_aux_id, pf->aux_idx);
> >   err_alloc_xa:
> > +     xa_erase(&ice_aux_id, pf->aux_idx);
>
> "
> Does this error path call xa_erase() with an uninitialized pf->aux_idx?
>
> When xa_alloc() fails at line 423, the code jumps to err_alloc_xa which
> now calls xa_erase(). But if xa_alloc() failed, nothing was allocated
> into the xarray, and pf->aux_idx contains an undefined value from the
> failed xa_alloc() call. Calling xa_erase() with this undefined index
> seems incorrect.
>
> In the original code, the err_plug_aux_dev label came after successful
> xa_alloc(), so xa_erase() was only called when something was actually
> allocated. The err_alloc_xa label came after err_plug_aux_dev and fell
> through without calling xa_erase().
> "
>

Right, after xa_alloc failed, it should not call xa_rease, I will remove it.

I will send v4 patch to fix 3 issues above.
Thanks for the detail review.

> Semi-related. I sent patch 1 of this series on since it seems like it
> can be independent of this one so no need to carry that one with this.
>

Thank you very much.
Aaron

> Thanks,
> Tony
>
> >       kfree(privd);
> >   err_privd_alloc:
> >       kfree(cdev);