1575843 - Detect AArch64 CPU features on FreeBSD

Status: RESOLVED FIXED

(NSS :: Libraries, enhancement, P2)

Comment 1

[Jan Beich]

Attachment: v1

Sorry, I can't submit on Phabricator due to bug 1536716.
https://treeherder.mozilla.org/#/jobs?repo=nss-try&revision=65258c255ba4658eb4ba94078d3b97567287241a

Comment 2

[Jan Beich]

Comment on attachment 9087394 [details] [diff] [review]
v1

Oops, when sys/auxv.h is missing such as on FreeBSD < 12 build fails:
```c
static unsigned long (*getauxval)(unsigned long) = NULL;
                       ^
```

Comment 3

[Jan Beich]

Attachment: v2

Sorry, I can't submit on Phabricator due to bug 1536716.
https://treeherder.mozilla.org/#/jobs?repo=nss-try&revision=ee7cb82a04e4c8012b35b099d0a4c31ae913fd34

Comment 4

[Val Packett]

Wait a second..

ID_AA64ISAR0_AES(id_aa64isar0) == ID_AA64ISAR0_AES_BASE
ID_AA64ISAR0_AES(id_aa64isar0) == ID_AA64ISAR0_AES_PMULL

That == looks like having PMULL support would detect as not having AES

Comment 5

[Jan Beich]

Attachment: v3

Does this work? Make sure you have https://github.com/freebsd/freebsd/commit/8d6dc914b6ab

https://treeherder.mozilla.org/#/jobs?repo=nss-try&revision=00ebd9f0cdf81e443ab1e9c033dcb6997a31bf8e

Comment 6

[Val Packett]

(In reply to Jan Beich from comment #5)

Created attachment 9087997 [details] [diff] [review]
v3
Does this work? Make sure you have https://github.com/freebsd/freebsd/commit/8d6dc914b6ab

uhh.. why? Don't use hwcap on aarch64, everything was good in v2, except one thing – I just said you need to compare the register value differently.

The AES field is 1 when only AES is supported and 2 when AES+PMULL are.
If you check it == 1 for AES, systems with PMULL (which is like all of them) would detect as not having AES.

I think just using >= on these fields is the best option.

Comment 7

[Val Packett]

Attachment: Bug 1575843 - Detect AArch64 CPU features on FreeBSD

Environment checks are reogranized to be separate from platform code
to make it impossible to forget to check disable_FEATURE on one platform
but not the other.

Comment 8

[Jan Beich]

Comment on attachment 9112745 [details]
Bug 1575843 - Detect AArch64 CPU features on FreeBSD

Don't forget to pick up -CURRENT fixes from
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=242104

Comment 9

[Jan Beich]

(In reply to greg v [:myfreeweb] from comment #6)

(In reply to Jan Beich from comment #5)

Created attachment 9087997 [details] [diff] [review]
v3
Does this work? Make sure you have https://github.com/freebsd/freebsd/commit/8d6dc914b6ab

uhh.. why?

Less FreeBSD-specific code to maintain and works under qemu-user.
If someone else does upstreaming (like you did in comment 7) then I don't care. ;) armv7 is a lower priority and doesn't support Firefox on FreeBSD yet without patching.

Don't use hwcap on aarch64

Not convincing unless you answer "why?" as well. Is it because big.LITTLE may support different features?

Comment 10

[Val Packett]

Is it because big.LITTLE may support different features?

haha, no, that would actually make the register read more concerning, but FreeBSD always returns the intersection of features of all cores, whatever method you use to access the values.

why?

It's not in 12.x, right? Mostly because of that. I don't follow STABLE though, maybe it's there now and will be in the next 12.something…

Comment 11

[Jan Beich]

Comment on attachment 9112745 [details]
Bug 1575843 - Detect AArch64 CPU features on FreeBSD

Still doesn't build on -CURRENT:

blinit.c:167:26: error: expected readable system register
        uint64_t isar0 = READ_SPECIALREG(ID_AA64ISAR0_EL1);
                         ^
/usr/include/machine/armreg.h:61:19: note: expanded from macro 'READ_SPECIALREG'
        __asm __volatile("mrs   %0, " __STRING(reg) : "=&r" (_val));    \
                         ^
<inline asm>:1:10: note: instantiated into assembly here
        mrs     x8, (((3) << 19) | ((0) << 16) | ((0) << 12) | ((6) << 8) | ((0) << 5))
                    ^

Comment 12

[Jan Beich]

@jcj, due to bug 1536716 Phabricator is read-only for me as I can't log in. Adding me as a reviewer is only useful to keep me in the loop as mail notification does work.

Comment 13

[J.C. Jones [:jcj] (he/him)]

Agh, I'm sorry Jan, I knew that but ::waves hand at the mess today with him forgetting the new release schedule:: I forgot.

Comment 14

[J.C. Jones [:jcj] (he/him)]

https://hg.mozilla.org/projects/nss/rev/fbde548e81145be44495f2469f6ec3a4dd18e481

Comment 15

[Jan Beich]

Adjusting Summary as armv6/armv7 bits were discarded (to be moved in a separate bug).