Re: [PATCH v4 07/27] arm64: Prepare sharing arm64 headers with s390

[Marc Zyngier <maz@kernel.org>]

On Mon, 06 Jul 2026 09:52:07 +0100,
Steffen Eiden <seiden@linux.ibm.com> wrote:
> 
> Prepare the sharing of arm64 headers with s390 by marking the shared
> regions or add a comment that the whole file is shared.
> The regions are marked with:
> 
>  #define ARM64_S390_COMMON
> 
>  /* insert shared definitions here */
> 
>  #endif /* ARM64_S390_COMMON */
> 
> The preprocessor symbol ARM64_S390_COMMON is always defined for arm64.
> Reduce the include scope where possible and beneficial for s390.
> No functional change.
> 
> Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
> ---
>  arch/arm64/Makefile                  |  2 ++
>  arch/arm64/include/asm/brk-imm.h     |  1 +
>  arch/arm64/include/asm/esr.h         |  5 +++--
>  arch/arm64/include/asm/kvm_arm.h     |  6 ++++--
>  arch/arm64/include/asm/kvm_emulate.h | 19 ++++++++++++++++++
>  arch/arm64/include/asm/kvm_host.h    | 28 +++++++++++++++++++++++++++
>  arch/arm64/include/asm/kvm_mmu.h     |  6 ++++++
>  arch/arm64/include/asm/ptrace.h      |  9 +++++++++
>  arch/arm64/include/asm/sysreg.h      | 29 +++++++++++++++++++++++++---
>  9 files changed, 98 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> index 6b005c8fef70..1cd7cd94a855 100644
> --- a/arch/arm64/Makefile
> +++ b/arch/arm64/Makefile
> @@ -43,7 +43,9 @@ CC_FLAGS_NO_FPU	:= -mgeneral-regs-only
>  KBUILD_CFLAGS	+= $(CC_FLAGS_NO_FPU) \
>  		   $(compat_vdso) $(cc_has_k_constraint)
>  KBUILD_CFLAGS	+= $(call cc-disable-warning, psabi)
> +KBUILD_CFLAGS	+= -DARM64_S390_COMMON
>  KBUILD_AFLAGS	+= $(compat_vdso)
> +KBUILD_AFLAGS	+= -DARM64_S390_COMMON
>  
>  ifeq ($(call rustc-min-version, 108500),y)
>  KBUILD_RUSTFLAGS += --target=aarch64-unknown-none-softfloat
> diff --git a/arch/arm64/include/asm/brk-imm.h b/arch/arm64/include/asm/brk-imm.h
> index beb42c62b6ac..dd2d153dc0d8 100644
> --- a/arch/arm64/include/asm/brk-imm.h
> +++ b/arch/arm64/include/asm/brk-imm.h
> @@ -2,6 +2,7 @@
>  /*
>   * Copyright (C) 2012 ARM Ltd.
>   */
> +/* Whole file is shared with s390 */
>  
>  #ifndef __ASM_BRK_IMM_H
>  #define __ASM_BRK_IMM_H
> diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h
> index 81c17320a588..e25b742b09ac 100644
> --- a/arch/arm64/include/asm/esr.h
> +++ b/arch/arm64/include/asm/esr.h
> @@ -3,11 +3,12 @@
>   * Copyright (C) 2013 - ARM Ltd
>   * Author: Marc Zyngier <marc.zyngier@arm.com>
>   */
> +/* Whole file is shared with s390 */
>  
>  #ifndef __ASM_ESR_H
>  #define __ASM_ESR_H
>  
> -#include <asm/memory.h>
> +#include <asm/brk-imm.h>
>  #include <asm/sysreg.h>
>  
>  #define ESR_ELx_EC_UNKNOWN	UL(0x00)
> @@ -435,7 +436,7 @@
>  #define ESR_ELx_IT_GCSPOPX		7
>  
>  #ifndef __ASSEMBLER__
> -#include <asm/types.h>
> +#include <linux/types.h>
>  
>  static inline unsigned long esr_brk_comment(unsigned long esr)
>  {
> diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
> index 3f9233b5a130..cc4175176f94 100644
> --- a/arch/arm64/include/asm/kvm_arm.h
> +++ b/arch/arm64/include/asm/kvm_arm.h
> @@ -3,14 +3,16 @@
>   * Copyright (C) 2012,2013 - ARM Ltd
>   * Author: Marc Zyngier <marc.zyngier@arm.com>
>   */
> +/* Whole file is shared with s390 */
>  
>  #ifndef __ARM64_KVM_ARM_H__
>  #define __ARM64_KVM_ARM_H__
>  
> +#include <linux/const.h>
> +#include <linux/bits.h>
> +#include <linux/types.h>
>  #include <asm/esr.h>
> -#include <asm/memory.h>
>  #include <asm/sysreg.h>
> -#include <asm/types.h>
>  
>  /*
>   * Because I'm terribly lazy and that repainting the whole of the KVM
> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> index 5bf3d7e1d92c..a1c92d2436ae 100644
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
> @@ -44,13 +44,18 @@ enum exception_type {
>  bool kvm_condition_valid32(const struct kvm_vcpu *vcpu);
>  void kvm_skip_instr32(struct kvm_vcpu *vcpu);
>  
> +#ifdef ARM64_S390_COMMON
>  void kvm_inject_undefined(struct kvm_vcpu *vcpu);
> +#endif /* ARM64_S390_COMMON */
>  void kvm_inject_sync(struct kvm_vcpu *vcpu, u64 esr);
>  int kvm_inject_serror_esr(struct kvm_vcpu *vcpu, u64 esr);
> +#ifdef ARM64_S390_COMMON
>  int kvm_inject_sea(struct kvm_vcpu *vcpu, bool iabt, u64 addr);
> +#endif /* ARM64_S390_COMMON */
>  int kvm_inject_dabt_excl_atomic(struct kvm_vcpu *vcpu, u64 addr);
>  void kvm_inject_size_fault(struct kvm_vcpu *vcpu);
>  
> +#ifdef ARM64_S390_COMMON
>  static inline int kvm_inject_sea_dabt(struct kvm_vcpu *vcpu, u64 addr)
>  {
>  	return kvm_inject_sea(vcpu, false, addr);
> @@ -61,6 +66,8 @@ static inline int kvm_inject_sea_iabt(struct kvm_vcpu *vcpu, u64 addr)
>  	return kvm_inject_sea(vcpu, true, addr);
>  }
>  
> +#endif /* ARM64_S390_COMMON */
> +
>  static inline int kvm_inject_serror(struct kvm_vcpu *vcpu)
>  {
>  	/*
> @@ -159,6 +166,7 @@ static inline void vcpu_set_thumb(struct kvm_vcpu *vcpu)
>  	*vcpu_cpsr(vcpu) |= PSR_AA32_T_BIT;
>  }
>  
> +#ifdef ARM64_S390_COMMON
>  /*
>   * vcpu_get_reg and vcpu_set_reg should always be passed a register number
>   * coming from a read of ESR_EL2. Otherwise, it may give the wrong result on
> @@ -177,6 +185,8 @@ static __always_inline void vcpu_set_reg(struct kvm_vcpu *vcpu, u8 reg_num,
>  		vcpu_gp_regs(vcpu)->regs[reg_num] = val;
>  }
>  
> +#endif /* ARM64_S390_COMMON */
> +
>  static inline bool vcpu_is_el2_ctxt(const struct kvm_cpu_context *ctxt)
>  {
>  	switch (ctxt->regs.pstate & (PSR_MODE32_BIT | PSR_MODE_MASK)) {
> @@ -360,6 +370,7 @@ static inline u64 kvm_vcpu_get_disr(const struct kvm_vcpu *vcpu)
>  	return vcpu->arch.fault.disr_el1;
>  }
>  
> +#ifdef ARM64_S390_COMMON
>  static inline u32 kvm_vcpu_hvc_get_imm(const struct kvm_vcpu *vcpu)
>  {
>  	return kvm_vcpu_get_esr(vcpu) & ESR_ELx_xVC_IMM_MASK;
> @@ -437,6 +448,8 @@ static __always_inline u8 kvm_vcpu_trap_get_fault(const struct kvm_vcpu *vcpu)
>  	return kvm_vcpu_get_esr(vcpu) & ESR_ELx_FSC;
>  }
>  
> +#endif /* ARM64_S390_COMMON */
> +
>  static inline
>  bool kvm_vcpu_trap_is_permission_fault(const struct kvm_vcpu *vcpu)
>  {
> @@ -477,6 +490,7 @@ static __always_inline int kvm_vcpu_sys_get_rt(struct kvm_vcpu *vcpu)
>  	return ESR_ELx_SYS64_ISS_RT(esr);
>  }
>  
> +#ifdef ARM64_S390_COMMON
>  static inline bool kvm_is_write_fault(struct kvm_vcpu *vcpu)
>  {
>  	if (kvm_vcpu_abt_iss1tw(vcpu)) {
> @@ -501,6 +515,8 @@ static inline bool kvm_is_write_fault(struct kvm_vcpu *vcpu)
>  	return kvm_vcpu_dabt_iswrite(vcpu);
>  }
>  
> +#endif /* ARM64_S390_COMMON */
> +
>  static inline unsigned long kvm_vcpu_get_mpidr_aff(struct kvm_vcpu *vcpu)
>  {
>  	return __vcpu_sys_reg(vcpu, MPIDR_EL1) & MPIDR_HWID_BITMASK;
> @@ -536,6 +552,7 @@ static inline bool kvm_vcpu_is_be(struct kvm_vcpu *vcpu)
>  	return vcpu_read_sys_reg(vcpu, r) & bit;
>  }
>  
> +#ifdef ARM64_S390_COMMON
>  static inline unsigned long vcpu_data_guest_to_host(struct kvm_vcpu *vcpu,
>  						    unsigned long data,
>  						    unsigned int len)
> @@ -611,6 +628,8 @@ static __always_inline void kvm_incr_pc(struct kvm_vcpu *vcpu)
>  		vcpu_set_flag((v), e);					\
>  	} while (0)
>  
> +#endif /* ARM64_S390_COMMON */
> +

It'd be worth looking at an initial patch reorganising this file to
minimise the number of such markers. This is equally valid for other
files in this patch.

[...]

> diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
> index 6eae7e7e2a68..b782cae771fe 100644
> --- a/arch/arm64/include/asm/kvm_mmu.h
> +++ b/arch/arm64/include/asm/kvm_mmu.h
> @@ -142,12 +142,15 @@ static __always_inline unsigned long __kern_hyp_va(unsigned long v)
>  
>  extern u32 __hyp_va_bits;
>  
> +#ifdef ARM64_S390_COMMON
>  /*
>   * We currently support using a VM-specified IPA size. For backward
>   * compatibility, the default IPA size is fixed to 40bits.
>   */
>  #define KVM_PHYS_SHIFT	(40)
>  
> +#endif /* ARM64_S390_COMMON */
> +
>  #define kvm_phys_shift(mmu)		VTCR_EL2_IPA((mmu)->vtcr)
>  #define kvm_phys_size(mmu)		(_AC(1, ULL) << kvm_phys_shift(mmu))
>  #define kvm_phys_mask(mmu)		(kvm_phys_size(mmu) - _AC(1, ULL))
> @@ -178,9 +181,12 @@ void stage2_unmap_vm(struct kvm *kvm);
>  int kvm_init_stage2_mmu(struct kvm *kvm, struct kvm_s2_mmu *mmu, unsigned long type);
>  void kvm_uninit_stage2_mmu(struct kvm *kvm);
>  void kvm_free_stage2_pgd(struct kvm_s2_mmu *mmu);
> +#ifdef ARM64_S390_COMMON
>  int kvm_phys_addr_ioremap(struct kvm *kvm, phys_addr_t guest_ipa,
>  			  phys_addr_t pa, unsigned long size, bool writable);

This only serves a purpose for GICv2, and I really hope your HW will
not implement this. You should probably find a way to avoid dragging
stuff that doesn't make sense on s390.

>
> +#endif /* ARM64_S390_COMMON */
> +
>  int kvm_handle_guest_sea(struct kvm_vcpu *vcpu);
>  int kvm_handle_guest_abort(struct kvm_vcpu *vcpu);
>  
> diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
> index 39582511ad72..6eb122eb5fa6 100644
> --- a/arch/arm64/include/asm/ptrace.h
> +++ b/arch/arm64/include/asm/ptrace.h
> @@ -10,8 +10,13 @@
>  
>  #include <asm/cpufeature.h>
>  
> +#ifdef ARM64_S390_COMMON
> +#include <asm/sysreg.h>
> +
>  #include <uapi/asm/ptrace.h>
>  
> +#endif /* ARM64_S390_COMMON */
> +
>  /* Current Exception Level values, as contained in CurrentEL */
>  #define CurrentEL_EL1		(1 << 2)
>  #define CurrentEL_EL2		(2 << 2)
> @@ -28,6 +33,8 @@
>  
>  #define GIC_PRIO_PSR_I_SET	GICV3_PRIO_PSR_I_SET
>  
> +#ifdef ARM64_S390_COMMON
> +
>  /* Additional SPSR bits not exposed in the UABI */
>  #define PSR_MODE_THREAD_BIT	(1 << 0)
>  #define PSR_IL_BIT		(1 << 20)
> @@ -68,6 +75,8 @@
>  #define PSR_AA32_IT_MASK	0x0600fc00	/* If-Then execution state mask */
>  #define PSR_AA32_GE_MASK	0x000f0000
>  
> +#endif /* ARM64_S390_COMMON */
> +
>  #ifdef CONFIG_CPU_BIG_ENDIAN
>  #define PSR_AA32_ENDSTATE	PSR_AA32_E_BIT
>  #else
> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> index 4bfdac9401bd..58912a6b6fcd 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
> @@ -9,13 +9,17 @@
>  #ifndef __ASM_SYSREG_H
>  #define __ASM_SYSREG_H
>  
> +#ifdef ARM64_S390_COMMON
>  #include <linux/bits.h>
> +#endif /* ARM64_S390_COMMON */
>  #include <linux/stringify.h>
>  #include <linux/kasan-tags.h>
>  #include <linux/kconfig.h>
>  
>  #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 +54,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 */
> +
>  #ifndef CONFIG_BROKEN_GAS_INST
>  
>  #ifdef __ASSEMBLER__
> @@ -79,6 +85,8 @@
>  
>  #endif	/* CONFIG_BROKEN_GAS_INST */
>  
> +#ifdef ARM64_S390_COMMON
> +
>  /*
>   * Instructions for modifying PSTATE fields.
>   * As per Arm ARM for v8-A, Section "C.5.1.3 op0 == 0b00, architectural hints,
> @@ -91,8 +99,6 @@
>   */
>  #define pstate_field(op1, op2)		((op1) << Op1_shift | (op2) << Op2_shift)
>  #define PSTATE_Imm_shift		CRm_shift
> -#define ENCODE_PSTATE(x, r)		(0xd500401f | PSTATE_ ## r | ((!!x) << PSTATE_Imm_shift))
> -#define SET_PSTATE(x, r)		__emit_inst(ENCODE_PSTATE(x, r))
>  
>  #define PSTATE_PAN			pstate_field(0, 4)
>  #define PSTATE_UAO			pstate_field(0, 3)
> @@ -100,6 +106,11 @@
>  #define PSTATE_DIT			pstate_field(3, 2)
>  #define PSTATE_TCO			pstate_field(3, 4)
>  
> +#endif /* ARM64_S390_COMMON */
> +
> +#define ENCODE_PSTATE(x, r)		(0xd500401f | PSTATE_ ## r | ((!!x) << PSTATE_Imm_shift))
> +#define SET_PSTATE(x, r)		__emit_inst(ENCODE_PSTATE(x, r))
> +
>  #define SET_PSTATE_PAN(x)		SET_PSTATE((x), PAN)
>  #define SET_PSTATE_UAO(x)		SET_PSTATE((x), UAO)
>  #define SET_PSTATE_SSBS(x)		SET_PSTATE((x), SSBS)
> @@ -123,6 +134,8 @@
>  #define GSB_SYS_BARRIER_INSN		__SYS_BARRIER_INSN(1, 0, 12, 0, 0, 31)
>  #define GSB_ACK_BARRIER_INSN		__SYS_BARRIER_INSN(1, 0, 12, 0, 1, 31)

What is the reason to avoid dragging these definitions? They seem
extremely similar to the rest.

>
> +#ifdef ARM64_S390_COMMON
> +
>  /* Data cache zero operations */
>  #define SYS_DC_ISW			sys_insn(1, 0, 7, 6, 2)
>  #define SYS_DC_IGSW			sys_insn(1, 0, 7, 6, 4)
> @@ -835,6 +848,8 @@
>  #define SCTLR_ELx_A	 (BIT(1))
>  #define SCTLR_ELx_M	 (BIT(0))
>  
> +#endif /* ARM64_S390_COMMON */
> +
>  #ifdef CONFIG_CPU_BIG_ENDIAN
>  #define ENDIAN_SET_EL2		SCTLR_ELx_EE
>  #else
> @@ -869,6 +884,8 @@
>  	 SCTLR_EL1_LSMAOE | SCTLR_EL1_nTLSMD | SCTLR_EL1_EIS   | \
>  	 SCTLR_EL1_TSCXT  | SCTLR_EL1_EOS)
>  
> +#ifdef ARM64_S390_COMMON
> +
>  /* MAIR_ELx memory attributes (used by Linux) */
>  #define MAIR_ATTR_DEVICE_nGnRnE		UL(0x00)
>  #define MAIR_ATTR_DEVICE_nGnRE		UL(0x04)
> @@ -1105,6 +1122,8 @@
>  #define GICV5_GICR_CDNMIA_TYPE_MASK	GENMASK_ULL(31, 29)
>  #define GICV5_GICR_CDNMIA_ID_MASK	GENMASK_ULL(23, 0)
>  
> +#endif /* ARM64_S390_COMMON */
> +
>  #define gicr_insn(insn)			read_sysreg_s(GICV5_OP_GICR_##insn)
>  #define gic_insn(v, insn)		write_sysreg_s(v, GICV5_OP_GIC_##insn)
>  
> @@ -1254,6 +1273,8 @@
>  	par;								\
>  })
>  
> +#ifdef ARM64_S390_COMMON
> +
>  #define SYS_FIELD_VALUE(reg, field, val)	reg##_##field##_##val
>  
>  #define SYS_FIELD_GET(reg, field, val)		\
> @@ -1266,6 +1287,8 @@
>  		 FIELD_PREP(reg##_##field##_MASK,	\
>  			    SYS_FIELD_VALUE(reg, field, val))
>  
> -#endif
> +#endif /* ARM64_S390_COMMON */
> +
> +#endif /* __ASSEMBLER__ */
>  
>  #endif	/* __ASM_SYSREG_H */

Overall, the fine-grained tagging is a bit hard to read, and there is
no obvious rationale on why some bits are needed, and not others are
not.

It definitely looks more manageable than the previous iteration, but
as I said above, it would benefit from a less scattergun approach.

Thanks,

	M.

-- 
Jazz isn't dead. It just smells funny.