Re: [PATCH v3 08/23] KVM: arm64: Share system register code with s390

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] KVM: arm64: piecemeal sys_regs.c sharing and mixed PAuth/SVE
  changes
--

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

KVM: arm64: Share system register code with s390

This commit wraps selected pointer authentication, SVE, and system register
helpers in arch/arm64/kvm/ with ARM64_S390_COMMON guards so they can be
compiled into s390 KVM.

> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 157bf4b812020..d83df5ff68f4a 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -366,6 +366,7 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
>  	kvm_arm_teardown_hypercalls(kvm);
>  }
>  
> +#if ARM64_S390_COMMON == 1
>  static bool kvm_has_full_ptr_auth(void)
>  {
>  	bool apa, gpa, api, gpi, apa3, gpa3;
> @@ -400,6 +401,8 @@ static bool kvm_has_full_ptr_auth(void)
>  		(apa + api + apa3) == 1);
>  }
>  
> +#endif /* ARM64_S390_COMMON == 1 */
> +
>  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 878ea4500c5c5..26ec4751e17e2 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 == 1 */
> -
>  #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 == 1 */

[ ... ]

> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index e71c1b169b491..fa1ed13daeb92 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c

[ ... ]

> @@ -3269,6 +3288,8 @@ static int set_imp_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r,
>  	.val = mask,					\
>  	}
>  
> +#endif /* ARM64_S390_COMMON == 1 */
> +
>  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);

[ ... ]

> @@ -5416,6 +5440,7 @@ int kvm_handle_sys_reg(struct kvm_vcpu *vcpu)
>  	return 1;
>  }
>  
> +#if ARM64_S390_COMMON == 1
>  static void reset_vm_ftr_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *reg)
>  {
>  	u32 id = reg_to_encoding(reg);

[Severity: Medium]
As reported by Marc Zyngier on v2 [1], why does this patch cherry-pick
isolated helper blocks from arch/arm64/kvm/sys_regs.c instead of sharing the
architectural system register code and sys_reg_descs[] table as a whole?

Leaving sys_reg_descs[], kvm_reset_sys_regs(), kvm_handle_sys_reg(), and
arm64_check_features() unshared causes commit "KVM: s390: arm64: Implement
arm sysreg managing infrastructure" to duplicate the architectural system
register table and core handling logic in arch/s390/kvm/arm64/sys_regs.c,
which risks the two implementations diverging over time. Abstracting the
underlying vCPU state accessors would allow reusing sys_regs.c and its
descriptor tables directly.

In addition, this commit bundles pointer authentication capability sharing in
arch/arm64/kvm/arm.c and SVE helper sharing in arch/arm64/kvm/guest.c into
the same patch as the system register changes, rather than splitting them to
match the corresponding s390 feature patches.

[1] https://lore.kernel.org/r/87o6ehmkzz.wl-maz@kernel.org

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106-sysreg-v3-0-75e95980fb10@linux.ibm.com?part=8