Re: [PATCH v2 14/20] KVM: s390: arm64: Integrate sysreg into the host

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Preemption and interrupts are erroneously re-enabled prior to guest entry and host sysreg writes, causing unstable `smp_processor_id()` usage and race conditions.

Pre-existing issues:
- [High] Missed KVM vCPU request (lost IPI) due to checking requests with interrupts enabled before entering guest mode.
--

commit 1f3a8a06b031d71852642f3d9a86276d457c62ad
Author: Steffen Eiden <seiden@linux.ibm.com>
Subject: KVM: s390: arm64: Integrate sysreg into the host

This commit integrates the system register managing infrastructure into the
host. It initializes sysregs during vcpu init, finalizes them on PID change,
and lets userspace read and modify guest system registers.

> diff --git a/arch/s390/kvm/arm64/arm.c b/arch/s390/kvm/arm64/arm.c
> index c573a3da5766e..3a0bba6e0a062 100644
> --- a/arch/s390/kvm/arm64/arm.c
> +++ b/arch/s390/kvm/arm64/arm.c
[ ... ]
> @@ -390,10 +399,20 @@ static void arm_vcpu_run(struct kvm_vcpu *vcpu)
>  	guest_state_enter_irqoff();
>  	local_irq_enable();
>  
> +	vcpu_write_host_sys_reg(vcpu, vcpu->arch.hcr_elz, SYS_HCR_EL2);
> +	vcpu_write_host_sys_reg(vcpu, vcpu->arch.hcrx_elz, SYS_HCRX_EL2);
> +	vcpu_write_host_sys_reg(vcpu, vcpu->arch.mpidr, SYS_VMPIDR_EL2);
> +
> +	_vcpu_write_sys_reg(vcpu, vcpu->arch.ctxt.elr_el1, SYS_ELR_EL1);
> +	_vcpu_write_sys_reg(vcpu, vcpu->arch.ctxt.spsr_el1, SYS_SPSR_EL1);

[Severity: High]
Does calling _vcpu_write_sys_reg() here cause problems with preemption being
enabled?

Because local_irq_enable() is called immediately before these sysreg writes,
the thread is fully preemptible. The _vcpu_write_sys_reg() macro invokes
smp_processor_id(), which triggers a CONFIG_DEBUG_PREEMPT BUG when used in
preemptible code.

Additionally, if the thread is preempted and migrated to another CPU during
this window, could this race condition cause the sysregs to be written to the
wrong physical CPU?

>  	sae_block->icptr = 0;
>  
>  	sae64a(sae_block);
>  
> +	vcpu->arch.ctxt.elr_el1 = _vcpu_read_sys_reg(vcpu, SYS_ELR_EL1);
> +	vcpu->arch.ctxt.spsr_el1 = _vcpu_read_sys_reg(vcpu, SYS_SPSR_EL1);
> +
>  	local_irq_disable();
>  	guest_state_exit_irqoff();
>  	guest_timing_exit_irqoff();
>  	local_irq_enable();

[Severity: High]
Does this sequence leave the actual guest execution window completely
preemptible?

It appears the logic disables IRQs only to satisfy the
guest_state_exit_irqoff() function, which mirrors the potentially inverted
logic for the entry path above.


While reviewing this patch, I also noticed a pre-existing issue in the vCPU
run loop:

arch/s390/kvm/arm64/arm.c:kvm_arch_vcpu_ioctl_run() {
	...
		smp_store_mb(vcpu->mode, IN_GUEST_MODE);

		if (kvm_request_pending(vcpu)) {
			vcpu->mode = OUTSIDE_GUEST_MODE;
			continue;
		}
	...
}

[Severity: High]
This is a pre-existing issue, but does checking kvm_request_pending() here
while host interrupts are fully enabled leave a race window for lost IPIs?

The vCPU run loop sets vcpu->mode = IN_GUEST_MODE and checks
kvm_request_pending() while host interrupts are fully enabled. Consequently, if
an IPI arrives immediately after the check, the host will consume it right
away. The vCPU thread could then proceed to enter the guest without noticing
the request, causing the request to be missed until a subsequent unrelated
exit.

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