[dpdk-dev,v3,6/6] test: add checks for cpu flags on armv8

Message ID 20151030161106.4657232.16920.465@rehivetech.com (mailing list archive)
State Not Applicable, archived
Headers

Commit Message

Jan Viktorin Oct. 30, 2015, 4:11 p.m. UTC
  Hmm, I see. It's good to fix this in the generated e-mails between format-patch and send-email calls. I always review those to be sure they meet my expectations ;).

Anyway, it is not clear, what has changed in the v3. Just the rte_cycles? You should explain that at least in the 0000 patch. Better to keep some history in each single commit (are there any rules in dpdk for this? Just look how they do in kernel).

I'll test the patchset in qemu anyway... so will probably send tested-by.

I've put this conversation to mailing list as I cannot see any reason why it is not CC'd there...

Jan Viktorin
RehiveTech
Sent from a mobile device
  Původní zpráva  
Od: Hunt, David
Odesláno: pátek, 30. října 2015 15:07
Komu: Jan Viktorin
Předmět: Fwd: [PATCH v3 6/6] test: add checks for cpu flags on armv8

Jan,
I had gone to the trouble of adding a "Reviewed-by" line in all the 
commit messages for each patch in the patch set, as well as addressing 
the comment about the armv8 files being in the arm dir.
However, the 'git format-patch' seems to have stripped out the
"Reviewed-by" line for some reason.
If you are happy with the latest patch set, could you reply and maybe 
say something like "series Reviewed-by..."?
Thanks for your help in this.
Regards,
Dave.



-------- Forwarded Message --------
Subject: [PATCH v3 6/6] test: add checks for cpu flags on armv8
Date: Fri, 30 Oct 2015 13:47:06 +0000
From: David Hunt <david.hunt@intel.com>
To: david.hunt@intel.com

Signed-off-by: David Hunt <david.hunt@intel.com>
---
app/test/test_cpuflags.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
  

Comments

Thomas Monjalon Oct. 30, 2015, 4:16 p.m. UTC | #1
2015-10-30 17:11, Jan Viktorin:
> Anyway, it is not clear, what has changed in the v3. Just the rte_cycles?
> You should explain that at least in the 0000 patch.
> Better to keep some history in each single commit (are there any rules in
> dpdk for this? Just look how they do in kernel).

The rule is to help reviewers ;)
History in the cover letter is good.
If there are also some history in each patch, it's better.
  
Hunt, David Oct. 30, 2015, 4:28 p.m. UTC | #2
On 30/10/2015 16:11, Jan Viktorin wrote:
> Hmm, I see. It's good to fix this in the generated e-mails between format-patch
 > and send-email calls. I always review those to be sure they meet my 
expectations ;).
> Anyway, it is not clear, what has changed in the v3. Just the rte_cycles?
> You should explain that at least in the 0000 patch. Better to keep some history
> in each single commit (are there any rules in dpdk for this? Just look how they do in kernel).
--snip--

Sure, I'll keep that in mind for the next time. A list of changes for 
each revision, and also changes in each patch in the patch set. As 
Thomas says - whatever helps the reviewer :)

For the moment there probably isn't a need to release a new patch set 
for these comments, so I'll just list them here:
1. v3 has just the additional comment in one of the patches to say that 
the armv8 header files are in the 'arm' include directory.
2. The rte_cycles is unchanged, the CONFIG_ is not needed.

If there is a need to post another patch set I'll include the change 
notes. Otherwise do we all think that the patch is there (or there 
abouts)? :)

Regards,
Dave.
  
Jerin Jacob Nov. 2, 2015, 6:32 a.m. UTC | #3
On Fri, Oct 30, 2015 at 04:28:25PM +0000, Hunt, David wrote:
> On 30/10/2015 16:11, Jan Viktorin wrote:
> >Hmm, I see. It's good to fix this in the generated e-mails between format-patch
> > and send-email calls. I always review those to be sure they meet my
> expectations ;).
> >Anyway, it is not clear, what has changed in the v3. Just the rte_cycles?
> >You should explain that at least in the 0000 patch. Better to keep some history
> >in each single commit (are there any rules in dpdk for this? Just look how they do in kernel).
> --snip--
> 
> Sure, I'll keep that in mind for the next time. A list of changes for each
> revision, and also changes in each patch in the patch set. As Thomas says -
> whatever helps the reviewer :)
> 
> For the moment there probably isn't a need to release a new patch set for
> these comments, so I'll just list them here:
> 1. v3 has just the additional comment in one of the patches to say that the
> armv8 header files are in the 'arm' include directory.
> 2. The rte_cycles is unchanged, the CONFIG_ is not needed.
> 
> If there is a need to post another patch set I'll include the change notes.
> Otherwise do we all think that the patch is there (or there abouts)? :)

Hi Jan and Dave,

I have reviewed your patches for arm[64] support. Please check the
review comments.

Cavium would like to contribute on armv8 port and remaining libraries
(ACL, LPM, HASH) implementation for armv8. Currently i am re-basing
our ACL,HASH libraries implementation based on existing patches.
Happy to work with you guys to have full fledged armv8 support for DPDK.

Jerin


other query on rte_cpu_get_flag_enabled for armv8,
I have tried to run the existing patches on armv8-thunderX platform.
But there application start failure due to mismatch in
rte_cpu_get_flag_enabled() encoding.

In my platform rte_cpu_get_flag_enabled() works based on
AT_HWCAP with following values[1] which different from
existing lib/librte_eal/common/include/arch/arm/rte_cpuflags.h

[1]http://lxr.free-electrons.com/source/arch/arm64/include/uapi/asm/hwcap.h

In order to debug this, Could provide the following 
values in tested armv8 platform. Look like its running 32bit compatible 
mode in your environment


root@arm64:/export/dpdk-arm64# LD_SHOW_AUXV=1 sleep 1000
AT_SYSINFO_EHDR: 0x3ff859f0000
AT_??? (0x26): 0x430f0a10
AT_HWCAP:        fb
AT_PAGESZ:       65536
AT_CLKTCK:       100
AT_PHDR:         0x400040
AT_PHENT:        56
AT_PHNUM:        7
AT_BASE:         0x3ff85a00000
AT_FLAGS:        0x0
AT_ENTRY:        0x401900
AT_UID:          0
AT_EUID:         0
AT_GID:          0
AT_EGID:         0
AT_SECURE:       0
AT_RANDOM:       0x3ffef1c7988
AT_EXECFN:       /bin/sleep
AT_PLATFORM:     aarch64

root@arm64:/export/dpdk-arm64# zcat /proc/config.gz  | grep CONFIG_COMPAT
# CONFIG_COMPAT_BRK is not set
CONFIG_COMPAT_BINFMT_ELF=y
CONFIG_COMPAT=y
CONFIG_COMPAT_NETLINK_MESSAGES=y


root@arm64:/export/dpdk-arm64# cat /proc/cpuinfo
Processor       : AArch64 Processor rev 0 (aarch64)
processor       : 0
processor       : 1
processor       : 2
processor       : 3
processor       : 4
processor       : 5
processor       : 6
processor       : 7
processor       : 8
processor       : 9
processor       : 10
processor       : 11
processor       : 12
processor       : 13
processor       : 14
processor       : 15
processor       : 16
processor       : 17
processor       : 18
processor       : 19
processor       : 20
processor       : 21
processor       : 22
processor       : 23
processor       : 24
processor       : 25
processor       : 26
processor       : 27
processor       : 28
processor       : 29
processor       : 30
processor       : 31
processor       : 32
processor       : 33
processor       : 34
processor       : 35
processor       : 36
processor       : 37
processor       : 38
processor       : 39
processor       : 40
processor       : 41
processor       : 42
processor       : 43
processor       : 44
processor       : 45
processor       : 46
processor       : 47
Features        : fp asimd aes pmull sha1 sha2 crc32
CPU implementer : 0x43
CPU architecture: AArch64
CPU variant     : 0x0
CPU part        : 0x0a1
CPU revision    : 0




> 
> Regards,
> Dave.
>
  
Hunt, David Nov. 2, 2015, 10:47 a.m. UTC | #4
On 02/11/2015 06:32, Jerin Jacob wrote:
> On Fri, Oct 30, 2015 at 04:28:25PM +0000, Hunt, David wrote:

--snip--

>
> Hi Jan and Dave,
>
> I have reviewed your patches for arm[64] support. Please check the
> review comments.

Hi Jerin,

I'm looking at the comments now, and working on getting the suggested 
changes merged into the patch-set.

> Cavium would like to contribute on armv8 port and remaining libraries
> (ACL, LPM, HASH) implementation for armv8. Currently i am re-basing
> our ACL,HASH libraries implementation based on existing patches.
> Happy to work with you guys to have full fledged armv8 support for DPDK.
>
> Jerin

Thanks for that, it's good news indeed.

> other query on rte_cpu_get_flag_enabled for armv8,
> I have tried to run the existing patches on armv8-thunderX platform.
> But there application start failure due to mismatch in
> rte_cpu_get_flag_enabled() encoding.
>
> In my platform rte_cpu_get_flag_enabled() works based on
> AT_HWCAP with following values[1] which different from
> existing lib/librte_eal/common/include/arch/arm/rte_cpuflags.h
>
> [1]http://lxr.free-electrons.com/source/arch/arm64/include/uapi/asm/hwcap.h
>
> In order to debug this, Could provide the following
> values in tested armv8 platform. Look like its running 32bit compatible
> mode in your environment

I'm using a Gigabyte MP30AR0 motherboard with an 8-core X-Gene, Running 
a 4.3.0-rc6 kernel.
Here's the information on the cpu_flags issue you requested:

> AT_SYSINFO_EHDR: 0x3ff859f0000
> AT_??? (0x26): 0x430f0a10
> AT_HWCAP:        fb
> AT_PAGESZ:       65536
> AT_CLKTCK:       100
> AT_PHDR:         0x400040
> AT_PHENT:        56
> AT_PHNUM:        7
> AT_BASE:         0x3ff85a00000
> AT_FLAGS:        0x0
> AT_ENTRY:        0x401900
> AT_UID:          0
> AT_EUID:         0
> AT_GID:          0
> AT_EGID:         0
> AT_SECURE:       0
> AT_RANDOM:       0x3ffef1c7988
> AT_EXECFN:       /bin/sleep
> AT_PLATFORM:     aarch64

root@mp30ar0:~# LD_SHOW_AUXV=1 sleep 1000
AT_SYSINFO_EHDR: 0x7f7956d000
AT_HWCAP:        7
AT_PAGESZ:       4096
AT_CLKTCK:       100
AT_PHDR:         0x400040
AT_PHENT:        56
AT_PHNUM:        7
AT_BASE:         0x7f79543000
AT_FLAGS:        0x0
AT_ENTRY:        0x401900
AT_UID:          0
AT_EUID:         0
AT_GID:          0
AT_EGID:         0
AT_SECURE:       0
AT_RANDOM:       0x7ffcaf2e48
AT_EXECFN:       /bin/sleep
AT_PLATFORM:     aarch64

> root@arm64:/export/dpdk-arm64# zcat /proc/config.gz  | grep CONFIG_COMPAT
> # CONFIG_COMPAT_BRK is not set
> CONFIG_COMPAT_BINFMT_ELF=y
> CONFIG_COMPAT=y
> CONFIG_COMPAT_NETLINK_MESSAGES=y

root@mp30ar0:~# zcat /proc/config.gz  | grep CONFIG_COMPAT
# CONFIG_COMPAT_BRK is not set
CONFIG_COMPAT_OLD_SIGACTION=y
CONFIG_COMPAT_BINFMT_ELF=y
CONFIG_COMPAT=y


> root@arm64:/export/dpdk-arm64# cat /proc/cpuinfo
> Processor       : AArch64 Processor rev 0 (aarch64)
> processor       : 0
> processor       : 1
--snip--
> processor       : 46
> processor       : 47
> Features        : fp asimd aes pmull sha1 sha2 crc32
> CPU implementer : 0x43
> CPU architecture: AArch64
> CPU variant     : 0x0
> CPU part        : 0x0a1
> CPU revision    : 0

root@mp30ar0:~# cat /proc/cpuinfo
processor       : 0
Features        : fp asimd evtstrm
CPU implementer : 0x50
CPU architecture: 8
CPU variant     : 0x0
CPU part        : 0x000
CPU revision    : 1

processor       : 1
Features        : fp asimd evtstrm
CPU implementer : 0x50
CPU architecture: 8
CPU variant     : 0x0
CPU part        : 0x000
CPU revision    : 1

processor       : 2
Features        : fp asimd evtstrm
CPU implementer : 0x50
CPU architecture: 8
CPU variant     : 0x0
CPU part        : 0x000
CPU revision    : 1

processor       : 3
Features        : fp asimd evtstrm
CPU implementer : 0x50
CPU architecture: 8
CPU variant     : 0x0
CPU part        : 0x000
CPU revision    : 1

processor       : 4
Features        : fp asimd evtstrm
CPU implementer : 0x50
CPU architecture: 8
CPU variant     : 0x0
CPU part        : 0x000
CPU revision    : 1

processor       : 5
Features        : fp asimd evtstrm
CPU implementer : 0x50
CPU architecture: 8
CPU variant     : 0x0
CPU part        : 0x000
CPU revision    : 1

processor       : 6
Features        : fp asimd evtstrm
CPU implementer : 0x50
CPU architecture: 8
CPU variant     : 0x0
CPU part        : 0x000
CPU revision    : 1

processor       : 7
Features        : fp asimd evtstrm
CPU implementer : 0x50
CPU architecture: 8
CPU variant     : 0x0
CPU part        : 0x000
CPU revision    : 1

root@mp30ar0:~#

Hope this helps.

Regards,
Dave.
  
Jerin Jacob Nov. 2, 2015, 1:17 p.m. UTC | #5
On Mon, Nov 02, 2015 at 10:47:53AM +0000, Hunt, David wrote:
> On 02/11/2015 06:32, Jerin Jacob wrote:
> >On Fri, Oct 30, 2015 at 04:28:25PM +0000, Hunt, David wrote:
>
> --snip--
>
> >
> >Hi Jan and Dave,
> >
> >I have reviewed your patches for arm[64] support. Please check the
> >review comments.
>
> Hi Jerin,
>
> I'm looking at the comments now, and working on getting the suggested
> changes merged into the patch-set.
>
> >Cavium would like to contribute on armv8 port and remaining libraries
> >(ACL, LPM, HASH) implementation for armv8. Currently i am re-basing
> >our ACL,HASH libraries implementation based on existing patches.
> >Happy to work with you guys to have full fledged armv8 support for DPDK.
> >
> >Jerin
>
> Thanks for that, it's good news indeed.
>
> >other query on rte_cpu_get_flag_enabled for armv8,
> >I have tried to run the existing patches on armv8-thunderX platform.
> >But there application start failure due to mismatch in
> >rte_cpu_get_flag_enabled() encoding.
> >
> >In my platform rte_cpu_get_flag_enabled() works based on
> >AT_HWCAP with following values[1] which different from
> >existing lib/librte_eal/common/include/arch/arm/rte_cpuflags.h
> >
> >[1]http://lxr.free-electrons.com/source/arch/arm64/include/uapi/asm/hwcap.h
> >
> >In order to debug this, Could provide the following
> >values in tested armv8 platform. Look like its running 32bit compatible
> >mode in your environment
>
> I'm using a Gigabyte MP30AR0 motherboard with an 8-core X-Gene, Running a
> 4.3.0-rc6 kernel.
> Here's the information on the cpu_flags issue you requested:
>
> >AT_SYSINFO_EHDR: 0x3ff859f0000
> >AT_??? (0x26): 0x430f0a10
> >AT_HWCAP:        fb
> >AT_PAGESZ:       65536
> >AT_CLKTCK:       100
> >AT_PHDR:         0x400040
> >AT_PHENT:        56
> >AT_PHNUM:        7
> >AT_BASE:         0x3ff85a00000
> >AT_FLAGS:        0x0
> >AT_ENTRY:        0x401900
> >AT_UID:          0
> >AT_EUID:         0
> >AT_GID:          0
> >AT_EGID:         0
> >AT_SECURE:       0
> >AT_RANDOM:       0x3ffef1c7988
> >AT_EXECFN:       /bin/sleep
> >AT_PLATFORM:     aarch64
>
> root@mp30ar0:~# LD_SHOW_AUXV=1 sleep 1000
> AT_SYSINFO_EHDR: 0x7f7956d000
> AT_HWCAP:        7
> AT_PAGESZ:       4096
> AT_CLKTCK:       100
> AT_PHDR:         0x400040
> AT_PHENT:        56
> AT_PHNUM:        7
> AT_BASE:         0x7f79543000
> AT_FLAGS:        0x0
> AT_ENTRY:        0x401900
> AT_UID:          0
> AT_EUID:         0
> AT_GID:          0
> AT_EGID:         0
> AT_SECURE:       0
> AT_RANDOM:       0x7ffcaf2e48
> AT_EXECFN:       /bin/sleep
> AT_PLATFORM:     aarch64
>

If am not wrong existing  rte_cpu_get_flag_enabled() implementation
should be broken in your platform also for arm64. as I could see only AT_HWCAP
not AT_HWCAP2 and AT_HWCAP is 0x7 that means your platform also
follows

http://lxr.free-electrons.com/source/arch/arm64/include/uapi/asm/hwcap.h

and the implmentation is

FEAT_DEF(SWP,       0x00000001, 0, REG_HWCAP,  0) // not correct for arm64
FEAT_DEF(HALF,      0x00000001, 0, REG_HWCAP,  1) // not correct for arm64
FEAT_DEF(THUMB,     0x00000001, 0, REG_HWCAP,  2) // not correct for arm64
FEAT_DEF(A26BIT,    0x00000001, 0, REG_HWCAP,  3)
FEAT_DEF(FAST_MULT, 0x00000001, 0, REG_HWCAP,  4)
FEAT_DEF(FPA,       0x00000001, 0, REG_HWCAP,  5)
FEAT_DEF(VFP,       0x00000001, 0, REG_HWCAP,  6)
FEAT_DEF(EDSP,      0x00000001, 0, REG_HWCAP,  7)
FEAT_DEF(JAVA,      0x00000001, 0, REG_HWCAP,  8)
FEAT_DEF(IWMMXT,    0x00000001, 0, REG_HWCAP,  9)
FEAT_DEF(CRUNCH,    0x00000001, 0, REG_HWCAP,  10)
FEAT_DEF(THUMBEE,   0x00000001, 0, REG_HWCAP,  11)
FEAT_DEF(NEON,      0x00000001, 0, REG_HWCAP,  12)
FEAT_DEF(VFPv3,     0x00000001, 0, REG_HWCAP,  13)
FEAT_DEF(VFPv3D16,  0x00000001, 0, REG_HWCAP,  14)
FEAT_DEF(TLS,       0x00000001, 0, REG_HWCAP,  15)
FEAT_DEF(VFPv4,     0x00000001, 0, REG_HWCAP,  16)
FEAT_DEF(IDIVA,     0x00000001, 0, REG_HWCAP,  17)
FEAT_DEF(IDIVT,     0x00000001, 0, REG_HWCAP,  18)
FEAT_DEF(VFPD32,    0x00000001, 0, REG_HWCAP,  19)
FEAT_DEF(LPAE,      0x00000001, 0, REG_HWCAP,  20)
FEAT_DEF(EVTSTRM,   0x00000001, 0, REG_HWCAP,  21)
FEAT_DEF(AES,       0x00000001, 0, REG_HWCAP2,  0)
FEAT_DEF(PMULL,     0x00000001, 0, REG_HWCAP2,  1)
FEAT_DEF(SHA1,      0x00000001, 0, REG_HWCAP2,  2)
FEAT_DEF(SHA2,      0x00000001, 0, REG_HWCAP2,  3)
FEAT_DEF(CRC32,     0x00000001, 0, REG_HWCAP2,  4)
FEAT_DEF(AARCH32,   0x00000001, 0, REG_PLATFORM, 0)
FEAT_DEF(AARCH64,   0x00000001, 0, REG_PLATFORM, 1)

Am I missing something ?


> >root@arm64:/export/dpdk-arm64# zcat /proc/config.gz  | grep CONFIG_COMPAT
> ># CONFIG_COMPAT_BRK is not set
> >CONFIG_COMPAT_BINFMT_ELF=y
> >CONFIG_COMPAT=y
> >CONFIG_COMPAT_NETLINK_MESSAGES=y
>
> root@mp30ar0:~# zcat /proc/config.gz  | grep CONFIG_COMPAT
> # CONFIG_COMPAT_BRK is not set
> CONFIG_COMPAT_OLD_SIGACTION=y
> CONFIG_COMPAT_BINFMT_ELF=y
> CONFIG_COMPAT=y
>
>
> >root@arm64:/export/dpdk-arm64# cat /proc/cpuinfo
> >Processor       : AArch64 Processor rev 0 (aarch64)
> >processor       : 0
> >processor       : 1
> --snip--
> >processor       : 46
> >processor       : 47
> >Features        : fp asimd aes pmull sha1 sha2 crc32
> >CPU implementer : 0x43
> >CPU architecture: AArch64
> >CPU variant     : 0x0
> >CPU part        : 0x0a1
> >CPU revision    : 0
>
> root@mp30ar0:~# cat /proc/cpuinfo
> processor       : 0
> Features        : fp asimd evtstrm
> CPU implementer : 0x50
> CPU architecture: 8
> CPU variant     : 0x0
> CPU part        : 0x000
> CPU revision    : 1
>
> processor       : 1
> Features        : fp asimd evtstrm
> CPU implementer : 0x50
> CPU architecture: 8
> CPU variant     : 0x0
> CPU part        : 0x000
> CPU revision    : 1
>
> processor       : 2
> Features        : fp asimd evtstrm
> CPU implementer : 0x50
> CPU architecture: 8
> CPU variant     : 0x0
> CPU part        : 0x000
> CPU revision    : 1
>
> processor       : 3
> Features        : fp asimd evtstrm
> CPU implementer : 0x50
> CPU architecture: 8
> CPU variant     : 0x0
> CPU part        : 0x000
> CPU revision    : 1
>
> processor       : 4
> Features        : fp asimd evtstrm
> CPU implementer : 0x50
> CPU architecture: 8
> CPU variant     : 0x0
> CPU part        : 0x000
> CPU revision    : 1
>
> processor       : 5
> Features        : fp asimd evtstrm
> CPU implementer : 0x50
> CPU architecture: 8
> CPU variant     : 0x0
> CPU part        : 0x000
> CPU revision    : 1
>
> processor       : 6
> Features        : fp asimd evtstrm
> CPU implementer : 0x50
> CPU architecture: 8
> CPU variant     : 0x0
> CPU part        : 0x000
> CPU revision    : 1
>
> processor       : 7
> Features        : fp asimd evtstrm
> CPU implementer : 0x50
> CPU architecture: 8
> CPU variant     : 0x0
> CPU part        : 0x000
> CPU revision    : 1
>
> root@mp30ar0:~#
>
> Hope this helps.
>
> Regards,
> Dave.
>
  
Hunt, David Nov. 2, 2015, 3:04 p.m. UTC | #6
On 02/11/2015 13:17, Jerin Jacob wrote:
-snip--
> If am not wrong existing  rte_cpu_get_flag_enabled() implementation
> should be broken in your platform also for arm64. as I could see only AT_HWCAP
> not AT_HWCAP2 and AT_HWCAP is 0x7 that means your platform also
> follows
>
> http://lxr.free-electrons.com/source/arch/arm64/include/uapi/asm/hwcap.h
>
> and the implmentation is
>
> FEAT_DEF(SWP,       0x00000001, 0, REG_HWCAP,  0) // not correct for arm64
> FEAT_DEF(HALF,      0x00000001, 0, REG_HWCAP,  1) // not correct for arm64
> FEAT_DEF(THUMB,     0x00000001, 0, REG_HWCAP,  2) // not correct for arm64
> FEAT_DEF(A26BIT,    0x00000001, 0, REG_HWCAP,  3)
--snip--
> FEAT_DEF(CRC32,     0x00000001, 0, REG_HWCAP2,  4)
> FEAT_DEF(AARCH32,   0x00000001, 0, REG_PLATFORM, 0)
> FEAT_DEF(AARCH64,   0x00000001, 0, REG_PLATFORM, 1)
>
> Am I missing something ?

You are correct. I need to re-visit this. In merging the ARMv7 and 
ARVv8, I should have split the hardware capabilities flags into 32-but 
and 64-bit versions. I'll do that in the next patch.
Thanks,
Dave.
  
Jan Viktorin Nov. 2, 2015, 3:13 p.m. UTC | #7
On Mon, 2 Nov 2015 15:04:14 +0000
"Hunt, David" <david.hunt@intel.com> wrote:

> On 02/11/2015 13:17, Jerin Jacob wrote:
> -snip--
> > If am not wrong existing  rte_cpu_get_flag_enabled() implementation
> > should be broken in your platform also for arm64. as I could see only AT_HWCAP
> > not AT_HWCAP2 and AT_HWCAP is 0x7 that means your platform also
> > follows
> >
> > http://lxr.free-electrons.com/source/arch/arm64/include/uapi/asm/hwcap.h
> >
> > and the implmentation is
> >
> > FEAT_DEF(SWP,       0x00000001, 0, REG_HWCAP,  0) // not correct for arm64
> > FEAT_DEF(HALF,      0x00000001, 0, REG_HWCAP,  1) // not correct for arm64
> > FEAT_DEF(THUMB,     0x00000001, 0, REG_HWCAP,  2) // not correct for arm64
> > FEAT_DEF(A26BIT,    0x00000001, 0, REG_HWCAP,  3)  
> --snip--
> > FEAT_DEF(CRC32,     0x00000001, 0, REG_HWCAP2,  4)
> > FEAT_DEF(AARCH32,   0x00000001, 0, REG_PLATFORM, 0)
> > FEAT_DEF(AARCH64,   0x00000001, 0, REG_PLATFORM, 1)
> >
> > Am I missing something ?  
> 
> You are correct. I need to re-visit this. In merging the ARMv7 and 
> ARVv8, I should have split the hardware capabilities flags into 32-but 
> and 64-bit versions. I'll do that in the next patch.
> Thanks,
> Dave.

Should I split the rte_atomic.h and rte_cpuflags.h then?

Jan

> 
> 
> 
> 
>
  
Hunt, David Nov. 2, 2015, 3:20 p.m. UTC | #8
On 02/11/2015 15:13, Jan Viktorin wrote:
> On Mon, 2 Nov 2015 15:04:14 +0000
> "Hunt, David" <david.hunt@intel.com> wrote:
>
>> On 02/11/2015 13:17, Jerin Jacob wrote:
>> -snip--
>>> If am not wrong existing  rte_cpu_get_flag_enabled() implementation
>>> should be broken in your platform also for arm64. as I could see only AT_HWCAP
>>> not AT_HWCAP2 and AT_HWCAP is 0x7 that means your platform also
>>> follows
>>>
>>> http://lxr.free-electrons.com/source/arch/arm64/include/uapi/asm/hwcap.h
>>>
>>> and the implmentation is
>>>
>>> FEAT_DEF(SWP,       0x00000001, 0, REG_HWCAP,  0) // not correct for arm64
>>> FEAT_DEF(HALF,      0x00000001, 0, REG_HWCAP,  1) // not correct for arm64
>>> FEAT_DEF(THUMB,     0x00000001, 0, REG_HWCAP,  2) // not correct for arm64
>>> FEAT_DEF(A26BIT,    0x00000001, 0, REG_HWCAP,  3)
>> --snip--
>>> FEAT_DEF(CRC32,     0x00000001, 0, REG_HWCAP2,  4)
>>> FEAT_DEF(AARCH32,   0x00000001, 0, REG_PLATFORM, 0)
>>> FEAT_DEF(AARCH64,   0x00000001, 0, REG_PLATFORM, 1)
>>>
>>> Am I missing something ?
>>
>> You are correct. I need to re-visit this. In merging the ARMv7 and
>> ARVv8, I should have split the hardware capabilities flags into 32-but
>> and 64-bit versions. I'll do that in the next patch.
>> Thanks,
>> Dave.
>
> Should I split the rte_atomic.h and rte_cpuflags.h then?
>
> Jan

It looks like we're headed in that direction, so yes, I think that would 
be a good idea.

Dave
  
Jan Viktorin Nov. 2, 2015, 3:24 p.m. UTC | #9
On Mon, 2 Nov 2015 10:47:53 +0000
"Hunt, David" <david.hunt@intel.com> wrote:

> On 02/11/2015 06:32, Jerin Jacob wrote:
> > On Fri, Oct 30, 2015 at 04:28:25PM +0000, Hunt, David wrote:  
> 
> --snip--
> 
> >
> > Hi Jan and Dave,
> >
> > I have reviewed your patches for arm[64] support. Please check the
> > review comments.  
> 
--snip--
> > In order to debug this, Could provide the following
> > values in tested armv8 platform. Look like its running 32bit compatible
> > mode in your environment  
> 
> I'm using a Gigabyte MP30AR0 motherboard with an 8-core X-Gene, Running 
> a 4.3.0-rc6 kernel.
> Here's the information on the cpu_flags issue you requested:
> 
--snip--
> 
> root@mp30ar0:~#
> 
> Hope this helps.
> 
> Regards,
> Dave.
> 

My few bits to compare to ARMv7. There is AT_PLATFORM=v7l (and no
aarch32), this is probably to be fixed...

Altera SoC FPGA:

# LD_SHOW_AUXV=1 sleep 1
AT_HWCAP:    swp half thumb fastmult vfp edsp thumbee neon vfpv3 tls
AT_PAGESZ:       4096
AT_CLKTCK:       100
AT_PHDR:         0x10034
AT_PHENT:        32
AT_PHNUM:        8
AT_BASE:         0x76fd3000
AT_FLAGS:        0x0
AT_ENTRY:        0x149d9
AT_UID:          0
AT_EUID:         0
AT_GID:          0
AT_EGID:         0
AT_SECURE:       0
AT_RANDOM:       0x7ebbcf2f
AT_EXECFN:       /bin/sleep
AT_PLATFORM:     v7l

# cat /proc/cpuinfo
processor       : 0
model name      : ARMv7 Processor rev 0 (v7l)
Features        : swp half thumb fastmult vfp edsp thumbee neon vfpv3
tls vfpd32 CPU implementer : 0x41
CPU architecture: 7
CPU variant     : 0x3
CPU part        : 0xc09
CPU revision    : 0

processor       : 1
model name      : ARMv7 Processor rev 0 (v7l)
Features        : swp half thumb fastmult vfp edsp thumbee neon vfpv3
tls vfpd32 CPU implementer : 0x41
CPU architecture: 7
CPU variant     : 0x3
CPU part        : 0xc09
CPU revision    : 0

Hardware        : Altera SOCFPGA
Revision        : 0000
Serial          : 0000000000000000


Odroid XU4:

# LD_SHOW_AUXV=1 sleep 1
AT_HWCAP:    swp half thumb fastmult vfp edsp neon vfpv3 tls vfpv4
AT_PAGESZ:       4096
AT_CLKTCK:       100
AT_PHDR:         0x10034
AT_PHENT:        32
AT_PHNUM:        9
AT_BASE:         0xb6f8c000
AT_FLAGS:        0x0
AT_ENTRY:        0x11191
AT_UID:          1000
AT_EUID:         1000
AT_GID:          1000
AT_EGID:         1000
AT_SECURE:       0
AT_RANDOM:       0xbec42ed6
AT_EXECFN:       /bin/sleep
AT_PLATFORM:     v7l

# cat /proc/cpuinfo
Processor       : ARMv7 Processor rev 1 (v7l)
processor       : 0
BogoMIPS        : 3.07

processor       : 1
BogoMIPS        : 3.07

processor       : 2
BogoMIPS        : 3.07

processor       : 3
BogoMIPS        : 3.07

Features        : swp half thumb fastmult vfp edsp neon vfpv3 tls vfpv4
CPU implementer : 0x41
CPU architecture: 7
CPU variant     : 0x0
CPU part        : 0xc05
CPU revision    : 1

Hardware        : ODROIDC
Revision        : 000a
Serial          : 1b00000000000000
  

Patch

diff --git a/app/test/test_cpuflags.c b/app/test/test_cpuflags.c
index 557458f..1689048 100644
--- a/app/test/test_cpuflags.c
+++ b/app/test/test_cpuflags.c
@@ -1,4 +1,4 @@ 
-/*-
+/*
* BSD LICENSE
*
* Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
@@ -115,9 +115,18 @@  test_cpuflags(void)
CHECK_FOR_FLAG(RTE_CPUFLAG_ICACHE_SNOOP);
#endif

-#if defined(RTE_ARCH_ARM)
+#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
+	printf("Checking for Floating Point:\t\t");
+	CHECK_FOR_FLAG(RTE_CPUFLAG_FPA);
+
printf("Check for NEON:\t\t");
CHECK_FOR_FLAG(RTE_CPUFLAG_NEON);
+
+	printf("Checking for ARM32 mode:\t\t");
+ CHECK_FOR_FLAG(RTE_CPUFLAG_AARCH32);
+
+	printf("Checking for ARM64 mode:\t\t");
+ CHECK_FOR_FLAG(RTE_CPUFLAG_AARCH64);
#endif

#if defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_I686)