Re: [PATCH] KVM: x86: Enforce use of EXPORT_SYMBOL_FOR_KVM_INTERNAL

[Chao Gao <chao.gao@intel.com>]

On Thu, Nov 06, 2025 at 12:28:11PM -0800, Sean Christopherson wrote:
>Add a (gnarly) inline "script" in the Makefile to fail the build if there
>is EXPORT_SYMBOL_GPL or EXPORT_SYMBOL usage in virt/kvm or arch/x86/kvm
>beyond the known-good/expected exports for other modules.  Remembering to
>use EXPORT_SYMBOL_FOR_KVM_INTERNAL is surprisingly difficult, and hoping
>to detect "bad" exports via code review is not a robust long-term strategy.
>
>Jump through a pile of hoops to coerce make into printing a human-friendly
>error message, with the offending files+lines cleanly separated.
>
>E.g. where <srctree> is the resolution of $(srctree), i.e. '.' for in-tree
>builds, and the absolute path for out-of-tree-builds:
>
>  <srctree>/arch/x86/kvm/Makefile:97: *** ERROR ***
>  found 2 unwanted occurrences of EXPORT_SYMBOL_GPL:
>    <srctree>/arch/x86/kvm/x86.c:686:EXPORT_SYMBOL_GPL(__kvm_set_user_return_msr);
>    <srctree>/arch/x86/kvm/x86.c:703:EXPORT_SYMBOL_GPL(kvm_set_user_return_msr);
>  in directories:
>    <srctree>/arch/x86/kvm
>    <srctree>/virt/kvm
>  Use EXPORT_SYMBOL_FOR_KVM_INTERNAL, not EXPORT_SYMBOL_GPL.  Stop.
>
>and
>
>  <srctree>/arch/x86/kvm/Makefile:98: *** ERROR ***
>  found 1 unwanted occurrences of EXPORT_SYMBOL:
>    <srctree>/arch/x86/kvm/x86.c:709:EXPORT_SYMBOL(kvm_get_user_return_msr);
>  in directories:
>    <srctree>/arch/x86/kvm
>    <srctree>/virt/kvm
>  Use EXPORT_SYMBOL_FOR_KVM_INTERNAL, not EXPORT_SYMBOL.  Stop.
>
>Put the enforcement in x86's Makefile even though the rule itself applies
>to virt/kvm, as putting the enforcement in virt/kvm/Makefile.kvm would
>effectively require exempting every architecture except x86.  PPC is the
>only other architecture with sub-modules, and PPC hasn't been switched to
>use EXPORT_SYMBOL_FOR_KVM_INTERNAL (and given its nearly-orphaned state,
>likely never will).  And for KVM architectures without sub-modules, that
>means that, barring truly spurious exports, the exports are intended for
>non-KVM usage and thus shouldn't be using EXPORT_SYMBOL_FOR_KVM_INTERNAL.
>
>Cc: Chao Gao <chao.gao@intel.com>
>Signed-off-by: Sean Christopherson <seanjc@google.com>

Tested-by: Chao Gao <chao.gao@intel.com>

>---
> arch/x86/kvm/Makefile | 56 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 56 insertions(+)
>
>diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
>index c4b8950c7abe..357138ac5cc6 100644
>--- a/arch/x86/kvm/Makefile
>+++ b/arch/x86/kvm/Makefile
>@@ -47,3 +47,59 @@ $(obj)/kvm-asm-offsets.h: $(obj)/kvm-asm-offsets.s FORCE
> 
> targets += kvm-asm-offsets.s
> clean-files += kvm-asm-offsets.h
>+
>+
>+# Fail the build if there is unexpected EXPORT_SYMBOL_GPL (or EXPORT_SYMBOL)
>+# usage.  All KVM-internal exports should use EXPORT_SYMBOL_FOR_KVM_INTERNAL.
>+# Only a handful of exports intended for other modules (VFIO, KVMGT) should
>+# use EXPORT_SYMBOL_GPL, and EXPORT_SYMBOL should never be used.
>+ifdef CONFIG_KVM_X86
>+define newline
>+
>+
>+endef
>+
>+# Search recursively for whole words and print line numbers.  Filter out the
>+# allowed set of exports, i.e. those that are intended for external usage.
>+exports_grep_trailer := --include='*.[ch]' -nrw $(srctree)/virt/kvm $(srctree)/arch/x86/kvm | \
>+			grep -v -e kvm_page_track_register_notifier \
>+				-e kvm_page_track_unregister_notifier \
>+				-e kvm_write_track_add_gfn \
>+				-e kvm_write_track_remove_gfn \
>+				-e kvm_get_kvm \
>+				-e kvm_get_kvm_safe \
>+				-e kvm_put_kvm
>+
>+# Force grep to emit a goofy group separator that can in turn be replaced with
>+# the above newline macro (newlines in Make are a nightmare).  Note, grep only
>+# prints the group separator when N lines of context are requested via -C,
>+# a.k.a. --NUM.  Simply request zero lines.  Print the separator only after
>+# filtering out expected exports to avoid extra newlines in the error message.
>+define get_kvm_exports
>+$(shell grep "$(1)" -C0 $(exports_grep_trailer) | grep "$(1)" -C0 --group-separator="AAAA")
>+endef
>+
>+define check_kvm_exports
>+nr_kvm_exports := $(shell grep "$(1)" $(exports_grep_trailer) | wc -l)
>+
>+ifneq (0,$$(nr_kvm_exports))
>+$$(error ERROR ***\
>+$$(newline)found $$(nr_kvm_exports) unwanted occurrences of $(1):\
>+$$(newline)  $(subst AAAA,$$(newline) ,$(call get_kvm_exports,$(1)))\
>+$$(newline)in directories:\
>+$$(newline)  $(srctree)/arch/x86/kvm\
>+$$(newline)  $(srctree)/virt/kvm\

any reason to print directories here? the error message already has the file
name and the line number.

>+$$(newline)Use EXPORT_SYMBOL_FOR_KVM_INTERNAL, not $(1))
>+endif # nr_kvm_exports != expected

nit: "expected" in the tailing comment is not defined anywhere; maybe use 0 instead.

>+undefine exports_advice

exports_advice is not defined.

>+undefine nr_kvm_exports
>+endef # check_kvm_exports
>+
>+$(eval $(call check_kvm_exports,EXPORT_SYMBOL_GPL))
>+$(eval $(call check_kvm_exports,EXPORT_SYMBOL))
>+
>+undefine check_kvm_exports
>+undefine get_kvm_exports
>+undefine exports_grep_trailer
>+undefine newline
>+endif # CONFIG_KVM_X86
>
>base-commit: a996dd2a5e1ec54dcf7d7b93915ea3f97e14e68a
>-- 
>2.51.2.1041.gc1ab5b90ca-goog
>