On Thu, Nov 1, 2018 at 11:12 AM Burakov, Anatoly <anatoly.bura...@intel.com> wrote:
> On 01-Nov-18 11:03 AM, Alejandro Lucero wrote: > > > > > > On Thu, Nov 1, 2018 at 10:29 AM Burakov, Anatoly > > <anatoly.bura...@intel.com <mailto:anatoly.bura...@intel.com>> wrote: > > > > On 31-Oct-18 5:29 PM, Alejandro Lucero wrote: > > > If DMA mask checks shows mapped memory out of the supported range > > > specified by the DMA mask, nothing can be done but return an error > > > an report the error. This can imply the app not being executed at > > > all or precluding dynamic memory allocation once the app is > running. > > > In any case, we can advice the user to force IOVA as PA if > currently > > > IOVA being VA and user being root. > > > > > > Signed-off-by: Alejandro Lucero <alejandro.luc...@netronome.com > > <mailto:alejandro.luc...@netronome.com>> > > > --- > > > > General comment - legacy memory will also need this check, correct? > > > > > > Yes, there is another patch adding this for both, legacy-mem and no-huge > > options. > > > > > lib/librte_eal/common/malloc_heap.c | 35 > > +++++++++++++++++++++++++---- > > > 1 file changed, 31 insertions(+), 4 deletions(-) > > > > > > diff --git a/lib/librte_eal/common/malloc_heap.c > > b/lib/librte_eal/common/malloc_heap.c > > > index 7d423089d..711622f19 100644 > > > --- a/lib/librte_eal/common/malloc_heap.c > > > +++ b/lib/librte_eal/common/malloc_heap.c > > > @@ -5,8 +5,10 @@ > > > #include <stddef.h> > > > #include <stdlib.h> > > > #include <stdio.h> > > > +#include <unistd.h> > > > #include <stdarg.h> > > > #include <errno.h> > > > +#include <sys/types.h> > > > #include <sys/queue.h> > > > > > > #include <rte_memory.h> > > > @@ -294,7 +296,6 @@ alloc_pages_on_heap(struct malloc_heap *heap, > > uint64_t pg_sz, size_t elt_size, > > > size_t alloc_sz; > > > int allocd_pages; > > > void *ret, *map_addr; > > > - uint64_t mask; > > > > > > alloc_sz = (size_t)pg_sz * n_segs; > > > > > > @@ -322,11 +323,37 @@ alloc_pages_on_heap(struct malloc_heap > > *heap, uint64_t pg_sz, size_t elt_size, > > > goto fail; > > > } > > > > > > + /* Once we have all the memseg lists configured, if there > > is a dma mask > > > + * set, check iova addresses are not out of range. > > Otherwise the device > > > + * setting the dma mask could have problems with the mapped > > memory. > > > + * > > > + * There are two situations when this can happen: > > > + * 1) memory initialization > > > + * 2) dynamic memory allocation > > > + * > > > + * For 1), an error when checking dma mask implies app can > > not be > > > + * executed. For 2) implies the new memory can not be added. > > > + */ > > > if (mcfg->dma_maskbits) { > > > if (rte_mem_check_dma_mask(mcfg->dma_maskbits)) { > > > - RTE_LOG(ERR, EAL, > > > - "%s(): couldn't allocate memory due > > to DMA mask\n", > > > - __func__); > > > + /* Currently this can only happen if IOMMU > > is enabled > > > + * with RTE_ARCH_X86. It is not safe to use > > this memory > > > + * so returning an error here. > > > > I don't think it's RTE_ARCH_X86-only. It can be any other arch with > an > > IOMMU that's reporting addressing limitations. > > > > > > Right, but it is just IOMMU hardware from this architecture having the > > current limitation. > > I was trying to just explain why this can happen but I can remove the > > reference to specific > > architecture problems. > > > > > > > + * > > > + * If IOVA is VA, advice to try with > > '--iova-mode pa' > > > + * which could solve some situations when > > IOVA VA is not > > > + * really needed. > > > + */ > > > + uid_t user = getuid(); > > > + if ((rte_eal_iova_mode() == RTE_IOVA_VA) && > > user == 0) > > > > rte_eal_using_phys_addrs()? > > > > (the above function name is a bit of a misnomer, it really checks if > > they are available, but not necessarily used - it will return true in > > RTE_IOVA_VA mode if you're running as root) > > > > > > rte_eal_iova_mode returns rte_eal_get_configuration()->iova_mode what > > is set during initialization. It can be PA not just because IOMMU (not > > after the patch) > > but because some PMD does not reports IOVA VA support or because UIO is > > in use. > > Checking for root is because IOVA PA can not be used if non root. > > You've misinterpreted my comment. > > rte_eal_using_phys_addrs() will effectively return true if you're > running as root. There's no need for an uid check. > > Ok. I got it now, and it is definitely better than adding the uid check. Thanks > The "misnomer" comment was about the rte_eal_using_phys_addrs() - it > reads like it would return false in IOVA_VA mode, but in reality, it > will return true even if IOVA_VA mode - it really should be named > "rte_eal_phys_addrs_available()" rather than > "rte_eal_using_phys_addrs()". This would make it clearer. > > > > > > > > + RTE_LOG(ERR, EAL, > > > + "%s(): couldn't allocate > > memory due to DMA mask.\n" > > > + "Try with 'iova-mode=pa'\n", > > > + __func__); > > > + else > > > + RTE_LOG(ERR, EAL, > > > + "%s(): couldn't allocate > > memory due to DMA mask\n", > > > + __func__); > > > > I don't think the error message is terribly descriptive. Looking at > it > > through the eyes of someone who sees it for the first time and who > has > > no idea what "iova-mode=pa" is, i think it would be more useful to > word > > it the following way: > > > > couldn't allocate memory due to IOVA exceeding limits of current DMA > > mask. > > [for non-using phys addrs case] Please try initializing EAL with > > --iova-mode=pa parameter. > > > > > > I'm happy with using your terrific description instead ;-) > > Thanks! > > > > Also, generally newlines in RTE_LOG look ugly unless you indent the > > line :) > > > > > goto fail; > > > } > > > } > > > > > > > > > -- > > Thanks, > > Anatoly > > > > > -- > Thanks, > Anatoly >
