From: Jan Blunck <jblunck@infradead.org>
To: Olivier Matz <olivier.matz@6wind.com>
Cc: Gaetan Rivet <gaetan.rivet@6wind.com>, dev <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH 1/8] eal: expose rte_eal_using_phys_addrs
Date: Mon, 3 Jul 2017 12:04:37 +0200 [thread overview]
Message-ID: <CALe+Z01cQwRfy7DgNqRhbeMZv63u+C7T9RyzzMeHPRrZJW1WPA@mail.gmail.com> (raw)
In-Reply-To: <20170703102500.75cf7cd7@platinum>
On Mon, Jul 3, 2017 at 10:25 AM, Olivier Matz <olivier.matz@6wind.com> wrote:
> Hi,
>
> On Fri, 30 Jun 2017 21:05:47 +0200, Jan Blunck <jblunck@infradead.org> wrote:
>> On Thu, Jun 1, 2017 at 12:14 PM, Gaetan Rivet <gaetan.rivet@6wind.com> 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 <gaetan.rivet@6wind.com>
>> > ---
>> > 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?
next prev parent reply other threads:[~2017-07-03 10:04 UTC|newest]
Thread overview: 63+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-01 10:14 [dpdk-dev] [PATCH 0/8] bus/pci: remove PCI bus from EAL Gaetan Rivet
2017-06-01 10:14 ` [dpdk-dev] [PATCH 1/8] eal: expose rte_eal_using_phys_addrs Gaetan Rivet
2017-06-30 19:05 ` Jan Blunck
2017-07-03 8:25 ` Olivier Matz
2017-07-03 10:04 ` Jan Blunck [this message]
2017-07-03 11:49 ` Olivier Matz
2017-06-01 10:14 ` [dpdk-dev] [PATCH 2/8] ethdev: remove useless PCI dependency Gaetan Rivet
2017-06-01 10:14 ` [dpdk-dev] [PATCH 3/8] pmdinfogen: move to drivers subdirectory Gaetan Rivet
2017-06-01 10:14 ` [dpdk-dev] [PATCH 4/8] cryptodev: disabled by default Gaetan Rivet
2017-06-01 10:14 ` [dpdk-dev] [PATCH 5/8] eventdev: " Gaetan Rivet
2017-06-01 10:14 ` [dpdk-dev] [PATCH 6/8] pdump: " Gaetan Rivet
2017-06-01 10:14 ` [dpdk-dev] [PATCH 7/8] kni: " Gaetan Rivet
2017-06-01 10:14 ` [dpdk-dev] [PATCH 8/8] bus/pci: introduce pci bus Gaetan Rivet
2017-06-07 23:58 ` [dpdk-dev] [PATCH 0/8] bus/pci: remove PCI bus from EAL Gaetan Rivet
2017-06-07 23:58 ` [dpdk-dev] [PATCH v2 01/12] eal: expose rte_eal_using_phys_addrs Gaetan Rivet
2017-06-07 23:58 ` [dpdk-dev] [PATCH v2 02/12] ethdev: remove useless PCI dependency Gaetan Rivet
2017-06-07 23:58 ` [dpdk-dev] [PATCH v2 03/12] bus: properly include rte_debug Gaetan Rivet
2017-06-07 23:59 ` [dpdk-dev] [PATCH v2 04/12] eal: remove references to PCI Gaetan Rivet
2017-06-07 23:59 ` [dpdk-dev] [PATCH v2 05/12] pmdinfogen: move to drivers subdirectory Gaetan Rivet
2017-06-07 23:59 ` [dpdk-dev] [PATCH v2 06/12] cryptodev: disabled by default Gaetan Rivet
2017-06-09 15:03 ` De Lara Guarch, Pablo
2017-06-14 8:00 ` Gaëtan Rivet
2017-06-07 23:59 ` [dpdk-dev] [PATCH v2 07/12] pdump: " Gaetan Rivet
2017-06-09 14:24 ` Pattan, Reshma
2017-06-11 19:42 ` Gaëtan Rivet
2017-06-13 17:15 ` Ferruh Yigit
2017-06-14 23:01 ` Gaëtan Rivet
2017-06-15 13:07 ` Ferruh Yigit
2017-06-14 9:09 ` Pattan, Reshma
2017-06-14 9:20 ` Gaëtan Rivet
2017-06-07 23:59 ` [dpdk-dev] [PATCH v2 08/12] kni: " Gaetan Rivet
2017-06-09 8:56 ` Ferruh Yigit
2017-06-09 9:06 ` Gaëtan Rivet
2017-06-15 13:09 ` Ferruh Yigit
2017-06-15 14:48 ` Gaëtan Rivet
2017-06-07 23:59 ` [dpdk-dev] [PATCH v2 09/12] bus/pci: introduce pci bus Gaetan Rivet
2017-06-07 23:59 ` [dpdk-dev] [PATCH v2 10/12] bus/pci: follow checkpatch Gaetan Rivet
2017-06-07 23:59 ` [dpdk-dev] [PATCH v2 11/12] drivers: update eventdev dependencies Gaetan Rivet
2017-06-07 23:59 ` [dpdk-dev] [PATCH v2 12/12] drivers: update cryptodev dependencies Gaetan Rivet
2017-06-20 23:36 ` [dpdk-dev] [PATCH v3 0/9] bus/pci: remove PCI bus from EAL Gaetan Rivet
2017-06-20 23:36 ` [dpdk-dev] [PATCH v3 1/9] kni: disabled by default Gaetan Rivet
2017-06-20 23:36 ` [dpdk-dev] [PATCH v3 2/9] eal: expose rte_eal_using_phys_addrs Gaetan Rivet
2017-06-21 7:44 ` Thomas Monjalon
2017-06-21 9:21 ` Gaëtan Rivet
2017-06-20 23:36 ` [dpdk-dev] [PATCH v3 3/9] ethdev: remove useless PCI dependency Gaetan Rivet
2017-06-20 23:36 ` [dpdk-dev] [PATCH v3 4/9] bus: properly include rte_debug Gaetan Rivet
2017-06-21 7:45 ` Thomas Monjalon
2017-06-21 9:26 ` Gaëtan Rivet
2017-06-20 23:36 ` [dpdk-dev] [PATCH v3 5/9] pmdinfogen: move to drivers subdirectory Gaetan Rivet
2017-06-21 7:57 ` Thomas Monjalon
2017-06-21 9:40 ` Gaëtan Rivet
2017-06-21 10:00 ` Thomas Monjalon
2017-06-21 11:39 ` Gaëtan Rivet
2017-06-21 12:14 ` Thomas Monjalon
2017-06-20 23:36 ` [dpdk-dev] [PATCH v3 6/9] bus/pci: introduce pci bus Gaetan Rivet
2017-06-20 23:36 ` [dpdk-dev] [PATCH v3 7/9] bus/pci: follow checkpatch Gaetan Rivet
2017-06-20 23:36 ` [dpdk-dev] [PATCH v3 8/9] drivers: update eventdev dependencies Gaetan Rivet
2017-06-20 23:36 ` [dpdk-dev] [PATCH v3 9/9] drivers: update cryptodev dependencies Gaetan Rivet
2017-06-23 3:29 ` [dpdk-dev] [PATCH v3 0/9] bus/pci: remove PCI bus from EAL Tan, Jianfeng
2017-06-23 8:19 ` Gaëtan Rivet
2017-06-23 12:48 ` Thomas Monjalon
2017-06-23 14:35 ` Tan, Jianfeng
2017-06-23 14:45 ` Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CALe+Z01cQwRfy7DgNqRhbeMZv63u+C7T9RyzzMeHPRrZJW1WPA@mail.gmail.com \
--to=jblunck@infradead.org \
--cc=dev@dpdk.org \
--cc=gaetan.rivet@6wind.com \
--cc=olivier.matz@6wind.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).