From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
by inbox.dpdk.org (Postfix) with ESMTP id 21FE7425FE;
Tue, 19 Sep 2023 11:01:00 +0200 (CEST)
Received: from mails.dpdk.org (localhost [127.0.0.1])
by mails.dpdk.org (Postfix) with ESMTP id EA8BE402DA;
Tue, 19 Sep 2023 11:00:59 +0200 (CEST)
Received: from us-smtp-delivery-124.mimecast.com
(us-smtp-delivery-124.mimecast.com [170.10.129.124])
by mails.dpdk.org (Postfix) with ESMTP id 0DF954026E
for <dev@dpdk.org>; Tue, 19 Sep 2023 11:00:57 +0200 (CEST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;
s=mimecast20190719; t=1695114057;
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:
content-transfer-encoding:content-transfer-encoding:
in-reply-to:in-reply-to:references:references;
bh=AG68bLKc8sw12wNJZxRLCkUi4mGNrDLv9r4aSL02pk8=;
b=YiL2tRbGsOzTi6JzSkc51Pb4BAK8ZtaOql4kr8i1cGS7ljJ4/Uq6QbZdh16WWK6kr//rVS
RubEoCE1miJxSxHqu6adUGIeXvI0OalpCnm/U3GEvGcGTnYa36phGsNf4pLnFJZ9ANKWHZ
4qnR1QAOPAVyfAE1wtBtE0oG81kg2Uk=
Received: from mail-lj1-f198.google.com (mail-lj1-f198.google.com
[209.85.208.198]) by relay.mimecast.com with ESMTP with STARTTLS
(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id
us-mta-683-doAYgu-jMo2i7-e67WL1Wg-1; Tue, 19 Sep 2023 05:00:54 -0400
X-MC-Unique: doAYgu-jMo2i7-e67WL1Wg-1
Received: by mail-lj1-f198.google.com with SMTP id
38308e7fff4ca-2bcc2fd542bso67029511fa.3
for <dev@dpdk.org>; Tue, 19 Sep 2023 02:00:53 -0700 (PDT)
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=1e100.net; s=20230601; t=1695114052; x=1695718852;
h=content-transfer-encoding:cc:to:subject:message-id:date:from
:in-reply-to:references:mime-version:x-gm-message-state:from:to:cc
:subject:date:message-id:reply-to;
bh=AG68bLKc8sw12wNJZxRLCkUi4mGNrDLv9r4aSL02pk8=;
b=j6bTOOSsPexg7CVdhAxJ8nvPsw1M/yArq53+n050DSgnwGJjcHLtRxnJOKh1E7WDJN
v7/44saco4cV26JMHbbSMzA5XMUf3i4BwbJB3tdV50C/8RgOUmkxSRzFUjkH7e/OccyL
oFUr3amCIfE5dFt/GYENp3AxodBORcjC15/eUvb0Bqo07UtitOt/rRikN+QZZGOvvodK
9QO6b3JQJec8k/c9kxIuFVUliK5KO6+0oCXFocgrtMZre3AxDaEfNw79o93eLTc0PpWB
s4doX/Ch1lNkMPfIhNfKOz5mtGvKroMM0XsoCDKtPhHGN2F849afifTLXw7cByYfUSsd
uBHw==
X-Gm-Message-State: AOJu0YwIJZvIFwc6Jm5wTwD5npZ4hnvj4OkSEtz+jPJbYaOnbvGLZgG1
Z9FX9SYW0L80pqSzWHO0o9pYy88zgNeNWJth6pnlGxqjYlCigvuPj+K7ZHmZvtM49R2Nm+tJPHV
430n1tuIUOTxEMWkzi/g=
X-Received: by 2002:a2e:3505:0:b0:2bd:10b7:4610 with SMTP id
z5-20020a2e3505000000b002bd10b74610mr9565475ljz.25.1695114052789;
Tue, 19 Sep 2023 02:00:52 -0700 (PDT)
X-Google-Smtp-Source: AGHT+IHp5f8IGgC3GbTuW7Wg4TcGmpMbFt/lmkL3NWs3jVL/gbw/dmRpKbDpS4+U+hbgjAc0RVtRwc4/AWWCLxYNOuk=
X-Received: by 2002:a2e:3505:0:b0:2bd:10b7:4610 with SMTP id
z5-20020a2e3505000000b002bd10b74610mr9565447ljz.25.1695114052470; Tue, 19 Sep
2023 02:00:52 -0700 (PDT)
MIME-Version: 1.0
References: <20230803075038.307012-1-david.marchand@redhat.com>
<20230821113549.3191921-1-david.marchand@redhat.com>
<20230821113549.3191921-5-david.marchand@redhat.com>
<CH3PR11MB836217E8783F4E3286B74A1F9CEEA@CH3PR11MB8362.namprd11.prod.outlook.com>
<CAJFAV8wY0cGGRwFn55eTjvhXzYMRRk7+AUMTxS60Awb4C6a5TQ@mail.gmail.com>
<CH3PR11MB8362AC96C4D8A9E1A9E420249CFAA@CH3PR11MB8362.namprd11.prod.outlook.com>
In-Reply-To: <CH3PR11MB8362AC96C4D8A9E1A9E420249CFAA@CH3PR11MB8362.namprd11.prod.outlook.com>
From: David Marchand <david.marchand@redhat.com>
Date: Tue, 19 Sep 2023 11:00:39 +0200
Message-ID: <CAJFAV8yj-0WJiZVtYBA5SO7nDXrumXCvCGgD336Ld1NmXQVH8Q@mail.gmail.com>
Subject: Re: [PATCH v2 04/15] bus/pci: find PCI capability
To: "Xia, Chenbo" <chenbo.xia@intel.com>
Cc: Maxime Coquelin <maxime.coquelin@redhat.com>, "dev@dpdk.org" <dev@dpdk.org>,
"thomas@monjalon.net" <thomas@monjalon.net>,
"ferruh.yigit@amd.com" <ferruh.yigit@amd.com>,
"nipun.gupta@amd.com" <nipun.gupta@amd.com>, "Richardson,
Bruce" <bruce.richardson@intel.com>,
"Burakov, Anatoly" <anatoly.burakov@intel.com>,
Jay Zhou <jianjay.zhou@huawei.com>,
"McDaniel, Timothy" <timothy.mcdaniel@intel.com>,
Julien Aube <julien_dpdk@jaube.fr>,
Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>, "Guo,
Junfeng" <junfeng.guo@intel.com>,
Jeroen de Borst <jeroendb@google.com>, Rushil Gupta <rushilg@google.com>,
Joshua Washington <joshwash@google.com>, Dongdong Liu <liudongdong3@huawei.com>,
Yisen Zhuang <yisen.zhuang@huawei.com>, Gaetan Rivet <grive@u256.net>
X-Mimecast-Spam-Score: 0
X-Mimecast-Originator: redhat.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
On Tue, Sep 19, 2023 at 4:19=E2=80=AFAM Xia, Chenbo <chenbo.xia@intel.com> =
wrote:
>
> Hi David,
>
> > -----Original Message-----
> > From: David Marchand <david.marchand@redhat.com>
> > Sent: Thursday, September 14, 2023 8:29 PM
> > To: Xia, Chenbo <chenbo.xia@intel.com>; Maxime Coquelin
> > <maxime.coquelin@redhat.com>
> > Cc: dev@dpdk.org; thomas@monjalon.net; ferruh.yigit@amd.com;
> > nipun.gupta@amd.com; Richardson, Bruce <bruce.richardson@intel.com>;
> > Burakov, Anatoly <anatoly.burakov@intel.com>; Jay Zhou
> > <jianjay.zhou@huawei.com>; McDaniel, Timothy <timothy.mcdaniel@intel.co=
m>;
> > Julien Aube <julien_dpdk@jaube.fr>; Rahul Lakkireddy
> > <rahul.lakkireddy@chelsio.com>; Guo, Junfeng <junfeng.guo@intel.com>;
> > Jeroen de Borst <jeroendb@google.com>; Rushil Gupta <rushilg@google.com=
>;
> > Joshua Washington <joshwash@google.com>; Dongdong Liu
> > <liudongdong3@huawei.com>; Yisen Zhuang <yisen.zhuang@huawei.com>; Gaet=
an
> > Rivet <grive@u256.net>
> > Subject: Re: [PATCH v2 04/15] bus/pci: find PCI capability
> >
> > Hello Chenbo,
> >
> > On Thu, Sep 7, 2023 at 2:43=E2=80=AFPM Xia, Chenbo <chenbo.xia@intel.co=
m> wrote:
> > > > Introduce two helpers so that drivers stop reinventing the wheel wh=
en
> > it
> > > > comes to finding capabilities in a device PCI configuration space.
> > > > Use it in existing drivers.
> > > >
> > > > Note:
> > > > - base/ drivers code is left untouched, only some wrappers in cxgbe
> > > > are touched,
> > > > - bnx2x maintained a per device cache of capabilities, this code ha=
s
> > been
> > > > reworked to only cache the capabilities used in this driver,
> > > >
> > > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > > > Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> > > > ---
> > > > Changes since v1:
> > > > - updated commitlog,
> > > > - separated VFIO changes for using standard PCI helper in a separat=
e
> > > > patch,
> > > > - marked new experimental symbols with current version,
> > > > - reordered defines in rte_pci.h,
> > > >
> > > > ---
> > > > drivers/bus/pci/linux/pci_vfio.c | 74 ++++--------------
> > > > drivers/bus/pci/pci_common.c | 45 +++++++++++
> > > > drivers/bus/pci/rte_bus_pci.h | 31 ++++++++
> > > > drivers/bus/pci/version.map | 4 +
> > > > drivers/crypto/virtio/virtio_pci.c | 57 +++++---------
> > > > drivers/event/dlb2/pf/dlb2_main.c | 42 +---------
> > > > drivers/net/bnx2x/bnx2x.c | 41 +++++-----
> > > > drivers/net/cxgbe/base/adapter.h | 28 +------
> > > > drivers/net/gve/gve_ethdev.c | 46 ++---------
> > > > drivers/net/gve/gve_ethdev.h | 4 -
> > > > drivers/net/hns3/hns3_ethdev_vf.c | 79 +++----------------
> > > > drivers/net/virtio/virtio_pci.c | 121 +++++--------------------=
---
> > -
> > > > lib/pci/rte_pci.h | 11 +++
> > > > 13 files changed, 186 insertions(+), 397 deletions(-)
> > > >
> > > > diff --git a/drivers/bus/pci/linux/pci_vfio.c
> > > > b/drivers/bus/pci/linux/pci_vfio.c
> > > > index 958f8b3b52..614ed5d696 100644
> > > > --- a/drivers/bus/pci/linux/pci_vfio.c
> > > > +++ b/drivers/bus/pci/linux/pci_vfio.c
> > > > @@ -110,74 +110,34 @@ static int
> > > > pci_vfio_get_msix_bar(const struct rte_pci_device *dev,
> > > > struct pci_msix_table *msix_table)
> > > > {
> > > > - int ret;
> > > > - uint32_t reg;
> > > > - uint16_t flags;
> > > > - uint8_t cap_id, cap_offset;
> > > > + off_t cap_offset;
> > > >
> > > > - /* read PCI capability pointer from config space */
> > > > - ret =3D rte_pci_read_config(dev, ®, sizeof(reg),
> > > > PCI_CAPABILITY_LIST);
> > > > - if (ret !=3D sizeof(reg)) {
> > > > - RTE_LOG(ERR, EAL,
> > > > - "Cannot read capability pointer from PCI conf=
ig
> > > > space!\n");
> > > > + cap_offset =3D rte_pci_find_capability(dev, PCI_CAP_ID_MSIX);
> > >
> > > I notice in some cases we use rte_pci_has_capability_list() to check
> > first,
> > > then looking for specific cap, in other cases we don't use
> > > rte_pci_has_capability_list(). Since we define this API, should we
> > always do
> > > the check?
> >
> > Hum, I am not sure of what you suggest here.
> >
> > I tried to do a 1:1 conversion of what existed.
> > Do you mean there are places where I missed converting some
> > rte_pci_read_config(PCI_CAPABILITY_LIST) to
> > rte_pci_has_capability_list()?
> > If so, could you point at them?
> >
> > Or are you suggesting to have this check as part of
> > rte_pci_find_capability() ?
>
> Your conversion is correct. I was saying about the usage of
> rte_pci_has_capability_list/rte_pci_find_capability, logically should we
> always call rte_pci_has_capability_list first before find capability?
>
> I don't have strong opinion on this. You can choose to keep the same or
> call rte_pci_has_capability_list more.
I prefer to keep it as is (the series is already big enough).
We can still add more checks in the future.
Thanks Chenbo.
--=20
David Marchand