From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f176.google.com (mail-wr0-f176.google.com [209.85.128.176]) by dpdk.org (Postfix) with ESMTP id ECF5B2BF2 for ; Mon, 3 Jul 2017 13:49:16 +0200 (CEST) Received: by mail-wr0-f176.google.com with SMTP id 77so233048986wrb.1 for ; Mon, 03 Jul 2017 04:49:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=l1MWDlNoEq0VyKLwsFvt2NARXfOMq8GWp6SDetw9/uo=; b=Fov+t38HF6odFmx27ojmzcXf0Klf4J3uaveDS9C5w6UTil02/pM/qPX1XcDt3tTjZQ X4EHcgkW6Mxlhz1SMZmifnvZtn5Oh3+s9geb4FqMEPJZbdUAjmv1ycHtynGmIIJDJ//c 0pWLyLeROlm582TWKH42kRARvJkDvNf++Ihlbmqxv7gYQQg5ZucApFJaumG20eSg+die AuptzHvjmI07Cm8WevDsf2GfCwQGmDH6HZkEJGjErwPmIPCAyW2KCmApAbjta7K5+Jbh 2zmo2cTE56Xf3Jcqn3hQGLjnxBfa2MzfvnVfszS/UqCbSgbOYEkiYRj8zF5SbwgbDibN 7/kA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=l1MWDlNoEq0VyKLwsFvt2NARXfOMq8GWp6SDetw9/uo=; b=N8SK7lmtbYEH3eIGfbx3cTiLuTn8vktgDvVcjSoMskWs9c2J/6QgLlAA+YBNsvhjCf HSiPh2o89ctV8CLptBhFE84J+Kq9qD1jC9mfNmsjKC4AQIh+WQVyXDnAMmplqQma8Z8y g4GuoU7ojtDGQ6qqkVRjBwKJEmFgcJ4qfTcPPWVYeQXnfKRSlDu500DNPXESCpMukPlT nhuRfeNvJiFJo9oLvL0qafmOLrs3yriWinYJv7/dVWVCDPpNeyE0narwfiB0RycpsS7c uQLf7Q4MnPhvqNN3gWpJh78NJJwmI/2thBOHlHedbGccOXXIEcp9ryr+x2bci8BmO9Cn sMTw== X-Gm-Message-State: AKS2vOzs3bjip4kCnl1vsEBpOAr6RaCR7MYqiimw+4BztyjwfuOzDf5b eO6qaKN9YpARRuHyjpo= X-Received: by 10.223.166.8 with SMTP id k8mr34246845wrc.172.1499082556545; Mon, 03 Jul 2017 04:49:16 -0700 (PDT) Received: from platinum (2a01cb0c03c651000226b0fffeed02fc.ipv6.abo.wanadoo.fr. [2a01:cb0c:3c6:5100:226:b0ff:feed:2fc]) by smtp.gmail.com with ESMTPSA id p87sm31272238wma.2.2017.07.03.04.49.16 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 03 Jul 2017 04:49:16 -0700 (PDT) Date: Mon, 3 Jul 2017 13:49:13 +0200 From: Olivier Matz To: Jan Blunck Cc: Gaetan Rivet , dev Message-ID: <20170703134913.2953592d@platinum> In-Reply-To: References: <138ba842ce615de6a6895f0ca74b5645aad48ad9.1496308649.git.gaetan.rivet@6wind.com> <20170703102500.75cf7cd7@platinum> X-Mailer: Claws Mail 3.14.1 (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH 1/8] eal: expose rte_eal_using_phys_addrs 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: , X-List-Received-Date: Mon, 03 Jul 2017 11:49:17 -0000 On Mon, 3 Jul 2017 12:04:37 +0200, Jan Blunck wrote: > On Mon, Jul 3, 2017 at 10:25 AM, Olivier Matz wrote: > > Hi, > > > > On Fri, 30 Jun 2017 21:05:47 +0200, Jan Blunck wrote: > >> On Thu, Jun 1, 2017 at 12:14 PM, Gaetan Rivet wrote: > >> > This function was previously private to the EAL layer. > >> > Other subsystems requires it, such as the PCI bus. > >> > > >> > This function is only exposed for linuxapps. > >> > > >> > In order not to force other components to include stdbool, which is > >> > incompatible with several NIC drivers, the return type has > >> > been changed from bool to int. > >> > > >> > Signed-off-by: Gaetan Rivet > >> > --- > >> > lib/librte_eal/common/eal_private.h | 11 ----- > >> > lib/librte_eal/linuxapp/eal/Makefile | 2 + > >> > lib/librte_eal/linuxapp/eal/eal_memory.c | 3 +- > >> > lib/librte_eal/linuxapp/eal/eal_pci.c | 1 + > >> > lib/librte_eal/linuxapp/eal/rte_eal_version.map | 1 + > >> > lib/librte_eal/linuxapp/eal/rte_memory_linux.h | 64 +++++++++++++++++++++++++ > >> > 6 files changed, 70 insertions(+), 12 deletions(-) > >> > create mode 100644 lib/librte_eal/linuxapp/eal/rte_memory_linux.h > >> > > >> > diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h > >> > index 6d2206a..b7e4cc6 100644 > >> > --- a/lib/librte_eal/common/eal_private.h > >> > +++ b/lib/librte_eal/common/eal_private.h > >> > @@ -327,17 +327,6 @@ int rte_eal_hugepage_init(void); > >> > */ > >> > int rte_eal_hugepage_attach(void); > >> > > >> > -/** > >> > - * Returns true if the system is able to obtain > >> > - * physical addresses. Return false if using DMA > >> > - * addresses through an IOMMU. > >> > - * > >> > - * Drivers based on uio will not load unless physical > >> > - * addresses are obtainable. It is only possible to get > >> > - * physical addresses when running as a privileged user. > >> > - */ > >> > -bool rte_eal_using_phys_addrs(void); > >> > - > >> > /* > >> > * Validate a bus name. > >> > * > >> > diff --git a/lib/librte_eal/linuxapp/eal/Makefile b/lib/librte_eal/linuxapp/eal/Makefile > >> > index 640afd0..530e286 100644 > >> > --- a/lib/librte_eal/linuxapp/eal/Makefile > >> > +++ b/lib/librte_eal/linuxapp/eal/Makefile > >> > @@ -131,4 +131,6 @@ INC := rte_interrupts.h rte_kni_common.h rte_dom0_common.h > >> > SYMLINK-$(CONFIG_RTE_EXEC_ENV_LINUXAPP)-include/exec-env := \ > >> > $(addprefix include/exec-env/,$(INC)) > >> > > >> > +SYMLINK-$(CONFIG_RTE_EXEC_ENV_LINUXAPP)-include += rte_memory_linux.h > >> > + > >> > include $(RTE_SDK)/mk/rte.lib.mk > >> > diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c b/lib/librte_eal/linuxapp/eal/eal_memory.c > >> > index ebe0683..072bfe4 100644 > >> > --- a/lib/librte_eal/linuxapp/eal/eal_memory.c > >> > +++ b/lib/librte_eal/linuxapp/eal/eal_memory.c > >> > @@ -99,6 +99,7 @@ > >> > #include "eal_internal_cfg.h" > >> > #include "eal_filesystem.h" > >> > #include "eal_hugepages.h" > >> > +#include "rte_memory_linux.h" > >> > > >> > #define PFN_MASK_SIZE 8 > >> > > >> > @@ -1472,7 +1473,7 @@ rte_eal_hugepage_attach(void) > >> > return -1; > >> > } > >> > > >> > -bool > >> > +int > >> > rte_eal_using_phys_addrs(void) > >> > { > >> > return phys_addrs_available; > >> > diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c > >> > index 595622b..9d5b051 100644 > >> > --- a/lib/librte_eal/linuxapp/eal/eal_pci.c > >> > +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c > >> > @@ -45,6 +45,7 @@ > >> > #include "eal_filesystem.h" > >> > #include "eal_private.h" > >> > #include "eal_pci_init.h" > >> > +#include "rte_memory_linux.h" > >> > > >> > /** > >> > * @file > >> > diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map > >> > index a5127d6..8916520 100644 > >> > --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map > >> > +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map > >> > @@ -207,5 +207,6 @@ DPDK_17.08 { > >> > > >> > rte_bus_from_name; > >> > rte_bus_from_dev; > >> > + rte_eal_using_phys_addrs; > >> > >> Is this only for the UIO mapping? Would it be better to let UIO skip > >> (and warn?) over RTE_BAD_PHYS_ADDR mappings? That way we don't need > >> this export and its probably more resilient to bad mappings. > >> > >> Thoughts? Olivier? > > > > From what I see, rte_eal_using_phys_addrs() is used by: > > > > rte_eal_pci_map_device(struct rte_pci_device *dev) > > { > > ... > > case RTE_KDRV_IGB_UIO: > > case RTE_KDRV_UIO_GENERIC: > > if (rte_eal_using_phys_addrs()) { > > /* map resources for devices that use uio */ > > ret = pci_uio_map_resource(dev); > > } > > > > > > Looking at pci_uio_map_resource() and its callees, I'm not sure it's > > easy to replace it something else. The functions that would return > > RTE_BAD_PHYS_ADDR (virt2phy-like) are not called there. > > > > Right, so why is it there in the first place? From what I can see the > code should work just fine even in cases we don't have physical > addresses available. Shouldn't the code actually requiring physical > addresses (e.g. RX queue setup, ...) check for this instead? My understanding of the commit cdc242f260e7 ("eal/linux: support running as unprivileged user") is: if dpdk is started with hugepages but without the privilege to get the physical address, the memsegs are filled with dummy physical addresses starting at 0. The mapping of these dummy physical addresses as IOVA is done in eal_vfio.c. So, back to our question, I think I was wrong in my previous answer, and virt2phy-like functions would not return RTE_BAD_PHYS_ADDR in that case, they will return the dummy physical address instead. Maybe the function rte_eal_using_phys_addrs() could have a better name, but I think having this guard here is correct. Olivier