All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nathan Chancellor <nathan@kernel.org>
To: Vincent Donnefort <vdonnefort@google.com>
Cc: rostedt@goodmis.org, mhiramat@kernel.org,
	mathieu.desnoyers@efficios.com,
	linux-trace-kernel@vger.kernel.org, maz@kernel.org,
	oliver.upton@linux.dev, joey.gouly@arm.com,
	suzuki.poulose@arm.com, yuzenghui@huawei.com,
	kvmarm@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
	jstultz@google.com, qperret@google.com, will@kernel.org,
	aneesh.kumar@kernel.org, kernel-team@android.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v14 18/30] tracing: Check for undefined symbols in simple_ring_buffer
Date: Wed, 11 Mar 2026 15:18:16 -0700	[thread overview]
Message-ID: <20260311221816.GA316631@ax162> (raw)
In-Reply-To: <20260309162516.2623589-19-vdonnefort@google.com>

Hi Vincent,

On Mon, Mar 09, 2026 at 04:25:04PM +0000, Vincent Donnefort wrote:
> The simple_ring_buffer implementation must remain simple enough to be
> used by the pKVM hypervisor. Prevent the object build if unresolved
> symbols are found.
> 
> Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
> 
> diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
> index d106beca8d7f..3182e1bc1cf7 100644
> --- a/kernel/trace/Makefile
> +++ b/kernel/trace/Makefile
> @@ -132,4 +132,20 @@ obj-$(CONFIG_TRACE_REMOTE) += trace_remote.o
>  obj-$(CONFIG_SIMPLE_RING_BUFFER) += simple_ring_buffer.o
>  obj-$(CONFIG_TRACE_REMOTE_TEST) += remote_test.o
>  
> +#
> +# simple_ring_buffer is used by the pKVM hypervisor which does not have access
> +# to all kernel symbols. Fail the build if forbidden symbols are found.
> +#
> +UNDEFINED_ALLOWLIST := memset alt_cb_patch_nops __x86 __ubsan __asan __kasan __gcov __aeabi_unwind
> +UNDEFINED_ALLOWLIST += __stack_chk_fail stackleak_track_stack __ref_stack __sanitizer
> +UNDEFINED_ALLOWLIST := $(addprefix -e , $(UNDEFINED_ALLOWLIST))
> +
> +quiet_cmd_check_undefined = NM      $<
> +      cmd_check_undefined = test -z "`$(NM) -u $< | grep -v $(UNDEFINED_ALLOWLIST)`"

This check triggers when building allmodconfig targeting arm, arm64,
powerpc, and x86_64 (at least, I did not test more at the moment) with
clang. If this is a hard failure, this really needs to print something
out to the developer/user to help them debug off the bat, versus having
to manually dig the $(NM) command out from the .cmd file or V=1. I came
up with

diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
index 3182e1bc1cf7..c725b06876bc 100644
--- a/kernel/trace/Makefile
+++ b/kernel/trace/Makefile
@@ -141,7 +141,13 @@ UNDEFINED_ALLOWLIST += __stack_chk_fail stackleak_track_stack __ref_stack __sani
 UNDEFINED_ALLOWLIST := $(addprefix -e , $(UNDEFINED_ALLOWLIST))
 
 quiet_cmd_check_undefined = NM      $<
-      cmd_check_undefined = test -z "`$(NM) -u $< | grep -v $(UNDEFINED_ALLOWLIST)`"
+      cmd_check_undefined = \
+          undefsyms=$$($(NM) -u $< | grep -v $(UNDEFINED_ALLOWLIST) || true); \
+          if [ -n "$$undefsyms" ]; then \
+              echo "Unexpected symbols in $<:" >&2; \
+              echo "$$undefsyms" >&2; \
+              false; \
+          fi
 
 $(obj)/%.o.checked: $(obj)/%.o FORCE
 	$(call if_changed,check_undefined)
--

which prints

  Unexpected symbols in kernel/trace/simple_ring_buffer.o:
                   U llvm_gcda_emit_arcs
                   U llvm_gcda_emit_function
                   U llvm_gcda_end_file
                   U llvm_gcda_start_file
                   U llvm_gcda_summary_info
                   U llvm_gcov_init

for arm64, which makes sense since these are LLVM specific GCOV symbols,
so they should probably get the same treatment as the other ones:

diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
index c725b06876bc..d464e3aa5bdd 100644
--- a/kernel/trace/Makefile
+++ b/kernel/trace/Makefile
@@ -136,8 +136,8 @@ obj-$(CONFIG_TRACE_REMOTE_TEST) += remote_test.o
 # simple_ring_buffer is used by the pKVM hypervisor which does not have access
 # to all kernel symbols. Fail the build if forbidden symbols are found.
 #
-UNDEFINED_ALLOWLIST := memset alt_cb_patch_nops __x86 __ubsan __asan __kasan __gcov __aeabi_unwind
-UNDEFINED_ALLOWLIST += __stack_chk_fail stackleak_track_stack __ref_stack __sanitizer
+UNDEFINED_ALLOWLIST := memset alt_cb_patch_nops __x86 __ubsan __asan __kasan __gcov llvm_gcda llvm_gcov
+UNDEFINED_ALLOWLIST += __aeabi_unwind __stack_chk_fail stackleak_track_stack __ref_stack __sanitizer
 UNDEFINED_ALLOWLIST := $(addprefix -e , $(UNDEFINED_ALLOWLIST))
 
 quiet_cmd_check_undefined = NM      $<
--

For x86_64, I see

  Unexpected symbols in kernel/trace/simple_ring_buffer.o:
                   U __clear_pages_unrolled
                   U __memmove
                   U copy_page

which comes from the use of KCFI_ADDRESSABLE(), since allmodconfig has
CONFIG_CFI=y.

For powerpc (with both clang and GCC), I see

  Unexpected symbols in kernel/trace/simple_ring_buffer.o:
                   U .TOC.

For arm (with both clang and GCC), I see

  Unexpected symbols in kernel/trace/simple_ring_buffer.o:
           U __stack_chk_guard
           U warn_slowpath_fmt

Presumably adding all of those should be fine as well?

diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
index d464e3aa5bdd..4f120cb8c79c 100644
--- a/kernel/trace/Makefile
+++ b/kernel/trace/Makefile
@@ -137,7 +137,8 @@ obj-$(CONFIG_TRACE_REMOTE_TEST) += remote_test.o
 # to all kernel symbols. Fail the build if forbidden symbols are found.
 #
 UNDEFINED_ALLOWLIST := memset alt_cb_patch_nops __x86 __ubsan __asan __kasan __gcov llvm_gcda llvm_gcov
-UNDEFINED_ALLOWLIST += __aeabi_unwind __stack_chk_fail stackleak_track_stack __ref_stack __sanitizer
+UNDEFINED_ALLOWLIST += __aeabi_unwind __stack_chk_fail __stack_chk_guard stackleak_track_stack __ref_stack __sanitizer
+UNDEFINED_ALLOWLIST += \.TOC\. __clear_pages_unrolled __memmove copy_page warn_slowpath_fmt
 UNDEFINED_ALLOWLIST := $(addprefix -e , $(UNDEFINED_ALLOWLIST))
 
 quiet_cmd_check_undefined = NM      $<
--

I don't mind sending a series for these, I just wanted to make sure I
was reasoning about everything correctly.

Cheers,
Nathan

  reply	other threads:[~2026-03-11 22:18 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-09 16:24 [PATCH v14 00/30] Tracefs support for pKVM Vincent Donnefort
2026-03-09 16:24 ` [PATCH v14 01/30] ring-buffer: Add page statistics to the meta-page Vincent Donnefort
2026-03-09 16:24 ` [PATCH v14 02/30] ring-buffer: Store bpage pointers into subbuf_ids Vincent Donnefort
2026-03-09 16:24 ` [PATCH v14 03/30] ring-buffer: Introduce ring-buffer remotes Vincent Donnefort
2026-03-09 16:24 ` [PATCH v14 04/30] ring-buffer: Add non-consuming read for " Vincent Donnefort
2026-03-09 16:24 ` [PATCH v14 05/30] tracing: Introduce trace remotes Vincent Donnefort
2026-03-09 16:24 ` [PATCH v14 06/30] tracing: Add reset to " Vincent Donnefort
2026-03-09 16:24 ` [PATCH v14 07/30] tracing: Add non-consuming read " Vincent Donnefort
2026-03-09 16:24 ` [PATCH v14 08/30] tracing: Add init callback " Vincent Donnefort
2026-03-09 16:24 ` [PATCH v14 09/30] tracing: Add events " Vincent Donnefort
2026-03-09 16:24 ` [PATCH v14 10/30] tracing: Add events/ root files " Vincent Donnefort
2026-03-09 16:24 ` [PATCH v14 11/30] tracing: Add helpers to create trace remote events Vincent Donnefort
2026-03-09 16:24 ` [PATCH v14 12/30] ring-buffer: Export buffer_data_page and macros Vincent Donnefort
2026-03-09 16:24 ` [PATCH v14 13/30] tracing: Introduce simple_ring_buffer Vincent Donnefort
2026-03-09 16:25 ` [PATCH v14 14/30] tracing: Add a trace remote module for testing Vincent Donnefort
2026-03-09 16:25 ` [PATCH v14 15/30] tracing: selftests: Add trace remote tests Vincent Donnefort
2026-03-09 16:25 ` [PATCH v14 16/30] Documentation: tracing: Add tracing remotes Vincent Donnefort
2026-03-09 16:25 ` [PATCH v14 17/30] tracing: load/unload page callbacks for simple_ring_buffer Vincent Donnefort
2026-03-09 16:25 ` [PATCH v14 18/30] tracing: Check for undefined symbols in simple_ring_buffer Vincent Donnefort
2026-03-11 22:18   ` Nathan Chancellor [this message]
2026-03-12  8:55     ` Vincent Donnefort
2026-03-12 14:07       ` Vincent Donnefort
2026-03-12 20:40         ` Nathan Chancellor
2026-03-09 16:25 ` [PATCH v14 19/30] KVM: arm64: Add PKVM_DISABLE_STAGE2_ON_PANIC Vincent Donnefort
2026-03-09 16:25 ` [PATCH v14 20/30] KVM: arm64: Add clock support to nVHE/pKVM hyp Vincent Donnefort
2026-03-09 16:25 ` [PATCH v14 21/30] KVM: arm64: Initialise hyp_nr_cpus for nVHE hyp Vincent Donnefort
2026-03-09 16:25 ` [PATCH v14 22/30] KVM: arm64: Support unaligned fixmap in the pKVM hyp Vincent Donnefort
2026-03-09 16:25 ` [PATCH v14 23/30] KVM: arm64: Add tracing capability for the nVHE/pKVM hyp Vincent Donnefort
2026-03-09 16:25 ` [PATCH v14 24/30] KVM: arm64: Add trace remote " Vincent Donnefort
2026-03-09 16:25 ` [PATCH v14 25/30] KVM: arm64: Sync boot clock with " Vincent Donnefort
2026-03-09 16:25 ` [PATCH v14 26/30] KVM: arm64: Add trace reset to " Vincent Donnefort
2026-03-09 16:25 ` [PATCH v14 27/30] KVM: arm64: Add event support to the nVHE/pKVM hyp and trace remote Vincent Donnefort
2026-03-09 16:25 ` [PATCH v14 28/30] KVM: arm64: Add hyp_enter/hyp_exit events to nVHE/pKVM hyp Vincent Donnefort
2026-03-09 16:25 ` [PATCH v14 29/30] KVM: arm64: Add selftest event support " Vincent Donnefort
2026-03-09 16:25 ` [PATCH v14 30/30] tracing: selftests: Add hypervisor trace remote tests Vincent Donnefort
2026-03-10 14:43 ` [PATCH v14 00/30] Tracefs support for pKVM Steven Rostedt
2026-03-11  9:02   ` Marc Zyngier
2026-03-11  8:58 ` (subset) " Marc Zyngier

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260311221816.GA316631@ax162 \
    --to=nathan@kernel.org \
    --cc=aneesh.kumar@kernel.org \
    --cc=joey.gouly@arm.com \
    --cc=jstultz@google.com \
    --cc=kernel-team@android.com \
    --cc=kvmarm@lists.linux.dev \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=maz@kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=oliver.upton@linux.dev \
    --cc=qperret@google.com \
    --cc=rostedt@goodmis.org \
    --cc=suzuki.poulose@arm.com \
    --cc=vdonnefort@google.com \
    --cc=will@kernel.org \
    --cc=yuzenghui@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.