DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Wang, Haiyue" <haiyue.wang@intel.com>
To: David Marchand <david.marchand@redhat.com>
Cc: dev <dev@dpdk.org>, "Zhang, Qi Z" <qi.z.zhang@intel.com>,
	"Wang, Liang-min" <liang-min.wang@intel.com>,
	"Wu, Jingjing" <jingjing.wu@intel.com>,
	"Xing, Beilei" <beilei.xing@intel.com>
Subject: Re: [dpdk-dev] [PATCH v4 2/3] net/iavf: enable PCI bus master after reset
Date: Thu, 6 May 2021 03:02:52 +0000
Message-ID: <BN8PR11MB3795B17FEF4572EDD083908DF7589@BN8PR11MB3795.namprd11.prod.outlook.com> (raw)
In-Reply-To: <CAJFAV8y=6XqQN_VWv2t13N+VxZZQ_uGCdM2w00j8mfc8cCxEVA@mail.gmail.com>

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Wednesday, May 5, 2021 16:39
> To: Wang, Haiyue <haiyue.wang@intel.com>
> Cc: dev <dev@dpdk.org>; Zhang, Qi Z <qi.z.zhang@intel.com>; Wang, Liang-min <liang-min.wang@intel.com>;
> Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> Subject: Re: [PATCH v4 2/3] net/iavf: enable PCI bus master after reset
> 
> On Wed, May 5, 2021 at 4:56 AM Wang, Haiyue <haiyue.wang@intel.com> wrote:
> >
> > > -----Original Message-----
> > > From: David Marchand <david.marchand@redhat.com>
> > > Sent: Tuesday, May 4, 2021 19:32
> > > To: Wang, Haiyue <haiyue.wang@intel.com>
> > > Cc: dev <dev@dpdk.org>; Zhang, Qi Z <qi.z.zhang@intel.com>; Wang, Liang-min <liang-
> min.wang@intel.com>;
> > > Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> > > Subject: Re: [PATCH v4 2/3] net/iavf: enable PCI bus master after reset
> > >
> > > On Tue, Apr 27, 2021 at 4:05 PM Haiyue Wang <haiyue.wang@intel.com> wrote:
> > > >
> > > > The VF reset can be triggerred by the PF reset event, in this case, the
> > > > PCI bus master will be cleared, then the VF is not allowed to issue any
> > > > Memory or I/O Requests.
> > > >
> > > > So after the reset event is detected, always enable the PCI bus master.
> > > >
> > > > Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> > > > ---
> > > >  drivers/net/iavf/iavf_ethdev.c | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
> > > > index d523a0618..9a0a20a29 100644
> > > > --- a/drivers/net/iavf/iavf_ethdev.c
> > > > +++ b/drivers/net/iavf/iavf_ethdev.c
> > > > @@ -2255,6 +2255,9 @@ iavf_dev_close(struct rte_eth_dev *dev)
> > > >         rte_free(vf->aq_resp);
> > > >         vf->aq_resp = NULL;
> > > >
> > > > +       if (vf->vf_reset)
> > > > +               rte_pci_set_bus_master(pci_dev, true);
> > > > +
> > > >         vf->vf_reset = false;
> > >
> > > Not checking for the return code can leave the device in an invalid state.
> > > Then after this, calling the init code will fail.
> >
> > From the upper application's view, if this bus master fix can't recover
> > the device into valid state, then the device hotplug API should be used
> > to make the device fully recover. So I'd prefer to call bus master "try
> > best" to fix. If still have error, the system may be in bad state.
> 
> I find it odd to put something required for (re)initialising in a
> .dev_close ops.
> Maybe the place is more into .dev_reset if you find it confusing in .dev_init.
> But this is your driver, I'll let it to your judgement.
> 

In fact, the .dev_close is close to .dev_reset.

static int
iavf_dev_reset(struct rte_eth_dev *dev)
{
	int ret;

	ret = iavf_dev_uninit(dev); --> iavf_dev_close
	if (ret)
		return ret;

	return iavf_dev_init(dev);
}

> 
> On the other hand, what is the point of not failing early/propagating
> the rte_pci_set_bus_master error?
> The driver can't work without bus master enabled, correct?

From this point of view, correct. ;-)

Now I changed the vf_reset handling as bellow, if bus master failed, the
device will still be in VF reset state, so that other modules will return
immediately (the PMD has considered the VF reset state already).

if (vf->vf_reset &&
    !rte_pci_set_bus_master(pci_dev, true))
	vf->vf_reset = false;

> 
> 
> --
> David Marchand


  reply	other threads:[~2021-05-06  3:03 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-21  5:02 [dpdk-dev] [PATCH v1 0/3] Fix PF reset causes VF memory request failure Haiyue Wang
2021-04-21  5:02 ` [dpdk-dev] [PATCH v1 1/3] bus/pci: enable PCI master in command register Haiyue Wang
2021-04-21  5:02 ` [dpdk-dev] [PATCH v1 2/3] net/iavf: enable PCI bus master after reset Haiyue Wang
2021-04-21  5:02 ` [dpdk-dev] [PATCH v1 3/3] net/i40e: " Haiyue Wang
2021-04-21 11:59 ` [dpdk-dev] [PATCH v1 0/3] Fix PF reset causes VF memory request failure Zhang, Qi Z
2021-04-22  1:18 ` [dpdk-dev] [PATCH v2 0/3] fix " Haiyue Wang
2021-04-22  1:18   ` [dpdk-dev] [PATCH v2 1/3] bus/pci: enable PCI master in command register Haiyue Wang
2021-04-23 10:43     ` Kinsella, Ray
2021-04-23 12:07       ` Wang, Haiyue
2021-04-22  1:18   ` [dpdk-dev] [PATCH v2 2/3] net/iavf: enable PCI bus master after reset Haiyue Wang
2021-04-22  1:18   ` [dpdk-dev] [PATCH v2 3/3] net/i40e: " Haiyue Wang
2021-04-23 11:39 ` [dpdk-dev] [PATCH v3 0/3] fix PF reset causes VF memory request failure Haiyue Wang
2021-04-23 11:39   ` [dpdk-dev] [PATCH v3 1/3] bus/pci: enable PCI master in command register Haiyue Wang
2021-04-23 12:32     ` Kinsella, Ray
2021-04-27  9:28     ` David Marchand
2021-04-27 13:34       ` Wang, Haiyue
2021-04-27 13:40         ` David Marchand
2021-04-23 11:40   ` [dpdk-dev] [PATCH v3 2/3] net/iavf: enable PCI bus master after reset Haiyue Wang
2021-04-23 11:40   ` [dpdk-dev] [PATCH v3 3/3] net/i40e: " Haiyue Wang
2021-04-27 13:39 ` [dpdk-dev] [PATCH v4 0/3] fix PF reset causes VF memory request failure Haiyue Wang
2021-04-27 13:39   ` [dpdk-dev] [PATCH v4 1/3] bus/pci: set PCI master in command register Haiyue Wang
2021-04-27 15:07     ` Kinsella, Ray
2021-04-27 13:39   ` [dpdk-dev] [PATCH v4 2/3] net/iavf: enable PCI bus master after reset Haiyue Wang
2021-04-28  3:34     ` Zhang, Qi Z
2021-05-04 11:32     ` David Marchand
2021-05-04 15:07       ` Wang, Haiyue
2021-05-05  2:56       ` Wang, Haiyue
2021-05-05  8:39         ` David Marchand
2021-05-06  3:02           ` Wang, Haiyue [this message]
2021-04-27 13:39   ` [dpdk-dev] [PATCH v4 3/3] net/i40e: " Haiyue Wang
2021-04-28  3:35     ` Zhang, Qi Z
2021-05-06  3:49 ` [dpdk-dev] [PATCH v5 0/3] fix PF reset causes VF memory request failure Haiyue Wang
2021-05-06  3:49   ` [dpdk-dev] [PATCH v5 1/3] bus/pci: set PCI master in command register Haiyue Wang
2021-05-06  3:49   ` [dpdk-dev] [PATCH v5 2/3] net/iavf: enable PCI bus master after reset Haiyue Wang
2021-05-06  3:49   ` [dpdk-dev] [PATCH v5 3/3] net/i40e: " Haiyue Wang
2021-05-23 11:46 ` [dpdk-dev] [PATCH v6 0/3] fix PF reset causes VF memory request failure Haiyue Wang
2021-05-23 11:46   ` [dpdk-dev] [PATCH v6 1/3] bus/pci: set PCI master in command register Haiyue Wang
2021-05-23 11:46   ` [dpdk-dev] [PATCH v6 2/3] net/iavf: enable PCI bus master after reset Haiyue Wang
2021-05-23 11:46   ` [dpdk-dev] [PATCH v6 3/3] net/i40e: " Haiyue Wang
2021-05-24  1:23 ` [dpdk-dev] [PATCH v7 0/3] fix PF reset causes VF memory request failure Haiyue Wang
2021-05-24  1:23   ` [dpdk-dev] [PATCH v7 1/3] bus/pci: set PCI master in command register Haiyue Wang
2021-05-24  1:23   ` [dpdk-dev] [PATCH v7 2/3] net/iavf: enable PCI bus master after reset Haiyue Wang
2021-06-04  2:07     ` Xing, Beilei
2021-05-24  1:23   ` [dpdk-dev] [PATCH v7 3/3] net/i40e: " Haiyue Wang
2021-06-04  1:58     ` Xing, Beilei
2021-06-08  8:31   ` [dpdk-dev] [PATCH v7 0/3] fix PF reset causes VF memory request failure David Marchand

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=BN8PR11MB3795B17FEF4572EDD083908DF7589@BN8PR11MB3795.namprd11.prod.outlook.com \
    --to=haiyue.wang@intel.com \
    --cc=beilei.xing@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=jingjing.wu@intel.com \
    --cc=liang-min.wang@intel.com \
    --cc=qi.z.zhang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git