From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 0A5FEA0524; Wed, 5 May 2021 10:39:40 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id B839640143; Wed, 5 May 2021 10:39:39 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id 4CF9540040 for ; Wed, 5 May 2021 10:39:38 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1620203977; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=qX6CyawhfD9IJAGbiO0S7EsWxXlj2lqrdzbAmMMAR2g=; b=PztoXEqc1pdeMAeyFN/e6nJqn9aG3HyLGef4CjQkfOVACFuvuAZ0Xi2imr3cEu6nlWHWR7 F1tWGy7O9KC8PLKz3EViYko9vbsBT96EWjBFnMIh9MI/kB/CdQoIUMZCHO+SvlMseeqc80 5vOv1tQj2rmsHd/kka4e7CwyaQ1+800= Received: from mail-vs1-f71.google.com (mail-vs1-f71.google.com [209.85.217.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-248-sty_vT3iOui6PGEkA6KDXA-1; Wed, 05 May 2021 04:39:36 -0400 X-MC-Unique: sty_vT3iOui6PGEkA6KDXA-1 Received: by mail-vs1-f71.google.com with SMTP id 3-20020a6717030000b029016d08542c7dso720505vsx.14 for ; Wed, 05 May 2021 01:39:36 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=qX6CyawhfD9IJAGbiO0S7EsWxXlj2lqrdzbAmMMAR2g=; b=SGpOZPqDtcpks0FGLZxTt9BauI/4nM5cMR3uodurAaJo0ZQZLDjEBduHP9CWGKXL06 dN9ldk/VDMceaV3vmsw+O+vzBC4oUYqR7fy83DaqCrISdgK0mVbG5McKWpWFLmDEKn83 hrQ/4nNt/eOh+ZGrSWfevvl62moFiPl428XlWU6RosJJ7Wp/+IaGsbsS8nDoDEchiBK6 qg5BkVVI7kBMCBcdUpHvbLlCQ3PPq1XT1Aloy3XE3wlHiPMOeuOeWxOJk4ig0oTkFNXc 8Ldw0ZTSKaFEiuLB+e+61SpgIGgCgR9sk2rOGqjaRl/OG3rkosAaLwx/Sw0Wv6CRYVmX AzjQ== X-Gm-Message-State: AOAM533qu37NyjxvRvi0dBCli/30+wX8qw01/nL7AEQyclxIs13NGv5e +/E7D4MVzqgRD8w1Xs2HKwv3GRU+DeDLqo8qqeLS/Us5jgRZM7iBjugkMRbtdus/R/4teiJSpBo NWi7xAfZN06yee3TMaWQ= X-Received: by 2002:a1f:d283:: with SMTP id j125mr24233796vkg.9.1620203975839; Wed, 05 May 2021 01:39:35 -0700 (PDT) X-Google-Smtp-Source: ABdhPJy1ZxMKoQcAtskvBXImtr6DdnVMBBoOT1t2hnOAQ29J3EJk/6g7ykgfvK+fxlggMk6KbMosF1/lFQL9v1CR894= X-Received: by 2002:a1f:d283:: with SMTP id j125mr24233785vkg.9.1620203975605; Wed, 05 May 2021 01:39:35 -0700 (PDT) MIME-Version: 1.0 References: <20210421050243.130585-1-haiyue.wang@intel.com> <20210427133912.261993-1-haiyue.wang@intel.com> <20210427133912.261993-3-haiyue.wang@intel.com> In-Reply-To: From: David Marchand Date: Wed, 5 May 2021 10:39:24 +0200 Message-ID: To: "Wang, Haiyue" Cc: dev , "Zhang, Qi Z" , "Wang, Liang-min" , "Wu, Jingjing" , "Xing, Beilei" Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dmarchan@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [PATCH v4 2/3] net/iavf: enable PCI bus master after reset X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Wed, May 5, 2021 at 4:56 AM Wang, Haiyue wrote: > > > -----Original Message----- > > From: David Marchand > > Sent: Tuesday, May 4, 2021 19:32 > > To: Wang, Haiyue > > Cc: dev ; Zhang, Qi Z ; Wang, Liang-min ; > > Wu, Jingjing ; Xing, Beilei > > 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 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 > > > --- > > > 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. 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? -- David Marchand