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

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

> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Monday, 20 February 2023 16.41
> 
> 20/02/2023 16:35, Bruce Richardson:
> > On Mon, Feb 20, 2023 at 04:30:20PM +0100, Thomas Monjalon wrote:
> > > 20/02/2023 16:07, Morten Brørup:
> > > > With the new viewing angle, the current define RTE_IOVA_AS_PA
> makes more sense to me now than before. So we should probably stick
> with it, rather than introduce something that might confuse developers
> who already have the same viewing angle.
> > > >
> > > > But it still seems counterintuitive to me that disabling some
> feature ("enable_iova_as_pa") is not supported throughout DPDK; the
> logic seems inverted. Apparently, it also makes it difficult to assign
> good titles to patches that support disabling such a feature. :-)
> > > >
> > > > <irony>
> > > > On the positive side, since everything supports this
> "enable_iova_as_pa" feature, we don't need to add it to the PMD feature
> list. If the logic wasn't inverted like this, the PMD feature list
> should probably reflect which PMDs supported the "iova_as_va_only"
> compile time option. ;-)
> > > > </irony>
> > >
> > > That's a change I would like to do:
> > > The Meson variable in the drivers should be "support_iova_as_va"
> > > and would mean we can compile the driver when "enable_iova_as_pa"
> is false.
> > >
> > All drivers (that I am aware of) support iova_as_va. What is missing
> is
> > drivers supporting "iova_as_va_only". Any reference to va without the
> word
> > "only" on it will be misleading.
> >
> > A third way of looking at it, is to work with the fact that the
> reason
> > drivers require changes to support this "va_only" mode, ro no-pa
> mode, is
> > due to the fact that the mbuf no longer tracks iovas and only VAs.
> > Therefore, we can have a variable called "require_iova_in_mbuf",
> which
> > would hopefully cut through this whole va vs pa addition/subtraction
> mess.
> > What do you think?
> 
> Yes "require_iova_in_mbuf" describes better the reality,
> so it is simpler to understand.

Agreed. It is a good idea narrowing the scope to the concrete issue.

Proceeding to also consider the source code using the corresponding #define, e.g. the rte_mbuf:
https://elixir.bootlin.com/dpdk/v23.03-rc1/source/lib/mbuf/rte_mbuf_core.h#L469

Would you also change the corresponding #define from RTE_IOVA_AS_PA to RTE_REQUIRE_IOVA_IN_MBUF?

Perhaps we could invert the value, e.g. "mbuf_without_iova" and #define RTE_MBUF_WITHOUT_IOVA? Intuitively, a default value of "false" better reflects that the feature is optional.

PS: I don't mind if you limit this to meson, and ignore my feature creep expanding into #define land.