From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Wu, Jingjing" Subject: Re: [PATCH v2 1/2] i40evf: allocate virtchnl cmd buffer for each vf Date: Sun, 14 Feb 2016 02:22:02 +0000 Message-ID: <9BB6961774997848B5B42BEC655768F8D98AD8@SHSMSX104.ccr.corp.intel.com> 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: "Tao, Zhe" , "dev@dpdk.org" Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 8FD025A4E for ; Sun, 14 Feb 2016 03:22:06 +0100 (CET) In-Reply-To: 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: Tao, Zhe > Sent: Friday, January 29, 2016 3:28 PM > To: Wu, Jingjing; dev@dpdk.org > Subject: RE: [dpdk-dev] [PATCH v2 1/2] i40evf: allocate virtchnl cmd buff= er > for each vf >=20 >=20 >=20 > > -----Original Message----- > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jingjing Wu > > Sent: Wednesday, January 27, 2016 9:50 AM > > To: dev@dpdk.org > > Subject: [dpdk-dev] [PATCH v2 1/2] i40evf: allocate virtchnl cmd > > buffer for each vf > > > > Currently, i40evf PMD uses a global static buffer to send virtchnl > > command to host driver. It is shared by multi VFs. > > This patch changed to allocate virtchnl cmd buffer for each VF. > > > > 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(-) > > > > 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 */ > > > > /* 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 */ > > }; > > > > -/* 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) } > > > > /* > > - * 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; > > > > + 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; > > + } > > + > > + 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; > > > > - /* 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; > > > > - 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; > > } > > > > -/* > > - * Polling read until command result return from pf driver or meet err= or. > > - */ > > -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; > > } > > > > +#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; > > > > 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; > > } > > > > - 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); > > + 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); > > > > - return (err | info.result); > > + return (err | vf->cmd_retval); > > } > > > Why combine the i40evf_parse_pfmsg and i40evf_read_pfmsg into one > function i40evf_read_pfmsg? Because what the i40evf_parse_pfmsg did is processing the pf event message,= and it is meaningless for checking whether the vf request message is done. And the pf event message will be processed in i40evf_handle_pf_event which = defined in the next patch of the same patch set. Thanks Jingjing