From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Zhang, Helin" Subject: Re: [PATCH v2 1/2] i40evf: allocate virtchnl cmd buffer for each vf Date: Mon, 22 Feb 2016 07:26:29 +0000 Message-ID: References: <1452688307-20213-1-git-send-email-jingjing.wu@intel.com> <1453859378-23912-1-git-send-email-jingjing.wu@intel.com> <1453859378-23912-2-git-send-email-jingjing.wu@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable To: "Wu, Jingjing" , "dev@dpdk.org" Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id B01DE56B7 for ; Mon, 22 Feb 2016 08:26:39 +0100 (CET) In-Reply-To: <1453859378-23912-2-git-send-email-jingjing.wu@intel.com> Content-Language: en-US List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" > -----Original Message----- > From: Wu, Jingjing > Sent: Wednesday, January 27, 2016 9:50 AM > To: dev@dpdk.org > Cc: Wu, Jingjing; Zhang, Helin; Lu, Wenzhuo; Pei, Yulong > Subject: [PATCH v2 1/2] i40evf: allocate virtchnl cmd buffer for each vf >=20 > Currently, i40evf PMD uses a global static buffer to send virtchnl comman= d to > host driver. It is shared by multi VFs. > This patch changed to allocate virtchnl cmd buffer for each VF. >=20 > Signed-off-by: Jingjing Wu > --- > drivers/net/i40e/i40e_ethdev.h | 2 + > drivers/net/i40e/i40e_ethdev_vf.c | 181 +++++++++++++++-----------------= -- > ---- > 2 files changed, 74 insertions(+), 109 deletions(-) >=20 > diff --git a/drivers/net/i40e/i40e_ethdev.h > b/drivers/net/i40e/i40e_ethdev.h index 1f9792b..93122ad 100644 > --- a/drivers/net/i40e/i40e_ethdev.h > +++ b/drivers/net/i40e/i40e_ethdev.h > @@ -494,7 +494,9 @@ struct i40e_vf { > bool link_up; > bool vf_reset; > volatile uint32_t pend_cmd; /* pending command not finished yet */ > + uint32_t cmd_retval; /* return value of the cmd response from PF */ > u16 pend_msg; /* flags indicates events from pf not handled yet */ > + uint8_t *aq_resp; /* buffer to store the adminq response from PF */ >=20 > /* VSI info */ > struct i40e_virtchnl_vf_resource *vf_res; /* All VSIs */ diff --git > a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c > index 14d2a50..64e6957 100644 > --- a/drivers/net/i40e/i40e_ethdev_vf.c > +++ b/drivers/net/i40e/i40e_ethdev_vf.c > @@ -103,9 +103,6 @@ enum i40evf_aq_result { > I40EVF_MSG_CMD, /* Read async command result */ > }; >=20 > -/* A share buffer to store the command result from PF driver */ -static > uint8_t cmd_result_buffer[I40E_AQ_BUF_SZ]; > - > static int i40evf_dev_configure(struct rte_eth_dev *dev); static int > i40evf_dev_start(struct rte_eth_dev *dev); static void > i40evf_dev_stop(struct rte_eth_dev *dev); @@ -237,31 +234,39 @@ > i40evf_set_mac_type(struct i40e_hw *hw) } >=20 > /* > - * Parse admin queue message. > - * > - * return value: > - * < 0: meet error > - * 0: read sys msg > - * > 0: read cmd result > + * Read data in admin queue to get msg from pf driver > */ > static enum i40evf_aq_result > -i40evf_parse_pfmsg(struct i40e_vf *vf, > - struct i40e_arq_event_info *event, > - struct i40evf_arq_msg_info *data) > +i40evf_read_pfmsg(struct rte_eth_dev *dev, struct i40evf_arq_msg_info > +*data) > { > - enum i40e_virtchnl_ops opcode =3D (enum i40e_virtchnl_ops)\ > - rte_le_to_cpu_32(event->desc.cookie_high); > - enum i40e_status_code retval =3D (enum i40e_status_code)\ > - rte_le_to_cpu_32(event->desc.cookie_low); > - enum i40evf_aq_result ret =3D I40EVF_MSG_CMD; > + struct i40e_hw *hw =3D I40E_DEV_PRIVATE_TO_HW(dev->data- > >dev_private); > + struct i40e_vf *vf =3D I40EVF_DEV_PRIVATE_TO_VF(dev->data- > >dev_private); > + struct i40e_arq_event_info event; > + enum i40e_virtchnl_ops opcode; > + enum i40e_status_code retval; > + int ret; > + enum i40evf_aq_result result =3D I40EVF_MSG_NON; >=20 > + event.buf_len =3D data->buf_len; > + event.msg_buf =3D data->msg; > + ret =3D i40e_clean_arq_element(hw, &event, NULL); > + /* Can't read any msg from adminQ */ > + if (ret) { > + if (ret =3D=3D I40E_ERR_ADMIN_QUEUE_NO_WORK) > + result =3D I40EVF_MSG_NON; No need to assign I40EVF_MSG_NON, as it was initialized at first. > + else > + result =3D I40EVF_MSG_ERR; > + return result; > + } > + > + opcode =3D (enum > i40e_virtchnl_ops)rte_le_to_cpu_32(event.desc.cookie_high); > + retval =3D (enum > +i40e_status_code)rte_le_to_cpu_32(event.desc.cookie_low); > /* pf sys event */ > if (opcode =3D=3D I40E_VIRTCHNL_OP_EVENT) { > struct i40e_virtchnl_pf_event *vpe =3D > - (struct i40e_virtchnl_pf_event *)event->msg_buf; > + (struct i40e_virtchnl_pf_event *)event.msg_buf; >=20 > - /* Initialize ret to sys event */ > - ret =3D I40EVF_MSG_SYS; > + result =3D I40EVF_MSG_SYS; > switch (vpe->event) { > case I40E_VIRTCHNL_EVENT_LINK_CHANGE: > vf->link_up =3D > @@ -286,74 +291,17 @@ i40evf_parse_pfmsg(struct i40e_vf *vf, > } > } else { > /* async reply msg on command issued by vf previously */ > - ret =3D I40EVF_MSG_CMD; > + result =3D I40EVF_MSG_CMD; > /* Actual data length read from PF */ > - data->msg_len =3D event->msg_len; > + data->msg_len =3D event.msg_len; > } > - /* fill the ops and result to notify VF */ > + > data->result =3D retval; > data->ops =3D opcode; >=20 > - return ret; > -} > - > -/* > - * Read data in admin queue to get msg from pf driver > - */ > -static enum i40evf_aq_result > -i40evf_read_pfmsg(struct rte_eth_dev *dev, struct i40evf_arq_msg_info > *data) -{ > - struct i40e_hw *hw =3D I40E_DEV_PRIVATE_TO_HW(dev->data- > >dev_private); > - struct i40e_vf *vf =3D I40EVF_DEV_PRIVATE_TO_VF(dev->data- > >dev_private); > - struct i40e_arq_event_info event; > - int ret; > - enum i40evf_aq_result result =3D I40EVF_MSG_NON; > - > - event.buf_len =3D data->buf_len; > - event.msg_buf =3D data->msg; > - ret =3D i40e_clean_arq_element(hw, &event, NULL); > - /* Can't read any msg from adminQ */ > - if (ret) { > - if (ret =3D=3D I40E_ERR_ADMIN_QUEUE_NO_WORK) > - result =3D I40EVF_MSG_NON; > - else > - result =3D I40EVF_MSG_ERR; > - return result; > - } > - > - /* Parse the event */ > - result =3D i40evf_parse_pfmsg(vf, &event, data); > - > return result; > } >=20 > -/* > - * Polling read until command result return from pf driver or meet error= . > - */ > -static int > -i40evf_wait_cmd_done(struct rte_eth_dev *dev, > - struct i40evf_arq_msg_info *data) > -{ > - int i =3D 0; > - enum i40evf_aq_result ret; > - > -#define MAX_TRY_TIMES 20 > -#define ASQ_DELAY_MS 100 > - do { > - /* Delay some time first */ > - rte_delay_ms(ASQ_DELAY_MS); > - ret =3D i40evf_read_pfmsg(dev, data); > - if (ret =3D=3D I40EVF_MSG_CMD) > - return 0; > - else if (ret =3D=3D I40EVF_MSG_ERR) > - return -1; > - > - /* If don't read msg or read sys event, continue */ > - } while(i++ < MAX_TRY_TIMES); > - > - return -1; > -} > - > /** > * clear current command. Only call in case execute > * _atomic_set_cmd successfully. > @@ -380,13 +328,18 @@ _atomic_set_cmd(struct i40e_vf *vf, enum > i40e_virtchnl_ops ops) > return !ret; > } >=20 > +#define MAX_TRY_TIMES 20 > +#define ASQ_DELAY_MS 100 > + > static int > i40evf_execute_vf_cmd(struct rte_eth_dev *dev, struct vf_cmd_info *args) > { > struct i40e_hw *hw =3D I40E_DEV_PRIVATE_TO_HW(dev->data- > >dev_private); > struct i40e_vf *vf =3D I40EVF_DEV_PRIVATE_TO_VF(dev->data- > >dev_private); > - int err =3D -1; > struct i40evf_arq_msg_info info; > + enum i40evf_aq_result ret; > + int err =3D -1; > + int i =3D 0; >=20 > if (_atomic_set_cmd(vf, args->ops)) > return -1; > @@ -404,19 +357,22 @@ i40evf_execute_vf_cmd(struct rte_eth_dev *dev, > struct vf_cmd_info *args) > return err; > } >=20 > - err =3D i40evf_wait_cmd_done(dev, &info); > - /* read message and it's expected one */ > - if (!err && args->ops =3D=3D info.ops) > - _clear_cmd(vf); > - else if (err) { > - PMD_DRV_LOG(ERR, "Failed to read message from > AdminQ"); > - _clear_cmd(vf); > - } > - else if (args->ops !=3D info.ops) > - PMD_DRV_LOG(ERR, "command mismatch, expect %u, > get %u", > - args->ops, info.ops); > + do { > + /* Delay some time first */ > + rte_delay_ms(ASQ_DELAY_MS); It would be better to read first, but not delay first. In addition, the value of ASG_DELAY_MS shouldn't be 100ms. It is a bit too long, try 5ms or 10ms, or even shorter, and see if it is go= od. I remember that there is a complaint here for the interval. Note that if ASG_DELAY_MS changed, MAX_TRY_TIMES should also be modified. > + ret =3D i40evf_read_pfmsg(dev, &info); > + if (ret =3D=3D I40EVF_MSG_CMD) { > + err =3D 0; > + break; > + } else if (ret =3D=3D I40EVF_MSG_ERR) { > + err =3D -1; > + break; > + } > + /* If don't read msg or read sys event, continue */ > + } while (i++ < MAX_TRY_TIMES); > + _clear_cmd(vf); >=20 > - return (err | info.result); > + return (err | vf->cmd_retval); > } >=20 > /*