Re: [PATCH v2 rdma-core] irdma: Add ice and irdma to kernel-boot rules

[Jason Gunthorpe <jgg@nvidia.com>]

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