On 5/9/2023 6:55 AM, Ruifeng Wang wrote: >> -----Original Message----- >> From: Ferruh Yigit <ferruh.yi...@amd.com> >> Sent: Monday, May 8, 2023 7:27 PM >> To: Gupta, Nipun <nipun.gu...@amd.com>; dev@dpdk.org; tho...@monjalon.net; >> david.march...@redhat.com; Honnappa Nagarahalli >> <honnappa.nagaraha...@arm.com>; Ruifeng >> Wang <ruifeng.w...@arm.com> >> Cc: Anand, Harpreet <harpreet.an...@amd.com>; Agarwal, Nikhil >> <nikhil.agar...@amd.com> >> Subject: Re: [PATCH v3 5/5] config/arm: add AMD CDX >> >> On 5/8/2023 11:24 AM, Gupta, Nipun wrote: >>> >>> >>>> -----Original Message----- >>>> From: Yigit, Ferruh <ferruh.yi...@amd.com> >>>> Sent: Thursday, May 4, 2023 8:59 PM >>>> To: Gupta, Nipun <nipun.gu...@amd.com>; dev@dpdk.org; >>>> tho...@monjalon.net; david.march...@redhat.com >>>> Cc: Anand, Harpreet <harpreet.an...@amd.com>; Agarwal, Nikhil >>>> <nikhil.agar...@amd.com> >>>> Subject: Re: [PATCH v3 5/5] config/arm: add AMD CDX >>>> >>>> On 4/21/2023 3:54 PM, Nipun Gupta wrote: >>>>> Adding support for AMD CDX devices >>>>> >>>>> Signed-off-by: Nipun Gupta <nipun.gu...@amd.com> >>>>> --- >>>>> config/arm/arm64_cdx_linux_gcc | 17 +++++++++++++++++ >>>>> config/arm/meson.build | 14 ++++++++++++++ >>>>> 2 files changed, 31 insertions(+) >>>>> create mode 100644 config/arm/arm64_cdx_linux_gcc >>>>> >>>>> diff --git a/config/arm/arm64_cdx_linux_gcc >>>> b/config/arm/arm64_cdx_linux_gcc >>>>> new file mode 100644 >>>>> index 0000000000..8e6d619dae >>>>> --- /dev/null >>>>> +++ b/config/arm/arm64_cdx_linux_gcc >>>>> @@ -0,0 +1,17 @@ >>>>> +[binaries] >>>>> +c = ['ccache', 'aarch64-linux-gnu-gcc'] cpp = ['ccache', >>>>> +'aarch64-linux-gnu-g++'] ar = 'aarch64-linux-gnu-ar' >>>>> +as = 'aarch64-linux-gnu-as' >>>>> +strip = 'aarch64-linux-gnu-strip' >>>>> +pkgconfig = 'aarch64-linux-gnu-pkg-config' >>>>> +pcap-config = '' >>>>> + >>>>> +[host_machine] >>>>> +system = 'linux' >>>>> +cpu_family = 'aarch64' >>>>> +cpu = 'armv8-a' >>>>> +endian = 'little' >>>>> + >>>>> +[properties] >>>>> +platform = 'cdx' >>>>> diff --git a/config/arm/meson.build b/config/arm/meson.build index >>>>> 5213434ca4..39b8929534 100644 >>>>> --- a/config/arm/meson.build >>>>> +++ b/config/arm/meson.build >>>>> @@ -305,6 +305,18 @@ soc_bluefield = { >>>>> 'numa': false >>>>> } >>>>> >>>>> +soc_cdx = { >>>>> + 'description': 'AMD CDX', >>>>> + 'implementer': '0x41', >>>>> + 'part_number': '0xd42', >>>>> + 'flags': [ >>>>> + ['RTE_MACHINE', '"cdx"'], >>>>> + ['RTE_MAX_LCORE', 16], >>>>> + ['RTE_MAX_NUMA_NODES', 1] >>>>> + ], >>>>> + 'numa': false >>>>> +} >>>> >>>> Hi Nipun, >>>> >>>> Why we need a new arm platform/config? Is it because of above flags? >>>> If it can work with default values, I think we can drop this patch. >>> >>> Hi Ferruh, >>> >>> CDX driver works with generic ARM compilation too (arm64_armv8_linux_gcc). >>> >>> The versal platforms supporting CDX have A78 cores, and adding this >>> cdx config Helps to provide gcc option "march= armv8.4-a" which is for >>> implementer "0xd42" (ARM cortex A78 cores)., whereas for generic ARM >>> compilation "march= armv8-a". >>> >>> Maybe ARM guys can provide more information regarding if there is any >>> impact on using generic architecture flag (i.e. march= armv8a) on A78 cores. >>> >> >> Hi Honnappa, Ruifeng, >> >> Can you please support on this question, what is the difference of 'march= >> armv8-a' flag >> (comparing march= armv8a)? >> Should we consider adding an arm config file to support this flag? > > I see there is a new version without change to config file. > FWIW, native build is fine without this change. Because the specific > (implementer, part number) flags > are already in place. What enabled by this change are options for soc build > (-Dplatform=cdx) and > cross-build (--cross-file arm64_cdx_linux_gcc). >
Hi Ruifeng, Honnappa, Config file will come as standalone patch, it only separated from this set. And config file is required mainly for '--march=armv8.4-a' parameter. There are multiple configs using the same parameter, is it a good option to create a common config for 'armv8.4-a', similar to 'generic' one? Or is it preferred that each SoC adding its own config, as done in this patch? Thanks, ferruh