Re: [PATCH v3 04/27] uapi: KVM: Provide arm64 UAPI for other host architectures

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 10 potential issue(s) to consider:
- [Critical] Including both the host and guest architecture KVM headers in the same file results in fatal compiler errors due to redefinition of standard structures.
- [Critical] `struct kvm_run` memory layout is corrupted for arm64 translation units running on s390 due to the absence of the `__KVM_S390` macro.
- [High] The newly introduced `include/uapi/arch/arm64/linux/kvm.h` and the existing generic `include/uapi/linux/kvm.h` use the identical include guard `__LINUX_KVM_H`.
- [High] Unconditional use of the `__uint128_t` compiler extension in the fallback definition of `user_fpsimd_state_arm64` breaks compilation for 32-bit targets.
- [High] Kbuild logic restricts cross-architecture header exports exclusively to the s390 architecture, breaking unified cross-platform compilation.
- [Medium] The patch exports headers into a new top-level `arch/` directory within the standard system include path.
- [Medium] Missing vital architectural constants due to the exclusion of `<asm/ptrace.h>` on non-aarch64 hosts.
- [Medium] Missing `$(srctree)/` prefix in Makefile pattern rule prerequisite can break out-of-tree builds.
- [Low] Unnecessary addition of the raw generated include directory to the global LINUXINCLUDE path.
- [Low] Mismatched header guard name in comment.
--

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

uapi: KVM: Provide arm64 UAPI for other host architectures

Enable the ARM64 userspace API to be used on non-arm64 host
architectures, with initial support for s390. This introduces
conditional aliases to achieve architecture independence and updates
the build system to export arm64 architecture headers for s390.

> diff --git a/Makefile b/Makefile
> index f056c921ea9cd..c34f823a9db05 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -588,6 +588,7 @@ LINUXINCLUDE    := \
>  		-I$(objtree)/arch/$(SRCARCH)/include/generated \
>  		-I$(srctree)/include \
>  		-I$(objtree)/include \
> +		-I$(objtree)/include/generated \

[Severity: Low]
Is this addition necessary? The kernel's USERINCLUDE variable already
includes -I$(objtree)/include/generated/uapi, which is where the arm64
generic headers are correctly generated. Adding the parent directory might
improperly expose internal generated headers to the global search path.

> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index 1c13bfa2d38aa..75cf5aff940e1 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
[ ... ]
> +#ifdef __aarch64__
>  #include <asm/ptrace.h>
> -typedef struct user_pt_regs user_pt_regs_arm64;
> -typedef struct user_fpsimd_state user_fpsimd_state_arm64;
> +#else

[Severity: Medium]
By excluding <asm/ptrace.h> on non-aarch64 hosts, are we missing vital
architectural constants (such as PSR_MODE_EL1h) needed to correctly initialize
the pstate variable in user_pt_regs_arm64? It looks like applications might be
forced to manually redefine these core arm64 architectural constants.

> +typedef struct {
> +	__u64		regs[31];
> +	__u64		sp;
> +	__u64		pc;
> +	__u64		pstate;
> +} user_pt_regs_arm64;
> +typedef struct {
> +	__uint128_t	vregs[32];

[Severity: High]
Does using the __uint128_t compiler extension here break compilation for
32-bit userspace targets? Since this type is not universally supported on
32-bit compilers (like 32-bit x86 or 32-bit s390 compat environments), would
it be safer to use a standard integer type like a __u64 array to guarantee
ABI compatibility?

> +	__u32		fpsr;
> +	__u32		fpcr;
> +	__u32		__reserved[2];
> +} __attribute__((aligned(16))) user_fpsimd_state_arm64;
> +#endif /* __aarch64__ */

[ ... ]
>  struct kvm_regs {
> -	struct user_pt_regs regs;	/* sp = sp_el0 */
> +	user_pt_regs_arm64 regs;	/* sp = sp_el0 */
>  
>  	__u64	sp_el1;
>  	__u64	elr_el1;
>  
>  	__u64	spsr[KVM_NR_SPSR];
>  
> -	struct user_fpsimd_state fp_regs;
> +	user_fpsimd_state_arm64 fp_regs;
>  };

[Severity: Critical]
If an application includes both the host's <asm/kvm.h> (e.g., s390) and the
guest's <arch/arm64/asm/kvm.h> to interact with both architectures, will this
result in compilation errors? Both files unconditionally define standard
architectural KVM structures like struct kvm_regs, struct kvm_sregs, and
struct kvm_fpu.

> diff --git a/include/uapi/Kbuild b/include/uapi/Kbuild
> index 61ee6e59c930b..452bf9d5c9c0f 100644
> --- a/include/uapi/Kbuild
> +++ b/include/uapi/Kbuild
> @@ -12,3 +12,7 @@ ifeq ($(wildcard $(objtree)/arch/$(SRCARCH)/include/generated/uapi/asm/kvm_para.
>  no-export-headers += linux/kvm_para.h
>  endif
>  endif
> +
> +ifneq ($(ARCH),s390)
> +no-export-headers += arch/%
> +endif

[Severity: High]
Does this logic restrict cross-architecture header exports exclusively to the
s390 architecture? It appears this will prevent include/uapi/arch/arm64/linux/kvm.h
from being exported during make headers_install on native arm64 systems,
meaning userspace applications cannot unconditionally include
<arch/arm64/linux/kvm.h> when targeting arm64 VMs.

> diff --git a/include/uapi/arch/arm64/linux/kvm.h b/include/uapi/arch/arm64/linux/kvm.h
> new file mode 100644
> index 0000000000000..21528a146b4d9
> --- /dev/null
> +++ b/include/uapi/arch/arm64/linux/kvm.h
> @@ -0,0 +1,8 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +#ifndef __LINUX_KVM_H
> +#define __LINUX_KVM_H

[Severity: High]
Is it safe to use __LINUX_KVM_H as the include guard here? The existing
generic include/uapi/linux/kvm.h uses the exact same guard. If the generic or
host KVM header is included first, the ARM64-specific definitions from this
file will be skipped, preventing userspace applications from simultaneously
accessing host and guest KVM ioctls and types.

[Severity: Medium]
Does exporting headers into a new top-level arch/ directory violate Linux
UAPI conventions? By placing headers in include/uapi/arch/arm64/,
make headers_install will create an arch/ directory in the global system
include directory (e.g., /usr/include/arch/). Standard convention dictates
that exported headers belong in linux/, asm/, or asm-generic/.

> +
> +#include <arch/arm64/asm/kvm.h>
> +#include <linux/kvm-generic.h>
> +
> +#endif /*__LINUX_KVM_H */

> diff --git a/include/uapi/linux/kvm-generic.h b/include/uapi/linux/kvm-generic.h
> new file mode 100644
> index 0000000000000..a9c757029808b
> --- /dev/null
> +++ b/include/uapi/linux/kvm-generic.h
[ ... ]
> +struct kvm_run {
> +	/* in */
> +	__u8 request_interrupt_window;
[ ... ]
> +	__u64 cr8;
> +	__u64 apic_base;
> +
> +#ifdef __KVM_S390
> +	/* the processor status word for s390 */
> +	__u64 psw_mask; /* psw upper half */
> +	__u64 psw_addr; /* psw lower half */
> +#endif

[Severity: Critical]
Could this cause memory layout corruption for arm64 translation units running
on s390? If a developer uses a separate translation unit for arm64 logic and
includes only <arch/arm64/linux/kvm.h>, the host's <asm/kvm.h> is not included
and __KVM_S390 remains undefined. 

This removes 16 bytes from the struct, shifting the offsets of all subsequent
union fields and causing a silent ABI mismatch with the host kernel.

[ ... ]
> +struct kvm_pre_fault_memory {
> +	__u64 gpa;
> +	__u64 size;
> +	__u64 flags;
> +	__u64 padding[5];
> +};
> +
> +#endif /* __LINUX_KVM_GENERIC_H */

[Severity: Low]
This isn't a bug, but there is a typo here. The #endif comment refers to
__LINUX_KVM_GENERIC_H, while the actual header guard defined at the top of
the file is __LINUX_GENERIC_KVM_H.

> diff --git a/scripts/Makefile.asm-headers b/scripts/Makefile.asm-headers
> index 8a4856e741803..86d4d7173fcdf 100644
> --- a/scripts/Makefile.asm-headers
> +++ b/scripts/Makefile.asm-headers
[ ... ]
> +# Let architectures define architecture specific headers that are shared with
> +# other architectures to enable single-system cross architecture communication
> +$(shared-uapi-y): $(obj)/%.h: $(shared-uapi-src)/%.h FORCE
> +	$(call if_changed,share_hdr)

[Severity: Medium]
Can omitting the $(srctree)/ prefix in this pattern rule prerequisite break
out-of-tree builds? Unlike other rules that properly use $(srctree)/, relying
entirely on VPATH resolution violates Kbuild best practices and can lead to
missing prerequisites during out-of-tree (O=...) builds.

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