RE: [PATCH 07/22] RDMA/irdma: Register an auxiliary driver and implement private channel OPs

Subject: Re: [PATCH 07/22] RDMA/irdma: Register an auxiliary driver and
implement private channel OPs

On Fri, Jan 22, 2021 at 05:48:12PM -0600, Shiraz Saleem wrote:

quoted

+/**
+ * irdma_init_dev - GEN_2 device init
+ * @aux_dev: auxiliary device
+ *
+ * Create device resources, set up queues, pble and hmc objects.
+ * Return 0 if successful, otherwise return error  */ int
+irdma_init_dev(struct auxiliary_device *aux_dev) {
+	struct iidc_auxiliary_object *vo = container_of(aux_dev,
+							struct

iidc_auxiliary_object,

quoted

+							adev);
+	struct iidc_peer_obj *peer_info = vo->peer_obj;
+	struct irdma_handler *hdl;
+	struct irdma_pci_f *rf;
+	struct irdma_sc_dev *dev;
+	struct irdma_priv_peer_info *priv_peer_info;
+	int err;
+
+	hdl = irdma_find_handler(peer_info->pdev);
+	if (hdl)
+		return -EBUSY;
+
+	hdl = kzalloc(sizeof(*hdl), GFP_KERNEL);
+	if (!hdl)
+		return -ENOMEM;
+
+	rf = &hdl->rf;
+	priv_peer_info = &rf->priv_peer_info;
+	rf->aux_dev = aux_dev;
+	rf->hdl = hdl;
+	dev = &rf->sc_dev;
+	dev->back_dev = rf;
+	rf->gen_ops.init_hw = icrdma_init_hw;
+	rf->gen_ops.request_reset = icrdma_request_reset;
+	rf->gen_ops.register_qset = irdma_lan_register_qset;
+	rf->gen_ops.unregister_qset = irdma_lan_unregister_qset;
+	priv_peer_info->peer_info = peer_info;
+	rf->rdma_ver = IRDMA_GEN_2;
+	irdma_set_config_params(rf);
+	dev->pci_rev = peer_info->pdev->revision;
+	rf->default_vsi.vsi_idx = peer_info->pf_vsi_num;
+	/* save information from peer_info to priv_peer_info*/
+	priv_peer_info->fn_num = PCI_FUNC(peer_info->pdev->devfn);
+	rf->hw.hw_addr = peer_info->hw_addr;
+	rf->pcidev = peer_info->pdev;
+	rf->netdev = peer_info->netdev;
+	priv_peer_info->ftype = peer_info->ftype;
+	priv_peer_info->msix_count = peer_info->msix_count;
+	priv_peer_info->msix_entries = peer_info->msix_entries;
+	irdma_add_handler(hdl);
+	if (irdma_ctrl_init_hw(rf)) {
+		err = -EIO;
+		goto err_ctrl_init;
+	}
+	peer_info->peer_ops = &irdma_peer_ops;
+	peer_info->peer_drv = &irdma_peer_drv;
+	err = peer_info->ops->peer_register(peer_info);
+	if (err)
+		goto err_peer_reg;

No to this, I don't want to see aux bus layered on top of another management
framework in new drivers. When this driver uses aux bus get rid of the old i40iw
stuff. I already said this in one of the older postings of this driver.

auxbus probe() for a RDMA driver should call ib_alloc_device() near its start and
ib_register_device() near the end its end.

drvdata for the aux device should point to the driver struct containing the
ib_device.

Just like any ordinary PCI based rdma driver, look at how something like
pvrdma_pci_probe() is structued.

I think this essentially means doing away with .open/.close piece. Or are you saying that is ok?
Yes we had a discussion in the past and I thought we concluded. But maybe I misunderstood.

https://lore.kernel.org/linux-rdma/9DD61F30A802C4429A01CA4200E302A7DCD4FD03@fmsmsx124.amr.corp.intel.com/ (local)

The concern is around losing the resources held for rdma VFs by rdma PF due to underlying config changes which require a de-reg/re-registration of the ibdevice.
Today such config changes are handled with the netdev PCI driver using the .close() private callback into rdma driver which unregister ibdevice in PF while allowing
the RDMA VF to survive.

Shiraz