From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 3B83AC47E for ; Mon, 29 Jun 2015 15:24:37 +0200 (CEST) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga101.jf.intel.com with ESMTP; 29 Jun 2015 06:24:36 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.13,698,1427785200"; d="scan'208";a="515934549" Received: from irsmsx110.ger.corp.intel.com ([163.33.3.25]) by FMSMGA003.fm.intel.com with ESMTP; 29 Jun 2015 06:24:35 -0700 Received: from irsmsx108.ger.corp.intel.com ([169.254.11.201]) by irsmsx110.ger.corp.intel.com ([163.33.3.25]) with mapi id 14.03.0224.002; Mon, 29 Jun 2015 14:24:34 +0100 From: "Iremonger, Bernard" To: Tetsuya Mukawa , "dev@dpdk.org" Thread-Topic: [PATCH v3 1/8] eal: Add pci_uio_alloc_uio_resource() Thread-Index: AQHQshdbIU+bJ6ERTUilMbyAcKgssp3Dd9xw Date: Mon, 29 Jun 2015 13:24:33 +0000 Message-ID: <8CEF83825BEC744B83065625E567D7C204A42D4B@IRSMSX108.ger.corp.intel.com> References: <1432016513-8456-5-git-send-email-mukawa@igel.co.jp> <1435546610-4533-1-git-send-email-mukawa@igel.co.jp> <1435546610-4533-2-git-send-email-mukawa@igel.co.jp> In-Reply-To: <1435546610-4533-2-git-send-email-mukawa@igel.co.jp> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v3 1/8] eal: Add pci_uio_alloc_uio_resource() X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 29 Jun 2015 13:24:37 -0000 > -----Original Message----- > From: Tetsuya Mukawa [mailto:mukawa@igel.co.jp] > Sent: Monday, June 29, 2015 3:57 AM > To: dev@dpdk.org > Cc: Iremonger, Bernard; david.marchand@6wind.com; Tetsuya.Mukawa > Subject: [PATCH v3 1/8] eal: Add pci_uio_alloc_uio_resource() >=20 > From: "Tetsuya.Mukawa" >=20 > This patch adds a new function called pci_uio_alloc_uio_resource(). > The function hides how to prepare uio resource in linuxapp and bsdapp. > With the function, pci_uio_map_resource() will be more abstracted. >=20 > Signed-off-by: Tetsuya Mukawa Hi Tetsuya, There are two comments inline below. > --- > lib/librte_eal/bsdapp/eal/eal_pci.c | 70 +++++++++++++++++++------= --- > lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 77 ++++++++++++++++++++++--- > ------ > 2 files changed, 104 insertions(+), 43 deletions(-) >=20 > diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c > b/lib/librte_eal/bsdapp/eal/eal_pci.c > index 06c564f..2d9f3a5 100644 > --- a/lib/librte_eal/bsdapp/eal/eal_pci.c > +++ b/lib/librte_eal/bsdapp/eal/eal_pci.c > @@ -189,28 +189,17 @@ pci_uio_map_secondary(struct rte_pci_device > *dev) > return 1; > } >=20 > -/* map the PCI resource of a PCI device in virtual memory */ static int= - > pci_uio_map_resource(struct rte_pci_device *dev) > +pci_uio_alloc_uio_resource(struct rte_pci_device *dev, > + struct mapped_pci_resource **uio_res) The name of this function is a bit longwinded, pci_uio_alloc_resource() mi= ght be better. > { > - int i, map_idx; > char devname[PATH_MAX]; /* contains the /dev/uioX */ > - void *mapaddr; > - uint64_t phaddr; > - uint64_t offset; > - uint64_t pagesz; > - struct rte_pci_addr *loc =3D &dev->addr; > - struct mapped_pci_resource *uio_res; > - struct mapped_pci_res_list *uio_res_list =3D > - RTE_TAILQ_CAST(rte_uio_tailq.head, > mapped_pci_res_list); > - struct pci_map *maps; > + struct rte_pci_addr *loc; >=20 > - dev->intr_handle.fd =3D -1; > - dev->intr_handle.type =3D RTE_INTR_HANDLE_UNKNOWN; > + if ((dev =3D=3D NULL) || (uio_res =3D=3D NULL)) > + return -1; >=20 > - /* secondary processes - use already recorded details */ > - if (rte_eal_process_type() !=3D RTE_PROC_PRIMARY) > - return pci_uio_map_secondary(dev); > + loc =3D &dev->addr; >=20 > snprintf(devname, sizeof(devname), "/dev/uio@pci:%u:%u:%u", > dev->addr.bus, dev->addr.devid, dev- > >addr.function); @@ -231,18 +220,56 @@ pci_uio_map_resource(struct > rte_pci_device *dev) > dev->intr_handle.type =3D RTE_INTR_HANDLE_UIO; >=20 > /* allocate the mapping details for secondary processes*/ > - if ((uio_res =3D rte_zmalloc("UIO_RES", sizeof (*uio_res), 0)) =3D=3D N= ULL) > { > + *uio_res =3D rte_zmalloc("UIO_RES", sizeof(**uio_res), 0); > + if (*uio_res =3D=3D NULL) { > RTE_LOG(ERR, EAL, > "%s(): cannot store uio mmap details\n", __func__); > goto close_fd; > } >=20 > - snprintf(uio_res->path, sizeof(uio_res->path), "%s", devname); > - memcpy(&uio_res->pci_addr, &dev->addr, sizeof(uio_res- > >pci_addr)); > + snprintf((*uio_res)->path, sizeof((*uio_res)->path), "%s", > devname); > + memcpy(&(*uio_res)->pci_addr, &dev->addr, > +sizeof((*uio_res)->pci_addr)); >=20 > + return 0; > + > +close_fd: > + close(dev->intr_handle.fd); > + dev->intr_handle.fd =3D -1; > + dev->intr_handle.type =3D RTE_INTR_HANDLE_UNKNOWN; > + > + return -1; > +} > + > +/* map the PCI resource of a PCI device in virtual memory */ static int > +pci_uio_map_resource(struct rte_pci_device *dev) { > + int i, map_idx, ret; > + char *devname; > + void *mapaddr; > + uint64_t phaddr; > + uint64_t offset; > + uint64_t pagesz; > + struct mapped_pci_resource *uio_res =3D NULL; > + struct mapped_pci_res_list *uio_res_list =3D > + RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list); > + struct pci_map *maps; > + > + dev->intr_handle.fd =3D -1; > + dev->intr_handle.type =3D RTE_INTR_HANDLE_UNKNOWN; > + > + /* secondary processes - use already recorded details */ > + if (rte_eal_process_type() !=3D RTE_PROC_PRIMARY) > + return pci_uio_map_secondary(dev); > + > + /* allocate uio resource */ > + ret =3D pci_uio_alloc_uio_resource(dev, &uio_res); > + if ((ret !=3D 0) || (uio_res =3D=3D NULL)) > + return ret; >=20 > /* Map all BARs */ > pagesz =3D sysconf(_SC_PAGESIZE); > + devname =3D uio_res->path; >=20 > maps =3D uio_res->maps; > for (i =3D 0, map_idx =3D 0; i !=3D PCI_MAX_RESOURCE; i++) { @@ -300,7 > +327,8 @@ free_uio_res: > for (i =3D 0; i < map_idx; i++) > rte_free(maps[i].path); > rte_free(uio_res); > -close_fd: > + > + /* close fd opened by pci_uio_alloc_uio_resource() */ > close(dev->intr_handle.fd); > dev->intr_handle.fd =3D -1; > dev->intr_handle.type =3D RTE_INTR_HANDLE_UNKNOWN; diff --git > a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c > b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c > index 19620fe..9e0b617 100644 > --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c > +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c > @@ -254,30 +254,20 @@ pci_get_uio_dev(struct rte_pci_device *dev, char > *dstbuf, > return uio_num; > } >=20 > -/* map the PCI resource of a PCI device in virtual memory */ -int - > pci_uio_map_resource(struct rte_pci_device *dev) > +static int > +pci_uio_alloc_uio_resource(struct rte_pci_device *dev, > + struct mapped_pci_resource **uio_res) The name of this function is a bit longwinded, pci_uio_alloc_resource() mi= ght be better. Regards, Bernard. > { > - int i, map_idx; > char dirname[PATH_MAX]; > char cfgname[PATH_MAX]; > char devname[PATH_MAX]; /* contains the /dev/uioX */ > - void *mapaddr; > int uio_num; > - uint64_t phaddr; > - struct rte_pci_addr *loc =3D &dev->addr; > - struct mapped_pci_resource *uio_res; > - struct mapped_pci_res_list *uio_res_list =3D > - RTE_TAILQ_CAST(rte_uio_tailq.head, > mapped_pci_res_list); > - struct pci_map *maps; > + struct rte_pci_addr *loc; >=20 > - dev->intr_handle.fd =3D -1; > - dev->intr_handle.uio_cfg_fd =3D -1; > - dev->intr_handle.type =3D RTE_INTR_HANDLE_UNKNOWN; > + if ((dev =3D=3D NULL) || (uio_res =3D=3D NULL)) > + return -1; >=20 > - /* secondary processes - use already recorded details */ > - if (rte_eal_process_type() !=3D RTE_PROC_PRIMARY) > - return pci_uio_map_secondary(dev); > + loc =3D &dev->addr; >=20 > /* find uio resource */ > uio_num =3D pci_get_uio_dev(dev, dirname, sizeof(dirname)); @@ - > 318,15 +308,57 @@ pci_uio_map_resource(struct rte_pci_device *dev) > } >=20 > /* allocate the mapping details for secondary processes*/ > - uio_res =3D rte_zmalloc("UIO_RES", sizeof(*uio_res), 0); > - if (uio_res =3D=3D NULL) { > + *uio_res =3D rte_zmalloc("UIO_RES", sizeof(**uio_res), 0); > + if (*uio_res =3D=3D NULL) { > RTE_LOG(ERR, EAL, > "%s(): cannot store uio mmap details\n", __func__); > goto close_fd; > } >=20 > - snprintf(uio_res->path, sizeof(uio_res->path), "%s", devname); > - memcpy(&uio_res->pci_addr, &dev->addr, sizeof(uio_res- > >pci_addr)); > + snprintf((*uio_res)->path, sizeof((*uio_res)->path), "%s", > devname); > + memcpy(&(*uio_res)->pci_addr, &dev->addr, > +sizeof((*uio_res)->pci_addr)); > + > + return 0; > + > +close_fd: > + if (dev->intr_handle.uio_cfg_fd >=3D 0) { > + close(dev->intr_handle.uio_cfg_fd); > + dev->intr_handle.uio_cfg_fd =3D -1; > + } > + > + close(dev->intr_handle.fd); > + dev->intr_handle.fd =3D -1; > + dev->intr_handle.type =3D RTE_INTR_HANDLE_UNKNOWN; > + > + return -1; > +} > + > +/* map the PCI resource of a PCI device in virtual memory */ int > +pci_uio_map_resource(struct rte_pci_device *dev) { > + int i, map_idx, ret; > + char devname[PATH_MAX]; /* contains the /dev/uioX */ > + void *mapaddr; > + uint64_t phaddr; > + struct rte_pci_addr *loc =3D &dev->addr; > + struct mapped_pci_resource *uio_res =3D NULL; > + struct mapped_pci_res_list *uio_res_list =3D > + RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list); > + struct pci_map *maps; > + > + dev->intr_handle.fd =3D -1; > + dev->intr_handle.uio_cfg_fd =3D -1; > + dev->intr_handle.type =3D RTE_INTR_HANDLE_UNKNOWN; > + > + /* secondary processes - use already recorded details */ > + if (rte_eal_process_type() !=3D RTE_PROC_PRIMARY) > + return pci_uio_map_secondary(dev); > + > + /* allocate uio resource */ > + ret =3D pci_uio_alloc_uio_resource(dev, &uio_res); > + if ((ret !=3D 0) || (uio_res =3D=3D NULL)) > + return ret; >=20 > /* Map all BARs */ > maps =3D uio_res->maps; > @@ -401,7 +433,8 @@ free_uio_res: > rte_free(maps[i].path); > } > rte_free(uio_res); > -close_fd: > + > + /* close fd opened by pci_uio_alloc_uio_resource() */ > if (dev->intr_handle.uio_cfg_fd >=3D 0) { > close(dev->intr_handle.uio_cfg_fd); > dev->intr_handle.uio_cfg_fd =3D -1; > -- > 2.1.4