Re: [PATCH] net/hns3: support disable IOVA as PA mode

[Thomas Monjalon <thomas@monjalon.net>]

20/02/2023 10:43, Morten Brørup:
> > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > Sent: Monday, 20 February 2023 08.45
> > 
> > 16/02/2023 09:36, Ruifeng Wang:
> > > From: Chengwen Feng <fengchengwen@huawei.com>
> > > > Subject: [PATCH] net/hns3: support disable IOVA as PA mode
> > 
> > Could we change the title to "support IOVA as VA" ?
> 
> The underlying problem is the meson configuration option name for this feature [1]:
> 
> option('enable_iova_as_pa', type: 'boolean', value: true, description:
>        'Support for IOVA as physical address. Disabling removes the buf_iova field of mbuf.')
> 
> [1]: https://elixir.bootlin.com/dpdk/v22.11.1/source/meson_options.txt#L43
> 
> Formally, the patch provides the ability to set a boolean configuration value ("enable_iova_as_pa") to false, and thus the patch title is correct.
> 
> Nonetheless, I agree that the title suggested by Thomas is an improvement.
> 
> 
> Going back to the root cause, I think the configuration option should be an enum instead of a boolean, e.g. "iova_mode" with values "iova_pa" and "iova_va".

We can enable both and have it decided at runtime. So I think the boolean is OK.

> It's somewhat similar to CPU endian macros. We have macros defining both Big Endian and Little Endian, not just one macro defining Big Endian or not.
> 
> @Bruce, would it be hard for you to change the IOVA configuration option from a boolean to a two-value enum?
> 
> Or - also considering the resulting #define's - would it be too difficult to keep a sufficient level of backwards/API compatibility?