Re: [dpdk-dev] [EXT] [PATCH 2/6] meson: change default cache line size for cortex-a72

[Yongseok Koh <yskoh@mellanox.com>]

> 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
>