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 DA605A052A; Fri, 10 Jul 2020 12:08:05 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 382E21D702; Fri, 10 Jul 2020 12:08:05 +0200 (CEST) Received: from new2-smtp.messagingengine.com (new2-smtp.messagingengine.com [66.111.4.224]) by dpdk.org (Postfix) with ESMTP id 14D1E1C1BB; Fri, 10 Jul 2020 12:08:04 +0200 (CEST) Received: from compute7.internal (compute7.nyi.internal [10.202.2.47]) by mailnew.nyi.internal (Postfix) with ESMTP id A063C580467; Fri, 10 Jul 2020 06:08:01 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute7.internal (MEProxy); Fri, 10 Jul 2020 06:08:01 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=fm1; bh= JXscnLo6SBasOyAbu/D1wZhBBRoO7ZMkUgdLm87hBBA=; b=CbgvF/RdA/OrqCYi Ad8r/LFXfO+0Fp76YR7UZmT7542yDcevA44zuOq0uf5v5jf2hFIXvjhSNR7EIxct xcr2ZFXrDb6EAC98z+Ym3KG7+++oy6xCdK26qg5asXMQSvftzuyzUAdmoEgFQjV8 53u1Ygc1DnJuzLNnsmBHSdXULHhYmW61si46gzLCq5m49WWxKXqKyJjd3vKn7IVO oRmLXD8BmpOiOu2qCV9bJ1rz6RXjQZn2hLOinKfb9WFYFSfCBziaj92K9uxkcVKr s/jwC2HkYhjPom23tjPEkCPvxEo3CpLJKdkr+EsFC7ztJGXuhz/oqj2v60P5RzY2 LAB4zw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm3; bh=JXscnLo6SBasOyAbu/D1wZhBBRoO7ZMkUgdLm87hB BA=; b=VRjg52H8eqqMUyo1SPX9m3oEd5Zpb5OyYUKEB7DSTBWABvUj/s4haLETs kPIaeY578l+oYaLdsuAInzyVwpGUeIvP7EmWpds6BOvSVHGn4+R9R0U0OAqzhM3Z iI4MC+sb1PaTCuPtwF9E4LQABN7x13YjH5+WaRFFlXqPxmBuRhoPX115mymOaeKt DKoT9YVXbJZRI+c15sRKuQmbn1g9glI4PuL8m3Ez/uAb54hzERCJ/nSOrw24q7Mv F/ikXklpYdyDxDwbcE2M1S/2f9Li4k/700dMf64RVxPwki7W5z/pAPOtScPwbvSN 9GBcNSxlekfZ4KFV9KnpJKAtRlK7A== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduiedrvddugddviecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefhvffufffkjghfggfgtgesthfuredttddtvdenucfhrhhomhepvfhhohhmrghs ucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenucggtf frrghtthgvrhhnpedugefgvdefudfftdefgeelgffhueekgfffhfeujedtteeutdejueei iedvffegheenucfkphepjeejrddufeegrddvtdefrddukeegnecuvehluhhsthgvrhfuih iivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepthhhohhmrghssehmohhnjhgrlhho nhdrnhgvth X-ME-Proxy: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id 9C8713280059; Fri, 10 Jul 2020 06:07:59 -0400 (EDT) From: Thomas Monjalon To: "Zhang, AlvinX" Cc: dev@dpdk.org, Beilei Xing , Jeff Guo , David Marchand , anatoly.burakov@intel.com, ci@dpdk.org, ferruh.yigit@intel.com, bruce.richardson@intel.com, qian.q.xu@intel.com, john.mcnamara@intel.com, talshn@mellanox.com, rasland@mellanox.com, asafp@mellanox.com Date: Fri, 10 Jul 2020 12:07:58 +0200 Message-ID: <3033441.08XpM1RNeG@thomas> In-Reply-To: References: <20200708092435.9776-1-alvinx.zhang@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH] bus/pci: fix mmap PCI resource 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" 10/07/2020 11:54, David Marchand: > On Wed, Jul 8, 2020 at 11:26 AM wrote: > > From: Alvin Zhang > > > > When mapping a PCI BAR containing an MSI-X table, some devices do not > > need to actually map this BAR or only need to map part of them, which > > may cause the mapping to fail. Now some checks are added and a non-NULL > > initial value is set to the variable to avoid this situation. Note: this regression would not have happened if we had some CI tests for simple device probing. Please let's invest more in CI. > > Fixes: 2fd3567e5425 ("pci: use OS generic memory mapping functions") > > Cc: talshn@mellanox.com No he was not Cc in the thread. Same for Anatoly. Adding more people in Cc... > > Signed-off-by: Alvin Zhang > > --- > > --- a/drivers/bus/pci/linux/pci_vfio.c > > +++ b/drivers/bus/pci/linux/pci_vfio.c > > @@ -547,6 +547,14 @@ > > bar_index, > > memreg[0].offset, memreg[0].size, > > memreg[1].offset, memreg[1].size); > > + > > + if (memreg[0].size == 0 && memreg[1].size == 0) { > > + /* No need to map this BAR */ > > + RTE_LOG(DEBUG, EAL, "Skipping BAR%d\n", bar_index); > > + bar->size = 0; > > + bar->addr = 0; > > + return 0; > > + } > > We already have a check on bar size == 0. > Why would we have this condition? > Broken hw? > > > > } else { > > memreg[0].offset = bar->offset; > > memreg[0].size = bar->size; > > @@ -556,7 +564,9 @@ > > bar_addr = mmap(bar->addr, bar->size, 0, MAP_PRIVATE | > > MAP_ANONYMOUS | additional_flags, -1, 0); > > if (bar_addr != MAP_FAILED) { > > - void *map_addr = NULL; > > + /* Set non NULL initial value for in case of no PCI mapping */ > > + void *map_addr = bar_addr; > > + > > It took me some time to understand this code... > Anyway, we have a regression in the librte_pci. > This is where the fix should be. Yes, I am going to send a fix. > We can cleanup this code later. Yes please, this function isn't understandable and lack of comments. Anatoly please?