Commit e2fffe1d authored by Linus Torvalds's avatar Linus Torvalds
Browse files
Pull CRC updates from Eric Biggers:
 "Update crc_kunit to test the CRC functions in softirq and hardirq
  contexts, similar to what the lib/crypto/ KUnit tests do. Move the
  helper function needed to do this into a common header.

  This is useful mainly to test fallback code paths for when
  FPU/SIMD/vector registers are unusable"

* tag 'crc-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/linux:
  Documentation/staging: Fix typo and incorrect citation in crc32.rst
  lib/crc: Drop inline from all *_mod_init_arch() functions
  lib/crc: Use underlying functions instead of crypto_simd_usable()
  lib/crc: crc_kunit: Test CRC computation in interrupt contexts
  kunit, lib/crypto: Move run_irq_test() to common header
parents d60ac92c 136d0296
Loading
Loading
Loading
Loading
+2 −2
Original line number Diff line number Diff line
@@ -34,7 +34,7 @@ do it in the right order, matching the endianness.
Just like with ordinary division, you proceed one digit (bit) at a time.
Each step of the division you take one more digit (bit) of the dividend
and append it to the current remainder.  Then you figure out the
appropriate multiple of the divisor to subtract to being the remainder
appropriate multiple of the divisor to subtract to bring the remainder
back into range.  In binary, this is easy - it has to be either 0 or 1,
and to make the XOR cancel, it's just a copy of bit 32 of the remainder.

@@ -116,7 +116,7 @@ for any fractional bytes at the end.
To reduce the number of conditional branches, software commonly uses
the byte-at-a-time table method, popularized by Dilip V. Sarwate,
"Computation of Cyclic Redundancy Checks via Table Look-Up", Comm. ACM
v.31 no.8 (August 1998) p. 1008-1013.
v.31 no.8 (August 1988) p. 1008-1013.

Here, rather than just shifting one bit of the remainder to decide
in the correct multiple to subtract, we can shift a byte at a time.
+129 −0
Original line number Diff line number Diff line
/* SPDX-License-Identifier: GPL-2.0-or-later */
/*
 * Helper function for testing code in interrupt contexts
 *
 * Copyright 2025 Google LLC
 */
#ifndef _KUNIT_RUN_IN_IRQ_CONTEXT_H
#define _KUNIT_RUN_IN_IRQ_CONTEXT_H

#include <kunit/test.h>
#include <linux/timekeeping.h>
#include <linux/hrtimer.h>
#include <linux/workqueue.h>

#define KUNIT_IRQ_TEST_HRTIMER_INTERVAL us_to_ktime(5)

struct kunit_irq_test_state {
	bool (*func)(void *test_specific_state);
	void *test_specific_state;
	bool task_func_reported_failure;
	bool hardirq_func_reported_failure;
	bool softirq_func_reported_failure;
	unsigned long hardirq_func_calls;
	unsigned long softirq_func_calls;
	struct hrtimer timer;
	struct work_struct bh_work;
};

static enum hrtimer_restart kunit_irq_test_timer_func(struct hrtimer *timer)
{
	struct kunit_irq_test_state *state =
		container_of(timer, typeof(*state), timer);

	WARN_ON_ONCE(!in_hardirq());
	state->hardirq_func_calls++;

	if (!state->func(state->test_specific_state))
		state->hardirq_func_reported_failure = true;

	hrtimer_forward_now(&state->timer, KUNIT_IRQ_TEST_HRTIMER_INTERVAL);
	queue_work(system_bh_wq, &state->bh_work);
	return HRTIMER_RESTART;
}

static void kunit_irq_test_bh_work_func(struct work_struct *work)
{
	struct kunit_irq_test_state *state =
		container_of(work, typeof(*state), bh_work);

	WARN_ON_ONCE(!in_serving_softirq());
	state->softirq_func_calls++;

	if (!state->func(state->test_specific_state))
		state->softirq_func_reported_failure = true;
}

/*
 * Helper function which repeatedly runs the given @func in task, softirq, and
 * hardirq context concurrently, and reports a failure to KUnit if any
 * invocation of @func in any context returns false.  @func is passed
 * @test_specific_state as its argument.  At most 3 invocations of @func will
 * run concurrently: one in each of task, softirq, and hardirq context.
 *
 * The main purpose of this interrupt context testing is to validate fallback
 * code paths that run in contexts where the normal code path cannot be used,
 * typically due to the FPU or vector registers already being in-use in kernel
 * mode.  These code paths aren't covered when the test code is executed only by
 * the KUnit test runner thread in task context.  The reason for the concurrency
 * is because merely using hardirq context is not sufficient to reach a fallback
 * code path on some architectures; the hardirq actually has to occur while the
 * FPU or vector unit was already in-use in kernel mode.
 *
 * Another purpose of this testing is to detect issues with the architecture's
 * irq_fpu_usable() and kernel_fpu_begin/end() or equivalent functions,
 * especially in softirq context when the softirq may have interrupted a task
 * already using kernel-mode FPU or vector (if the arch didn't prevent that).
 * Crypto functions are often executed in softirqs, so this is important.
 */
static inline void kunit_run_irq_test(struct kunit *test, bool (*func)(void *),
				      int max_iterations,
				      void *test_specific_state)
{
	struct kunit_irq_test_state state = {
		.func = func,
		.test_specific_state = test_specific_state,
	};
	unsigned long end_jiffies;

	/*
	 * Set up a hrtimer (the way we access hardirq context) and a work
	 * struct for the BH workqueue (the way we access softirq context).
	 */
	hrtimer_setup_on_stack(&state.timer, kunit_irq_test_timer_func,
			       CLOCK_MONOTONIC, HRTIMER_MODE_REL_HARD);
	INIT_WORK_ONSTACK(&state.bh_work, kunit_irq_test_bh_work_func);

	/* Run for up to max_iterations or 1 second, whichever comes first. */
	end_jiffies = jiffies + HZ;
	hrtimer_start(&state.timer, KUNIT_IRQ_TEST_HRTIMER_INTERVAL,
		      HRTIMER_MODE_REL_HARD);
	for (int i = 0; i < max_iterations && !time_after(jiffies, end_jiffies);
	     i++) {
		if (!func(test_specific_state))
			state.task_func_reported_failure = true;
	}

	/* Cancel the timer and work. */
	hrtimer_cancel(&state.timer);
	flush_work(&state.bh_work);

	/* Sanity check: the timer and BH functions should have been run. */
	KUNIT_EXPECT_GT_MSG(test, state.hardirq_func_calls, 0,
			    "Timer function was not called");
	KUNIT_EXPECT_GT_MSG(test, state.softirq_func_calls, 0,
			    "BH work function was not called");

	/* Check for incorrect hash values reported from any context. */
	KUNIT_EXPECT_FALSE_MSG(
		test, state.task_func_reported_failure,
		"Incorrect hash values reported from task context");
	KUNIT_EXPECT_FALSE_MSG(
		test, state.hardirq_func_reported_failure,
		"Incorrect hash values reported from hardirq context");
	KUNIT_EXPECT_FALSE_MSG(
		test, state.softirq_func_reported_failure,
		"Incorrect hash values reported from softirq context");
}

#endif /* _KUNIT_RUN_IN_IRQ_CONTEXT_H */
+3 −5
Original line number Diff line number Diff line
@@ -5,8 +5,6 @@
 * Copyright (C) 2016 Linaro Ltd <ard.biesheuvel@linaro.org>
 */

#include <crypto/internal/simd.h>

#include <asm/neon.h>
#include <asm/simd.h>

@@ -23,7 +21,7 @@ static inline u16 crc_t10dif_arch(u16 crc, const u8 *data, size_t length)
{
	if (length >= CRC_T10DIF_PMULL_CHUNK_SIZE) {
		if (static_branch_likely(&have_pmull)) {
			if (crypto_simd_usable()) {
			if (likely(may_use_simd())) {
				kernel_neon_begin();
				crc = crc_t10dif_pmull64(crc, data, length);
				kernel_neon_end();
@@ -31,7 +29,7 @@ static inline u16 crc_t10dif_arch(u16 crc, const u8 *data, size_t length)
			}
		} else if (length > CRC_T10DIF_PMULL_CHUNK_SIZE &&
			   static_branch_likely(&have_neon) &&
			   crypto_simd_usable()) {
			   likely(may_use_simd())) {
			u8 buf[16] __aligned(16);

			kernel_neon_begin();
@@ -45,7 +43,7 @@ static inline u16 crc_t10dif_arch(u16 crc, const u8 *data, size_t length)
}

#define crc_t10dif_mod_init_arch crc_t10dif_mod_init_arch
static inline void crc_t10dif_mod_init_arch(void)
static void crc_t10dif_mod_init_arch(void)
{
	if (elf_hwcap & HWCAP_NEON) {
		static_branch_enable(&have_neon);
+3 −5
Original line number Diff line number Diff line
@@ -7,8 +7,6 @@

#include <linux/cpufeature.h>

#include <crypto/internal/simd.h>

#include <asm/hwcap.h>
#include <asm/neon.h>
#include <asm/simd.h>
@@ -34,7 +32,7 @@ static inline u32 crc32_le_scalar(u32 crc, const u8 *p, size_t len)
static inline u32 crc32_le_arch(u32 crc, const u8 *p, size_t len)
{
	if (len >= PMULL_MIN_LEN + 15 &&
	    static_branch_likely(&have_pmull) && crypto_simd_usable()) {
	    static_branch_likely(&have_pmull) && likely(may_use_simd())) {
		size_t n = -(uintptr_t)p & 15;

		/* align p to 16-byte boundary */
@@ -63,7 +61,7 @@ static inline u32 crc32c_scalar(u32 crc, const u8 *p, size_t len)
static inline u32 crc32c_arch(u32 crc, const u8 *p, size_t len)
{
	if (len >= PMULL_MIN_LEN + 15 &&
	    static_branch_likely(&have_pmull) && crypto_simd_usable()) {
	    static_branch_likely(&have_pmull) && likely(may_use_simd())) {
		size_t n = -(uintptr_t)p & 15;

		/* align p to 16-byte boundary */
@@ -85,7 +83,7 @@ static inline u32 crc32c_arch(u32 crc, const u8 *p, size_t len)
#define crc32_be_arch crc32_be_base /* not implemented on this arch */

#define crc32_mod_init_arch crc32_mod_init_arch
static inline void crc32_mod_init_arch(void)
static void crc32_mod_init_arch(void)
{
	if (elf_hwcap2 & HWCAP2_CRC32)
		static_branch_enable(&have_crc32);
+3 −5
Original line number Diff line number Diff line
@@ -7,8 +7,6 @@

#include <linux/cpufeature.h>

#include <crypto/internal/simd.h>

#include <asm/neon.h>
#include <asm/simd.h>

@@ -25,7 +23,7 @@ static inline u16 crc_t10dif_arch(u16 crc, const u8 *data, size_t length)
{
	if (length >= CRC_T10DIF_PMULL_CHUNK_SIZE) {
		if (static_branch_likely(&have_pmull)) {
			if (crypto_simd_usable()) {
			if (likely(may_use_simd())) {
				kernel_neon_begin();
				crc = crc_t10dif_pmull_p64(crc, data, length);
				kernel_neon_end();
@@ -33,7 +31,7 @@ static inline u16 crc_t10dif_arch(u16 crc, const u8 *data, size_t length)
			}
		} else if (length > CRC_T10DIF_PMULL_CHUNK_SIZE &&
			   static_branch_likely(&have_asimd) &&
			   crypto_simd_usable()) {
			   likely(may_use_simd())) {
			u8 buf[16];

			kernel_neon_begin();
@@ -47,7 +45,7 @@ static inline u16 crc_t10dif_arch(u16 crc, const u8 *data, size_t length)
}

#define crc_t10dif_mod_init_arch crc_t10dif_mod_init_arch
static inline void crc_t10dif_mod_init_arch(void)
static void crc_t10dif_mod_init_arch(void)
{
	if (cpu_have_named_feature(ASIMD)) {
		static_branch_enable(&have_asimd);
Loading