Re: [dpdk-dev] [PATCH v2 2/3] eal: compute IOVA mode based on PA availability

["Burakov, Anatoly" <anatoly.burakov@intel.com>]

On 14-Jun-19 10:39 AM, David Marchand wrote:
> From: Ben Walker <benjamin.walker@intel.com>
> 
> Currently, if the bus selects IOVA as PA, the memory init can fail when
> lacking access to physical addresses.
> This can be quite hard for normal users to understand what is wrong
> since this is the default behavior.
> 
> Catch this situation earlier in eal init by validating physical addresses
> availability, or select IOVA when no clear preferrence had been expressed.
> 
> The bus code is changed so that it reports when it does not care about
> the IOVA mode and let the eal init decide.
> 
> In Linux implementation, rework rte_eal_using_phys_addrs() so that it can
> be called earlier but still avoid a circular dependency with
> rte_mem_virt2phys().
> In FreeBSD implementation, rte_eal_using_phys_addrs() always returns
> false, so the detection part is left as is.
> 
> If librte_kni is compiled in and the KNI kmod is loaded,
> - if the buses requested VA, force to PA if physical addresses are
>    available as it was done before,
> - else, keep iova as VA, KNI init will fail later.
> 
> Signed-off-by: Ben Walker <benjamin.walker@intel.com>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---

<snip>

> +		/* autodetect the IOVA mapping mode */
> +		enum rte_iova_mode iova_mode = rte_bus_get_iommu_class();
>   
> +		if (iova_mode == RTE_IOVA_DC) {
> +			iova_mode = phys_addrs ? RTE_IOVA_PA : RTE_IOVA_VA;
> +			RTE_LOG(DEBUG, EAL,
> +				"Buses did not request a specific IOVA mode, using '%s' based on physical addresses availability.\n",
> +				phys_addrs ? "PA" : "VA");
> +		}
> +#ifdef RTE_LIBRTE_KNI
>   		/* Workaround for KNI which requires physical address to work */
> -		if (rte_eal_get_configuration()->iova_mode == RTE_IOVA_VA &&
> +		if (iova_mode == RTE_IOVA_VA &&
>   				rte_eal_check_module("rte_kni") == 1) {
> -			rte_eal_get_configuration()->iova_mode = RTE_IOVA_PA;
> -			RTE_LOG(WARNING, EAL,
> -				"Some devices want IOVA as VA but PA will be used because.. "
> -				"KNI module inserted\n");
> +			if (phys_addrs) {
> +				iova_mode = RTE_IOVA_PA;
> +				RTE_LOG(WARNING, EAL, "Forcing IOVA as 'PA' because KNI module is loaded\n");
> +			} else {
> +				RTE_LOG(DEBUG, EAL, "KNI can not work since physical addresses are unavailable\n");
> +			}
>   		}
> +#endif

Why the ifdefs? I don't think there's something specific to KNI there, 
rte_eal_check_module() works absent of KNI.

Otherwise LGTM,

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

I wish we would make IOVA as VA the default already, but at least 
picking it automatically when physical addresses aren't available is a 
good start :)

-- 
Thanks,
Anatoly