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 604FAA0526; Fri, 10 Jul 2020 07:03:49 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 6D3B71DA9A; Fri, 10 Jul 2020 07:03:48 +0200 (CEST) Received: from mail-il1-f195.google.com (mail-il1-f195.google.com [209.85.166.195]) by dpdk.org (Postfix) with ESMTP id 9B1E71D974 for ; Fri, 10 Jul 2020 07:03:46 +0200 (CEST) Received: by mail-il1-f195.google.com with SMTP id x9so4036301ila.3 for ; Thu, 09 Jul 2020 22:03:46 -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=zhzM1JcylZUD8tPJahBwn8jy4a75dkZ79/Tg7jjt1Bg=; b=nT8YYn8hPhSQIm78rnb2VBqdr+Nhlu7BZdwe9mR55O5y+P9ruXzCdz25liPzDu/7Iy zboHC1IbrWj7WhD4rsFHNVjFXmyJty/Rvfn0pAkEOywJcPFRIDaJ6MNUawOnI1uFs0FI 0F7WsnXX/YPX1ER7pePnryoXPqVheYh/KF/iXdo9DN8mi+9UjCPEiBPvY8BzcXk8QDLO 3129jg5yxbkjbDto5/Xyb2T2vi6bl9lXHmVmMjr99H/DWrlRJkAs+He4QFPHODrxHC3N JBPbC5+o2j6STsvca1LjEymw7YVr9++PlJJ9vWHGFZt5w5WDe1feHFYctURQGwyAQX65 eiNQ== 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=zhzM1JcylZUD8tPJahBwn8jy4a75dkZ79/Tg7jjt1Bg=; b=DJp/yUJC5pL8+yW9D0AvZlxdOpE/UhBTu2rShi6/wQOPqRfCz7yBUC1rHVDi04JBpv dWIkZ7mbCTsIhjoQ11tCXuAsubirpJykWcrbFPghZnpzVDWdGk8hYvhi+YiHVB29aeNY gbjnm9WhP0xUuTgSTOUEBqtqL+3QhbSHUgiiB5btnpamEQCX6I6j7r+O+5xZFWz4RHas cqg0BgOwnG8qJyubUrXZRQbifGd0vKCnOca3fZJNW15rDH1hXVWL049SNE2bwzAyIacO zoF9/+iuvb4LmD5kgz5n1Nk0jdi26dhsuYiX1RuVRAUGUfQMl1rGZXHmbYNfKS8BkshM UNdA== X-Gm-Message-State: AOAM531fqSF3vUV73/lw9yWI7mQ/Pz1KIaSU9JtBJ8KfOzUlxI+NtLUR KPvHABav2nYE1SFzFq+4SRpzQMVuLZ/mErTTgZI= X-Google-Smtp-Source: ABdhPJwZqLUIjF1z4/Q4COotOMT5jPhk62cScXepDac7E9Uyo2Fv5pwtofjac2juxftSCnZ/0xj3Rtir3vA4AL1Utf8= X-Received: by 2002:a05:6e02:eb3:: with SMTP id u19mr52061581ilj.130.1594357425824; Thu, 09 Jul 2020 22:03:45 -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: Fri, 10 Jul 2020 10:33:29 +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 Fri, Jul 10, 2020 at 3:58 AM Manish Chopra wrote: > > > -----Original Message----- > > From: Jerin Jacob > > Sent: Thursday, July 9, 2020 9:41 PM > > To: Manish Chopra > > Cc: Gaetan Rivet ; Jerin Jacob Kollanukkaran > > ; Ferruh Yigit ; dpdk-dev > > ; Igor Russkikh ; Rasesh Mody > > ; GR-Everest-DPDK-Dev > Dev@marvell.com> > > Subject: Re: [EXT] Re: [PATCH 1/6] net/qede: define PCI config space specific > > osals > > > > 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 > > Hi Jerin, > > It seems like adding these in lib/librte_pci/rte_pci.[c|h]) is not straight w/o doing forward declarations of func/structs > like (strcut rte_pci_device, rte_pci_read_config()) which are being referenced in pci_find_next_ext_capability(), > as rte_bus_pci.h already have include of rte_pci.h > > So, how about adding them in drivers/bus/pci/pci_common.c and drivers/bus/pci/rte_bus_pci.h files directly ? Good to me. > > Also, most of the PCI* defines above are already available from /usr/include/pci_regs.h so I think we don't need them to re-define any again in DPDK tree's headers. > (Assuming that all supported kernels would have latest /usr/include/pci_regs.h) I think, we can avoid that path to avoid Linux kernel or specific package dependency > > Thanks, > Manish