From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id C3649A0528; Thu, 9 Jul 2020 18:11:28 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 970241E8E6; Thu, 9 Jul 2020 18:11:28 +0200 (CEST) Received: from mail-il1-f194.google.com (mail-il1-f194.google.com [209.85.166.194]) by dpdk.org (Postfix) with ESMTP id B12AB1E8BF for ; Thu, 9 Jul 2020 18:11:27 +0200 (CEST) Received: by mail-il1-f194.google.com with SMTP id e18so2480820ilr.7 for ; Thu, 09 Jul 2020 09:11:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=W5og4pZDKCaXW46jEpazIlMnOtEoBW+7rw16Li10xJI=; b=Qn7bLsydaWY5hy0pDDPXZIcPcQHGXS58bIKwhArWguED2UH3ajTrcq/61/WAq9Q9Fh DL7oae+V2lV9kSHazIZjaRP0gSZdphSVKo2ZKj5l4+CnIi4IIxTxn8VD5t+ECW5DrjPn 6DTgyLusSU1gXQowDCCnE8wtdxYEjCJWb+yGshkQxQWwViVAnNglYC9UhdJSktrZvxoQ uedweYP0Kf4wgVLY+nLGEw1bFkJgsRyAzErS5DmWfJf3UTkbfxa7cZqsuf8iMvBqTh+O vB4edBi1XiLRuBMO466Jl3SrSQVMLwerO+i4cRE83Yp3FdIcWusUGYD9b9eF3Vrzo5KU Onew== 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=W5og4pZDKCaXW46jEpazIlMnOtEoBW+7rw16Li10xJI=; b=nXamaTDGKvWbi7DaUibdb+EjF874TCC6Yxi0uaNIq1LYtXmjfrEzKKVW+ONOMuzlP+ 1IUQMi1Ko6L7HMxgxxgmz1Rp7NzDZ4iLiFD4UiquKLqSEMXaTKbEhqKF0+BtZQGtRfks AVejX3baeHrHlV38qZiMh1RMyl6I8Mgmd2RU9aeaYIM9+Qv6At0PMxanGboZmBh1n5bB 0calLxvlDJbARQZppXpi6oejOw8FIcPYYmTlRnfzxtxxFMVnZUhiAwskD/GC+QE3njbk 6HkSW/IA3UfjTmutfqFZ4rtVVzp8gQrWH7bhcyGy95ZzIV1hCRdC7OSNqKdAD4xpE8EF Zvvw== X-Gm-Message-State: AOAM531ovaRD3THrpSBpo4T2GM1OxSqasz5CcxePmYQy43m1K9DPAiMA gL/yKEjRC5PwprVhETulv7CE0nBLTMz7NTy/5wc= X-Google-Smtp-Source: ABdhPJwzBoNHLSoQSAGuEa7zRx5k3F4bPXbdh7AgU6pdayjEGbjTXu50EAEgw9SWk+VcpcpoE7xPrh02eUa8eOc7ccY= X-Received: by 2002:a92:c10b:: with SMTP id p11mr27720772ile.60.1594311087008; Thu, 09 Jul 2020 09:11:27 -0700 (PDT) MIME-Version: 1.0 References: <20200609194207.24328-1-manishc@marvell.com> <20200609194207.24328-2-manishc@marvell.com> In-Reply-To: From: Jerin Jacob Date: Thu, 9 Jul 2020 21:41:10 +0530 Message-ID: To: Manish Chopra Cc: Gaetan Rivet , Jerin Jacob Kollanukkaran , Ferruh Yigit , dpdk-dev , Igor Russkikh , Rasesh Mody , GR-Everest-DPDK-Dev Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [EXT] Re: [PATCH 1/6] net/qede: define PCI config space specific osals X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 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 Thu, Jul 9, 2020 at 8:35 PM Manish Chopra wrote: > > > -----Original Message----- > > From: Jerin Jacob > > Sent: Friday, June 26, 2020 10:24 AM > > To: Manish Chopra ; Gaetan Rivet > > > > Cc: Jerin Jacob Kollanukkaran ; Ferruh Yigit > > ; dpdk-dev ; Igor Russkikh > > ; Rasesh Mody ; GR-Everest- > > DPDK-Dev > > Subject: [EXT] Re: [PATCH 1/6] net/qede: define PCI config space specific > > osals > > > > External Email > > > > ---------------------------------------------------------------------- > > On Wed, Jun 10, 2020 at 1:13 AM Manish Chopra > > wrote: > > > > > > This patch defines various PCI config space access APIs in order to > > > read and find IOV specific PCI capabilities. > > > > > > With these definitions implemented, it enables the base driver to do > > > SR-IOV specific initialization and HW specific configuration required > > > from PF-PMD driver instance. > > > > > > Signed-off-by: Manish Chopra > > > Signed-off-by: Igor Russkikh > > > Signed-off-by: Rasesh Mody > > > --- > > > + > > > +int osal_pci_find_next_ext_capability(struct rte_pci_device *dev, > > > + int cap) > > > > > > + Gaetan (PCI maintainer) > > > > Manish, > > It must be a candidate for a generic PCI API as it is nothing to do with qede. > > Please move to common PCI code if such API is not already present. > > > > > > > +{ > > > + int pos = PCI_CFG_SPACE_SIZE; > > > + uint32_t header; > > > + int ttl; > > > + > > > + /* minimum 8 bytes per capability */ > > > + ttl = (PCI_CFG_SPACE_EXP_SIZE - PCI_CFG_SPACE_SIZE) / 8; > > > + > > > + if (rte_pci_read_config(dev, &header, 4, pos) < 0) > > > + return -1; > > > + > > > + /* > > > + * If we have no capabilities, this is indicated by cap ID, > > > + * cap version and next pointer all being 0. > > > + */ > > > + if (header == 0) > > > + return 0; > > > + > > > + while (ttl-- > 0) { > > > + if (PCI_EXT_CAP_ID(header) == cap) > > > + return pos; > > > + > > > + pos = PCI_EXT_CAP_NEXT(header); > > > + > > > + if (pos < PCI_CFG_SPACE_SIZE) > > > + break; > > > + > > > + if (rte_pci_read_config(dev, &header, 4, pos) < 0) > > > + return -1; > > > + } > > > + > > > + return 0; > > > +} > > > > > > > > > > > +#define PCICFG_VENDOR_ID_OFFSET 0x00 > > > +#define PCICFG_DEVICE_ID_OFFSET 0x02 > > > +#define PCI_CFG_SPACE_SIZE 256 > > > +#define PCI_EXP_DEVCTL 0x0008 > > > +#define PCI_EXT_CAP_ID(header) (int)((header) & 0x0000ffff) #define > > > +PCI_EXT_CAP_NEXT(header) (((header) >> 20) & 0xffc) #define > > > +PCI_CFG_SPACE_EXP_SIZE 4096 > > > + > > > +#define PCI_SRIOV_CTRL 0x08 /* SR-IOV Control */ #define > > > +PCI_SRIOV_TOTAL_VF 0x0e /* Total VFs */ #define PCI_SRIOV_INITIAL_VF > > > +0x0c /* Initial VFs */ #define PCI_SRIOV_NUM_VF 0x10 /* Number of VFs > > > +*/ #define PCI_SRIOV_VF_OFFSET 0x14 /* First VF Offset */ #define > > > +PCI_SRIOV_VF_STRIDE 0x16 /* Following VF Stride */ #define > > > +PCI_SRIOV_VF_DID 0x1a #define PCI_SRIOV_SUP_PGSIZE 0x1c #define > > > +PCI_SRIOV_CAP 0x04 #define PCI_SRIOV_FUNC_LINK 0x12 #define > > > +PCI_EXT_CAP_ID_SRIOV 0x10 > > > > Dont DEFINE PCI_ symbols in drivers, It may conflict with other PCI > > definitions in the future. > > Please move GENERIC PCI_ symbols to the generic PCI layer. > > > > > > > > Hi Jerin/Gaetan, > > Which generic PCI code/files these defines/API should be added to ? (lib/librte_pci/rte_pci.[c|h]) ? Since it generic, To me, lib/librte_pci/rte_pci.[c|h]) is the correct place. > Just FYI, note that it can't be done without cleaning up other vendors, as I can see that various other vendors have also > defined this function to find pci extended cap and some of these PCI_* macro defines as well in their respective drivers. > > Thanks, > Manish