Re: [PATCH v8 17/29] KVM: arm64: Share arm64 code with s390

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] kvm: arm64: severe cross-architecture coupling using preprocessor markers
- [Low] kvm: arm64: incomplete abstraction of ELR_EL1 and SPSR registers
--

commit 336c61211f9a8d50d065b41830fda851e8868db0
Author: Steffen Eiden <seiden@linux.ibm.com>
Subject: KVM: arm64: Share arm64 code with s390

This commit marks functions in the arm64 KVM implementation that s390 can 
use as shared functions. It does this by wrapping them in ARM64_S390_COMMON 
preprocessor guards.

> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 8b080804bc90b..6b92a3c1d490d 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -1603,6 +1603,7 @@ static unsigned long system_supported_vcpu_features(void)
>  	return features;
>  }
>  
> +#ifdef ARM64_S390_COMMON
>  static int kvm_vcpu_init_check_features(struct kvm_vcpu *vcpu,

[Severity: Medium]
Does this approach create a severe cross-architecture coupling?

Using a preprocessor macro like this as a purely textual marker for an awk 
script (copy-arm64c.awk) on s390 to blindly extract arm64 C code leaves
callers unprotected. To prevent the arm64 build from failing, the macro must
be globally appended to KBUILD_CFLAGS in arch/arm64/Makefile, because
callers of the wrapped functions (e.g., kvm_arm_num_regs() calling
num_core_regs()) are left outside the macro block.

Should cross-architecture sharing be properly implemented via common kernel
directories (such as virt/kvm), rather than by parsing macros?

[ ... ]
> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> index 773f6c8e50265..6ca5a9f357cdb 100644
> --- a/arch/arm64/kvm/guest.c
> +++ b/arch/arm64/kvm/guest.c
> @@ -62,6 +62,7 @@ const struct kvm_stats_header kvm_vcpu_stats_header = {
>  		       sizeof(kvm_vcpu_stats_desc),
>  };
>  
> +#ifdef ARM64_S390_COMMON
>  static bool core_reg_offset_is_vreg(u64 off)

[Severity: Low]
Does this incomplete abstraction force downstream workarounds in s390?

By wrapping this section to share it with s390, the abstraction of register
accesses remains incomplete. For instance, core_reg_addr() accesses ELR_EL1
and all SPSR variants by directly referencing the ARM64-specific
vcpu->arch.ctxt structure:

case KVM_REG_ARM_CORE_REG(elr_el1):
        return __ctxt_sys_reg(&vcpu->arch.ctxt, ELR_EL1);
case KVM_REG_ARM_CORE_REG(spsr[KVM_SPSR_ABT]):
        return &vcpu->arch.ctxt.spsr_abt;

Does this force the s390 architecture to artificially replicate the ARM64
kvm_cpu_context layout with dummy structures (like spsr_abt) to allow the
extracted code to compile, defeating the purpose of the register abstraction
layer?

[ ... ]
> @@ -543,6 +546,7 @@ int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
>  	return -EINVAL;
>  }
>  
> +#ifdef ARM64_S390_COMMON
>  static int copy_core_reg_indices(const struct kvm_vcpu *vcpu,
>  				 u64 __user *uindices)

[Severity: Medium]
This is another instance of wrapping arm64 code with s390-specific macros
for code extraction. Are callers of this function properly protected?

For example, callers like kvm_arm_num_regs() that call num_core_regs()
remain outside the macro block, leading to the same architectural coupling
issues mentioned above.

[ ... ]
> diff --git a/arch/arm64/kvm/mmio.c b/arch/arm64/kvm/mmio.c
> index d1c3a352d5a22..5161102da5077 100644
> --- a/arch/arm64/kvm/mmio.c
> +++ b/arch/arm64/kvm/mmio.c
> @@ -10,6 +10,7 @@
>  
>  #include "trace.h"
>  
> +#ifdef ARM64_S390_COMMON
>  void kvm_mmio_write_buf(void *buf, unsigned int len, unsigned long data)

[Severity: Medium]
Here the macro is used to wrap almost the entire file's contents, showing
the extreme extent of this cross-architecture code sharing mechanism. 

Does this indicate that the shared logic should be abstracted into a common
file rather than conditionally compiled out of arm64 specific files?

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