On Mon, Mar 06, 2023 at 05:13:27PM +0100, Thomas Monjalon wrote:
> The impact of the option "enable_iova_as_pa" is explained for users.
>
> Also the code flag "RTE_IOVA_AS_PA" is renamed as "RTE_IOVA_IN_MBUF"
> in order to be more accurate (IOVA mode is decided at runtime),
> and more readable in the code.
>
> Similarly the drivers are using the variable "require_iova_in_mbuf"
> instead of "pmd_supports_disable_iova_as_pa" with an opposite meaning.
> By default, it is assumed that drivers require the IOVA field in mbuf.
> The drivers which support removing this field have to declare themselves.
>
> If the option "enable_iova_as_pa" is disabled, the unsupported drivers
> will be listed with the new reason text "requires IOVA in mbuf".
>
> Suggested-by: Bruce Richardson <bruce.richard...@intel.com>
> Signed-off-by: Thomas Monjalon <tho...@monjalon.net>
> ---
Reviewed-by: Bruce Richardson <bruce.richard...@intel.com>
Couple of comments inline below.
> app/test/test_mbuf.c | 2 +-
<snip>
> @@ -127,9 +127,9 @@ foreach subpath:subdirs
> # pull in driver directory which should update all the local
> variables
> subdir(drv_path)
>
> - if dpdk_conf.get('RTE_IOVA_AS_PA') == 0 and not
> pmd_supports_disable_iova_as_pa and not always_enable.contains(drv_path)
> + if not get_option('enable_iova_as_pa') and require_iova_in_mbuf
> and not always_enable.contains(drv_path)
I don't particularly like the always_enable check at the end. If a driver
is set to always-enable and it doesn't support the configured mode, we
should throw an error immediately IMHO, rather than silently continuing to
build the driver.
> build = false
> - reason = 'driver does not support disabling IOVA as PA mode'
> + reason = 'requires IOVA in mbuf'
> endif
I think the reason given is more accurate, but for those users not familiar
with the internals of DPDK builds it could be confusing. I think the
message needs to reference the 'enable_iova_as_pa' option. How about?
reason = 'requires IOVA in mbuf (set enable_iova_as_pa option)'
/Bruce
