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
