Re: [PATCH v4 12/27] KVM: s390: Use arm64 code

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Low] Sharing C code between architectures via textual extraction using awk and architecture-specific markers is highly irregular and fragile.
- [Low] The `#undef __INCL_GEN_ARM_FILE` directive emitted by the awk script is placed inside the `#ifndef` block, rendering it unreachable and useless.
- [Low] The awk script silently succeeds and extracts the entire remainder of the file if an end marker is missing.
--

commit 984c79bce287186aa6e2e94e2238ab50e9164186
Author: Steffen Eiden <seiden@linux.ibm.com>

KVM: s390: Use arm64 code

This commit adds the infrastructure to extract KVM/arm64 code into s390 at
build time. It introduces an awk script that detects ARM64_S390_COMMON markers
and extracts marked sections into an .inc file for consumption.

> diff --git a/arch/s390/kvm/arm64/copy-arm64c.awk b/arch/s390/kvm/arm64/copy-arm64c.awk
> new file mode 100644
> index 0000000000000..45d9c7034c177
> --- /dev/null
> +++ b/arch/s390/kvm/arm64/copy-arm64c.awk
> @@ -0,0 +1,69 @@
> +#!/usr/bin/awk -f
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Extract marked sections from ARM64 C files for sharing with s390 KVM
> +#
> +# Usage: share-arm64-cfile.awk <input_file>
> +#
> +# Extracts all sections between start/end markers. If no markers found, signals failure.

[Severity: Low]
Is this code sharing approach via build-time regex extraction the intended
long-term design? Sharing C code between architectures by using awk to extract
textual blocks based on markers seems unusual and could obscure dependencies or
complicate maintenance compared to traditional code sharing via common files or
headers.

[ ... ]

> +	print "#ifndef __INCL_GEN_ARM_FILE"
> +	print "#error included .inc file w/o proper guard definition"
> +	print "#undef __INCL_GEN_ARM_FILE"
> +	print "#endif /* __INCL_GEN_ARM_FILE */"

[Severity: Low]
Is the placement of the #undef directive correct here?

If a caller correctly defines __INCL_GEN_ARM_FILE before inclusion, the
preprocessor evaluates the #ifndef to false and skips the entire block. If it
is not defined, the #error halts compilation. In either case, the #undef inside
the #ifndef block appears to be unreachable.

[ ... ]

> +copying {
> +	gsub(/#include <asm\//, "#include <arm64/")
> +	gsub(/#include <uapi\/asm\//, "#include <uapi/arm64/")
> +	print
> +	next
> +}
> +
> +END {
> +	exit !found_marker
> +}

[Severity: Low]
Does this END block handle a missing end marker properly?

If an ARM64_S390_COMMON end marker is accidentally omitted or malformed in the
source file, the awk script leaves the copying flag set to 1 and dumps the
remainder of the file. Since found_marker is true, the script will exit with
status 0 (success). Could we check if copying == 0 to catch unbalanced markers
during extraction instead of having it fail later during C compilation?

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