[dpdk-stable] [PATCH 1/6] mem: add function for checking memsegs IOVAs addresses

[Burakov, Anatoly]

On 02-Jul-18 6:26 PM, Alejandro Lucero wrote:
> A device can suffer addressing limitations. This functions checks
> memsegs have iovas within the supported range based on dma mask.
> 
> PMD should use this during initialization if supported devices
> suffer addressing limitations, returning an error if this function
> returns memsegs out of range.
> 
> Another potential usage is for emulated IOMMU hardware with addressing
> limitations.
> 
> Signed-off-by: Alejandro Lucero <alejandro.lucero at netronome.com>
> ---

<snip>

> +	const struct rte_mem_config *mcfg;
> +	uint64_t mask;
> +	int i;
> +	int ret = 0;
> +
> +	/* create dma mask */
> +	mask = 1ULL << maskbits;
> +	mask -= 1;

mask = ~((1ULL << maskbits) - 1);

? IMO this makes it much more clear what you're trying to do.

> +
> +	/* get pointer to global configuration */
> +	mcfg = rte_eal_get_configuration()->mem_config;
> +
> +	for (i = 0; i < RTE_MAX_MEMSEG; i++) {
> +		if (mcfg->memseg[i].addr == NULL)
> +			break;
> +
> +		if (mcfg->memseg[i].iova & ~mask) {
> +			ret = -1;
> +			break;
> +		}
> +	}
> +
> +	if (!ret)
> +		return ret;
> +
> +	RTE_LOG(INFO, EAL, "memseg[%d] iova %"PRIx64" out of range:\n",
> +			   i, mcfg->memseg[i].iova);
> +	RTE_LOG(INFO, EAL, "\tusing dma mask %"PRIx64"\n", mask);
> +
> +	return -1;

The control flow looks weird to me. You break if iova has any bits that 
are in the mask, then you display log messages and return -1. How about 
just logging error and returning -1, and simply returning 0 after the loop?

-- 
Thanks,
Anatoly