Re: [PATCH v6 23/33] s390: Use arm64 headers

[email protected]
Newsgroups dev.linux.lists.kvmarm,org.kernel.vger.kvm,org.kernel.vger.linux-s390
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] The header sharing mechanism generates sysreg-gen.h instead of sysreg.h, causing unresolved includes. esr.h is fully copied since it has no ARM64_S390_COMMON markers, meaning its `#include <asm/sysreg.h>` is retained and transformed to `#include <arm64/sysreg.h>`. However, because sysreg.h is mapped to sysreg-gen.h in ARM64_SHARED_HEADERS, the target file is generated as arm64/sysreg-gen.h. As a result, the include in esr.h will fail to find arm64/sysreg.h.
- [Low] The Make rule for generating $(ARM64_TARGETS) uses `$(call cmd,gen_arm_hdr)` without `if_changed` or `FORCE`. In Kbuild, files added to `targets` must be generated using `if_changed` so that `.cmd` files are created to track command-line modifications. The omission of `if_changed` defeats this dependency tracking.
--

--- Patch [23]: [PATCH v6 23/33] s390: Use arm64 headers ---
commit 505a7f9c35b46d1872a2e9da2ab63e0bc5a7471f
Author: Steffen Eiden <[email protected]>
s390: Use arm64 headers

This commit introduces a flexible system for sharing ARM64 headers with
s390 KVM. It adds awk scripts to extract marked sections or copy full files,
and transforms includes to ensure correct path resolution on s390.

> diff --git a/arch/s390/tools/Makefile.arm64h b/arch/s390/tools/Makefile.arm64h
> new file mode 100644
> index 0000000000000..3e033630470a1
> --- /dev/null
> +++ b/arch/s390/tools/Makefile.arm64h
> @@ -0,0 +1,57 @@

[ ... ]

> +ARM64_SHARED_HEADERS := \
> +	brk-imm.h \
> +	esr.h \
> +	kvm_arm.h \
> +	kvm_emulate-gen.h \
> +	kvm_host.h \
> +	ptrace-gen.h \
> +	sysreg-gen.h \

[Severity: Low]
Will this result in an unresolved include when esr.h is compiled?

esr.h has no ARM64_S390_COMMON markers, so it is fully copied using
copy-arm64h-full.awk. That script transforms #include <asm/sysreg.h> into
#include <arm64/sysreg.h>. However, since ARM64_SHARED_HEADERS maps
sysreg.h to sysreg-gen.h, the generated file will be arm64/sysreg-gen.h,
and the include in esr.h will fail to find arm64/sysreg.h.

[ ... ]

> +ARM64_TARGETS := $(sort $(foreach hdr,$(ARM64_SHARED_HEADERS),$(call arm64-dst,$(hdr))))
> +
> +$(ARM64_TARGETS):
> +	$(call cmd,gen_arm_hdr)

[Severity: Low]
Should this use if_changed and FORCE instead of directly calling cmd?

In Kbuild, files added to targets must be generated using if_changed so that
.cmd files are created to track command-line modifications. Omitting
if_changed here prevents automatic rebuilds if the command-line arguments
or rules change.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=23
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.