Re: [PATCH v6 33/33] KVM: s390: arm64: Enable KVM_ARM64 config and Kbuild

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] Trace event name collisions completely break tracing for the kvm-arm64 subsystem.
- [Low] The `$(NM) -jU` command is incompatible with GNU nm, causing build failures.
- [Low] The Makefile passes `-O default` to `objcopy`, which is an invalid BFD target.
--

--- Patch [33]: [PATCH v6 33/33] KVM: s390: arm64: Enable KVM_ARM64 config and Kbuild ---
commit 9d8a545589b9d6b384f0fa20e8bcb8b16fc11bd1
Author: Steffen Eiden <[email protected]>

KVM: s390: arm64: Enable KVM_ARM64 config and Kbuild

Adds Kbuild/Makefile configurations to build a second KVM module on s390
implementing the arm64-KVM API. To prevent symbol conflicts with kvm-s390,
internal symbols in kvm-arm64 are mangled if compiled as built-in.

> diff --git a/arch/s390/kvm/arm64/Makefile b/arch/s390/kvm/arm64/Makefile
> new file mode 100644
> index 0000000000000..2ae4a5624151d
> --- /dev/null
> +++ b/arch/s390/kvm/arm64/Makefile
> @@ -0,0 +1,91 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +KVM := ../../../../virt/kvm
> +include $(srctree)/virt/kvm/Makefile.kvm
> +include $(srctree)/arch/s390/kvm/gmap/Makefile
> +include $(src)/Makefile.gen
> +
> +ccflags-y += -I $(src) -I $(objtree)/$(obj) -I$(srctree)/arch/s390/kvm/gmap -DKVM_S390_ARM64
> +
> +kvm-arm64-obj := \
> +	arm.o \
> +	guest.o \
> +	handle_exit.o \
> +	inject_fault.o \
> +	mmu.o \
> +	mmio.o \
> +	reset.o \
> +
> +kvm-arm64-obj += $(patsubst %.o,%-arm64.o,$(kvm-y))

[Severity: Medium]
Because kvm_main.c is compiled into both the s390 kvm.ko and kvm-arm64.ko
modules, it includes the trace headers with TRACE_SYSTEM kvm. Does this mean
both modules will generate trace event structures mapped to the exact same
system and event names? If eventfs processes these tracepoints during module
load, could the kvm-arm64 trace events be permanently shadowed by the kvm
subsystem's events?

[ ... ]
> +prereq-o-cmd = $(foreach o, $(filter %.o, $^), $(dir $(o)).$(notdir $(o)).cmd)
> +cmd_gen_symversions_o = \
> +	grep --no-filename "^\#SYMVER" $(prereq-o-cmd) >> $(dot-target).cmd || true
> +
> +define rule_ld_o_o
> +	$(call cmd_and_savecmd,ld)
> +	$(call cmd,gen_symversions_o)
> +endef
> +
> +LDFLAGS_kvm-unnamespaced.o := -r --whole-archive
> +$(obj)/kvm-unnamespaced.o: $(addprefix $(obj)/,$(kvm-arm64-obj)) FORCE
> +	$(call if_changed_rule,ld_o_o)
> +
> +quiet_cmd_nm_filter = NMFLTR  $@
> +	cmd_nm_filter = $(NM) -jU $< | sort -u > $@

[Severity: Low]
Are the -j and -U flags compatible with standard GNU nm? GNU nm does not
support these flags natively and requires --defined-only instead. Will this
cause a build failure on systems using standard GCC/binutils toolchains?

> +
> +$(obj)/kvm_symbol_list: $(obj)/kvm-unnamespaced.o FORCE
> +	$(call if_changed,nm_filter)
> +
> +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 default --localize-symbols=$(obj)/kvm_symbol_list

[Severity: Low]
Is default a valid BFD architecture/target name for objcopy? GNU objcopy
strictly requires a recognized target like elf64-s390, or for the -O flag to
be omitted entirely to preserve the input format. Could this cause a fatal
invalid bfd target error?

> +$(obj)/kvm-namespaced.o: $(obj)/kvm-unnamespaced.o $(obj)/kvm_symbol_list FORCE
> +	$(call if_changed_rule,oc_o_o)
> +
> +kvm-arm64-y = kvm-namespaced.o
> +
> +targets += kvm-unnamespaced.o kvm_symbol_list kvm-namespaced.o
> +
> +endif
> +
> +obj-$(CONFIG_KVM_ARM64) += kvm-arm64.o

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=33