Re: [PATCH v7 3/8] net/gve: add support for device initialization

[Ferruh Yigit <ferruh.yigit@amd.com>]

On 10/24/2022 6:04 AM, Guo, Junfeng wrote:
> 
> 
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@amd.com>
>> Sent: Friday, October 21, 2022 17:50
>> To: Guo, Junfeng <junfeng.guo@intel.com>; Zhang, Qi Z
>> <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Xing,
>> Beilei <beilei.xing@intel.com>
>> Cc: dev@dpdk.org; Li, Xiaoyun <xiaoyun.li@intel.com>;
>> awogbemila@google.com; Richardson, Bruce
>> <bruce.richardson@intel.com>; hemant.agrawal@nxp.com;
>> stephen@networkplumber.org; Xia, Chenbo <chenbo.xia@intel.com>;
>> Zhang, Helin <helin.zhang@intel.com>; Wang, Haiyue
>> <haiyue.wang@intel.com>
>> Subject: Re: [PATCH v7 3/8] net/gve: add support for device initialization
>>
>> On 10/21/2022 10:19 AM, Junfeng Guo wrote:
>>
>>>
>>> Support device init and add following devops skeleton:
>>>    - dev_configure
>>>    - dev_start
>>>    - dev_stop
>>>    - dev_close
>>>
>>> Note that build system (including doc) is also added in this patch.
>>>
>>> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
>>> Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
>>> Signed-off-by: Junfeng Guo <junfeng.guo@intel.com>
>>
>> <...>
>>
>>> +static int
>>> +gve_dev_init(struct rte_eth_dev *eth_dev)
>>> +{
>>> +       struct gve_priv *priv = eth_dev->data->dev_private;
>>> +       int max_tx_queues, max_rx_queues;
>>> +       struct rte_pci_device *pci_dev;
>>> +       struct gve_registers *reg_bar;
>>> +       rte_be32_t *db_bar;
>>> +       int err;
>>> +
>>> +       eth_dev->dev_ops = &gve_eth_dev_ops;
>>> +
>>> +       if (rte_eal_process_type() != RTE_PROC_PRIMARY)
>>> +               return 0;
>>> +
>>> +       pci_dev = RTE_DEV_TO_PCI(eth_dev->device);
>>> +
>>> +       reg_bar = pci_dev->mem_resource[GVE_REG_BAR].addr;
>>> +       if (!reg_bar) {
>>> +               PMD_DRV_LOG(ERR, "Failed to map pci bar!");
>>> +               return -ENOMEM;
>>> +       }
>>> +
>>> +       db_bar = pci_dev->mem_resource[GVE_DB_BAR].addr;
>>> +       if (!db_bar) {
>>> +               PMD_DRV_LOG(ERR, "Failed to map doorbell bar!");
>>> +               return -ENOMEM;
>>> +       }
>>> +
>>> +       gve_write_version(&reg_bar->driver_version);
>>> +       /* Get max queues to alloc etherdev */
>>> +       max_tx_queues = ioread32be(&reg_bar->max_tx_queues);
>>> +       max_rx_queues = ioread32be(&reg_bar->max_rx_queues);
>>> +
>>> +       priv->reg_bar0 = reg_bar;
>>> +       priv->db_bar2 = db_bar;
>>> +       priv->pci_dev = pci_dev;
>>> +       priv->state_flags = 0x0;
>>> +
>>> +       priv->max_nb_txq = max_tx_queues;
>>> +       priv->max_nb_rxq = max_rx_queues;
>>> +
>>> +       err = gve_init_priv(priv, false);
>>> +       if (err)
>>> +               return err;
>>> +
>>> +       eth_dev->data->mac_addrs = &priv->dev_addr;
>>> +
>>
>> [copy/paste from previous version]
>>
>> What is the value in 'priv->dev_addr'?
>> Even allocating memory for 'eth_dev->data->mac_addrs' removed or not,
>> as
>> we discussed, independent from it, need to set a valid value to
>> 'priv->dev_addr'.
> 
> The value in 'priv->dev_addr' is the 'real' mac address of the gvnic port.
> So I suppose that there is no need to set a default valid one, since we can
> get them/it from the backend in gve_adminq_describe_device(priv).

Ack, thanks for clarification.

In 'gve_adminq_describe_device()', RTE_ETHER_ADDR_PRT_FMT & 
RTE_ETHER_ADDR_BYTES can be used for log, I will comment on patch.