Re: [dpdk-dev] [PATCH 3/7] mem: add function for setting DMA mask

[Alejandro Lucero <alejandro.lucero@netronome.com>]

On Thu, Nov 1, 2018 at 10:11 AM Burakov, Anatoly <anatoly.burakov@intel.com>
wrote:

> On 31-Oct-18 5:29 PM, Alejandro Lucero wrote:
> > This patch adds the possibility of setting a dma mask to be used
> > once the memory initialization is done.
> >
> > This is currently needed when IOVA mode is set by PCI related
> > code and an x86 IOMMU hardware unit is present. Current code calls
> > rte_mem_check_dma_mask but it is wrong to do so at that point
> > because the memory has not been initialized yet.
> >
> > Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>
> > ---
> >   lib/librte_eal/common/eal_common_memory.c  | 10 ++++++++++
> >   lib/librte_eal/common/include/rte_memory.h | 10 ++++++++++
> >   lib/librte_eal/rte_eal_version.map         |  1 +
> >   3 files changed, 21 insertions(+)
> >
> > diff --git a/lib/librte_eal/common/eal_common_memory.c
> b/lib/librte_eal/common/eal_common_memory.c
> > index e0f08f39a..24b72fcb0 100644
> > --- a/lib/librte_eal/common/eal_common_memory.c
> > +++ b/lib/librte_eal/common/eal_common_memory.c
> > @@ -480,6 +480,16 @@ rte_mem_check_dma_mask(uint8_t maskbits)
> >       return 0;
> >   }
> >
> > +/* set dma mask to use when memory initialization is done */
> > +void __rte_experimental
> > +rte_mem_set_dma_mask(uint8_t maskbits)
> > +{
> > +     struct rte_mem_config *mcfg =
> rte_eal_get_configuration()->mem_config;
> > +
> > +     mcfg->dma_maskbits = mcfg->dma_maskbits == 0 ? maskbits :
> > +                          RTE_MIN(mcfg->dma_maskbits, maskbits);
> > +}
> > +
> >   /* return the number of memory channels */
> >   unsigned rte_memory_get_nchannel(void)
> >   {
> > diff --git a/lib/librte_eal/common/include/rte_memory.h
> b/lib/librte_eal/common/include/rte_memory.h
> > index ad3f3cfb0..eff028db1 100644
> > --- a/lib/librte_eal/common/include/rte_memory.h
> > +++ b/lib/librte_eal/common/include/rte_memory.h
> > @@ -466,6 +466,16 @@ unsigned rte_memory_get_nrank(void);
> >   /* check memsegs iovas are within a range based on dma mask */
> >   int __rte_experimental rte_mem_check_dma_mask(uint8_t maskbits);
> >
> > +/**
> > + *  * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice
>
> I think there's a copy-paste error here (extra star and indent before
> warning).
>

I will fix this.

Thanks.


>
> > + *
> > + *  Set dma mask to use once memory initialization is done.
> > + *  Previous function rte_mem_check_dma_mask can not be used
> > + *  safely until memory has been initialized.
>
> IMO this really requires a big bold warning saying that this function
> must only be called early during initialization, before memory is
> initialized, and not to be called in user code.
>
>
what about adding a new EAL variable for keeping memory initialization
status?
It would be something like "uninit" until is changed to "done" after the
memory initialization is completed.
That variable would help to avoid effective calls to set the dma mask after
initialization.


> > + */
> > +void __rte_experimental rte_mem_set_dma_mask(uint8_t maskbits);
> > +
> >   /**
> >    * Drivers based on uio will not load unless physical
> >    * addresses are obtainable. It is only possible to get
> > diff --git a/lib/librte_eal/rte_eal_version.map
> b/lib/librte_eal/rte_eal_version.map
> > index ef8126a97..ae24b5c73 100644
> > --- a/lib/librte_eal/rte_eal_version.map
> > +++ b/lib/librte_eal/rte_eal_version.map
> > @@ -296,6 +296,7 @@ EXPERIMENTAL {
> >       rte_devargs_remove;
> >       rte_devargs_type_count;
> >       rte_mem_check_dma_mask;
> > +     rte_mem_set_dma_mask;
>
> Same, this needs to be alphabetic.
>
> >       rte_eal_cleanup;
> >       rte_fbarray_attach;
> >       rte_fbarray_destroy;
> >
>
>
> --
> Thanks,
> Anatoly
>