Skip to content

arm64: Batch PSTATE.TCO handling in kernel nofault loops - #13439

Closed
kernel-patches-daemon-bpf[bot] wants to merge 7 commits into
bpf-next_basefrom
series/1151050=>bpf-next
Closed

arm64: Batch PSTATE.TCO handling in kernel nofault loops#13439
kernel-patches-daemon-bpf[bot] wants to merge 7 commits into
bpf-next_basefrom
series/1151050=>bpf-next

Conversation

@kernel-patches-daemon-bpf

Copy link
Copy Markdown

Pull request for series with
subject: arm64: Batch PSTATE.TCO handling in kernel nofault loops
version: 1
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=1151050

@kernel-patches-daemon-bpf

Copy link
Copy Markdown
Author

Upstream branch: 5e289c5
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1151050
version: 1

@kernel-patches-review-bot

Copy link
Copy Markdown
commit 5185333e16101baeb8777b9dee9bb6e70adc70bc
Author: Muhammad Usama Anjum <usama.anjum@arm.com>

maccess: Skip setup for zero-sized kernel nofault copies

This commit adds early-return checks to copy_from_kernel_nofault() and
copy_to_kernel_nofault() when size is zero, skipping the pagefault state
changes that would otherwise occur.

> A zero-sized kernel nofault copy does not enter an access loop, but it
> still disables and re-enables page faults.
>
> Zero sizes are valid. BPF probe-read helpers accept them, and KGDB memory
> packets may carry a zero length.
>
> Return before changing page-fault state when there is nothing to copy.
> For reads, keep architecture-specific address validation before the fast
> path so its behavior is unchanged.

This isn't a bug, but would it help to say in the changelog that a later
patch in the series wraps these loops in a bare nofault (TCO) region and
relies on the operation never being empty, rather than framing this purely
as skipping the page-fault state change?

The current description focuses on the mechanical effect (skip
pagefault_disable/enable when there is nothing to copy), but on its own the
change appears to be a no-op: pagefault_disable and pagefault_enable only
increment and decrement current->pagefault_disabled, so a zero-size copy
already returned 0 without touching memory.



AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

In-Reply-To-Subject: maccess: Skip setup for zero-sized kernel nofault copies
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/32751368606

@kernel-patches-review-bot

Copy link
Copy Markdown
commit 5fc843d315c09f20fd822ad70c77be1cb621ea3d
Author: Muhammad Usama Anjum <usama.anjum@arm.com>

uaccess: Add scope guard for bare kernel nofault regions

This commit adds a scope guard wrapper for the batched kernel nofault
region hooks, allowing callers to maintain natural control flow while
ensuring the architecture state remains balanced on all exit paths.

> diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
> index 7ae185467347..b25e778ddc38 100644
> --- a/include/linux/uaccess.h
> +++ b/include/linux/uaccess.h
> @@ -653,6 +653,10 @@ do {							\
>  #define __end_kernel_nofault_bare()	do {} while (0)
>  #endif
>
> +DEFINE_LOCK_GUARD_0(__kernel_nofault_bare,
> +		    __begin_kernel_nofault_bare(),
> +		    __end_kernel_nofault_bare())
> +

This isn't a bug, but would an unprefixed guard name such as
kernel_nofault_bare read more consistently at the call sites, alongside
guard(pagefault) a few hundred lines up in the same header?

Every other guard class in the tree uses an unprefixed name, including
the directly comparable one at line 303 in this same file:
DEFINE_LOCK_GUARD_0(pagefault, pagefault_disable(), pagefault_enable()).
The '__' prefix is meaningful on __begin/__end_kernel_nofault_bare()
because those are arch-implementation hooks, but the guard name is the
caller-facing spelling.

>  /**
>   * get_kernel_nofault(): safely attempt to read from a location
>   * @val: read into this variable



AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

In-Reply-To-Subject: uaccess: Add scope guard for bare kernel nofault regions
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/32751368606

@kernel-patches-review-bot

Copy link
Copy Markdown
commit dbbe84aa8e6da699eb9d9857f988281f9afdd9fc
Author: Muhammad Usama Anjum <usama.anjum@arm.com>

maccess: Use a scoped guard for page faults

This commit refactors kernel nofault copy and string functions to use
scoped_guard(pagefault) instead of explicit pagefault_disable/enable
calls, simplifying the cleanup paths.

> diff --git a/mm/maccess.c b/mm/maccess.c
> index c59a0e092d24..f695ceefe6fc 100644
> --- a/mm/maccess.c
> +++ b/mm/maccess.c
> @@ -38,18 +38,17 @@ long copy_from_kernel_nofault(void *dst, const void *src, size_t size)
>  	if (!size)
>  		return 0;
>
> -	pagefault_disable();
> -	if (!(align & 7))
> -		copy_from_kernel_nofault_loop(dst, src, size, u64, Efault);
> -	if (!(align & 3))
> -		copy_from_kernel_nofault_loop(dst, src, size, u32, Efault);
> -	if (!(align & 1))
> -		copy_from_kernel_nofault_loop(dst, src, size, u16, Efault);
> -	copy_from_kernel_nofault_loop(dst, src, size, u8, Efault);
> -	pagefault_enable();
> +	scoped_guard(pagefault) {
> +		if (!(align & 7))
> +			copy_from_kernel_nofault_loop(dst, src, size, u64, Efault);
> +		if (!(align & 3))
> +			copy_from_kernel_nofault_loop(dst, src, size, u32, Efault);
> +		if (!(align & 1))
> +			copy_from_kernel_nofault_loop(dst, src, size, u16, Efault);
> +		copy_from_kernel_nofault_loop(dst, src, size, u8, Efault);
> +	}
>  	return 0;
>  Efault:
> -	pagefault_enable();
>  	return -EFAULT;
>  }

A subsystem pattern flags this as potentially concerning: include/linux/
cleanup.h states "the expectation is that usage of 'goto' and cleanup
helpers is never mixed in the same function. I.e. for a given routine,
convert all resources that need a 'goto' cleanup to scope-based cleanup,
or convert none of them."

After this change, copy_from_kernel_nofault() declares scoped_guard(
pagefault) while still using the goto-based Efault: label. Each
copy_from_kernel_nofault_loop() expands to __get_kernel_nofault(...,
Efault), which does 'goto Efault' from inside the guard scope to a label
outside it.

The concern is whether jumping out of the guard scope with goto is the
pattern cleanup.h discourages. Is this mixing safe in practice, or should
the function be fully converted to scope-based cleanup without the goto
label?

> @@ -73,18 +72,17 @@ long copy_to_kernel_nofault(void *dst, const void *src, size_t size)
>  	if (!IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS))
>  		align = (unsigned long)dst | (unsigned long)src;
>
> -	pagefault_disable();
> -	if (!(align & 7))
> -		copy_to_kernel_nofault_loop(dst, src, size, u64, Efault);
> -	if (!(align & 3))
> -		copy_to_kernel_nofault_loop(dst, src, size, u32, Efault);
> -	if (!(align & 1))
> -		copy_to_kernel_nofault_loop(dst, src, size, u16, Efault);
> -	copy_to_kernel_nofault_loop(dst, src, size, u8, Efault);
> -	pagefault_enable();
> +	scoped_guard(pagefault) {
> +		if (!(align & 7))
> +			copy_to_kernel_nofault_loop(dst, src, size, u64, Efault);
> +		if (!(align & 3))
> +			copy_to_kernel_nofault_loop(dst, src, size, u32, Efault);
> +		if (!(align & 1))
> +			copy_to_kernel_nofault_loop(dst, src, size, u16, Efault);
> +		copy_to_kernel_nofault_loop(dst, src, size, u8, Efault);
> +	}
>  	return 0;
>  Efault:
> -	pagefault_enable();
>  	return -EFAULT;
>  }

Same pattern here: copy_to_kernel_nofault() now has scoped_guard(pagefault)
with copy_to_kernel_nofault_loop() calls that 'goto Efault' out of the
guard scope.

> @@ -97,18 +95,17 @@ long strncpy_from_kernel_nofault(char *dst, const void *unsafe_addr, long count)
>  	if (!copy_from_kernel_nofault_allowed(unsafe_addr, count))
>  		return -ERANGE;
>
> -	pagefault_disable();
> -	do {
> -		__get_kernel_nofault(dst, src, u8, Efault);
> -		dst++;
> -		src++;
> -	} while (dst[-1] && src - unsafe_addr < count);
> -	pagefault_enable();
> +	scoped_guard(pagefault) {
> +		do {
> +			__get_kernel_nofault(dst, src, u8, Efault);
> +			dst++;
> +			src++;
> +		} while (dst[-1] && src - unsafe_addr < count);
> +	}
>
>  	dst[-1] = '\0';
>  	return src - unsafe_addr;
>  Efault:
> -	pagefault_enable();
>  	dst[0] = '\0';
>  	return -EFAULT;
>  }

And again in strncpy_from_kernel_nofault(): scoped_guard(pagefault) with
__get_kernel_nofault(dst, src, u8, Efault) jumping out of the guard scope.

Does this refactoring align with the guidance in cleanup.h about not mixing
goto and cleanup helpers, or is there precedent for this pattern in uaccess
code?



AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

In-Reply-To-Subject: maccess: Use a scoped guard for page faults
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/32751368606

@kernel-patches-review-bot

Copy link
Copy Markdown
commit 582a0a0c5c8589832f42bc3de95bce3c2fba5778
Author: Muhammad Usama Anjum <usama.anjum@arm.com>

bpf: Batch TCO handling in string kfuncs

This commit converts BPF string kfuncs to use batched TCO (Tag Check
Override) handling by replacing __get_kernel_nofault with
__get_kernel_nofault_bare under guard(__kernel_nofault_bare)(). This
reduces MSR TCO executions from 4N to 2 for two-load comparisons.

> commit 582a0a0c5c8589832f42bc3de95bce3c2fba5778
> Author: Muhammad Usama Anjum <usama.anjum@arm.com>
>
>     bpf: Batch TCO handling in string kfuncs
>
>     BPF string kfuncs cannot rely on NUL termination, so they scan memory with
>     kernel nofault loads. With Hardware Tag-Based KASAN in asynchronous or
>     asymmetric mode, every arm64 load sets and clears PSTATE.TCO.
>
>     Use bare loads and hold one nofault region across each string operation.
>     The scope guard restores TCO before page faults are enabled again on every
>     exit, including an access fault.
>
>     A character comparison performs two nofault loads. For N compared
>     characters, the number of dynamic MSR TCO executions therefore falls from
>     4N to 2.
>
>     Signed-off-by: Muhammad Usama Anjum <usama.anjum@arm.com>

This isn't a bug, but would it be worth scoping the statement to kernel
nofault accesses rather than "every arm64 load"? The TCO set/clear
happens in __get_kernel_nofault()/__put_kernel_nofault() and
load_unaligned_zeropad(), not ordinary loads. Also, the 4N-to-2 figure
applies to the two-load-per-character comparison helpers like
__bpf_strncasecmp and __bpf_strnstr, while six of the eight converted
functions (bpf_strnchr, bpf_strchrnul, bpf_strrchr, bpf_strnlen,
bpf_strspn, bpf_strcspn) perform one load per character and so go from
2N to 2. The sibling patch 022e4f83d5cb spells out its per-shape figures
in a small table.

> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index 3574a9a5721e..f8c557161584 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c

[ ... ]

> @@ -4014,12 +4020,13 @@ __bpf_kfunc int bpf_strspn(const char *s__ign, const char *accept__ign)
>  	}
>
>  	guard(pagefault)();
> +	guard(__kernel_nofault_bare)();
>  	for (i = 0; i < XATTR_SIZE_MAX; i++) {
> -		__get_kernel_nofault(&cs, s__ign, char, err_out);
> +		__get_kernel_nofault_bare(&cs, s__ign, char, err_out);
>  		if (cs == '\0')
>  			return i;
>  		for (j = 0; j < XATTR_SIZE_MAX; j++) {
> -			__get_kernel_nofault(&ca, accept__ign + j, char, err_out);
> +			__get_kernel_nofault_bare(&ca, accept__ign + j, char, err_out);
>  			if (cs == ca || ca == '\0')
>  				break;
>  		}
> @@ -4027,7 +4034,7 @@ __bpf_kfunc int bpf_strspn(const char *s__ign, const char *accept__ign)
>  	return -E2BIG;
>
>  err_out:
>  	return -EFAULT;
>  }

A subsystem pattern flags this as potentially concerning: eight
functions in this patch now contain both scope guards (guard(pagefault)()
and the newly added guard(__kernel_nofault_bare)()) and goto-based error
exit to the err_out: label. The __get_kernel_nofault_bare macro expands
to 'goto err_label' on an access fault, so bpf_strspn (shown above),
bpf_strcspn, __bpf_strnstr, and the five single-loop helpers all mix
goto and guard() cleanup in the same function. The subsystem guide
directly flags this combination.

My own reading is that this is probably unavoidable. The err_out: label
is an error-return label, not a cleanup label - its entire body is
'return -EFAULT;'. No resource is released there; both the page-fault
state and the TCO override are released by the guards' destructors,
which run on that return because err_out: sits in the same block as the
guard variables. The goto does not leave the scope, so the cleanups fire
at the return, not at the goto. The goto is imposed by the
__get_kernel_nofault*() API, which is inherently label-based. The same
mixing already existed before this patch (guard(pagefault)() plus
err_out: were introduced earlier in the series), and the sibling
conversion in mm/maccess.c (022e4f83d5cb) uses the identical shape.

Does this concern you, or is the pattern acceptable here given the
constraints of the nofault API?



AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

In-Reply-To-Subject: bpf: Batch TCO handling in string kfuncs
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/32751368606

@kernel-patches-review-bot

Copy link
Copy Markdown
commit 022e4f83d5cb9d715def8d459d84adaec1f873a1
Author: Muhammad Usama Anjum <usama.anjum@arm.com>

maccess: Batch TCO handling in kernel nofault loops

This commit batches Hardware Tag-Based KASAN TCO handling in arm64 kernel
nofault loops. Instead of setting and clearing PSTATE.TCO for each access,
the patch wraps entire copy operations in one bare nofault region, reducing
TCO MSR overhead from 2N per N-access operation to a constant 2.

> The number of dynamic MSR TCO executions therefore changes as follows:
>
>     Work                         Before    After
>     N nofault accesses          2N        2
>     4 KiB nofault copy          1,024     2
>     N-byte strncpy              2N        2
>
> The 4 KiB case assumes 512 64-bit accesses. These figures come from the
> control flow rather than a runtime measurement, so the time saved depends
> on the CPU and workload.

This isn't a bug, but would it be worth either dropping the table in favour
of one sentence (one TCO pair per operation instead of one per access) or
backing it with a measurement on a real HW-tag-KASAN async config? Also,
could the "N nofault accesses" row say whether N accesses are within a
single batched operation?

> diff --git a/mm/maccess.c b/mm/maccess.c
> index f695ceefe6fc..87486bf98d15 100644
> --- a/mm/maccess.c
> +++ b/mm/maccess.c
> @@ -19,7 +19,7 @@ bool __weak copy_from_kernel_nofault_allowed(const void *unsafe_src,
>   */
>  #define copy_from_kernel_nofault_loop(dst, src, len, type, err_label)	\
>  	while (len >= sizeof(type)) {					\
> -		__get_kernel_nofault(dst, src, type, err_label);	\
> +		__get_kernel_nofault_bare(dst, src, type, err_label);	\
>  		kmsan_check_memory(src, sizeof(type));			\
>  		dst += sizeof(type);					\
>  		src += sizeof(type);					\
> @@ -39,13 +39,15 @@ long copy_from_kernel_nofault(void *dst, const void *src, size_t size)
>  		return 0;
>
>  	scoped_guard(pagefault) {
> -		if (!(align & 7))
> -			copy_from_kernel_nofault_loop(dst, src, size, u64, Efault);
> -		if (!(align & 3))
> -			copy_from_kernel_nofault_loop(dst, src, size, u32, Efault);
> -		if (!(align & 1))
> -			copy_from_kernel_nofault_loop(dst, src, size, u16, Efault);
> -		copy_from_kernel_nofault_loop(dst, src, size, u8, Efault);
> +		scoped_guard(__kernel_nofault_bare) {
> +			if (!(align & 7))
> +				copy_from_kernel_nofault_loop(dst, src, size, u64, Efault);
> +			if (!(align & 3))
> +				copy_from_kernel_nofault_loop(dst, src, size, u32, Efault);
> +			if (!(align & 1))
> +				copy_from_kernel_nofault_loop(dst, src, size, u16, Efault);
> +			copy_from_kernel_nofault_loop(dst, src, size, u8, Efault);
> +		}
>  	}
>  	return 0;
>  Efault:

A subsystem pattern flags this as potentially concerning: the nested
scoped_guard(__kernel_nofault_bare) introduces a second cleanup scope
inside an existing goto-based error path. include/linux/cleanup.h states
"the expectation is that usage of 'goto' and cleanup helpers is never
mixed in the same function. I.e. for a given routine, convert all
resources that need a 'goto' cleanup to scope-based cleanup, or convert
none of them." Every goto Efault now escapes two nested __cleanup()
scopes (the inner __kernel_nofault_bare and outer pagefault), relying on
the compiler to run both destructors in the correct order. The same
pattern also appears in copy_to_kernel_nofault and
strncpy_from_kernel_nofault below.

Tracing through the implementation suggests the unwind is correct in
practice: GCC and Clang run __attribute__((cleanup)) destructors when
leaving a scope via goto, in reverse order of declaration, so goto Efault
runs __end_kernel_nofault_bare() followed by pagefault_enable() in the
expected order. include/linux/uaccess.h's scoped-access documentation also
explicitly blesses leaving these scopes "with 'return', 'break' and 'goto'
at any point".

Is this an acceptable deepening of the goto-plus-guard mixing to two
levels, or should this be restructured to avoid the nested cleanup scopes?

This isn't a bug, but could this use guard(__kernel_nofault_bare)() as the
first statement of the existing pagefault scope instead of a nested
scoped_guard(), to keep the reindentation out of the diff and match the
form used in the BPF patch later in the series?

> @@ -55,7 +57,7 @@ EXPORT_SYMBOL_GPL(copy_from_kernel_nofault);
>
>  #define copy_to_kernel_nofault_loop(dst, src, len, type, err_label)	\
>  	while (len >= sizeof(type)) {					\
> -		__put_kernel_nofault(dst, src, type, err_label);	\
> +		__put_kernel_nofault_bare(dst, src, type, err_label);	\
>  		instrument_write(dst, sizeof(type));			\
>  		dst += sizeof(type);					\
>  		src += sizeof(type);					\

[ ... ]



AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

In-Reply-To-Subject: maccess: Batch TCO handling in kernel nofault loops
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/32751368606

@kernel-patches-review-bot

Copy link
Copy Markdown
commit 49414f05033b99f9b5c5d6a4d4de615aaa484781
Author: Muhammad Usama Anjum <usama.anjum@arm.com>

arm64: uaccess: Add batched kernel nofault accessors

This patch separates TCO management from fault-tolerant access and adds
begin and end hooks for batching several accesses, avoiding repeated
PSTATE.TCO manipulation in loops.

> diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
> index 9f5bd9c69c24..1a14eb2a51dc 100644
> --- a/arch/arm64/include/asm/uaccess.h
> +++ b/arch/arm64/include/asm/uaccess.h
> @@ -270,28 +270,43 @@ do {									\
>  #define get_user	__get_user
>
>  /*
> - * We must not call into the scheduler between __mte_enable_tco_async() and
> - * __mte_disable_tco_async(). As `dst` and `src` may contain blocking
> - * functions, we must evaluate these outside of the critical section.
> + * Nofault load without TCO management for use inside a
> + * __begin/__end_kernel_nofault_bare() region.
>   */
> -#define __get_kernel_nofault(dst, src, type, err_label)			\
> +#define __get_kernel_nofault_bare(dst, src, type, err_label)		\
>  do {									\
>  	__typeof__(dst) __gkn_dst = (dst);				\
>  	__typeof__(src) __gkn_src = (src);				\
>  	do { 								\
>  		__label__ __gkn_label;					\
> -									\
> -		__mte_enable_tco_async();				\
>  		__raw_get_mem("ldr", *((type *)(__gkn_dst)),		\
>  		      (__force type *)(__gkn_src), __gkn_label, K);	\
> -		__mte_disable_tco_async();				\
>  		break;							\
>  	__gkn_label:							\
> -		__mte_disable_tco_async();				\
>  		goto err_label;						\
>  	} while (0);							\
>  } while (0)
>
>  /*
> + * We must not call into the scheduler between __mte_enable_tco_async() and
> + * __mte_disable_tco_async(). As dst and src may contain blocking functions,
> + * evaluate them before overriding TCO.
> + */
> +#define __get_kernel_nofault(dst, src, type, err_label)			\
> +do {									\
> +	__label__ __gkn_tco_err;					\
> +	__typeof__(dst) __gkn_tco_dst = (dst);				\
> +	__typeof__(src) __gkn_tco_src = (src);				\
> +	__mte_enable_tco_async();					\
> +	__get_kernel_nofault_bare(__gkn_tco_dst, __gkn_tco_src, type,	\
> +				     __gkn_tco_err);			\
> +	__mte_disable_tco_async();					\
> +	break;								\
> +__gkn_tco_err:								\
> +	__mte_disable_tco_async();					\
> +	goto err_label;							\
> +} while (0)

This isn't a bug, but the get and put variants now carry the same rule
in two different wordings - would it read better to keep the two
comments identical, or to have the put side just point at the get side?

The comment above __get_kernel_nofault says "As dst and src may contain
blocking functions, evaluate them before overriding TCO," while the
comment above __put_kernel_nofault keeps the original "As `dst` and
`src` may contain blocking functions, we must evaluate these outside of
the critical section" - including the backticks that the get variant
dropped.

Also, should the wrapped argument line up under the open paren here
(four tabs plus two spaces rather than five)?

> +	__get_kernel_nofault_bare(__gkn_tco_dst, __gkn_tco_src, type,	\
> +				     __gkn_tco_err);			\

The continuation line sits at column 37, while the open parenthesis of
__get_kernel_nofault_bare( sits at column 34. The same three-column
offset appears in __put_kernel_nofault at line 412.

[ ... ]

> @@ -366,28 +381,48 @@ do {									\
>
>  #define put_user	__put_user
>
> -/*
> - * We must not call into the scheduler between __mte_enable_tco_async() and
> - * __mte_disable_tco_async(). As `dst` and `src` may contain blocking
> - * functions, we must evaluate these outside of the critical section.
> - */
> -#define __put_kernel_nofault(dst, src, type, err_label)			\
> +/* Nofault store without TCO management; see __get_kernel_nofault_bare. */
> +#define __put_kernel_nofault_bare(dst, src, type, err_label)		\
>  do {									\
>  	__typeof__(dst) __pkn_dst = (dst);				\
>  	__typeof__(src) __pkn_src = (src);				\
>  									\
>  	do {								\
>  		__label__ __pkn_err;					\
> -		__mte_enable_tco_async();				\
>  		__raw_put_mem("str", *((type *)(__pkn_src)),		\
>  			      (__force type *)(__pkn_dst), __pkn_err, K);	\
> -		__mte_disable_tco_async();				\
>  		break;							\
>  	__pkn_err:							\
> -		__mte_disable_tco_async();				\
>  		goto err_label;						\
>  	} while (0);							\
> -} while(0)
> +} while (0)
> +
> +/*
> + * We must not call into the scheduler between __mte_enable_tco_async() and
> + * __mte_disable_tco_async(). As `dst` and `src` may contain blocking
> + * functions, we must evaluate these outside of the critical section.
> + */

[ ... ]



AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

In-Reply-To-Subject: arm64: uaccess: Add batched kernel nofault accessors
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/32751368606

@kernel-patches-daemon-bpf

Copy link
Copy Markdown
Author

@kernel-patches-daemon-bpf

Copy link
Copy Markdown
Author

@kernel-patches-daemon-bpf

Copy link
Copy Markdown
Author

@kernel-patches-daemon-bpf

Copy link
Copy Markdown
Author

@kernel-patches-daemon-bpf

Copy link
Copy Markdown
Author

@kernel-patches-daemon-bpf

Copy link
Copy Markdown
Author

@kernel-patches-daemon-bpf

Copy link
Copy Markdown
Author

Upstream branch: d83fba2
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1151050
version: 1

@kernel-patches-daemon-bpf

Copy link
Copy Markdown
Author

Upstream branch: ce36e38
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1151050
version: 1

@kernel-patches-daemon-bpf

Copy link
Copy Markdown
Author

Upstream branch: 05ea1b6
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1151050
version: 1

@kernel-patches-daemon-bpf

Copy link
Copy Markdown
Author

Upstream branch: 1555de3
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1151050
version: 1

Muhammad Usama Anjum added 7 commits August 25, 2026 12:48
With Hardware Tag-Based KASAN in asynchronous or asymmetric mode, arm64
sets and clears PSTATE.TCO around every kernel nofault load or store. A
loop pays that cost for every access even though tag checking can stay
disabled until the operation finishes.

Separate TCO management from the fault-tolerant access and add begin and
end hooks for callers that want to batch several accesses. Keep the
existing accessors self-contained, and provide aliases and no-op hooks
for architectures that do not need special handling.

A context switch re-enables tag checking, so a batched region must not
schedule. Continue to evaluate accessor arguments before overriding TCO,
as those expressions may block.

Signed-off-by: Muhammad Usama Anjum <usama.anjum@arm.com>
A batched nofault region must run its end hook on every exit. Pairing the
hooks by hand makes early returns and error paths easy to get wrong.

Add a scope guard that starts the region on entry and ends it when the
scope is left. Callers can keep their natural control flow while the
architecture state remains balanced on success and failure.

No functional change until a caller uses the guard.

Signed-off-by: Muhammad Usama Anjum <usama.anjum@arm.com>
A zero-sized kernel nofault copy does not enter an access loop, but it
still disables and re-enables page faults.

Zero sizes are valid. BPF probe-read helpers accept them, and KGDB memory
packets may carry a zero length.

Return before changing page-fault state when there is nothing to copy.
For reads, keep architecture-specific address validation before the fast
path so its behavior is unchanged.

Signed-off-by: Muhammad Usama Anjum <usama.anjum@arm.com>
Kernel nofault copy and string paths open-code page-fault disable and
enable around label-based loops, duplicating cleanup on success and
failure.

Use a page-fault scope guard instead. Leaving the scope now re-enables
page faults on both paths without separate cleanup at the fault label.

No functional change.

Signed-off-by: Muhammad Usama Anjum <usama.anjum@arm.com>
With Hardware Tag-Based KASAN in asynchronous or asymmetric mode, every
arm64 kernel nofault access sets and clears PSTATE.TCO. Copy and string
loops repeat that pair even though tag checking can stay disabled for the
whole operation.

Cover each non-empty operation with one bare nofault region and use bare
accessors in the loop. Leaving the region restores TCO before page faults
are enabled again, including after an access fault. Existing empty-work
checks ensure that every new region performs at least one access.

The number of dynamic MSR TCO executions therefore changes as follows:

    Work                         Before    After
    N nofault accesses          2N        2
    4 KiB nofault copy          1,024     2
    N-byte strncpy              2N        2

The 4 KiB case assumes 512 64-bit accesses. These figures come from the
control flow rather than a runtime measurement, so the time saved depends
on the CPU and workload. Generic fallbacks leave other architectures
unchanged.

Signed-off-by: Muhammad Usama Anjum <usama.anjum@arm.com>
A zero limit is valid for several length-bounded BPF string operations.
Their loops perform no load in that case, but they still enter and leave a
page-fault-disabled region.

Return the existing empty result before changing page-fault state. Keep
address validation first so an invalid pointer continues to return
-ERANGE.

Signed-off-by: Muhammad Usama Anjum <usama.anjum@arm.com>
BPF string kfuncs cannot rely on NUL termination, so they scan memory with
kernel nofault loads. With Hardware Tag-Based KASAN in asynchronous or
asymmetric mode, every arm64 load sets and clears PSTATE.TCO.

Use bare loads and hold one nofault region across each string operation.
The scope guard restores TCO before page faults are enabled again on every
exit, including an access fault.

A character comparison performs two nofault loads. For N compared
characters, the number of dynamic MSR TCO executions therefore falls from
4N to 2.

Signed-off-by: Muhammad Usama Anjum <usama.anjum@arm.com>
@kernel-patches-daemon-bpf

Copy link
Copy Markdown
Author

At least one diff in series https://patchwork.kernel.org/project/netdevbpf/list/?series=1151050 irrelevant now. Closing PR.

@kernel-patches-daemon-bpf
kernel-patches-daemon-bpf Bot deleted the series/1151050=>bpf-next branch August 26, 2026 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants