lib: Fix and clean up crc implementation

Fixes detection of PMULL on aarch64 without crypto extensions. Adds a
crc64_nvme_step helper function in CRC64 to avoid code
duplication and cleans up the comments.

Signed-off-by: Dimitri Staessens <dimitri@ouroboros.rocks>
Signed-off-by: Sander Vrijders <sander@ouroboros.rocks>

4 files changed, 84 insertions, 67 deletions

diff --git a/cmake/utils/CPUUtils.cmake b/cmake/utils/CPUUtils.cmake
index e158792a..8ca7683a 100644
--- a/cmake/utils/CPUUtils.cmake
+++ b/cmake/utils/CPUUtils.cmake
@@ -1,32 +1,51 @@
-include(CheckCSourceCompiles)
+include(CheckCSourceRuns)
 
-# Detect a CPU feature by attempting to compile a small program with
-# the matching compiler flag.
+# Compile + run a probe so we only enable a feature the host CPU
+# actually implements (toolchains accept flags the silicon may lack).
+# Cross-compile without an emulator: feature off.
 function(detect_cpu_feature _result_var _flags _source)
   set(_save_flags "${CMAKE_REQUIRED_FLAGS}")
   set(_save_quiet "${CMAKE_REQUIRED_QUIET}")
   set(CMAKE_REQUIRED_FLAGS "${_save_flags} ${_flags}")
   set(CMAKE_REQUIRED_QUIET TRUE)
-  check_c_source_compiles("${_source}" ${_result_var})
+  if(CMAKE_CROSSCOMPILING AND NOT CMAKE_CROSSCOMPILING_EMULATOR)
+    set(${_result_var} FALSE CACHE INTERNAL
+        "${_result_var} (cross-compile without emulator: off)")
+  else()
+    check_c_source_runs("${_source}" ${_result_var})
+  endif()
   set(CMAKE_REQUIRED_FLAGS "${_save_flags}")
   set(CMAKE_REQUIRED_QUIET "${_save_quiet}")
 endfunction()
 
-# x86 PCLMULQDQ (carry-less multiply) + SSE4.1.  Sets HAVE_PCLMUL only
-# when both intrinsic groups compile.
+# x86 PCLMULQDQ + SSE4.1. argc-derived input defeats constant folding;
+# SIGILL handler exits cleanly so the kernel skips the core dump.
 function(detect_pclmul)
   detect_cpu_feature(_HAVE_PCLMUL "-mpclmul"
 "#include <wmmintrin.h>
-int main(void) {
-        __m128i a = _mm_setzero_si128();
-        __m128i b = _mm_clmulepi64_si128(a, a, 0);
-        return (int) _mm_cvtsi128_si32(b);
+#include <signal.h>
+#include <unistd.h>
+static void on_sigill(int sig) { (void) sig; _exit(1); }
+int main(int argc, char ** argv) {
+        __m128i a;
+        __m128i b;
+        (void) argv;
+        signal(SIGILL, on_sigill);
+        a = _mm_set1_epi32(argc);
+        b = _mm_clmulepi64_si128(a, a, 0);
+        return _mm_cvtsi128_si32(b) & 0;
 }")
   detect_cpu_feature(_HAVE_SSE41 "-msse4.1"
 "#include <smmintrin.h>
-int main(void) {
-        __m128i a = _mm_setzero_si128();
-        return _mm_extract_epi32(a, 0);
+#include <signal.h>
+#include <unistd.h>
+static void on_sigill(int sig) { (void) sig; _exit(1); }
+int main(int argc, char ** argv) {
+        __m128i a;
+        (void) argv;
+        signal(SIGILL, on_sigill);
+        a = _mm_set1_epi32(argc);
+        return _mm_extract_epi32(a, 0) & 0;
 }")
   if(_HAVE_PCLMUL AND _HAVE_SSE41)
     set(HAVE_PCLMUL TRUE CACHE INTERNAL
@@ -36,14 +55,23 @@ int main(void) {
   endif()
 endfunction()
 
-# aarch64 FEAT_PMULL (vmull_p64 / vmull_high_p64) carry-less multiply.
+# aarch64 FEAT_PMULL (vmull_p64). Pi 4's BCM2711 accepts +crypto at
+# compile time but lacks the hardware — the runtime probe catches that.
 function(detect_pmull)
   detect_cpu_feature(_HAVE_PMULL "-march=armv8-a+crypto"
 "#include <arm_neon.h>
-int main(void) {
-        poly64_t   a = (poly64_t) 0;
-        poly128_t  c = vmull_p64(a, a);
-        return (int) vgetq_lane_u64((uint64x2_t) c, 0);
+#include <signal.h>
+#include <stdint.h>
+#include <unistd.h>
+static void on_sigill(int sig) { (void) sig; _exit(1); }
+int main(int argc, char ** argv) {
+        poly64_t   a;
+        poly128_t  c;
+        (void) argv;
+        signal(SIGILL, on_sigill);
+        a = (poly64_t) (uint64_t) argc;
+        c = vmull_p64(a, a);
+        return (int) (vgetq_lane_u64((uint64x2_t) c, 0) & 0);
 }")
   if(_HAVE_PMULL)
     set(HAVE_PMULL TRUE CACHE INTERNAL
diff --git a/src/lib/crc/crc16.c b/src/lib/crc/crc16.c
index 55af8647..9dc59429 100644
--- a/src/lib/crc/crc16.c
+++ b/src/lib/crc/crc16.c
@@ -34,10 +34,7 @@
 
 #include <ouroboros/crc16.h>
 
-/* Bit-by-bit MSB-first CRC.  Header-check use case rarely exceeds a
- * few hundred bytes; a 256-entry uint16_t table costs 512 B of
- * .rodata and is easy to add later if profiling demands it.
- */
+/*  Bit-by-bit MSB-first CRC. */
 void crc16_ccitt_false(uint16_t *   crc,
                        const void * buf,
                        size_t       len)
diff --git a/src/lib/crc/crc64.c b/src/lib/crc/crc64.c
index 3ee5e798..1b6fb5f6 100644
--- a/src/lib/crc/crc64.c
+++ b/src/lib/crc/crc64.c
@@ -34,7 +34,8 @@
 
 #include <ouroboros/crc64.h>
 
-/* Reflected CRC-64/NVMe table.  Polynomial in reflected form:
+/*
+ * Reflected CRC-64/NVMe table. Polynomial in reflected form:
  * 0x9a6c9329ac4bc9b5 (bitrev of 0xad93d23594c93659).
  */
 static const uint64_t crc64_nvme_tab[256] = {
@@ -168,20 +169,28 @@ static const uint64_t crc64_nvme_tab[256] = {
         0x55b4a08fdfd90e51ULL, 0x2ada5047efec8728ULL
 };
 
+static __inline__ uint64_t crc64_nvme_step(uint64_t        c,
+                                           const uint8_t * p,
+                                           size_t          len)
+{
+        size_t n;
+
+        for (n = 0; n < len; n++)
+                c = crc64_nvme_tab[(c ^ p[n]) & 0xff] ^ (c >> 8);
+
+        return c;
+}
+
 void crc64_nvme_table(uint64_t *   crc,
                       const void * buf,
                       size_t       len)
 {
-        size_t n;
+        uint64_t c;
 
-        *crc = *crc ^ 0xffffffffffffffffULL;
+        c = crc64_nvme_step(*crc ^ UINT64_MAX,
+                            (const uint8_t *) buf, len);
 
-        for (n = 0; n < len; n++)
-                *crc = crc64_nvme_tab[(*crc ^ ((uint8_t *) buf)[n])
-                                      & 0xff]
-                        ^ (*crc >> 8);
-
-        *crc = *crc ^ 0xffffffffffffffffULL;
+        *crc = c ^ UINT64_MAX;
 }
 
 #ifdef HAVE_PCLMUL
@@ -189,7 +198,8 @@ void crc64_nvme_table(uint64_t *   crc,
 #include <smmintrin.h>
 #include <wmmintrin.h>
 
-/* Fold-by-16 constants for reflected CRC-64/NVMe. Properties of the
+/*
+ * Fold-by-16 constants for reflected CRC-64/NVMe. Properties of the
  * polynomial; identical between the PCLMUL and PMULL backends.
  *   k3 = bitrev64(x^(128+64) mod P) << 1
  *   k4 = bitrev64(x^(128+0)  mod P) << 1
@@ -209,9 +219,10 @@ static __m128i fold16(__m128i x,
         return _mm_xor_si128(lo, hi);
 }
 
-/* Fold-by-16 over 16-byte chunks; the 128-bit folded state is then
+/*
+ * Fold-by-16 over 16-byte chunks; the 128-bit folded state is then
  * emitted as 16 little-endian bytes and run through the byte-table
- * loop together with any tail (<=15 bytes).  The 16-byte minimum on
+ * loop together with any tail (<=15 bytes). The 16-byte minimum on
  * the bulk loop is why the short-input path uses the table directly.
  */
 __attribute__((target("pclmul,sse4.1")))
@@ -223,7 +234,6 @@ static void crc64_nvme_clmul(uint64_t *   crc,
         uint64_t        seed;
         uint64_t        c;
         size_t          off;
-        size_t          n;
         __m128i         x;
         __m128i         k;
         uint8_t         post[16];
@@ -232,17 +242,14 @@ static void crc64_nvme_clmul(uint64_t *   crc,
         seed = *crc;
 
         if (len < 16) {
-                c = seed ^ 0xffffffffffffffffULL;
-                for (n = 0; n < len; n++)
-                        c = crc64_nvme_tab[(c ^ p[n]) & 0xff]
-                                ^ (c >> 8);
-                *crc = c ^ 0xffffffffffffffffULL;
+                c = crc64_nvme_step(seed ^ UINT64_MAX, p, len);
+                *crc = c ^ UINT64_MAX;
                 return;
         }
 
         x = _mm_loadu_si128((const __m128i *) p);
         x = _mm_xor_si128(x, _mm_cvtsi64_si128((int64_t)
-                          (seed ^ 0xffffffffffffffffULL)));
+                          (seed ^ UINT64_MAX)));
 
         k = _mm_set_epi64x((int64_t) k4_clmul, (int64_t) k3_clmul);
 
@@ -257,14 +264,10 @@ static void crc64_nvme_clmul(uint64_t *   crc,
 
         _mm_storeu_si128((__m128i *) post, x);
 
-        c = 0;
-        for (n = 0; n < 16; n++)
-                c = crc64_nvme_tab[(c ^ post[n]) & 0xff] ^ (c >> 8);
+        c = crc64_nvme_step(0, post, 16);
+        c = crc64_nvme_step(c, p + off, len - off);
 
-        for (n = off; n < len; n++)
-                c = crc64_nvme_tab[(c ^ p[n]) & 0xff] ^ (c >> 8);
-
-        *crc = c ^ 0xffffffffffffffffULL;
+        *crc = c ^ UINT64_MAX;
 }
 
 #endif /* HAVE_PCLMUL */
@@ -304,7 +307,6 @@ static void crc64_nvme_pmull(uint64_t *   crc,
         uint64_t        seed;
         uint64_t        c;
         size_t          off;
-        size_t          n;
         uint64x2_t      x;
         uint64x2_t      k;
         uint64_t        seed_lane[2];
@@ -315,16 +317,13 @@ static void crc64_nvme_pmull(uint64_t *   crc,
         seed = *crc;
 
         if (len < 16) {
-                c = seed ^ 0xffffffffffffffffULL;
-                for (n = 0; n < len; n++)
-                        c = crc64_nvme_tab[(c ^ p[n]) & 0xff]
-                                ^ (c >> 8);
-                *crc = c ^ 0xffffffffffffffffULL;
+                c = crc64_nvme_step(seed ^ UINT64_MAX, p, len);
+                *crc = c ^ UINT64_MAX;
                 return;
         }
 
         x = vld1q_u64((const uint64_t *) p);
-        seed_lane[0] = seed ^ 0xffffffffffffffffULL;
+        seed_lane[0] = seed ^ UINT64_MAX;
         seed_lane[1] = 0;
         x = veorq_u64(x, vld1q_u64(seed_lane));
 
@@ -343,16 +342,11 @@ static void crc64_nvme_pmull(uint64_t *   crc,
 
         vst1q_u8(post, vreinterpretq_u8_u64(x));
 
-        c = 0;
-        for (n = 0; n < 16; n++)
-                c = crc64_nvme_tab[(c ^ post[n]) & 0xff] ^ (c >> 8);
-
-        for (n = off; n < len; n++)
-                c = crc64_nvme_tab[(c ^ p[n]) & 0xff] ^ (c >> 8);
+        c = crc64_nvme_step(0, post, 16);
+        c = crc64_nvme_step(c, p + off, len - off);
 
-        *crc = c ^ 0xffffffffffffffffULL;
+        *crc = c ^ UINT64_MAX;
 }
-
 #endif /* HAVE_PMULL */
 
 void crc64_nvme(uint64_t *   crc,
diff --git a/src/lib/crc/crc8.c b/src/lib/crc/crc8.c
index e8b9685a..20976b29 100644
--- a/src/lib/crc/crc8.c
+++ b/src/lib/crc/crc8.c
@@ -34,10 +34,8 @@
 
 #include <ouroboros/crc8.h>
 
-/* Bit-by-bit MSB-first CRC.  The expected use case is header check
- * sequences of a handful of bytes; a 256-byte lookup table would not
- * pay for the extra .rodata footprint at typical input sizes.
- */
+
+ /* Bit-by-bit MSB-first CRC. */
 void crc8_autosar(uint8_t *    crc,
                   const void * buf,
                   size_t       len)