config: change default cache line size for ARMv8 with meson

Message ID 20190109093915.40882-1-yskoh@mellanox.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series config: change default cache line size for ARMv8 with meson |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/intel-Performance-Testing success Performance Testing PASS
ci/Intel-compilation success Compilation OK

Commit Message

Yongseok Koh Jan. 9, 2019, 9:39 a.m. UTC
  In config/arm64_armv8_linuxapp_gcc, maximum available cache line size
(128B) in arm64 implementations is set by default for generic config.
However, setting 64B is preferable for meson build in order to support
majority of CPUs which don't have Implementor ID or Part Number programmed
on chip.

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---

Discussion on the mailing list:
    https://mails.dpdk.org/archives/dev/2019-January/122441.html

 config/arm/meson.build | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)
  

Comments

Jerin Jacob Kollanukkaran Jan. 9, 2019, 10:09 a.m. UTC | #1
On Wed, 2019-01-09 at 01:39 -0800, Yongseok Koh wrote:
> 
> -------------------------------------------------------------------
> ---
> In config/arm64_armv8_linuxapp_gcc, maximum available cache line size
> (128B) in arm64 implementations is set by default for generic config.
> However, setting 64B is preferable for meson build in order to
> support
> majority of CPUs which don't have Implementor ID or Part Number
> programmed
> on chip.

Adding Luca to get input from distro build perspective.

If I understand it correctly, distro build will be using
the generic config/arm/arm64_armv8_linuxapp_gcc for generic build.
If so, We can not change cache line size for generic config
as mentined the reason are
https://mails.dpdk.org/archives/dev/2019-January/122441.html


I think, I way forward is to add config/arm/arm64_a72_linuxapp_gcc
for meson. This config can be used for all SoC with A72 armv8 
implementation and may have sym link to specfific SoC to avoid
confusion to end users.



> 
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> ---
> 
> Discussion on the mailing list:
>     https://mails.dpdk.org/archives/dev/2019-January/122441.html
> 
>  config/arm/meson.build | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/config/arm/meson.build b/config/arm/meson.build
> index dae55d6b26..3af256a5ec 100644
> --- a/config/arm/meson.build
> +++ b/config/arm/meson.build
> @@ -47,8 +47,7 @@ flags_common_default = [
>  flags_generic = [
>  	['RTE_MACHINE', '"armv8a"'],
>  	['RTE_MAX_LCORE', 256],
> -	['RTE_USE_C11_MEM_MODEL', true],
> -	['RTE_CACHE_LINE_SIZE', 128]]
> +	['RTE_USE_C11_MEM_MODEL', true]]
>  flags_cavium = [
>  	['RTE_MACHINE', '"thunderx"'],
>  	['RTE_CACHE_LINE_SIZE', 128],
> @@ -89,15 +88,19 @@ impl_dpaa2 = ['NXP DPAA2', flags_dpaa2,
> machine_args_generic]
>  
>  dpdk_conf.set('RTE_FORCE_INTRINSICS', 1)
>  
> +# In config/arm64_armv8_linuxapp_gcc, maximum available cache line
> size (128B)
> +# in arm64 implementations is set by default for generic config.
> However,
> +# setting 64B is preferable for meson build in order to support
> majority of CPUs
> +# which don't have Implementor ID or Part Number programmed on chip.
> +dpdk_conf.set('RTE_CACHE_LINE_SIZE', 64)
> +
>  if cc.sizeof('void *') != 8
> -	dpdk_conf.set('RTE_CACHE_LINE_SIZE', 64)
>  	dpdk_conf.set('RTE_ARCH_ARM', 1)
>  	dpdk_conf.set('RTE_ARCH_ARMv7', 1)
>  	# the minimum architecture supported, armv7-a, needs the
> following,
>  	# mk/machine/armv7a/rte.vars.mk sets it too
>  	machine_args += '-mfpu=neon'
>  else
> -	dpdk_conf.set('RTE_CACHE_LINE_SIZE', 128)
>  	dpdk_conf.set('RTE_ARCH_ARM64', 1)
>  	dpdk_conf.set('RTE_ARCH_64', 1)
>
  
Luca Boccassi Jan. 9, 2019, 10:19 a.m. UTC | #2
On Wed, 2019-01-09 at 10:09 +0000, Jerin Jacob Kollanukkaran wrote:
> On Wed, 2019-01-09 at 01:39 -0800, Yongseok Koh wrote:
> > 
> > -------------------------------------------------------------------
> > ---
> > In config/arm64_armv8_linuxapp_gcc, maximum available cache line
> > size
> > (128B) in arm64 implementations is set by default for generic
> > config.
> > However, setting 64B is preferable for meson build in order to
> > support
> > majority of CPUs which don't have Implementor ID or Part Number
> > programmed
> > on chip.
> 
> Adding Luca to get input from distro build perspective.
> 
> If I understand it correctly, distro build will be using
> the generic config/arm/arm64_armv8_linuxapp_gcc for generic build.
> If so, We can not change cache line size for generic config
> as mentined the reason are
> https://mails.dpdk.org/archives/dev/2019-January/122441.html
> 
> 
> I think, I way forward is to add config/arm/arm64_a72_linuxapp_gcc
> for meson. This config can be used for all SoC with A72 armv8 
> implementation and may have sym link to specfific SoC to avoid
> confusion to end users.

For Meson, I recently sent a patch to use a common and stable baseline
machine arg if the uses sets it to "default":

https://patches.dpdk.org/patch/49477/

For distros, I think if, within that path, the cache size is set to the
minimum common denominator like the -march is, it will be fine.

At that point, what happens for "native" or otherwise specified builds
it will not affect distros (as long as they use "default" like Debian
and Ubuntu do).

Does this answer your question?

> > 
> > Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> > ---
> > 
> > Discussion on the mailing list:
> >     https://mails.dpdk.org/archives/dev/2019-January/122441.html
> > 
> >  config/arm/meson.build | 11 +++++++----
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> > 
> > diff --git a/config/arm/meson.build b/config/arm/meson.build
> > index dae55d6b26..3af256a5ec 100644
> > --- a/config/arm/meson.build
> > +++ b/config/arm/meson.build
> > @@ -47,8 +47,7 @@ flags_common_default = [
> >  flags_generic = [
> >  	['RTE_MACHINE', '"armv8a"'],
> >  	['RTE_MAX_LCORE', 256],
> > -	['RTE_USE_C11_MEM_MODEL', true],
> > -	['RTE_CACHE_LINE_SIZE', 128]]
> > +	['RTE_USE_C11_MEM_MODEL', true]]
> >  flags_cavium = [
> >  	['RTE_MACHINE', '"thunderx"'],
> >  	['RTE_CACHE_LINE_SIZE', 128],
> > @@ -89,15 +88,19 @@ impl_dpaa2 = ['NXP DPAA2', flags_dpaa2,
> > machine_args_generic]
> >  
> >  dpdk_conf.set('RTE_FORCE_INTRINSICS', 1)
> >  
> > +# In config/arm64_armv8_linuxapp_gcc, maximum available cache line
> > size (128B)
> > +# in arm64 implementations is set by default for generic config.
> > However,
> > +# setting 64B is preferable for meson build in order to support
> > majority of CPUs
> > +# which don't have Implementor ID or Part Number programmed on
> > chip.
> > +dpdk_conf.set('RTE_CACHE_LINE_SIZE', 64)
> > +
> >  if cc.sizeof('void *') != 8
> > -	dpdk_conf.set('RTE_CACHE_LINE_SIZE', 64)
> >  	dpdk_conf.set('RTE_ARCH_ARM', 1)
> >  	dpdk_conf.set('RTE_ARCH_ARMv7', 1)
> >  	# the minimum architecture supported, armv7-a, needs the
> > following,
> >  	# mk/machine/armv7a/rte.vars.mk sets it too
> >  	machine_args += '-mfpu=neon'
> >  else
> > -	dpdk_conf.set('RTE_CACHE_LINE_SIZE', 128)
> >  	dpdk_conf.set('RTE_ARCH_ARM64', 1)
> >  	dpdk_conf.set('RTE_ARCH_64', 1)
> >
  
Yongseok Koh Jan. 9, 2019, 10:22 a.m. UTC | #3
> On Jan 9, 2019, at 2:09 AM, Jerin Jacob Kollanukkaran <jerinj@marvell.com> wrote:
> 
> On Wed, 2019-01-09 at 01:39 -0800, Yongseok Koh wrote:
>> 
>> -------------------------------------------------------------------
>> ---
>> In config/arm64_armv8_linuxapp_gcc, maximum available cache line size
>> (128B) in arm64 implementations is set by default for generic config.
>> However, setting 64B is preferable for meson build in order to
>> support
>> majority of CPUs which don't have Implementor ID or Part Number
>> programmed
>> on chip.
> 
> Adding Luca to get input from distro build perspective.
> 
> If I understand it correctly, distro build will be using
> the generic config/arm/arm64_armv8_linuxapp_gcc for generic build.
> If so, We can not change cache line size for generic config
> as mentined the reason are
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmails.dpdk.org%2Farchives%2Fdev%2F2019-January%2F122441.html&amp;data=02%7C01%7Cyskoh%40mellanox.com%7Cf0818214ecbf492d5c1408d6761a9136%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636826253828868015&amp;sdata=7pT%2BPTNslOpTCIqFjaZ223Hg3btpDB3dGC9xiy13wDA%3D&amp;reserved=0

My understanding from your comment was distro build doesn't use meson
but make with config/defconfig_arm64-armv8a-linuxapp-gcc.
Let's hear from Luca.

> I think, I way forward is to add config/arm/arm64_a72_linuxapp_gcc
> for meson. This config can be used for all SoC with A72 armv8 
> implementation and may have sym link to specfific SoC to avoid
> confusion to end users.

Is config/arm/arm64_a72_linuxapp_gcc valid? Others have arm64_[IMPLEMENTOR]_linuxapp_gcc.
a72 is got from PartNum.

Thanks,
Yongseok

>> 
>> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
>> ---
>> 
>> Discussion on the mailing list:
>>    https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmails.dpdk.org%2Farchives%2Fdev%2F2019-January%2F122441.html&amp;data=02%7C01%7Cyskoh%40mellanox.com%7Cf0818214ecbf492d5c1408d6761a9136%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636826253828868015&amp;sdata=7pT%2BPTNslOpTCIqFjaZ223Hg3btpDB3dGC9xiy13wDA%3D&amp;reserved=0
>> 
>> config/arm/meson.build | 11 +++++++----
>> 1 file changed, 7 insertions(+), 4 deletions(-)
>> 
>> diff --git a/config/arm/meson.build b/config/arm/meson.build
>> index dae55d6b26..3af256a5ec 100644
>> --- a/config/arm/meson.build
>> +++ b/config/arm/meson.build
>> @@ -47,8 +47,7 @@ flags_common_default = [
>> flags_generic = [
>> 	['RTE_MACHINE', '"armv8a"'],
>> 	['RTE_MAX_LCORE', 256],
>> -	['RTE_USE_C11_MEM_MODEL', true],
>> -	['RTE_CACHE_LINE_SIZE', 128]]
>> +	['RTE_USE_C11_MEM_MODEL', true]]
>> flags_cavium = [
>> 	['RTE_MACHINE', '"thunderx"'],
>> 	['RTE_CACHE_LINE_SIZE', 128],
>> @@ -89,15 +88,19 @@ impl_dpaa2 = ['NXP DPAA2', flags_dpaa2,
>> machine_args_generic]
>> 
>> dpdk_conf.set('RTE_FORCE_INTRINSICS', 1)
>> 
>> +# In config/arm64_armv8_linuxapp_gcc, maximum available cache line
>> size (128B)
>> +# in arm64 implementations is set by default for generic config.
>> However,
>> +# setting 64B is preferable for meson build in order to support
>> majority of CPUs
>> +# which don't have Implementor ID or Part Number programmed on chip.
>> +dpdk_conf.set('RTE_CACHE_LINE_SIZE', 64)
>> +
>> if cc.sizeof('void *') != 8
>> -	dpdk_conf.set('RTE_CACHE_LINE_SIZE', 64)
>> 	dpdk_conf.set('RTE_ARCH_ARM', 1)
>> 	dpdk_conf.set('RTE_ARCH_ARMv7', 1)
>> 	# the minimum architecture supported, armv7-a, needs the
>> following,
>> 	# mk/machine/armv7a/rte.vars.mk sets it too
>> 	machine_args += '-mfpu=neon'
>> else
>> -	dpdk_conf.set('RTE_CACHE_LINE_SIZE', 128)
>> 	dpdk_conf.set('RTE_ARCH_ARM64', 1)
>> 	dpdk_conf.set('RTE_ARCH_64', 1)
>>
  
Jerin Jacob Kollanukkaran Jan. 9, 2019, 10:49 a.m. UTC | #4
On Wed, 2019-01-09 at 10:22 +0000, Yongseok Koh wrote:
> > On Jan 9, 2019, at 2:09 AM, Jerin Jacob Kollanukkaran <
> > jerinj@marvell.com> wrote:
> > 
> > On Wed, 2019-01-09 at 01:39 -0800, Yongseok Koh wrote:
> > > ---------------------------------------------------------------
> > > ----
> > > ---
> > > In config/arm64_armv8_linuxapp_gcc, maximum available cache line
> > > size
> > > (128B) in arm64 implementations is set by default for generic
> > > config.
> > > However, setting 64B is preferable for meson build in order to
> > > support
> > > majority of CPUs which don't have Implementor ID or Part Number
> > > programmed
> > > on chip.
> > 
> > Adding Luca to get input from distro build perspective.
> > 
> > If I understand it correctly, distro build will be using
> > the generic config/arm/arm64_armv8_linuxapp_gcc for generic build.
> > If so, We can not change cache line size for generic config
> > as mentined the reason are
> > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmails.dpdk.org%2Farchives%2Fdev%2F2019-January%2F122441.html&amp;data=02%7C01%7Cyskoh%40mellanox.com%7Cf0818214ecbf492d5c1408d6761a9136%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636826253828868015&amp;sdata=7pT%2BPTNslOpTCIqFjaZ223Hg3btpDB3dGC9xiy13wDA%3D&amp;reserved=0
> 
> My understanding from your comment was distro build doesn't use meson
> but make with config/defconfig_arm64-armv8a-linuxapp-gcc.

Thats for now, Going forward distro build will switch to meson.

> Let's hear from Luca.
> 
> > I think, I way forward is to add config/arm/arm64_a72_linuxapp_gcc
> > for meson. This config can be used for all SoC with A72 armv8 
> > implementation and may have sym link to specfific SoC to avoid
> > confusion to end users.
> 
> Is config/arm/arm64_a72_linuxapp_gcc valid? Others have 

Yes. For cross compiling for A72.

Having not seperate IMPLEMENTOR ID is a chip design issue.
I think it can work around by creating
config/arm/arm64_<your_soc_name>_linuxapp_gcc
and build on x86 or arm64 through

meson build --cross-file config/arm/arm64_<your_soc_name>_linuxapp_gcc


> arm64_[IMPLEMENTOR]_linuxapp_gcc.
> a72 is got from PartNum.
> 
> Thanks,
> Yongseok
> 
> > > Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> > > ---
> > > 
> > > Discussion on the mailing list:
> > >    
> > > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmails.dpdk.org%2Farchives%2Fdev%2F2019-January%2F122441.html&amp;data=02%7C01%7Cyskoh%40mellanox.com%7Cf0818214ecbf492d5c1408d6761a9136%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636826253828868015&amp;sdata=7pT%2BPTNslOpTCIqFjaZ223Hg3btpDB3dGC9xiy13wDA%3D&amp;reserved=0
> > > 
> > > config/arm/meson.build | 11 +++++++----
> > > 1 file changed, 7 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/config/arm/meson.build b/config/arm/meson.build
> > > index dae55d6b26..3af256a5ec 100644
> > > --- a/config/arm/meson.build
> > > +++ b/config/arm/meson.build
> > > @@ -47,8 +47,7 @@ flags_common_default = [
> > > flags_generic = [
> > > 	['RTE_MACHINE', '"armv8a"'],
> > > 	['RTE_MAX_LCORE', 256],
> > > -	['RTE_USE_C11_MEM_MODEL', true],
> > > -	['RTE_CACHE_LINE_SIZE', 128]]
> > > +	['RTE_USE_C11_MEM_MODEL', true]]
> > > flags_cavium = [
> > > 	['RTE_MACHINE', '"thunderx"'],
> > > 	['RTE_CACHE_LINE_SIZE', 128],
> > > @@ -89,15 +88,19 @@ impl_dpaa2 = ['NXP DPAA2', flags_dpaa2,
> > > machine_args_generic]
> > > 
> > > dpdk_conf.set('RTE_FORCE_INTRINSICS', 1)
> > > 
> > > +# In config/arm64_armv8_linuxapp_gcc, maximum available cache
> > > line
> > > size (128B)
> > > +# in arm64 implementations is set by default for generic config.
> > > However,
> > > +# setting 64B is preferable for meson build in order to support
> > > majority of CPUs
> > > +# which don't have Implementor ID or Part Number programmed on
> > > chip.
> > > +dpdk_conf.set('RTE_CACHE_LINE_SIZE', 64)
> > > +
> > > if cc.sizeof('void *') != 8
> > > -	dpdk_conf.set('RTE_CACHE_LINE_SIZE', 64)
> > > 	dpdk_conf.set('RTE_ARCH_ARM', 1)
> > > 	dpdk_conf.set('RTE_ARCH_ARMv7', 1)
> > > 	# the minimum architecture supported, armv7-a, needs the
> > > following,
> > > 	# mk/machine/armv7a/rte.vars.mk sets it too
> > > 	machine_args += '-mfpu=neon'
> > > else
> > > -	dpdk_conf.set('RTE_CACHE_LINE_SIZE', 128)
> > > 	dpdk_conf.set('RTE_ARCH_ARM64', 1)
> > > 	dpdk_conf.set('RTE_ARCH_64', 1)
> > >
  
Jerin Jacob Kollanukkaran Jan. 9, 2019, 10:52 a.m. UTC | #5
On Wed, 2019-01-09 at 10:19 +0000, Luca Boccassi wrote:
> On Wed, 2019-01-09 at 10:09 +0000, Jerin Jacob Kollanukkaran wrote:
> > On Wed, 2019-01-09 at 01:39 -0800, Yongseok Koh wrote:
> > > ---------------------------------------------------------------
> > > ----
> > > ---
> > > In config/arm64_armv8_linuxapp_gcc, maximum available cache line
> > > size
> > > (128B) in arm64 implementations is set by default for generic
> > > config.
> > > However, setting 64B is preferable for meson build in order to
> > > support
> > > majority of CPUs which don't have Implementor ID or Part Number
> > > programmed
> > > on chip.
> > 
> > Adding Luca to get input from distro build perspective.
> > 
> > If I understand it correctly, distro build will be using
> > the generic config/arm/arm64_armv8_linuxapp_gcc for generic build.
> > If so, We can not change cache line size for generic config
> > as mentined the reason are
> > https://mails.dpdk.org/archives/dev/2019-January/122441.html
> > 
> > 
> > I think, I way forward is to add config/arm/arm64_a72_linuxapp_gcc
> > for meson. This config can be used for all SoC with A72 armv8 
> > implementation and may have sym link to specfific SoC to avoid
> > confusion to end users.
> 
> For Meson, I recently sent a patch to use a common and stable
> baseline
> machine arg if the uses sets it to "default":
> 
> https://patches.dpdk.org/patch/49477/
> 
> For distros, I think if, within that path, the cache size is set to
> the
> minimum common denominator like the -march is, it will be fine.

> 
> At that point, what happens for "native" or otherwise specified
> builds
> it will not affect distros (as long as they use "default" like Debian
> and Ubuntu do).
> 

Thanks for the input

> Does this answer your question?


Kind of.. Will you be building "the generic image" on ANY arm64 machine
with following command?

meson configure -Dmachine=default
meson build
cd build
ninja
ninja install

If so, This patch will change the cache line size of generic build.
Right?

> 
> > > Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> > > ---
> > > 
> > > Discussion on the mailing list:
> > >     https://mails.dpdk.org/archives/dev/2019-January/122441.html
> > > 
> > >  config/arm/meson.build | 11 +++++++----
> > >  1 file changed, 7 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/config/arm/meson.build b/config/arm/meson.build
> > > index dae55d6b26..3af256a5ec 100644
> > > --- a/config/arm/meson.build
> > > +++ b/config/arm/meson.build
> > > @@ -47,8 +47,7 @@ flags_common_default = [
> > >  flags_generic = [
> > >  	['RTE_MACHINE', '"armv8a"'],
> > >  	['RTE_MAX_LCORE', 256],
> > > -	['RTE_USE_C11_MEM_MODEL', true],
> > > -	['RTE_CACHE_LINE_SIZE', 128]]
> > > +	['RTE_USE_C11_MEM_MODEL', true]]
> > >  flags_cavium = [
> > >  	['RTE_MACHINE', '"thunderx"'],
> > >  	['RTE_CACHE_LINE_SIZE', 128],
> > > @@ -89,15 +88,19 @@ impl_dpaa2 = ['NXP DPAA2', flags_dpaa2,
> > > machine_args_generic]
> > >  
> > >  dpdk_conf.set('RTE_FORCE_INTRINSICS', 1)
> > >  
> > > +# In config/arm64_armv8_linuxapp_gcc, maximum available cache
> > > line
> > > size (128B)
> > > +# in arm64 implementations is set by default for generic config.
> > > However,
> > > +# setting 64B is preferable for meson build in order to support
> > > majority of CPUs
> > > +# which don't have Implementor ID or Part Number programmed on
> > > chip.
> > > +dpdk_conf.set('RTE_CACHE_LINE_SIZE', 64)
> > > +
> > >  if cc.sizeof('void *') != 8
> > > -	dpdk_conf.set('RTE_CACHE_LINE_SIZE', 64)
> > >  	dpdk_conf.set('RTE_ARCH_ARM', 1)
> > >  	dpdk_conf.set('RTE_ARCH_ARMv7', 1)
> > >  	# the minimum architecture supported, armv7-a, needs the
> > > following,
> > >  	# mk/machine/armv7a/rte.vars.mk sets it too
> > >  	machine_args += '-mfpu=neon'
> > >  else
> > > -	dpdk_conf.set('RTE_CACHE_LINE_SIZE', 128)
> > >  	dpdk_conf.set('RTE_ARCH_ARM64', 1)
> > >  	dpdk_conf.set('RTE_ARCH_64', 1)
> > >
  
Thomas Monjalon Jan. 9, 2019, 11:28 a.m. UTC | #6
09/01/2019 11:49, Jerin Jacob Kollanukkaran:
> On Wed, 2019-01-09 at 10:22 +0000, Yongseok Koh wrote:
> > On Jan 9, 2019, at 2:09 AM, Jerin Jacob Kollanukkaran wrote:
> > > I think, I way forward is to add config/arm/arm64_a72_linuxapp_gcc
> > > for meson. This config can be used for all SoC with A72 armv8 
> > > implementation and may have sym link to specfific SoC to avoid
> > > confusion to end users.
> > 
> > Is config/arm/arm64_a72_linuxapp_gcc valid? Others have 
> 
> Yes. For cross compiling for A72.

Any cross-compilation with meson requires a config file.
The default Arm cross-compilation is done with
	config/arm/arm64_armv8_linuxapp_gcc
which set implementor_id = 'generic'

For native compilation, implementor_id is detected from
/sys/devices/system/cpu/cpu0/regs/identification/midr_el1

So each Arm machine needs 2 things:
	- a cross-compilation file
	- settings based on implementor_id in config/arm/meson.build

> Having not seperate IMPLEMENTOR ID is a chip design issue.

No I don't think it's a design issue.
If the Arm core has no modification, it does not need to be
specially identified.

> I think it can work around by creating
> config/arm/arm64_<your_soc_name>_linuxapp_gcc
> and build on x86 or arm64 through
> 
> meson build --cross-file config/arm/arm64_<your_soc_name>_linuxapp_gcc

No, it is a real A72, so it should work with default settings.

The only issue we have is that the default cache line size for Aarch64
is set to 128 in config/arm/meson.build, and this is wrong.
The default cache line is 64 bits.
This is already overriden for Cavium machines which have 128-bit cache lines.
It may be needed to do the same change for other machines (Qualcomm?)
having Arm core modified to 128-bit cache lines.

The other concern is about running a generic Arm build.
Given 64-bit should be the default, generic builds will have this value.
Is it a big issue for running generic 64-bit build on Cavium machines?
  
Jerin Jacob Kollanukkaran Jan. 9, 2019, 12:47 p.m. UTC | #7
On Wed, 2019-01-09 at 12:28 +0100, Thomas Monjalon wrote:
> 09/01/2019 11:49, Jerin Jacob Kollanukkaran:
> > On Wed, 2019-01-09 at 10:22 +0000, Yongseok Koh wrote:
> > > On Jan 9, 2019, at 2:09 AM, Jerin Jacob Kollanukkaran wrote:
> > > > I think, I way forward is to add
> > > > config/arm/arm64_a72_linuxapp_gcc
> > > > for meson. This config can be used for all SoC with A72 armv8 
> > > > implementation and may have sym link to specfific SoC to avoid
> > > > confusion to end users.
> > > 
> > > Is config/arm/arm64_a72_linuxapp_gcc valid? Others have 
> > 
> > Yes. For cross compiling for A72.
> 
> Any cross-compilation with meson requires a config file.
> The default Arm cross-compilation is done with
> 	config/arm/arm64_armv8_linuxapp_gcc
> which set implementor_id = 'generic'
> 
> For native compilation, implementor_id is detected from
> /sys/devices/system/cpu/cpu0/regs/identification/midr_el1
> 
> So each Arm machine needs 2 things:
> 	- a cross-compilation file
> 	- settings based on implementor_id in config/arm/meson.build

Yes. config/arm/arm64_armv8_linuxapp_gcc sets the implementor_id =
'generic' which assumed to generic across all the armv8 platform.
If tomorrow there is new core from ARM which A100 with armv8.2 specific
we can not tune the generic params armv8.2 as it will break other 
CPU.


> > Having not seperate IMPLEMENTOR ID is a chip design issue.
> 
> No I don't think it's a design issue.
> If the Arm core has no modification, it does not need to be
> specially identified.

Thats right. It does not need to be specially identified,
then should have default config is enough. 


> 
> > I think it can work around by creating
> > config/arm/arm64_<your_soc_name>_linuxapp_gcc
> > and build on x86 or arm64 through
> > 
> > meson build --cross-file
> > config/arm/arm64_<your_soc_name>_linuxapp_gcc
> 
> No, it is a real A72, so it should work with default settings.
> 
> The only issue we have is that the default cache line size for
> Aarch64
> is set to 128 in config/arm/meson.build, and this is wrong.
> The default cache line is 64 bits.

The cache line size as per ARM spec it is IMPLEMENTATION DEFINED.
So no default there. So the default is something work on all platforms.
Actually Cavium has machine with 64B and 128B CL and same image should
work on both for generic build.

> This is already overriden for Cavium machines which have 128-bit
> cache lines.
> It may be needed to do the same change for other machines (Qualcomm?)
> having Arm core modified to 128-bit cache lines.

Assume you meant 128B here. Building the image Naively(on 128B CL
machine) and cross compile (on x86) is not an issue.

> 
> The other concern is about running a generic Arm build.

Yes. That's the ONLY concern.

> Given 64-bit should be the default, generic builds will have this
> value.
> Is it a big issue for running generic 64-bit build on Cavium
> machines?

Cavium has both 64B and 128B CL machines. So putting generic form,

You can run 128B configured image on 64B machine, It will waste some
memory not beyond that. Other way around will result in HW misbehavior.
ie Running 64B CL image on 128B target.






> 
>
  
Luca Boccassi Jan. 9, 2019, 1:14 p.m. UTC | #8
On Wed, 2019-01-09 at 10:52 +0000, Jerin Jacob Kollanukkaran wrote:
> On Wed, 2019-01-09 at 10:19 +0000, Luca Boccassi wrote:
> > On Wed, 2019-01-09 at 10:09 +0000, Jerin Jacob Kollanukkaran wrote:
> > > On Wed, 2019-01-09 at 01:39 -0800, Yongseok Koh wrote:
> > > > ---------------------------------------------------------------
> > > > ----
> > > > ---
> > > > In config/arm64_armv8_linuxapp_gcc, maximum available cache
> > > > line
> > > > size
> > > > (128B) in arm64 implementations is set by default for generic
> > > > config.
> > > > However, setting 64B is preferable for meson build in order to
> > > > support
> > > > majority of CPUs which don't have Implementor ID or Part Number
> > > > programmed
> > > > on chip.
> > > 
> > > Adding Luca to get input from distro build perspective.
> > > 
> > > If I understand it correctly, distro build will be using
> > > the generic config/arm/arm64_armv8_linuxapp_gcc for generic
> > > build.
> > > If so, We can not change cache line size for generic config
> > > as mentined the reason are
> > > https://mails.dpdk.org/archives/dev/2019-January/122441.html
> > > 
> > > 
> > > I think, I way forward is to add
> > > config/arm/arm64_a72_linuxapp_gcc
> > > for meson. This config can be used for all SoC with A72 armv8 
> > > implementation and may have sym link to specfific SoC to avoid
> > > confusion to end users.
> > 
> > For Meson, I recently sent a patch to use a common and stable
> > baseline
> > machine arg if the uses sets it to "default":
> > 
> > https://patches.dpdk.org/patch/49477/
> > 
> > For distros, I think if, within that path, the cache size is set to
> > the
> > minimum common denominator like the -march is, it will be fine.
> > 
> > At that point, what happens for "native" or otherwise specified
> > builds
> > it will not affect distros (as long as they use "default" like
> > Debian
> > and Ubuntu do).
> > 
> 
> Thanks for the input
> 
> > Does this answer your question?
> 
> 
> Kind of.. Will you be building "the generic image" on ANY arm64
> machine
> with following command?
> 
> meson configure -Dmachine=default
> meson build
> cd build
> ninja
> ninja install
> 
> If so, This patch will change the cache line size of generic build.
> Right?

Yes, that's the command used everywhere on Debian/Ubuntu, regardless of
the machine type, and this patch will change the value for those cases
as far as I can see.

Please bear in mind that I know really little about arm, so I am not
sure which should be the default for maximum compatibility.
  
Thomas Monjalon Jan. 9, 2019, 1:30 p.m. UTC | #9
09/01/2019 13:47, Jerin Jacob Kollanukkaran:
> On Wed, 2019-01-09 at 12:28 +0100, Thomas Monjalon wrote:
> > 09/01/2019 11:49, Jerin Jacob Kollanukkaran:
> > > On Wed, 2019-01-09 at 10:22 +0000, Yongseok Koh wrote:
> > > > On Jan 9, 2019, at 2:09 AM, Jerin Jacob Kollanukkaran wrote:
> > > > > I think, I way forward is to add
> > > > > config/arm/arm64_a72_linuxapp_gcc
> > > > > for meson. This config can be used for all SoC with A72 armv8 
> > > > > implementation and may have sym link to specfific SoC to avoid
> > > > > confusion to end users.
> > > > 
> > > > Is config/arm/arm64_a72_linuxapp_gcc valid? Others have 
> > > 
> > > Yes. For cross compiling for A72.
> > 
> > Any cross-compilation with meson requires a config file.
> > The default Arm cross-compilation is done with
> > 	config/arm/arm64_armv8_linuxapp_gcc
> > which set implementor_id = 'generic'
> > 
> > For native compilation, implementor_id is detected from
> > /sys/devices/system/cpu/cpu0/regs/identification/midr_el1
> > 
> > So each Arm machine needs 2 things:
> > 	- a cross-compilation file
> > 	- settings based on implementor_id in config/arm/meson.build
> 
> Yes. config/arm/arm64_armv8_linuxapp_gcc sets the implementor_id =
> 'generic' which assumed to generic across all the armv8 platform.
> If tomorrow there is new core from ARM which A100 with armv8.2 specific
> we can not tune the generic params armv8.2 as it will break other 
> CPU.
> 
> 
> > > Having not seperate IMPLEMENTOR ID is a chip design issue.
> > 
> > No I don't think it's a design issue.
> > If the Arm core has no modification, it does not need to be
> > specially identified.
> 
> Thats right. It does not need to be specially identified,
> then should have default config is enough. 
> 
> 
> > 
> > > I think it can work around by creating
> > > config/arm/arm64_<your_soc_name>_linuxapp_gcc
> > > and build on x86 or arm64 through
> > > 
> > > meson build --cross-file
> > > config/arm/arm64_<your_soc_name>_linuxapp_gcc
> > 
> > No, it is a real A72, so it should work with default settings.
> > 
> > The only issue we have is that the default cache line size for
> > Aarch64
> > is set to 128 in config/arm/meson.build, and this is wrong.
> > The default cache line is 64 bits.
> 
> The cache line size as per ARM spec it is IMPLEMENTATION DEFINED.

In A72 spec, it is said
"Returns 0b010 to indicate that the cache line size is 64 bytes."
But I guess we cannot say it is always true for all models.
So let's assume there is no default.

> So no default there. So the default is something work on all platforms.
> Actually Cavium has machine with 64B and 128B CL and same image should
> work on both for generic build.
> 
> > This is already overriden for Cavium machines which have 128-bit
> > cache lines.
> > It may be needed to do the same change for other machines (Qualcomm?)
> > having Arm core modified to 128-bit cache lines.
> 
> Assume you meant 128B here.

Yes, sorry I mixed bits and bytes :)

> Building the image Naively(on 128B CL
> machine) and cross compile (on x86) is not an issue.
> 
> > 
> > The other concern is about running a generic Arm build.
> 
> Yes. That's the ONLY concern.
> 
> > Given 64-bit should be the default, generic builds will have this
> > value.
> > Is it a big issue for running generic 64-bit build on Cavium
> > machines?
> 
> Cavium has both 64B and 128B CL machines. So putting generic form,
> 
> You can run 128B configured image on 64B machine, It will waste some
> memory not beyond that. Other way around will result in HW misbehavior.
> ie Running 64B CL image on 128B target.

Indeed it is the main concern.
Running DPDK tuned for 128 bytes on a core having 64 bytes cache line
will result in lower performances. It is less an issue than HW misbehavior.

If we agree to keep 128 bytes as generic cache line size for Arm,
we need a way to get 64 bytes size for unmodified cores.
In other words, the generic build settings must be different of
the default settings.

Please make a difference between default 'armv8' and 'generic'
as implementor_id in config/arm/meson.build.
I propose arm64_armv8_linuxapp_gcc being the default config (for armv8)
and creating arm64_generic_linuxapp_gcc for the generic build (for distros).

OK?
  
Jerin Jacob Kollanukkaran Jan. 9, 2019, 2:23 p.m. UTC | #10
On Wed, 2019-01-09 at 14:30 +0100, Thomas Monjalon wrote:
> 09/01/2019 13:47, Jerin Jacob Kollanukkaran:
> > On Wed, 2019-01-09 at 12:28 +0100, Thomas Monjalon wrote:
> > > 09/01/2019 11:49, Jerin Jacob Kollanukkaran:
> > > > On Wed, 2019-01-09 at 10:22 +0000, Yongseok Koh wrote:
> > > > > On Jan 9, 2019, at 2:09 AM, Jerin Jacob Kollanukkaran wrote:
> > > > > > I think, I way forward is to add
> > > > > > config/arm/arm64_a72_linuxapp_gcc
> > > > > > for meson. This config can be used for all SoC with A72
> > > > > > armv8 
> > > > > > implementation and may have sym link to specfific SoC to
> > > > > > avoid
> > > > > > confusion to end users.
> > > > > 
> > > > > Is config/arm/arm64_a72_linuxapp_gcc valid? Others have 
> > > > 
> > > > Yes. For cross compiling for A72.
> > > 
> > > Any cross-compilation with meson requires a config file.
> > > The default Arm cross-compilation is done with
> > > 	config/arm/arm64_armv8_linuxapp_gcc
> > > which set implementor_id = 'generic'
> > > 
> > > For native compilation, implementor_id is detected from
> > > /sys/devices/system/cpu/cpu0/regs/identification/midr_el1
> > > 
> > > So each Arm machine needs 2 things:
> > > 	- a cross-compilation file
> > > 	- settings based on implementor_id in config/arm/meson.build
> > 
> > Yes. config/arm/arm64_armv8_linuxapp_gcc sets the implementor_id =
> > 'generic' which assumed to generic across all the armv8 platform.
> > If tomorrow there is new core from ARM which A100 with armv8.2
> > specific
> > we can not tune the generic params armv8.2 as it will break other 
> > CPU.
> > 
> > 
> > > > Having not seperate IMPLEMENTOR ID is a chip design issue.
> > > 
> > > No I don't think it's a design issue.
> > > If the Arm core has no modification, it does not need to be
> > > specially identified.
> > 
> > Thats right. It does not need to be specially identified,
> > then should have default config is enough. 
> > 
> > 
> > > > I think it can work around by creating
> > > > config/arm/arm64_<your_soc_name>_linuxapp_gcc
> > > > and build on x86 or arm64 through
> > > > 
> > > > meson build --cross-file
> > > > config/arm/arm64_<your_soc_name>_linuxapp_gcc
> > > 
> > > No, it is a real A72, so it should work with default settings.
> > > 
> > > The only issue we have is that the default cache line size for
> > > Aarch64
> > > is set to 128 in config/arm/meson.build, and this is wrong.
> > > The default cache line is 64 bits.
> > 
> > The cache line size as per ARM spec it is IMPLEMENTATION DEFINED.
> 
> In A72 spec, it is said
> "Returns 0b010 to indicate that the cache line size is 64 bytes."
> But I guess we cannot say it is always true for all models.
> So let's assume there is no default.

Please note, A72 is not armv8 spec. A72 is just an IMPLEMENTATION of
armv8.


> > So no default there. So the default is something work on all
> > platforms.
> > Actually Cavium has machine with 64B and 128B CL and same image
> > should
> > work on both for generic build.
> > 
> > > This is already overriden for Cavium machines which have 128-bit
> > > cache lines.
> > > It may be needed to do the same change for other machines
> > > (Qualcomm?)
> > > having Arm core modified to 128-bit cache lines.
> > 
> > Assume you meant 128B here.
> 
> Yes, sorry I mixed bits and bytes :)
> 
> > Building the image Naively(on 128B CL
> > machine) and cross compile (on x86) is not an issue.
> > 
> > > The other concern is about running a generic Arm build.
> > 
> > Yes. That's the ONLY concern.
> > 
> > > Given 64-bit should be the default, generic builds will have this
> > > value.
> > > Is it a big issue for running generic 64-bit build on Cavium
> > > machines?
> > 
> > Cavium has both 64B and 128B CL machines. So putting generic form,
> > 
> > You can run 128B configured image on 64B machine, It will waste
> > some
> > memory not beyond that. Other way around will result in HW
> > misbehavior.
> > ie Running 64B CL image on 128B target.
> 
> Indeed it is the main concern.
> Running DPDK tuned for 128 bytes on a core having 64 bytes cache line
> will result in lower performances. It is less an issue than HW
> misbehavior.

Do you see performance issue or it more memory usage? It nothing
do with thread just of out curosity. Becase, our 64CL machine does
take more memory, performance seems to same for both. Note we are
using 512MB hugepage size.



> 
> If we agree to keep 128 bytes as generic cache line size for Arm,
> we need a way to get 64 bytes size for unmodified cores.
> In other words, the generic build settings must be different of
> the default settings.

Please send a patch.

If MIDR value is set to A72, we can set to 64B cache, no issue.

> Please make a difference between default 'armv8' and 'generic'
> as implementor_id in config/arm/meson.build.
> I propose arm64_armv8_linuxapp_gcc being the default config (for
> armv8)
> and creating arm64_generic_linuxapp_gcc for the generic build (for
> distros).

It should be inline with how distro guys build the image. I guess
we dont want DPDK to be a exception.

Please check below thread and patch.

http://mails.dpdk.org/archives/dev/2019-January/122676.html
https://patches.dpdk.org/patch/49477/

Debian folks are building like this for the _generic_ image.
What ever works for every distros, I am fine with that.

meson configure -Dmachine=default
meson build
cd build
ninja
ninja install


> 
> OK?
> 
>
  
Thomas Monjalon Jan. 9, 2019, 2:57 p.m. UTC | #11
09/01/2019 15:23, Jerin Jacob Kollanukkaran:
> On Wed, 2019-01-09 at 14:30 +0100, Thomas Monjalon wrote:
> > 09/01/2019 13:47, Jerin Jacob Kollanukkaran:
> > > On Wed, 2019-01-09 at 12:28 +0100, Thomas Monjalon wrote:
> > > > 09/01/2019 11:49, Jerin Jacob Kollanukkaran:
> > > > > On Wed, 2019-01-09 at 10:22 +0000, Yongseok Koh wrote:
> > > > > > On Jan 9, 2019, at 2:09 AM, Jerin Jacob Kollanukkaran wrote:
> > > > > > > I think, I way forward is to add
> > > > > > > config/arm/arm64_a72_linuxapp_gcc
> > > > > > > for meson. This config can be used for all SoC with A72
> > > > > > > armv8 
> > > > > > > implementation and may have sym link to specfific SoC to
> > > > > > > avoid
> > > > > > > confusion to end users.
> > > > > > 
> > > > > > Is config/arm/arm64_a72_linuxapp_gcc valid? Others have 
> > > > > 
> > > > > Yes. For cross compiling for A72.
> > > > 
> > > > Any cross-compilation with meson requires a config file.
> > > > The default Arm cross-compilation is done with
> > > > 	config/arm/arm64_armv8_linuxapp_gcc
> > > > which set implementor_id = 'generic'
> > > > 
> > > > For native compilation, implementor_id is detected from
> > > > /sys/devices/system/cpu/cpu0/regs/identification/midr_el1
> > > > 
> > > > So each Arm machine needs 2 things:
> > > > 	- a cross-compilation file
> > > > 	- settings based on implementor_id in config/arm/meson.build
> > > 
> > > Yes. config/arm/arm64_armv8_linuxapp_gcc sets the implementor_id =
> > > 'generic' which assumed to generic across all the armv8 platform.
> > > If tomorrow there is new core from ARM which A100 with armv8.2
> > > specific
> > > we can not tune the generic params armv8.2 as it will break other 
> > > CPU.
> > > 
> > > 
> > > > > Having not seperate IMPLEMENTOR ID is a chip design issue.
> > > > 
> > > > No I don't think it's a design issue.
> > > > If the Arm core has no modification, it does not need to be
> > > > specially identified.
> > > 
> > > Thats right. It does not need to be specially identified,
> > > then should have default config is enough. 
> > > 
> > > 
> > > > > I think it can work around by creating
> > > > > config/arm/arm64_<your_soc_name>_linuxapp_gcc
> > > > > and build on x86 or arm64 through
> > > > > 
> > > > > meson build --cross-file
> > > > > config/arm/arm64_<your_soc_name>_linuxapp_gcc
> > > > 
> > > > No, it is a real A72, so it should work with default settings.
> > > > 
> > > > The only issue we have is that the default cache line size for
> > > > Aarch64
> > > > is set to 128 in config/arm/meson.build, and this is wrong.
> > > > The default cache line is 64 bits.
> > > 
> > > The cache line size as per ARM spec it is IMPLEMENTATION DEFINED.
> > 
> > In A72 spec, it is said
> > "Returns 0b010 to indicate that the cache line size is 64 bytes."
> > But I guess we cannot say it is always true for all models.
> > So let's assume there is no default.
> 
> Please note, A72 is not armv8 spec. A72 is just an IMPLEMENTATION of
> armv8.

Yes, this my understanding. That's why I agree with you.

> > > So no default there. So the default is something work on all
> > > platforms.
> > > Actually Cavium has machine with 64B and 128B CL and same image
> > > should
> > > work on both for generic build.
> > > 
> > > > This is already overriden for Cavium machines which have 128-bit
> > > > cache lines.
> > > > It may be needed to do the same change for other machines
> > > > (Qualcomm?)
> > > > having Arm core modified to 128-bit cache lines.
> > > 
> > > Assume you meant 128B here.
> > 
> > Yes, sorry I mixed bits and bytes :)
> > 
> > > Building the image Naively(on 128B CL
> > > machine) and cross compile (on x86) is not an issue.
> > > 
> > > > The other concern is about running a generic Arm build.
> > > 
> > > Yes. That's the ONLY concern.
> > > 
> > > > Given 64-bit should be the default, generic builds will have this
> > > > value.
> > > > Is it a big issue for running generic 64-bit build on Cavium
> > > > machines?
> > > 
> > > Cavium has both 64B and 128B CL machines. So putting generic form,
> > > 
> > > You can run 128B configured image on 64B machine, It will waste
> > > some
> > > memory not beyond that. Other way around will result in HW
> > > misbehavior.
> > > ie Running 64B CL image on 128B target.
> > 
> > Indeed it is the main concern.
> > Running DPDK tuned for 128 bytes on a core having 64 bytes cache line
> > will result in lower performances. It is less an issue than HW
> > misbehavior.
> 
> Do you see performance issue or it more memory usage? It nothing
> do with thread just of out curosity. Becase, our 64CL machine does
> take more memory, performance seems to same for both. Note we are
> using 512MB hugepage size.

Yes, we see better performance with 64B cache line on Bluefield.

> > If we agree to keep 128 bytes as generic cache line size for Arm,
> > we need a way to get 64 bytes size for unmodified cores.
> > In other words, the generic build settings must be different of
> > the default settings.
> 
> Please send a patch.
> 
> If MIDR value is set to A72, we can set to 64B cache, no issue.
> 
> > Please make a difference between default 'armv8' and 'generic'
> > as implementor_id in config/arm/meson.build.
> > I propose arm64_armv8_linuxapp_gcc being the default config (for
> > armv8)
> > and creating arm64_generic_linuxapp_gcc for the generic build (for
> > distros).
> 
> It should be inline with how distro guys build the image. I guess
> we dont want DPDK to be a exception.

The machine option is specific to DPDK, so we can define it as we want.

> Please check below thread and patch.
> 
> http://mails.dpdk.org/archives/dev/2019-January/122676.html
> https://patches.dpdk.org/patch/49477/
> 
> Debian folks are building like this for the _generic_ image.
> What ever works for every distros, I am fine with that.
> 
> meson configure -Dmachine=default
> meson build
> cd build
> ninja
> ninja install

I think we agree on the idea of having different configs
for unmodified A72 core and generic build working for all.
The remaining bits to discuss are:
	- do we want to use the armv8 config for unmodified A72?
	- what should be the name of the generic config?

When digging more the config files in meson, I found this:
	http://mesonbuild.com/Cross-compilation.html#cross-file-locations
It says that distros or compilers should provide some config files.
It means we should check if some standard names are emerging
and try to follow the same naming, or even re-use existing config files.
  
Jerin Jacob Kollanukkaran Jan. 9, 2019, 3:34 p.m. UTC | #12
> > Please check below thread and patch.
> > 
> > http://mails.dpdk.org/archives/dev/2019-January/122676.html
> > https://patches.dpdk.org/patch/49477/
> > 
> > Debian folks are building like this for the _generic_ image.
> > What ever works for every distros, I am fine with that.
> > 
> > meson configure -Dmachine=default
> > meson build
> > cd build
> > ninja
> > ninja install
> 
> I think we agree on the idea of having different configs
> for unmodified A72 core and generic build working for all.

Yes, I agree. config or some scheme to address the generic and default
usecase.

> The remaining bits to discuss are:
> 	- do we want to use the armv8 config for unmodified A72?
> 	- what should be the name of the generic config?

If all distros following "meson configure -Dmachine=default" scheme
why not follow that to make generic image. i.e when machine=default
set then Cache lize size 128B CL specific stuff be kicked in else
it probe the value based on MIDR from sysfs.


> When digging more the config files in meson, I found this:
> 	
> http://mesonbuild.com/Cross-compilation.html#cross-file-locations
> It says that distros or compilers should provide some config files.
> It means we should check if some standard names are emerging
> and try to follow the same naming, or even re-use existing config
> files.

I think this for cross compilation. if I understand it correctly,
distro folks build arm64 image on arm64 target(not on x86) with generic
images with lowest denominator cpu flags and such that works for all
platforms.

Something similar found in arch linux website for arm64.
https://archlinuxarm.org/platforms/armv8/generic

See Platforms: ARMv8

Anyway I am leaving this decision to distro folks or whoever is expert
in meson based distro build generic creation.


>
  
Luca Boccassi Jan. 9, 2019, 3:41 p.m. UTC | #13
On Wed, 2019-01-09 at 15:34 +0000, Jerin Jacob Kollanukkaran wrote:
> > > Please check below thread and patch.
> > > 
> > > http://mails.dpdk.org/archives/dev/2019-January/122676.html
> > > https://patches.dpdk.org/patch/49477/
> > > 
> > > Debian folks are building like this for the _generic_ image.
> > > What ever works for every distros, I am fine with that.
> > > 
> > > meson configure -Dmachine=default
> > > meson build
> > > cd build
> > > ninja
> > > ninja install
> > 
> > I think we agree on the idea of having different configs
> > for unmodified A72 core and generic build working for all.
> 
> Yes, I agree. config or some scheme to address the generic and
> default
> usecase.
> 
> > The remaining bits to discuss are:
> > 	- do we want to use the armv8 config for unmodified A72?
> > 	- what should be the name of the generic config?
> 
> If all distros following "meson configure -Dmachine=default" scheme
> why not follow that to make generic image. i.e when machine=default
> set then Cache lize size 128B CL specific stuff be kicked in else
> it probe the value based on MIDR from sysfs.
> 
> 
> > When digging more the config files in meson, I found this:
> > 	
> > http://mesonbuild.com/Cross-compilation.html#cross-file-locations
> > It says that distros or compilers should provide some config files.
> > It means we should check if some standard names are emerging
> > and try to follow the same naming, or even re-use existing config
> > files.
> 
> I think this for cross compilation. if I understand it correctly,
> distro folks build arm64 image on arm64 target(not on x86) with
> generic
> images with lowest denominator cpu flags and such that works for all
> platforms.
> 
> Something similar found in arch linux website for arm64.
> https://archlinuxarm.org/platforms/armv8/generic
> 
> See Platforms: ARMv8
> 
> Anyway I am leaving this decision to distro folks or whoever is
> expert
> in meson based distro build generic creation.

Just a comment to confirm that yes, in general distributions do not do
cross-builds, but native builds. That is certainly the case for Debian,
and I am 99% sure it is also the case for Ubuntu and SUSE.
  
Pavan Nikhilesh Bhagavatula Jan. 9, 2019, 4:36 p.m. UTC | #14
On Wed, 2019-01-09 at 15:41 +0000, Luca Boccassi wrote:
> External Email
> 
> -------------------------------------------------------------------
> ---
> On Wed, 2019-01-09 at 15:34 +0000, Jerin Jacob Kollanukkaran wrote:
> > > > Please check below thread and patch.
> > > > 
> > > > http://mails.dpdk.org/archives/dev/2019-January/122676.html
> > > > https://patches.dpdk.org/patch/49477/
> > > > 
> > > > Debian folks are building like this for the _generic_ image.
> > > > What ever works for every distros, I am fine with that.
> > > > 
> > > > meson configure -Dmachine=default
> > > > meson build
> > > > cd build
> > > > ninja
> > > > ninja install
> > > 
> > > I think we agree on the idea of having different configs
> > > for unmodified A72 core and generic build working for all.
> > 
> > Yes, I agree. config or some scheme to address the generic and
> > default
> > usecase.
> > 
> > > The remaining bits to discuss are:
> > > 	- do we want to use the armv8 config for unmodified A72?
> > > 	- what should be the name of the generic config?
> > 
> > If all distros following "meson configure -Dmachine=default" scheme
> > why not follow that to make generic image. i.e when machine=default
> > set then Cache lize size 128B CL specific stuff be kicked in else
> > it probe the value based on MIDR from sysfs.
> > 

When we are not cross-compiling  can read CTR_EL0[1] register
(DminLine) to detect the cacheline size based on the native machine.
This method would satisfy all requirements and we need not maintain a
mapping for cacheline_sizes w.r.t part numbers.


[1] 
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.100095_0001_02_en/way1382037583047.html

(newer kernels expose it as a sysfs entry
 /sys/devices/system/cpu/cpu0/cache/index0/coherency_line_size, we can
use meson run_command to either run a python program with c byte code
or use cc.run('<program>') to get result directly).


> > 
> > > When digging more the config files in meson, I found this:
> > > 	
> > > http://mesonbuild.com/Cross-compilation.html#cross-file-locations
> > > It says that distros or compilers should provide some config
> > > files.
> > > It means we should check if some standard names are emerging
> > > and try to follow the same naming, or even re-use existing config
> > > files.
> > 
> > I think this for cross compilation. if I understand it correctly,
> > distro folks build arm64 image on arm64 target(not on x86) with
> > generic
> > images with lowest denominator cpu flags and such that works for
> > all
> > platforms.
> > 
> > Something similar found in arch linux website for arm64.
> > https://archlinuxarm.org/platforms/armv8/generic
> > 
> > See Platforms: ARMv8
> > 
> > Anyway I am leaving this decision to distro folks or whoever is
> > expert
> > in meson based distro build generic creation.
> 
> Just a comment to confirm that yes, in general distributions do not
> do
> cross-builds, but native builds. That is certainly the case for
> Debian,
> and I am 99% sure it is also the case for Ubuntu and SUSE.
> 

Regards,
Pavan.
  
Luca Boccassi Jan. 9, 2019, 4:52 p.m. UTC | #15
On Wed, 2019-01-09 at 16:36 +0000, Pavan Nikhilesh Bhagavatula wrote:
> On Wed, 2019-01-09 at 15:41 +0000, Luca Boccassi wrote:
> > External Email
> > 
> > -------------------------------------------------------------------
> > ---
> > On Wed, 2019-01-09 at 15:34 +0000, Jerin Jacob Kollanukkaran wrote:
> > > > > Please check below thread and patch.
> > > > > 
> > > > > http://mails.dpdk.org/archives/dev/2019-January/122676.html
> > > > > https://patches.dpdk.org/patch/49477/
> > > > > 
> > > > > Debian folks are building like this for the _generic_ image.
> > > > > What ever works for every distros, I am fine with that.
> > > > > 
> > > > > meson configure -Dmachine=default
> > > > > meson build
> > > > > cd build
> > > > > ninja
> > > > > ninja install
> > > > 
> > > > I think we agree on the idea of having different configs
> > > > for unmodified A72 core and generic build working for all.
> > > 
> > > Yes, I agree. config or some scheme to address the generic and
> > > default
> > > usecase.
> > > 
> > > > The remaining bits to discuss are:
> > > > 	- do we want to use the armv8 config for unmodified
> > > > A72?
> > > > 	- what should be the name of the generic config?
> > > 
> > > If all distros following "meson configure -Dmachine=default"
> > > scheme
> > > why not follow that to make generic image. i.e when
> > > machine=default
> > > set then Cache lize size 128B CL specific stuff be kicked in else
> > > it probe the value based on MIDR from sysfs.
> > > 
> 
> When we are not cross-compiling  can read CTR_EL0[1] register
> (DminLine) to detect the cacheline size based on the native machine.
> This method would satisfy all requirements and we need not maintain a
> mapping for cacheline_sizes w.r.t part numbers.
> 
> 
> [1] 
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.100095_00
> 01_02_en/way1382037583047.html
> 
> (newer kernels expose it as a sysfs entry
>  /sys/devices/system/cpu/cpu0/cache/index0/coherency_line_size, we
> can
> use meson run_command to either run a python program with c byte code
> or use cc.run('<program>') to get result directly).

Please, not for the machine=default case - we need a stable, invariant
option that does not depend on the build worker.
  
Pavan Nikhilesh Bhagavatula Jan. 9, 2019, 5:01 p.m. UTC | #16
On Wed, 2019-01-09 at 16:52 +0000, Luca Boccassi wrote:
> On Wed, 2019-01-09 at 16:36 +0000, Pavan Nikhilesh Bhagavatula wrote:
> > On Wed, 2019-01-09 at 15:41 +0000, Luca Boccassi wrote:
> > > External Email
> > > 
> > > ---------------------------------------------------------------
> > > ----
> > > ---
> > > On Wed, 2019-01-09 at 15:34 +0000, Jerin Jacob Kollanukkaran
> > > wrote:
> > > > > > Please check below thread and patch.
> > > > > > 
> > > > > > http://mails.dpdk.org/archives/dev/2019-January/122676.html
> > > > > > https://patches.dpdk.org/patch/49477/
> > > > > > 
> > > > > > Debian folks are building like this for the _generic_
> > > > > > image.
> > > > > > What ever works for every distros, I am fine with that.
> > > > > > 
> > > > > > meson configure -Dmachine=default
> > > > > > meson build
> > > > > > cd build
> > > > > > ninja
> > > > > > ninja install
> > > > > 
> > > > > I think we agree on the idea of having different configs
> > > > > for unmodified A72 core and generic build working for all.
> > > > 
> > > > Yes, I agree. config or some scheme to address the generic and
> > > > default
> > > > usecase.
> > > > 
> > > > > The remaining bits to discuss are:
> > > > > 	- do we want to use the armv8 config for unmodified
> > > > > A72?
> > > > > 	- what should be the name of the generic config?
> > > > 
> > > > If all distros following "meson configure -Dmachine=default"
> > > > scheme
> > > > why not follow that to make generic image. i.e when
> > > > machine=default
> > > > set then Cache lize size 128B CL specific stuff be kicked in
> > > > else
> > > > it probe the value based on MIDR from sysfs.
> > > > 
> > 
> > When we are not cross-compiling  can read CTR_EL0[1] register
> > (DminLine) to detect the cacheline size based on the native
> > machine.
> > This method would satisfy all requirements and we need not maintain
> > a
> > mapping for cacheline_sizes w.r.t part numbers.
> > 
> > 
> > [1] 
> > http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.100095_00
> > 01_02_en/way1382037583047.html
> > 
> > (newer kernels expose it as a sysfs entry
> >  /sys/devices/system/cpu/cpu0/cache/index0/coherency_line_size, we
> > can
> > use meson run_command to either run a python program with c byte
> > code
> > or use cc.run('<program>') to get result directly).
> 
> Please, not for the machine=default case - we need a stable,
> invariant
> option that does not depend on the build worker.
> 
Agreed, Something like :

read_ctr_el0 =
'''
#include <stdio.h>
int main(int argc, char **argv) {
       int32_t ctr_el0;
       __asm__ __volatile__("mrs %0, CTR_EL0\n\t"
                       : "=r" (ctr_el0) :  : "memory");
       printf("%d\n", 1 << ((ctr_el0 >> 16) & 0xf));
       return 0;
}
'''

if not meson.is_cross_build() and not arm_force_default_march
       result = cc.run(read_ctr_el0, name : 'cacheline check')
       if not result.returncode
               dpdk_conf.set('RTE_CACHE_LINE_SIZE', 		  					result.
stdout().strip());
       else
               dpdk_conf.set('RTE_CACHE_LINE_SIZE', 128);
endif
  
Yongseok Koh Jan. 14, 2019, 4:32 a.m. UTC | #17
> On Jan 9, 2019, at 6:57 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> 09/01/2019 15:23, Jerin Jacob Kollanukkaran:
>> On Wed, 2019-01-09 at 14:30 +0100, Thomas Monjalon wrote:
>>> 09/01/2019 13:47, Jerin Jacob Kollanukkaran:
>>>> On Wed, 2019-01-09 at 12:28 +0100, Thomas Monjalon wrote:
>>>>> 09/01/2019 11:49, Jerin Jacob Kollanukkaran:
>>>>>> On Wed, 2019-01-09 at 10:22 +0000, Yongseok Koh wrote:
>>>>>>> On Jan 9, 2019, at 2:09 AM, Jerin Jacob Kollanukkaran wrote:
>>>>>>>> I think, I way forward is to add
>>>>>>>> config/arm/arm64_a72_linuxapp_gcc
>>>>>>>> for meson. This config can be used for all SoC with A72
>>>>>>>> armv8 
>>>>>>>> implementation and may have sym link to specfific SoC to
>>>>>>>> avoid
>>>>>>>> confusion to end users.
>>>>>>> 
>>>>>>> Is config/arm/arm64_a72_linuxapp_gcc valid? Others have 
>>>>>> 
>>>>>> Yes. For cross compiling for A72.
>>>>> 
>>>>> Any cross-compilation with meson requires a config file.
>>>>> The default Arm cross-compilation is done with
>>>>> 	config/arm/arm64_armv8_linuxapp_gcc
>>>>> which set implementor_id = 'generic'
>>>>> 
>>>>> For native compilation, implementor_id is detected from
>>>>> /sys/devices/system/cpu/cpu0/regs/identification/midr_el1
>>>>> 
>>>>> So each Arm machine needs 2 things:
>>>>> 	- a cross-compilation file
>>>>> 	- settings based on implementor_id in config/arm/meson.build
>>>> 
>>>> Yes. config/arm/arm64_armv8_linuxapp_gcc sets the implementor_id =
>>>> 'generic' which assumed to generic across all the armv8 platform.
>>>> If tomorrow there is new core from ARM which A100 with armv8.2
>>>> specific
>>>> we can not tune the generic params armv8.2 as it will break other 
>>>> CPU.
>>>> 
>>>> 
>>>>>> Having not seperate IMPLEMENTOR ID is a chip design issue.
>>>>> 
>>>>> No I don't think it's a design issue.
>>>>> If the Arm core has no modification, it does not need to be
>>>>> specially identified.
>>>> 
>>>> Thats right. It does not need to be specially identified,
>>>> then should have default config is enough. 
>>>> 
>>>> 
>>>>>> I think it can work around by creating
>>>>>> config/arm/arm64_<your_soc_name>_linuxapp_gcc
>>>>>> and build on x86 or arm64 through
>>>>>> 
>>>>>> meson build --cross-file
>>>>>> config/arm/arm64_<your_soc_name>_linuxapp_gcc
>>>>> 
>>>>> No, it is a real A72, so it should work with default settings.
>>>>> 
>>>>> The only issue we have is that the default cache line size for
>>>>> Aarch64
>>>>> is set to 128 in config/arm/meson.build, and this is wrong.
>>>>> The default cache line is 64 bits.
>>>> 
>>>> The cache line size as per ARM spec it is IMPLEMENTATION DEFINED.
>>> 
>>> In A72 spec, it is said
>>> "Returns 0b010 to indicate that the cache line size is 64 bytes."
>>> But I guess we cannot say it is always true for all models.
>>> So let's assume there is no default.
>> 
>> Please note, A72 is not armv8 spec. A72 is just an IMPLEMENTATION of
>> armv8.
> 
> Yes, this my understanding. That's why I agree with you.
> 
>>>> So no default there. So the default is something work on all
>>>> platforms.
>>>> Actually Cavium has machine with 64B and 128B CL and same image
>>>> should
>>>> work on both for generic build.
>>>> 
>>>>> This is already overriden for Cavium machines which have 128-bit
>>>>> cache lines.
>>>>> It may be needed to do the same change for other machines
>>>>> (Qualcomm?)
>>>>> having Arm core modified to 128-bit cache lines.
>>>> 
>>>> Assume you meant 128B here.
>>> 
>>> Yes, sorry I mixed bits and bytes :)
>>> 
>>>> Building the image Naively(on 128B CL
>>>> machine) and cross compile (on x86) is not an issue.
>>>> 
>>>>> The other concern is about running a generic Arm build.
>>>> 
>>>> Yes. That's the ONLY concern.
>>>> 
>>>>> Given 64-bit should be the default, generic builds will have this
>>>>> value.
>>>>> Is it a big issue for running generic 64-bit build on Cavium
>>>>> machines?
>>>> 
>>>> Cavium has both 64B and 128B CL machines. So putting generic form,
>>>> 
>>>> You can run 128B configured image on 64B machine, It will waste
>>>> some
>>>> memory not beyond that. Other way around will result in HW
>>>> misbehavior.
>>>> ie Running 64B CL image on 128B target.
>>> 
>>> Indeed it is the main concern.
>>> Running DPDK tuned for 128 bytes on a core having 64 bytes cache line
>>> will result in lower performances. It is less an issue than HW
>>> misbehavior.
>> 
>> Do you see performance issue or it more memory usage? It nothing
>> do with thread just of out curosity. Becase, our 64CL machine does
>> take more memory, performance seems to same for both. Note we are
>> using 512MB hugepage size.
> 
> Yes, we see better performance with 64B cache line on Bluefield.
> 
>>> If we agree to keep 128 bytes as generic cache line size for Arm,
>>> we need a way to get 64 bytes size for unmodified cores.
>>> In other words, the generic build settings must be different of
>>> the default settings.
>> 
>> Please send a patch.
>> 
>> If MIDR value is set to A72, we can set to 64B cache, no issue.
>> 
>>> Please make a difference between default 'armv8' and 'generic'
>>> as implementor_id in config/arm/meson.build.
>>> I propose arm64_armv8_linuxapp_gcc being the default config (for
>>> armv8)
>>> and creating arm64_generic_linuxapp_gcc for the generic build (for
>>> distros).
>> 
>> It should be inline with how distro guys build the image. I guess
>> we dont want DPDK to be a exception.
> 
> The machine option is specific to DPDK, so we can define it as we want.
> 
>> Please check below thread and patch.
>> 
>> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmails.dpdk.org%2Farchives%2Fdev%2F2019-January%2F122676.html&amp;data=02%7C01%7Cyskoh%40mellanox.com%7C96576e66c4b6434b47ad08d67642be9a%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636826426371146146&amp;sdata=MNMzpjs7e71l4vZAmoyqicpElp7UIFO48UuamggQWHQ%3D&amp;reserved=0
>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatches.dpdk.org%2Fpatch%2F49477%2F&amp;data=02%7C01%7Cyskoh%40mellanox.com%7C96576e66c4b6434b47ad08d67642be9a%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636826426371146146&amp;sdata=okIrz7Idc8t7nMbFkcRnjxZg2wMn9ZTjqaTLlEXCnaU%3D&amp;reserved=0
>> 
>> Debian folks are building like this for the _generic_ image.
>> What ever works for every distros, I am fine with that.
>> 
>> meson configure -Dmachine=default
>> meson build
>> cd build
>> ninja
>> ninja install
> 
> I think we agree on the idea of having different configs
> for unmodified A72 core and generic build working for all.
> The remaining bits to discuss are:
> 	- do we want to use the armv8 config for unmodified A72?
> 	- what should be the name of the generic config?
> 
> When digging more the config files in meson, I found this:
> 	https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmesonbuild.com%2FCross-compilation.html%23cross-file-locations&amp;data=02%7C01%7Cyskoh%40mellanox.com%7C96576e66c4b6434b47ad08d67642be9a%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636826426371146146&amp;sdata=J8XiCovgwqxm8HHRCJ5bSUbx4yTHCO2YuZz2ryZJx8I%3D&amp;reserved=0
> It says that distros or compilers should provide some config files.
> It means we should check if some standard names are emerging
> and try to follow the same naming, or even re-use existing config files.

I'll come up with a new patch based on the discussion here.
A few things noted,
- we still want it to be 128B for generic build
- we at least agreed on changing it to 64B for A72
- As Qualcomm Centriq CPU has 128B cache line with A72, they should
  create a profile in meson.build based on their impl_id.

I talked to Thomas and we'll shoot it for 19.05.

Thanks,
Yongseok
  
Honnappa Nagarahalli Jan. 14, 2019, 7:44 a.m. UTC | #18
> >>>>>> On Wed, 2019-01-09 at 10:22 +0000, Yongseok Koh wrote:
> >>>>>>> On Jan 9, 2019, at 2:09 AM, Jerin Jacob Kollanukkaran wrote:
> >>>>>>>> I think, I way forward is to add
> >>>>>>>> config/arm/arm64_a72_linuxapp_gcc for meson. This config can be
> >>>>>>>> used for all SoC with A72
> >>>>>>>> armv8
> >>>>>>>> implementation and may have sym link to specfific SoC to avoid
> >>>>>>>> confusion to end users.
> >>>>>>>
> >>>>>>> Is config/arm/arm64_a72_linuxapp_gcc valid? Others have
> >>>>>>
> >>>>>> Yes. For cross compiling for A72.
> >>>>>
> >>>>> Any cross-compilation with meson requires a config file.
> >>>>> The default Arm cross-compilation is done with
> >>>>> 	config/arm/arm64_armv8_linuxapp_gcc
> >>>>> which set implementor_id = 'generic'
> >>>>>
> >>>>> For native compilation, implementor_id is detected from
> >>>>> /sys/devices/system/cpu/cpu0/regs/identification/midr_el1
> >>>>>
> >>>>> So each Arm machine needs 2 things:
> >>>>> 	- a cross-compilation file
> >>>>> 	- settings based on implementor_id in config/arm/meson.build
> >>>>
> >>>> Yes. config/arm/arm64_armv8_linuxapp_gcc sets the implementor_id =
> >>>> 'generic' which assumed to generic across all the armv8 platform.
> >>>> If tomorrow there is new core from ARM which A100 with armv8.2
> >>>> specific we can not tune the generic params armv8.2 as it will
> >>>> break other CPU.
> >>>>
> >>>>
> >>>>>> Having not seperate IMPLEMENTOR ID is a chip design issue.
> >>>>>
> >>>>> No I don't think it's a design issue.
> >>>>> If the Arm core has no modification, it does not need to be
> >>>>> specially identified.
> >>>>
> >>>> Thats right. It does not need to be specially identified, then
> >>>> should have default config is enough.
> >>>>
> >>>>
> >>>>>> I think it can work around by creating
> >>>>>> config/arm/arm64_<your_soc_name>_linuxapp_gcc
> >>>>>> and build on x86 or arm64 through
> >>>>>>
> >>>>>> meson build --cross-file
> >>>>>> config/arm/arm64_<your_soc_name>_linuxapp_gcc
> >>>>>
> >>>>> No, it is a real A72, so it should work with default settings.
> >>>>>
> >>>>> The only issue we have is that the default cache line size for
> >>>>> Aarch64
> >>>>> is set to 128 in config/arm/meson.build, and this is wrong.
> >>>>> The default cache line is 64 bits.
> >>>>
> >>>> The cache line size as per ARM spec it is IMPLEMENTATION DEFINED.
> >>>
> >>> In A72 spec, it is said
> >>> "Returns 0b010 to indicate that the cache line size is 64 bytes."
> >>> But I guess we cannot say it is always true for all models.
> >>> So let's assume there is no default.
> >>
> >> Please note, A72 is not armv8 spec. A72 is just an IMPLEMENTATION of
> >> armv8.
> >
> > Yes, this my understanding. That's why I agree with you.
> >
> >>>> So no default there. So the default is something work on all
> >>>> platforms.
> >>>> Actually Cavium has machine with 64B and 128B CL and same image
> >>>> should work on both for generic build.
> >>>>
> >>>>> This is already overriden for Cavium machines which have 128-bit
> >>>>> cache lines.
> >>>>> It may be needed to do the same change for other machines
> >>>>> (Qualcomm?)
> >>>>> having Arm core modified to 128-bit cache lines.
> >>>>
> >>>> Assume you meant 128B here.
> >>>
> >>> Yes, sorry I mixed bits and bytes :)
> >>>
> >>>> Building the image Naively(on 128B CL
> >>>> machine) and cross compile (on x86) is not an issue.
> >>>>
> >>>>> The other concern is about running a generic Arm build.
> >>>>
> >>>> Yes. That's the ONLY concern.
> >>>>
> >>>>> Given 64-bit should be the default, generic builds will have this
> >>>>> value.
> >>>>> Is it a big issue for running generic 64-bit build on Cavium
> >>>>> machines?
> >>>>
> >>>> Cavium has both 64B and 128B CL machines. So putting generic form,
> >>>>
> >>>> You can run 128B configured image on 64B machine, It will waste
> >>>> some memory not beyond that. Other way around will result in HW
> >>>> misbehavior.
> >>>> ie Running 64B CL image on 128B target.
> >>>
> >>> Indeed it is the main concern.
> >>> Running DPDK tuned for 128 bytes on a core having 64 bytes cache
> >>> line will result in lower performances. It is less an issue than HW
> >>> misbehavior.
> >>
> >> Do you see performance issue or it more memory usage? It nothing do
> >> with thread just of out curosity. Becase, our 64CL machine does take
> >> more memory, performance seems to same for both. Note we are using
> >> 512MB hugepage size.
> >
> > Yes, we see better performance with 64B cache line on Bluefield.
> >
> >>> If we agree to keep 128 bytes as generic cache line size for Arm, we
> >>> need a way to get 64 bytes size for unmodified cores.
> >>> In other words, the generic build settings must be different of the
> >>> default settings.
> >>
> >> Please send a patch.
> >>
> >> If MIDR value is set to A72, we can set to 64B cache, no issue.
> >>
> >>> Please make a difference between default 'armv8' and 'generic'
> >>> as implementor_id in config/arm/meson.build.
> >>> I propose arm64_armv8_linuxapp_gcc being the default config (for
> >>> armv8)
> >>> and creating arm64_generic_linuxapp_gcc for the generic build (for
> >>> distros).
> >>
> >> It should be inline with how distro guys build the image. I guess we
> >> dont want DPDK to be a exception.
> >
> > The machine option is specific to DPDK, so we can define it as we want.
> >
> >> Please check below thread and patch.
> >>
> >>
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmai
> >> ls.dpdk.org%2Farchives%2Fdev%2F2019-
> January%2F122676.html&amp;data=02
> >> %7C01%7Cyskoh%40mellanox.com%7C96576e66c4b6434b47ad08d67642
> be9a%7Ca65
> >>
> 2971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636826426371146146&a
> mp;sdata=
> >>
> MNMzpjs7e71l4vZAmoyqicpElp7UIFO48UuamggQWHQ%3D&amp;reserved=0
> >>
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpa
> >>
> tches.dpdk.org%2Fpatch%2F49477%2F&amp;data=02%7C01%7Cyskoh%40m
> ellanox
> >> .com%7C96576e66c4b6434b47ad08d67642be9a%7Ca652971c7d2e4d9ba
> 6a4d149256
> >>
> f461b%7C0%7C0%7C636826426371146146&amp;sdata=okIrz7Idc8t7nMbFkc
> RnjxZg
> >> 2wMn9ZTjqaTLlEXCnaU%3D&amp;reserved=0
> >>
> >> Debian folks are building like this for the _generic_ image.
> >> What ever works for every distros, I am fine with that.
> >>
> >> meson configure -Dmachine=default
> >> meson build
> >> cd build
> >> ninja
> >> ninja install
> >
> > I think we agree on the idea of having different configs for
> > unmodified A72 core and generic build working for all.
> > The remaining bits to discuss are:
> > 	- do we want to use the armv8 config for unmodified A72?
> > 	- what should be the name of the generic config?
> >
> > When digging more the config files in meson, I found this:
> >
> >
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmeso
> > nbuild.com%2FCross-compilation.html%23cross-file-
> locations&amp;data=02
> > %7C01%7Cyskoh%40mellanox.com%7C96576e66c4b6434b47ad08d67642b
> e9a%7Ca652
> >
> 971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636826426371146146&am
> p;sdata=J8
> > XiCovgwqxm8HHRCJ5bSUbx4yTHCO2YuZz2ryZJx8I%3D&amp;reserved=0
> > It says that distros or compilers should provide some config files.
> > It means we should check if some standard names are emerging and try
> > to follow the same naming, or even re-use existing config files.
> 
> I'll come up with a new patch based on the discussion here.
> A few things noted,
> - we still want it to be 128B for generic build
> - we at least agreed on changing it to 64B for A72
How will this be done? Will you add config/arm/arm64_bluefield_linuxapp_gcc?

> - As Qualcomm Centriq CPU has 128B cache line with A72, they should
>   create a profile in meson.build based on their impl_id.
Qualcomm is not A72 core. Can it not use RTE_CACHE_LINE_SIZE from 'flags_generic' in config/arm/meson.build?

> 
> I talked to Thomas and we'll shoot it for 19.05.
> 
> Thanks,
> Yongseok
  
Honnappa Nagarahalli Jan. 16, 2019, 2:02 a.m. UTC | #19
> > >>>>>> On Wed, 2019-01-09 at 10:22 +0000, Yongseok Koh wrote:
> > >>>>>>> On Jan 9, 2019, at 2:09 AM, Jerin Jacob Kollanukkaran wrote:
> > >>>>>>>> I think, I way forward is to add
> > >>>>>>>> config/arm/arm64_a72_linuxapp_gcc for meson. This config can
> > >>>>>>>> be used for all SoC with A72
> > >>>>>>>> armv8
> > >>>>>>>> implementation and may have sym link to specfific SoC to
> > >>>>>>>> avoid confusion to end users.
> > >>>>>>>
> > >>>>>>> Is config/arm/arm64_a72_linuxapp_gcc valid? Others have
> > >>>>>>
> > >>>>>> Yes. For cross compiling for A72.
> > >>>>>
> > >>>>> Any cross-compilation with meson requires a config file.
> > >>>>> The default Arm cross-compilation is done with
> > >>>>> 	config/arm/arm64_armv8_linuxapp_gcc
> > >>>>> which set implementor_id = 'generic'
> > >>>>>
> > >>>>> For native compilation, implementor_id is detected from
> > >>>>> /sys/devices/system/cpu/cpu0/regs/identification/midr_el1
> > >>>>>
> > >>>>> So each Arm machine needs 2 things:
> > >>>>> 	- a cross-compilation file
> > >>>>> 	- settings based on implementor_id in
> config/arm/meson.build
> > >>>>
> > >>>> Yes. config/arm/arm64_armv8_linuxapp_gcc sets the implementor_id
> > >>>> = 'generic' which assumed to generic across all the armv8 platform.
> > >>>> If tomorrow there is new core from ARM which A100 with armv8.2
> > >>>> specific we can not tune the generic params armv8.2 as it will
> > >>>> break other CPU.
> > >>>>
> > >>>>
> > >>>>>> Having not seperate IMPLEMENTOR ID is a chip design issue.
> > >>>>>
> > >>>>> No I don't think it's a design issue.
> > >>>>> If the Arm core has no modification, it does not need to be
> > >>>>> specially identified.
> > >>>>
> > >>>> Thats right. It does not need to be specially identified, then
> > >>>> should have default config is enough.
> > >>>>
> > >>>>
> > >>>>>> I think it can work around by creating
> > >>>>>> config/arm/arm64_<your_soc_name>_linuxapp_gcc
> > >>>>>> and build on x86 or arm64 through
> > >>>>>>
> > >>>>>> meson build --cross-file
> > >>>>>> config/arm/arm64_<your_soc_name>_linuxapp_gcc
> > >>>>>
> > >>>>> No, it is a real A72, so it should work with default settings.
> > >>>>>
> > >>>>> The only issue we have is that the default cache line size for
> > >>>>> Aarch64
> > >>>>> is set to 128 in config/arm/meson.build, and this is wrong.
> > >>>>> The default cache line is 64 bits.
> > >>>>
> > >>>> The cache line size as per ARM spec it is IMPLEMENTATION DEFINED.
> > >>>
> > >>> In A72 spec, it is said
> > >>> "Returns 0b010 to indicate that the cache line size is 64 bytes."
> > >>> But I guess we cannot say it is always true for all models.
> > >>> So let's assume there is no default.
> > >>
> > >> Please note, A72 is not armv8 spec. A72 is just an IMPLEMENTATION
> > >> of armv8.
> > >
> > > Yes, this my understanding. That's why I agree with you.
> > >
> > >>>> So no default there. So the default is something work on all
> > >>>> platforms.
> > >>>> Actually Cavium has machine with 64B and 128B CL and same image
> > >>>> should work on both for generic build.
> > >>>>
> > >>>>> This is already overriden for Cavium machines which have 128-bit
> > >>>>> cache lines.
> > >>>>> It may be needed to do the same change for other machines
> > >>>>> (Qualcomm?)
> > >>>>> having Arm core modified to 128-bit cache lines.
> > >>>>
> > >>>> Assume you meant 128B here.
> > >>>
> > >>> Yes, sorry I mixed bits and bytes :)
> > >>>
> > >>>> Building the image Naively(on 128B CL
> > >>>> machine) and cross compile (on x86) is not an issue.
> > >>>>
> > >>>>> The other concern is about running a generic Arm build.
> > >>>>
> > >>>> Yes. That's the ONLY concern.
> > >>>>
> > >>>>> Given 64-bit should be the default, generic builds will have
> > >>>>> this value.
> > >>>>> Is it a big issue for running generic 64-bit build on Cavium
> > >>>>> machines?
> > >>>>
> > >>>> Cavium has both 64B and 128B CL machines. So putting generic
> > >>>> form,
> > >>>>
> > >>>> You can run 128B configured image on 64B machine, It will waste
> > >>>> some memory not beyond that. Other way around will result in HW
> > >>>> misbehavior.
> > >>>> ie Running 64B CL image on 128B target.
> > >>>
> > >>> Indeed it is the main concern.
> > >>> Running DPDK tuned for 128 bytes on a core having 64 bytes cache
> > >>> line will result in lower performances. It is less an issue than
> > >>> HW misbehavior.
> > >>
> > >> Do you see performance issue or it more memory usage? It nothing do
> > >> with thread just of out curosity. Becase, our 64CL machine does
> > >> take more memory, performance seems to same for both. Note we are
> > >> using 512MB hugepage size.
> > >
> > > Yes, we see better performance with 64B cache line on Bluefield.
> > >
> > >>> If we agree to keep 128 bytes as generic cache line size for Arm,
> > >>> we need a way to get 64 bytes size for unmodified cores.
> > >>> In other words, the generic build settings must be different of
> > >>> the default settings.
> > >>
> > >> Please send a patch.
> > >>
> > >> If MIDR value is set to A72, we can set to 64B cache, no issue.
> > >>
> > >>> Please make a difference between default 'armv8' and 'generic'
> > >>> as implementor_id in config/arm/meson.build.
> > >>> I propose arm64_armv8_linuxapp_gcc being the default config (for
> > >>> armv8)
> > >>> and creating arm64_generic_linuxapp_gcc for the generic build (for
> > >>> distros).
> > >>
> > >> It should be inline with how distro guys build the image. I guess
> > >> we dont want DPDK to be a exception.
> > >
> > > The machine option is specific to DPDK, so we can define it as we want.
> > >
> > >> Please check below thread and patch.
> > >>
> > >>
> > https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmai
> > >> ls.dpdk.org%2Farchives%2Fdev%2F2019-
> > January%2F122676.html&amp;data=02
> > >> %7C01%7Cyskoh%40mellanox.com%7C96576e66c4b6434b47ad08d6764
> 2
> > be9a%7Ca65
> > >>
> >
> 2971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636826426371146146&a
> > mp;sdata=
> > >>
> >
> MNMzpjs7e71l4vZAmoyqicpElp7UIFO48UuamggQWHQ%3D&amp;reserved=0
> > >>
> > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpa
> > >>
> >
> tches.dpdk.org%2Fpatch%2F49477%2F&amp;data=02%7C01%7Cyskoh%40m
> > ellanox
> > >> .com%7C96576e66c4b6434b47ad08d67642be9a%7Ca652971c7d2e4d9
> ba
> > 6a4d149256
> > >>
> >
> f461b%7C0%7C0%7C636826426371146146&amp;sdata=okIrz7Idc8t7nMbFkc
> > RnjxZg
> > >> 2wMn9ZTjqaTLlEXCnaU%3D&amp;reserved=0
> > >>
> > >> Debian folks are building like this for the _generic_ image.
> > >> What ever works for every distros, I am fine with that.
> > >>
> > >> meson configure -Dmachine=default
> > >> meson build
> > >> cd build
> > >> ninja
> > >> ninja install
> > >
> > > I think we agree on the idea of having different configs for
> > > unmodified A72 core and generic build working for all.
> > > The remaining bits to discuss are:
> > > 	- do we want to use the armv8 config for unmodified A72?
> > > 	- what should be the name of the generic config?
> > >
> > > When digging more the config files in meson, I found this:
> > >
> > >
> >
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmeso
> > > nbuild.com%2FCross-compilation.html%23cross-file-
> > locations&amp;data=02
> > > %7C01%7Cyskoh%40mellanox.com%7C96576e66c4b6434b47ad08d6764
> 2b
> > e9a%7Ca652
> > >
> >
> 971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636826426371146146&am
> > p;sdata=J8
> > > XiCovgwqxm8HHRCJ5bSUbx4yTHCO2YuZz2ryZJx8I%3D&amp;reserved=0
> > > It says that distros or compilers should provide some config files.
> > > It means we should check if some standard names are emerging and try
> > > to follow the same naming, or even re-use existing config files.
> >
> > I'll come up with a new patch based on the discussion here.
> > A few things noted,
> > - we still want it to be 128B for generic build
> > - we at least agreed on changing it to 64B for A72
> How will this be done? Will you add
> config/arm/arm64_bluefield_linuxapp_gcc?
I asked this question as there was a proposal containing 'a72' in the file name. IMO, the file name should contain 'bluefield', not on a72.

> 
> > - As Qualcomm Centriq CPU has 128B cache line with A72, they should
> >   create a profile in meson.build based on their impl_id.
> Qualcomm is not A72 core. Can it not use RTE_CACHE_LINE_SIZE from
> 'flags_generic' in config/arm/meson.build?
> 
> >
> > I talked to Thomas and we'll shoot it for 19.05.
> >
> > Thanks,
> > Yongseok
  
Yongseok Koh Jan. 19, 2019, 7:09 a.m. UTC | #20
On Wed, Jan 16, 2019 at 02:02:26AM +0000, Honnappa Nagarahalli wrote:
> > > >>>>>> On Wed, 2019-01-09 at 10:22 +0000, Yongseok Koh wrote:
> > > >>>>>>> On Jan 9, 2019, at 2:09 AM, Jerin Jacob Kollanukkaran wrote:
> > > >>>>>>>> I think, I way forward is to add
> > > >>>>>>>> config/arm/arm64_a72_linuxapp_gcc for meson. This config can
> > > >>>>>>>> be used for all SoC with A72
> > > >>>>>>>> armv8
> > > >>>>>>>> implementation and may have sym link to specfific SoC to
> > > >>>>>>>> avoid confusion to end users.
> > > >>>>>>>
> > > >>>>>>> Is config/arm/arm64_a72_linuxapp_gcc valid? Others have
> > > >>>>>>
> > > >>>>>> Yes. For cross compiling for A72.
> > > >>>>>
> > > >>>>> Any cross-compilation with meson requires a config file.
> > > >>>>> The default Arm cross-compilation is done with
> > > >>>>> 	config/arm/arm64_armv8_linuxapp_gcc
> > > >>>>> which set implementor_id = 'generic'
> > > >>>>>
> > > >>>>> For native compilation, implementor_id is detected from
> > > >>>>> /sys/devices/system/cpu/cpu0/regs/identification/midr_el1
> > > >>>>>
> > > >>>>> So each Arm machine needs 2 things:
> > > >>>>> 	- a cross-compilation file
> > > >>>>> 	- settings based on implementor_id in
> > config/arm/meson.build
> > > >>>>
> > > >>>> Yes. config/arm/arm64_armv8_linuxapp_gcc sets the implementor_id
> > > >>>> = 'generic' which assumed to generic across all the armv8 platform.
> > > >>>> If tomorrow there is new core from ARM which A100 with armv8.2
> > > >>>> specific we can not tune the generic params armv8.2 as it will
> > > >>>> break other CPU.
> > > >>>>
> > > >>>>
> > > >>>>>> Having not seperate IMPLEMENTOR ID is a chip design issue.
> > > >>>>>
> > > >>>>> No I don't think it's a design issue.
> > > >>>>> If the Arm core has no modification, it does not need to be
> > > >>>>> specially identified.
> > > >>>>
> > > >>>> Thats right. It does not need to be specially identified, then
> > > >>>> should have default config is enough.
> > > >>>>
> > > >>>>
> > > >>>>>> I think it can work around by creating
> > > >>>>>> config/arm/arm64_<your_soc_name>_linuxapp_gcc
> > > >>>>>> and build on x86 or arm64 through
> > > >>>>>>
> > > >>>>>> meson build --cross-file
> > > >>>>>> config/arm/arm64_<your_soc_name>_linuxapp_gcc
> > > >>>>>
> > > >>>>> No, it is a real A72, so it should work with default settings.
> > > >>>>>
> > > >>>>> The only issue we have is that the default cache line size for
> > > >>>>> Aarch64
> > > >>>>> is set to 128 in config/arm/meson.build, and this is wrong.
> > > >>>>> The default cache line is 64 bits.
> > > >>>>
> > > >>>> The cache line size as per ARM spec it is IMPLEMENTATION DEFINED.
> > > >>>
> > > >>> In A72 spec, it is said
> > > >>> "Returns 0b010 to indicate that the cache line size is 64 bytes."
> > > >>> But I guess we cannot say it is always true for all models.
> > > >>> So let's assume there is no default.
> > > >>
> > > >> Please note, A72 is not armv8 spec. A72 is just an IMPLEMENTATION
> > > >> of armv8.
> > > >
> > > > Yes, this my understanding. That's why I agree with you.
> > > >
> > > >>>> So no default there. So the default is something work on all
> > > >>>> platforms.
> > > >>>> Actually Cavium has machine with 64B and 128B CL and same image
> > > >>>> should work on both for generic build.
> > > >>>>
> > > >>>>> This is already overriden for Cavium machines which have 128-bit
> > > >>>>> cache lines.
> > > >>>>> It may be needed to do the same change for other machines
> > > >>>>> (Qualcomm?)
> > > >>>>> having Arm core modified to 128-bit cache lines.
> > > >>>>
> > > >>>> Assume you meant 128B here.
> > > >>>
> > > >>> Yes, sorry I mixed bits and bytes :)
> > > >>>
> > > >>>> Building the image Naively(on 128B CL
> > > >>>> machine) and cross compile (on x86) is not an issue.
> > > >>>>
> > > >>>>> The other concern is about running a generic Arm build.
> > > >>>>
> > > >>>> Yes. That's the ONLY concern.
> > > >>>>
> > > >>>>> Given 64-bit should be the default, generic builds will have
> > > >>>>> this value.
> > > >>>>> Is it a big issue for running generic 64-bit build on Cavium
> > > >>>>> machines?
> > > >>>>
> > > >>>> Cavium has both 64B and 128B CL machines. So putting generic
> > > >>>> form,
> > > >>>>
> > > >>>> You can run 128B configured image on 64B machine, It will waste
> > > >>>> some memory not beyond that. Other way around will result in HW
> > > >>>> misbehavior.
> > > >>>> ie Running 64B CL image on 128B target.
> > > >>>
> > > >>> Indeed it is the main concern.
> > > >>> Running DPDK tuned for 128 bytes on a core having 64 bytes cache
> > > >>> line will result in lower performances. It is less an issue than
> > > >>> HW misbehavior.
> > > >>
> > > >> Do you see performance issue or it more memory usage? It nothing do
> > > >> with thread just of out curosity. Becase, our 64CL machine does
> > > >> take more memory, performance seems to same for both. Note we are
> > > >> using 512MB hugepage size.
> > > >
> > > > Yes, we see better performance with 64B cache line on Bluefield.
> > > >
> > > >>> If we agree to keep 128 bytes as generic cache line size for Arm,
> > > >>> we need a way to get 64 bytes size for unmodified cores.
> > > >>> In other words, the generic build settings must be different of
> > > >>> the default settings.
> > > >>
> > > >> Please send a patch.
> > > >>
> > > >> If MIDR value is set to A72, we can set to 64B cache, no issue.
> > > >>
> > > >>> Please make a difference between default 'armv8' and 'generic'
> > > >>> as implementor_id in config/arm/meson.build.
> > > >>> I propose arm64_armv8_linuxapp_gcc being the default config (for
> > > >>> armv8)
> > > >>> and creating arm64_generic_linuxapp_gcc for the generic build (for
> > > >>> distros).
> > > >>
> > > >> It should be inline with how distro guys build the image. I guess
> > > >> we dont want DPDK to be a exception.
> > > >
> > > > The machine option is specific to DPDK, so we can define it as we want.
> > > >
> > > >> Please check below thread and patch.
> > > >>
> > > >>
> > > https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmai
> > > >> ls.dpdk.org%2Farchives%2Fdev%2F2019-
> > > January%2F122676.html&amp;data=02
> > > >> %7C01%7Cyskoh%40mellanox.com%7C96576e66c4b6434b47ad08d6764
> > 2
> > > be9a%7Ca65
> > > >>
> > >
> > 2971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636826426371146146&a
> > > mp;sdata=
> > > >>
> > >
> > MNMzpjs7e71l4vZAmoyqicpElp7UIFO48UuamggQWHQ%3D&amp;reserved=0
> > > >>
> > > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpa
> > > >>
> > >
> > tches.dpdk.org%2Fpatch%2F49477%2F&amp;data=02%7C01%7Cyskoh%40m
> > > ellanox
> > > >> .com%7C96576e66c4b6434b47ad08d67642be9a%7Ca652971c7d2e4d9
> > ba
> > > 6a4d149256
> > > >>
> > >
> > f461b%7C0%7C0%7C636826426371146146&amp;sdata=okIrz7Idc8t7nMbFkc
> > > RnjxZg
> > > >> 2wMn9ZTjqaTLlEXCnaU%3D&amp;reserved=0
> > > >>
> > > >> Debian folks are building like this for the _generic_ image.
> > > >> What ever works for every distros, I am fine with that.
> > > >>
> > > >> meson configure -Dmachine=default
> > > >> meson build
> > > >> cd build
> > > >> ninja
> > > >> ninja install
> > > >
> > > > I think we agree on the idea of having different configs for
> > > > unmodified A72 core and generic build working for all.
> > > > The remaining bits to discuss are:
> > > > 	- do we want to use the armv8 config for unmodified A72?
> > > > 	- what should be the name of the generic config?
> > > >
> > > > When digging more the config files in meson, I found this:
> > > >
> > > >
> > >
> > https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmeso
> > > > nbuild.com%2FCross-compilation.html%23cross-file-
> > > locations&amp;data=02
> > > > %7C01%7Cyskoh%40mellanox.com%7C96576e66c4b6434b47ad08d6764
> > 2b
> > > e9a%7Ca652
> > > >
> > >
> > 971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636826426371146146&am
> > > p;sdata=J8
> > > > XiCovgwqxm8HHRCJ5bSUbx4yTHCO2YuZz2ryZJx8I%3D&amp;reserved=0
> > > > It says that distros or compilers should provide some config files.
> > > > It means we should check if some standard names are emerging and try
> > > > to follow the same naming, or even re-use existing config files.
> > >
> > > I'll come up with a new patch based on the discussion here.
> > > A few things noted,
> > > - we still want it to be 128B for generic build
> > > - we at least agreed on changing it to 64B for A72
> > How will this be done? Will you add
> > config/arm/arm64_bluefield_linuxapp_gcc?
> I asked this question as there was a proposal containing 'a72' in the file
> name. IMO, the file name should contain 'bluefield', not on a72.

Sorry for late reply. It's been buried for some reason. :-)

Nope, I won't create such a file. That's for cross-compiler AFAIK.
I'm thinking about changing meson.build. Currently, one CL size is applied to
all kinds of cores. Consequently, for armv8a, both 'default' and 'a72' have to
have the same CL size. And one more thing raised from ARM was that 'crypto' in
-march can't be a default.

> > > - As Qualcomm Centriq CPU has 128B cache line with A72, they should
> > >   create a profile in meson.build based on their impl_id.
> > Qualcomm is not A72 core. Can it not use RTE_CACHE_LINE_SIZE from
> > 'flags_generic' in config/arm/meson.build?

We can add flags_qualcomm for their processor.

Thanks,
Yongseok
  
Honnappa Nagarahalli Jan. 22, 2019, 6:51 p.m. UTC | #21
Removed tspeier@qti.qualcomm.com as the email address is not reachable.

> > > > I'll come up with a new patch based on the discussion here.
> > > > A few things noted,
> > > > - we still want it to be 128B for generic build
> > > > - we at least agreed on changing it to 64B for A72
> > > How will this be done? Will you add
> > > config/arm/arm64_bluefield_linuxapp_gcc?
> > I asked this question as there was a proposal containing 'a72' in the
> > file name. IMO, the file name should contain 'bluefield', not on a72.
> 
> Sorry for late reply. It's been buried for some reason. :-)
No problem

> 
> Nope, I won't create such a file. That's for cross-compiler AFAIK.
> I'm thinking about changing meson.build. Currently, one CL size is applied to
> all kinds of cores. Consequently, for armv8a, both 'default' and 'a72' have to
> have the same CL size. And one more thing raised from ARM was that
> 'crypto' in -march can't be a default.
Yes, crypto is optional. So, the distro builds (and any other builds for binary compatibility) should not expect crypto to be present by default.

> 
> > > > - As Qualcomm Centriq CPU has 128B cache line with A72, they should
> > > >   create a profile in meson.build based on their impl_id.
> > > Qualcomm is not A72 core. Can it not use RTE_CACHE_LINE_SIZE from
> > > 'flags_generic' in config/arm/meson.build?
> 
> We can add flags_qualcomm for their processor.
> 
> Thanks,
> Yongseok
  
Jerin Jacob Kollanukkaran Jan. 23, 2019, 8:56 a.m. UTC | #22
On Tue, 2019-01-22 at 18:51 +0000, Honnappa Nagarahalli wrote:
> External Email
> 
> Removed tspeier@qti.qualcomm.com as the email address is not
> reachable.
> 
> > > > > I'll come up with a new patch based on the discussion here.
> > > > > A few things noted,
> > > > > - we still want it to be 128B for generic build
> > > > > - we at least agreed on changing it to 64B for A72
> > > > How will this be done? Will you add
> > > > config/arm/arm64_bluefield_linuxapp_gcc?
> > > I asked this question as there was a proposal containing 'a72' in
> > > the
> > > file name. IMO, the file name should contain 'bluefield', not on
> > > a72.
> > 
> > Sorry for late reply. It's been buried for some reason. :-)
> No problem
> 
> > Nope, I won't create such a file. That's for cross-compiler AFAIK.
> > I'm thinking about changing meson.build. Currently, one CL size is
> > applied to
> > all kinds of cores. Consequently, for armv8a, both 'default' and
> > 'a72' have to
> > have the same CL size. And one more thing raised from ARM was that
> > 'crypto' in -march can't be a default.
> Yes, crypto is optional. So, the distro builds (and any other builds
> for binary compatibility) should not expect crypto to be present by
> default.

IMO, crypto case is different where DPDK code check at runtime to see
crypto instruction present in the given CPU before it uses any crypto
instructions. so IMO, There is no harm in building with crypto enabled
unlike other gcc flags.
  
Honnappa Nagarahalli Jan. 23, 2019, 4:24 p.m. UTC | #23
> > > > > > I'll come up with a new patch based on the discussion here.
> > > > > > A few things noted,
> > > > > > - we still want it to be 128B for generic build
> > > > > > - we at least agreed on changing it to 64B for A72
> > > > > How will this be done? Will you add
> > > > > config/arm/arm64_bluefield_linuxapp_gcc?
> > > > I asked this question as there was a proposal containing 'a72' in
> > > > the file name. IMO, the file name should contain 'bluefield', not
> > > > on a72.
> > >
> > > Sorry for late reply. It's been buried for some reason. :-)
> > No problem
> >
> > > Nope, I won't create such a file. That's for cross-compiler AFAIK.
> > > I'm thinking about changing meson.build. Currently, one CL size is
> > > applied to all kinds of cores. Consequently, for armv8a, both
> > > 'default' and 'a72' have to have the same CL size. And one more
> > > thing raised from ARM was that 'crypto' in -march can't be a
> > > default.
> > Yes, crypto is optional. So, the distro builds (and any other builds
> > for binary compatibility) should not expect crypto to be present by
> > default.
> 
> IMO, crypto case is different where DPDK code check at runtime to see crypto
> instruction present in the given CPU before it uses any crypto instructions. so
> IMO, There is no harm in building with crypto enabled unlike other gcc flags.
> 
Right now, DPDK does not boot on a platform with crypto disabled. Do you suggest we remove the run time check in DPDK?
  
Jerin Jacob Kollanukkaran Jan. 23, 2019, 5:19 p.m. UTC | #24
On Wed, 2019-01-23 at 16:24 +0000, Honnappa Nagarahalli wrote:
> > > > > > > I'll come up with a new patch based on the discussion
> > > > > > > here.
> > > > > > > A few things noted,
> > > > > > > - we still want it to be 128B for generic build
> > > > > > > - we at least agreed on changing it to 64B for A72
> > > > > > How will this be done? Will you add
> > > > > > config/arm/arm64_bluefield_linuxapp_gcc?
> > > > > I asked this question as there was a proposal containing
> > > > > 'a72' in
> > > > > the file name. IMO, the file name should contain 'bluefield',
> > > > > not
> > > > > on a72.
> > > > 
> > > > Sorry for late reply. It's been buried for some reason. :-)
> > > No problem
> > > 
> > > > Nope, I won't create such a file. That's for cross-compiler
> > > > AFAIK.
> > > > I'm thinking about changing meson.build. Currently, one CL size
> > > > is
> > > > applied to all kinds of cores. Consequently, for armv8a, both
> > > > 'default' and 'a72' have to have the same CL size. And one more
> > > > thing raised from ARM was that 'crypto' in -march can't be a
> > > > default.
> > > Yes, crypto is optional. So, the distro builds (and any other
> > > builds
> > > for binary compatibility) should not expect crypto to be present
> > > by
> > > default.
> > 
> > IMO, crypto case is different where DPDK code check at runtime to
> > see crypto
> > instruction present in the given CPU before it uses any crypto
> > instructions. so
> > IMO, There is no harm in building with crypto enabled unlike other
> > gcc flags.
> > 
> Right now, DPDK does not boot on a platform with crypto disabled. Do
> you suggest we remove the run time check in DPDK?

May be we need to introduce a framework to define mandatory cpu flags
and optional flags. Check only mandatory flags at bootup or some
abstraction where we can express above scenario like cryto flag case.



>
  

Patch

diff --git a/config/arm/meson.build b/config/arm/meson.build
index dae55d6b26..3af256a5ec 100644
--- a/config/arm/meson.build
+++ b/config/arm/meson.build
@@ -47,8 +47,7 @@  flags_common_default = [
 flags_generic = [
 	['RTE_MACHINE', '"armv8a"'],
 	['RTE_MAX_LCORE', 256],
-	['RTE_USE_C11_MEM_MODEL', true],
-	['RTE_CACHE_LINE_SIZE', 128]]
+	['RTE_USE_C11_MEM_MODEL', true]]
 flags_cavium = [
 	['RTE_MACHINE', '"thunderx"'],
 	['RTE_CACHE_LINE_SIZE', 128],
@@ -89,15 +88,19 @@  impl_dpaa2 = ['NXP DPAA2', flags_dpaa2, machine_args_generic]
 
 dpdk_conf.set('RTE_FORCE_INTRINSICS', 1)
 
+# In config/arm64_armv8_linuxapp_gcc, maximum available cache line size (128B)
+# in arm64 implementations is set by default for generic config. However,
+# setting 64B is preferable for meson build in order to support majority of CPUs
+# which don't have Implementor ID or Part Number programmed on chip.
+dpdk_conf.set('RTE_CACHE_LINE_SIZE', 64)
+
 if cc.sizeof('void *') != 8
-	dpdk_conf.set('RTE_CACHE_LINE_SIZE', 64)
 	dpdk_conf.set('RTE_ARCH_ARM', 1)
 	dpdk_conf.set('RTE_ARCH_ARMv7', 1)
 	# the minimum architecture supported, armv7-a, needs the following,
 	# mk/machine/armv7a/rte.vars.mk sets it too
 	machine_args += '-mfpu=neon'
 else
-	dpdk_conf.set('RTE_CACHE_LINE_SIZE', 128)
 	dpdk_conf.set('RTE_ARCH_ARM64', 1)
 	dpdk_conf.set('RTE_ARCH_64', 1)