> -----Original Message-----
> From: Ye, Xiaolong <xiaolong...@intel.com>
> Sent: Friday, March 13, 2020 16:00
> To: Wang, Haiyue <haiyue.w...@intel.com>
> Cc: dev@dpdk.org; Zhang, Qi Z <qi.z.zh...@intel.com>; Yang, Qiming
> <qiming.y...@intel.com>; Xing,
> Beilei <beilei.x...@intel.com>; Zhao1, Wei <wei.zh...@intel.com>
> Subject: Re: [PATCH v2 6/7] net/ice: handle the PF initialization by DCF
>
> On 03/10, Haiyue Wang wrote:
> >The DCF (Device Config Function) works at the user PF level, it can't
> >access the real PF hardware directly. So it will proxy the PF's AdminQ
> >command through DCF's mailbox.
> >
> >And the DCF is mainly used to control the flow setting of other VFs, so
> >it only needs to initialize some core functions about the flow .
> >
> >Signed-off-by: Haiyue Wang <haiyue.w...@intel.com>
> >---
> > drivers/net/ice/Makefile | 1 +
> > drivers/net/ice/ice_dcf.c | 7 +-
> > drivers/net/ice/ice_dcf.h | 3 +
> > drivers/net/ice/ice_dcf_ethdev.c | 10 +-
> > drivers/net/ice/ice_dcf_ethdev.h | 11 +-
> > drivers/net/ice/ice_dcf_parent.c | 263 +++++++++++++++++++++++++++++++
> > drivers/net/ice/meson.build | 3 +-
> > 7 files changed, 292 insertions(+), 6 deletions(-)
> > create mode 100644 drivers/net/ice/ice_dcf_parent.c
> >
> >diff --git a/drivers/net/ice/Makefile b/drivers/net/ice/Makefile
> >index f493c9ed7..3ecc72219 100644
> >--- a/drivers/net/ice/Makefile
> >+++ b/drivers/net/ice/Makefile
> >@@ -88,6 +88,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_ICE_PMD) += ice_generic_flow.c
> >
> > SRCS-$(CONFIG_RTE_LIBRTE_ICE_PMD) += ice_dcf.c
> > SRCS-$(CONFIG_RTE_LIBRTE_ICE_PMD) += ice_dcf_ethdev.c
> >+SRCS-$(CONFIG_RTE_LIBRTE_ICE_PMD) += ice_dcf_parent.c
> >
> > # install this header file
> > SYMLINK-$(CONFIG_RTE_LIBRTE_ICE_PMD)-include := rte_pmd_ice.h
> >diff --git a/drivers/net/ice/ice_dcf.c b/drivers/net/ice/ice_dcf.c
> >index 24ed31f35..480c449f5 100644
> >--- a/drivers/net/ice/ice_dcf.c
> >+++ b/drivers/net/ice/ice_dcf.c
> >@@ -124,8 +124,13 @@ ice_dcf_aq_cmd_handle(struct ice_dcf_hw *hw, struct
> >iavf_arq_event_info *info)
> > }
> >
> > v_op = rte_le_to_cpu_32(info->desc.cookie_high);
> >- if (unlikely(v_op == VIRTCHNL_OP_EVENT))
> >+ if (unlikely(v_op == VIRTCHNL_OP_EVENT)) {
>
> Is this unlikely really needed?
>
Looks like 'unlikely' a little abuse. Will remove it in v3 ;-0
> >+ if (hw->vc_event_msg_cb != NULL)
> >+ hw->vc_event_msg_cb(hw,
> >+ info->msg_buf,
> >+ info->msg_len);
> > return;
> >+ }
> >
> > v_ret = rte_le_to_cpu_32(info->desc.cookie_low);
> >
> >diff --git a/drivers/net/ice/ice_dcf.h b/drivers/net/ice/ice_dcf.h
> >index 99bd53b02..ecd6303a0 100644
> >--- a/drivers/net/ice/ice_dcf.h
> >+++ b/drivers/net/ice/ice_dcf.h
> >@@ -36,6 +36,9 @@ struct ice_dcf_hw {
> > rte_spinlock_t vc_cmd_send_lock;
> > rte_spinlock_t vc_cmd_queue_lock;
> > TAILQ_HEAD(, dcf_virtchnl_cmd) vc_cmd_queue;
> >+ void (*vc_event_msg_cb)(struct ice_dcf_hw *dcf_hw,
> >+ uint8_t *msg, uint16_t msglen);
> >+
> > uint8_t *arq_buf;
> >
> > struct virtchnl_version_info virtchnl_version;
> >diff --git a/drivers/net/ice/ice_dcf_ethdev.c
> >b/drivers/net/ice/ice_dcf_ethdev.c
> >index 23f82a487..af94caeff 100644
> >--- a/drivers/net/ice/ice_dcf_ethdev.c
> >+++ b/drivers/net/ice/ice_dcf_ethdev.c
> >@@ -145,8 +145,8 @@ ice_dcf_dev_close(struct rte_eth_dev *dev)
> > dev->dev_ops = NULL;
> > dev->rx_pkt_burst = NULL;
> > dev->tx_pkt_burst = NULL;
> >- dev->data->mac_addrs = NULL;
> >
> >+ ice_dcf_uninit_parent_adapter(dev);
> > ice_dcf_uninit_hw(dev, &adapter->real_hw);
> > }
> >
> >@@ -225,13 +225,17 @@ ice_dcf_dev_init(struct rte_eth_dev *eth_dev)
> >
> > eth_dev->data->dev_flags |= RTE_ETH_DEV_CLOSE_REMOVE;
> >
> >+ adapter->real_hw.vc_event_msg_cb = ice_dcf_handle_pf_event_msg;
> > if (ice_dcf_init_hw(eth_dev, &adapter->real_hw) != 0) {
> > PMD_INIT_LOG(ERR, "Failed to init DCF hardware");
> > return -1;
> > }
> >
> >- rte_eth_random_addr(adapter->mac_addr.addr_bytes);
> >- eth_dev->data->mac_addrs = &adapter->mac_addr;
> >+ if (ice_dcf_init_parent_adapter(eth_dev) != 0) {
> >+ PMD_INIT_LOG(ERR, "Failed to init DCF parent adapter");
> >+ ice_dcf_uninit_hw(eth_dev, &adapter->real_hw);
> >+ return -1;
> >+ }
> >
> > return 0;
> > }
> >diff --git a/drivers/net/ice/ice_dcf_ethdev.h
> >b/drivers/net/ice/ice_dcf_ethdev.h
> >index 0c34a0095..e60e808d8 100644
> >--- a/drivers/net/ice/ice_dcf_ethdev.h
> >+++ b/drivers/net/ice/ice_dcf_ethdev.h
> >@@ -5,6 +5,9 @@
> > #ifndef _ICE_DCF_ETHDEV_H_
> > #define _ICE_DCF_ETHDEV_H_
> >
> >+#include "base/ice_common.h"
> >+#include "base/ice_adminq_cmd.h"
> >+
> > #include "ice_ethdev.h"
> > #include "ice_dcf.h"
> >
> >@@ -15,10 +18,16 @@ struct ice_dcf_queue {
> > };
> >
> > struct ice_dcf_adapter {
> >+ struct ice_adapter parent; /* Must be first */
> >+
> > struct ice_dcf_hw real_hw;
> >- struct rte_ether_addr mac_addr;
>
> This one isn't needed at the first place.
>
> > struct ice_dcf_queue rxqs[ICE_DCF_MAX_RINGS];
> > struct ice_dcf_queue txqs[ICE_DCF_MAX_RINGS];
> > };
> >
> >+void ice_dcf_handle_pf_event_msg(struct ice_dcf_hw *dcf_hw,
> >+ uint8_t *msg, uint16_t msglen);
> >+int ice_dcf_init_parent_adapter(struct rte_eth_dev *eth_dev);
> >+void ice_dcf_uninit_parent_adapter(struct rte_eth_dev *eth_dev);
> >+
> > #endif /* _ICE_DCF_ETHDEV_H_ */
> >diff --git a/drivers/net/ice/ice_dcf_parent.c
> >b/drivers/net/ice/ice_dcf_parent.c
> >new file mode 100644
> >index 000000000..4c3bb68b1
> >--- /dev/null
> >+++ b/drivers/net/ice/ice_dcf_parent.c
> >@@ -0,0 +1,263 @@
> >+/* SPDX-License-Identifier: BSD-3-Clause
> >+ * Copyright(c) 2020 Intel Corporation
> >+ */
> >+#include <sys/types.h>
> >+#include <sys/stat.h>
> >+#include <unistd.h>
> >+
> >+#include "ice_dcf_ethdev.h"
> >+
> >+void
> >+ice_dcf_handle_pf_event_msg(__rte_unused struct ice_dcf_hw *dcf_hw,
> >+ uint8_t *msg, uint16_t msglen)
> >+{
> >+ struct virtchnl_pf_event *pf_msg = (struct virtchnl_pf_event *)msg;
> >+
> >+ if (msglen < sizeof(struct virtchnl_pf_event)) {
> >+ PMD_DRV_LOG(DEBUG, "Invalid event message length : %u", msglen);
> >+ return;
> >+ }
> >+
> >+ switch (pf_msg->event) {
> >+ case VIRTCHNL_EVENT_RESET_IMPENDING:
> >+ PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_RESET_IMPENDING event");
> >+ break;
> >+ case VIRTCHNL_EVENT_LINK_CHANGE:
> >+ PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_LINK_CHANGE event");
> >+ break;
> >+ case VIRTCHNL_EVENT_PF_DRIVER_CLOSE:
> >+ PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_PF_DRIVER_CLOSE event");
> >+ break;
> >+ default:
> >+ PMD_DRV_LOG(ERR, "Unknown event received %u", pf_msg->event);
> >+ break;
> >+ }
> >+}
> >+
> >+static int
> >+ice_dcf_init_parent_hw(struct ice_hw *hw)
> >+{
> >+ struct ice_aqc_get_phy_caps_data *pcaps;
> >+ enum ice_status status;
> >+
> >+ status = ice_aq_get_fw_ver(hw, NULL);
> >+ if (status)
> >+ return status;
> >+
> >+ status = ice_get_caps(hw);
> >+ if (status)
> >+ return status;
> >+
> >+ hw->port_info = (struct ice_port_info *)
> >+ ice_malloc(hw, sizeof(*hw->port_info));
> >+ if (!hw->port_info)
> >+ return ICE_ERR_NO_MEMORY;
> >+
> >+ /* set the back pointer to HW */
> >+ hw->port_info->hw = hw;
> >+
> >+ /* Initialize port_info struct with switch configuration data */
> >+ status = ice_get_initial_sw_cfg(hw);
> >+ if (status)
> >+ goto err_unroll_alloc;
> >+
> >+ pcaps = (struct ice_aqc_get_phy_caps_data *)
> >+ ice_malloc(hw, sizeof(*pcaps));
> >+ if (!pcaps) {
> >+ status = ICE_ERR_NO_MEMORY;
> >+ goto err_unroll_alloc;
> >+ }
> >+
> >+ /* Initialize port_info struct with PHY capabilities */
> >+ status = ice_aq_get_phy_caps(hw->port_info, false,
> >+ ICE_AQC_REPORT_TOPO_CAP, pcaps, NULL);
> >+ ice_free(hw, pcaps);
> >+ if (status)
> >+ goto err_unroll_alloc;
> >+
> >+ /* Initialize port_info struct with link information */
> >+ status = ice_aq_get_link_info(hw->port_info, false, NULL, NULL);
> >+ if (status)
> >+ goto err_unroll_alloc;
> >+
> >+ status = ice_init_fltr_mgmt_struct(hw);
> >+ if (status)
> >+ goto err_unroll_alloc;
> >+
> >+ status = ice_init_hw_tbls(hw);
> >+ if (status)
> >+ goto err_unroll_fltr_mgmt_struct;
> >+
> >+ PMD_INIT_LOG(INFO,
> >+ "firmware %d.%d.%d api %d.%d.%d build 0x%08x",
> >+ hw->fw_maj_ver, hw->fw_min_ver, hw->fw_patch,
> >+ hw->api_maj_ver, hw->api_min_ver, hw->api_patch,
> >+ hw->fw_build);
> >+
> >+ return ICE_SUCCESS;
> >+
> >+err_unroll_fltr_mgmt_struct:
> >+ ice_cleanup_fltr_mgmt_struct(hw);
> >+err_unroll_alloc:
> >+ ice_free(hw, hw->port_info);
> >+ hw->port_info = NULL;
> >+
> >+ return status;
> >+}
> >+
> >+static void ice_dcf_uninit_parent_hw(struct ice_hw *hw)
> >+{
> >+ ice_cleanup_fltr_mgmt_struct(hw);
> >+
> >+ ice_free_seg(hw);
> >+ ice_free_hw_tbls(hw);
> >+
> >+ ice_free(hw, hw->port_info);
> >+ hw->port_info = NULL;
> >+
> >+ ice_clear_all_vsi_ctx(hw);
> >+}
> >+
> >+static int
> >+ice_dcf_request_pkg_name(struct ice_hw *hw, char *pkg_name)
> >+{
> >+ struct ice_dcf_adapter *dcf_adapter =
> >+ container_of(hw, struct ice_dcf_adapter, parent.hw);
> >+
> >+ /* TODO: check with DSN firstly by iAVF */
> >+ PMD_INIT_LOG(DEBUG,
> >+ "DCF VSI_ID = %u",
> >+ dcf_adapter->real_hw.vsi_id);
> >+
> >+ snprintf(pkg_name,
> >+ ICE_MAX_PKG_FILENAME_SIZE, "%s", ICE_PKG_FILE_UPDATES);
> >+ if (!access(pkg_name, 0))
> >+ return 0;
> >+
> >+ snprintf(pkg_name,
> >+ ICE_MAX_PKG_FILENAME_SIZE, "%s", ICE_PKG_FILE_DEFAULT);
> >+ if (!access(pkg_name, 0))
> >+ return 0;
> >+
> >+ return -1;
> >+}
> >+
> >+static int
> >+ice_dcf_load_pkg(struct ice_hw *hw)
> >+{
> >+ char pkg_name[ICE_MAX_PKG_FILENAME_SIZE];
> >+ uint8_t *pkg_buf;
> >+ uint32_t buf_len;
> >+ struct stat st;
> >+ FILE *fp;
> >+ int err;
> >+
> >+ if (ice_dcf_request_pkg_name(hw, pkg_name)) {
> >+ PMD_INIT_LOG(ERR, "failed to locate the package file");
>
> s/failed/Failed
>
Fixed in v3.
> >+ return -ENOENT;
> >+ }
> >+
> >+ PMD_INIT_LOG(DEBUG, "DDP package name: %s", pkg_name);
> >+
> >+ err = stat(pkg_name, &st);
> >+ if (err) {
> >+ PMD_INIT_LOG(ERR, "failed to get file status");
>
> s/failed/Failed
>
Fixed in v3.
> >+ return err;
> >+ }
> >+
> >+ buf_len = st.st_size;
> >+ pkg_buf = rte_malloc(NULL, buf_len, 0);
> >+ if (!pkg_buf) {
> >+ PMD_INIT_LOG(ERR, "failed to allocate buffer of size %u for
> >package",
> >+ buf_len);
> >+ return -1;
> >+ }
> >+
> >+ fp = fopen(pkg_name, "rb");
> >+ if (!fp) {
> >+ PMD_INIT_LOG(ERR, "failed to open file: %s", pkg_name);
> >+ err = -1;
> >+ goto ret;
> >+ }
> >+
> >+ err = fread(pkg_buf, buf_len, 1, fp);
> >+ fclose(fp);
> >+ if (err != 1) {
> >+ PMD_INIT_LOG(ERR, "failed to read package data");
> >+ err = -1;
> >+ goto ret;
> >+ }
> >+
> >+ err = ice_copy_and_init_pkg(hw, pkg_buf, buf_len);
> >+ if (err)
> >+ PMD_INIT_LOG(ERR, "ice_copy_and_init_hw failed: %d", err);
> >+
> >+ret:
> >+ rte_free(pkg_buf);
> >+ return err;
> >+}
> >+
> >+int
> >+ice_dcf_init_parent_adapter(struct rte_eth_dev *eth_dev)
> >+{
> >+ struct ice_dcf_adapter *adapter = eth_dev->data->dev_private;
> >+ struct ice_adapter *parent_adapter = &adapter->parent;
> >+ struct ice_hw *parent_hw = &parent_adapter->hw;
> >+ struct ice_dcf_hw *hw = &adapter->real_hw;
> >+ const struct rte_ether_addr *mac;
> >+ int err;
> >+
> >+ parent_adapter->eth_dev = eth_dev;
> >+ parent_adapter->pf.adapter = parent_adapter;
> >+ parent_adapter->pf.dev_data = eth_dev->data;
> >+ parent_hw->back = parent_adapter;
> >+ parent_hw->mac_type = ICE_MAC_GENERIC;
> >+ parent_hw->vendor_id = ICE_INTEL_VENDOR_ID;
> >+
> >+ ice_init_lock(&parent_hw->adminq.sq_lock);
> >+ ice_init_lock(&parent_hw->adminq.rq_lock);
> >+ parent_hw->aq_send_cmd_fn = ice_dcf_send_aq_cmd;
> >+ parent_hw->aq_send_cmd_param = &adapter->real_hw;
> >+ parent_hw->dcf_enabled = true;
> >+
> >+ err = ice_dcf_init_parent_hw(parent_hw);
> >+ if (err) {
> >+ PMD_INIT_LOG(ERR, "failed to init the DCF parent hardware with
> >error %d",
> >+ err);
> >+ return err;
> >+ }
> >+
> >+ err = ice_dcf_load_pkg(parent_hw);
> >+ if (err) {
> >+ PMD_INIT_LOG(ERR, "failed to load package with error %d",
> >+ err);
> >+ goto uninit_hw;
> >+ }
> >+ parent_adapter->active_pkg_type = ice_load_pkg_type(parent_hw);
> >+
> >+ mac = (const struct rte_ether_addr *)hw->avf.mac.addr;
> >+ if (rte_is_valid_assigned_ether_addr(mac))
> >+ rte_ether_addr_copy(mac, &parent_adapter->pf.dev_addr);
> >+ else
> >+ rte_eth_random_addr(parent_adapter->pf.dev_addr.addr_bytes);
> >+
> >+ eth_dev->data->mac_addrs = &parent_adapter->pf.dev_addr;
> >+
> >+ return 0;
> >+
> >+uninit_hw:
> >+ ice_dcf_uninit_parent_hw(parent_hw);
> >+ return err;
> >+}
> >+
> >+void
> >+ice_dcf_uninit_parent_adapter(struct rte_eth_dev *eth_dev)
> >+{
> >+ struct ice_dcf_adapter *adapter = eth_dev->data->dev_private;
> >+ struct ice_adapter *parent_adapter = &adapter->parent;
> >+ struct ice_hw *parent_hw = &parent_adapter->hw;
> >+
> >+ eth_dev->data->mac_addrs = NULL;
> >+
> >+ ice_dcf_uninit_parent_hw(parent_hw);
> >+}
> >diff --git a/drivers/net/ice/meson.build b/drivers/net/ice/meson.build
> >index 0ba9668d1..7e9037f3b 100644
> >--- a/drivers/net/ice/meson.build
> >+++ b/drivers/net/ice/meson.build
> >@@ -38,6 +38,7 @@ if arch_subdir == 'x86'
> > endif
> >
> > sources += files('ice_dcf.c',
> >- 'ice_dcf_ethdev.c')
> >+ 'ice_dcf_ethdev.c',
> >+ 'ice_dcf_parent.c')
> >
> > install_headers('rte_pmd_ice.h')
> >--
> >2.25.1
> >
