Re: [PATCH v7 10/23] arm64: Share arm64 headers with s390

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Using #ifdef ARM64_S390_COMMON as a marker for shared code hides critical macros from userspace tools, breaking their compilation.
--

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

arm64: Share arm64 headers with s390

This commit modifies arm64 headers to allow sharing definitions with s390 by
enclosing shared regions within an ARM64_S390_COMMON ifdef block. The
preprocessor symbol is defined for arm64 via the Makefile.

> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> index 6b005c8fef706..12cbad460258f 100644
> --- a/arch/arm64/Makefile
> +++ b/arch/arm64/Makefile
> @@ -45,6 +45,11 @@ KBUILD_CFLAGS	+= $(CC_FLAGS_NO_FPU) \
>  KBUILD_CFLAGS	+= $(call cc-disable-warning, psabi)
>  KBUILD_AFLAGS	+= $(compat_vdso)
>  
> +# Enable all code shared to s390
> +KBUILD_CFLAGS	+= -DARM64_S390_COMMON

[Severity: Medium]
Will this break standalone compilation environments? The macro is only
injected into the core kernel build flags (KBUILD_CFLAGS) in
arch/arm64/Makefile. Standalone tools like KVM selftests or perf, as well
as out-of-tree tracing scripts that do not inherit these core flags, will
not have ARM64_S390_COMMON defined.

[ ... ]

> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 5261c21a437cc..a8ddce3921b87 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -39,6 +39,7 @@
>  
>  #define KVM_MAX_VCPUS VGIC_V3_MAX_CPUS
>  
> +#ifdef ARM64_S390_COMMON
>  #define KVM_VCPU_MAX_FEATURES 10

[Severity: Medium]
Does this code break the struct kvm_arch definition for environments where
ARM64_S390_COMMON is not defined? The KVM_VCPU_MAX_FEATURES macro in
arch/arm64/include/asm/kvm_host.h is hidden inside the ifdef block, but it
is used outside the block to define the size in
DECLARE_BITMAP(vcpu_features, KVM_VCPU_MAX_FEATURES).

[ ... ]

> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> index ab205f9db94ad..1c5c4df260bea 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
> @@ -16,6 +16,8 @@
>  
>  #include <asm/gpr-num.h>
>  
> +#ifdef ARM64_S390_COMMON
> +
>  /*
>   * ARMv8 ARM reserves the following encoding for system registers:
>   * (Ref: ARMv8 ARM, Section: "System instruction class encoding overview",
> @@ -50,6 +52,8 @@
>  #define sys_reg_CRm(id)	(((id) >> CRm_shift) & CRm_mask)
>  #define sys_reg_Op2(id)	(((id) >> Op2_shift) & Op2_mask)
>  
> +#endif /* ARM64_S390_COMMON */

[Severity: Medium]
Does hiding the sys_reg macro inside the ifdef block break the compilation
of all SYS_* registers for tools that sync arch/arm64/include/asm/sysreg.h?
If the tool does not define ARM64_S390_COMMON, dependent macros like
SYS_SCTLR_EL1 will become undefined.

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