Re: [PATCH v8 28/29] KVM: s390: arm64: Integrate arm on s390

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] kconfig: HAS_IOMEM dependency unintentionally breaks native s390 KVM
- [Low] kbuild: duplicate symbol exports and tracepoint conflicts in kvm-arm64
--

commit 884410e372d8436737ad12a739eafeab45f8d49c
Author: Steffen Eiden <seiden@linux.ibm.com>

KVM: s390: arm64: Integrate arm on s390

This commit adds the Kbuild and Makefile configurations necessary to build a
second KVM module on s390 that implements the arm64-KVM API. It introduces
symbol mangling for built-in compilation to prevent conflicts with kvm-s390,
and it forces the KBUILD_MODNAME to kvm-arm64 to handle module parameters
correctly.

> diff --git a/arch/s390/kvm/Kconfig b/arch/s390/kvm/Kconfig
> index a6ff2e5aa19b7..4e060185c2fb3 100644
> --- a/arch/s390/kvm/Kconfig
> +++ b/arch/s390/kvm/Kconfig
> @@ -19,6 +19,7 @@ if VIRTUALIZATION
>  config KVM
>  	def_tristate y
>  	prompt "Kernel-based Virtual Machine (KVM) support"
> +	depends on HAS_IOMEM

[Severity: High]
Could this new dependency on HAS_IOMEM unintentionally break backwards
compatibility for native s390 KVM?

On the s390 architecture, HAS_IOMEM is defined strictly as PCI support.

If a user disables PCI, HAS_IOMEM evaluates to false, making the entire KVM
subsystem unselectable.

Native s390 KVM does not actually require PCI support to function properly.

>  	select HAVE_KVM_CPU_RELAX_INTERCEPT
>  	select KVM_ASYNC_PF
 
[ ... ]

> diff --git a/arch/s390/kvm/arm64/Makefile b/arch/s390/kvm/arm64/Makefile
> new file mode 100644
> index 0000000000000..bd78d64939d1a
> --- /dev/null
> +++ b/arch/s390/kvm/arm64/Makefile
> @@ -0,0 +1,93 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +KVM := ../../../../virt/kvm

[ ... ]

> +kvm-arm64-obj := \
> +	arm.o \
> +	guest.o \
> +	handle_exit.o \
> +	inject_fault.o \
> +	mmio.o \
> +	mmu.o \
> +	reset.o \
> +
> +kvm-arm64-obj += $(patsubst %.o,%-arm64.o,$(kvm-y))

[Severity: Low]
Will this pattern force generic kvm files, which contain exported macros and
tracepoint definitions, to be compiled a second time?

This can lead to the kernel build failing during the modpost phase due to
duplicate exported symbols.

> +kvm-arm64-obj += $(patsubst %.o,%-arm64.o,$(gmap-y))
> +
> +targets += $(kvm-arm64-obj)
> +
> +$(obj)/%-arm64.o: $(src)/%.c FORCE
> +	@mkdir -p $(dir $@)
> +	$(call if_changed_rule,cc_o_c)
> +
> +ifeq ($(CONFIG_KVM),m)
> +
> +kvm-arm64-y = $(kvm-arm64-obj)

[Severity: Low]
Is it intentional that no symbol localization is attempted here for the
module case?

This seems to guarantee a module namespace conflict. Both kvm.ko and
kvm-arm64.ko will globally export the same tracepoints.

> +
> +else ifeq ($(CONFIG_KVM),y)

[ ... ]

> +define rule_oc_o_o
> +	$(call cmd_and_savecmd,objcopy)
> +	$(call cmd,gen_objtooldep)
> +	$(call cmd,gen_symversions_o)
> +endef
> +
> +OBJCOPYFLAGS_kvm-namespaced.o := -O $(LD_BFD) --localize-symbols=$(obj)/kvm_symbol_list
> +$(obj)/kvm-namespaced.o: $(obj)/kvm-unnamespaced.o $(obj)/kvm_symbol_list FORCE
> +	$(call if_changed_rule,oc_o_o)

[Severity: Low]
Can this objcopy command create a state that modpost will reject when
checking the vmlinux binary?

While this transforms target symbols to local, it preserves the
.export_symbol structures.

Modpost intentionally terminates the build when it encounters local symbols
that have .export_symbol sections.

> +
> +kvm-arm64-y = kvm-namespaced.o
> +
> +targets += kvm-unnamespaced.o kvm_symbol_list kvm-namespaced.o
> +
> +endif

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260918133107.1042730-1-seiden@linux.ibm.com?part=28