DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH] common/cnxk: allow enabling IOVA field in mbuf
@ 2024-10-14 11:06 Shijith Thotton
  2024-10-24 11:10 ` Jerin Jacob
  2024-10-25  9:50 ` [PATCH v2] " Shijith Thotton
  0 siblings, 2 replies; 11+ messages in thread
From: Shijith Thotton @ 2024-10-14 11:06 UTC (permalink / raw)
  To: jerinj, pbhagavatula
  Cc: dev, Shijith Thotton, Wathsala Vithanage, Bruce Richardson,
	Nithin Dabilpuram, Kiran Kumar K, Sunil Kumar Kori, Satha Rao,
	Harman Kalra

Value of RTE_IOVA_IN_MBUF was always disabled on cnxk platforms, as IOVA
in the mbuf is not required. This change modifies that behavior,
allowing RTE_IOVA_IN_MBUF to be enabled if the build option
-Denable_iova_as_pa=true is explicitly specified.

Signed-off-by: Shijith Thotton <sthotton@marvell.com>
---
 config/arm/meson.build          | 8 ++------
 drivers/common/cnxk/meson.build | 9 +++++++++
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/config/arm/meson.build b/config/arm/meson.build
index 012935d5d7..ca54524376 100644
--- a/config/arm/meson.build
+++ b/config/arm/meson.build
@@ -439,10 +439,7 @@ soc_cn9k = {
     'description': 'Marvell OCTEON 9',
     'implementer': '0x43',
     'part_number': '0xb2',
-    'numa': false,
-    'flags': [
-        ['RTE_IOVA_IN_MBUF', 0]
-    ]
+    'numa': false
 }
 
 soc_cn10k = {
@@ -451,8 +448,7 @@ soc_cn10k = {
     'flags': [
         ['RTE_MAX_LCORE', 24],
         ['RTE_MAX_NUMA_NODES', 1],
-        ['RTE_MEMPOOL_ALIGN', 128],
-        ['RTE_IOVA_IN_MBUF', 0]
+        ['RTE_MEMPOOL_ALIGN', 128]
     ],
     'part_number': '0xd49',
     'extra_march_features': ['crypto'],
diff --git a/drivers/common/cnxk/meson.build b/drivers/common/cnxk/meson.build
index dc2ddf1f20..bba780e750 100644
--- a/drivers/common/cnxk/meson.build
+++ b/drivers/common/cnxk/meson.build
@@ -108,4 +108,13 @@ deps += ['bus_pci', 'net', 'telemetry']
 
 require_iova_in_mbuf = false
 
+cnxk_socs = ['cn9k', 'cn10k', 'cn20k']
+
+# Enable RTE_IOVA_IN_MBUF only if enable_iova_as_pa is set explicitly, else disable it
+if meson.version().version_compare('>=1.1.0')
+    if '-Denable_iova_as_pa' not in meson.build_options() and soc_type in cnxk_socs
+        dpdk_conf.set10('RTE_IOVA_IN_MBUF', false)
+    endif
+endif
+
 annotate_locks = false
-- 
2.25.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] common/cnxk: allow enabling IOVA field in mbuf
  2024-10-14 11:06 [PATCH] common/cnxk: allow enabling IOVA field in mbuf Shijith Thotton
@ 2024-10-24 11:10 ` Jerin Jacob
  2024-10-24 12:09   ` Bruce Richardson
  2024-10-25  9:50 ` [PATCH v2] " Shijith Thotton
  1 sibling, 1 reply; 11+ messages in thread
From: Jerin Jacob @ 2024-10-24 11:10 UTC (permalink / raw)
  To: Shijith Thotton
  Cc: jerinj, pbhagavatula, dev, Wathsala Vithanage, Bruce Richardson,
	Nithin Dabilpuram, Kiran Kumar K, Sunil Kumar Kori, Satha Rao,
	Harman Kalra, Thomas Monjalon, David Marchand

On Mon, Oct 14, 2024 at 4:37 PM Shijith Thotton <sthotton@marvell.com> wrote:
>
> Value of RTE_IOVA_IN_MBUF was always disabled on cnxk platforms, as IOVA
> in the mbuf is not required. This change modifies that behavior,
> allowing RTE_IOVA_IN_MBUF to be enabled if the build option
> -Denable_iova_as_pa=true is explicitly specified.
>
> Signed-off-by: Shijith Thotton <sthotton@marvell.com>
> ---
>
> diff --git a/config/arm/meson.build b/config/arm/meson.build
> index 012935d5d7..ca54524376 100644
> --- a/config/arm/meson.build
> +++ b/config/arm/meson.build
> @@ -439,10 +439,7 @@ soc_cn9k = {
>      'description': 'Marvell OCTEON 9',
>      'implementer': '0x43',
>      'part_number': '0xb2',
> -    'numa': false,
> -    'flags': [
> -        ['RTE_IOVA_IN_MBUF', 0]
> -    ]
> +    'numa': false
>  }
>
>  soc_cn10k = {
> @@ -451,8 +448,7 @@ soc_cn10k = {
>      'flags': [
>          ['RTE_MAX_LCORE', 24],
>          ['RTE_MAX_NUMA_NODES', 1],
> -        ['RTE_MEMPOOL_ALIGN', 128],
> -        ['RTE_IOVA_IN_MBUF', 0]
> +        ['RTE_MEMPOOL_ALIGN', 128]
>      ],
>      'part_number': '0xd49',
>      'extra_march_features': ['crypto'],
> diff --git a/drivers/common/cnxk/meson.build b/drivers/common/cnxk/meson.build
> index dc2ddf1f20..bba780e750 100644
> --- a/drivers/common/cnxk/meson.build
> +++ b/drivers/common/cnxk/meson.build
> @@ -108,4 +108,13 @@ deps += ['bus_pci', 'net', 'telemetry']
>
>  require_iova_in_mbuf = false
>
> +cnxk_socs = ['cn9k', 'cn10k', 'cn20k']
> +
> +# Enable RTE_IOVA_IN_MBUF only if enable_iova_as_pa is set explicitly, else disable it
> +if meson.version().version_compare('>=1.1.0')
> +    if '-Denable_iova_as_pa' not in meson.build_options() and soc_type in cnxk_socs
> +        dpdk_conf.set10('RTE_IOVA_IN_MBUF', false)
> +    endif
> +endif

Since this is added in driver/common/cnxk, it will be late to decide.
For example,

Following PMDs will have mis match:
common - cpt, dpaax, idpf, ionic
bus - cdx, dpaa, fslmc, ifpga, uacce

I think, this check needs to move up in the chain. @Richardson, Bruce
Any thoughts on cleanly adding this kind of check in top-level meson
objects?


> +
>  annotate_locks = false
> --
> 2.25.1
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] common/cnxk: allow enabling IOVA field in mbuf
  2024-10-24 11:10 ` Jerin Jacob
@ 2024-10-24 12:09   ` Bruce Richardson
  2024-10-24 16:13     ` [EXTERNAL] " Shijith Thotton
  0 siblings, 1 reply; 11+ messages in thread
From: Bruce Richardson @ 2024-10-24 12:09 UTC (permalink / raw)
  To: Jerin Jacob
  Cc: Shijith Thotton, jerinj, pbhagavatula, dev, Wathsala Vithanage,
	Nithin Dabilpuram, Kiran Kumar K, Sunil Kumar Kori, Satha Rao,
	Harman Kalra, Thomas Monjalon, David Marchand

On Thu, Oct 24, 2024 at 04:40:40PM +0530, Jerin Jacob wrote:
> On Mon, Oct 14, 2024 at 4:37 PM Shijith Thotton <sthotton@marvell.com> wrote:
> >
> > Value of RTE_IOVA_IN_MBUF was always disabled on cnxk platforms, as IOVA
> > in the mbuf is not required. This change modifies that behavior,
> > allowing RTE_IOVA_IN_MBUF to be enabled if the build option
> > -Denable_iova_as_pa=true is explicitly specified.
> >
> > Signed-off-by: Shijith Thotton <sthotton@marvell.com>
> > ---
> >
> > diff --git a/config/arm/meson.build b/config/arm/meson.build
> > index 012935d5d7..ca54524376 100644
> > --- a/config/arm/meson.build
> > +++ b/config/arm/meson.build
> > @@ -439,10 +439,7 @@ soc_cn9k = {
> >      'description': 'Marvell OCTEON 9',
> >      'implementer': '0x43',
> >      'part_number': '0xb2',
> > -    'numa': false,
> > -    'flags': [
> > -        ['RTE_IOVA_IN_MBUF', 0]
> > -    ]
> > +    'numa': false
> >  }
> >
> >  soc_cn10k = {
> > @@ -451,8 +448,7 @@ soc_cn10k = {
> >      'flags': [
> >          ['RTE_MAX_LCORE', 24],
> >          ['RTE_MAX_NUMA_NODES', 1],
> > -        ['RTE_MEMPOOL_ALIGN', 128],
> > -        ['RTE_IOVA_IN_MBUF', 0]
> > +        ['RTE_MEMPOOL_ALIGN', 128]
> >      ],
> >      'part_number': '0xd49',
> >      'extra_march_features': ['crypto'],
> > diff --git a/drivers/common/cnxk/meson.build b/drivers/common/cnxk/meson.build
> > index dc2ddf1f20..bba780e750 100644
> > --- a/drivers/common/cnxk/meson.build
> > +++ b/drivers/common/cnxk/meson.build
> > @@ -108,4 +108,13 @@ deps += ['bus_pci', 'net', 'telemetry']
> >
> >  require_iova_in_mbuf = false
> >
> > +cnxk_socs = ['cn9k', 'cn10k', 'cn20k']
> > +
> > +# Enable RTE_IOVA_IN_MBUF only if enable_iova_as_pa is set explicitly, else disable it
> > +if meson.version().version_compare('>=1.1.0')
> > +    if '-Denable_iova_as_pa' not in meson.build_options() and soc_type in cnxk_socs
> > +        dpdk_conf.set10('RTE_IOVA_IN_MBUF', false)
> > +    endif
> > +endif
> 
> Since this is added in driver/common/cnxk, it will be late to decide.
> For example,
> 
> Following PMDs will have mis match:
> common - cpt, dpaax, idpf, ionic
> bus - cdx, dpaa, fslmc, ifpga, uacce
> 
> I think, this check needs to move up in the chain. @Richardson, Bruce
> Any thoughts on cleanly adding this kind of check in top-level meson
> objects?
> 
Can you explain what you mean by a mismatch?

Can I assume that the common/cnxk is processed before any of these other
drivers? If so, then whatever values or variables set by that meson.build
file can be queried by all the others.

/Bruce

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [EXTERNAL] Re: [PATCH] common/cnxk: allow enabling IOVA field in mbuf
  2024-10-24 12:09   ` Bruce Richardson
@ 2024-10-24 16:13     ` Shijith Thotton
  2024-10-24 16:44       ` Bruce Richardson
  0 siblings, 1 reply; 11+ messages in thread
From: Shijith Thotton @ 2024-10-24 16:13 UTC (permalink / raw)
  To: Bruce Richardson, Jerin Jacob
  Cc: Jerin Jacob, Pavan Nikhilesh Bhagavatula, dev,
	Wathsala Vithanage, Nithin Kumar Dabilpuram,
	Kiran Kumar Kokkilagadda, Sunil Kumar Kori,
	Satha Koteswara Rao Kottidi, Harman Kalra, Thomas Monjalon,
	David Marchand

>On Thu, Oct 24, 2024 at 04:40:40PM +0530, Jerin Jacob wrote:
>> On Mon, Oct 14, 2024 at 4:37 PM Shijith Thotton <sthotton@marvell.com>
>wrote:
>> >
>> > Value of RTE_IOVA_IN_MBUF was always disabled on cnxk platforms, as
>IOVA
>> > in the mbuf is not required. This change modifies that behavior,
>> > allowing RTE_IOVA_IN_MBUF to be enabled if the build option
>> > -Denable_iova_as_pa=true is explicitly specified.
>> >
>> > Signed-off-by: Shijith Thotton <sthotton@marvell.com>
>> > ---
>> >
>> > diff --git a/config/arm/meson.build b/config/arm/meson.build
>> > index 012935d5d7..ca54524376 100644
>> > --- a/config/arm/meson.build
>> > +++ b/config/arm/meson.build
>> > @@ -439,10 +439,7 @@ soc_cn9k = {
>> >      'description': 'Marvell OCTEON 9',
>> >      'implementer': '0x43',
>> >      'part_number': '0xb2',
>> > -    'numa': false,
>> > -    'flags': [
>> > -        ['RTE_IOVA_IN_MBUF', 0]
>> > -    ]
>> > +    'numa': false
>> >  }
>> >
>> >  soc_cn10k = {
>> > @@ -451,8 +448,7 @@ soc_cn10k = {
>> >      'flags': [
>> >          ['RTE_MAX_LCORE', 24],
>> >          ['RTE_MAX_NUMA_NODES', 1],
>> > -        ['RTE_MEMPOOL_ALIGN', 128],
>> > -        ['RTE_IOVA_IN_MBUF', 0]
>> > +        ['RTE_MEMPOOL_ALIGN', 128]
>> >      ],
>> >      'part_number': '0xd49',
>> >      'extra_march_features': ['crypto'],
>> > diff --git a/drivers/common/cnxk/meson.build
>b/drivers/common/cnxk/meson.build
>> > index dc2ddf1f20..bba780e750 100644
>> > --- a/drivers/common/cnxk/meson.build
>> > +++ b/drivers/common/cnxk/meson.build
>> > @@ -108,4 +108,13 @@ deps += ['bus_pci', 'net', 'telemetry']
>> >
>> >  require_iova_in_mbuf = false
>> >
>> > +cnxk_socs = ['cn9k', 'cn10k', 'cn20k']
>> > +
>> > +# Enable RTE_IOVA_IN_MBUF only if enable_iova_as_pa is set explicitly,
>else disable it
>> > +if meson.version().version_compare('>=1.1.0')
>> > +    if '-Denable_iova_as_pa' not in meson.build_options() and soc_type in
>cnxk_socs
>> > +        dpdk_conf.set10('RTE_IOVA_IN_MBUF', false)
>> > +    endif
>> > +endif
>>
>> Since this is added in driver/common/cnxk, it will be late to decide.
>> For example,
>>
>> Following PMDs will have mis match:
>> common - cpt, dpaax, idpf, ionic
>> bus - cdx, dpaa, fslmc, ifpga, uacce
>>
>> I think, this check needs to move up in the chain. @Richardson, Bruce
>> Any thoughts on cleanly adding this kind of check in top-level meson
>> objects?
>>
>Can you explain what you mean by a mismatch?
>
>Can I assume that the common/cnxk is processed before any of these other
>drivers? If so, then whatever values or variables set by that meson.build
>file can be queried by all the others.
>

The goal is to change the default value of enable_iova_as_pa to false on
CNXK platforms.

Jerin pointed out the following issue with the current patch, which causes a
mismatch during the default build (enable_iova_as_pa not specified):

1. The configuration initially sets RTE_IOVA_IN_MBUF to 1.
2. Meson proceeds through the lib and driver directories, enabling the
    build of drivers that require IOVA.
3. When Meson reaches the common/cnxk folder, it sets RTE_IOVA_IN_MBUF
     to 0.
4. The remaining configuration then disables the build of drivers that depend
     on IOVA.

The problem arises due to the extra PMDs being enabled during step 2.

We could avoid this issue by moving the check added in this patch to a
top-level Meson file, such as config/arm/meson.build.

We would like your feedback on this approach or suggestions for a better
alternative.

Thanks,
Shijith

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [EXTERNAL] Re: [PATCH] common/cnxk: allow enabling IOVA field in mbuf
  2024-10-24 16:13     ` [EXTERNAL] " Shijith Thotton
@ 2024-10-24 16:44       ` Bruce Richardson
  2024-10-25  9:41         ` Shijith Thotton
  0 siblings, 1 reply; 11+ messages in thread
From: Bruce Richardson @ 2024-10-24 16:44 UTC (permalink / raw)
  To: Shijith Thotton
  Cc: Jerin Jacob, Jerin Jacob, Pavan Nikhilesh Bhagavatula, dev,
	Wathsala Vithanage, Nithin Kumar Dabilpuram,
	Kiran Kumar Kokkilagadda, Sunil Kumar Kori,
	Satha Koteswara Rao Kottidi, Harman Kalra, Thomas Monjalon,
	David Marchand

On Thu, Oct 24, 2024 at 04:13:19PM +0000, Shijith Thotton wrote:
> >On Thu, Oct 24, 2024 at 04:40:40PM +0530, Jerin Jacob wrote:
> >> On Mon, Oct 14, 2024 at 4:37 PM Shijith Thotton <sthotton@marvell.com>
> >wrote:
> >> >
> >> > Value of RTE_IOVA_IN_MBUF was always disabled on cnxk platforms, as
> >IOVA
> >> > in the mbuf is not required. This change modifies that behavior,
> >> > allowing RTE_IOVA_IN_MBUF to be enabled if the build option
> >> > -Denable_iova_as_pa=true is explicitly specified.
> >> >
> >> > Signed-off-by: Shijith Thotton <sthotton@marvell.com>
> >> > ---
> >> >
> >> > diff --git a/config/arm/meson.build b/config/arm/meson.build
> >> > index 012935d5d7..ca54524376 100644
> >> > --- a/config/arm/meson.build
> >> > +++ b/config/arm/meson.build
> >> > @@ -439,10 +439,7 @@ soc_cn9k = {
> >> >      'description': 'Marvell OCTEON 9',
> >> >      'implementer': '0x43',
> >> >      'part_number': '0xb2',
> >> > -    'numa': false,
> >> > -    'flags': [
> >> > -        ['RTE_IOVA_IN_MBUF', 0]
> >> > -    ]
> >> > +    'numa': false
> >> >  }
> >> >
> >> >  soc_cn10k = {
> >> > @@ -451,8 +448,7 @@ soc_cn10k = {
> >> >      'flags': [
> >> >          ['RTE_MAX_LCORE', 24],
> >> >          ['RTE_MAX_NUMA_NODES', 1],
> >> > -        ['RTE_MEMPOOL_ALIGN', 128],
> >> > -        ['RTE_IOVA_IN_MBUF', 0]
> >> > +        ['RTE_MEMPOOL_ALIGN', 128]
> >> >      ],
> >> >      'part_number': '0xd49',
> >> >      'extra_march_features': ['crypto'],
> >> > diff --git a/drivers/common/cnxk/meson.build
> >b/drivers/common/cnxk/meson.build
> >> > index dc2ddf1f20..bba780e750 100644
> >> > --- a/drivers/common/cnxk/meson.build
> >> > +++ b/drivers/common/cnxk/meson.build
> >> > @@ -108,4 +108,13 @@ deps += ['bus_pci', 'net', 'telemetry']
> >> >
> >> >  require_iova_in_mbuf = false
> >> >
> >> > +cnxk_socs = ['cn9k', 'cn10k', 'cn20k']
> >> > +
> >> > +# Enable RTE_IOVA_IN_MBUF only if enable_iova_as_pa is set explicitly,
> >else disable it
> >> > +if meson.version().version_compare('>=1.1.0')
> >> > +    if '-Denable_iova_as_pa' not in meson.build_options() and soc_type in
> >cnxk_socs
> >> > +        dpdk_conf.set10('RTE_IOVA_IN_MBUF', false)
> >> > +    endif
> >> > +endif
> >>
> >> Since this is added in driver/common/cnxk, it will be late to decide.
> >> For example,
> >>
> >> Following PMDs will have mis match:
> >> common - cpt, dpaax, idpf, ionic
> >> bus - cdx, dpaa, fslmc, ifpga, uacce
> >>
> >> I think, this check needs to move up in the chain. @Richardson, Bruce
> >> Any thoughts on cleanly adding this kind of check in top-level meson
> >> objects?
> >>
> >Can you explain what you mean by a mismatch?
> >
> >Can I assume that the common/cnxk is processed before any of these other
> >drivers? If so, then whatever values or variables set by that meson.build
> >file can be queried by all the others.
> >
> 
> The goal is to change the default value of enable_iova_as_pa to false on
> CNXK platforms.
> 
> Jerin pointed out the following issue with the current patch, which causes a
> mismatch during the default build (enable_iova_as_pa not specified):
> 
> 1. The configuration initially sets RTE_IOVA_IN_MBUF to 1.
> 2. Meson proceeds through the lib and driver directories, enabling the
>     build of drivers that require IOVA.
> 3. When Meson reaches the common/cnxk folder, it sets RTE_IOVA_IN_MBUF
>      to 0.
> 4. The remaining configuration then disables the build of drivers that depend
>      on IOVA.
> 
> The problem arises due to the extra PMDs being enabled during step 2.
> 
> We could avoid this issue by moving the check added in this patch to a
> top-level Meson file, such as config/arm/meson.build.
> 
> We would like your feedback on this approach or suggestions for a better
> alternative.
> 
Two alternatives I see:

1. One, you document that for CNXK platforms that the value must be set to
false in the build configuration. Then when processing common/cnxk folder,
you error out the configuration if the value is set to true, giving the
user a proper error message.
2. You see about changing the value as you suggest in a config meson.build
file.

Of the 2, option 1 would be by far the most preferred option. If an option
is set by the user to X, the build system should not override it to Y. This
is why meson does not provide a set_option() function, only a get_option()
one.

/Bruce

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] common/cnxk: allow enabling IOVA field in mbuf
  2024-10-24 16:44       ` Bruce Richardson
@ 2024-10-25  9:41         ` Shijith Thotton
  0 siblings, 0 replies; 11+ messages in thread
From: Shijith Thotton @ 2024-10-25  9:41 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: Jerin Jacob, Jerin Jacob, Pavan Nikhilesh Bhagavatula, dev,
	Wathsala Vithanage, Nithin Kumar Dabilpuram,
	Kiran Kumar Kokkilagadda, Sunil Kumar Kori,
	Satha Koteswara Rao Kottidi, Harman Kalra, Thomas Monjalon,
	David Marchand

>> >> >
>> >> > Value of RTE_IOVA_IN_MBUF was always disabled on cnxk platforms, as
>> >IOVA
>> >> > in the mbuf is not required. This change modifies that behavior,
>> >> > allowing RTE_IOVA_IN_MBUF to be enabled if the build option
>> >> > -Denable_iova_as_pa=true is explicitly specified.
>> >> >
>> >> > Signed-off-by: Shijith Thotton <sthotton@marvell.com>
>> >> > ---
>> >> >
>> >> > diff --git a/config/arm/meson.build b/config/arm/meson.build
>> >> > index 012935d5d7..ca54524376 100644
>> >> > --- a/config/arm/meson.build
>> >> > +++ b/config/arm/meson.build
>> >> > @@ -439,10 +439,7 @@ soc_cn9k = {
>> >> >      'description': 'Marvell OCTEON 9',
>> >> >      'implementer': '0x43',
>> >> >      'part_number': '0xb2',
>> >> > -    'numa': false,
>> >> > -    'flags': [
>> >> > -        ['RTE_IOVA_IN_MBUF', 0]
>> >> > -    ]
>> >> > +    'numa': false
>> >> >  }
>> >> >
>> >> >  soc_cn10k = {
>> >> > @@ -451,8 +448,7 @@ soc_cn10k = {
>> >> >      'flags': [
>> >> >          ['RTE_MAX_LCORE', 24],
>> >> >          ['RTE_MAX_NUMA_NODES', 1],
>> >> > -        ['RTE_MEMPOOL_ALIGN', 128],
>> >> > -        ['RTE_IOVA_IN_MBUF', 0]
>> >> > +        ['RTE_MEMPOOL_ALIGN', 128]
>> >> >      ],
>> >> >      'part_number': '0xd49',
>> >> >      'extra_march_features': ['crypto'],
>> >> > diff --git a/drivers/common/cnxk/meson.build
>> >b/drivers/common/cnxk/meson.build
>> >> > index dc2ddf1f20..bba780e750 100644
>> >> > --- a/drivers/common/cnxk/meson.build
>> >> > +++ b/drivers/common/cnxk/meson.build
>> >> > @@ -108,4 +108,13 @@ deps += ['bus_pci', 'net', 'telemetry']
>> >> >
>> >> >  require_iova_in_mbuf = false
>> >> >
>> >> > +cnxk_socs = ['cn9k', 'cn10k', 'cn20k']
>> >> > +
>> >> > +# Enable RTE_IOVA_IN_MBUF only if enable_iova_as_pa is set explicitly,
>> >else disable it
>> >> > +if meson.version().version_compare('>=1.1.0')
>> >> > +    if '-Denable_iova_as_pa' not in meson.build_options() and soc_type
>in
>> >cnxk_socs
>> >> > +        dpdk_conf.set10('RTE_IOVA_IN_MBUF', false)
>> >> > +    endif
>> >> > +endif
>> >>
>> >> Since this is added in driver/common/cnxk, it will be late to decide.
>> >> For example,
>> >>
>> >> Following PMDs will have mis match:
>> >> common - cpt, dpaax, idpf, ionic
>> >> bus - cdx, dpaa, fslmc, ifpga, uacce
>> >>
>> >> I think, this check needs to move up in the chain. @Richardson, Bruce
>> >> Any thoughts on cleanly adding this kind of check in top-level meson
>> >> objects?
>> >>
>> >Can you explain what you mean by a mismatch?
>> >
>> >Can I assume that the common/cnxk is processed before any of these other
>> >drivers? If so, then whatever values or variables set by that meson.build
>> >file can be queried by all the others.
>> >
>>
>> The goal is to change the default value of enable_iova_as_pa to false on
>> CNXK platforms.
>>
>> Jerin pointed out the following issue with the current patch, which causes a
>> mismatch during the default build (enable_iova_as_pa not specified):
>>
>> 1. The configuration initially sets RTE_IOVA_IN_MBUF to 1.
>> 2. Meson proceeds through the lib and driver directories, enabling the
>>     build of drivers that require IOVA.
>> 3. When Meson reaches the common/cnxk folder, it sets RTE_IOVA_IN_MBUF
>>      to 0.
>> 4. The remaining configuration then disables the build of drivers that depend
>>      on IOVA.
>>
>> The problem arises due to the extra PMDs being enabled during step 2.
>>
>> We could avoid this issue by moving the check added in this patch to a
>> top-level Meson file, such as config/arm/meson.build.
>>
>> We would like your feedback on this approach or suggestions for a better
>> alternative.
>>
>Two alternatives I see:
>
>1. One, you document that for CNXK platforms that the value must be set to
>false in the build configuration. Then when processing common/cnxk folder,
>you error out the configuration if the value is set to true, giving the
>user a proper error message.
>2. You see about changing the value as you suggest in a config meson.build
>file.
>
>Of the 2, option 1 would be by far the most preferred option. If an option
>is set by the user to X, the build system should not override it to Y. This
>is why meson does not provide a set_option() function, only a get_option()
>one.
>

Understood. We’ll proceed with option 1. Instead of throwing an error, we’ll
display a warning when processing the common/cnxk folder if IOVA is enabled.
This way, the user can choose whether to disable IOVA in the mbuf.

Thanks,
Shijith

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2] common/cnxk: allow enabling IOVA field in mbuf
  2024-10-14 11:06 [PATCH] common/cnxk: allow enabling IOVA field in mbuf Shijith Thotton
  2024-10-24 11:10 ` Jerin Jacob
@ 2024-10-25  9:50 ` Shijith Thotton
  2024-10-25 10:06   ` Bruce Richardson
  2024-10-25 11:51   ` [PATCH v3] " Shijith Thotton
  1 sibling, 2 replies; 11+ messages in thread
From: Shijith Thotton @ 2024-10-25  9:50 UTC (permalink / raw)
  To: jerinj, bruce.richardson, pbhagavatula
  Cc: Shijith Thotton, dev, Wathsala Vithanage, Nithin Dabilpuram,
	Kiran Kumar K, Sunil Kumar Kori, Satha Rao, Harman Kalra

The value of RTE_IOVA_IN_MBUF has always been disabled on CNXK
platforms, as IOVA in the mbuf is unnecessary. This update changes that
behavior to respect the value set by the user. A warning message will be
printed if the build is configured to enable IOVA on the CNXK platform.

Signed-off-by: Shijith Thotton <sthotton@marvell.com>
---
v2:
- Don't override the default value of RTE_IOVA_IN_MBUF in CNXK platform.
- Print a warning message if IOVA in mbuf is enabled on CNXK platform.

 config/arm/meson.build          | 8 ++------
 drivers/common/cnxk/meson.build | 6 ++++++
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/config/arm/meson.build b/config/arm/meson.build
index 55be7c8711..20f7f6508c 100644
--- a/config/arm/meson.build
+++ b/config/arm/meson.build
@@ -459,10 +459,7 @@ soc_cn9k = {
     'description': 'Marvell OCTEON 9',
     'implementer': '0x43',
     'part_number': '0xb2',
-    'numa': false,
-    'flags': [
-        ['RTE_IOVA_IN_MBUF', 0]
-    ]
+    'numa': false
 }
 
 soc_cn10k = {
@@ -471,8 +468,7 @@ soc_cn10k = {
     'flags': [
         ['RTE_MAX_LCORE', 24],
         ['RTE_MAX_NUMA_NODES', 1],
-        ['RTE_MEMPOOL_ALIGN', 128],
-        ['RTE_IOVA_IN_MBUF', 0]
+        ['RTE_MEMPOOL_ALIGN', 128]
     ],
     'part_number': '0xd49',
     'extra_march_features': ['crypto'],
diff --git a/drivers/common/cnxk/meson.build b/drivers/common/cnxk/meson.build
index abb0f6f01f..293a47524b 100644
--- a/drivers/common/cnxk/meson.build
+++ b/drivers/common/cnxk/meson.build
@@ -108,4 +108,10 @@ deps += ['bus_pci', 'net', 'telemetry']
 
 require_iova_in_mbuf = false
 
+cnxk_socs = ['cn9k', 'cn10k', 'cn20k']
+
+if dpdk_conf.get('RTE_IOVA_IN_MBUF') == 1 and soc_type in cnxk_socs
+    warning('IOVA in mbuf is not required on platform ' + soc_type)
+endif
+
 annotate_locks = false
-- 
2.25.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] common/cnxk: allow enabling IOVA field in mbuf
  2024-10-25  9:50 ` [PATCH v2] " Shijith Thotton
@ 2024-10-25 10:06   ` Bruce Richardson
  2024-10-25 10:12     ` Shijith Thotton
  2024-10-25 11:51   ` [PATCH v3] " Shijith Thotton
  1 sibling, 1 reply; 11+ messages in thread
From: Bruce Richardson @ 2024-10-25 10:06 UTC (permalink / raw)
  To: Shijith Thotton
  Cc: jerinj, pbhagavatula, dev, Wathsala Vithanage, Nithin Dabilpuram,
	Kiran Kumar K, Sunil Kumar Kori, Satha Rao, Harman Kalra

On Fri, Oct 25, 2024 at 03:20:40PM +0530, Shijith Thotton wrote:
> The value of RTE_IOVA_IN_MBUF has always been disabled on CNXK platforms,
> as IOVA in the mbuf is unnecessary. This update changes that behavior to
> respect the value set by the user. A warning message will be printed if
> the build is configured to enable IOVA on the CNXK platform.
> 
> Signed-off-by: Shijith Thotton <sthotton@marvell.com> --- v2: - Don't
> override the default value of RTE_IOVA_IN_MBUF in CNXK platform.  - Print
> a warning message if IOVA in mbuf is enabled on CNXK platform.
> 
>  config/arm/meson.build          | 8 ++------
>  drivers/common/cnxk/meson.build | 6 ++++++ 2 files changed, 8
>  insertions(+), 6 deletions(-)
> 
I think you probably need some doc updates for this. The current doc (cnxk
platform guide) says "Meson build option enable_iova_as_pa is disabled on
CNXK platforms." This is no longer true. Instead, you probably need to put
- in more than one place in the docs, I suggest - a note telling the user
that the option should be disabled.

/Bruce

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] common/cnxk: allow enabling IOVA field in mbuf
  2024-10-25 10:06   ` Bruce Richardson
@ 2024-10-25 10:12     ` Shijith Thotton
  0 siblings, 0 replies; 11+ messages in thread
From: Shijith Thotton @ 2024-10-25 10:12 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: Jerin Jacob, Pavan Nikhilesh Bhagavatula, dev,
	Wathsala Vithanage, Nithin Kumar Dabilpuram,
	Kiran Kumar Kokkilagadda, Sunil Kumar Kori,
	Satha Koteswara Rao Kottidi, Harman Kalra

>On Fri, Oct 25, 2024 at 03: 20: 40PM +0530, Shijith Thotton wrote: > The value
>of RTE_IOVA_IN_MBUF has always been disabled on CNXK platforms, > as
>IOVA in the mbuf is unnecessary. This update changes that behavior to >
>respect the value
>
>On Fri, Oct 25, 2024 at 03:20:40PM +0530, Shijith Thotton wrote:
>> The value of RTE_IOVA_IN_MBUF has always been disabled on CNXK
>platforms,
>> as IOVA in the mbuf is unnecessary. This update changes that behavior to
>> respect the value set by the user. A warning message will be printed if
>> the build is configured to enable IOVA on the CNXK platform.
>>
>> Signed-off-by: Shijith Thotton <sthotton@marvell.com> --- v2: - Don't
>> override the default value of RTE_IOVA_IN_MBUF in CNXK platform.  - Print
>> a warning message if IOVA in mbuf is enabled on CNXK platform.
>>
>>  config/arm/meson.build          | 8 ++------
>>  drivers/common/cnxk/meson.build | 6 ++++++ 2 files changed, 8
>>  insertions(+), 6 deletions(-)
>>
>I think you probably need some doc updates for this. The current doc (cnxk
>platform guide) says "Meson build option enable_iova_as_pa is disabled on
>CNXK platforms." This is no longer true. Instead, you probably need to put
>- in more than one place in the docs, I suggest - a note telling the user
>that the option should be disabled.
>

Thanks, will update the documentation.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v3] common/cnxk: allow enabling IOVA field in mbuf
  2024-10-25  9:50 ` [PATCH v2] " Shijith Thotton
  2024-10-25 10:06   ` Bruce Richardson
@ 2024-10-25 11:51   ` Shijith Thotton
  2024-10-25 12:39     ` Bruce Richardson
  1 sibling, 1 reply; 11+ messages in thread
From: Shijith Thotton @ 2024-10-25 11:51 UTC (permalink / raw)
  To: jerinj, bruce.richardson, pbhagavatula
  Cc: Shijith Thotton, dev, Wathsala Vithanage, Nithin Dabilpuram,
	Kiran Kumar K, Sunil Kumar Kori, Satha Rao, Harman Kalra

The value of RTE_IOVA_IN_MBUF has always been disabled on CNXK
platforms, as IOVA in the mbuf is unnecessary. This update changes that
behavior to respect the value set by the user. A warning message will be
printed if the build is configured to enable IOVA on the CNXK platform.

Signed-off-by: Shijith Thotton <sthotton@marvell.com>
---
v3:
- Updated documentation.

v2:
- Don't override the default value of RTE_IOVA_IN_MBUF in CNXK platform.
- Print a warning message if IOVA in mbuf is enabled on CNXK platform.

 config/arm/meson.build          |  8 ++------
 doc/guides/platform/cnxk.rst    | 13 +++++++------
 drivers/common/cnxk/meson.build |  7 +++++++
 3 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/config/arm/meson.build b/config/arm/meson.build
index 55be7c8711..20f7f6508c 100644
--- a/config/arm/meson.build
+++ b/config/arm/meson.build
@@ -459,10 +459,7 @@ soc_cn9k = {
     'description': 'Marvell OCTEON 9',
     'implementer': '0x43',
     'part_number': '0xb2',
-    'numa': false,
-    'flags': [
-        ['RTE_IOVA_IN_MBUF', 0]
-    ]
+    'numa': false
 }
 
 soc_cn10k = {
@@ -471,8 +468,7 @@ soc_cn10k = {
     'flags': [
         ['RTE_MAX_LCORE', 24],
         ['RTE_MAX_NUMA_NODES', 1],
-        ['RTE_MEMPOOL_ALIGN', 128],
-        ['RTE_IOVA_IN_MBUF', 0]
+        ['RTE_MEMPOOL_ALIGN', 128]
     ],
     'part_number': '0xd49',
     'extra_march_features': ['crypto'],
diff --git a/doc/guides/platform/cnxk.rst b/doc/guides/platform/cnxk.rst
index 0e61bc91d9..cd7a0adb96 100644
--- a/doc/guides/platform/cnxk.rst
+++ b/doc/guides/platform/cnxk.rst
@@ -587,8 +587,9 @@ Compile DPDK
 
 DPDK may be compiled either natively on OCTEON CN9K/CN10K platform or cross-compiled on
 an x86 based platform.
-Meson build option ``enable_iova_as_pa`` is disabled on CNXK platforms.
-So only PMDs supporting this option are enabled on CNXK platform builds.
+The Meson build option ``enable_iova_as_pa`` should be set to false because, on
+CNXK platforms, IOVA is same as the virtual address. Disabling the iova field
+in the mbuf frees it up to be used as a dynamic field.
 
 Native Compilation
 ~~~~~~~~~~~~~~~~~~
@@ -599,14 +600,14 @@ CN9K:
 
 .. code-block:: console
 
-        meson setup -Dplatform=cn9k build
+        meson setup -Dplatform=cn9k -Denable_iova_as_pa=false build
         ninja -C build
 
 CN10K:
 
 .. code-block:: console
 
-        meson setup -Dplatform=cn10k build
+        meson setup -Dplatform=cn10k -Denable_iova_as_pa=false build
         ninja -C build
 
 Cross Compilation
@@ -618,14 +619,14 @@ CN9K:
 
 .. code-block:: console
 
-        meson setup build --cross-file config/arm/arm64_cn9k_linux_gcc
+        meson setup -Denable_iova_as_pa=false build --cross-file config/arm/arm64_cn9k_linux_gcc
         ninja -C build
 
 CN10K:
 
 .. code-block:: console
 
-        meson setup build --cross-file config/arm/arm64_cn10k_linux_gcc
+        meson setup -Denable_iova_as_pa=false build --cross-file config/arm/arm64_cn10k_linux_gcc
         ninja -C build
 
 .. note::
diff --git a/drivers/common/cnxk/meson.build b/drivers/common/cnxk/meson.build
index abb0f6f01f..9f07a694a5 100644
--- a/drivers/common/cnxk/meson.build
+++ b/drivers/common/cnxk/meson.build
@@ -108,4 +108,11 @@ deps += ['bus_pci', 'net', 'telemetry']
 
 require_iova_in_mbuf = false
 
+cnxk_socs = ['cn9k', 'cn10k', 'cn20k']
+
+if dpdk_conf.get('RTE_IOVA_IN_MBUF') == 1 and soc_type in cnxk_socs
+    warning('IOVA in mbuf is not required on cnxk platforms. ' +
+            'Set the enable_iova_as_pa option to false to save mbuf space.')
+endif
+
 annotate_locks = false
-- 
2.25.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3] common/cnxk: allow enabling IOVA field in mbuf
  2024-10-25 11:51   ` [PATCH v3] " Shijith Thotton
@ 2024-10-25 12:39     ` Bruce Richardson
  0 siblings, 0 replies; 11+ messages in thread
From: Bruce Richardson @ 2024-10-25 12:39 UTC (permalink / raw)
  To: Shijith Thotton
  Cc: jerinj, pbhagavatula, dev, Wathsala Vithanage, Nithin Dabilpuram,
	Kiran Kumar K, Sunil Kumar Kori, Satha Rao, Harman Kalra

On Fri, Oct 25, 2024 at 05:21:22PM +0530, Shijith Thotton wrote:
> The value of RTE_IOVA_IN_MBUF has always been disabled on CNXK
> platforms, as IOVA in the mbuf is unnecessary. This update changes that
> behavior to respect the value set by the user. A warning message will be
> printed if the build is configured to enable IOVA on the CNXK platform.
> 
> Signed-off-by: Shijith Thotton <sthotton@marvell.com>
> ---
> v3:
> - Updated documentation.
> 
> v2:
> - Don't override the default value of RTE_IOVA_IN_MBUF in CNXK platform.
> - Print a warning message if IOVA in mbuf is enabled on CNXK platform.
> 
>  config/arm/meson.build          |  8 ++------
>  doc/guides/platform/cnxk.rst    | 13 +++++++------
>  drivers/common/cnxk/meson.build |  7 +++++++
>  3 files changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/config/arm/meson.build b/config/arm/meson.build
> index 55be7c8711..20f7f6508c 100644
> --- a/config/arm/meson.build
> +++ b/config/arm/meson.build
> @@ -459,10 +459,7 @@ soc_cn9k = {
>      'description': 'Marvell OCTEON 9',
>      'implementer': '0x43',
>      'part_number': '0xb2',
> -    'numa': false,
> -    'flags': [
> -        ['RTE_IOVA_IN_MBUF', 0]
> -    ]
> +    'numa': false
>  }
>  
>  soc_cn10k = {
> @@ -471,8 +468,7 @@ soc_cn10k = {
>      'flags': [
>          ['RTE_MAX_LCORE', 24],
>          ['RTE_MAX_NUMA_NODES', 1],
> -        ['RTE_MEMPOOL_ALIGN', 128],
> -        ['RTE_IOVA_IN_MBUF', 0]
> +        ['RTE_MEMPOOL_ALIGN', 128]

FYI, meson is ok with trailing commas [1], so you don't need to modify this
line at all, just leave the comma at the end.

/Bruce

[1] https://mesonbuild.com/Style-guide.html#trailing-commas

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2024-10-25 12:39 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-10-14 11:06 [PATCH] common/cnxk: allow enabling IOVA field in mbuf Shijith Thotton
2024-10-24 11:10 ` Jerin Jacob
2024-10-24 12:09   ` Bruce Richardson
2024-10-24 16:13     ` [EXTERNAL] " Shijith Thotton
2024-10-24 16:44       ` Bruce Richardson
2024-10-25  9:41         ` Shijith Thotton
2024-10-25  9:50 ` [PATCH v2] " Shijith Thotton
2024-10-25 10:06   ` Bruce Richardson
2024-10-25 10:12     ` Shijith Thotton
2024-10-25 11:51   ` [PATCH v3] " Shijith Thotton
2024-10-25 12:39     ` Bruce Richardson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).