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 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 ; 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 ; 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> In-Reply-To: From: David Marchand Date: Tue, 19 Sep 2023 11:00:39 +0200 Message-ID: Subject: Re: [PATCH v2 04/15] bus/pci: find PCI capability To: "Xia, Chenbo" Cc: Maxime Coquelin , "dev@dpdk.org" , "thomas@monjalon.net" , "ferruh.yigit@amd.com" , "nipun.gupta@amd.com" , "Richardson, Bruce" , "Burakov, Anatoly" , Jay Zhou , "McDaniel, Timothy" , Julien Aube , Rahul Lakkireddy , "Guo, Junfeng" , Jeroen de Borst , Rushil Gupta , Joshua Washington , Dongdong Liu , Yisen Zhuang , Gaetan Rivet 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Tue, Sep 19, 2023 at 4:19=E2=80=AFAM Xia, Chenbo = wrote: > > Hi David, > > > -----Original Message----- > > From: David Marchand > > Sent: Thursday, September 14, 2023 8:29 PM > > To: Xia, Chenbo ; Maxime Coquelin > > > > Cc: dev@dpdk.org; thomas@monjalon.net; ferruh.yigit@amd.com; > > nipun.gupta@amd.com; Richardson, Bruce ; > > Burakov, Anatoly ; Jay Zhou > > ; McDaniel, Timothy ; > > Julien Aube ; Rahul Lakkireddy > > ; Guo, Junfeng ; > > Jeroen de Borst ; Rushil Gupta ; > > Joshua Washington ; Dongdong Liu > > ; Yisen Zhuang ; Gaet= an > > Rivet > > 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 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 > > > > Acked-by: Bruce Richardson > > > > --- > > > > 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