On 01-Nov-18 10:48 AM, Alejandro Lucero wrote:
On Thu, Nov 1, 2018 at 10:11 AM Burakov, Anatoly <anatoly.bura...@intel.com <mailto:anatoly.bura...@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.luc...@netronome.com <mailto:alejandro.luc...@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.
I'm not opposed to it in principle, however, this is a slippery slope towards having each and every subsystem storing their init status :)
That said, while it's not a complete solution because it won't prevent spurious calls to this function _after_ memory has initialized, there is a variable in internal_config that you can use to deny calls to this function after EAL init is complete (see internal_config->init_complete).
You could also store a static variable in eal_common_memory.c, and change it at the end of rte_eal_memory_init() call, but that does not really complete the memory initialization, because we then go to rte_eal_malloc_heap_init(), which actually completes the memory init.
I would greatly prefer using internal_config->init_complete - it is enough to forbid user code from calling it, while driver/DPDK developers presumably know what they're doing enough to read the warning and not try to call this after memory init :)
> + */ > +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
-- Thanks, Anatoly
