diff mbox series

[2/6] meson: change default cache line size for cortex-a72

Message ID 20190412232451.30197-3-yskoh@mellanox.com (mailing list archive)
State Superseded, archived
Headers
Series build: fix build for arm64 |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Yongseok Koh April 12, 2019, 11:24 p.m. UTC 
  Per the email discussion [1], the default cache line size of armv8
cortex-a72 is changed to 64 bytes.

[1] https://mails.dpdk.org/archives/dev/2019-January/123218.html

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---
 config/arm/meson.build | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Jerin Jacob Kollanukkaran April 13, 2019, 6:43 a.m. UTC | #1 
> -----Original Message-----
> From: Yongseok Koh <yskoh@mellanox.com>
> Sent: Saturday, April 13, 2019 4:55 AM
> To: bruce.richardson@intel.com; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>; Pavan Nikhilesh Bhagavatula
> <pbhagavatula@marvell.com>; shahafs@mellanox.com
> Cc: dev@dpdk.org; thomas@monjalon.net; gavin.hu@arm.com;
> Honnappa.Nagarahalli@arm.com
> Subject: [EXT] [PATCH 2/6] meson: change default cache line size for cortex-a72
> 
> ----------------------------------------------------------------------
> Per the email discussion [1], the default cache line size of armv8
> cortex-a72 is changed to 64 bytes.

IMO, In git commit you remove the reference to specific discussion and
Update the reason correctly.


> 
> [1] https://mails.dpdk.org/archives/dev/2019-January/123218.html
> 
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> ---
>  config/arm/meson.build | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/config/arm/meson.build b/config/arm/meson.build index
> e00b894523..73c581948c 100644
> --- a/config/arm/meson.build
> +++ b/config/arm/meson.build
> @@ -51,6 +51,8 @@ flags_dpaa2 = [
>  	['RTE_MAX_LCORE', 16],
>  	['RTE_LIBRTE_DPAA2_USE_PHYS_IOVA', false]]  flags_default_extra = []
> +flags_cortex_a72_extra = [
> +	['RTE_CACHE_LINE_SIZE', 64]]
>  flags_thunderx_extra = [
>  	['RTE_MACHINE', '"thunderx"'],
>  	['RTE_USE_C11_MEM_MODEL', false]]
> @@ -73,7 +75,7 @@ machine_args_generic = [
>  	['0xd03', ['-mcpu=cortex-a53']],
>  	['0xd04', ['-mcpu=cortex-a35']],
>  	['0xd07', ['-mcpu=cortex-a57']],
> -	['0xd08', ['-mcpu=cortex-a72']],
> +	['0xd08', ['-mcpu=cortex-a72'], flags_cortex_a72_extra],
>  	['0xd09', ['-mcpu=cortex-a73']],
>  	['0xd0a', ['-mcpu=cortex-a75']]]

I think, flags_cortex_a72_extra() can be changed to flags_vendor_arm_extra or something similar
And update the following CPUs also not just cortex-a72.

	['0xd03', ['-mcpu=cortex-a53']],
	['0xd04', ['-mcpu=cortex-a35']],
	['0xd05', ['-mcpu=cortex-a55']],
	['0xd07', ['-mcpu=cortex-a57']],
	['0xd08', ['-mcpu=cortex-a72']],
	['0xd09', ['-mcpu=cortex-a73']],
	['0xd0a', ['-mcpu=cortex-a75']],
	['0xd0b', ['-mcpu=cortex-a76']],


> 
> --
> 2.21.0.196.g041f5ea
Honnappa Nagarahalli April 15, 2019, 4:35 a.m. UTC | #2 
> >
> > ----------------------------------------------------------------------
> > Per the email discussion [1], the default cache line size of armv8
> > cortex-a72 is changed to 64 bytes.
> 
> IMO, In git commit you remove the reference to specific discussion and
> Update the reason correctly.
> 
> 
> >
> > [1] https://mails.dpdk.org/archives/dev/2019-January/123218.html
> >
> > Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> > ---
> >  config/arm/meson.build | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/config/arm/meson.build b/config/arm/meson.build index
> > e00b894523..73c581948c 100644
> > --- a/config/arm/meson.build
> > +++ b/config/arm/meson.build
> > @@ -51,6 +51,8 @@ flags_dpaa2 = [
> >  	['RTE_MAX_LCORE', 16],
> >  	['RTE_LIBRTE_DPAA2_USE_PHYS_IOVA', false]]  flags_default_extra
> = []
> > +flags_cortex_a72_extra = [
> > +	['RTE_CACHE_LINE_SIZE', 64]]
> >  flags_thunderx_extra = [
Which tree does this patch apply to? I do not see the above line in master.

> >  	['RTE_MACHINE', '"thunderx"'],
> >  	['RTE_USE_C11_MEM_MODEL', false]]
> > @@ -73,7 +75,7 @@ machine_args_generic = [
> >  	['0xd03', ['-mcpu=cortex-a53']],
> >  	['0xd04', ['-mcpu=cortex-a35']],
> >  	['0xd07', ['-mcpu=cortex-a57']],
> > -	['0xd08', ['-mcpu=cortex-a72']],
> > +	['0xd08', ['-mcpu=cortex-a72'], flags_cortex_a72_extra],
> >  	['0xd09', ['-mcpu=cortex-a73']],
> >  	['0xd0a', ['-mcpu=cortex-a75']]]
> 
> I think, flags_cortex_a72_extra() can be changed to
> flags_vendor_arm_extra or something similar And update the following
> CPUs also not just cortex-a72.
> 
Why not add 'flags_arm' similar to flags_dpaa2/flag_cavium etc? All the listed Arm cores are 64B cache line size.

> 	['0xd03', ['-mcpu=cortex-a53']],
> 	['0xd04', ['-mcpu=cortex-a35']],
> 	['0xd05', ['-mcpu=cortex-a55']],
> 	['0xd07', ['-mcpu=cortex-a57']],
> 	['0xd08', ['-mcpu=cortex-a72']],
> 	['0xd09', ['-mcpu=cortex-a73']],
> 	['0xd0a', ['-mcpu=cortex-a75']],
> 	['0xd0b', ['-mcpu=cortex-a76']],
> 
> 
> >
> > --
> > 2.21.0.196.g041f5ea
Honnappa Nagarahalli April 15, 2019, 1:40 p.m. UTC | #3 
> 
> > >
> > > --------------------------------------------------------------------
> > > -- Per the email discussion [1], the default cache line size of
> > > armv8
> > > cortex-a72 is changed to 64 bytes.
> >
> > IMO, In git commit you remove the reference to specific discussion and
> > Update the reason correctly.
> >
> >
> > >
> > > [1] https://mails.dpdk.org/archives/dev/2019-January/123218.html
> > >
> > > Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> > > ---
> > >  config/arm/meson.build | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/config/arm/meson.build b/config/arm/meson.build index
> > > e00b894523..73c581948c 100644
> > > --- a/config/arm/meson.build
> > > +++ b/config/arm/meson.build
> > > @@ -51,6 +51,8 @@ flags_dpaa2 = [
> > >  	['RTE_MAX_LCORE', 16],
> > >  	['RTE_LIBRTE_DPAA2_USE_PHYS_IOVA', false]]  flags_default_extra
> > = []
> > > +flags_cortex_a72_extra = [
> > > +	['RTE_CACHE_LINE_SIZE', 64]]
> > >  flags_thunderx_extra = [
> Which tree does this patch apply to? I do not see the above line in master.
Please ignore this comment, I missed the dependency provided in 0/6

> 
> > >  	['RTE_MACHINE', '"thunderx"'],
> > >  	['RTE_USE_C11_MEM_MODEL', false]]
> > > @@ -73,7 +75,7 @@ machine_args_generic = [
> > >  	['0xd03', ['-mcpu=cortex-a53']],
> > >  	['0xd04', ['-mcpu=cortex-a35']],
> > >  	['0xd07', ['-mcpu=cortex-a57']],
> > > -	['0xd08', ['-mcpu=cortex-a72']],
> > > +	['0xd08', ['-mcpu=cortex-a72'], flags_cortex_a72_extra],
> > >  	['0xd09', ['-mcpu=cortex-a73']],
> > >  	['0xd0a', ['-mcpu=cortex-a75']]]
> >
> > I think, flags_cortex_a72_extra() can be changed to
> > flags_vendor_arm_extra or something similar And update the following
> > CPUs also not just cortex-a72.
> >
> Why not add 'flags_arm' similar to flags_dpaa2/flag_cavium etc? All the
> listed Arm cores are 64B cache line size.
Just to complete the thought, impl_0x41 can use 'flags_arm' instead of 'flags_generic'. IMO, current use of 'flags_generic' in impl_0x41 is incorrect.

> 
> > 	['0xd03', ['-mcpu=cortex-a53']],
> > 	['0xd04', ['-mcpu=cortex-a35']],
> > 	['0xd05', ['-mcpu=cortex-a55']],
> > 	['0xd07', ['-mcpu=cortex-a57']],
> > 	['0xd08', ['-mcpu=cortex-a72']],
> > 	['0xd09', ['-mcpu=cortex-a73']],
> > 	['0xd0a', ['-mcpu=cortex-a75']],
> > 	['0xd0b', ['-mcpu=cortex-a76']],
> >
> >
> > >
> > > --
> > > 2.21.0.196.g041f5ea
Yongseok Koh April 15, 2019, 8:40 p.m. UTC | #4 
> On Apr 15, 2019, at 6:40 AM, Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com> wrote:
> 
>> 
>>>> 
>>>> --------------------------------------------------------------------
>>>> -- Per the email discussion [1], the default cache line size of
>>>> armv8
>>>> cortex-a72 is changed to 64 bytes.
>>> 
>>> IMO, In git commit you remove the reference to specific discussion and
>>> Update the reason correctly.
>>> 
>>> 
>>>> 
>>>> [1] https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmails.dpdk.org%2Farchives%2Fdev%2F2019-January%2F123218.html&amp;data=02%7C01%7Cyskoh%40mellanox.com%7C4c0cdd9535c84c8dd3c008d6c1a7f5eb%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636909324474698429&amp;sdata=UJO2lBtnYWSs5ud8CsAL7oGXH571f6zGjrVmP2SRChw%3D&amp;reserved=0
>>>> 
>>>> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
>>>> ---
>>>> config/arm/meson.build | 4 +++-
>>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>> 
>>>> diff --git a/config/arm/meson.build b/config/arm/meson.build index
>>>> e00b894523..73c581948c 100644
>>>> --- a/config/arm/meson.build
>>>> +++ b/config/arm/meson.build
>>>> @@ -51,6 +51,8 @@ flags_dpaa2 = [
>>>> 	['RTE_MAX_LCORE', 16],
>>>> 	['RTE_LIBRTE_DPAA2_USE_PHYS_IOVA', false]]  flags_default_extra
>>> = []
>>>> +flags_cortex_a72_extra = [
>>>> +	['RTE_CACHE_LINE_SIZE', 64]]
>>>> flags_thunderx_extra = [
>> Which tree does this patch apply to? I do not see the above line in master.
> Please ignore this comment, I missed the dependency provided in 0/6
> 
>> 
>>>> 	['RTE_MACHINE', '"thunderx"'],
>>>> 	['RTE_USE_C11_MEM_MODEL', false]]
>>>> @@ -73,7 +75,7 @@ machine_args_generic = [
>>>> 	['0xd03', ['-mcpu=cortex-a53']],
>>>> 	['0xd04', ['-mcpu=cortex-a35']],
>>>> 	['0xd07', ['-mcpu=cortex-a57']],
>>>> -	['0xd08', ['-mcpu=cortex-a72']],
>>>> +	['0xd08', ['-mcpu=cortex-a72'], flags_cortex_a72_extra],
>>>> 	['0xd09', ['-mcpu=cortex-a73']],
>>>> 	['0xd0a', ['-mcpu=cortex-a75']]]
>>> 
>>> I think, flags_cortex_a72_extra() can be changed to
>>> flags_vendor_arm_extra or something similar And update the following
>>> CPUs also not just cortex-a72.
>>> 
>> Why not add 'flags_arm' similar to flags_dpaa2/flag_cavium etc? All the
>> listed Arm cores are 64B cache line size.

If so, I'd take your approach - flags_arm.
If we have an exception (CL size is 128 for some cpu) someday,
then we can add an extra flag for that.

> Just to complete the thought, impl_0x41 can use 'flags_arm' instead of 'flags_generic'. IMO, current use of 'flags_generic' in impl_0x41 is incorrect.
> 
>> 
>>> 	['0xd03', ['-mcpu=cortex-a53']],
>>> 	['0xd04', ['-mcpu=cortex-a35']],
>>> 	['0xd05', ['-mcpu=cortex-a55']],
>>> 	['0xd07', ['-mcpu=cortex-a57']],
>>> 	['0xd08', ['-mcpu=cortex-a72']],
>>> 	['0xd09', ['-mcpu=cortex-a73']],
>>> 	['0xd0a', ['-mcpu=cortex-a75']],
>>> 	['0xd0b', ['-mcpu=cortex-a76']],
>>> 
>>> 
>>>> 
>>>> --
>>>> 2.21.0.196.g041f5ea
>
Honnappa Nagarahalli April 15, 2019, 8:44 p.m. UTC | #5 
> >
> >>
> >>>>
> >>>> -------------------------------------------------------------------
> >>>> -
> >>>> -- Per the email discussion [1], the default cache line size of
> >>>> armv8
> >>>> cortex-a72 is changed to 64 bytes.
> >>>
> >>> IMO, In git commit you remove the reference to specific discussion
> >>> and Update the reason correctly.
> >>>
> >>>
> >>>>
> >>>> [1]
> >>>>
> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fm
> >>>> ails.dpdk.org%2Farchives%2Fdev%2F2019-
> January%2F123218.html&amp;dat
> >>>>
> a=02%7C01%7Cyskoh%40mellanox.com%7C4c0cdd9535c84c8dd3c008d6c1a
> 7f5eb
> >>>> %7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C6369093244
> 74698429&am
> >>>>
> p;sdata=UJO2lBtnYWSs5ud8CsAL7oGXH571f6zGjrVmP2SRChw%3D&amp;re
> served
> >>>> =0
> >>>>
> >>>> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> >>>> ---
> >>>> config/arm/meson.build | 4 +++-
> >>>> 1 file changed, 3 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/config/arm/meson.build b/config/arm/meson.build index
> >>>> e00b894523..73c581948c 100644
> >>>> --- a/config/arm/meson.build
> >>>> +++ b/config/arm/meson.build
> >>>> @@ -51,6 +51,8 @@ flags_dpaa2 = [
> >>>> 	['RTE_MAX_LCORE', 16],
> >>>> 	['RTE_LIBRTE_DPAA2_USE_PHYS_IOVA', false]]  flags_default_extra
> >>> = []
> >>>> +flags_cortex_a72_extra = [
> >>>> +	['RTE_CACHE_LINE_SIZE', 64]]
> >>>> flags_thunderx_extra = [
> >> Which tree does this patch apply to? I do not see the above line in
> master.
> > Please ignore this comment, I missed the dependency provided in 0/6
> >
> >>
> >>>> 	['RTE_MACHINE', '"thunderx"'],
> >>>> 	['RTE_USE_C11_MEM_MODEL', false]]
> >>>> @@ -73,7 +75,7 @@ machine_args_generic = [
> >>>> 	['0xd03', ['-mcpu=cortex-a53']],
> >>>> 	['0xd04', ['-mcpu=cortex-a35']],
> >>>> 	['0xd07', ['-mcpu=cortex-a57']],
> >>>> -	['0xd08', ['-mcpu=cortex-a72']],
> >>>> +	['0xd08', ['-mcpu=cortex-a72'], flags_cortex_a72_extra],
> >>>> 	['0xd09', ['-mcpu=cortex-a73']],
> >>>> 	['0xd0a', ['-mcpu=cortex-a75']]]
> >>>
> >>> I think, flags_cortex_a72_extra() can be changed to
> >>> flags_vendor_arm_extra or something similar And update the
> following
> >>> CPUs also not just cortex-a72.
> >>>
> >> Why not add 'flags_arm' similar to flags_dpaa2/flag_cavium etc? All
> >> the listed Arm cores are 64B cache line size.
> 
> If so, I'd take your approach - flags_arm.
> If we have an exception (CL size is 128 for some cpu) someday, then we
> can add an extra flag for that.
> 
Agree. I see the likelihood to be slim given the list of CPUs with 64B

> > Just to complete the thought, impl_0x41 can use 'flags_arm' instead of
> 'flags_generic'. IMO, current use of 'flags_generic' in impl_0x41 is incorrect.
> >
> >>
> >>> 	['0xd03', ['-mcpu=cortex-a53']],
> >>> 	['0xd04', ['-mcpu=cortex-a35']],
> >>> 	['0xd05', ['-mcpu=cortex-a55']],
> >>> 	['0xd07', ['-mcpu=cortex-a57']],
> >>> 	['0xd08', ['-mcpu=cortex-a72']],
> >>> 	['0xd09', ['-mcpu=cortex-a73']],
> >>> 	['0xd0a', ['-mcpu=cortex-a75']],
> >>> 	['0xd0b', ['-mcpu=cortex-a76']],
> >>>
> >>>
> >>>>
> >>>> --
> >>>> 2.21.0.196.g041f5ea
> >
diff mbox series

Patch

diff --git a/config/arm/meson.build b/config/arm/meson.build
index e00b894523..73c581948c 100644
--- a/config/arm/meson.build
+++ b/config/arm/meson.build
@@ -51,6 +51,8 @@  flags_dpaa2 = [
 	['RTE_MAX_LCORE', 16],
 	['RTE_LIBRTE_DPAA2_USE_PHYS_IOVA', false]]
 flags_default_extra = []
+flags_cortex_a72_extra = [
+	['RTE_CACHE_LINE_SIZE', 64]]
 flags_thunderx_extra = [
 	['RTE_MACHINE', '"thunderx"'],
 	['RTE_USE_C11_MEM_MODEL', false]]
@@ -73,7 +75,7 @@  machine_args_generic = [
 	['0xd03', ['-mcpu=cortex-a53']],
 	['0xd04', ['-mcpu=cortex-a35']],
 	['0xd07', ['-mcpu=cortex-a57']],
-	['0xd08', ['-mcpu=cortex-a72']],
+	['0xd08', ['-mcpu=cortex-a72'], flags_cortex_a72_extra],
 	['0xd09', ['-mcpu=cortex-a73']],
 	['0xd0a', ['-mcpu=cortex-a75']]]