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 8CB6BA052A; Fri, 10 Jul 2020 14:31:41 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 0C1F01DC45; Fri, 10 Jul 2020 14:31:40 +0200 (CEST) Received: from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com [66.111.4.25]) by dpdk.org (Postfix) with ESMTP id 7E78A1D8EC; Fri, 10 Jul 2020 14:31:37 +0200 (CEST) Received: from compute7.internal (compute7.nyi.internal [10.202.2.47]) by mailout.nyi.internal (Postfix) with ESMTP id E03745C0120; Fri, 10 Jul 2020 08:31:36 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute7.internal (MEProxy); Fri, 10 Jul 2020 08:31:36 -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= dSZWwGwNyKSshzJ0Y8FSZkfuNdaP8XUOZ8knID8tIas=; b=tOKsB6AZkA7oJEqs dY8SJNUbYU0YWq5Gc9OAnB+Qf1KCxtoJBakPnFBn6gANTminbupWSNqr+9Gqqk8Y 1C79igt5G1TSLWWwv8P6VeOj/Hi5E0++BNOz5oEXYFLZeu9/ieFjklkhVfW6UVAa Gq2AKNbqaB54+/UKQhfmw0s4MuNVQrtg0F+fqpmTMS96MjhuDwpzhowWMWqwV8bD hqQ8G5kpHSb9IM7PfkTWgbDV+8tWpq7RszvQES7T41MA3mJ3tLj1gKUn8zH28uk4 og8gPvww7n9LlYnjRW1uBxFb3L5C0EuoLwlu5jN76ksVcR+aYUKkqJr2iRs9HQ0R g3z5Ng== 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=dSZWwGwNyKSshzJ0Y8FSZkfuNdaP8XUOZ8knID8tI as=; b=OeSdM301B1IfoigdZpsqsOnJ8cXaVU7x1sADdo3Tu+cw70C39+tzlo0bo xj47P/0VTglYJWptRr/Fs2lUgoWtOQnJV28vWWxbsv/Z/oNc5t9ZoaIvS62ZqQ7U AonUsE1LaGhNxKVsl5SYePBrsi/ngx7LYcOQjUM+DP/6Bessc1OQvpyHSj1hv1a3 kN6cVrIcWI0AowXOtktc1LPtaawAOnJ3lqH1hKRe41VVcFVOPDPbOTWe++tY1FmO Sde8S3Y6jCzlHz0ARAIaUWG4kZKTVhLI/IrwkGLmRVgwt7Su4EDg37mDOpXkg6X+ Mlha8ggkXk4AM2bY74vlG+7Nhgrjw== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduiedrvddugdehjecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefhvffufffkjghfggfgtgesthfuredttddtvdenucfhrhhomhepvfhhohhmrghs ucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenucggtf frrghtthgvrhhnpeffvdffjeeuteelfeeileduudeugfetjeelveefkeejfeeigeehteff vdekfeegudenucffohhmrghinhepughpughkrdhorhhgnecukfhppeejjedrudefgedrvd dtfedrudekgeenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhr ohhmpehthhhomhgrshesmhhonhhjrghlohhnrdhnvght 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 2690E30600B4; Fri, 10 Jul 2020 08:31:35 -0400 (EDT) From: Thomas Monjalon To: "Zhang, AlvinX" , anatoly.burakov@intel.com Cc: dev@dpdk.org, Beilei Xing , Jeff Guo , David Marchand , ci@dpdk.org, ferruh.yigit@intel.com, bruce.richardson@intel.com, talshn@mellanox.com Date: Fri, 10 Jul 2020 14:31:34 +0200 Message-ID: <4953813.kqYPlVRg1Q@thomas> In-Reply-To: <3033441.08XpM1RNeG@thomas> References: <20200708092435.9776-1-alvinx.zhang@intel.com> <3033441.08XpM1RNeG@thomas> 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 12:07, Thomas Monjalon: > 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. [...] > > > --- 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. Patch sent: https://patches.dpdk.org/patch/73741/ This patch is marked as rejected, but please follow-up on cleanup. > > We can cleanup this code later. > > Yes please, this function isn't understandable and lack of comments. > Anatoly please?