On Tue, Jul 3, 2018 at 10:07 AM, Burakov, Anatoly <anatoly.bura...@intel.com
> wrote:
> 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.luc...@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.
>
>
Sure. I will change it.
> +
>> + /* 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?
>
>
I agree. The truth is I did that initially, but the log lines were too long
with the indent. I will go back to the original.
Thanks
> --
> Thanks,
> Anatoly
>
