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

[Bruce Richardson]

On Mon, Aug 29, 2022 at 08:18:56PM +0200, Morten Brørup wrote:
> > From: Shijith Thotton [mailto:sthotton at 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 at 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?
> 
My reading is that it's not quite equivalent, and in the undef case it
can't be directly used in C code. You can't do "if (RTE_IOVA_AS_VA)", for
example. However, rather than adding a function, in meson you could also
add "dpdk_conf.set10(RTE_IOVA....)" to define a second macro that is 0 in
the undef case, and which therefore could be used in C conditionals.

/Bruce