Re: Detection of hadware feature => please do not use signal - Mailing list pgsql-bugs

I wrote:
> But that developer.arm.com page seems pretty definitive.  Tomorrow
> I'll have a go at spinning up a 32-bit NetBSD installation and test
> it.  Thanks for the research!

Conclusions:

1. Unsurprisingly, a 64-bit kernel exposes machdep.cpuN.cpu_id but
not machdep.id_isar; a 32-bit kernel the reverse.

2. 32-bit userland does not provide <aarch64/armreg.h>.  (So I did
not try to use the ID_AA64ISAR0_EL1_CRC32 macro: we'd have had to
hard-code the field position anyway for 32-bit.)

3. It doesn't look to me like NetBSD really supports 32-bit userland
under a 64-bit kernel.  Maybe it'd kind of work, but utilities like
cpuctl seem to be built for only one of the kernel APIs.

So I end up proposing the attached.  If we do somehow find ourselves
running 32-bit with a 64-bit kernel, this will report "no CRC"
because the sysctl call will fail.  I don't think it's worth
sweating over that case.

BTW, I found out that by default we'll build a software-CRC-only
implementation on 32-bit ARM NetBSD, at least in the "generic"
eabihf build:

checking for __crc32cb, __crc32ch, __crc32cw, and __crc32cd with CFLAGS=... no
checking for __crc32cb, __crc32ch, __crc32cw, and __crc32cd with CFLAGS=-march=armv8-a+crc... no
checking which CRC-32C implementation to use... slicing-by-8

It turns out that -march=armv8-a+crc would work, except:

configure:17624: checking for __crc32cb, __crc32ch, __crc32cw, and __crc32cd with CFLAGS=-march=armv8-a+crc
configure:17648: ccache gcc -o conftest -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement
-Werror=vla-Wendif-labels -Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wcast-function-type
-Wshadow=compatible-local-Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard
-Wno-format-truncation-Wno-stringop-truncation -g -O2 -march=armv8-a+crc    -Wl,-z,now -Wl,-z,relro  conftest.c -lz
-ledit-lcurses -lpthread -lexecinfo -lrt -lm  >&5 
cc1: error: '-mfloat-abi=hard': selected processor lacks an FPU

If you force it with a compatible -mfpu switch (I used -mfpu=neon)
then you get a build with runtime CRC check, and all seems to work.
I did not try very hard to determine what the default -mfpu is here
or why -march=armv8-a+crc overrides whatever the default is.  This
probably means that no end user is going to see any benefit from this
exercise, because they are not likely to try adding -mfpu by itself.
If they add "-march=armv8-a+crc -mfpu=whatever" to CFLAGS manually
then configure will deem that a runtime check is not required, so this
code still doesn't get exercised.

tl;dr: a lot of time spent on a probably-useless exercise.  Still,
now that we've got the code we may as well commit it.  The configure
issue was there all along.

            regards, tom lane

diff --git a/src/port/pg_crc32c_armv8_choose.c b/src/port/pg_crc32c_armv8_choose.c
index 306500154e..11e27978fd 100644
--- a/src/port/pg_crc32c_armv8_choose.c
+++ b/src/port/pg_crc32c_armv8_choose.c
@@ -31,6 +31,13 @@
 #endif
 #endif

+#if defined(__NetBSD__)
+#include <sys/sysctl.h>
+#if defined(__aarch64__)
+#include <aarch64/armreg.h>
+#endif
+#endif
+
 #include "port/pg_crc32c.h"

 static bool
@@ -52,6 +59,47 @@ pg_crc32c_armv8_available(void)
 #else
     return (getauxval(AT_HWCAP2) & HWCAP2_CRC32) != 0;
 #endif
+#elif defined(__NetBSD__)
+    /*
+     * On NetBSD we can read the Instruction Set Attribute Registers via
+     * sysctl.  For doubtless-historical reasons the sysctl interface is
+     * completely different on 64-bit than 32-bit, but the underlying
+     * registers contain the same fields.
+     */
+#define ISAR0_CRC32_BITPOS 16
+#define ISAR0_CRC32_BITWIDTH 4
+#define WIDTHMASK(w)    ((1 << (w)) - 1)
+#define SYSCTL_CPU_ID_MAXSIZE 64
+
+    size_t        len;
+    uint64        sysctlbuf[SYSCTL_CPU_ID_MAXSIZE];
+#if defined(__aarch64__)
+    /* We assume cpu0 is representative of all the machine's CPUs. */
+    const char *path = "machdep.cpu0.cpu_id";
+    size_t        expected_len = sizeof(struct aarch64_sysctl_cpu_id);
+#define ISAR0 ((struct aarch64_sysctl_cpu_id *) sysctlbuf)->ac_aa64isar0
+#else
+    const char *path = "machdep.id_isar";
+    size_t        expected_len = 6 * sizeof(int);
+#define ISAR0 ((int *) sysctlbuf)[5]
+#endif
+    uint64        fld;
+
+    len = sizeof(sysctlbuf);
+    memset(sysctlbuf, 0, len);
+    if (sysctlbyname(path, sysctlbuf, &len, NULL, 0) != 0)
+        return false;            /* perhaps kernel is 64-bit and we aren't? */
+    if (len != expected_len)
+        return false;            /* kernel API change? */
+
+    fld = (ISAR0 >> ISAR0_CRC32_BITPOS) & WIDTHMASK(ISAR0_CRC32_BITWIDTH);
+
+    /*
+     * Current documentation defines only the field values 0 (No CRC32) and 1
+     * (CRC32B/CRC32H/CRC32W/CRC32X/CRC32CB/CRC32CH/CRC32CW/CRC32CX).  Assume
+     * that any future nonzero value will be a superset of 1.
+     */
+    return (fld != 0);
 #else
     return false;
 #endif