RE: [PATCH v1 1/4] build: add meson option to configure IOVA mode as VA

["Morten Brørup" <mb@smartsharesystems.com>]

> From: Shijith Thotton [mailto:sthotton@marvell.com]
> Sent: Monday, 29 August 2022 17.16
> 
> IOVA mode in DPDK is either PA or VA. The new build option iova_as_va
> configures the mode to VA at compile time and prevents setting it to PA
> at runtime. For now, all drivers which are not always enabled are
> disabled with this option. Supported driver can set the flag
> pmd_iova_as_va in its build file to enable build.
> 
> mbuf structure holds the physical (PA) and virtual address (VA) of a
> buffer. if IOVA mode is set to VA, PA is redundant as it is the same as
> VA. So PA field need not be updated and marked invalid if the build is
> configured to use only VA.
> 
> Signed-off-by: Shijith Thotton <sthotton@marvell.com>
> ---

[...]

> diff --git a/app/test/test_mbuf.c b/app/test/test_mbuf.c
> index e09b2549ca..992b8c64ab 100644
> --- a/app/test/test_mbuf.c
> +++ b/app/test/test_mbuf.c
> @@ -1232,11 +1232,13 @@ test_failing_mbuf_sanity_check(struct
> rte_mempool *pktmbuf_pool)
>  		return -1;
>  	}
> 
> -	badbuf = *buf;
> -	badbuf.buf_iova = 0;
> -	if (verify_mbuf_check_panics(&badbuf)) {
> -		printf("Error with bad-physaddr mbuf test\n");
> -		return -1;
> +	if (!rte_is_iova_as_va_build()) {
> +		badbuf = *buf;
> +		rte_mbuf_iova_set(&badbuf, 0);
> +		if (verify_mbuf_check_panics(&badbuf)) {
> +			printf("Error with bad-physaddr mbuf test\n");
> +			return -1;
> +		}
>  	}
> 
>  	badbuf = *buf;
> diff --git a/config/meson.build b/config/meson.build
> index 7f7b6c92fd..1ff1cd774b 100644
> --- a/config/meson.build
> +++ b/config/meson.build
> @@ -309,6 +309,9 @@ endif
>  if get_option('mbuf_refcnt_atomic')
>      dpdk_conf.set('RTE_MBUF_REFCNT_ATOMIC', true)
>  endif
> +if get_option('iova_as_va')
> +    dpdk_conf.set('RTE_IOVA_AS_VA', true)
> +endif
> 
>  compile_time_cpuflags = []
>  subdir(arch_subdir)
> diff --git a/drivers/meson.build b/drivers/meson.build
> index b22c2adda7..469e60f1fa 100644
> --- a/drivers/meson.build
> +++ b/drivers/meson.build
> @@ -103,6 +103,7 @@ foreach subpath:subdirs
>          ext_deps = []
>          pkgconfig_extra_libs = []
>          testpmd_sources = []
> +        pmd_iova_as_va = false
> 
>          if not enable_drivers.contains(drv_path)
>              build = false
> @@ -120,6 +121,11 @@ foreach subpath:subdirs
>              # pull in driver directory which should update all the
> local variables
>              subdir(drv_path)
> 
> +            if dpdk_conf.has('RTE_IOVA_AS_VA') and not pmd_iova_as_va
> and not always_enable.contains(drv_path)
> +                build = false
> +                reason = 'driver does not support IOVA as VA mode'
> +            endif
> +
>              # get dependency objs from strings
>              shared_deps = ext_deps
>              static_deps = ext_deps
> diff --git a/lib/eal/include/rte_common.h
> b/lib/eal/include/rte_common.h
> index a96cc2a138..0010ad7c7d 100644
> --- a/lib/eal/include/rte_common.h
> +++ b/lib/eal/include/rte_common.h
> @@ -921,6 +921,23 @@ __rte_noreturn void
>  rte_exit(int exit_code, const char *format, ...)
>  	__rte_format_printf(2, 3);
> 
> +/**
> + * Check if build is configured to use IOVA as VA.
> + *
> + * @return
> + *   1 if true, 0 otherwise
> + *
> + */
> +static inline int
> +rte_is_iova_as_va_build(void)
> +{
> +#ifdef RTE_IOVA_AS_VA
> +	return 1;
> +#else
> +	return 0;
> +#endif
> +}

The rte_is_iova_as_va_build() function is effectively a shadow of the RTE_IOVA_AS_VA definition. Why the need to camouflage RTE_IOVA_AS_VA through a function, instead of just using RTE_IOVA_AS_VA everywhere?