On Wed, Nov 02, 2022 at 04:40:20PM +0000, Nikolova, Tatyana E wrote: > Hi Jason, > > I know it has been a while since we discussed this. Based on your feedback, we are proposing another solution for the irdma kernel-boot rules. Could you please review it? > > > > udevadm info --attribute-walk /sys/class/infiniband/rocep47s0f0 > > > > > > looking at device > > '/devices/pci0000:2e/0000:2e:00.0/0000:2f:00.0/infiniband/rocep47s0f0': > > > > This looks like the problem. For any of this to work the infiniband > > device needs to be parented to the aux device, not the PCI device. > > > > mlx5 did not due this for backwards compat reasons, but this is a new > > driver so it could do it properly. > > > > Then you can use SUBSYSTEMS and so forth as I first suggested. > > Here is a patch for irdma driver to set aux_dev as the ibdev parent: > > diff --git a/drivers/infiniband/hw/irdma/i40iw_if.c b/drivers/infiniband/hw/irdma/i40iw_if.c > index 4053ead32416..e08fbfe148ea 100644 > --- a/drivers/infiniband/hw/irdma/i40iw_if.c > +++ b/drivers/infiniband/hw/irdma/i40iw_if.c > @@ -90,6 +90,7 @@ static void i40iw_fill_device_info(struct irdma_device *iwdev, struct i40e_info > iwdev->rcv_wnd = IRDMA_CM_DEFAULT_RCV_WND_SCALED; > iwdev->rcv_wscale = IRDMA_CM_DEFAULT_RCV_WND_SCALE; > iwdev->netdev = cdev_info->netdev; > + iwdev->aux_dev = cdev_info->aux_dev; > iwdev->vsi_num = 0; > } > > diff --git a/drivers/infiniband/hw/irdma/main.c b/drivers/infiniband/hw/irdma/main.c > index 514453777e07..835a087a3329 100644 > --- a/drivers/infiniband/hw/irdma/main.c > +++ b/drivers/infiniband/hw/irdma/main.c > @@ -279,6 +279,7 @@ static int irdma_probe(struct auxiliary_device *aux_dev, const struct auxiliary_ > } > > irdma_fill_device_info(iwdev, pf, vsi); > + iwdev->aux_dev = aux_dev; > rf = iwdev->rf; > > err = irdma_ctrl_init_hw(rf); > diff --git a/drivers/infiniband/hw/irdma/main.h b/drivers/infiniband/hw/irdma/main.h > index 65e966ad3453..f2f86b882cef 100644 > --- a/drivers/infiniband/hw/irdma/main.h > +++ b/drivers/infiniband/hw/irdma/main.h > @@ -329,6 +329,7 @@ struct irdma_device { > struct ib_device ibdev; > struct irdma_pci_f *rf; > struct net_device *netdev; > + struct auxiliary_device *aux_dev; > struct workqueue_struct *cleanup_wq; > struct irdma_sc_vsi vsi; > struct irdma_cm_core cm_core; > diff --git a/drivers/infiniband/hw/irdma/verbs.c b/drivers/infiniband/hw/irdma/verbs.c > index a22afbb25bc5..f1304b6b58b3 100644 > --- a/drivers/infiniband/hw/irdma/verbs.c > +++ b/drivers/infiniband/hw/irdma/verbs.c > @@ -4549,7 +4549,6 @@ static int irdma_init_iw_device(struct irdma_device *iwdev) > */ > static int irdma_init_rdma_device(struct irdma_device *iwdev) { > - struct pci_dev *pcidev = iwdev->rf->pcidev; > int ret; > > if (iwdev->roce_mode) { > @@ -4561,7 +4560,7 @@ static int irdma_init_rdma_device(struct irdma_device *iwdev) > } > iwdev->ibdev.phys_port_cnt = 1; > iwdev->ibdev.num_comp_vectors = iwdev->rf->ceqs_count; > - iwdev->ibdev.dev.parent = &pcidev->dev; > + iwdev->ibdev.dev.parent = &iwdev->aux_dev->dev; > ib_set_device_ops(&iwdev->ibdev, &irdma_dev_ops); > > return 0; > > Here is the udev output after this change: > > looking at device > '/devices/pci0000:2e/0000:2e:00.0/0000:2f:00.0/ice.roce.0/infiniband/rocep47s0f0': > KERNEL=="rocep47s0f0" > SUBSYSTEM=="infiniband" > DRIVER=="" > ATTR{fw_ver}=="1.48" > ATTR{node_desc}=="" > ATTR{node_guid}=="6a05:caff:fec1:c790" > ATTR{node_type}=="1: CA" > ATTR{sys_image_guid}=="6a05:caff:fec1:c790" > > looking at parent device > '/devices/pci0000:2e/0000:2e:00.0/0000:2f:00.0/ice.roce.0': > KERNELS=="ice.roce.0" > SUBSYSTEMS=="auxiliary" > DRIVERS=="irdma" > > looking at parent device > '/devices/pci0000:2e/0000:2e:00.0/0000:2f:00.0': > KERNELS=="0000:2f:00.0" > SUBSYSTEMS=="pci" > DRIVERS=="ice" This looks right to me Jason