Re: [PATCH v3 16/23] KVM: s390: arm64: Implement arm sysreg managing infrastructure

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] KVM: s390: arm64: preemptible smp_processor_id() in
  _vcpu_read/write_sys_reg()
--

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

KVM: s390: arm64: Implement arm sysreg managing infrastructure

This commit implements the infrastructure for the s390 host to read, write,
and trap guest arm64 system registers using EASR/SASR instructions and
shared arm64 sysreg tables.

> diff --git a/arch/s390/include/asm/kvm_host_arm64.h b/arch/s390/include/asm/kvm_host_arm64.h
> index 9afe37f6696c8..35b6d8972359c 100644
> --- a/arch/s390/include/asm/kvm_host_arm64.h
> +++ b/arch/s390/include/asm/kvm_host_arm64.h
[ ... ]
> @@ -219,4 +245,181 @@ static inline void kvm_arch_async_page_present_queued(struct kvm_vcpu *vcpu)
>  
>  #define kvm_supports_32bit_el0() false
>  
> +#define __vcpu_sys_reg(__vcpu, __reg) \
> +	vcpu_read_sys_reg(__vcpu, __reg)
> +
> +#define __vcpu_assign_sys_reg(__vcpu, __reg, __val) \
> +	vcpu_write_sys_reg(__vcpu, __val, __reg)
> +
> +#define __vcpu_rmw_sys_reg(v, r, op, val)				\
> +	do {								\
> +		u64 __v = vcpu_read_sys_reg(v, r);			\
> +		__v op (val);						\
> +		vcpu_write_sys_reg(v, __v, r);				\
> +	} while (0)
> +
> +/**
> + * _vcpu_read_sys_reg() - read a guest sysreg with easr
> + * @C: vcpu to read from; if loaded, may only be called from its own thread
> + * @R: sysreg id; must be readable by easr; must be compile time constant
> + *
> + *   if SYSREGS_ON_CPU: proceed with flags = 0
> + *   otherwise:         proceed with flags = EASR_FLAG_SA
> + *
> + */
> +#define _vcpu_read_sys_reg(C, R) \
> +	({	BUILD_BUG_ON(!__builtin_constant_p((R))); \
> +		KVM_BUG_ON(vcpu_is_loaded(C) && smp_processor_id() != (C)->cpu, (C)->kvm); \
> +		(vcpu_is_loaded(C)) \
> +			? __vcpu_read_sr((C), (R), 0) \
> +			: __vcpu_read_sr((C), (R), EASR_FLAG_SA); })
> +
> +/**
> + * _vcpu_write_sys_reg() - write a guest sysreg with sasr
> + *
> + * @C: vcpu to write to; if loaded, may only be called from its own thread
> + * @V: value to write
> + * @R: sysreg id; must be writable by sasr; must be compile time constant
> +
> + *   if SYSREGS_ON_CPU: proceed with flags = 0
> + *   otherwise:         proceed with flags = SASR_FLAG_INITIALIZED
> + */
> +#define _vcpu_write_sys_reg(C, V, R) \
> +	({	BUILD_BUG_ON(!__builtin_constant_p((R))); \
> +		KVM_BUG_ON(vcpu_is_loaded(C) && smp_processor_id() != (C)->cpu, (C)->kvm); \
> +		(vcpu_is_loaded(C)) \
> +			? __vcpu_write_sr((C), (V), (R), 0) \
> +			: __vcpu_write_sr((C), (V), (R), SASR_FLAG_INITIALIZED); })

[Severity: High]
Can _vcpu_read_sys_reg() and _vcpu_write_sys_reg() call smp_processor_id()
with preemption enabled?

vcpu_load() sets vcpu->cpu and then calls put_cpu(), and
kvm_arch_vcpu_ioctl_run() re-enables local interrupts before calling
handle_exit().  When exit handlers such as kvm_handle_sys_reg() or
kvm_handle_guest_abort() read or write save-area registers,
vcpu_is_loaded() is true in preemptible context.

On CONFIG_DEBUG_PREEMPT=y kernels, smp_processor_id() triggers a
check_preemption_disabled() BUG splat.  On any preemptible kernel, if the
vCPU thread is migrated between reading (C)->cpu and evaluating
smp_processor_id(), the preempt notifier updates (C)->cpu to the new CPU
and KVM_BUG_ON() fires falsely, killing the VM.

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