[dpdk-dev] [PATCH 1/8] eal: expose rte_eal_using_phys_addrs

Jan Blunck jblunck at infradead.org
Mon Jul 3 12:04:37 CEST 2017


On Mon, Jul 3, 2017 at 10:25 AM, Olivier Matz <olivier.matz at 6wind.com> wrote:
> Hi,
>
> On Fri, 30 Jun 2017 21:05:47 +0200, Jan Blunck <jblunck at infradead.org> wrote:
>> On Thu, Jun 1, 2017 at 12:14 PM, Gaetan Rivet <gaetan.rivet at 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 at 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?


More information about the dev mailing list