[PATCH v2 07/20] KVM: arm64: Share more arm64 code with s390
Marc Zyngier
maz at kernel.org
Tue Sep 1 01:30:40 PDT 2026
On Mon, 31 Aug 2026 15:55:22 +0100,
Steffen Eiden <seiden at linux.ibm.com> wrote:
>
> Mark functions that s390 can use to implement arm on s390 as shared
> functions.
More of what? For what purpose? What is the logic behind exposing
snippets of code to S390? This patch tags some PAuth stuff, a bunch of
SVE definitions, and a whole lot of sysreg helpers.
What is the rationale for this organisation? I can't see any, and it
just looks like a very random approach to the problem.
I want to see logic, a progression. Something I can follow.
>
> Signed-off-by: Steffen Eiden <seiden at linux.ibm.com>
> ---
> arch/arm64/kvm/arm.c | 3 +++
> arch/arm64/kvm/guest.c | 11 +++++++----
> arch/arm64/kvm/sys_regs.c | 36 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 46 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 6b92a3c1d490..62e8b32ee74e 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -340,6 +340,7 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
> kvm_arm_teardown_hypercalls(kvm);
> }
>
> +#ifdef ARM64_S390_COMMON
> static bool kvm_has_full_ptr_auth(void)
> {
> bool apa, gpa, api, gpi, apa3, gpa3;
> @@ -374,6 +375,8 @@ static bool kvm_has_full_ptr_auth(void)
> (apa + api + apa3) == 1);
> }
>
> +#endif /* ARM64_S390_COMMON */
> +
This could have been done much earlier.
> int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> {
> int r;
> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> index 6ca5a9f357cd..e978b2fbeb76 100644
> --- a/arch/arm64/kvm/guest.c
> +++ b/arch/arm64/kvm/guest.c
> @@ -307,12 +307,12 @@ static int set_core_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
> return err;
> }
>
> -#endif /* ARM64_S390_COMMON */
> -
> #define vq_word(vq) (((vq) - SVE_VQ_MIN) / 64)
> #define vq_mask(vq) ((u64)1 << ((vq) - SVE_VQ_MIN) % 64)
> #define vq_present(vqs, vq) (!!((vqs)[vq_word(vq)] & vq_mask(vq)))
>
> +#endif /* ARM64_S390_COMMON */
> +
Pointless churn.
> static int get_sve_vls(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
> {
> unsigned int max_vq, vq;
> @@ -383,6 +383,7 @@ static int set_sve_vls(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
> return 0;
> }
>
> +#ifdef ARM64_S390_COMMON
> #define SVE_REG_SLICE_SHIFT 0
> #define SVE_REG_SLICE_BITS 5
> #define SVE_REG_ID_SHIFT (SVE_REG_SLICE_SHIFT + SVE_REG_SLICE_BITS)
> @@ -399,6 +400,8 @@ static int set_sve_vls(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
> #define KVM_SVE_ZREG_SIZE KVM_REG_SIZE(KVM_REG_ARM64_SVE_ZREG(0, 0))
> #define KVM_SVE_PREG_SIZE KVM_REG_SIZE(KVM_REG_ARM64_SVE_PREG(0, 0))
>
> +#endif /* ARM64_S390_COMMON */
> +
> /*
> * Number of register slices required to cover each whole SVE register.
> * NOTE: Only the first slice every exists, for now.
> @@ -595,8 +598,6 @@ static unsigned long num_core_regs(const struct kvm_vcpu *vcpu)
> return copy_core_reg_indices(vcpu, NULL);
> }
>
> -#endif /* ARM64_S390_COMMON */
> -
> static unsigned long num_sve_regs(const struct kvm_vcpu *vcpu)
> {
> const unsigned int slices = vcpu_sve_slices(vcpu);
> @@ -658,6 +659,8 @@ static int copy_sve_reg_indices(const struct kvm_vcpu *vcpu,
> return num_regs;
> }
>
> +#endif /* ARM64_S390_COMMON */
> +
> /**
> * kvm_arm_num_regs - how many registers do we present via KVM_GET_ONE_REG
> * @vcpu: the vCPU pointer
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 53d096803d9d..b167f9d06fd4 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -45,6 +45,7 @@
> * 64bit interface.
> */
>
> +#ifdef ARM64_S390_COMMON
> static u64 sys_reg_to_index(const struct sys_reg_desc *reg);
> static int set_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> u64 val);
> @@ -82,6 +83,8 @@ static bool write_to_read_only(struct kvm_vcpu *vcpu,
> "sys_reg write to read-only register");
> }
>
> +#endif /* ARM64_S390_COMMON */
> +
> enum sr_loc_attr {
> SR_LOC_MEMORY = 0, /* Register definitely in memory */
> SR_LOC_LOADED = BIT(0), /* Register on CPU, unless it cannot */
> @@ -428,6 +431,7 @@ void vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, enum vcpu_sysreg reg)
> __vcpu_assign_sys_reg(vcpu, reg, val);
> }
>
> +#ifdef ARM64_S390_COMMON
> /* CSSELR values; used to index KVM_REG_ARM_DEMUX_ID_CCSIDR */
> #define CSSELR_MAX 14
>
> @@ -533,6 +537,8 @@ static bool access_rw(struct kvm_vcpu *vcpu,
> return true;
> }
>
> +#endif /* ARM64_S390_COMMON */
> +
> /*
> * See note at ARMv7 ARM B1.14.4 (TL;DR: S/W ops are not easily virtualized).
> */
> @@ -800,6 +806,7 @@ static bool access_gicv5_ppi_enabler(struct kvm_vcpu *vcpu,
> return true;
> }
>
> +#ifdef ARM64_S390_COMMON
> static bool trap_raz_wi(struct kvm_vcpu *vcpu,
> struct sys_reg_params *p,
> const struct sys_reg_desc *r)
> @@ -853,6 +860,8 @@ static bool trap_oslsr_el1(struct kvm_vcpu *vcpu,
> return true;
> }
>
> +#endif /* ARM64_S390_COMMON */
> +
> static int set_oslsr_el1(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> u64 val)
> {
> @@ -1011,6 +1020,7 @@ static u64 reset_actlr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
> return actlr;
> }
>
> +#ifdef ARM64_S390_COMMON
> static u64 reset_mpidr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
> {
> u64 mpidr = kvm_calculate_mpidr(vcpu);
> @@ -1019,6 +1029,8 @@ static u64 reset_mpidr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
> return mpidr;
> }
>
> +#endif /* ARM64_S390_COMMON */
> +
> static unsigned int hidden_visibility(const struct kvm_vcpu *vcpu,
> const struct sys_reg_desc *r)
> {
> @@ -1922,6 +1934,7 @@ static u8 pmuver_to_perfmon(u8 pmuver)
> }
> }
>
> +#ifdef ARM64_S390_COMMON
> static u64 sanitise_id_aa64pfr0_el1(const struct kvm_vcpu *vcpu, u64 val);
> static u64 sanitise_id_aa64pfr1_el1(const struct kvm_vcpu *vcpu, u64 val);
> static u64 sanitise_id_aa64pfr2_el1(const struct kvm_vcpu *vcpu, u64 val);
> @@ -2078,6 +2091,8 @@ static bool access_id_reg(struct kvm_vcpu *vcpu,
> return true;
> }
>
> +#endif /* ARM64_S390_COMMON */
> +
> static unsigned int aa32_id_visibility(const struct kvm_vcpu *vcpu,
> const struct sys_reg_desc *r)
> {
> @@ -2127,6 +2142,7 @@ static unsigned int fp8_visibility(const struct kvm_vcpu *vcpu,
> return REG_HIDDEN;
> }
>
> +#ifdef ARM64_S390_COMMON
> static u64 sanitise_id_aa64pfr0_el1(const struct kvm_vcpu *vcpu, u64 val)
> {
> if (!vcpu_has_sve(vcpu))
> @@ -2288,6 +2304,8 @@ static int set_id_aa64dfr0_el1(struct kvm_vcpu *vcpu,
> return set_id_reg(vcpu, rd, val);
> }
>
> +#endif /* ARM64_S390_COMMON */
> +
> static u64 read_sanitised_id_dfr0_el1(struct kvm_vcpu *vcpu,
> const struct sys_reg_desc *rd)
> {
> @@ -2332,6 +2350,7 @@ static int set_id_dfr0_el1(struct kvm_vcpu *vcpu,
> return set_id_reg(vcpu, rd, val);
> }
>
> +#ifdef ARM64_S390_COMMON
> static int set_id_aa64pfr0_el1(struct kvm_vcpu *vcpu,
> const struct sys_reg_desc *rd, u64 user_val)
> {
> @@ -2661,6 +2680,8 @@ static bool access_ccsidr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> return true;
> }
>
> +#endif /* ARM64_S390_COMMON */
> +
> static int get_raz_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> u64 *val)
> {
> @@ -2764,6 +2785,7 @@ static bool bad_redir_trap(struct kvm_vcpu *vcpu,
> SYS_REG_USER_FILTER(name, access_arch_timer, reset_val, 0, \
> arch_timer_get_user, arch_timer_set_user, vis)
>
> +#ifdef ARM64_S390_COMMON
> /*
> * Since reset() callback and field val are not used for idregs, they will be
> * used for specific purposes for idregs.
> @@ -2840,6 +2862,8 @@ static bool bad_redir_trap(struct kvm_vcpu *vcpu,
> .val = 0, \
> }
>
> +#endif /* ARM64_S390_COMMON */
> +
> static bool access_sp_el1(struct kvm_vcpu *vcpu,
> struct sys_reg_params *p,
> const struct sys_reg_desc *r)
> @@ -3212,6 +3236,7 @@ static void init_imp_id_regs(void)
> boot_cpu_aidr_val = read_sysreg(aidr_el1);
> }
>
> +#ifdef ARM64_S390_COMMON
> static u64 reset_imp_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
> {
> switch (reg_to_encoding(r)) {
> @@ -3269,6 +3294,8 @@ static int set_imp_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r,
> .val = mask, \
> }
>
> +#endif /* ARM64_S390_COMMON */
> +
> static u64 reset_mdcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
> {
> __vcpu_assign_sys_reg(vcpu, r->reg, vcpu->kvm->arch.nr_pmu_counters);
> @@ -4833,6 +4860,7 @@ static const struct sys_reg_desc cp15_64_regs[] = {
> { SYS_DESC(SYS_AARCH32_CNTVCTSS), access_arch_timer },
> };
>
> +#ifdef ARM64_S390_COMMON
> static bool check_sysreg_table(const struct sys_reg_desc *table, unsigned int n,
> bool reset_check)
> {
> @@ -4882,6 +4910,8 @@ static void perform_access(struct kvm_vcpu *vcpu,
> kvm_incr_pc(vcpu);
> }
>
> +#endif /* ARM64_S390_COMMON */
> +
> /*
> * emulate_cp -- tries to match a sys_reg access in a handling table, and
> * call the corresponding trap handler.
> @@ -5340,6 +5370,7 @@ void kvm_sys_regs_create_debugfs(struct kvm *kvm)
> &sr_resx_fops);
> }
>
> +#ifdef ARM64_S390_COMMON
> static void reset_vm_ftr_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *reg)
> {
> u32 id = reg_to_encoding(reg);
> @@ -5360,6 +5391,8 @@ static void reset_vcpu_ftr_id_reg(struct kvm_vcpu *vcpu,
> reg->reset(vcpu, reg);
> }
>
> +#endif /* ARM64_S390_COMMON */
> +
> /**
> * kvm_reset_sys_regs - sets system registers to reset value
> * @vcpu: The VCPU pointer
> @@ -5432,6 +5465,7 @@ int kvm_handle_sys_reg(struct kvm_vcpu *vcpu)
> return 1;
> }
>
> +#ifdef ARM64_S390_COMMON
> /******************************************************************************
> * Userspace API
> *****************************************************************************/
> @@ -5753,6 +5787,8 @@ int kvm_arm_copy_sys_reg_indices(struct kvm_vcpu *vcpu, u64 __user *uindices)
> return write_demux_regids(uindices);
> }
>
> +#endif /* ARM64_S390_COMMON */
> +
> #define KVM_ARM_FEATURE_ID_RANGE_INDEX(r) \
> KVM_ARM_FEATURE_ID_RANGE_IDX(sys_reg_Op0(r), \
> sys_reg_Op1(r), \
The way you pick and chose random helpers feels *VERY* wrong. I really
want to understand the rationale for not using most of it, *including*
the sysreg arrays as a whole. Everything in this file is (or at leats
aims to be) architectural, and should be directly reused.
It should only be a matter of providing the required indirection to
access the S390 representation of the arm64 state.
M.
--
Jazz isn't dead. It just smells funny.
More information about the linux-arm-kernel
mailing list