Re: [External] : [PATCH net-next v05 1/5] hinic3: Add PF framework

[ALOK TIWARI <alok.a.tiwari@oracle.com>]

> +
> +void hinic3_sync_time_to_fw(struct hinic3_hwdev *hwdev)
> +{
> +	struct timespec64 ts = {};
> +	u64 time;
> +	int err;
> +
> +	ktime_get_real_ts64(&ts);
> +	time = (u64)(ts.tv_sec * MSEC_PER_SEC + ts.tv_nsec / NSEC_PER_MSEC);
> +
> +	err = hinic3_sync_time(hwdev, time);
> +	if (err)
> +		dev_err(hwdev->dev,
> +			"Synchronize UTC time to firmware failed, errno:%d.\n",

what about "failed, err=%d\n" ?

> +			err);
> +}
> +
>   static int get_hw_rx_buf_size_idx(int rx_buf_sz, u16 *buf_sz_idx)
>   {
>   	/* Supported RX buffer sizes in bytes. Configured by array index. */
> diff --git a/drivers/net/ethernet/huawei/hinic3/hinic3_hw_comm.h b/drivers/net/ethernet/huawei/hinic3/hinic3_hw_comm.h
> index 304f5691f0c2..c9c6b4fbcb12 100644
>   
>   	err = hinic3_mapping_bar(pdev, pci_adapter);
> @@ -331,8 +362,24 @@ static int hinic3_probe_func(struct hinic3_pcidev *pci_adapter)
>   	if (err)
>   		goto err_unmap_bar;
>   
> +	if (HINIC3_IS_PF(pci_adapter->hwdev)) {
> +		bdf_info.function_idx =
> +			hinic3_global_func_id(pci_adapter->hwdev);
> +		bdf_info.bus = pdev->bus->number;
> +		bdf_info.device = PCI_SLOT(pdev->devfn);
> +		bdf_info.function =  PCI_FUNC(pdev->devfn);
> +
> +		err = hinic3_set_bdf_ctxt(pci_adapter->hwdev, &bdf_info);
> +		if (err) {
> +			dev_err(&pdev->dev, "Failed to set BDF info to fw\n");
> +			goto err_uninit_func;
> +		}
> +	}
> +
>   	return 0;
>   
> +err_uninit_func:
> +	hinic3_func_uninit(pdev);
>   err_unmap_bar:
>   	hinic3_unmapping_bar(pci_adapter);
>   
> diff --git a/drivers/net/ethernet/huawei/hinic3/hinic3_main.c b/drivers/net/ethernet/huawei/hinic3/hinic3_main.c
> index 6d87d4d895ba..a7c9c5bca53a 100644
> --- a/drivers/net/ethernet/huawei/hinic3/hinic3_main.c
> +++ b/drivers/net/ethernet/huawei/hinic3/hinic3_main.c
> @@ -130,6 +130,7 @@ static int hinic3_sw_init(struct net_device *netdev)
>   {
>   	struct hinic3_nic_dev *nic_dev = netdev_priv(netdev);
>   	struct hinic3_hwdev *hwdev = nic_dev->hwdev;
> +	u8 mac_addr[ETH_ALEN];
>   	int err;
>   
>   	nic_dev->q_params.sq_depth = HINIC3_SQ_DEPTH;
> @@ -137,16 +138,29 @@ static int hinic3_sw_init(struct net_device *netdev)
>   
>   	hinic3_try_to_enable_rss(netdev);
>   
> -	/* VF driver always uses random MAC address. During VM migration to a
> -	 * new device, the new device should learn the VMs old MAC rather than
> -	 * provide its own MAC. The product design assumes that every VF is
> -	 * suspectable to migration so the device avoids offering MAC address
> -	 * to VFs.
> -	 */
> -	eth_hw_addr_random(netdev);
> +	if (HINIC3_IS_VF(hwdev)) {
> +		/* VF driver always uses random MAC address. During VM migration
> +		 * to a new device, the new device should learn the VMs old MAC
> +		 * rather than provide its own MAC. The product design assumes
> +		 * that every VF is suspectable to migration so the device

susceptible ?

> +		 * avoids offering MAC address to VFs.
> +		 */
> +		eth_hw_addr_random(netdev);
> +	} else {
> +		err = hinic3_get_default_mac(hwdev, mac_addr);
> +		if (err) {
> +			dev_err(hwdev->dev, "Failed to get MAC address\n");
> +			goto err_clear_rss_config;
> +		}
> +		eth_hw_addr_set(netdev, mac_addr);
> +	}
> +
>   	err = hinic3_set_mac(hwdev, netdev->dev_addr, 0,
>   			     hinic3_global_func_id(hwdev));
> -	if (err) {
> +	/* Failure to set MAC is not a fatal error for VF since its MAC may have
> +	 * already been set by PF
> +	 */
> +	if (err && err != -EADDRINUSE) {
>   		dev_err(hwdev->dev, "Failed to set default MAC\n");
>   		goto err_clear_rss_config;
>   	}
> diff --git a/drivers/net/ethernet/huawei/hinic3/hinic3_mbox.c b/drivers/net/ethernet/huawei/hinic3/hinic3_mbox.c
> index cf67e26acece..b4e151e88a13 100644
> --- a/drivers/net/ethernet/huawei/hinic3/hinic3_mbox.c
> +++ b/drivers/net/ethernet/huawei/hinic3/hinic3_mbox.c
> @@ -82,10 +82,27 @@ static struct hinic3_msg_desc *get_mbox_msg_desc(struct hinic3_mbox *mbox,
>   						 enum mbox_msg_direction_type dir,
>   						 u16 src_func_id)
>   {
> +	struct hinic3_hwdev *hwdev = mbox->hwdev;
>   	struct hinic3_msg_channel *msg_ch;
> -
> -	msg_ch = (src_func_id == MBOX_MGMT_FUNC_ID) ?
> -		&mbox->mgmt_msg : mbox->func_msg;
> +	u16 id;
> +
> +	if (src_func_id == MBOX_MGMT_FUNC_ID) {
> +		msg_ch = &mbox->mgmt_msg;
> +	} else if (HINIC3_IS_VF(hwdev)) {
> +		/* message from pf */
> +		msg_ch = mbox->func_msg;
> +		if (src_func_id != hinic3_pf_id_of_vf(hwdev) || !msg_ch)
> +			return NULL;
> +	} else if (src_func_id > hinic3_glb_pf_vf_offset(hwdev)) {
> +		/* message from vf */
> +		id = (src_func_id - 1) - hinic3_glb_pf_vf_offset(hwdev);
> +		if (id >= 1)
> +			return NULL;

hard coding id >= 1, is only one VF supported?

> +
> +		msg_ch = &mbox->func_msg[id];
> +	} else {
> +		return NULL;
> +	}
>   
>   	return (dir == MBOX_MSG_SEND) ?
>   		&msg_ch->recv_msg : &msg_ch->resp_msg;
> @@ -409,6 +426,13 @@ int hinic3_init_mbox(struct hinic3_hwdev *hwdev)
>   	if (err)
>   		goto err_destroy_workqueue;
>   
> +	if (HINIC3_IS_VF(hwdev)) {
> +		/* VF to PF mbox message channel */
> +		err = hinic3_init_func_mbox_msg_channel(hwdev);
> +		if (err)
> +			goto err_uninit_mgmt_msg_ch;
> +	}
> +
>   	err = hinic3_init_func_mbox_msg_channel(hwdev);

is hinic3_init_func* second init for PF and
VF executes both calls, is that correct?

>   	if (err)
>   		goto err_uninit_mgmt_msg_ch;
> @@ -424,8 +448,8 @@ int hinic3_init_mbox(struct hinic3_hwdev *hwdev)
>   	return 0;
>   
>   err_uninit_func_mbox_msg_ch:
> -	hinic3_uninit_func_mbox_msg_channel(hwdev);
> -
> +	if (HINIC3_IS_VF(hwdev))
> +		hinic3_uninit_func_mbox_msg_channel(hwdev);
>   err_uninit_mgmt_msg_ch:
>   	uninit_mgmt_msg_channel(mbox);
>   

Thanks,
Alok